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

[Stefan Agner <stefan@agner.ch>]

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
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot