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/[email protected]/
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
This makes it easier to understand which values are allowed for the
"queue" variable.
Signed-off-by: Martin Blumenstingl <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Martin Blumenstingl <[email protected]> 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 <[email protected]>
> ---
> 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
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> Neo Jou <[email protected]>; Jernej Skrabec <[email protected]>; Pkshih <[email protected]>; Martin
> Blumenstingl <[email protected]>
> 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 <[email protected]>
Acked-by: Ping-Ke Shih <[email protected]>
[...]
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> Neo Jou <[email protected]>; Jernej Skrabec <[email protected]>; Pkshih <[email protected]>; Martin
> Blumenstingl <[email protected]>
> 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 <[email protected]>
[...]
> 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
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Saturday, January 15, 2022 7:48 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> Neo Jou <[email protected]>; Jernej Skrabec <[email protected]>; Pkshih <[email protected]>; Martin
> Blumenstingl <[email protected]>
> 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 <[email protected]>
Acked-by: Ping-Ke Shih <[email protected]>
[...]
Hi Kalle,
On Wed, Jan 19, 2022 at 7:20 AM Kalle Valo <[email protected]> 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
Martin Blumenstingl <[email protected]> writes:
> Hi Kalle,
>
> On Wed, Jan 19, 2022 at 7:20 AM Kalle Valo <[email protected]> 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