Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 28 Sep 2020 20:52:53 +0200
From:   =?iso-8859-2?Q?Micha=B3_Miros=B3aw?= <mirq-linux@rere.qmqm.pl>
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Stephen Warren <swarren@nvidia.com>,
        alsa-devel@alsa-project.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ASoC: tegra20-spdif: remove "default m"
Message-ID: <20200928185253.GB25616@qmqm.qmqm.pl>
References: <ede103cf7f6914054a73cf8f1d9725ee13a7cf5d.1601149261.git.mirq-linux@rere.qmqm.pl>
 <20200928081026.GH2837573@ulmo>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-2
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200928081026.GH2837573@ulmo>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

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@rere.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