2019-12-26 09:24:42

by Jean-Philippe Brucker

[permalink] [raw]
Subject: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling
brcmf_bus_started()") changed the initialization order of the brcmfmac
SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now
called before the sdiodev->bus_if initialization, it reads the wrong
chip ID and fails to initialize the GPIO on brcm43362. Thus the chip
cannot send interrupts and fails to probe:

[ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
[ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110
[ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
[ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed

Initialize the bus interface earlier to ensure that
brcmf_sdiod_intr_register() properly sets up the OOB interrupt.

BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438
Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling brcmf_bus_started()")
Signed-off-by: Jean-Philippe Brucker <[email protected]>
---
A workaround [1] disabling the OOB interrupt is being discussed. It
works for me, but this patch fixes the wifi problem on my cubietruck.

[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
---
.../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 264ad63232f8..058069a03693 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4220,38 +4220,38 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
brcmf_sdio_sr_init(bus);
} else {
/* Restore previous clock setting */
brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_CHIPCLKCSR,
saveclk, &err);
}

if (err == 0) {
+ /* Assign bus interface call back */
+ sdiod->bus_if->dev = sdiod->dev;
+ sdiod->bus_if->ops = &brcmf_sdio_bus_ops;
+ sdiod->bus_if->chip = bus->ci->chip;
+ sdiod->bus_if->chiprev = bus->ci->chiprev;
+
/* Allow full data communication using DPC from now on. */
brcmf_sdiod_change_state(bus->sdiodev, BRCMF_SDIOD_DATA);

err = brcmf_sdiod_intr_register(sdiod);
if (err != 0)
brcmf_err("intr register failed:%d\n", err);
}

/* If we didn't come up, turn off backplane clock */
if (err != 0) {
brcmf_sdio_clkctl(bus, CLK_NONE, false);
goto checkdied;
}

sdio_release_host(sdiod->func1);

- /* Assign bus interface call back */
- sdiod->bus_if->dev = sdiod->dev;
- sdiod->bus_if->ops = &brcmf_sdio_bus_ops;
- sdiod->bus_if->chip = bus->ci->chip;
- sdiod->bus_if->chiprev = bus->ci->chiprev;
-
err = brcmf_alloc(sdiod->dev, sdiod->settings);
if (err) {
brcmf_err("brcmf_alloc failed\n");
goto claim;
}

/* Attach to the common layer, reserve hdr space */
err = brcmf_attach(sdiod->dev);
--
2.24.0


2019-12-26 09:47:55

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

On December 26, 2019 10:23:41 AM Jean-Philippe Brucker
<[email protected]> wrote:

> Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling
> brcmf_bus_started()") changed the initialization order of the brcmfmac
> SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now
> called before the sdiodev->bus_if initialization, it reads the wrong
> chip ID and fails to initialize the GPIO on brcm43362. Thus the chip
> cannot send interrupts and fails to probe:
>
> [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110
> [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
> [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed
>
> Initialize the bus interface earlier to ensure that
> brcmf_sdiod_intr_register() properly sets up the OOB interrupt.
>
> BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438
> Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling
> brcmf_bus_started()")

Reviewed-by: Arend van Spriel <[email protected]>

> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> ---
> A workaround [1] disabling the OOB interrupt is being discussed. It
> works for me, but this patch fixes the wifi problem on my cubietruck.

I missed that one. Too bad it was not sent to linux-wireless as well. Good
find here. I did see another patch dealing with the OOB interrupt on Nvidia
Tegra. Now I wonder if this is the same issue.

Regards,
Arend

> [1]
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
> ---
> .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)



2019-12-26 14:38:35

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

26.12.2019 12:47, Arend Van Spriel пишет:
> On December 26, 2019 10:23:41 AM Jean-Philippe Brucker
> <[email protected]> wrote:
>
>> Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling
>> brcmf_bus_started()") changed the initialization order of the brcmfmac
>> SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now
>> called before the sdiodev->bus_if initialization, it reads the wrong
>> chip ID and fails to initialize the GPIO on brcm43362. Thus the chip
>> cannot send interrupts and fails to probe:
>>
>> [   12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
>> [   12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110
>> [   12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding:
>> err=-110
>> [   12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach
>> failed
>>
>> Initialize the bus interface earlier to ensure that
>> brcmf_sdiod_intr_register() properly sets up the OOB interrupt.
>>
>> BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438
>> Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before
>> calling brcmf_bus_started()")
>
> Reviewed-by: Arend van Spriel <[email protected]>
>
>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
>> ---
>> A workaround [1] disabling the OOB interrupt is being discussed. It
>> works for me, but this patch fixes the wifi problem on my cubietruck.
>
> I missed that one. Too bad it was not sent to linux-wireless as well.
> Good find here. I did see another patch dealing with the OOB interrupt
> on Nvidia Tegra. Now I wonder if this is the same issue.
>
> Regards,
> Arend
>
>> [1]
>> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>>
>> ---
>> .../net/wireless/broadcom/brcm80211/brcmfmac/sdio.c  | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)

I haven't seen any driver probe failures due to OOB on NVIDIA Tegra,
only suspend-resume was problematic due to the unbalanced OOB
interrupt-wake enabling.

But maybe checking whether OOB interrupt-wake works by invoking
enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for
the cubietruck board.

@Jean-Philippe, could you please try this change (on top of recent
linux-next):

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index b684a5b6d904..80d7106b10a9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev
*sdiodev)
}
sdiodev->oob_irq_requested = true;

- ret = enable_irq_wake(pdata->oob_irq_nr);
- if (ret != 0) {
- brcmf_err("enable_irq_wake failed %d\n", ret);
- return ret;
- }
- disable_irq_wake(pdata->oob_irq_nr);
-
sdio_claim_host(sdiodev->func1);

if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {

2020-01-06 19:21:44

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

Hi Dmitry,

On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote:
> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra,
> only suspend-resume was problematic due to the unbalanced OOB
> interrupt-wake enabling.
>
> But maybe checking whether OOB interrupt-wake works by invoking
> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for
> the cubietruck board.
>
> @Jean-Philippe, could you please try this change (on top of recent
> linux-next):

Sorry for the delay, linux-next doesn't boot for me at the moment and I
have little time to investigate why, so I might retry closer to the merge
window.

However, isn't the interrupt-wake issue independent from the problem
(introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB
wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it
doesn't seem to cause a regression, but the wifi only works if I apply my
patch as well.

Thanks,
Jean

>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> index b684a5b6d904..80d7106b10a9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev
> *sdiodev)
> }
> sdiodev->oob_irq_requested = true;
>
> - ret = enable_irq_wake(pdata->oob_irq_nr);
> - if (ret != 0) {
> - brcmf_err("enable_irq_wake failed %d\n", ret);
> - return ret;
> - }
> - disable_irq_wake(pdata->oob_irq_nr);
> -
> sdio_claim_host(sdiodev->func1);
>
> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {

2020-01-06 23:16:08

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

06.01.2020 22:19, Jean-Philippe Brucker пишет:
> Hi Dmitry,
>
> On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote:
>> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra,
>> only suspend-resume was problematic due to the unbalanced OOB
>> interrupt-wake enabling.
>>
>> But maybe checking whether OOB interrupt-wake works by invoking
>> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for
>> the cubietruck board.
>>
>> @Jean-Philippe, could you please try this change (on top of recent
>> linux-next):
>
> Sorry for the delay, linux-next doesn't boot for me at the moment and I
> have little time to investigate why, so I might retry closer to the merge
> window.
>
> However, isn't the interrupt-wake issue independent from the problem
> (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB
> wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it
> doesn't seem to cause a regression, but the wifi only works if I apply my
> patch as well.
>
> Thanks,
> Jean
>
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> index b684a5b6d904..80d7106b10a9 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev
>> *sdiodev)
>> }
>> sdiodev->oob_irq_requested = true;
>>
>> - ret = enable_irq_wake(pdata->oob_irq_nr);
>> - if (ret != 0) {
>> - brcmf_err("enable_irq_wake failed %d\n", ret);
>> - return ret;
>> - }
>> - disable_irq_wake(pdata->oob_irq_nr);
>> -
>> sdio_claim_host(sdiodev->func1);
>>
>> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {

Hello Jean,

Could you please clarify whether you applied [1] and then the above
snippet on top of it or you only applied [1] without the snippet?

[1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled

2020-01-07 07:24:35

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

On Tue, Jan 07, 2020 at 02:15:18AM +0300, Dmitry Osipenko wrote:
> 06.01.2020 22:19, Jean-Philippe Brucker пишет:
> > Hi Dmitry,
> >
> > On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote:
> >> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra,
> >> only suspend-resume was problematic due to the unbalanced OOB
> >> interrupt-wake enabling.
> >>
> >> But maybe checking whether OOB interrupt-wake works by invoking
> >> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for
> >> the cubietruck board.
> >>
> >> @Jean-Philippe, could you please try this change (on top of recent
> >> linux-next):
> >
> > Sorry for the delay, linux-next doesn't boot for me at the moment and I
> > have little time to investigate why, so I might retry closer to the merge
> > window.
> >
> > However, isn't the interrupt-wake issue independent from the problem
> > (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB
> > wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it
> > doesn't seem to cause a regression, but the wifi only works if I apply my
> > patch as well.
> >
> > Thanks,
> > Jean
> >
> >>
> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> >> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> >> index b684a5b6d904..80d7106b10a9 100644
> >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
> >> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev
> >> *sdiodev)
> >> }
> >> sdiodev->oob_irq_requested = true;
> >>
> >> - ret = enable_irq_wake(pdata->oob_irq_nr);
> >> - if (ret != 0) {
> >> - brcmf_err("enable_irq_wake failed %d\n", ret);
> >> - return ret;
> >> - }
> >> - disable_irq_wake(pdata->oob_irq_nr);
> >> -
> >> sdio_claim_host(sdiodev->func1);
> >>
> >> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
>
> Hello Jean,
>
> Could you please clarify whether you applied [1] and then the above
> snippet on top of it or you only applied [1] without the snippet?

I applied [1] without the snippet

Thanks,
Jean

>
> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled

2020-01-07 16:26:28

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

07.01.2020 10:23, Jean-Philippe Brucker пишет:
> On Tue, Jan 07, 2020 at 02:15:18AM +0300, Dmitry Osipenko wrote:
>> 06.01.2020 22:19, Jean-Philippe Brucker пишет:
>>> Hi Dmitry,
>>>
>>> On Thu, Dec 26, 2019 at 05:37:58PM +0300, Dmitry Osipenko wrote:
>>>> I haven't seen any driver probe failures due to OOB on NVIDIA Tegra,
>>>> only suspend-resume was problematic due to the unbalanced OOB
>>>> interrupt-wake enabling.
>>>>
>>>> But maybe checking whether OOB interrupt-wake works by invoking
>>>> enable_irq_wake() during brcmf_sdiod_intr_register() causes trouble for
>>>> the cubietruck board.
>>>>
>>>> @Jean-Philippe, could you please try this change (on top of recent
>>>> linux-next):
>>>
>>> Sorry for the delay, linux-next doesn't boot for me at the moment and I
>>> have little time to investigate why, so I might retry closer to the merge
>>> window.
>>>
>>> However, isn't the interrupt-wake issue independent from the problem
>>> (introduced in v4.17) that my patch fixes? I applied "brcmfmac: Keep OOB
>>> wake-interrupt disabled when it shouldn't be enabled" on v5.5-rc5 and it
>>> doesn't seem to cause a regression, but the wifi only works if I apply my
>>> patch as well.
>>>
>>> Thanks,
>>> Jean
>>>
>>>>
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>>>> index b684a5b6d904..80d7106b10a9 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
>>>> @@ -115,13 +115,6 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev
>>>> *sdiodev)
>>>> }
>>>> sdiodev->oob_irq_requested = true;
>>>>
>>>> - ret = enable_irq_wake(pdata->oob_irq_nr);
>>>> - if (ret != 0) {
>>>> - brcmf_err("enable_irq_wake failed %d\n", ret);
>>>> - return ret;
>>>> - }
>>>> - disable_irq_wake(pdata->oob_irq_nr);
>>>> -
>>>> sdio_claim_host(sdiodev->func1);
>>>>
>>>> if (sdiodev->bus_if->chip == BRCM_CC_43362_CHIP_ID) {
>>
>> Hello Jean,
>>
>> Could you please clarify whether you applied [1] and then the above
>> snippet on top of it or you only applied [1] without the snippet?
>
> I applied [1] without the snippet
>
> Thanks,
> Jean
>
>>
>> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled

Will you be able to test *with* the snippet? I guess chances that it
will make any difference are not high, nevertheless will be good to know
for sure.

2020-01-08 07:50:05

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

On Tue, Jan 07, 2020 at 07:23:32PM +0300, Dmitry Osipenko wrote:
> >> Hello Jean,
> >>
> >> Could you please clarify whether you applied [1] and then the above
> >> snippet on top of it or you only applied [1] without the snippet?
> >
> > I applied [1] without the snippet
> >
> > Thanks,
> > Jean
> >
> >>
> >> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled
>
> Will you be able to test *with* the snippet? I guess chances that it
> will make any difference are not high, nevertheless will be good to know
> for sure.

I tested it with the snippet and didn't notice a difference

Thanks,
Jean

2020-01-08 15:05:13

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

08.01.2020 10:39, Jean-Philippe Brucker пишет:
> On Tue, Jan 07, 2020 at 07:23:32PM +0300, Dmitry Osipenko wrote:
>>>> Hello Jean,
>>>>
>>>> Could you please clarify whether you applied [1] and then the above
>>>> snippet on top of it or you only applied [1] without the snippet?
>>>
>>> I applied [1] without the snippet
>>>
>>> Thanks,
>>> Jean
>>>
>>>>
>>>> [1] brcmfmac: Keep OOB wake-interrupt disabled when it shouldn't be enabled
>>
>> Will you be able to test *with* the snippet? I guess chances that it
>> will make any difference are not high, nevertheless will be good to know
>> for sure.
>
> I tested it with the snippet and didn't notice a difference

Okay

2020-01-26 15:42:47

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

Jean-Philippe Brucker <[email protected]> wrote:

> Commit 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling
> brcmf_bus_started()") changed the initialization order of the brcmfmac
> SDIO driver. Unfortunately since brcmf_sdiod_intr_register() is now
> called before the sdiodev->bus_if initialization, it reads the wrong
> chip ID and fails to initialize the GPIO on brcm43362. Thus the chip
> cannot send interrupts and fails to probe:
>
> [ 12.517023] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
> [ 12.531214] ieee80211 phy0: brcmf_bus_started: failed: -110
> [ 12.536976] ieee80211 phy0: brcmf_attach: dongle is not responding: err=-110
> [ 12.566467] brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed
>
> Initialize the bus interface earlier to ensure that
> brcmf_sdiod_intr_register() properly sets up the OOB interrupt.
>
> BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=908438
> Fixes: 262f2b53f679 ("brcmfmac: call brcmf_attach() just before calling brcmf_bus_started()")
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> Reviewed-by: Arend van Spriel <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

8c8e60fb86a9 brcmfmac: sdio: Fix OOB interrupt initialization on brcm43362

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

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