[BUG] staging: r8822be: RTL8822be can't find any wireless AP

Commit Message

Ping-Ke Shih July 5, 2018, 7:36 a.m. UTC

On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:
> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:

> > We will have to agree to disagree.

> >

> > I have no idea what the vendors are doing that cause some motherboards to

> > need a different aspm value. What I do know is that we have had to live with

> > the idiocy of some vendors saving a few pennies by only including a single

> > antenna, rather than two, and then making a problem by miscoding the EFUSE

> > bit that indicates which connector is actually in use. As we have no means

> > that I know about to detect which boxes have the problem, a module parameter

> > was created, just as in this case.

> >

> > I agree that drivers should work "out of the box", but finite resources and

> > lack of vendor cooperation make this a goal that may not be attainable.

> 

> As you touched on, the ideal situation is that Realtek solve the

> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from

> the commit log. The context is that the r8822 driver fails on several

> platforms unless setting aspm=0 (the default is 1).


It's hard to have all laptop or motherboards and all rtl8822be modules in my side,
so what I can do is to analyze the issue when user encountered.

> 

> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba

> https://bugzilla.kernel.org/show_bug.cgi?id=199651

> 

> 

> If we don't get a timely fix from Realtek though, I think there is a

> key difference between the antenna selection headache and this one. In

> the antenna case, there isn't a good value that you can set that will

> work on all systems. If you change the default behaviour you will

> solve the issue for some users while simultanously introducing the

> problem on other systems that were previously fine.

> 

> However in this case, it's highly likely that setting aspm=0 (off) by

> default would work for everyone. It has the disadvantage of using a

> bit more power, but especially with the indications that this issue

> affects a significant number of systems, I think that having the

> driver working out of the box everywhere is more important. The module

> parameter can be left in place so that unaffected users that want to

> save power can set aspm=1.

> 


I think this issue may be due to L1 latency, so below patch would be
helpful but not sure because I don't have the same laptop.
Is there anyone can help to test?



---
PK

Comments

Jian-Hong Pan July 5, 2018, 9:26 a.m. UTC | #1

2018-07-05 15:36 GMT+08:00 Pkshih <[e-mail]>:
> On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:
>> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:
>> > We will have to agree to disagree.
>> >
>> > I have no idea what the vendors are doing that cause some motherboards to
>> > need a different aspm value. What I do know is that we have had to live with
>> > the idiocy of some vendors saving a few pennies by only including a single
>> > antenna, rather than two, and then making a problem by miscoding the EFUSE
>> > bit that indicates which connector is actually in use. As we have no means
>> > that I know about to detect which boxes have the problem, a module parameter
>> > was created, just as in this case.
>> >
>> > I agree that drivers should work "out of the box", but finite resources and
>> > lack of vendor cooperation make this a goal that may not be attainable.
>>
>> As you touched on, the ideal situation is that Realtek solve the
>> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from
>> the commit log. The context is that the r8822 driver fails on several
>> platforms unless setting aspm=0 (the default is 1).
>
> It's hard to have all laptop or motherboards and all rtl8822be modules in my side,
> so what I can do is to analyze the issue when user encountered.
>
>>
>> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba
>> https://bugzilla.kernel.org/show_bug.cgi?id=199651
>>
>>
>> If we don't get a timely fix from Realtek though, I think there is a
>> key difference between the antenna selection headache and this one. In
>> the antenna case, there isn't a good value that you can set that will
>> work on all systems. If you change the default behaviour you will
>> solve the issue for some users while simultanously introducing the
>> problem on other systems that were previously fine.
>>
>> However in this case, it's highly likely that setting aspm=0 (off) by
>> default would work for everyone. It has the disadvantage of using a
>> bit more power, but especially with the indications that this issue
>> affects a significant number of systems, I think that having the
>> driver working out of the box everywhere is more important. The module
>> parameter can be left in place so that unaffected users that want to
>> save power can set aspm=1.
>>
>
> I think this issue may be due to L1 latency, so below patch would be
> helpful but not sure because I don't have the same laptop.
> Is there anyone can help to test?

I do not know why I cannot apply this patch directly.  So, I apply
each line manually.

However, here is a good news.  I tested this fix, and it works
correctly on ASUS X530UN.  System can scan and get the access points
list.  Finally, it also connects to the access point.
Maybe, you can have a formal patch for this.

Thanks,
Jian-Hong Pan

>
> diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c b/drivers/staging/rtlwifi/rtl8822be/hw.c
> index 7947edb239a1..88ba5b2fea6a 100644
> --- a/drivers/staging/rtlwifi/rtl8822be/hw.c
> +++ b/drivers/staging/rtlwifi/rtl8822be/hw.c
> @@ -803,7 +803,7 @@ static void _rtl8822be_enable_aspm_back_door(struct ieee80211_hw *hw)
>                 return;
>
>         pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp);
> -       pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7));
> +       pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3);
>
>         pci_read_config_byte(rtlpci->pdev, 0x719, &tmp);
>         pci_write_config_byte(rtlpci->pdev, 0x719, tmp | BIT(3) | BIT(4));
> diff --git a/drivers/staging/rtlwifi/wifi.h b/drivers/staging/rtlwifi/wifi.h
> index 012fb618840b..a45f0eb69d3f 100644
> --- a/drivers/staging/rtlwifi/wifi.h
> +++ b/drivers/staging/rtlwifi/wifi.h
> @@ -88,6 +88,7 @@
>  #define RTL_USB_MAX_RX_COUNT                   100
>  #define QBSS_LOAD_SIZE                         5
>  #define MAX_WMMELE_LENGTH                      64
> +#define ASPM_L1_LATENCY                                7
>
>  #define TOTAL_CAM_ENTRY                                32
>
>
> ---
> PK
>

Dan Carpenter July 5, 2018, 9:32 a.m. UTC | #2

On Thu, Jul 05, 2018 at 05:26:28PM +0800, Jian-Hong Pan wrote:
> 2018-07-05 15:36 GMT+08:00 Pkshih <[e-mail]>:
> > On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:
> >> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:
> >> > We will have to agree to disagree.
> >> >
> >> > I have no idea what the vendors are doing that cause some motherboards to
> >> > need a different aspm value. What I do know is that we have had to live with
> >> > the idiocy of some vendors saving a few pennies by only including a single
> >> > antenna, rather than two, and then making a problem by miscoding the EFUSE
> >> > bit that indicates which connector is actually in use. As we have no means
> >> > that I know about to detect which boxes have the problem, a module parameter
> >> > was created, just as in this case.
> >> >
> >> > I agree that drivers should work "out of the box", but finite resources and
> >> > lack of vendor cooperation make this a goal that may not be attainable.
> >>
> >> As you touched on, the ideal situation is that Realtek solve the
> >> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from
> >> the commit log. The context is that the r8822 driver fails on several
> >> platforms unless setting aspm=0 (the default is 1).
> >
> > It's hard to have all laptop or motherboards and all rtl8822be modules in my side,
> > so what I can do is to analyze the issue when user encountered.
> >
> >>
> >> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba
> >> https://bugzilla.kernel.org/show_bug.cgi?id=199651
> >>
> >>
> >> If we don't get a timely fix from Realtek though, I think there is a
> >> key difference between the antenna selection headache and this one. In
> >> the antenna case, there isn't a good value that you can set that will
> >> work on all systems. If you change the default behaviour you will
> >> solve the issue for some users while simultanously introducing the
> >> problem on other systems that were previously fine.
> >>
> >> However in this case, it's highly likely that setting aspm=0 (off) by
> >> default would work for everyone. It has the disadvantage of using a
> >> bit more power, but especially with the indications that this issue
> >> affects a significant number of systems, I think that having the
> >> driver working out of the box everywhere is more important. The module
> >> parameter can be left in place so that unaffected users that want to
> >> save power can set aspm=1.
> >>
> >
> > I think this issue may be due to L1 latency, so below patch would be
> > helpful but not sure because I don't have the same laptop.
> > Is there anyone can help to test?
> 
> I do not know why I cannot apply this patch directly.  So, I apply
> each line manually.
> 
> However, here is a good news.  I tested this fix, and it works
> correctly on ASUS X530UN.  System can scan and get the access points
> list.  Finally, it also connects to the access point.
> Maybe, you can have a formal patch for this.
>

That's good news.  What does the "dmesg | grep r8822be" say now?

regards,
dan carpenter

Jian-Hong Pan July 5, 2018, 9:57 a.m. UTC | #3

2018-07-05 17:32 GMT+08:00 Dan Carpenter <[e-mail]>:
> On Thu, Jul 05, 2018 at 05:26:28PM +0800, Jian-Hong Pan wrote:
>> 2018-07-05 15:36 GMT+08:00 Pkshih <[e-mail]>:
>> > On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:
>> >> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:
>> >> > We will have to agree to disagree.
>> >> >
>> >> > I have no idea what the vendors are doing that cause some motherboards to
>> >> > need a different aspm value. What I do know is that we have had to live with
>> >> > the idiocy of some vendors saving a few pennies by only including a single
>> >> > antenna, rather than two, and then making a problem by miscoding the EFUSE
>> >> > bit that indicates which connector is actually in use. As we have no means
>> >> > that I know about to detect which boxes have the problem, a module parameter
>> >> > was created, just as in this case.
>> >> >
>> >> > I agree that drivers should work "out of the box", but finite resources and
>> >> > lack of vendor cooperation make this a goal that may not be attainable.
>> >>
>> >> As you touched on, the ideal situation is that Realtek solve the
>> >> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from
>> >> the commit log. The context is that the r8822 driver fails on several
>> >> platforms unless setting aspm=0 (the default is 1).
>> >
>> > It's hard to have all laptop or motherboards and all rtl8822be modules in my side,
>> > so what I can do is to analyze the issue when user encountered.
>> >
>> >>
>> >> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba
>> >> https://bugzilla.kernel.org/show_bug.cgi?id=199651
>> >>
>> >>
>> >> If we don't get a timely fix from Realtek though, I think there is a
>> >> key difference between the antenna selection headache and this one. In
>> >> the antenna case, there isn't a good value that you can set that will
>> >> work on all systems. If you change the default behaviour you will
>> >> solve the issue for some users while simultanously introducing the
>> >> problem on other systems that were previously fine.
>> >>
>> >> However in this case, it's highly likely that setting aspm=0 (off) by
>> >> default would work for everyone. It has the disadvantage of using a
>> >> bit more power, but especially with the indications that this issue
>> >> affects a significant number of systems, I think that having the
>> >> driver working out of the box everywhere is more important. The module
>> >> parameter can be left in place so that unaffected users that want to
>> >> save power can set aspm=1.
>> >>
>> >
>> > I think this issue may be due to L1 latency, so below patch would be
>> > helpful but not sure because I don't have the same laptop.
>> > Is there anyone can help to test?
>>
>> I do not know why I cannot apply this patch directly.  So, I apply
>> each line manually.
>>
>> However, here is a good news.  I tested this fix, and it works
>> correctly on ASUS X530UN.  System can scan and get the access points
>> list.  Finally, it also connects to the access point.
>> Maybe, you can have a formal patch for this.
>>
>
> That's good news.  What does the "dmesg | grep r8822be" say now?

dev@endless:~/linux-eos$ dmesg | grep r8822be
[    8.538404] r8822be: module is from the staging directory, the
quality is unknown, you have been warned.
[    8.541706] r8822be 0000:02:00.0: enabling device (0000 -> 0003)
[    8.571568] r8822be: Using firmware rtlwifi/rtl8822befw.bin
[    8.572370] r8822be: rtlwifi: wireless switch is on
[    8.573338] r8822be 0000:02:00.0 wlp2s0: renamed from wlan0
[   11.346560] Modules linked in: nouveau(+) snd_hda_codec_hdmi
snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel mxm_wmi
snd_hda_codec r8822be(C) x86_pkg_temp_thermal snd_hwdep snd_hda_core
efivarfs i915

The warning message comes from nouveau.  That is no matter.

Here is the full dmesg
https://gist.github.com/starnight/29b1ead5659868fcfb7f172e51a96535#file-gistfile1-txt

Regards,
Jian-Hong Pan

Larry Finger July 5, 2018, 5:06 p.m. UTC | #4

On 07/05/2018 02:36 AM, Pkshih wrote:
> On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:
>> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:
>>> We will have to agree to disagree.
>>>
>>> I have no idea what the vendors are doing that cause some motherboards to
>>> need a different aspm value. What I do know is that we have had to live with
>>> the idiocy of some vendors saving a few pennies by only including a single
>>> antenna, rather than two, and then making a problem by miscoding the EFUSE
>>> bit that indicates which connector is actually in use. As we have no means
>>> that I know about to detect which boxes have the problem, a module parameter
>>> was created, just as in this case.
>>>
>>> I agree that drivers should work "out of the box", but finite resources and
>>> lack of vendor cooperation make this a goal that may not be attainable.
>>
>> As you touched on, the ideal situation is that Realtek solve the
>> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from
>> the commit log. The context is that the r8822 driver fails on several
>> platforms unless setting aspm=0 (the default is 1).
> 
> It's hard to have all laptop or motherboards and all rtl8822be modules in my side,
> so what I can do is to analyze the issue when user encountered.
> 
>>
>> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba
>> https://bugzilla.kernel.org/show_bug.cgi?id=199651
>>
>>
>> If we don't get a timely fix from Realtek though, I think there is a
>> key difference between the antenna selection headache and this one. In
>> the antenna case, there isn't a good value that you can set that will
>> work on all systems. If you change the default behaviour you will
>> solve the issue for some users while simultanously introducing the
>> problem on other systems that were previously fine.
>>
>> However in this case, it's highly likely that setting aspm=0 (off) by
>> default would work for everyone. It has the disadvantage of using a
>> bit more power, but especially with the indications that this issue
>> affects a significant number of systems, I think that having the
>> driver working out of the box everywhere is more important. The module
>> parameter can be left in place so that unaffected users that want to
>> save power can set aspm=1.
>>
> 
> I think this issue may be due to L1 latency, so below patch would be
> helpful but not sure because I don't have the same laptop.
> Is there anyone can help to test?
> 
> 
> diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c b/drivers/staging/rtlwifi/rtl8822be/hw.c
> index 7947edb239a1..88ba5b2fea6a 100644
> --- a/drivers/staging/rtlwifi/rtl8822be/hw.c
> +++ b/drivers/staging/rtlwifi/rtl8822be/hw.c
> @@ -803,7 +803,7 @@ static void _rtl8822be_enable_aspm_back_door(struct ieee80211_hw *hw)
>   		return;
>   
>   	pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp);
> -	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7));
> +	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3);

This patch loses the BIT(7). Did you really mean to do that?

I now agree that this is a bug. A similar problem had been found in a few boxes 
with RTL8723BE or RTL8821AE cards, but that it might apply here completely 
slipped through the ever larger cracks in my mind.

Larry

Ping-Ke Shih July 6, 2018, 3:34 a.m. UTC | #5

On Thu, 2018-07-05 at 12:06 -0500, Larry Finger wrote:
> On 07/05/2018 02:36 AM, Pkshih wrote:

> > On Wed, 2018-07-04 at 10:33 -0500, Daniel Drake wrote:

> >> On Wed, Jul 4, 2018 at 10:13 AM, Larry Finger <[e-mail]> wrote:

> >>> We will have to agree to disagree.

> >>>

> >>> I have no idea what the vendors are doing that cause some motherboards to

> >>> need a different aspm value. What I do know is that we have had to live with

> >>> the idiocy of some vendors saving a few pennies by only including a single

> >>> antenna, rather than two, and then making a problem by miscoding the EFUSE

> >>> bit that indicates which connector is actually in use. As we have no means

> >>> that I know about to detect which boxes have the problem, a module parameter

> >>> was created, just as in this case.

> >>>

> >>> I agree that drivers should work "out of the box", but finite resources and

> >>> lack of vendor cooperation make this a goal that may not be attainable.

> >>

> >> As you touched on, the ideal situation is that Realtek solve the

> >> issue. Ping-Ke Shih is on CC and I am adding a few more contacts from

> >> the commit log. The context is that the r8822 driver fails on several

> >> platforms unless setting aspm=0 (the default is 1).

> > 

> > It's hard to have all laptop or motherboards and all rtl8822be modules in my side,

> > so what I can do is to analyze the issue when user encountered.

> > 

> >>

> >> https://gist.github.com/dsd/20c05f0c6d66ee2ef9bfbb17f93f18ba

> >> https://bugzilla.kernel.org/show_bug.cgi?id=199651

> >>

> >>

> >> If we don't get a timely fix from Realtek though, I think there is a

> >> key difference between the antenna selection headache and this one. In

> >> the antenna case, there isn't a good value that you can set that will

> >> work on all systems. If you change the default behaviour you will

> >> solve the issue for some users while simultanously introducing the

> >> problem on other systems that were previously fine.

> >>

> >> However in this case, it's highly likely that setting aspm=0 (off) by

> >> default would work for everyone. It has the disadvantage of using a

> >> bit more power, but especially with the indications that this issue

> >> affects a significant number of systems, I think that having the

> >> driver working out of the box everywhere is more important. The module

> >> parameter can be left in place so that unaffected users that want to

> >> save power can set aspm=1.

> >>

> > 

> > I think this issue may be due to L1 latency, so below patch would be

> > helpful but not sure because I don't have the same laptop.

> > Is there anyone can help to test?

> > 

> > 

> > diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c b/drivers/staging/rtlwifi/rtl8822be/hw.c

> > index 7947edb239a1..88ba5b2fea6a 100644

> > --- a/drivers/staging/rtlwifi/rtl8822be/hw.c

> > +++ b/drivers/staging/rtlwifi/rtl8822be/hw.c

> > @@ -803,7 +803,7 @@ static void _rtl8822be_enable_aspm_back_door(struct ieee80211_hw *hw)

> >   		return;

> >   

> >   	pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp);

> > -	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7));

> > +	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3);

> 

> This patch loses the BIT(7). Did you really mean to do that?

> 

> I now agree that this is a bug. A similar problem had been found in a few boxes 

> with RTL8723BE or RTL8821AE cards, but that it might apply here completely 

> slipped through the ever larger cracks in my mind.

> 


Yes, I remove BIT(7) intentionally, because this is defined as reserved bit in rtl8822be.
For the older chips, I need to check with my colleagues.

Regards,
PK

Patch

diff --git a/drivers/staging/rtlwifi/rtl8822be/hw.c b/drivers/staging/rtlwifi/rtl8822be/hw.c
index 7947edb239a1..88ba5b2fea6a 100644
--- a/drivers/staging/rtlwifi/rtl8822be/hw.c
+++ b/drivers/staging/rtlwifi/rtl8822be/hw.c
@@ -803,7 +803,7 @@  static void _rtl8822be_enable_aspm_back_door(struct ieee80211_hw *hw)
 		return;
 
 	pci_read_config_byte(rtlpci->pdev, 0x70f, &tmp);
-	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | BIT(7));
+	pci_write_config_byte(rtlpci->pdev, 0x70f, tmp | ASPM_L1_LATENCY << 3);
 
 	pci_read_config_byte(rtlpci->pdev, 0x719, &tmp);
 	pci_write_config_byte(rtlpci->pdev, 0x719, tmp | BIT(3) | BIT(4));
diff --git a/drivers/staging/rtlwifi/wifi.h b/drivers/staging/rtlwifi/wifi.h
index 012fb618840b..a45f0eb69d3f 100644
--- a/drivers/staging/rtlwifi/wifi.h
+++ b/drivers/staging/rtlwifi/wifi.h
@@ -88,6 +88,7 @@ 
 #define RTL_USB_MAX_RX_COUNT			100
 #define QBSS_LOAD_SIZE				5
 #define MAX_WMMELE_LENGTH			64
+#define ASPM_L1_LATENCY				7
 
 #define TOTAL_CAM_ENTRY				32