[OpenWrt-Devel] Fwd: Re: [PATCH 10/10] brcmfmac: Add support for multiple PCIE devices in nvram.

Ian Kent raven at themaw.net
Mon Apr 20 06:26:40 EDT 2015

On Mon, 2015-04-20 at 11:27 +0200, Arend van Spriel wrote:
> On 04/20/15 03:00, Ian Kent wrote:
> > On Fri, 2015-04-17 at 10:55 +0200, Arend van Spriel wrote:
> >> Resend as it bounced on openwrt-devel list.
> >>
> >> -------- Original Message --------
> >> Subject: Re: [PATCH 10/10] brcmfmac: Add support for multiple PCIE
> >> devices in nvram.
> >> Date: Fri, 17 Apr 2015 10:50:08 +0200
> >> From: Arend van Spriel<arend at broadcom.com>
> >> To: Rafał Miłecki<zajec5 at gmail.com>
> >> CC: Hante Meuleman<meuleman at broadcom.com>,
> >> "linux-wireless at vger.kernel.org"	<linux-wireless at vger.kernel.org>, Kalle
> >> Valo<kvalo at codeaurora.org>,	mailinglist
> >> <openwrt-devel at lists.openwrt.org>, Florian Fainelli	<fainelli at broadcom.com>
> >>
> >> + openwrt-devel list
> >>
> >> On 04/17/15 09:45, Rafał Miłecki wrote:
> >>> Huh, why dropping linux-wireless (and top posting btw)? Please let
> >>> everyone follow the discussion :)
> >>>
> >>> On 15 April 2015 at 21:20, Hante Meuleman<meuleman at broadcom.com>   wrote:
> >>>> As I wrote to you in a mail and on the openwrt forum, this patch is indeed an attempt to support more complex nvram files. I also wrote, that in order to be able to use it, the nvram contents of the device (r8000) needs tobe put a specific file. Now for your concerns, we can perhaps add something which will read the nvram contents directly from an nvram store. But thatis irrelevant to this patch. The parsing is still needed, and all we wouldneed to add is something which is reading the nvram contents from some other place
> >>>
> >>> So it makes me wonder if we need this patch in its current form. I
> >>> think getting NVRAM directly from the platform is much user friendly.
> >>> It doesn't require user to install some extra tools for dumping NVRAM
> >>> and putting it in a specific file. One extra layer less.
> >>> With that said I think it's hard to review your code for parsing
> >>> NVRAM. We don't know how it's going to be fetched in the first place.
> >>
> >> You already made that point and we agreed to look for a solution.
> >>
> >>>> though it would have to be put under some kernel config flag as this would not be supported in non-router systems. The contents of the nvram would however still need to be parsed in exactly the same way as the nvram files we read from disk.
> >>>
> >>> Again, it's hard to say for me. Are you going to use
> >>> bcm47xx_nvram_getenv? Are you going to use MTD subsystem? Are you
> >>> going to develop different solution? When using e.g.
> >>> bcm47xx_nvram_getenv you won't want all this parsing stuff at all.
> >>
> >> Please look at the usage scenario here. The brcmfmac driver is not
> >> needing a few key,value pairs. It needs a portion of NVRAM to download
> >> to the device. The patch provides the functionality to do just that. Get
> >> the appropriate portion, strip comments and whitespace, and send it to
> >> the device. Using bcm47xx_nvram_getenv is not a useful api as it would
> >> mean we need brcmfmac to know which key ids to ask for, reassemble it to
> >> key=value string and send it to the fullmac device.
> >
> > Following an "nvram erase" none of the needed<key, value>  pairs remain
> > in nvram. So we probably can't use nvram in a reliable way to create the
> > wireless configuration.
> So why do "nvram erase"? The assumption that it is not needed because 
> you are running an OpenWRT image is wrong or at least only partially 
> true, ie. for the user-space settings.

I'm saying that this does happen, and could break things when it does.

Perhaps what I say isn't quite right now since I haven't tried going
from Vendor firmware to OpenWRT for quite a long time now and things
have changed somewhat.

Nevertheless people will do an "nvram erase" and possibly get into
trouble and, even without doing an "nvram erase", I've observed that the
nvram content is significantly less than seen on a Vendor install. Just
exactly what that means will need further testing, maybe the wireless
nvram values will remain. But I can say that they do remain after an
initial OpenWRT install from Vendor firmware.

More detail from me on that on that will have to wait for future OpenWRT
installs going from Vendor firmware to OpenWRT and upgrades of OpenWRT
I'm afraid.

But it does worry me a bit based on my experience so far.

> > But there's no information about what the (I guess) device firmware
> > actually needs.
> There is because those keys have a prefix.

Umm, your assuming the nvram values exist, and I'm saying there's every
chance they won't (but more information is needed). That's what I'm
concerned about. Again maybe I'm not quite right, time will tell!

> > Is there a list of key value pairs used/needed?
> > What are there default values?
> > Are sensible default values used for key value pairs that are not
> > present in the configuration?
> Not on R8000. Almost all configuration has to come from nvram.

Yeah, but I don't understand the reason for that, as I say below here.

> > Point is there should be only a few entries needed in a configuration to
> > alter some specific default values for a sane implementation.
> Why?

Because, such configurations should be largely the same between devices
and only a few name value pairs should need to change. This approach
would make these configurations readable, understandable and relatively
easy to customize (assuming the base configuration was commented
somewhat) by firmware or human.

> > Why use pcie domain and bus number?
> > What do you get from hard coding things that might change over time with
> > implementation?
> What implementation do you have in mind here.

Not sure, but I didn't see the devpathX entries that are in fact present
in the original nvram dump of my device so maybe I'll back pedal on
> > The nvram from a vendor install doesn't use pcie domain and bus number,
> > it uses "0:", "1:" and "2:" prefixes, and as much as I don't like that
> > either, it is implementation independent.
> This is explained in the patch as compressed format. The nvram has a 
> couple of devpathX keys that provide mapping of those prefixes to pcie 
> domain and bus number.

And, due to the use of the bcma subsystem within OpenWRT these aren't
the same. Perhaps (probably) they can be mapped reliably from the pseudo
pcie domain and bus of the Vendor firmware to what is seen on OpenWRT.

> > Knowing more about what is really needed and how it is handled (for
> > which there is no information whatsoever that I can find) might help me
> > understand why the driver doesn't work on my R8000.
> >
> > Perhaps that's a bit harsh, the driver does work partially.
> >
> > Extracting each prefixed section and replacing the prefix with<pcie
> > domain>/<bus>/ doesn't seem to make any difference. The driver still
> > insists these devices are 2.4Ghz only and barfs at any 5Ghz
> > configuration.
> So did you try this patch. If so there should be no need to replace 
> things. Just dump the entire nvram content and put it in a file so 
> brcmfmac can request it through the firmware api.

OK, fair call, I didn't read the patch thoroughly, I just skimmed it,
and probably should have, since I replaced the "<n>:" with the
appropriate <domain>/<bus>/ prefix and that possibly broke things.

I'm not in a position to try again for a while as I'm giving the new
Netgear Dynamic QoS a try, but when I swap the main router (currently
the R8000) out with another I'll give it another try.

Do you think my change above would have confused the brcmfmac driver
nvram parser?

> Regards,
> Arend
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org

More information about the openwrt-devel mailing list