| Message ID | 20190404040106.40519-1-dianders@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b82d6c1f8f8288f744a9dcc16cd3085d535decca |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | mwifiex: Make resume actually do something useful again on SDIO cards | expand |
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.
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
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 :)
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
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.
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
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;
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(-)