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

[Biju Das]

Hi Geert/Wolfram,

> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 08 August 2026 16:21
> Subject: RE: [PATCH v23 05/14] mmc: renesas_sdhi: Make clock divider mask configurable
> 
> Hi Geert,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Sent: 07 August 2026 14:16
> > Subject: Re: [PATCH v23 05/14] mmc: renesas_sdhi: Make clock divider
> > mask configurable
> >
> > 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?
> 
> No, this is the value for 1:1 option.
> 
>     [    2.648050] ###########clk/clock 4000ff/ff
> 
> if we do, (clock != host->pdata->clk_div_mask) then the check will always true for RZ/G3L(GENMASK(9,0))
> and it never set 1:1 divider value.
> 
> The 1:1 divider values:
> 
> Non-RZ/G3L, it is 1111 1111b
> For RZ/G3L, it is 00 1111 1111b

I will introduce a macro for the 1:1 divider case 
RENESAS_SD_CLK_CTL_DIV1  (0xff), so that, it is separated from
CLK_CTL_DIV_MASK(Non RZ/G3L) and RZG3L_ CLK_CTL_DIV_MASK(RZ/G3L)

Cheers,
Biju

> 
> >
> > >                 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?
> 
> Agreed.
> 
> Cheers,
> Biju