> -----Original Message----- > From: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> > Sent: Sunday, April 9, 2023 10:11 PM > To: Ping-Ke Shih <pkshih@xxxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx > Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx> > 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@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. > > 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.
