Add some new members to rtl8xxxu_fileops and use them instead of
checking priv->rtl_chip.
Signed-off-by: Bitterblue Smith <[email protected]>
---
.../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++++
.../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 1 +
.../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 5 ++++
.../realtek/rtl8xxxu/rtl8xxxu_8192e.c | 1 +
.../realtek/rtl8xxxu/rtl8xxxu_8710b.c | 9 +++++++
.../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 3 +++
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++--------------
7 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 9d48c69ffece..39fee07917e7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -1923,6 +1923,11 @@ struct rtl8xxxu_fileops {
u8 has_tx_report:1;
u8 gen2_thermal_meter:1;
u8 needs_full_init:1;
+ u8 init_reg_rxfltmap:1;
+ u8 init_reg_pkt_life_time:1;
+ u8 init_reg_hmtfr:1;
+ u8 ampdu_max_time;
+ u8 ustime_tsf_edca;
u32 adda_1t_init;
u32 adda_1t_path_on;
u32 adda_2t_path_on_a;
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
index 6a82ec47568e..af8436070ba7 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
@@ -1883,6 +1883,7 @@ struct rtl8xxxu_fileops rtl8188eu_fops = {
.rx_desc_size = sizeof(struct rtl8xxxu_rxdesc16),
.tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
.has_tx_report = 1,
+ .init_reg_pkt_life_time = 1,
.gen2_thermal_meter = 1,
.adda_1t_init = 0x0b1b25a0,
.adda_1t_path_on = 0x0bdb25a0,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
index 82dee1fed477..dfb250adb168 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
@@ -1746,6 +1746,11 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
.has_tx_report = 1,
.gen2_thermal_meter = 1,
.needs_full_init = 1,
+ .init_reg_rxfltmap = 1,
+ .init_reg_pkt_life_time = 1,
+ .init_reg_hmtfr = 1,
+ .ampdu_max_time = 0x70,
+ .ustime_tsf_edca = 0x28,
.adda_1t_init = 0x03c00014,
.adda_1t_path_on = 0x03c00014,
.trxff_boundary = 0x3f7f,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 4498748164af..9581e858a9c5 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -1821,6 +1821,7 @@ struct rtl8xxxu_fileops rtl8192eu_fops = {
.has_s0s1 = 0,
.gen2_thermal_meter = 1,
.needs_full_init = 1,
+ .init_reg_pkt_life_time = 1,
.adda_1t_init = 0x0fc01616,
.adda_1t_path_on = 0x0fc01616,
.adda_2t_path_on_a = 0x0fc01616,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c
index 920466e39604..22d4704dd31e 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8710b.c
@@ -1865,6 +1865,15 @@ struct rtl8xxxu_fileops rtl8710bu_fops = {
.has_tx_report = 1,
.gen2_thermal_meter = 1,
.needs_full_init = 1,
+ .init_reg_rxfltmap = 1,
+ .init_reg_pkt_life_time = 1,
+ .init_reg_hmtfr = 1,
+ .ampdu_max_time = 0x5e,
+ /*
+ * The RTL8710BU vendor driver uses 0x50 here and it works fine,
+ * but in rtl8xxxu 0x50 causes slow upload and random packet loss. Why?
+ */
+ .ustime_tsf_edca = 0x28,
.adda_1t_init = 0x03c00016,
.adda_1t_path_on = 0x03c00016,
.trxff_boundary = 0x3f7f,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index d99538eb8398..c31c2b52ac77 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1741,6 +1741,9 @@ struct rtl8xxxu_fileops rtl8723bu_fops = {
.has_tx_report = 1,
.gen2_thermal_meter = 1,
.needs_full_init = 1,
+ .init_reg_hmtfr = 1,
+ .ampdu_max_time = 0x5e,
+ .ustime_tsf_edca = 0x50,
.adda_1t_init = 0x01c00014,
.adda_1t_path_on = 0x01c00014,
.adda_2t_path_on_a = 0x01c00014,
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index c152b228606f..62dd53a57659 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1916,7 +1916,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
/*
* Init H2C command
*/
- if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
+ if (priv->fops->init_reg_hmtfr)
rtl8xxxu_write8(priv, REG_HMTFR, 0x0f);
exit:
return ret;
@@ -3864,11 +3864,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
- if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8710B)
- val8 = 0x5e;
- else if (priv->rtl_chip == RTL8188F)
- val8 = 0x70; /* 0x5e would make it very slow */
- rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8);
+ rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B,
+ priv->fops->ampdu_max_time);
rtl8xxxu_write32(priv, REG_AGGLEN_LMT, 0xffffffff);
rtl8xxxu_write8(priv, REG_RX_PKT_LIMIT, 0x18);
rtl8xxxu_write8(priv, REG_PIFS, 0x00);
@@ -3876,16 +3873,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL, FWHW_TXQ_CTRL_AMPDU_RETRY);
rtl8xxxu_write32(priv, REG_FAST_EDCA_CTRL, 0x03086666);
}
- /*
- * The RTL8710BU vendor driver uses 0x50 here and it works fine,
- * but in rtl8xxxu 0x50 causes slow upload and random packet loss. Why?
- */
- if (priv->rtl_chip == RTL8723B)
- val8 = 0x50;
- else if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
- val8 = 0x28; /* 0x50 would make the upload slow */
- rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, val8);
- rtl8xxxu_write8(priv, REG_USTIME_EDCA, val8);
+ rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, priv->fops->ustime_tsf_edca);
+ rtl8xxxu_write8(priv, REG_USTIME_EDCA, priv->fops->ustime_tsf_edca);
/* to prevent mac is reseted by bus. */
val8 = rtl8xxxu_read8(priv, REG_RSV_CTRL);
@@ -4102,7 +4091,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
RCR_APPEND_PHYSTAT | RCR_APPEND_ICV | RCR_APPEND_MIC;
rtl8xxxu_write32(priv, REG_RCR, val32);
- if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B) {
+ if (fops->init_reg_rxfltmap) {
/* Accept all data frames */
rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
@@ -4187,8 +4176,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
if (fops->init_aggregation)
fops->init_aggregation(priv);
- if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8188E ||
- priv->rtl_chip == RTL8710B) {
+ if (fops->init_reg_pkt_life_time) {
rtl8xxxu_write16(priv, REG_PKT_VO_VI_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
rtl8xxxu_write16(priv, REG_PKT_BE_BK_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
}
--
2.39.2
Handle them the same way as the devices with 3-4 USB out endpoints.
This is needed for the RTL8192FU.
Signed-off-by: Bitterblue Smith <[email protected]>
---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 39fee07917e7..82a0290ccb29 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -27,7 +27,7 @@
#define RTL8XXXU_MAX_REG_POLL 500
#define USB_INTR_CONTENT_LENGTH 56
-#define RTL8XXXU_OUT_ENDPOINTS 4
+#define RTL8XXXU_OUT_ENDPOINTS 6
#define REALTEK_USB_READ 0xc0
#define REALTEK_USB_WRITE 0x40
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 62dd53a57659..6106b47d0c37 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1663,6 +1663,8 @@ int rtl8xxxu_config_endpoints_no_sie(struct rtl8xxxu_priv *priv)
struct device *dev = &priv->udev->dev;
switch (priv->nr_out_eps) {
+ case 6:
+ case 5:
case 4:
case 3:
priv->ep_tx_low_queue = 1;
--
2.39.2
Hi Bitterblue,
On Sat, Apr 1, 2023 at 7:18 AM Bitterblue Smith <[email protected]> wrote:
>
> Add some new members to rtl8xxxu_fileops and use them instead of
> checking priv->rtl_chip.
>
> Signed-off-by: Bitterblue Smith <[email protected]>
> ---
> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++++
> .../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 1 +
> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 5 ++++
> .../realtek/rtl8xxxu/rtl8xxxu_8192e.c | 1 +
> .../realtek/rtl8xxxu/rtl8xxxu_8710b.c | 9 +++++++
> .../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 3 +++
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++--------------
> 7 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> index 6a82ec47568e..af8436070ba7 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
> @@ -1883,6 +1883,7 @@ struct rtl8xxxu_fileops rtl8188eu_fops = {
> .rx_desc_size = sizeof(struct rtl8xxxu_rxdesc16),
> .tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
> .has_tx_report = 1,
> + .init_reg_pkt_life_time = 1,
I'm sure it's safe, but the fops structs that don't set the
ampdu_max_time and ustime_tsf_edca values feel odd.
> .gen2_thermal_meter = 1,
> .adda_1t_init = 0x0b1b25a0,
> .adda_1t_path_on = 0x0bdb25a0,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> index 82dee1fed477..dfb250adb168 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
> @@ -1746,6 +1746,11 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
> .has_tx_report = 1,
> .gen2_thermal_meter = 1,
> .needs_full_init = 1,
> + .init_reg_rxfltmap = 1,
> + .init_reg_pkt_life_time = 1,
> + .init_reg_hmtfr = 1,
> + .ampdu_max_time = 0x70,
> + .ustime_tsf_edca = 0x28,
The original code had comments for why the 8188fu had different values
for ampdu_max_time and ustime_tsf_edca. Should they be copied here?
> .adda_1t_init = 0x03c00014,
> .adda_1t_path_on = 0x03c00014,
> .trxff_boundary = 0x3f7f,
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
On 01/04/2023 07:23, Julian Calaby wrote:
> Hi Bitterblue,
>
> On Sat, Apr 1, 2023 at 7:18 AM Bitterblue Smith <[email protected]> wrote:
>>
>> Add some new members to rtl8xxxu_fileops and use them instead of
>> checking priv->rtl_chip.
>>
>> Signed-off-by: Bitterblue Smith <[email protected]>
>> ---
>> .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 5 ++++
>> .../realtek/rtl8xxxu/rtl8xxxu_8188e.c | 1 +
>> .../realtek/rtl8xxxu/rtl8xxxu_8188f.c | 5 ++++
>> .../realtek/rtl8xxxu/rtl8xxxu_8192e.c | 1 +
>> .../realtek/rtl8xxxu/rtl8xxxu_8710b.c | 9 +++++++
>> .../realtek/rtl8xxxu/rtl8xxxu_8723b.c | 3 +++
>> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 26 +++++--------------
>> 7 files changed, 31 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> index 6a82ec47568e..af8436070ba7 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188e.c
>> @@ -1883,6 +1883,7 @@ struct rtl8xxxu_fileops rtl8188eu_fops = {
>> .rx_desc_size = sizeof(struct rtl8xxxu_rxdesc16),
>> .tx_desc_size = sizeof(struct rtl8xxxu_txdesc32),
>> .has_tx_report = 1,
>> + .init_reg_pkt_life_time = 1,
>
> I'm sure it's safe, but the fops structs that don't set the
> ampdu_max_time and ustime_tsf_edca values feel odd.
>
They don't set those because they don't use them. Maybe one day
I will initialise all the members -- I read somewhere that it's
good practice -- but that's not what this patch is about.
>> .gen2_thermal_meter = 1,
>> .adda_1t_init = 0x0b1b25a0,
>> .adda_1t_path_on = 0x0bdb25a0,
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> index 82dee1fed477..dfb250adb168 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8188f.c
>> @@ -1746,6 +1746,11 @@ struct rtl8xxxu_fileops rtl8188fu_fops = {
>> .has_tx_report = 1,
>> .gen2_thermal_meter = 1,
>> .needs_full_init = 1,
>> + .init_reg_rxfltmap = 1,
>> + .init_reg_pkt_life_time = 1,
>> + .init_reg_hmtfr = 1,
>> + .ampdu_max_time = 0x70,
>> + .ustime_tsf_edca = 0x28,
>
> The original code had comments for why the 8188fu had different values
> for ampdu_max_time and ustime_tsf_edca. Should they be copied here?
>
I don't know if those comments are that useful.
>> .adda_1t_init = 0x03c00014,
>> .adda_1t_path_on = 0x03c00014,
>> .trxff_boundary = 0x3f7f,
>
> Thanks,
>
> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Saturday, April 1, 2023 4:17 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
> Subject: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs
>
> Add some new members to rtl8xxxu_fileops and use them instead of
> checking priv->rtl_chip.
>
> Signed-off-by: Bitterblue Smith <[email protected]>
> ---
[...]
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index c152b228606f..62dd53a57659 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -1916,7 +1916,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
> /*
> * Init H2C command
> */
> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
> + if (priv->fops->init_reg_hmtfr)
> rtl8xxxu_write8(priv, REG_HMTFR, 0x0f);
> exit:
> return ret;
> @@ -3864,11 +3864,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
> rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
>
> rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8710B)
> - val8 = 0x5e;
> - else if (priv->rtl_chip == RTL8188F)
> - val8 = 0x70; /* 0x5e would make it very slow */
> - rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8);
> + rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B,
> + priv->fops->ampdu_max_time);
Should it be
if (priv->fops->ampdu_max_time)
val8 = priv->fops->ampdu_max_time;
rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8); // this line doesn't change?
Because originally val8 is read from REG_HT_SINGLE_AMPDU_8723B and add
HT_SINGLE_AMPDU_ENABLE bit.
... I review further and want to add similar comment. I wonder you do this
intentionally, so I find rtl8xxxu_init_burst() is only used by three chips
RTL8723B, RTL8710B and RTL8188F. I'm not sure if other people could misuse
this function in the future, any idea?
> rtl8xxxu_write32(priv, REG_AGGLEN_LMT, 0xffffffff);
> rtl8xxxu_write8(priv, REG_RX_PKT_LIMIT, 0x18);
> rtl8xxxu_write8(priv, REG_PIFS, 0x00);
> @@ -3876,16 +3873,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
> rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL, FWHW_TXQ_CTRL_AMPDU_RETRY);
> rtl8xxxu_write32(priv, REG_FAST_EDCA_CTRL, 0x03086666);
> }
> - /*
> - * The RTL8710BU vendor driver uses 0x50 here and it works fine,
> - * but in rtl8xxxu 0x50 causes slow upload and random packet loss. Why?
> - */
> - if (priv->rtl_chip == RTL8723B)
> - val8 = 0x50;
> - else if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
> - val8 = 0x28; /* 0x50 would make the upload slow */
> - rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, val8);
> - rtl8xxxu_write8(priv, REG_USTIME_EDCA, val8);
> + rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, priv->fops->ustime_tsf_edca);
> + rtl8xxxu_write8(priv, REG_USTIME_EDCA, priv->fops->ustime_tsf_edca);
>
> /* to prevent mac is reseted by bus. */
> val8 = rtl8xxxu_read8(priv, REG_RSV_CTRL);
> @@ -4102,7 +4091,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> RCR_APPEND_PHYSTAT | RCR_APPEND_ICV | RCR_APPEND_MIC;
> rtl8xxxu_write32(priv, REG_RCR, val32);
>
> - if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B) {
> + if (fops->init_reg_rxfltmap) {
> /* Accept all data frames */
> rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
>
> @@ -4187,8 +4176,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
> if (fops->init_aggregation)
> fops->init_aggregation(priv);
>
> - if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8188E ||
> - priv->rtl_chip == RTL8710B) {
> + if (fops->init_reg_pkt_life_time) {
Originally, 8192E doesn't do this. Just make sure you want to do it?
> rtl8xxxu_write16(priv, REG_PKT_VO_VI_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
> rtl8xxxu_write16(priv, REG_PKT_BE_BK_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
> }
> --
> 2.39.2
>
> ------Please consider the environment before printing this e-mail.
> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Saturday, April 1, 2023 4:18 AM
> To: [email protected]
> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
> Subject: [PATCH 2/2] wifi: rtl8xxxu: Support devices with 5-6 out endpoints
>
> Handle them the same way as the devices with 3-4 USB out endpoints.
> This is needed for the RTL8192FU.
>
> Signed-off-by: Bitterblue Smith <[email protected]>
Reviewed-by: Ping-Ke Shih <[email protected]>
> ---
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +-
> drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 39fee07917e7..82a0290ccb29 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -27,7 +27,7 @@
> #define RTL8XXXU_MAX_REG_POLL 500
> #define USB_INTR_CONTENT_LENGTH 56
>
> -#define RTL8XXXU_OUT_ENDPOINTS 4
> +#define RTL8XXXU_OUT_ENDPOINTS 6
>
> #define REALTEK_USB_READ 0xc0
> #define REALTEK_USB_WRITE 0x40
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 62dd53a57659..6106b47d0c37 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -1663,6 +1663,8 @@ int rtl8xxxu_config_endpoints_no_sie(struct rtl8xxxu_priv *priv)
> struct device *dev = &priv->udev->dev;
>
> switch (priv->nr_out_eps) {
> + case 6:
> + case 5:
> case 4:
> case 3:
> priv->ep_tx_low_queue = 1;
> --
> 2.39.2
>
> ------Please consider the environment before printing this e-mail.
On 06/04/2023 04:16, Ping-Ke Shih wrote:
>
>
>> -----Original Message-----
>> From: Bitterblue Smith <[email protected]>
>> Sent: Saturday, April 1, 2023 4:17 AM
>> To: [email protected]
>> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
>> Subject: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs
>>
>> Add some new members to rtl8xxxu_fileops and use them instead of
>> checking priv->rtl_chip.
>>
>> Signed-off-by: Bitterblue Smith <[email protected]>
>> ---
>
> [...]
>
>> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> index c152b228606f..62dd53a57659 100644
>> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
>> @@ -1916,7 +1916,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
>> /*
>> * Init H2C command
>> */
>> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
>> + if (priv->fops->init_reg_hmtfr)
>> rtl8xxxu_write8(priv, REG_HMTFR, 0x0f);
>> exit:
>> return ret;
>> @@ -3864,11 +3864,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
>> rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
>>
>> rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
>> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8710B)
>> - val8 = 0x5e;
>> - else if (priv->rtl_chip == RTL8188F)
>> - val8 = 0x70; /* 0x5e would make it very slow */
>> - rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8);
>> + rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B,
>> + priv->fops->ampdu_max_time);
>
> Should it be
>
> if (priv->fops->ampdu_max_time)
> val8 = priv->fops->ampdu_max_time;>
> rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8); // this line doesn't change?
>
> Because originally val8 is read from REG_HT_SINGLE_AMPDU_8723B and add
> HT_SINGLE_AMPDU_ENABLE bit.
No, the value read from REG_HT_SINGLE_AMPDU_8723B is not supposed to be
written to REG_AMPDU_MAX_TIME_8723B. And it never was, because only
RTL8723B, RTL8710B, and RTL8188F use this function. This was clearer in
the original version of the code, when it was used only by RTL8723B:
val8 = rtl8xxxu_read8(priv, REG_HT_SINGLE_AMPDU_8723B);
val8 |= BIT(7);
rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, 0x5e);
>
> ... I review further and want to add similar comment. I wonder you do this
> intentionally, so I find rtl8xxxu_init_burst() is only used by three chips
> RTL8723B, RTL8710B and RTL8188F. I'm not sure if other people could misuse
> this function in the future, any idea?
>
That will be their own mistake. :)
>> rtl8xxxu_write32(priv, REG_AGGLEN_LMT, 0xffffffff);
>> rtl8xxxu_write8(priv, REG_RX_PKT_LIMIT, 0x18);
>> rtl8xxxu_write8(priv, REG_PIFS, 0x00);
>> @@ -3876,16 +3873,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
>> rtl8xxxu_write8(priv, REG_FWHW_TXQ_CTRL, FWHW_TXQ_CTRL_AMPDU_RETRY);
>> rtl8xxxu_write32(priv, REG_FAST_EDCA_CTRL, 0x03086666);
>> }
>> - /*
>> - * The RTL8710BU vendor driver uses 0x50 here and it works fine,
>> - * but in rtl8xxxu 0x50 causes slow upload and random packet loss. Why?
>> - */
>> - if (priv->rtl_chip == RTL8723B)
>> - val8 = 0x50;
>> - else if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
>> - val8 = 0x28; /* 0x50 would make the upload slow */
>> - rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, val8);
>> - rtl8xxxu_write8(priv, REG_USTIME_EDCA, val8);
>> + rtl8xxxu_write8(priv, REG_USTIME_TSF_8723B, priv->fops->ustime_tsf_edca);
>> + rtl8xxxu_write8(priv, REG_USTIME_EDCA, priv->fops->ustime_tsf_edca);
>>
>> /* to prevent mac is reseted by bus. */
>> val8 = rtl8xxxu_read8(priv, REG_RSV_CTRL);
>> @@ -4102,7 +4091,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>> RCR_APPEND_PHYSTAT | RCR_APPEND_ICV | RCR_APPEND_MIC;
>> rtl8xxxu_write32(priv, REG_RCR, val32);
>>
>> - if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B) {
>> + if (fops->init_reg_rxfltmap) {
>> /* Accept all data frames */
>> rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
>>
>> @@ -4187,8 +4176,7 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>> if (fops->init_aggregation)
>> fops->init_aggregation(priv);
>>
>> - if (priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8188E ||
>> - priv->rtl_chip == RTL8710B) {
>> + if (fops->init_reg_pkt_life_time) {
>
> Originally, 8192E doesn't do this. Just make sure you want to do it?
>
I did want to do it. The RTL8192EU driver sets those registers. But
maybe that change should be in a separate patch. I'll send v2 where
RTL8192E doesn't set init_reg_pkt_life_time.
>> rtl8xxxu_write16(priv, REG_PKT_VO_VI_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
>> rtl8xxxu_write16(priv, REG_PKT_BE_BK_LIFE_TIME, 0x0400); /* unit: 256us. 256ms */
>> }
>> --
>> 2.39.2
>>
>> ------Please consider the environment before printing this e-mail.
> -----Original Message-----
> From: Bitterblue Smith <[email protected]>
> Sent: Sunday, April 9, 2023 10:11 PM
> To: Ping-Ke Shih <[email protected]>; [email protected]
> Cc: Jes Sorensen <[email protected]>
> Subject: Re: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs
>
> On 06/04/2023 04:16, Ping-Ke Shih wrote:
> >
> >
> >> -----Original Message-----
> >> From: Bitterblue Smith <[email protected]>
> >> Sent: Saturday, April 1, 2023 4:17 AM
> >> To: [email protected]
> >> Cc: Jes Sorensen <[email protected]>; Ping-Ke Shih <[email protected]>
> >> Subject: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs
> >>
> >> Add some new members to rtl8xxxu_fileops and use them instead of
> >> checking priv->rtl_chip.
> >>
> >> Signed-off-by: Bitterblue Smith <[email protected]>
> >> ---
> >
> > [...]
> >
> >> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> index c152b228606f..62dd53a57659 100644
> >> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> >> @@ -1916,7 +1916,7 @@ static int rtl8xxxu_start_firmware(struct rtl8xxxu_priv *priv)
> >> /*
> >> * Init H2C command
> >> */
> >> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8188F || priv->rtl_chip == RTL8710B)
> >> + if (priv->fops->init_reg_hmtfr)
> >> rtl8xxxu_write8(priv, REG_HMTFR, 0x0f);
> >> exit:
> >> return ret;
> >> @@ -3864,11 +3864,8 @@ void rtl8xxxu_init_burst(struct rtl8xxxu_priv *priv)
> >> rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
> >>
> >> rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
> >> - if (priv->rtl_chip == RTL8723B || priv->rtl_chip == RTL8710B)
> >> - val8 = 0x5e;
> >> - else if (priv->rtl_chip == RTL8188F)
> >> - val8 = 0x70; /* 0x5e would make it very slow */
> >> - rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8);
> >> + rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B,
> >> + priv->fops->ampdu_max_time);
> >
> > Should it be
> >
> > if (priv->fops->ampdu_max_time)
> > val8 = priv->fops->ampdu_max_time;>
> > rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, val8); // this line doesn't change?
> >
> > Because originally val8 is read from REG_HT_SINGLE_AMPDU_8723B and add
> > HT_SINGLE_AMPDU_ENABLE bit.
>
> No, the value read from REG_HT_SINGLE_AMPDU_8723B is not supposed to be
> written to REG_AMPDU_MAX_TIME_8723B. And it never was, because only
> RTL8723B, RTL8710B, and RTL8188F use this function. This was clearer in
> the original version of the code, when it was used only by RTL8723B:
>
> val8 = rtl8xxxu_read8(priv, REG_HT_SINGLE_AMPDU_8723B);
> val8 |= BIT(7);
> rtl8xxxu_write8(priv, REG_HT_SINGLE_AMPDU_8723B, val8);
>
> rtl8xxxu_write16(priv, REG_MAX_AGGR_NUM, 0x0c14);
> rtl8xxxu_write8(priv, REG_AMPDU_MAX_TIME_8723B, 0x5e);
>
Oops. Somehow I misunderstood the code. Sorry for the noise.