All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rtlwifi: remove redundant assignment to variable badworden
@ 2019-05-30 18:40 ` Colin King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2019-05-30 18:40 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

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

The variable badworden is assigned with a value that is never read and
it is re-assigned a new value immediately afterwards.  The assignment is
redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..37ab582a8afb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -986,7 +986,6 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
 		} else if (write_state == PG_STATE_DATA) {
 			RTPRINT(rtlpriv, FEEPROM, EFUSE_PG,
 				"efuse PG_STATE_DATA\n");
-			badworden = 0x0f;
 			badworden =
 			    enable_efuse_data_write(hw, efuse_addr + 1,
 						    target_pkt.word_en,
-- 
2.20.1


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

* [PATCH] rtlwifi: remove redundant assignment to variable badworden
@ 2019-05-30 18:40 ` Colin King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2019-05-30 18:40 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

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

The variable badworden is assigned with a value that is never read and
it is re-assigned a new value immediately afterwards.  The assignment is
redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..37ab582a8afb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -986,7 +986,6 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
 		} else if (write_state = PG_STATE_DATA) {
 			RTPRINT(rtlpriv, FEEPROM, EFUSE_PG,
 				"efuse PG_STATE_DATA\n");
-			badworden = 0x0f;
 			badworden  			    enable_efuse_data_write(hw, efuse_addr + 1,
 						    target_pkt.word_en,
-- 
2.20.1

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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
  2019-05-30 18:40 ` Colin King
@ 2019-05-30 18:50   ` Larry Finger
  -1 siblings, 0 replies; 14+ messages in thread
From: Larry Finger @ 2019-05-30 18:50 UTC (permalink / raw)
  To: Colin King, Ping-Ke Shih, Kalle Valo, David S . Miller,
	linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On 5/30/19 1:40 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable badworden is assigned with a value that is never read and
> it is re-assigned a new value immediately afterwards.  The assignment is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

>   drivers/net/wireless/realtek/rtlwifi/efuse.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> index e68340dfd980..37ab582a8afb 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> @@ -986,7 +986,6 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
>   		} else if (write_state == PG_STATE_DATA) {
>   			RTPRINT(rtlpriv, FEEPROM, EFUSE_PG,
>   				"efuse PG_STATE_DATA\n");
> -			badworden = 0x0f;
>   			badworden =
>   			    enable_efuse_data_write(hw, efuse_addr + 1,
>   						    target_pkt.word_en,
> 


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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
@ 2019-05-30 18:50   ` Larry Finger
  0 siblings, 0 replies; 14+ messages in thread
From: Larry Finger @ 2019-05-30 18:50 UTC (permalink / raw)
  To: Colin King, Ping-Ke Shih, Kalle Valo, David S . Miller,
	linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On 5/30/19 1:40 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable badworden is assigned with a value that is never read and
> it is re-assigned a new value immediately afterwards.  The assignment is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

>   drivers/net/wireless/realtek/rtlwifi/efuse.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> index e68340dfd980..37ab582a8afb 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> @@ -986,7 +986,6 @@ static int efuse_pg_packet_write(struct ieee80211_hw *hw,
>   		} else if (write_state = PG_STATE_DATA) {
>   			RTPRINT(rtlpriv, FEEPROM, EFUSE_PG,
>   				"efuse PG_STATE_DATA\n");
> -			badworden = 0x0f;
>   			badworden >   			    enable_efuse_data_write(hw, efuse_addr + 1,
>   						    target_pkt.word_en,
> 

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

* [PATCH] rtlwifi: remove redundant assignment to variable k
  2019-05-30 18:40 ` Colin King
@ 2019-05-31 14:14 ` Colin King
  -1 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2019-05-31 14:14 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

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

The assignment of 0 to variable k is never read once we break out of
the loop, so the assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..83e5318ca04f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
 						 rtlpriv->cfg->
 						 maps[EFUSE_CTRL] + 3);
 			k++;
-			if (k == 1000) {
-				k = 0;
+			if (k == 1000)
 				break;
-			}
 		}
 		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
 		return data;
-- 
2.20.1


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

* [PATCH] rtlwifi: remove redundant assignment to variable k
@ 2019-05-31 14:14 ` Colin King
  0 siblings, 0 replies; 14+ messages in thread
From: Colin King @ 2019-05-31 14:14 UTC (permalink / raw)
  To: Ping-Ke Shih, Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

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

The assignment of 0 to variable k is never read once we break out of
the loop, so the assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..83e5318ca04f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
 						 rtlpriv->cfg->
 						 maps[EFUSE_CTRL] + 3);
 			k++;
-			if (k = 1000) {
-				k = 0;
+			if (k = 1000)
 				break;
-			}
 		}
 		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
 		return data;
-- 
2.20.1

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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
  2019-05-31 14:14 ` Colin King
@ 2019-05-31 17:29   ` Larry Finger
  -1 siblings, 0 replies; 14+ messages in thread
From: Larry Finger @ 2019-05-31 17:29 UTC (permalink / raw)
  To: Colin King, Ping-Ke Shih, Kalle Valo, David S . Miller,
	linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On 5/31/19 9:14 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of 0 to variable k is never read once we break out of
> the loop, so the assignment is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> index e68340dfd980..83e5318ca04f 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> @@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
>   						 rtlpriv->cfg->
>   						 maps[EFUSE_CTRL] + 3);
>   			k++;
> -			if (k == 1000) {
> -				k = 0;
> +			if (k == 1000)
>   				break;
> -			}
>   		}
>   		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
>   		return data;

Colin,

Your patch is not wrong, but it fails to address a basic deficiency of this code 
snippet - when an error is detected, there is no attempt to either fix the 
problem or report it upstream. As the data returned will be garbage if this 
condition happens, we might as well replace the "break" with "return 0xFF", as 
well as deleting the "k = 0" line. Most of the callers of efuse_read_1byte() 
ignore the returned result when bits 0 and 4 are set, thus returning all 8 bits 
is not a bad fixup.

My suspicion is that this test is in the code merely to prevent an potential 
unterminated "while" loop, and that this condition is extremely unlikely to happen.

Larry



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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
@ 2019-05-31 17:29   ` Larry Finger
  0 siblings, 0 replies; 14+ messages in thread
From: Larry Finger @ 2019-05-31 17:29 UTC (permalink / raw)
  To: Colin King, Ping-Ke Shih, Kalle Valo, David S . Miller,
	linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On 5/31/19 9:14 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of 0 to variable k is never read once we break out of
> the loop, so the assignment is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> index e68340dfd980..83e5318ca04f 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> @@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
>   						 rtlpriv->cfg->
>   						 maps[EFUSE_CTRL] + 3);
>   			k++;
> -			if (k = 1000) {
> -				k = 0;
> +			if (k = 1000)
>   				break;
> -			}
>   		}
>   		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
>   		return data;

Colin,

Your patch is not wrong, but it fails to address a basic deficiency of this code 
snippet - when an error is detected, there is no attempt to either fix the 
problem or report it upstream. As the data returned will be garbage if this 
condition happens, we might as well replace the "break" with "return 0xFF", as 
well as deleting the "k = 0" line. Most of the callers of efuse_read_1byte() 
ignore the returned result when bits 0 and 4 are set, thus returning all 8 bits 
is not a bad fixup.

My suspicion is that this test is in the code merely to prevent an potential 
unterminated "while" loop, and that this condition is extremely unlikely to happen.

Larry

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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
  2019-05-31 17:29   ` Larry Finger
@ 2019-05-31 19:41     ` Joe Perches
  -1 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-05-31 19:41 UTC (permalink / raw)
  To: Larry Finger, Colin King, Ping-Ke Shih, Kalle Valo,
	David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On Fri, 2019-05-31 at 12:29 -0500, Larry Finger wrote:
> On 5/31/19 9:14 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > The assignment of 0 to variable k is never read once we break out of
> > the loop, so the assignment is redundant and can be removed.
> > 
> > Addresses-Coverity: ("Unused value")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> >   drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > index e68340dfd980..83e5318ca04f 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > @@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
> >   						 rtlpriv->cfg->
> >   						 maps[EFUSE_CTRL] + 3);
> >   			k++;
> > -			if (k == 1000) {
> > -				k = 0;
> > +			if (k == 1000)
> >   				break;
> > -			}
> >   		}
> >   		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
> >   		return data;
> 
> Colin,
> 
> Your patch is not wrong, but it fails to address a basic deficiency of this code 
> snippet - when an error is detected, there is no attempt to either fix the 
> problem or report it upstream. As the data returned will be garbage if this 
> condition happens, we might as well replace the "break" with "return 0xFF", as 
> well as deleting the "k = 0" line. Most of the callers of efuse_read_1byte() 
> ignore the returned result when bits 0 and 4 are set, thus returning all 8 bits 
> is not a bad fixup.
> 
> My suspicion is that this test is in the code merely to prevent an potential 
> unterminated "while" loop, and that this condition is extremely unlikely to happen.
> 
> Larry

The function is also overly verbose with many
unnecessary rtlpriv->cfg->maps dereferences.

I'd've written it more like:
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 57 +++++++++++-----------------
 1 file changed, 22 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..db253f45e87d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -87,46 +87,33 @@ void efuse_initialize(struct ieee80211_hw *hw)
 u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
 {
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
-	u8 data;
 	u8 bytetemp;
 	u8 temp;
-	u32 k = 0;
-	const u32 efuse_len =
-		rtlpriv->cfg->maps[EFUSE_REAL_CONTENT_SIZE];
-
-	if (address < efuse_len) {
-		temp = address & 0xFF;
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 1,
-			       temp);
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 2);
-		temp = ((address >> 8) & 0x03) | (bytetemp & 0xFC);
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 2,
-			       temp);
-
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 3);
-		temp = bytetemp & 0x7F;
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 3,
-			       temp);
+	int k = 0;
+	u32 *maps = rtlpriv->cfg->maps;
+	const u32 efuse_len = maps[EFUSE_REAL_CONTENT_SIZE];
 
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 3);
-		while (!(bytetemp & 0x80)) {
-			bytetemp = rtl_read_byte(rtlpriv,
-						 rtlpriv->cfg->
-						 maps[EFUSE_CTRL] + 3);
-			k++;
-			if (k == 1000) {
-				k = 0;
-				break;
-			}
-		}
-		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
-		return data;
-	} else
+	if (address >= efuse_len)
 		return 0xFF;
 
+	temp = address & 0xFF;
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 1, temp);
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 2);
+	temp = ((address >> 8) & 0x03) | (bytetemp & 0xFC);
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 2, temp);
+
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+	temp = bytetemp & 0x7F;
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 3, temp);
+
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+	while (!(bytetemp & 0x80)) {
+		bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+		if (++k >= 1000)
+			return 0xFF;	/* Likely defect */
+	}
+
+	return rtl_read_byte(rtlpriv, maps[EFUSE_CTRL]);
 }
 EXPORT_SYMBOL(efuse_read_1byte);
 


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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
@ 2019-05-31 19:41     ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-05-31 19:41 UTC (permalink / raw)
  To: Larry Finger, Colin King, Ping-Ke Shih, Kalle Valo,
	David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

On Fri, 2019-05-31 at 12:29 -0500, Larry Finger wrote:
> On 5/31/19 9:14 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> > 
> > The assignment of 0 to variable k is never read once we break out of
> > the loop, so the assignment is redundant and can be removed.
> > 
> > Addresses-Coverity: ("Unused value")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
> >   drivers/net/wireless/realtek/rtlwifi/efuse.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > index e68340dfd980..83e5318ca04f 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
> > @@ -117,10 +117,8 @@ u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
> >   						 rtlpriv->cfg->
> >   						 maps[EFUSE_CTRL] + 3);
> >   			k++;
> > -			if (k = 1000) {
> > -				k = 0;
> > +			if (k = 1000)
> >   				break;
> > -			}
> >   		}
> >   		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
> >   		return data;
> 
> Colin,
> 
> Your patch is not wrong, but it fails to address a basic deficiency of this code 
> snippet - when an error is detected, there is no attempt to either fix the 
> problem or report it upstream. As the data returned will be garbage if this 
> condition happens, we might as well replace the "break" with "return 0xFF", as 
> well as deleting the "k = 0" line. Most of the callers of efuse_read_1byte() 
> ignore the returned result when bits 0 and 4 are set, thus returning all 8 bits 
> is not a bad fixup.
> 
> My suspicion is that this test is in the code merely to prevent an potential 
> unterminated "while" loop, and that this condition is extremely unlikely to happen.
> 
> Larry

The function is also overly verbose with many
unnecessary rtlpriv->cfg->maps dereferences.

I'd've written it more like:
---
 drivers/net/wireless/realtek/rtlwifi/efuse.c | 57 +++++++++++-----------------
 1 file changed, 22 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/efuse.c b/drivers/net/wireless/realtek/rtlwifi/efuse.c
index e68340dfd980..db253f45e87d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/efuse.c
+++ b/drivers/net/wireless/realtek/rtlwifi/efuse.c
@@ -87,46 +87,33 @@ void efuse_initialize(struct ieee80211_hw *hw)
 u8 efuse_read_1byte(struct ieee80211_hw *hw, u16 address)
 {
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
-	u8 data;
 	u8 bytetemp;
 	u8 temp;
-	u32 k = 0;
-	const u32 efuse_len -		rtlpriv->cfg->maps[EFUSE_REAL_CONTENT_SIZE];
-
-	if (address < efuse_len) {
-		temp = address & 0xFF;
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 1,
-			       temp);
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 2);
-		temp = ((address >> 8) & 0x03) | (bytetemp & 0xFC);
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 2,
-			       temp);
-
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 3);
-		temp = bytetemp & 0x7F;
-		rtl_write_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL] + 3,
-			       temp);
+	int k = 0;
+	u32 *maps = rtlpriv->cfg->maps;
+	const u32 efuse_len = maps[EFUSE_REAL_CONTENT_SIZE];
 
-		bytetemp = rtl_read_byte(rtlpriv,
-					 rtlpriv->cfg->maps[EFUSE_CTRL] + 3);
-		while (!(bytetemp & 0x80)) {
-			bytetemp = rtl_read_byte(rtlpriv,
-						 rtlpriv->cfg->
-						 maps[EFUSE_CTRL] + 3);
-			k++;
-			if (k = 1000) {
-				k = 0;
-				break;
-			}
-		}
-		data = rtl_read_byte(rtlpriv, rtlpriv->cfg->maps[EFUSE_CTRL]);
-		return data;
-	} else
+	if (address >= efuse_len)
 		return 0xFF;
 
+	temp = address & 0xFF;
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 1, temp);
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 2);
+	temp = ((address >> 8) & 0x03) | (bytetemp & 0xFC);
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 2, temp);
+
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+	temp = bytetemp & 0x7F;
+	rtl_write_byte(rtlpriv, maps[EFUSE_CTRL] + 3, temp);
+
+	bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+	while (!(bytetemp & 0x80)) {
+		bytetemp = rtl_read_byte(rtlpriv, maps[EFUSE_CTRL] + 3);
+		if (++k >= 1000)
+			return 0xFF;	/* Likely defect */
+	}
+
+	return rtl_read_byte(rtlpriv, maps[EFUSE_CTRL]);
 }
 EXPORT_SYMBOL(efuse_read_1byte);
 

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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
  2019-05-30 18:40 ` Colin King
@ 2019-06-25  4:59   ` Kalle Valo
  -1 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2019-06-25  4:59 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable badworden is assigned with a value that is never read and
> it is re-assigned a new value immediately afterwards.  The assignment is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Patch applied to wireless-drivers-next.git, thanks.

5315f9d40191 rtlwifi: remove redundant assignment to variable badworden

-- 
https://patchwork.kernel.org/patch/10969175/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
@ 2019-06-25  4:59   ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2019-06-25  4:59 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable badworden is assigned with a value that is never read and
> it is re-assigned a new value immediately afterwards.  The assignment is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Patch applied to wireless-drivers-next.git, thanks.

5315f9d40191 rtlwifi: remove redundant assignment to variable badworden

-- 
https://patchwork.kernel.org/patch/10969175/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
  2019-05-31 14:14 ` Colin King
@ 2019-06-25  5:00   ` Kalle Valo
  -1 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2019-06-25  5:00 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of 0 to variable k is never read once we break out of
> the loop, so the assignment is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

f0822dfc5887 rtlwifi: remove redundant assignment to variable k

-- 
https://patchwork.kernel.org/patch/10970261/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


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

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
@ 2019-06-25  5:00   ` Kalle Valo
  0 siblings, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2019-06-25  5:00 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of 0 to variable k is never read once we break out of
> the loop, so the assignment is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

f0822dfc5887 rtlwifi: remove redundant assignment to variable k

-- 
https://patchwork.kernel.org/patch/10970261/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

end of thread, other threads:[~2019-06-25  5:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-30 18:40 [PATCH] rtlwifi: remove redundant assignment to variable badworden Colin King
2019-05-30 18:40 ` Colin King
2019-05-30 18:50 ` Larry Finger
2019-05-30 18:50   ` Larry Finger
2019-06-25  4:59 ` Kalle Valo
2019-06-25  4:59   ` Kalle Valo
2019-05-31 14:14 [PATCH] rtlwifi: remove redundant assignment to variable k Colin King
2019-05-31 14:14 ` Colin King
2019-05-31 17:29 ` Larry Finger
2019-05-31 17:29   ` Larry Finger
2019-05-31 19:41   ` Joe Perches
2019-05-31 19:41     ` Joe Perches
2019-06-25  5:00 ` Kalle Valo
2019-06-25  5:00   ` Kalle Valo

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.