RE: [PATCH v23 13/14] mmc: renesas_sdhi: Add RZ/G3L HS400 support

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <[email protected]>
> Sent: 07 August 2026 14:25
> Subject: Re: [PATCH v23 13/14] mmc: renesas_sdhi: Add RZ/G3L HS400 support
> 
> Hi Biju,
> 
> On Thu, 30 Jul 2026 at 13:32, Biju <[email protected]> wrote:
> > From: Biju Das <[email protected]>
> >
> > Add HS400 support for the RZ/G3L SoC.
> >
> > RZ/G3L needs a second HS400 enable register, SDm_SCC_HS400MODE2
> > (offset 0x020), alongside the existing TMPPORT2 path. Add a
> > TMIO_MMC_HS400MODE2 flag (bit 16) for controllers that need it.
> >
> > renesas_sdhi_hs400_complete() now also sets HS400EN2 in HS400MODE2
> > when the flag is set, and renesas_sdhi_reset_hs400_mode() clears it on
> > exit. Add renesas_sdhi_set_tmpport() to mask off only the lower
> > 16 bits (TMPOUT) of TMPPORT2 when writing it, preserving the upper
> > bits on controllers with HS400MODE2, instead of the previous
> > unconditional zero write.
> >
> > RZ/G3L HS400 supports only one divider value, so
> > renesas_sdhi_set_clock() now forces the clock value to 0 before
> > masking when HS400MODE2 is set and the internal divider isn't
> > currently enabled, rejecting any other divider. This also completes
> > renesas_sdhi_is_internal_divider_enabled() from earlier in the series,
> > which now additionally checks that the current timing mode is not
> > MMC_TIMING_MMC_HS400 before reporting the internal divider as enabled.
> >
> > Enable TMIO_MMC_HS400MODE2 in of_data_rzg3l to complete RZ/G3L HS400
> > support.
> >
> > Signed-off-by: Biju Das <[email protected]>
> > ---
> > v22->v23:
> >  * No change.
> > v21->v22:
> >  * Updated commit description
> >  * Added a renesas_sdhi_set_tmpport() helper to set delay values.
> >  * Added a check in renesas_sdhi_is_internal_divider_enabled() to
> >    return disabled status when in HS400 mode.
> 
> Thanks for the update!
> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> 
> > @@ -355,6 +366,22 @@ static inline void sd_scc_write32(struct tmio_mmc_host *host,
> >         writel(val, priv->scc_ctl + (addr << host->bus_shift));  }
> >
> > +static void renesas_sdhi_set_tmpport(struct tmio_mmc_host *host, u32
> > +tmpport) {
> > +       struct renesas_sdhi *priv = host_to_priv(host);
> > +       u32 val;
> 
> u32 val = tmpport;

OK.
> 
> > +
> > +       if (host->pdata->flags & TMIO_MMC_HS400MODE2) {
> > +               val = sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2);
> > +               val &= ~GENMASK(15, 0); /* TMPOUT MASK */
> 
> Add a #define for this magic value?

Will introduce the macro SH_MOBILE_SDHI_SCC_TMPPORT2_TMPOUT

> 
> val |= sd_scc_read32(...) & ...;

OK.

> 
> > +               val |= tmpport;
> > +       } else {
> > +               val = tmpport;
> > +       }
> 
> And then you can drop these...

OK. Will fix this in next version.

Cheers,
Biju