DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] Remove warning: "dubious: x | !y" detected by sparse
@ 2020-01-15  1:25 Felipe Cardoso Resende
  2020-01-15  6:21 ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Felipe Cardoso Resende @ 2020-01-15  1:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Geordan Neukum, Hao Xu, Jamal Shareef; +Cc: devel

The way I chose to remove the warning was to define a macro to
make it clear if a flag will be enable or not.

Let me know if you would prefer it to be done in a different way.

Signed-off-by: Felipe Cardoso Resende <felipecardoso.fcr@gmail.com>
---
 drivers/staging/kpc2000/kpc2000_i2c.c | 80 +++++++++++++++------------
 1 file changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000_i2c.c b/drivers/staging/kpc2000/kpc2000_i2c.c
index 5460bf973c9c..10b9cee9bffd 100644
--- a/drivers/staging/kpc2000/kpc2000_i2c.c
+++ b/drivers/staging/kpc2000/kpc2000_i2c.c
@@ -572,6 +572,10 @@ static s32 i801_access(struct i2c_adapter *adap, u16 addr,
 	return 0;
 }
 
+#define enable_flag(x) (x)
+#define disable_flag(x) 0
+#define enable_flag_if(x, cond) ((cond) ? (x) : 0)
+
 static u32 i801_func(struct i2c_adapter *adapter)
 {
 	struct i2c_device *priv = i2c_get_adapdata(adapter);
@@ -588,42 +592,42 @@ static u32 i801_func(struct i2c_adapter *adapter)
 	// http://lxr.free-electrons.com/source/include/uapi/linux/i2c.h#L85
 
 	u32 f =
-		I2C_FUNC_I2C                     | /* 0x00000001(I enabled this
-						    * one)
-						    */
-		!I2C_FUNC_10BIT_ADDR             |		/* 0x00000002 */
-		!I2C_FUNC_PROTOCOL_MANGLING      |		/* 0x00000004 */
-		((priv->features & FEATURE_SMBUS_PEC) ?
-			I2C_FUNC_SMBUS_PEC : 0)  |		/* 0x00000008 */
-		!I2C_FUNC_SMBUS_BLOCK_PROC_CALL  |		/* 0x00008000 */
-		I2C_FUNC_SMBUS_QUICK             |		/* 0x00010000 */
-		!I2C_FUNC_SMBUS_READ_BYTE	 |		/* 0x00020000 */
-		!I2C_FUNC_SMBUS_WRITE_BYTE       |		/* 0x00040000 */
-		!I2C_FUNC_SMBUS_READ_BYTE_DATA   |		/* 0x00080000 */
-		!I2C_FUNC_SMBUS_WRITE_BYTE_DATA  |		/* 0x00100000 */
-		!I2C_FUNC_SMBUS_READ_WORD_DATA   |		/* 0x00200000 */
-		!I2C_FUNC_SMBUS_WRITE_WORD_DATA  |		/* 0x00400000 */
-		!I2C_FUNC_SMBUS_PROC_CALL        |		/* 0x00800000 */
-		!I2C_FUNC_SMBUS_READ_BLOCK_DATA  |		/* 0x01000000 */
-		!I2C_FUNC_SMBUS_WRITE_BLOCK_DATA |		/* 0x02000000 */
-		((priv->features & FEATURE_I2C_BLOCK_READ) ?
-			I2C_FUNC_SMBUS_READ_I2C_BLOCK : 0) |	/* 0x04000000 */
-		I2C_FUNC_SMBUS_WRITE_I2C_BLOCK   |		/* 0x08000000 */
-
-		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
+		enable_flag(I2C_FUNC_I2C) | /* 0x00000001(I enabled this one) */
+		disable_flag(I2C_FUNC_10BIT_ADDR)             | /* 0x00000002 */
+		disable_flag(I2C_FUNC_PROTOCOL_MANGLING)      | /* 0x00000004 */
+		enable_flag_if(I2C_FUNC_SMBUS_PEC,
+			       priv->features & FEATURE_SMBUS_PEC) |
+								/* 0x00000008 */
+		disable_flag(I2C_FUNC_SMBUS_BLOCK_PROC_CALL)  | /* 0x00008000 */
+		enable_flag(I2C_FUNC_SMBUS_QUICK)             | /* 0x00010000 */
+		disable_flag(I2C_FUNC_SMBUS_READ_BYTE)	      |	/* 0x00020000 */
+		disable_flag(I2C_FUNC_SMBUS_WRITE_BYTE)       |	/* 0x00040000 */
+		disable_flag(I2C_FUNC_SMBUS_READ_BYTE_DATA)   |	/* 0x00080000 */
+		disable_flag(I2C_FUNC_SMBUS_WRITE_BYTE_DATA)  |	/* 0x00100000 */
+		disable_flag(I2C_FUNC_SMBUS_READ_WORD_DATA)   |	/* 0x00200000 */
+		disable_flag(I2C_FUNC_SMBUS_WRITE_WORD_DATA)  |	/* 0x00400000 */
+		disable_flag(I2C_FUNC_SMBUS_PROC_CALL)        |	/* 0x00800000 */
+		disable_flag(I2C_FUNC_SMBUS_READ_BLOCK_DATA)  |	/* 0x01000000 */
+		disable_flag(I2C_FUNC_SMBUS_WRITE_BLOCK_DATA) |	/* 0x02000000 */
+		enable_flag_if(I2C_FUNC_SMBUS_READ_I2C_BLOCK,
+			       priv->features & FEATURE_I2C_BLOCK_READ) |
+								/* 0x04000000 */
+		enable_flag(I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)   |	/* 0x08000000 */
+
+		enable_flag(I2C_FUNC_SMBUS_BYTE) | /* _READ_BYTE  _WRITE_BYTE */
+		enable_flag(I2C_FUNC_SMBUS_BYTE_DATA)  | /* _READ_BYTE_DATA
+							  * _WRITE_BYTE_DATA
+							  */
+		enable_flag(I2C_FUNC_SMBUS_WORD_DATA)  | /* _READ_WORD_DATA
+							  * _WRITE_WORD_DATA
+							  */
+		enable_flag(I2C_FUNC_SMBUS_BLOCK_DATA) | /* _READ_BLOCK_DATA
+							  * _WRITE_BLOCK_DATA
+							  */
+		disable_flag(I2C_FUNC_SMBUS_I2C_BLOCK) | /* _READ_I2C_BLOCK
+							  * _WRITE_I2C_BLOCK
+							  */
+		disable_flag(I2C_FUNC_SMBUS_EMUL); /* _QUICK  _BYTE
 						    * _BYTE_DATA  _WORD_DATA
 						    * _PROC_CALL
 						    * _WRITE_BLOCK_DATA
@@ -632,6 +636,10 @@ static u32 i801_func(struct i2c_adapter *adapter)
 	return f;
 }
 
+#undef enable_flag
+#undef disable_flag
+#undef enable_flag_if
+
 static const struct i2c_algorithm smbus_algorithm = {
 	.smbus_xfer     = i801_access,
 	.functionality  = i801_func,
-- 
2.24.1

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

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

* Re: [PATCH] Remove warning: "dubious: x | !y" detected by sparse
  2020-01-15  1:25 [PATCH] Remove warning: "dubious: x | !y" detected by sparse Felipe Cardoso Resende
@ 2020-01-15  6:21 ` Dan Carpenter
  2020-01-16  3:05   ` Felipe Cardoso Resende
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-01-15  6:21 UTC (permalink / raw)
  To: Felipe Cardoso Resende
  Cc: devel, Greg Kroah-Hartman, Geordan Neukum, Hao Xu, Jamal Shareef

Add a subsystem prefix to the subject.  "Staging: kpc2000:"

On Tue, Jan 14, 2020 at 10:25:15PM -0300, Felipe Cardoso Resende wrote:
> The way I chose to remove the warning was to define a macro to
> make it clear if a flag will be enable or not.
> 
> Let me know if you would prefer it to be done in a different way.
> 

All this should go under the --- cut off because we don't want it in the
final git history.

> Signed-off-by: Felipe Cardoso Resende <felipecardoso.fcr@gmail.com>
> ---
  ^^^

The commit message should be something like.  "Sparse complains about
"dubious: x | !y".  This patch adds some macros to make Sparse happy and
the code more readable.

I like the patch.  enable_flag_if() is slightly weird because normally
the condition would come first.  It feels sort of like Perl or something
to put the condition afterwards.  But this patch is very small and self
contained so it's fine.

Just fix up the subject and the commit message and resend.

regards,
dan carpenter

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

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

* Re: [PATCH] Remove warning: "dubious: x | !y" detected by sparse
  2020-01-15  6:21 ` Dan Carpenter
@ 2020-01-16  3:05   ` Felipe Cardoso Resende
  2020-01-16  6:12     ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Felipe Cardoso Resende @ 2020-01-16  3:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: devel, Greg Kroah-Hartman, Geordan Neukum, Hao Xu, Jamal Shareef

Hi Dan,

Thank you for you comments, I'll fix the issues in v2. Also, sorry for the 
duplicate email, I sent the first email only to you (now I think I'll get it 
right).

On Wed, Jan 15, 2020 at 09:21:56AM +0300, Dan Carpenter wrote:
> Add a subsystem prefix to the subject.  "Staging: kpc2000:"
> 

I totally forgot about this guideline for staging.

> On Tue, Jan 14, 2020 at 10:25:15PM -0300, Felipe Cardoso Resende wrote:
> > The way I chose to remove the warning was to define a macro to
> > make it clear if a flag will be enable or not.
> > 
> > Let me know if you would prefer it to be done in a different way.
> > 
> 
> All this should go under the --- cut off because we don't want it in the
> final git history.
> 

That makes sense.

> > Signed-off-by: Felipe Cardoso Resende <felipecardoso.fcr@gmail.com>
> > ---
>   ^^^
> 
> The commit message should be something like.  "Sparse complains about
> "dubious: x | !y".  This patch adds some macros to make Sparse happy and
> the code more readable.
> 

I see.

> I like the patch.  enable_flag_if() is slightly weird because normally
> the condition would come first.  It feels sort of like Perl or something
> to put the condition afterwards.  But this patch is very small and self
> contained so it's fine.
> 

I totally agree it's not usual to put the condition at the end. I did that
so the three macros would start with the flag as the first argument, but after
writing the code it seemed to me that it helped to keep the focus in the flag.

> Just fix up the subject and the commit message and resend.
> 
> regards,
> dan carpenter
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

* Re: [PATCH] Remove warning: "dubious: x | !y" detected by sparse
  2020-01-16  3:05   ` Felipe Cardoso Resende
@ 2020-01-16  6:12     ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2020-01-16  6:12 UTC (permalink / raw)
  To: Felipe Cardoso Resende
  Cc: devel, Greg Kroah-Hartman, Geordan Neukum, Hao Xu, Jamal Shareef

Sounds reasonable.  Just resend with the fixed commit message.

regards,
dan carpenter

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

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15  1:25 [PATCH] Remove warning: "dubious: x | !y" detected by sparse Felipe Cardoso Resende
2020-01-15  6:21 ` Dan Carpenter
2020-01-16  3:05   ` Felipe Cardoso Resende
2020-01-16  6:12     ` Dan Carpenter

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org
	public-inbox-index driverdev-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git