b43-dev.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes
@ 2023-12-30  4:51 Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional Rahul Rameshbabu
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, 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(-)

-- 
2.42.0



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional
  2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
@ 2023-12-30  4:51 ` Rahul Rameshbabu
  2023-12-30 13:34   ` Michael Büsch
  2023-12-30  4:51 ` [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled Rahul Rameshbabu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, Rahul Rameshbabu

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.

Fixes: 097b0e1bf18a ("b43: fix crash with OpenFWWF")
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/net/wireless/broadcom/b43/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index 92ca0b2ca286..c81117a22ebf 100644
--- 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",
-- 
2.42.0



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional Rahul Rameshbabu
@ 2023-12-30  4:51 ` Rahul Rameshbabu
  2023-12-30  7:48   ` Julian Calaby
  2023-12-30  4:51 ` [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO " Rahul Rameshbabu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, Rahul Rameshbabu

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.

Log of issue before change (with kernel parameter qos=0):
    [  +5.112651] ------------[ cut here ]------------
    [  +0.000005] WARNING: CPU: 7 PID: 25513 at net/mac80211/util.c:449 __ieee80211_wake_queue+0xd5/0x180 [mac80211]
    [  +0.000067] Modules linked in: b43(O) snd_seq_dummy snd_hrtimer snd_seq snd_seq_device nft_chain_nat xt_MASQUERADE nf_nat xfrm_user xfrm_algo xt_addrtype overlay ccm af_packet amdgpu snd_hda_codec_cirrus snd_hda_codec_generic ledtrig_audio drm_exec amdxcp gpu_sched xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6t_rpfilter ipt_rpfilter xt_pkttype xt_LOG nf_log_syslog xt_tcpudp nft_compat nf_tables nfnetlink sch_fq_codel btusb uinput iTCO_wdt ctr btrtl intel_pmc_bxt i915 intel_rapl_msr mei_hdcp mei_pxp joydev at24 watchdog btintel atkbd libps2 serio radeon btbcm vivaldi_fmap btmtk intel_rapl_common snd_hda_codec_hdmi bluetooth uvcvideo nls_iso8859_1 applesmc nls_cp437 x86_pkg_temp_thermal snd_hda_intel intel_powerclamp vfat videobuf2_vmalloc coretemp fat snd_intel_dspcfg crc32_pclmul uvc polyval_clmulni snd_intel_sdw_acpi loop videobuf2_memops snd_hda_codec tun drm_suballoc_helper polyval_generic drm_ttm_helper drm_buddy tap ecdh_generic videobuf2_v4l2 gf128mul macvlan ttm ghash_clmulni_intel ecc tg3
    [  +0.000044]  videodev bridge snd_hda_core rapl crc16 drm_display_helper cec mousedev snd_hwdep evdev intel_cstate bcm5974 hid_appleir videobuf2_common stp mac_hid libphy snd_pcm drm_kms_helper acpi_als mei_me intel_uncore llc mc snd_timer intel_gtt industrialio_triggered_buffer apple_mfi_fastcharge i2c_i801 mei snd lpc_ich agpgart ptp i2c_smbus thunderbolt apple_gmux i2c_algo_bit kfifo_buf video industrialio soundcore pps_core wmi tiny_power_button sbs sbshc button ac cordic bcma mac80211 cfg80211 ssb rfkill libarc4 kvm_intel kvm drm irqbypass fuse backlight firmware_class efi_pstore configfs efivarfs dmi_sysfs ip_tables x_tables autofs4 dm_crypt cbc encrypted_keys trusted asn1_encoder tee tpm rng_core input_leds hid_apple led_class hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft crc64 crc_t10dif crct10dif_generic ahci libahci libata uhci_hcd ehci_pci ehci_hcd crct10dif_pclmul crct10dif_common sha512_ssse3 sha512_generic sha256_ssse3 sha1_ssse3 aesni_intel usbcore scsi_mod libaes crypto_simd cryptd scsi_common
    [  +0.000055]  usb_common rtc_cmos btrfs blake2b_generic libcrc32c crc32c_generic crc32c_intel xor raid6_pq dm_snapshot dm_bufio dm_mod dax [last unloaded: b43(O)]
    [  +0.000009] CPU: 7 PID: 25513 Comm: irq/17-b43 Tainted: G        W  O       6.6.7 #1-NixOS
    [  +0.000003] Hardware name: Apple Inc. MacBookPro8,3/Mac-942459F5819B171B, BIOS 87.0.0.0.0 06/13/2019
    [  +0.000001] RIP: 0010:__ieee80211_wake_queue+0xd5/0x180 [mac80211]
    [  +0.000046] Code: 00 45 85 e4 0f 85 9b 00 00 00 48 8d bd 40 09 00 00 f0 48 0f ba ad 48 09 00 00 00 72 0f 5b 5d 41 5c 41 5d 41 5e e9 cb 6d 3c d0 <0f> 0b 5b 5d 41 5c 41 5d 41 5e c3 cc cc cc cc 48 8d b4 16 94 00 00
    [  +0.000002] RSP: 0018:ffffc90003c77d60 EFLAGS: 00010097
    [  +0.000001] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 0000000000000000
    [  +0.000001] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffff88820b924900
    [  +0.000002] RBP: ffff88820b924900 R08: ffffc90003c77d90 R09: 000000000003bfd0
    [  +0.000001] R10: ffff88820b924900 R11: ffffc90003c77c68 R12: 0000000000000000
    [  +0.000001] R13: 0000000000000000 R14: ffffc90003c77d90 R15: ffffffffc0fa6f40
    [  +0.000001] FS:  0000000000000000(0000) GS:ffff88846fb80000(0000) knlGS:0000000000000000
    [  +0.000001] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [  +0.000001] CR2: 00007fafda7ae008 CR3: 000000046d220005 CR4: 00000000000606e0
    [  +0.000002] Call Trace:
    [  +0.000003]  <TASK>
    [  +0.000001]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
    [  +0.000044]  ? __warn+0x81/0x130
    [  +0.000005]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
    [  +0.000045]  ? report_bug+0x171/0x1a0
    [  +0.000004]  ? handle_bug+0x41/0x70
    [  +0.000004]  ? exc_invalid_op+0x17/0x70
    [  +0.000003]  ? asm_exc_invalid_op+0x1a/0x20
    [  +0.000005]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
    [  +0.000043]  ieee80211_wake_queue+0x4a/0x80 [mac80211]
    [  +0.000044]  b43_dma_handle_txstatus+0x29c/0x3a0 [b43]
    [  +0.000016]  ? __pfx_irq_thread_fn+0x10/0x10
    [  +0.000002]  b43_handle_txstatus+0x61/0x80 [b43]
    [  +0.000012]  b43_interrupt_thread_handler+0x3f9/0x6b0 [b43]
    [  +0.000011]  irq_thread_fn+0x23/0x60
    [  +0.000002]  irq_thread+0xfe/0x1c0
    [  +0.000002]  ? __pfx_irq_thread_dtor+0x10/0x10
    [  +0.000001]  ? __pfx_irq_thread+0x10/0x10
    [  +0.000001]  kthread+0xe8/0x120
    [  +0.000003]  ? __pfx_kthread+0x10/0x10
    [  +0.000003]  ret_from_fork+0x34/0x50
    [  +0.000002]  ? __pfx_kthread+0x10/0x10
    [  +0.000002]  ret_from_fork_asm+0x1b/0x30
    [  +0.000004]  </TASK>
    [  +0.000001] ---[ end trace 0000000000000000 ]---

    [  +0.000065] ------------[ cut here ]------------
    [  +0.000001] WARNING: CPU: 0 PID: 56077 at net/mac80211/util.c:514 __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
    [  +0.000077] Modules linked in: b43(O) snd_seq_dummy snd_hrtimer snd_seq snd_seq_device nft_chain_nat xt_MASQUERADE nf_nat xfrm_user xfrm_algo xt_addrtype overlay ccm af_packet amdgpu snd_hda_codec_cirrus snd_hda_codec_generic ledtrig_audio drm_exec amdxcp gpu_sched xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6t_rpfilter ipt_rpfilter xt_pkttype xt_LOG nf_log_syslog xt_tcpudp nft_compat nf_tables nfnetlink sch_fq_codel btusb uinput iTCO_wdt ctr btrtl intel_pmc_bxt i915 intel_rapl_msr mei_hdcp mei_pxp joydev at24 watchdog btintel atkbd libps2 serio radeon btbcm vivaldi_fmap btmtk intel_rapl_common snd_hda_codec_hdmi bluetooth uvcvideo nls_iso8859_1 applesmc nls_cp437 x86_pkg_temp_thermal snd_hda_intel intel_powerclamp vfat videobuf2_vmalloc coretemp fat snd_intel_dspcfg crc32_pclmul uvc polyval_clmulni snd_intel_sdw_acpi loop videobuf2_memops snd_hda_codec tun drm_suballoc_helper polyval_generic drm_ttm_helper drm_buddy tap ecdh_generic videobuf2_v4l2 gf128mul macvlan ttm ghash_clmulni_intel ecc tg3
    [  +0.000073]  videodev bridge snd_hda_core rapl crc16 drm_display_helper cec mousedev snd_hwdep evdev intel_cstate bcm5974 hid_appleir videobuf2_common stp mac_hid libphy snd_pcm drm_kms_helper acpi_als mei_me intel_uncore llc mc snd_timer intel_gtt industrialio_triggered_buffer apple_mfi_fastcharge i2c_i801 mei snd lpc_ich agpgart ptp i2c_smbus thunderbolt apple_gmux i2c_algo_bit kfifo_buf video industrialio soundcore pps_core wmi tiny_power_button sbs sbshc button ac cordic bcma mac80211 cfg80211 ssb rfkill libarc4 kvm_intel kvm drm irqbypass fuse backlight firmware_class efi_pstore configfs efivarfs dmi_sysfs ip_tables x_tables autofs4 dm_crypt cbc encrypted_keys trusted asn1_encoder tee tpm rng_core input_leds hid_apple led_class hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft crc64 crc_t10dif crct10dif_generic ahci libahci libata uhci_hcd ehci_pci ehci_hcd crct10dif_pclmul crct10dif_common sha512_ssse3 sha512_generic sha256_ssse3 sha1_ssse3 aesni_intel usbcore scsi_mod libaes crypto_simd cryptd scsi_common
    [  +0.000084]  usb_common rtc_cmos btrfs blake2b_generic libcrc32c crc32c_generic crc32c_intel xor raid6_pq dm_snapshot dm_bufio dm_mod dax [last unloaded: b43]
    [  +0.000012] CPU: 0 PID: 56077 Comm: kworker/u16:17 Tainted: G        W  O       6.6.7 #1-NixOS
    [  +0.000003] Hardware name: Apple Inc. MacBookPro8,3/Mac-942459F5819B171B, BIOS 87.0.0.0.0 06/13/2019
    [  +0.000001] Workqueue: phy7 b43_tx_work [b43]
    [  +0.000019] RIP: 0010:__ieee80211_stop_queue+0xcc/0xe0 [mac80211]
    [  +0.000076] Code: 74 11 48 8b 78 08 0f b7 d6 89 e9 4c 89 e6 e8 ab f4 00 00 65 ff 0d 9c b7 34 3f 0f 85 55 ff ff ff 0f 1f 44 00 00 e9 4b ff ff ff <0f> 0b 5b 5d 41 5c 41 5d c3 cc cc cc cc 0f 1f 80 00 00 00 00 90 90
    [  +0.000002] RSP: 0000:ffffc90004157d50 EFLAGS: 00010097
    [  +0.000002] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 0000000000000000
    [  +0.000002] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffff8882d65d0900
    [  +0.000002] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
    [  +0.000001] R10: 00000000000000ff R11: ffff88814d0155a0 R12: ffff8882d65d0900
    [  +0.000002] R13: 0000000000000000 R14: ffff8881002d2800 R15: 00000000000000d0
    [  +0.000002] FS:  0000000000000000(0000) GS:ffff88846f800000(0000) knlGS:0000000000000000
    [  +0.000003] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [  +0.000002] CR2: 00007f2e8c10c880 CR3: 0000000385b66005 CR4: 00000000000606f0
    [  +0.000002] Call Trace:
    [  +0.000001]  <TASK>
    [  +0.000001]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
    [  +0.000075]  ? __warn+0x81/0x130
    [  +0.000004]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
    [  +0.000075]  ? report_bug+0x171/0x1a0
    [  +0.000005]  ? handle_bug+0x41/0x70
    [  +0.000003]  ? exc_invalid_op+0x17/0x70
    [  +0.000004]  ? asm_exc_invalid_op+0x1a/0x20
    [  +0.000004]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
    [  +0.000076]  ieee80211_stop_queue+0x36/0x50 [mac80211]
    [  +0.000077]  b43_dma_tx+0x550/0x780 [b43]
    [  +0.000023]  b43_tx_work+0x90/0x130 [b43]
    [  +0.000018]  process_one_work+0x174/0x340
    [  +0.000003]  worker_thread+0x27b/0x3a0
    [  +0.000004]  ? __pfx_worker_thread+0x10/0x10
    [  +0.000002]  kthread+0xe8/0x120
    [  +0.000003]  ? __pfx_kthread+0x10/0x10
    [  +0.000004]  ret_from_fork+0x34/0x50
    [  +0.000002]  ? __pfx_kthread+0x10/0x10
    [  +0.000003]  ret_from_fork_asm+0x1b/0x30
    [  +0.000006]  </TASK>
    [  +0.000001] ---[ end trace 0000000000000000 ]---

Fixes: e6f5b934fba8 ("b43: Add QOS support")
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/net/wireless/broadcom/b43/dma.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
index 760d1a28edc6..68671de27569 100644
--- a/drivers/net/wireless/broadcom/b43/dma.c
+++ b/drivers/net/wireless/broadcom/b43/dma.c
@@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
 	    should_inject_overflow(ring)) {
 		/* This TX ring is full. */
 		unsigned int skb_mapping = skb_get_queue_mapping(skb);
-		ieee80211_stop_queue(dev->wl->hw, skb_mapping);
+		if (dev->qos_enabled)
+			ieee80211_stop_queue(dev->wl->hw, skb_mapping);
+		else
+			ieee80211_stop_queue(dev->wl->hw, 0);
 		dev->wl->tx_queue_stopped[skb_mapping] = true;
 		ring->stopped = true;
 		if (b43_debug(dev, B43_DBG_DMAVERBOSE)) {
@@ -1570,7 +1573,10 @@ void b43_dma_handle_txstatus(struct b43_wldev *dev,
 	} else {
 		/* If the driver queue is running wake the corresponding
 		 * mac80211 queue. */
-		ieee80211_wake_queue(dev->wl->hw, ring->queue_prio);
+		if (dev->qos_enabled)
+			ieee80211_wake_queue(dev->wl->hw, ring->queue_prio);
+		else
+			ieee80211_wake_queue(dev->wl->hw, 0);
 		if (b43_debug(dev, B43_DBG_DMAVERBOSE)) {
 			b43dbg(dev->wl, "Woke up TX ring %d\n", ring->index);
 		}
-- 
2.42.0



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled Rahul Rameshbabu
@ 2023-12-30  4:51 ` Rahul Rameshbabu
  2023-12-30 18:04   ` Larry Finger
  2023-12-30  4:51 ` [PATCH wireless 4/5] wifi: b43: Stop correct queue in DMA worker " Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability Rahul Rameshbabu
  4 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, Rahul Rameshbabu

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



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH wireless 4/5] wifi: b43: Stop correct queue in DMA worker when QoS is disabled
  2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
                   ` (2 preceding siblings ...)
  2023-12-30  4:51 ` [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO " Rahul Rameshbabu
@ 2023-12-30  4:51 ` Rahul Rameshbabu
  2023-12-30  4:51 ` [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability Rahul Rameshbabu
  4 siblings, 0 replies; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, Rahul Rameshbabu

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

Fixes: bad691946966 ("b43: avoid packet losses in the dma worker code.")
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/net/wireless/broadcom/b43/main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index c81117a22ebf..b6ac1526c0e8 100644
--- a/drivers/net/wireless/broadcom/b43/main.c
+++ b/drivers/net/wireless/broadcom/b43/main.c
@@ -3603,7 +3603,10 @@ static void b43_tx_work(struct work_struct *work)
 				err = b43_dma_tx(dev, skb);
 			if (err == -ENOSPC) {
 				wl->tx_queue_stopped[queue_num] = true;
-				ieee80211_stop_queue(wl->hw, queue_num);
+				if (dev->qos_enabled)
+					ieee80211_stop_queue(wl->hw, queue_num);
+				else
+					ieee80211_stop_queue(wl->hw, 0);
 				skb_queue_head(&wl->tx_queue[queue_num], skb);
 				break;
 			}
@@ -3636,11 +3639,12 @@ static void b43_op_tx(struct ieee80211_hw *hw,
 	B43_WARN_ON(skb_shinfo(skb)->nr_frags);
 
 	skb_queue_tail(&wl->tx_queue[skb->queue_mapping], skb);
-	if (!wl->tx_queue_stopped[skb->queue_mapping]) {
+	if (!wl->tx_queue_stopped[skb->queue_mapping])
 		ieee80211_queue_work(wl->hw, &wl->tx_work);
-	} else {
+	else if (wl->current_dev->qos_enabled)
 		ieee80211_stop_queue(wl->hw, skb->queue_mapping);
-	}
+	else
+		ieee80211_stop_queue(wl->hw, 0);
 }
 
 static void b43_qos_params_upload(struct b43_wldev *dev,
-- 
2.42.0



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability
  2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
                   ` (3 preceding siblings ...)
  2023-12-30  4:51 ` [PATCH wireless 4/5] wifi: b43: Stop correct queue in DMA worker " Rahul Rameshbabu
@ 2023-12-30  4:51 ` Rahul Rameshbabu
  2023-12-30 13:45   ` Michael Büsch
  4 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30  4:51 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel, Rahul Rameshbabu

Add b43_qos_not_supported functionality for disabling QoS. Currently, the
function checks an array to see if the chip id matches a value in the
array. If so, disable QoS.

bcm4331 appears to lack QoS support. When queues that are not of the
default "best effort" priority are selected, traffic appears to not
transmit out of the hardware while no errors are returned. This behavior is
present among all the other priority queues: video, voice, and background.
While this can be worked around by setting a kernel parameter, the default
behavior is problematic for most users and may be difficult to debug. This
patch offers a working out-of-box experience for bcm4331 users.

Log of the issue (using ssh low-priority traffic as an example):
    ssh -T -vvvv git@github.com
    OpenSSH_9.6p1, OpenSSL 3.0.12 24 Oct 2023
    debug1: Reading configuration data /etc/ssh/ssh_config
    debug2: checking match for 'host * exec "/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0rypm7sh1i8js8w-gnupg-2.4.1/bin/gpg-connect-agent --quiet updatestartuptty /bye >/dev/null 2>&1'"' host github.com originally github.com
    debug3: /etc/ssh/ssh_config line 5: matched 'host "github.com"'
    debug1: Executing command: '/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0rypm7sh1i8js8w-gnupg-2.4.1/bin/gpg-connect-agent --quiet updatestartuptty /bye >/dev/null 2>&1''
    debug3: command returned status 0
    debug3: /etc/ssh/ssh_config line 5: matched 'exec "/nix/store/q1c2flcykgr4wwg5a6h450hxbk4ch589-bash-5.2-p15/bin/bash -c '/nix/store/c015armnkhr6v18za0r"'
    debug2: match found
    debug1: /etc/ssh/ssh_config line 9: Applying options for *
    debug3: expanded UserKnownHostsFile '~/.ssh/known_hosts' -> '/home/binary-eater/.ssh/known_hosts'
    debug3: expanded UserKnownHostsFile '~/.ssh/known_hosts2' -> '/home/binary-eater/.ssh/known_hosts2'
    debug2: resolving "github.com" port 22
    debug3: resolve_host: lookup github.com:22
    debug3: channel_clear_timeouts: clearing
    debug3: ssh_connect_direct: entering
    debug1: Connecting to github.com [192.30.255.113] port 22.
    debug3: set_sock_tos: set socket 3 IP_TOS 0x48

Fixes: e6f5b934fba8 ("b43: Add QOS support")
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 drivers/net/wireless/broadcom/b43/main.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/main.c b/drivers/net/wireless/broadcom/b43/main.c
index b6ac1526c0e8..39906bebef7b 100644
--- a/drivers/net/wireless/broadcom/b43/main.c
+++ b/drivers/net/wireless/broadcom/b43/main.c
@@ -359,6 +359,22 @@ static struct ieee80211_supported_band b43_band_2ghz_limited = {
 	.n_bitrates	= b43_g_ratetable_size,
 };
 
+static const u16 b43_no_qos_chip_ids[] = {
+	BCMA_CHIP_ID_BCM4331,
+	0,
+};
+
+static bool b43_qos_not_supported(struct b43_wldev *dev)
+{
+	int idx;
+
+	for (idx = 0; b43_no_qos_chip_ids[idx]; idx++)
+		if (dev->dev->chip_id == b43_no_qos_chip_ids[idx])
+			return true;
+
+	return false;
+}
+
 static void b43_wireless_core_exit(struct b43_wldev *dev);
 static int b43_wireless_core_init(struct b43_wldev *dev);
 static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev);
@@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work)
 
 start_ieee80211:
 	wl->hw->queues = B43_QOS_QUEUE_NUM;
-	if (!modparam_qos || dev->fw.opensource)
+	if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev))
 		wl->hw->queues = 1;
 
 	err = ieee80211_register_hw(wl->hw);
-- 
2.42.0



_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30  4:51 ` [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled Rahul Rameshbabu
@ 2023-12-30  7:48   ` Julian Calaby
  2023-12-30 13:40     ` Michael Büsch
  0 siblings, 1 reply; 22+ messages in thread
From: Julian Calaby @ 2023-12-30  7:48 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

Hi Rahul,

On Sat, Dec 30, 2023 at 3:52 PM Rahul Rameshbabu
<sergeantsagara@protonmail.com> wrote:
>
> 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.
>
> Log of issue before change (with kernel parameter qos=0):
>     [  +5.112651] ------------[ cut here ]------------
>     [  +0.000005] WARNING: CPU: 7 PID: 25513 at net/mac80211/util.c:449 __ieee80211_wake_queue+0xd5/0x180 [mac80211]
>     [  +0.000067] Modules linked in: b43(O) snd_seq_dummy snd_hrtimer snd_seq snd_seq_device nft_chain_nat xt_MASQUERADE nf_nat xfrm_user xfrm_algo xt_addrtype overlay ccm af_packet amdgpu snd_hda_codec_cirrus snd_hda_codec_generic ledtrig_audio drm_exec amdxcp gpu_sched xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6t_rpfilter ipt_rpfilter xt_pkttype xt_LOG nf_log_syslog xt_tcpudp nft_compat nf_tables nfnetlink sch_fq_codel btusb uinput iTCO_wdt ctr btrtl intel_pmc_bxt i915 intel_rapl_msr mei_hdcp mei_pxp joydev at24 watchdog btintel atkbd libps2 serio radeon btbcm vivaldi_fmap btmtk intel_rapl_common snd_hda_codec_hdmi bluetooth uvcvideo nls_iso8859_1 applesmc nls_cp437 x86_pkg_temp_thermal snd_hda_intel intel_powerclamp vfat videobuf2_vmalloc coretemp fat snd_intel_dspcfg crc32_pclmul uvc polyval_clmulni snd_intel_sdw_acpi loop videobuf2_memops snd_hda_codec tun drm_suballoc_helper polyval_generic drm_ttm_helper drm_buddy tap ecdh_generic videobuf2_v4l2 gf128mul macvlan ttm ghash_clmulni_intel ecc tg3
>     [  +0.000044]  videodev bridge snd_hda_core rapl crc16 drm_display_helper cec mousedev snd_hwdep evdev intel_cstate bcm5974 hid_appleir videobuf2_common stp mac_hid libphy snd_pcm drm_kms_helper acpi_als mei_me intel_uncore llc mc snd_timer intel_gtt industrialio_triggered_buffer apple_mfi_fastcharge i2c_i801 mei snd lpc_ich agpgart ptp i2c_smbus thunderbolt apple_gmux i2c_algo_bit kfifo_buf video industrialio soundcore pps_core wmi tiny_power_button sbs sbshc button ac cordic bcma mac80211 cfg80211 ssb rfkill libarc4 kvm_intel kvm drm irqbypass fuse backlight firmware_class efi_pstore configfs efivarfs dmi_sysfs ip_tables x_tables autofs4 dm_crypt cbc encrypted_keys trusted asn1_encoder tee tpm rng_core input_leds hid_apple led_class hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft crc64 crc_t10dif crct10dif_generic ahci libahci libata uhci_hcd ehci_pci ehci_hcd crct10dif_pclmul crct10dif_common sha512_ssse3 sha512_generic sha256_ssse3 sha1_ssse3 aesni_intel usbcore scsi_mod libaes crypto_simd cryptd scsi_common
>     [  +0.000055]  usb_common rtc_cmos btrfs blake2b_generic libcrc32c crc32c_generic crc32c_intel xor raid6_pq dm_snapshot dm_bufio dm_mod dax [last unloaded: b43(O)]
>     [  +0.000009] CPU: 7 PID: 25513 Comm: irq/17-b43 Tainted: G        W  O       6.6.7 #1-NixOS
>     [  +0.000003] Hardware name: Apple Inc. MacBookPro8,3/Mac-942459F5819B171B, BIOS 87.0.0.0.0 06/13/2019
>     [  +0.000001] RIP: 0010:__ieee80211_wake_queue+0xd5/0x180 [mac80211]
>     [  +0.000046] Code: 00 45 85 e4 0f 85 9b 00 00 00 48 8d bd 40 09 00 00 f0 48 0f ba ad 48 09 00 00 00 72 0f 5b 5d 41 5c 41 5d 41 5e e9 cb 6d 3c d0 <0f> 0b 5b 5d 41 5c 41 5d 41 5e c3 cc cc cc cc 48 8d b4 16 94 00 00
>     [  +0.000002] RSP: 0018:ffffc90003c77d60 EFLAGS: 00010097
>     [  +0.000001] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 0000000000000000
>     [  +0.000001] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffff88820b924900
>     [  +0.000002] RBP: ffff88820b924900 R08: ffffc90003c77d90 R09: 000000000003bfd0
>     [  +0.000001] R10: ffff88820b924900 R11: ffffc90003c77c68 R12: 0000000000000000
>     [  +0.000001] R13: 0000000000000000 R14: ffffc90003c77d90 R15: ffffffffc0fa6f40
>     [  +0.000001] FS:  0000000000000000(0000) GS:ffff88846fb80000(0000) knlGS:0000000000000000
>     [  +0.000001] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     [  +0.000001] CR2: 00007fafda7ae008 CR3: 000000046d220005 CR4: 00000000000606e0
>     [  +0.000002] Call Trace:
>     [  +0.000003]  <TASK>
>     [  +0.000001]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
>     [  +0.000044]  ? __warn+0x81/0x130
>     [  +0.000005]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
>     [  +0.000045]  ? report_bug+0x171/0x1a0
>     [  +0.000004]  ? handle_bug+0x41/0x70
>     [  +0.000004]  ? exc_invalid_op+0x17/0x70
>     [  +0.000003]  ? asm_exc_invalid_op+0x1a/0x20
>     [  +0.000005]  ? __ieee80211_wake_queue+0xd5/0x180 [mac80211]
>     [  +0.000043]  ieee80211_wake_queue+0x4a/0x80 [mac80211]
>     [  +0.000044]  b43_dma_handle_txstatus+0x29c/0x3a0 [b43]
>     [  +0.000016]  ? __pfx_irq_thread_fn+0x10/0x10
>     [  +0.000002]  b43_handle_txstatus+0x61/0x80 [b43]
>     [  +0.000012]  b43_interrupt_thread_handler+0x3f9/0x6b0 [b43]
>     [  +0.000011]  irq_thread_fn+0x23/0x60
>     [  +0.000002]  irq_thread+0xfe/0x1c0
>     [  +0.000002]  ? __pfx_irq_thread_dtor+0x10/0x10
>     [  +0.000001]  ? __pfx_irq_thread+0x10/0x10
>     [  +0.000001]  kthread+0xe8/0x120
>     [  +0.000003]  ? __pfx_kthread+0x10/0x10
>     [  +0.000003]  ret_from_fork+0x34/0x50
>     [  +0.000002]  ? __pfx_kthread+0x10/0x10
>     [  +0.000002]  ret_from_fork_asm+0x1b/0x30
>     [  +0.000004]  </TASK>
>     [  +0.000001] ---[ end trace 0000000000000000 ]---
>
>     [  +0.000065] ------------[ cut here ]------------
>     [  +0.000001] WARNING: CPU: 0 PID: 56077 at net/mac80211/util.c:514 __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
>     [  +0.000077] Modules linked in: b43(O) snd_seq_dummy snd_hrtimer snd_seq snd_seq_device nft_chain_nat xt_MASQUERADE nf_nat xfrm_user xfrm_algo xt_addrtype overlay ccm af_packet amdgpu snd_hda_codec_cirrus snd_hda_codec_generic ledtrig_audio drm_exec amdxcp gpu_sched xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip6t_rpfilter ipt_rpfilter xt_pkttype xt_LOG nf_log_syslog xt_tcpudp nft_compat nf_tables nfnetlink sch_fq_codel btusb uinput iTCO_wdt ctr btrtl intel_pmc_bxt i915 intel_rapl_msr mei_hdcp mei_pxp joydev at24 watchdog btintel atkbd libps2 serio radeon btbcm vivaldi_fmap btmtk intel_rapl_common snd_hda_codec_hdmi bluetooth uvcvideo nls_iso8859_1 applesmc nls_cp437 x86_pkg_temp_thermal snd_hda_intel intel_powerclamp vfat videobuf2_vmalloc coretemp fat snd_intel_dspcfg crc32_pclmul uvc polyval_clmulni snd_intel_sdw_acpi loop videobuf2_memops snd_hda_codec tun drm_suballoc_helper polyval_generic drm_ttm_helper drm_buddy tap ecdh_generic videobuf2_v4l2 gf128mul macvlan ttm ghash_clmulni_intel ecc tg3
>     [  +0.000073]  videodev bridge snd_hda_core rapl crc16 drm_display_helper cec mousedev snd_hwdep evdev intel_cstate bcm5974 hid_appleir videobuf2_common stp mac_hid libphy snd_pcm drm_kms_helper acpi_als mei_me intel_uncore llc mc snd_timer intel_gtt industrialio_triggered_buffer apple_mfi_fastcharge i2c_i801 mei snd lpc_ich agpgart ptp i2c_smbus thunderbolt apple_gmux i2c_algo_bit kfifo_buf video industrialio soundcore pps_core wmi tiny_power_button sbs sbshc button ac cordic bcma mac80211 cfg80211 ssb rfkill libarc4 kvm_intel kvm drm irqbypass fuse backlight firmware_class efi_pstore configfs efivarfs dmi_sysfs ip_tables x_tables autofs4 dm_crypt cbc encrypted_keys trusted asn1_encoder tee tpm rng_core input_leds hid_apple led_class hid_generic usbhid hid sd_mod t10_pi crc64_rocksoft crc64 crc_t10dif crct10dif_generic ahci libahci libata uhci_hcd ehci_pci ehci_hcd crct10dif_pclmul crct10dif_common sha512_ssse3 sha512_generic sha256_ssse3 sha1_ssse3 aesni_intel usbcore scsi_mod libaes crypto_simd cryptd scsi_common
>     [  +0.000084]  usb_common rtc_cmos btrfs blake2b_generic libcrc32c crc32c_generic crc32c_intel xor raid6_pq dm_snapshot dm_bufio dm_mod dax [last unloaded: b43]
>     [  +0.000012] CPU: 0 PID: 56077 Comm: kworker/u16:17 Tainted: G        W  O       6.6.7 #1-NixOS
>     [  +0.000003] Hardware name: Apple Inc. MacBookPro8,3/Mac-942459F5819B171B, BIOS 87.0.0.0.0 06/13/2019
>     [  +0.000001] Workqueue: phy7 b43_tx_work [b43]
>     [  +0.000019] RIP: 0010:__ieee80211_stop_queue+0xcc/0xe0 [mac80211]
>     [  +0.000076] Code: 74 11 48 8b 78 08 0f b7 d6 89 e9 4c 89 e6 e8 ab f4 00 00 65 ff 0d 9c b7 34 3f 0f 85 55 ff ff ff 0f 1f 44 00 00 e9 4b ff ff ff <0f> 0b 5b 5d 41 5c 41 5d c3 cc cc cc cc 0f 1f 80 00 00 00 00 90 90
>     [  +0.000002] RSP: 0000:ffffc90004157d50 EFLAGS: 00010097
>     [  +0.000002] RAX: 0000000000000001 RBX: 0000000000000002 RCX: 0000000000000000
>     [  +0.000002] RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffff8882d65d0900
>     [  +0.000002] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
>     [  +0.000001] R10: 00000000000000ff R11: ffff88814d0155a0 R12: ffff8882d65d0900
>     [  +0.000002] R13: 0000000000000000 R14: ffff8881002d2800 R15: 00000000000000d0
>     [  +0.000002] FS:  0000000000000000(0000) GS:ffff88846f800000(0000) knlGS:0000000000000000
>     [  +0.000003] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     [  +0.000002] CR2: 00007f2e8c10c880 CR3: 0000000385b66005 CR4: 00000000000606f0
>     [  +0.000002] Call Trace:
>     [  +0.000001]  <TASK>
>     [  +0.000001]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
>     [  +0.000075]  ? __warn+0x81/0x130
>     [  +0.000004]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
>     [  +0.000075]  ? report_bug+0x171/0x1a0
>     [  +0.000005]  ? handle_bug+0x41/0x70
>     [  +0.000003]  ? exc_invalid_op+0x17/0x70
>     [  +0.000004]  ? asm_exc_invalid_op+0x1a/0x20
>     [  +0.000004]  ? __ieee80211_stop_queue+0xcc/0xe0 [mac80211]
>     [  +0.000076]  ieee80211_stop_queue+0x36/0x50 [mac80211]
>     [  +0.000077]  b43_dma_tx+0x550/0x780 [b43]
>     [  +0.000023]  b43_tx_work+0x90/0x130 [b43]
>     [  +0.000018]  process_one_work+0x174/0x340
>     [  +0.000003]  worker_thread+0x27b/0x3a0
>     [  +0.000004]  ? __pfx_worker_thread+0x10/0x10
>     [  +0.000002]  kthread+0xe8/0x120
>     [  +0.000003]  ? __pfx_kthread+0x10/0x10
>     [  +0.000004]  ret_from_fork+0x34/0x50
>     [  +0.000002]  ? __pfx_kthread+0x10/0x10
>     [  +0.000003]  ret_from_fork_asm+0x1b/0x30
>     [  +0.000006]  </TASK>
>     [  +0.000001] ---[ end trace 0000000000000000 ]---
>
> Fixes: e6f5b934fba8 ("b43: Add QOS support")
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
> ---
>  drivers/net/wireless/broadcom/b43/dma.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/b43/dma.c b/drivers/net/wireless/broadcom/b43/dma.c
> index 760d1a28edc6..68671de27569 100644
> --- a/drivers/net/wireless/broadcom/b43/dma.c
> +++ b/drivers/net/wireless/broadcom/b43/dma.c
> @@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
>             should_inject_overflow(ring)) {
>                 /* This TX ring is full. */
>                 unsigned int skb_mapping = skb_get_queue_mapping(skb);
> -               ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> +               if (dev->qos_enabled)
> +                       ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> +               else
> +                       ieee80211_stop_queue(dev->wl->hw, 0);

Would this be a little cleaner if we only look up the queue mapping if
QOS is enabled? I.e.


unsigned int skb_mapping = 0;

if (dev->qos_enabled) {
    skb_mapping = skb_get_queue_mapping(skb);
}

ieee80211_stop_queue(dev->wl->hw, skb_mapping);


Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional
  2023-12-30  4:51 ` [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional Rahul Rameshbabu
@ 2023-12-30 13:34   ` Michael Büsch
  2023-12-30 17:17     ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Büsch @ 2023-12-30 13:34 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1062 bytes --]

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.


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

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30  7:48   ` Julian Calaby
@ 2023-12-30 13:40     ` Michael Büsch
  2023-12-30 17:15       ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Büsch @ 2023-12-30 13:40 UTC (permalink / raw)
  To: Julian Calaby
  Cc: Rahul Rameshbabu, Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1046 bytes --]

On Sat, 30 Dec 2023 18:48:45 +1100
Julian Calaby <julian.calaby@gmail.com> wrote:
> > --- a/drivers/net/wireless/broadcom/b43/dma.c
> > +++ b/drivers/net/wireless/broadcom/b43/dma.c
> > @@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
> >             should_inject_overflow(ring)) {
> >                 /* This TX ring is full. */
> >                 unsigned int skb_mapping = skb_get_queue_mapping(skb);
> > -               ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> > +               if (dev->qos_enabled)
> > +                       ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> > +               else
> > +                       ieee80211_stop_queue(dev->wl->hw, 0);  
> 
> Would this be a little cleaner if we only look up the queue mapping if
> QOS is enabled? I.e.

No. It would break the other uses of skb_mapping.

But I am wondering why skb_mapping is non-zero in the first place.
I think the actual bug might be somewhere else.

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

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability
  2023-12-30  4:51 ` [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability Rahul Rameshbabu
@ 2023-12-30 13:45   ` Michael Büsch
  2023-12-30 17:10     ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Büsch @ 2023-12-30 13:45 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1362 bytes --]

On Sat, 30 Dec 2023 04:51:51 +0000
Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote:

> bcm4331 appears to lack QoS support.

I think that's rather unlikely.
The firmware probably is just too old for this device.

> +static const u16 b43_no_qos_chip_ids[] = {
> +	BCMA_CHIP_ID_BCM4331,
> +	0,
> +};
> +
> +static bool b43_qos_not_supported(struct b43_wldev *dev)
> +{
> +	int idx;
> +
> +	for (idx = 0; b43_no_qos_chip_ids[idx]; idx++)
> +		if (dev->dev->chip_id == b43_no_qos_chip_ids[idx])
> +			return true;
> +
> +	return false;
> +}
> +
>  static void b43_wireless_core_exit(struct b43_wldev *dev);
>  static int b43_wireless_core_init(struct b43_wldev *dev);
>  static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev);
> @@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work)
>  
>  start_ieee80211:
>  	wl->hw->queues = B43_QOS_QUEUE_NUM;
> -	if (!modparam_qos || dev->fw.opensource)
> +	if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev))

This looks a bit over-engineered to me.
Can we just instead do it like this, please?

	if (!modparam_qos || dev->fw.opensource || dev->dev->chip_id == BCMA_CHIP_ID_BCM4331)

>  		wl->hw->queues = 1;
>  
>  	err = ieee80211_register_hw(wl->hw);





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

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability
  2023-12-30 13:45   ` Michael Büsch
@ 2023-12-30 17:10     ` Rahul Rameshbabu
  2023-12-30 17:43       ` Michael Büsch
  0 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30 17:10 UTC (permalink / raw)
  To: Michael Büsch; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

On Sat, 30 Dec, 2023 14:45:23 +0100 Michael Büsch <m@bues.ch> wrote:
> [[PGP Signed Part:Undecided]]
> On Sat, 30 Dec 2023 04:51:51 +0000
> Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote:
>
>> bcm4331 appears to lack QoS support.
>
> I think that's rather unlikely.
> The firmware probably is just too old for this device.

I just retested with newer firmware released on 2012-08-15. I still see
the same issue with QoS. This appears to be the newest firmware I can
acquire from http://lwfinger.com/b43-firmware/, which I extract from
broadcom-wl-6.30.163.46.

    [   11.661972] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19)
    [   11.919942] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19)
    [   13.717460] [drm] amdgpu kernel modesetting enabled.
    [   13.717705] amdgpu: Virtual CRAT table created for CPU
    [   13.717719] amdgpu: Topology: Add CPU node
    [   13.776896] NET: Registered PF_PACKET protocol family
    [   15.234058] b43-phy0: Loading firmware version 784.2 (2012-08-15 21:35:19)
    [   15.319388] wlp3s0b1: authenticate with 1c:87:2c:6f:f4:e0
    [   15.333239] wlp3s0b1: send auth to 1c:87:2c:6f:f4:e0 (try 1/3)
    [   15.341672] wlp3s0b1: authenticated
    [   15.341921] wlp3s0b1: associate with 1c:87:2c:6f:f4:e0 (try 1/3)
    [   15.346912] wlp3s0b1: RX AssocResp from 1c:87:2c:6f:f4:e0 (capab=0x411 status=0 aid=3)
    [   15.347255] wlp3s0b1: associated

>
>> +static const u16 b43_no_qos_chip_ids[] = {
>> +	BCMA_CHIP_ID_BCM4331,
>> +	0,
>> +};
>> +
>> +static bool b43_qos_not_supported(struct b43_wldev *dev)
>> +{
>> +	int idx;
>> +
>> +	for (idx = 0; b43_no_qos_chip_ids[idx]; idx++)
>> +		if (dev->dev->chip_id == b43_no_qos_chip_ids[idx])
>> +			return true;
>> +
>> +	return false;
>> +}
>> +
>>  static void b43_wireless_core_exit(struct b43_wldev *dev);
>>  static int b43_wireless_core_init(struct b43_wldev *dev);
>>  static struct b43_wldev * b43_wireless_core_stop(struct b43_wldev *dev);
>> @@ -2587,7 +2603,7 @@ static void b43_request_firmware(struct work_struct *work)
>>  
>>  start_ieee80211:
>>  	wl->hw->queues = B43_QOS_QUEUE_NUM;
>> -	if (!modparam_qos || dev->fw.opensource)
>> +	if (!modparam_qos || dev->fw.opensource || b43_qos_not_supported(wl->current_dev))
>
> This looks a bit over-engineered to me.
> Can we just instead do it like this, please?
>
> 	if (!modparam_qos || dev->fw.opensource || dev->dev->chip_id == BCMA_CHIP_ID_BCM4331)
>

Ack.

>>  		wl->hw->queues = 1;
>>  
>>  	err = ieee80211_register_hw(wl->hw);

--
Thanks,

Rahul Rameshbabu


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30 13:40     ` Michael Büsch
@ 2023-12-30 17:15       ` Rahul Rameshbabu
  2023-12-30 17:41         ` Michael Büsch
  0 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30 17:15 UTC (permalink / raw)
  To: Michael Büsch
  Cc: Julian Calaby, Kalle Valo, linux-wireless, b43-dev, linux-kernel

On Sat, 30 Dec, 2023 14:40:36 +0100 Michael Büsch <m@bues.ch> wrote:
> [[PGP Signed Part:Undecided]]
> On Sat, 30 Dec 2023 18:48:45 +1100
> Julian Calaby <julian.calaby@gmail.com> wrote:
>> > --- a/drivers/net/wireless/broadcom/b43/dma.c
>> > +++ b/drivers/net/wireless/broadcom/b43/dma.c
>> > @@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
>> >             should_inject_overflow(ring)) {
>> >                 /* This TX ring is full. */
>> >                 unsigned int skb_mapping = skb_get_queue_mapping(skb);
>> > -               ieee80211_stop_queue(dev->wl->hw, skb_mapping);
>> > +               if (dev->qos_enabled)
>> > +                       ieee80211_stop_queue(dev->wl->hw, skb_mapping);
>> > +               else
>> > +                       ieee80211_stop_queue(dev->wl->hw, 0);  
>> 
>> Would this be a little cleaner if we only look up the queue mapping if
>> QOS is enabled? I.e.
>
> No. It would break the other uses of skb_mapping.
>
> But I am wondering why skb_mapping is non-zero in the first place.
> I think the actual bug might be somewhere else.

Right, skb_mapping is used to map to the correct software structures DMA
mapped to the device. The reason the mapping for the best effort queue
(the default/defacto when QoS is disabled) is not zero is due to the way
initialization of the queues/rings occurs in the driver. The best effort
queue is mapped as the third queue, which leads to this issue when QoS
is disabled. Would it make more sense to change the mappings in
initialization such that the best effort queue is by default mapped to
zero, so we would not need such conditionals?

--
Thanks,

Rahul Rameshbabu


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional
  2023-12-30 13:34   ` Michael Büsch
@ 2023-12-30 17:17     ` Rahul Rameshbabu
  0 siblings, 0 replies; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30 17:17 UTC (permalink / raw)
  To: Michael Büsch; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30 17:15       ` Rahul Rameshbabu
@ 2023-12-30 17:41         ` Michael Büsch
  2023-12-30 19:37           ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Büsch @ 2023-12-30 17:41 UTC (permalink / raw)
  To: Rahul Rameshbabu
  Cc: Julian Calaby, Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2045 bytes --]

On Sat, 30 Dec 2023 17:15:18 +0000
Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote:

> On Sat, 30 Dec, 2023 14:40:36 +0100 Michael Büsch <m@bues.ch> wrote:
> > [[PGP Signed Part:Undecided]]
> > On Sat, 30 Dec 2023 18:48:45 +1100
> > Julian Calaby <julian.calaby@gmail.com> wrote:  
> >> > --- a/drivers/net/wireless/broadcom/b43/dma.c
> >> > +++ b/drivers/net/wireless/broadcom/b43/dma.c
> >> > @@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
> >> >             should_inject_overflow(ring)) {
> >> >                 /* This TX ring is full. */
> >> >                 unsigned int skb_mapping = skb_get_queue_mapping(skb);
> >> > -               ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> >> > +               if (dev->qos_enabled)
> >> > +                       ieee80211_stop_queue(dev->wl->hw, skb_mapping);
> >> > +               else
> >> > +                       ieee80211_stop_queue(dev->wl->hw, 0);    
> >> 
> >> Would this be a little cleaner if we only look up the queue mapping if
> >> QOS is enabled? I.e.  
> >
> > No. It would break the other uses of skb_mapping.
> >
> > But I am wondering why skb_mapping is non-zero in the first place.
> > I think the actual bug might be somewhere else.  
> 
> Right, skb_mapping is used to map to the correct software structures DMA
> mapped to the device. The reason the mapping for the best effort queue
> (the default/defacto when QoS is disabled) is not zero is due to the way
> initialization of the queues/rings occurs in the driver. The best effort
> queue is mapped as the third queue, which leads to this issue when QoS
> is disabled. Would it make more sense to change the mappings in
> initialization such that the best effort queue is by default mapped to
> zero, so we would not need such conditionals?

Maybe it is a good idea to find the patch that broke non-QoS.
That possibly helps to understand the situation.

Non-QoS used to work just fine.

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

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability
  2023-12-30 17:10     ` Rahul Rameshbabu
@ 2023-12-30 17:43       ` Michael Büsch
  0 siblings, 0 replies; 22+ messages in thread
From: Michael Büsch @ 2023-12-30 17:43 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 692 bytes --]

On Sat, 30 Dec 2023 17:10:26 +0000
Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote:

> > The firmware probably is just too old for this device.  
> 
> I just retested with newer firmware released on 2012-08-15. I still see
> the same issue with QoS. This appears to be the newest firmware I can
> acquire from http://lwfinger.com/b43-firmware/, which I extract from
> broadcom-wl-6.30.163.46.

It's still pretty old.
Nobody has worked on getting a newer version extracted for a long time.

But I'm fine with just disabling QoS on this device.
Upgrading to a newer firmware probably is a bigger change.
That could be done later.

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

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-30  4:51 ` [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO " Rahul Rameshbabu
@ 2023-12-30 18:04   ` Larry Finger
  2023-12-30 19:43     ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Larry Finger @ 2023-12-30 18:04 UTC (permalink / raw)
  To: Rahul Rameshbabu, Kalle Valo; +Cc: linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled
  2023-12-30 17:41         ` Michael Büsch
@ 2023-12-30 19:37           ` Rahul Rameshbabu
  0 siblings, 0 replies; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30 19:37 UTC (permalink / raw)
  To: Michael Büsch
  Cc: Julian Calaby, Kalle Valo, linux-wireless, b43-dev, linux-kernel

On Sat, 30 Dec, 2023 18:41:13 +0100 Michael Büsch <m@bues.ch> wrote:
> [[PGP Signed Part:Undecided]]
> On Sat, 30 Dec 2023 17:15:18 +0000
> Rahul Rameshbabu <sergeantsagara@protonmail.com> wrote:
>
>> On Sat, 30 Dec, 2023 14:40:36 +0100 Michael Büsch <m@bues.ch> wrote:
>> > [[PGP Signed Part:Undecided]]
>> > On Sat, 30 Dec 2023 18:48:45 +1100
>> > Julian Calaby <julian.calaby@gmail.com> wrote:  
>> >> > --- a/drivers/net/wireless/broadcom/b43/dma.c
>> >> > +++ b/drivers/net/wireless/broadcom/b43/dma.c
>> >> > @@ -1399,7 +1399,10 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
>> >> >             should_inject_overflow(ring)) {
>> >> >                 /* This TX ring is full. */
>> >> >                 unsigned int skb_mapping = skb_get_queue_mapping(skb);
>> >> > -               ieee80211_stop_queue(dev->wl->hw, skb_mapping);
>> >> > +               if (dev->qos_enabled)
>> >> > +                       ieee80211_stop_queue(dev->wl->hw, skb_mapping);
>> >> > +               else
>> >> > +                       ieee80211_stop_queue(dev->wl->hw, 0);    
>> >> 
>> >> Would this be a little cleaner if we only look up the queue mapping if
>> >> QOS is enabled? I.e.  
>> >
>> > No. It would break the other uses of skb_mapping.
>> >
>> > But I am wondering why skb_mapping is non-zero in the first place.
>> > I think the actual bug might be somewhere else.  
>> 
>> Right, skb_mapping is used to map to the correct software structures DMA
>> mapped to the device. The reason the mapping for the best effort queue
>> (the default/defacto when QoS is disabled) is not zero is due to the way
>> initialization of the queues/rings occurs in the driver. The best effort
>> queue is mapped as the third queue, which leads to this issue when QoS
>> is disabled. Would it make more sense to change the mappings in
>> initialization such that the best effort queue is by default mapped to
>> zero, so we would not need such conditionals?
>
> Maybe it is a good idea to find the patch that broke non-QoS.
> That possibly helps to understand the situation.
>
> Non-QoS used to work just fine.

I did some git analysis, and I actually believe that this issue has been
present since the commit e6f5b934fba8 ("b43: Add QOS support"). Before
then, non-QOS would not trigger this issue. There is a cosmetic change
after this commit, b27faf8ebf25 ("b43: Rename the DMA ring pointers"),
but this change does not introduce the issue (but makes it more
obvious). I think the reason nobody has ever reported this is that even
when the warnings are triggered, everything appears to work fine. I
think therefore the two options are the following.

1. Remap the BE queue to 0 instead of the BK queue.
2. Use this kind of conditional to handle the mapping when QoS is
   disabled.

--
Thanks,

Rahul Rameshbabu


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-30 18:04   ` Larry Finger
@ 2023-12-30 19:43     ` Rahul Rameshbabu
  2023-12-30 22:23       ` Larry Finger
  0 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-30 19:43 UTC (permalink / raw)
  To: Larry Finger; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-30 19:43     ` Rahul Rameshbabu
@ 2023-12-30 22:23       ` Larry Finger
  2023-12-31  0:02         ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Larry Finger @ 2023-12-30 22:23 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-30 22:23       ` Larry Finger
@ 2023-12-31  0:02         ` Rahul Rameshbabu
  2023-12-31  9:33           ` Michael Büsch
  0 siblings, 1 reply; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-31  0:02 UTC (permalink / raw)
  To: Larry Finger; +Cc: Kalle Valo, linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-31  0:02         ` Rahul Rameshbabu
@ 2023-12-31  9:33           ` Michael Büsch
  2023-12-31 17:29             ` Rahul Rameshbabu
  0 siblings, 1 reply; 22+ messages in thread
From: Michael Büsch @ 2023-12-31  9:33 UTC (permalink / raw)
  To: Rahul Rameshbabu
  Cc: Larry Finger, Kalle Valo, linux-wireless, b43-dev, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1063 bytes --]

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/

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

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO Tx path when QoS is disabled
  2023-12-31  9:33           ` Michael Büsch
@ 2023-12-31 17:29             ` Rahul Rameshbabu
  0 siblings, 0 replies; 22+ messages in thread
From: Rahul Rameshbabu @ 2023-12-31 17:29 UTC (permalink / raw)
  To: Michael Büsch
  Cc: Larry Finger, Kalle Valo, linux-wireless, b43-dev, linux-kernel

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


_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-12-31 17:29 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-30  4:51 [PATCH wireless 0/5] wifi: b43: Various QoS-related fixes Rahul Rameshbabu
2023-12-30  4:51 ` [PATCH wireless 1/5] wifi: b43: Correct OpenFW QoS capability warning conditional Rahul Rameshbabu
2023-12-30 13:34   ` Michael Büsch
2023-12-30 17:17     ` Rahul Rameshbabu
2023-12-30  4:51 ` [PATCH wireless 2/5] wifi: b43: Stop/wake correct queue in DMA Tx path when QoS is disabled Rahul Rameshbabu
2023-12-30  7:48   ` Julian Calaby
2023-12-30 13:40     ` Michael Büsch
2023-12-30 17:15       ` Rahul Rameshbabu
2023-12-30 17:41         ` Michael Büsch
2023-12-30 19:37           ` Rahul Rameshbabu
2023-12-30  4:51 ` [PATCH wireless 3/5] wifi: b43: Stop/wake correct queue in PIO " Rahul Rameshbabu
2023-12-30 18:04   ` Larry Finger
2023-12-30 19:43     ` Rahul Rameshbabu
2023-12-30 22:23       ` Larry Finger
2023-12-31  0:02         ` Rahul Rameshbabu
2023-12-31  9:33           ` Michael Büsch
2023-12-31 17:29             ` Rahul Rameshbabu
2023-12-30  4:51 ` [PATCH wireless 4/5] wifi: b43: Stop correct queue in DMA worker " Rahul Rameshbabu
2023-12-30  4:51 ` [PATCH wireless 5/5] wifi: b43: Support advertising lack of QoS capability Rahul Rameshbabu
2023-12-30 13:45   ` Michael Büsch
2023-12-30 17:10     ` Rahul Rameshbabu
2023-12-30 17:43       ` Michael Büsch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).