Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each
@ 2019-10-09 20:43 Jules Irenge
  2019-10-09 20:48 ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 5+ messages in thread
From: Jules Irenge @ 2019-10-09 20:43 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: gregkh, GR-Linux-NIC-Dev, netdev, devel, linux-kernel, Jules Irenge

Fix multiple assignments warning " check
 issued by checkpatch.pl tool:
"CHECK: multiple assignments should be avoided".

Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
---
 drivers/staging/qlge/qlge_dbg.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 086f067fd899..69bd4710c5ec 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -141,8 +141,10 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
 	u32 *direct_ptr, temp;
 	u32 *indirect_ptr;
 
-	xfi_direct_valid = xfi_indirect_valid = 0;
-	xaui_direct_valid = xaui_indirect_valid = 1;
+	xfi_indirect_valid = 0;
+	xfi_direct_valid = xfi_indirect_valid;
+	xaui_indirect_valid = 1;
+	xaui_direct_valid = xaui_indirect_valid
 
 	/* The XAUI needs to be read out per port */
 	status = ql_read_other_func_serdes_reg(qdev,
-- 
2.21.0


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

* Re: [Outreachy kernel] [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each
  2019-10-09 20:43 [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each Jules Irenge
@ 2019-10-09 20:48 ` " Julia Lawall
  2019-10-09 23:54   ` Joe Perches
  0 siblings, 1 reply; 5+ messages in thread
From: Julia Lawall @ 2019-10-09 20:48 UTC (permalink / raw)
  To: Jules Irenge
  Cc: outreachy-kernel, gregkh, GR-Linux-NIC-Dev, netdev, devel, linux-kernel



On Wed, 9 Oct 2019, Jules Irenge wrote:

> Fix multiple assignments warning " check
>  issued by checkpatch.pl tool:
> "CHECK: multiple assignments should be avoided".
>
> Signed-off-by: Jules Irenge <jbi.octave@gmail.com>
> ---
>  drivers/staging/qlge/qlge_dbg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> index 086f067fd899..69bd4710c5ec 100644
> --- a/drivers/staging/qlge/qlge_dbg.c
> +++ b/drivers/staging/qlge/qlge_dbg.c
> @@ -141,8 +141,10 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
>  	u32 *direct_ptr, temp;
>  	u32 *indirect_ptr;
>
> -	xfi_direct_valid = xfi_indirect_valid = 0;
> -	xaui_direct_valid = xaui_indirect_valid = 1;
> +	xfi_indirect_valid = 0;
> +	xfi_direct_valid = xfi_indirect_valid;
> +	xaui_indirect_valid = 1;
> +	xaui_direct_valid = xaui_indirect_valid

Despite checkpatch, I think that the original code was easier to
understand.

julia

>
>  	/* The XAUI needs to be read out per port */
>  	status = ql_read_other_func_serdes_reg(qdev,
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20191009204311.7988-1-jbi.octave%40gmail.com.
>

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

* Re: [Outreachy kernel] [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each
  2019-10-09 20:48 ` [Outreachy kernel] " Julia Lawall
@ 2019-10-09 23:54   ` Joe Perches
  2019-10-10  5:37     ` Julia Lawall
  2019-10-10  9:09     ` Dan Carpenter
  0 siblings, 2 replies; 5+ messages in thread
From: Joe Perches @ 2019-10-09 23:54 UTC (permalink / raw)
  To: Julia Lawall, Jules Irenge
  Cc: outreachy-kernel, gregkh, GR-Linux-NIC-Dev, netdev, devel, linux-kernel

On Wed, 2019-10-09 at 22:48 +0200, Julia Lawall wrote:
> On Wed, 9 Oct 2019, Jules Irenge wrote:
> > Fix multiple assignments warning " check
> >  issued by checkpatch.pl tool:
> > "CHECK: multiple assignments should be avoided".
[]
> > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
[]
> > @@ -141,8 +141,10 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
> >  	u32 *direct_ptr, temp;
> >  	u32 *indirect_ptr;
> > 
> > -	xfi_direct_valid = xfi_indirect_valid = 0;
> > -	xaui_direct_valid = xaui_indirect_valid = 1;
> > +	xfi_indirect_valid = 0;
> > +	xfi_direct_valid = xfi_indirect_valid;
> > +	xaui_indirect_valid = 1;
> > +	xaui_direct_valid = xaui_indirect_valid
> 
> Despite checkpatch, I think that the original code was easier to
> understand.

It'd likely be easier to understand if all the
<foo>_valid uses were bool and the ql_get_both_serdes
<foo>_valid arguments were change to bool from
unsigned int as well.

btw: qlge likely is going to be deleted and not updated.

---
 drivers/staging/qlge/qlge_dbg.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 7e16066a3527..90ab37d4c49d 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -112,7 +112,7 @@ static int ql_read_serdes_reg(struct ql_adapter *qdev, u32 reg, u32 *data)
 
 static void ql_get_both_serdes(struct ql_adapter *qdev, u32 addr,
 			u32 *direct_ptr, u32 *indirect_ptr,
-			unsigned int direct_valid, unsigned int indirect_valid)
+			bool direct_valid, bool indirect_valid)
 {
 	unsigned int status;
 
@@ -136,14 +136,12 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
 				struct ql_mpi_coredump *mpi_coredump)
 {
 	int status;
-	unsigned int xfi_direct_valid, xfi_indirect_valid, xaui_direct_valid;
-	unsigned int xaui_indirect_valid, i;
+	bool xfi_direct_valid = false, xfi_indirect_valid = false;
+	bool xaui_direct_valid = true, xaui_indirect_valid = true;
+	unsigned int i;
 	u32 *direct_ptr, temp;
 	u32 *indirect_ptr;
 
-	xfi_direct_valid = xfi_indirect_valid = 0;
-	xaui_direct_valid = xaui_indirect_valid = 1;
-
 	/* The XAUI needs to be read out per port */
 	status = ql_read_other_func_serdes_reg(qdev,
 			XG_SERDES_XAUI_HSS_PCS_START, &temp);
@@ -152,7 +150,7 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
 
 	if ((temp & XG_SERDES_ADDR_XAUI_PWR_DOWN) ==
 				XG_SERDES_ADDR_XAUI_PWR_DOWN)
-		xaui_indirect_valid = 0;
+		xaui_indirect_valid = false;
 
 	status = ql_read_serdes_reg(qdev, XG_SERDES_XAUI_HSS_PCS_START, &temp);
 
@@ -161,7 +159,7 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
 
 	if ((temp & XG_SERDES_ADDR_XAUI_PWR_DOWN) ==
 				XG_SERDES_ADDR_XAUI_PWR_DOWN)
-		xaui_direct_valid = 0;
+		xaui_direct_valid = false;
 
 	/*
 	 * XFI register is shared so only need to read one
@@ -176,18 +174,18 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
 		/* now see if i'm NIC 1 or NIC 2 */
 		if (qdev->func & 1)
 			/* I'm NIC 2, so the indirect (NIC1) xfi is up. */
-			xfi_indirect_valid = 1;
+			xfi_indirect_valid = true;
 		else
-			xfi_direct_valid = 1;
+			xfi_direct_valid = true;
 	}
 	if ((temp & XG_SERDES_ADDR_XFI2_PWR_UP) ==
 					XG_SERDES_ADDR_XFI2_PWR_UP) {
 		/* now see if i'm NIC 1 or NIC 2 */
 		if (qdev->func & 1)
 			/* I'm NIC 2, so the indirect (NIC1) xfi is up. */
-			xfi_direct_valid = 1;
+			xfi_direct_valid = true;
 		else
-			xfi_indirect_valid = 1;
+			xfi_indirect_valid = true;
 	}
 
 	/* Get XAUI_AN register block. */


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

* Re: [Outreachy kernel] [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each
  2019-10-09 23:54   ` Joe Perches
@ 2019-10-10  5:37     ` Julia Lawall
  2019-10-10  9:09     ` Dan Carpenter
  1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2019-10-10  5:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jules Irenge, outreachy-kernel, gregkh, GR-Linux-NIC-Dev, netdev,
	devel, linux-kernel



On Wed, 9 Oct 2019, Joe Perches wrote:

> On Wed, 2019-10-09 at 22:48 +0200, Julia Lawall wrote:
> > On Wed, 9 Oct 2019, Jules Irenge wrote:
> > > Fix multiple assignments warning " check
> > >  issued by checkpatch.pl tool:
> > > "CHECK: multiple assignments should be avoided".
> []
> > > diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> []
> > > @@ -141,8 +141,10 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
> > >  	u32 *direct_ptr, temp;
> > >  	u32 *indirect_ptr;
> > >
> > > -	xfi_direct_valid = xfi_indirect_valid = 0;
> > > -	xaui_direct_valid = xaui_indirect_valid = 1;
> > > +	xfi_indirect_valid = 0;
> > > +	xfi_direct_valid = xfi_indirect_valid;
> > > +	xaui_indirect_valid = 1;
> > > +	xaui_direct_valid = xaui_indirect_valid
> >
> > Despite checkpatch, I think that the original code was easier to
> > understand.
>
> It'd likely be easier to understand if all the
> <foo>_valid uses were bool and the ql_get_both_serdes
> <foo>_valid arguments were change to bool from
> unsigned int as well.

Indeed, given the names and the values, bool would be much better.

> btw: qlge likely is going to be deleted and not updated.

OK.  Jules, if you want to make this change, you can, but it could be
better to move on to some other driver.

thanks,
julia

>
> ---
>  drivers/staging/qlge/qlge_dbg.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
> index 7e16066a3527..90ab37d4c49d 100644
> --- a/drivers/staging/qlge/qlge_dbg.c
> +++ b/drivers/staging/qlge/qlge_dbg.c
> @@ -112,7 +112,7 @@ static int ql_read_serdes_reg(struct ql_adapter *qdev, u32 reg, u32 *data)
>
>  static void ql_get_both_serdes(struct ql_adapter *qdev, u32 addr,
>  			u32 *direct_ptr, u32 *indirect_ptr,
> -			unsigned int direct_valid, unsigned int indirect_valid)
> +			bool direct_valid, bool indirect_valid)
>  {
>  	unsigned int status;
>
> @@ -136,14 +136,12 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
>  				struct ql_mpi_coredump *mpi_coredump)
>  {
>  	int status;
> -	unsigned int xfi_direct_valid, xfi_indirect_valid, xaui_direct_valid;
> -	unsigned int xaui_indirect_valid, i;
> +	bool xfi_direct_valid = false, xfi_indirect_valid = false;
> +	bool xaui_direct_valid = true, xaui_indirect_valid = true;
> +	unsigned int i;
>  	u32 *direct_ptr, temp;
>  	u32 *indirect_ptr;
>
> -	xfi_direct_valid = xfi_indirect_valid = 0;
> -	xaui_direct_valid = xaui_indirect_valid = 1;
> -
>  	/* The XAUI needs to be read out per port */
>  	status = ql_read_other_func_serdes_reg(qdev,
>  			XG_SERDES_XAUI_HSS_PCS_START, &temp);
> @@ -152,7 +150,7 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
>
>  	if ((temp & XG_SERDES_ADDR_XAUI_PWR_DOWN) ==
>  				XG_SERDES_ADDR_XAUI_PWR_DOWN)
> -		xaui_indirect_valid = 0;
> +		xaui_indirect_valid = false;
>
>  	status = ql_read_serdes_reg(qdev, XG_SERDES_XAUI_HSS_PCS_START, &temp);
>
> @@ -161,7 +159,7 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
>
>  	if ((temp & XG_SERDES_ADDR_XAUI_PWR_DOWN) ==
>  				XG_SERDES_ADDR_XAUI_PWR_DOWN)
> -		xaui_direct_valid = 0;
> +		xaui_direct_valid = false;
>
>  	/*
>  	 * XFI register is shared so only need to read one
> @@ -176,18 +174,18 @@ static int ql_get_serdes_regs(struct ql_adapter *qdev,
>  		/* now see if i'm NIC 1 or NIC 2 */
>  		if (qdev->func & 1)
>  			/* I'm NIC 2, so the indirect (NIC1) xfi is up. */
> -			xfi_indirect_valid = 1;
> +			xfi_indirect_valid = true;
>  		else
> -			xfi_direct_valid = 1;
> +			xfi_direct_valid = true;
>  	}
>  	if ((temp & XG_SERDES_ADDR_XFI2_PWR_UP) ==
>  					XG_SERDES_ADDR_XFI2_PWR_UP) {
>  		/* now see if i'm NIC 1 or NIC 2 */
>  		if (qdev->func & 1)
>  			/* I'm NIC 2, so the indirect (NIC1) xfi is up. */
> -			xfi_direct_valid = 1;
> +			xfi_direct_valid = true;
>  		else
> -			xfi_indirect_valid = 1;
> +			xfi_indirect_valid = true;
>  	}
>
>  	/* Get XAUI_AN register block. */
>
>

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

* Re: [Outreachy kernel] [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each
  2019-10-09 23:54   ` Joe Perches
  2019-10-10  5:37     ` Julia Lawall
@ 2019-10-10  9:09     ` Dan Carpenter
  1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2019-10-10  9:09 UTC (permalink / raw)
  To: Joe Perches
  Cc: Julia Lawall, Jules Irenge, devel, GR-Linux-NIC-Dev, netdev,
	linux-kernel, outreachy-kernel, gregkh

I was just about to give a newbie a Reviewed-by cookie until I saw it
was a Joe Perches patch without a commit message or a sign off.  And
then I was annoyed that I had invested any time in it at all.  I even
dropped out of my email client for this!

:P

If you want to resend as a proper commit then you can still have my
Reviewed-by I guess.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter


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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 20:43 [PATCH] staging: qlge: Fix multiple assignments warning by splitting the assignement into two each Jules Irenge
2019-10-09 20:48 ` [Outreachy kernel] " Julia Lawall
2019-10-09 23:54   ` Joe Perches
2019-10-10  5:37     ` Julia Lawall
2019-10-10  9:09     ` Dan Carpenter

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

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

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


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