RE: [PATCH v24 08/15] mmc: renesas_sdhi: Add TMIO_MMC_INTERNAL_DIVIDER flag

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: wsa+renesas <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Sent: 22 October 2026 10:31
> Subject: Re: [PATCH v24 08/15] mmc: renesas_sdhi: Add TMIO_MMC_INTERNAL_DIVIDER flag
> 
> Hi Biju,
> 
> > > static unsigned int renesas_sdhi_clk_get_rate(struct tmio_mmc_host *host,
> > > 					      struct clk *clk)
> > > {
> > > 	unsigned int freq = clk_get_rate(clk);
> > >
> > > 	return host->pdata->flags & TMIO_MMC_INTERNAL_DIVIDER ? freq / 2 :
> > > freq; }
> > >
> > > Was this AI generated?
> >
> > I kept 2 function as renesas_sdhi_is_internal_divider_enabled() is
> > reused in renesas_sdhi_set_clock()while adding support for HS400 mode.
> >
> > Currently, we decided to focus only for basic RZ/G3L support. So I can
> > merge it into Single function as you suggested.
> 
> Please do. And if the flag is checked later again, I think we can just check the flag. We don't need a
> helper for checking each flag, or?

OK.

> 
> >
> > >
> > > > @@ -184,10 +205,14 @@ static unsigned int
> > > > renesas_sdhi_clk_update(struct tmio_mmc_host *host,
> > > >
> > > >  	clk_set_rate(ref_clk, best_freq);
> > > >
> > > > -	if (priv->clkh)
> > > > +	if (priv->clkh) {
> > > > +		if (host->pdata->flags & TMIO_MMC_INTERNAL_DIVIDER)
> > > > +			clkh_shift = 1;
> > >
> > > No, this needs to be handled a bit above where clkh_shift is
> > > initially set up, so we have it all in one block.
> >
> > There is a difference here.
> >
> > SDx_Clock (HS clk) Figure 4.4-8 Block Diagram of the Deformed Clock
> > System (7) IMCLK = SDx_Clock/2
> >
> > SD_Clock  Figure 6.2-2 Functional Diagram of Clock Divider for SD_CLK
> >
> > Currently framework request a clk to set, we set HS clk first,
> > followed by SD_CLK.
> >
> > On RZ/G3L, SD_CLK value is based on internal divider.
> >
> > For HS400 mode, SD_CLK = SDx_Clock(HS clk)/2
> >
> > For Non-HS400 mode, SD_CLK = SDx_Clock(HS clk)/4
> >
> > I don't find an easy way to move this logic to the above.
> 
> I get an idea what is the problem here, but I am still confused about one thing. I'll summarize what I
> understood:
> 
> * as of this patch, HS400 is not supported on G3L
> * so, in renesas_sdhi_clk_update(), 'clk_shift' value is 2 before clock
>   calculation
> * as HS400 is not supported yet, the internal divider is always active
> * so, you need to decrease the 'clk_shift' value from 2 to 1 after clock
>   calculation
> 
> So far, so good. My expectation is now that when you add HS400 mode, you would need some extra code
> keeping 'clk_shift' at 2 because the internal divider is disabled then. But I don't find such code in
> later patches.
> 
> What am I missing?

See patch[1] has relevant changes.

<snippet>
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 23332539ac65..5d9d3c7dedf9 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -135,7 +135,8 @@  static bool renesas_sdhi_is_internal_divider_enabled(struct tmio_mmc_host *host)
 {
 	bool enable = false;
 
-	if (host->pdata->flags & TMIO_MMC_INTERNAL_DIVIDER)
+	if ((host->pdata->flags & TMIO_MMC_INTERNAL_DIVIDER) &&
+	    host->mmc->ios.timing != MMC_TIMING_MMC_HS400)
 		enable = true;

"This will make sure /2 for HS400 and /4 for non-HS400"
 
 	return enable;
@@ -255,6 +256,14 @@  static void renesas_sdhi_set_clock(struct tmio_mmc_host *host,
 			clk &= ~SDHI_SD_CLK_CTL_DIV1;
 	}
 
+	/*
+	 * RZ/G3L SoC HS400 mode has only 1 divider value.
+	 * Other divider values are prohibited
+	 */
+	if ((host->pdata->flags & TMIO_MMC_HS400MODE2) &&
+	    !renesas_sdhi_is_internal_divider_enabled(host))
+		clk = 0;
+
</snippet>

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20260820171657.62815-15-biju.das.jz@xxxxxxxxxxxxxx/

Cheers,
Biju