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