Re: [PATCH] Revert "ASoC: amd: acp: Power on/off the speaker enable gpio pin based on DAPM callback."

From: Mark Brown <broonie@kernel.org>
To: Curtis Malainey <cujomalainey@gmail.com>
Cc: alsa-devel@alsa-project.org,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Ajit Kumar Pandey <AjitKumar.Pandey@amd.com>,
	Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	V sujith kumar Reddy <vsujithkumar.reddy@amd.com>,
	Curtis Malainey <cujomalainey@chromium.org>,
	Eric Peers <epeers@google.com>, Rob Barnes <robbarnes@google.com>
Subject: Re: [PATCH] Revert "ASoC: amd: acp: Power on/off the speaker enable gpio pin based on DAPM callback."
Date: Tue, 1 Feb 2022 18:40:45 +0000	[thread overview]
Message-ID: <Yfl+rZEucvLEmFjD@sirena.org.uk> (raw)
In-Reply-To: <CAGXAbSq4+YY3qNt2c8J-P278QtUMTkJAo7x3=6UvJof4bH2C2Q@mail.gmail.com>

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

On Mon, Jan 31, 2022 at 10:39:00AM -0800, Curtis Malainey wrote:

> > I feel instead of reverting this complete patch we can quickly submit a
> > new patch set with "gpio_spk_en = NONE" for maxim codec case. As codec
> > driver is anyhow controlling that gpio we don't need to do it from
> > machine driver. We've already done that here
> > https://patchwork.kernel.org/project/alsa-devel/patch/20220131203225.1418648-1-vsujithkumar.reddy@amd.com/

> I'm pretty sure the proper solution is to shove this logic into the
> alc1019 driver like it is in the max98357 driver. The header is
> already there for gpio which makes me think it was planned but
> forgotten. Otherwise everyone else who uses this codec and associated
> pin will have to duplicate this logic in their machine driver.

In general if there's something like a speaker amplifier that's just
controlled via GPIO I'd expect that to be something that's set up by the
machine driver.  If the CODEC is a GPIO provider then the pattern should
be that the CODEC registers with gpiolib and then the machine driver
uses that GPIO, that way the GPIO can get used for any other purpose and
if a system picks another GPIO for whatever reason then that'll just
work.

This gets more annoying for ACPI systems due to their lack of
standards based enumeration of course, the endemic reliance on board
specific quirks causes breakage all over - it looks like this is just an
example of some ACPI systems using firmware description and some not.
Are the systems that need the hard coding here shipping, for example if
the Windows drivers rely on such hard coding rather than enumeration
from ACPI?  If we need the quirking then the fix isn't just a simple
revert, we need to do something that ensures that the support for all
the different systems plays nicely with each other.

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