Re: [PATCH net-next] net: vxge: Add MODULE_FIRMWARE

[Ben Hutchings]

On Mon, 2012-04-16 at 06:21 -0600, Tim Gardner wrote:
> On 04/15/2012 07:56 AM, Ben Hutchings wrote:
> > On Thu, 2012-04-12 at 14:34 -0600, Tim Gardner wrote:
> >> Cc: Jon Mason <jdmason@xxxxxxxx> Cc: netdev@xxxxxxxxxxxxxxx 
> >> Signed-off-by: Tim Gardner <tim.gardner@xxxxxxxxxxxxx> --- 
> >> drivers/net/ethernet/neterion/vxge/vxge-main.c |    2 ++ 1 file
> >> changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c
> >> b/drivers/net/ethernet/neterion/vxge/vxge-main.c index
> >> 51387c3..dcef72d 100644 ---
> >> a/drivers/net/ethernet/neterion/vxge/vxge-main.c +++
> >> b/drivers/net/ethernet/neterion/vxge/vxge-main.c @@ -4856,3
> >> +4856,5 @@ vxge_closer(void) } module_init(vxge_starter); 
> >> module_exit(vxge_closer); +MODULE_FIRMWARE("vxge/X3fw-pxe.ncf"); 
> >> +MODULE_FIRMWARE("vxge/X3fw.ncf");
> > 
> > I don't agree; these firmware files are updates for the flash and
> > only need to be loaded once.
> > 
> > Also: this driver's behaviour of automatically updating flash
> > without any confirmation seems quite dangerous.  The driver also
> > isn't usable after it performs such an update:
> > 
> > printk(KERN_NOTICE "Upgrade of firmware successful!  Adapter must
> > be " "hard reset before using, thus requiring a system reboot or a
> > " "hotplug event.\n");
> > 
> > So what is the point of integrating firmware update into the driver
> > at all?
> > 
> > Ben.
> > 
> 
> I guess I'm confused about use of the MODULE_FIRMWARE() macro. I
> thought it merely described the names of the firmware files that were
> actually used by the driver and had no run-time impact. Regardless of
> whether firmware files are used on every load, why _not_ describe them
> to modinfo ?
[...]

Ah, that's a good question.  Quoting my own interpretation from
<1257629601.15927.361.camel@localhost>:

> Drivers that must load 'firmware' into the devices they drive should 
> declare the names of the files they will request, using the
> MODULE_FIRMWARE() macro.  This enables other tools to discover these
> dependencies statically, and warn the user if firmware files are
> missing.

In Debian we use this to decide which files need to be copied into an
initramfs.  You use that too unless you've changed this feature of
initramfs-tools.  We warn when building an initramfs and during a major
kernel upgrade if it looks like a driver will be used and the
corresponding firmware isn't installed.

I also have an (unfinished) patch that will use this information for
CONFIG_FIRMWARE_IN_KERNEL.

In this case the firmware files are used to upgrade old flash, but since
the vendor has closed down there aren't going to be any further updates.
So the likelihood of the files actually being needed by the driver is
very small.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

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