RE: [PATCH] staging:kpc2000:Fix dubious x | !y sparse warning

2019-08-01 Thread Matt Sickler
>-Original Message-
>From: devel  On Behalf Of Greg 
>KH
>Sent: Thursday, August 01, 2019 11:35 AM
>To: Harsh Jain 
>Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
>Subject: Re: [PATCH] staging:kpc2000:Fix dubious x | !y sparse warning
>
>On Thu, Aug 01, 2019 at 12:06:06AM +0530, Harsh Jain wrote:
>> Bitwise OR(|) operation with 0 always yield same result.
>> It fixes dubious x | !y sparse warning.
>>
>> Signed-off-by: Harsh Jain 
>> ---
>>  drivers/staging/kpc2000/kpc2000_i2c.c | 16 +---
>>  1 file changed, 1 insertion(+), 15 deletions(-)
>>
>> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c 
>> b/drivers/staging/kpc2000/kpc2000_i2c.c
>> index b108da4..5f027d7c 100644
>> --- a/drivers/staging/kpc2000/kpc2000_i2c.c
>> +++ b/drivers/staging/kpc2000/kpc2000_i2c.c
>> @@ -536,29 +536,15 @@ static u32 i801_func(struct i2c_adapter *adapter)
>>
>>   u32 f =
>>   I2C_FUNC_I2C | /* 0x0001 (I enabled 
>> this one) */
>> - !I2C_FUNC_10BIT_ADDR | /* 0x0002 */
>> - !I2C_FUNC_PROTOCOL_MANGLING  | /* 0x0004 */
>>   ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 
>> 0) | /* 0x0008 */
>> - !I2C_FUNC_SMBUS_BLOCK_PROC_CALL  | /* 0x8000 */
>>   I2C_FUNC_SMBUS_QUICK | /* 0x0001 */
>> - !I2C_FUNC_SMBUS_READ_BYTE| /* 0x0002 */
>> - !I2C_FUNC_SMBUS_WRITE_BYTE   | /* 0x0004 */
>> - !I2C_FUNC_SMBUS_READ_BYTE_DATA   | /* 0x0008 */
>> - !I2C_FUNC_SMBUS_WRITE_BYTE_DATA  | /* 0x0010 */
>> - !I2C_FUNC_SMBUS_READ_WORD_DATA   | /* 0x0020 */
>> - !I2C_FUNC_SMBUS_WRITE_WORD_DATA  | /* 0x0040 */
>> - !I2C_FUNC_SMBUS_PROC_CALL| /* 0x0080 */
>> - !I2C_FUNC_SMBUS_READ_BLOCK_DATA  | /* 0x0100 */
>> - !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x0200 */
>
>This is ok, it is showing you that these bits are explicitly being not
>set.  Which is good, now you can go through the list and see that all
>are accounted for.
>
>So I think this should stay as-is, thanks.

I was going to say the same thing, but I didn't know what the kernel style 
guideline was.
Would Linus prefer this style or would commenting them out be preferred?
Seems like the sparse warnings means the current style is not acceptable?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging:kpc2000:Fix dubious x | !y sparse warning

2019-08-01 Thread Greg KH
On Thu, Aug 01, 2019 at 07:22:13PM +, Matt Sickler wrote:
> >-Original Message-
> >From: devel  On Behalf Of 
> >Greg KH
> >Sent: Thursday, August 01, 2019 11:35 AM
> >To: Harsh Jain 
> >Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org
> >Subject: Re: [PATCH] staging:kpc2000:Fix dubious x | !y sparse warning
> >
> >On Thu, Aug 01, 2019 at 12:06:06AM +0530, Harsh Jain wrote:
> >> Bitwise OR(|) operation with 0 always yield same result.
> >> It fixes dubious x | !y sparse warning.
> >>
> >> Signed-off-by: Harsh Jain 
> >> ---
> >>  drivers/staging/kpc2000/kpc2000_i2c.c | 16 +---
> >>  1 file changed, 1 insertion(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c 
> >> b/drivers/staging/kpc2000/kpc2000_i2c.c
> >> index b108da4..5f027d7c 100644
> >> --- a/drivers/staging/kpc2000/kpc2000_i2c.c
> >> +++ b/drivers/staging/kpc2000/kpc2000_i2c.c
> >> @@ -536,29 +536,15 @@ static u32 i801_func(struct i2c_adapter *adapter)
> >>
> >>   u32 f =
> >>   I2C_FUNC_I2C | /* 0x0001 (I enabled 
> >> this one) */
> >> - !I2C_FUNC_10BIT_ADDR | /* 0x0002 */
> >> - !I2C_FUNC_PROTOCOL_MANGLING  | /* 0x0004 */
> >>   ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 
> >> 0) | /* 0x0008 */
> >> - !I2C_FUNC_SMBUS_BLOCK_PROC_CALL  | /* 0x8000 */
> >>   I2C_FUNC_SMBUS_QUICK | /* 0x0001 */
> >> - !I2C_FUNC_SMBUS_READ_BYTE| /* 0x0002 */
> >> - !I2C_FUNC_SMBUS_WRITE_BYTE   | /* 0x0004 */
> >> - !I2C_FUNC_SMBUS_READ_BYTE_DATA   | /* 0x0008 */
> >> - !I2C_FUNC_SMBUS_WRITE_BYTE_DATA  | /* 0x0010 */
> >> - !I2C_FUNC_SMBUS_READ_WORD_DATA   | /* 0x0020 */
> >> - !I2C_FUNC_SMBUS_WRITE_WORD_DATA  | /* 0x0040 */
> >> - !I2C_FUNC_SMBUS_PROC_CALL| /* 0x0080 */
> >> - !I2C_FUNC_SMBUS_READ_BLOCK_DATA  | /* 0x0100 */
> >> - !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x0200 */
> >
> >This is ok, it is showing you that these bits are explicitly being not
> >set.  Which is good, now you can go through the list and see that all
> >are accounted for.
> >
> >So I think this should stay as-is, thanks.
> 
> I was going to say the same thing, but I didn't know what the kernel style 
> guideline was.
> Would Linus prefer this style or would commenting them out be preferred?
> Seems like the sparse warnings means the current style is not acceptable?
> 

Sparse is just warning that you really are not doing anything here, in
case you think you are, as it's a common pattern for bugs.

So all should be fine, don't worry about it for now.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging:kpc2000:Fix dubious x | !y sparse warning

2019-08-01 Thread Greg KH
On Thu, Aug 01, 2019 at 12:06:06AM +0530, Harsh Jain wrote:
> Bitwise OR(|) operation with 0 always yield same result.
> It fixes dubious x | !y sparse warning.
> 
> Signed-off-by: Harsh Jain 
> ---
>  drivers/staging/kpc2000/kpc2000_i2c.c | 16 +---
>  1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c 
> b/drivers/staging/kpc2000/kpc2000_i2c.c
> index b108da4..5f027d7c 100644
> --- a/drivers/staging/kpc2000/kpc2000_i2c.c
> +++ b/drivers/staging/kpc2000/kpc2000_i2c.c
> @@ -536,29 +536,15 @@ static u32 i801_func(struct i2c_adapter *adapter)
>  
>   u32 f =
>   I2C_FUNC_I2C | /* 0x0001 (I enabled 
> this one) */
> - !I2C_FUNC_10BIT_ADDR | /* 0x0002 */
> - !I2C_FUNC_PROTOCOL_MANGLING  | /* 0x0004 */
>   ((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) 
> | /* 0x0008 */
> - !I2C_FUNC_SMBUS_BLOCK_PROC_CALL  | /* 0x8000 */
>   I2C_FUNC_SMBUS_QUICK | /* 0x0001 */
> - !I2C_FUNC_SMBUS_READ_BYTE| /* 0x0002 */
> - !I2C_FUNC_SMBUS_WRITE_BYTE   | /* 0x0004 */
> - !I2C_FUNC_SMBUS_READ_BYTE_DATA   | /* 0x0008 */
> - !I2C_FUNC_SMBUS_WRITE_BYTE_DATA  | /* 0x0010 */
> - !I2C_FUNC_SMBUS_READ_WORD_DATA   | /* 0x0020 */
> - !I2C_FUNC_SMBUS_WRITE_WORD_DATA  | /* 0x0040 */
> - !I2C_FUNC_SMBUS_PROC_CALL| /* 0x0080 */
> - !I2C_FUNC_SMBUS_READ_BLOCK_DATA  | /* 0x0100 */
> - !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x0200 */

This is ok, it is showing you that these bits are explicitly being not
set.  Which is good, now you can go through the list and see that all
are accounted for.

So I think this should stay as-is, thanks.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging:kpc2000:Fix dubious x | !y sparse warning

2019-07-31 Thread Harsh Jain
Bitwise OR(|) operation with 0 always yield same result.
It fixes dubious x | !y sparse warning.

Signed-off-by: Harsh Jain 
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c 
b/drivers/staging/kpc2000/kpc2000_i2c.c
index b108da4..5f027d7c 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -536,29 +536,15 @@ static u32 i801_func(struct i2c_adapter *adapter)
 
u32 f =
I2C_FUNC_I2C | /* 0x0001 (I enabled 
this one) */
-   !I2C_FUNC_10BIT_ADDR | /* 0x0002 */
-   !I2C_FUNC_PROTOCOL_MANGLING  | /* 0x0004 */
((priv->features & FEATURE_SMBUS_PEC) ? I2C_FUNC_SMBUS_PEC : 0) 
| /* 0x0008 */
-   !I2C_FUNC_SMBUS_BLOCK_PROC_CALL  | /* 0x8000 */
I2C_FUNC_SMBUS_QUICK | /* 0x0001 */
-   !I2C_FUNC_SMBUS_READ_BYTE| /* 0x0002 */
-   !I2C_FUNC_SMBUS_WRITE_BYTE   | /* 0x0004 */
-   !I2C_FUNC_SMBUS_READ_BYTE_DATA   | /* 0x0008 */
-   !I2C_FUNC_SMBUS_WRITE_BYTE_DATA  | /* 0x0010 */
-   !I2C_FUNC_SMBUS_READ_WORD_DATA   | /* 0x0020 */
-   !I2C_FUNC_SMBUS_WRITE_WORD_DATA  | /* 0x0040 */
-   !I2C_FUNC_SMBUS_PROC_CALL| /* 0x0080 */
-   !I2C_FUNC_SMBUS_READ_BLOCK_DATA  | /* 0x0100 */
-   !I2C_FUNC_SMBUS_WRITE_BLOCK_DATA | /* 0x0200 */
((priv->features & FEATURE_I2C_BLOCK_READ) ? 
I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0) | /* 0x0400 */
I2C_FUNC_SMBUS_WRITE_I2C_BLOCK   | /* 0x0800 */
 
I2C_FUNC_SMBUS_BYTE  | /* _READ_BYTE  _WRITE_BYTE */
I2C_FUNC_SMBUS_BYTE_DATA | /* _READ_BYTE_DATA  
_WRITE_BYTE_DATA */
I2C_FUNC_SMBUS_WORD_DATA | /* _READ_WORD_DATA  
_WRITE_WORD_DATA */
-   I2C_FUNC_SMBUS_BLOCK_DATA| /* _READ_BLOCK_DATA  
_WRITE_BLOCK_DATA */
-   !I2C_FUNC_SMBUS_I2C_BLOCK| /* _READ_I2C_BLOCK  
_WRITE_I2C_BLOCK */
-   !I2C_FUNC_SMBUS_EMUL;  /* _QUICK  _BYTE  _BYTE_DATA 
 _WORD_DATA  _PROC_CALL  _WRITE_BLOCK_DATA  _I2C_BLOCK _PEC */
+   I2C_FUNC_SMBUS_BLOCK_DATA; /* _READ_BLOCK_DATA  
_WRITE_BLOCK_DATA */
return f;
 }
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel