RE: [PATCH v24 07/15] mmc: renesas_sdhi: Add tuning delay support for RZ/G2L

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> Sent: 03 September 2026 10:30
> Subject: Re: [PATCH v24 07/15] mmc: renesas_sdhi: Add tuning delay support for RZ/G2L
> 
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] The new function renesas_sdhi_set_hw_adjustment_delay() accesses SCC registers via
> sd_scc_write32() without checking if priv->scc_ctl is initialized.
> > --
> >
> > --- Patch [7]: [PATCH v24 07/15] mmc: renesas_sdhi: Add tuning delay
> > support for RZ/G2L --- commit 4e31e96fb980d0bb6400d3a60dfcc29dca208dcc
> > Author: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > mmc: renesas_sdhi: Add tuning delay support for RZ/G2L
> >
> > This commit adds hardware adjustment delay support for RZ/G2L SoCs
> > during signal-voltage switch. It introduces the TMIO_MMC_TUNING_DELAY
> > flag and applies a delay by writing to SCC_TMPPORT2 based on the regulator voltage.
> >
> > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c
> > > b/drivers/mmc/host/renesas_sdhi_core.c
> > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > [ ... ]
> > > @@ -350,6 +316,55 @@ 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_hw_adjustment_delay(struct
> > > +tmio_mmc_host *host) {
> > > +	struct renesas_sdhi *priv = host_to_priv(host);
> > > +
> > > +	if (!(host->pdata->flags & TMIO_MMC_TUNING_DELAY))
> > > +		return;

This check is available only for RZ/G2L+ SoCs.

> > > +
> > > +	if (host->mmc->ios.signal_voltage == MMC_SIGNAL_VOLTAGE_330)
> > > +		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2, 0x0);
> > > +	else
> > > +		sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_TMPPORT2, 0x1);
> >
> > [Severity: High]
> > Is it possible for priv->scc_ctl to be NULL here?

This issue can happen only if there is a severe memory corruption in the system during boot.

> >
> > If an incorrect device tree binding or a hardware fault causes the
> > controller to report a version older than SDHI_VER_GEN2_SDR104,
> > priv->scc_ctl might not be mapped during probe.

I agree if there is a memory corruption, any check can lead to
Unpredictable result.

> >
> > However, renesas_sdhi_start_signal_voltage_switch() is registered
> > based on TMIO_MMC_MIN_RCAR2 and calls this new function, which writes
> > directly to the SCC register via sd_scc_write32().
> >
> > Could this cause a NULL pointer dereference during card insertion?

This can happen only if there is a memory corruption, in that case the below check
passes for all the SoCs.

if (!(host->pdata->flags & TMIO_MMC_TUNING_DELAY))
		return;

> 
> I agree. We should ensure in probe() that either
> 
> a) TMIO_MMC_TUNING_DELAY has a valid SCC or bail out otherwise
> 
> or
> 
> b) clear the TMIO_MMC_TUNING_DELAY flag if there is no SCC present
> 
> What do you think?

I agree, we can add defensive check in probe().

TBH, I am not convinced with Sashiko report here, but please recheck my comments
and provide feedback

Or

I may be completely wrong, please correct me in that case.


Cheers,
Biju