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

Reply via email to