Re: [PATCH] spi: cadence-quadspi: Silence shiftTooManyBitsSigned warning

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Fri, Mar 05, 2021 at 01:04:27AM +0530, Pratyush Yadav wrote:
> On 04/03/21 04:08PM, Dan Carpenter wrote:
> > On Thu, Mar 04, 2021 at 06:47:52PM +0800, Jay Fang wrote:
> > > drivers/spi/spi-cadence-quadspi.c:267:18: warning: Shifting signed 32-bit
> > > value by 31 bits is undefined behaviour [shiftTooManyBitsSigned]
> > >     return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
> > >                     ^
> > > 
> > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > > Signed-off-by: Jay Fang <f.fangjian@xxxxxxxxxx>
> > > ---
> > >  drivers/spi/spi-cadence-quadspi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> > > index 442cc7c..9a2798a5 100644
> > > --- a/drivers/spi/spi-cadence-quadspi.c
> > > +++ b/drivers/spi/spi-cadence-quadspi.c
> > > @@ -264,7 +264,7 @@ static bool cqspi_is_idle(struct cqspi_st *cqspi)
> > >  {
> > >  	u32 reg = readl(cqspi->iobase + CQSPI_REG_CONFIG);
> > >  
> > > -	return reg & (1 << CQSPI_REG_CONFIG_IDLE_LSB);
> > > +	return reg & (1UL << CQSPI_REG_CONFIG_IDLE_LSB);
> > 
> > This is always going to be false because reg is a u32.
> 
> Hmm... I don't see why it would always be false. reg would promoted to 
> unsigned long and the result should then depend on the actual value of 
> the bit, which can be represented by an unsigned long. There is no loss 
> of information.
> 
> Anyway, it still makes more sense to make it 1U because reg is u32. Just 
> keep the types same and avoid all the conversion rules.

Ah, crap.  I'm sorry.  I somehow thought when I forwarded this bug the
other day that CQSPI_REG_CONFIG_IDLE_LSB was more than 31.

regards,
dan carpenter