Re: [PATCH 2/3] firmware: Add CONFIG_BUILTIN_FIRMWARE option

[Johannes Berg <johannes@sipsolutions.net>]

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


> No it is not. The kobject doesn't allow "/" and why should  
> request_firmware() be an exception. Also see my other comment on how  
> the kernel handles device nodes and on how udev maps them to real  
> device nodes on the filesystem.

As I said, kobjects have nothing to do with it, they don't need to have
a filename based on the firmware key.

> And again. This is up to the userspace to handle and not the kernel.  
> In userspace you could do a general approach to support these kind of  
> testing, but you decide to only do this for your driver. You are fully  
> exploiting the request_firmware() interface and making any kind of  
> userspace policy impossible. This in return actually means that if we  
> would improve the request_firmware(), we have to maintain special  
> cases for the b43 drivers, because your driver does some hacking of  
> firmware files inside the kernel. You actually proved my point that we  
> should not allow this inside the kernel.

That makes no sense at all.

Michael is exploiting the firmware API that you are suggesting to
change, and so far I don't see a technical reason for changing it. In
kernel space, he's simply requesting varying firmware blobs based on
different keys, which happen to be "b43%s/%s" because that's making
things simpler for him on the other end.

On the other hand, you're saying that we have all kinds of policy about
this in userspace, so why are you saying that directory separators
(slashes, but you seem to distinguish those on some level I do not
understand) should be special in the firmware key?

The firmware key is just that, a *key*. It's only exposed to userspace
via an environment variable in a kobject named
"/devices/pci0001:10/0001:10:12.0/ssb0:0/firmware/ssb0:0" or similar.

The fact that userspace uses the key as a filename is maybe unfortunate,
maybe fortunate, but shouldn't have anything to do with what sort of
keys the kernel allows.

If Michael wants to serve his firmware blobs from an SQL database, he'd
use a simple table like this:

CREATE TABLE firmware (
	ID INTEGER,
	name VARCHAR(100),
	data BLOB
);

I don't see any problem with that.

Also, you said above (quoting again):

> You are fully  
> exploiting the request_firmware() interface and making any kind of  
> userspace policy impossible.

That's not true at all. If you decide that the userspace policy should
be to load $modulename/$firmwarekey then you'd maybe have something
like /lib/firmware/b43/b43-test/ucode5.fw
and /lib/firmware/b43/b43-osfw/ucode5.fw
and /lib/firmware/b43/b43/ucode5.fw, this doesn't preclude the use.

Now, if it had been like that from the beginning, Michael probably
wouldn't have used the string "b43" (or "b43-*") but rather requested
"broadcom/ucode5.fw" by default and "osfw/ucode5.fw" for the open source
firmware, but since it's just a key that doesn't matter.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]