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
On Sun, 3 Sep 2006 19:55:53 -0700
Victor Hugo <[email protected]> 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.
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.
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
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
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
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.
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
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.