Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Binbin Zhou" <zhoubinbin@loongson.cn>
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org
Subject: Re: [PATCH v2 7/7] ASoC: loongson: Add headphone jack detection and DAPM routing
Date: Thu, 04 Jun 2026 02:27:04 +0000	[thread overview]
Message-ID: <20260604022704.6E8AD1F00893@smtp.kernel.org> (raw)
In-Reply-To: <2736a44ee11bcc0bbf282c69e99cb05b12469207.1780538113.git.zhoubinbin@loongson.cn>

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

  parent reply	other threads:[~2026-06-04  2:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04  2:11 [PATCH v2 0/7] ASoC: Add Loongson-2K0300 I2S controller and sound card support Binbin Zhou
2026-06-04  2:11 ` [PATCH v2 1/7] ASoC: dt-bindings: loongson,ls2k1000-i2s: Document Loongson-2K0300 compatible Binbin Zhou
2026-06-04  2:11 ` [PATCH v2 2/7] ASoC: loongson: Add Loongson-2K0300 I2S controller support Binbin Zhou
2026-06-04  2:24   ` sashiko-bot
2026-06-04  2:11 ` [PATCH v2 3/7] ASoC: dt-bindings: loongson,ls-audio-card: Use common sound card Binbin Zhou
2026-06-04  2:11 ` [PATCH v2 4/7] ASoC: dt-bindings: loongson,ls-audio-card: Add ctcisz forever pi compatible Binbin Zhou
2026-06-04  2:21   ` sashiko-bot
     [not found] ` <2736a44ee11bcc0bbf282c69e99cb05b12469207.1780538113.git.zhoubinbin@loongson.cn>
2026-06-04  2:27   ` sashiko-bot [this message]
2026-06-04 14:56   ` [PATCH v2 7/7] ASoC: loongson: Add headphone jack detection and DAPM routing Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260604022704.6E8AD1F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=zhoubinbin@loongson.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox