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

["Yann E. MORIN" <yann.morin.1998@free.fr>]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot