Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

[H Buus]

I have an old 32 bit laptop with a BCM4401-B0 100Base-TX ethernet
controller. For every kernel from 4.19-rc1 going forward, I get a
warning and call trace within a few seconds of start up (see dmesg
snippet below). I have traced it to a specific commit (see commit
below). On the face of it, I would think it is a regression, but it
doesn't seem to cause a problem, since networking over ethernet is working.

I might not have noticed the warning except for the fact that something
in the 4.19 time frame is causing this laptop to be unstable. I still
have to identify the source of the instability. The laptop appears
stable with this commit but not with 4.19-rc7 or more recent kernels.

I thought I should at least ask if the following warning is an issue. If
it is, I am happy to help with creating a proper bug report as well as
debugging and testing. Is this an issue that should be addressed to the
netdev or linux-wireless lists, or both, since it appears to be an
interaction between the b44 driver which belongs to netdev and the ssb
driver which belongs to linux-wireless?

--- Begin dmesg snippet ---
[    5.145764] ssb: Found chip with id 0x4318, rev 0x02 and package 0x02
[    5.265914] b43-pci-bridge 0000:02:03.0: Sonics Silicon Backplane
found on PCI device 0000:02:03.0
[    5.353718] ssb: Found chip with id 0x4401, rev 0x02 and package 0x00
[    5.421787] WARNING: CPU: 0 PID: 157 at drivers/ssb/driver_gpio.c:464
ssb_gpio_init+0xa0/0xb0 [ssb]
[    5.425679] Modules linked in: b44(+) psmouse pata_acpi ssb mii
[    5.425679] CPU: 0 PID: 157 Comm: systemd-udevd Not tainted
4.19.31-041931-generic #201903231635
[    5.425679] Hardware name: Dell Inc. ME051
/0DK344, BIOS A10 11/07/2006
[    5.425679] EIP: ssb_gpio_init+0xa0/0xb0 [ssb]
[    5.425679] Code: 00 31 c0 85 c9 0f 95 c0 31 c9 f7 d8 89 82 18 05 00
00 8d 82 d8 04 00 00 6a 00 e8 1b b6 17 dd 5a c9 c3 8d b4 26 00 00 00 00
90 <0f> 0b c9 b8 ff ff ff ff c3 8d b4 26 00 00 00 00 3e 8d 74 26 00 55
[    5.425679] EAX: f58a3800 EBX: f58a3800 ECX: 00000000 EDX: f7e1ef94
[    5.425679] ESI: 00000000 EDI: f58a3800 EBP: f585bc80 ESP: f585bc80
[    5.425679] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010246
[    5.425679] CR0: 80050033 CR2: b7ab702b CR3: 3586c000 CR4: 000006f0
[    5.425679] Call Trace:
[    5.425679]  ssb_attach_queued_buses+0xe2/0x310 [ssb]
[    5.425679]  ssb_bus_register+0x167/0x1c0 [ssb]
[    5.425679]  ? ssb_pci_xtal+0x1d0/0x1d0 [ssb]
[    5.425679]  ssb_bus_pcibus_register+0x29/0x80 [ssb]
[    5.425679]  ssb_pcihost_probe+0xb7/0x110 [ssb]
[    5.425679]  ? ssb_pcihost_remove+0x40/0x40 [ssb]
[    5.425679]  pci_device_probe+0xc7/0x160
[    5.425679]  really_probe+0x1fe/0x390
[    5.425679]  driver_probe_device+0xe1/0x120
[    5.425679]  ? pci_match_device+0xde/0x110
[    5.425679]  ? _cond_resched+0x17/0x30
[    5.425679]  __driver_attach+0xd9/0x100
[    5.425679]  ? driver_probe_device+0x120/0x120
[    5.425679]  bus_for_each_dev+0x5b/0xa0
[    5.425679]  driver_attach+0x19/0x20
[    5.425679]  ? driver_probe_device+0x120/0x120
[    5.425679]  bus_add_driver+0x117/0x210
[    5.425679]  ? pci_bus_num_vf+0x20/0x20
[    5.425679]  driver_register+0x66/0xb0
[    5.425679]  ? 0xf8365000
[    5.425679]  __pci_register_driver+0x3d/0x40
[    5.425679]  ssb_pcihost_register+0x2c/0x30 [ssb]
[    5.425679]  b44_init+0x1d/0x1000 [b44]
[    5.425679]  do_one_initcall+0x42/0x19a
[    5.425679]  ? vunmap_page_range+0x1c9/0x260
[    5.425679]  ? free_pcp_prepare+0x5d/0xf0
[    5.425679]  ? _cond_resched+0x17/0x30
[    5.425679]  ? kmem_cache_alloc_trace+0x15f/0x1b0
[    5.425679]  ? do_init_module+0x21/0x210
[    5.425679]  ? do_init_module+0x21/0x210
[    5.425679]  do_init_module+0x50/0x210
[    5.425679]  load_module+0x1368/0x1630
[    5.425679]  ? security_kernel_post_read_file+0x54/0x60
[    5.425679]  sys_finit_module+0x8a/0xe0
[    5.425679]  do_fast_syscall_32+0x87/0x1e0
[    5.425679]  entry_SYSENTER_32+0x6b/0xbe
[    5.425679] EIP: 0xb7eead61
[    5.425679] Code: f6 ff ff 55 89 e5 8b 55 08 8b 80 5c cd ff ff 85 d2
74 02 89 02 5d c3 8b 04 24 c3 8b 1c 24 c3 90 90 51 52 55 89 e5 0f 34 cd
80 <5d> 5a 59 c3 90 90 90 90 8d 76 00 58 b8 77 00 00 00 cd 80 90 8d 76
[    5.425679] EAX: ffffffda EBX: 0000000d ECX: b7cfda15 EDX: 00000000
[    5.425679] ESI: 00fcaf40 EDI: 00fcca40 EBP: 00000000 ESP: bfa404dc
[    5.425679] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000296
[    5.425679] ---[ end trace b0d87e1b3433e0ca ]---
[    6.245890] b44 0000:02:00.0: Sonics Silicon Backplane found on PCI
device 0000:02:00.0
[    6.260263] b44: Broadcom 44xx/47xx 10/100 PCI ethernet driver
version 2.0
[    6.299684] b44 ssb1:0 eth0: Broadcom 44xx/47xx 10/100 PCI ethernet
driver 00:14:22:af:83:9b
[    6.367622] random: fast init done
--- End dmesg snippet ---

--- Begin commit ---
commit 209b43759d65b2cc99ce7757249aacc82b03c4e2
Author: Michael Büsch <m@bues.ch>
Date:   Tue Jul 31 22:15:09 2018 +0200

    ssb: Remove SSB_WARN_ON, SSB_BUG_ON and SSB_DEBUG

    Use the standard WARN_ON instead.
    If a small kernel is desired, WARN_ON can be disabled globally.

    Also remove SSB_DEBUG. Besides WARN_ON it only adds a tiny debug check.
    Include this check unconditionally.

    Signed-off-by: Michael Buesch <m@bues.ch>
    Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
--- End commit ---

Re: Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

[Michael Büsch]

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

On Sun, 9 Jun 2019 17:44:10 -0400
H Buus <ubuntu@hbuus.com> wrote:

> I have an old 32 bit laptop with a BCM4401-B0 100Base-TX ethernet
> controller. For every kernel from 4.19-rc1 going forward, I get a
> warning and call trace within a few seconds of start up (see dmesg
> snippet below). I have traced it to a specific commit (see commit
> below). On the face of it, I would think it is a regression, but it
> doesn't seem to cause a problem, since networking over ethernet is working.


This warning is not a problem. The commit just exposes a warning, that
has always been there.
I suggest we just remove the WARN_ON from ssb_gpio_init and
ssb_gpio_unregister.
I don't see a reason to throw a warning in that case.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

[Larry Finger]

On 6/9/19 4:57 PM, Michael Büsch wrote:
> On Sun, 9 Jun 2019 17:44:10 -0400
> H Buus <ubuntu@hbuus.com> wrote:
> 
>> I have an old 32 bit laptop with a BCM4401-B0 100Base-TX ethernet
>> controller. For every kernel from 4.19-rc1 going forward, I get a
>> warning and call trace within a few seconds of start up (see dmesg
>> snippet below). I have traced it to a specific commit (see commit
>> below). On the face of it, I would think it is a regression, but it
>> doesn't seem to cause a problem, since networking over ethernet is working.
> 
> 
> This warning is not a problem. The commit just exposes a warning, that
> has always been there.
> I suggest we just remove the WARN_ON from ssb_gpio_init and
> ssb_gpio_unregister.
> I don't see a reason to throw a warning in that case.

Michael,

I agree. Do you want to prepare the patch, or should I?

Larry

Re: Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

[H Buus]

On 6/9/2019 8:13 PM, Larry Finger wrote:
> On 6/9/19 4:57 PM, Michael Büsch wrote:
>> On Sun, 9 Jun 2019 17:44:10 -0400
>> H Buus <ubuntu@hbuus.com> wrote:
>>
>>> I have an old 32 bit laptop with a BCM4401-B0 100Base-TX ethernet
>>> controller. For every kernel from 4.19-rc1 going forward, I get a
>>> warning and call trace within a few seconds of start up (see dmesg
>>> snippet below). I have traced it to a specific commit (see commit
>>> below). On the face of it, I would think it is a regression, but it
>>> doesn't seem to cause a problem, since networking over ethernet is
>>> working.
>>
>>
>> This warning is not a problem. The commit just exposes a warning, that
>> has always been there.
>> I suggest we just remove the WARN_ON from ssb_gpio_init and
>> ssb_gpio_unregister.
>> I don't see a reason to throw a warning in that case.
> 
> Michael,
> 
> I agree. Do you want to prepare the patch, or should I?
> 
> Larry

Let me know if you would like me to verify a patch when/if it is
available. Since kernels 4.19 thru 5.1 are unstable on this laptop, I
would either need to test with a 4.18 or older kernel, or limit my
testing to recovery mode & verifying that the b44 module is initialized
without causing a warning & call trace. Unless I get lucky and figure
out what commit is making newer kernels unstable on this laptop.

Re: Should b44_init lead to WARN_ON in drivers/ssb/driver_gpio.c:464?

[Michael Büsch]

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

On Mon, 10 Jun 2019 01:40:17 -0400
H Buus <ubuntu@hbuus.com> wrote:

> Unless I get lucky and figure
> out what commit is making newer kernels unstable on this laptop.


You can use 'git bisect' to quickly find a commit that breaks something.

I'll prepare a patch to get rid of the warning asap.

-- 
Michael

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[Michael Büsch]

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

The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
The warning serves no purpose. So let's just remove it.

Reported-by: H Buus <ubuntu@hbuus.com>
Signed-off-by: Michael Büsch <m@bues.ch>

---

diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
index e809dae4c470..66a76fd83248 100644
--- a/drivers/ssb/driver_gpio.c
+++ b/drivers/ssb/driver_gpio.c
@@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
 		return ssb_gpio_chipco_init(bus);
 	else if (ssb_extif_available(&bus->extif))
 		return ssb_gpio_extif_init(bus);
-	else
-		WARN_ON(1);
-
 	return -1;
 }
 
@@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
 	    ssb_extif_available(&bus->extif)) {
 		gpiochip_remove(&bus->gpio);
 		return 0;
-	} else {
-		WARN_ON(1);
 	}
-
 	return -1;
 }

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[Larry Finger]

On 6/10/19 1:49 PM, Michael Büsch wrote:
> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> The warning serves no purpose. So let's just remove it.
> 
> Reported-by: H Buus <ubuntu@hbuus.com>
> Signed-off-by: Michael Büsch <m@bues.ch>
> 
> ---

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

> 
> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
> index e809dae4c470..66a76fd83248 100644
> --- a/drivers/ssb/driver_gpio.c
> +++ b/drivers/ssb/driver_gpio.c
> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
>   		return ssb_gpio_chipco_init(bus);
>   	else if (ssb_extif_available(&bus->extif))
>   		return ssb_gpio_extif_init(bus);
> -	else
> -		WARN_ON(1);
> -
>   	return -1;
>   }
>   
> @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
>   	    ssb_extif_available(&bus->extif)) {
>   		gpiochip_remove(&bus->gpio);
>   		return 0;
> -	} else {
> -		WARN_ON(1);
>   	}
> -
>   	return -1;
>   }
> 

Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[H Buus]

On 6/10/2019 3:16 PM, Larry Finger wrote:
> On 6/10/19 1:49 PM, Michael Büsch wrote:
>> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
>> The warning serves no purpose. So let's just remove it.
>>
>> Reported-by: H Buus <ubuntu@hbuus.com>
>> Signed-off-by: Michael Büsch <m@bues.ch>
>>
>> ---
> 
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
> 
> Larry

Works for me. Thanks!

>>
>> diff --git a/drivers/ssb/driver_gpio.c b/drivers/ssb/driver_gpio.c
>> index e809dae4c470..66a76fd83248 100644
>> --- a/drivers/ssb/driver_gpio.c
>> +++ b/drivers/ssb/driver_gpio.c
>> @@ -460,9 +460,6 @@ int ssb_gpio_init(struct ssb_bus *bus)
>>           return ssb_gpio_chipco_init(bus);
>>       else if (ssb_extif_available(&bus->extif))
>>           return ssb_gpio_extif_init(bus);
>> -    else
>> -        WARN_ON(1);
>> -
>>       return -1;
>>   }
>>   @@ -472,9 +469,6 @@ int ssb_gpio_unregister(struct ssb_bus *bus)
>>           ssb_extif_available(&bus->extif)) {
>>           gpiochip_remove(&bus->gpio);
>>           return 0;
>> -    } else {
>> -        WARN_ON(1);
>>       }
>> -
>>       return -1;
>>   }
>>
> 

Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[Kalle Valo]

Michael Büsch <m@bues.ch> writes:

> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> The warning serves no purpose. So let's just remove it.
>
> Reported-by: H Buus <ubuntu@hbuus.com>
> Signed-off-by: Michael Büsch <m@bues.ch>

For some reason patchwork (or pwcli script) didn't like this patch so
manually applied to wireless-drivers-next:

e73e43246da6 ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

I have a faint recollection that I had a similar problem with another
patch from Michael, did we ever conclude what was the issue?

-- 
Kalle Valo

Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[Jonas Gorski]

On Tue, 25 Jun 2019 at 12:06, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Michael Büsch <m@bues.ch> writes:
>
> > The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> > The warning serves no purpose. So let's just remove it.
> >
> > Reported-by: H Buus <ubuntu@hbuus.com>
> > Signed-off-by: Michael Büsch <m@bues.ch>
>
> For some reason patchwork (or pwcli script) didn't like this patch so
> manually applied to wireless-drivers-next:
>
> e73e43246da6 ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
>
> I have a faint recollection that I had a similar problem with another
> patch from Michael, did we ever conclude what was the issue?

Yes [1], a bug in kernel.org's version of patchwork. The fix was
included in v2.1.1; latest is v2.1.3. kernel.org is (still) at v2.1.0.
Maybe it's time to poke helpdesk?

Regards
Jonas

[1] https://marc.info/?l=linux-wireless&m=153388992528252&w=2

Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

[Kalle Valo]

Jonas Gorski <jonas.gorski@gmail.com> writes:

> On Tue, 25 Jun 2019 at 12:06, Kalle Valo <kvalo@codeaurora.org> wrote:
>>
>> Michael Büsch <m@bues.ch> writes:
>>
>> > The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
>> > The warning serves no purpose. So let's just remove it.
>> >
>> > Reported-by: H Buus <ubuntu@hbuus.com>
>> > Signed-off-by: Michael Büsch <m@bues.ch>
>>
>> For some reason patchwork (or pwcli script) didn't like this patch so
>> manually applied to wireless-drivers-next:
>>
>> e73e43246da6 ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
>>
>> I have a faint recollection that I had a similar problem with another
>> patch from Michael, did we ever conclude what was the issue?
>
> Yes [1], a bug in kernel.org's version of patchwork.

Ah, that was it. Thanks!

> The fix was included in v2.1.1; latest is v2.1.3. kernel.org is
> (still) at v2.1.0. Maybe it's time to poke helpdesk?

Indeed, that would be a good idea. Apparently v2.1.0 was released a year
ago:

https://github.com/getpatchwork/patchwork/releases

So if your or anyone else has time, please do report to kernel.org
helpdesk and hopefully they can help.

-- 
Kalle Valo