Re: [PATCH v3 08/10] ASoC: renesas: fsi: add fsi_clk_prepare/unprepare()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced clk_unprepare() for the shared SPU clock in the error path of fsi_clk_prepare().
--

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

ASoC: renesas: fsi: add fsi_clk_prepare/unprepare()

This commit introduces fsi_clk_prepare() and fsi_clk_unprepare() helper
functions for managing FSI clocks. These centralize the prepare and unprepare
logic to facilitate moving clock management into sleepable contexts in future
patches.

> diff --git a/sound/soc/renesas/fsi.c b/sound/soc/renesas/fsi.c
> index 8c46d68069581..b93809b5da15e 100644
> --- a/sound/soc/renesas/fsi.c
> +++ b/sound/soc/renesas/fsi.c

[ ... ]

> @@ -732,6 +732,72 @@ static int fsi_clk_is_valid(struct fsi_priv *fsi)
>  		fsi->clock.rate;
>  }
>  
> +static int fsi_clk_prepare(struct fsi_priv *fsi)
> +{
> +	struct fsi_clk *clock = &fsi->clock;
> +	struct clk *spu = fsi->master->clk_spu;
> +	struct clk *xck = clock->xck;
> +	struct clk *ick = clock->ick;
> +	struct clk *div = clock->div;
> +	int ret;
> +
> +	if (!IS_ERR_OR_NULL(spu) && fsi->master->spu_count == 0) {
> +		ret = clk_prepare(spu);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!IS_ERR_OR_NULL(xck)) {
> +		ret = clk_prepare(xck);
> +		if (ret)
> +			goto err_spu;
> +	}

[ ... ]

> +	return 0;
> +
> +err_ick:
> +	clk_unprepare(ick);
> +err_xck:
> +	clk_unprepare(xck);
> +err_spu:
> +	clk_unprepare(spu);

Will this result in an unbalanced clk_unprepare() for the spu clock?

When fsi->master->spu_count is greater than 0, clk_prepare(spu) is skipped
at the beginning of the function. If a subsequent clock preparation like
clk_prepare(xck) fails, the error path jumps to err_spu and unconditionally
calls clk_unprepare(spu).

Since clk_prepare(spu) was bypassed, does the err_spu label need to mirror
the preparation logic and check if spu_count is 0 to avoid decrementing the
clock framework's internal prepare count prematurely?

> +
> +	return ret;
> +}

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