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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux