2023-02-04 23:30:34

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 0/4] rtw88: four small code-cleanups and refactorings

Hello,

this series consists of four small patches which clean up and refactor
existing code in preparation for SDIO support. Functionality is
supposed to stay the same with these changes.

The goal of the first two patches is to make it easier to understand
the allowed values in the queue by using enum rtw_tx_queue_type instead
of u8.

The third patch in this series moves the rtw_tx_queue_type code out of
pci.c so it can be re-used by SDIO (and also USB) HCIs.

The last patch is another small cleanup to improve readability of the
code by using (already existing) macros instead of magic BIT(n).


Changes since v1 at [0]:
- add "wifi" to the subject of all patches
- add Ping-Ke's Acked-by to patches 2 and 4 (thank you!)
- add const keyword in patch 1
- add array bounds checking in patch 3
- remove references to another series from the cover letter as it's
not needed as a precondition / dependency anymore


[0] https://lore.kernel.org/netdev/[email protected]/


Martin Blumenstingl (4):
wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and
ac_to_hwq
wifi: rtw88: pci: Change queue datatype to enum rtw_tx_queue_type
wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}
wifi: rtw88: mac: Use existing macros in rtw_pwr_seq_parser()

drivers/net/wireless/realtek/rtw88/mac.c | 4 +-
drivers/net/wireless/realtek/rtw88/pci.c | 50 ++++++------------------
drivers/net/wireless/realtek/rtw88/tx.c | 41 +++++++++++++++++++
drivers/net/wireless/realtek/rtw88/tx.h | 3 ++
4 files changed, 57 insertions(+), 41 deletions(-)

--
2.39.1



2023-02-04 23:30:37

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 1/4] wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq

rtw_hw_queue_mapping() and ac_to_hwq[] hold values of type enum
rtw_tx_queue_type. Change their types to reflect this to make it easier
to understand this part of the code.

While here, also change the array to be static const as it is not
supposed to be modified at runtime.

Signed-off-by: Martin Blumenstingl <[email protected]>
---

Changes from v1 -> v2:
- also make ac_to_hwq static const (instead of just static)
- reword subject and include the "wifi" prefix


drivers/net/wireless/realtek/rtw88/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 0975d27240e4..45ce7e624c03 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -669,7 +669,7 @@ static void rtw_pci_deep_ps(struct rtw_dev *rtwdev, bool enter)
spin_unlock_bh(&rtwpci->irq_lock);
}

-static u8 ac_to_hwq[] = {
+static const enum rtw_tx_queue_type ac_to_hwq[] = {
[IEEE80211_AC_VO] = RTW_TX_QUEUE_VO,
[IEEE80211_AC_VI] = RTW_TX_QUEUE_VI,
[IEEE80211_AC_BE] = RTW_TX_QUEUE_BE,
@@ -678,12 +678,12 @@ static u8 ac_to_hwq[] = {

static_assert(ARRAY_SIZE(ac_to_hwq) == IEEE80211_NUM_ACS);

-static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
+static enum rtw_tx_queue_type rtw_hw_queue_mapping(struct sk_buff *skb)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
__le16 fc = hdr->frame_control;
u8 q_mapping = skb_get_queue_mapping(skb);
- u8 queue;
+ enum rtw_tx_queue_type queue;

if (unlikely(ieee80211_is_beacon(fc)))
queue = RTW_TX_QUEUE_BCN;
--
2.39.1


2023-02-04 23:30:40

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 3/4] wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}

This code is not specific to the PCIe bus type but can be re-used by USB
and SDIO bus types. Move it to tx.{c,h} to avoid code-duplication in the
future. While here, add checking of the ac argument in
rtw_tx_ac_to_hwq() so we're not accessing entries beyond the end of the
array.

Signed-off-by: Martin Blumenstingl <[email protected]>
---

Changes from v1 -> v2:
- add out of bounds check (with a fallback) in rtw_tx_queue_type()
- reword subject and include the "wifi" prefix


drivers/net/wireless/realtek/rtw88/pci.c | 35 ++------------------
drivers/net/wireless/realtek/rtw88/tx.c | 41 ++++++++++++++++++++++++
drivers/net/wireless/realtek/rtw88/tx.h | 3 ++
3 files changed, 46 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 5492107fc85b..b4bd831c9845 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -670,37 +670,6 @@ static void rtw_pci_deep_ps(struct rtw_dev *rtwdev, bool enter)
spin_unlock_bh(&rtwpci->irq_lock);
}

-static const enum rtw_tx_queue_type ac_to_hwq[] = {
- [IEEE80211_AC_VO] = RTW_TX_QUEUE_VO,
- [IEEE80211_AC_VI] = RTW_TX_QUEUE_VI,
- [IEEE80211_AC_BE] = RTW_TX_QUEUE_BE,
- [IEEE80211_AC_BK] = RTW_TX_QUEUE_BK,
-};
-
-static_assert(ARRAY_SIZE(ac_to_hwq) == IEEE80211_NUM_ACS);
-
-static enum rtw_tx_queue_type rtw_hw_queue_mapping(struct sk_buff *skb)
-{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
- __le16 fc = hdr->frame_control;
- u8 q_mapping = skb_get_queue_mapping(skb);
- enum rtw_tx_queue_type queue;
-
- if (unlikely(ieee80211_is_beacon(fc)))
- queue = RTW_TX_QUEUE_BCN;
- else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
- queue = RTW_TX_QUEUE_MGMT;
- else if (is_broadcast_ether_addr(hdr->addr1) ||
- is_multicast_ether_addr(hdr->addr1))
- queue = RTW_TX_QUEUE_HI0;
- else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
- queue = ac_to_hwq[IEEE80211_AC_BE];
- else
- queue = ac_to_hwq[q_mapping];
-
- return queue;
-}
-
static void rtw_pci_release_rsvd_page(struct rtw_pci *rtwpci,
struct rtw_pci_tx_ring *ring)
{
@@ -798,7 +767,7 @@ static void rtw_pci_flush_queues(struct rtw_dev *rtwdev, u32 queues, bool drop)
} else {
for (i = 0; i < rtwdev->hw->queues; i++)
if (queues & BIT(i))
- pci_queues |= BIT(ac_to_hwq[i]);
+ pci_queues |= BIT(rtw_tx_ac_to_hwq(i));
}

__rtw_pci_flush_queues(rtwdev, pci_queues, drop);
@@ -952,7 +921,7 @@ static int rtw_pci_tx_write(struct rtw_dev *rtwdev,
struct rtw_tx_pkt_info *pkt_info,
struct sk_buff *skb)
{
- enum rtw_tx_queue_type queue = rtw_hw_queue_mapping(skb);
+ enum rtw_tx_queue_type queue = rtw_tx_queue_mapping(skb);
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct rtw_pci_tx_ring *ring;
int ret;
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index ab39245e9c2f..bb5c7492c98b 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -682,3 +682,44 @@ void rtw_txq_cleanup(struct rtw_dev *rtwdev, struct ieee80211_txq *txq)
list_del_init(&rtwtxq->list);
spin_unlock_bh(&rtwdev->txq_lock);
}
+
+static const enum rtw_tx_queue_type ac_to_hwq[] = {
+ [IEEE80211_AC_VO] = RTW_TX_QUEUE_VO,
+ [IEEE80211_AC_VI] = RTW_TX_QUEUE_VI,
+ [IEEE80211_AC_BE] = RTW_TX_QUEUE_BE,
+ [IEEE80211_AC_BK] = RTW_TX_QUEUE_BK,
+};
+
+static_assert(ARRAY_SIZE(ac_to_hwq) == IEEE80211_NUM_ACS);
+
+enum rtw_tx_queue_type rtw_tx_ac_to_hwq(enum ieee80211_ac_numbers ac)
+{
+ if (WARN_ON(unlikely(ac >= IEEE80211_NUM_ACS)))
+ return RTW_TX_QUEUE_BE;
+
+ return ac_to_hwq[ac];
+}
+EXPORT_SYMBOL(rtw_tx_ac_to_hwq);
+
+enum rtw_tx_queue_type rtw_tx_queue_mapping(struct sk_buff *skb)
+{
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ __le16 fc = hdr->frame_control;
+ u8 q_mapping = skb_get_queue_mapping(skb);
+ enum rtw_tx_queue_type queue;
+
+ if (unlikely(ieee80211_is_beacon(fc)))
+ queue = RTW_TX_QUEUE_BCN;
+ else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
+ queue = RTW_TX_QUEUE_MGMT;
+ else if (is_broadcast_ether_addr(hdr->addr1) ||
+ is_multicast_ether_addr(hdr->addr1))
+ queue = RTW_TX_QUEUE_HI0;
+ else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
+ queue = ac_to_hwq[IEEE80211_AC_BE];
+ else
+ queue = ac_to_hwq[q_mapping];
+
+ return queue;
+}
+EXPORT_SYMBOL(rtw_tx_queue_mapping);
diff --git a/drivers/net/wireless/realtek/rtw88/tx.h b/drivers/net/wireless/realtek/rtw88/tx.h
index a2f3ac326041..197d5868c8ad 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.h
+++ b/drivers/net/wireless/realtek/rtw88/tx.h
@@ -131,6 +131,9 @@ rtw_tx_write_data_h2c_get(struct rtw_dev *rtwdev,
struct rtw_tx_pkt_info *pkt_info,
u8 *buf, u32 size);

+enum rtw_tx_queue_type rtw_tx_ac_to_hwq(enum ieee80211_ac_numbers ac);
+enum rtw_tx_queue_type rtw_tx_queue_mapping(struct sk_buff *skb);
+
static inline
void fill_txdesc_checksum_common(u8 *txdesc, size_t words)
{
--
2.39.1


2023-02-04 23:30:43

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 2/4] wifi: rtw88: pci: Change queue datatype to enum rtw_tx_queue_type

This makes it easier to understand which values are allowed for the
"queue" variable when using the enum instead of an u8.

Acked-by: Ping-Ke Shih <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---

Changes from v1 -> v2:
- reword subject and include the "wifi" prefix
- add Ping-Ke's Acked-by


drivers/net/wireless/realtek/rtw88/pci.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 45ce7e624c03..5492107fc85b 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -30,7 +30,8 @@ static u32 rtw_pci_tx_queue_idx_addr[] = {
[RTW_TX_QUEUE_H2C] = RTK_PCI_TXBD_IDX_H2CQ,
};

-static u8 rtw_pci_get_tx_qsel(struct sk_buff *skb, u8 queue)
+static u8 rtw_pci_get_tx_qsel(struct sk_buff *skb,
+ enum rtw_tx_queue_type queue)
{
switch (queue) {
case RTW_TX_QUEUE_BCN:
@@ -542,7 +543,7 @@ static int rtw_pci_setup(struct rtw_dev *rtwdev)
static void rtw_pci_dma_release(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci)
{
struct rtw_pci_tx_ring *tx_ring;
- u8 queue;
+ enum rtw_tx_queue_type queue;

rtw_pci_reset_trx_ring(rtwdev);
for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
@@ -608,8 +609,8 @@ static void rtw_pci_deep_ps_enter(struct rtw_dev *rtwdev)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct rtw_pci_tx_ring *tx_ring;
+ enum rtw_tx_queue_type queue;
bool tx_empty = true;
- u8 queue;

if (rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_TX_WAKE))
goto enter_deep_ps;
@@ -803,7 +804,8 @@ static void rtw_pci_flush_queues(struct rtw_dev *rtwdev, u32 queues, bool drop)
__rtw_pci_flush_queues(rtwdev, pci_queues, drop);
}

-static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev, u8 queue)
+static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev,
+ enum rtw_tx_queue_type queue)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct rtw_pci_tx_ring *ring;
@@ -822,7 +824,7 @@ static void rtw_pci_tx_kick_off_queue(struct rtw_dev *rtwdev, u8 queue)
static void rtw_pci_tx_kick_off(struct rtw_dev *rtwdev)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
- u8 queue;
+ enum rtw_tx_queue_type queue;

for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++)
if (test_and_clear_bit(queue, rtwpci->tx_queued))
@@ -831,7 +833,8 @@ static void rtw_pci_tx_kick_off(struct rtw_dev *rtwdev)

static int rtw_pci_tx_write_data(struct rtw_dev *rtwdev,
struct rtw_tx_pkt_info *pkt_info,
- struct sk_buff *skb, u8 queue)
+ struct sk_buff *skb,
+ enum rtw_tx_queue_type queue)
{
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
const struct rtw_chip_info *chip = rtwdev->chip;
@@ -949,9 +952,9 @@ static int rtw_pci_tx_write(struct rtw_dev *rtwdev,
struct rtw_tx_pkt_info *pkt_info,
struct sk_buff *skb)
{
+ enum rtw_tx_queue_type queue = rtw_hw_queue_mapping(skb);
struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
struct rtw_pci_tx_ring *ring;
- u8 queue = rtw_hw_queue_mapping(skb);
int ret;

ret = rtw_pci_tx_write_data(rtwdev, pkt_info, skb, queue);
--
2.39.1


2023-02-04 23:30:45

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH v2 4/4] wifi: rtw88: mac: Use existing macros in rtw_pwr_seq_parser()

Replace the magic numbers for the intf_mask with their existing
RTW_PWR_INTF_PCI_MSK and RTW_PWR_INTF_USB_MSK macros to make the code
easier to understand.

Acked-by: Ping-Ke Shih <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---

Changes from v1 -> v2:
- reword subject and include the "wifi" prefix
- add Ping-Ke's Acked-by


drivers/net/wireless/realtek/rtw88/mac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 98777f294945..4e5c194aac29 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -217,10 +217,10 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,
cut_mask = cut_version_to_mask(cut);
switch (rtw_hci_type(rtwdev)) {
case RTW_HCI_TYPE_PCIE:
- intf_mask = BIT(2);
+ intf_mask = RTW_PWR_INTF_PCI_MSK;
break;
case RTW_HCI_TYPE_USB:
- intf_mask = BIT(1);
+ intf_mask = RTW_PWR_INTF_USB_MSK;
break;
default:
return -EINVAL;
--
2.39.1


2023-02-05 13:37:35

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq

On Sun, Feb 05, 2023 at 12:29:58AM +0100, Martin Blumenstingl wrote:
> rtw_hw_queue_mapping() and ac_to_hwq[] hold values of type enum
> rtw_tx_queue_type. Change their types to reflect this to make it easier
> to understand this part of the code.
>
> While here, also change the array to be static const as it is not
> supposed to be modified at runtime.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2023-02-05 13:38:05

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] wifi: rtw88: pci: Change queue datatype to enum rtw_tx_queue_type

On Sun, Feb 05, 2023 at 12:29:59AM +0100, Martin Blumenstingl wrote:
> This makes it easier to understand which values are allowed for the
> "queue" variable when using the enum instead of an u8.
>
> Acked-by: Ping-Ke Shih <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2023-02-05 13:39:37

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}

On Sun, Feb 05, 2023 at 12:30:00AM +0100, Martin Blumenstingl wrote:
> This code is not specific to the PCIe bus type but can be re-used by USB
> and SDIO bus types. Move it to tx.{c,h} to avoid code-duplication in the
> future. While here, add checking of the ac argument in
> rtw_tx_ac_to_hwq() so we're not accessing entries beyond the end of the
> array.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-02-05 13:40:26

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] wifi: rtw88: mac: Use existing macros in rtw_pwr_seq_parser()

On Sun, Feb 05, 2023 at 12:30:01AM +0100, Martin Blumenstingl wrote:
> Replace the magic numbers for the intf_mask with their existing
> RTW_PWR_INTF_PCI_MSK and RTW_PWR_INTF_USB_MSK macros to make the code
> easier to understand.
>
> Acked-by: Ping-Ke Shih <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Simon Horman <[email protected]>

2023-02-08 00:27:30

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH v2 1/4] wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq



> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Sunday, February 5, 2023 7:30 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Neo
> Jou <[email protected]>; Jernej Skrabec <[email protected]>; Ping-Ke Shih <[email protected]>;
> Martin Blumenstingl <[email protected]>
> Subject: [PATCH v2 1/4] wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq
>
> rtw_hw_queue_mapping() and ac_to_hwq[] hold values of type enum
> rtw_tx_queue_type. Change their types to reflect this to make it easier
> to understand this part of the code.
>
> While here, also change the array to be static const as it is not
> supposed to be modified at runtime.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Ping-Ke Shih <[email protected]>

[...]


2023-02-08 00:29:24

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH v2 3/4] wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}



> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Sunday, February 5, 2023 7:30 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; Neo
> Jou <[email protected]>; Jernej Skrabec <[email protected]>; Ping-Ke Shih <[email protected]>;
> Martin Blumenstingl <[email protected]>
> Subject: [PATCH v2 3/4] wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}
>
> This code is not specific to the PCIe bus type but can be re-used by USB
> and SDIO bus types. Move it to tx.{c,h} to avoid code-duplication in the
> future. While here, add checking of the ac argument in
> rtw_tx_ac_to_hwq() so we're not accessing entries beyond the end of the
> array.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Ping-Ke Shih <[email protected]>

[...]


2023-02-13 15:16:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq

Martin Blumenstingl <[email protected]> wrote:

> rtw_hw_queue_mapping() and ac_to_hwq[] hold values of type enum
> rtw_tx_queue_type. Change their types to reflect this to make it easier
> to understand this part of the code.
>
> While here, also change the array to be static const as it is not
> supposed to be modified at runtime.
>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> Reviewed-by: Simon Horman <[email protected]>
> Reviewed-by: Ping-Ke Shih <[email protected]>

4 patches applied to wireless-next.git, thanks.

6152b649a708 wifi: rtw88: pci: Use enum type for rtw_hw_queue_mapping() and ac_to_hwq
c90897960c19 wifi: rtw88: pci: Change queue datatype to enum rtw_tx_queue_type
7b6e9df91133 wifi: rtw88: Move enum rtw_tx_queue_type mapping code to tx.{c,h}
24d54855ff36 wifi: rtw88: mac: Use existing macros in rtw_pwr_seq_parser()

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches