* [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
@ 2021-11-15 16:06 Wolfram Sang
2021-11-15 16:31 ` Biju Das
2021-11-19 10:12 ` Geert Uytterhoeven
0 siblings, 2 replies; 14+ messages in thread
From: Wolfram Sang @ 2021-11-15 16:06 UTC (permalink / raw)
To: linux-renesas-soc
Cc: linux-mmc, devicetree, Rob Herring, Wolfram Sang, Ulf Hansson
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-15 16:06 [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock Wolfram Sang
@ 2021-11-15 16:31 ` Biju Das
2021-11-15 19:24 ` Wolfram Sang
2021-11-19 10:12 ` Geert Uytterhoeven
1 sibling, 1 reply; 14+ messages in thread
From: Biju Das @ 2021-11-15 16:31 UTC (permalink / raw)
To: Wolfram Sang, linux-renesas-soc
Cc: linux-mmc, devicetree, Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-15 16:31 ` Biju Das
@ 2021-11-15 19:24 ` Wolfram Sang
2021-11-15 20:24 ` Biju Das
0 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2021-11-15 19:24 UTC (permalink / raw)
To: Biju Das
Cc: linux-renesas-soc, linux-mmc, devicetree, Rob Herring, Ulf Hansson
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-15 19:24 ` Wolfram Sang
@ 2021-11-15 20:24 ` Biju Das
2021-11-16 10:26 ` Geert Uytterhoeven
0 siblings, 1 reply; 14+ messages in thread
From: Biju Das @ 2021-11-15 20:24 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, linux-mmc, devicetree, Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-15 20:24 ` Biju Das
@ 2021-11-16 10:26 ` Geert Uytterhoeven
2021-11-16 11:26 ` Biju Das
0 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2021-11-16 10:26 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 10:26 ` Geert Uytterhoeven
@ 2021-11-16 11:26 ` Biju Das
2021-11-16 13:22 ` Geert Uytterhoeven
0 siblings, 1 reply; 14+ messages in thread
From: Biju Das @ 2021-11-16 11:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 11:26 ` Biju Das
@ 2021-11-16 13:22 ` Geert Uytterhoeven
2021-11-16 16:09 ` Biju Das
0 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2021-11-16 13:22 UTC (permalink / raw)
To: Biju Das
Cc: Wolfram Sang, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 13:22 ` Geert Uytterhoeven
@ 2021-11-16 16:09 ` Biju Das
2021-11-16 19:41 ` Biju Das
0 siblings, 1 reply; 14+ messages in thread
From: Biju Das @ 2021-11-16 16:09 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 16:09 ` Biju Das
@ 2021-11-16 19:41 ` Biju Das
2021-11-16 20:29 ` Wolfram Sang
2021-11-17 10:31 ` Wolfram Sang
0 siblings, 2 replies; 14+ messages in thread
From: Biju Das @ 2021-11-16 19:41 UTC (permalink / raw)
To: Geert Uytterhoeven, Wolfram Sang
Cc: Wolfram Sang, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 19:41 ` Biju Das
@ 2021-11-16 20:29 ` Wolfram Sang
2021-11-17 14:00 ` Biju Das
2021-11-17 10:31 ` Wolfram Sang
1 sibling, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2021-11-16 20:29 UTC (permalink / raw)
To: Biju Das
Cc: Geert Uytterhoeven, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 19:41 ` Biju Das
2021-11-16 20:29 ` Wolfram Sang
@ 2021-11-17 10:31 ` Wolfram Sang
2021-11-17 11:43 ` Biju Das
1 sibling, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2021-11-17 10:31 UTC (permalink / raw)
To: Biju Das
Cc: Geert Uytterhoeven, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-17 10:31 ` Wolfram Sang
@ 2021-11-17 11:43 ` Biju Das
0 siblings, 0 replies; 14+ messages in thread
From: Biju Das @ 2021-11-17 11:43 UTC (permalink / raw)
To: Wolfram Sang
Cc: Geert Uytterhoeven, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-16 20:29 ` Wolfram Sang
@ 2021-11-17 14:00 ` Biju Das
0 siblings, 0 replies; 14+ messages in thread
From: Biju Das @ 2021-11-17 14:00 UTC (permalink / raw)
To: Wolfram Sang
Cc: Geert Uytterhoeven, linux-renesas-soc, linux-mmc, devicetree,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
2021-11-15 16:06 [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock Wolfram Sang
2021-11-15 16:31 ` Biju Das
@ 2021-11-19 10:12 ` Geert Uytterhoeven
1 sibling, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2021-11-19 10:12 UTC (permalink / raw)
To: Wolfram Sang
Cc: Linux-Renesas, Linux MMC List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Rob Herring, Ulf Hansson
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-11-19 10:13 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 16:06 [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock Wolfram Sang
2021-11-15 16:31 ` Biju Das
2021-11-15 19:24 ` Wolfram Sang
2021-11-15 20:24 ` Biju Das
2021-11-16 10:26 ` Geert Uytterhoeven
2021-11-16 11:26 ` Biju Das
2021-11-16 13:22 ` Geert Uytterhoeven
2021-11-16 16:09 ` Biju Das
2021-11-16 19:41 ` Biju Das
2021-11-16 20:29 ` Wolfram Sang
2021-11-17 14:00 ` Biju Das
2021-11-17 10:31 ` Wolfram Sang
2021-11-17 11:43 ` Biju Das
2021-11-19 10:12 ` Geert Uytterhoeven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.