Re: [PATCH v2] ASoC: rt5668: fix teardown order and stale state across card rebind

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] ASoC: rt5668: duplicate IRQ teardown and missing irq_disabled
  updates
--

commit db40c33702a3afc67ec1edbbd986308c05f58e4d
Author: Tristan Madani <tristan@talencesecurity.com>

ASoC: rt5668: fix teardown order and stale state across card rebind

This commit clears headset state and marks the regmap cache dirty on
component removal, and adds a devres action and shutdown check to disable
the IRQ and cancel delayed works across teardown paths.

> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index 29d70802a046a..c8aeaf25a7fd8 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c
[ ... ]
> @@ -2213,14 +2214,26 @@ static int rt5668_set_bias_level(struct snd_soc_component *component,
>  	return 0;
>  }
>  
> +static void rt5668_i2c_disable_irq_and_cancel_work(void *data)
> +{
> +	struct rt5668_priv *rt5668 = data;
> +
> +	if (rt5668->irq_acquired && !rt5668->irq_disabled)
> +		disable_irq(rt5668->i2c->irq);
> +	cancel_delayed_work_sync(&rt5668->jack_detect_work);
> +	cancel_delayed_work_sync(&rt5668->jd_check_work);
> +}
[ ... ]
> @@ -2230,17 +2243,24 @@ static int rt5668_probe(struct snd_soc_component *component)
>  static void rt5668_remove(struct snd_soc_component *component)
>  {
>  	struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
> -	struct i2c_client *i2c = to_i2c_client(component->dev);
>  
>  	if (rt5668->irq_acquired) {
> -		disable_irq(i2c->irq);
> +		disable_irq(rt5668->i2c->irq);
>  		rt5668->irq_disabled = true;
>  	}
>  
>  	cancel_delayed_work_sync(&rt5668->jack_detect_work);
>  	cancel_delayed_work_sync(&rt5668->jd_check_work);
[ ... ]
> @@ -2584,6 +2611,11 @@ static void rt5668_i2c_shutdown(struct i2c_client *client)
>  {
>  	struct rt5668_priv *rt5668 = i2c_get_clientdata(client);
>  
> +	if (rt5668->irq_acquired && !rt5668->irq_disabled)
> +		disable_irq(client->irq);
> +	cancel_delayed_work_sync(&rt5668->jack_detect_work);
> +	cancel_delayed_work_sync(&rt5668->jd_check_work);
> +
>  	rt5668_reset(rt5668->regmap);
>  }

[Severity: Low]
Can this call disable_irq() twice if the device or card is unbound after
rt5668_i2c_shutdown() runs?

rt5668_i2c_shutdown() and rt5668_i2c_disable_irq_and_cancel_work() do not
set rt5668->irq_disabled = true after disabling the IRQ, and
rt5668_remove() does not check !rt5668->irq_disabled.  Setting
rt5668->irq_disabled = true inside
rt5668_i2c_disable_irq_and_cancel_work() and calling that helper from
both rt5668_remove() and rt5668_i2c_shutdown() would avoid duplicating
this teardown sequence and keep irq_disabled consistent.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107161024.1233587-1-tristmd@gmail.com?part=1