Re: [PATCH v23 05/14] mmc: renesas_sdhi: Make clock divider mask configurable

[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 a clk_div_mask field to renesas_sdhi_of_data and tmio_mmc_data
> so each SoC can specify its own divider mask instead of the
> hardcoded CLK_CTL_DIV_MASK used in renesas_sdhi_set_clock().
>
> Default to GENMASK(7, 0) at probe time when unset, and populate all
> existing internal/sys DMAC of_data tables with this value to keep
> current SoCs unaffected. This lets RZ/G3L, which has a wider
> divider field, reuse the same clock-setting code.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
>  * Assigned of_data->clk_div_mask in renesas_sdhi_probe().
> v21->v22:
>  * Updated commit description.
>  * Added clk_div_mask to of_default_cfg and of_rcar_gen2_compatible.

Thanks for the update!

> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -41,6 +41,7 @@ struct renesas_sdhi_of_data {
>         unsigned long sdhi_flags;
>         u64 clk_mask;
>         int max_divider;
> +       u16 clk_div_mask;
>  };
>
>  #define SDHI_CALIB_TABLE_MAX 32
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index d893a263e770..5dcbb431e1e1 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -224,7 +224,7 @@ static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
>                         clk &= ~0xff;
>         }
>
> -       clock = clk & CLK_CTL_DIV_MASK;
> +       clock = clk & host->pdata->clk_div_mask;
>         if (clock != CLK_CTL_DIV_MASK)

This strikes me as a bit odd; should CLK_CTL_DIV_MASK be changed to
host->pdata->clk_div_mask in the check, too?

>                 host->mmc->actual_clock /= (1 << (ffs(clock) + 1));
>
> @@ -1138,6 +1138,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>                 mmc_data->max_segs = of_data->max_segs;
>                 mmc_data->clk_mask = of_data->clk_mask;
>                 mmc_data->max_divider = of_data->max_divider;
> +               mmc_data->clk_div_mask = of_data->clk_div_mask;
>                 dma_priv->dma_buswidth = of_data->dma_buswidth;
>                 host->bus_shift = of_data->bus_shift;
>                 /* Fallback for old DTs */
> @@ -1186,6 +1187,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>         if (!mmc_data->max_divider)
>                 mmc_data->max_divider = SDHI_MAX_DIVIDER_DEFAULT;
>
> +       if (!mmc_data->clk_div_mask)
> +               mmc_data->clk_div_mask = GENMASK(7, 0);

CLK_CTL_DIV_MASK, perhaps?

> +
>         dma_priv->filter = shdma_chan_filter;
>         dma_priv->enable = renesas_sdhi_enable_dma;
>

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