Regression in 9390ace brcmfmac: free net device when registration fails

Regression in 9390ace brcmfmac: free net device when registration fails

[Stephen Warren]

(John, note that this commit is part of the pull request you sent to
Dave today)

Arend,

Commit 9390ace "brcmfmac: free net device when registration fails"
causes a regression.

I had the BCM4330 firmware present in my root fs, whereas my HW is a
BCM4329. With this patch applied, I get a kernel panic on boot. With it
reverted, I see that no issues of that nature. This is true in
next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
Linus's tree followed by a merge of John's wireless pull request from
today pointed at this same commit.

Related: Since the two hardware require different firmware, I wonder why
the driver doesn't use a firmware filename that encodes the HW device ID
instead of using the same name for all HW. This means that when I move
my SD card between development systems, I have to copy different
firmware over the top. That's a little painful...

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[Arend van Spriel]

On 06/04/2013 12:56 AM, Stephen Warren wrote:
> (John, note that this commit is part of the pull request you sent to
> Dave today)
>
> Arend,
>
> Commit 9390ace "brcmfmac: free net device when registration fails"
> causes a regression.
>
> I had the BCM4330 firmware present in my root fs, whereas my HW is a
> BCM4329. With this patch applied, I get a kernel panic on boot. With it
> reverted, I see that no issues of that nature. This is true in
> next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
> Linus's tree followed by a merge of John's wireless pull request from
> today pointed at this same commit.

Thanks, Stephen

I will look into this.

> Related: Since the two hardware require different firmware, I wonder why
> the driver doesn't use a firmware filename that encodes the HW device ID
> instead of using the same name for all HW. This means that when I move
> my SD card between development systems, I have to copy different
> firmware over the top. That's a little painful...

I agree that for development switching cards it is kinda awkward and I 
have been fooled by it as well. We may want to change or if your itch is 
annoying enough feel free to send a patch for it ;-)

Regards,
Arend

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On 06/04/13 14:58, Arend van Spriel wrote:
> On 06/04/2013 12:56 AM, Stephen Warren wrote:
>> (John, note that this commit is part of the pull request you sent to
>> Dave today)
>>
>> Arend,
>>
>> Commit 9390ace "brcmfmac: free net device when registration fails"
>> causes a regression.
>>
>> I had the BCM4330 firmware present in my root fs, whereas my HW is a
>> BCM4329. With this patch applied, I get a kernel panic on boot. With it
>> reverted, I see that no issues of that nature. This is true in
>> next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
>> Linus's tree followed by a merge of John's wireless pull request from
>> today pointed at this same commit.
>
> Thanks, Stephen
>
> I will look into this.

Hi Stephen,

Can you try the attached patch? If it does not solve the issue, could 
you provide a log.

>> Related: Since the two hardware require different firmware, I wonder why
>> the driver doesn't use a firmware filename that encodes the HW device ID
>> instead of using the same name for all HW. This means that when I move
>> my SD card between development systems, I have to copy different
>> firmware over the top. That's a little painful...
>
> I agree that for development switching cards it is kinda awkward and I
> have been fooled by it as well. We may want to change or if your itch is
> annoying enough feel free to send a patch for it ;-)

We have a patch for this that we can submit to wireless-next.

Regards,
Arend

[-- Attachment #2: 0001-brcmfmac-free-primary-net_device-when-brcmf_bus_star.patch --]
[-- Type: text/plain, Size: 1080 bytes --]

>From 2c0bf2dc8ecf91175a4b9a28dc2c4e84aceee4cf Mon Sep 17 00:00:00 2001
From: Arend van Spriel <arend@broadcom.com>
Date: Thu, 6 Jun 2013 22:43:41 +0200
Subject: [PATCH] brcmfmac: free primary net_device when brcmf_bus_start()
 fails

When initialization within brcmf_bus_start() fails on steps
before the brcmf_net_attach() the net_device for the primary
interface needs to be freed.

Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index 63cadf6..d7cf98e 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -941,6 +941,10 @@ fail:
 			brcmf_fws_del_interface(ifp);
 			brcmf_fws_deinit(drvr);
 		}
+		if (drvr->iflist[0]) {
+			free_netdev(ipf->ndev);
+			drvr->iflist[0] = NULL;
+		}
 		if (p2p_ifp) {
 			free_netdev(p2p_ifp->ndev);
 			drvr->iflist[1] = NULL;
-- 
1.7.10.4

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[Stephen Warren]

On 06/06/2013 03:10 PM, Arend van Spriel wrote:
> On 06/04/13 14:58, Arend van Spriel wrote:
>> On 06/04/2013 12:56 AM, Stephen Warren wrote:
>>> (John, note that this commit is part of the pull request you sent to
>>> Dave today)
>>>
>>> Arend,
>>>
>>> Commit 9390ace "brcmfmac: free net device when registration fails"
>>> causes a regression.
>>>
>>> I had the BCM4330 firmware present in my root fs, whereas my HW is a
>>> BCM4329. With this patch applied, I get a kernel panic on boot. With it
>>> reverted, I see that no issues of that nature. This is true in
>>> next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
>>> Linus's tree followed by a merge of John's wireless pull request from
>>> today pointed at this same commit.
>>
>> Thanks, Stephen
>>
>> I will look into this.
> 
> Hi Stephen,
> 
> Can you try the attached patch? If it does not solve the issue, could
> you provide a log.

Yes, that works great, thanks.

Tested-by: Stephen Warren <swarren@nvidia.com>

>>> Related: Since the two hardware require different firmware, I wonder why
>>> the driver doesn't use a firmware filename that encodes the HW device ID
>>> instead of using the same name for all HW. This means that when I move
>>> my SD card between development systems, I have to copy different
>>> firmware over the top. That's a little painful...
>>
>> I agree that for development switching cards it is kinda awkward and I
>> have been fooled by it as well. We may want to change or if your itch is
>> annoying enough feel free to send a patch for it ;-)
> 
> We have a patch for this that we can submit to wireless-next.

Oh, great! If you could Cc me, that'd be helpful, since I'm not on that
list. Thanks.

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[Stephen Warren]

On 06/06/2013 03:36 PM, Stephen Warren wrote:
> On 06/06/2013 03:10 PM, Arend van Spriel wrote:
>> On 06/04/13 14:58, Arend van Spriel wrote:
>>> On 06/04/2013 12:56 AM, Stephen Warren wrote:
>>>> (John, note that this commit is part of the pull request you sent to
>>>> Dave today)
>>>>
>>>> Arend,
>>>>
>>>> Commit 9390ace "brcmfmac: free net device when registration fails"
>>>> causes a regression.
>>>>
>>>> I had the BCM4330 firmware present in my root fs, whereas my HW is a
>>>> BCM4329. With this patch applied, I get a kernel panic on boot. With it
>>>> reverted, I see that no issues of that nature. This is true in
>>>> next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
>>>> Linus's tree followed by a merge of John's wireless pull request from
>>>> today pointed at this same commit.
>>>
>>> Thanks, Stephen
>>>
>>> I will look into this.
>>
>> Hi Stephen,
>>
>> Can you try the attached patch? If it does not solve the issue, could
>> you provide a log.
> 
> Yes, that works great, thanks.
> 
> Tested-by: Stephen Warren <swarren@nvidia.com>

Did this patch get applied anywhere? I was hoping it'd go into 3.10-rc*
given it's a user-triggerable backtrace introduced in 3.10-rc*.

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[Arend van Spriel]

On 06/12/2013 11:19 PM, Stephen Warren wrote:
> On 06/06/2013 03:36 PM, Stephen Warren wrote:
>> On 06/06/2013 03:10 PM, Arend van Spriel wrote:
>>> On 06/04/13 14:58, Arend van Spriel wrote:
>>>> On 06/04/2013 12:56 AM, Stephen Warren wrote:
>>>>> (John, note that this commit is part of the pull request you sent to
>>>>> Dave today)
>>>>>
>>>>> Arend,
>>>>>
>>>>> Commit 9390ace "brcmfmac: free net device when registration fails"
>>>>> causes a regression.
>>>>>
>>>>> I had the BCM4330 firmware present in my root fs, whereas my HW is a
>>>>> BCM4329. With this patch applied, I get a kernel panic on boot. With it
>>>>> reverted, I see that no issues of that nature. This is true in
>>>>> next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
>>>>> Linus's tree followed by a merge of John's wireless pull request from
>>>>> today pointed at this same commit.
>>>>
>>>> Thanks, Stephen
>>>>
>>>> I will look into this.
>>>
>>> Hi Stephen,
>>>
>>> Can you try the attached patch? If it does not solve the issue, could
>>> you provide a log.
>>
>> Yes, that works great, thanks.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>
> Did this patch get applied anywhere? I was hoping it'd go into 3.10-rc*
> given it's a user-triggerable backtrace introduced in 3.10-rc*.

Hi Stephen,

The patch went into the wireless-next tree. Given recent events with our 
patch series I am a bit more carsful to tag patches for 3.10. Your issue 
is user-triggerable but the scenario to get there can be avoided. But 
you are right it was introduced in the 3.10-rc cycle. I leave it to John 
to decide.

Regards,
Arend

Re: Regression in 9390ace brcmfmac: free net device when registration fails

[John W. Linville]

On Thu, Jun 13, 2013 at 10:09:58AM +0200, Arend van Spriel wrote:
> On 06/12/2013 11:19 PM, Stephen Warren wrote:
> >On 06/06/2013 03:36 PM, Stephen Warren wrote:
> >>On 06/06/2013 03:10 PM, Arend van Spriel wrote:
> >>>On 06/04/13 14:58, Arend van Spriel wrote:
> >>>>On 06/04/2013 12:56 AM, Stephen Warren wrote:
> >>>>>(John, note that this commit is part of the pull request you sent to
> >>>>>Dave today)
> >>>>>
> >>>>>Arend,
> >>>>>
> >>>>>Commit 9390ace "brcmfmac: free net device when registration fails"
> >>>>>causes a regression.
> >>>>>
> >>>>>I had the BCM4330 firmware present in my root fs, whereas my HW is a
> >>>>>BCM4329. With this patch applied, I get a kernel panic on boot. With it
> >>>>>reverted, I see that no issues of that nature. This is true in
> >>>>>next-20130531 (and also 5/30 and 6/1 but not earlier). A bisect of
> >>>>>Linus's tree followed by a merge of John's wireless pull request from
> >>>>>today pointed at this same commit.
> >>>>
> >>>>Thanks, Stephen
> >>>>
> >>>>I will look into this.
> >>>
> >>>Hi Stephen,
> >>>
> >>>Can you try the attached patch? If it does not solve the issue, could
> >>>you provide a log.
> >>
> >>Yes, that works great, thanks.
> >>
> >>Tested-by: Stephen Warren <swarren@nvidia.com>
> >
> >Did this patch get applied anywhere? I was hoping it'd go into 3.10-rc*
> >given it's a user-triggerable backtrace introduced in 3.10-rc*.
> 
> Hi Stephen,
> 
> The patch went into the wireless-next tree. Given recent events with
> our patch series I am a bit more carsful to tag patches for 3.10.
> Your issue is user-triggerable but the scenario to get there can be
> avoided. But you are right it was introduced in the 3.10-rc cycle. I
> leave it to John to decide.
> 
> Regards,
> Arend

I've just applied it to the wireless tree.  I doubt if it will make
-rc6, but it might make the final 3.10.

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