Re: [PATCH v24 10/15] mmc: renesas_sdhi: Add write32_hook for CTL_SD_CARD_CLK_CTL register writes

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1927 bytes --]

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] tmio_mmc_host_probe() clears write16_hook when TMIO_MMC_HAS_IDLE_WAIT is not set, but fails to clear the newly added write32_hook.
> --
> 
> --- Patch [10]: [PATCH v24 10/15] mmc: renesas_sdhi: Add write32_hook for CTL_SD_CARD_CLK_CTL register writes ---
> commit 57d55ff6257b421b4e27c4306c5ee441c02c3087
> Author: Biju Das <biju.das.jz@bp.renesas.com>
> 
> mmc: renesas_sdhi: Add write32_hook for CTL_SD_CARD_CLK_CTL register writes
> 
> This patch introduces a write32_hook for the Renesas SDHI driver to wait
> for the SD bus to become idle before 32-bit writes to the
> CTL_SD_CARD_CLK_CTL register. Writes are skipped if the hook returns an
> error.
> 
> > 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
> 
> [ ... ]
> 
> > @@ -1203,6 +1211,7 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> >  	}
> >  
> >  	host->write16_hook = renesas_sdhi_write16_hook;
> > +	host->write32_hook = renesas_sdhi_write32_hook;
> 
> [Severity: High]
> Should write32_hook also be conditionally cleared in tmio_mmc_host_probe()
> when TMIO_MMC_HAS_IDLE_WAIT is missing?
> 
> Looking at drivers/mmc/host/tmio_mmc_core.c:tmio_mmc_host_probe(),
> write16_hook is actively cleared for devices without this flag:
> 
>         if (!(pdata->flags & TMIO_MMC_HAS_IDLE_WAIT))
>                 _host->write16_hook = NULL;
> 
> If write32_hook remains set on older or fallback configurations lacking this
> flag, polling for TMIO_STAT_SCLKDIVEN will time out after 1ms. 
> 
> The hook would then return -EBUSY, causing sd_ctrl_write32() to skip the
> register write entirely, which ultimately fails SD controller initialization.

Please check this, too.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]