2020-09-03 18:15:50

by Krzysztof Kozlowski

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

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 <[email protected]>

---

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


2020-09-03 18:16:29

by Krzysztof Kozlowski

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

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 <[email protected]>
---
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

2020-09-03 18:17:32

by Krzysztof Kozlowski

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

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 <[email protected]>
---
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

2020-09-04 06:49:41

by Marek Szyprowski

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

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 <[email protected]>
>
> ---
>
> 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

2020-09-04 07:05:40

by Marek Szyprowski

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

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 <[email protected]>
> ---
> 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

2020-09-04 10:23:40

by Sylwester Nawrocki

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

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 <[email protected]>
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 <[email protected]>
---
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<---------------------------

2020-09-06 12:48:49

by Krzysztof Kozlowski

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

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 <[email protected]>
> >
> > ---
> >
> > 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
>

2020-09-06 12:58:29

by Krzysztof Kozlowski

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

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 <[email protected]>
> > ---
> > 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
>

2020-09-06 13:09:50

by Krzysztof Kozlowski

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

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 <[email protected]>
> 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

2020-09-07 08:36:59

by Sylwester Nawrocki

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

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.