[PATCH 0/2] Expose firmware paths via procfs

[PATCH 0/2] Expose firmware paths via procfs

[Dimitris Papastamos]

Hi all, sorry missed the relevant CCs before.  This patch set
adds support for exposing the firmware paths via procfs.  I've added
an entry named /proc/fw_path which is a whitespace separated list
of firmware paths.

Once I have some time I hope to also send out a patch that allows
the user to dynamically configure these paths.

Dimitris Papastamos (2):
  firmware: Convert firmware path setup from an array to a list
  firmware: Add /proc/fw_path entry to list the firmware paths

 drivers/base/firmware_class.c | 127 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 119 insertions(+), 8 deletions(-)

-- 
1.7.12.2

[PATCH 1/2] 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 | 72 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 8 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8154145..2153eab 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -36,13 +36,13 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
 MODULE_LICENSE("GPL");
 
-static const char *fw_path[] = {
-	"/lib/firmware/updates/" UTS_RELEASE,
-	"/lib/firmware/updates",
-	"/lib/firmware/" UTS_RELEASE,
-	"/lib/firmware"
+struct fw_path_rec {
+	const char *name;
+	struct list_head list;
 };
 
+static LIST_HEAD(fw_path_list);
+
 /* Don't inline this: 'struct kstat' is biggish */
 static noinline long fw_file_size(struct file *file)
 {
@@ -78,13 +78,14 @@ static bool fw_read_file_contents(struct file *file, struct firmware *fw)
 
 static bool fw_get_filesystem_firmware(struct firmware *fw, const char *name)
 {
-	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], name);
+		snprintf(path, PATH_MAX, "%s/%s", fwp->name,
+			 name);
 
 		file = filp_open(path, O_RDONLY, 0);
 		if (IS_ERR(file))
@@ -1385,6 +1386,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);
@@ -1409,7 +1454,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);
 }
 
@@ -1419,6 +1474,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.7.12.2

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

[Greg Kroah-Hartman]

On Wed, Oct 10, 2012 at 11:56:24AM +0100, Dimitris Papastamos 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.
> 
> Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

This series doesn't apply anymore due to changes in this code from Ming.
Can you redo it against the my staging-linus branch, or the next
linux-next release, so that I can apply them?

thanks,

greg k-h

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

[Dimitris Papastamos]

On Mon, Oct 22, 2012 at 08:42:02AM -0700, Greg Kroah-Hartman wrote:
> On Wed, Oct 10, 2012 at 11:56:24AM +0100, Dimitris Papastamos 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.
> > 
> > Signed-off-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
> 
> This series doesn't apply anymore due to changes in this code from Ming.
> Can you redo it against the my staging-linus branch, or the next
> linux-next release, so that I can apply them?
> 
> thanks,
> 
> greg k-h

Hi, I've not had time to implement this properly as discussed in the thread.
Should the current changes on top of Ming's changes nevertheless?

Thanks,
Dimitris

[PATCH 2/2] firmware: Add /proc/fw_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 2153eab..22cef4d 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>
 
@@ -1430,6 +1432,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);
@@ -1465,6 +1517,8 @@ static int __init firmware_class_init(void)
 		return ret;
 	}
 
+	proc_create("fw_path", S_IRUGO, NULL, &fw_path_seq_fops);
+
 	return class_register(&firmware_class);
 }
 
@@ -1474,6 +1528,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("fw_path", NULL);
 	fw_free_path_list();
 	class_unregister(&firmware_class);
 }
-- 
1.7.12.2

Re: [PATCH 2/2] firmware: Add /proc/fw_path entry to list the firmware paths

[Greg Kroah-Hartman]

On Wed, Oct 10, 2012 at 11:56:25AM +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.
> 
> 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>

What about /proc/sys/kernel/firmware_path instead?  Isn't that a better
place for this?

greg k-h

Re: [PATCH 2/2] firmware: Add /proc/fw_path entry to list the firmware paths

[Josh Boyer]

On Wed, Oct 10, 2012 at 9:36 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Oct 10, 2012 at 11:56:25AM +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.
>>
>> 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>
>
> What about /proc/sys/kernel/firmware_path instead?  Isn't that a better
> place for this?

Without painting the bikeshed too many colors, I was wondering if /proc
was the right place for this at all.  Perhaps /sys/kernel/firmware_path
or /sys/firmware/firmware_path?

josh

Re: [PATCH 2/2] firmware: Add /proc/fw_path entry to list the firmware paths

[Dimitris Papastamos]

On Wed, Oct 10, 2012 at 10:36:55PM +0900, Greg Kroah-Hartman wrote:
> On Wed, Oct 10, 2012 at 11:56:25AM +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.
> > 
> > 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>
> 
> What about /proc/sys/kernel/firmware_path instead?  Isn't that a better
> place for this?
> 
> greg k-h

Yes that makes more sense to me.

Thanks,
Dimitris

Re: [PATCH 2/2] firmware: Add /proc/fw_path entry to list the firmware paths

[Dimitris Papastamos]

On Wed, Oct 10, 2012 at 10:36:55PM +0900, Greg Kroah-Hartman wrote:
> On Wed, Oct 10, 2012 at 11:56:25AM +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.
> > 
> > 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>
> 
> What about /proc/sys/kernel/firmware_path instead?  Isn't that a better
> place for this?
> 
> greg k-h

Btw, a primitive set of patches along with write support are stashed
at http://opensource.wolfsonmicro.com/~dp/patches/firmware/ - will clean
these up and place the file at /proc/sys/kernel/firmware_path and
will send out a new version of the patch set.

Thanks,
Dimitris

Re: [PATCH 0/2] Expose firmware paths via procfs

[Ming Lei]

On Wed, Oct 10, 2012 at 6:56 PM, Dimitris Papastamos
<dp@opensource.wolfsonmicro.com> wrote:
> Hi all, sorry missed the relevant CCs before.  This patch set
> adds support for exposing the firmware paths via procfs.  I've added
> an entry named /proc/fw_path which is a whitespace separated list
> of firmware paths.

Looks the paths are hard-coded in udev and mdev, which don't
export these paths to users , so I am wondering if users are interested
in the information.  But it is surely better to document the default
search paths.

Do you have some good use case about retrieving the info
at runtime?

>
> Once I have some time I hope to also send out a patch that allows
> the user to dynamically configure these paths.

IMO, kernel parameter should be an appropriate way to configure
the path, and the way of /proc or /sys may be a bit late.


Thanks,
--
Ming Lei