[PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Victor Hugo]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

Hi Everyone,

I'm trying to become the firmware loader maintainer  
(request_firmware) and I felt the first thing I should do is update  
the incomplete example driver under Documentaion/firmware_class/.   
The example (firmware_sample_driver.c) doesn't work out of box and  
needs tweaking just to get it to compile.  I've added two files which  
use request_firmware and the asynchronous request_firmware_nowait.

The next step would be to update the documentation to make hotplug/ 
udev functionality a little more clear.  Hopefully I can get this in  
the next couple of days.

Also, I'd like to comment on Jon Masters push to include the  
MODULE_FIRMWARE api addition.  I strongly believe that policy should  
not be included in driver code, in this case it's in the form of a  
filename.

The firmware loader currently needs a filename passed to it so it can  
then pass the $FIRMWARE environment variable to the hotplug script.   
This is ok if you provide a generic filename like "firmware.bin" and  
then let the hotplug script worry about version numbers, i.e.  
"firmware-x.y.z.bin"

MODULE_FIRMWARE should be used to provide the generic filenames and  
which order the files should be loaded (request_firmware can be  
called various times), but I think its bad to have to change the  
driver everytime a new firmware version is released.

That said, here's the patch...


[-- Attachment #2: firmware_examples.patch --]
[-- Type: application/octet-stream, Size: 7494 bytes --]

diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_example.c linux/Documentation/firmware_class/firmware_example.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_example.c	1969-12-31 16:00:00.000000000 -0800
+++ linux/Documentation/firmware_class/firmware_example.c	2006-09-04 00:42:29.000000000 -0700
@@ -0,0 +1,69 @@
+/*
+ * firmware_example.c -
+ *
+ * Copyright (c) 2006 Victor Hugo <victor@vhugo.net>
+ * Based on firmware_sample_driver.c by Manuel Estrada Sainz
+ * Sample code on how to use request_firmware() from drivers.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/firmware.h>
+
+static void sample_firmware_load(char *firmware, int size)
+{
+	u8 buf[size + 1];
+	memcpy(buf, firmware, size);
+	buf[size] = '\0';
+	printk(KERN_INFO "firmware_example: Firmware: %s\n", buf);
+}
+
+static void sample_probe(struct device *dev)
+{
+	/* uses the default method to get the firmware */
+	const struct firmware *fw_entry;
+	printk(KERN_INFO
+	       "firmware_example: ghost device inserted\n");
+
+	if (request_firmware(&fw_entry, "sample_firware.bin", dev) != 0) {
+		printk(KERN_ERR
+		       "firmware_example: Firmware not available\n");
+		return;
+	}
+
+	sample_firmware_load(fw_entry->data, fw_entry->size);
+
+	release_firmware(fw_entry);
+
+	/* finish setting up the device */
+}
+
+static void ghost_release(struct device *dev)
+{
+	printk(KERN_DEBUG "firmware_example : ghost device released\n");
+}
+
+static struct device ghost_device = {
+	.bus_id = "ghost0",
+	.release = ghost_release
+};
+
+static int __init sample_init(void)
+{
+	device_register(&ghost_device);
+	sample_probe(&ghost_device);
+	return 0;
+}
+static void __exit sample_exit(void)
+{
+	device_unregister(&ghost_device);
+}
+
+module_init(sample_init);
+module_exit(sample_exit);
+
+MODULE_LICENSE("GPL");
diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_nowait_example.c linux/Documentation/firmware_class/firmware_nowait_example.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_nowait_example.c	1969-12-31 16:00:00.000000000 -0800
+++ linux/Documentation/firmware_class/firmware_nowait_example.c	2006-09-04 00:44:59.000000000 -0700
@@ -0,0 +1,84 @@
+/*
+ * firmware_nowait_example.c -
+ *
+ * Copyright (c) 2006 Victor Hugo <victor@vhugo.net>
+ * Based on firmware_sample_driver.c by Manuel Estrada Sainz
+ * Sample code on how to use request_firmware() from drivers.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/firmware.h>
+
+static void sample_firmware_load(char *firmware, int size)
+{
+	u8 buf[size+1];
+	memcpy(buf, firmware, size);
+	buf[size] = '\0';
+	printk(KERN_INFO "firmware_nowait_example: Firmware: %s\n", buf);
+}
+
+static void sample_probe_async_cont(const struct firmware *fw, void *context)
+{
+	if(!fw)
+	{
+		printk(KERN_ERR "firmware_nowait_example: Firmware not available\n");
+		return;
+	}
+
+
+	printk(KERN_INFO "firmware_nowait_example: Device Pointer \"%s\"\n",
+	       (char *)context);
+	sample_firmware_load(fw->data, fw->size);
+
+}
+
+static void sample_probe_async(struct device *dev)
+{
+	/* Let's say I can't sleep */
+	int error;
+	
+	printk(KERN_INFO
+	       "firmware_example: ghost device inserted\n");
+
+	error = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG,
+					"sample_firmware.bin", dev,
+					"my device pointer",sample_probe_async_cont);
+
+	if(error)
+	{
+		printk(KERN_ERR 
+		       "firmware_nowait_example: request_firmware_nowait Failed\n");
+	}
+
+}
+
+static void ghost_release(struct device *dev)
+{
+	printk(KERN_DEBUG "firmware_nowait_example: ghost device released\n");
+}
+
+static struct device ghost_device = {
+	.bus_id    = "ghost0",
+	.release   = ghost_release
+};
+
+static int __init sample_init(void)
+{
+	device_register(&ghost_device);
+	sample_probe_async(&ghost_device);
+return 0;
+}
+static void __exit sample_exit(void)
+{
+	device_unregister(&ghost_device);
+}
+
+module_init (sample_init);
+module_exit (sample_exit);
+
+MODULE_LICENSE("GPL");
diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_sample_driver.c linux/Documentation/firmware_class/firmware_sample_driver.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_sample_driver.c	2006-08-23 14:16:33.000000000 -0700
+++ linux/Documentation/firmware_class/firmware_sample_driver.c	1969-12-31 16:00:00.000000000 -0800
@@ -1,115 +0,0 @@
-/*
- * firmware_sample_driver.c -
- *
- * Copyright (c) 2003 Manuel Estrada Sainz <ranty@debian.org>
- *
- * Sample code on how to use request_firmware() from drivers.
- *
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/device.h>
-#include <linux/string.h>
-
-#include "linux/firmware.h"
-
-static struct device ghost_device = {
-	.bus_id    = "ghost0",
-};
-
-
-static void sample_firmware_load(char *firmware, int size)
-{
-	u8 buf[size+1];
-	memcpy(buf, firmware, size);
-	buf[size] = '\0';
-	printk(KERN_INFO "firmware_sample_driver: firmware: %s\n", buf);
-}
-
-static void sample_probe_default(void)
-{
-	/* uses the default method to get the firmware */
-        const struct firmware *fw_entry;
-	printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted :)\n");
-
-        if(request_firmware(&fw_entry, "sample_driver_fw", &ghost_device)!=0)
-	{
-		printk(KERN_ERR
-		       "firmware_sample_driver: Firmware not available\n");
-		return;
-	}
-	
-	sample_firmware_load(fw_entry->data, fw_entry->size);
-
-	release_firmware(fw_entry);
-
-	/* finish setting up the device */
-}
-static void sample_probe_specific(void)
-{
-	/* Uses some specific hotplug support to get the firmware from
-	 * userspace  directly into the hardware, or via some sysfs file */
-
-	/* NOTE: This currently doesn't work */
-
-	printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted :)\n");
-
-        if(request_firmware(NULL, "sample_driver_fw", &ghost_device)!=0)
-	{
-		printk(KERN_ERR
-		       "firmware_sample_driver: Firmware load failed\n");
-		return;
-	}
-	
-	/* request_firmware blocks until userspace finished, so at
-	 * this point the firmware should be already in the device */
-
-	/* finish setting up the device */
-}
-static void sample_probe_async_cont(const struct firmware *fw, void *context)
-{
-	if(!fw){
-		printk(KERN_ERR
-		       "firmware_sample_driver: firmware load failed\n");
-		return;
-	}
-
-	printk(KERN_INFO "firmware_sample_driver: device pointer \"%s\"\n",
-	       (char *)context);
-	sample_firmware_load(fw->data, fw->size);
-}
-static void sample_probe_async(void)
-{
-	/* Let's say that I can't sleep */
-	int error;
-	error = request_firmware_nowait (THIS_MODULE, FW_ACTION_NOHOTPLUG,
-					 "sample_driver_fw", &ghost_device,
-					 "my device pointer",
-					 sample_probe_async_cont);
-	if(error){
-		printk(KERN_ERR 
-		       "firmware_sample_driver:"
-		       " request_firmware_nowait failed\n");
-	}
-}
-
-static int sample_init(void)
-{
-	device_initialize(&ghost_device);
-	/* since there is no real hardware insertion I just call the
-	 * sample probe functions here */
-	sample_probe_specific();
-	sample_probe_default();
-	sample_probe_async();
-	return 0;
-}
-static void __exit sample_exit(void)
-{
-}
-
-module_init (sample_init);
-module_exit (sample_exit);
-
-MODULE_LICENSE("GPL");

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Andrew Morton]

On Sun, 3 Sep 2006 19:55:53 -0700
Victor Hugo <victor@vhugo.net> wrote:

> Hi Everyone,
> 
> I'm trying to become the firmware loader maintainer  
> (request_firmware) and I felt the first thing I should do is update  
> the incomplete example driver under Documentaion/firmware_class/.   
> The example (firmware_sample_driver.c) doesn't work out of box and  
> needs tweaking just to get it to compile.  I've added two files which  
> use request_firmware and the asynchronous request_firmware_nowait.

Thanks.

> The next step would be to update the documentation to make hotplug/ 
> udev functionality a little more clear.  Hopefully I can get this in  
> the next couple of days.

Documentation updates are welcome.

> Also, I'd like to comment on Jon Masters push to include the  
> MODULE_FIRMWARE api addition.  I strongly believe that policy should  
> not be included in driver code, in this case it's in the form of a  
> filename.

You should have cc'ed him!  Some people are not subscribed, and few read
it all.

> The firmware loader currently needs a filename passed to it so it can  
> then pass the $FIRMWARE environment variable to the hotplug script.   
> This is ok if you provide a generic filename like "firmware.bin" and  
> then let the hotplug script worry about version numbers, i.e.  
> "firmware-x.y.z.bin"
> 
> MODULE_FIRMWARE should be used to provide the generic filenames and  
> which order the files should be loaded (request_firmware can be  
> called various times), but I think its bad to have to change the  
> driver everytime a new firmware version is released.

Yes, that does sound bad, but what would I know ;)

> That said, here's the patch...

application/octet-stream.  Please don't do that.  Inlined-in-the-body is
much preferred.  If you cannot do that (ie: your MUA is lame but you don't
want to switch) then, in extremis, text/plain attachments are tolerated.

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Jon Masters]

On Mon, 2006-09-04 at 14:02 -0700, Andrew Morton wrote:

> > Also, I'd like to comment on Jon Masters push to include the  
> > MODULE_FIRMWARE api addition.  I strongly believe that policy should  
> > not be included in driver code, in this case it's in the form of a  
> > filename.
> 
> You should have cc'ed him!  Some people are not subscribed, and few read
> it all.

Thanks. My lkml subscription bounced last week after I decided to
overhaul my home email (moving country in a couple of weeks...).

I disagree that MODULE_FIRMWARE encodes policy in the driver. It encodes
a reference - nothing more. We need to know what firmware we're going to
be asked for if we have any hope of being able to package up drivers for
use in distro initrds and the like. I think we can agree on that much.

Some alternatives:

* The firmware API get hacked yet again to use static strings we can
parse (ugly) or otherwise somehow provide this information.

* We add hacks in userspace "if it's this driver and someone used
MODULE_VERSION correctly, and we can determine the right firmware...".

> > The firmware loader currently needs a filename passed to it so it can  
> > then pass the $FIRMWARE environment variable to the hotplug script.   
> > This is ok if you provide a generic filename like "firmware.bin" and  
> > then let the hotplug script worry about version numbers, i.e.  
> > "firmware-x.y.z.bin"

> > MODULE_FIRMWARE should be used to provide the generic filenames and  
> > which order the files should be loaded (request_firmware can be  
> > called various times), but I think its bad to have to change the  
> > driver everytime a new firmware version is released.
> 
> Yes, that does sound bad, but what would I know ;)

I'm ok with the idea of having a generic name, but it does add more
processing in userspace. Frankly, I don't care what is adopted but
something needs to be done - sooner or later, it's going to be more
drivers than Qlogic (and a few others) we need this working for :-)

I'm optimistic that we'll find the right one line patch eventually!

Jon.

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Marcel Holtmann]

Hi Victor,

> Also, I'd like to comment on Jon Masters push to include the  
> MODULE_FIRMWARE api addition.  I strongly believe that policy should  
> not be included in driver code, in this case it's in the form of a  
> filename.
> 
> The firmware loader currently needs a filename passed to it so it can  
> then pass the $FIRMWARE environment variable to the hotplug script.   
> This is ok if you provide a generic filename like "firmware.bin" and  
> then let the hotplug script worry about version numbers, i.e.  
> "firmware-x.y.z.bin"
> 
> MODULE_FIRMWARE should be used to provide the generic filenames and  
> which order the files should be loaded (request_firmware can be  
> called various times), but I think its bad to have to change the  
> driver everytime a new firmware version is released.

actually it has never been really a filename. It was a simple pattern
that the initial hotplug script and later the udev script mapped 1:1 to
a filename on your filesystem. If you check the mailing list archives of
LKML and linux-hotplug you will see that I always resisted in allowing
drivers to include a directory path in that call. A couple of people
tried this and it is not what it was meant to be.

The MODULE_FIRMWARE approach simply makes this pattern visible via
modinfo, because otherwise you would have to scan the source code to
find this pattern. And to make it use you have to apply the same policy
the firmware script is applying when choosing the file. Currently this
is a 1:1 mapping.

Regards

Marcel

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Victor Hugo]

Hi Marcel,

On Sep 5, 2006, at 12:33 AM, Marcel Holtmann wrote:
>
> actually it has never been really a filename. It was a simple pattern
> that the initial hotplug script and later the udev script mapped  
> 1:1 to
> a filename on your filesystem. If you check the mailing list  
> archives of
> LKML and linux-hotplug you will see that I always resisted in allowing
> drivers to include a directory path in that call. A couple of people
> tried this and it is not what it was meant to be.
>
> The MODULE_FIRMWARE approach simply makes this pattern visible via
> modinfo, because otherwise you would have to scan the source code to
> find this pattern. And to make it use you have to apply the same  
> policy
> the firmware script is applying when choosing the file. Currently this
> is a 1:1 mapping.
>
> Regards
>
> Marcel

You're right, I should have been more specific when I said  
"filename", I really meant a 1:1 mapping to a file in /lib/firmware.

My question is, should we have a generic 1:1 mapping and make it  
visible through MODULE_FIRMWARE.

  Or like Jon Masters suggested have specific version numbers in the  
pattern and have them map to specific versions in /lib/firmware and  
make them all visible through MODULE_FIRMWARE.  I believe the  
reasoning behind this was to make packaging drivers easier.


I believe that we should have a generic mapping in the driver (i.e,  
"firmware.bin") and let the admin or the userspace hotplug scripts  
take care of filename policy with a link to the correct firmware  
version.

example :

firmware.bin -> firmware-xyz.bin

The main reason for not including speciic mapping in the driver is  
that everytime a new firmware version is released the driver has to  
be updated and recompiled.  Its much easier to change a hotplug script.


-Victor

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Marcel Holtmann]

Hi Victor,

> > actually it has never been really a filename. It was a simple pattern
> > that the initial hotplug script and later the udev script mapped  
> > 1:1 to
> > a filename on your filesystem. If you check the mailing list  
> > archives of
> > LKML and linux-hotplug you will see that I always resisted in allowing
> > drivers to include a directory path in that call. A couple of people
> > tried this and it is not what it was meant to be.
> >
> > The MODULE_FIRMWARE approach simply makes this pattern visible via
> > modinfo, because otherwise you would have to scan the source code to
> > find this pattern. And to make it use you have to apply the same  
> > policy
> > the firmware script is applying when choosing the file. Currently this
> > is a 1:1 mapping.
> 
> You're right, I should have been more specific when I said  
> "filename", I really meant a 1:1 mapping to a file in /lib/firmware.
> 
> My question is, should we have a generic 1:1 mapping and make it  
> visible through MODULE_FIRMWARE.

please re-read my email and the initial thread about MODULE_FIRMWARE.
The MODULE_FIRMWARE extension only export the pattern used by
request_firmware(), because otherwise some tools don't know what a
kernel driver actually expects.

It happens to be a 1:1 mapping, but that is pure coincidence and a
little bit of laziness from the early users (mainly me) when providing
the first firmware script for the hotplug package. For all my drivers, I
don't need any fancy mapping.

>   Or like Jon Masters suggested have specific version numbers in the  
> pattern and have them map to specific versions in /lib/firmware and  
> make them all visible through MODULE_FIRMWARE.  I believe the  
> reasoning behind this was to make packaging drivers easier.

No. We need to make the actual file for loading the firmware appear
under the device in sysfs. So the userspace can read extra information
from the driver or device and then make its decision on which firmware
to load.

> I believe that we should have a generic mapping in the driver (i.e,  
> "firmware.bin") and let the admin or the userspace hotplug scripts  
> take care of filename policy with a link to the correct firmware  
> version.
> 
> example :
> 
> firmware.bin -> firmware-xyz.bin
> 
> The main reason for not including speciic mapping in the driver is  
> that everytime a new firmware version is released the driver has to  
> be updated and recompiled.  Its much easier to change a hotplug script.

I say, leave this up to the driver. For a couple of drivers this works
perfectly fine and I don't see any need to change my drivers and make
them more complex if it is not needed.

>From my point I don't see any advantage to make firmware loading more
complex from the kernel perspective. All this weird stuff needs to be
done in userspace. Our current way works quite good for a number devices
that need binary firmware.

I think it is better to first collect the needs of the driver authors
and make sure they understand their own needs. This is not always true.
And then we can discuss any extensions to change something. And I prefer
to have actual hardware and their drivers as an example.

The MODULE_FIRMWARE() extension is an easy and simple extension that
goes along with the current idea of request_firmware().

Regards

Marcel

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Jon Masters]

On Tue, 2006-09-05 at 11:26 -0700, Victor Hugo wrote:

> On Sep 5, 2006, at 12:33 AM, Marcel Holtmann wrote:
> >
> > actually it has never been really a filename. It was a simple pattern
> > that the initial hotplug script and later the udev script mapped  
> > 1:1 to a filename on your filesystem. If you check the mailing list  
> > archives of LKML and linux-hotplug you will see that I always resisted
> > in allowing drivers to include a directory path in that call. A couple
> > of people tried this and it is not what it was meant to be.

That's fine. I agree with the idea - *but* it strikes me that we don't
really have a co-ordinated database of what module "patterns" map to
what on-disk firmware, aside from hotplug/udev scripts. We need to
co-ordinate this stuff a lot more. Or am I missing something? I'm happy
to setup a database on the kerneltools.org wiki if that's useful...

[ I've got a plan to extend some of the stuff I've been looking at in
the Fedora space - and some of you won't like me for it :P Basically I
want to put a GUI in front of DKMS/kmods/other module packaging/firmware
update utilities and automatically locate/build/install third-party GPL
(but not yet upstream) drivers on non-moving target distros. Evil stuff
like that also would find knowing about external firmware useful. ]

> Or like Jon Masters suggested have specific version numbers in the  
> pattern and have them map to specific versions in /lib/firmware and  
> make them all visible through MODULE_FIRMWARE.  I believe the  
> reasoning behind this was to make packaging drivers easier.

The reasoning behind MODULE_FIRMWARE is because firmware names can be
dynamically generated in the module so we can't know what it's going to
need ahead of time. I'd like to hook this up with the PCI ID/etc. of the
hardware that needs that firmware - so we really only package the
mimimum, but I think that's overkill.

> I believe that we should have a generic mapping in the driver (i.e,  
> "firmware.bin") and let the admin or the userspace hotplug scripts  
> take care of filename policy with a link to the correct firmware  
> version.
> 
> example :
> 
> firmware.bin -> firmware-xyz.bin
> 
> The main reason for not including speciic mapping in the driver is  
> that everytime a new firmware version is released the driver has to  
> be updated and recompiled.  Its much easier to change a hotplug script.

I'm trying to avoid the need to have lots of different places in
userland needing to track firmware versioning. But on some level, I just
need to know that a given driver is going to ask for 1 firmware with the
ID of "foo" - and a way to extrapolate where it is on disk to package.

Jon.

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Marcel Holtmann]

Hi Jon,

> > > actually it has never been really a filename. It was a simple pattern
> > > that the initial hotplug script and later the udev script mapped  
> > > 1:1 to a filename on your filesystem. If you check the mailing list  
> > > archives of LKML and linux-hotplug you will see that I always resisted
> > > in allowing drivers to include a directory path in that call. A couple
> > > of people tried this and it is not what it was meant to be.
> 
> That's fine. I agree with the idea - *but* it strikes me that we don't
> really have a co-ordinated database of what module "patterns" map to
> what on-disk firmware, aside from hotplug/udev scripts. We need to
> co-ordinate this stuff a lot more. Or am I missing something? I'm happy
> to setup a database on the kerneltools.org wiki if that's useful...

that is true, but it is actually not a problem of the kernel and your
proposed MODULE_FIRMWARE patch. However it might be a good idea to start
something like this. It will also help to see what is actually needed.

> I'm trying to avoid the need to have lots of different places in
> userland needing to track firmware versioning. But on some level, I just
> need to know that a given driver is going to ask for 1 firmware with the
> ID of "foo" - and a way to extrapolate where it is on disk to package.

Let start collecting these information and then go from there.

Regards

Marcel

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Jon Masters]

On Thu, 2006-09-07 at 17:10 +0200, Marcel Holtmann wrote:
> Hi Jon,
> 
> > > > actually it has never been really a filename. It was a simple pattern
> > > > that the initial hotplug script and later the udev script mapped  
> > > > 1:1 to a filename on your filesystem. If you check the mailing list  
> > > > archives of LKML and linux-hotplug you will see that I always resisted
> > > > in allowing drivers to include a directory path in that call. A couple
> > > > of people tried this and it is not what it was meant to be.
> > 
> > That's fine. I agree with the idea - *but* it strikes me that we don't
> > really have a co-ordinated database of what module "patterns" map to
> > what on-disk firmware, aside from hotplug/udev scripts. We need to
> > co-ordinate this stuff a lot more. Or am I missing something? I'm happy
> > to setup a database on the kerneltools.org wiki if that's useful...
> 
> that is true, but it is actually not a problem of the kernel and your
> proposed MODULE_FIRMWARE patch. However it might be a good idea to start
> something like this. It will also help to see what is actually needed.

I reali[sz]e it's not a direct problem, but it does need fixing. I'll
spend some time over the weekend and then send an update about that.

Jon.

[PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Victor Hugo]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

Hi Everyone,

I'm trying to become the firmware loader maintainer
(request_firmware) and I felt the first thing I should
do is update the incomplete example driver under
Documentaion/firmware_class/.  The example
(firmware_sample_driver.c) doesn't work out of box and
needs tweaking just to get it to compile.  I've added
two files which use request_firmware and the
asynchronous request_firmware_nowait.

The next step would be to update the documentation to
make hotplug/udev functionality a little more clear. 
Hopefully I can get this in the next couple of days.

Also, I'd like to comment on Jon Masters push to
include the MODULE_FIRMWARE api addition.  I strongly
believe that policy should not be included in driver
code, in this case it's in the form of a filename. 

The firmware loader currently needs a filename passed
to it so it can then pass the $FIRMWARE environment
variable to the hotplug script.  This is ok if you
provide a generic filename like "firmware.bin" and
then let the hotplug script worry about version
numbers, i.e. "firmware-x.y.z.bin"

MODULE_FIRMWARE should be used to provide the generic
filenames and which order the files should be loaded
(request_firmware can be called various times), but I
think its bad to have to change the driver everytime a
new firmware version is released.

That said, here's the patch...

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pat882669590 --]
[-- Type: text/x-patch; name="firmware_examples.patch", Size: 7494 bytes --]

diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_example.c linux/Documentation/firmware_class/firmware_example.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_example.c	1969-12-31 16:00:00.000000000 -0800
+++ linux/Documentation/firmware_class/firmware_example.c	2006-09-04 00:42:29.000000000 -0700
@@ -0,0 +1,69 @@
+/*
+ * firmware_example.c -
+ *
+ * Copyright (c) 2006 Victor Hugo <victor@vhugo.net>
+ * Based on firmware_sample_driver.c by Manuel Estrada Sainz
+ * Sample code on how to use request_firmware() from drivers.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/firmware.h>
+
+static void sample_firmware_load(char *firmware, int size)
+{
+	u8 buf[size + 1];
+	memcpy(buf, firmware, size);
+	buf[size] = '\0';
+	printk(KERN_INFO "firmware_example: Firmware: %s\n", buf);
+}
+
+static void sample_probe(struct device *dev)
+{
+	/* uses the default method to get the firmware */
+	const struct firmware *fw_entry;
+	printk(KERN_INFO
+	       "firmware_example: ghost device inserted\n");
+
+	if (request_firmware(&fw_entry, "sample_firware.bin", dev) != 0) {
+		printk(KERN_ERR
+		       "firmware_example: Firmware not available\n");
+		return;
+	}
+
+	sample_firmware_load(fw_entry->data, fw_entry->size);
+
+	release_firmware(fw_entry);
+
+	/* finish setting up the device */
+}
+
+static void ghost_release(struct device *dev)
+{
+	printk(KERN_DEBUG "firmware_example : ghost device released\n");
+}
+
+static struct device ghost_device = {
+	.bus_id = "ghost0",
+	.release = ghost_release
+};
+
+static int __init sample_init(void)
+{
+	device_register(&ghost_device);
+	sample_probe(&ghost_device);
+	return 0;
+}
+static void __exit sample_exit(void)
+{
+	device_unregister(&ghost_device);
+}
+
+module_init(sample_init);
+module_exit(sample_exit);
+
+MODULE_LICENSE("GPL");
diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_nowait_example.c linux/Documentation/firmware_class/firmware_nowait_example.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_nowait_example.c	1969-12-31 16:00:00.000000000 -0800
+++ linux/Documentation/firmware_class/firmware_nowait_example.c	2006-09-04 00:44:59.000000000 -0700
@@ -0,0 +1,84 @@
+/*
+ * firmware_nowait_example.c -
+ *
+ * Copyright (c) 2006 Victor Hugo <victor@vhugo.net>
+ * Based on firmware_sample_driver.c by Manuel Estrada Sainz
+ * Sample code on how to use request_firmware() from drivers.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+#include <linux/firmware.h>
+
+static void sample_firmware_load(char *firmware, int size)
+{
+	u8 buf[size+1];
+	memcpy(buf, firmware, size);
+	buf[size] = '\0';
+	printk(KERN_INFO "firmware_nowait_example: Firmware: %s\n", buf);
+}
+
+static void sample_probe_async_cont(const struct firmware *fw, void *context)
+{
+	if(!fw)
+	{
+		printk(KERN_ERR "firmware_nowait_example: Firmware not available\n");
+		return;
+	}
+
+
+	printk(KERN_INFO "firmware_nowait_example: Device Pointer \"%s\"\n",
+	       (char *)context);
+	sample_firmware_load(fw->data, fw->size);
+
+}
+
+static void sample_probe_async(struct device *dev)
+{
+	/* Let's say I can't sleep */
+	int error;
+	
+	printk(KERN_INFO
+	       "firmware_example: ghost device inserted\n");
+
+	error = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG,
+					"sample_firmware.bin", dev,
+					"my device pointer",sample_probe_async_cont);
+
+	if(error)
+	{
+		printk(KERN_ERR 
+		       "firmware_nowait_example: request_firmware_nowait Failed\n");
+	}
+
+}
+
+static void ghost_release(struct device *dev)
+{
+	printk(KERN_DEBUG "firmware_nowait_example: ghost device released\n");
+}
+
+static struct device ghost_device = {
+	.bus_id    = "ghost0",
+	.release   = ghost_release
+};
+
+static int __init sample_init(void)
+{
+	device_register(&ghost_device);
+	sample_probe_async(&ghost_device);
+return 0;
+}
+static void __exit sample_exit(void)
+{
+	device_unregister(&ghost_device);
+}
+
+module_init (sample_init);
+module_exit (sample_exit);
+
+MODULE_LICENSE("GPL");
diff -Nur linux-2.6.17.11/Documentation/firmware_class/firmware_sample_driver.c linux/Documentation/firmware_class/firmware_sample_driver.c
--- linux-2.6.17.11/Documentation/firmware_class/firmware_sample_driver.c	2006-08-23 14:16:33.000000000 -0700
+++ linux/Documentation/firmware_class/firmware_sample_driver.c	1969-12-31 16:00:00.000000000 -0800
@@ -1,115 +0,0 @@
-/*
- * firmware_sample_driver.c -
- *
- * Copyright (c) 2003 Manuel Estrada Sainz <ranty@debian.org>
- *
- * Sample code on how to use request_firmware() from drivers.
- *
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/device.h>
-#include <linux/string.h>
-
-#include "linux/firmware.h"
-
-static struct device ghost_device = {
-	.bus_id    = "ghost0",
-};
-
-
-static void sample_firmware_load(char *firmware, int size)
-{
-	u8 buf[size+1];
-	memcpy(buf, firmware, size);
-	buf[size] = '\0';
-	printk(KERN_INFO "firmware_sample_driver: firmware: %s\n", buf);
-}
-
-static void sample_probe_default(void)
-{
-	/* uses the default method to get the firmware */
-        const struct firmware *fw_entry;
-	printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted :)\n");
-
-        if(request_firmware(&fw_entry, "sample_driver_fw", &ghost_device)!=0)
-	{
-		printk(KERN_ERR
-		       "firmware_sample_driver: Firmware not available\n");
-		return;
-	}
-	
-	sample_firmware_load(fw_entry->data, fw_entry->size);
-
-	release_firmware(fw_entry);
-
-	/* finish setting up the device */
-}
-static void sample_probe_specific(void)
-{
-	/* Uses some specific hotplug support to get the firmware from
-	 * userspace  directly into the hardware, or via some sysfs file */
-
-	/* NOTE: This currently doesn't work */
-
-	printk(KERN_INFO "firmware_sample_driver: a ghost device got inserted :)\n");
-
-        if(request_firmware(NULL, "sample_driver_fw", &ghost_device)!=0)
-	{
-		printk(KERN_ERR
-		       "firmware_sample_driver: Firmware load failed\n");
-		return;
-	}
-	
-	/* request_firmware blocks until userspace finished, so at
-	 * this point the firmware should be already in the device */
-
-	/* finish setting up the device */
-}
-static void sample_probe_async_cont(const struct firmware *fw, void *context)
-{
-	if(!fw){
-		printk(KERN_ERR
-		       "firmware_sample_driver: firmware load failed\n");
-		return;
-	}
-
-	printk(KERN_INFO "firmware_sample_driver: device pointer \"%s\"\n",
-	       (char *)context);
-	sample_firmware_load(fw->data, fw->size);
-}
-static void sample_probe_async(void)
-{
-	/* Let's say that I can't sleep */
-	int error;
-	error = request_firmware_nowait (THIS_MODULE, FW_ACTION_NOHOTPLUG,
-					 "sample_driver_fw", &ghost_device,
-					 "my device pointer",
-					 sample_probe_async_cont);
-	if(error){
-		printk(KERN_ERR 
-		       "firmware_sample_driver:"
-		       " request_firmware_nowait failed\n");
-	}
-}
-
-static int sample_init(void)
-{
-	device_initialize(&ghost_device);
-	/* since there is no real hardware insertion I just call the
-	 * sample probe functions here */
-	sample_probe_specific();
-	sample_probe_default();
-	sample_probe_async();
-	return 0;
-}
-static void __exit sample_exit(void)
-{
-}
-
-module_init (sample_init);
-module_exit (sample_exit);
-
-MODULE_LICENSE("GPL");

Re: [PATCH][RFC] request_firmware examples and MODULE_FIRMWARE

[Alan Cox]

Ar Llu, 2006-09-04 am 01:39 -0700, ysgrifennodd Victor Hugo:
> That said, here's the patch...
> 
> tatic void sample_firmware_load(char *firmware, int size)
+{
+       u8 buf[size + 1];
+       memcpy(buf, firmware, size);

Please don't use this GNUism in the kernel code. It makes it very hard
to tell how much is being put on the (limited) stack. Better to use
kmalloc explicitly or a fixed size.

Rest looks sane.


-- 
VGER BF report: H 1.48569e-10