Re: [PATCH 10/11] iwlwifi: rely on API version read from firmware

[Marcel Holtmann <holtmann@linux.intel.com>]

Hi Reinette,

> > > This adds the infrastructure to support older firmware APIs.
> > > The API version number is stored as part of the filename, we first try to
> > > load the most recent firmware and progressively try lower versions.
> > > The API version is also read from the firmware self and stored as part
> > > of the iwl_priv structure. Only firmware that is supported by driver will
> > > be loaded. The version number read from firmware is compared
> > > to supported versions in the driver not the API version used as part of
> > > filename.
> > 
> > thanks for doing this. This can really help smooth upgrade path in case
> > the firmware API changes.
> >  
> > > -MODULE_FIRMWARE(IWL5000_MODULE_FIRMWARE);
> > > -MODULE_FIRMWARE(IWL5150_MODULE_FIRMWARE);
> > > +MODULE_FIRMWARE(IWL5000_MODULE_FIRMWARE(IWL5000_UCODE_API_MAX));
> > > +MODULE_FIRMWARE(IWL5150_MODULE_FIRMWARE(IWL5150_UCODE_API_MAX));
> > 
> > So we don't have clear semantics on how MODULE_FIRMWARE should be used.
> > The current way is that it list all potential firmware versions. So in
> > cases it support API -1 and -2, then it has to list both. And not only
> > the latest.
> 
> Even though the driver supports older firmware we really want the user
> to use the latest firmware. The driver will also print a clear error if
> the user is using API -1 and the latest available is -2. For this reason
> we do want to make clear through MODULE_FIRMWARE which firmware the
> driver would like to work with: the latest.

as I said, we never clearly defined the semantics of MODULE_FIRMWARE for
these cases. The only problem that I see is that some initrd tools are
actually using MODULE_FIRMWARE to figure which files need to be included
for the builtin kernel drivers. So on a system with old firmware only,
it will not include these.

Maybe we need to extend MODULE_FIRMWARE. Maybe something like
MODULE_ALTERNATE_FIRMWARE or some extra flags.

> > Personally I think we should not create too many macros here and just
> > list all the firmware files manually.
> 
> Having one spot to change when a new API is supported makes the code
> less error prone.

I agree on that, but it doesn't make it easier to read through the
source code.

Regards

Marcel