Re: [PATCH 3/6] mmc: renesas_sdhi: Add compatible string for RZ/G2L family, RZ/G3S, and RZ/V2M SoCs

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Apr 23, 2024 at 7:53 AM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Mon, Apr 22, 2024 at 11:30 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > - RZ/G2UL and RZ/Five ("r9a07g043")
> > - RZ/G2L(C) ("r9a07g044")
> > - RZ/V2L ("r9a07g054")
> > - RZ/G3S ("r9a08g045")
> > - RZ/V2M ("r9a09g011")
> >
> > The above SoCs have HS400 disabled and use fixed address mode. Add a
> > generic compatible 'renesas,rzg2l-sdhi' fallback string for these SoCs,
> > where fixed_addr_mode and hs400_disabled quirks are applied.
>
> Thanks for your patch!
> >
> > Note, 'renesas,sdhi-r9a09g011' is dropped as we will be using
> > 'renesas,rzg2l-sdhi' as a fallback string for RZ/V2M SoC.
>
> Doesn't that break backwards compatibility with existing DTBs?
>
I was in two minds here, as similarly to the RZ/G2L family the RZ/V2M
has Gen3 compat string as a fallback in the DTSI. So for existing DTBs
this would still work (but with no quirks applied). But as you say
I'll keep this to make sure we dont break anything,

> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> >  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > index 53d34c3eddce..1828c37e0198 100644
> > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > @@ -210,7 +210,7 @@ static const struct renesas_sdhi_quirks sdhi_quirks_r8a77990 = {
> >         .manual_tap_correction = true,
> >  };
> >
> > -static const struct renesas_sdhi_quirks sdhi_quirks_r9a09g011 = {
> > +static const struct renesas_sdhi_quirks sdhi_quirks_rzg2l = {
> >         .fixed_addr_mode = true,
> >         .hs400_disabled = true,
> >  };
> > @@ -255,9 +255,9 @@ static const struct renesas_sdhi_of_data_with_quirks of_r8a77990_compatible = {
> >         .quirks = &sdhi_quirks_r8a77990,
> >  };
> >
> > -static const struct renesas_sdhi_of_data_with_quirks of_r9a09g011_compatible = {
> > +static const struct renesas_sdhi_of_data_with_quirks of_rzg2l_compatible = {
> >         .of_data = &of_data_rcar_gen3,
> > -       .quirks = &sdhi_quirks_r9a09g011,
> > +       .quirks = &sdhi_quirks_rzg2l,
> >  };
> >
> >  static const struct renesas_sdhi_of_data_with_quirks of_rcar_gen3_compatible = {
> > @@ -283,7 +283,7 @@ static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
> >         { .compatible = "renesas,sdhi-r8a77970", .data = &of_r8a77970_compatible, },
> >         { .compatible = "renesas,sdhi-r8a77990", .data = &of_r8a77990_compatible, },
> >         { .compatible = "renesas,sdhi-r8a77995", .data = &of_rcar_gen3_nohs400_compatible, },
> > -       { .compatible = "renesas,sdhi-r9a09g011", .data = &of_r9a09g011_compatible, },
>
> Hence I think the above line should be kept, but changed to point
> to &of_rzg2l_compatible.
>
OK, will do.

Cheers,
Prabhakar