Expose firmware paths via procfs
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
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
@@ -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_bufstatic 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
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
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
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
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
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
PATCH v2 2/3 firmware: Add /proc/firmware_path entry to list the firmware paths
In reply to Dimitris PapastamosThis 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
@@ -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
Greg Kroah-Hartman
maintainerRe: [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
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
Greg Kroah-Hartman
maintainerRe: [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
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
@@ -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