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

[Geert Uytterhoeven]

Hi Biju,

On Thu, 30 Jul 2026 at 13:32, Biju <biju.das.au@xxxxxxxxx> wrote:
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> 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 <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> 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;

> +
> +       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?

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

> +               val |= tmpport;
> +       } else {
> +               val = tmpport;
> +       }

And then you can drop these...

> +
> +       sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2, val);
> +}
> +
>  static void renesas_sdhi_set_hw_adjustment_delay(struct tmio_mmc_host *host)
>  {
>         struct renesas_sdhi *priv = host_to_priv(host);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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