Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

[Arend Van Spriel]

On 27-1-2017 8:33, Kalle Valo wrote:
> Pali RohÃr <pali.rohar@xxxxxxxxx> writes:
> 
>> NVS calibration data for wl1251 are model specific. Every one device with
>> wl1251 chip has different and calibrated in factory.
>>
>> Not all wl1251 chips have own EEPROM where are calibration data stored. And
>> in that case there is no "standard" place. Every device has stored them on
>> different place (some in rootfs file, some in dedicated nand partition,
>> some in another proprietary structure).
>>
>> Kernel wl1251 driver cannot support every one different storage decided by
>> device manufacture so it will use request_firmware_prefer_user() call for
>> loading NVS calibration data and userspace helper will be responsible to
>> prepare correct data.
>>
>> In case userspace helper fails request_firmware_prefer_user() still try to
>> load data file directly from VFS as fallback mechanism.
>>
>> On Nokia N900 device which has wl1251 chip, NVS calibration data are stored
>> in CAL nand partition. CAL is proprietary Nokia key/value format for nand
>> devices.
>>
>> With this patch it is finally possible to load correct model specific NVS
>> calibration data for Nokia N900.
>>
>> Signed-off-by: Pali RohÃr <pali.rohar@xxxxxxxxx>
>> ---
>>  drivers/net/wireless/ti/wl1251/Kconfig |    1 +
>>  drivers/net/wireless/ti/wl1251/main.c  |    2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ti/wl1251/Kconfig b/drivers/net/wireless/ti/wl1251/Kconfig
>> index 7142ccf..affe154 100644
>> --- a/drivers/net/wireless/ti/wl1251/Kconfig
>> +++ b/drivers/net/wireless/ti/wl1251/Kconfig
>> @@ -2,6 +2,7 @@ config WL1251
>>  	tristate "TI wl1251 driver support"
>>  	depends on MAC80211
>>  	select FW_LOADER
>> +	select FW_LOADER_USER_HELPER
>>  	select CRC7
>>  	---help---
>>  	  This will enable TI wl1251 driver support. The drivers make
>> diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
>> index 208f062..24f8866 100644
>> --- a/drivers/net/wireless/ti/wl1251/main.c
>> +++ b/drivers/net/wireless/ti/wl1251/main.c
>> @@ -110,7 +110,7 @@ static int wl1251_fetch_nvs(struct wl1251 *wl)
>>  	struct device *dev = wiphy_dev(wl->hw->wiphy);
>>  	int ret;
>>  
>> -	ret = request_firmware(&fw, WL1251_NVS_NAME, dev);
>> +	ret = request_firmware_prefer_user(&fw, WL1251_NVS_NAME, dev);
> 
> I don't see the need for this. Just remove the default nvs file from
> filesystem and the fallback user helper will be always used, right?

Indeed. The only remaining issue would be that an error message is
logged. Also note the fallback is only used if selected in Kconfig.

> Like we discussed earlier, the default nvs file should not be used by
> normal users.

Yup.

Regards,
Arend