Re: [PATCH v2 7/7] ASoC: loongson: Add headphone jack detection and DAPM routing

Re: [PATCH v2 7/7] ASoC: loongson: Add headphone jack detection and DAPM routing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The headphone mute GPIO is incorrectly asserted when the headphone widget powers on, muting the audio.
- [High] Double-free / reference count underflow of the headphone detection GPIO descriptor on device unbind or probe failure.
- [Medium] The sound card probe fails completely if the optional headphone detection GPIO is omitted from the device tree.
--

commit 5cf0c58f5dc0c2adba751fce8cc39fa382767c93
Author: Binbin Zhou <zhoubinbin@loongson.cn>
Subject: ASoC: loongson: Add headphone jack detection and DAPM routing

This commit adds headphone jack detection and DAPM routing to the Loongson
audio machine driver. It enables proper switching between headphones and
speakers on the Loongson-2K0300 ATK-DL2K0300B board.

> diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
> index 9200e8476c1fa..ee0cf305187a7 100644
> --- a/sound/soc/loongson/loongson_card.c
> +++ b/sound/soc/loongson/loongson_card.c

[ ... ]

> +/* DAPM widget event: control headphone mute GPIO */
> +static int headphone_widget_event(struct snd_soc_dapm_widget *w,
> +				  struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
> +	struct loongson_card_data *priv = snd_soc_card_get_drvdata(card);
> +
> +	gpiod_set_value_cansleep(priv->gpiod_hp_mute, SND_SOC_DAPM_EVENT_ON(event));

[Severity: High]
Does this logic assert the mute signal exactly when the headphones should be
active? 

The hp-mute GPIO is initialized with GPIOD_OUT_LOW (unmuted) during probe.
When the widget powers on, SND_SOC_DAPM_EVENT_ON evaluates to 1, asserting
the mute and resulting in no sound. When it powers off, it evaluates to 0,
unmuting.

Should this logic be inverted so the headphones remain muted while powered
off and unmuted when active?

> +
> +	return 0;
> +}

[ ... ]

> +static int loongson_asoc_machine_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct snd_soc_card *card = rtd->card;
> +	struct loongson_card_data *ls_priv = snd_soc_card_get_drvdata(card);
> +	int ret = 0;
> +
> +	if (!ls_priv->cfg->add_hp_jack)
> +		return 0;
> +
> +	ret = snd_soc_card_jack_new_pins(card, "Headphones Jack",
> +					 SND_JACK_HEADPHONE,
> +					 &loongson_asoc_hp_jack,
> +					 loongson_asoc_hp_jack_pins,
> +					 ARRAY_SIZE(loongson_asoc_hp_jack_pins));
> +	if (ret) {
> +		dev_err(rtd->dev, "Headphones Jack creation failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	loongson_asoc_hp_jack_gpio.desc = ls_priv->gpiod_hp_det;
> +
> +	ret = snd_soc_jack_add_gpios(&loongson_asoc_hp_jack, 1, &loongson_asoc_hp_jack_gpio);

[Severity: Medium]
Will this cause the sound card probe to fail completely if the optional
headphone detection GPIO is omitted from the device tree?

The loongson,hp-det GPIO is acquired using devm_gpiod_get_optional() during
loongson_asoc_card_probe(), which returns NULL if the property is absent.

If the descriptor is NULL, passing it to snd_soc_jack_add_gpios() without
setting gpiod_dev causes it to return -EINVAL. This propagates back through
the DAI link initialization, breaking the device initialization.

[Severity: High]
Will this result in a double-free of the headphone detection GPIO descriptor
on device unbind or probe failure?

The gpiod_hp_det descriptor is acquired with devm_gpiod_get_optional(), which
registers a devres action to call gpiod_put() when the device is unbound.

Passing this explicitly acquired descriptor directly to
snd_soc_jack_add_gpios() transfers ownership, as it registers its own devres
action that also unconditionally calls gpiod_put(). 

On probe failure or unbind, devres will execute both cleanup actions, causing
a reference count underflow and leading to a use-after-free of the gpio_device.

> +	if (ret)
> +		dev_err(rtd->dev, "Headphone GPIO not added: %d\n", ret);
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780538113.git.zhoubinbin@loongson.cn?part=7