linux-kernel - Re: [PATCH] ASoC: tegra20-spdif: remove "default m"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200928185253.GB25616@qmqm.qmqm.pl>
Date:   Mon, 28 Sep 2020 20:52:53 +0200
From:   Michał Mirosław <mirq-linux@...e.qmqm.pl>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Stephen Warren <swarren@...dia.com>,
        alsa-devel@...a-project.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: tegra20-spdif: remove "default m"

On Mon, Sep 28, 2020 at 10:10:26AM +0200, Thierry Reding wrote:
> On Sat, Sep 26, 2020 at 09:42:40PM +0200, Michał Mirosław wrote:
> > Make tegra20-spdif default to N as all other drivers do.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
> > Fixes: 774fec338bfc ("ASoC: Tegra: Implement SPDIF CPU DAI")
> 
> I don't think this is warranted. This doesn't fix a bug or anything.
> It's merely a change in the default configuration. The presence of a
> Fixes: tag is typically used as a hint for people to pick this up into
> stable releases, but I don't think this qualifies.
[...]

Fixes is just for pointing where the bug or issue originated.  I usually
include it to help you -- the reviewer -- and backporters if they ever
want to use this patch. It is not specific to stable-directed patches.

For stable candidates there is 'Cc: stable' tag (no need for this patch).

> So now by default this driver will be disabled, which means that Linux
> is going to regress for people that rely on this driver.

The bug is that this driver (and only this driver in the whole
sound/soc/tegra directory) defaults to m, where all other drivers default
to n (as the policy aboud drivers seems to be [1]). This won't affect
oldconfig, allyesconfig nor allmodconfig, but will not be selected now
for clean builds - meaning less work for those not building for Tegra2.

[1] https://lkml.org/lkml/2017/11/18/257

> You need to at least follow this up with a patch that makes the
> corresponding change in both tegra_defconfig and multi_v7_defconfig to
> ensure that this driver is going to get built by default.

This I can do. Not all such drivers are enabled, though: eg. AHUB driver
is not. Maybe we need bigger refresh of the defconfigs instead?

> Given the above it's probably also a good idea to explain a bit more in
> the commit message about what you're trying to achieve. Yes, "default n"
> is usually the right thing to do and I'm honestly not sure why Stephen
> chose to make this "default m" back in the day. Given that it depends on
> SND_SOC_TEGRA, which itself is "default n", I think this makes some
> sense, even if in retrospect it ended up being a bit inconsistent (you
> could probably argue that all patches after this are the ones that were
> inconsistent instead). This was merged over 9 years ago and a lot of
> common practices have changed over that period of time.

Yes, this is a cleanup. :-)

Best Regards
Michał Mirosław