On Thu, Sep 09, 2021 at 10:39:58AM +0200, Soeren Moch wrote:

Hi Shawn,

On 09.09.21 04:20, Shawn Guo wrote:

quoted

On Wed, Sep 08, 2021 at 07:08:06AM +0200, Soeren Moch wrote:

quoted

Hi Shawn,

On 08.09.21 03:00, Shawn Guo wrote:

quoted

Hi Soeren,

On Tue, Sep 07, 2021 at 09:22:52PM +0200, Soeren Moch wrote:

quoted

On 25.04.21 13:02, Shawn Guo wrote:

quoted

Instead of aborting country code setup in firmware, use ISO3166 country
code and 0 rev as fallback, when country_codes mapping table is not
configured.  This fallback saves the country_codes table setup for recent
brcmfmac chipsets/firmwares, which just use ISO3166 code and require no
revision number.

This patch breaks wireless support on RockPro64. At least the access
point is not usable, station mode not tested.

brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM4359/9 wl0: Mar  6 2017
10:16:06 version 9.87.51.7 (r686312) FWID 01-4dcc75d9

Reverting this patch makes the access point show up again with linux-5.14 .

Sorry for breaking your device!

So it sounds like you do not have country_codes configured for your
BCM4359/9 device, while it needs particular `rev` setup for the ccode
you are testing with.  It was "working" likely because you have a static
`ccode` and `regrev` setting in nvram file.

It always has been a mystery to me how country codes are configured for
this device. Before I read your patch I did not even know that a
translation table is required. Is there some documentation how this is
supposed to work? Not sure if this makes a difference, BCM4359/9 is a
Cypress device I think, I added mainline support for it some time ago.

One way to add the translation table is using DT.  You can find more
info and example in following commits:

b41936227078 ("dt-bindings: bcm4329-fmac: add optional brcm,ccode-map")
1a3ac5c651a0 ("brcmfmac: support parse country code map from DT")

OK, thanks.
When one way is to use DT, what is the 'traditional way' to add such table?

To be honest, I don't know what the 'traditional way' is, because I
haven't seen how `country_codes` is used before I add DT support of it.

And maybe the more interesting question, where can these settings be
obtained from? The tweaked device specific settings probably from the
device vendor, good luck!
But the general country specific settings, as you are obviously
interested in with your trivial mapping, shouldn't they go into driver
directly? Only to be overruled when device specific settings are
available via DT? And of course only for device/firmware combinations
that support this general mapping, so that other devices with 'unknown
mapping' are not broken by this enhancement?

The patch was accepted based on Arend's assumption[1] that every
chipset/firmware include a rev 0 for each country code , but
it's never been officially confirmed.  And from what you report here,
it doesn't seem to stand unfortunately.

[1] https://lore.kernel.org/netdev/17998013ac0.279b.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com/ (local)

quoted

quoted

I have installed different firmware files, brcmfmac4359-sdio.clm_blob,
brcmfmac4359-sdio.bin, brcmfmac4359-sdio.txt, the latter also linked as
brcmfmac4359-sdio.pine64,rockpro64-2.1.txt. This probably is the nvram
file. ccode and regrev are set to zero, which probably means
'international save settings".

I'm not sure how this 'international save settings' works for brcmfmac
devices.  Do you have more info or any pointers?

The correct term in this context probably is 'world regulatory domain',
the most restrictive wifi settings that can be used all over the world.
This usually is taken as default by cfg80211, apparently also for
(some?) brcmfmac devices/firmwares.

These 'world' settings can be replaced by more permissive country
specific regulatory domain settings, but for brcmfmac devices this seems
to be firmware specific and requires this country mapping.

I have seen a country code "00" for the world regulatory domain in the
past, not sure if this is standard or a device/driver/software specific
hack and if this can be used for brcmfmac (mapping from string "00" to
country_code=0 ?). For sure here are more experienced wifi developers
who know better.

Yeah, it would be nice if someone can help clarify what both `ccode` and
`regrev` in nvram file being zero means, like what should be working and
what's not.

quoted

quoted

quoted

But roaming to a different
region will mostly get you a broken WiFi support.  Is it possible to set
up the country_codes for your device to get it work properly?

In linux-5.13 it worked, probably with save settings (not all channels
selectable, limited tx power), with linux-5.14 it stopped working, so it
is a regression.
I personally would like to learn how all this is configured properly.
For general use I think save settings are better than no wifi at all
with this patch. This fallback to ISO CC seams to work with newer
(Synaptics?) devices only.

I do not mind you send a reverting if you have problem to add a proper
translation table for your device.  But that would mean I have to add
a pretty "meaningless" translation table for my devices :(

Is this not the usual DT policy, that missing optional properties should
not prevent a device to work, that old dtbs should still work when new
properties are added?

I'm not sure what's the best way forward. A plain revert of this patch
would at least bring back wifi support for RockPro64 devices with
existing dtbs. Maybe someone else has a better proposal how to proceed.

Go ahead to revert if we do not hear a better solution, I would say.

Shawn