[PATCH 0/4] rtw88: four small code-cleanups and refactorings

[PATCH 0/4] rtw88: four small code-cleanups and refactorings

[Martin Blumenstingl]

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).

This series is built on top of v3 of my other series called "rtw88:
prepare locking for SDIO support" [0].


[0] https://lore.kernel.org/linux-wireless/20220108005533.947787-1-martin.blumenstingl@googlemail.com/


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

 drivers/net/wireless/realtek/rtw88/mac.c |  4 +-
 drivers/net/wireless/realtek/rtw88/pci.c | 47 ++++++------------------
 drivers/net/wireless/realtek/rtw88/tx.c  | 35 ++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/tx.h  |  3 ++
 4 files changed, 51 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH 1/4] rtw88: pci: Change type of rtw_hw_queue_mapping() and ac_to_hwq to enum

[Martin Blumenstingl]

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.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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 a0991d3f15c0..13f1f50b2867 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 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.34.1

[PATCH 2/4] rtw88: pci: Change queue datatype from u8 to enum rtw_tx_queue_type

[Martin Blumenstingl]

This makes it easier to understand which values are allowed for the
"queue" variable.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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 13f1f50b2867..2da057d18188 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;
@@ -800,7 +801,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;
@@ -819,7 +821,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))
@@ -828,7 +830,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;
 	struct rtw_chip_info *chip = rtwdev->chip;
@@ -946,9 +949,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.34.1

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

[Martin Blumenstingl]

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.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 32 ++--------------------
 drivers/net/wireless/realtek/rtw88/tx.c  | 35 ++++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/tx.h  |  3 ++
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 2da057d18188..945411a5947b 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -670,34 +670,6 @@ static void rtw_pci_deep_ps(struct rtw_dev *rtwdev, bool enter)
 	spin_unlock_bh(&rtwpci->irq_lock);
 }
 
-static 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 (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)
 {
@@ -795,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);
@@ -949,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 efcc1b0371a8..ec6a3683c3f8 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -665,3 +665,38 @@ void rtw_txq_cleanup(struct rtw_dev *rtwdev, struct ieee80211_txq *txq)
 		list_del_init(&rtwtxq->list);
 	spin_unlock_bh(&rtwdev->txq_lock);
 }
+
+static 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)
+{
+	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 (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 56371eff9f7f..940f944fd39c 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.h
+++ b/drivers/net/wireless/realtek/rtw88/tx.h
@@ -119,4 +119,7 @@ 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);
+
 #endif
-- 
2.34.1

[PATCH 4/4] rtw88: mac: Use existing interface mask macros in rtw_pwr_seq_parser()

[Martin Blumenstingl]

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.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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 d1678aed9d9c..aa6b3d2eaa38 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.34.1

Re: [PATCH 1/4] rtw88: pci: Change type of rtw_hw_queue_mapping() and ac_to_hwq to enum

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> 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.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  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 a0991d3f15c0..13f1f50b2867 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 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,

Shouldn't ac_to_hwq be static const?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH 1/4] rtw88: pci: Change type of rtw_hw_queue_mapping() and ac_to_hwq to enum

[Martin Blumenstingl]

Hi Kalle,

On Mon, Jan 17, 2022 at 1:11 PM Kalle Valo <kvalo@kernel.org> wrote:
[...]
> > -static u8 ac_to_hwq[] = {
> > +static 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,
>
> Shouldn't ac_to_hwq be static const?
Good point, thanks for this suggestion!
I will include that in v2 of this series (which I will send in a few
days so others can share their thoughts about these patches as well).


Best regards,
Martin

RE: [PATCH 2/4] rtw88: pci: Change queue datatype from u8 to enum rtw_tx_queue_type

[Pkshih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: linux-wireless@vger.kernel.org
> Cc: tony0620emma@gmail.com; kvalo@codeaurora.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Neo Jou <neojou@gmail.com>; Jernej Skrabec <jernej.skrabec@gmail.com>; Pkshih <pkshih@realtek.com>; Martin
> Blumenstingl <martin.blumenstingl@googlemail.com>
> Subject: [PATCH 2/4] rtw88: pci: Change queue datatype from u8 to enum rtw_tx_queue_type
> 
> This makes it easier to understand which values are allowed for the
> "queue" variable.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

[...]

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

[Pkshih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: linux-wireless@vger.kernel.org
> Cc: tony0620emma@gmail.com; kvalo@codeaurora.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Neo Jou <neojou@gmail.com>; Jernej Skrabec <jernej.skrabec@gmail.com>; Pkshih <pkshih@realtek.com>; Martin
> Blumenstingl <martin.blumenstingl@googlemail.com>
> Subject: [PATCH 3/4] 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.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[...]

> diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
> index efcc1b0371a8..ec6a3683c3f8 100644
> --- a/drivers/net/wireless/realtek/rtw88/tx.c
> +++ b/drivers/net/wireless/realtek/rtw88/tx.c
> @@ -665,3 +665,38 @@ void rtw_txq_cleanup(struct rtw_dev *rtwdev, struct ieee80211_txq *txq)
>  		list_del_init(&rtwtxq->list);
>  	spin_unlock_bh(&rtwdev->txq_lock);
>  }
> +
> +static 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)
> +{
> +	return ac_to_hwq[ac];
> +}
> +EXPORT_SYMBOL(rtw_tx_ac_to_hwq);
> +

Could I know why we can't just export the array ac_to_hwq[]?
Is there a strict rule?

--
Ping-Ke

RE: [PATCH 4/4] rtw88: mac: Use existing interface mask macros in rtw_pwr_seq_parser()

[Pkshih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: linux-wireless@vger.kernel.org
> Cc: tony0620emma@gmail.com; kvalo@codeaurora.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> Neo Jou <neojou@gmail.com>; Jernej Skrabec <jernej.skrabec@gmail.com>; Pkshih <pkshih@realtek.com>; Martin
> Blumenstingl <martin.blumenstingl@googlemail.com>
> Subject: [PATCH 4/4] rtw88: mac: Use existing interface mask 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.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

[...]

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

[Kalle Valo]

Pkshih <pkshih@realtek.com> writes:

>> -----Original Message-----
>> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Sent: Saturday, January 15, 2022 7:48 AM
>> To: linux-wireless@vger.kernel.org
>> Cc: tony0620emma@gmail.com; kvalo@codeaurora.org;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>> Neo Jou <neojou@gmail.com>; Jernej Skrabec
>> <jernej.skrabec@gmail.com>; Pkshih <pkshih@realtek.com>; Martin
>> Blumenstingl <martin.blumenstingl@googlemail.com>
>> Subject: [PATCH 3/4] 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.
>> 
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>
> [...]
>
>> diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
>> index efcc1b0371a8..ec6a3683c3f8 100644
>> --- a/drivers/net/wireless/realtek/rtw88/tx.c
>> +++ b/drivers/net/wireless/realtek/rtw88/tx.c
>> @@ -665,3 +665,38 @@ void rtw_txq_cleanup(struct rtw_dev *rtwdev, struct ieee80211_txq *txq)
>>  		list_del_init(&rtwtxq->list);
>>  	spin_unlock_bh(&rtwdev->txq_lock);
>>  }
>> +
>> +static 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)
>> +{
>> +	return ac_to_hwq[ac];
>> +}
>> +EXPORT_SYMBOL(rtw_tx_ac_to_hwq);
>> +
>
> Could I know why we can't just export the array ac_to_hwq[]?
> Is there a strict rule?

I was about to answer that with a helper function it's easier to catch
out of bands access, but then noticed the helper doesn't have a check
for that. Should it have one?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

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

[Martin Blumenstingl]

Hi Kalle,

On Wed, Jan 19, 2022 at 7:20 AM Kalle Valo <kvalo@kernel.org> wrote:
[...]
> I was about to answer that with a helper function it's easier to catch
> out of bands access, but then noticed the helper doesn't have a check
> for that. Should it have one?
you mean something like:
    if (ac >= IEEE80211_NUM_ACS)
        return RTW_TX_QUEUE_BE;

Possibly also with a WARN_ON/WARN_ON_ONCE


Best regards,
Martin

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

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Kalle,
>
> On Wed, Jan 19, 2022 at 7:20 AM Kalle Valo <kvalo@kernel.org> wrote:
> [...]
>> I was about to answer that with a helper function it's easier to catch
>> out of bands access, but then noticed the helper doesn't have a check
>> for that. Should it have one?
>
> you mean something like:
>     if (ac >= IEEE80211_NUM_ACS)
>         return RTW_TX_QUEUE_BE;
>
> Possibly also with a WARN_ON/WARN_ON_ONCE

Yeah, something like that would be good.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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