Skip to contentSkip to conversation

Expose firmware paths via procfs

PATCH In review Dimitris Papastamos 3 participants 12 messages Oct 23, 2012 – Oct 26, 2012 Driver core

lore
Share & tools
lore
#0 lore raw

v2 cover Expose firmware paths via procfs

Hi Greg, I've rebased these on top of linux-next as you requested.

Dimitris Papastamos (3):
firmware: Convert firmware path setup from an array to a list
firmware: Add /proc/firmware_path entry to list the firmware paths
firmware: Factor out code to add paths to the firmware path list

 drivers/base/firmware_class.c | 137 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 128 insertions(+), 9 deletions(-)
Signature · 1 line
1.8.0
#1 lore raw

PATCH v2 1/3 firmware: Convert firmware path setup from an array to a list

In preparation to support dynamic listing/updating of firmware paths via procfs, this patch converts the firmware path configuration from an array to a list.

---
 drivers/base/firmware_class.c | 74 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 9 deletions(-)
drivers/base/firmware_class.c +65 -9
index 8945f4e..d76152b 100644
@@ -36,6 +36,13 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
 MODULE_LICENSE("GPL");
 
+struct fw_path_rec {
+	const char *name;
+	struct list_head list;
+};
+
+static LIST_HEAD(fw_path_list);
+
 /* Builtin firmware support */
 
 #ifdef CONFIG_FW_LOADER
@@ -267,12 +274,6 @@ static void fw_free_buf(struct firmware_buf *buf)
 }
 
 /* direct firmware loading support */
-static const char *fw_path[] = {
-	"/lib/firmware/updates/" UTS_RELEASE,
-	"/lib/firmware/updates",
-	"/lib/firmware/" UTS_RELEASE,
-	"/lib/firmware"
-};
 
 /* Don't inline this: 'struct kstat' is biggish */
 static noinline long fw_file_size(struct file *file)
@@ -309,13 +310,13 @@ static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf
 
 static bool fw_get_filesystem_firmware(struct firmware_buf *buf)
 {
-	int i;
 	bool success = false;
 	char *path = __getname();
+	struct fw_path_rec *fwp;
 
-	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+	list_for_each_entry(fwp, &fw_path_list, list) {
 		struct file *file;
-		snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id);
+		snprintf(path, PATH_MAX, "%s/%s", fwp->name, buf->fw_id);
 
 		file = filp_open(path, O_RDONLY, 0);
 		if (IS_ERR(file))
@@ -1423,6 +1424,50 @@ static int fw_cache_piggyback_on_request(const char *name)
 }
 #endif
 
+static void fw_free_path_list(void)
+{
+	struct fw_path_rec *fwp;
+
+	while (!list_empty(&fw_path_list)) {
+		fwp = list_first_entry(&fw_path_list,
+				       struct fw_path_rec,
+				       list);
+		kfree(fwp->name);
+		list_del(&fwp->list);
+		kfree(fwp);
+	}
+}
+
+static int fw_populate_path_list(void)
+{
+	int i;
+	struct fw_path_rec *fwp;
+	static const char *fw_path[] = {
+		"/lib/firmware/updates/" UTS_RELEASE,
+		"/lib/firmware/updates",
+		"/lib/firmware/" UTS_RELEASE,
+		"/lib/firmware"
+	};
+
+	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+		fwp = kmalloc(sizeof(*fwp), GFP_KERNEL);
+		if (!fwp)
+			goto err_fwp_alloc;
+		fwp->name = kstrdup(fw_path[i], GFP_KERNEL);
+		if (!fwp->name)
+			goto err_fwp_name_alloc;
+		list_add_tail(&fwp->list, &fw_path_list);
+	}
+
+	return 0;
+
+err_fwp_name_alloc:
+	kfree(fwp);
+err_fwp_alloc:
+	fw_free_path_list();
+	return -ENOMEM;
+}
+
 static void __init fw_cache_init(void)
 {
 	spin_lock_init(&fw_cache.lock);
@@ -1445,7 +1490,17 @@ static void __init fw_cache_init(void)
 
 static int __init firmware_class_init(void)
 {
+	int ret;
+
 	fw_cache_init();
+
+	ret = fw_populate_path_list();
+	if (ret < 0) {
+		pr_err("%s: Failed to populate firmware path list: %d\n",
+		       __func__, ret);
+		return ret;
+	}
+
 	return class_register(&firmware_class);
 }
 
@@ -1455,6 +1510,7 @@ static void __exit firmware_class_exit(void)
 	unregister_syscore_ops(&fw_syscore_ops);
 	unregister_pm_notifier(&fw_cache.pm_notify);
 #endif
+	fw_free_path_list();
 	class_unregister(&firmware_class);
 }
 
Signature · 1 line
1.8.0
#2 lore raw

Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list

Quoted 3 lines · Dimitris Papastamos

In preparation to support dynamic listing/updating of firmware paths via procfs, this patch converts the firmware path configuration from an array to a list.

I remembered that I have questioned on dynamic listing/updating of firmware via procfs, looks you don't reply, :-(

      http://marc.info/?l=linux-kernel&m=134988208617179&w=2

IMO, it is not a good idea to update firmware path dynamically via proc because it may be too late to update a dynamic search path via /proc and drivers need request firmware just after rootfs is mounted.

kernel parameter should be a good way to pass one customerized path.

Thanks,

Signature · 1 line
Ming Lei
#3 lore raw

Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list

Quoted 11 lines · Ming Lei
Quoted 3 lines · Dimitris Papastamos

In preparation to support dynamic listing/updating of firmware paths via procfs, this patch converts the firmware path configuration from an array to a list.

I remembered that I have questioned on dynamic listing/updating of firmware via procfs, looks you don't reply, :-(

      http://marc.info/?l=linux-kernel&m=134988208617179&w=2
Quoted 10 lines

IMO, it is not a good idea to update firmware path dynamically via proc because it may be too late to update a dynamic search path via /proc and drivers need request firmware just after rootfs is mounted.

kernel parameter should be a good way to pass one customerized path.

Thanks,

Signature · 1 line
Ming Lei

Hi sorry for being unclear, I mentioned in the old thread that I've not had time to fix this properly (or think about a proper fix), Greg requested a rebase on top of linux-next, however, at least for firmware path listing.

Should I amend the commit description so it is not misleading and remove the 'updating' part?

Thanks,
Dimitris

#4 lore raw

Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list

Quoted 8 lines · Dimitris Papastamos

Hi sorry for being unclear, I mentioned in the old thread that I've not had time to fix this properly (or think about a proper fix), Greg requested a rebase on top of linux-next, however, at least for firmware path listing.

Should I amend the commit description so it is not misleading and remove the 'updating' part?

In fact, I also questioned on path listing too in last thread too.

Thanks,

Signature · 1 line
Ming Lei
#5 lore raw

Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list

Quoted 9 lines · Ming Lei
Quoted 6 lines · Dimitris Papastamos

Hi sorry for being unclear, I mentioned in the old thread that I've not had time to fix this properly (or think about a proper fix), Greg requested a rebase on top of linux-next, however, at least for firmware path listing.

Quoted 4 lines
Quoted 2 lines

Should I amend the commit description so it is not misleading and remove the 'updating' part?

In fact, I also questioned on path listing too in last thread too.

I don't currently have a use-case for this, so not sure how useful it is to list the default fw paths.

Thanks,
Dimitris

#6 lore raw

Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list

Quoted 3 lines · Dimitris Papastamos

I don't currently have a use-case for this, so not sure how useful it is to list the default fw paths.

OK, got it, thanks for your effort.

Greg, so could we just hold on until it is useful?

Thanks,

Signature · 1 line
Ming Lei
#7 lore raw

PATCH v2 2/3 firmware: Add /proc/firmware_path entry to list the firmware paths

In reply to Dimitris Papastamos

This patch provides the aforementioned procfs file that lists the default firmware paths that are used during firmware lookup.

The file contains a white space separated list of paths.

There will be another patch on top of this that adds the functionality to modify the paths at runtime.

---
 drivers/base/firmware_class.c | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
drivers/base/firmware_class.c +55 -0
index d76152b..322fff3 100644
@@ -27,6 +27,8 @@
 #include <linux/pm.h>
 #include <linux/suspend.h>
 #include <linux/syscore_ops.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
 
 #include <generated/utsrelease.h>
 
@@ -1468,6 +1470,56 @@ err_fwp_alloc:
 	return -ENOMEM;
 }
 
+static void *fw_path_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	return seq_list_start_head(&fw_path_list, *pos);
+}
+
+static void *fw_path_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &fw_path_list, pos);
+}
+
+static int fw_path_seq_show(struct seq_file *seq, void *v)
+{
+	const struct fw_path_rec *fwp;
+	struct list_head *l = v;
+
+	if (l == &fw_path_list)
+		return 0;
+	fwp = list_entry(v, struct fw_path_rec, list);
+	seq_puts(seq, fwp->name);
+	if (l->next != &fw_path_list)
+		seq_putc(seq, ' ');
+	else
+		seq_putc(seq, '\n');
+	return 0;
+}
+
+static void fw_path_seq_stop(struct seq_file *seq, void *v)
+{
+}
+
+static const struct seq_operations fw_path_ops = {
+	.start = fw_path_seq_start,
+	.next = fw_path_seq_next,
+	.stop = fw_path_seq_stop,
+	.show = fw_path_seq_show,
+};
+
+static int fw_path_open(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &fw_path_ops);
+}
+
+static const struct file_operations fw_path_seq_fops = {
+	.owner = THIS_MODULE,
+	.open = fw_path_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+
 static void __init fw_cache_init(void)
 {
 	spin_lock_init(&fw_cache.lock);
@@ -1501,6 +1553,8 @@ static int __init firmware_class_init(void)
 		return ret;
 	}
 
+	proc_create("firmware_path", S_IRUGO, NULL, &fw_path_seq_fops);
+
 	return class_register(&firmware_class);
 }
 
@@ -1510,6 +1564,7 @@ static void __exit firmware_class_exit(void)
 	unregister_syscore_ops(&fw_syscore_ops);
 	unregister_pm_notifier(&fw_cache.pm_notify);
 #endif
+	remove_proc_entry("firmware_path", NULL);
 	fw_free_path_list();
 	class_unregister(&firmware_class);
 }
Signature · 1 line
1.8.0
#8 lore raw

Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths

Quoted 4 lines · Dimitris Papastamos

This patch provides the aforementioned procfs file that lists the default firmware paths that are used during firmware lookup.

The file contains a white space separated list of paths.

Paths can have whitespaces :(

How about using the "universal" path seperator of ':' that shells are used to using? Yeah, it gets messy if you have a ':' in a path, but from what I can tell, only the openSUSE build system does looney things like that.

Quoted 2 lines

There will be another patch on top of this that adds the functionality to modify the paths at runtime.

Did I miss that patch somewhere?

thanks,

greg k-h

#9 lore raw

Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths

Quoted 4 lines · Greg Kroah-Hartman
Quoted 3 lines · Dimitris Papastamos

This patch provides the aforementioned procfs file that lists the default firmware paths that are used during firmware lookup.

Quoted 8 lines
Quoted 1 line

The file contains a white space separated list of paths.

Paths can have whitespaces :(

How about using the "universal" path seperator of ':' that shells are used to using? Yeah, it gets messy if you have a ':' in a path, but from what I can tell, only the openSUSE build system does looney things like that.

Sure I can do that, however, could you comment on Ming's comments? Is this patchset something we find useful at the moment as regards listing and/or updating the firmware path list.

Quoted 4 lines
Quoted 2 lines

There will be another patch on top of this that adds the functionality to modify the paths at runtime.

Did I miss that patch somewhere?

That patch is here:
http://opensource.wolfsonmicro.com/~dp/patches/firmware/0004-firmware-Add-write-support-to-proc-fw_path.patch

However I've not rebased that patch on top of linux-next and I did not send it for review as it seems that it may be a more sensible solution to pass this as a kernel parameter.

Let me know what you think and I'll squeeze some time this weekend to look at this.

Thanks,
Dimitris

#10 lore raw

Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths

Quoted 9 lines · Dimitris Papastamos
Quoted 8 lines · Greg Kroah-Hartman
Quoted 4 lines · Dimitris Papastamos

This patch provides the aforementioned procfs file that lists the default firmware paths that are used during firmware lookup.

The file contains a white space separated list of paths.

Paths can have whitespaces :(

Quoted 8 lines
Quoted 4 lines

How about using the "universal" path seperator of ':' that shells are used to using? Yeah, it gets messy if you have a ':' in a path, but from what I can tell, only the openSUSE build system does looney things like that.

Sure I can do that, however, could you comment on Ming's comments? Is this patchset something we find useful at the moment as regards listing and/or updating the firmware path list.

I really don't remember anymore, sorry. That was a few hunderd or so patches ago that I've reviewed...

greg k-h

#11 lore raw

PATCH v2 3/3 firmware: Factor out code to add paths to the firmware path list

In reply to Dimitris Papastamos
---
 drivers/base/firmware_class.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)
drivers/base/firmware_class.c +22 -14
index 322fff3..cf4aa5f 100644
@@ -1426,6 +1426,22 @@ static int fw_cache_piggyback_on_request(const char *name)
 }
 #endif
 
+static int fw_add_path_to_list(const char *path)
+{
+	struct fw_path_rec *fwp;
+
+	fwp = kmalloc(sizeof(*fwp), GFP_KERNEL);
+	if (!fwp)
+		return -ENOMEM;
+	fwp->name = kstrdup(path, GFP_KERNEL);
+	if (!fwp->name) {
+		kfree(fwp);
+		return -ENOMEM;
+	}
+	list_add_tail(&fwp->list, &fw_path_list);
+	return 0;
+}
+
 static void fw_free_path_list(void)
 {
 	struct fw_path_rec *fwp;
@@ -1443,7 +1459,7 @@ static void fw_free_path_list(void)
 static int fw_populate_path_list(void)
 {
 	int i;
-	struct fw_path_rec *fwp;
+	int ret;
 	static const char *fw_path[] = {
 		"/lib/firmware/updates/" UTS_RELEASE,
 		"/lib/firmware/updates",
@@ -1452,22 +1468,14 @@ static int fw_populate_path_list(void)
 	};
 
 	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
-		fwp = kmalloc(sizeof(*fwp), GFP_KERNEL);
-		if (!fwp)
-			goto err_fwp_alloc;
-		fwp->name = kstrdup(fw_path[i], GFP_KERNEL);
-		if (!fwp->name)
-			goto err_fwp_name_alloc;
-		list_add_tail(&fwp->list, &fw_path_list);
+		ret = fw_add_path_to_list(fw_path[i]);
+		if (ret < 0) {
+			fw_free_path_list();
+			return ret;
+		}
 	}
 
 	return 0;
-
-err_fwp_name_alloc:
-	kfree(fwp);
-err_fwp_alloc:
-	fw_free_path_list();
-	return -ENOMEM;
 }
 
 static void *fw_path_seq_start(struct seq_file *seq, loff_t *pos)
Signature · 1 line
1.8.0