Search Linux Wireless

RE: [PATCH 1/2] wifi: rtl8xxxu: Clean up some messy ifs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> Sent: Saturday, April 1, 2023 4:17 AM
> To: linux-wireless@xxxxxxxxxxxxxxx
> Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> 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@xxxxxxxxx>
> ---

[...]

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




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux