[linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property
From: Hans de Goede <hidden>
Date: 2016-06-30 10:25:15
Also in:
linux-devicetree, linux-wireless
Hi, On 30-06-16 12:18, Jonas Gorski wrote:
Hi, On 30 June 2016 at 12:04, Hans de Goede [off-list ref] wrote:quoted
Hi, On 30-06-16 11:58, Kalle Valo wrote:quoted
Hans de Goede [off-list ref] writes:quoted
Hi, On 30-06-16 11:02, Kalle Valo wrote:quoted
Priit Laes [off-list ref] writes:quoted
quoted
What is the size of this nvram file? As it's board specific, I wonder if we can simply include it inside of the DT verbatim. I remember doing that (in the pre-dtb days, on real open firmware) for the "spidernet" ethernet driver.It contains a bit too much info: This is what CubieTruck requires: http://dl.cubieboard.org/public/Cubieboard/benn/firmware/ap6210/nvram_ap6210.txtIn the nvram file I see lots of identifiers: manfid=0x2d0 prodid=0x492 vendid=0x14e4 devid=0x4343 boardtype=0x0598 boardrev=0x1307 boardnum=777 Are any of these (or a combination of them) unique for each board type? What if one of these is provided through DT and then the driver could choose the nvram file based on the board id? Just another idea...That would require updating a table in the driver every time a new board comes out, I do not believe that that is a good solution.You don't necessarily need to have a table in the driver if you embed the id to the filename, for example something like this: nvram-0598-1307.txt nvram-<boardtype>-<boardrev>.txtDownside of this is, that as Arend already said, the nvram files are not readily available. Typically the boards we are talking about are shipped with android, and the mainline kernel bringup is done by a user, not the manufacturer. So the nvram file needs to be extracted from e.g. an android image, having ap6210 in the filename allows the user to see that his module (which is clearly labelled ap6210 is already supported, where as the boardtype/boardrev magic numbers are a big unknown. So I believe that using a human readable string is better here.So then how about making use of a more specific compatible string? e.g. brcmf { compatible = "foo,ap6210", "brcm,bcm4329-fmac"; ... }; and if the compatible has more than one element you request FW_NAME_<compatible>.txt as the nvram file. Or try each comptible (and lastly no suffix) until you get a match. (AFAICT, this is what the "model" property was originally intended for anyway, but almost nobody did it right, and everyone put a user readable string into "model" for boards instead of the ePAPR defined compatible string).
Hmm, interesting idea. Not sure how easy / hard it will be to implement this, but from a dt binding point of view it seems elegant. Kalle, Arend, what do you think of this ? Regards, Hans