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@xxxxxxxxxxxxxx> > 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