This struct is used to access a sequence of bytes received from the
wifi chip. It must not have any padding bytes between the members.
This doesn't change anything on my system, possibly because currently
none of the members need more than byte alignment.
Fixes: b2b43b7837ba ("rtl8xxxu: Initial functionality to handle C2H events for 8723bu")
Signed-off-by: Bitterblue Smith <[email protected]>
---
v2:
- Patch is new in v2.
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 136992f0200c..d26df4095da0 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1240,7 +1240,7 @@ struct rtl8723bu_c2h {
u8 bw;
} __packed ra_report;
};
-};
+} __packed;
struct rtl8xxxu_fileops;
--
2.38.0
The gen 2 chips RTL8192EU and RTL8188FU periodically send the driver
reports about the TX rate, and the driver passes these reports to
sta_statistics. The reports from RTL8192EU may or may not include the
channel width. The reports from RTL8188FU do not include it.
Only access the c2h->ra_report.bw field if the report (skb) is big
enough.
The other problem fixed here is that the code was actually never
changing the channel width initially reported by
rtl8xxxu_bss_info_changed because the value of RATE_INFO_BW_20 is 0.
Fixes: 0985d3a410ac ("rtl8xxxu: Feed current txrate information for mac80211")
Signed-off-by: Bitterblue Smith <[email protected]>
---
v2:
- Eliminate the magic numbers.
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 ++
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index d26df4095da0..9be53e0f25cc 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1187,6 +1187,8 @@ enum rtl8xxxu_bw_mode {
RTL8XXXU_CHANNEL_WIDTH_MAX = 5,
};
+#define RTL8XXXU_C2H_HDR_LEN 2 /* id and seq */
+
struct rtl8723bu_c2h {
u8 id;
u8 seq;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 28f136064297..352779fe4b78 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5569,7 +5569,6 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
rarpt->txrate.flags = 0;
rate = c2h->ra_report.rate;
sgi = c2h->ra_report.sgi;
- bw = c2h->ra_report.bw;
if (rate < DESC_RATE_MCS0) {
rarpt->txrate.legacy =
@@ -5586,8 +5585,14 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
RATE_INFO_FLAGS_SHORT_GI;
}
- if (bw == RATE_INFO_BW_20)
- rarpt->txrate.bw |= RATE_INFO_BW_20;
+ if (skb->len > RTL8XXXU_C2H_HDR_LEN +
+ offsetof(typeof(c2h->ra_report), bw)) {
+ if (c2h->ra_report.bw == RTL8XXXU_CHANNEL_WIDTH_40)
+ bw = RATE_INFO_BW_40;
+ else
+ bw = RATE_INFO_BW_20;
+ rarpt->txrate.bw = bw;
+ }
}
bit_rate = cfg80211_calculate_bitrate(&rarpt->txrate);
rarpt->bit_rate = bit_rate;
--
2.38.0
The ra_report struct is used for reporting the TX rate via
sta_statistics. The code which fills it out is duplicated in two
places, and the RTL8188EU will need it in a third place. Move this
code into a new function rtl8xxxu_update_ra_report.
Signed-off-by: Bitterblue Smith <[email protected]>
---
v2:
- Compare against DESC_RATE_54M instead of DESC_RATE_MCS0.
---
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 107 ++++++++----------
1 file changed, 46 insertions(+), 61 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 352779fe4b78..4d067e9b9c8a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -4598,6 +4598,32 @@ static void rtl8xxxu_set_aifs(struct rtl8xxxu_priv *priv, u8 slot_time)
}
}
+static void rtl8xxxu_update_ra_report(struct rtl8xxxu_ra_report *rarpt,
+ u8 rate, u8 sgi, u8 bw)
+{
+ u8 mcs, nss;
+
+ rarpt->txrate.flags = 0;
+
+ if (rate <= DESC_RATE_54M) {
+ rarpt->txrate.legacy = rtl8xxxu_legacy_ratetable[rate].bitrate;
+ } else {
+ rtl8xxxu_desc_to_mcsrate(rate, &mcs, &nss);
+ rarpt->txrate.flags |= RATE_INFO_FLAGS_MCS;
+
+ rarpt->txrate.mcs = mcs;
+ rarpt->txrate.nss = nss;
+
+ if (sgi)
+ rarpt->txrate.flags |= RATE_INFO_FLAGS_SHORT_GI;
+
+ rarpt->txrate.bw = bw;
+ }
+
+ rarpt->bit_rate = cfg80211_calculate_bitrate(&rarpt->txrate);
+ rarpt->desc_rate = rate;
+}
+
static void
rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct ieee80211_bss_conf *bss_conf, u64 changed)
@@ -4620,9 +4646,7 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
u32 ramask;
int sgi = 0;
u8 highest_rate;
- u8 mcs = 0, nss = 0;
- u32 bit_rate;
-
+ u8 bw;
rcu_read_lock();
sta = ieee80211_find_sta(vif, bss_conf->bssid);
@@ -4647,37 +4671,19 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
sgi = 1;
highest_rate = fls(ramask) - 1;
- if (highest_rate < DESC_RATE_MCS0) {
- rarpt->txrate.legacy =
- rtl8xxxu_legacy_ratetable[highest_rate].bitrate;
- } else {
- rtl8xxxu_desc_to_mcsrate(highest_rate,
- &mcs, &nss);
- rarpt->txrate.flags |= RATE_INFO_FLAGS_MCS;
-
- rarpt->txrate.mcs = mcs;
- rarpt->txrate.nss = nss;
-
- if (sgi) {
- rarpt->txrate.flags |=
- RATE_INFO_FLAGS_SHORT_GI;
- }
-
- if (rtl8xxxu_ht40_2g &&
- (sta->deflink.ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40))
- rarpt->txrate.bw = RATE_INFO_BW_40;
- else
- rarpt->txrate.bw = RATE_INFO_BW_20;
- }
+ if (rtl8xxxu_ht40_2g &&
+ (sta->deflink.ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40))
+ bw = RATE_INFO_BW_40;
+ else
+ bw = RATE_INFO_BW_20;
rcu_read_unlock();
- bit_rate = cfg80211_calculate_bitrate(&rarpt->txrate);
- rarpt->bit_rate = bit_rate;
- rarpt->desc_rate = highest_rate;
+
+ rtl8xxxu_update_ra_report(rarpt, highest_rate, sgi, bw);
priv->vif = vif;
priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
- priv->fops->update_rate_mask(priv, ramask, 0, sgi, rarpt->txrate.bw == RATE_INFO_BW_40);
+ priv->fops->update_rate_mask(priv, ramask, 0, sgi, bw == RATE_INFO_BW_40);
rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
@@ -5538,9 +5544,7 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
u8 bt_info = 0;
struct rtl8xxxu_btcoex *btcoex;
struct rtl8xxxu_ra_report *rarpt;
- u8 rate, sgi, bw;
- u32 bit_rate;
- u8 mcs = 0, nss = 0;
+ u8 bw;
priv = container_of(work, struct rtl8xxxu_priv, c2hcmd_work);
btcoex = &priv->bt_coex;
@@ -5566,37 +5570,18 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
rtl8723bu_handle_bt_info(priv);
break;
case C2H_8723B_RA_REPORT:
- rarpt->txrate.flags = 0;
- rate = c2h->ra_report.rate;
- sgi = c2h->ra_report.sgi;
-
- if (rate < DESC_RATE_MCS0) {
- rarpt->txrate.legacy =
- rtl8xxxu_legacy_ratetable[rate].bitrate;
- } else {
- rtl8xxxu_desc_to_mcsrate(rate, &mcs, &nss);
- rarpt->txrate.flags |= RATE_INFO_FLAGS_MCS;
+ bw = rarpt->txrate.bw;
- rarpt->txrate.mcs = mcs;
- rarpt->txrate.nss = nss;
-
- if (sgi) {
- rarpt->txrate.flags |=
- RATE_INFO_FLAGS_SHORT_GI;
- }
-
- if (skb->len > RTL8XXXU_C2H_HDR_LEN +
- offsetof(typeof(c2h->ra_report), bw)) {
- if (c2h->ra_report.bw == RTL8XXXU_CHANNEL_WIDTH_40)
- bw = RATE_INFO_BW_40;
- else
- bw = RATE_INFO_BW_20;
- rarpt->txrate.bw = bw;
- }
+ if (skb->len > RTL8XXXU_C2H_HDR_LEN +
+ offsetof(typeof(c2h->ra_report), bw)) {
+ if (c2h->ra_report.bw == RTL8XXXU_CHANNEL_WIDTH_40)
+ bw = RATE_INFO_BW_40;
+ else
+ bw = RATE_INFO_BW_20;
}
- bit_rate = cfg80211_calculate_bitrate(&rarpt->txrate);
- rarpt->bit_rate = bit_rate;
- rarpt->desc_rate = rate;
+
+ rtl8xxxu_update_ra_report(rarpt, c2h->ra_report.rate,
+ c2h->ra_report.sgi, bw);
break;
default:
break;
--
2.38.0
> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Tuesday, November 29, 2022 3:30 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
> Subject: [PATCH v2 2/3] wifi: rtl8xxxu: Fix the channel width reporting
>
> The gen 2 chips RTL8192EU and RTL8188FU periodically send the driver
> reports about the TX rate, and the driver passes these reports to
> sta_statistics. The reports from RTL8192EU may or may not include the
> channel width. The reports from RTL8188FU do not include it.
>
> Only access the c2h->ra_report.bw field if the report (skb) is big
> enough.
>
> The other problem fixed here is that the code was actually never
> changing the channel width initially reported by
> rtl8xxxu_bss_info_changed because the value of RATE_INFO_BW_20 is 0.
>
> Fixes: 0985d3a410ac ("rtl8xxxu: Feed current txrate information for mac80211")
> Signed-off-by: Bitterblue Smith <[email protected]>
> ---
> v2:
> - Eliminate the magic numbers.
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 ++
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++++---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index d26df4095da0..9be53e0f25cc 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1187,6 +1187,8 @@ enum rtl8xxxu_bw_mode {
> RTL8XXXU_CHANNEL_WIDTH_MAX = 5,
> };
>
> +#define RTL8XXXU_C2H_HDR_LEN 2 /* id and seq */
> +
> struct rtl8723bu_c2h {
> u8 id;
> u8 seq;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 28f136064297..352779fe4b78 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -5569,7 +5569,6 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
> rarpt->txrate.flags = 0;
> rate = c2h->ra_report.rate;
> sgi = c2h->ra_report.sgi;
> - bw = c2h->ra_report.bw;
>
> if (rate < DESC_RATE_MCS0) {
> rarpt->txrate.legacy =
> @@ -5586,8 +5585,14 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
> RATE_INFO_FLAGS_SHORT_GI;
> }
>
> - if (bw == RATE_INFO_BW_20)
> - rarpt->txrate.bw |= RATE_INFO_BW_20;
> + if (skb->len > RTL8XXXU_C2H_HDR_LEN +
> + offsetof(typeof(c2h->ra_report), bw)) {
I think this is not very intuitive, because this logic is correct only if 'bw' is u8.
Then, this option may be not bad:
if (skb->len >= RTL8XXXU_C2H_HDR_LEN + offsetof(typeof(c2h->ra_report), bw) + sizeof(c2h->ra_report.bw))
or
Not need RTL8XXXU_C2H_HDR_LEN. Just use offsetof() to calculate:
if (skb->len >= offsetof(typeof(*c2h), ra_report.bw) + sizeof(c2h->ra_report.bw))
or
If you still prefer 'skb->len >', this may be more concise than original one.
if (skb->len > offsetof(typeof(*c2h), ra_report.bw))
> + if (c2h->ra_report.bw == RTL8XXXU_CHANNEL_WIDTH_40)
> + bw = RATE_INFO_BW_40;
> + else
> + bw = RATE_INFO_BW_20;
> + rarpt->txrate.bw = bw;
> + }
> }
> bit_rate = cfg80211_calculate_bitrate(&rarpt->txrate);
> rarpt->bit_rate = bit_rate;
> --
> 2.38.0
>
> ------Please consider the environment before printing this e-mail.
On 29/11/2022 03:22, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <[email protected]>
>> Sent: Tuesday, November 29, 2022 3:30 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
>> Subject: [PATCH v2 2/3] wifi: rtl8xxxu: Fix the channel width reporting
>>
>> The gen 2 chips RTL8192EU and RTL8188FU periodically send the driver
>> reports about the TX rate, and the driver passes these reports to
>> sta_statistics. The reports from RTL8192EU may or may not include the
>> channel width. The reports from RTL8188FU do not include it.
>>
>> Only access the c2h->ra_report.bw field if the report (skb) is big
>> enough.
>>
>> The other problem fixed here is that the code was actually never
>> changing the channel width initially reported by
>> rtl8xxxu_bss_info_changed because the value of RATE_INFO_BW_20 is 0.
>>
>> Fixes: 0985d3a410ac ("rtl8xxxu: Feed current txrate information for mac80211")
>> Signed-off-by: Bitterblue Smith <[email protected]>
>> ---
>> v2:
>> - Eliminate the magic numbers.
>> ---
>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 ++
>> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 11 ++++++++---
>> 2 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> index d26df4095da0..9be53e0f25cc 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
>> @@ -1187,6 +1187,8 @@ enum rtl8xxxu_bw_mode {
>> RTL8XXXU_CHANNEL_WIDTH_MAX = 5,
>> };
>>
>> +#define RTL8XXXU_C2H_HDR_LEN 2 /* id and seq */
>> +
>> struct rtl8723bu_c2h {
>> u8 id;
>> u8 seq;
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index 28f136064297..352779fe4b78 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -5569,7 +5569,6 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
>> rarpt->txrate.flags = 0;
>> rate = c2h->ra_report.rate;
>> sgi = c2h->ra_report.sgi;
>> - bw = c2h->ra_report.bw;
>>
>> if (rate < DESC_RATE_MCS0) {
>> rarpt->txrate.legacy =
>> @@ -5586,8 +5585,14 @@ static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
>> RATE_INFO_FLAGS_SHORT_GI;
>> }
>>
>> - if (bw == RATE_INFO_BW_20)
>> - rarpt->txrate.bw |= RATE_INFO_BW_20;
>> + if (skb->len > RTL8XXXU_C2H_HDR_LEN +
>> + offsetof(typeof(c2h->ra_report), bw)) {
>
> I think this is not very intuitive, because this logic is correct only if 'bw' is u8.
>
> Then, this option may be not bad:
> if (skb->len >= RTL8XXXU_C2H_HDR_LEN + offsetof(typeof(c2h->ra_report), bw) + sizeof(c2h->ra_report.bw))
>
> or
> Not need RTL8XXXU_C2H_HDR_LEN. Just use offsetof() to calculate:
> if (skb->len >= offsetof(typeof(*c2h), ra_report.bw) + sizeof(c2h->ra_report.bw))
>
Nice! I didn't know offsetof can do that.