On Mon, 2015-04-20 at 18:40 +0800, Ian Kent wrote: > On Mon, 2015-04-20 at 18:26 +0800, Ian Kent wrote: > > 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<ar...@broadcom.com> > > > >> To: Rafał Miłecki<zaj...@gmail.com> > > > >> CC: Hante Meuleman<meule...@broadcom.com>, > > > >> "linux-wirel...@vger.kernel.org" > > > >> <linux-wirel...@vger.kernel.org>, Kalle > > > >> Valo<kv...@codeaurora.org>, mailinglist > > > >> <openwrt-devel@lists.openwrt.org>, Florian Fainelli > > > >> <faine...@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<meule...@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 > > this. > > > > > > > > > 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?
In answer to my own question, yes, my change will confuse the parser by the look of the patch code, oops! So I don't actually know if the changes work OK or not. > > And since I didn't actually answer the question, yes I did try the > patch(es). > > Currently I have several patches (which are probably meaningless to you > but the last two are the brcmfmac patches I scrounged from mailing list > archives to add the id and parse the nvram settings file): > [raven@perseus openwrt.git]$ stg ser > + bcm53xx-update-sprom-from-nvram-to-handle-rev-11.patch > + bcm53xx-deal-with-R8000-mac-address-settings.patch > + bcm53xx-R8000-handle-PEX8603-switch.patch > + bcm53xx-brcmfmac-add-43602-nvram-settings-file.patch Actually this adds my broken nvram settings file. > > brcmfmac-update.patch And this is a patch of the two patches to add the id and the nvram parser. Ian _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel