[PATCH 0/3 v2] Expose firmware paths via procfs

[PATCH 0/3 v2] Expose firmware paths via procfs

[Dimitris Papastamos]

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(-)

-- 
1.8.0

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

[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.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/firmware_class.c | 74 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 65 insertions(+), 9 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8945f4e..d76152b 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -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);
 }
 
-- 
1.8.0

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

[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.

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/firmware_class.c | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index d76152b..322fff3 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -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);
 }
-- 
1.8.0

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

[Dimitris Papastamos]

Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/firmware_class.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 322fff3..cf4aa5f 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -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)
-- 
1.8.0

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

[Ming Lei]

On Tue, Oct 23, 2012 at 8:52 PM, Dimitris Papastamos
<dp@opensource.wolfsonmicro.com> wrote:
> 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,
-- 
Ming Lei

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

[Dimitris Papastamos]

On Tue, Oct 23, 2012 at 09:08:50PM +0800, Ming Lei wrote:
> On Tue, Oct 23, 2012 at 8:52 PM, Dimitris Papastamos
> <dp@opensource.wolfsonmicro.com> wrote:
> > 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,
> -- 
> 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

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

[Ming Lei]

On Tue, Oct 23, 2012 at 9:29 PM, Dimitris Papastamos
<dp@opensource.wolfsonmicro.com> wrote:

>
> 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,
-- 
Ming Lei

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

[Dimitris Papastamos]

On Tue, Oct 23, 2012 at 09:37:24PM +0800, Ming Lei wrote:
> On Tue, Oct 23, 2012 at 9:29 PM, Dimitris Papastamos
> <dp@opensource.wolfsonmicro.com> wrote:
> 
> >
> > 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.

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

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

[Ming Lei]

On Tue, Oct 23, 2012 at 9:44 PM, Dimitris Papastamos
<dp@opensource.wolfsonmicro.com> wrote:
>
> 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,
-- 
Ming Lei

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

[Greg Kroah-Hartman]

On Tue, Oct 23, 2012 at 01:52:55PM +0100, Dimitris Papastamos wrote:
> 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.

> 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

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

[Dimitris Papastamos]

On Thu, Oct 25, 2012 at 11:48:33AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Oct 23, 2012 at 01:52:55PM +0100, Dimitris Papastamos wrote:
> > 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.

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.

> > 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

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

[Greg Kroah-Hartman]

On Fri, Oct 26, 2012 at 11:07:31AM +0100, Dimitris Papastamos wrote:
> On Thu, Oct 25, 2012 at 11:48:33AM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Oct 23, 2012 at 01:52:55PM +0100, Dimitris Papastamos wrote:
> > > 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.
> 
> 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