linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition
@ 2011-11-29  0:21 Philipp Dreimann
  2011-11-29  0:42 ` Larry Finger
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Dreimann @ 2011-11-29  0:21 UTC (permalink / raw)
  To: linux-wireless; +Cc: larry.finger, Philipp Dreimann

Hello!

I noticed that ppsc->rfpwr_state could be set to ERFSLEEP even 
though the device is actually in ERFOFF.

Or am I missing something?

BR,
 Philipp

---
 drivers/net/wireless/rtlwifi/rtl8192ce/phy.c |    3 ++-
 drivers/net/wireless/rtlwifi/rtl8192cu/phy.c |    3 ++-
 drivers/net/wireless/rtlwifi/rtl8192de/phy.c |    2 +-
 drivers/net/wireless/rtlwifi/rtl8192se/phy.c |    3 ++-
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
index 592a10a..7f6b933 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
@@ -569,7 +569,8 @@ static bool _rtl92ce_phy_set_rf_power_state(struct ieee80211_hw *hw,
 		}
 	case ERFSLEEP:{
 			if (ppsc->rfpwr_state == ERFOFF)
-				break;
+				return false;
+				
 			for (queue_id = 0, i = 0;
 			     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
 				ring = &pcipriv->dev.tx_ring[queue_id];
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
index 7285290..33ada66 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
@@ -548,7 +548,8 @@ static bool _rtl92cu_phy_set_rf_power_state(struct ieee80211_hw *hw,
 		break;
 	case ERFSLEEP:
 		if (ppsc->rfpwr_state == ERFOFF)
-			break;
+			return false;
+			
 		for (queue_id = 0, i = 0;
 		     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
 			ring = &pcipriv->dev.tx_ring[queue_id];
diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
index 3ac7af1..0883349 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
@@ -3374,7 +3374,7 @@ bool rtl92d_phy_set_rf_power_state(struct ieee80211_hw *hw,
 		break;
 	case ERFSLEEP:
 		if (ppsc->rfpwr_state == ERFOFF)
-			break;
+			return false;
 
 		for (queue_id = 0, i = 0;
 		     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
index f27171a..539172e 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
@@ -602,7 +602,8 @@ bool rtl92s_phy_set_rf_power_state(struct ieee80211_hw *hw,
 		}
 	case ERFSLEEP:
 			if (ppsc->rfpwr_state == ERFOFF)
-				break;
+				return false;
+				
 
 			for (queue_id = 0, i = 0;
 			     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
-- 
1.7.7.3


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

* Re: [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition
  2011-11-29  0:21 [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition Philipp Dreimann
@ 2011-11-29  0:42 ` Larry Finger
  2011-12-06 20:03   ` John W. Linville
  0 siblings, 1 reply; 4+ messages in thread
From: Larry Finger @ 2011-11-29  0:42 UTC (permalink / raw)
  To: Philipp Dreimann; +Cc: linux-wireless, 'Chaoming_Li'

On 11/28/2011 06:21 PM, Philipp Dreimann wrote:
> Hello!
>
> I noticed that ppsc->rfpwr_state could be set to ERFSLEEP even
> though the device is actually in ERFOFF.
>
> Or am I missing something?
>
> BR,
>   Philipp
>
> ---
>   drivers/net/wireless/rtlwifi/rtl8192ce/phy.c |    3 ++-
>   drivers/net/wireless/rtlwifi/rtl8192cu/phy.c |    3 ++-
>   drivers/net/wireless/rtlwifi/rtl8192de/phy.c |    2 +-
>   drivers/net/wireless/rtlwifi/rtl8192se/phy.c |    3 ++-
>   4 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> index 592a10a..7f6b933 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> @@ -569,7 +569,8 @@ static bool _rtl92ce_phy_set_rf_power_state(struct ieee80211_hw *hw,
>   		}
>   	case ERFSLEEP:{
>   			if (ppsc->rfpwr_state == ERFOFF)
> -				break;
> +				return false;
> +				
>   			for (queue_id = 0, i = 0;
>   			     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
>   				ring =&pcipriv->dev.tx_ring[queue_id];
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> index 7285290..33ada66 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> @@ -548,7 +548,8 @@ static bool _rtl92cu_phy_set_rf_power_state(struct ieee80211_hw *hw,
>   		break;
>   	case ERFSLEEP:
>   		if (ppsc->rfpwr_state == ERFOFF)
> -			break;
> +			return false;
> +			
>   		for (queue_id = 0, i = 0;
>   		     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
>   			ring =&pcipriv->dev.tx_ring[queue_id];
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> index 3ac7af1..0883349 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> @@ -3374,7 +3374,7 @@ bool rtl92d_phy_set_rf_power_state(struct ieee80211_hw *hw,
>   		break;
>   	case ERFSLEEP:
>   		if (ppsc->rfpwr_state == ERFOFF)
> -			break;
> +			return false;
>
>   		for (queue_id = 0, i = 0;
>   		     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
> diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> index f27171a..539172e 100644
> --- a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> @@ -602,7 +602,8 @@ bool rtl92s_phy_set_rf_power_state(struct ieee80211_hw *hw,
>   		}
>   	case ERFSLEEP:
>   			if (ppsc->rfpwr_state == ERFOFF)
> -				break;
> +				return false;
> +				
>
>   			for (queue_id = 0, i = 0;
>   			     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {

Yes, it would be possible to jump from ERFSLEEP to ERFOFF in the code.

Chaoming: Is this what you want, or is it a bug?

Thanks,

Larry

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

* Re: [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition
  2011-11-29  0:42 ` Larry Finger
@ 2011-12-06 20:03   ` John W. Linville
  0 siblings, 0 replies; 4+ messages in thread
From: John W. Linville @ 2011-12-06 20:03 UTC (permalink / raw)
  To: Larry Finger; +Cc: Philipp Dreimann, linux-wireless, 'Chaoming_Li'

Still waiting to see Chaoming's answer...  Also, the patch needs
to be posted with a Signed-off-by and without greetings or other
non-pertinent stuff in the changelog.

John

On Mon, Nov 28, 2011 at 06:42:29PM -0600, Larry Finger wrote:
> On 11/28/2011 06:21 PM, Philipp Dreimann wrote:
> >Hello!
> >
> >I noticed that ppsc->rfpwr_state could be set to ERFSLEEP even
> >though the device is actually in ERFOFF.
> >
> >Or am I missing something?
> >
> >BR,
> >  Philipp
> >
> >---
> >  drivers/net/wireless/rtlwifi/rtl8192ce/phy.c |    3 ++-
> >  drivers/net/wireless/rtlwifi/rtl8192cu/phy.c |    3 ++-
> >  drivers/net/wireless/rtlwifi/rtl8192de/phy.c |    2 +-
> >  drivers/net/wireless/rtlwifi/rtl8192se/phy.c |    3 ++-
> >  4 files changed, 7 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> >index 592a10a..7f6b933 100644
> >--- a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> >+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
> >@@ -569,7 +569,8 @@ static bool _rtl92ce_phy_set_rf_power_state(struct ieee80211_hw *hw,
> >  		}
> >  	case ERFSLEEP:{
> >  			if (ppsc->rfpwr_state == ERFOFF)
> >-				break;
> >+				return false;
> >+				
> >  			for (queue_id = 0, i = 0;
> >  			     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
> >  				ring =&pcipriv->dev.tx_ring[queue_id];
> >diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> >index 7285290..33ada66 100644
> >--- a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> >+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
> >@@ -548,7 +548,8 @@ static bool _rtl92cu_phy_set_rf_power_state(struct ieee80211_hw *hw,
> >  		break;
> >  	case ERFSLEEP:
> >  		if (ppsc->rfpwr_state == ERFOFF)
> >-			break;
> >+			return false;
> >+			
> >  		for (queue_id = 0, i = 0;
> >  		     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
> >  			ring =&pcipriv->dev.tx_ring[queue_id];
> >diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> >index 3ac7af1..0883349 100644
> >--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> >+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> >@@ -3374,7 +3374,7 @@ bool rtl92d_phy_set_rf_power_state(struct ieee80211_hw *hw,
> >  		break;
> >  	case ERFSLEEP:
> >  		if (ppsc->rfpwr_state == ERFOFF)
> >-			break;
> >+			return false;
> >
> >  		for (queue_id = 0, i = 0;
> >  		     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
> >diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> >index f27171a..539172e 100644
> >--- a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> >+++ b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
> >@@ -602,7 +602,8 @@ bool rtl92s_phy_set_rf_power_state(struct ieee80211_hw *hw,
> >  		}
> >  	case ERFSLEEP:
> >  			if (ppsc->rfpwr_state == ERFOFF)
> >-				break;
> >+				return false;
> >+				
> >
> >  			for (queue_id = 0, i = 0;
> >  			     queue_id<  RTL_PCI_MAX_TX_QUEUE_COUNT;) {
> 
> Yes, it would be possible to jump from ERFSLEEP to ERFOFF in the code.
> 
> Chaoming: Is this what you want, or is it a bug?
> 
> Thanks,
> 
> Larry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

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

* [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition
@ 2011-12-07  4:15 Larry Finger
  0 siblings, 0 replies; 4+ messages in thread
From: Larry Finger @ 2011-12-07  4:15 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless, Philipp Dreimann

From: Philipp Dreimann <philipp@dreimann.net>

In drivers rtl8192ce, rtl8192cu, rtl8192se, and rtl8192de, missing break 
statements would allow ppsc->rfpwr_state to be changed to ERFSLEEP even though 
the device is actually in ERFOFF.

Signed-off-by: Philipp Dreimann <philipp@dreimann.net>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---

John,

I have not heard anything from Chaoming, but this code certainly looks wrong. 
Please apply to 3.2 if possible.

Larry

---
  drivers/net/wireless/rtlwifi/rtl8192ce/phy.c |    3 ++-
  drivers/net/wireless/rtlwifi/rtl8192cu/phy.c |    3 ++-
  drivers/net/wireless/rtlwifi/rtl8192de/phy.c |    2 +-
  drivers/net/wireless/rtlwifi/rtl8192se/phy.c |    3 ++-
  4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c 
b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
index 592a10a..7f6b933 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/phy.c
@@ -569,7 +569,8 @@ static bool _rtl92ce_phy_set_rf_power_state(struct 
ieee80211_hw *hw,
  		}
  	case ERFSLEEP:{
  			if (ppsc->rfpwr_state == ERFOFF)
-				break;
+				return false;
+				
  			for (queue_id = 0, i = 0;
  			     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
  				ring = &pcipriv->dev.tx_ring[queue_id];
diff --git a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c 
b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
index 7285290..33ada66 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192cu/phy.c
@@ -548,7 +548,8 @@ static bool _rtl92cu_phy_set_rf_power_state(struct 
ieee80211_hw *hw,
  		break;
  	case ERFSLEEP:
  		if (ppsc->rfpwr_state == ERFOFF)
-			break;
+			return false;
+			
  		for (queue_id = 0, i = 0;
  		     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
  			ring = &pcipriv->dev.tx_ring[queue_id];
diff --git a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c 
b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
index 3ac7af1..0883349 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
@@ -3374,7 +3374,7 @@ bool rtl92d_phy_set_rf_power_state(struct ieee80211_hw *hw,
  		break;
  	case ERFSLEEP:
  		if (ppsc->rfpwr_state == ERFOFF)
-			break;
+			return false;
   		for (queue_id = 0, i = 0;
  		     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
diff --git a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c 
b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
index f27171a..539172e 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192se/phy.c
@@ -602,7 +602,8 @@ bool rtl92s_phy_set_rf_power_state(struct ieee80211_hw *hw,
  		}
  	case ERFSLEEP:
  			if (ppsc->rfpwr_state == ERFOFF)
-				break;
+				return false;
+				
   			for (queue_id = 0, i = 0;
  			     queue_id < RTL_PCI_MAX_TX_QUEUE_COUNT;) {
-- 
1.7.7.3



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

end of thread, other threads:[~2011-12-07  4:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-29  0:21 [PATCH] rtl8192{ce,cu,de,se}: avoid problems because of possible ERFOFF -> ERFSLEEP -> .. transition Philipp Dreimann
2011-11-29  0:42 ` Larry Finger
2011-12-06 20:03   ` John W. Linville
  -- strict thread matches above, loose matches on Subject: below --
2011-12-07  4:15 Larry Finger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).