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

[Michael Buesch]

On Sunday 25 May 2008 11:30:37 Johannes Berg wrote:
> > I explained this a couple of times. The request_firmware() is an  
> > abstract mechanism that can request a firmware file. The location of  
> > the firmware file is up to the userspace. The kernel requests a  
> > particular file and that is it. All namespacing has to be done by the  
> > firmware helper script (nowadays udev). That the current  
> > implementation of the firmware helper maps the filename 1:1 to a file  
> > under /lib/firmware/ just works, but doesn't have to work all the  
> > time. It is not the agreed contract between kernel and userspace.
> 
> I don't buy this argument. I could agree if you said that the "agreed
> contract" between the kernel and userspace is for the kernel to request
> a firmware file /keyed by an arbitrary, null-terminated string/.

I fully agree with johannes, that using / as a grouping-separator is a
_nice_ idea and userspace must support it, regardless of the actual
storage system it is using.

Currently using a single directory for one driver really enhances the way
firmware can be handled. For example, we can have several different versions
cleanly installed in /lib/firmware. For example, on my development machine
I have
/lib/firmware/b43
/lib/firmware/b43-old
/lib/firmware/b43-open
/lib/firmware/b43legacy
Putting all these files into plain /lib/firmware would require changing the
actual filenames and that would be a real pain. (This can be up to about 100 files).

> The fact that it is usually stored on a filesystem where / means a
> directory (and thus grouping) can be seen as a nice convenience of the
> filesystem storage, but if firmware was stored elsewhere then you could
> degrade to the simple key-based lookup that happens to allow "/" as a
> character in the keys.
> 
> And because the kernel is nice, it allows userspace to use a filesystem
> storage by not using paths like "../../lib/firmware/asdf". But
> fundamentally, I don't even see anything wrong with that.

I agree that .. and . should probably be avoided.
And I also don't see a good reason to use them.

> Put another way, you can have pretty arbitrary firmware firmware names
> (though since humans need to handle them you want printable characters),
> and I don't see why now all the sudden you would treat "/" specially by
> *explicitly* disallowing it.
> 
> b43 comes with 22 firmware files for a single driver, and groups them

Depending on the firmware version you have, there are over 30 files
for b43 and b43legacy each.

> using "b43/<name>". What you're proposing will make firmware fail
> *again* for all users, and we got a *LOT* of flak from all kinds of
> stakeholders (not just the users) when firmware upgrades were required,
> doing it again for such a petty reason is ridiculous.

Yeah. I'm not going to change that. The users are going to kill us.
Besides that, there were very good reasons to start grouping the files
in b43 (bcm43xx didn't do it), as it simply was unmaintainable in bcm43xx.

-- 
Greetings Michael.
--
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/