Re: [GIT *] Allow request_firmware() to be satisfied from in-kernel,use it in more drivers.

[david]

On Mon, 14 Jul 2008, Linus Torvalds wrote:

> On Mon, 14 Jul 2008, david@xxxxxxx wrote:
> > > Does it waste some ram? Sure. Tough.
> >
> > I agree with this, but the proponents of the seperate firmware are listing the
> > fact that the firmware doesn't tie up ram as one of the big reasons for making
> > the change.
>
> That's a totally bogus argument.
>
> The fact is, if you build it into your module, you'll waste at _least_ as
> much ram as if you just load it once at module load time.
>
> So there is no actual valid reason to object to "request_firmware()".

you misunderstood me. the people pushing request_firmware() are doing so 
on the basis that they won't have to use kernel ram to hold the firmware. 
the people pushing for having the option of building the firmware into the 
module are acknowleding that this may use a little more ram, but they see 
it as being more reliable.

> I don't know why people get confused about this. I suspect that people
> kind of expect that since they need to reload the firmware when resuming
> the device, they should also do the "request_firmware()" at resume time.

according to David W they would, becouse the driver would not keep the 
firmware in kernel memory after it's initialized, so if you need to 
reinitialize the hardware after a resume you would need to do 
request_firmware() at resume time (the option of doing request_firmware() 
just before going to sleep has been floated as an option to avoid this 
problem)

> Maybe it's worth explicitly documenting that request_firmware()/release()
> should be done as a module init/exit (or a device detect-eject) time
> option. Quite frankly, I think the current firmware docs are actually
> actively misleading, because they link the request_firmware() with the
> copying to device: quoting from Documentation/firmware_class/README:
>
> 	 High level behavior (driver code):
> 	 ==================================
>
> 	         if(request_firmware(&fw_entry, $FIRMWARE, device) == 0)
> 	                copy_fw_to_device(fw_entry->data, fw_entry->size);
> 	         release(fw_entry);
>
> and that is a fundamentally broken world-view.

but that is what David W has been telling everyone is one of the big 
benifits of using request_firmware(), that the memory gets released once 
the device is initialized. (as always, it's possible I have misunderstood 
the argument, but I can't see any other way that it would save memory)

> The logic _should_ be that the firmware is requested at module init or
> device discovery, and the release is done at module exit or device eject.

this would go a long way to satisfying the people who have been objecting 
to the request_firmware() push. It would solve the various 
reinitialization problems (at resume, and at other times for some 
drivers).

the only thing it doesn't satisfy is the people who don't want to be 
forced to have seperate firmware files out on the filesystem.

As I understand the arguments, the reason to not force the firmeware files 
to be seperate is that it adds complexity and introduces the possibility 
that the firmware may be incompatible with the driver, forcing the 
creation of some sort of userspace/filesystem versioning.

the reson to force the firmware files to be seperate is that it avoids 
using kernel memory to hold them and it allows for the firmware to be 
removed from the kernel tree entirely and shipped seperately (apparently 
Fedora is eager to do the latter for perceived licensing reasons)

> The "request_firmware()" should absolutely *not* be mentally tied to
> "copy_fw_to_device" at all. They are very distinct issues, and in fact
> must be totally separate for any driver that supports hotplug.

in the last flamewar it seemed pretty clear that the expectation was that 
a hotplug event would trigger a new request_firmware() call out to 
userspace.

David Lang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/