| Message ID | 1614854872-8694-1-git-send-email-f.fangjian@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 31890269c0a031e704f995bbd39e1fd77a381207 |
| Headers | show |
| Series | spi: cadence-quadspi: Silence shiftTooManyBitsSigned warning | expand |
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@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Jay Fang <f.fangjian@huawei.com> > --- > 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. regards, dan carpenter
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@intel.com> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Jay Fang <f.fangjian@huawei.com> > > --- > > 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.
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@intel.com> > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > > Signed-off-by: Jay Fang <f.fangjian@huawei.com> > > > --- > > > 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
On Thu, 4 Mar 2021 18:47:52 +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); > ^ Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: cadence-quadspi: Silence shiftTooManyBitsSigned warning commit: 55794b1d8623f73d9a4bf12e4343bc8fc96024e1 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
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); } static u32 cqspi_get_rd_sram_level(struct cqspi_st *cqspi)
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@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Jay Fang <f.fangjian@huawei.com> --- drivers/spi/spi-cadence-quadspi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)