Re: [PATCH 3/3] ASoC: renesas: fsi: Fix hang by enabling SPU clock

[Mark Brown]

On Fri, Apr 03, 2026 at 06:26:55PM +0700, phucduc.bui@xxxxxxxxx wrote:

> @@ -1554,6 +1555,11 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
>  			   struct snd_soc_dai *dai)
>  {
>  	struct fsi_priv *fsi = fsi_get_priv(substream);
> +	int ret;
> +
> +	ret = clk_prepare_enable(fsi->master->clk_spu);
> +	if (ret)
> +		return ret;
>  

Should we also be managing the clock during system suspend, or if the
power consumption doesn't really matter should we just keep it enabled
all the time and not worry about starting and stopping it?

> +	/* SPU clock is required for FSI register access */
> +	master->clk_spu = devm_clk_get(&pdev->dev, "spu");
> +	if (IS_ERR(master->clk_spu)) {
> +		dev_err(&pdev->dev, "Failed to get spu clock\n");
> +		return PTR_ERR(master->clk_spu);
> +	}
> +

This is going to unconditionally require a clock called "spu" on all
devices using this driver, not just the one SoC you mentioned as
requiring it.  Presumably this worked at least somewhere (possibly the
clock is always on, or they're just lucky that something else enables
it) and this will cause regressions for those platforms?

This should either (ideally) be conditional, or use _optional.

Attachment: signature.asc
Description: PGP signature