Re: wl1251: NVS firmware data

Pali Rohár
2014-12-06T13:02:27Z

[-- Attachment #1: Type: Text/Plain, Size: 6283 bytes --]

On Saturday 06 December 2014 13:49:54 Pavel Machek wrote:
> On Thu 2014-11-27 07:58:40, Greg Kroah-Hartman wrote:
> > On Thu, Nov 27, 2014 at 04:22:58PM +0100, Pali Rohár wrote:
> > > On Thursday 27 November 2014 16:16:55 Greg Kroah-Hartman wrote:
> > > > On Thu, Nov 27, 2014 at 03:43:23PM +0100, Pali Rohár wrote:
> > > > > On Thursday 27 November 2014 15:21:44 Ming Lei wrote:
> > > > > > On Thu, Nov 27, 2014 at 10:06 PM, Pali Rohár
> > > > >
> > > > > <pali.rohar@gmail.com> wrote:
> > > > > > > Hello,
> > > > > > >
> > > > > > > wifi driver wl1251 needs NVS calibration data for
> > > > > > > working. These data are loaded by driver via
> > > > > > > request_firmware from userspace file:
> > > > > > > ti-connectivity/wl1251-nvs.bin. In linux-fimrware
> > > > > > > git tree there is generic wl1251-nvs.bin file
> > > > > > > which is used by default.
> > > > > > >
> > > > > > > Driver wl1251 is used on Nokia N900 cellphone for
> > > > > > > its wifi chip. This cellphone has one special MTD
> > > > > > > partition (called CAL) where are stored some
> > > > > > > configuration data in special binary (key-value)
> > > > > > > format. And there is also stored correct
> > > > > > > calibration data for specific device (each device
> > > > > > > has different data). It is preferred to use those
> > > > > > > data instead generic one (provided by
> > > > > > > linux-firmware git tree).
> > > > > > >
> > > > > > > Now my question is: How to correctly load
> > > > > > > calibration data from special Nokia N900 CAL
> > > > > > > partition into wl1251 kernel driver?
> > > > > >
> > > > > > It is better to let user space script handle the
> > > > > > request.
> > > > >
> > > > > Yes, this makes sense. Implementing CAL parser in
> > > > > kernel wl1251 driver would be hard...
> > > > >
> > > > > > > By default kernel reads
> > > > > > > ti-connectivity/wl1251-nvs.bin file from VFS if
> > > > > > > exists without any userspace support. If it fails
> > > > > > > then it fallback to loading via udev.
> > > > > >
> > > > > > You can remove or rename this file so that loading
> > > > > > from user space can be triggered.
> > > > >
> > > > > It is no so easy... In case when CAL does not contains
> > > > > NVS data then we want to use this generic NVS file.
> > > > > And telling everybody to rename this is file is not
> > > > > good solution...
> > > >
> > > > But that's up to your system configuration, not the
> > > > kernel. Make a userspace package for the firmware that
> > > > creates it in the format you need it to be in, for the
> > > > hardware you have, and then there would not be any need
> > > > for a kernel change, right?
> > > >
> > > > greg k-h
> > >
> > > Not so simple as you think. Some parts of NVS data are
> > > configured based on location and cellular station. Data
> > > are not static.
> >
> > Then you need a dynamic program that you control, in
> > userspace, to dump the needed data into the kernel. Don't
> > try to do it with "static" firmware files. Use the binary
> > sysfs file interface for this if you want.
>
> Actually, this seems to be similar situation to fpga
> programming.
>
> There, it is static firmware for 90% users, but some special
> use cases want it more dynamic.
> Pavel

Greg, Ming, what do you think about this approach?

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 3d785eb..810c4b9 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -111,6 +111,11 @@ static inline long firmware_loading_timeout(void)
#define FW_OPT_FALLBACK 0
#endif
#define FW_OPT_NO_WARN (1U << 3)
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+#define FW_OPT_PREFER_USER (1U << 4)
+#else
+#define FW_OPT_PREFER_USER 0
+#endif

struct firmware_cache {
/* firmware_buf instance will be added into the below list */
@@ -1131,7 +1136,20 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
}
}

- ret = fw_get_filesystem_firmware(device, fw->priv);
+ if (opt_flags & FW_OPT_PREFER_USER) {
+ ret = fw_load_from_user_helper(fw, name, device, opt_flags, timeout);
+ if (ret) {
+ dev_warn(device,
+ "User helper firmware load for %s failed with error %d\n",
+ name, ret);
+ dev_warn(device, "Falling back to direct firmware load\n");
+ }
+ } else {
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
@@ -1218,6 +1236,28 @@ int request_firmware_direct(const struct firmware **firmware_p,
EXPORT_SYMBOL_GPL(request_firmware_direct);

/**
+ * request_firmware_prefer_user: - prefer usermode helper for loading firmware
+ * @firmware_p: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function works pretty much like request_firmware(), but it prefer
+ * usermode helper. If usermode helper fails then it fallback to direct access.
+ * Usefull for dynamic or model specific firmware data.
+ **/
+int request_firmware_prefer_user(const struct firmware **firmware_p,
+ const char *name, struct device *device)
+{
+ int ret;
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device,
+ FW_OPT_UEVENT | FW_OPT_PREFER_USER);
+ module_put(THIS_MODULE);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_prefer_user);
+
+/**
* release_firmware: - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 5c41c5e..d35c511 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -47,6 +47,8 @@ int request_firmware_nowait(
void (*cont)(const struct firmware *fw, void *context));
int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_prefer_user(const struct firmware **fw, const char *name,
+ struct device *device);

void release_firmware(const struct firmware *fw);
#else

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]