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

[Pratyush Yadav <p.yadav@xxxxxx>]

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.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.