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

[Geert Uytterhoeven <geert@linux-m68k.org>]

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