[PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Hans de Goede]

In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
or "ccode=XT", to specify "worldwide" compatible settings, but these
ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
to not be available.

ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
with a bit of special handling for nvram settings coming from an EFI
variable. Extend this handling to also deal with nvram settings from
EFI variables which contain "ccode=XT", which has similar issues to
"ccode=XV".

This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.

This was also tested on a Lenovo Thinkpad 8 tablet which also uses
"ccode=XT" and this causes no adverse effects there.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/firmware.c    | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
index 0eb13e5df517..f150af8ede21 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
@@ -432,10 +432,10 @@ struct brcmf_fw {
 };
 
 #ifdef CONFIG_EFI
-/* In some cases the EFI-var stored nvram contains "ccode=ALL" or "ccode=XV"
- * to specify "worldwide" compatible settings, but these 2 ccode-s do not work
- * properly. "ccode=ALL" causes channels 12 and 13 to not be available,
- * "ccode=XV" causes all 5GHz channels to not be available. So we replace both
+/* In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV" or "ccode=XT",
+ * to specify "worldwide" compatible settings, but these ccode-s do not work properly.
+ * "ccode=ALL" causes channels 12 and 13 to not be available, "ccode=XV" / "ccode=XT"
+ * may cause all 5GHz channels to not be available. So we replace these
  * with "ccode=X2" which allows channels 12+13 and 5Ghz channels in
  * no-Initiate-Radiation mode. This means that we will never send on these
  * channels without first having received valid wifi traffic on the channel.
@@ -447,6 +447,8 @@ static void brcmf_fw_fix_efi_nvram_ccode(char *data, unsigned long data_len)
 	ccode = strnstr((char *)data, "ccode=ALL", data_len);
 	if (!ccode)
 		ccode = strnstr((char *)data, "ccode=XV\r", data_len);
+	if (!ccode)
+		ccode = strnstr((char *)data, "ccode=XT\r", data_len);
 	if (!ccode)
 		return;
 
-- 
2.31.1

Re: [PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Kalle Valo]

Hans de Goede <hdegoede@redhat.com> writes:

> In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
> or "ccode=XT", to specify "worldwide" compatible settings, but these
> ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
> not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
> to not be available.
>
> ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
> with a bit of special handling for nvram settings coming from an EFI
> variable. Extend this handling to also deal with nvram settings from
> EFI variables which contain "ccode=XT", which has similar issues to
> "ccode=XV".
>
> This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.
>
> This was also tested on a Lenovo Thinkpad 8 tablet which also uses
> "ccode=XT" and this causes no adverse effects there.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

To me worldwide compatible settings mean that channels 12 and 13 should
be disabled, so I'm quite hesitant about this patch.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Hans de Goede]

Hi Kalle,

On 10/5/21 7:36 AM, Kalle Valo wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
> 
>> In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
>> or "ccode=XT", to specify "worldwide" compatible settings, but these
>> ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
>> not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
>> to not be available.
>>
>> ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
>> with a bit of special handling for nvram settings coming from an EFI
>> variable. Extend this handling to also deal with nvram settings from
>> EFI variables which contain "ccode=XT", which has similar issues to
>> "ccode=XV".
>>
>> This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.
>>
>> This was also tested on a Lenovo Thinkpad 8 tablet which also uses
>> "ccode=XT" and this causes no adverse effects there.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> To me worldwide compatible settings mean that channels 12 and 13 should
> be disabled, so I'm quite hesitant about this patch.

The X2 setting puts channel 12 and 13 in passive / listen-only modes
and only starts using them if there is an AP on them.

AFAIK this is the same with the XT/XV settings. The problem is that the XT
setting results in 5G not being available on some boards even though the
hw supports it.

Also note that we already use the X2 setting for any EFI supplied nvram
files where ccode=ALL or ccode=XV, this just extends the handling we
already have to also patch ccode=XT.

Regards,

Hans

Re: [PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Arend van Spriel]

On Tue, Oct 5, 2021 at 10:02 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kalle,
>
> On 10/5/21 7:36 AM, Kalle Valo wrote:
> > Hans de Goede <hdegoede@redhat.com> writes:
> >
> >> In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
> >> or "ccode=XT", to specify "worldwide" compatible settings, but these
> >> ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
> >> not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
> >> to not be available.
> >>
> >> ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
> >> with a bit of special handling for nvram settings coming from an EFI
> >> variable. Extend this handling to also deal with nvram settings from
> >> EFI variables which contain "ccode=XT", which has similar issues to
> >> "ccode=XV".
> >>
> >> This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.
> >>
> >> This was also tested on a Lenovo Thinkpad 8 tablet which also uses
> >> "ccode=XT" and this causes no adverse effects there.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >
> > To me worldwide compatible settings mean that channels 12 and 13 should
> > be disabled, so I'm quite hesitant about this patch.
>
> The X2 setting puts channel 12 and 13 in passive / listen-only modes
> and only starts using them if there is an AP on them.
>
> AFAIK this is the same with the XT/XV settings. The problem is that the XT
> setting results in 5G not being available on some boards even though the
> hw supports it.
>
> Also note that we already use the X2 setting for any EFI supplied nvram
> files where ccode=ALL or ccode=XV, this just extends the handling we
> already have to also patch ccode=XT.

I am not overly excited about this approach that is already in use.
AFAIK these worldwide codes are tailored for specific
devices/customers based on their RF components. Using it as fallback
for other devices in such a generic way could even result in exceeding
regulatory limits. However, I do not have a better solution for this.
I am surprised to learn there are nvram out there with ccode=ALL as
that is for internal use only, but if these devices has SROM than the
nvram value is ignored. Hopefully, that is the case although given the
fact that changing it to X2 helps suggests otherwise :-(

Regards,
Arend

Re: [PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Kalle Valo]

Arend van Spriel <aspriel@gmail.com> writes:

> On Tue, Oct 5, 2021 at 10:02 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Kalle,
>>
>> On 10/5/21 7:36 AM, Kalle Valo wrote:
>> > Hans de Goede <hdegoede@redhat.com> writes:
>> >
>> >> In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
>> >> or "ccode=XT", to specify "worldwide" compatible settings, but these
>> >> ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
>> >> not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
>> >> to not be available.
>> >>
>> >> ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
>> >> with a bit of special handling for nvram settings coming from an EFI
>> >> variable. Extend this handling to also deal with nvram settings from
>> >> EFI variables which contain "ccode=XT", which has similar issues to
>> >> "ccode=XV".
>> >>
>> >> This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.
>> >>
>> >> This was also tested on a Lenovo Thinkpad 8 tablet which also uses
>> >> "ccode=XT" and this causes no adverse effects there.
>> >>
>> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> >
>> > To me worldwide compatible settings mean that channels 12 and 13 should
>> > be disabled, so I'm quite hesitant about this patch.
>>
>> The X2 setting puts channel 12 and 13 in passive / listen-only modes
>> and only starts using them if there is an AP on them.
>>
>> AFAIK this is the same with the XT/XV settings. The problem is that the XT
>> setting results in 5G not being available on some boards even though the
>> hw supports it.
>>
>> Also note that we already use the X2 setting for any EFI supplied nvram
>> files where ccode=ALL or ccode=XV, this just extends the handling we
>> already have to also patch ccode=XT.
>
> I am not overly excited about this approach that is already in use.
> AFAIK these worldwide codes are tailored for specific
> devices/customers based on their RF components. Using it as fallback
> for other devices in such a generic way could even result in exceeding
> regulatory limits. However, I do not have a better solution for this.
> I am surprised to learn there are nvram out there with ccode=ALL as
> that is for internal use only, but if these devices has SROM than the
> nvram value is ignored. Hopefully, that is the case although given the
> fact that changing it to X2 helps suggests otherwise :-(

What's the conclusion? Should I take this or drop it?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] brcmfmac: firmware: Treat EFI nvram ccode=XT the same as ccode=XV

[Hans de Goede]

Hi,

On 10/5/21 10:37, Arend van Spriel wrote:
> On Tue, Oct 5, 2021 at 10:02 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Kalle,
>>
>> On 10/5/21 7:36 AM, Kalle Valo wrote:
>>> Hans de Goede <hdegoede@redhat.com> writes:
>>>
>>>> In some cases the EFI-var stored nvram contains "ccode=ALL", "ccode=XV"
>>>> or "ccode=XT", to specify "worldwide" compatible settings, but these
>>>> ccode-s do not work properly. "ccode=ALL" causes channels 12 and 13 to
>>>> not be available, "ccode=XV" / "ccode=XT" may cause all 5GHz channels
>>>> to not be available.
>>>>
>>>> ccode="ALL" and ccode="XV" where already being replaced with ccode="X2"
>>>> with a bit of special handling for nvram settings coming from an EFI
>>>> variable. Extend this handling to also deal with nvram settings from
>>>> EFI variables which contain "ccode=XT", which has similar issues to
>>>> "ccode=XV".
>>>>
>>>> This fixes 5GHz wifi not working on the HP ElitePad 1000 G2.
>>>>
>>>> This was also tested on a Lenovo Thinkpad 8 tablet which also uses
>>>> "ccode=XT" and this causes no adverse effects there.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>
>>> To me worldwide compatible settings mean that channels 12 and 13 should
>>> be disabled, so I'm quite hesitant about this patch.
>>
>> The X2 setting puts channel 12 and 13 in passive / listen-only modes
>> and only starts using them if there is an AP on them.
>>
>> AFAIK this is the same with the XT/XV settings. The problem is that the XT
>> setting results in 5G not being available on some boards even though the
>> hw supports it.
>>
>> Also note that we already use the X2 setting for any EFI supplied nvram
>> files where ccode=ALL or ccode=XV, this just extends the handling we
>> already have to also patch ccode=XT.
> 
> I am not overly excited about this approach that is already in use.
> AFAIK these worldwide codes are tailored for specific
> devices/customers based on their RF components. Using it as fallback
> for other devices in such a generic way could even result in exceeding
> regulatory limits. However, I do not have a better solution for this.
> I am surprised to learn there are nvram out there with ccode=ALL as
> that is for internal use only, but if these devices has SROM than the
> nvram value is ignored. Hopefully, that is the case although given the
> fact that changing it to X2 helps suggests otherwise :-(

Ping? How should we move forward with this. Not having working 5Ghz wifi,
where it does work under Windows really is not good. So we need to fix
this one way or the other.

I would prefer to just treat ccode=XT as we do ccode=XV and replace it
with ccode=X2, but alternatively we could also look at the regrev.

ccode=XT it self seems to be fine, it is the combination of ccode=XT with
regrev=29 (IIRC) which is the problem. The Toshiba wt8-a-102 tablet uses
ccode=XT with regrev=13 and that works fine.

I believe that the issue is that the firmware which we are shipping in
Linux does not support regrev=29 and thus fallsback to some safe
defaults which completely excludes 5G bands.

Patching the regrev is going to involve adding some extra nvram
patching code to the kernel; and should probably be limited to
brcmfmac43241b4 with ccode=XT but if that is more acceptable I
would be happy to do this instead.

Regards,

Hans