RE: [PATCH v24 04/15] mmc: renesas_sdhi: Add max_divider field to support SoC-specific clock ranges

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Sent: 02 September 2026 12:06
> Subject: Re: [PATCH v24 04/15] mmc: renesas_sdhi: Add max_divider field to support SoC-specific clock
> ranges
> 
> 
> > v22->v23:
> >  * Changed the data type of max_divider from unsigned int to signed int.
> 
> You changed this because of a Sashiko report. While the report was correct, I think the solution of
> simply using 'int' is suboptimal...

Yes that is correct, based on Sashiko report I changed the data type.

> 
> > -	mmc->f_min = max(clk_round_rate(priv->clk, 1) / 512, 1L);
> > +	mmc->f_min = max(clk_round_rate(priv->clk, 1) /
> > +host->pdata->max_divider, 1L);
> 
> ... because the existing code is just too subtle / fragile and hard to understand. I think we should move
> clk_round_rate() out of max() and handle it seperatly before. What do you think?

You mean use, max(clk_get_rate(priv->clk) / 512, 1);

clk_get_rate() returns unsigned long compared to clk_round_rate() which return long??

> 
> >  	 * greater than, new_clock.  As we can divide by 1 << i for
> > -	 * any i in [0, 9] we want the input clock to be as close as
> > +	 * any i in [0, {9,11}] we want the input clock to be as close as
> 
> Maybe instead of "{9,11}" use "ilog2(max_divider)" or something? This matches the code and is more
> future-proof if the divider gets extended again.
OK.

> 
> > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > index c6db0418de15..2bf354331b2d 100644
> > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > @@ -102,6 +102,7 @@ static const struct renesas_sdhi_of_data of_data_rza2 = {
> >  	.max_blk_count	= UINT_MAX / TMIO_MAX_BLK_SIZE,
> >  	.max_segs	= 1,
> >  	.clk_mask	= SDHI_CLK_MASK_DEFAULT,
> > +	.max_divider	= SDHI_MAX_DIVIDER_DEFAULT,
> 
> Same comment as for clk_mask, I would not initialize to the default value.

Agreed will remove.

> 
> > @@ -62,5 +62,6 @@ struct tmio_mmc_data {
> >  	unsigned int			max_blk_count;
> >  	unsigned short			max_segs;
> >  	u64				clk_mask;
> > +	int				max_divider;
> 
> I hope that moving clk_round_rate() out of max() will allow 'unsigned int' here again.

By using clk_get_rate() I can still use unsigned int here.

Or

if you prefer to remove max() then the option is to use clk_round_rate(),
assign the value to 1 if It is 0 or negative.

Please let me know how to proceed.

Cheers,
Biju