linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
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"
Date: Mon, 28 Sep 2020 10:10:26 +0200	[thread overview]
Message-ID: <20200928081026.GH2837573@ulmo> (raw)
In-Reply-To: <ede103cf7f6914054a73cf8f1d9725ee13a7cf5d.1601149261.git.mirq-linux@rere.qmqm.pl>

[-- Attachment #1: Type: text/plain, Size: 2089 bytes --]

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.

> ---
>  sound/soc/tegra/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> index 3d91bd3e59cd..a62cc87551ac 100644
> --- a/sound/soc/tegra/Kconfig
> +++ b/sound/soc/tegra/Kconfig
> @@ -39,7 +39,6 @@ config SND_SOC_TEGRA20_I2S
>  config SND_SOC_TEGRA20_SPDIF
>  	tristate "Tegra20 SPDIF interface"
>  	depends on SND_SOC_TEGRA
> -	default m
>  	help
>  	  Say Y or M if you want to add support for the Tegra20 SPDIF interface.
>  	  You will also need to select the individual machine drivers to support

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

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.

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-09-28  8:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-26 19:42 [PATCH] ASoC: tegra20-spdif: remove "default m" Michał Mirosław
2020-09-28  8:10 ` Thierry Reding [this message]
2020-09-28 18:52   ` Michał Mirosław

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200928081026.GH2837573@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=jonathanh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=perex@perex.cz \
    --cc=swarren@nvidia.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).