* [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 @ 2022-04-05 8:44 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Cezary Rojewski, alsa-devel, Bard Liao, Jie Yang, Takashi Iwai, Liam Girdwood, Hans de Goede, Mark Brown, Mauro Carvalho Chehab, Péter Ujfalusi, linux-kernel At Huawei Matebook D15 two different GPIOs are used to control the output: - gpio0 controls the speaker output; - gpio1 controls the headphone output. Changing both at the same time cause spurious events that are mis-interpreted as input events, causing troubles on apps. So, a delay is needed before turning on such gpios. With this patch, plugging a headphone causes a jack event to trigger the speaker supply, powering down the speaker and powering up the headphone output. Removing the headphone also triggers the power supply, powering up the speaker and powering down the headphone. Mauro Carvalho Chehab (2): ASoC: Intel: sof_es8336: support a separate gpio to control headphone ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio sound/soc/intel/boards/sof_es8336.c | 68 ++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 10 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 @ 2022-04-05 8:44 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Mauro Carvalho Chehab, Hans de Goede, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel At Huawei Matebook D15 two different GPIOs are used to control the output: - gpio0 controls the speaker output; - gpio1 controls the headphone output. Changing both at the same time cause spurious events that are mis-interpreted as input events, causing troubles on apps. So, a delay is needed before turning on such gpios. With this patch, plugging a headphone causes a jack event to trigger the speaker supply, powering down the speaker and powering up the headphone output. Removing the headphone also triggers the power supply, powering up the speaker and powering down the headphone. Mauro Carvalho Chehab (2): ASoC: Intel: sof_es8336: support a separate gpio to control headphone ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio sound/soc/intel/boards/sof_es8336.c | 68 ++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 10 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone 2022-04-05 8:44 ` Mauro Carvalho Chehab @ 2022-04-05 8:44 ` Mauro Carvalho Chehab -1 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Cezary Rojewski, alsa-devel, Mauro Carvalho Chehab, Bard Liao, Jie Yang, Takashi Iwai, Liam Girdwood, Hans de Goede, Mark Brown, Mauro Carvalho Chehab, Péter Ujfalusi, linux-kernel From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Some devices may use both gpio0 and gpio1 to independently switch the speaker and the headphone. Add support for that. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> --- See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index 5e0529aa4f1d..bcd80870d252 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -30,6 +30,7 @@ #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) #define SOF_ES8336_ENABLE_DMIC BIT(5) #define SOF_ES8336_JD_INVERTED BIT(6) +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) static unsigned long quirk; @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); struct sof_es8336_private { struct device *codec_dev; - struct gpio_desc *gpio_pa; + struct gpio_desc *gpio_pa, *gpio_pa_headphone; struct snd_soc_jack jack; struct list_head hdmi_pcm_list; bool speaker_en; @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { int device; }; -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; + static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { - { "pa-enable-gpios", &pa_enable_gpio, 1 }, + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, { } }; -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, + { } +}; + +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, + { } +}; + +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, { } }; @@ -71,6 +85,8 @@ static void log_quirks(struct device *dev) dev_info(dev, "quirk SSP%ld\n", SOF_ES8336_SSP_CODEC(quirk)); if (quirk & SOF_ES8336_ENABLE_DMIC) dev_info(dev, "quirk DMIC enabled\n"); + if (quirk & SOF_ES8336_HEADPHONE_GPIO) + dev_info(dev, "quirk headphone GPIO enabled\n"); if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) dev_info(dev, "quirk TGL GPIO enabled\n"); if (quirk & SOF_ES8336_JD_INVERTED) @@ -83,13 +99,24 @@ static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w, struct snd_soc_card *card = w->dapm->card; struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card); + if (priv->speaker_en == !SND_SOC_DAPM_EVENT_ON(event)) + return 0; + + priv->speaker_en = !SND_SOC_DAPM_EVENT_ON(event); + if (SND_SOC_DAPM_EVENT_ON(event)) - priv->speaker_en = false; - else - priv->speaker_en = true; + msleep(70); gpiod_set_value_cansleep(priv->gpio_pa, priv->speaker_en); + if (!(quirk & SOF_ES8336_HEADPHONE_GPIO)) + return 0; + + if (SND_SOC_DAPM_EVENT_ON(event)) + msleep(70); + + gpiod_set_value_cansleep(priv->gpio_pa_headphone, priv->speaker_en); + return 0; } @@ -114,7 +141,7 @@ static const struct snd_soc_dapm_route sof_es8316_audio_map[] = { /* * There is no separate speaker output instead the speakers are muxed to - * the HP outputs. The mux is controlled by the "Speaker Power" supply. + * the HP outputs. The mux is controlled Speaker and/or headphone switch. */ {"Speaker", NULL, "HPOL"}, {"Speaker", NULL, "HPOR"}, @@ -233,8 +260,14 @@ static int sof_es8336_quirk_cb(const struct dmi_system_id *id) { quirk = (unsigned long)id->driver_data; - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) + if (quirk & SOF_ES8336_HEADPHONE_GPIO) { + if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) + gpio_mapping = quirk_tgl_acpi_headphone_es8336_gpios; + else + gpio_mapping = quirk_acpi_headphone_es8336_gpios; + } else if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) { gpio_mapping = quirk_acpi_es8336_gpios; + } return 1; } @@ -592,6 +625,13 @@ static int sof_es8336_probe(struct platform_device *pdev) goto err_put_codec; } + priv->gpio_pa_headphone = gpiod_get_optional(codec_dev, "pa-enable-headphone", GPIOD_OUT_LOW); + if (IS_ERR(priv->gpio_pa_headphone)) { + ret = dev_err_probe(dev, PTR_ERR(priv->gpio_pa_headphone), + "could not get pa-enable-headphone GPIO\n"); + goto err_put_codec; + } + INIT_LIST_HEAD(&priv->hdmi_pcm_list); snd_soc_card_set_drvdata(card, priv); -- 2.35.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone @ 2022-04-05 8:44 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Mauro Carvalho Chehab, Hans de Goede, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel, Mauro Carvalho Chehab From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Some devices may use both gpio0 and gpio1 to independently switch the speaker and the headphone. Add support for that. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> --- See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index 5e0529aa4f1d..bcd80870d252 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -30,6 +30,7 @@ #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) #define SOF_ES8336_ENABLE_DMIC BIT(5) #define SOF_ES8336_JD_INVERTED BIT(6) +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) static unsigned long quirk; @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); struct sof_es8336_private { struct device *codec_dev; - struct gpio_desc *gpio_pa; + struct gpio_desc *gpio_pa, *gpio_pa_headphone; struct snd_soc_jack jack; struct list_head hdmi_pcm_list; bool speaker_en; @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { int device; }; -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; + static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { - { "pa-enable-gpios", &pa_enable_gpio, 1 }, + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, { } }; -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, + { } +}; + +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, + { } +}; + +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, { } }; @@ -71,6 +85,8 @@ static void log_quirks(struct device *dev) dev_info(dev, "quirk SSP%ld\n", SOF_ES8336_SSP_CODEC(quirk)); if (quirk & SOF_ES8336_ENABLE_DMIC) dev_info(dev, "quirk DMIC enabled\n"); + if (quirk & SOF_ES8336_HEADPHONE_GPIO) + dev_info(dev, "quirk headphone GPIO enabled\n"); if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) dev_info(dev, "quirk TGL GPIO enabled\n"); if (quirk & SOF_ES8336_JD_INVERTED) @@ -83,13 +99,24 @@ static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w, struct snd_soc_card *card = w->dapm->card; struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card); + if (priv->speaker_en == !SND_SOC_DAPM_EVENT_ON(event)) + return 0; + + priv->speaker_en = !SND_SOC_DAPM_EVENT_ON(event); + if (SND_SOC_DAPM_EVENT_ON(event)) - priv->speaker_en = false; - else - priv->speaker_en = true; + msleep(70); gpiod_set_value_cansleep(priv->gpio_pa, priv->speaker_en); + if (!(quirk & SOF_ES8336_HEADPHONE_GPIO)) + return 0; + + if (SND_SOC_DAPM_EVENT_ON(event)) + msleep(70); + + gpiod_set_value_cansleep(priv->gpio_pa_headphone, priv->speaker_en); + return 0; } @@ -114,7 +141,7 @@ static const struct snd_soc_dapm_route sof_es8316_audio_map[] = { /* * There is no separate speaker output instead the speakers are muxed to - * the HP outputs. The mux is controlled by the "Speaker Power" supply. + * the HP outputs. The mux is controlled Speaker and/or headphone switch. */ {"Speaker", NULL, "HPOL"}, {"Speaker", NULL, "HPOR"}, @@ -233,8 +260,14 @@ static int sof_es8336_quirk_cb(const struct dmi_system_id *id) { quirk = (unsigned long)id->driver_data; - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) + if (quirk & SOF_ES8336_HEADPHONE_GPIO) { + if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) + gpio_mapping = quirk_tgl_acpi_headphone_es8336_gpios; + else + gpio_mapping = quirk_acpi_headphone_es8336_gpios; + } else if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) { gpio_mapping = quirk_acpi_es8336_gpios; + } return 1; } @@ -592,6 +625,13 @@ static int sof_es8336_probe(struct platform_device *pdev) goto err_put_codec; } + priv->gpio_pa_headphone = gpiod_get_optional(codec_dev, "pa-enable-headphone", GPIOD_OUT_LOW); + if (IS_ERR(priv->gpio_pa_headphone)) { + ret = dev_err_probe(dev, PTR_ERR(priv->gpio_pa_headphone), + "could not get pa-enable-headphone GPIO\n"); + goto err_put_codec; + } + INIT_LIST_HEAD(&priv->hdmi_pcm_list); snd_soc_card_set_drvdata(card, priv); -- 2.35.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone 2022-04-05 8:44 ` Mauro Carvalho Chehab @ 2022-04-05 14:57 ` Pierre-Louis Bossart -1 siblings, 0 replies; 16+ messages in thread From: Pierre-Louis Bossart @ 2022-04-05 14:57 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Cezary Rojewski, alsa-devel, Mauro Carvalho Chehab, Bard Liao, Jie Yang, Takashi Iwai, Liam Girdwood, Hans de Goede, Mark Brown, Péter Ujfalusi, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2966 bytes --] On 4/5/22 03:44, Mauro Carvalho Chehab wrote: > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Some devices may use both gpio0 and gpio1 to independently switch > the speaker and the headphone. > > Add support for that. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> > --- > > See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ > > sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- > 1 file changed, 50 insertions(+), 10 deletions(-) > > diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c > index 5e0529aa4f1d..bcd80870d252 100644 > --- a/sound/soc/intel/boards/sof_es8336.c > +++ b/sound/soc/intel/boards/sof_es8336.c > @@ -30,6 +30,7 @@ > #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) > #define SOF_ES8336_ENABLE_DMIC BIT(5) > #define SOF_ES8336_JD_INVERTED BIT(6) > +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) > > static unsigned long quirk; > > @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); > > struct sof_es8336_private { > struct device *codec_dev; > - struct gpio_desc *gpio_pa; > + struct gpio_desc *gpio_pa, *gpio_pa_headphone; > struct snd_soc_jack jack; > struct list_head hdmi_pcm_list; > bool speaker_en; > @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { > int device; > }; > > -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; > +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; > +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; > + > static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { > - { "pa-enable-gpios", &pa_enable_gpio, 1 }, > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > { } > }; > > -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; > static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { > - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > + { } > +}; > + > +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, > + { } > +}; > + > +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, > { } This is starting to be a bit messy, the initial gpios were really intended for speakers and should be clearly referring to speakers now. the TGL quirk really means gpio1 is used instead of gpio0, and I can't figure out what the 'pa' prefix is needed for. Can I suggest the attached cleanup patch be added first? That would make it clearer and more readable IMHO. Compile-tested only since I don't have hardware. Thanks! [-- Attachment #2: 0001-ASoC-Intel-sof_es8336-simplify-speaker-gpio-naming.patch --] [-- Type: text/x-patch, Size: 5176 bytes --] From f3d7144d553349f807fbfe0755aefa9ac04a688c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Date: Tue, 5 Apr 2022 09:47:13 -0500 Subject: [PATCH] ASoC: Intel: sof_es8336: simplify speaker gpio naming In preparation for the support of an additional gpio for headphone control, rename GPIOs to make explicit references to speakers and gpio0 or gpio1. No functionality change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/intel/boards/sof_es8336.c | 42 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index 5e0529aa4f1d..4bd819d75cf2 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -27,7 +27,7 @@ #define SOF_ES8336_SSP_CODEC(quirk) ((quirk) & GENMASK(3, 0)) #define SOF_ES8336_SSP_CODEC_MASK (GENMASK(3, 0)) -#define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) +#define SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK BIT(4) #define SOF_ES8336_ENABLE_DMIC BIT(5) #define SOF_ES8336_JD_INVERTED BIT(6) @@ -39,7 +39,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); struct sof_es8336_private { struct device *codec_dev; - struct gpio_desc *gpio_pa; + struct gpio_desc *gpio_speakers; struct snd_soc_jack jack; struct list_head hdmi_pcm_list; bool speaker_en; @@ -51,19 +51,19 @@ struct sof_hdmi_pcm { int device; }; -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; -static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { - { "pa-enable-gpios", &pa_enable_gpio, 1 }, +static const struct acpi_gpio_params speakers_enable_gpio0 = { 0, 0, true }; +static const struct acpi_gpio_mapping acpi_speakers_enable_gpio0[] = { + { "speakers-enable-gpios", &speakers_enable_gpio0, 1 }, { } }; -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; -static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, +static const struct acpi_gpio_params speakers_enable_gpio1 = { 1, 0, true }; +static const struct acpi_gpio_mapping acpi_speakers_enable_gpio1[] = { + { "speakers-enable-gpios", &speakers_enable_gpio1, 1 }, { } }; -static const struct acpi_gpio_mapping *gpio_mapping = acpi_es8336_gpios; +static const struct acpi_gpio_mapping *gpio_mapping = acpi_speakers_enable_gpio0; static void log_quirks(struct device *dev) { @@ -71,8 +71,8 @@ static void log_quirks(struct device *dev) dev_info(dev, "quirk SSP%ld\n", SOF_ES8336_SSP_CODEC(quirk)); if (quirk & SOF_ES8336_ENABLE_DMIC) dev_info(dev, "quirk DMIC enabled\n"); - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) - dev_info(dev, "quirk TGL GPIO enabled\n"); + if (quirk & SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) + dev_info(dev, "Speakers GPIO1 quirk enabled\n"); if (quirk & SOF_ES8336_JD_INVERTED) dev_info(dev, "quirk JD inverted enabled\n"); } @@ -88,7 +88,7 @@ static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w, else priv->speaker_en = true; - gpiod_set_value_cansleep(priv->gpio_pa, priv->speaker_en); + gpiod_set_value_cansleep(priv->gpio_speakers, priv->speaker_en); return 0; } @@ -233,8 +233,8 @@ static int sof_es8336_quirk_cb(const struct dmi_system_id *id) { quirk = (unsigned long)id->driver_data; - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) - gpio_mapping = quirk_acpi_es8336_gpios; + if (quirk & SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) + gpio_mapping = acpi_speakers_enable_gpio1; return 1; } @@ -257,7 +257,7 @@ static const struct dmi_system_id sof_es8336_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "IP3 tech"), DMI_MATCH(DMI_BOARD_NAME, "WN1"), }, - .driver_data = (void *)(SOF_ES8336_TGL_GPIO_QUIRK) + .driver_data = (void *)(SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) }, {} }; @@ -585,10 +585,10 @@ static int sof_es8336_probe(struct platform_device *pdev) if (ret) dev_warn(codec_dev, "unable to add GPIO mapping table\n"); - priv->gpio_pa = gpiod_get_optional(codec_dev, "pa-enable", GPIOD_OUT_LOW); - if (IS_ERR(priv->gpio_pa)) { - ret = dev_err_probe(dev, PTR_ERR(priv->gpio_pa), - "could not get pa-enable GPIO\n"); + priv->gpio_speakers = gpiod_get_optional(codec_dev, "speakers-enable", GPIOD_OUT_LOW); + if (IS_ERR(priv->gpio_speakers)) { + ret = dev_err_probe(dev, PTR_ERR(priv->gpio_speakers), + "could not get speakers-enable GPIO\n"); goto err_put_codec; } @@ -604,7 +604,7 @@ static int sof_es8336_probe(struct platform_device *pdev) ret = devm_snd_soc_register_card(dev, card); if (ret) { - gpiod_put(priv->gpio_pa); + gpiod_put(priv->gpio_speakers); dev_err(dev, "snd_soc_register_card failed: %d\n", ret); goto err_put_codec; } @@ -622,7 +622,7 @@ static int sof_es8336_remove(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card); - gpiod_put(priv->gpio_pa); + gpiod_put(priv->gpio_speakers); device_remove_software_node(priv->codec_dev); put_device(priv->codec_dev); -- 2.30.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone @ 2022-04-05 14:57 ` Pierre-Louis Bossart 0 siblings, 0 replies; 16+ messages in thread From: Pierre-Louis Bossart @ 2022-04-05 14:57 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Mauro Carvalho Chehab, Hans de Goede, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2966 bytes --] On 4/5/22 03:44, Mauro Carvalho Chehab wrote: > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Some devices may use both gpio0 and gpio1 to independently switch > the speaker and the headphone. > > Add support for that. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> > --- > > See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ > > sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- > 1 file changed, 50 insertions(+), 10 deletions(-) > > diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c > index 5e0529aa4f1d..bcd80870d252 100644 > --- a/sound/soc/intel/boards/sof_es8336.c > +++ b/sound/soc/intel/boards/sof_es8336.c > @@ -30,6 +30,7 @@ > #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) > #define SOF_ES8336_ENABLE_DMIC BIT(5) > #define SOF_ES8336_JD_INVERTED BIT(6) > +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) > > static unsigned long quirk; > > @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); > > struct sof_es8336_private { > struct device *codec_dev; > - struct gpio_desc *gpio_pa; > + struct gpio_desc *gpio_pa, *gpio_pa_headphone; > struct snd_soc_jack jack; > struct list_head hdmi_pcm_list; > bool speaker_en; > @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { > int device; > }; > > -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; > +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; > +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; > + > static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { > - { "pa-enable-gpios", &pa_enable_gpio, 1 }, > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > { } > }; > > -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; > static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { > - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > + { } > +}; > + > +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, > + { } > +}; > + > +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, > { } This is starting to be a bit messy, the initial gpios were really intended for speakers and should be clearly referring to speakers now. the TGL quirk really means gpio1 is used instead of gpio0, and I can't figure out what the 'pa' prefix is needed for. Can I suggest the attached cleanup patch be added first? That would make it clearer and more readable IMHO. Compile-tested only since I don't have hardware. Thanks! [-- Attachment #2: 0001-ASoC-Intel-sof_es8336-simplify-speaker-gpio-naming.patch --] [-- Type: text/x-patch, Size: 5176 bytes --] From f3d7144d553349f807fbfe0755aefa9ac04a688c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Date: Tue, 5 Apr 2022 09:47:13 -0500 Subject: [PATCH] ASoC: Intel: sof_es8336: simplify speaker gpio naming In preparation for the support of an additional gpio for headphone control, rename GPIOs to make explicit references to speakers and gpio0 or gpio1. No functionality change. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> --- sound/soc/intel/boards/sof_es8336.c | 42 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index 5e0529aa4f1d..4bd819d75cf2 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -27,7 +27,7 @@ #define SOF_ES8336_SSP_CODEC(quirk) ((quirk) & GENMASK(3, 0)) #define SOF_ES8336_SSP_CODEC_MASK (GENMASK(3, 0)) -#define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) +#define SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK BIT(4) #define SOF_ES8336_ENABLE_DMIC BIT(5) #define SOF_ES8336_JD_INVERTED BIT(6) @@ -39,7 +39,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); struct sof_es8336_private { struct device *codec_dev; - struct gpio_desc *gpio_pa; + struct gpio_desc *gpio_speakers; struct snd_soc_jack jack; struct list_head hdmi_pcm_list; bool speaker_en; @@ -51,19 +51,19 @@ struct sof_hdmi_pcm { int device; }; -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; -static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { - { "pa-enable-gpios", &pa_enable_gpio, 1 }, +static const struct acpi_gpio_params speakers_enable_gpio0 = { 0, 0, true }; +static const struct acpi_gpio_mapping acpi_speakers_enable_gpio0[] = { + { "speakers-enable-gpios", &speakers_enable_gpio0, 1 }, { } }; -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; -static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, +static const struct acpi_gpio_params speakers_enable_gpio1 = { 1, 0, true }; +static const struct acpi_gpio_mapping acpi_speakers_enable_gpio1[] = { + { "speakers-enable-gpios", &speakers_enable_gpio1, 1 }, { } }; -static const struct acpi_gpio_mapping *gpio_mapping = acpi_es8336_gpios; +static const struct acpi_gpio_mapping *gpio_mapping = acpi_speakers_enable_gpio0; static void log_quirks(struct device *dev) { @@ -71,8 +71,8 @@ static void log_quirks(struct device *dev) dev_info(dev, "quirk SSP%ld\n", SOF_ES8336_SSP_CODEC(quirk)); if (quirk & SOF_ES8336_ENABLE_DMIC) dev_info(dev, "quirk DMIC enabled\n"); - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) - dev_info(dev, "quirk TGL GPIO enabled\n"); + if (quirk & SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) + dev_info(dev, "Speakers GPIO1 quirk enabled\n"); if (quirk & SOF_ES8336_JD_INVERTED) dev_info(dev, "quirk JD inverted enabled\n"); } @@ -88,7 +88,7 @@ static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w, else priv->speaker_en = true; - gpiod_set_value_cansleep(priv->gpio_pa, priv->speaker_en); + gpiod_set_value_cansleep(priv->gpio_speakers, priv->speaker_en); return 0; } @@ -233,8 +233,8 @@ static int sof_es8336_quirk_cb(const struct dmi_system_id *id) { quirk = (unsigned long)id->driver_data; - if (quirk & SOF_ES8336_TGL_GPIO_QUIRK) - gpio_mapping = quirk_acpi_es8336_gpios; + if (quirk & SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) + gpio_mapping = acpi_speakers_enable_gpio1; return 1; } @@ -257,7 +257,7 @@ static const struct dmi_system_id sof_es8336_quirk_table[] = { DMI_MATCH(DMI_SYS_VENDOR, "IP3 tech"), DMI_MATCH(DMI_BOARD_NAME, "WN1"), }, - .driver_data = (void *)(SOF_ES8336_TGL_GPIO_QUIRK) + .driver_data = (void *)(SOF_ES8336_SPEAKERS_EN_GPIO1_QUIRK) }, {} }; @@ -585,10 +585,10 @@ static int sof_es8336_probe(struct platform_device *pdev) if (ret) dev_warn(codec_dev, "unable to add GPIO mapping table\n"); - priv->gpio_pa = gpiod_get_optional(codec_dev, "pa-enable", GPIOD_OUT_LOW); - if (IS_ERR(priv->gpio_pa)) { - ret = dev_err_probe(dev, PTR_ERR(priv->gpio_pa), - "could not get pa-enable GPIO\n"); + priv->gpio_speakers = gpiod_get_optional(codec_dev, "speakers-enable", GPIOD_OUT_LOW); + if (IS_ERR(priv->gpio_speakers)) { + ret = dev_err_probe(dev, PTR_ERR(priv->gpio_speakers), + "could not get speakers-enable GPIO\n"); goto err_put_codec; } @@ -604,7 +604,7 @@ static int sof_es8336_probe(struct platform_device *pdev) ret = devm_snd_soc_register_card(dev, card); if (ret) { - gpiod_put(priv->gpio_pa); + gpiod_put(priv->gpio_speakers); dev_err(dev, "snd_soc_register_card failed: %d\n", ret); goto err_put_codec; } @@ -622,7 +622,7 @@ static int sof_es8336_remove(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card); - gpiod_put(priv->gpio_pa); + gpiod_put(priv->gpio_speakers); device_remove_software_node(priv->codec_dev); put_device(priv->codec_dev); -- 2.30.2 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone 2022-04-05 14:57 ` Pierre-Louis Bossart @ 2022-04-05 15:37 ` Mauro Carvalho Chehab -1 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 15:37 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Cezary Rojewski, alsa-devel, Mauro Carvalho Chehab, Bard Liao, Jie Yang, Takashi Iwai, Liam Girdwood, Hans de Goede, Mark Brown, Péter Ujfalusi, linux-kernel Em Tue, 5 Apr 2022 09:57:30 -0500 Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> escreveu: > On 4/5/22 03:44, Mauro Carvalho Chehab wrote: > > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > > Some devices may use both gpio0 and gpio1 to independently switch > > the speaker and the headphone. > > > > Add support for that. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> > > --- > > > > See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ > > > > sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- > > 1 file changed, 50 insertions(+), 10 deletions(-) > > > > diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c > > index 5e0529aa4f1d..bcd80870d252 100644 > > --- a/sound/soc/intel/boards/sof_es8336.c > > +++ b/sound/soc/intel/boards/sof_es8336.c > > @@ -30,6 +30,7 @@ > > #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) > > #define SOF_ES8336_ENABLE_DMIC BIT(5) > > #define SOF_ES8336_JD_INVERTED BIT(6) > > +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) > > > > static unsigned long quirk; > > > > @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); > > > > struct sof_es8336_private { > > struct device *codec_dev; > > - struct gpio_desc *gpio_pa; > > + struct gpio_desc *gpio_pa, *gpio_pa_headphone; > > struct snd_soc_jack jack; > > struct list_head hdmi_pcm_list; > > bool speaker_en; > > @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { > > int device; > > }; > > > > -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; > > +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; > > +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; > > + > > static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { > > - { "pa-enable-gpios", &pa_enable_gpio, 1 }, > > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > > { } > > }; > > > > -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; > > static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { > > - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, > > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > > + { } > > +}; > > + > > +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { > > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > > + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, > > + { } > > +}; > > + > > +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { > > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > > + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, > > { } > > This is starting to be a bit messy, the initial gpios were really > intended for speakers and should be clearly referring to speakers now. > the TGL quirk really means gpio1 is used instead of gpio0, and I can't > figure out what the 'pa' prefix is needed for. > > Can I suggest the attached cleanup patch be added first? That would make > it clearer and more readable IMHO. Compile-tested only since I don't > have hardware. Makes sense. I'll place it before the first patch, rebase them, test and re-submit. > Thanks! Thanks! Mauro ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone @ 2022-04-05 15:37 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 15:37 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Mauro Carvalho Chehab, Hans de Goede, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel Em Tue, 5 Apr 2022 09:57:30 -0500 Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> escreveu: > On 4/5/22 03:44, Mauro Carvalho Chehab wrote: > > From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > > > Some devices may use both gpio0 and gpio1 to independently switch > > the speaker and the headphone. > > > > Add support for that. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> > > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> > > --- > > > > See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ > > > > sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++++----- > > 1 file changed, 50 insertions(+), 10 deletions(-) > > > > diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c > > index 5e0529aa4f1d..bcd80870d252 100644 > > --- a/sound/soc/intel/boards/sof_es8336.c > > +++ b/sound/soc/intel/boards/sof_es8336.c > > @@ -30,6 +30,7 @@ > > #define SOF_ES8336_TGL_GPIO_QUIRK BIT(4) > > #define SOF_ES8336_ENABLE_DMIC BIT(5) > > #define SOF_ES8336_JD_INVERTED BIT(6) > > +#define SOF_ES8336_HEADPHONE_GPIO BIT(7) > > > > static unsigned long quirk; > > > > @@ -39,7 +40,7 @@ MODULE_PARM_DESC(quirk, "Board-specific quirk override"); > > > > struct sof_es8336_private { > > struct device *codec_dev; > > - struct gpio_desc *gpio_pa; > > + struct gpio_desc *gpio_pa, *gpio_pa_headphone; > > struct snd_soc_jack jack; > > struct list_head hdmi_pcm_list; > > bool speaker_en; > > @@ -51,15 +52,28 @@ struct sof_hdmi_pcm { > > int device; > > }; > > > > -static const struct acpi_gpio_params pa_enable_gpio = { 0, 0, true }; > > +static const struct acpi_gpio_params pa_enable_gpio0 = { 0, 0, true }; > > +static const struct acpi_gpio_params pa_enable_gpio1 = { 1, 0, true }; > > + > > static const struct acpi_gpio_mapping acpi_es8336_gpios[] = { > > - { "pa-enable-gpios", &pa_enable_gpio, 1 }, > > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > > { } > > }; > > > > -static const struct acpi_gpio_params quirk_pa_enable_gpio = { 1, 0, true }; > > static const struct acpi_gpio_mapping quirk_acpi_es8336_gpios[] = { > > - { "pa-enable-gpios", &quirk_pa_enable_gpio, 1 }, > > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > > + { } > > +}; > > + > > +static const struct acpi_gpio_mapping quirk_acpi_headphone_es8336_gpios[] = { > > + { "pa-enable-gpios", &pa_enable_gpio0, 1 }, > > + { "pa-enable-headphone-gpios", &pa_enable_gpio1, 1 }, > > + { } > > +}; > > + > > +static const struct acpi_gpio_mapping quirk_tgl_acpi_headphone_es8336_gpios[] = { > > + { "pa-enable-gpios", &pa_enable_gpio1, 1 }, > > + { "pa-enable-headphone-gpios", &pa_enable_gpio0, 1 }, > > { } > > This is starting to be a bit messy, the initial gpios were really > intended for speakers and should be clearly referring to speakers now. > the TGL quirk really means gpio1 is used instead of gpio0, and I can't > figure out what the 'pa' prefix is needed for. > > Can I suggest the attached cleanup patch be added first? That would make > it clearer and more readable IMHO. Compile-tested only since I don't > have hardware. Makes sense. I'll place it before the first patch, rebase them, test and re-submit. > Thanks! Thanks! Mauro ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio 2022-04-05 8:44 ` Mauro Carvalho Chehab @ 2022-04-05 8:44 ` Mauro Carvalho Chehab -1 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Cezary Rojewski, alsa-devel, Mauro Carvalho Chehab, Bard Liao, Jie Yang, Takashi Iwai, Liam Girdwood, Hans de Goede, Mark Brown, Mauro Carvalho Chehab, Péter Ujfalusi, linux-kernel From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone. Add a quirk for that. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> --- See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ sound/soc/intel/boards/sof_es8336.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index bcd80870d252..79cf9777c4fc 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -292,6 +292,14 @@ static const struct dmi_system_id sof_es8336_quirk_table[] = { }, .driver_data = (void *)(SOF_ES8336_TGL_GPIO_QUIRK) }, + { + .callback = sof_es8336_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"), + DMI_MATCH(DMI_BOARD_NAME, "BOHB-WAX9-PCB-B2"), + }, + .driver_data = (void *)(SOF_ES8336_HEADPHONE_GPIO) + }, {} }; -- 2.35.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio @ 2022-04-05 8:44 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 8:44 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Mauro Carvalho Chehab, Hans de Goede, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel, Mauro Carvalho Chehab From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Based on experimental tests, Huawei Matebook D15 actually uses both gpio0 and gpio1: the first one controls the speaker, while the other one controls the headphone. Add a quirk for that. Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org> --- See [PATCH v2 0/2] at: https://lore.kernel.org/all/cover.1649147890.git.mchehab@kernel.org/ sound/soc/intel/boards/sof_es8336.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index bcd80870d252..79cf9777c4fc 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -292,6 +292,14 @@ static const struct dmi_system_id sof_es8336_quirk_table[] = { }, .driver_data = (void *)(SOF_ES8336_TGL_GPIO_QUIRK) }, + { + .callback = sof_es8336_quirk_cb, + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "HUAWEI"), + DMI_MATCH(DMI_BOARD_NAME, "BOHB-WAX9-PCB-B2"), + }, + .driver_data = (void *)(SOF_ES8336_HEADPHONE_GPIO) + }, {} }; -- 2.35.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 2022-04-05 8:44 ` Mauro Carvalho Chehab @ 2022-04-05 9:00 ` Hans de Goede -1 siblings, 0 replies; 16+ messages in thread From: Hans de Goede @ 2022-04-05 9:00 UTC (permalink / raw) To: Mauro Carvalho Chehab, Pierre-Louis Bossart Cc: Cezary Rojewski, alsa-devel, Péter Ujfalusi, Jie Yang, Takashi Iwai, Liam Girdwood, Mark Brown, Bard Liao, linux-kernel Hi, On 4/5/22 10:44, Mauro Carvalho Chehab wrote: > At Huawei Matebook D15 two different GPIOs are used to control the output: > - gpio0 controls the speaker output; > - gpio1 controls the headphone output. > > Changing both at the same time cause spurious events that are mis-interpreted > as input events, causing troubles on apps. So, a delay is needed before turning > on such gpios. > > With this patch, plugging a headphone causes a jack event to trigger the speaker > supply, powering down the speaker and powering up the headphone output. > Removing the headphone also triggers the power supply, powering up the speaker > and powering down the headphone. > > Mauro Carvalho Chehab (2): > ASoC: Intel: sof_es8336: support a separate gpio to control headphone > ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio There is something weird with the patches here, at least for me in Thunderbird both patches show up as nameless attachments to emails with empty bodies. After saving the attachments the patches look good to me, so: Acked-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 @ 2022-04-05 9:00 ` Hans de Goede 0 siblings, 0 replies; 16+ messages in thread From: Hans de Goede @ 2022-04-05 9:00 UTC (permalink / raw) To: Mauro Carvalho Chehab, Pierre-Louis Bossart Cc: Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel Hi, On 4/5/22 10:44, Mauro Carvalho Chehab wrote: > At Huawei Matebook D15 two different GPIOs are used to control the output: > - gpio0 controls the speaker output; > - gpio1 controls the headphone output. > > Changing both at the same time cause spurious events that are mis-interpreted > as input events, causing troubles on apps. So, a delay is needed before turning > on such gpios. > > With this patch, plugging a headphone causes a jack event to trigger the speaker > supply, powering down the speaker and powering up the headphone output. > Removing the headphone also triggers the power supply, powering up the speaker > and powering down the headphone. > > Mauro Carvalho Chehab (2): > ASoC: Intel: sof_es8336: support a separate gpio to control headphone > ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio There is something weird with the patches here, at least for me in Thunderbird both patches show up as nameless attachments to emails with empty bodies. After saving the attachments the patches look good to me, so: Acked-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 2022-04-05 9:00 ` Hans de Goede @ 2022-04-05 13:48 ` Mauro Carvalho Chehab -1 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 13:48 UTC (permalink / raw) To: Hans de Goede Cc: Cezary Rojewski, alsa-devel, Péter Ujfalusi, Takashi Iwai, Jie Yang, Pierre-Louis Bossart, Liam Girdwood, Mark Brown, Bard Liao, linux-kernel Em Tue, 5 Apr 2022 11:00:44 +0200 Hans de Goede <hdegoede@redhat.com> escreveu: > Hi, > > On 4/5/22 10:44, Mauro Carvalho Chehab wrote: > > At Huawei Matebook D15 two different GPIOs are used to control the output: > > - gpio0 controls the speaker output; > > - gpio1 controls the headphone output. > > > > Changing both at the same time cause spurious events that are mis-interpreted > > as input events, causing troubles on apps. So, a delay is needed before turning > > on such gpios. > > > > With this patch, plugging a headphone causes a jack event to trigger the speaker > > supply, powering down the speaker and powering up the headphone output. > > Removing the headphone also triggers the power supply, powering up the speaker > > and powering down the headphone. > > > > Mauro Carvalho Chehab (2): > > ASoC: Intel: sof_es8336: support a separate gpio to control headphone > > ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio > > There is something weird with the patches here, at least for me in Thunderbird > both patches show up as nameless attachments to emails with empty bodies. Weird... I tested here on Thunderbird (Fedora 35), reading it from the ML and they opened ok here. There *is* an issue on such patches though: they ended using my past e-mail :-) I'll re-send those with your ack using the right SoB/From mchehab@kernel.org. > > After saving the attachments the patches look good to me, so: > > Acked-by: Hans de Goede <hdegoede@redhat.com> Thanks! Regards, Mauro ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 @ 2022-04-05 13:48 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 16+ messages in thread From: Mauro Carvalho Chehab @ 2022-04-05 13:48 UTC (permalink / raw) To: Hans de Goede Cc: Pierre-Louis Bossart, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel Em Tue, 5 Apr 2022 11:00:44 +0200 Hans de Goede <hdegoede@redhat.com> escreveu: > Hi, > > On 4/5/22 10:44, Mauro Carvalho Chehab wrote: > > At Huawei Matebook D15 two different GPIOs are used to control the output: > > - gpio0 controls the speaker output; > > - gpio1 controls the headphone output. > > > > Changing both at the same time cause spurious events that are mis-interpreted > > as input events, causing troubles on apps. So, a delay is needed before turning > > on such gpios. > > > > With this patch, plugging a headphone causes a jack event to trigger the speaker > > supply, powering down the speaker and powering up the headphone output. > > Removing the headphone also triggers the power supply, powering up the speaker > > and powering down the headphone. > > > > Mauro Carvalho Chehab (2): > > ASoC: Intel: sof_es8336: support a separate gpio to control headphone > > ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio > > There is something weird with the patches here, at least for me in Thunderbird > both patches show up as nameless attachments to emails with empty bodies. Weird... I tested here on Thunderbird (Fedora 35), reading it from the ML and they opened ok here. There *is* an issue on such patches though: they ended using my past e-mail :-) I'll re-send those with your ack using the right SoB/From mchehab@kernel.org. > > After saving the attachments the patches look good to me, so: > > Acked-by: Hans de Goede <hdegoede@redhat.com> Thanks! Regards, Mauro ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 2022-04-05 13:48 ` Mauro Carvalho Chehab @ 2022-04-05 13:56 ` Hans de Goede -1 siblings, 0 replies; 16+ messages in thread From: Hans de Goede @ 2022-04-05 13:56 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Cezary Rojewski, alsa-devel, Péter Ujfalusi, Takashi Iwai, Jie Yang, Pierre-Louis Bossart, Liam Girdwood, Mark Brown, Bard Liao, linux-kernel Hi, On 4/5/22 15:48, Mauro Carvalho Chehab wrote: > Em Tue, 5 Apr 2022 11:00:44 +0200 > Hans de Goede <hdegoede@redhat.com> escreveu: > >> Hi, >> >> On 4/5/22 10:44, Mauro Carvalho Chehab wrote: >>> At Huawei Matebook D15 two different GPIOs are used to control the output: >>> - gpio0 controls the speaker output; >>> - gpio1 controls the headphone output. >>> >>> Changing both at the same time cause spurious events that are mis-interpreted >>> as input events, causing troubles on apps. So, a delay is needed before turning >>> on such gpios. >>> >>> With this patch, plugging a headphone causes a jack event to trigger the speaker >>> supply, powering down the speaker and powering up the headphone output. >>> Removing the headphone also triggers the power supply, powering up the speaker >>> and powering down the headphone. >>> >>> Mauro Carvalho Chehab (2): >>> ASoC: Intel: sof_es8336: support a separate gpio to control headphone >>> ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio >> >> There is something weird with the patches here, at least for me in Thunderbird >> both patches show up as nameless attachments to emails with empty bodies. > > Weird... I tested here on Thunderbird (Fedora 35), reading it from the ML > and they opened ok here. Yes I checked lore.kernel.org and they look fine there, so as you said: weird. > There *is* an issue on such patches though: they ended using my past > e-mail :-) > > I'll re-send those with your ack using the right SoB/From > mchehab@kernel.org. And in the resend they look fine in my Thunderbird too. Regards, Hans ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 @ 2022-04-05 13:56 ` Hans de Goede 0 siblings, 0 replies; 16+ messages in thread From: Hans de Goede @ 2022-04-05 13:56 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Pierre-Louis Bossart, Péter Ujfalusi, Bard Liao, Cezary Rojewski, Jaroslav Kysela, Jie Yang, Liam Girdwood, Mark Brown, Takashi Iwai, alsa-devel, linux-kernel Hi, On 4/5/22 15:48, Mauro Carvalho Chehab wrote: > Em Tue, 5 Apr 2022 11:00:44 +0200 > Hans de Goede <hdegoede@redhat.com> escreveu: > >> Hi, >> >> On 4/5/22 10:44, Mauro Carvalho Chehab wrote: >>> At Huawei Matebook D15 two different GPIOs are used to control the output: >>> - gpio0 controls the speaker output; >>> - gpio1 controls the headphone output. >>> >>> Changing both at the same time cause spurious events that are mis-interpreted >>> as input events, causing troubles on apps. So, a delay is needed before turning >>> on such gpios. >>> >>> With this patch, plugging a headphone causes a jack event to trigger the speaker >>> supply, powering down the speaker and powering up the headphone output. >>> Removing the headphone also triggers the power supply, powering up the speaker >>> and powering down the headphone. >>> >>> Mauro Carvalho Chehab (2): >>> ASoC: Intel: sof_es8336: support a separate gpio to control headphone >>> ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio >> >> There is something weird with the patches here, at least for me in Thunderbird >> both patches show up as nameless attachments to emails with empty bodies. > > Weird... I tested here on Thunderbird (Fedora 35), reading it from the ML > and they opened ok here. Yes I checked lore.kernel.org and they look fine there, so as you said: weird. > There *is* an issue on such patches though: they ended using my past > e-mail :-) > > I'll re-send those with your ack using the right SoB/From > mchehab@kernel.org. And in the resend they look fine in my Thunderbird too. Regards, Hans ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2022-04-06 0:51 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-04-05 8:44 [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 Mauro Carvalho Chehab 2022-04-05 8:44 ` Mauro Carvalho Chehab 2022-04-05 8:44 ` [PATCH v2 1/2] ASoC: Intel: sof_es8336: support a separate gpio to control headphone Mauro Carvalho Chehab 2022-04-05 8:44 ` Mauro Carvalho Chehab 2022-04-05 14:57 ` Pierre-Louis Bossart 2022-04-05 14:57 ` Pierre-Louis Bossart 2022-04-05 15:37 ` Mauro Carvalho Chehab 2022-04-05 15:37 ` Mauro Carvalho Chehab 2022-04-05 8:44 ` [PATCH v2 2/2] ASoC: Intel: sof_es8336: Huawei Matebook D15 uses a headphone gpio Mauro Carvalho Chehab 2022-04-05 8:44 ` Mauro Carvalho Chehab 2022-04-05 9:00 ` [PATCH v2 0/2] Make headphone work on Huawei Matebook D15 Hans de Goede 2022-04-05 9:00 ` Hans de Goede 2022-04-05 13:48 ` Mauro Carvalho Chehab 2022-04-05 13:48 ` Mauro Carvalho Chehab 2022-04-05 13:56 ` Hans de Goede 2022-04-05 13:56 ` Hans de Goede
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.