[PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[V sujith kumar Reddy]

Maxim codec driver already enabling/disabling spk_en_gpio in form of sd_mode gpio
hence remove such gpio access control from machine driver to avoid conflict

Signed-off-by: V sujith kumar Reddy <vsujithkumar.reddy@amd.com>
---
 sound/soc/amd/acp/acp-mach.h     | 1 -
 sound/soc/amd/acp/acp-sof-mach.c | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
index fd6299844ebe..c855f50d6b34 100644
--- a/sound/soc/amd/acp/acp-mach.h
+++ b/sound/soc/amd/acp/acp-mach.h
@@ -21,7 +21,6 @@
 #include <linux/gpio/consumer.h>
 
 #define EN_SPKR_GPIO_GB                0x11F
-#define EN_SPKR_GPIO_NK                0x146
 #define EN_SPKR_GPIO_NONE      -EINVAL
 
 enum be_id {
diff --git a/sound/soc/amd/acp/acp-sof-mach.c b/sound/soc/amd/acp/acp-sof-mach.c
index c1d9650fc222..3346677949e3 100644
--- a/sound/soc/amd/acp/acp-sof-mach.c
+++ b/sound/soc/amd/acp/acp-sof-mach.c
@@ -37,7 +37,7 @@ static struct acp_card_drvdata sof_rt5682_max_data = {
 	.hs_codec_id = RT5682,
 	.amp_codec_id = MAX98360A,
 	.dmic_codec_id = DMIC,
-	.gpio_spkr_en = EN_SPKR_GPIO_NK,
+	.gpio_spkr_en = EN_SPKR_GPIO_NONE,
 };
 
 static struct acp_card_drvdata sof_rt5682s_rt1019_data = {
@@ -56,7 +56,7 @@ static struct acp_card_drvdata sof_rt5682s_max_data = {
 	.hs_codec_id = RT5682S,
 	.amp_codec_id = MAX98360A,
 	.dmic_codec_id = DMIC,
-	.gpio_spkr_en = EN_SPKR_GPIO_NK,
+	.gpio_spkr_en = EN_SPKR_GPIO_NONE,
 };
 
 static const struct snd_kcontrol_new acp_controls[] = {
-- 
2.25.1

Re: [PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[Mark Brown]

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

On Tue, Feb 01, 2022 at 02:02:15AM +0530, V sujith kumar Reddy wrote:

> Maxim codec driver already enabling/disabling spk_en_gpio in form of sd_mode gpio
> hence remove such gpio access control from machine driver to avoid conflict


> -	.gpio_spkr_en = EN_SPKR_GPIO_NK,
> +	.gpio_spkr_en = EN_SPKR_GPIO_NONE,
>  };
>  
>  static struct acp_card_drvdata sof_rt5682s_rt1019_data = {
> @@ -56,7 +56,7 @@ static struct acp_card_drvdata sof_rt5682s_max_data = {
>  	.hs_codec_id = RT5682S,
>  	.amp_codec_id = MAX98360A,
>  	.dmic_codec_id = DMIC,
> -	.gpio_spkr_en = EN_SPKR_GPIO_NK,
> +	.gpio_spkr_en = EN_SPKR_GPIO_NONE,
>  };

This looks like a good fix for the immediate issue which fixes the
MAX9360A systems without breaking the RT1019 ones, however as I said in
the thread about Curtis' revert it's not clear what the ideal thing is
here.  There's no documentation about the RT1019 that I can find so it's
not clear to me what exactly the GPIO is controlling, if it's part of
the RT1019 or something else.  That influences where the most tasteful
place to control this GPIO is.  Curtis noted that the RT1019 driver
includes gpiolib headers but that could just as easily be cut'n'paste as
intentional.  What's the story here?

I do also note that the current code just turns the GPIO on and leaves
it on which presumably isn't great from a power management point of
view.

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

Re: [PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[Curtis Malainey]

On Tue, Feb 1, 2022 at 10:56 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Feb 01, 2022 at 02:02:15AM +0530, V sujith kumar Reddy wrote:
>
> > Maxim codec driver already enabling/disabling spk_en_gpio in form of sd_mode gpio
> > hence remove such gpio access control from machine driver to avoid conflict
>
>
> > -     .gpio_spkr_en = EN_SPKR_GPIO_NK,
> > +     .gpio_spkr_en = EN_SPKR_GPIO_NONE,
> >  };
> >
> >  static struct acp_card_drvdata sof_rt5682s_rt1019_data = {
> > @@ -56,7 +56,7 @@ static struct acp_card_drvdata sof_rt5682s_max_data = {
> >       .hs_codec_id = RT5682S,
> >       .amp_codec_id = MAX98360A,
> >       .dmic_codec_id = DMIC,
> > -     .gpio_spkr_en = EN_SPKR_GPIO_NK,
> > +     .gpio_spkr_en = EN_SPKR_GPIO_NONE,
> >  };
>
> This looks like a good fix for the immediate issue which fixes the
> MAX9360A systems without breaking the RT1019 ones, however as I said in
> the thread about Curtis' revert it's not clear what the ideal thing is
> here.  There's no documentation about the RT1019 that I can find so it's
> not clear to me what exactly the GPIO is controlling, if it's part of
> the RT1019 or something else.  That influences where the most tasteful
> place to control this GPIO is.  Curtis noted that the RT1019 driver
> includes gpiolib headers but that could just as easily be cut'n'paste as
> intentional.  What's the story here?
>
> I do also note that the current code just turns the GPIO on and leaves
> it on which presumably isn't great from a power management point of
> view.

Yes that is correct, this is the quick fix that will alleviate the
gpio contention issue. But I think there is a better solution here.
According to the sheet I have, the pin for the alc1019 is the same as
the max98357a and its a shutdown pin for the amp.

Re: [PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[Mark Brown]

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

On Tue, Feb 01, 2022 at 01:43:38PM -0800, Curtis Malainey wrote:

> Yes that is correct, this is the quick fix that will alleviate the
> gpio contention issue. But I think there is a better solution here.
> According to the sheet I have, the pin for the alc1019 is the same as
> the max98357a and its a shutdown pin for the amp.

OK, that sounds like it should be added to the driver then.

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

Re: [PATCH] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver

[Mark Brown]

On Tue, 1 Feb 2022 02:02:15 +0530, V sujith kumar Reddy wrote:
> Maxim codec driver already enabling/disabling spk_en_gpio in form of sd_mode gpio
> hence remove such gpio access control from machine driver to avoid conflict
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-linus

Thanks!

[1/1] ASoC: amd: acp: Set gpio_spkr_en to None for max speaker amplifer in machine driver
      commit: 7fa5c33d043160eba3be9fb8e21588dff2a467c7

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark