| Message ID | eb152b5b-fe65-3783-a3d9-71c9cb7ef9d3@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/2] wifi: rtl8xxxu: Clean up some messy ifs | expand |
Hi Bitterblue, On Sat, Apr 1, 2023 at 7:18 AM Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: > > Add some new members to rtl8xxxu_fileops and use them instead of > checking priv->rtl_chip. > > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > --- > .../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,
On 01/04/2023 07:23, Julian Calaby wrote: > Hi Bitterblue, > > On Sat, Apr 1, 2023 at 7:18 AM Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote: >> >> Add some new members to rtl8xxxu_fileops and use them instead of >> checking priv->rtl_chip. >> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> --- >> .../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 <rtl8821cerfe2@gmail.com> > Sent: Saturday, April 1, 2023 4:17 AM > To: linux-wireless@vger.kernel.org > Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com> > 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 <rtl8821cerfe2@gmail.com> > --- [...] > 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.
On 06/04/2023 04:16, Ping-Ke Shih wrote: > > >> -----Original Message----- >> From: Bitterblue Smith <rtl8821cerfe2@gmail.com> >> Sent: Saturday, April 1, 2023 4:17 AM >> To: linux-wireless@vger.kernel.org >> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com> >> 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 <rtl8821cerfe2@gmail.com> >> --- > > [...] > >> 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 <rtl8821cerfe2@gmail.com> > Sent: Sunday, April 9, 2023 10:11 PM > To: Ping-Ke Shih <pkshih@realtek.com>; linux-wireless@vger.kernel.org > Cc: Jes Sorensen <Jes.Sorensen@gmail.com> > 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 <rtl8821cerfe2@gmail.com> > >> Sent: Saturday, April 1, 2023 4:17 AM > >> To: linux-wireless@vger.kernel.org > >> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Ping-Ke Shih <pkshih@realtek.com> > >> 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 <rtl8821cerfe2@gmail.com> > >> --- > > > > [...] > > > >> 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.
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 */ }
Add some new members to rtl8xxxu_fileops and use them instead of checking priv->rtl_chip. Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- .../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(-)