Re: [PATCH RFC v2 02/11] ASoC: meson: aiu-encoder-i2s: use gx_iface and gx_stream structures

[Mark Brown]

On Sat, Apr 11, 2026 at 04:57:27PM +0200, Valerio Setti wrote:

> @@ -200,13 +200,17 @@ static int aiu_encoder_i2s_hw_params(struct snd_pcm_substream *substream,

> -	aiu_encoder_i2s_divider_enable(component, true);
> +	ret = gx_stream_set_cont_clocks(ts, iface->fmt);
> +	if (ret)
> +		dev_err(dai->dev, "failed to apply continuous clock setting\n");
> +
> +	aiu_encoder_i2s_divider_enable(component, 1);

If we're checking the error here we should probably return it as well.
Including the error code in the log message is also generally helpful.

> @@ -214,16 +218,20 @@ static int aiu_encoder_i2s_hw_params(struct snd_pcm_substream *substream,
>  static int aiu_encoder_i2s_hw_free(struct snd_pcm_substream *substream,
>  				   struct snd_soc_dai *dai)
>  {
> +	struct gx_stream *ts = snd_soc_dai_get_dma_data(dai, substream);
>  	struct snd_soc_component *component = dai->component;
>  
> -	aiu_encoder_i2s_divider_enable(component, false);
> -
> -	return 0;
> +	/* This is the last substream open and that is going to be closed. */
> +	if (snd_soc_dai_active(dai) <= 1)
> +		aiu_encoder_i2s_divider_enable(component, 0);
> +	return gx_stream_set_cont_clocks(ts, 0);
>  }

Note that we only hw_free() if we preprared, but we enable in
hw_params().

> @@ -284,6 +295,8 @@ static int aiu_encoder_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>  	if (ret)
>  		dev_err(dai->dev, "Failed to set sysclk to %uHz", freq);
>  
> +	aiu->i2s.iface.mclk_rate = freq;
> +
>  	return ret;
>  }

This means we store the new rate even if the set above failed.

Attachment: signature.asc
Description: PGP signature