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

Mark Brown
2026-04-14T16:14:03Z

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]