When QoS is disabled, the queue priority value will not map to the correct
ieee80211 queue since there is only one queue. Stop/wake queue 0 when QoS
is disabled to prevent trying to stop/wake a non-existent queue and failing
to stop/wake the actual queue instantiated.

Fixes: 5100d5ac81b9 ("b43: Add PIO support for PCMCIA devices")
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/net/wireless/broadcom/b43/pio.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/pio.c b/drivers/net/wireless/broadcom/b43/pio.c
index 0cf70fdb60a6..7168fe50af5b 100644
--- a/drivers/net/wireless/broadcom/b43/pio.c
+++ b/drivers/net/wireless/broadcom/b43/pio.c
@@ -525,7 +525,11 @@ int b43_pio_tx(struct b43_wldev *dev, struct sk_buff *skb)
 	if (total_len > (q->buffer_size - q->buffer_used)) {
 		/* Not enough memory on the queue. */
 		err = -EBUSY;
-		ieee80211_stop_queue(dev->wl->hw, skb_get_queue_mapping(skb));
+		if (dev->qos_enabled)
+			ieee80211_stop_queue(dev->wl->hw,
+					     skb_get_queue_mapping(skb));
+		else
+			ieee80211_stop_queue(dev->wl->hw, 0);
 		q->stopped = true;
 		goto out;
 	}
@@ -552,7 +556,11 @@ int b43_pio_tx(struct b43_wldev *dev, struct sk_buff *skb)
 	if (((q->buffer_size - q->buffer_used) < roundup(2 + 2 + 6, 4)) ||
 	    (q->free_packet_slots == 0)) {
 		/* The queue is full. */
-		ieee80211_stop_queue(dev->wl->hw, skb_get_queue_mapping(skb));
+		if (dev->qos_enabled)
+			ieee80211_stop_queue(dev->wl->hw,
+					     skb_get_queue_mapping(skb));
+		else
+			ieee80211_stop_queue(dev->wl->hw, 0);
 		q->stopped = true;
 	}
 
@@ -587,7 +595,10 @@ void b43_pio_handle_txstatus(struct b43_wldev *dev,
 	list_add(&pack->list, &q->packets_list);
 
 	if (q->stopped) {
-		ieee80211_wake_queue(dev->wl->hw, q->queue_prio);
+		if (dev->qos_enabled)
+			ieee80211_wake_queue(dev->wl->hw, q->queue_prio);
+		else
+			ieee80211_wake_queue(dev->wl->hw, 0);
 		q->stopped = false;
 	}
 }
-- 
2.42.0

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?

We certainly could try extracting firmware from a newer binary. Any suggestions?

Larry

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.

-- 
Michael Büsch
https://bues.ch/

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