All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: phucduc.bui@gmail.com
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org,
	wsa+renesas@sang-engineering.com
Subject: Re: [PATCH v6 07/11] ASoC: renesas: fsi: Use devm_clk_get_optional() for optional clocks
Date: Tue, 09 Jun 2026 11:56:57 +0000	[thread overview]
Message-ID: <20260609115659.A4C131F00893@smtp.kernel.org> (raw)
In-Reply-To: <20260609113836.45079-8-phucduc.bui@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] fsi_clk_enable() returns -EINVAL when called on an already enabled clock, breaking full-duplex operation.
--

commit e751270870bfce8cae5b9b1175c1bc9e265d605e
Author: bui duc phuc <phucduc.bui@gmail.com>

ASoC: renesas: fsi: Use devm_clk_get_optional() for optional clocks

The xck, ick, and div clocks are optional. Switch from devm_clk_get()
to devm_clk_get_optional() to correctly handle cases where these clocks
are missing.

> diff --git a/sound/soc/renesas/fsi.c b/sound/soc/renesas/fsi.c
> index 1df7dc0143630..e29be2bcf952c 100644
> --- a/sound/soc/renesas/fsi.c
> +++ b/sound/soc/renesas/fsi.c
> @@ -994,7 +994,7 @@ static int fsi_clk_init(struct device *dev,
>  
>  	/* external clock */
>  	if (xck) {
> -		clock->xck = devm_clk_get(dev, is_porta ? "xcka" : "xckb");
> +		clock->xck = devm_clk_get_optional(dev, is_porta ? "xcka" : "xckb");
>  		if (IS_ERR(clock->xck)) {
>  			dev_err(dev, "can't get xck clock\n");
>  			return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing these
clock changes, I noticed a pre-existing issue in fsi_clk_enable().

sound/soc/renesas/fsi.c:fsi_clk_enable() {
	struct fsi_clk *clock = &fsi->clock;
	int ret = -EINVAL;

	if (!fsi_clk_is_valid(fsi))
		return ret;

	if (0 == clock->count) {
		...
		clock->count++;
	}

	return ret;
}

When a second stream starts on the same port (for example, capture starting
while playback is already running), count is 1. The code skips the
(0 == clock->count) block and returns the initial ret value of -EINVAL.

Does this break full-duplex operation by causing fsi_hw_startup() to fail
when starting the second stream?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609113836.45079-1-phucduc.bui@gmail.com?part=7

  reply	other threads:[~2026-06-09 11:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 11:38 [PATCH v6 00/11] ASoC: renesas: fsi: Fix system hang by adding SPU clock phucduc.bui
2026-06-09 11:38 ` [PATCH v6 01/11] ASoC: dt-bindings: renesas,fsi: add support multiple clocks phucduc.bui
2026-06-09 11:50   ` sashiko-bot
2026-06-09 16:04   ` Krzysztof Kozlowski
2026-06-09 11:38 ` [PATCH v6 02/11] ARM: dts: renesas: r8a7740: Add clocks for FSI phucduc.bui
2026-06-09 11:38 ` [PATCH v6 03/11] ASoC: renesas: fsi: Fix trigger stop ordering phucduc.bui
2026-06-09 12:01   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 04/11] ASoC: renesas: fsi: Move fsi_stream_is_working() phucduc.bui
2026-06-09 11:51   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 05/11] ASoC: renesas: fsi: Fix register access from in-flight IRQ after shutdown phucduc.bui
2026-06-09 11:59   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 06/11] ASoC: renesas: fsi: Move fsi_clk_init() phucduc.bui
2026-06-09 11:55   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 07/11] ASoC: renesas: fsi: Use devm_clk_get_optional() for optional clocks phucduc.bui
2026-06-09 11:56   ` sashiko-bot [this message]
2026-06-09 11:38 ` [PATCH v6 08/11] ASoC: renesas: fsi: refactor clock initialization phucduc.bui
2026-06-09 12:06   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 09/11] ASoC: renesas: fsi: Add SPU clock support phucduc.bui
2026-06-09 11:38 ` [PATCH v6 10/11] ASoC: renesas: fsi: add fsi_clk_prepare/unprepare() phucduc.bui
2026-06-09 12:08   ` sashiko-bot
2026-06-09 11:38 ` [PATCH v6 11/11] ASoC: renesas: fsi: Add SPU clock control in hw_startup/shutdown phucduc.bui
2026-06-09 12:10   ` sashiko-bot
2026-06-09 23:24   ` 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=20260609115659.A4C131F00893@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=phucduc.bui@gmail.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wsa+renesas@sang-engineering.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.