[linux-sunxi] Re: [PATCH 1/4] brcmfmac: Add brcm, nvram_file_name dt property

[Hans de Goede]

Hi,

On 29-06-16 20:51, 'Arend Van Spriel' via linux-sunxi wrote:
> On 29-6-2016 20:01, Hans de Goede wrote:
>> Hi,
>>
>> On 29-06-16 19:00, Kalle Valo wrote:
>>> Hans de Goede <hdegoede at redhat.com> writes:
>>>
>>>> Hi,
>>>>
>>>> On 29-06-16 16:42, Jonas Gorski wrote:
>>>>> Hi,
>>>>>
>>>>> On 29 June 2016 at 16:04, Hans de Goede <hdegoede at redhat.com> wrote:
>>>>>> Add a brcm,nvram_file_name dt property to allow overruling the default
>>>>>> nvram filename for sdio devices. The idea is that we can specify a
>>>>>> board specific nvram file, e.g. brcmfmac43362-ap6210.txt for boards
>>>>>> with an ap6210 wifi sdio module and ship this in linux-firmware, so
>>>>>> that wifi will work out of the box, without requiring users to find
>>>>>> and then manually install the right nvram file for their board.
>>>>>
>>>>> Directly defining a filename doesn't seem like a good OS-agnostic
>>>>> approach. Maybe an alternative would be to add a model-property to the
>>>>> nodes (this is allowed) and make brcmfmac to request
>>>>> "FWFILENAME-<model>" as firmware if set? That would leave it to the OS
>>>>> on how the filename is set.
>>>>
>>>> It only defines the base-filename, not the entire path, how / where
>>>> this file is searched for / loaded-from is then left up to the os
>>>
>>> It's still a bad idea. The filename, including the path, should be
>>> created in the driver. Can't you provide chipname (or similar) via
>>> device tree and then the driver can choose what image to use?
>>
>> No, the driver already does that, but this is not ...
>>
>>> Can you tell more about the naming the firmware image, how does it work
>>> exactly?
>>
>> About firmware, this is about the nvram file which is board specific,
>> rather then chip specific.
>
> The nvram filename is determined pretty much the same as the firmware
> filename, but indeed the nvram data is board specific. This is main
> reason why we do not submit nvram files to linux-firmware, because we
> simply do not have those files.
>
>> Typical wifi devices will have some sort of non volatile storage
>> on board to not only store the ethernet(mac) address, but also
>> to contain e.g. info about the antenna gain so that the firmware
>> and/or the driver can take the antenna gain into account and ensure
>> that they never exceed the maximum allowed broadcast strength.
>>
>> However on some embedded devices there is no non-volatile storage
>> for the wifi (for cost reasons) and instead this configuration info
>> (which is board / pcb specific) is loaded in the form of a
>> file which contains the contents which would normally be in the
>> non-volatile storage.
>>
>> Since we are dealing with a per-board config-file here, which is
>> loaded from the os filesystem we really need to specify a basename
>> here as the list of possible boards is endless, so we cannot
>> have a lookup table in the driver.
>
> As Jonas mentioned the general principle of device tree is to be
> agnostic with regards to OS and/or driver as you undoubtedly know. His
> proposal seems like a usable solution for your problem while complying
> to the device tree principle. So instead of overriding the default
> brcmfmac should modify it when dt specifies the "module" property, ie:
>
> no "module" in DT:	nvram filename = brcm/brcmfmac43362-sdio.txt
> "module=ap6210" in DT:	nvram filename = brcm/brcmfmac43362-ap6210.txt

Yes that seems like a good solution, I will send a v2 implementing this.

> By the way, the example in the bindings file does not seem to specify a
> basename, but path+basename+fileext.

fileext is always part of a file basename, but you're right that it
does include a relative path because of the way the existing firmware
files are organized under /lib/firmware under Linux and yes I'll admit
that that is a bit os specific :)

Regards,

Hans