* [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 12:52 [PATCH 0/3 v2] Expose firmware paths via procfs Dimitris Papastamos
@ 2012-10-23 12:52 ` Dimitris Papastamos
2012-10-23 13:08 ` Ming Lei
2012-10-23 12:52 ` [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths Dimitris Papastamos
2012-10-23 12:52 ` [PATCH 3/3 v2] firmware: Factor out code to add paths to the firmware path list Dimitris Papastamos
2 siblings, 1 reply; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-23 12:52 UTC (permalink / raw)
To: linux-kernel; +Cc: Greg Kroah-Hartman
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 12:52 ` [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list Dimitris Papastamos
@ 2012-10-23 13:08 ` Ming Lei
2012-10-23 13:29 ` Dimitris Papastamos
0 siblings, 1 reply; 12+ messages in thread
From: Ming Lei @ 2012-10-23 13:08 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 13:08 ` Ming Lei
@ 2012-10-23 13:29 ` Dimitris Papastamos
2012-10-23 13:37 ` Ming Lei
0 siblings, 1 reply; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-23 13:29 UTC (permalink / raw)
To: Ming Lei; +Cc: linux-kernel, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 13:29 ` Dimitris Papastamos
@ 2012-10-23 13:37 ` Ming Lei
2012-10-23 13:44 ` Dimitris Papastamos
0 siblings, 1 reply; 12+ messages in thread
From: Ming Lei @ 2012-10-23 13:37 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 13:37 ` Ming Lei
@ 2012-10-23 13:44 ` Dimitris Papastamos
2012-10-23 14:37 ` Ming Lei
0 siblings, 1 reply; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-23 13:44 UTC (permalink / raw)
To: Ming Lei; +Cc: linux-kernel, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list
2012-10-23 13:44 ` Dimitris Papastamos
@ 2012-10-23 14:37 ` Ming Lei
0 siblings, 0 replies; 12+ messages in thread
From: Ming Lei @ 2012-10-23 14:37 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel, Greg Kroah-Hartman
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths
2012-10-23 12:52 [PATCH 0/3 v2] Expose firmware paths via procfs Dimitris Papastamos
2012-10-23 12:52 ` [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list Dimitris Papastamos
@ 2012-10-23 12:52 ` Dimitris Papastamos
2012-10-25 18:48 ` Greg Kroah-Hartman
2012-10-23 12:52 ` [PATCH 3/3 v2] firmware: Factor out code to add paths to the firmware path list Dimitris Papastamos
2 siblings, 1 reply; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-23 12:52 UTC (permalink / raw)
To: linux-kernel; +Cc: Greg Kroah-Hartman
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths
2012-10-23 12:52 ` [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths Dimitris Papastamos
@ 2012-10-25 18:48 ` Greg Kroah-Hartman
2012-10-26 10:07 ` Dimitris Papastamos
0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2012-10-25 18:48 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths
2012-10-25 18:48 ` Greg Kroah-Hartman
@ 2012-10-26 10:07 ` Dimitris Papastamos
2012-10-26 23:56 ` Greg Kroah-Hartman
0 siblings, 1 reply; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-26 10:07 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths
2012-10-26 10:07 ` Dimitris Papastamos
@ 2012-10-26 23:56 ` Greg Kroah-Hartman
0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2012-10-26 23:56 UTC (permalink / raw)
To: Dimitris Papastamos; +Cc: linux-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3 v2] firmware: Factor out code to add paths to the firmware path list
2012-10-23 12:52 [PATCH 0/3 v2] Expose firmware paths via procfs Dimitris Papastamos
2012-10-23 12:52 ` [PATCH 1/3 v2] firmware: Convert firmware path setup from an array to a list Dimitris Papastamos
2012-10-23 12:52 ` [PATCH 2/3 v2] firmware: Add /proc/firmware_path entry to list the firmware paths Dimitris Papastamos
@ 2012-10-23 12:52 ` Dimitris Papastamos
2 siblings, 0 replies; 12+ messages in thread
From: Dimitris Papastamos @ 2012-10-23 12:52 UTC (permalink / raw)
To: linux-kernel; +Cc: Greg Kroah-Hartman
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
^ permalink raw reply related [flat|nested] 12+ messages in thread