| Message ID | 20250304-mt8188-accdet-v2-3-27f496c4aede@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Allow retrieving accessory detection reference on MT8188 | expand |
Il 04/03/25 22:35, Nícolas F. R. A. Prado ha scritto: > Enable headset jack detection for MT8188 platforms that use the MT6359 > ACCDET block for it, indicated by the mediatek,accdet property in DT. > For those platforms, register a jack and initialize the ACCDET block to > report jack events through 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> We chatted about the issue that I raised before with mt8188_headset_jack_pins[], it's fine to have it for now, as whatever we do with that would still duplicate things and there's no way around that for now. The sound card driver should get a bigger cleanup anyway, and it's not really possible even because otherwise we'd break UCM2 confs, so it is what it is. > --- > sound/soc/mediatek/Kconfig | 1 + > sound/soc/mediatek/mt8188/mt8188-mt6359.c | 43 +++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > > diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig > index 3033e2d3fe16840631a465dd967da77f7a6ff43c..64155ea7965a1cc50593fef442ce90761836f328 100644 > --- a/sound/soc/mediatek/Kconfig > +++ b/sound/soc/mediatek/Kconfig > @@ -237,6 +237,7 @@ config SND_SOC_MT8188_MT6359 > select SND_SOC_NAU8825 > select SND_SOC_RT5682S > select SND_SOC_ES8326 > + select SND_SOC_MT6359_ACCDET The only remaining complaint that I have is that this driver shouldn't really add a select SND_SOC_MT6359_ACCDET. Instead, the mt6359-accdet.h header should be modified to stub-ify the accdet driver functions in the case in which it's not built in, or not available as a module anyway. This is because the accdet driver is not *mandatory* for this driver to work, and being it an optional peripheral, adding a dependency doesn't really make a lot of sense as it'd only force in bloat when not wanted. So... Get to that mt6359-accdet.h header and: #if IS_ENABLED(CONFIG_SND_SOC_MT6359_ACCDET) function signatures #else static .... { return -EINVAL; (or whatever else) } #endif That said, for the future v3 of this commit, after removing the select SND_SOC_MT6359_ACCDET (and granted that the proposed header fix is in place), you can add my Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cheers, Angelo
On Wed, Mar 05, 2025 at 06:18:41PM +0100, AngeloGioacchino Del Regno wrote: > Get to that mt6359-accdet.h header and: > #if IS_ENABLED(CONFIG_SND_SOC_MT6359_ACCDET) > function signatures > #else > static .... { return -EINVAL; (or whatever else) } > #endif > It should probably be return 0 for the registration/unregistration functions so the caller doesn't fail if it checks the return code when the accessory detection is configured out.
diff --git a/sound/soc/mediatek/Kconfig b/sound/soc/mediatek/Kconfig index 3033e2d3fe16840631a465dd967da77f7a6ff43c..64155ea7965a1cc50593fef442ce90761836f328 100644 --- a/sound/soc/mediatek/Kconfig +++ b/sound/soc/mediatek/Kconfig @@ -237,6 +237,7 @@ config SND_SOC_MT8188_MT6359 select SND_SOC_NAU8825 select SND_SOC_RT5682S select SND_SOC_ES8326 + select SND_SOC_MT6359_ACCDET help This adds support for ASoC machine driver for MediaTek MT8188 boards with the MT6359 and other I2S audio codecs. diff --git a/sound/soc/mediatek/mt8188/mt8188-mt6359.c b/sound/soc/mediatek/mt8188/mt8188-mt6359.c index 420b1427b71dc1424a52f7ab6140c14659036733..20dc9470ba76b2a750e79a5ae3dafabd7c597f40 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" @@ -271,6 +272,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"), }; @@ -506,6 +518,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 = @@ -518,6 +559,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; }