Re: [PATCH 2/2] ASoC: Intel: Skylake: Correct the handling of fmt_config flexible array

On 30/06/2022 09:33, Peter Ujfalusi wrote:

The struct nhlt_format's fmt_config is a flexible array, it must not be used as normal array. When moving to the next nhlt_fmt_cfg we need to take into account the data behind the ->config.caps (indicated by ->config.size).

The logic of the code also changed: it is no longer saves the _last_ fmt_cfg for all found rates.

At the same time prepare the code for converting the fmt_config to a u8 flexible array to prevent further abuse.

I should have dropped this sentence, I don't know why it is here. The hinted change is the only way to please sparse, but I'm not sure if I will ever send it as a patch.

Fixes: bc2bd45b1f7f3 ("ASoC: Intel: Skylake: Parse nhlt and register clock device") Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com

---

sound/soc/intel/skylake/skl-nhlt.c | 37 ++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c index 366f7bd9bc02..deb7b820325e 100644 --- a/sound/soc/intel/skylake/skl-nhlt.c +++ b/sound/soc/intel/skylake/skl-nhlt.c @@ -111,11 +111,12 @@ static void skl_get_ssp_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks, if (fmt->fmt_count == 0) return;

• fmt_cfg = (struct nhlt_fmt_cfg *)fmt->fmt_config; for (i = 0; i < fmt->fmt_count; i++) {

• struct nhlt_fmt_cfg *saved_fmt_cfg = fmt_cfg;
  

  bool present = false;

• fmt_cfg = &fmt->fmt_config[i];
  

• wav_fmt = &fmt_cfg->fmt_ext;
  

• wav_fmt = &saved_fmt_cfg->fmt_ext;
  
  

  channels = wav_fmt->fmt.channels; bps = wav_fmt->fmt.bits_per_sample;

@@ -133,12 +134,18 @@ static void skl_get_ssp_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks, * derive the rate. */ for (j = i; j < fmt->fmt_count; j++) {

• 	fmt_cfg = &fmt->fmt_config[j];
  

• 	wav_fmt = &fmt_cfg->fmt_ext;
  

• 	struct nhlt_fmt_cfg *tmp_fmt_cfg = fmt_cfg;
  

• 	wav_fmt = &tmp_fmt_cfg->fmt_ext;
  if ((fs == wav_fmt->fmt.samples_per_sec) &&
  

• 	   (bps == wav_fmt->fmt.bits_per_sample))
  

• 	   (bps == wav_fmt->fmt.bits_per_sample)) {
  	channels = max_t(u16, channels,
  			wav_fmt->fmt.channels);
  

• 		saved_fmt_cfg = tmp_fmt_cfg;
  

• 	}
  

• 	/* Move to the next nhlt_fmt_cfg */
  

• 	tmp_fmt_cfg = (struct nhlt_fmt_cfg *)(tmp_fmt_cfg->config.caps +
  

• 					      tmp_fmt_cfg->config.size);
  

  }

  rate = channels * bps * fs;

@@ -154,8 +161,11 @@ static void skl_get_ssp_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks,

/* Fill rate and parent for sclk/sclkfs */
if (!present) {

• 	struct nhlt_fmt_cfg *first_fmt_cfg;
  

• 	first_fmt_cfg = (struct nhlt_fmt_cfg *)fmt->fmt_config;
  i2s_config_ext = (struct skl_i2s_config_blob_ext *)
  

• 				fmt->fmt_config[0].config.caps;
  

• 				first_fmt_cfg->config.caps;
  
  /* MCLK Divider Source Select */
  if (is_legacy_blob(i2s_config_ext->hdr.sig)) {
  

@@ -169,6 +179,9 @@ static void skl_get_ssp_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks,

	parent = skl_get_parent_clk(clk_src);

• 	/* Move to the next nhlt_fmt_cfg */
  

• 	fmt_cfg = (struct nhlt_fmt_cfg *)(fmt_cfg->config.caps +
  

• 					  fmt_cfg->config.size);
  /*
   * Do not copy the config data if there is no parent
   * clock available for this clock source select
  

@@ -177,9 +190,9 @@ static void skl_get_ssp_clks(struct skl_dev *skl, struct skl_ssp_clk *ssp_clks, continue;

	sclk[id].rate_cfg[rate_index].rate = rate;

• 	sclk[id].rate_cfg[rate_index].config = fmt_cfg;
  

• 	sclk[id].rate_cfg[rate_index].config = saved_fmt_cfg;
  sclkfs[id].rate_cfg[rate_index].rate = rate;
  

• 	sclkfs[id].rate_cfg[rate_index].config = fmt_cfg;
  

• 	sclkfs[id].rate_cfg[rate_index].config = saved_fmt_cfg;
  sclk[id].parent_name = parent->name;
  sclkfs[id].parent_name = parent->name;
  
  

@@ -193,13 +206,13 @@ static void skl_get_mclk(struct skl_dev *skl, struct skl_ssp_clk *mclk, { struct skl_i2s_config_blob_ext *i2s_config_ext; struct skl_i2s_config_blob_legacy *i2s_config;

• struct nhlt_specific_cfg *fmt_cfg;

• struct nhlt_fmt_cfg *fmt_cfg; struct skl_clk_parent_src *parent; u32 clkdiv, div_ratio; u8 clk_src;

• fmt_cfg = &fmt->fmt_config[0].config;
• i2s_config_ext = (struct skl_i2s_config_blob_ext *)fmt_cfg->caps;

• fmt_cfg = (struct nhlt_fmt_cfg *)fmt->fmt_config;

• i2s_config_ext = (struct skl_i2s_config_blob_ext *)fmt_cfg->config.caps;

  /* MCLK Divider Source Select and divider */ if (is_legacy_blob(i2s_config_ext->hdr.sig)) {

@@ -228,7 +241,7 @@ static void skl_get_mclk(struct skl_dev *skl, struct skl_ssp_clk *mclk, return;

mclk[id].rate_cfg[0].rate = parent->rate/div_ratio;

• mclk[id].rate_cfg[0].config = &fmt->fmt_config[0];

• mclk[id].rate_cfg[0].config = fmt_cfg; mclk[id].parent_name = parent->name;

}