The rtl8192e and rtl8723 fail to reconnect to an AP after being
disconnected. Ths patch fixes that without affecting the rtl8192cu.
I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
After going through the orginal realtek code for the rtl8723, I am
confident the patch is applicable to both.
Signed-off-by: Barry Day <[email protected]>
---
rtl8xxxu_core.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
index 04141e5..6ac10d2 100644
--- a/rtl8xxxu_core.c
+++ b/rtl8xxxu_core.c
@@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
u8 macid, bool connect)
{
+ u8 val8;
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));
+ } else {
+ val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
+ val8 &= ~BEACON_FUNCTION_ENABLE;
+
+ rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
+ rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
+ rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
+ }
- rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
}
void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
@@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
sgi = 1;
rcu_read_unlock();
+ rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
+
priv->fops->update_rate_mask(priv, ramask, sgi);
rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
--
2.9.2
Jes Sorensen <[email protected]> writes:
> Barry Day <[email protected]> writes:
>> The rtl8192e and rtl8723 fail to reconnect to an AP after being
>> disconnected. Ths patch fixes that without affecting the rtl8192cu.
>> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
>> After going through the orginal realtek code for the rtl8723, I am
>> confident the patch is applicable to both.
>>
>> Signed-off-by: Barry Day <[email protected]>
>> ---
>> rtl8xxxu_core.c | 18 ++++++++++++++----
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi Barry,
>
> Thank you for the patch. There are a couple of items which I am not
> 100% sure about the order of.
>
>> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
>> index 04141e5..6ac10d2 100644
>> --- a/rtl8xxxu_core.c
>> +++ b/rtl8xxxu_core.c
>> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
>> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>> u8 macid, bool connect)
>> {
>> + u8 val8;
>> 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));
>> + } else {
>> + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> + val8 &= ~BEACON_FUNCTION_ENABLE;
>> +
>> + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
>> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
>> + }
>>
>> - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
>> }
Barry,
So looking at this again, I am pretty sure you will break monitor mode
with this. By setting REG_RXFLTMAP2 to 0x0000 you stop reception of all
data frames.
The other thing here is that you change removes the part notifying the
firmware that we disconnected since you now only send the
media_start_rpt command on connect, but not on disconnect.
I am curious if the problem goes away if you simply add the BEACON_CTRL
and REG_DUAL_TSF_RST parts?
>
> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
>> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> sgi = 1;
>> rcu_read_unlock();
>>
>> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
>> +
>> priv->fops->update_rate_mask(priv, ramask, sgi);
>>
>> rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
I believe this change only matters because you disable RXFLTMAP2
above. If we really have to write to RXFLTMAP2 to make this work, I
suspect we need to keep some sort of state information.
I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
and we do not account for that.
Cheers,
Jes
On Mon, Nov 14, 2016 at 08:24:45AM -0500, Jes Sorensen wrote:
> Barry Day <[email protected]> writes:
> > The rtl8192e and rtl8723 fail to reconnect to an AP after being
> > disconnected. Ths patch fixes that without affecting the rtl8192cu.
> > I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
> > After going through the orginal realtek code for the rtl8723, I am
> > confident the patch is applicable to both.
> >
> > Signed-off-by: Barry Day <[email protected]>
> > ---
> > rtl8xxxu_core.c | 18 ++++++++++++++----
> > 1 file changed, 14 insertions(+), 4 deletions(-)
>
> Hi Barry,
>
> Thank you for the patch. There are a couple of items which I am not
> 100% sure about the order of.
>
> > diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
> > index 04141e5..6ac10d2 100644
> > --- a/rtl8xxxu_core.c
> > +++ b/rtl8xxxu_core.c
> > @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> > void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> > u8 macid, bool connect)
> > {
> > + u8 val8;
> > 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));
> > + } else {
> > + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> > + val8 &= ~BEACON_FUNCTION_ENABLE;
> > +
> > + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> > + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
> > + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
> > + }
> >
> > - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> > }
>
> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
Even in monitor mode the interface has to brought up to use it which invokes
rtl8xxxu_start which sets it back to accepting frames.
> > void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
> > @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> > sgi = 1;
> > rcu_read_unlock();
> >
> > + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
> > +
> > priv->fops->update_rate_mask(priv, ramask, sgi);
> >
> > rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>
> Here you enable RXFLTMAP2 for all devices - this doesn't balance with
> the above.
>
The original realtek code I have for the 8192cu does the disconnect the
same way as in this patch. I tested the 8192cu using the patched
gen2_report connect and it works. That would make things consistent across
all chipsets.
Barry Day <[email protected]> writes:
> On Mon, Nov 21, 2016 at 11:57:22AM -0500, Jes Sorensen wrote:
>> Jes Sorensen <[email protected]> writes:
>> >> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> >> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> >> sgi = 1;
>> >> rcu_read_unlock();
>> >>
>> >> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
>> >> +
>> >> priv->fops->update_rate_mask(priv, ramask, sgi);
>> >>
>> >> rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>>
>> I believe this change only matters because you disable RXFLTMAP2
>> above. If we really have to write to RXFLTMAP2 to make this work, I
>> suspect we need to keep some sort of state information.
>>
>> I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
>> and we do not account for that.
>>
>> Cheers,
>> Jes
>
> I'll redo the patch without touching REG_RXFLTMAP2. I don't think it's needed
> to fix the fail to reconnect issue.
>
> I haven't had a proper look at the 8723 chips yet but the vendor drivers for
> the others don't do a h2c cmd for disconnect but I'll test leaving it in to see
> if it makes any difference. A 8723bu arrived in the mail today so now I can
> test it too and I discovered yesterday I have a 8723au but it's in a cheap
> Android tablet.
Let me know what you find out - if the h2c command causes the failure
that would be very bizarre but certainly interesting to learn.
Cheers,
Jes
Barry Day <[email protected]> writes:
> On Mon, Nov 14, 2016 at 08:24:45AM -0500, Jes Sorensen wrote:
>> Barry Day <[email protected]> writes:
>> > The rtl8192e and rtl8723 fail to reconnect to an AP after being
>> > disconnected. Ths patch fixes that without affecting the rtl8192cu.
>> > I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
>> > After going through the orginal realtek code for the rtl8723, I am
>> > confident the patch is applicable to both.
>> >
>> > Signed-off-by: Barry Day <[email protected]>
>> > ---
>> > rtl8xxxu_core.c | 18 ++++++++++++++----
>> > 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> Hi Barry,
>>
>> Thank you for the patch. There are a couple of items which I am not
>> 100% sure about the order of.
>>
>> > diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
>> > index 04141e5..6ac10d2 100644
>> > --- a/rtl8xxxu_core.c
>> > +++ b/rtl8xxxu_core.c
>> > @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
>> > void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
>> > u8 macid, bool connect)
>> > {
>> > + u8 val8;
>> > 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));
>> > + } else {
>> > + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
>> > + val8 &= ~BEACON_FUNCTION_ENABLE;
>> > +
>> > + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>> > + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
>> > + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
>> > + }
>> >
>> > - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
>> > }
>>
>> This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
>> on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
>
> Even in monitor mode the interface has to brought up to use it which invokes
> rtl8xxxu_start which sets it back to accepting frames.
>
>
>> > void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
>> > @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>> > sgi = 1;
>> > rcu_read_unlock();
>> >
>> > + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
>> > +
>> > priv->fops->update_rate_mask(priv, ramask, sgi);
>> >
>> > rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>>
>> Here you enable RXFLTMAP2 for all devices - this doesn't balance with
>> the above.
>
> The original realtek code I have for the 8192cu does the disconnect the
> same way as in this patch. I tested the 8192cu using the patched
> gen2_report connect and it works. That would make things consistent across
> all chipsets.
My concern is that you only set FLATMAP and BEACON_CTRL for gen2
devices, which has zero impact on gen1 devices, such as the 8192cu. If
need to do this for gen2 (8723bu/8192eu) I assume we need to do it for
gen1 as well.
I'd like to see us do a little more investigating on this first.
Jes
Barry Day <[email protected]> writes:
> The rtl8192e and rtl8723 fail to reconnect to an AP after being
> disconnected. Ths patch fixes that without affecting the rtl8192cu.
> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
> After going through the orginal realtek code for the rtl8723, I am
> confident the patch is applicable to both.
>
> Signed-off-by: Barry Day <[email protected]>
> ---
> rtl8xxxu_core.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
Hi Barry,
Thank you for the patch. There are a couple of items which I am not
100% sure about the order of.
> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
> index 04141e5..6ac10d2 100644
> --- a/rtl8xxxu_core.c
> +++ b/rtl8xxxu_core.c
> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> u8 macid, bool connect)
> {
> + u8 val8;
> 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));
> + } else {
> + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> + val8 &= ~BEACON_FUNCTION_ENABLE;
> +
> + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
> + }
>
> - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> }
This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> sgi = 1;
> rcu_read_unlock();
>
> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
> +
> priv->fops->update_rate_mask(priv, ramask, sgi);
>
> rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
Here you enable RXFLTMAP2 for all devices - this doesn't balance with
the above.
I am not against the aproach, but I think we need to investigate a
little further what is going on.
Cheers,
Jes
On Mon, Nov 21, 2016 at 11:57:22AM -0500, Jes Sorensen wrote:
> Jes Sorensen <[email protected]> writes:
> > Barry Day <[email protected]> writes:
> >> The rtl8192e and rtl8723 fail to reconnect to an AP after being
> >> disconnected. Ths patch fixes that without affecting the rtl8192cu.
> >> I don't have a rtl8723 to test but it has been tested on a rtl8192eu.
> >> After going through the orginal realtek code for the rtl8723, I am
> >> confident the patch is applicable to both.
> >>
> >> Signed-off-by: Barry Day <[email protected]>
> >> ---
> >> rtl8xxxu_core.c | 18 ++++++++++++++----
> >> 1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > Hi Barry,
> >
> > Thank you for the patch. There are a couple of items which I am not
> > 100% sure about the order of.
> >
> >> diff --git a/rtl8xxxu_core.c b/rtl8xxxu_core.c
> >> index 04141e5..6ac10d2 100644
> >> --- a/rtl8xxxu_core.c
> >> +++ b/rtl8xxxu_core.c
> >> @@ -4372,17 +4372,25 @@ void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
> >> void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> >> u8 macid, bool connect)
> >> {
> >> + u8 val8;
> >> 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));
> >> + } else {
> >> + val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> >> + val8 &= ~BEACON_FUNCTION_ENABLE;
> >> +
> >> + rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
> >> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0x00);
> >> + rtl8xxxu_write8(priv, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
> >> + }
> >>
> >> - rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.media_status_rpt));
> >> }
>
> Barry,
>
> So looking at this again, I am pretty sure you will break monitor mode
> with this. By setting REG_RXFLTMAP2 to 0x0000 you stop reception of all
> data frames.
>
> The other thing here is that you change removes the part notifying the
> firmware that we disconnected since you now only send the
> media_start_rpt command on connect, but not on disconnect.
>
> I am curious if the problem goes away if you simply add the BEACON_CTRL
> and REG_DUAL_TSF_RST parts?
>
> >
> > This only affects 8192eu and not 8192cu - we left RXFLTMAP2 out of here
> > on purpose for monitor mode, but you now disable it for 8192eu/8723bu.
> >
> >> void rtl8xxxu_gen1_init_aggregation(struct rtl8xxxu_priv *priv)
> >> @@ -4515,6 +4523,8 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
> >> sgi = 1;
> >> rcu_read_unlock();
> >>
> >> + rtl8xxxu_write16(priv, REG_RXFLTMAP2, 0xffff);
> >> +
> >> priv->fops->update_rate_mask(priv, ramask, sgi);
> >>
> >> rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>
> I believe this change only matters because you disable RXFLTMAP2
> above. If we really have to write to RXFLTMAP2 to make this work, I
> suspect we need to keep some sort of state information.
>
> I would also be curious if RXFLTMAP2 gets reset somehow by the firmware,
> and we do not account for that.
>
> Cheers,
> Jes
>
I'll redo the patch without touching REG_RXFLTMAP2. I don't think it's needed
to fix the fail to reconnect issue.
I haven't had a proper look at the 8723 chips yet but the vendor drivers for
the others don't do a h2c cmd for disconnect but I'll test leaving it in to see
if it makes any difference. A 8723bu arrived in the mail today so now I can
test it too and I discovered yesterday I have a 8723au but it's in a cheap
Android tablet.
Barry