[PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Douglas Anderson]

The commit fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
adapter status variables") had a fairly straightforward bug in it.  It
contained this bit of diff:

 - if (!adapter->is_suspended) {
 + if (test_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags)) {

As you can see the patch missed the "!" when converting to the atomic
bitops.  This meant that the resume hasn't done anything at all since
that commit landed and suspend/resume for mwifiex SDIO cards has been
totally broken.

After fixing this mwifiex suspend/resume appears to work again, at
least with the simple testing I've done.

Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent adapter status variables")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/net/wireless/marvell/mwifiex/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
index a85648342d15..d5a70340a945 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -181,7 +181,7 @@ static int mwifiex_sdio_resume(struct device *dev)
 
 	adapter = card->adapter;
 
-	if (test_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags)) {
+	if (!test_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags)) {
 		mwifiex_dbg(adapter, WARN,
 			    "device already resumed\n");
 		return 0;
-- 
2.21.0.392.gf8f6787159e-goog

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Kalle Valo]

Douglas Anderson <dianders@chromium.org> writes:

> The commit fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
> adapter status variables") had a fairly straightforward bug in it.  It
> contained this bit of diff:
>
>  - if (!adapter->is_suspended) {
>  + if (test_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags)) {
>
> As you can see the patch missed the "!" when converting to the atomic
> bitops.  This meant that the resume hasn't done anything at all since
> that commit landed and suspend/resume for mwifiex SDIO cards has been
> totally broken.
>
> After fixing this mwifiex suspend/resume appears to work again, at
> least with the simple testing I've done.
>
> Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent adapter status variables")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

I'm planning to queue this for 5.1.

-- 
Kalle Valo

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Brian Norris]

On Wed, Apr 3, 2019 at 10:20 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> Douglas Anderson <dianders@chromium.org> writes:
> > Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent adapter status variables")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> I'm planning to queue this for 5.1.

With an explicit

Cc: <stable@vger.kernel.org>

please? Somebody would likely pick it up anyway, but it's nice to be explicit.

Not that it really matters, but also:

Reviewed-by: Brian Norris <briannorris@chromium.org>

Thanks,
Brian

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Kalle Valo]

Brian Norris <briannorris@chromium.org> writes:

> On Wed, Apr 3, 2019 at 10:20 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>> Douglas Anderson <dianders@chromium.org> writes:
>> > Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
>> > adapter status variables")
>> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>
>> I'm planning to queue this for 5.1.
>
> With an explicit
>
> Cc: <stable@vger.kernel.org>
>
> please? Somebody would likely pick it up anyway, but it's nice to be
> explicit.

Ok, I'll add that.

> Not that it really matters, but also:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>

This patchwork picks up automatically so I don't have to do anything.
But I so wish it would do the same for Cc and Fixes tags :)

-- 
Kalle Valo

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Doug Anderson]

Hi,

On Thu, Apr 4, 2019 at 8:35 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Brian Norris <briannorris@chromium.org> writes:
>
> > On Wed, Apr 3, 2019 at 10:20 PM Kalle Valo <kvalo@codeaurora.org> wrote:
> >> Douglas Anderson <dianders@chromium.org> writes:
> >> > Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
> >> > adapter status variables")
> >> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >>
> >> I'm planning to queue this for 5.1.

Do you know when you plan to queue this?  I don't see the patch in
Linus's tree nor in linux-next.  Just want to make sure it didn't get
forgotten...

Thanks!

-Doug

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Kalle Valo]

Doug Anderson <dianders@chromium.org> writes:

> On Thu, Apr 4, 2019 at 8:35 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Brian Norris <briannorris@chromium.org> writes:
>>
>> > On Wed, Apr 3, 2019 at 10:20 PM Kalle Valo <kvalo@codeaurora.org> wrote:
>> >> Douglas Anderson <dianders@chromium.org> writes:
>> >> > Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
>> >> > adapter status variables")
>> >> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>> >>
>> >> I'm planning to queue this for 5.1.
>
> Do you know when you plan to queue this?  I don't see the patch in
> Linus's tree nor in linux-next.  Just want to make sure it didn't get
> forgotten...

It's on my queue and not forgotten:

https://patchwork.kernel.org/patch/10884883/

Due to various reasons I'm really struggling with patches right now,
sorry about that, but I'm slowly catching up. I should be able to apply
this within a week.

-- 
Kalle Valo

Re: [PATCH] mwifiex: Make resume actually do something useful again on SDIO cards

[Kalle Valo]

Douglas Anderson <dianders@chromium.org> wrote:

> The commit fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent
> adapter status variables") had a fairly straightforward bug in it.  It
> contained this bit of diff:
> 
>  - if (!adapter->is_suspended) {
>  + if (test_bit(MWIFIEX_IS_SUSPENDED, &adapter->work_flags)) {
> 
> As you can see the patch missed the "!" when converting to the atomic
> bitops.  This meant that the resume hasn't done anything at all since
> that commit landed and suspend/resume for mwifiex SDIO cards has been
> totally broken.
> 
> After fixing this mwifiex suspend/resume appears to work again, at
> least with the simple testing I've done.
> 
> Fixes: fc3a2fcaa1ba ("mwifiex: use atomic bitops to represent adapter status variables")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Reviewed-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers.git, thanks.

b82d6c1f8f82 mwifiex: Make resume actually do something useful again on SDIO cards

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

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