| Message ID | 20231230045105.91351-1-sergeantsagara@protonmail.com |
|---|---|
| Headers | show |
| Series | wifi: b43: Various QoS-related fixes | expand |
On Sat, 30 Dec, 2023 14:34:55 +0100 Michael Büsch <m@bues.ch> wrote: > [[PGP Signed Part:Undecided]] > On Sat, 30 Dec 2023 04:51:29 +0000 > Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > >> Trigger the warning message should be when the OpenFW capability for QoS >> does not advertise QoS support. Previously, the warning would be >> incorrectly triggered when OpenFW reported QoS capability is present. > >> --- a/drivers/net/wireless/broadcom/b43/main.c >> +++ b/drivers/net/wireless/broadcom/b43/main.c >> @@ -2713,7 +2713,7 @@ static int b43_upload_microcode(struct b43_wldev *dev) >> dev->hwcrypto_enabled = false; >> } >> /* adding QoS support should use an offline discovery mechanism */ >> - WARN(fwcapa & B43_FWCAPA_QOS, "QoS in OpenFW not supported\n"); >> + WARN(!(fwcapa & B43_FWCAPA_QOS), "QoS in OpenFW not supported\n"); >> } else { >> b43info(dev->wl, "Loading firmware version %u.%u " >> "(20%.2i-%.2i-%.2i %.2i:%.2i:%.2i)\n", > > I don't think this patch is correct. > It should warn, if the firmware advertises QoS, because that is not > supported by b43 in case of OpenFW. Thanks. I had a hard time understanding the intention of this warning. I figured it could be the case where the warning is about the driver disabling QoS when firmware has support but was not sure. Will drop this patch going forward. -- Thanks, Rahul Rameshbabu
On Sat, 30 Dec, 2023 12:04:02 -0600 "Larry Finger" <Larry.Finger@lwfinger.net> wrote: > On 12/29/23 22:51, Rahul Rameshbabu wrote: >> + if (dev->qos_enabled) >> + ieee80211_stop_queue(dev->wl->hw, >> + skb_get_queue_mapping(skb)); >> + else >> + ieee80211_stop_queue(dev->wl->hw, 0); > > This code sequence occurs in several places. Would it be better to put this in a > routine specific to b43, thus it would only be used once? Right, I am waiting to converge on the discussion in the second patch in this series before making this refactor, but I agree that this pattern is prevelant and should be refactored if this is the approach taken. > > We certainly could try extracting firmware from a newer binary. Any suggestions? Unfortunately, new firmware would not prevent the need to fix up the code with regards to QoS being disabled via the kernel parameter or using OpenFW. That said, new firmware could help us drop the fifth patch in this series. I am thinking about using b43-fwcutter to extract proprietary fw from a newer release of broadcom-wl to see if that makes a difference. That said, I am a bit puzzled since the device I am testing on released in early 2011, and I used firmware released in late 2012. -- Thanks, Rahul Rameshbabu
On 12/30/23 13:43, Rahul Rameshbabu wrote: > Unfortunately, new firmware would not prevent the need to fix up the > code with regards to QoS being disabled via the kernel parameter or > using OpenFW. That said, new firmware could help us drop the fifth patch > in this series. I am thinking about using b43-fwcutter to extract > proprietary fw from a newer release of broadcom-wl to see if that makes > a difference. That said, I am a bit puzzled since the device I am > testing on released in early 2011, and I used firmware released in late > 2012. Unfortunately, it is very difficult to get the parameters for fwcutter from an x86 binary. Some of the other architectures are easier. Larry
On Sat, 30 Dec, 2023 16:23:58 -0600 "Larry Finger" <Larry.Finger@lwfinger.net> wrote: > On 12/30/23 13:43, Rahul Rameshbabu wrote: >> Unfortunately, new firmware would not prevent the need to fix up the >> code with regards to QoS being disabled via the kernel parameter or >> using OpenFW. That said, new firmware could help us drop the fifth patch >> in this series. I am thinking about using b43-fwcutter to extract >> proprietary fw from a newer release of broadcom-wl to see if that makes >> a difference. That said, I am a bit puzzled since the device I am >> testing on released in early 2011, and I used firmware released in late >> 2012. > > Unfortunately, it is very difficult to get the parameters for fwcutter from an > x86 binary. Some of the other architectures are easier. Just tried this with the x86 binary just because and ran into extraction issues as expected. I could not find other architecture options from Broadcom's download page, but I may not have been looking well enough... ❯ b43-fwcutter ./wlc_hybrid.o_shipped Sorry, the input file is either wrong or not supported by b43-fwcutter. This file has an unknown MD5sum 6889dbd24abf8006de5cc6eddd138518. https://www.broadcom.com/support/download-search?pg=Wireless+Embedded+Solutions+and+RF+Components&pf=Legacy+Wireless&pn=&pa=Driver&po=&dk=BCM4331&pl=&l=true I guess, for now, we can keep the exception for bcm4331 and see if future firmware extractions help. -- Thanks, Rahul Rameshbabu
On Sun, 31 Dec 2023 00:02:32 +0000 Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > > Unfortunately, it is very difficult to get the parameters for fwcutter from an > > x86 binary. Some of the other architectures are easier. > > Just tried this with the x86 binary just because and ran into extraction > issues as expected. I could not find other architecture options from > Broadcom's download page, but I may not have been looking well enough... > > ❯ b43-fwcutter ./wlc_hybrid.o_shipped > Sorry, the input file is either wrong or not supported by b43-fwcutter. > This file has an unknown MD5sum 6889dbd24abf8006de5cc6eddd138518. b43-fwcutter works only on known files. It has a table of hashes of these files. But there is a script that can be used to create a hash table entry for a .o file: https://bues.ch/cgit/b43-tools.git/plain/fwcutter/mklist.py This probably doesn't work on x86 binaries, though. But maybe by reading the script you can get an idea how this works.
On Sun, 31 Dec, 2023 10:33:02 +0100 Michael Büsch <m@bues.ch> wrote: > [[PGP Signed Part:Undecided]] > On Sun, 31 Dec 2023 00:02:32 +0000 > Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote: > >> > Unfortunately, it is very difficult to get the parameters for fwcutter from an >> > x86 binary. Some of the other architectures are easier. >> >> Just tried this with the x86 binary just because and ran into extraction >> issues as expected. I could not find other architecture options from >> Broadcom's download page, but I may not have been looking well enough... >> >> ❯ b43-fwcutter ./wlc_hybrid.o_shipped >> Sorry, the input file is either wrong or not supported by b43-fwcutter. >> This file has an unknown MD5sum 6889dbd24abf8006de5cc6eddd138518. > > b43-fwcutter works only on known files. It has a table of hashes of these files. > > But there is a script that can be used to create a hash table entry for a .o file: > https://bues.ch/cgit/b43-tools.git/plain/fwcutter/mklist.py > > This probably doesn't work on x86 binaries, though. > But maybe by reading the script you can get an idea how this works. Thanks for the pointer. I will take a look and follow up with you and Larry with the b43-dev mailing list CCed. If we can get QoS working on bcm4331, then I can send a revert for the disablement patch. -- Thanks, Rahul Rameshbabu
Recently acquired a MacBookPro8,3, which has a bcm4331 card. Noticed some issues with the wireless driver, specifically related to QoS, when using this device. Out of the box, applications like ssh appear to not work with the device when QoS is enabled. This series attempts to improve the out-of-box experience while cleaning up some fundamental issues in the driver when QoS is disabled, either by the related kernel parameter or the newly introduced QoS disablement function. Running FW 666.2 during testing. Log: [ +0.169771] b43-phy7: Loading firmware version 666.2 (2011-02-23 01:15:07) [ +0.249032] b43-phy7: Loading firmware version 666.2 (2011-02-23 01:15:07) [ +1.394130] b43-phy7: Loading firmware version 666.2 (2011-02-23 01:15:07) Rahul Rameshbabu (5): wifi: b43: Correct OpenFW QoS capability warning conditional wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled wifi: b43: Stop correct queue in DMA worker when QoS is disabled wifi: b43: Support advertising lack of QoS capability drivers/net/wireless/broadcom/b43/dma.c | 10 ++++++-- drivers/net/wireless/broadcom/b43/main.c | 32 +++++++++++++++++++----- drivers/net/wireless/broadcom/b43/pio.c | 17 ++++++++++--- 3 files changed, 48 insertions(+), 11 deletions(-)