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

[wsa+renesas]

Hi Biju,

> > > 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.

I was more worried about the incorrect DT binding than the memory
corruption.

> > 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

Well... your statements make sense with the current state of the driver.
However, there is de-facto a new requirement added that
TMIO_MMC_TUNING_DELAY needs an SCC. This is currently handled correctly,
but since it is not addressed explicitly, I would consider it a little
fragile. SDHI is complex and I am worried that some future logic changes
might break the logic chain you quoted above. This is why I think an
explicit check makes sense - it cannot break and it makes logic more
obvious for future hackers.

Can you agree?

Happy hacking,

   Wolfram

Attachment: signature.asc
Description: PGP signature