All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] rtl818x_pci: make RSSI code more readable
@ 2014-05-31 16:29 Andrea Merello
  2014-05-31 20:05 ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Andrea Merello @ 2014-05-31 16:29 UTC (permalink / raw)
  To: linville
  Cc: linux-wireless, Larry.Finger, bernhard, dan.carpenter,
	Andrea Merello

remove the if-else chains and use switch-case to make code more
readable and avoiding long lines that broke in several lines

Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
---
 drivers/net/wireless/rtl818x/rtl8180/dev.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
index 2c1c02b..c2dd5e6 100644
--- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
@@ -209,7 +209,7 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
 	struct rtl8180_priv *priv = dev->priv;
 	struct rtl818x_rx_cmd_desc *cmd_desc;
 	unsigned int count = 32;
-	u8 signal, agc, sq;
+	u8 agc, sq, signal = 1;
 	dma_addr_t mapping;
 
 	while (count--) {
@@ -266,18 +266,21 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
 			rx_status.rate_idx = (flags >> 20) & 0xF;
 			agc = (flags2 >> 17) & 0x7F;
 
-			if (priv->chip_family == RTL818X_CHIP_FAMILY_RTL8185) {
+			switch (priv->chip_family) {
+			case RTL818X_CHIP_FAMILY_RTL8185:
 				if (rx_status.rate_idx > 3)
 					signal = 90 - clamp_t(u8, agc, 25, 90);
 				else
 					signal = 95 - clamp_t(u8, agc, 30, 95);
-			} else if (priv->chip_family ==
-				   RTL818X_CHIP_FAMILY_RTL8180) {
+				break;
+			case RTL818X_CHIP_FAMILY_RTL8180:
 				sq = flags2 & 0xff;
 				signal = priv->rf->calc_rssi(agc, sq);
-			} else {
+				break;
+			case RTL818X_CHIP_FAMILY_RTL8187SE:
 				/* TODO: rtl8187se rssi */
 				signal = 10;
+				break;
 			}
 			rx_status.signal = signal;
 			rx_status.freq = dev->conf.chandef.chan->center_freq;
-- 
1.9.1


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

* Re: [PATCH 1/3] rtl818x_pci: make RSSI code more readable
  2014-05-31 16:29 [PATCH 1/3] rtl818x_pci: make RSSI code more readable Andrea Merello
@ 2014-05-31 20:05 ` Dan Carpenter
  2014-06-01  0:37   ` Andrea Merello
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2014-05-31 20:05 UTC (permalink / raw)
  To: Andrea Merello; +Cc: linville, linux-wireless, Larry.Finger, bernhard

On Sat, May 31, 2014 at 06:29:46PM +0200, Andrea Merello wrote:
> remove the if-else chains and use switch-case to make code more
> readable and avoiding long lines that broke in several lines
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> ---
>  drivers/net/wireless/rtl818x/rtl8180/dev.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> index 2c1c02b..c2dd5e6 100644
> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
> @@ -209,7 +209,7 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>  	struct rtl8180_priv *priv = dev->priv;
>  	struct rtl818x_rx_cmd_desc *cmd_desc;
>  	unsigned int count = 32;
> -	u8 signal, agc, sq;
> +	u8 agc, sq, signal = 1;

Shouldn't the 1 be 10?  Why did the defaults change?

regards,
dan carpenter

>  	dma_addr_t mapping;
>  
>  	while (count--) {
> @@ -266,18 +266,21 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>  			rx_status.rate_idx = (flags >> 20) & 0xF;
>  			agc = (flags2 >> 17) & 0x7F;
>  
> -			if (priv->chip_family == RTL818X_CHIP_FAMILY_RTL8185) {
> +			switch (priv->chip_family) {
> +			case RTL818X_CHIP_FAMILY_RTL8185:
>  				if (rx_status.rate_idx > 3)
>  					signal = 90 - clamp_t(u8, agc, 25, 90);
>  				else
>  					signal = 95 - clamp_t(u8, agc, 30, 95);
> -			} else if (priv->chip_family ==
> -				   RTL818X_CHIP_FAMILY_RTL8180) {
> +				break;
> +			case RTL818X_CHIP_FAMILY_RTL8180:
>  				sq = flags2 & 0xff;
>  				signal = priv->rf->calc_rssi(agc, sq);
> -			} else {
> +				break;
> +			case RTL818X_CHIP_FAMILY_RTL8187SE:
>  				/* TODO: rtl8187se rssi */
>  				signal = 10;
> +				break;
>  			}
>  			rx_status.signal = signal;
>  			rx_status.freq = dev->conf.chandef.chan->center_freq;
> -- 
> 1.9.1

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

* Re: [PATCH 1/3] rtl818x_pci: make RSSI code more readable
  2014-05-31 20:05 ` Dan Carpenter
@ 2014-06-01  0:37   ` Andrea Merello
  2014-06-01  5:26     ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Andrea Merello @ 2014-06-01  0:37 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: John Linville, Linux Wireless List, Larry Finger,
	Bernhard Schiffner

IHMO it does not matter.

The 10 is a stub value for rtl8187se (because of not-implemented-yet
RSSI calculation), and indeed it is still here, in the rtl8187se case.
This has the functional role of providing a valid >0 value for reported RSSI.
This should hopefully go away soon :)
So, for rtl8187se case, it always reported 10, and it does continue to do this.

The initialization to 1 has no any functional role, it is just to shut
up gcc complaints about not initialized variable, but it should be
always overwritten by one of the three cases, and we should never see
this value really reported on RSSI measure.

Indeed they are two random values (chosen >0, just in case it does matter..)..

Andrea

On Sat, May 31, 2014 at 10:05 PM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> On Sat, May 31, 2014 at 06:29:46PM +0200, Andrea Merello wrote:
>> remove the if-else chains and use switch-case to make code more
>> readable and avoiding long lines that broke in several lines
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> ---
>>  drivers/net/wireless/rtl818x/rtl8180/dev.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rtl818x/rtl8180/dev.c b/drivers/net/wireless/rtl818x/rtl8180/dev.c
>> index 2c1c02b..c2dd5e6 100644
>> --- a/drivers/net/wireless/rtl818x/rtl8180/dev.c
>> +++ b/drivers/net/wireless/rtl818x/rtl8180/dev.c
>> @@ -209,7 +209,7 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>>       struct rtl8180_priv *priv = dev->priv;
>>       struct rtl818x_rx_cmd_desc *cmd_desc;
>>       unsigned int count = 32;
>> -     u8 signal, agc, sq;
>> +     u8 agc, sq, signal = 1;
>
> Shouldn't the 1 be 10?  Why did the defaults change?
>
> regards,
> dan carpenter
>
>>       dma_addr_t mapping;
>>
>>       while (count--) {
>> @@ -266,18 +266,21 @@ static void rtl8180_handle_rx(struct ieee80211_hw *dev)
>>                       rx_status.rate_idx = (flags >> 20) & 0xF;
>>                       agc = (flags2 >> 17) & 0x7F;
>>
>> -                     if (priv->chip_family == RTL818X_CHIP_FAMILY_RTL8185) {
>> +                     switch (priv->chip_family) {
>> +                     case RTL818X_CHIP_FAMILY_RTL8185:
>>                               if (rx_status.rate_idx > 3)
>>                                       signal = 90 - clamp_t(u8, agc, 25, 90);
>>                               else
>>                                       signal = 95 - clamp_t(u8, agc, 30, 95);
>> -                     } else if (priv->chip_family ==
>> -                                RTL818X_CHIP_FAMILY_RTL8180) {
>> +                             break;
>> +                     case RTL818X_CHIP_FAMILY_RTL8180:
>>                               sq = flags2 & 0xff;
>>                               signal = priv->rf->calc_rssi(agc, sq);
>> -                     } else {
>> +                             break;
>> +                     case RTL818X_CHIP_FAMILY_RTL8187SE:
>>                               /* TODO: rtl8187se rssi */
>>                               signal = 10;
>> +                             break;
>>                       }
>>                       rx_status.signal = signal;
>>                       rx_status.freq = dev->conf.chandef.chan->center_freq;
>> --
>> 1.9.1

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

* Re: [PATCH 1/3] rtl818x_pci: make RSSI code more readable
  2014-06-01  0:37   ` Andrea Merello
@ 2014-06-01  5:26     ` Dan Carpenter
  0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2014-06-01  5:26 UTC (permalink / raw)
  To: Andrea Merello
  Cc: John Linville, Linux Wireless List, Larry Finger,
	Bernhard Schiffner

On Sun, Jun 01, 2014 at 02:37:51AM +0200, Andrea Merello wrote:
> IHMO it does not matter.
> 
> The 10 is a stub value for rtl8187se (because of not-implemented-yet
> RSSI calculation), and indeed it is still here, in the rtl8187se case.
> This has the functional role of providing a valid >0 value for reported RSSI.
> This should hopefully go away soon :)
> So, for rtl8187se case, it always reported 10, and it does continue to do this.
> 
> The initialization to 1 has no any functional role, it is just to shut
> up gcc complaints about not initialized variable, but it should be
> always overwritten by one of the three cases, and we should never see
> this value really reported on RSSI measure.
> 
> Indeed they are two random values (chosen >0, just in case it does matter..)..
> 

Ah.  Ok.  Thanks.

regards,
dan carpenter


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

end of thread, other threads:[~2014-06-01  5:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-31 16:29 [PATCH 1/3] rtl818x_pci: make RSSI code more readable Andrea Merello
2014-05-31 20:05 ` Dan Carpenter
2014-06-01  0:37   ` Andrea Merello
2014-06-01  5:26     ` Dan Carpenter

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.