2022-11-25 12:00:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks for Exynos850

Exynos850 has dedicated clock for accessing SYSREGs. Allow it, even
though Linux currently does not enable them and relies on bootloader.

Signed-off-by: Krzysztof Kozlowski <[email protected]>

---

Cc: Sriranjani P <[email protected]>
Cc: Chanho Park <[email protected]>
Cc: Sam Protsenko <[email protected]>
---
.../soc/samsung/samsung,exynos-sysreg.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
index 42357466005e..27cea934a286 100644
--- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
+++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
@@ -36,10 +36,25 @@ properties:
reg:
maxItems: 1

+ clocks:
+ maxItems: 1
+
required:
- compatible
- reg

+allOf:
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - samsung,exynos850-sysreg
+ then:
+ properties:
+ clocks: false
+
additionalProperties: false

examples:
--
2.34.1


2022-11-25 15:08:56

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks for Exynos850

On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
<[email protected]> wrote:
>
> Exynos850 has dedicated clock for accessing SYSREGs. Allow it, even
> though Linux currently does not enable them and relies on bootloader.
>

Not sure if this description is correct. Of course, there is no driver
for "samsung,exynos850-sysreg" compatible at the moment, so the next
compatible from the list ("syscon") is used for Exynos850. And
"syscon" driver (drivers/mfd/syscon.c) actually does control the
clocks. I remember adding "clocks" property to Exynos850 dts to fix
actual problem. Also, the "clocks" property is not described in
Documentation/devicetree/bindings/mfd/syscon.yaml, didn't really check
if it's ok or it's just missing.

Other than that comment:

Reviewed-by: Sam Protsenko <[email protected]>

> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> Cc: Sriranjani P <[email protected]>
> Cc: Chanho Park <[email protected]>
> Cc: Sam Protsenko <[email protected]>
> ---
> .../soc/samsung/samsung,exynos-sysreg.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> index 42357466005e..27cea934a286 100644
> --- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> +++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-sysreg.yaml
> @@ -36,10 +36,25 @@ properties:
> reg:
> maxItems: 1
>
> + clocks:
> + maxItems: 1
> +
> required:
> - compatible
> - reg
>
> +allOf:
> + - if:
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - samsung,exynos850-sysreg
> + then:
> + properties:
> + clocks: false
> +
> additionalProperties: false
>
> examples:
> --
> 2.34.1
>

2022-11-25 15:09:50

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks for Exynos850

On 25/11/2022 15:38, Sam Protsenko wrote:
> On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
> <[email protected]> wrote:
>>
>> Exynos850 has dedicated clock for accessing SYSREGs. Allow it, even
>> though Linux currently does not enable them and relies on bootloader.
>>
>
> Not sure if this description is correct. Of course, there is no driver
> for "samsung,exynos850-sysreg" compatible at the moment, so the next
> compatible from the list ("syscon") is used for Exynos850. And
> "syscon" driver (drivers/mfd/syscon.c) actually does control the
> clocks. I remember adding "clocks" property to Exynos850 dts to fix
> actual problem. Also, the "clocks" property is not described in
> Documentation/devicetree/bindings/mfd/syscon.yaml, didn't really check
> if it's ok or it's just missing.
>
> Other than that comment:
>
> Reviewed-by: Sam Protsenko <[email protected]>
>
>> Signed-off-by: Krzysztof Kozlowski <[email protected]>

Ah, then commit msg is not good. I'll update it and maybe the clocks
should be required for Exynos850?

Best regards,
Krzysztof

2022-11-25 15:23:21

by Sam Protsenko

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks for Exynos850

On Fri, 25 Nov 2022 at 08:49, Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 25/11/2022 15:38, Sam Protsenko wrote:
> > On Fri, 25 Nov 2022 at 05:22, Krzysztof Kozlowski
> > <[email protected]> wrote:
> >>
> >> Exynos850 has dedicated clock for accessing SYSREGs. Allow it, even
> >> though Linux currently does not enable them and relies on bootloader.
> >>
> >
> > Not sure if this description is correct. Of course, there is no driver
> > for "samsung,exynos850-sysreg" compatible at the moment, so the next
> > compatible from the list ("syscon") is used for Exynos850. And
> > "syscon" driver (drivers/mfd/syscon.c) actually does control the
> > clocks. I remember adding "clocks" property to Exynos850 dts to fix
> > actual problem. Also, the "clocks" property is not described in
> > Documentation/devicetree/bindings/mfd/syscon.yaml, didn't really check
> > if it's ok or it's just missing.
> >
> > Other than that comment:
> >
> > Reviewed-by: Sam Protsenko <[email protected]>
> >
> >> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> Ah, then commit msg is not good. I'll update it and maybe the clocks
> should be required for Exynos850?
>

Yeah, looks like all Exynos850 sysreg blocks have dedicated clock.
Please make it required. And thanks for working on that! :)

> Best regards,
> Krzysztof
>

2022-11-26 04:49:11

by Sriranjani P

[permalink] [raw]
Subject: RE: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks for Exynos850



> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:[email protected]]
> Sent: 25 November 2022 16:52
> To: Lee Jones <[email protected]>; Rob Herring <[email protected]>;
Krzysztof
> Kozlowski <[email protected]>; Alim Akhtar
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected];
linux-samsung-
> [email protected]
> Cc: Krzysztof Kozlowski <[email protected]>; Sriranjani P
> <[email protected]>; Chanho Park <[email protected]>;
> Sam Protsenko <[email protected]>
> Subject: [PATCH 4/4] dt-bindings: soc: samsung: exynos-sysreg: add clocks
for
> Exynos850
>
> Exynos850 has dedicated clock for accessing SYSREGs. Allow it, even
though
> Linux currently does not enable them and relies on bootloader.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>
> ---
>
> Cc: Sriranjani P <[email protected]>
> Cc: Chanho Park <[email protected]>
> Cc: Sam Protsenko <[email protected]>
> ---

Reviewed-by: Sriranjani P <[email protected]>

> .../soc/samsung/samsung,exynos-sysreg.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> index 42357466005e..27cea934a286 100644
> --- a/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysreg.yaml
> +++ b/Documentation/devicetree/bindings/soc/samsung/samsung,exynos-
> sysre
> +++ g.yaml
> @@ -36,10 +36,25 @@ properties:
> reg:
> maxItems: 1
>
> + clocks:
> + maxItems: 1
> +
> required:
> - compatible
> - reg
>
> +allOf:
> + - if:
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - samsung,exynos850-sysreg
> + then:
> + properties:
> + clocks: false
> +
> additionalProperties: false
>
> examples:
> --
> 2.34.1