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

Hi Johannes,


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.

our current usage is suboptimal and we should use kobjects for representing the loading entity. Please keep in mind that when we created request_firmware() most of the driver model developers still thought that having class devices was a good idea. That has changed and to represent firmware loading in a clean and race free environment where you could load multipl firmwares at the same time for the same driver, we need changes here.

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.

I disagree with you. The kernel should be free of these kind of subdirectory stuff. We saw devfs failing and we have a flat device node names in the kernel. Why do we have to duplicate information in the firmware filenames where we have all the information already present in the driver model. The reason that people are lazy doesn't work for me.

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.

That something works at the moment is not a reason for me not to fix it and improve the current framework around firmware loading. I have been a lot of times saying that the request_firmware() should not include "/" in the filename and driver authors followed it. Some of them did it anyway and so these need fixing now.

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/

Relevant Pages

  • Re: Linux GPL and binary module exception clause?
    ... >> But also note how it's only the BINARY MODULE that is a derived work. ... > a device driver containing a binary blob that was explicitly written as ... Clearly it no longer depends on the kernel, and you can show that to ... And I think this argument is _especially_ strong for things like firmware ...
    (Linux-Kernel)
  • Re: [PATCH] request_firmware: skip timeout if userspace was not notified
    ... needs to be removed from the kernel, that's known for a very long ... your point is that the firmware loader is ... or use a kernel module for the driver that needs ... kernel on boot if built in unless an initramfs is carefully ...
    (Linux-Kernel)
  • [PATCH 00/04] Load keyspan firmware with hotplug
    ... > That people didn't like the inclusion of firmware, ... - make the keyspan driver use request_firmware. ... - converter program used to dump the keyspan headers as IHex files. ... Open Source operating system kernel'. ...
    (Linux-Kernel)
  • Re: ipw2200 and wpa_supplicant
    ... > theres a wpa fix in the latest kernel prepatch ... > and there was also a small patch in the thread on sf.net i posted the ... the new ipw2200 driver. ... Back old ipw2200 firmware files, ...
    (Fedora)
  • Re: [GIT PATCH] Remove devfs from 2.6.17
    ... This information is currently useless. ... knows that you want this driver to be loaded. ... want is for the kernel to know that those modules are available, and therefore mention that drivers could be found for those devices, and therefore udev would create the device nodes for them, even though the kernel doesn't contain a module that drives them yet. ...
    (Linux-Kernel)