All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
@ 2016-05-31 19:03 Andy Shevchenko
  2016-06-01  4:44 ` Yong Li
  2016-06-08  8:03 ` Linus Walleij
  0 siblings, 2 replies; 8+ messages in thread
From: Andy Shevchenko @ 2016-05-31 19:03 UTC (permalink / raw)
  To: linux-gpio, Linus Walleij; +Cc: Andy Shevchenko, Yong Li, Phil Reid

The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value for register
word write") fixed regression in pca953x_write_regs(). At the same time the
solution introduced a sparse warning:

drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in argument 3 (different base types)
drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short [unsigned] [usertype] value
drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16 [usertype] <noident>

Fix the code by enforcing the type of i2c_smbus_write_word_data() parameter.

Cc: Yong Li <sdliyong@gmail.com>
Cc: Phil Reid <preid@electromag.com.au>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-pca953x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d8233be..21b21cd 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -163,10 +163,13 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
 					NBANK(chip), val);
 	} else {
 		switch (chip->chip_type) {
-		case PCA953X_TYPE:
-			ret = i2c_smbus_write_word_data(chip->client,
-			    reg << 1, cpu_to_le16(get_unaligned((u16 *)val)));
+		case PCA953X_TYPE: {
+			__le16 word = cpu_to_le16(get_unaligned((u16 *)val));
+
+			ret = i2c_smbus_write_word_data(chip->client, reg << 1,
+							(__force u16)word);
 			break;
+		}
 		case PCA957X_TYPE:
 			ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
 							val[0]);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-05-31 19:03 [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data() Andy Shevchenko
@ 2016-06-01  4:44 ` Yong Li
  2016-06-01 12:35   ` Andy Shevchenko
  2016-06-08  8:03 ` Linus Walleij
  1 sibling, 1 reply; 8+ messages in thread
From: Yong Li @ 2016-06-01  4:44 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij, Phil Reid

Hi Andy,

Could you let me know which build system you are using? I am using
Ubuntu 1604(64 bit) to build the kernel source, but I did not find the
warning.

Thanks,
Yong

2016-06-01 3:03 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value for register
> word write") fixed regression in pca953x_write_regs(). At the same time the
> solution introduced a sparse warning:
>
> drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in argument 3 (different base types)
> drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short [unsigned] [usertype] value
> drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16 [usertype] <noident>
>
> Fix the code by enforcing the type of i2c_smbus_write_word_data() parameter.
>
> Cc: Yong Li <sdliyong@gmail.com>
> Cc: Phil Reid <preid@electromag.com.au>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpio-pca953x.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index d8233be..21b21cd 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -163,10 +163,13 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val)
>                                         NBANK(chip), val);
>         } else {
>                 switch (chip->chip_type) {
> -               case PCA953X_TYPE:
> -                       ret = i2c_smbus_write_word_data(chip->client,
> -                           reg << 1, cpu_to_le16(get_unaligned((u16 *)val)));
> +               case PCA953X_TYPE: {
> +                       __le16 word = cpu_to_le16(get_unaligned((u16 *)val));
> +
> +                       ret = i2c_smbus_write_word_data(chip->client, reg << 1,
> +                                                       (__force u16)word);
>                         break;
> +               }
>                 case PCA957X_TYPE:
>                         ret = i2c_smbus_write_byte_data(chip->client, reg << 1,
>                                                         val[0]);
> --
> 2.8.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-06-01  4:44 ` Yong Li
@ 2016-06-01 12:35   ` Andy Shevchenko
  2016-06-01 15:19     ` Yong Li
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2016-06-01 12:35 UTC (permalink / raw)
  To: Yong Li; +Cc: linux-gpio, Linus Walleij, Phil Reid

On Wed, 2016-06-01 at 12:44 +0800, Yong Li wrote:
> Hi Andy,
> 
> Could you let me know which build system you are using? I am using
> Ubuntu 1604(64 bit) to build the kernel source, but I did not find the
> warning.

Hmm... I'm confused what exactly you are asking here. My host system is
Debian unstable, kernel is built by it's own KBuild infrastructure.

I'm pretty sure you are talking about this:

https://lwn.net/Articles/205624/

> 
> Thanks,
> Yong
> 
> 2016-06-01 3:03 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.int
> el.com>:
> > The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value for
> > register
> > word write") fixed regression in pca953x_write_regs(). At the same
> > time the
> > solution introduced a sparse warning:
> > 
> > drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in
> > argument 3 (different base types)
> > drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short
> > [unsigned] [usertype] value
> > drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16
> > [usertype] <noident>
> > 
> > Fix the code by enforcing the type of i2c_smbus_write_word_data()
> > parameter.
> > 
> > Cc: Yong Li <sdliyong@gmail.com>
> > Cc: Phil Reid <preid@electromag.com.au>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/gpio/gpio-pca953x.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-
> > pca953x.c
> > index d8233be..21b21cd 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -163,10 +163,13 @@ static int pca953x_write_regs(struct
> > pca953x_chip *chip, int reg, u8 *val)
> >                                         NBANK(chip), val);
> >         } else {
> >                 switch (chip->chip_type) {
> > -               case PCA953X_TYPE:
> > -                       ret = i2c_smbus_write_word_data(chip-
> > >client,
> > -                           reg << 1, cpu_to_le16(get_unaligned((u16
> > *)val)));
> > +               case PCA953X_TYPE: {
> > +                       __le16 word = cpu_to_le16(get_unaligned((u16
> > *)val));
> > +
> > +                       ret = i2c_smbus_write_word_data(chip-
> > >client, reg << 1,
> > +                                                       (__force
> > u16)word);
> >                         break;
> > +               }
> >                 case PCA957X_TYPE:
> >                         ret = i2c_smbus_write_byte_data(chip-
> > >client, reg << 1,
> >                                                         val[0]);
> > --
> > 2.8.1
> > 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-06-01 12:35   ` Andy Shevchenko
@ 2016-06-01 15:19     ` Yong Li
  2016-06-02 14:43       ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Yong Li @ 2016-06-01 15:19 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Linus Walleij, Phil Reid

Hi andy,

Sorry for the confusing. I tested the patch today, using Ubuntu 1604 +
gcc 5.3, the make bzImage does not output any warning messages about
this .c file. I am wondering how you captured the warnings. It seems
that I need the "make C=2" command.

Thanks,
Yong

2016-06-01 20:35 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Wed, 2016-06-01 at 12:44 +0800, Yong Li wrote:
>> Hi Andy,
>>
>> Could you let me know which build system you are using? I am using
>> Ubuntu 1604(64 bit) to build the kernel source, but I did not find the
>> warning.
>
> Hmm... I'm confused what exactly you are asking here. My host system is
> Debian unstable, kernel is built by it's own KBuild infrastructure.
>
> I'm pretty sure you are talking about this:
>
> https://lwn.net/Articles/205624/
>
>>
>> Thanks,
>> Yong
>>
>> 2016-06-01 3:03 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.int
>> el.com>:
>> > The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value for
>> > register
>> > word write") fixed regression in pca953x_write_regs(). At the same
>> > time the
>> > solution introduced a sparse warning:
>> >
>> > drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in
>> > argument 3 (different base types)
>> > drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short
>> > [unsigned] [usertype] value
>> > drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16
>> > [usertype] <noident>
>> >
>> > Fix the code by enforcing the type of i2c_smbus_write_word_data()
>> > parameter.
>> >
>> > Cc: Yong Li <sdliyong@gmail.com>
>> > Cc: Phil Reid <preid@electromag.com.au>
>> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > ---
>> >  drivers/gpio/gpio-pca953x.c | 9 ++++++---
>> >  1 file changed, 6 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-
>> > pca953x.c
>> > index d8233be..21b21cd 100644
>> > --- a/drivers/gpio/gpio-pca953x.c
>> > +++ b/drivers/gpio/gpio-pca953x.c
>> > @@ -163,10 +163,13 @@ static int pca953x_write_regs(struct
>> > pca953x_chip *chip, int reg, u8 *val)
>> >                                         NBANK(chip), val);
>> >         } else {
>> >                 switch (chip->chip_type) {
>> > -               case PCA953X_TYPE:
>> > -                       ret = i2c_smbus_write_word_data(chip-
>> > >client,
>> > -                           reg << 1, cpu_to_le16(get_unaligned((u16
>> > *)val)));
>> > +               case PCA953X_TYPE: {
>> > +                       __le16 word = cpu_to_le16(get_unaligned((u16
>> > *)val));
>> > +
>> > +                       ret = i2c_smbus_write_word_data(chip-
>> > >client, reg << 1,
>> > +                                                       (__force
>> > u16)word);
>> >                         break;
>> > +               }
>> >                 case PCA957X_TYPE:
>> >                         ret = i2c_smbus_write_byte_data(chip-
>> > >client, reg << 1,
>> >                                                         val[0]);
>> > --
>> > 2.8.1
>> >
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-06-01 15:19     ` Yong Li
@ 2016-06-02 14:43       ` Andy Shevchenko
  2016-06-08  8:03         ` Linus Walleij
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2016-06-02 14:43 UTC (permalink / raw)
  To: Yong Li; +Cc: linux-gpio, Linus Walleij, Phil Reid

On Wed, 2016-06-01 at 23:19 +0800, Yong Li wrote:
> Hi andy,
> 
> Sorry for the confusing. I tested the patch today, using Ubuntu 1604 +
> gcc 5.3, the make bzImage does not output any warning messages about
> this .c file. I am wondering how you captured the warnings. It seems
> that I need the "make C=2" command.


It requires to explicitly enable endianess check via CF= parameter.

Linus, by the way do we need to go with this to stable and -rc2?

> 
> Thanks,
> Yong
> 
> 2016-06-01 20:35 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux.in
> tel.com>:
> > On Wed, 2016-06-01 at 12:44 +0800, Yong Li wrote:
> > > Hi Andy,
> > > 
> > > Could you let me know which build system you are using? I am using
> > > Ubuntu 1604(64 bit) to build the kernel source, but I did not find
> > > the
> > > warning.
> > 
> > Hmm... I'm confused what exactly you are asking here. My host system
> > is
> > Debian unstable, kernel is built by it's own KBuild infrastructure.
> > 
> > I'm pretty sure you are talking about this:
> > 
> > https://lwn.net/Articles/205624/
> > 
> > > 
> > > Thanks,
> > > Yong
> > > 
> > > 2016-06-01 3:03 GMT+08:00 Andy Shevchenko <andriy.shevchenko@linux
> > > .int
> > > el.com>:
> > > > The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value
> > > > for
> > > > register
> > > > word write") fixed regression in pca953x_write_regs(). At the
> > > > same
> > > > time the
> > > > solution introduced a sparse warning:
> > > > 
> > > > drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in
> > > > argument 3 (different base types)
> > > > drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short
> > > > [unsigned] [usertype] value
> > > > drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16
> > > > [usertype] <noident>
> > > > 
> > > > Fix the code by enforcing the type of
> > > > i2c_smbus_write_word_data()
> > > > parameter.
> > > > 
> > > > Cc: Yong Li <sdliyong@gmail.com>
> > > > Cc: Phil Reid <preid@electromag.com.au>
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.co
> > > > m>
> > > > ---
> > > >  drivers/gpio/gpio-pca953x.c | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-
> > > > pca953x.c
> > > > index d8233be..21b21cd 100644
> > > > --- a/drivers/gpio/gpio-pca953x.c
> > > > +++ b/drivers/gpio/gpio-pca953x.c
> > > > @@ -163,10 +163,13 @@ static int pca953x_write_regs(struct
> > > > pca953x_chip *chip, int reg, u8 *val)
> > > >                                         NBANK(chip), val);
> > > >         } else {
> > > >                 switch (chip->chip_type) {
> > > > -               case PCA953X_TYPE:
> > > > -                       ret = i2c_smbus_write_word_data(chip-
> > > > > client,
> > > > -                           reg << 1,
> > > > cpu_to_le16(get_unaligned((u16
> > > > *)val)));
> > > > +               case PCA953X_TYPE: {
> > > > +                       __le16 word =
> > > > cpu_to_le16(get_unaligned((u16
> > > > *)val));
> > > > +
> > > > +                       ret = i2c_smbus_write_word_data(chip-
> > > > > client, reg << 1,
> > > > +                                                       (__force
> > > > u16)word);
> > > >                         break;
> > > > +               }
> > > >                 case PCA957X_TYPE:
> > > >                         ret = i2c_smbus_write_byte_data(chip-
> > > > > client, reg << 1,
> > > >                                                         val[0]);
> > > > --
> > > > 2.8.1
> > > > 
> > 
> > --
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Intel Finland Oy

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-05-31 19:03 [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data() Andy Shevchenko
  2016-06-01  4:44 ` Yong Li
@ 2016-06-08  8:03 ` Linus Walleij
  1 sibling, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2016-06-08  8:03 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-gpio, Yong Li, Phil Reid

On Tue, May 31, 2016 at 9:03 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> The commit 9b8e3ec34318 ("gpio: pca953x: Use correct u16 value for register
> word write") fixed regression in pca953x_write_regs(). At the same time the
> solution introduced a sparse warning:
>
> drivers/gpio/gpio-pca953x.c:168:39: warning: incorrect type in argument 3 (different base types)
> drivers/gpio/gpio-pca953x.c:168:39:    expected unsigned short [unsigned] [usertype] value
> drivers/gpio/gpio-pca953x.c:168:39:    got restricted __le16 [usertype] <noident>
>
> Fix the code by enforcing the type of i2c_smbus_write_word_data() parameter.
>
> Cc: Yong Li <sdliyong@gmail.com>
> Cc: Phil Reid <preid@electromag.com.au>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-06-02 14:43       ` Andy Shevchenko
@ 2016-06-08  8:03         ` Linus Walleij
  2016-06-08  9:06           ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2016-06-08  8:03 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Yong Li, linux-gpio, Phil Reid

On Thu, Jun 2, 2016 at 4:43 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Linus, by the way do we need to go with this to stable and -rc2?

If it's just a compilation warning, no.

If it is causing real regressions on hardware yes.

I have applied it for the next kernel as of now.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data()
  2016-06-08  8:03         ` Linus Walleij
@ 2016-06-08  9:06           ` Andy Shevchenko
  0 siblings, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2016-06-08  9:06 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Yong Li, linux-gpio, Phil Reid

On Wed, 2016-06-08 at 10:03 +0200, Linus Walleij wrote:
> On Thu, Jun 2, 2016 at 4:43 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Linus, by the way do we need to go with this to stable and -rc2?
> 
> If it's just a compilation warning, no.
> 
> If it is causing real regressions on hardware yes.

I believe it's just a static analyzer warning.

> 
> I have applied it for the next kernel as of now.

Thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-06-08  9:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-31 19:03 [PATCH v1 1/1] gpio: pca953x: enfore type for i2c_smbus_write_word_data() Andy Shevchenko
2016-06-01  4:44 ` Yong Li
2016-06-01 12:35   ` Andy Shevchenko
2016-06-01 15:19     ` Yong Li
2016-06-02 14:43       ` Andy Shevchenko
2016-06-08  8:03         ` Linus Walleij
2016-06-08  9:06           ` Andy Shevchenko
2016-06-08  8:03 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.