| 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.
On Wed, Mar 05, 2025 at 05:23:29PM +0000, Mark Brown wrote: > 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. Actually, given this function will only be called on platforms where the accessory detection is present, I agree with Angelo that we should return an error (and print the error message), so the user can easily figure out that the accdet initialization failed due to the missing config. Thanks, Nícolas
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; }