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

[AngeloGioacchino Del Regno]

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@xxxxxxxxxxxx>
> Signed-off-by: Zoran Zhan <zoran.zhan@xxxxxxxxxxxx>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>

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@xxxxxxxxxxxxx>

Cheers,
Angelo