Augh.
This was very badly done, and I'm not talking about the deferred probe
timeout things that caused problems for people.
On Fri, Jun 3, 2022 at 4:28 AM Greg KH <[email protected]> wrote:
>
> - firmware_loader reorganization and additions including the
> ability to have XZ compressed firmware images and the ability
> for userspace to initiate the firmware load when it needs to,
> instead of being always initiated by the kernel.
This is actively misleading.
We *always* supported XZ compressed firmware images, and it was
enabled by CONFIG_FW_LOADER_COMPRESS,
What's new is the option to use ZSTD compression.
However, the Kconfig file addition for this was done as badly as the
above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
with a help message and a default value that both are complete
garbage.
So when you do "make oldconfig", you would be expected to say 'N', and
in the process you lose the existing XZ compression.
Only when the resulting kernel doesn't boot, and you spent half an
hour trying to bisect things, and you start looking closer, do you
notice that "ooh, the config changed in bad ways".
Yeah, I'm a bit grumpy. This was *really* annoying.
The commit that does this breakage is literally called "firmware: Add
the support for ZSTD-compressed firmware files", and only when looking
closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
DEFAULT.
Because even when keeping the FW_LOADER_COMPRESS option enabled, the
XZ compression is just gone, gone, gone, unless you realize that it
was implicitly enabled before, and now needs that default disable of
FW_LOADER_COMPRESS_XZ to be enabled.
I've said this before, and I'll say it here again (and I bet I'll have
to say it in the future too): the kernel config is probably the most
annoying part of building a kernel for anybody.
And it damn well does NOT HELP when people then actively break things,
and ask actively bad and misleading questions. In this case, for
example, it's not just that the XZ option is now misleading by
default, it's also that the whole thing has been set up so that you
can say "enable compressed images", but then HAVE NO ACTUAL
COMPRESSION METHOD!
Grr. This was *REALLY* badly done.
Linus
On Sat, 04 Jun 2022 00:23:18 +0200,
Linus Torvalds wrote:
>
> Augh.
>
> This was very badly done, and I'm not talking about the deferred probe
> timeout things that caused problems for people.
>
> On Fri, Jun 3, 2022 at 4:28 AM Greg KH <[email protected]> wrote:
> >
> > - firmware_loader reorganization and additions including the
> > ability to have XZ compressed firmware images and the ability
> > for userspace to initiate the firmware load when it needs to,
> > instead of being always initiated by the kernel.
>
> This is actively misleading.
>
> We *always* supported XZ compressed firmware images, and it was
> enabled by CONFIG_FW_LOADER_COMPRESS,
>
> What's new is the option to use ZSTD compression.
>
> However, the Kconfig file addition for this was done as badly as the
> above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
> with a help message and a default value that both are complete
> garbage.
>
> So when you do "make oldconfig", you would be expected to say 'N', and
> in the process you lose the existing XZ compression.
>
> Only when the resulting kernel doesn't boot, and you spent half an
> hour trying to bisect things, and you start looking closer, do you
> notice that "ooh, the config changed in bad ways".
>
> Yeah, I'm a bit grumpy. This was *really* annoying.
>
> The commit that does this breakage is literally called "firmware: Add
> the support for ZSTD-compressed firmware files", and only when looking
> closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
> DEFAULT.
>
> Because even when keeping the FW_LOADER_COMPRESS option enabled, the
> XZ compression is just gone, gone, gone, unless you realize that it
> was implicitly enabled before, and now needs that default disable of
> FW_LOADER_COMPRESS_XZ to be enabled.
>
> I've said this before, and I'll say it here again (and I bet I'll have
> to say it in the future too): the kernel config is probably the most
> annoying part of building a kernel for anybody.
>
> And it damn well does NOT HELP when people then actively break things,
> and ask actively bad and misleading questions. In this case, for
> example, it's not just that the XZ option is now misleading by
> default, it's also that the whole thing has been set up so that you
> can say "enable compressed images", but then HAVE NO ACTUAL
> COMPRESSION METHOD!
>
> Grr. This was *REALLY* badly done.
Mea culpa, I hesitated to add "default y", but it should have been
added at least for the case with the config takeover case. Now I see
that you've already added "default y" to
CONFIG_FW_LOADER_COMPRESS_XZ. Thanks for taking care of this!
Takashi
On Sat, Jun 04, 2022 at 08:32:29AM +0200, Takashi Iwai wrote:
> On Sat, 04 Jun 2022 00:23:18 +0200,
> Linus Torvalds wrote:
> >
> > Augh.
> >
> > This was very badly done, and I'm not talking about the deferred probe
> > timeout things that caused problems for people.
> >
> > On Fri, Jun 3, 2022 at 4:28 AM Greg KH <[email protected]> wrote:
> > >
> > > - firmware_loader reorganization and additions including the
> > > ability to have XZ compressed firmware images and the ability
> > > for userspace to initiate the firmware load when it needs to,
> > > instead of being always initiated by the kernel.
> >
> > This is actively misleading.
> >
> > We *always* supported XZ compressed firmware images, and it was
> > enabled by CONFIG_FW_LOADER_COMPRESS,
> >
> > What's new is the option to use ZSTD compression.
> >
> > However, the Kconfig file addition for this was done as badly as the
> > above explanation was, and the FW_LOADER_COMPRESS_XZ option was added
> > with a help message and a default value that both are complete
> > garbage.
> >
> > So when you do "make oldconfig", you would be expected to say 'N', and
> > in the process you lose the existing XZ compression.
> >
> > Only when the resulting kernel doesn't boot, and you spent half an
> > hour trying to bisect things, and you start looking closer, do you
> > notice that "ooh, the config changed in bad ways".
> >
> > Yeah, I'm a bit grumpy. This was *really* annoying.
> >
> > The commit that does this breakage is literally called "firmware: Add
> > the support for ZSTD-compressed firmware files", and only when looking
> > closer do you notice that IT REMOVES SUPPORT FOR XZ COMPRESSION BY
> > DEFAULT.
> >
> > Because even when keeping the FW_LOADER_COMPRESS option enabled, the
> > XZ compression is just gone, gone, gone, unless you realize that it
> > was implicitly enabled before, and now needs that default disable of
> > FW_LOADER_COMPRESS_XZ to be enabled.
> >
> > I've said this before, and I'll say it here again (and I bet I'll have
> > to say it in the future too): the kernel config is probably the most
> > annoying part of building a kernel for anybody.
> >
> > And it damn well does NOT HELP when people then actively break things,
> > and ask actively bad and misleading questions. In this case, for
> > example, it's not just that the XZ option is now misleading by
> > default, it's also that the whole thing has been set up so that you
> > can say "enable compressed images", but then HAVE NO ACTUAL
> > COMPRESSION METHOD!
> >
> > Grr. This was *REALLY* badly done.
>
> Mea culpa, I hesitated to add "default y", but it should have been
> added at least for the case with the config takeover case. Now I see
> that you've already added "default y" to
> CONFIG_FW_LOADER_COMPRESS_XZ. Thanks for taking care of this!
Sorry about that, I missed it too as I just selected that option to
ensure it built and worked properly on my systems :(
greg k-h