[v4,7/7] riscv: dts: starfive: jh7110: Add PLL clock node and modify syscrg node

Message ID 20230512022036.97987-8-xingyu.wu@starfivetech.com
State New
Headers
Series Add PLL clocks driver and syscon for StarFive JH7110 SoC |

Commit Message

Xingyu Wu May 12, 2023, 2:20 a.m. UTC
  Add the PLL clock node for the Starfive JH7110 SoC and
modify the SYSCRG node to add PLL clocks input.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
  

Comments

Krzysztof Kozlowski May 12, 2023, 6:37 a.m. UTC | #1
On 12/05/2023 04:20, Xingyu Wu wrote:
> Add the PLL clock node for the Starfive JH7110 SoC and
> modify the SYSCRG node to add PLL clocks input.


> @@ -465,6 +469,12 @@ syscrg: clock-controller@13020000 {
>  		sys_syscon: syscon@13030000 {
>  			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
>  			reg = <0x0 0x13030000 0x0 0x1000>;
> +
> +			pllclk: clock-controller {
> +				compatible = "starfive,jh7110-pll";
> +				clocks = <&osc>;
> +				#clock-cells = <1>;

This should be part of previous patch. You just added that node. Don't
add half of devices but entire device.

Best regards,
Krzysztof
  
Xingyu Wu May 12, 2023, 7:15 a.m. UTC | #2
On 2023/5/12 14:37, Krzysztof Kozlowski wrote:
> On 12/05/2023 04:20, Xingyu Wu wrote:
>> Add the PLL clock node for the Starfive JH7110 SoC and
>> modify the SYSCRG node to add PLL clocks input.
> 
> 
>> @@ -465,6 +469,12 @@ syscrg: clock-controller@13020000 {
>>  		sys_syscon: syscon@13030000 {
>>  			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
>>  			reg = <0x0 0x13030000 0x0 0x1000>;
>> +
>> +			pllclk: clock-controller {
>> +				compatible = "starfive,jh7110-pll";
>> +				clocks = <&osc>;
>> +				#clock-cells = <1>;
> 
> This should be part of previous patch. You just added that node. Don't
> add half of devices but entire device.
> 

So do I merge the patch 6 and patch 7 into one patch and add syscon and
clock-controller together?

Best regards,
Xingyu Wu
  
Krzysztof Kozlowski May 12, 2023, 7:22 a.m. UTC | #3
On 12/05/2023 09:15, Xingyu Wu wrote:
> On 2023/5/12 14:37, Krzysztof Kozlowski wrote:
>> On 12/05/2023 04:20, Xingyu Wu wrote:
>>> Add the PLL clock node for the Starfive JH7110 SoC and
>>> modify the SYSCRG node to add PLL clocks input.
>>
>>
>>> @@ -465,6 +469,12 @@ syscrg: clock-controller@13020000 {
>>>  		sys_syscon: syscon@13030000 {
>>>  			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
>>>  			reg = <0x0 0x13030000 0x0 0x1000>;
>>> +
>>> +			pllclk: clock-controller {
>>> +				compatible = "starfive,jh7110-pll";
>>> +				clocks = <&osc>;
>>> +				#clock-cells = <1>;
>>
>> This should be part of previous patch. You just added that node. Don't
>> add half of devices but entire device.
>>
> 
> So do I merge the patch 6 and patch 7 into one patch and add syscon and
> clock-controller together?

I am okay with adding users of clocks in separate patch, but the clock
controller - so part of SYS - should be added when adding SYS.

Best regards,
Krzysztof
  
Xingyu Wu May 12, 2023, 7:25 a.m. UTC | #4
On 2023/5/12 15:22, Krzysztof Kozlowski wrote:
> On 12/05/2023 09:15, Xingyu Wu wrote:
>> On 2023/5/12 14:37, Krzysztof Kozlowski wrote:
>>> On 12/05/2023 04:20, Xingyu Wu wrote:
>>>> Add the PLL clock node for the Starfive JH7110 SoC and
>>>> modify the SYSCRG node to add PLL clocks input.
>>>
>>>
>>>> @@ -465,6 +469,12 @@ syscrg: clock-controller@13020000 {
>>>>  		sys_syscon: syscon@13030000 {
>>>>  			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
>>>>  			reg = <0x0 0x13030000 0x0 0x1000>;
>>>> +
>>>> +			pllclk: clock-controller {
>>>> +				compatible = "starfive,jh7110-pll";
>>>> +				clocks = <&osc>;
>>>> +				#clock-cells = <1>;
>>>
>>> This should be part of previous patch. You just added that node. Don't
>>> add half of devices but entire device.
>>>
>> 
>> So do I merge the patch 6 and patch 7 into one patch and add syscon and
>> clock-controller together?
> 
> I am okay with adding users of clocks in separate patch, but the clock
> controller - so part of SYS - should be added when adding SYS.
> 

Got it. Thanks.

Best regards,
Xingyu Wu
  

Patch

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index fa27fd4169a8..cdfd036a0e6c 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -452,12 +452,16 @@  syscrg: clock-controller@13020000 {
 				 <&gmac1_rgmii_rxin>,
 				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
 				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
-				 <&tdm_ext>, <&mclk_ext>;
+				 <&tdm_ext>, <&mclk_ext>,
+				 <&pllclk JH7110_CLK_PLL0_OUT>,
+				 <&pllclk JH7110_CLK_PLL1_OUT>,
+				 <&pllclk JH7110_CLK_PLL2_OUT>;
 			clock-names = "osc", "gmac1_rmii_refin",
 				      "gmac1_rgmii_rxin",
 				      "i2stx_bclk_ext", "i2stx_lrck_ext",
 				      "i2srx_bclk_ext", "i2srx_lrck_ext",
-				      "tdm_ext", "mclk_ext";
+				      "tdm_ext", "mclk_ext",
+				      "pll0_out", "pll1_out", "pll2_out";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
@@ -465,6 +469,12 @@  syscrg: clock-controller@13020000 {
 		sys_syscon: syscon@13030000 {
 			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
 			reg = <0x0 0x13030000 0x0 0x1000>;
+
+			pllclk: clock-controller {
+				compatible = "starfive,jh7110-pll";
+				clocks = <&osc>;
+				#clock-cells = <1>;
+			};
 		};
 
 		sysgpio: pinctrl@13040000 {