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

From: Pavel Machek <pavel@ucw.cz>
To: "Pali Rohár" <pali.rohar@gmail.com>
Cc: Arend Van Spriel <arend.vanspriel@broadcom.com>,
	Ming Lei <ming.lei@canonical.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	David Gnedt <david.gnedt@davizone.at>,
	Michal Kazior <michal.kazior@tieto.com>,
	Daniel Wagner <wagi@monom.org>, Tony Lindgren <tony@atomide.com>,
	Sebastian Reichel <sre@kernel.org>,
	Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>,
	Aaro Koskinen <aaro.koskinen@iki.fi>,
	Grazvydas Ignotas <notasas@gmail.com>,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
Date: Mon, 26 Dec 2016 16:43:53 +0100	[thread overview]
Message-ID: <20161226154353.GA27087@amd> (raw)
In-Reply-To: <201612252146.44496@pali>

[-- Attachment #1: Type: text/plain, Size: 2621 bytes --]

Hi!

> > > 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.
> > 
> > Responding to this patch as it provides a lot of context to discuss.
> > As you might have gathered from earlier discussions I am not a fan
> > of using user-space helper. I can agree that the kernel driver,
> > wl1251 in this case, should be agnostic to platform specific details
> > regarding storage solutions and the firmware api should hide that.
> > However, it seems your only solution is adding user-space to the mix
> > and changing the api towards that. Can we solve it without
> > user-space help?
> 
> Without userspace helper it means that userspace helper code must be 
> integrated into kernel.
> 
> So what is userspace helper doing?
> 
> 1) Read MAC address from CAL
> 2) Read NVS data from CAL
> 3) Modify MAC address in memory NVS data (new for this patch series)
> 4) Modify in memory NVS data if we in FCC country
> 
> Checking for country is done via dbus call to either Maemo cellular 
> daemon or alternatively via REGDOMAIN in /etc/default/crda. I have plan 
> to use ofono (instead Maemo cellular daemon) too...
> 
> Currently we are using closed Nokia proprietary CAL library.
> 
> Steps 1) and 2) needs closed library, step 4) needs dbus call.

I guess pointer to the source code implementing this would be welcome.

> > But on other devices that use wl1251, but for instance have no
> > userspace helper the request to userspace will fail (after 60 sec?)
> > and try VFS after that. Maybe not so nice.
> 
> Currently support for those devices is broken (like for N900) as without 
> proper NVS data they do not work correctly...

Is it expected to work at all, perhaps with degraded performance /
range? Because it seems to work for me.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]