On Tue, 11 Apr 2023 13:10:34 +0200,
Johannes Berg wrote:
>
> [removing Luca, he no longer works on wifi]
>
> Hi,
>
> On Wed, 2023-04-05 at 08:35 +0200, Takashi Iwai wrote:
> > A few models require *.pnvm files while we don't declare them via
> > MODULE_FIRMWARE(). This resulted in the breakage of WiFi on the
> > system that relies on the information from modinfo (e.g. openSUSE
> > installer image).
> >
> > This patch adds those missing MODULE_FIRMWARE() entries for *.pnvm
> > files.
>
> Makes sense. They (may) also exist the previous generation of devices,
> but weren't strictly required then.
>
> > The fix is obviously ad hoc.
>
> Yeah. Maybe we'll merge it anyway though? Do you think this should still
> go to 6.3? Pretty close I guess.
It's a long-standing issue, so no urgent fix is needed.
It can be postponed to 6.4 merge.
> > Here I added the lines with the explicit string since *_PRE definition
> > contains the tailing dash and can't be used for *.pnvm file.
>
> Yeah, we thought about changing that - but I have a larger set of rework
> in this area just done a short while ago, which would make it a bit hard
> to do. Hence maybe we should merge this for 6.3/6.4 and do the larger
> rework plus getting rid of the dash in the *_PRE definitions in 6.5,
> what do you think?
Agreed.
> > Alternatively, we may put a single line
> > MODULE_FIRMWARE("iwlwifi-*.pnvm");
> > to catch all, too.
> >
>
> Unrelated discussion, but ... I didn't even know that was possible.
>
> Maybe this gives us a way out of something else I was thinking about
> recently - the MODULE_FIRMWARE() here in iwlwifi usually only states the
> latest version that the driver accepts, however:
>
> * the driver might be ahead of the firmware releases - in fact that's
> how it usually should be, just due to various issues we haven't been
> upstreaming as quickly as we'd like
> * sometimes we (have to) skip firmware releases due to other issues
> * etc.
>
> So it could be that 6.4 kernel will state e.g. the max version is 78,
> but we end up never even releasing 78 firmware. The MODULE_FIRMWARE()
> would then state 78, but that file would never exist.
>
> Have we just been very lucky with never running into any of these
> issues, and the distro kernels being "old enough" that usually all the
> max version firmware was already released by the time it was used? Or
> did you work around this in some other way?
Heh, we had occasionally a "hot fix" patch for avoiding to point to
the non-existing versions for distro kernels.
> Anyway, if we can use wildcards, maybe instead of stating the max API
> version number of the filename, we should have a wildcard there for the
> number? OTOH, iwlwifi *already* comes with a *lot* of firmware files for
> all the various families of devices and radios, and making the API
> version a wildcard would make it much bigger again, to the point where
> we might as well state something like
>
> MODULE_FIRMWARE("iwlwifi-*")
>
> which is a lot of files ...
Right, this may end up with too many files.
Although there were recent actions in linux-firmware tree to drop the
unused versions, there are still quite many in total, and iwlwifi
firmware files are relatively large, unfortunately.
> Did you see any issues with this versioning thing in the past? And what
> would you think (from a distro POV) about making this a wildcard on the
> version, i.e. having, in things like
>
> #define IWL_QU_B_HR_B_MODULE_FIRMWARE(api) \
> IWL_QU_B_HR_B_FW_PRE __stringify(api) ".ucode"
>
>
> "*" instead of __stringify(api).
>
>
> Some input on this would be nice.
>
> Thanks,
> johannes
As of now, using the wildcard for matching all iwlwifi firmware files
would be worse for us, as it'll lead to drag all those files into the
openSUSE/SUSE installer image and grow the image size significantly.
>From that POV, the current MODULE_FIRMWARE() has been working "good
enough"; as already mentioned, we occasionally fix in the downstream
side to point to the existing firmware version, but that's OK-ish.
thanks,
Takashi
