Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1567921 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data Kalle Valo <kvalo@codeaurora.org> - 2017-01-27 08:50 +0100
    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 10:50 +0100
      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Arend Van Spriel <arend.vanspriel@broadcom.com> - 2017-01-27 11:20 +0100
        Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 11:20 +0100
          Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Arend Van Spriel <arend.vanspriel@broadcom.com> - 2017-01-27 11:30 +0100
            Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 12:00 +0100
              Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data Kalle Valo <kvalo@codeaurora.org> - 2017-01-27 12:50 +0100
                Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 13:00 +0100
                  Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data Kalle Valo <kvalo@codeaurora.org> - 2017-01-27 13:40 +0100
                    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Arend Van Spriel <arend.vanspriel@broadcom.com> - 2017-01-27 14:00 +0100
                      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 14:20 +0100
                    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 14:20 +0100
                      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data Kalle Valo <kvalo@codeaurora.org> - 2017-01-27 16:30 +0100
                        Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 17:00 +0100
                        Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pavel Machek <pavel@ucw.cz> - 2017-01-27 20:50 +0100
                          Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Tony Lindgren <tony@atomide.com> - 2017-01-30 19:00 +0100
                            Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-01-30 19:10 +0100
                      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-01-29 18:30 +0100
          Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pavel Machek <pavel@ucw.cz> - 2017-01-27 13:10 +0100
    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Arend Van Spriel <arend.vanspriel@broadcom.com> - 2017-01-27 21:40 +0100

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

Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
> ---
>  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?

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

-- 
Kalle Valo

[toc] | [next] | [standalone]

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

On Friday 27 January 2017 09:33:40 Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
> > ---
> >  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?

It is part of linux-firmware repository. And already part of all
previous versions of linux-firmware packages in lot of linux
distributions. So removing it is not possible...

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

But already is and we need to deal with this fact.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

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

On 27-1-2017 10:43, Pali Rohár wrote:
> On Friday 27 January 2017 09:33:40 Kalle Valo wrote:
>> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
>>> ---
>>>  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?
> 
> It is part of linux-firmware repository. And already part of all
> previous versions of linux-firmware packages in lot of linux
> distributions. So removing it is not possible...

You are probably saying that on your platform you can not remove
anything from /lib/firmware, right? I don't see how you come from "it is
part of firmware package" to "removing is not possible". Trying to
understand this and it makes no sense.

>> Like we discussed earlier, the default nvs file should not be used by
>> normal users.
> 
> But already is and we need to deal with this fact.

Why? Are there other platforms that use the default nvs file and have a
working wifi. So your "removing is not possible" would be about
regression for those?

Regards,
Arend

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 11:05:32 Arend Van Spriel wrote:
> On 27-1-2017 10:43, Pali Rohár wrote:
> > On Friday 27 January 2017 09:33:40 Kalle Valo wrote:
> >> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
> >>> ---
> >>>  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?
> > 
> > It is part of linux-firmware repository. And already part of all
> > previous versions of linux-firmware packages in lot of linux
> > distributions. So removing it is not possible...
> 
> You are probably saying that on your platform you can not remove
> anything from /lib/firmware, right? I don't see how you come from "it is
> part of firmware package" to "removing is not possible". Trying to
> understand this and it makes no sense.

It is already in linux distribution packages. If I remove that file from
file system it will be placed there again by package management or it it
will throw error message about system integrity (missing file, etc...).

Also that file is already in linux-firmware git and so is propagated to
/lib/firmware by anybody who is using linux-firmware.

> >> Like we discussed earlier, the default nvs file should not be used by
> >> normal users.
> > 
> > But already is and we need to deal with this fact.
> 
> Why?

Because everybody has already installed it.

> Are there other platforms that use the default nvs file and have a
> working wifi.

I do not know.

> So your "removing is not possible" would be about
> regression for those?

Yes, that is possible.

Also you can use wifi on Nokia N900 with this default file. Yes it is
not recommended and probably has performance problems... but more people
use it for SSH and it is working. Pavel could confirm this.

So yes, if you remove that file *now* there is regression for Nokia N900
when you are using SSH over wifi.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

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

On 27-1-2017 11:10, Pali Rohár wrote:
> On Friday 27 January 2017 11:05:32 Arend Van Spriel wrote:
>> On 27-1-2017 10:43, Pali Rohár wrote:
>>> On Friday 27 January 2017 09:33:40 Kalle Valo wrote:
>>>> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
>>>>> ---
>>>>>  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?
>>>
>>> It is part of linux-firmware repository. And already part of all
>>> previous versions of linux-firmware packages in lot of linux
>>> distributions. So removing it is not possible...
>>
>> You are probably saying that on your platform you can not remove
>> anything from /lib/firmware, right? I don't see how you come from "it is
>> part of firmware package" to "removing is not possible". Trying to
>> understand this and it makes no sense.
> 
> It is already in linux distribution packages. If I remove that file from
> file system it will be placed there again by package management or it it
> will throw error message about system integrity (missing file, etc...).
> 
> Also that file is already in linux-firmware git and so is propagated to
> /lib/firmware by anybody who is using linux-firmware.
> 
>>>> Like we discussed earlier, the default nvs file should not be used by
>>>> normal users.
>>>
>>> But already is and we need to deal with this fact.
>>
>> Why?
> 
> Because everybody has already installed it.
> 
>> Are there other platforms that use the default nvs file and have a
>> working wifi.
> 
> I do not know.
> 
>> So your "removing is not possible" would be about
>> regression for those?
> 
> Yes, that is possible.
> 
> Also you can use wifi on Nokia N900 with this default file. Yes it is
> not recommended and probably has performance problems... but more people
> use it for SSH and it is working. Pavel could confirm this.
> 
> So yes, if you remove that file *now* there is regression for Nokia N900
> when you are using SSH over wifi.

So you are changing the behavior for all platforms using wl1251, but the
user-helper preference is (probably) only applicable for N900, right? So
for those other platforms there will be a delay waiting for user-mode
helper to fail, before trying to get nvs file from /lib/firmware.

Regards,
Arend

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 11:19:25 Arend Van Spriel wrote:
> On 27-1-2017 11:10, Pali Rohár wrote:
> > On Friday 27 January 2017 11:05:32 Arend Van Spriel wrote:
> >> On 27-1-2017 10:43, Pali Rohár wrote:
> >>> On Friday 27 January 2017 09:33:40 Kalle Valo wrote:
> >>>> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
> >>>>> ---
> >>>>>  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?
> >>>
> >>> It is part of linux-firmware repository. And already part of all
> >>> previous versions of linux-firmware packages in lot of linux
> >>> distributions. So removing it is not possible...
> >>
> >> You are probably saying that on your platform you can not remove
> >> anything from /lib/firmware, right? I don't see how you come from "it is
> >> part of firmware package" to "removing is not possible". Trying to
> >> understand this and it makes no sense.
> > 
> > It is already in linux distribution packages. If I remove that file from
> > file system it will be placed there again by package management or it it
> > will throw error message about system integrity (missing file, etc...).
> > 
> > Also that file is already in linux-firmware git and so is propagated to
> > /lib/firmware by anybody who is using linux-firmware.
> > 
> >>>> Like we discussed earlier, the default nvs file should not be used by
> >>>> normal users.
> >>>
> >>> But already is and we need to deal with this fact.
> >>
> >> Why?
> > 
> > Because everybody has already installed it.
> > 
> >> Are there other platforms that use the default nvs file and have a
> >> working wifi.
> > 
> > I do not know.
> > 
> >> So your "removing is not possible" would be about
> >> regression for those?
> > 
> > Yes, that is possible.
> > 
> > Also you can use wifi on Nokia N900 with this default file. Yes it is
> > not recommended and probably has performance problems... but more people
> > use it for SSH and it is working. Pavel could confirm this.
> > 
> > So yes, if you remove that file *now* there is regression for Nokia N900
> > when you are using SSH over wifi.
> 
> So you are changing the behavior for all platforms using wl1251, but the
> user-helper preference is (probably) only applicable for N900, right?

No. Some wl1251 chips have internal EEPROM where is stored MAC address
and NVS data. And kernel driver already can read it. So this change is
only for platforms without internal EEPROM.

And all platforms without internal EEPROM should use userspace helper to
provide correct NVS data (and ideally also MAC address).

Except Nokia N900 I know just Pandora who has also wl1251 chip. But
Pandora has EEPROM.

Grepping linux source code... and I see only defines for Nokia N900 and
Pandora. There can be also another user with external DTS file, but what
is reality? Is there still really any other user of wl1251 chip with
upstream kernel? If yes, we can prepare userspace helper if he does not
have NVS stored in EEPROM...

> So
> for those other platforms there will be a delay waiting for user-mode
> helper to fail, before trying to get nvs file from /lib/firmware.

Yes, there will be. But there is no easy way to fix this problem that
kernel is trying to use default/example NVS data...

When helper is not available this patch just adds delay, but
functionality is still there and same. With helper support will be
finally fixed.

And I have no idea if those default NVS data are somehow usable on other
platforms...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1568236

Pali Rohár <pali.rohar@gmail.com> writes:

>> So
>> for those other platforms there will be a delay waiting for user-mode
>> helper to fail, before trying to get nvs file from /lib/firmware.
>
> Yes, there will be. But there is no easy way to fix this problem that
> kernel is trying to use default/example NVS data...

Kernel is doing correctly and requesting NVS data as expected, the
problem here is that linux-firmware claims that the example NVS data is
real calibration data (which it is not). Distros should not use that,
only developers for testing purposes. We should not courage users using
example calibration data.

The simple fix is to rename the NVS file in linux-firmware to something
like wl1251-nvs.bin.example, no need to workaround this in kernel. If
you send a patch to linux-firmware I'm happy to ack that.

-- 
Kalle Valo

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 13:49:03 Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
> >> So
> >> for those other platforms there will be a delay waiting for user-mode
> >> helper to fail, before trying to get nvs file from /lib/firmware.
> >
> > Yes, there will be. But there is no easy way to fix this problem that
> > kernel is trying to use default/example NVS data...
> 
> Kernel is doing correctly and requesting NVS data as expected, the
> problem here is that linux-firmware claims that the example NVS data is
> real calibration data (which it is not). Distros should not use that,
> only developers for testing purposes. We should not courage users using
> example calibration data.
> 
> The simple fix is to rename the NVS file in linux-firmware to something
> like wl1251-nvs.bin.example, no need to workaround this in kernel. If
> you send a patch to linux-firmware I'm happy to ack that.

I agree with rename and fact that default/example data should not be
used.

But...

1) Kernel should not read device/model specific data from VFS where
are stored not-device-specific files preinstalled by linux
distributions.

And linux distributions are already putting files into VFS and kernel
cannot enforce userspace to not do that (as they are already doing it).

2) It was already tested that example NVS data can be used for N900 e.g.
for SSH connection. If real correct data are not available it is better
to use at least those example (and probably log warning message) so user
can connect via SSH and start investigating where is problem.

3) If we do rename *now* we will totally break wifi support on Nokia
N900.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1568332

Pali Rohár <pali.rohar@gmail.com> writes:

> On Friday 27 January 2017 13:49:03 Kalle Valo wrote:
>> Pali Rohár <pali.rohar@gmail.com> writes:
>> 
>> >> So
>> >> for those other platforms there will be a delay waiting for user-mode
>> >> helper to fail, before trying to get nvs file from /lib/firmware.
>> >
>> > Yes, there will be. But there is no easy way to fix this problem that
>> > kernel is trying to use default/example NVS data...
>> 
>> Kernel is doing correctly and requesting NVS data as expected, the
>> problem here is that linux-firmware claims that the example NVS data is
>> real calibration data (which it is not). Distros should not use that,
>> only developers for testing purposes. We should not courage users using
>> example calibration data.
>> 
>> The simple fix is to rename the NVS file in linux-firmware to something
>> like wl1251-nvs.bin.example, no need to workaround this in kernel. If
>> you send a patch to linux-firmware I'm happy to ack that.
>
> I agree with rename and fact that default/example data should not be
> used.
>
> But...
>
> 1) Kernel should not read device/model specific data from VFS where
> are stored not-device-specific files preinstalled by linux
> distributions.
>
> And linux distributions are already putting files into VFS and kernel
> cannot enforce userspace to not do that (as they are already doing it).

I'm having problems to understand what you are saying here.

> 2) It was already tested that example NVS data can be used for N900 e.g.
> for SSH connection. If real correct data are not available it is better
> to use at least those example (and probably log warning message) so user
> can connect via SSH and start investigating where is problem.

I disagree. Allowing default calibration data to be used can be
unnoticed by user and left her wondering why wifi works so badly.

> 3) If we do rename *now* we will totally break wifi support on Nokia
> N900.

Then the distro should fix that when updating the linux-firmware
packages. Can you provide details about the setup, what distro etc?

-- 
Kalle Valo

[toc] | [prev] | [next] | [standalone]

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

On 27-1-2017 13:26, Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
>> On Friday 27 January 2017 13:49:03 Kalle Valo wrote:
>>> Pali Rohár <pali.rohar@gmail.com> writes:
>>>
>>>>> So
>>>>> for those other platforms there will be a delay waiting for user-mode
>>>>> helper to fail, before trying to get nvs file from /lib/firmware.
>>>>
>>>> Yes, there will be. But there is no easy way to fix this problem that
>>>> kernel is trying to use default/example NVS data...
>>>
>>> Kernel is doing correctly and requesting NVS data as expected, the
>>> problem here is that linux-firmware claims that the example NVS data is
>>> real calibration data (which it is not). Distros should not use that,
>>> only developers for testing purposes. We should not courage users using
>>> example calibration data.
>>>
>>> The simple fix is to rename the NVS file in linux-firmware to something
>>> like wl1251-nvs.bin.example, no need to workaround this in kernel. If
>>> you send a patch to linux-firmware I'm happy to ack that.
>>
>> I agree with rename and fact that default/example data should not be
>> used.
>>
>> But...
>>
>> 1) Kernel should not read device/model specific data from VFS where
>> are stored not-device-specific files preinstalled by linux
>> distributions.
>>
>> And linux distributions are already putting files into VFS and kernel
>> cannot enforce userspace to not do that (as they are already doing it).
> 
> I'm having problems to understand what you are saying here.

This is a personal opinion. I read it as: /lib/firmware can only contain
files for from linux-firmware.

At least the device-specific vs. non-device-specific does not seem to
hold. The firmware files that we have in the linux-firmware repository
are very device-specific. Unless you mean the 'platform' when talking
about 'device'.

Regards,
Arend

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 13:53:28 Arend Van Spriel wrote:
> On 27-1-2017 13:26, Kalle Valo wrote:
> > Pali Rohár <pali.rohar@gmail.com> writes:
> > 
> >> On Friday 27 January 2017 13:49:03 Kalle Valo wrote:
> >>> Pali Rohár <pali.rohar@gmail.com> writes:
> >>>
> >>>>> So
> >>>>> for those other platforms there will be a delay waiting for user-mode
> >>>>> helper to fail, before trying to get nvs file from /lib/firmware.
> >>>>
> >>>> Yes, there will be. But there is no easy way to fix this problem that
> >>>> kernel is trying to use default/example NVS data...
> >>>
> >>> Kernel is doing correctly and requesting NVS data as expected, the
> >>> problem here is that linux-firmware claims that the example NVS data is
> >>> real calibration data (which it is not). Distros should not use that,
> >>> only developers for testing purposes. We should not courage users using
> >>> example calibration data.
> >>>
> >>> The simple fix is to rename the NVS file in linux-firmware to something
> >>> like wl1251-nvs.bin.example, no need to workaround this in kernel. If
> >>> you send a patch to linux-firmware I'm happy to ack that.
> >>
> >> I agree with rename and fact that default/example data should not be
> >> used.
> >>
> >> But...
> >>
> >> 1) Kernel should not read device/model specific data from VFS where
> >> are stored not-device-specific files preinstalled by linux
> >> distributions.
> >>
> >> And linux distributions are already putting files into VFS and kernel
> >> cannot enforce userspace to not do that (as they are already doing it).
> > 
> > I'm having problems to understand what you are saying here.
> 
> This is a personal opinion. I read it as: /lib/firmware can only contain
> files for from linux-firmware.
> 
> At least the device-specific vs. non-device-specific does not seem to
> hold. The firmware files that we have in the linux-firmware repository
> are very device-specific. Unless you mean the 'platform' when talking
> about 'device'.

Here I'm talking about files which are specific per unit. Every one N900
has different NVS file (stored in CAL) and so every one N900 device
needs its own NVS file. And we cannot store thousands of NVS files into
linux-firmware repository for each N900 which was ever produced in
factory.

Firmware files in linux-firmware repository are "device" specific, but
"filename" of that file describe exactly for which "device" it is
specific.

But there are thousands of different NVS files for one filename
"wl1251-nvs.bin" and we cannot use one particular for another device. In
linux-firmware is stored "wl1251-nvs.bin" file with example data.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
> > On Friday 27 January 2017 13:49:03 Kalle Valo wrote:
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> 
> >> >> So
> >> >> for those other platforms there will be a delay waiting for user-mode
> >> >> helper to fail, before trying to get nvs file from /lib/firmware.
> >> >
> >> > Yes, there will be. But there is no easy way to fix this problem that
> >> > kernel is trying to use default/example NVS data...
> >> 
> >> Kernel is doing correctly and requesting NVS data as expected, the
> >> problem here is that linux-firmware claims that the example NVS data is
> >> real calibration data (which it is not). Distros should not use that,
> >> only developers for testing purposes. We should not courage users using
> >> example calibration data.
> >> 
> >> The simple fix is to rename the NVS file in linux-firmware to something
> >> like wl1251-nvs.bin.example, no need to workaround this in kernel. If
> >> you send a patch to linux-firmware I'm happy to ack that.
> >
> > I agree with rename and fact that default/example data should not be
> > used.
> >
> > But...
> >
> > 1) Kernel should not read device/model specific data from VFS where
> > are stored not-device-specific files preinstalled by linux
> > distributions.
> >
> > And linux distributions are already putting files into VFS and kernel
> > cannot enforce userspace to not do that (as they are already doing it).
> 
> I'm having problems to understand what you are saying here.

I'm saying that linux distributions are putting files to /lib/firmware
which comes from some sources already released. You cannot force linux
distributions to stop putting particular file to /lib/firmware *now*
after it was already released and recommended.

> > 2) It was already tested that example NVS data can be used for N900 e.g.
> > for SSH connection. If real correct data are not available it is better
> > to use at least those example (and probably log warning message) so user
> > can connect via SSH and start investigating where is problem.
> 
> I disagree. Allowing default calibration data to be used can be
> unnoticed by user and left her wondering why wifi works so badly.

So there are only two options:

1) Disallow it and so these users will have non-working wifi.

2) Allow those data to be used as fallback mechanism.

And personally I'm against 1) because it will break wifi support for
*all* Nokia N900 devices right now.

> > 3) If we do rename *now* we will totally break wifi support on Nokia
> > N900.
> 
> Then the distro should fix that when updating the linux-firmware
> packages. Can you provide details about the setup, what distro etc?

Debian stable, Ubuntu LTSs 14.04, 16.04. And I think that other
LTS distributions contains that example nvs file too (I'm not going to
verify others, but list will be probably long). Upgrading linux-firmware
is against policy of those distributions. So no this is not an solution.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1568436

Pali Rohár <pali.rohar@gmail.com> writes:

> On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
>> Pali Rohár <pali.rohar@gmail.com> writes:
>> 
>> > 2) It was already tested that example NVS data can be used for N900 e.g.
>> > for SSH connection. If real correct data are not available it is better
>> > to use at least those example (and probably log warning message) so user
>> > can connect via SSH and start investigating where is problem.
>> 
>> I disagree. Allowing default calibration data to be used can be
>> unnoticed by user and left her wondering why wifi works so badly.
>
> So there are only two options:
>
> 1) Disallow it and so these users will have non-working wifi.
>
> 2) Allow those data to be used as fallback mechanism.
>
> And personally I'm against 1) because it will break wifi support for
> *all* Nokia N900 devices right now.

All two of them? :)

But not working is exactly my point, if correct calibration data is not
available wifi should not work. And it's not only about functionality
problems, there's also the regulatory aspect.

>> > 3) If we do rename *now* we will totally break wifi support on Nokia
>> > N900.
>> 
>> Then the distro should fix that when updating the linux-firmware
>> packages. Can you provide details about the setup, what distro etc?
>
> Debian stable, Ubuntu LTSs 14.04, 16.04. 

You can run these out of box on N900?

> And I think that other LTS distributions contains that example nvs
> file too (I'm not going to verify others, but list will be probably
> long). Upgrading linux-firmware is against policy of those
> distributions. So no this is not an solution.

So instead we should workaround distro policies in kernel? Come on.

Seriously, just rename the file in linux-firmware and file a bug (with a
patch) to distros. If they don't fix the bug you just have to do a
custom hack for N900. But such is life.

-- 
Kalle Valo

[toc] | [prev] | [next] | [standalone]

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

On Friday 27 January 2017 17:23:07 Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
> > On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> 
> >> > 2) It was already tested that example NVS data can be used for N900 e.g.
> >> > for SSH connection. If real correct data are not available it is better
> >> > to use at least those example (and probably log warning message) so user
> >> > can connect via SSH and start investigating where is problem.
> >> 
> >> I disagree. Allowing default calibration data to be used can be
> >> unnoticed by user and left her wondering why wifi works so badly.
> >
> > So there are only two options:
> >
> > 1) Disallow it and so these users will have non-working wifi.
> >
> > 2) Allow those data to be used as fallback mechanism.
> >
> > And personally I'm against 1) because it will break wifi support for
> > *all* Nokia N900 devices right now.
> 
> All two of them? :)

Ehm...

> But not working is exactly my point, if correct calibration data is not
> available wifi should not work. And it's not only about functionality
> problems, there's also the regulatory aspect.

About functionality, Pavel confirmed too that SSH is somehow working...

Regulatory aspect is different, but via iw can be manually configured
some settings.

> >> > 3) If we do rename *now* we will totally break wifi support on Nokia
> >> > N900.
> >> 
> >> Then the distro should fix that when updating the linux-firmware
> >> packages. Can you provide details about the setup, what distro etc?
> >
> > Debian stable, Ubuntu LTSs 14.04, 16.04. 
> 
> You can run these out of box on N900?

Out-of-box I can run Kubuntu 12.04 (which is LTS too). They had prepared
special image for N900 and I still have it on uSD card.

I guess that new versions of Ubuntu could somehow work (maybe not
out-of-box but with some changes) and Pavel has working Debian.

Also basic support needed for wifi and SSH server is probably working
with any distribution targeting armv7-a or omap3. So yes, I can say it
is out-of-box. We will not have GSM calls or camera support, but wifi
breakage is there.

> > And I think that other LTS distributions contains that example nvs
> > file too (I'm not going to verify others, but list will be probably
> > long). Upgrading linux-firmware is against policy of those
> > distributions. So no this is not an solution.
> 
> So instead we should workaround distro policies in kernel? Come on.
> 
> Seriously, just rename the file in linux-firmware and file a bug (with a
> patch) to distros. If they don't fix the bug you just have to do a
> custom hack for N900. But such is life.

I do not see point what will be changed. I rename that file and after
system update (or integrity check) it will be there again.

And if I do that, what prevents kernel to stop using NVS file from
/lib/firmware/? Nothing, original problem (which is going solved by this
patch series) still remains.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

On Fri 2017-01-27 17:23:07, Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> writes:
> 
> > On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
> >> Pali Rohár <pali.rohar@gmail.com> writes:
> >> 
> >> > 2) It was already tested that example NVS data can be used for N900 e.g.
> >> > for SSH connection. If real correct data are not available it is better
> >> > to use at least those example (and probably log warning message) so user
> >> > can connect via SSH and start investigating where is problem.
> >> 
> >> I disagree. Allowing default calibration data to be used can be
> >> unnoticed by user and left her wondering why wifi works so badly.
> >
> > So there are only two options:
> >
> > 1) Disallow it and so these users will have non-working wifi.
> >
> > 2) Allow those data to be used as fallback mechanism.
> >
> > And personally I'm against 1) because it will break wifi support for
> > *all* Nokia N900 devices right now.
> 
> All two of them? :)

Umm. You clearly want a flock of angry penguins at your doorsteps :-).

> But not working is exactly my point, if correct calibration data is not
> available wifi should not work. And it's not only about functionality
> problems, there's also the regulatory aspect.

If you break existing configuration that's called "regression".

> >> > 3) If we do rename *now* we will totally break wifi support on Nokia
> >> > N900.
> >> 
> >> Then the distro should fix that when updating the linux-firmware
> >> packages. Can you provide details about the setup, what distro etc?
> >
> > Debian stable, Ubuntu LTSs 14.04, 16.04. 
> 
> You can run these out of box on N900?

Debian stable does.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

* Pavel Machek <pavel@ucw.cz> [170127 11:41]:
> On Fri 2017-01-27 17:23:07, Kalle Valo wrote:
> > Pali Rohár <pali.rohar@gmail.com> writes:
> > 
> > > On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
> > >> Pali Rohár <pali.rohar@gmail.com> writes:
> > >> 
> > >> > 2) It was already tested that example NVS data can be used for N900 e.g.
> > >> > for SSH connection. If real correct data are not available it is better
> > >> > to use at least those example (and probably log warning message) so user
> > >> > can connect via SSH and start investigating where is problem.
> > >> 
> > >> I disagree. Allowing default calibration data to be used can be
> > >> unnoticed by user and left her wondering why wifi works so badly.
> > >
> > > So there are only two options:
> > >
> > > 1) Disallow it and so these users will have non-working wifi.
> > >
> > > 2) Allow those data to be used as fallback mechanism.
> > >
> > > And personally I'm against 1) because it will break wifi support for
> > > *all* Nokia N900 devices right now.
> > 
> > All two of them? :)
> 
> Umm. You clearly want a flock of angry penguins at your doorsteps :-).

Well this silly issue of symlinking and renaming nvs files in a standard
Linux distro was also hitting me on various devices with wl12xx/wl18xx
trying to use the same rootfs.

Why don't we just set a custom compatible property for n900 that then
picks up some other nvs file instead of the default?

Regards,

Tony

[toc] | [prev] | [next] | [standalone]

#1569973

[Multipart message — attachments visible in raw view] — view raw

On Monday 30 January 2017 18:53:09 Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [170127 11:41]:
> > On Fri 2017-01-27 17:23:07, Kalle Valo wrote:
> > > Pali Rohár <pali.rohar@gmail.com> writes:
> > > > On Friday 27 January 2017 14:26:22 Kalle Valo wrote:
> > > >> Pali Rohár <pali.rohar@gmail.com> writes:
> > > >> > 2) It was already tested that example NVS data can be used
> > > >> > for N900 e.g. for SSH connection. If real correct data are
> > > >> > not available it is better to use at least those example
> > > >> > (and probably log warning message) so user can connect via
> > > >> > SSH and start investigating where is problem.
> > > >> 
> > > >> I disagree. Allowing default calibration data to be used can
> > > >> be unnoticed by user and left her wondering why wifi works so
> > > >> badly.
> > > > 
> > > > So there are only two options:
> > > > 
> > > > 1) Disallow it and so these users will have non-working wifi.
> > > > 
> > > > 2) Allow those data to be used as fallback mechanism.
> > > > 
> > > > And personally I'm against 1) because it will break wifi
> > > > support for *all* Nokia N900 devices right now.
> > > 
> > > All two of them? :)
> > 
> > Umm. You clearly want a flock of angry penguins at your doorsteps
> > :-).
> 
> Well this silly issue of symlinking and renaming nvs files in a
> standard Linux distro was also hitting me on various devices with
> wl12xx/wl18xx trying to use the same rootfs.

wl12xx/wl18xx have probably exactly same problem as wl1251.

> Why don't we just set a custom compatible property for n900 that then
> picks up some other nvs file instead of the default?

But that still does not solve this problem correctly. Every n900 device 
have different NVS file. If we allow to load firmware directly from VFS 
without userspace helper we would see again same problem.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

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

On Fri, Jan 27, 2017 at 02:11:46PM +0100, Pali Rohár wrote:
> So there are only two options:
> 
> 1) Disallow it and so these users will have non-working wifi.
> 
> 2) Allow those data to be used as fallback mechanism.

There is one "custom fallback" user in kernel which we recently
determined was a total mistake. A sysfs interface should have
been defined to enable custom LED settings. Can't a series of
sysfs interfaces be used to enable override ? So is that not a
third option worth consideration?

  Luis

[toc] | [prev] | [next] | [standalone]

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

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > You are probably saying that on your platform you can not remove
> > anything from /lib/firmware, right? I don't see how you come from "it is
> > part of firmware package" to "removing is not possible". Trying to
> > understand this and it makes no sense.
> 
> It is already in linux distribution packages. If I remove that file from
> file system it will be placed there again by package management or it it
> will throw error message about system integrity (missing file, etc...).
> 
> Also that file is already in linux-firmware git and so is propagated to
> /lib/firmware by anybody who is using linux-firmware.
> 
> > >> Like we discussed earlier, the default nvs file should not be used by
> > >> normal users.
> > > 
> > > But already is and we need to deal with this fact.
> > 
> > Why?
> 
> Because everybody has already installed it.
> 
> > Are there other platforms that use the default nvs file and have a
> > working wifi.
> 
> I do not know.
> 
> > So your "removing is not possible" would be about
> > regression for those?
> 
> Yes, that is possible.
> 
> Also you can use wifi on Nokia N900 with this default file. Yes it is
> not recommended and probably has performance problems... but more people
> use it for SSH and it is working. Pavel could confirm this.

Yes, wifi somehow works on N900. .. depending on userspace and kernel
versions.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

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

On 27-1-2017 8:33, Kalle Valo wrote:
> Pali Rohár <pali.rohar@gmail.com> 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@gmail.com>
>> ---
>>  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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web