Re: [PATCH v6 3/5] ASoC: es9356-sdca: Add ES9356 SDCA driver

[Mark Brown <broonie@kernel.org>]

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

On Thu, Apr 09, 2026 at 10:12:53AM +0800, Zhang Yi wrote:
> This is the codec driver for es9356-sdca.

> +static int es9356_sdca_headset_detect(struct es9356_sdw_priv *es9356)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(es9356->regmap,
> +			SDW_SDCA_CTL(FUNC_NUM_UAJ, ES9356_SDCA_ENT_GE35, ES9356_SDCA_CTL_DETECTED_MODE, 0), &reg);
> +
> +	if (ret < 0)
> +		goto io_error;
> +
> +	switch (reg) {

> +	default:
> +		es9356->jack_type = -1;
> +		break;

This is going to wind up as an all bits set, meaning that whenever we
detect a button we'll end up reporting everything possible as detected.
It's probably best to either ensure we can't report anything if there's
no jack detected or figure out what would cause the hardware to produce
a bogus value and report that as a fallback (eg, go with headphone since
we know there's *something* in the jack and it's safer than headset).

> +	if (status->sdca_cascade && !es9356->disable_irq)
> +		mod_delayed_work(system_power_efficient_wq,
> +			&es9356->jack_detect_work, msecs_to_jiffies(280));
> +
> +	mutex_unlock(&es9356->jack_lock);

> +static int es9356_sdca_dev_suspend(struct device *dev)
> +{
> +	struct es9356_sdw_priv *es9356 = dev_get_drvdata(dev);
> +
> +	es9356->disable_irq = true;
> +	cancel_delayed_work_sync(&es9356->jack_detect_work);
> +	cancel_delayed_work_sync(&es9356->button_detect_work);

This doesn't hold jack_lock so we might still race with an interrupt
callback, the above code needs to take the lock to ensure a callback
doesn't reschedule the work after we thought we cancelled it.

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