Re: gpio / ACPI: Add support for ACPI GPIO operation regions

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Mon, Mar 31, 2014 at 04:45:57PM +0300, Mika Westerberg wrote:
> On Mon, Mar 31, 2014 at 03:11:33PM +0300, Dan Carpenter wrote:
> > On Mon, Mar 31, 2014 at 03:05:25PM +0300, Mika Westerberg wrote:
> > > On Fri, Mar 28, 2014 at 11:37:32AM +0300, Dan Carpenter wrote:
> > > > Hello Mika Westerberg,
> > > > 
> > > > The patch 473ed7be0da0: "gpio / ACPI: Add support for ACPI GPIO
> > > > operation regions" from Mar 14, 2014, leads to the following static
> > > > checker warning:
> > > > 
> > > > 	drivers/gpio/gpiolib-acpi.c:454 acpi_gpio_adr_space_handler()
> > > > 	warn: should 'gpiod_get_raw_value(desc) << i' be a 64 bit type?
> > > 
> > > Thanks for the report. However, I'm not able to reproduce this warning with
> > > sparse. How did you get this?
> > 
> > It's not a Sparse warning.  It's some unreleased stuff (too many false
> > positives).  I sort through the warnings manually and send the ones
> > which seem valid.
> 
> I see.
> 
> What do you think about the patch below? I have to admit that this kind of
> stuff is in my "gray" area of understanding.

It looks good to me.  The question, I guess is can pin_table_length ever
be more than 31.  gpiod_get_raw_value() returns and int of 0-1 so
if "i" is 31 then *value is an unexpected number because the shift is
undefined (it wraps around in GCC).

If it can't go higher than 31 then the existing code is fine.  (We never
use the upper 32 bits of *value).  But your patch silences a valid
looking static checker warning.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html