diff mbox series

[RFC] package/linux-firmware: Add more Intel WiFi 22000 series

Message ID 0e18605c9938776a707a4aab032be74a1a9afe8e.1660828116.git.stefan@agner.ch
State Changes Requested
Headers show
Series [RFC] package/linux-firmware: Add more Intel WiFi 22000 series | expand

Commit Message

Stefan Agner Aug. 18, 2022, 1:18 p.m. UTC
Add more Intel WiFi 22000 series firmware files. Allow to select the
firmware version using shell globs.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
With the Intel WiFi 22000 series the amount of firmwares is just stagering. So
much so that it causes size issues in our case.

Intel firmwares seem to have an API version, which is the last digit before the
file ending .ucode. The Linux kernel sources have defines which API each
individual driver supports (e.g. Linux 5.15 22000 series kernel driver
supports ucode API 39 up to 66).

This crude method uses just file glob matching to allow to select firmwares
which match the kernel in use. However, this approach has a problem: Some
API versions are missing. E.g. simply using "66" doesn't work since some
firmware are not available with the latest API supported (e.g.
iwlwifi-so-a0-gf-a0-64.ucode).

Also if an older kernel is in use, some firmware files with an older maximum
API might just be missing, which leads to tar complaining about missing files.

In this case, we can use 6[46], which happens to select the newest version of
all firmwares (it seems that firmwares which are available with API version 66
have only been released with version 63 before).

However, this of course might be just a coincidence. Ideally Buildroot would
select the newest version available for a particular API version. Not sure if
this is easily doable.

Thoughts?

--
Stefan

 package/linux-firmware/Config.in         |  9 +++++++++
 package/linux-firmware/linux-firmware.mk | 15 ++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Aug. 20, 2022, 12:13 p.m. UTC | #1
Stefan, All,

On 2022-08-18 15:18 +0200, Stefan Agner spake thusly:
> Add more Intel WiFi 22000 series firmware files. Allow to select the
> firmware version using shell globs.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> With the Intel WiFi 22000 series the amount of firmwares is just stagering. So
> much so that it causes size issues in our case.

You know that you can write a post-build script to tweak the target/
directory at the end of the build? That can be a workaround until a
better solution is devised.

> Intel firmwares seem to have an API version, which is the last digit before the
> file ending .ucode. The Linux kernel sources have defines which API each
> individual driver supports (e.g. Linux 5.15 22000 series kernel driver
> supports ucode API 39 up to 66).
> 
> This crude method uses just file glob matching to allow to select firmwares
> which match the kernel in use. However, this approach has a problem: Some
> API versions are missing. E.g. simply using "66" doesn't work since some
> firmware are not available with the latest API supported (e.g.
> iwlwifi-so-a0-gf-a0-64.ucode).
> 
> Also if an older kernel is in use, some firmware files with an older maximum
> API might just be missing, which leads to tar complaining about missing files.
> 
> In this case, we can use 6[46], which happens to select the newest version of
> all firmwares (it seems that firmwares which are available with API version 66
> have only been released with version 63 before).
> 
> However, this of course might be just a coincidence. Ideally Buildroot would
> select the newest version available for a particular API version. Not sure if
> this is easily doable.
> 
> Thoughts?

First: on one hand, I like the simplicity of that patch; if we can't
come up with better, I can see going with it.

But on the other hand, it is also very un-userfriendly. I don't see how
we could _reasonably_ come up with a way to cherry-pick appropriate
firmware files.

First, the linux package depends on linux-frimware, so linux-firmware
can't depend on linux; we could still use a patch-dependency, so that
linux gets at least extracted and patched before linux-firmware gets
installed, but that is going to be tricky to maintain.

We could then grab IWL_22000_UCODE_API_MIN and IWL_22000_UCODE_API_MAX
from drivers/net/wireless/intel/iwlwifi/cfg/22000.c

And then, we'd have to code some non-trivial magic that iterates over
all iwlwifi-Qu{,Z}-*.ucode and check if their API part is in the range,
and for a single "family" of firmwares, keep the highest one (how do we
know that two firmware files are f the same family? Just because they
only differ in API version?) That's a bit brittle...

[--SNIP--]
> diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk
> index 64d096df14..2283cd7d5e 100644
> --- a/package/linux-firmware/linux-firmware.mk
> +++ b/package/linux-firmware/linux-firmware.mk
> @@ -436,7 +436,20 @@ LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.QualcommAtheros_ath10k
>  endif
>  
>  ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000),y)
> -LINUX_FIRMWARE_FILES += iwlwifi-QuZ-*.ucode iwlwifi-Qu-*.ucode
> +LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
> +LINUX_FIRMWARE_FILES += \
> +	iwlwifi-Qu-b0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-c0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-b0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-c0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-QuZ-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-QuZ-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-cc-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-ty-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-gf4-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode

This list is not entirely alphabetically sorted.

Also, why do you extend the prefixes, from iwlwifi-QuZ- and iwlwifi-Qu-,
to include extra c0, b0, a0 and so on? Why can we just have:
    iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
    iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode

Oh, and in at least linu 5.17, there are also references to
iwlwifi-QuQnj-, iwlwifi-SoSnj- and a bunch of others. And
specifically, there is also iwlwifi-cc-a0- which in Buildroot is
installed with BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260 and not
BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000

So, maybe we could split the families further as an alternate solution?

Regards,
Yann E. MORIN.

>  LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENCE.iwlwifi_firmware
>  endif
>  
> -- 
> 2.37.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Stefan Agner Aug. 20, 2022, 1:40 p.m. UTC | #2
On 2022-08-20 14:13, Yann E. MORIN wrote:
> Stefan, All,
> 
> On 2022-08-18 15:18 +0200, Stefan Agner spake thusly:
>> Add more Intel WiFi 22000 series firmware files. Allow to select the
>> firmware version using shell globs.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> With the Intel WiFi 22000 series the amount of firmwares is just stagering. So
>> much so that it causes size issues in our case.
> 
> You know that you can write a post-build script to tweak the target/
> directory at the end of the build? That can be a workaround until a
> better solution is devised.

Thanks, good idea, I'll consider that!

> 
>> Intel firmwares seem to have an API version, which is the last digit before the
>> file ending .ucode. The Linux kernel sources have defines which API each
>> individual driver supports (e.g. Linux 5.15 22000 series kernel driver
>> supports ucode API 39 up to 66).
>>
>> This crude method uses just file glob matching to allow to select firmwares
>> which match the kernel in use. However, this approach has a problem: Some
>> API versions are missing. E.g. simply using "66" doesn't work since some
>> firmware are not available with the latest API supported (e.g.
>> iwlwifi-so-a0-gf-a0-64.ucode).
>>
>> Also if an older kernel is in use, some firmware files with an older maximum
>> API might just be missing, which leads to tar complaining about missing files.
>>
>> In this case, we can use 6[46], which happens to select the newest version of
>> all firmwares (it seems that firmwares which are available with API version 66
>> have only been released with version 63 before).
>>
>> However, this of course might be just a coincidence. Ideally Buildroot would
>> select the newest version available for a particular API version. Not sure if
>> this is easily doable.
>>
>> Thoughts?
> 
> First: on one hand, I like the simplicity of that patch; if we can't
> come up with better, I can see going with it.
> 
> But on the other hand, it is also very un-userfriendly. I don't see how
> we could _reasonably_ come up with a way to cherry-pick appropriate
> firmware files.
> 
> First, the linux package depends on linux-frimware, so linux-firmware
> can't depend on linux; we could still use a patch-dependency, so that
> linux gets at least extracted and patched before linux-firmware gets
> installed, but that is going to be tricky to maintain.
> 
> We could then grab IWL_22000_UCODE_API_MIN and IWL_22000_UCODE_API_MAX
> from drivers/net/wireless/intel/iwlwifi/cfg/22000.c
> 
> And then, we'd have to code some non-trivial magic that iterates over
> all iwlwifi-Qu{,Z}-*.ucode and check if their API part is in the range,
> and for a single "family" of firmwares, keep the highest one (how do we
> know that two firmware files are f the same family? Just because they
> only differ in API version?) That's a bit brittle...

To avoid the direct dependency I was thinking of using the
BR2_TOOLCHAIN_HEADERS_AT_LEAST Kconfigs, essentially maintain a list of
default max supported API per kernel version in Kconfig. Not sure if
that is a good idea.


> 
> [--SNIP--]
>> diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk
>> index 64d096df14..2283cd7d5e 100644
>> --- a/package/linux-firmware/linux-firmware.mk
>> +++ b/package/linux-firmware/linux-firmware.mk
>> @@ -436,7 +436,20 @@ LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.QualcommAtheros_ath10k
>>  endif
>>
>>  ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000),y)
>> -LINUX_FIRMWARE_FILES += iwlwifi-QuZ-*.ucode iwlwifi-Qu-*.ucode
>> +LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
>> +LINUX_FIRMWARE_FILES += \
>> +	iwlwifi-Qu-b0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-Qu-c0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-Qu-b0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-Qu-c0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-QuZ-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-QuZ-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-cc-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-so-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-so-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-so-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-ty-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>> +	iwlwifi-so-a0-gf4-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
> 
> This list is not entirely alphabetically sorted.

This is on purpose, I've used the order in
drivers/net/wireless/intel/iwlwifi/cfg/22000.c.

> 
> Also, why do you extend the prefixes, from iwlwifi-QuZ- and iwlwifi-Qu-,
> to include extra c0, b0, a0 and so on? Why can we just have:
>     iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
>     iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
> 

Same reason: Align with kernel sources.

> Oh, and in at least linu 5.17, there are also references to
> iwlwifi-QuQnj-, iwlwifi-SoSnj- and a bunch of others. And
> specifically, there is also iwlwifi-cc-a0- which in Buildroot is
> installed with BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260 and not
> BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000

There are more firmwares in the kernel sources than present in the
linux-firmware git repository. This is essentially the common
denominator of Linux 5.15 and the current version of linux-firmware. It
might deploy too many firmwares for an older kernel, but it is
guaranteed to work (list a non existing firmware causes the tar command
in the build step to complain).

> 
> So, maybe we could split the families further as an alternate solution?

I'd prefer to group them as they are grouped in the kernel sources.

Some kernel drivers have multiple API max versions (e.g. 6000.c), for
those it probably would make sense to have one config per (distinct) API
version max.

--
Stefan

> 
> Regards,
> Yann E. MORIN.
> 
>>  LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENCE.iwlwifi_firmware
>>  endif
>>
>> --
>> 2.37.2
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
Yann E. MORIN Aug. 20, 2022, 2:21 p.m. UTC | #3
Stefan, All,

On 2022-08-20 15:40 +0200, Stefan Agner spake thusly:
> On 2022-08-20 14:13, Yann E. MORIN wrote:
> > And then, we'd have to code some non-trivial magic that iterates over
> > all iwlwifi-Qu{,Z}-*.ucode and check if their API part is in the range,
> > and for a single "family" of firmwares, keep the highest one (how do we
> > know that two firmware files are f the same family? Just because they
> > only differ in API version?) That's a bit brittle...
> To avoid the direct dependency I was thinking of using the
> BR2_TOOLCHAIN_HEADERS_AT_LEAST Kconfigs, essentially maintain a list of
> default max supported API per kernel version in Kconfig. Not sure if
> that is a good idea.

Indeed, not really, because the version of the running kernel may be
different than the version of the kernel headers used to buid the
toolchain.

For example, you could build a toolchain with kernel headers 3.0, and
run a 5.19 kernel. In such a case, the highest API derived from the
kernel headers version may very well be lower than the lowest API
supported by the running kernel.

> >> +LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
> >> +LINUX_FIRMWARE_FILES += \
[--SNIP--]
> > This list is not entirely alphabetically sorted.
> This is on purpose, I've used the order in
> drivers/net/wireless/intel/iwlwifi/cfg/22000.c.

Then, add a comment above, like:
    # Keep this list in the same order as in kernel's driver

> > Also, why do you extend the prefixes, from iwlwifi-QuZ- and iwlwifi-Qu-,
> > to include extra c0, b0, a0 and so on? Why can we just have:
> >     iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
> >     iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
> Same reason: Align with kernel sources.

But then, users would had a need for one of the firmwares now excluded
from the list will have no way to enable them.

> > Oh, and in at least linu 5.17, there are also references to
> > iwlwifi-QuQnj-, iwlwifi-SoSnj- and a bunch of others. And
> > specifically, there is also iwlwifi-cc-a0- which in Buildroot is
> > installed with BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260 and not
> > BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
> There are more firmwares in the kernel sources than present in the
> linux-firmware git repository. This is essentially the common
> denominator of Linux 5.15 and the current version of linux-firmware. It

But why limit ourselves to what is know to 5.15? If someone uses 5.19,
they might need other firmware files? And even if 5.15 and 5.17 have
exactly the same set, then the upcoming 6.0 might need more or a newer
familly (the QUQnj or QUSnj for example).

> might deploy too many firmwares for an older kernel, but it is
> guaranteed to work (list a non existing firmware causes the tar command
> in the build step to complain).

Well, it is easy to solve:

    # Fiter-out firmware famillies that do not match the API version glob
    # so that 'tar' does not whine later on.
    LINUX_FIRMWARE_FILES += \
        $(notdir $(wildcard $(patsubst %,$(@D)/%, \
            iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
            iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
            iwlwifi-QuQnj-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
            iwlwifi-QuSnj-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
        )))

But adding new firmware "categories" (QuQnj or QuSnj et al.) should be a
seaprate patch, of course.

> > So, maybe we could split the families further as an alternate solution?
> I'd prefer to group them as they are grouped in the kernel sources.

I am slightly conflicted on this one. I would prefer they be grouped by
whatever the user can use to identify the chip: by actually looking up
the reference on the chip, by looking up lspci/lsusb/lshw/... But I can
also see the appeal of matching the kernel driver... But if we were to
change the categorisation, that would be in a separate patch.

But maybe, going for boolean entries was not a good idea to begin with,
and just a single glob option for all of the iwlwifi family as a whole
would be better. Like:

    config BR2_PKG_LNX_FW_IWLWIFI
        bool "iwlwifi familly/ies"

    config BR2_PKG_LNX_FW_IWLWIFI_GLOBS
        string "iwlwifi familly globs"
        default "*"
        depends on BR2_PKG_LNX_FW_IWLWIFI
        help
          List of shell globs to match firmware files.

          For example:
            - BR2_PKG_LNX_FW_IWLWIFI_GLOBS="*"
              would match all of:
                iwlwifi-*.ucode

            - BR2_PKG_LNX_FW_IWLWIFI_GLOBS="Qu-* QuZ-*"
              would match all of:
                iwlwifi-Qu-*.ucode
                iwlwifi-QuZ-*.ucode

This is way easier to do and, with the $(wildcard) clause I suggested
above), should cover all of the iwlwifi cases, and we can coalesce the
22000, 22260, 3160, 3168, 3945, 4965, 5000, 6000G2A, 6000G2B, 7260,
7265, 7265D, 8000C, 8265, and 9XXX, into a single option with a glob.
But that is not very user-friendly...

Thomas, Arnout, Peter: thoughts?

Regards,
Yann E. MORIN.
Arnout Vandecappelle Aug. 23, 2022, 7:20 p.m. UTC | #4
On 20/08/2022 16:21, Yann E. MORIN wrote:
> Stefan, All,
> 
> On 2022-08-20 15:40 +0200, Stefan Agner spake thusly:
>> On 2022-08-20 14:13, Yann E. MORIN wrote:
>>> And then, we'd have to code some non-trivial magic that iterates over
>>> all iwlwifi-Qu{,Z}-*.ucode and check if their API part is in the range,
>>> and for a single "family" of firmwares, keep the highest one (how do we
>>> know that two firmware files are f the same family? Just because they
>>> only differ in API version?) That's a bit brittle...
>> To avoid the direct dependency I was thinking of using the
>> BR2_TOOLCHAIN_HEADERS_AT_LEAST Kconfigs, essentially maintain a list of
>> default max supported API per kernel version in Kconfig. Not sure if
>> that is a good idea.
> 
> Indeed, not really, because the version of the running kernel may be
> different than the version of the kernel headers used to buid the
> toolchain.
> 
> For example, you could build a toolchain with kernel headers 3.0, and
> run a 5.19 kernel. In such a case, the highest API derived from the
> kernel headers version may very well be lower than the lowest API
> supported by the running kernel.
> 
>>>> +LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
>>>> +LINUX_FIRMWARE_FILES += \
> [--SNIP--]
>>> This list is not entirely alphabetically sorted.
>> This is on purpose, I've used the order in
>> drivers/net/wireless/intel/iwlwifi/cfg/22000.c.
> 
> Then, add a comment above, like:
>      # Keep this list in the same order as in kernel's driver
> 
>>> Also, why do you extend the prefixes, from iwlwifi-QuZ- and iwlwifi-Qu-,
>>> to include extra c0, b0, a0 and so on? Why can we just have:
>>>      iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
>>>      iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode

  Note that this could be simplified to just
iwlwifi-Q*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode

>> Same reason: Align with kernel sources.
> 
> But then, users would had a need for one of the firmwares now excluded
> from the list will have no way to enable them.

  To allow the user full freedom there, the u, uZ and a0 etc. could be made part 
of the glob, i.e.
iwlwifi-Q$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode


>>> Oh, and in at least linu 5.17, there are also references to
>>> iwlwifi-QuQnj-, iwlwifi-SoSnj- and a bunch of others. And
>>> specifically, there is also iwlwifi-cc-a0- which in Buildroot is
>>> installed with BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260 and not
>>> BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
>> There are more firmwares in the kernel sources than present in the
>> linux-firmware git repository. This is essentially the common
>> denominator of Linux 5.15 and the current version of linux-firmware. It
> 
> But why limit ourselves to what is know to 5.15? If someone uses 5.19,
> they might need other firmware files? And even if 5.15 and 5.17 have
> exactly the same set, then the upcoming 6.0 might need more or a newer
> familly (the QUQnj or QUSnj for example).

  Also, when we update linux-firmware, we're not going to look at all the new 
blobs that are in there. So we want to be as future-safe as possible with the 
globs we use.


>> might deploy too many firmwares for an older kernel, but it is
>> guaranteed to work (list a non existing firmware causes the tar command
>> in the build step to complain).
> 
> Well, it is easy to solve:
> 
>      # Fiter-out firmware famillies that do not match the API version glob
>      # so that 'tar' does not whine later on.
>      LINUX_FIRMWARE_FILES += \
>          $(notdir $(wildcard $(patsubst %,$(@D)/%, \

  Maybe we should just add "shopt nullglob" to the command that creates the tarball?

>              iwlwifi-Qu-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \

  It would make sense for the globs to be allowed to be a list of globs instead 
of a single one - but that would be a bit complicated to implement together with 
the wildcard.

>              iwlwifi-QuZ-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>              iwlwifi-QuQnj-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>              iwlwifi-QuSnj-*-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
>          )))
> 
> But adding new firmware "categories" (QuQnj or QuSnj et al.) should be a
> seaprate patch, of course.
> 
>>> So, maybe we could split the families further as an alternate solution?
>> I'd prefer to group them as they are grouped in the kernel sources.
> 
> I am slightly conflicted on this one. I would prefer they be grouped by
> whatever the user can use to identify the chip: by actually looking up
> the reference on the chip, by looking up lspci/lsusb/lshw/... But I can
> also see the appeal of matching the kernel driver... But if we were to
> change the categorisation, that would be in a separate patch.

  Well, you actually want to have just the firmware that matches both the chip 
and the driver... Our current options for linux-firmware are a bit in the middle.


> But maybe, going for boolean entries was not a good idea to begin with,
> and just a single glob option for all of the iwlwifi family as a whole
> would be better. Like:
> 
>      config BR2_PKG_LNX_FW_IWLWIFI
>          bool "iwlwifi familly/ies"
> 
>      config BR2_PKG_LNX_FW_IWLWIFI_GLOBS
>          string "iwlwifi familly globs"
>          default "*"
>          depends on BR2_PKG_LNX_FW_IWLWIFI
>          help
>            List of shell globs to match firmware files.
> 
>            For example:
>              - BR2_PKG_LNX_FW_IWLWIFI_GLOBS="*"
>                would match all of:
>                  iwlwifi-*.ucode
> 
>              - BR2_PKG_LNX_FW_IWLWIFI_GLOBS="Qu-* QuZ-*"
>                would match all of:
>                  iwlwifi-Qu-*.ucode
>                  iwlwifi-QuZ-*.ucode
> 
> This is way easier to do and, with the $(wildcard) clause I suggested
> above), should cover all of the iwlwifi cases, and we can coalesce the
> 22000, 22260, 3160, 3168, 3945, 4965, 5000, 6000G2A, 6000G2B, 7260,
> 7265, 7265D, 8000C, 8265, and 9XXX, into a single option with a glob.
> But that is not very user-friendly...

  I think the way it is split now is OK. Most of the other chips have just a few 
versions and are much less bad than 22000.


> Thomas, Arnout, Peter: thoughts?

  I think the current patch, if simplified as proposed by Yann, strikes a good 
middle ground.

  One way how we could improve the situation globally is to add an open-ended 
string option that allows the user to specify a list of files or directories or 
globs to include. That way, if you don't want the redundant API versions, you 
can simply only set that option and not the named ones, with exactly the 
firmware files (or globs) that you need.

  Regards,
  Arnout

> 
> Regards,
> Yann E. MORIN.
>
Yann E. MORIN April 16, 2023, 11:54 a.m. UTC | #5
Stefan, All,

On 2022-08-18 15:18 +0200, Stefan Agner spake thusly:
> Add more Intel WiFi 22000 series firmware files. Allow to select the
> firmware version using shell globs.

There has been some feedback suggesting some alternative approach, so
I've marked this patch as changes requested in patchwork, until there is
an improved patch.

Thanks!

Regards,
Yann E. MORIN.

> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> With the Intel WiFi 22000 series the amount of firmwares is just stagering. So
> much so that it causes size issues in our case.
> 
> Intel firmwares seem to have an API version, which is the last digit before the
> file ending .ucode. The Linux kernel sources have defines which API each
> individual driver supports (e.g. Linux 5.15 22000 series kernel driver
> supports ucode API 39 up to 66).
> 
> This crude method uses just file glob matching to allow to select firmwares
> which match the kernel in use. However, this approach has a problem: Some
> API versions are missing. E.g. simply using "66" doesn't work since some
> firmware are not available with the latest API supported (e.g.
> iwlwifi-so-a0-gf-a0-64.ucode).
> 
> Also if an older kernel is in use, some firmware files with an older maximum
> API might just be missing, which leads to tar complaining about missing files.
> 
> In this case, we can use 6[46], which happens to select the newest version of
> all firmwares (it seems that firmwares which are available with API version 66
> have only been released with version 63 before).
> 
> However, this of course might be just a coincidence. Ideally Buildroot would
> select the newest version available for a particular API version. Not sure if
> this is easily doable.
> 
> Thoughts?
> 
> --
> Stefan
> 
>  package/linux-firmware/Config.in         |  9 +++++++++
>  package/linux-firmware/linux-firmware.mk | 15 ++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in
> index 8ce71140da..49a40283c8 100644
> --- a/package/linux-firmware/Config.in
> +++ b/package/linux-firmware/Config.in
> @@ -181,6 +181,15 @@ config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
>  	  Firmware files for the Intel Wifi 22000 devices supported by
>  	  the iwlwifi kernel driver.
>  
> +if BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
> +
> +config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB
> +        string "*"
> +        help
> +	  API level for firmware files of the Intel Wifi 22000 devices.
> +
> +endif
> +
>  config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260
>  	bool "Intel iwlwifi 22260"
>  	help
> diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk
> index 64d096df14..2283cd7d5e 100644
> --- a/package/linux-firmware/linux-firmware.mk
> +++ b/package/linux-firmware/linux-firmware.mk
> @@ -436,7 +436,20 @@ LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.QualcommAtheros_ath10k
>  endif
>  
>  ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000),y)
> -LINUX_FIRMWARE_FILES += iwlwifi-QuZ-*.ucode iwlwifi-Qu-*.ucode
> +LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
> +LINUX_FIRMWARE_FILES += \
> +	iwlwifi-Qu-b0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-c0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-b0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-Qu-c0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-QuZ-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-QuZ-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-cc-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-ty-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
> +	iwlwifi-so-a0-gf4-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
>  LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENCE.iwlwifi_firmware
>  endif
>  
> -- 
> 2.37.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in
index 8ce71140da..49a40283c8 100644
--- a/package/linux-firmware/Config.in
+++ b/package/linux-firmware/Config.in
@@ -181,6 +181,15 @@  config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
 	  Firmware files for the Intel Wifi 22000 devices supported by
 	  the iwlwifi kernel driver.
 
+if BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000
+
+config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB
+        string "*"
+        help
+	  API level for firmware files of the Intel Wifi 22000 devices.
+
+endif
+
 config BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22260
 	bool "Intel iwlwifi 22260"
 	help
diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk
index 64d096df14..2283cd7d5e 100644
--- a/package/linux-firmware/linux-firmware.mk
+++ b/package/linux-firmware/linux-firmware.mk
@@ -436,7 +436,20 @@  LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.QualcommAtheros_ath10k
 endif
 
 ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000),y)
-LINUX_FIRMWARE_FILES += iwlwifi-QuZ-*.ucode iwlwifi-Qu-*.ucode
+LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB = $(call qstrip,$(BR2_PACKAGE_LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB))
+LINUX_FIRMWARE_FILES += \
+	iwlwifi-Qu-b0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-Qu-c0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-Qu-b0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-Qu-c0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-QuZ-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-QuZ-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-cc-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-so-a0-jf-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-so-a0-hr-b0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-so-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-ty-a0-gf-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode \
+	iwlwifi-so-a0-gf4-a0-$(LINUX_FIRMWARE_IWLWIFI_22000_UCODE_API_GLOB).ucode
 LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENCE.iwlwifi_firmware
 endif