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

From: Marcel Holtmann
Date: Sun May 25 2008 - 09:13:08 EST


Hi Johannes,

The kernel should not in any case have knowledge about directories or
subdirectories where the firmware files are stored. That is fully
irrelevant for the kernel.

Exactly my point! Hence, the kernel just gives it an arbitrary name. The
fact that userspace uses this as a filename could be regarded as a bug,
but it works fine. Therefore, the kernel simply assigns an arbitrary,
NULL-terminated string as the name. It happens to include a / because it
is convenient with the current userspace implementation, but that's mere
detail, the ABI/API here is to allow any arbitrary names.

Especially with the case of built-in firmwares now, it because more
important to do it right. The one reason why we have to handover the
struct device to request_firmware() is that we can give the helper
script full access to the device and driver information of the caller.
Hence adding for example b43/ as prefix simply duplicates everything
since the struct device has a link to the driver that is requesting a
firmware file.

But that doesn't matter at all! Dave's work to build firmware files into
the kernel will simply result in an entry in the kernel firmware table
that has a '.name = "b43/pcm5.fw"', nothing needs to know that b43 is a
module name, in fact, it could very well be 'broadcom wlan/pcm5.fw' as
well.

That is not what I am proposing. What I am proposing is that we do
this the right way. Meaning that we fix udev to do the namespacing. I
am working on a way to have this change in a backward compatible way.

That will introduce a "flag-day" where you have to upgrade userspace
with the kernel, and vice versa, OR userspace will have to guess. Not a
good solution either.

not it does not. I have a fix for udev that will allow a smooth transition and keep full backward compatibility. Need to do some more testing.

And bumping the required udev version after 12 month is perfectly fine since we have the future removal document.

Why are you so fixated on special-casing the single character '/'?

It is not the "/" character. It is the directory separator. Just happened to be "/" on Unix operating systems. You are really missing my point here. The kernel should not be involved in enforcing this kind of namespaces.

Look at the device nodes. The kernel has mouse0 for example and udev will translate this into /dev/input/mouse0. Nobody expects the kernel to use input/mouse0 and actually you even can't do that at all since the device model forbids "/" as bus id. Same applies for the firmware filenames.

Also at some point we might change the actual implementation of request_firmware() to allow running multiple request_firmware() at the same time to improve the init time of devices (if that makes sense). In that case the filename would become a kobject and then the directory separator would become illegal.

Regards

Marcel

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