linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] firmware_class: Add firmware filename overrides
@ 2015-03-04 23:25 Charlie Mooney
  2015-03-04 23:57 ` Greg KH
  0 siblings, 1 reply; 17+ messages in thread
From: Charlie Mooney @ 2015-03-04 23:25 UTC (permalink / raw)
  To: linux-kernel; +Cc: dtor, ming.lei, gregkh, Charlie Mooney

This patch adds an additional feature to the firmware_class.c module.
To allow a unified method of specifying new firmware locations when
drivers request a firmware binary to update their devices with, we
have added the concept of a "fw override"

A fw override is a rule that matches firmware requests based on the
name of the device requesting the fw and what the filename for the
fw they are requesting is, and overrides their requests with a new
value.

Overrides are set up by piping a description of the override into
an attribute set up at /sys/class/firmware/fw_override.  The string
should be a null-deliminited list of the firmware name you want to
over ride, the new name to replace it with, and the device name that
you want the override to apply to.   For example you could set up
an override for a device called "my_device" so that any time it
requests a firmware called "my_fw.bin" it instead gets "new_fw.bin"
with the following command:

echo -e 'my_fw.bin\0new_fw.bin\0my_device\0' >
				/sys/class/firmware/fw_override

The device name is an optional field, and if no string is there it
will apply to ANY device requesting a firmware of that name.  For
example if you want all devices looking for my_fw.bin to get
new_fw.bin instead you might use this command:

echo -e 'my_fw.bin\0new_fw.bin\0\0' > /sys/class/firmware/fw_override

Existing overrides can be viewed by cat-ing that attribute and can
be overwritten with new rules at will.

To delete an override simply put an empty string in the new firmware
field and the matching rule will be removed. eg: to remove the rule
set up in the first above example, one might run this command.

echo -e 'my_fw.bin\0\0my_device\0' > /sys/class/firmware/fw_override

Note: To make this feature usable even when CONFIG_FW_LOADER_USER_HELPER
is not set, the firmware class is brought back.  However, "timeout"
and the other functionality remain disabled.

Signed-off-by: Charlie Mooney <charliemooney@chromium.org>
---
 drivers/base/firmware_class.c | 196 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 180 insertions(+), 16 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 6c5c9ed..863ced5 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -440,6 +440,163 @@ static int fw_add_devm_name(struct device *dev, const char *name)
 }
 #endif
 
+struct fw_override {
+	struct list_head node;
+	const char *old_name;
+	const char *new_name;
+	const char *device_name;
+};
+static LIST_HEAD(fw_override_list);
+
+/*
+ * find_fw_override -- Find the override for the fw name and device specified
+ *
+ * Given a fw filename and the devices's name, this function finds the fw
+ * override assigned to it (if one exists).
+ *
+ * Returns a pointer to the fw_override struct in question or NULL if it
+ * does not exist.
+ */
+static struct fw_override *find_fw_override(const char *fw_name,
+					     const char *device_name)
+{
+	struct fw_override *override;
+
+	if (!fw_name)
+		return NULL;
+
+	list_for_each_entry(override, &fw_override_list, node) {
+		if (!strcmp(override->old_name, fw_name) &&
+		    (!device_name ||
+		     !strcmp(device_name, override->device_name))) {
+			return override;
+		}
+	}
+	return NULL;
+}
+
+/* Determine what is the actual filename that should be loaded for a given
+ * firmware and device.  If there is no overrides in place for this device
+ * and firmware, then the name will be returned unchanged.  Otherwise, the
+ * overridden firmware name is returned.
+ *
+ * This is called from _request_firmware() to make sure that overrides are
+ * applied to every firmware request regardless of how the request is made.
+ */
+static const char *firmware_overridden_name(const char *name,
+					    struct device *device)
+{
+	struct fw_override *override;
+
+	/* Check first for device-specific overrides, before checking for more
+	 * generic overrides that might apply to this device.
+	 */
+	override = find_fw_override(name, dev_name(device));
+	if (!override)
+		override = find_fw_override(name, NULL);
+
+	if (override)
+		dev_info(device, "firmware: '%s' overridden to '%s'.\n",
+			name, override->new_name);
+
+	return override ? override->new_name : name;
+}
+
+
+/*
+ * delete_fw_override -- Remove an override for the fw
+ *
+ * Given a firmware filename and the device name, remove any overrides that may
+ * have been set for it.  If the device name is NULL, then this looks only for
+ * global overrides.
+ */
+static void delete_fw_override(const char *fw_name, const char *device_name)
+{
+	struct fw_override *to_delete;
+
+	to_delete = find_fw_override(fw_name, device_name);
+	if (to_delete) {
+		list_del(&to_delete->node);
+		kfree(to_delete);
+	}
+}
+
+/*
+ * fw_override_show -- Display the current table of fw overrides
+ *
+ * When reading from this attribute, this function returns a table of
+ * fw filenames and their overridden values.
+ */
+static ssize_t fw_override_show(struct class *class,
+				struct class_attribute *attr,
+				char *buf)
+{
+	struct fw_override *override;
+	int len = 0;
+
+	list_for_each_entry(override, &fw_override_list, node) {
+		len += scnprintf(&buf[len],
+				 PAGE_SIZE - len,
+				 "%s\t%s\t(%s)\n",
+				 override->old_name, override->new_name,
+				 (strlen(override->device_name) > 0 ?
+					override->device_name : "Any"));
+	}
+
+	return len;
+}
+
+/*
+ * fw_override_store -- Add a new fw override
+ *
+ * This function takes a string as input of the form
+ *   "old_fw_name\0new_fw_name\0device_name\0"
+ * and adds it to the list of existing fw overrides.
+ *
+ * When any device requests a FW with the "old_fw_name" it will instead
+ * be given the FW at new_fw_name.
+ *
+ * Note: The device_name is optional, but if it's included this new rule
+ * will only apply to that specific device as opposed to ALL devices.
+ */
+static ssize_t fw_override_store(struct class *class,
+				 struct class_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct fw_override *new_override;
+	int i, num_strings = 0;
+
+	/* First check that there are exactly 3 null terminated strings and
+	 * that the first one is not the empty string.
+	 */
+	for (i = 0; i < count; i++)
+		num_strings += (buf[i] == '\0') ? 1 : 0;
+	if (num_strings != 3 || strlen(buf) == 0)
+		return -EINVAL;
+
+	/* Fill a new fw_override struct with the supplied information */
+	new_override = kmalloc(sizeof(struct fw_override) + count, GFP_KERNEL);
+	if (!new_override)
+		return -ENOMEM;
+	memcpy((char *)new_override + sizeof(struct fw_override), buf, count);
+	new_override->old_name = (char *)new_override +
+				 sizeof(struct fw_override);
+	new_override->new_name = new_override->old_name +
+				 strlen(new_override->old_name) + 1;
+	new_override->device_name = new_override->new_name +
+				    strlen(new_override->new_name) + 1;
+
+	/* Delete any existing overrides with this old_name and device_name */
+	delete_fw_override(new_override->old_name, new_override->device_name);
+
+	/* If there was a new fw name specified, store this override */
+	if (strlen(new_override->new_name) > 0)
+		list_add_tail(&new_override->node, &fw_override_list);
+	else
+		kfree(new_override);
+
+	return count;
+}
 
 /*
  * user-mode helper code
@@ -532,11 +689,6 @@ static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
 	return count;
 }
 
-static struct class_attribute firmware_class_attrs[] = {
-	__ATTR_RW(timeout),
-	__ATTR_NULL
-};
-
 static void fw_dev_release(struct device *dev)
 {
 	struct firmware_priv *fw_priv = to_firmware_priv(dev);
@@ -557,14 +709,26 @@ static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 	return 0;
 }
+#endif /* CONFIG_FW_LOADER_USER_HELPER */
+
+static struct class_attribute firmware_class_attrs[] = {
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+	__ATTR_RW(timeout),
+#endif
+	__ATTR_RW(fw_override),
+	__ATTR_NULL
+};
 
 static struct class firmware_class = {
 	.name		= "firmware",
 	.class_attrs	= firmware_class_attrs,
+#ifdef CONFIG_FW_LOADER_USER_HELPER
 	.dev_uevent	= firmware_uevent,
 	.dev_release	= fw_dev_release,
+#endif
 };
 
+#ifdef CONFIG_FW_LOADER_USER_HELPER
 static ssize_t firmware_loading_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
@@ -978,7 +1142,6 @@ static inline void kill_requests_without_uevent(void) { }
 
 #endif /* CONFIG_FW_LOADER_USER_HELPER */
 
-
 /* wait until the shared firmware_buf becomes ready (or error) */
 static int sync_cached_firmware_buf(struct firmware_buf *buf)
 {
@@ -1087,6 +1250,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 {
 	struct firmware *fw;
 	long timeout;
+	const char *overridden_name;
 	int ret;
 
 	if (!firmware_p)
@@ -1095,7 +1259,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 	if (!name || name[0] == '\0')
 		return -EINVAL;
 
-	ret = _request_firmware_prepare(&fw, name, device);
+	overridden_name = firmware_overridden_name(name, device);
+	ret = _request_firmware_prepare(&fw, overridden_name, device);
 	if (ret <= 0) /* error or already assigned */
 		goto out;
 
@@ -1105,7 +1270,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		timeout = usermodehelper_read_lock_wait(timeout);
 		if (!timeout) {
 			dev_dbg(device, "firmware: %s loading timed out\n",
-				name);
+				overridden_name);
 			ret = -EBUSY;
 			goto out;
 		}
@@ -1113,7 +1278,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		ret = usermodehelper_read_trylock();
 		if (WARN_ON(ret)) {
 			dev_err(device, "firmware: %s will not be loaded\n",
-				name);
+				overridden_name);
 			goto out;
 		}
 	}
@@ -1123,11 +1288,12 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		if (!(opt_flags & FW_OPT_NO_WARN))
 			dev_warn(device,
 				 "Direct firmware load for %s failed with error %d\n",
-				 name, ret);
+				 overridden_name, ret);
 		if (opt_flags & FW_OPT_USERHELPER) {
 			dev_warn(device, "Falling back to user helper\n");
-			ret = fw_load_from_user_helper(fw, name, device,
-						       opt_flags, timeout);
+			ret = fw_load_from_user_helper(fw, overridden_name,
+						       device, opt_flags,
+						       timeout);
 		}
 	}
 
@@ -1659,10 +1825,8 @@ static int __init firmware_class_init(void)
 	fw_cache_init();
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 	register_reboot_notifier(&fw_shutdown_nb);
-	return class_register(&firmware_class);
-#else
-	return 0;
 #endif
+	return class_register(&firmware_class);
 }
 
 static void __exit firmware_class_exit(void)
@@ -1673,8 +1837,8 @@ static void __exit firmware_class_exit(void)
 #endif
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 	unregister_reboot_notifier(&fw_shutdown_nb);
-	class_unregister(&firmware_class);
 #endif
+	class_unregister(&firmware_class);
 }
 
 fs_initcall(firmware_class_init);
-- 
2.1.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-04 23:25 [PATCH] firmware_class: Add firmware filename overrides Charlie Mooney
@ 2015-03-04 23:57 ` Greg KH
  2015-03-05  0:48   ` Dmitry Torokhov
  0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2015-03-04 23:57 UTC (permalink / raw)
  To: Charlie Mooney; +Cc: linux-kernel, dtor, ming.lei

On Wed, Mar 04, 2015 at 03:25:10PM -0800, Charlie Mooney wrote:
> This patch adds an additional feature to the firmware_class.c module.
> To allow a unified method of specifying new firmware locations when
> drivers request a firmware binary to update their devices with, we
> have added the concept of a "fw override"
> 
> A fw override is a rule that matches firmware requests based on the
> name of the device requesting the fw and what the filename for the
> fw they are requesting is, and overrides their requests with a new
> value.
> 
> Overrides are set up by piping a description of the override into
> an attribute set up at /sys/class/firmware/fw_override.  The string
> should be a null-deliminited list of the firmware name you want to
> over ride, the new name to replace it with, and the device name that
> you want the override to apply to.   For example you could set up
> an override for a device called "my_device" so that any time it
> requests a firmware called "my_fw.bin" it instead gets "new_fw.bin"
> with the following command:
> 
> echo -e 'my_fw.bin\0new_fw.bin\0my_device\0' >
> 				/sys/class/firmware/fw_override

I hate to ask, but I have to, why do you need this?

Also, this needs to be documented somewhere, Documentation/ABI/ please?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-04 23:57 ` Greg KH
@ 2015-03-05  0:48   ` Dmitry Torokhov
  2015-03-05  8:14     ` Ming Lei
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2015-03-05  0:48 UTC (permalink / raw)
  To: Greg KH; +Cc: Charlie Mooney, linux-kernel, ming.lei

On Wed, Mar 04, 2015 at 03:57:09PM -0800, Greg KH wrote:
> On Wed, Mar 04, 2015 at 03:25:10PM -0800, Charlie Mooney wrote:
> > This patch adds an additional feature to the firmware_class.c module.
> > To allow a unified method of specifying new firmware locations when
> > drivers request a firmware binary to update their devices with, we
> > have added the concept of a "fw override"
> > 
> > A fw override is a rule that matches firmware requests based on the
> > name of the device requesting the fw and what the filename for the
> > fw they are requesting is, and overrides their requests with a new
> > value.
> > 
> > Overrides are set up by piping a description of the override into
> > an attribute set up at /sys/class/firmware/fw_override.  The string
> > should be a null-deliminited list of the firmware name you want to
> > over ride, the new name to replace it with, and the device name that
> > you want the override to apply to.   For example you could set up
> > an override for a device called "my_device" so that any time it
> > requests a firmware called "my_fw.bin" it instead gets "new_fw.bin"
> > with the following command:
> > 
> > echo -e 'my_fw.bin\0new_fw.bin\0my_device\0' >
> > 				/sys/class/firmware/fw_override
> 
> I hate to ask, but I have to, why do you need this?

We may have single driver serve several devices (a touchscreen and a
touchpad) that require different firmware/config file to function. We have
several options:

- modify the driver to allow changing firmware name from userspace - gets
  old when there are several drivers that need that;
- encode part numbers in the driver and request different firmware - not
  easily maintainable, still has an issue that same part might be used for
  different devices;
- replace the firmware file on disk before initiating firmware load - does
  not work well with read-only file systems;
- have udev supply different data - udev went out of fashion;
- have one central place (firmware loader) that users can control to
  override the names - this solution.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05  0:48   ` Dmitry Torokhov
@ 2015-03-05  8:14     ` Ming Lei
  2015-03-05 17:29       ` Charles Mooney
  0 siblings, 1 reply; 17+ messages in thread
From: Ming Lei @ 2015-03-05  8:14 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Greg KH, Charlie Mooney, Linux Kernel Mailing List

On Thu, Mar 5, 2015 at 8:48 AM, Dmitry Torokhov <dtor@chromium.org> wrote:
> On Wed, Mar 04, 2015 at 03:57:09PM -0800, Greg KH wrote:
>> On Wed, Mar 04, 2015 at 03:25:10PM -0800, Charlie Mooney wrote:
>> > This patch adds an additional feature to the firmware_class.c module.
>> > To allow a unified method of specifying new firmware locations when
>> > drivers request a firmware binary to update their devices with, we
>> > have added the concept of a "fw override"
>> >
>> > A fw override is a rule that matches firmware requests based on the
>> > name of the device requesting the fw and what the filename for the
>> > fw they are requesting is, and overrides their requests with a new
>> > value.
>> >
>> > Overrides are set up by piping a description of the override into
>> > an attribute set up at /sys/class/firmware/fw_override.  The string
>> > should be a null-deliminited list of the firmware name you want to
>> > over ride, the new name to replace it with, and the device name that
>> > you want the override to apply to.   For example you could set up
>> > an override for a device called "my_device" so that any time it
>> > requests a firmware called "my_fw.bin" it instead gets "new_fw.bin"
>> > with the following command:
>> >
>> > echo -e 'my_fw.bin\0new_fw.bin\0my_device\0' >
>> >                             /sys/class/firmware/fw_override
>>
>> I hate to ask, but I have to, why do you need this?
>
> We may have single driver serve several devices (a touchscreen and a
> touchpad) that require different firmware/config file to function. We have

I remember some wifi drivers have similar usage too: one driver serves
several functions, and each function need its own firmware. So looks
it is fine to use different firmware name for each different function, or
my understanding about the use case is wrong?

Thanks,
Ming Lei

> several options:
>
> - modify the driver to allow changing firmware name from userspace - gets
>   old when there are several drivers that need that;
> - encode part numbers in the driver and request different firmware - not
>   easily maintainable, still has an issue that same part might be used for
>   different devices;
> - replace the firmware file on disk before initiating firmware load - does
>   not work well with read-only file systems;
> - have udev supply different data - udev went out of fashion;
> - have one central place (firmware loader) that users can control to
>   override the names - this solution.
>
> Thanks.
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05  8:14     ` Ming Lei
@ 2015-03-05 17:29       ` Charles Mooney
  2015-03-05 17:39         ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Charles Mooney @ 2015-03-05 17:29 UTC (permalink / raw)
  To: Ming Lei, Dmitry Torokhov; +Cc: Greg KH, Linux Kernel Mailing List

Specifically this was motivated by a situation where we have one
device with a dual-sourced touchscreen. Both use the same driver but
have different hardware & fw. Our FW updating software therefore,
needs to be able to update with the correct FW and detect all this at
runtime due to a read-only partition (so moving the firmware binaries
around isn't really an option)
Here the device has only one touchscreen at a time, but it isn't known
until run-time which will be present.

So in this case the driver is serving the same function in each
situation (running a touchscreen) but may be working with different
hardware.

Another situation where I've personally wanted this functionality is
on a device that uses the same touch driver for both a touchscreen and
a touchpad on the same device. If the driver only grabs a copy of FW
from, say, /lib/firmware/touch_fw.bin then you either need to move the
firmware binaries around on disk to update either device, or have a
change like this that allows you to override which filename it loads.
The moving option is not viable if you're using a RO filesystem.

-Charlie

On Thu, Mar 5, 2015 at 12:14 AM Ming Lei <ming.lei@canonical.com> wrote:

On Thu, Mar 5, 2015 at 8:48 AM, Dmitry Torokhov <dtor@chromium.org> wrote:
> On Wed, Mar 04, 2015 at 03:57:09PM -0800, Greg KH wrote:
>> On Wed, Mar 04, 2015 at 03:25:10PM -0800, Charlie Mooney wrote:
>> > This patch adds an additional feature to the firmware_class.c module.
>> > To allow a unified method of specifying new firmware locations when
>> > drivers request a firmware binary to update their devices with, we
>> > have added the concept of a "fw override"
>> >
>> > A fw override is a rule that matches firmware requests based on the
>> > name of the device requesting the fw and what the filename for the
>> > fw they are requesting is, and overrides their requests with a new
>> > value.
>> >
>> > Overrides are set up by piping a description of the override into
>> > an attribute set up at /sys/class/firmware/fw_override. The string
>> > should be a null-deliminited list of the firmware name you want to
>> > over ride, the new name to replace it with, and the device name that
>> > you want the override to apply to. For example you could set up
>> > an override for a device called "my_device" so that any time it
>> > requests a firmware called "my_fw.bin" it instead gets "new_fw.bin"
>> > with the following command:
>> >
>> > echo -e 'my_fw.bin\0new_fw.bin\0my_device\0' >
>> > /sys/class/firmware/fw_override
>>
>> I hate to ask, but I have to, why do you need this?
>
> We may have single driver serve several devices (a touchscreen and a
> touchpad) that require different firmware/config file to function. We have

I remember some wifi drivers have similar usage too: one driver serves
several functions, and each function need its own firmware. So looks
it is fine to use different firmware name for each different function, or
my understanding about the use case is wrong?

Thanks,
Ming Lei

> several options:
>
> - modify the driver to allow changing firmware name from userspace - gets
> old when there are several drivers that need that;
> - encode part numbers in the driver and request different firmware - not
> easily maintainable, still has an issue that same part might be used for
> different devices;
> - replace the firmware file on disk before initiating firmware load - does
> not work well with read-only file systems;
> - have udev supply different data - udev went out of fashion;
> - have one central place (firmware loader) that users can control to
> override the names - this solution.
>
> Thanks.
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 17:29       ` Charles Mooney
@ 2015-03-05 17:39         ` Marcel Holtmann
  2015-03-05 17:52           ` Charles Mooney
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2015-03-05 17:39 UTC (permalink / raw)
  To: Charles Mooney
  Cc: Ming Lei, Dmitry Torokhov, Greg KH, Linux Kernel Mailing List

Hi Charles,

> Specifically this was motivated by a situation where we have one
> device with a dual-sourced touchscreen. Both use the same driver but
> have different hardware & fw. Our FW updating software therefore,
> needs to be able to update with the correct FW and detect all this at
> runtime due to a read-only partition (so moving the firmware binaries
> around isn't really an option)
> Here the device has only one touchscreen at a time, but it isn't known
> until run-time which will be present.
> 
> So in this case the driver is serving the same function in each
> situation (running a touchscreen) but may be working with different
> hardware.
> 
> Another situation where I've personally wanted this functionality is
> on a device that uses the same touch driver for both a touchscreen and
> a touchpad on the same device. If the driver only grabs a copy of FW
> from, say, /lib/firmware/touch_fw.bin then you either need to move the
> firmware binaries around on disk to update either device, or have a
> change like this that allows you to override which filename it loads.
> The moving option is not viable if you're using a RO filesystem.

what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.

Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.

Regards

Marcel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 17:39         ` Marcel Holtmann
@ 2015-03-05 17:52           ` Charles Mooney
  2015-03-05 18:13             ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Charles Mooney @ 2015-03-05 17:52 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Ming Lei, Dmitry Torokhov, Greg KH, Linux Kernel Mailing List

On Thu, Mar 5, 2015 at 9:39 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Charles,
>
>> Specifically this was motivated by a situation where we have one
>> device with a dual-sourced touchscreen. Both use the same driver but
>> have different hardware & fw. Our FW updating software therefore,
>> needs to be able to update with the correct FW and detect all this at
>> runtime due to a read-only partition (so moving the firmware binaries
>> around isn't really an option)
>> Here the device has only one touchscreen at a time, but it isn't known
>> until run-time which will be present.
>>
>> So in this case the driver is serving the same function in each
>> situation (running a touchscreen) but may be working with different
>> hardware.
>>
>> Another situation where I've personally wanted this functionality is
>> on a device that uses the same touch driver for both a touchscreen and
>> a touchpad on the same device. If the driver only grabs a copy of FW
>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>> firmware binaries around on disk to update either device, or have a
>> change like this that allows you to override which filename it loads.
>> The moving option is not viable if you're using a RO filesystem.
>
> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>
> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>
> Regards
>
> Marcel
>

I totally agree, this functionality is not novel.  We could have added
this feature into the specific driver in question, but then we will
have to do the same thing on all the other drivers we might want to do
this on.  I guess the real problem that this solves is by adding the
change here, it allows you to override firmware names for *any* driver
without having to duplicate the functionality in each one as they come
up.

For a specific instance, here at ChromiumOS we have devices that use
Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
all can encounter this issue.  The Atmel driver has a similar version
of this feature baked into it but the others don't.  We could add a
fw_filename attribute to each of these drivers, but then it would have
to be maintained across (at least) four drivers.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 17:52           ` Charles Mooney
@ 2015-03-05 18:13             ` Marcel Holtmann
  2015-03-05 18:20               ` Dmitry Torokhov
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2015-03-05 18:13 UTC (permalink / raw)
  To: Charles Mooney
  Cc: Ming Lei, Dmitry Torokhov, Greg KH, Linux Kernel Mailing List

Hi Charles,

>>> Specifically this was motivated by a situation where we have one
>>> device with a dual-sourced touchscreen. Both use the same driver but
>>> have different hardware & fw. Our FW updating software therefore,
>>> needs to be able to update with the correct FW and detect all this at
>>> runtime due to a read-only partition (so moving the firmware binaries
>>> around isn't really an option)
>>> Here the device has only one touchscreen at a time, but it isn't known
>>> until run-time which will be present.
>>> 
>>> So in this case the driver is serving the same function in each
>>> situation (running a touchscreen) but may be working with different
>>> hardware.
>>> 
>>> Another situation where I've personally wanted this functionality is
>>> on a device that uses the same touch driver for both a touchscreen and
>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>> firmware binaries around on disk to update either device, or have a
>>> change like this that allows you to override which filename it loads.
>>> The moving option is not viable if you're using a RO filesystem.
>> 
>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>> 
>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>> 
> 
> I totally agree, this functionality is not novel.  We could have added
> this feature into the specific driver in question, but then we will
> have to do the same thing on all the other drivers we might want to do
> this on.  I guess the real problem that this solves is by adding the
> change here, it allows you to override firmware names for *any* driver
> without having to duplicate the functionality in each one as they come
> up.
> 
> For a specific instance, here at ChromiumOS we have devices that use
> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
> all can encounter this issue.  The Atmel driver has a similar version
> of this feature baked into it but the others don't.  We could add a
> fw_filename attribute to each of these drivers, but then it would have
> to be maintained across (at least) four drivers.

what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need. However if that is the case, then this seems to be something that should be solved with device tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 18:13             ` Marcel Holtmann
@ 2015-03-05 18:20               ` Dmitry Torokhov
  2015-03-05 19:11                 ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2015-03-05 18:20 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Charles Mooney, Ming Lei, Dmitry Torokhov, Greg KH,
	Linux Kernel Mailing List

On Thu, Mar 5, 2015 at 10:13 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Charles,
>
>>>> Specifically this was motivated by a situation where we have one
>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>> have different hardware & fw. Our FW updating software therefore,
>>>> needs to be able to update with the correct FW and detect all this at
>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>> around isn't really an option)
>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>> until run-time which will be present.
>>>>
>>>> So in this case the driver is serving the same function in each
>>>> situation (running a touchscreen) but may be working with different
>>>> hardware.
>>>>
>>>> Another situation where I've personally wanted this functionality is
>>>> on a device that uses the same touch driver for both a touchscreen and
>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>> firmware binaries around on disk to update either device, or have a
>>>> change like this that allows you to override which filename it loads.
>>>> The moving option is not viable if you're using a RO filesystem.
>>>
>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>
>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>
>>
>> I totally agree, this functionality is not novel.  We could have added
>> this feature into the specific driver in question, but then we will
>> have to do the same thing on all the other drivers we might want to do
>> this on.  I guess the real problem that this solves is by adding the
>> change here, it allows you to override firmware names for *any* driver
>> without having to duplicate the functionality in each one as they come
>> up.
>>
>> For a specific instance, here at ChromiumOS we have devices that use
>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>> all can encounter this issue.  The Atmel driver has a similar version
>> of this feature baked into it but the others don't.  We could add a
>> fw_filename attribute to each of these drivers, but then it would have
>> to be maintained across (at least) four drivers.
>
> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.

Right, the drivers in question (bunch of input drivers such as
elan_ts, atmel_mxt_ts, etc) might not be able to determine the
"proper" configuration to load. Factories quite often swap
pin-compatible parts and want to use the same image. Also the parts
can be swapped out during RMA while keeping the same image. Userspace
is able to query magnitude of sources and make an informed decision
that it then communicates to the kernel.

> However if that is the case, then this seems to be something that should be solved with device tree.

Why are we making device tree a hard dependency here?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 18:20               ` Dmitry Torokhov
@ 2015-03-05 19:11                 ` Marcel Holtmann
  2015-03-05 19:14                   ` Dmitry Torokhov
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2015-03-05 19:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Charles Mooney, Ming Lei, Greg KH, Linux Kernel Mailing List

Hi Dmitry,

>>>>> Specifically this was motivated by a situation where we have one
>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>> needs to be able to update with the correct FW and detect all this at
>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>> around isn't really an option)
>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>> until run-time which will be present.
>>>>> 
>>>>> So in this case the driver is serving the same function in each
>>>>> situation (running a touchscreen) but may be working with different
>>>>> hardware.
>>>>> 
>>>>> Another situation where I've personally wanted this functionality is
>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>> firmware binaries around on disk to update either device, or have a
>>>>> change like this that allows you to override which filename it loads.
>>>>> The moving option is not viable if you're using a RO filesystem.
>>>> 
>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>> 
>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>> 
>>> 
>>> I totally agree, this functionality is not novel.  We could have added
>>> this feature into the specific driver in question, but then we will
>>> have to do the same thing on all the other drivers we might want to do
>>> this on.  I guess the real problem that this solves is by adding the
>>> change here, it allows you to override firmware names for *any* driver
>>> without having to duplicate the functionality in each one as they come
>>> up.
>>> 
>>> For a specific instance, here at ChromiumOS we have devices that use
>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>> all can encounter this issue.  The Atmel driver has a similar version
>>> of this feature baked into it but the others don't.  We could add a
>>> fw_filename attribute to each of these drivers, but then it would have
>>> to be maintained across (at least) four drivers.
>> 
>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
> 
> Right, the drivers in question (bunch of input drivers such as
> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
> "proper" configuration to load. Factories quite often swap
> pin-compatible parts and want to use the same image. Also the parts
> can be swapped out during RMA while keeping the same image. Userspace
> is able to query magnitude of sources and make an informed decision
> that it then communicates to the kernel.
> 
>> However if that is the case, then this seems to be something that should be solved with device tree.
> 
> Why are we making device tree a hard dependency here?

device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.

Regards

Marcel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 19:11                 ` Marcel Holtmann
@ 2015-03-05 19:14                   ` Dmitry Torokhov
  2015-03-05 22:12                     ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2015-03-05 19:14 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Dmitry Torokhov, Charles Mooney, Ming Lei, Greg KH,
	Linux Kernel Mailing List

Hi Marcel,

On Thu, Mar 5, 2015 at 11:11 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Dmitry,
>
>>>>>> Specifically this was motivated by a situation where we have one
>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>> around isn't really an option)
>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>> until run-time which will be present.
>>>>>>
>>>>>> So in this case the driver is serving the same function in each
>>>>>> situation (running a touchscreen) but may be working with different
>>>>>> hardware.
>>>>>>
>>>>>> Another situation where I've personally wanted this functionality is
>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>> change like this that allows you to override which filename it loads.
>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>
>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>
>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>
>>>>
>>>> I totally agree, this functionality is not novel.  We could have added
>>>> this feature into the specific driver in question, but then we will
>>>> have to do the same thing on all the other drivers we might want to do
>>>> this on.  I guess the real problem that this solves is by adding the
>>>> change here, it allows you to override firmware names for *any* driver
>>>> without having to duplicate the functionality in each one as they come
>>>> up.
>>>>
>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>> of this feature baked into it but the others don't.  We could add a
>>>> fw_filename attribute to each of these drivers, but then it would have
>>>> to be maintained across (at least) four drivers.
>>>
>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>
>> Right, the drivers in question (bunch of input drivers such as
>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>> "proper" configuration to load. Factories quite often swap
>> pin-compatible parts and want to use the same image. Also the parts
>> can be swapped out during RMA while keeping the same image. Userspace
>> is able to query magnitude of sources and make an informed decision
>> that it then communicates to the kernel.
>>
>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>
>> Why are we making device tree a hard dependency here?
>
> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.

And if I do not have device tree?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 19:14                   ` Dmitry Torokhov
@ 2015-03-05 22:12                     ` Marcel Holtmann
  2015-03-05 22:36                       ` Dmitry Torokhov
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2015-03-05 22:12 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Charles Mooney, Ming Lei, Greg KH, Linux Kernel Mailing List

Hi Dmitry,

>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>> around isn't really an option)
>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>> until run-time which will be present.
>>>>>>> 
>>>>>>> So in this case the driver is serving the same function in each
>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>> hardware.
>>>>>>> 
>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>> 
>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>> 
>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>> 
>>>>> 
>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>> this feature into the specific driver in question, but then we will
>>>>> have to do the same thing on all the other drivers we might want to do
>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>> change here, it allows you to override firmware names for *any* driver
>>>>> without having to duplicate the functionality in each one as they come
>>>>> up.
>>>>> 
>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>> of this feature baked into it but the others don't.  We could add a
>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>> to be maintained across (at least) four drivers.
>>>> 
>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>> 
>>> Right, the drivers in question (bunch of input drivers such as
>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>> "proper" configuration to load. Factories quite often swap
>>> pin-compatible parts and want to use the same image. Also the parts
>>> can be swapped out during RMA while keeping the same image. Userspace
>>> is able to query magnitude of sources and make an informed decision
>>> that it then communicates to the kernel.
>>> 
>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>> 
>>> Why are we making device tree a hard dependency here?
>> 
>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
> 
> And if I do not have device tree?

so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.

And even if you really don't, nothing is stopping you from adding device tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 22:12                     ` Marcel Holtmann
@ 2015-03-05 22:36                       ` Dmitry Torokhov
  2015-03-06  1:27                         ` Ming Lei
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2015-03-05 22:36 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Dmitry Torokhov, Charles Mooney, Ming Lei, Greg KH,
	Linux Kernel Mailing List

On Thu, Mar 5, 2015 at 2:12 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Dmitry,
>
>>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>>> around isn't really an option)
>>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>>> until run-time which will be present.
>>>>>>>>
>>>>>>>> So in this case the driver is serving the same function in each
>>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>>> hardware.
>>>>>>>>
>>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>>>
>>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>>>
>>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>>>
>>>>>>
>>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>>> this feature into the specific driver in question, but then we will
>>>>>> have to do the same thing on all the other drivers we might want to do
>>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>>> change here, it allows you to override firmware names for *any* driver
>>>>>> without having to duplicate the functionality in each one as they come
>>>>>> up.
>>>>>>
>>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>>> of this feature baked into it but the others don't.  We could add a
>>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>>> to be maintained across (at least) four drivers.
>>>>>
>>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>>>
>>>> Right, the drivers in question (bunch of input drivers such as
>>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>>> "proper" configuration to load. Factories quite often swap
>>>> pin-compatible parts and want to use the same image. Also the parts
>>>> can be swapped out during RMA while keeping the same image. Userspace
>>>> is able to query magnitude of sources and make an informed decision
>>>> that it then communicates to the kernel.
>>>>
>>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>>>
>>>> Why are we making device tree a hard dependency here?
>>>
>>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
>>
>> And if I do not have device tree?
>
> so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.
>
> And even if you really don't, nothing is stopping you from adding device tree.

We have ACPI (for example) and no, it is not 5.0.

Thanks,
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-05 22:36                       ` Dmitry Torokhov
@ 2015-03-06  1:27                         ` Ming Lei
  2015-03-06 22:50                           ` Charles Mooney
  0 siblings, 1 reply; 17+ messages in thread
From: Ming Lei @ 2015-03-06  1:27 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Marcel Holtmann, Charles Mooney, Greg KH, Linux Kernel Mailing List

On Fri, Mar 6, 2015 at 6:36 AM, Dmitry Torokhov <dtor@chromium.org> wrote:
> On Thu, Mar 5, 2015 at 2:12 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Dmitry,
>>
>>>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>>>> around isn't really an option)
>>>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>>>> until run-time which will be present.
>>>>>>>>>
>>>>>>>>> So in this case the driver is serving the same function in each
>>>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>>>> hardware.
>>>>>>>>>
>>>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>>>>
>>>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>>>>
>>>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>>>>
>>>>>>>
>>>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>>>> this feature into the specific driver in question, but then we will
>>>>>>> have to do the same thing on all the other drivers we might want to do
>>>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>>>> change here, it allows you to override firmware names for *any* driver
>>>>>>> without having to duplicate the functionality in each one as they come
>>>>>>> up.
>>>>>>>
>>>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>>>> of this feature baked into it but the others don't.  We could add a
>>>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>>>> to be maintained across (at least) four drivers.
>>>>>>
>>>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>>>>
>>>>> Right, the drivers in question (bunch of input drivers such as
>>>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>>>> "proper" configuration to load. Factories quite often swap
>>>>> pin-compatible parts and want to use the same image. Also the parts
>>>>> can be swapped out during RMA while keeping the same image. Userspace
>>>>> is able to query magnitude of sources and make an informed decision
>>>>> that it then communicates to the kernel.
>>>>>
>>>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>>>>
>>>>> Why are we making device tree a hard dependency here?
>>>>
>>>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
>>>
>>> And if I do not have device tree?
>>
>> so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.
>>
>> And even if you really don't, nothing is stopping you from adding device tree.
>
> We have ACPI (for example) and no, it is not 5.0.

It depends if the driver can determinate what the device is from
ACPI. If yes, you can just load the corresponding fw image
for the current device. Otherwise the ACPI can't help your problem.

Thanks,
Ming Lei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-06  1:27                         ` Ming Lei
@ 2015-03-06 22:50                           ` Charles Mooney
  2015-03-06 23:33                             ` Marcel Holtmann
  0 siblings, 1 reply; 17+ messages in thread
From: Charles Mooney @ 2015-03-06 22:50 UTC (permalink / raw)
  To: Ming Lei
  Cc: Dmitry Torokhov, Marcel Holtmann, Greg KH, Linux Kernel Mailing List

On Thu, Mar 5, 2015 at 5:27 PM, Ming Lei <ming.lei@canonical.com> wrote:
> On Fri, Mar 6, 2015 at 6:36 AM, Dmitry Torokhov <dtor@chromium.org> wrote:
>> On Thu, Mar 5, 2015 at 2:12 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>>> Hi Dmitry,
>>>
>>>>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>>>>> around isn't really an option)
>>>>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>>>>> until run-time which will be present.
>>>>>>>>>>
>>>>>>>>>> So in this case the driver is serving the same function in each
>>>>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>>>>> hardware.
>>>>>>>>>>
>>>>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>>>>>
>>>>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>>>>>
>>>>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>>>>> this feature into the specific driver in question, but then we will
>>>>>>>> have to do the same thing on all the other drivers we might want to do
>>>>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>>>>> change here, it allows you to override firmware names for *any* driver
>>>>>>>> without having to duplicate the functionality in each one as they come
>>>>>>>> up.
>>>>>>>>
>>>>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>>>>> of this feature baked into it but the others don't.  We could add a
>>>>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>>>>> to be maintained across (at least) four drivers.
>>>>>>>
>>>>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>>>>>
>>>>>> Right, the drivers in question (bunch of input drivers such as
>>>>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>>>>> "proper" configuration to load. Factories quite often swap
>>>>>> pin-compatible parts and want to use the same image. Also the parts
>>>>>> can be swapped out during RMA while keeping the same image. Userspace
>>>>>> is able to query magnitude of sources and make an informed decision
>>>>>> that it then communicates to the kernel.
>>>>>>
>>>>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>>>>>
>>>>>> Why are we making device tree a hard dependency here?
>>>>>
>>>>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
>>>>
>>>> And if I do not have device tree?
>>>
>>> so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.
>>>
>>> And even if you really don't, nothing is stopping you from adding device tree.
>>
>> We have ACPI (for example) and no, it is not 5.0.
>
> It depends if the driver can determinate what the device is from
> ACPI. If yes, you can just load the corresponding fw image
> for the current device. Otherwise the ACPI can't help your problem.
>
> Thanks,
> Ming Lei

We run into the situation where to very similar devices (all the same
HW models) need to ship with the same OS image.  One device may have a
pin-for-pin compatable, 2nd source version of some piece of hardware.
The device tree/etc is all the same because the two slighty different
parts are connected the same way (same i2c address, or similar).

The only way to tell them apart is by talking to the driver once the
device is already up and running.  In our motivating case it's reading
a sysfs attribute to get a manufacturer ID, but it could be anything
similar.

If you want to be able to put a different FW on these two very similar
devices (that can only be differentiated once they're up and running)
I think this might be the best way to do it -- apart from altering the
driver for every part that needs this kind of special treatment.

-Charlie

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-06 22:50                           ` Charles Mooney
@ 2015-03-06 23:33                             ` Marcel Holtmann
  2015-03-10 15:51                               ` Charles Mooney
  0 siblings, 1 reply; 17+ messages in thread
From: Marcel Holtmann @ 2015-03-06 23:33 UTC (permalink / raw)
  To: Charles Mooney
  Cc: Ming Lei, Dmitry Torokhov, Greg KH, Linux Kernel Mailing List

Hi Charles,

>>>>>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>>>>>> around isn't really an option)
>>>>>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>>>>>> until run-time which will be present.
>>>>>>>>>>> 
>>>>>>>>>>> So in this case the driver is serving the same function in each
>>>>>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>>>>>> hardware.
>>>>>>>>>>> 
>>>>>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>>>>>> 
>>>>>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>>>>>> 
>>>>>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>>>>>> this feature into the specific driver in question, but then we will
>>>>>>>>> have to do the same thing on all the other drivers we might want to do
>>>>>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>>>>>> change here, it allows you to override firmware names for *any* driver
>>>>>>>>> without having to duplicate the functionality in each one as they come
>>>>>>>>> up.
>>>>>>>>> 
>>>>>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>>>>>> of this feature baked into it but the others don't.  We could add a
>>>>>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>>>>>> to be maintained across (at least) four drivers.
>>>>>>>> 
>>>>>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>>>>>> 
>>>>>>> Right, the drivers in question (bunch of input drivers such as
>>>>>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>>>>>> "proper" configuration to load. Factories quite often swap
>>>>>>> pin-compatible parts and want to use the same image. Also the parts
>>>>>>> can be swapped out during RMA while keeping the same image. Userspace
>>>>>>> is able to query magnitude of sources and make an informed decision
>>>>>>> that it then communicates to the kernel.
>>>>>>> 
>>>>>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>>>>>> 
>>>>>>> Why are we making device tree a hard dependency here?
>>>>>> 
>>>>>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
>>>>> 
>>>>> And if I do not have device tree?
>>>> 
>>>> so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.
>>>> 
>>>> And even if you really don't, nothing is stopping you from adding device tree.
>>> 
>>> We have ACPI (for example) and no, it is not 5.0.
>> 
>> It depends if the driver can determinate what the device is from
>> ACPI. If yes, you can just load the corresponding fw image
>> for the current device. Otherwise the ACPI can't help your problem.
> 
> We run into the situation where to very similar devices (all the same
> HW models) need to ship with the same OS image.  One device may have a
> pin-for-pin compatable, 2nd source version of some piece of hardware.
> The device tree/etc is all the same because the two slighty different
> parts are connected the same way (same i2c address, or similar).
> 
> The only way to tell them apart is by talking to the driver once the
> device is already up and running.  In our motivating case it's reading
> a sysfs attribute to get a manufacturer ID, but it could be anything
> similar.
> 
> If you want to be able to put a different FW on these two very similar
> devices (that can only be differentiated once they're up and running)
> I think this might be the best way to do it -- apart from altering the
> driver for every part that needs this kind of special treatment.

if you can get a manufacturer ID over sysfs, then you can obviously pick the right firmware from within the driver. No need to play any tricks in userspace or the request_firmware interface.

Regards

Marcel


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] firmware_class: Add firmware filename overrides
  2015-03-06 23:33                             ` Marcel Holtmann
@ 2015-03-10 15:51                               ` Charles Mooney
  0 siblings, 0 replies; 17+ messages in thread
From: Charles Mooney @ 2015-03-10 15:51 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Ming Lei, Dmitry Torokhov, Greg KH, Linux Kernel Mailing List

Okay, thanks for the reviews.  My hope was to avoid having to add that
feature into each driver, but that's okay.
-Charlie

On Fri, Mar 6, 2015 at 3:33 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Charles,
>
>>>>>>>>>>>> Specifically this was motivated by a situation where we have one
>>>>>>>>>>>> device with a dual-sourced touchscreen. Both use the same driver but
>>>>>>>>>>>> have different hardware & fw. Our FW updating software therefore,
>>>>>>>>>>>> needs to be able to update with the correct FW and detect all this at
>>>>>>>>>>>> runtime due to a read-only partition (so moving the firmware binaries
>>>>>>>>>>>> around isn't really an option)
>>>>>>>>>>>> Here the device has only one touchscreen at a time, but it isn't known
>>>>>>>>>>>> until run-time which will be present.
>>>>>>>>>>>>
>>>>>>>>>>>> So in this case the driver is serving the same function in each
>>>>>>>>>>>> situation (running a touchscreen) but may be working with different
>>>>>>>>>>>> hardware.
>>>>>>>>>>>>
>>>>>>>>>>>> Another situation where I've personally wanted this functionality is
>>>>>>>>>>>> on a device that uses the same touch driver for both a touchscreen and
>>>>>>>>>>>> a touchpad on the same device. If the driver only grabs a copy of FW
>>>>>>>>>>>> from, say, /lib/firmware/touch_fw.bin then you either need to move the
>>>>>>>>>>>> firmware binaries around on disk to update either device, or have a
>>>>>>>>>>>> change like this that allows you to override which filename it loads.
>>>>>>>>>>>> The moving option is not viable if you're using a RO filesystem.
>>>>>>>>>>>
>>>>>>>>>>> what is the actual problem here? We have drivers that load multiple firmware files and we have drivers that pick a different firmware depending on some parameters it reads from the device.
>>>>>>>>>>>
>>>>>>>>>>> Seems this is all possible already at the moment with the existing framework. You just need to update the drivers to operate properly.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I totally agree, this functionality is not novel.  We could have added
>>>>>>>>>> this feature into the specific driver in question, but then we will
>>>>>>>>>> have to do the same thing on all the other drivers we might want to do
>>>>>>>>>> this on.  I guess the real problem that this solves is by adding the
>>>>>>>>>> change here, it allows you to override firmware names for *any* driver
>>>>>>>>>> without having to duplicate the functionality in each one as they come
>>>>>>>>>> up.
>>>>>>>>>>
>>>>>>>>>> For a specific instance, here at ChromiumOS we have devices that use
>>>>>>>>>> Atmel, Cypress, Synaptics, and Elan touchpads and touchscreens that
>>>>>>>>>> all can encounter this issue.  The Atmel driver has a similar version
>>>>>>>>>> of this feature baked into it but the others don't.  We could add a
>>>>>>>>>> fw_filename attribute to each of these drivers, but then it would have
>>>>>>>>>> to be maintained across (at least) four drivers.
>>>>>>>>>
>>>>>>>>> what I am hearing here is that you can not query the hardware and figure out which manufacturer it is and with that don't know which firmware you need.
>>>>>>>>
>>>>>>>> Right, the drivers in question (bunch of input drivers such as
>>>>>>>> elan_ts, atmel_mxt_ts, etc) might not be able to determine the
>>>>>>>> "proper" configuration to load. Factories quite often swap
>>>>>>>> pin-compatible parts and want to use the same image. Also the parts
>>>>>>>> can be swapped out during RMA while keeping the same image. Userspace
>>>>>>>> is able to query magnitude of sources and make an informed decision
>>>>>>>> that it then communicates to the kernel.
>>>>>>>>
>>>>>>>>> However if that is the case, then this seems to be something that should be solved with device tree.
>>>>>>>>
>>>>>>>> Why are we making device tree a hard dependency here?
>>>>>>>
>>>>>>> device tree is suppose to describe the hardware in your devices. If you can not determinate your hardware by enumeration or other means, then it should be done via device tree. Seems the perfect fit here.
>>>>>>
>>>>>> And if I do not have device tree?
>>>>>
>>>>> so if you do not have an enumeration method for your hardware, then I assume you most likely have device tree in one form or another.
>>>>>
>>>>> And even if you really don't, nothing is stopping you from adding device tree.
>>>>
>>>> We have ACPI (for example) and no, it is not 5.0.
>>>
>>> It depends if the driver can determinate what the device is from
>>> ACPI. If yes, you can just load the corresponding fw image
>>> for the current device. Otherwise the ACPI can't help your problem.
>>
>> We run into the situation where to very similar devices (all the same
>> HW models) need to ship with the same OS image.  One device may have a
>> pin-for-pin compatable, 2nd source version of some piece of hardware.
>> The device tree/etc is all the same because the two slighty different
>> parts are connected the same way (same i2c address, or similar).
>>
>> The only way to tell them apart is by talking to the driver once the
>> device is already up and running.  In our motivating case it's reading
>> a sysfs attribute to get a manufacturer ID, but it could be anything
>> similar.
>>
>> If you want to be able to put a different FW on these two very similar
>> devices (that can only be differentiated once they're up and running)
>> I think this might be the best way to do it -- apart from altering the
>> driver for every part that needs this kind of special treatment.
>
> if you can get a manufacturer ID over sysfs, then you can obviously pick the right firmware from within the driver. No need to play any tricks in userspace or the request_firmware interface.
>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-03-10 15:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-04 23:25 [PATCH] firmware_class: Add firmware filename overrides Charlie Mooney
2015-03-04 23:57 ` Greg KH
2015-03-05  0:48   ` Dmitry Torokhov
2015-03-05  8:14     ` Ming Lei
2015-03-05 17:29       ` Charles Mooney
2015-03-05 17:39         ` Marcel Holtmann
2015-03-05 17:52           ` Charles Mooney
2015-03-05 18:13             ` Marcel Holtmann
2015-03-05 18:20               ` Dmitry Torokhov
2015-03-05 19:11                 ` Marcel Holtmann
2015-03-05 19:14                   ` Dmitry Torokhov
2015-03-05 22:12                     ` Marcel Holtmann
2015-03-05 22:36                       ` Dmitry Torokhov
2015-03-06  1:27                         ` Ming Lei
2015-03-06 22:50                           ` Charles Mooney
2015-03-06 23:33                             ` Marcel Holtmann
2015-03-10 15:51                               ` Charles Mooney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).