[PATCH v2 1/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos3250

[PATCH v2 1/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos3250

[Krzysztof Kozlowski]

Commit 52005dece527 ("ARM: dts: Add assigned clock parents to CMU node
for exynos3250") added assigned clocks under Clock Management Unit to
fix hangs when accessing ISP registers.

However the dtschema expects "clocks" property if "assigned-clocks" are
used.  Add reference to input clock, the parent used in
"assigned-clock-parents" to silence the dtschema warnings:

  arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes since v1:
1. Add clocks property.

This is a v2 for:
https://lore.kernel.org/linux-samsung-soc/20200901101534.GE23793@kozik-lap/T/#me85ac382b847dadbc3f6ebf30e94e70b5df1ebb6
---
 arch/arm/boot/dts/exynos3250.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index a1e93fb7f694..89b160280469 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -214,6 +214,7 @@
 			compatible = "samsung,exynos3250-cmu";
 			reg = <0x10030000 0x20000>;
 			#clock-cells = <1>;
+			clocks = <&cmu CLK_FIN_PLL>;
 			assigned-clocks = <&cmu CLK_MOUT_ACLK_400_MCUISP_SUB>,
 					  <&cmu CLK_MOUT_ACLK_266_SUB>;
 			assigned-clock-parents = <&cmu CLK_FIN_PLL>,
-- 
2.17.1

[PATCH v2 2/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos4412 Odroid

[Krzysztof Kozlowski]

Commit 68605101460e ("ARM: dts: exynos: Add support for audio over HDMI
for Odroid X/X2/U3") added assigned clocks under Clock Management Unit.

However the dtschema expects "clocks" property if "assigned-clocks" are
used.  Add reference to input clock, the parent used in
"assigned-clock-parents" to silence the dtschema warnings:

  arch/arm/boot/dts/exynos4412-odroidu3.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index ca3c78e0966c..9375df064076 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -120,6 +120,7 @@
 };
 
 &clock {
+	clocks = <&clock CLK_FOUT_EPLL>;
 	assigned-clocks = <&clock CLK_FOUT_EPLL>;
 	assigned-clock-rates = <45158401>;
 };
-- 
2.17.1

Re: [PATCH v2 2/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos4412 Odroid

[Marek Szyprowski]

Hi Krzysztof,

On 03.09.2020 20:14, Krzysztof Kozlowski wrote:
> Commit 68605101460e ("ARM: dts: exynos: Add support for audio over HDMI
> for Odroid X/X2/U3") added assigned clocks under Clock Management Unit.
>
> However the dtschema expects "clocks" property if "assigned-clocks" are
> used.  Add reference to input clock, the parent used in
> "assigned-clock-parents" to silence the dtschema warnings:
>
>    arch/arm/boot/dts/exynos4412-odroidu3.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>   arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> index ca3c78e0966c..9375df064076 100644
> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -120,6 +120,7 @@
>   };
>   
>   &clock {
> +	clocks = <&clock CLK_FOUT_EPLL>;

This should be one of xusbxti or xxti, because this is the proper input 
clock for the clock controller. However in case of Exynos4, those clocks 
needs much more cleanup. For the historical reasons, they don't use 
generic 'fixed-clock' property, but the custom one and they are no 
instantiated by clock framework, but the exynos4 clock driver...

>   	assigned-clocks = <&clock CLK_FOUT_EPLL>;
>   	assigned-clock-rates = <45158401>;
>   };

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 2/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos4412 Odroid

[Krzysztof Kozlowski]

On Fri, Sep 04, 2020 at 09:02:23AM +0200, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 03.09.2020 20:14, Krzysztof Kozlowski wrote:
> > Commit 68605101460e ("ARM: dts: exynos: Add support for audio over HDMI
> > for Odroid X/X2/U3") added assigned clocks under Clock Management Unit.
> >
> > However the dtschema expects "clocks" property if "assigned-clocks" are
> > used.  Add reference to input clock, the parent used in
> > "assigned-clock-parents" to silence the dtschema warnings:
> >
> >    arch/arm/boot/dts/exynos4412-odroidu3.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >   arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > index ca3c78e0966c..9375df064076 100644
> > --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> > @@ -120,6 +120,7 @@
> >   };
> >   
> >   &clock {
> > +	clocks = <&clock CLK_FOUT_EPLL>;
> 
> This should be one of xusbxti or xxti, because this is the proper input 
> clock for the clock controller. However in case of Exynos4, those clocks 
> needs much more cleanup. For the historical reasons, they don't use 
> generic 'fixed-clock' property, but the custom one and they are no 
> instantiated by clock framework, but the exynos4 clock driver...

Indeed, so it would be like:

&clock {
	clocks = <&clock CLK_XUSBXTI>;
};

... or convert the driver to take external clocks while keeping the ABI
(and being bisectable).

Best regards,
Krzysztof


> 
> >   	assigned-clocks = <&clock CLK_FOUT_EPLL>;
> >   	assigned-clock-rates = <45158401>;
> >   };
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 

[PATCH v2 3/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos5422 Odroid XU3

[Krzysztof Kozlowski]

Commit 78a68acf3d33 ("ARM: dts: exynos: Switch to dedicated Odroid XU3
sound card binding") added assigned clocks under sound device node.

However the dtschema expects "clocks" property if "assigned-clocks" are
used.  Add reference to input clock, the parent used in
"assigned-clock-parents" to silence the dtschema warnings:

  arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml: sound: 'clocks' is a dependency of 'assigned-clocks'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
index c3c2d85267da..44467a10c3b8 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
@@ -29,6 +29,11 @@
 			"HiFi Playback", "Mixer DAI TX",
 			"Mixer DAI RX", "HiFi Capture";
 
+		clocks = <&clock CLK_FOUT_EPLL>,
+			 <&clock CLK_MOUT_EPLL>,
+			 <&clock CLK_MOUT_MAU_EPLL>,
+			 <&clock CLK_MAU_EPLL>,
+			 <&clock_audss EXYNOS_MOUT_AUDSS>;
 		assigned-clocks = <&clock CLK_MOUT_EPLL>,
 				<&clock CLK_MOUT_MAU_EPLL>,
 				<&clock CLK_MOUT_USER_MAU_EPLL>,
-- 
2.17.1

Re: [PATCH v2 3/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos5422 Odroid XU3

[Sylwester Nawrocki]

On 9/3/20 20:14, Krzysztof Kozlowski wrote:
> Commit 78a68acf3d33 ("ARM: dts: exynos: Switch to dedicated Odroid XU3
> sound card binding") added assigned clocks under sound device node.
> 
> However the dtschema expects "clocks" property if "assigned-clocks" are
> used.  Add reference to input clock, the parent used in
> "assigned-clock-parents" to silence the dtschema warnings:

I'm afraid it doesn't improve anything, we just add another violation of
the DT binding rules as the 'sound' node doesn't represent a real HW and
shouldn't have 'clocks' property. Instead we could move the assigned-clock*
properties to the I2S node, as in below patch. I have tested that already 
on xu3.

----------------------------------8<---------------------------
From f98d2f5ac86d1ae13a77ef481fcbf073a1740f26 Mon Sep 17 00:00:00 2001
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
Date: Fri, 4 Sep 2020 12:02:11 +0200
Subject: [PATCH] ARM: dts: samsung: odroid-xu3: Move assigned-clock*
 properties to i2s0 node

The purpose of those assigned-clock-* properties is to configure clock for
for the I2S device so move them to respective node.

This suppresses the dtbs_check warning:
arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml: sound: 'clocks' is a dependency 
of 'assigned-clocks'

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi | 60 ++++++++++-------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
index c3c2d85..b5ec4f4 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3-audio.dtsi
@@ -29,30 +29,6 @@
 			"HiFi Playback", "Mixer DAI TX",
 			"Mixer DAI RX", "HiFi Capture";
 
-		assigned-clocks = <&clock CLK_MOUT_EPLL>,
-				<&clock CLK_MOUT_MAU_EPLL>,
-				<&clock CLK_MOUT_USER_MAU_EPLL>,
-				<&clock_audss EXYNOS_MOUT_AUDSS>,
-				<&clock_audss EXYNOS_MOUT_I2S>,
-				<&clock_audss EXYNOS_DOUT_SRP>,
-				<&clock_audss EXYNOS_DOUT_AUD_BUS>,
-				<&clock_audss EXYNOS_DOUT_I2S>;
-
-		assigned-clock-parents = <&clock CLK_FOUT_EPLL>,
-				<&clock CLK_MOUT_EPLL>,
-				<&clock CLK_MOUT_MAU_EPLL>,
-				<&clock CLK_MAU_EPLL>,
-				<&clock_audss EXYNOS_MOUT_AUDSS>;
-
-		assigned-clock-rates = <0>,
-				<0>,
-				<0>,
-				<0>,
-				<0>,
-				<196608001>,
-				<(196608002 / 2)>,
-				<196608000>;
-
 		cpu {
 			sound-dai = <&i2s0 0>, <&i2s0 1>;
 		};
@@ -62,13 +38,6 @@
 	};
 };
 
-&clock_audss {
-	assigned-clocks = <&clock_audss EXYNOS_DOUT_SRP>,
-			  <&clock CLK_FOUT_EPLL>;
-	assigned-clock-rates = <(196608000 / 256)>,
-			       <196608000>;
-};
-
 &hsi2c_5 {
 	status = "okay";
 	max98090: max98090@10 {
@@ -84,6 +53,31 @@
 
 &i2s0 {
 	status = "okay";
-	assigned-clocks = <&i2s0 CLK_I2S_RCLK_SRC>;
-	assigned-clock-parents = <&clock_audss EXYNOS_SCLK_I2S>;
+	assigned-clocks = <&clock CLK_MOUT_EPLL>,
+			<&clock CLK_MOUT_MAU_EPLL>,
+			<&clock CLK_MOUT_USER_MAU_EPLL>,
+			<&clock_audss EXYNOS_MOUT_AUDSS>,
+			<&clock_audss EXYNOS_MOUT_I2S>,
+			<&i2s0 CLK_I2S_RCLK_SRC>,
+			<&clock_audss EXYNOS_DOUT_SRP>,
+			<&clock_audss EXYNOS_DOUT_AUD_BUS>,
+			<&clock_audss EXYNOS_DOUT_I2S>;
+
+	assigned-clock-parents = <&clock CLK_FOUT_EPLL>,
+			<&clock CLK_MOUT_EPLL>,
+			<&clock CLK_MOUT_MAU_EPLL>,
+			<&clock CLK_MAU_EPLL>,
+			<&clock_audss EXYNOS_MOUT_AUDSS>,
+			<&clock_audss EXYNOS_SCLK_I2S>;
+
+	assigned-clock-rates = <0>,
+			<0>,
+			<0>,
+			<0>,
+			<0>,
+			<0>,
+			<196608001>,
+			<(196608002 / 2)>,
+			<196608000>;
+
 };
-- 
2.7.4

----------------------------------8<---------------------------
 

Re: [PATCH v2 3/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos5422 Odroid XU3

[Krzysztof Kozlowski]

On Fri, Sep 04, 2020 at 12:20:36PM +0200, Sylwester Nawrocki wrote:
> On 9/3/20 20:14, Krzysztof Kozlowski wrote:
> > Commit 78a68acf3d33 ("ARM: dts: exynos: Switch to dedicated Odroid XU3
> > sound card binding") added assigned clocks under sound device node.
> > 
> > However the dtschema expects "clocks" property if "assigned-clocks" are
> > used.  Add reference to input clock, the parent used in
> > "assigned-clock-parents" to silence the dtschema warnings:
> 
> I'm afraid it doesn't improve anything, we just add another violation of
> the DT binding rules as the 'sound' node doesn't represent a real HW and
> shouldn't have 'clocks' property. Instead we could move the assigned-clock*
> properties to the I2S node, as in below patch. I have tested that already 
> on xu3.
> 
> ----------------------------------8<---------------------------
> From f98d2f5ac86d1ae13a77ef481fcbf073a1740f26 Mon Sep 17 00:00:00 2001
> From: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Date: Fri, 4 Sep 2020 12:02:11 +0200
> Subject: [PATCH] ARM: dts: samsung: odroid-xu3: Move assigned-clock*
>  properties to i2s0 node
> 
> The purpose of those assigned-clock-* properties is to configure clock for
> for the I2S device so move them to respective node.
> 
> This suppresses the dtbs_check warning:
> arch/arm/boot/dts/exynos5422-odroidxu3.dt.yaml: sound: 'clocks' is a dependency 
> of 'assigned-clocks'

Thanks, this is a good idea.

Applied.

Best regards,
Krzysztof

Re: [PATCH v2 1/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos3250

[Marek Szyprowski]

Hi Krzysztof,

On 03.09.2020 20:14, Krzysztof Kozlowski wrote:
> Commit 52005dece527 ("ARM: dts: Add assigned clock parents to CMU node
> for exynos3250") added assigned clocks under Clock Management Unit to
> fix hangs when accessing ISP registers.
>
> However the dtschema expects "clocks" property if "assigned-clocks" are
> used.  Add reference to input clock, the parent used in
> "assigned-clock-parents" to silence the dtschema warnings:
>
>    arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> ---
>
> Changes since v1:
> 1. Add clocks property.
>
> This is a v2 for:
> https://lore.kernel.org/linux-samsung-soc/20200901101534.GE23793@kozik-lap/T/#me85ac382b847dadbc3f6ebf30e94e70b5df1ebb6
> ---
>   arch/arm/boot/dts/exynos3250.dtsi | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> index a1e93fb7f694..89b160280469 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -214,6 +214,7 @@
>   			compatible = "samsung,exynos3250-cmu";
>   			reg = <0x10030000 0x20000>;
>   			#clock-cells = <1>;
> +			clocks = <&cmu CLK_FIN_PLL>;

This is not a correct input clock for this CMU. Please assign it to 
xusbxti, xxti or xtcxo in the respective board dts, as this is a board 
property.

>   			assigned-clocks = <&cmu CLK_MOUT_ACLK_400_MCUISP_SUB>,
>   					  <&cmu CLK_MOUT_ACLK_266_SUB>;
>   			assigned-clock-parents = <&cmu CLK_FIN_PLL>,

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH v2 1/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos3250

[Krzysztof Kozlowski]

On Fri, Sep 04, 2020 at 08:47:10AM +0200, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 03.09.2020 20:14, Krzysztof Kozlowski wrote:
> > Commit 52005dece527 ("ARM: dts: Add assigned clock parents to CMU node
> > for exynos3250") added assigned clocks under Clock Management Unit to
> > fix hangs when accessing ISP registers.
> >
> > However the dtschema expects "clocks" property if "assigned-clocks" are
> > used.  Add reference to input clock, the parent used in
> > "assigned-clock-parents" to silence the dtschema warnings:
> >
> >    arch/arm/boot/dts/exynos3250-artik5-eval.dt.yaml: clock-controller@10030000: 'clocks' is a dependency of 'assigned-clocks'
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >
> > ---
> >
> > Changes since v1:
> > 1. Add clocks property.
> >
> > This is a v2 for:
> > https://lore.kernel.org/linux-samsung-soc/20200901101534.GE23793@kozik-lap/T/#me85ac382b847dadbc3f6ebf30e94e70b5df1ebb6
> > ---
> >   arch/arm/boot/dts/exynos3250.dtsi | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
> > index a1e93fb7f694..89b160280469 100644
> > --- a/arch/arm/boot/dts/exynos3250.dtsi
> > +++ b/arch/arm/boot/dts/exynos3250.dtsi
> > @@ -214,6 +214,7 @@
> >   			compatible = "samsung,exynos3250-cmu";
> >   			reg = <0x10030000 0x20000>;
> >   			#clock-cells = <1>;
> > +			clocks = <&cmu CLK_FIN_PLL>;
> 
> This is not a correct input clock for this CMU. Please assign it to 
> xusbxti, xxti or xtcxo in the respective board dts, as this is a board 
> property.

Makes sense, although all this is kind of a hack as neither the bindings
nor the driver take the input clock.


Best regards,
Krzysztof

> 
> >   			assigned-clocks = <&cmu CLK_MOUT_ACLK_400_MCUISP_SUB>,
> >   					  <&cmu CLK_MOUT_ACLK_266_SUB>;
> >   			assigned-clock-parents = <&cmu CLK_FIN_PLL>,
> 
> Best regards
> -- 
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
> 

Re: [PATCH v2 1/3] ARM: dts: exynos: Add assigned clock parent to CMU in Exynos3250

[Sylwester Nawrocki]

On 06.09.2020 14:44, Krzysztof Kozlowski wrote:
>>> diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
>>> index a1e93fb7f694..89b160280469 100644
>>> --- a/arch/arm/boot/dts/exynos3250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos3250.dtsi
>>> @@ -214,6 +214,7 @@
>>>   			compatible = "samsung,exynos3250-cmu";
>>>   			reg = <0x10030000 0x20000>;
>>>   			#clock-cells = <1>;
>>> +			clocks = <&cmu CLK_FIN_PLL>;
>> This is not a correct input clock for this CMU. Please assign it to 
>> xusbxti, xxti or xtcxo in the respective board dts, as this is a board 
>> property.

> Makes sense, although all this is kind of a hack as neither the bindings
> nor the driver take the input clock.

I think we should update the bindings so possible input clocks
to the CMU are documented for all SoCs. This is actually a bug 
in the clock controller DT bindings that the input clocks are
missing. Then the driver would handle both the old and the 
updated bindings but the "clocks" property would be documented 
as mandatory. I will try to have a look at this.