[GIT PULL 1/3] Keystone SOC driver updates for 4.4

[santosh shilimkar]

10/9/2015 8:21 AM, Arnd Bergmann wrote:
> On Friday 09 October 2015 15:09:56 Karicheri, Muralidharan wrote:
>>> Normally, the firmware name is fixed in the driver. If the API changes in order to support a
>>> new feature, the driver of course needs to be aware of that, but it should not require a device
>>> tree change to update the file name.
>>>
>>> Conversely, if you get a new chip that needs a slightly different blob but has an identical API,
>>> the driver should ideally not need to be changed, but still see  a new file name.
>>>
>>> The first can be done once you need it, e.g. by appending the number of the API version to
>>> the file name inside of the driver, and trying the highest version supported by the driver first,
>>> before falling back to older version in reverse order until the oldest version that is supported
>>> by the driver.
>>>
>> What I gather is the firmware file name is to be part of the driver itself instead of adding the
>> same to DT. This way as firmware API change, additional filename strings can be added and handled
>> as needed. As API is not expected to change that often, this change will be needed very rarely.
>
> Right.
>
>> But how to deal with the case where firmware blob changes, but no API changes which is mostly
>> the case for keystone. For example currently we have 2.0.0, and 2.0.1 is available, but no API
>> change. Probably I could name the file as file_2.0.x in driver and as long as there is no API change,
>> one could provide a soft link in file system to point to different minor versions.
>>
>> ln -s /lib/firmware/file_2.0.1 /lib/firmware/file_2.0.x
>>
>> In the case of qmss firmware, it will be named as ks2_qmss_pdsp_acc48_k2_le_1_0_0_x.bin in the
>> driver source code as per the above suggestion. In the file system, there will be a soft link to
>> ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin.
>> ln -s /lib/firmware/ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin /lib/firmware/ ks2_qmss_pdsp_acc48_k2_le_1_0_0_x.bin
>>
>> Is this acceptable? or do you see any issue with this approach?
>
> I don't know what the policy is for the linux-firmware repository. My
> first thought would have been that you only ever put the latest version
> into that tree and never change the name. Symlinks can obviously work
> as well, and I see some cases of this in my /lib/firmware directory:
>
> $ find /lib/firmware/ -type l | xargs ls -l
> find: `/lib/firmware/b43': Permission denied
> lrwxrwxrwx 1 root root 16 Jan 31  2014 /lib/firmware/cxgb4/t4fw.bin -> t4fw-1.6.2.0.bin
> lrwxrwxrwx 1 root root 18 Jan 31  2014 /lib/firmware/libertas/sd8688.bin -> ../mrvl/sd8688.bin
> lrwxrwxrwx 1 root root 25 Jan 31  2014 /lib/firmware/libertas/sd8688_helper.bin -> ../mrvl/sd8688_helper.bin
> lrwxrwxrwx 1 root root 10 Jan 31  2014 /lib/firmware/rt3070.bin -> rt2870.bin
> lrwxrwxrwx 1 root root 10 Jan 31  2014 /lib/firmware/rt3090.bin -> rt2860.bin
> lrwxrwxrwx 1 root root 17 Jan 31  2014 /lib/firmware/s2250.fw -> go7007/s2250-2.fw
> lrwxrwxrwx 1 root root 17 Jan 31  2014 /lib/firmware/s2250_loader.fw -> go7007/s2250-1.fw
> lrwxrwxrwx 1 root root 14 Jan 31  2014 /lib/firmware/ti-connectivity/wl1271-nvs.bin -> wl127x-nvs.bin
> lrwxrwxrwx 1 root root 14 Jan 31  2014 /lib/firmware/ti-connectivity/wl12xx-nvs.bin -> wl127x-nvs.bin
>
>>> The second one basically depends on the "compatible" string of the device, which identifies
>>> which device you have. The driver can then look up the file name for each device it supports
>>> based on this string, and by using multiple compatible strings in DT, you can provide the
>>> specific version of the hardware that is used for the file name without having to match that
>>> hardware name in the driver
>>>
>>
>> This make sense.
>>
>>>> W.r.t to the patch for documentation update, can I send an incremental
>>>> patch to the update the linux-firmware.git reference as well?
>>>
>>> I'd rather skip that documentation change until we have decided on how to handle the
>>> firmware loading in the future.
>>
>> So if the above is acceptable, I will add the ks2_qmss_pdsp_acc48_k2_le_1_0_0_x.bin name to
>> the Kconfig description as you have proposed.
>
> Maybe something shorter for the symlink? I don't know what all the parts of the
> name mean, but I assume that the _1_0_0_x can go, and the _le presumably refers
> to little-endian and can be removed as well, as the kernel expects the firmware
> to have a fixed endianess, independent of how the kernel is built.
>
Adding firmware name in Kconfig is really odd. I don't think we should
do that. We need something better.

Regards,
Santosh