All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-08 22:53 ` Colin King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2020-05-08 22:53 UTC (permalink / raw)
  To: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Currently the value for 'off' is computed using a multiplication and
a couple of statements later off is being incremented by len and
this value is never read.  Clean up the code by removing the
multiplication and just increment off by len on each iteration. Also
use len instead of TG3_OCIR_LEN.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ff98a82b7bc4..9dd9bd506bcc 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
 static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
 {
 	int i;
+	u32 off, len = TG3_OCIR_LEN;
 
-	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
-		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
-
+	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
 		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
-		off += len;
 
 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
-			memset(ocir, 0, TG3_OCIR_LEN);
+			memset(ocir, 0, len);
 	}
 }
 
-- 
2.25.1


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

* [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-08 22:53 ` Colin King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2020-05-08 22:53 UTC (permalink / raw)
  To: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Currently the value for 'off' is computed using a multiplication and
a couple of statements later off is being incremented by len and
this value is never read.  Clean up the code by removing the
multiplication and just increment off by len on each iteration. Also
use len instead of TG3_OCIR_LEN.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ff98a82b7bc4..9dd9bd506bcc 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
 static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
 {
 	int i;
+	u32 off, len = TG3_OCIR_LEN;
 
-	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
-		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
-
+	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
 		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
-		off += len;
 
 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
-			memset(ocir, 0, TG3_OCIR_LEN);
+			memset(ocir, 0, len);
 	}
 }
 
-- 
2.25.1

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-08 22:53 ` Colin King
@ 2020-05-08 23:04   ` Michael Chan
  -1 siblings, 0 replies; 14+ messages in thread
From: Michael Chan @ 2020-05-08 23:04 UTC (permalink / raw)
  To: Colin King
  Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, Netdev, kernel-janitors, open list

On Fri, May 8, 2020 at 3:53 PM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the value for 'off' is computed using a multiplication and
> a couple of statements later off is being incremented by len and
> this value is never read.  Clean up the code by removing the
> multiplication and just increment off by len on each iteration. Also
> use len instead of TG3_OCIR_LEN.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index ff98a82b7bc4..9dd9bd506bcc 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>  {
>         int i;
> +       u32 off, len = TG3_OCIR_LEN;

Please use reverse X-mas tree style variable declarations.  Other than
that, it looks good.

Thanks.

>
> -       for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
> -               u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
> -
> +       for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>                 tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
> -               off += len;
>
>                 if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>                     !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> -                       memset(ocir, 0, TG3_OCIR_LEN);
> +                       memset(ocir, 0, len);
>         }
>  }
>
> --
> 2.25.1
>

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-08 23:04   ` Michael Chan
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Chan @ 2020-05-08 23:04 UTC (permalink / raw)
  To: Colin King
  Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, Netdev, kernel-janitors, open list

On Fri, May 8, 2020 at 3:53 PM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the value for 'off' is computed using a multiplication and
> a couple of statements later off is being incremented by len and
> this value is never read.  Clean up the code by removing the
> multiplication and just increment off by len on each iteration. Also
> use len instead of TG3_OCIR_LEN.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index ff98a82b7bc4..9dd9bd506bcc 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>  {
>         int i;
> +       u32 off, len = TG3_OCIR_LEN;

Please use reverse X-mas tree style variable declarations.  Other than
that, it looks good.

Thanks.

>
> -       for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
> -               u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
> -
> +       for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>                 tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
> -               off += len;
>
>                 if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>                     !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> -                       memset(ocir, 0, TG3_OCIR_LEN);
> +                       memset(ocir, 0, len);
>         }
>  }
>
> --
> 2.25.1
>

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-08 22:53 ` Colin King
@ 2020-05-08 23:21   ` Joe Perches
  -1 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2020-05-08 23:21 UTC (permalink / raw)
  To: Colin King, Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

On Fri, 2020-05-08 at 23:53 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently the value for 'off' is computed using a multiplication and
> a couple of statements later off is being incremented by len and
> this value is never read.  Clean up the code by removing the
> multiplication and just increment off by len on each iteration. Also
> use len instead of TG3_OCIR_LEN.

I think this is a lot harder to read.

> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
[]
> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>  {
>  	int i;
> +	u32 off, len = TG3_OCIR_LEN;
>  
> -	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
> -		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
> -
> +	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>  		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
> -		off += len;
>  
>  		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>  		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> -			memset(ocir, 0, TG3_OCIR_LEN);
> +			memset(ocir, 0, len);
>  	}
>  }

My preference would be for

{
	int i;
	u32 off = 0;

	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);

		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
			memset(ocir, 0, TG3_OCIR_LEN);

		off += TG3_OCIR_LEN;
		ocir++;
	}



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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-08 23:21   ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2020-05-08 23:21 UTC (permalink / raw)
  To: Colin King, Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

On Fri, 2020-05-08 at 23:53 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently the value for 'off' is computed using a multiplication and
> a couple of statements later off is being incremented by len and
> this value is never read.  Clean up the code by removing the
> multiplication and just increment off by len on each iteration. Also
> use len instead of TG3_OCIR_LEN.

I think this is a lot harder to read.

> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
[]
> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>  {
>  	int i;
> +	u32 off, len = TG3_OCIR_LEN;
>  
> -	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
> -		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
> -
> +	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>  		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
> -		off += len;
>  
>  		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>  		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> -			memset(ocir, 0, TG3_OCIR_LEN);
> +			memset(ocir, 0, len);
>  	}
>  }

My preference would be for

{
	int i;
	u32 off = 0;

	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);

		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
			memset(ocir, 0, TG3_OCIR_LEN);

		off += TG3_OCIR_LEN;
		ocir++;
	}

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-08 23:21   ` Joe Perches
@ 2020-05-08 23:31     ` Colin Ian King
  -1 siblings, 0 replies; 14+ messages in thread
From: Colin Ian King @ 2020-05-08 23:31 UTC (permalink / raw)
  To: Joe Perches, Siva Reddy Kallam, Prashant Sreedharan,
	Michael Chan, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

On 09/05/2020 00:21, Joe Perches wrote:
> On Fri, 2020-05-08 at 23:53 +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Currently the value for 'off' is computed using a multiplication and
>> a couple of statements later off is being incremented by len and
>> this value is never read.  Clean up the code by removing the
>> multiplication and just increment off by len on each iteration. Also
>> use len instead of TG3_OCIR_LEN.
> 
> I think this is a lot harder to read.
> 
>> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> []
>> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>>  {
>>  	int i;
>> +	u32 off, len = TG3_OCIR_LEN;
>>  
>> -	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
>> -		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
>> -
>> +	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>>  		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
>> -		off += len;
>>  
>>  		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>>  		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
>> -			memset(ocir, 0, TG3_OCIR_LEN);
>> +			memset(ocir, 0, len);
>>  	}
>>  }
> 
> My preference would be for
> 
> {
> 	int i;
> 	u32 off = 0;
> 
> 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> 
> 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> 			memset(ocir, 0, TG3_OCIR_LEN);
> 
> 		off += TG3_OCIR_LEN;
> 		ocir++;
> 	}
> 
OK, I'll send a V3 tomorrow.


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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-08 23:31     ` Colin Ian King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin Ian King @ 2020-05-08 23:31 UTC (permalink / raw)
  To: Joe Perches, Siva Reddy Kallam, Prashant Sreedharan,
	Michael Chan, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

On 09/05/2020 00:21, Joe Perches wrote:
> On Fri, 2020-05-08 at 23:53 +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Currently the value for 'off' is computed using a multiplication and
>> a couple of statements later off is being incremented by len and
>> this value is never read.  Clean up the code by removing the
>> multiplication and just increment off by len on each iteration. Also
>> use len instead of TG3_OCIR_LEN.
> 
> I think this is a lot harder to read.
> 
>> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> []
>> @@ -10798,16 +10798,14 @@ static int tg3_init_hw(struct tg3 *tp, bool reset_phy)
>>  static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
>>  {
>>  	int i;
>> +	u32 off, len = TG3_OCIR_LEN;
>>  
>> -	for (i = 0; i < TG3_SD_NUM_RECS; i++, ocir++) {
>> -		u32 off = i * TG3_OCIR_LEN, len = TG3_OCIR_LEN;
>> -
>> +	for (i = 0, off = 0; i < TG3_SD_NUM_RECS; i++, ocir++, off += len) {
>>  		tg3_ape_scratchpad_read(tp, (u32 *) ocir, off, len);
>> -		off += len;
>>  
>>  		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
>>  		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
>> -			memset(ocir, 0, TG3_OCIR_LEN);
>> +			memset(ocir, 0, len);
>>  	}
>>  }
> 
> My preference would be for
> 
> {
> 	int i;
> 	u32 off = 0;
> 
> 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> 
> 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> 			memset(ocir, 0, TG3_OCIR_LEN);
> 
> 		off += TG3_OCIR_LEN;
> 		ocir++;
> 	}
> 
OK, I'll send a V3 tomorrow.

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-08 23:31     ` Colin Ian King
@ 2020-05-09  1:48       ` Jakub Kicinski
  -1 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2020-05-09  1:48 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Joe Perches, Siva Reddy Kallam, Prashant Sreedharan,
	Michael Chan, David S . Miller, netdev, kernel-janitors,
	linux-kernel

On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > My preference would be for
> > 
> > {
> > 	int i;
> > 	u32 off = 0;
> > 
> > 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > 
> > 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > 			memset(ocir, 0, TG3_OCIR_LEN);
> > 
> > 		off += TG3_OCIR_LEN;
> > 		ocir++;
> > 	}
> >   
> OK, I'll send a V3 tomorrow.

I already reviewed and applied v2, just waiting for builds to finish,
let's leave it.

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-09  1:48       ` Jakub Kicinski
  0 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2020-05-09  1:48 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Joe Perches, Siva Reddy Kallam, Prashant Sreedharan,
	Michael Chan, David S . Miller, netdev, kernel-janitors,
	linux-kernel

On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > My preference would be for
> > 
> > {
> > 	int i;
> > 	u32 off = 0;
> > 
> > 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > 
> > 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > 			memset(ocir, 0, TG3_OCIR_LEN);
> > 
> > 		off += TG3_OCIR_LEN;
> > 		ocir++;
> > 	}
> >   
> OK, I'll send a V3 tomorrow.

I already reviewed and applied v2, just waiting for builds to finish,
let's leave it.

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-09  1:48       ` Jakub Kicinski
@ 2020-05-09  2:31         ` Joe Perches
  -1 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2020-05-09  2:31 UTC (permalink / raw)
  To: Jakub Kicinski, Colin Ian King
  Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev, kernel-janitors, linux-kernel

On Fri, 2020-05-08 at 18:48 -0700, Jakub Kicinski wrote:
> On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > > My preference would be for
> > > 
> > > {
> > > 	int i;
> > > 	u32 off = 0;
> > > 
> > > 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > > 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > > 
> > > 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > > 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > > 			memset(ocir, 0, TG3_OCIR_LEN);
> > > 
> > > 		off += TG3_OCIR_LEN;
> > > 		ocir++;
> > > 	}
> > >   
> > OK, I'll send a V3 tomorrow.
> 
> I already reviewed and applied v2, just waiting for builds to finish,
> let's leave it.


I think clarity should be preferred.
Are you a maintainer of this file?

$ ./scripts/get_maintainer.pl -f drivers/net/ethernet/broadcom/tg3.c
Siva Reddy Kallam <siva.kallam@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
Prashant Sreedharan <prashant@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
Michael Chan <mchan@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
"David S. Miller" <davem@davemloft.net> (odd fixer:NETWORKING DRIVERS)
netdev@vger.kernel.org (open list:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
linux-kernel@vger.kernel.org (open list)



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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-09  2:31         ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2020-05-09  2:31 UTC (permalink / raw)
  To: Jakub Kicinski, Colin Ian King
  Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
	David S . Miller, netdev, kernel-janitors, linux-kernel

On Fri, 2020-05-08 at 18:48 -0700, Jakub Kicinski wrote:
> On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > > My preference would be for
> > > 
> > > {
> > > 	int i;
> > > 	u32 off = 0;
> > > 
> > > 	for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > > 		tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > > 
> > > 		if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > > 		    !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > > 			memset(ocir, 0, TG3_OCIR_LEN);
> > > 
> > > 		off += TG3_OCIR_LEN;
> > > 		ocir++;
> > > 	}
> > >   
> > OK, I'll send a V3 tomorrow.
> 
> I already reviewed and applied v2, just waiting for builds to finish,
> let's leave it.


I think clarity should be preferred.
Are you a maintainer of this file?

$ ./scripts/get_maintainer.pl -f drivers/net/ethernet/broadcom/tg3.c
Siva Reddy Kallam <siva.kallam@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
Prashant Sreedharan <prashant@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
Michael Chan <mchan@broadcom.com> (supporter:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
"David S. Miller" <davem@davemloft.net> (odd fixer:NETWORKING DRIVERS)
netdev@vger.kernel.org (open list:BROADCOM TG3 GIGABIT ETHERNET DRIVER)
linux-kernel@vger.kernel.org (open list)

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
  2020-05-09  2:31         ` Joe Perches
@ 2020-05-09  4:48           ` Michael Chan
  -1 siblings, 0 replies; 14+ messages in thread
From: Michael Chan @ 2020-05-09  4:48 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jakub Kicinski, Colin Ian King, Siva Reddy Kallam,
	Prashant Sreedharan, Michael Chan, David S . Miller, Netdev,
	kernel-janitors, open list

On Fri, May 8, 2020 at 7:31 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2020-05-08 at 18:48 -0700, Jakub Kicinski wrote:
> > On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > > > My preference would be for
> > > >
> > > > {
> > > >   int i;
> > > >   u32 off = 0;
> > > >
> > > >   for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > > >           tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > > >
> > > >           if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > > >               !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > > >                   memset(ocir, 0, TG3_OCIR_LEN);
> > > >
> > > >           off += TG3_OCIR_LEN;
> > > >           ocir++;
> > > >   }
> > > >
> > > OK, I'll send a V3 tomorrow.
> >
> > I already reviewed and applied v2, just waiting for builds to finish,
> > let's leave it.
>
>
> I think clarity should be preferred.

Either way is fine with me.  I'm fine with v2 since it's already applied.

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

* Re: [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply
@ 2020-05-09  4:48           ` Michael Chan
  0 siblings, 0 replies; 14+ messages in thread
From: Michael Chan @ 2020-05-09  4:48 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jakub Kicinski, Colin Ian King, Siva Reddy Kallam,
	Prashant Sreedharan, Michael Chan, David S . Miller, Netdev,
	kernel-janitors, open list

On Fri, May 8, 2020 at 7:31 PM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2020-05-08 at 18:48 -0700, Jakub Kicinski wrote:
> > On Sat, 9 May 2020 00:31:03 +0100 Colin Ian King wrote:
> > > > My preference would be for
> > > >
> > > > {
> > > >   int i;
> > > >   u32 off = 0;
> > > >
> > > >   for (i = 0; i < TG3_SD_NUM_RECS; i++) {
> > > >           tg3_ape_scratchpad_read(tp, (u32 *)ocir, off, TC3_OCIR_LEN);
> > > >
> > > >           if (ocir->signature != TG3_OCIR_SIG_MAGIC ||
> > > >               !(ocir->version_flags & TG3_OCIR_FLAG_ACTIVE))
> > > >                   memset(ocir, 0, TG3_OCIR_LEN);
> > > >
> > > >           off += TG3_OCIR_LEN;
> > > >           ocir++;
> > > >   }
> > > >
> > > OK, I'll send a V3 tomorrow.
> >
> > I already reviewed and applied v2, just waiting for builds to finish,
> > let's leave it.
>
>
> I think clarity should be preferred.

Either way is fine with me.  I'm fine with v2 since it's already applied.

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

end of thread, other threads:[~2020-05-09  4:48 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08 22:53 [PATCH] net: tg3: tidy up loop, remove need to compute off with a multiply Colin King
2020-05-08 22:53 ` Colin King
2020-05-08 23:04 ` Michael Chan
2020-05-08 23:04   ` Michael Chan
2020-05-08 23:21 ` Joe Perches
2020-05-08 23:21   ` Joe Perches
2020-05-08 23:31   ` Colin Ian King
2020-05-08 23:31     ` Colin Ian King
2020-05-09  1:48     ` Jakub Kicinski
2020-05-09  1:48       ` Jakub Kicinski
2020-05-09  2:31       ` Joe Perches
2020-05-09  2:31         ` Joe Perches
2020-05-09  4:48         ` Michael Chan
2020-05-09  4:48           ` Michael Chan

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.