* [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function
2012-02-13 4:48 [PATCH v2 0/4] rt2x00: Update MCU operations during device initialization Jakub Kicinski
@ 2012-02-13 4:48 ` Jakub Kicinski
2012-02-14 10:47 ` [rt2x00-users] " Gertjan van Wingerde
2012-02-13 4:48 ` [PATCH v2 2/4] rt2800usb: remove rt2800usb_set_state function Jakub Kicinski
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-13 4:48 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, users, Jakub Kicinski
rt2800pci_set_state was introduced to group requests preceded with
common introduction. As that introduction is no longer used
rt2800pci_set_state can be removed.
Note that this patch changes behaviour for STATE_DEEP_SLEEP and
STATE_STANDBY, but those states are not used in rt2800 anyway.
Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 38 ++++++++++--------------------
1 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 6ad6929..6ab4637 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -517,23 +517,6 @@ static void rt2800pci_disable_radio(struct rt2x00_dev *rt2x00dev)
}
}
-static int rt2800pci_set_state(struct rt2x00_dev *rt2x00dev,
- enum dev_state state)
-{
- if (state == STATE_AWAKE) {
- rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
- rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
- } else if (state == STATE_SLEEP) {
- rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS,
- 0xffffffff);
- rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID,
- 0xffffffff);
- rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
- }
-
- return 0;
-}
-
static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
enum dev_state state)
{
@@ -546,7 +529,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
* to be woken up. After that it needs a bit of time
* to be fully awake and then the radio can be enabled.
*/
- rt2800pci_set_state(rt2x00dev, STATE_AWAKE);
+ rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
msleep(1);
retval = rt2800pci_enable_radio(rt2x00dev);
break;
@@ -556,17 +539,22 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
* be put to sleep for powersaving.
*/
rt2800pci_disable_radio(rt2x00dev);
- rt2800pci_set_state(rt2x00dev, STATE_SLEEP);
- break;
- case STATE_RADIO_IRQ_ON:
- case STATE_RADIO_IRQ_OFF:
- rt2800pci_toggle_irq(rt2x00dev, state);
- break;
+ /* fall through */
case STATE_DEEP_SLEEP:
case STATE_SLEEP:
case STATE_STANDBY:
+ /* PCIe devices won't report status after SLEEP request. */
+ rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
+ rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
+ rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
+ break;
case STATE_AWAKE:
- retval = rt2800pci_set_state(rt2x00dev, state);
+ rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
+ rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
+ break;
+ case STATE_RADIO_IRQ_ON:
+ case STATE_RADIO_IRQ_OFF:
+ rt2800pci_toggle_irq(rt2x00dev, state);
break;
default:
retval = -ENOTSUPP;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function
2012-02-13 4:48 ` [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function Jakub Kicinski
@ 2012-02-14 10:47 ` Gertjan van Wingerde
2012-02-14 17:05 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Gertjan van Wingerde @ 2012-02-14 10:47 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: linville, linux-wireless, users
On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> rt2800pci_set_state was introduced to group requests preceded with
> common introduction. As that introduction is no longer used
> rt2800pci_set_state can be removed.
>
> Note that this patch changes behaviour for STATE_DEEP_SLEEP and
> STATE_STANDBY, but those states are not used in rt2800 anyway.
>
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
I know this patch has been out there for a while, but I only now have
taken a good look at it.
First of all, I think that it would have been easy to not change the
behaviour for STATE_DEEP_SLEEP
and STATE_STANDBY, even though these are not used in rt2800. They can
simply be put as their
own cases in the switch statement in which nothing is done.
Also some comments to the actual code below.
> ---
> drivers/net/wireless/rt2x00/rt2800pci.c | 38 ++++++++++--------------------
> 1 files changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 6ad6929..6ab4637 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -517,23 +517,6 @@ static void rt2800pci_disable_radio(struct rt2x00_dev *rt2x00dev)
> }
> }
>
> -static int rt2800pci_set_state(struct rt2x00_dev *rt2x00dev,
> - enum dev_state state)
> -{
> - if (state == STATE_AWAKE) {
> - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
> - rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
> - } else if (state == STATE_SLEEP) {
> - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS,
> - 0xffffffff);
> - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID,
> - 0xffffffff);
> - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
> - }
> -
> - return 0;
> -}
> -
> static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> enum dev_state state)
> {
> @@ -546,7 +529,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> * to be woken up. After that it needs a bit of time
> * to be fully awake and then the radio can be enabled.
> */
> - rt2800pci_set_state(rt2x00dev, STATE_AWAKE);
> + rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
> msleep(1);
> retval = rt2800pci_enable_radio(rt2x00dev);
> break;
I don't like this recursive call, but this is removed in patch 4, so I
don't mind it here.
> @@ -556,17 +539,22 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> * be put to sleep for powersaving.
> */
> rt2800pci_disable_radio(rt2x00dev);
> - rt2800pci_set_state(rt2x00dev, STATE_SLEEP);
> - break;
> - case STATE_RADIO_IRQ_ON:
> - case STATE_RADIO_IRQ_OFF:
> - rt2800pci_toggle_irq(rt2x00dev, state);
> - break;
> + /* fall through */
I don't really like this falling through, which is just there out of
convenience, as the states have
some common code. However, in reality these states have not a lot in
common, so just a bit
of code duplication would have been better here, IMHO.
> case STATE_DEEP_SLEEP:
> case STATE_SLEEP:
> case STATE_STANDBY:
> + /* PCIe devices won't report status after SLEEP request. */
> + rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
> + rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
> + rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
> + break;
> case STATE_AWAKE:
> - retval = rt2800pci_set_state(rt2x00dev, state);
> + rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
> + rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
> + break;
> + case STATE_RADIO_IRQ_ON:
> + case STATE_RADIO_IRQ_OFF:
> + rt2800pci_toggle_irq(rt2x00dev, state);
> break;
> default:
> retval = -ENOTSUPP;
---
Gertjan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function
2012-02-14 10:47 ` [rt2x00-users] " Gertjan van Wingerde
@ 2012-02-14 17:05 ` Jakub Kicinski
2012-02-15 15:42 ` Gertjan van Wingerde
0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-14 17:05 UTC (permalink / raw)
To: Gertjan van Wingerde; +Cc: linux-wireless, users
On Tue, 14 Feb 2012 11:47:37 +0100 Gertjan van Wingerde <gwingerde@gmail.com> wrote:
> On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > rt2800pci_set_state was introduced to group requests preceded with
> > common introduction. As that introduction is no longer used
> > rt2800pci_set_state can be removed.
> >
> > Note that this patch changes behaviour for STATE_DEEP_SLEEP and
> > STATE_STANDBY, but those states are not used in rt2800 anyway.
> >
> > Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
>
> I know this patch has been out there for a while, but I only now have
> taken a good look at it.
> First of all, I think that it would have been easy to not change the
> behaviour for STATE_DEEP_SLEEP
> and STATE_STANDBY, even though these are not used in rt2800. They can
> simply be put as their
> own cases in the switch statement in which nothing is done.
True, I have decided to make the change because:
1) I couldn't find any reason to treat those states differently than
normal SLEEP;
2) rt61pci and rt2500pci treats them equally;
3) the differentiation was introduced by Jay in 7f6e144f and the change
looks like it may have not been 100% intentional. At least my newbie
mind is telling me that. No one asked about it at the time and commit
message also doesn't explain this change.
Please let me know if this reasoning makes any sense to you.
> Also some comments to the actual code below.
>
> > ---
> > drivers/net/wireless/rt2x00/rt2800pci.c | 38 ++++++++++--------------------
> > 1 files changed, 13 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> > index 6ad6929..6ab4637 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> > @@ -517,23 +517,6 @@ static void rt2800pci_disable_radio(struct rt2x00_dev *rt2x00dev)
> > }
> > }
> >
> > -static int rt2800pci_set_state(struct rt2x00_dev *rt2x00dev,
> > - enum dev_state state)
> > -{
> > - if (state == STATE_AWAKE) {
> > - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
> > - rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
> > - } else if (state == STATE_SLEEP) {
> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS,
> > - 0xffffffff);
> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID,
> > - 0xffffffff);
> > - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> > enum dev_state state)
> > {
> > @@ -546,7 +529,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> > * to be woken up. After that it needs a bit of time
> > * to be fully awake and then the radio can be enabled.
> > */
> > - rt2800pci_set_state(rt2x00dev, STATE_AWAKE);
> > + rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
> > msleep(1);
> > retval = rt2800pci_enable_radio(rt2x00dev);
> > break;
>
> I don't like this recursive call, but this is removed in patch 4, so I
> don't mind it here.
>
> > @@ -556,17 +539,22 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> > * be put to sleep for powersaving.
> > */
> > rt2800pci_disable_radio(rt2x00dev);
> > - rt2800pci_set_state(rt2x00dev, STATE_SLEEP);
> > - break;
> > - case STATE_RADIO_IRQ_ON:
> > - case STATE_RADIO_IRQ_OFF:
> > - rt2800pci_toggle_irq(rt2x00dev, state);
> > - break;
> > + /* fall through */
>
> I don't really like this falling through, which is just there out of
> convenience, as the states have
> some common code. However, in reality these states have not a lot in
> common, so just a bit
> of code duplication would have been better here, IMHO.
Ok, I will change that and perhaps put that "duplicated" code into
rt2800pci_disable_radio to keep switch statement short.
-- Kuba
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function
2012-02-14 17:05 ` Jakub Kicinski
@ 2012-02-15 15:42 ` Gertjan van Wingerde
2012-02-15 19:10 ` Jakub Kicinski
0 siblings, 1 reply; 12+ messages in thread
From: Gertjan van Wingerde @ 2012-02-15 15:42 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: linux-wireless, users
On Tue, Feb 14, 2012 at 6:05 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Tue, 14 Feb 2012 11:47:37 +0100 Gertjan van Wingerde <gwingerde@gmail.com> wrote:
>> On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
>> > rt2800pci_set_state was introduced to group requests preceded with
>> > common introduction. As that introduction is no longer used
>> > rt2800pci_set_state can be removed.
>> >
>> > Note that this patch changes behaviour for STATE_DEEP_SLEEP and
>> > STATE_STANDBY, but those states are not used in rt2800 anyway.
>> >
>> > Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
>>
>> I know this patch has been out there for a while, but I only now have
>> taken a good look at it.
>> First of all, I think that it would have been easy to not change the
>> behaviour for STATE_DEEP_SLEEP
>> and STATE_STANDBY, even though these are not used in rt2800. They can
>> simply be put as their
>> own cases in the switch statement in which nothing is done.
>
> True, I have decided to make the change because:
> 1) I couldn't find any reason to treat those states differently than
> normal SLEEP;
> 2) rt61pci and rt2500pci treats them equally;
> 3) the differentiation was introduced by Jay in 7f6e144f and the change
> looks like it may have not been 100% intentional. At least my newbie
> mind is telling me that. No one asked about it at the time and commit
> message also doesn't explain this change.
>
> Please let me know if this reasoning makes any sense to you.
OK. Looking at this closer it actually looks like that these two
states are not used at
all in rt2x00. So, I guess I can be fine with this.
>
>> Also some comments to the actual code below.
>>
>> > ---
>> > drivers/net/wireless/rt2x00/rt2800pci.c | 38 ++++++++++--------------------
>> > 1 files changed, 13 insertions(+), 25 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
>> > index 6ad6929..6ab4637 100644
>> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
>> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
>> > @@ -517,23 +517,6 @@ static void rt2800pci_disable_radio(struct rt2x00_dev *rt2x00dev)
>> > }
>> > }
>> >
>> > -static int rt2800pci_set_state(struct rt2x00_dev *rt2x00dev,
>> > - enum dev_state state)
>> > -{
>> > - if (state == STATE_AWAKE) {
>> > - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
>> > - rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
>> > - } else if (state == STATE_SLEEP) {
>> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS,
>> > - 0xffffffff);
>> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID,
>> > - 0xffffffff);
>> > - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
>> > - }
>> > -
>> > - return 0;
>> > -}
>> > -
>> > static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>> > enum dev_state state)
>> > {
>> > @@ -546,7 +529,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>> > * to be woken up. After that it needs a bit of time
>> > * to be fully awake and then the radio can be enabled.
>> > */
>> > - rt2800pci_set_state(rt2x00dev, STATE_AWAKE);
>> > + rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
>> > msleep(1);
>> > retval = rt2800pci_enable_radio(rt2x00dev);
>> > break;
>>
>> I don't like this recursive call, but this is removed in patch 4, so I
>> don't mind it here.
>>
>> > @@ -556,17 +539,22 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>> > * be put to sleep for powersaving.
>> > */
>> > rt2800pci_disable_radio(rt2x00dev);
>> > - rt2800pci_set_state(rt2x00dev, STATE_SLEEP);
>> > - break;
>> > - case STATE_RADIO_IRQ_ON:
>> > - case STATE_RADIO_IRQ_OFF:
>> > - rt2800pci_toggle_irq(rt2x00dev, state);
>> > - break;
>> > + /* fall through */
>>
>> I don't really like this falling through, which is just there out of
>> convenience, as the states have
>> some common code. However, in reality these states have not a lot in
>> common, so just a bit
>> of code duplication would have been better here, IMHO.
>
> Ok, I will change that and perhaps put that "duplicated" code into
> rt2800pci_disable_radio to keep switch statement short.
>
Fine with me. Maybe the same could be done with respect to the new code
and rt2800pci_enable_radio.
---
Gertjan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function
2012-02-15 15:42 ` Gertjan van Wingerde
@ 2012-02-15 19:10 ` Jakub Kicinski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-15 19:10 UTC (permalink / raw)
To: Gertjan van Wingerde; +Cc: linux-wireless, users
On Wed, 15 Feb 2012 16:42:49 +0100 Gertjan van Wingerde <gwingerde@gmail.com> wrote:
> On Tue, Feb 14, 2012 at 6:05 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Tue, 14 Feb 2012 11:47:37 +0100 Gertjan van Wingerde <gwingerde@gmail.com> wrote:
> >> On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> >> > rt2800pci_set_state was introduced to group requests preceded with
> >> > common introduction. As that introduction is no longer used
> >> > rt2800pci_set_state can be removed.
> >> >
> >> > Note that this patch changes behaviour for STATE_DEEP_SLEEP and
> >> > STATE_STANDBY, but those states are not used in rt2800 anyway.
> >> >
> >> > Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> >>
> >> I know this patch has been out there for a while, but I only now have
> >> taken a good look at it.
> >> First of all, I think that it would have been easy to not change the
> >> behaviour for STATE_DEEP_SLEEP
> >> and STATE_STANDBY, even though these are not used in rt2800. They can
> >> simply be put as their
> >> own cases in the switch statement in which nothing is done.
> >
> > True, I have decided to make the change because:
> > 1) I couldn't find any reason to treat those states differently than
> > normal SLEEP;
> > 2) rt61pci and rt2500pci treats them equally;
> > 3) the differentiation was introduced by Jay in 7f6e144f and the change
> > looks like it may have not been 100% intentional. At least my newbie
> > mind is telling me that. No one asked about it at the time and commit
> > message also doesn't explain this change.
> >
> > Please let me know if this reasoning makes any sense to you.
>
> OK. Looking at this closer it actually looks like that these two
> states are not used at
> all in rt2x00. So, I guess I can be fine with this.
>
> >
> >> Also some comments to the actual code below.
> >>
> >> > ---
> >> > drivers/net/wireless/rt2x00/rt2800pci.c | 38 ++++++++++--------------------
> >> > 1 files changed, 13 insertions(+), 25 deletions(-)
> >> >
> >> > diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> >> > index 6ad6929..6ab4637 100644
> >> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> >> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> >> > @@ -517,23 +517,6 @@ static void rt2800pci_disable_radio(struct rt2x00_dev *rt2x00dev)
> >> > }
> >> > }
> >> >
> >> > -static int rt2800pci_set_state(struct rt2x00_dev *rt2x00dev,
> >> > - enum dev_state state)
> >> > -{
> >> > - if (state == STATE_AWAKE) {
> >> > - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
> >> > - rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
> >> > - } else if (state == STATE_SLEEP) {
> >> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS,
> >> > - 0xffffffff);
> >> > - rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID,
> >> > - 0xffffffff);
> >> > - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
> >> > - }
> >> > -
> >> > - return 0;
> >> > -}
> >> > -
> >> > static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> >> > enum dev_state state)
> >> > {
> >> > @@ -546,7 +529,7 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> >> > * to be woken up. After that it needs a bit of time
> >> > * to be fully awake and then the radio can be enabled.
> >> > */
> >> > - rt2800pci_set_state(rt2x00dev, STATE_AWAKE);
> >> > + rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
> >> > msleep(1);
> >> > retval = rt2800pci_enable_radio(rt2x00dev);
> >> > break;
> >>
> >> I don't like this recursive call, but this is removed in patch 4, so I
> >> don't mind it here.
> >>
> >> > @@ -556,17 +539,22 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> >> > * be put to sleep for powersaving.
> >> > */
> >> > rt2800pci_disable_radio(rt2x00dev);
> >> > - rt2800pci_set_state(rt2x00dev, STATE_SLEEP);
> >> > - break;
> >> > - case STATE_RADIO_IRQ_ON:
> >> > - case STATE_RADIO_IRQ_OFF:
> >> > - rt2800pci_toggle_irq(rt2x00dev, state);
> >> > - break;
> >> > + /* fall through */
> >>
> >> I don't really like this falling through, which is just there out of
> >> convenience, as the states have
> >> some common code. However, in reality these states have not a lot in
> >> common, so just a bit
> >> of code duplication would have been better here, IMHO.
> >
> > Ok, I will change that and perhaps put that "duplicated" code into
> > rt2800pci_disable_radio to keep switch statement short.
> >
>
> Fine with me. Maybe the same could be done with respect to the new code
> and rt2800pci_enable_radio.
Well, that would be nice. Maybe the right way is to move all the
additional code from the switch to appropriate functions and create a
function for SLEEP as well (so it could be called from disable_radio
without recursion), leaving nice and concise switch without duplicated
code. However, that would probably had to be done in rt2800usb as well
causing even more code-moving and patches (move something out of switch
and remove an unneeded function aren't the same thing, right?) and the
purpose of this patch was originally to get rid of a simple error! ;-)
-- Kuba
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] rt2800usb: remove rt2800usb_set_state function
2012-02-13 4:48 [PATCH v2 0/4] rt2x00: Update MCU operations during device initialization Jakub Kicinski
2012-02-13 4:48 ` [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function Jakub Kicinski
@ 2012-02-13 4:48 ` Jakub Kicinski
2012-02-14 10:48 ` [rt2x00-users] " Gertjan van Wingerde
2012-02-13 4:48 ` [PATCH v2 3/4] rt2800: Add documentation on MCU requests Jakub Kicinski
2012-02-13 4:48 ` [PATCH v2 4/4] rt2800pci: Fix 'Error - MCU request failed' during initialization Jakub Kicinski
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-13 4:48 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, users, Jakub Kicinski
Remove rt2800usb_set_state to make the flow inside USB driver
similar to that of PCI driver.
Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
---
drivers/net/wireless/rt2x00/rt2800usb.c | 28 +++++++++-------------------
1 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index d009b6b..15cce5e 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -272,17 +272,6 @@ static void rt2800usb_disable_radio(struct rt2x00_dev *rt2x00dev)
rt2x00usb_disable_radio(rt2x00dev);
}
-static int rt2800usb_set_state(struct rt2x00_dev *rt2x00dev,
- enum dev_state state)
-{
- if (state == STATE_AWAKE)
- rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
- else
- rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0xff, 0xff, 2);
-
- return 0;
-}
-
static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
enum dev_state state)
{
@@ -295,7 +284,7 @@ static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
* to be woken up. After that it needs a bit of time
* to be fully awake and then the radio can be enabled.
*/
- rt2800usb_set_state(rt2x00dev, STATE_AWAKE);
+ rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
msleep(1);
retval = rt2800usb_enable_radio(rt2x00dev);
break;
@@ -305,17 +294,18 @@ static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
* be put to sleep for powersaving.
*/
rt2800usb_disable_radio(rt2x00dev);
- rt2800usb_set_state(rt2x00dev, STATE_SLEEP);
- break;
- case STATE_RADIO_IRQ_ON:
- case STATE_RADIO_IRQ_OFF:
- /* No support, but no error either */
- break;
+ /* fall through */
case STATE_DEEP_SLEEP:
case STATE_SLEEP:
case STATE_STANDBY:
+ rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0xff, 0xff, 2);
+ break;
case STATE_AWAKE:
- retval = rt2800usb_set_state(rt2x00dev, state);
+ rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
+ break;
+ case STATE_RADIO_IRQ_ON:
+ case STATE_RADIO_IRQ_OFF:
+ /* No support, but no error either */
break;
default:
retval = -ENOTSUPP;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 2/4] rt2800usb: remove rt2800usb_set_state function
2012-02-13 4:48 ` [PATCH v2 2/4] rt2800usb: remove rt2800usb_set_state function Jakub Kicinski
@ 2012-02-14 10:48 ` Gertjan van Wingerde
0 siblings, 0 replies; 12+ messages in thread
From: Gertjan van Wingerde @ 2012-02-14 10:48 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: linville, linux-wireless, users
On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> Remove rt2800usb_set_state to make the flow inside USB driver
> similar to that of PCI driver.
>
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
The same comment with respect to falling through the state cases as
for patch 1 of the
series applies here.
> ---
> drivers/net/wireless/rt2x00/rt2800usb.c | 28 +++++++++-------------------
> 1 files changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
> index d009b6b..15cce5e 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -272,17 +272,6 @@ static void rt2800usb_disable_radio(struct rt2x00_dev *rt2x00dev)
> rt2x00usb_disable_radio(rt2x00dev);
> }
>
> -static int rt2800usb_set_state(struct rt2x00_dev *rt2x00dev,
> - enum dev_state state)
> -{
> - if (state == STATE_AWAKE)
> - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
> - else
> - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0xff, 0xff, 2);
> -
> - return 0;
> -}
> -
> static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
> enum dev_state state)
> {
> @@ -295,7 +284,7 @@ static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
> * to be woken up. After that it needs a bit of time
> * to be fully awake and then the radio can be enabled.
> */
> - rt2800usb_set_state(rt2x00dev, STATE_AWAKE);
> + rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
> msleep(1);
> retval = rt2800usb_enable_radio(rt2x00dev);
> break;
> @@ -305,17 +294,18 @@ static int rt2800usb_set_device_state(struct rt2x00_dev *rt2x00dev,
> * be put to sleep for powersaving.
> */
> rt2800usb_disable_radio(rt2x00dev);
> - rt2800usb_set_state(rt2x00dev, STATE_SLEEP);
> - break;
> - case STATE_RADIO_IRQ_ON:
> - case STATE_RADIO_IRQ_OFF:
> - /* No support, but no error either */
> - break;
> + /* fall through */
> case STATE_DEEP_SLEEP:
> case STATE_SLEEP:
> case STATE_STANDBY:
> + rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0xff, 0xff, 2);
> + break;
> case STATE_AWAKE:
> - retval = rt2800usb_set_state(rt2x00dev, state);
> + rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, 0xff, 0, 2);
> + break;
> + case STATE_RADIO_IRQ_ON:
> + case STATE_RADIO_IRQ_OFF:
> + /* No support, but no error either */
> break;
> default:
> retval = -ENOTSUPP;
---
Gertjan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] rt2800: Add documentation on MCU requests
2012-02-13 4:48 [PATCH v2 0/4] rt2x00: Update MCU operations during device initialization Jakub Kicinski
2012-02-13 4:48 ` [PATCH v2 1/4] rt2800pci: remove rt2800pci_set_state function Jakub Kicinski
2012-02-13 4:48 ` [PATCH v2 2/4] rt2800usb: remove rt2800usb_set_state function Jakub Kicinski
@ 2012-02-13 4:48 ` Jakub Kicinski
2012-02-14 10:50 ` [rt2x00-users] " Gertjan van Wingerde
2012-02-13 4:48 ` [PATCH v2 4/4] rt2800pci: Fix 'Error - MCU request failed' during initialization Jakub Kicinski
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-13 4:48 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, users, Jakub Kicinski
Add documentation on MCU communication, some of known commands and
their arguments. Supplement command ids.
Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
---
drivers/net/wireless/rt2x00/rt2800.h | 14 +++++++++++++-
drivers/net/wireless/rt2x00/rt2800pci.c | 8 +++++---
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800.h b/drivers/net/wireless/rt2x00/rt2800.h
index 06acabd..4d771db 100644
--- a/drivers/net/wireless/rt2x00/rt2800.h
+++ b/drivers/net/wireless/rt2x00/rt2800.h
@@ -1627,6 +1627,7 @@ struct mac_iveiv_entry {
/*
* H2M_MAILBOX_CSR: Host-to-MCU Mailbox.
+ * CMD_TOKEN: Command id, 0xff disable status reporting
*/
#define H2M_MAILBOX_CSR 0x7010
#define H2M_MAILBOX_CSR_ARG0 FIELD32(0x000000ff)
@@ -1636,6 +1637,8 @@ struct mac_iveiv_entry {
/*
* H2M_MAILBOX_CID:
+ * Free slots contain 0xff. MCU will store command's token to lowest free slot.
+ * If all slots are occupied status will be dropped.
*/
#define H2M_MAILBOX_CID 0x7014
#define H2M_MAILBOX_CID_CMD0 FIELD32(0x000000ff)
@@ -1645,6 +1648,7 @@ struct mac_iveiv_entry {
/*
* H2M_MAILBOX_STATUS:
+ * Command status will be saved to same slot as command id.
*/
#define H2M_MAILBOX_STATUS 0x701c
@@ -2288,6 +2292,12 @@ struct mac_iveiv_entry {
/*
* MCU mailbox commands.
+ * MCU_SLEEP - go to power-save mode.
+ * arg1: 1: save as much power as possible, 0: save less power
+ * status: 1: success, 2: already asleep,
+ * 3: maybe MAC is busy so can't finish this task
+ * MCU_RADIO_OFF
+ * arg0: 0: do power-saving, NOT turn off radio
*/
#define MCU_SLEEP 0x30
#define MCU_WAKEUP 0x31
@@ -2308,7 +2318,9 @@ struct mac_iveiv_entry {
/*
* MCU mailbox tokens
*/
-#define TOKEN_WAKUP 3
+#define TOKEN_SLEEP 1
+#define TOKEN_RADIO_OFF 2
+#define TOKEN_WAKEUP 3
/*
* DMA descriptor defines.
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 6ab4637..f184a4f 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -546,11 +546,13 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
/* PCIe devices won't report status after SLEEP request. */
rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
- rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
+ rt2800_mcu_request(rt2x00dev, MCU_SLEEP, TOKEN_SLEEP,
+ 0xff, 0x01);
break;
case STATE_AWAKE:
- rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
- rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
+ rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKEUP,
+ 0, 0x02);
+ rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKEUP);
break;
case STATE_RADIO_IRQ_ON:
case STATE_RADIO_IRQ_OFF:
--
1.7.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 3/4] rt2800: Add documentation on MCU requests
2012-02-13 4:48 ` [PATCH v2 3/4] rt2800: Add documentation on MCU requests Jakub Kicinski
@ 2012-02-14 10:50 ` Gertjan van Wingerde
0 siblings, 0 replies; 12+ messages in thread
From: Gertjan van Wingerde @ 2012-02-14 10:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: linville, linux-wireless, users
On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> Add documentation on MCU communication, some of known commands and
> their arguments. Supplement command ids.
>
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
provided that my comments to patches 1 & 2 are handled.
> ---
> drivers/net/wireless/rt2x00/rt2800.h | 14 +++++++++++++-
> drivers/net/wireless/rt2x00/rt2800pci.c | 8 +++++---
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800.h b/drivers/net/wireless/rt2x00/rt2800.h
> index 06acabd..4d771db 100644
> --- a/drivers/net/wireless/rt2x00/rt2800.h
> +++ b/drivers/net/wireless/rt2x00/rt2800.h
> @@ -1627,6 +1627,7 @@ struct mac_iveiv_entry {
>
> /*
> * H2M_MAILBOX_CSR: Host-to-MCU Mailbox.
> + * CMD_TOKEN: Command id, 0xff disable status reporting
> */
> #define H2M_MAILBOX_CSR 0x7010
> #define H2M_MAILBOX_CSR_ARG0 FIELD32(0x000000ff)
> @@ -1636,6 +1637,8 @@ struct mac_iveiv_entry {
>
> /*
> * H2M_MAILBOX_CID:
> + * Free slots contain 0xff. MCU will store command's token to lowest free slot.
> + * If all slots are occupied status will be dropped.
> */
> #define H2M_MAILBOX_CID 0x7014
> #define H2M_MAILBOX_CID_CMD0 FIELD32(0x000000ff)
> @@ -1645,6 +1648,7 @@ struct mac_iveiv_entry {
>
> /*
> * H2M_MAILBOX_STATUS:
> + * Command status will be saved to same slot as command id.
> */
> #define H2M_MAILBOX_STATUS 0x701c
>
> @@ -2288,6 +2292,12 @@ struct mac_iveiv_entry {
>
> /*
> * MCU mailbox commands.
> + * MCU_SLEEP - go to power-save mode.
> + * arg1: 1: save as much power as possible, 0: save less power
> + * status: 1: success, 2: already asleep,
> + * 3: maybe MAC is busy so can't finish this task
> + * MCU_RADIO_OFF
> + * arg0: 0: do power-saving, NOT turn off radio
> */
> #define MCU_SLEEP 0x30
> #define MCU_WAKEUP 0x31
> @@ -2308,7 +2318,9 @@ struct mac_iveiv_entry {
> /*
> * MCU mailbox tokens
> */
> -#define TOKEN_WAKUP 3
> +#define TOKEN_SLEEP 1
> +#define TOKEN_RADIO_OFF 2
> +#define TOKEN_WAKEUP 3
>
> /*
> * DMA descriptor defines.
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 6ab4637..f184a4f 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -546,11 +546,13 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
> /* PCIe devices won't report status after SLEEP request. */
> rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
> rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
> - rt2800_mcu_request(rt2x00dev, MCU_SLEEP, 0x01, 0xff, 0x01);
> + rt2800_mcu_request(rt2x00dev, MCU_SLEEP, TOKEN_SLEEP,
> + 0xff, 0x01);
> break;
> case STATE_AWAKE:
> - rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKUP, 0, 0x02);
> - rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKUP);
> + rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKEUP,
> + 0, 0x02);
> + rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKEUP);
> break;
> case STATE_RADIO_IRQ_ON:
> case STATE_RADIO_IRQ_OFF:
> --
> 1.7.7.6
>
>
> _______________________________________________
> users mailing list
> users@rt2x00.serialmonkey.com
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
--
---
Gertjan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] rt2800pci: Fix 'Error - MCU request failed' during initialization
2012-02-13 4:48 [PATCH v2 0/4] rt2x00: Update MCU operations during device initialization Jakub Kicinski
` (2 preceding siblings ...)
2012-02-13 4:48 ` [PATCH v2 3/4] rt2800: Add documentation on MCU requests Jakub Kicinski
@ 2012-02-13 4:48 ` Jakub Kicinski
2012-02-14 10:50 ` [rt2x00-users] " Gertjan van Wingerde
3 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2012-02-13 4:48 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, users, Jakub Kicinski
Bring MCU operations during device initialization to sync
with legacy driver.
This should fix following error:
phy0 -> rt2800pci_mcu_status: Error - MCU request failed,
no response from hardware
Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 20 +++++++++++++-------
1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index f184a4f..773d078 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -524,14 +524,20 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
switch (state) {
case STATE_RADIO_ON:
- /*
- * Before the radio can be enabled, the device first has
- * to be woken up. After that it needs a bit of time
- * to be fully awake and then the radio can be enabled.
- */
- rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
- msleep(1);
+ /* Initialise all registers and send MCU_BOOT_SIGNAL. */
retval = rt2800pci_enable_radio(rt2x00dev);
+
+ /* After resume MCU_BOOT_SIGNAL will trash these. */
+ rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
+ rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
+
+ /* Finish initialization procedure. */
+ rt2800_mcu_request(rt2x00dev, MCU_SLEEP, TOKEN_RADIO_OFF,
+ 0xff, 0x02);
+ rt2800pci_mcu_status(rt2x00dev, TOKEN_RADIO_OFF);
+
+ rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKEUP, 0, 0);
+ rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKEUP);
break;
case STATE_RADIO_OFF:
/*
--
1.7.7.6
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [rt2x00-users] [PATCH v2 4/4] rt2800pci: Fix 'Error - MCU request failed' during initialization
2012-02-13 4:48 ` [PATCH v2 4/4] rt2800pci: Fix 'Error - MCU request failed' during initialization Jakub Kicinski
@ 2012-02-14 10:50 ` Gertjan van Wingerde
0 siblings, 0 replies; 12+ messages in thread
From: Gertjan van Wingerde @ 2012-02-14 10:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: linville, linux-wireless, users
On Mon, Feb 13, 2012 at 5:48 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> Bring MCU operations during device initialization to sync
> with legacy driver.
>
> This should fix following error:
> phy0 -> rt2800pci_mcu_status: Error - MCU request failed,
> no response from hardware
>
> Signed-off-by: Jakub Kicinski <kubakici@wp.pl>
> Acked-by: Ivo van Doorn <IvDoorn@gmail.com>
Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
provided that my comments to patches 1 & 2 are handled.
> ---
> drivers/net/wireless/rt2x00/rt2800pci.c | 20 +++++++++++++-------
> 1 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index f184a4f..773d078 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -524,14 +524,20 @@ static int rt2800pci_set_device_state(struct rt2x00_dev *rt2x00dev,
>
> switch (state) {
> case STATE_RADIO_ON:
> - /*
> - * Before the radio can be enabled, the device first has
> - * to be woken up. After that it needs a bit of time
> - * to be fully awake and then the radio can be enabled.
> - */
> - rt2800pci_set_device_state(rt2x00dev, STATE_AWAKE);
> - msleep(1);
> + /* Initialise all registers and send MCU_BOOT_SIGNAL. */
> retval = rt2800pci_enable_radio(rt2x00dev);
> +
> + /* After resume MCU_BOOT_SIGNAL will trash these. */
> + rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_STATUS, ~0);
> + rt2x00pci_register_write(rt2x00dev, H2M_MAILBOX_CID, ~0);
> +
> + /* Finish initialization procedure. */
> + rt2800_mcu_request(rt2x00dev, MCU_SLEEP, TOKEN_RADIO_OFF,
> + 0xff, 0x02);
> + rt2800pci_mcu_status(rt2x00dev, TOKEN_RADIO_OFF);
> +
> + rt2800_mcu_request(rt2x00dev, MCU_WAKEUP, TOKEN_WAKEUP, 0, 0);
> + rt2800pci_mcu_status(rt2x00dev, TOKEN_WAKEUP);
> break;
> case STATE_RADIO_OFF:
> /*
> --
> 1.7.7.6
>
>
> _______________________________________________
> users mailing list
> users@rt2x00.serialmonkey.com
> http://rt2x00.serialmonkey.com/mailman/listinfo/users_rt2x00.serialmonkey.com
--
---
Gertjan
^ permalink raw reply [flat|nested] 12+ messages in thread