[PATCH 0/3] Allow retrieving accessory detection reference on MT8188

[PATCH 0/3] Allow retrieving accessory detection reference on MT8188

[Nícolas F. R. A. Prado]

This series enables the MT8188-MT6359 sound driver to retrieve the
MT6359 ACCDET sound component from a mediatek,accdet DT property, which
allows detecting jack insertion/removal.

Patch 1 describes the new property in the binding. Patch 2 implements
the sound component retrieval in the common MTK soundcard driver. Patch
3 updates the MT8188-MT6359 sound driver to register the audio jack and
initialize the ACCDET driver for detection, if the property is present.

Tested on the Genio 700 EVK board.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
Nícolas F. R. A. Prado (3):
      ASoC: dt-bindings: mediatek,mt8188-mt6359: Add mediatek,accdet
      ASoC: mediatek: common: Handle mediatek,accdet property
      ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

 .../bindings/sound/mediatek,mt8188-mt6359.yaml     |  6 +++
 sound/soc/mediatek/common/mtk-soc-card.h           |  1 +
 sound/soc/mediatek/common/mtk-soundcard-driver.c   | 15 +++++++-
 sound/soc/mediatek/mt8188/mt8188-mt6359.c          | 43 ++++++++++++++++++++++
 4 files changed, 64 insertions(+), 1 deletion(-)
---
base-commit: 7a6b5c348058d075ce0b20710f027a055d33a71d
change-id: 20250214-mt8188-accdet-4deabb85534d

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>

[PATCH 1/3] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add mediatek,accdet

[Nícolas F. R. A. Prado]

Add a mediatek,accdet phandle property to allow getting a reference to
the MT6359 ACCDET block, which is responsible for detecting jack
insertion/removal.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
index 362e729b51b43ec16716aee70ad736420def81f3..41d303e28e4dabd152140531ec0bf7375741b659 100644
--- a/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
+++ b/Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml
@@ -40,6 +40,12 @@ properties:
       hardware that provides additional audio functionalities if present.
       The AFE will link to ADSP when the phandle is provided.
 
+  mediatek,accdet:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle to the MT6359 accessory detection block, which detects audio
+      jack insertion and removal.
+
 patternProperties:
   "^dai-link-[0-9]+$":
     type: object

-- 
2.48.1

[PATCH 2/3] ASoC: mediatek: common: Handle mediatek,accdet property

[Nícolas F. R. A. Prado]

Handle the optional mediatek,accdet property. When present, retrieve the
sound component from its phandle, so the machine sound driver can use it
to register the audio jack and initialize the MT6359 ACCDET for jack
detection.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 sound/soc/mediatek/common/mtk-soc-card.h         |  1 +
 sound/soc/mediatek/common/mtk-soundcard-driver.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/common/mtk-soc-card.h b/sound/soc/mediatek/common/mtk-soc-card.h
index 3f6e24dd22df13ddb3aef1625a77d2e29e46fccf..a1d2794ac1f717997fb47023d5951a9ac4897788 100644
--- a/sound/soc/mediatek/common/mtk-soc-card.h
+++ b/sound/soc/mediatek/common/mtk-soc-card.h
@@ -16,6 +16,7 @@ struct mtk_soc_card_data {
 	const struct mtk_sof_priv *sof_priv;
 	struct list_head sof_dai_link_list;
 	struct mtk_platform_card_data *card_data;
+	struct snd_soc_component *accdet;
 	void *mach_priv;
 };
 
diff --git a/sound/soc/mediatek/common/mtk-soundcard-driver.c b/sound/soc/mediatek/common/mtk-soundcard-driver.c
index f4314dddc46075d554b4e3f3aae8bcb5498f8ab5..4e1c546a994f5e6b7068f70ac8dc2af7e87d141a 100644
--- a/sound/soc/mediatek/common/mtk-soundcard-driver.c
+++ b/sound/soc/mediatek/common/mtk-soundcard-driver.c
@@ -8,6 +8,7 @@
 
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <sound/soc.h>
 
 #include "mtk-dsp-sof-common.h"
@@ -192,7 +193,9 @@ EXPORT_SYMBOL_GPL(mtk_soundcard_common_capture_ops);
 
 int mtk_soundcard_common_probe(struct platform_device *pdev)
 {
-	struct device_node *platform_node, *adsp_node;
+	struct device_node *platform_node, *adsp_node, *accdet_node;
+	struct snd_soc_component *accdet_comp;
+	struct platform_device *accdet_pdev;
 	const struct mtk_soundcard_pdata *pdata;
 	struct mtk_soc_card_data *soc_card_data;
 	struct snd_soc_dai_link *orig_dai_link, *dai_link;
@@ -250,6 +253,16 @@ int mtk_soundcard_common_probe(struct platform_device *pdev)
 
 	soc_card_data->card_data->jacks = jacks;
 
+	accdet_node = of_parse_phandle(pdev->dev.of_node, "mediatek,accdet", 0);
+	if (!IS_ERR(accdet_node)) {
+		accdet_pdev = of_find_device_by_node(accdet_node);
+		if (!IS_ERR(accdet_pdev)) {
+			accdet_comp = snd_soc_lookup_component(&accdet_pdev->dev, NULL);
+			if (!IS_ERR(accdet_comp))
+				soc_card_data->accdet = accdet_comp;
+		}
+	}
+
 	platform_node = of_parse_phandle(pdev->dev.of_node, "mediatek,platform", 0);
 	if (!platform_node)
 		return dev_err_probe(&pdev->dev, -EINVAL,

-- 
2.48.1

[PATCH 3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

[Nícolas F. R. A. Prado]

Enable headset jack detection for MT8188 platforms using the MT6359
ACCDET block for it.

Co-developed-by: Zoran Zhan <zoran.zhan@mediatek.com>
Signed-off-by: Zoran Zhan <zoran.zhan@mediatek.com>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 sound/soc/mediatek/mt8188/mt8188-mt6359.c | 43 +++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
index 2d0d04e0232da07ba43a030b14853322427d55e7..4e19e6cfad1e1f42863b2e2f27131f880c5883bf 100644
--- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
+++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
@@ -17,6 +17,7 @@
 #include "mt8188-afe-common.h"
 #include "../../codecs/nau8825.h"
 #include "../../codecs/mt6359.h"
+#include "../../codecs/mt6359-accdet.h"
 #include "../../codecs/rt5682.h"
 #include "../common/mtk-afe-platform-driver.h"
 #include "../common/mtk-soundcard-driver.h"
@@ -266,6 +267,17 @@ static struct snd_soc_jack_pin nau8825_jack_pins[] = {
 	},
 };
 
+static struct snd_soc_jack_pin mt8188_headset_jack_pins[] = {
+	{
+		.pin    = "Headphone",
+		.mask   = SND_JACK_HEADPHONE,
+	},
+	{
+		.pin    = "Headset Mic",
+		.mask   = SND_JACK_MICROPHONE,
+	},
+};
+
 static const struct snd_kcontrol_new mt8188_dumb_spk_controls[] = {
 	SOC_DAPM_PIN_SWITCH("Ext Spk"),
 };
@@ -500,6 +512,35 @@ static int mt8188_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
+static int mt8188_mt6359_accdet_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_jack *jack = &soc_card_data->card_data->jacks[MT8188_JACK_HEADSET];
+	int ret;
+
+	if (!soc_card_data->accdet)
+		return 0;
+
+	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
+				   SND_JACK_HEADSET | SND_JACK_BTN_0 |
+				   SND_JACK_BTN_1 | SND_JACK_BTN_2 |
+				   SND_JACK_BTN_3,
+				   jack, mt8188_headset_jack_pins,
+				   ARRAY_SIZE(mt8188_headset_jack_pins));
+	if (ret) {
+		dev_err(rtd->dev, "Headset Jack create failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = mt6359_accdet_enable_jack_detect(soc_card_data->accdet, jack);
+	if (ret) {
+		dev_err(rtd->dev, "Headset Jack enable failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_component *cmpnt_codec =
@@ -512,6 +553,8 @@ static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
 	/* mtkaif calibration */
 	mt8188_mt6359_mtkaif_calibration(rtd);
 
+	mt8188_mt6359_accdet_init(rtd);
+
 	return 0;
 }
 

-- 
2.48.1

Re: [PATCH 1/3] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add mediatek,accdet

[Rob Herring (Arm)]

On Fri, 14 Feb 2025 12:14:29 -0300, Nícolas F. R. A. Prado wrote:
> Add a mediatek,accdet phandle property to allow getting a reference to
> the MT6359 ACCDET block, which is responsible for detecting jack
> insertion/removal.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  Documentation/devicetree/bindings/sound/mediatek,mt8188-mt6359.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 1/3] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add mediatek,accdet

[AngeloGioacchino Del Regno]

Il 14/02/25 16:14, Nícolas F. R. A. Prado ha scritto:
> Add a mediatek,accdet phandle property to allow getting a reference to
> the MT6359 ACCDET block, which is responsible for detecting jack
> insertion/removal.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 2/3] ASoC: mediatek: common: Handle mediatek,accdet property

[AngeloGioacchino Del Regno]

Il 14/02/25 16:14, Nícolas F. R. A. Prado ha scritto:
> Handle the optional mediatek,accdet property. When present, retrieve the
> sound component from its phandle, so the machine sound driver can use it
> to register the audio jack and initialize the MT6359 ACCDET for jack
> detection.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

[AngeloGioacchino Del Regno]

Il 14/02/25 16:14, Nícolas F. R. A. Prado ha scritto:
> Enable headset jack detection for MT8188 platforms using the MT6359
> ACCDET block for it.
> 
> Co-developed-by: Zoran Zhan <zoran.zhan@mediatek.com>
> Signed-off-by: Zoran Zhan <zoran.zhan@mediatek.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 43 +++++++++++++++++++++++++++++++
>   1 file changed, 43 insertions(+)
> 
> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> index 2d0d04e0232da07ba43a030b14853322427d55e7..4e19e6cfad1e1f42863b2e2f27131f880c5883bf 100644
> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> @@ -17,6 +17,7 @@
>   #include "mt8188-afe-common.h"
>   #include "../../codecs/nau8825.h"
>   #include "../../codecs/mt6359.h"
> +#include "../../codecs/mt6359-accdet.h"
>   #include "../../codecs/rt5682.h"
>   #include "../common/mtk-afe-platform-driver.h"
>   #include "../common/mtk-soundcard-driver.h"
> @@ -266,6 +267,17 @@ static struct snd_soc_jack_pin nau8825_jack_pins[] = {
>   	},
>   };
>   
> +static struct snd_soc_jack_pin mt8188_headset_jack_pins[] = {

This is the same as nau8825_jack_pins... perhaps we could reuse that?

> +	{
> +		.pin    = "Headphone",
> +		.mask   = SND_JACK_HEADPHONE,
> +	},
> +	{
> +		.pin    = "Headset Mic",
> +		.mask   = SND_JACK_MICROPHONE,
> +	},
> +};
> +
>   static const struct snd_kcontrol_new mt8188_dumb_spk_controls[] = {
>   	SOC_DAPM_PIN_SWITCH("Ext Spk"),
>   };
> @@ -500,6 +512,35 @@ static int mt8188_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
>   	return 0;
>   }
>   
> +static int mt8188_mt6359_accdet_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_jack *jack = &soc_card_data->card_data->jacks[MT8188_JACK_HEADSET];
> +	int ret;
> +
> +	if (!soc_card_data->accdet)
> +		return 0;

I'm not sure... if we have mediatek,accdet (so accdet is present here), but we also
have a NAU8825, or RT5682S, or ES8326 codec, this function will create a headset
jack for MT6359, but then mt8188_headset_codec_init() will do the same again!

I think we should find a way to avoid that situation, as I'm mostly sure that this
will give issues in the long run.

Even if it wouldn't, having two headset jacks exposed, of which one doesn't work
because it doesn't exist on the physical board... would be confusing for the user.

I guess that the best option here would be:
  - Let the `for_each_card_prelinks()` loop finish
  - Check if any of the external codecs are providing 3.5mm jack
    - External codec providing jack means that the detection should be performed
      by the external codec, as the jack should not be physically routed to the
      MediaTek ACCDET related PMIC pins (right?)
    - No external codec means that the accessory detection can only be performed
      by the MediaTek ACCDET IP
  - If external codec manages 3.5mm jack: do nothing
  - If no external codec managing 3.5mm jack: check if accdet provided in DT and
    initialize it

....unless I'm wrong - and if I am, please explain why (and also add explanation
to the commit description).

Cheers,
Angelo

> +
> +	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
> +				   SND_JACK_HEADSET | SND_JACK_BTN_0 |
> +				   SND_JACK_BTN_1 | SND_JACK_BTN_2 |
> +				   SND_JACK_BTN_3,
> +				   jack, mt8188_headset_jack_pins,
> +				   ARRAY_SIZE(mt8188_headset_jack_pins));
> +	if (ret) {
> +		dev_err(rtd->dev, "Headset Jack create failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = mt6359_accdet_enable_jack_detect(soc_card_data->accdet, jack);
> +	if (ret) {
> +		dev_err(rtd->dev, "Headset Jack enable failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
>   {
>   	struct snd_soc_component *cmpnt_codec =
> @@ -512,6 +553,8 @@ static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
>   	/* mtkaif calibration */
>   	mt8188_mt6359_mtkaif_calibration(rtd);
>   
> +	mt8188_mt6359_accdet_init(rtd);
> +
>   	return 0;
>   }
>   
> 

Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

[Mark Brown]

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

On Fri, Feb 14, 2025 at 12:14:31PM -0300, Nícolas F. R. A. Prado wrote:
> Enable headset jack detection for MT8188 platforms using the MT6359
> ACCDET block for it.

This breaks an arm64 defconfig build:

ERROR: modpost: "mt6359_accdet_enable_jack_detect" [sound/soc/mediatek/mt8188/mt8188-mt6359.ko] undefined!

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

Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

[Nícolas F. R. A. Prado]

On Tue, Mar 04, 2025 at 04:39:33PM +0100, AngeloGioacchino Del Regno wrote:
> Il 14/02/25 16:14, Nícolas F. R. A. Prado ha scritto:
> > Enable headset jack detection for MT8188 platforms using the MT6359
> > ACCDET block for it.
> > 
> > Co-developed-by: Zoran Zhan <zoran.zhan@mediatek.com>
> > Signed-off-by: Zoran Zhan <zoran.zhan@mediatek.com>
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> >   sound/soc/mediatek/mt8188/mt8188-mt6359.c | 43 +++++++++++++++++++++++++++++++
> >   1 file changed, 43 insertions(+)
> > 
> > diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > index 2d0d04e0232da07ba43a030b14853322427d55e7..4e19e6cfad1e1f42863b2e2f27131f880c5883bf 100644
> > --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
> > @@ -17,6 +17,7 @@
> >   #include "mt8188-afe-common.h"
> >   #include "../../codecs/nau8825.h"
> >   #include "../../codecs/mt6359.h"
> > +#include "../../codecs/mt6359-accdet.h"
> >   #include "../../codecs/rt5682.h"
> >   #include "../common/mtk-afe-platform-driver.h"
> >   #include "../common/mtk-soundcard-driver.h"
> > @@ -266,6 +267,17 @@ static struct snd_soc_jack_pin nau8825_jack_pins[] = {
> >   	},
> >   };
> > +static struct snd_soc_jack_pin mt8188_headset_jack_pins[] = {
> 
> This is the same as nau8825_jack_pins... perhaps we could reuse that?

The difference is the pin name: "Headphone Jack", which results in a difference
in the widget's name. I remember you wanted to not have the "Jack" in the
widget's name to standardize it among other MTK platforms, and we could do that
here by dropping it from the nau8825_jack_pins. But since that name is also used
on the audio routes in a few DTs, those would need updating as well:

arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku3.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku2.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts

The name is not used in upstream alsa-ucm-conf though, so it should be safe to
update.

In any case, I didn't want to expand the scope of this series to that unrelated
change, so that's why I introduced this separate struct, thinking we could
commonize as a later step.

> 
> > +	{
> > +		.pin    = "Headphone",
> > +		.mask   = SND_JACK_HEADPHONE,
> > +	},
> > +	{
> > +		.pin    = "Headset Mic",
> > +		.mask   = SND_JACK_MICROPHONE,
> > +	},
> > +};
> > +
> >   static const struct snd_kcontrol_new mt8188_dumb_spk_controls[] = {
> >   	SOC_DAPM_PIN_SWITCH("Ext Spk"),
> >   };
> > @@ -500,6 +512,35 @@ static int mt8188_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
> >   	return 0;
> >   }
> > +static int mt8188_mt6359_accdet_init(struct snd_soc_pcm_runtime *rtd)
> > +{
> > +	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(rtd->card);
> > +	struct snd_soc_jack *jack = &soc_card_data->card_data->jacks[MT8188_JACK_HEADSET];
> > +	int ret;
> > +
> > +	if (!soc_card_data->accdet)
> > +		return 0;
> 
> I'm not sure... if we have mediatek,accdet (so accdet is present here), but we also
> have a NAU8825, or RT5682S, or ES8326 codec, this function will create a headset
> jack for MT6359, but then mt8188_headset_codec_init() will do the same again!
> 
> I think we should find a way to avoid that situation, as I'm mostly sure that this
> will give issues in the long run.
> 
> Even if it wouldn't, having two headset jacks exposed, of which one doesn't work
> because it doesn't exist on the physical board... would be confusing for the user.

IMO that situation happening would mean the DT is wrong. If the board has the
headset jack pins wired to a headset codec, and that's responsible for jack
detection, then those pins won't be wired to the ACCDET block, and therefore the
board DT shouldn't have the mediatek,accdet property.

I think you're imagining having the mediatek,accdet property always present, and
the logic you propose would allow specifying a headset codec to essentially
override it (making the mediatek,accdet property useless in this case). But
there can also be boards with no jacks at all, and in those cases we don't want
jack widgets exposed to userspace, so the mediatek,accdet property should be
omitted for those boards, and only present on boards where the ACCDET is
actually wired to.

> 
> I guess that the best option here would be:
>  - Let the `for_each_card_prelinks()` loop finish
>  - Check if any of the external codecs are providing 3.5mm jack
>    - External codec providing jack means that the detection should be performed
>      by the external codec, as the jack should not be physically routed to the
>      MediaTek ACCDET related PMIC pins (right?)
>    - No external codec means that the accessory detection can only be performed
>      by the MediaTek ACCDET IP
>  - If external codec manages 3.5mm jack: do nothing
>  - If no external codec managing 3.5mm jack: check if accdet provided in DT and
>    initialize it
> 
> ....unless I'm wrong - and if I am, please explain why (and also add explanation
> to the commit description).

I can add to the commit message:

Only boards that have jack detection managed by the MT6359 ACCDET block will
have the mediatek,accdet property present.

Thanks,
Nícolas

> 
> Cheers,
> Angelo
> 
> > +
> > +	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
> > +				   SND_JACK_HEADSET | SND_JACK_BTN_0 |
> > +				   SND_JACK_BTN_1 | SND_JACK_BTN_2 |
> > +				   SND_JACK_BTN_3,
> > +				   jack, mt8188_headset_jack_pins,
> > +				   ARRAY_SIZE(mt8188_headset_jack_pins));
> > +	if (ret) {
> > +		dev_err(rtd->dev, "Headset Jack create failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = mt6359_accdet_enable_jack_detect(soc_card_data->accdet, jack);
> > +	if (ret) {
> > +		dev_err(rtd->dev, "Headset Jack enable failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
> >   {
> >   	struct snd_soc_component *cmpnt_codec =
> > @@ -512,6 +553,8 @@ static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
> >   	/* mtkaif calibration */
> >   	mt8188_mt6359_mtkaif_calibration(rtd);
> > +	mt8188_mt6359_accdet_init(rtd);
> > +
> >   	return 0;
> >   }
> > 

Re: [PATCH 3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support

[AngeloGioacchino Del Regno]

Il 04/03/25 21:16, Nícolas F. R. A. Prado ha scritto:
> On Tue, Mar 04, 2025 at 04:39:33PM +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/02/25 16:14, Nícolas F. R. A. Prado ha scritto:
>>> Enable headset jack detection for MT8188 platforms using the MT6359
>>> ACCDET block for it.
>>>
>>> Co-developed-by: Zoran Zhan <zoran.zhan@mediatek.com>
>>> Signed-off-by: Zoran Zhan <zoran.zhan@mediatek.com>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>> ---
>>>    sound/soc/mediatek/mt8188/mt8188-mt6359.c | 43 +++++++++++++++++++++++++++++++
>>>    1 file changed, 43 insertions(+)
>>>
>>> diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>>> index 2d0d04e0232da07ba43a030b14853322427d55e7..4e19e6cfad1e1f42863b2e2f27131f880c5883bf 100644
>>> --- a/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>>> +++ b/sound/soc/mediatek/mt8188/mt8188-mt6359.c
>>> @@ -17,6 +17,7 @@
>>>    #include "mt8188-afe-common.h"
>>>    #include "../../codecs/nau8825.h"
>>>    #include "../../codecs/mt6359.h"
>>> +#include "../../codecs/mt6359-accdet.h"
>>>    #include "../../codecs/rt5682.h"
>>>    #include "../common/mtk-afe-platform-driver.h"
>>>    #include "../common/mtk-soundcard-driver.h"
>>> @@ -266,6 +267,17 @@ static struct snd_soc_jack_pin nau8825_jack_pins[] = {
>>>    	},
>>>    };
>>> +static struct snd_soc_jack_pin mt8188_headset_jack_pins[] = {
>>
>> This is the same as nau8825_jack_pins... perhaps we could reuse that?
> 
> The difference is the pin name: "Headphone Jack", which results in a difference
> in the widget's name. I remember you wanted to not have the "Jack" in the
> widget's name to standardize it among other MTK platforms, and we could do that
> here by dropping it from the nau8825_jack_pins. But since that name is also used
> on the audio routes in a few DTs, those would need updating as well:
> 

No it's that "Jack" should be automatically appended if not present in the
jack name :-)

> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku0.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku1.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku3.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku2.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku5.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku7.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku4.dts
> arch/arm64/boot/dts/mediatek/mt8188-geralt-ciri-sku6.dts
> 
> The name is not used in upstream alsa-ucm-conf though, so it should be safe to
> update.
> 
> In any case, I didn't want to expand the scope of this series to that unrelated
> change, so that's why I introduced this separate struct, thinking we could
> commonize as a later step.
> 

That's fine for me, but are we adding unnecessary duplication?

>>
>>> +	{
>>> +		.pin    = "Headphone",
>>> +		.mask   = SND_JACK_HEADPHONE,
>>> +	},
>>> +	{
>>> +		.pin    = "Headset Mic",
>>> +		.mask   = SND_JACK_MICROPHONE,
>>> +	},
>>> +};
>>> +
>>>    static const struct snd_kcontrol_new mt8188_dumb_spk_controls[] = {
>>>    	SOC_DAPM_PIN_SWITCH("Ext Spk"),
>>>    };
>>> @@ -500,6 +512,35 @@ static int mt8188_mt6359_mtkaif_calibration(struct snd_soc_pcm_runtime *rtd)
>>>    	return 0;
>>>    }
>>> +static int mt8188_mt6359_accdet_init(struct snd_soc_pcm_runtime *rtd)
>>> +{
>>> +	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(rtd->card);
>>> +	struct snd_soc_jack *jack = &soc_card_data->card_data->jacks[MT8188_JACK_HEADSET];
>>> +	int ret;
>>> +
>>> +	if (!soc_card_data->accdet)
>>> +		return 0;
>>
>> I'm not sure... if we have mediatek,accdet (so accdet is present here), but we also
>> have a NAU8825, or RT5682S, or ES8326 codec, this function will create a headset
>> jack for MT6359, but then mt8188_headset_codec_init() will do the same again!
>>
>> I think we should find a way to avoid that situation, as I'm mostly sure that this
>> will give issues in the long run.
>>
>> Even if it wouldn't, having two headset jacks exposed, of which one doesn't work
>> because it doesn't exist on the physical board... would be confusing for the user.
> 
> IMO that situation happening would mean the DT is wrong. If the board has the
> headset jack pins wired to a headset codec, and that's responsible for jack
> detection, then those pins won't be wired to the ACCDET block, and therefore the
> board DT shouldn't have the mediatek,accdet property.
> 

That might make sense.

> I think you're imagining having the mediatek,accdet property always present, and
> the logic you propose would allow specifying a headset codec to essentially
> override it (making the mediatek,accdet property useless in this case). But
> there can also be boards with no jacks at all, and in those cases we don't want
> jack widgets exposed to userspace, so the mediatek,accdet property should be
> omitted for those boards, and only present on boards where the ACCDET is
> actually wired to.
> 

I am imagining that because ACCDET is something that is always present on the
MT6359 PMIC, as it is always integrated, physically, into the silicon.

Having that always present in the PMIC DT doesn't mean that the actual sound card
node would have it, so... okay, I get the point, looks good, but it does only if

.... (scroll down)

>>
>> I guess that the best option here would be:
>>   - Let the `for_each_card_prelinks()` loop finish
>>   - Check if any of the external codecs are providing 3.5mm jack
>>     - External codec providing jack means that the detection should be performed
>>       by the external codec, as the jack should not be physically routed to the
>>       MediaTek ACCDET related PMIC pins (right?)
>>     - No external codec means that the accessory detection can only be performed
>>       by the MediaTek ACCDET IP
>>   - If external codec manages 3.5mm jack: do nothing
>>   - If no external codec managing 3.5mm jack: check if accdet provided in DT and
>>     initialize it
>>
>> ....unless I'm wrong - and if I am, please explain why (and also add explanation
>> to the commit description).
> 
> I can add to the commit message:
> 
> Only boards that have jack detection managed by the MT6359 ACCDET block will
> have the mediatek,accdet property present.

....only if you add that, because this really makes things clear - and had you
added that to the commit description in this version, I wouldn't have had this
question ;-)

So yes, please add it.

On another note, I don't understand what's the problem with using nau8825_jack_pins
instead of adding a mt8188_headset_jack_pins before you cleanup.

Really, it's just about doing:

/*
  * nau8825_jack_pins lists the exact same pin names and types as the
  * MT6359 PMIC: reuse to avoid useless duplication
  */
ret = snd_soc_card_jack_new_pins(rtd->card, .... etc etc

Cheers!

> 
> Thanks,
> Nícolas
> 
>>
>> Cheers,
>> Angelo
>>
>>> +
>>> +	ret = snd_soc_card_jack_new_pins(rtd->card, "Headset Jack",
>>> +				   SND_JACK_HEADSET | SND_JACK_BTN_0 |
>>> +				   SND_JACK_BTN_1 | SND_JACK_BTN_2 |
>>> +				   SND_JACK_BTN_3,
>>> +				   jack, mt8188_headset_jack_pins,
>>> +				   ARRAY_SIZE(mt8188_headset_jack_pins));
>>> +	if (ret) {
>>> +		dev_err(rtd->dev, "Headset Jack create failed: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = mt6359_accdet_enable_jack_detect(soc_card_data->accdet, jack);
>>> +	if (ret) {
>>> +		dev_err(rtd->dev, "Headset Jack enable failed: %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
>>>    {
>>>    	struct snd_soc_component *cmpnt_codec =
>>> @@ -512,6 +553,8 @@ static int mt8188_mt6359_init(struct snd_soc_pcm_runtime *rtd)
>>>    	/* mtkaif calibration */
>>>    	mt8188_mt6359_mtkaif_calibration(rtd);
>>> +	mt8188_mt6359_accdet_init(rtd);
>>> +
>>>    	return 0;
>>>    }
>>>

Re: [PATCH 0/3] Allow retrieving accessory detection reference on MT8188

[Mark Brown]

On Fri, 14 Feb 2025 12:14:28 -0300, Nícolas F. R. A. Prado wrote:
> This series enables the MT8188-MT6359 sound driver to retrieve the
> MT6359 ACCDET sound component from a mediatek,accdet DT property, which
> allows detecting jack insertion/removal.
> 
> Patch 1 describes the new property in the binding. Patch 2 implements
> the sound component retrieval in the common MTK soundcard driver. Patch
> 3 updates the MT8188-MT6359 sound driver to register the audio jack and
> initialize the ACCDET driver for detection, if the property is present.
> 
> [...]

Applied to

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

Thanks!

[1/3] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add mediatek,accdet
      commit: 3fec903f2cb18805b1ef22a0e310498020c1f15e
[2/3] ASoC: mediatek: common: Handle mediatek,accdet property
      commit: cf536e2622e2b0a60c99e799995b6e9acf539c17
[3/3] ASoC: mediatek: mt8188-mt6359: Add headset jack detect support
      (no commit info)

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