[PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Wolfram Sang]

This only applies to R-Car Gen2 and later generations, so we need to
distinguish.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
---

v1 and v2 were part of a 21-patch-series which was accepted now except
for this patch. Updated according to Geert's comments and finally also
sent to Rob and the DT mailing list.

Tested with:
m dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml

I hope it really does what I intended to check.

If so, the patch can be applied individually. I think, however, it is
most convenient if Geert picks it up together with the 20 other patches.

 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 40 ++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 9f1e7092cf44..43fc6ac56038 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -129,15 +129,37 @@ allOf:
         - clock-names
         - resets
     else:
-      properties:
-        clocks:
-          minItems: 1
-          maxItems: 2
-        clock-names:
-          minItems: 1
-          items:
-            - const: core
-            - const: cd
+      if:
+        properties:
+          compatible:
+            contains:
+              enum:
+                - renesas,rcar-gen2-sdhi
+                - renesas,rcar-gen3-sdhi
+      then:
+        properties:
+          clocks:
+            minItems: 1
+            maxItems: 3
+          clock-names:
+            minItems: 1
+            maxItems: 3
+            uniqueItems: true
+            items:
+              - const: core
+              - enum: [ clkh, cd ]
+              - const: cd
+      else:
+        properties:
+          clocks:
+            minItems: 1
+            maxItems: 2
+          clock-names:
+            minItems: 1
+            maxItems: 2
+            items:
+              - const: core
+              - const: cd
 
   - if:
       properties:
-- 
2.30.2

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Wolfram,

Thanks for the patch.

> Subject: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> This only applies to R-Car Gen2 and later generations, so we need to
> distinguish.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> v1 and v2 were part of a 21-patch-series which was accepted now except for
> this patch. Updated according to Geert's comments and finally also sent to
> Rob and the DT mailing list.
> 
> Tested with:
> m dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> 
> I hope it really does what I intended to check.
> 
> If so, the patch can be applied individually. I think, however, it is most
> convenient if Geert picks it up together with the 20 other patches.
> 
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 40 ++++++++++++++-----
>  1 file changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index 9f1e7092cf44..43fc6ac56038 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -129,15 +129,37 @@ allOf:
>          - clock-names
>          - resets
>      else:
> -      properties:
> -        clocks:
> -          minItems: 1
> -          maxItems: 2
> -        clock-names:
> -          minItems: 1
> -          items:
> -            - const: core
> -            - const: cd
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - renesas,rcar-gen2-sdhi
> +                - renesas,rcar-gen3-sdhi

What about RZ/G2L, as it has 4 clocks?

Regards,
Biju

> +      then:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 3
> +          clock-names:
> +            minItems: 1
> +            maxItems: 3
> +            uniqueItems: true
> +            items:
> +              - const: core
> +              - enum: [ clkh, cd ]
> +              - const: cd
> +      else:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 2
> +          clock-names:
> +            minItems: 1
> +            maxItems: 2
> +            items:
> +              - const: core
> +              - const: cd
> 
>    - if:
>        properties:
> --
> 2.30.2

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]


> > +      if:
> > +        properties:
> > +          compatible:
> > +            contains:
> > +              enum:
> > +                - renesas,rcar-gen2-sdhi
> > +                - renesas,rcar-gen3-sdhi
> 
> What about RZ/G2L, as it has 4 clocks?

They are a few lines above this in a seperate block if I am not
confusing the SoC numbering.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Wolfram,

> Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> 
> > > +      if:
> > > +        properties:
> > > +          compatible:
> > > +            contains:
> > > +              enum:
> > > +                - renesas,rcar-gen2-sdhi
> > > +                - renesas,rcar-gen3-sdhi
> >
> > What about RZ/G2L, as it has 4 clocks?
> 
> They are a few lines above this in a seperate block if I am not confusing
> the SoC numbering.

Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi compatible, We need to move that
Separate block inside this if block. With in gen3 compatible, if RZG2L then 
Max 4 clocks else Max 3 Clocks. I may be completely wrong.

Regards,
Biju

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Geert Uytterhoeven]

Hi Biju,

On Mon, Nov 15, 2021 at 9:32 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> > clock
> >
> >
> > > > +      if:
> > > > +        properties:
> > > > +          compatible:
> > > > +            contains:
> > > > +              enum:
> > > > +                - renesas,rcar-gen2-sdhi
> > > > +                - renesas,rcar-gen3-sdhi
> > >
> > > What about RZ/G2L, as it has 4 clocks?
> >
> > They are a few lines above this in a seperate block if I am not confusing
> > the SoC numbering.
>
> Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi compatible, We need to move that
> Separate block inside this if block. With in gen3 compatible, if RZG2L then
> Max 4 clocks else Max 3 Clocks. I may be completely wrong.

But is it working?
With this series, the driver looks for the "sdh" clock, while it is
called "clk_hs" on RZ/G2L.

As the RZ/G2L part declares compatibility with the generic
rcar-gen3-sdhi compatible, it is subject to SDHI_FLAG_NEED_CLKH_FALLBACK.
In the absence of an "sdh" clock, the driver will use
clk_get_parent(clk_get_parent(priv->clk) instead.
On RZ/G2L, we have:
  SD0 -> SD0_DIV4 -> imclk
      -> clk_hs
So that will pick up SD0, which might be correct, accidentally ;-)
As quirks is not set, it will use clkh_shift = 2.

So all is good? I think we still want the driver to check for "clk_hs",
too, to avoid having to depend on the fallback.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> Hi Biju,
> 
> On Mon, Nov 15, 2021 at 9:32 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional
> > > SDnH clock
> > >
> > >
> > > > > +      if:
> > > > > +        properties:
> > > > > +          compatible:
> > > > > +            contains:
> > > > > +              enum:
> > > > > +                - renesas,rcar-gen2-sdhi
> > > > > +                - renesas,rcar-gen3-sdhi
> > > >
> > > > What about RZ/G2L, as it has 4 clocks?
> > >
> > > They are a few lines above this in a seperate block if I am not
> > > confusing the SoC numbering.
> >
> > Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi
> > compatible, We need to move that Separate block inside this if block.
> > With in gen3 compatible, if RZG2L then Max 4 clocks else Max 3 Clocks. I
> may be completely wrong.
> 
> But is it working?
> With this series, the driver looks for the "sdh" clock, while it is called
> "clk_hs" on RZ/G2L.
> 
> As the RZ/G2L part declares compatibility with the generic rcar-gen3-sdhi
> compatible, it is subject to SDHI_FLAG_NEED_CLKH_FALLBACK.
> In the absence of an "sdh" clock, the driver will use
> clk_get_parent(clk_get_parent(priv->clk) instead.
> On RZ/G2L, we have:
>   SD0 -> SD0_DIV4 -> imclk
>       -> clk_hs
> So that will pick up SD0, which might be correct, accidentally ;-) As
> quirks is not set, it will use clkh_shift = 2.
> 
> So all is good? I think we still want the driver to check for "clk_hs",
> too, to avoid having to depend on the fallback.

I have added below piece of code and tested clk_hs. It works ok.

Can we change the binding to update to use "clkh" instead of "clk_hs" for RZ/G2L?, so that we don't need
below code change in driver. Any way it is optional.

+       
+       if(!priv->clkh) {
+               priv->clkh = devm_clk_get_optional(&pdev->dev, "clk_hs");
+               if (IS_ERR(priv->clkh))
+                       return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkh), "cannot get clk_hs");

Regards,
Biju


> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Geert Uytterhoeven]

Hi Biju,

On Tue, Nov 16, 2021 at 12:26 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> > clock
> > On Mon, Nov 15, 2021 at 9:32 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional
> > > > SDnH clock
> > > >
> > > >
> > > > > > +      if:
> > > > > > +        properties:
> > > > > > +          compatible:
> > > > > > +            contains:
> > > > > > +              enum:
> > > > > > +                - renesas,rcar-gen2-sdhi
> > > > > > +                - renesas,rcar-gen3-sdhi
> > > > >
> > > > > What about RZ/G2L, as it has 4 clocks?
> > > >
> > > > They are a few lines above this in a seperate block if I am not
> > > > confusing the SoC numbering.
> > >
> > > Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi
> > > compatible, We need to move that Separate block inside this if block.
> > > With in gen3 compatible, if RZG2L then Max 4 clocks else Max 3 Clocks. I
> > may be completely wrong.
> >
> > But is it working?
> > With this series, the driver looks for the "sdh" clock, while it is called
> > "clk_hs" on RZ/G2L.
> >
> > As the RZ/G2L part declares compatibility with the generic rcar-gen3-sdhi
> > compatible, it is subject to SDHI_FLAG_NEED_CLKH_FALLBACK.
> > In the absence of an "sdh" clock, the driver will use
> > clk_get_parent(clk_get_parent(priv->clk) instead.
> > On RZ/G2L, we have:
> >   SD0 -> SD0_DIV4 -> imclk
> >       -> clk_hs
> > So that will pick up SD0, which might be correct, accidentally ;-) As
> > quirks is not set, it will use clkh_shift = 2.
> >
> > So all is good? I think we still want the driver to check for "clk_hs",
> > too, to avoid having to depend on the fallback.
>
> I have added below piece of code and tested clk_hs. It works ok.
>
> Can we change the binding to update to use "clkh" instead of "clk_hs" for RZ/G2L?, so that we don't need
> below code change in driver. Any way it is optional.

Fine for me.
Should we also rename imclk2 to cd?

Note that on RZ/G2L, it will be handled by Runtime PM regardless.

> +
> +       if(!priv->clkh) {
> +               priv->clkh = devm_clk_get_optional(&pdev->dev, "clk_hs");
> +               if (IS_ERR(priv->clkh))
> +                       return dev_err_probe(&pdev->dev, PTR_ERR(priv->clkh), "cannot get clk_hs");

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> Hi Biju,
> 
> On Tue, Nov 16, 2021 at 12:26 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional
> > > SDnH clock On Mon, Nov 15, 2021 at 9:32 PM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add
> > > > > optional SDnH clock
> > > > >
> > > > >
> > > > > > > +      if:
> > > > > > > +        properties:
> > > > > > > +          compatible:
> > > > > > > +            contains:
> > > > > > > +              enum:
> > > > > > > +                - renesas,rcar-gen2-sdhi
> > > > > > > +                - renesas,rcar-gen3-sdhi
> > > > > >
> > > > > > What about RZ/G2L, as it has 4 clocks?
> > > > >
> > > > > They are a few lines above this in a seperate block if I am not
> > > > > confusing the SoC numbering.
> > > >
> > > > Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi
> > > > compatible, We need to move that Separate block inside this if
> block.
> > > > With in gen3 compatible, if RZG2L then Max 4 clocks else Max 3
> > > > Clocks. I
> > > may be completely wrong.
> > >
> > > But is it working?
> > > With this series, the driver looks for the "sdh" clock, while it is
> > > called "clk_hs" on RZ/G2L.
> > >
> > > As the RZ/G2L part declares compatibility with the generic
> > > rcar-gen3-sdhi compatible, it is subject to
> SDHI_FLAG_NEED_CLKH_FALLBACK.
> > > In the absence of an "sdh" clock, the driver will use
> > > clk_get_parent(clk_get_parent(priv->clk) instead.
> > > On RZ/G2L, we have:
> > >   SD0 -> SD0_DIV4 -> imclk
> > >       -> clk_hs
> > > So that will pick up SD0, which might be correct, accidentally ;-)
> > > As quirks is not set, it will use clkh_shift = 2.
> > >
> > > So all is good? I think we still want the driver to check for
> > > "clk_hs", too, to avoid having to depend on the fallback.
> >
> > I have added below piece of code and tested clk_hs. It works ok.
> >
> > Can we change the binding to update to use "clkh" instead of "clk_hs"
> > for RZ/G2L?, so that we don't need below code change in driver. Any way
> it is optional.
> 
> Fine for me.
> Should we also rename imclk2 to cd?

Yes, we can rename imclk2 to cd.

Regards,
Biju

> 
> Note that on RZ/G2L, it will be handled by Runtime PM regardless.
> > +
> > +       if(!priv->clkh) {
> > +               priv->clkh = devm_clk_get_optional(&pdev->dev,
> "clk_hs");
> > +               if (IS_ERR(priv->clkh))
> > +                       return dev_err_probe(&pdev->dev,
> > + PTR_ERR(priv->clkh), "cannot get clk_hs");
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Geert and Wolfram,

> Subject: RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional
> > SDnH clock
> >
> > Hi Biju,
> >
> > On Tue, Nov 16, 2021 at 12:26 PM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add
> > > > optional SDnH clock On Mon, Nov 15, 2021 at 9:32 PM Biju Das
> > > > <biju.das.jz@bp.renesas.com>
> > > > wrote:
> > > > > > Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add
> > > > > > optional SDnH clock
> > > > > >
> > > > > >
> > > > > > > > +      if:
> > > > > > > > +        properties:
> > > > > > > > +          compatible:
> > > > > > > > +            contains:
> > > > > > > > +              enum:
> > > > > > > > +                - renesas,rcar-gen2-sdhi
> > > > > > > > +                - renesas,rcar-gen3-sdhi
> > > > > > >
> > > > > > > What about RZ/G2L, as it has 4 clocks?
> > > > > >
> > > > > > They are a few lines above this in a seperate block if I am
> > > > > > not confusing the SoC numbering.
> > > > >
> > > > > Ah ok, I thought, since RZ/G2L using generic rcar-gen3-sdhi
> > > > > compatible, We need to move that Separate block inside this if
> > block.
> > > > > With in gen3 compatible, if RZG2L then Max 4 clocks else Max 3
> > > > > Clocks. I
> > > > may be completely wrong.
> > > >
> > > > But is it working?
> > > > With this series, the driver looks for the "sdh" clock, while it
> > > > is called "clk_hs" on RZ/G2L.
> > > >
> > > > As the RZ/G2L part declares compatibility with the generic
> > > > rcar-gen3-sdhi compatible, it is subject to
> > SDHI_FLAG_NEED_CLKH_FALLBACK.
> > > > In the absence of an "sdh" clock, the driver will use
> > > > clk_get_parent(clk_get_parent(priv->clk) instead.
> > > > On RZ/G2L, we have:
> > > >   SD0 -> SD0_DIV4 -> imclk
> > > >       -> clk_hs
> > > > So that will pick up SD0, which might be correct, accidentally ;-)
> > > > As quirks is not set, it will use clkh_shift = 2.
> > > >
> > > > So all is good? I think we still want the driver to check for
> > > > "clk_hs", too, to avoid having to depend on the fallback.
> > >
> > > I have added below piece of code and tested clk_hs. It works ok.
> > >
> > > Can we change the binding to update to use "clkh" instead of "clk_hs"
> > > for RZ/G2L?, so that we don't need below code change in driver. Any
> > > way
> > it is optional.
> >
> > Fine for me.
> > Should we also rename imclk2 to cd?
> 
> Yes, we can rename imclk2 to cd.

Please let me know, if you want me to do this changes as separate patch(binding + dtsi) or
Will you take care this? Both are ok to me.

Regards,
Biju

> 
> >
> > Note that on RZ/G2L, it will be handled by Runtime PM regardless.
> > > +
> > > +       if(!priv->clkh) {
> > > +               priv->clkh = devm_clk_get_optional(&pdev->dev,
> > "clk_hs");
> > > +               if (IS_ERR(priv->clkh))
> > > +                       return dev_err_probe(&pdev->dev,
> > > + PTR_ERR(priv->clkh), "cannot get clk_hs");
> >
> > Gr{oetje,eeting}s,
> >
> >                         Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > geert@linux- m68k.org
> >
> > In personal conversations with technical people, I call myself a hacker.
> > But when I'm talking to journalists I just say "programmer" or
> > something like that.
> >                                 -- Linus Torvalds

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

Hi Biju,

> Please let me know, if you want me to do this changes as separate patch(binding + dtsi) or
> Will you take care this? Both are ok to me.

I think it is better if you do it. You can do testing of these patches.
I'll happily review it, of course.

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]


> Please let me know, if you want me to do this changes as separate patch(binding + dtsi) or
> Will you take care this? Both are ok to me.

This patch is then good as-is with Biju sending the incremental patches?
Or did I miss something from this discussion?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Wolfram,

> Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> 
> > Please let me know, if you want me to do this changes as separate
> > patch(binding + dtsi) or Will you take care this? Both are ok to me.
> 
> This patch is then good as-is with Biju sending the incremental patches?
> Or did I miss something from this discussion?

Just a question, Can we make max clocks to 4 for generic compatible?

Then the patch will be like this.

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 43fc6ac56038..6a9d58b33ef5 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -103,6 +103,39 @@ properties:
 allOf:
   - $ref: "mmc-controller.yaml"
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - renesas,rcar-gen2-sdhi
+              - renesas,rcar-gen3-sdhi
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 4
+        clock-names:
+          minItems: 1
+          maxItems: 4
+          uniqueItems: true
+          items:
+            - const: core
+            - enum: [ clkh, cd ]
+            - const: cd
+            - const: aclk
+    else:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 2
+        clock-names:
+          minItems: 1
+          maxItems: 2
+          items:
+            - const: core
+            - const: cd
+
   - if:
       properties:
         compatible:
@@ -113,53 +146,21 @@ allOf:
         clocks:
           items:
             - description: IMCLK, SDHI channel main clock1.
+            - description: CLK_HS, SDHI channel High speed clock which operates
+                           4 times that of SDHI channel main clock1.
             - description: IMCLK2, SDHI channel main clock2. When this clock is
                            turned off, external SD card detection cannot be
                            detected.
-            - description: CLK_HS, SDHI channel High speed clock which operates
-                           4 times that of SDHI channel main clock1.
             - description: ACLK, SDHI channel bus clock.
         clock-names:
           items:
-            - const: imclk
-            - const: imclk2
-            - const: clk_hs
+            - const: core
+            - const: clkh
+            - const: cd
             - const: aclk
       required:
         - clock-names
         - resets
-    else:
-      if:
-        properties:
-          compatible:
-            contains:
-              enum:
-                - renesas,rcar-gen2-sdhi
-                - renesas,rcar-gen3-sdhi
-      then:
-        properties:
-          clocks:
-            minItems: 1
-            maxItems: 3
-          clock-names:
-            minItems: 1
-            maxItems: 3
-            uniqueItems: true
-            items:
-              - const: core
-              - enum: [ clkh, cd ]
-              - const: cd
-      else:
-        properties:
-          clocks:
-            minItems: 1
-            maxItems: 2
-          clock-names:
-            minItems: 1
-            maxItems: 2
-            items:
-              - const: core
-              - const: cd

RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Biju Das]

Hi Wolfram,

> Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH
> clock
> 
> Hi Biju,
> 
> > Please let me know, if you want me to do this changes as separate
> > patch(binding + dtsi) or Will you take care this? Both are ok to me.
> 
> I think it is better if you do it. You can do testing of these patches.
> I'll happily review it, of course.

Agreed, will send incremental patch after this.

Regards,
Biju

Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, Nov 15, 2021 at 5:08 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> This only applies to R-Car Gen2 and later generations, so we need to
> distinguish.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks for your patch!

> ---
>
> v1 and v2 were part of a 21-patch-series which was accepted now except
> for this patch. Updated according to Geert's comments and finally also
> sent to Rob and the DT mailing list.
>
> Tested with:
> m dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
>
> I hope it really does what I intended to check.
>
> If so, the patch can be applied individually. I think, however, it is
> most convenient if Geert picks it up together with the 20 other patches.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.17, with the below fixed.

> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -129,15 +129,37 @@ allOf:
>          - clock-names
>          - resets
>      else:
> -      properties:
> -        clocks:
> -          minItems: 1
> -          maxItems: 2
> -        clock-names:
> -          minItems: 1
> -          items:
> -            - const: core
> -            - const: cd
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - renesas,rcar-gen2-sdhi
> +                - renesas,rcar-gen3-sdhi
> +      then:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 3
> +          clock-names:
> +            minItems: 1
> +            maxItems: 3

"maxItems" is not needed with an "items" list

"make dt_bindings_check" doesn't complain, presumably because this
is part of an if/else block.

> +            uniqueItems: true
> +            items:
> +              - const: core
> +              - enum: [ clkh, cd ]
> +              - const: cd
> +      else:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 2
> +          clock-names:
> +            minItems: 1
> +            maxItems: 2

Likewise ("git show --color-words" shows it wasn't present before).

> +            items:
> +              - const: core
> +              - const: cd
>
>    - if:
>        properties:

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds