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

[Johannes Berg]

> so using "/" within the name parameter for request_firmware() is  
> actually forbidden. I know that some driver authers think it is a good  
> idea, but it is not.

Can you explain why it is allowed now? And maybe why the API was
designed in a way that easily allows it?

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

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.

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
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.

johannes

Attachment: signature.asc
Description: This is a digitally signed message part