All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Valerio Setti" <vsetti@baylibre.com>
Cc: robh@kernel.org, linux-amlogic@lists.infradead.org,
	neil.armstrong@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v2 4/6] ASoC: meson: aiu: add I2S Capture DAI
Date: Thu, 17 Sep 2026 21:20:18 +0000	[thread overview]
Message-ID: <20260917212019.4ECE61F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260917-audin-v2-4-38de4bc1de34@baylibre.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: meson: permanent clock divider leak in aiu_encoder_i2s_hw_free()
--

commit 7076c7f044003c89d276b88e15e27dd0533c4d92
Author: Valerio Setti <vsetti@baylibre.com>

ASoC: meson: aiu: add I2S Capture DAI

This commit adds a capture stream to the CPU_I2S_ENCODER, enabling it to
handle both playback and capture streams concurrently. It also formats
and buffers the captured data using existing components.

> diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
> index 535fbf7c16394..14d0e20a4bd66 100644
> --- a/sound/soc/meson/aiu.c
> +++ b/sound/soc/meson/aiu.c
> @@ -153,6 +153,13 @@ static struct snd_soc_dai_driver aiu_cpu_dai_drv[] = {
>  			.rates = SNDRV_PCM_RATE_8000_192000,
>  			.formats = AIU_FORMATS,
>  		},
> +		.capture = {
> +			.stream_name = "I2S Encoder Capture",
> +			.channels_min = 2,
> +			.channels_max = 2,
> +			.rates = SNDRV_PCM_RATE_8000_192000,
> +			.formats = SNDRV_PCM_FMTBIT_S16_LE,
> +		},

[Severity: High]
Does adding this capture stream introduce a permanent clock divider leak when
hw_free is explicitly called?

Now that both playback and capture streams can be active simultaneously,
snd_soc_dai_active(dai) can evaluate to 2.

In sound/soc/meson/aiu-encoder-i2s.c:aiu_encoder_i2s_hw_free():

    if (snd_soc_dai_active(dai) <= 1)
        aiu_encoder_i2s_divider_enable(component, 0);

If a user opens both streams so snd_soc_dai_active(dai) evaluates to 2, and
explicitly calls SNDRV_PCM_IOCTL_HW_FREE before closing them,
snd_soc_dai_active(dai) remains 2. This skips disabling the clock divider.

When the streams are subsequently closed, the ALSA core skips invoking hw_free
again because the state was already changed to SNDRV_PCM_STATE_OPEN. This
would leave the clock divider permanently enabled, potentially draining battery
or blocking system suspend.

>  		.ops = &aiu_encoder_i2s_dai_ops,
>  		.symmetric_rate = 1,
>  	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-audin-v2-0-38de4bc1de34@baylibre.com?part=4

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Valerio Setti" <vsetti@baylibre.com>
Cc: robh@kernel.org, linux-amlogic@lists.infradead.org,
	neil.armstrong@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v2 4/6] ASoC: meson: aiu: add I2S Capture DAI
Date: Thu, 17 Sep 2026 21:20:18 +0000	[thread overview]
Message-ID: <20260917212019.4ECE61F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260917-audin-v2-4-38de4bc1de34@baylibre.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: meson: permanent clock divider leak in aiu_encoder_i2s_hw_free()
--

commit 7076c7f044003c89d276b88e15e27dd0533c4d92
Author: Valerio Setti <vsetti@baylibre.com>

ASoC: meson: aiu: add I2S Capture DAI

This commit adds a capture stream to the CPU_I2S_ENCODER, enabling it to
handle both playback and capture streams concurrently. It also formats
and buffers the captured data using existing components.

> diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
> index 535fbf7c16394..14d0e20a4bd66 100644
> --- a/sound/soc/meson/aiu.c
> +++ b/sound/soc/meson/aiu.c
> @@ -153,6 +153,13 @@ static struct snd_soc_dai_driver aiu_cpu_dai_drv[] = {
>  			.rates = SNDRV_PCM_RATE_8000_192000,
>  			.formats = AIU_FORMATS,
>  		},
> +		.capture = {
> +			.stream_name = "I2S Encoder Capture",
> +			.channels_min = 2,
> +			.channels_max = 2,
> +			.rates = SNDRV_PCM_RATE_8000_192000,
> +			.formats = SNDRV_PCM_FMTBIT_S16_LE,
> +		},

[Severity: High]
Does adding this capture stream introduce a permanent clock divider leak when
hw_free is explicitly called?

Now that both playback and capture streams can be active simultaneously,
snd_soc_dai_active(dai) can evaluate to 2.

In sound/soc/meson/aiu-encoder-i2s.c:aiu_encoder_i2s_hw_free():

    if (snd_soc_dai_active(dai) <= 1)
        aiu_encoder_i2s_divider_enable(component, 0);

If a user opens both streams so snd_soc_dai_active(dai) evaluates to 2, and
explicitly calls SNDRV_PCM_IOCTL_HW_FREE before closing them,
snd_soc_dai_active(dai) remains 2. This skips disabling the clock divider.

When the streams are subsequently closed, the ALSA core skips invoking hw_free
again because the state was already changed to SNDRV_PCM_STATE_OPEN. This
would leave the clock divider permanently enabled, potentially draining battery
or blocking system suspend.

>  		.ops = &aiu_encoder_i2s_dai_ops,
>  		.symmetric_rate = 1,
>  	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-audin-v2-0-38de4bc1de34@baylibre.com?part=4

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2026-09-17 21:20 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 21:02 [PATCH v2 0/6] ASoC: meson: gx: add base support for I2S audio input Valerio Setti
2026-09-17 21:02 ` Valerio Setti
2026-09-17 21:02 ` [PATCH v2 1/6] ASoC: dt-bindings: amlogic,gx-audin: add schema Valerio Setti
2026-09-17 21:02   ` Valerio Setti
2026-09-17 21:07   ` sashiko-bot
2026-09-17 21:07     ` sashiko-bot
2026-09-17 21:02 ` [PATCH v2 2/6] ASoC: meson: build gx-formatter as a separate module Valerio Setti
2026-09-17 21:02   ` Valerio Setti
2026-09-17 21:02 ` [PATCH v2 3/6] ASoC: meson: add AUDIN driver Valerio Setti
2026-09-17 21:02   ` Valerio Setti
2026-09-17 21:17   ` sashiko-bot
2026-09-17 21:17     ` sashiko-bot
2026-09-17 21:02 ` [PATCH v2 4/6] ASoC: meson: aiu: add I2S Capture DAI Valerio Setti
2026-09-17 21:02   ` Valerio Setti
2026-09-17 21:20   ` sashiko-bot [this message]
2026-09-17 21:20     ` sashiko-bot
2026-09-18  9:56     ` Valerio Setti
2026-09-18  9:56       ` Valerio Setti
2026-09-17 21:02 ` [PATCH v2 5/6] ASoC: meson: gx-card: add support for audin FIFO Valerio Setti
2026-09-17 21:02   ` Valerio Setti
2026-09-17 21:02 ` [PATCH v2 6/6] arm64: dts: amlogic: gx: add AUDIN node Valerio Setti
2026-09-17 21:02   ` Valerio Setti

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=20260917212019.4ECE61F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vsetti@baylibre.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.