Re: [PATCH] mmc: sdhci-cadence: Fix -Wuninitialized in sdhci_cdns_tune_blkgap()

[Nathan Chancellor <nathan@kernel.org>]

On Wed, Aug 20, 2025 at 01:43:09PM +0300, Dan Carpenter wrote:
> On Wed, Aug 20, 2025 at 10:07:35AM +0200, Benoît Monin wrote:
> > > --- a/drivers/mmc/host/sdhci-cadence.c
> > > +++ b/drivers/mmc/host/sdhci-cadence.c
> > > @@ -288,13 +288,12 @@ static int sdhci_cdns_tune_blkgap(struct mmc_host *mmc)
> > >  	void __iomem *hrs38_reg = priv->hrs_addr + SDHCI_CDNS_HRS38;
> > >  	int ret;
> > >  	u32 gap;
> > > -	u32 hrs37_mode;
> > >  
> > >  	/* Currently only needed in HS200 mode */
> > >  	if (host->timing != MMC_TIMING_MMC_HS200)
> > >  		return 0;
> > >  
> > > -	writel(hrs37_mode, hrs37_reg);
> > > +	writel(SDHCI_CDNS_HRS37_MODE_MMC_HS200, hrs37_reg);
> > >  
> > >  	for (gap = 0; gap <= SDHCI_CDNS_HRS38_BLKGAP_MAX; gap++) {
> > >  		writel(gap, hrs38_reg);
> > > 
> > Thanks for the catch!
> > 
> > I don't get how gcc does not raise a warning here, only clang does. I
> > did build with gcc-11 and gcc-15 and they don't complain about this
> > uninitialized variable.
> > 
> 
> We disabled uninitialized variable checking on GCC.  It was too crazy
> and especially if we want to use -Werror.
> 
> Smatch also detects this bug.

Technically, -Wuninitialized is on for GCC as well but as soon as there
is control flow between the declaration of the variable and its usage,
it gets changed into -Wmaybe-uninitialized :/

In file included from arch/arm64/include/asm/io.h:288,
                 from include/linux/io.h:12,
                 from include/linux/iopoll.h:14,
                 from drivers/mmc/host/sdhci-cadence.c:9:
In function 'writel',
    inlined from 'sdhci_cdns_tune_blkgap' at drivers/mmc/host/sdhci-cadence.c:297:2,
    inlined from 'sdhci_cdns_execute_tuning' at drivers/mmc/host/sdhci-cadence.c:352:9:
include/asm-generic/io.h:276:9: error: 'hrs37_mode' may be used uninitialized [-Werror=maybe-uninitialized]
  276 |         log_write_mmio(value, 32, addr, _THIS_IP_, _RET_IP_);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/mmc/host/sdhci-cadence.c: In function 'sdhci_cdns_execute_tuning':
drivers/mmc/host/sdhci-cadence.c:291:13: note: 'hrs37_mode' was declared here
  291 |         u32 hrs37_mode;
      |             ^~~~~~~~~~

vs

diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index a2a4a5b0ab96..b968c0fb925d 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -290,12 +290,12 @@ static int sdhci_cdns_tune_blkgap(struct mmc_host *mmc)
        u32 gap;
        u32 hrs37_mode;

+       writel(hrs37_mode, hrs37_reg);
+
        /* Currently only needed in HS200 mode */
        if (host->timing != MMC_TIMING_MMC_HS200)
                return 0;

-       writel(hrs37_mode, hrs37_reg);
-
        for (gap = 0; gap <= SDHCI_CDNS_HRS38_BLKGAP_MAX; gap++) {
                writel(gap, hrs38_reg);
                ret = mmc_read_tuning(mmc, 512, 32);

In file included from arch/arm64/include/asm/io.h:288,
                 from include/linux/io.h:12,
                 from include/linux/iopoll.h:14,
                 from drivers/mmc/host/sdhci-cadence.c:9:
drivers/mmc/host/sdhci-cadence.c: In function 'sdhci_cdns_tune_blkgap':
include/asm-generic/io.h:273:16: error: 'hrs37_mode' is used uninitialized [-Werror=uninitialized]
  273 | #define writel writel
drivers/mmc/host/sdhci-cadence.c:293:9: note: in expansion of macro 'writel'
  293 |         writel(hrs37_mode, hrs37_reg);
      |         ^~~~~~
drivers/mmc/host/sdhci-cadence.c:291:13: note: 'hrs37_mode' was declared here
  291 |         u32 hrs37_mode;
      |             ^~~~~~~~~~

It would be nice if GCC could adopt clang's semantics of "uninitialized
when used _here_" for -Wuninitalized versus the current scheme of
converting it to a "may be used uninitialized" warning.

Cheers,
Nathan