| Message ID | 78cec57b-2678-acf3-99b3-271e0f9bdbad@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: rtl8xxxu: A few improvements | expand |
On 9/18/22 08:35, Bitterblue Smith wrote: > Inform the firmware when connecting to a network. This makes the > firmware enable the rate control, which makes the upload faster. > > Don't inform the firmware when disconnecting from a network, because > that makes reconnecting impossible for some reason: Have you dug through the vendor driver to see what it does here? Thanks, Jes > wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 1/3) > wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 2/3) > wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 3/3) > wlp0s20f0u3: authentication with 90:55:de:__:__:__ timed out > > Not informing the firmware about disconnecting doesn't seem to have > any downside. > > Tested only with RTL8188FU. Someone should test the other gen2 chips: > RTL8723BU and RTL8192EU. > > Fixes: c59f13bbead4 ("rtl8xxxu: Work around issue with 8192eu and 8723bu devices not reconn…") > Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> > --- > .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > index 32ab6ed5b9b6..7978d5dcc826 100644 > --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c > @@ -4471,25 +4471,34 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, > void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv, > u8 macid, bool connect) > { > -#ifdef RTL8XXXU_GEN2_REPORT_CONNECT > /* > * Barry Day reports this causes issues with 8192eu and 8723bu > * devices reconnecting. The reason for this is unclear, but > * until it is better understood, leave the code in place but > * disabled, so it is not lost. > */ > + /* > + * It also affects 8188FU. However, without this the rate control > + * is not on. Probably it only enables the rate control when it > + * knows it's connected to a network. > + * > + * Hack: don't report the disconnect. This way the rate control > + * is on and reconnecting also works. TODO Test 8192EU and 8723BU. > + */ > struct h2c_cmd h2c; > > memset(&h2c, 0, sizeof(struct h2c_cmd)); > > h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT; > - if (connect) > + if (connect) { > h2c.media_status_rpt.parm |= BIT(0); > + /* > else > h2c.media_status_rpt.parm &= ~BIT(0); > + */ > > - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt)); > -#endif > + rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt)); > + } > } > > void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
On 25/09/2022 19:53, Jes Sorensen wrote: > On 9/18/22 08:35, Bitterblue Smith wrote: >> Inform the firmware when connecting to a network. This makes the >> firmware enable the rate control, which makes the upload faster. >> >> Don't inform the firmware when disconnecting from a network, because >> that makes reconnecting impossible for some reason: > > Have you dug through the vendor driver to see what it does here? > > Thanks, > Jes > I hadn't investigated, but since you asked :) I looked into it today. The vendor driver doesn't do anything weird. Our report_connect function *should* work. And it turns out it does work! I restored the original form of the function to test something and reconnecting worked. I couldn't reproduce the problem anymore. Not much has changed in rtl8xxxu since the last time I tried this, so it was easy to find the reason: fixing the queue selection [0] fixed the reconnecting problem. Before, it was sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps queue 0x7 is not functional when the firmware knows it's not connected to a network? I guess I have to send a different patch for this now. [0] https://lore.kernel.org/linux-wireless/7fa4819a-4f20-b2af-b7a6-8ee01ac49295@gmail.com/
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes: > On 25/09/2022 19:53, Jes Sorensen wrote: >> On 9/18/22 08:35, Bitterblue Smith wrote: >>> Inform the firmware when connecting to a network. This makes the >>> firmware enable the rate control, which makes the upload faster. >>> >>> Don't inform the firmware when disconnecting from a network, because >>> that makes reconnecting impossible for some reason: >> >> Have you dug through the vendor driver to see what it does here? >> >> Thanks, >> Jes >> > > I hadn't investigated, but since you asked :) I looked into it today. > > The vendor driver doesn't do anything weird. Our report_connect > function *should* work. > > And it turns out it does work! I restored the original form of the > function to test something and reconnecting worked. I couldn't > reproduce the problem anymore. Not much has changed in rtl8xxxu since > the last time I tried this, so it was easy to find the reason: fixing > the queue selection [0] fixed the reconnecting problem. Before, it was > sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the > queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps > queue 0x7 is not functional when the firmware knows it's not connected > to a network? > > I guess I have to send a different patch for this now. So what should I do with this patchset? Can I take patches 2-4?
On 28/09/2022 11:39, Kalle Valo wrote: > Bitterblue Smith <rtl8821cerfe2@gmail.com> writes: > >> On 25/09/2022 19:53, Jes Sorensen wrote: >>> On 9/18/22 08:35, Bitterblue Smith wrote: >>>> Inform the firmware when connecting to a network. This makes the >>>> firmware enable the rate control, which makes the upload faster. >>>> >>>> Don't inform the firmware when disconnecting from a network, because >>>> that makes reconnecting impossible for some reason: >>> >>> Have you dug through the vendor driver to see what it does here? >>> >>> Thanks, >>> Jes >>> >> >> I hadn't investigated, but since you asked :) I looked into it today. >> >> The vendor driver doesn't do anything weird. Our report_connect >> function *should* work. >> >> And it turns out it does work! I restored the original form of the >> function to test something and reconnecting worked. I couldn't >> reproduce the problem anymore. Not much has changed in rtl8xxxu since >> the last time I tried this, so it was easy to find the reason: fixing >> the queue selection [0] fixed the reconnecting problem. Before, it was >> sending the auth attempts using queue 0x7 (TXDESC_QUEUE_VO). With the >> queue selection fix it uses queue 0x12 (TXDESC_QUEUE_MGNT). Perhaps >> queue 0x7 is not functional when the firmware knows it's not connected >> to a network? >> >> I guess I have to send a different patch for this now. > > So what should I do with this patchset? Can I take patches 2-4? > Yes, please. They're all independent.
Bitterblue Smith <rtl8821cerfe2@gmail.com> writes: > On 28/09/2022 11:39, Kalle Valo wrote: > >> Bitterblue Smith <rtl8821cerfe2@gmail.com> writes: >> >>> I guess I have to send a different patch for this now. >> >> So what should I do with this patchset? Can I take patches 2-4? > > Yes, please. They're all independent. Very good, so I'll then take patches 2-4. I'll drop patch 1 and wait for the next version.
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 32ab6ed5b9b6..7978d5dcc826 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -4471,25 +4471,34 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv, void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv, u8 macid, bool connect) { -#ifdef RTL8XXXU_GEN2_REPORT_CONNECT /* * Barry Day reports this causes issues with 8192eu and 8723bu * devices reconnecting. The reason for this is unclear, but * until it is better understood, leave the code in place but * disabled, so it is not lost. */ + /* + * It also affects 8188FU. However, without this the rate control + * is not on. Probably it only enables the rate control when it + * knows it's connected to a network. + * + * Hack: don't report the disconnect. This way the rate control + * is on and reconnecting also works. TODO Test 8192EU and 8723BU. + */ struct h2c_cmd h2c; memset(&h2c, 0, sizeof(struct h2c_cmd)); h2c.media_status_rpt.cmd = H2C_8723B_MEDIA_STATUS_RPT; - if (connect) + if (connect) { h2c.media_status_rpt.parm |= BIT(0); + /* else h2c.media_status_rpt.parm &= ~BIT(0); + */ - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt)); -#endif + rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt)); + } } void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
Inform the firmware when connecting to a network. This makes the firmware enable the rate control, which makes the upload faster. Don't inform the firmware when disconnecting from a network, because that makes reconnecting impossible for some reason: wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 1/3) wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 2/3) wlp0s20f0u3: send auth to 90:55:de:__:__:__ (try 3/3) wlp0s20f0u3: authentication with 90:55:de:__:__:__ timed out Not informing the firmware about disconnecting doesn't seem to have any downside. Tested only with RTL8188FU. Someone should test the other gen2 chips: RTL8723BU and RTL8192EU. Fixes: c59f13bbead4 ("rtl8xxxu: Work around issue with 8192eu and 8723bu devices not reconn…") Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com> --- .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)