[PATCH 1/1] i2c: core: fix a code to suppress a warning

[PATCH 1/1] i2c: core: fix a code to suppress a warning

[Andy Shevchenko]

There is a warning when compiling i2c-core.c
drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y

Fix it by using ternary operator.

Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 3a4c54e..d13a8a0 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -2591,7 +2591,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count)
 static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
 {
 	/* The address will be sent first */
-	u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
+	u8 addr = (msg->addr << 1) | ((msg->flags & I2C_M_RD) ? 1 : 0);
 	pec = i2c_smbus_pec(pec, &addr, 1);
 
 	/* The data buffer follows */
-- 
2.5.1

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Alexander Sverdlin]

On 15/09/15 12:11, EXT Andy Shevchenko wrote:
> There is a warning when compiling i2c-core.c
> drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y
> 
> Fix it by using ternary operator.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Alexander Sverdlin <alexander.sverdlin-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/i2c/i2c-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 3a4c54e..d13a8a0 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2591,7 +2591,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count)
>  static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
>  {
>  	/* The address will be sent first */
> -	u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
> +	u8 addr = (msg->addr << 1) | ((msg->flags & I2C_M_RD) ? 1 : 0);
>  	pec = i2c_smbus_pec(pec, &addr, 1);
>  
>  	/* The data buffer follows */

-- 
Best regards,
Alexander Sverdlin.

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Vladimir Zapolskiy]

Hello Andy,

On 15.09.2015 13:11, Andy Shevchenko wrote:
> There is a warning when compiling i2c-core.c
> drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y
> 
> Fix it by using ternary operator.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/i2c/i2c-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 3a4c54e..d13a8a0 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2591,7 +2591,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t count)
>  static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
>  {
>  	/* The address will be sent first */
> -	u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
> +	u8 addr = (msg->addr << 1) | ((msg->flags & I2C_M_RD) ? 1 : 0);
>  	pec = i2c_smbus_pec(pec, &addr, 1);
>  
>  	/* The data buffer follows */
> 

I2C_M_RD is defined as 1, probably (msg->flags & I2C_M_RD) is good
enough here.

--
With best wishes,
Vladimir

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Andy Shevchenko]

On Tue, 2015-09-15 at 14:25 +0300, Vladimir Zapolskiy wrote:
> Hello Andy,
> 
> On 15.09.2015 13:11, Andy Shevchenko wrote:
> > There is a warning when compiling i2c-core.c
> > drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y
> > 
> > Fix it by using ternary operator.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> >  drivers/i2c/i2c-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 3a4c54e..d13a8a0 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -2591,7 +2591,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t 
> > count)
> >  static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
> >  {
> >  	/* The address will be sent first */
> > -	u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
> > +	u8 addr = (msg->addr << 1) | ((msg->flags & I2C_M_RD) ? 1 
> > : 0);
> >  	pec = i2c_smbus_pec(pec, &addr, 1);
> >  
> >  	/* The data buffer follows */
> > 
> 
> I2C_M_RD is defined as 1, probably (msg->flags & I2C_M_RD) is good
> enough here.

Today is 1, tomorrow is 0x80, so, I would stay as I put in the initial
fix.

> 
> --
> With best wishes,
> Vladimir

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Vladimir Zapolskiy]

On 15.09.2015 15:06, Andy Shevchenko wrote:
> On Tue, 2015-09-15 at 14:25 +0300, Vladimir Zapolskiy wrote:
>> Hello Andy,
>>
>> On 15.09.2015 13:11, Andy Shevchenko wrote:
>>> There is a warning when compiling i2c-core.c
>>> drivers/i2c/i2c-core.c:2561:36: warning: dubious: x | !y
>>>
>>> Fix it by using ternary operator.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>> ---
>>>  drivers/i2c/i2c-core.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>> index 3a4c54e..d13a8a0 100644
>>> --- a/drivers/i2c/i2c-core.c
>>> +++ b/drivers/i2c/i2c-core.c
>>> @@ -2591,7 +2591,7 @@ static u8 i2c_smbus_pec(u8 crc, u8 *p, size_t 
>>> count)
>>>  static u8 i2c_smbus_msg_pec(u8 pec, struct i2c_msg *msg)
>>>  {
>>>  	/* The address will be sent first */
>>> -	u8 addr = (msg->addr << 1) | !!(msg->flags & I2C_M_RD);
>>> +	u8 addr = (msg->addr << 1) | ((msg->flags & I2C_M_RD) ? 1 
>>> : 0);
>>>  	pec = i2c_smbus_pec(pec, &addr, 1);
>>>  
>>>  	/* The data buffer follows */
>>>
>>
>> I2C_M_RD is defined as 1, probably (msg->flags & I2C_M_RD) is good
>> enough here.
> 
> Today is 1, tomorrow is 0x80, so, I would stay as I put in the initial
> fix.

I believe I2C_M_RD will never get any other value than 1, since this
value is deliberately set to 1 and it is in active use by userspace
applications for years, nobody intends to break ABI tomorrow.

--
With best wishes,
Vladimir

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]


> >> I2C_M_RD is defined as 1, probably (msg->flags & I2C_M_RD) is good
> >> enough here.
> > 
> > Today is 1, tomorrow is 0x80, so, I would stay as I put in the initial
> > fix.
> 
> I believe I2C_M_RD will never get any other value than 1, since this
> value is deliberately set to 1 and it is in active use by userspace
> applications for years, nobody intends to break ABI tomorrow.

I agree. That is one thing that permanently slips through the cracks,
but I wanted to add a comment saying the I2C_M_RD is guaranteed to be 1
and then simplify the drivers.

Maybe I should start with the comment right now...


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/1] i2c: core: fix a code to suppress a warning

[Andy Shevchenko]

On Tue, 2015-09-15 at 15:00 +0200, Wolfram Sang wrote:
> > 
> > > > I2C_M_RD is defined as 1, probably (msg->flags & I2C_M_RD) is 
> > > > good
> > > > enough here.
> > > 
> > > Today is 1, tomorrow is 0x80, so, I would stay as I put in the 
> > > initial
> > > fix.
> > 
> > I believe I2C_M_RD will never get any other value than 1, since 
> > this
> > value is deliberately set to 1 and it is in active use by userspace
> > applications for years, nobody intends to break ABI tomorrow.
> 
> I agree. That is one thing that permanently slips through the cracks,
> but I wanted to add a comment saying the I2C_M_RD is guaranteed to be 
> 1
> and then simplify the drivers.

Yes, with comment on I2C_M_RD we may drop wrong assumptions.

> 
> Maybe I should start with the comment right now...

Please, update my patch accordingly, or ping me to update it.
Right now busy with something else.

-- 
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Intel Finland Oy