All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
@ 2021-04-16  7:11 ` Thierry Reding
  0 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-16  7:11 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Takashi Iwai, Jaroslav Kysela, Kuninori Morimoto, Jon Hunter,
	alsa-devel, linux-tegra

From: Thierry Reding <treding@nvidia.com>

The DAI counting code doesn't propagate errors when the number of
maximum links is exceeded, which causes subsequent initialization code
to continue to run and that eventually leads to memory corruption with
the code trying to access memory that is out of bounds.

Fix this by propgating errors when the maximum number of links is
reached, which ensures that the driver fails to load and prevents the
memory corruption.

Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
 sound/soc/generic/simple-card.c      | 17 ++++++++++-------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 0582fe296471..2401212281c2 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -539,8 +539,8 @@ static int graph_for_each_link(struct asoc_simple_priv *priv,
 	return ret;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li);
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li);
 
 int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 {
@@ -552,7 +552,10 @@ int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 	card->dev = dev;
 
 	memset(&li, 0, sizeof(li));
-	graph_get_dais_count(priv, &li);
+	ret = graph_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;
 
@@ -657,8 +660,8 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li)
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li)
 {
 	/*
 	 * link_num :	number of links.
@@ -706,9 +709,9 @@ static void graph_get_dais_count(struct asoc_simple_priv *priv,
 	 *	=> 4 DAIs  = 2xCPU + 2xCodec
 	 *	=> 1 ccnf  = 1xdummy-Codec
 	 */
-	graph_for_each_link(priv, li,
-			    graph_count_noml,
-			    graph_count_dpcm);
+	return graph_for_each_link(priv, li,
+				   graph_count_noml,
+				   graph_count_dpcm);
 }
 
 int audio_graph_card_probe(struct snd_soc_card *card)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bf5ddf1ea65f..1d1c4309b582 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -526,8 +526,8 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void simple_get_dais_count(struct asoc_simple_priv *priv,
-				  struct link_info *li)
+static int simple_get_dais_count(struct asoc_simple_priv *priv,
+				 struct link_info *li)
 {
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *top = dev->of_node;
@@ -584,12 +584,12 @@ static void simple_get_dais_count(struct asoc_simple_priv *priv,
 		li->num[0].platforms	= 1;
 
 		li->link = 1;
-		return;
+		return 0;
 	}
 
-	simple_for_each_link(priv, li,
-			     simple_count_noml,
-			     simple_count_dpcm);
+	return simple_for_each_link(priv, li,
+				    simple_count_noml,
+				    simple_count_dpcm);
 }
 
 static int simple_soc_probe(struct snd_soc_card *card)
@@ -628,7 +628,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
 	card->probe		= simple_soc_probe;
 
 	memset(&li, 0, sizeof(li));
-	simple_get_dais_count(priv, &li);
+	ret = simple_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
@ 2021-04-16  7:11 ` Thierry Reding
  0 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-16  7:11 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: alsa-devel, Kuninori Morimoto, Takashi Iwai, Jon Hunter, linux-tegra

From: Thierry Reding <treding@nvidia.com>

The DAI counting code doesn't propagate errors when the number of
maximum links is exceeded, which causes subsequent initialization code
to continue to run and that eventually leads to memory corruption with
the code trying to access memory that is out of bounds.

Fix this by propgating errors when the maximum number of links is
reached, which ensures that the driver fails to load and prevents the
memory corruption.

Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
 sound/soc/generic/simple-card.c      | 17 ++++++++++-------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 0582fe296471..2401212281c2 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -539,8 +539,8 @@ static int graph_for_each_link(struct asoc_simple_priv *priv,
 	return ret;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li);
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li);
 
 int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 {
@@ -552,7 +552,10 @@ int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 	card->dev = dev;
 
 	memset(&li, 0, sizeof(li));
-	graph_get_dais_count(priv, &li);
+	ret = graph_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;
 
@@ -657,8 +660,8 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void graph_get_dais_count(struct asoc_simple_priv *priv,
-				 struct link_info *li)
+static int graph_get_dais_count(struct asoc_simple_priv *priv,
+				struct link_info *li)
 {
 	/*
 	 * link_num :	number of links.
@@ -706,9 +709,9 @@ static void graph_get_dais_count(struct asoc_simple_priv *priv,
 	 *	=> 4 DAIs  = 2xCPU + 2xCodec
 	 *	=> 1 ccnf  = 1xdummy-Codec
 	 */
-	graph_for_each_link(priv, li,
-			    graph_count_noml,
-			    graph_count_dpcm);
+	return graph_for_each_link(priv, li,
+				   graph_count_noml,
+				   graph_count_dpcm);
 }
 
 int audio_graph_card_probe(struct snd_soc_card *card)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bf5ddf1ea65f..1d1c4309b582 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -526,8 +526,8 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
 	return 0;
 }
 
-static void simple_get_dais_count(struct asoc_simple_priv *priv,
-				  struct link_info *li)
+static int simple_get_dais_count(struct asoc_simple_priv *priv,
+				 struct link_info *li)
 {
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *top = dev->of_node;
@@ -584,12 +584,12 @@ static void simple_get_dais_count(struct asoc_simple_priv *priv,
 		li->num[0].platforms	= 1;
 
 		li->link = 1;
-		return;
+		return 0;
 	}
 
-	simple_for_each_link(priv, li,
-			     simple_count_noml,
-			     simple_count_dpcm);
+	return simple_for_each_link(priv, li,
+				    simple_count_noml,
+				    simple_count_dpcm);
 }
 
 static int simple_soc_probe(struct snd_soc_card *card)
@@ -628,7 +628,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
 	card->probe		= simple_soc_probe;
 
 	memset(&li, 0, sizeof(li));
-	simple_get_dais_count(priv, &li);
+	ret = simple_get_dais_count(priv, &li);
+	if (ret < 0)
+		return ret;
+
 	if (!li.link)
 		return -EINVAL;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-16  7:11 ` Thierry Reding
@ 2021-04-16  7:11   ` Thierry Reding
  -1 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-16  7:11 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: Takashi Iwai, Jaroslav Kysela, Kuninori Morimoto, Jon Hunter,
	alsa-devel, linux-tegra

From: Thierry Reding <treding@nvidia.com>

On Tegra186 and later, the number of links can go up to 72, so bump the
maximum number of links to the next power of two (128).

Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/sound/simple_card_utils.h    | 4 +++-
 sound/soc/generic/audio-graph-card.c | 4 ++--
 sound/soc/generic/simple-card.c      | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index fac3b832d982..e318a2d4ac44 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -115,10 +115,12 @@ struct asoc_simple_priv {
 		     ((codec) = simple_props_to_dai_codec(props, i));	\
 	     (i)++)
 
+#define SNDRV_MAX_LINKS 128
+
 struct link_info {
 	int link; /* number of link */
 	int cpu;  /* turn for CPU / Codec */
-	struct prop_nums num[SNDRV_MINOR_DEVICES];
+	struct prop_nums num[SNDRV_MAX_LINKS];
 };
 
 int asoc_simple_parse_daifmt(struct device *dev,
diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 2401212281c2..d6d90285967c 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -616,7 +616,7 @@ static int graph_count_noml(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		dev_err(dev, "too many links\n");
 		return -EINVAL;
 	}
@@ -639,7 +639,7 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		dev_err(dev, "too many links\n");
 		return -EINVAL;
 	}
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 1d1c4309b582..59b41019c65f 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -484,7 +484,7 @@ static int simple_count_noml(struct asoc_simple_priv *priv,
 			     struct device_node *codec,
 			     struct link_info *li, bool is_top)
 {
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		struct device *dev = simple_priv_to_dev(priv);
 
 		dev_err(dev, "too many links\n");
@@ -505,7 +505,7 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
 			     struct device_node *codec,
 			     struct link_info *li, bool is_top)
 {
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		struct device *dev = simple_priv_to_dev(priv);
 
 		dev_err(dev, "too many links\n");
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-16  7:11   ` Thierry Reding
  0 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-16  7:11 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood
  Cc: alsa-devel, Kuninori Morimoto, Takashi Iwai, Jon Hunter, linux-tegra

From: Thierry Reding <treding@nvidia.com>

On Tegra186 and later, the number of links can go up to 72, so bump the
maximum number of links to the next power of two (128).

Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/sound/simple_card_utils.h    | 4 +++-
 sound/soc/generic/audio-graph-card.c | 4 ++--
 sound/soc/generic/simple-card.c      | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index fac3b832d982..e318a2d4ac44 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -115,10 +115,12 @@ struct asoc_simple_priv {
 		     ((codec) = simple_props_to_dai_codec(props, i));	\
 	     (i)++)
 
+#define SNDRV_MAX_LINKS 128
+
 struct link_info {
 	int link; /* number of link */
 	int cpu;  /* turn for CPU / Codec */
-	struct prop_nums num[SNDRV_MINOR_DEVICES];
+	struct prop_nums num[SNDRV_MAX_LINKS];
 };
 
 int asoc_simple_parse_daifmt(struct device *dev,
diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 2401212281c2..d6d90285967c 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -616,7 +616,7 @@ static int graph_count_noml(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		dev_err(dev, "too many links\n");
 		return -EINVAL;
 	}
@@ -639,7 +639,7 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		dev_err(dev, "too many links\n");
 		return -EINVAL;
 	}
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 1d1c4309b582..59b41019c65f 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -484,7 +484,7 @@ static int simple_count_noml(struct asoc_simple_priv *priv,
 			     struct device_node *codec,
 			     struct link_info *li, bool is_top)
 {
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		struct device *dev = simple_priv_to_dev(priv);
 
 		dev_err(dev, "too many links\n");
@@ -505,7 +505,7 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
 			     struct device_node *codec,
 			     struct link_info *li, bool is_top)
 {
-	if (li->link >= SNDRV_MINOR_DEVICES) {
+	if (li->link >= SNDRV_MAX_LINKS) {
 		struct device *dev = simple_priv_to_dev(priv);
 
 		dev_err(dev, "too many links\n");
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
  2021-04-16  7:11 ` Thierry Reding
@ 2021-04-16 14:49   ` Jon Hunter
  -1 siblings, 0 replies; 20+ messages in thread
From: Jon Hunter @ 2021-04-16 14:49 UTC (permalink / raw)
  To: Thierry Reding, Mark Brown, Liam Girdwood
  Cc: Takashi Iwai, Jaroslav Kysela, Kuninori Morimoto, alsa-devel,
	linux-tegra


On 16/04/2021 08:11, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is

s/propgating/propagating

> reached, which ensures that the driver fails to load and prevents the
> memory corruption.
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
>  sound/soc/generic/simple-card.c      | 17 ++++++++++-------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 0582fe296471..2401212281c2 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -539,8 +539,8 @@ static int graph_for_each_link(struct asoc_simple_priv *priv,
>  	return ret;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li);
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li);
>  
>  int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  {
> @@ -552,7 +552,10 @@ int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  	card->dev = dev;
>  
>  	memset(&li, 0, sizeof(li));
> -	graph_get_dais_count(priv, &li);
> +	ret = graph_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> @@ -657,8 +660,8 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li)
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li)
>  {
>  	/*
>  	 * link_num :	number of links.
> @@ -706,9 +709,9 @@ static void graph_get_dais_count(struct asoc_simple_priv *priv,
>  	 *	=> 4 DAIs  = 2xCPU + 2xCodec
>  	 *	=> 1 ccnf  = 1xdummy-Codec
>  	 */
> -	graph_for_each_link(priv, li,
> -			    graph_count_noml,
> -			    graph_count_dpcm);
> +	return graph_for_each_link(priv, li,
> +				   graph_count_noml,
> +				   graph_count_dpcm);
>  }
>  
>  int audio_graph_card_probe(struct snd_soc_card *card)
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index bf5ddf1ea65f..1d1c4309b582 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -526,8 +526,8 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void simple_get_dais_count(struct asoc_simple_priv *priv,
> -				  struct link_info *li)
> +static int simple_get_dais_count(struct asoc_simple_priv *priv,
> +				 struct link_info *li)
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  	struct device_node *top = dev->of_node;
> @@ -584,12 +584,12 @@ static void simple_get_dais_count(struct asoc_simple_priv *priv,
>  		li->num[0].platforms	= 1;
>  
>  		li->link = 1;
> -		return;
> +		return 0;
>  	}
>  
> -	simple_for_each_link(priv, li,
> -			     simple_count_noml,
> -			     simple_count_dpcm);
> +	return simple_for_each_link(priv, li,
> +				    simple_count_noml,
> +				    simple_count_dpcm);
>  }
>  
>  static int simple_soc_probe(struct snd_soc_card *card)
> @@ -628,7 +628,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
>  	card->probe		= simple_soc_probe;
>  
>  	memset(&li, 0, sizeof(li));
> -	simple_get_dais_count(priv, &li);
> +	ret = simple_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
@ 2021-04-16 14:49   ` Jon Hunter
  0 siblings, 0 replies; 20+ messages in thread
From: Jon Hunter @ 2021-04-16 14:49 UTC (permalink / raw)
  To: Thierry Reding, Mark Brown, Liam Girdwood
  Cc: linux-tegra, alsa-devel, Takashi Iwai, Kuninori Morimoto


On 16/04/2021 08:11, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is

s/propgating/propagating

> reached, which ensures that the driver fails to load and prevents the
> memory corruption.
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  sound/soc/generic/audio-graph-card.c | 19 +++++++++++--------
>  sound/soc/generic/simple-card.c      | 17 ++++++++++-------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 0582fe296471..2401212281c2 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -539,8 +539,8 @@ static int graph_for_each_link(struct asoc_simple_priv *priv,
>  	return ret;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li);
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li);
>  
>  int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  {
> @@ -552,7 +552,10 @@ int audio_graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
>  	card->dev = dev;
>  
>  	memset(&li, 0, sizeof(li));
> -	graph_get_dais_count(priv, &li);
> +	ret = graph_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> @@ -657,8 +660,8 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void graph_get_dais_count(struct asoc_simple_priv *priv,
> -				 struct link_info *li)
> +static int graph_get_dais_count(struct asoc_simple_priv *priv,
> +				struct link_info *li)
>  {
>  	/*
>  	 * link_num :	number of links.
> @@ -706,9 +709,9 @@ static void graph_get_dais_count(struct asoc_simple_priv *priv,
>  	 *	=> 4 DAIs  = 2xCPU + 2xCodec
>  	 *	=> 1 ccnf  = 1xdummy-Codec
>  	 */
> -	graph_for_each_link(priv, li,
> -			    graph_count_noml,
> -			    graph_count_dpcm);
> +	return graph_for_each_link(priv, li,
> +				   graph_count_noml,
> +				   graph_count_dpcm);
>  }
>  
>  int audio_graph_card_probe(struct snd_soc_card *card)
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index bf5ddf1ea65f..1d1c4309b582 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -526,8 +526,8 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
>  	return 0;
>  }
>  
> -static void simple_get_dais_count(struct asoc_simple_priv *priv,
> -				  struct link_info *li)
> +static int simple_get_dais_count(struct asoc_simple_priv *priv,
> +				 struct link_info *li)
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  	struct device_node *top = dev->of_node;
> @@ -584,12 +584,12 @@ static void simple_get_dais_count(struct asoc_simple_priv *priv,
>  		li->num[0].platforms	= 1;
>  
>  		li->link = 1;
> -		return;
> +		return 0;
>  	}
>  
> -	simple_for_each_link(priv, li,
> -			     simple_count_noml,
> -			     simple_count_dpcm);
> +	return simple_for_each_link(priv, li,
> +				    simple_count_noml,
> +				    simple_count_dpcm);
>  }
>  
>  static int simple_soc_probe(struct snd_soc_card *card)
> @@ -628,7 +628,10 @@ static int asoc_simple_probe(struct platform_device *pdev)
>  	card->probe		= simple_soc_probe;
>  
>  	memset(&li, 0, sizeof(li));
> -	simple_get_dais_count(priv, &li);
> +	ret = simple_get_dais_count(priv, &li);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (!li.link)
>  		return -EINVAL;
>  
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-16  7:11   ` Thierry Reding
@ 2021-04-16 14:50     ` Jon Hunter
  -1 siblings, 0 replies; 20+ messages in thread
From: Jon Hunter @ 2021-04-16 14:50 UTC (permalink / raw)
  To: Thierry Reding, Mark Brown, Liam Girdwood
  Cc: Takashi Iwai, Jaroslav Kysela, Kuninori Morimoto, alsa-devel,
	linux-tegra


On 16/04/2021 08:11, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> On Tegra186 and later, the number of links can go up to 72, so bump the
> maximum number of links to the next power of two (128).
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/sound/simple_card_utils.h    | 4 +++-
>  sound/soc/generic/audio-graph-card.c | 4 ++--
>  sound/soc/generic/simple-card.c      | 4 ++--
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
> index fac3b832d982..e318a2d4ac44 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -115,10 +115,12 @@ struct asoc_simple_priv {
>  		     ((codec) = simple_props_to_dai_codec(props, i));	\
>  	     (i)++)
>  
> +#define SNDRV_MAX_LINKS 128
> +
>  struct link_info {
>  	int link; /* number of link */
>  	int cpu;  /* turn for CPU / Codec */
> -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> +	struct prop_nums num[SNDRV_MAX_LINKS];
>  };
>  
>  int asoc_simple_parse_daifmt(struct device *dev,
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 2401212281c2..d6d90285967c 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -616,7 +616,7 @@ static int graph_count_noml(struct asoc_simple_priv *priv,
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		dev_err(dev, "too many links\n");
>  		return -EINVAL;
>  	}
> @@ -639,7 +639,7 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		dev_err(dev, "too many links\n");
>  		return -EINVAL;
>  	}
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 1d1c4309b582..59b41019c65f 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -484,7 +484,7 @@ static int simple_count_noml(struct asoc_simple_priv *priv,
>  			     struct device_node *codec,
>  			     struct link_info *li, bool is_top)
>  {
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		struct device *dev = simple_priv_to_dev(priv);
>  
>  		dev_err(dev, "too many links\n");
> @@ -505,7 +505,7 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
>  			     struct device_node *codec,
>  			     struct link_info *li, bool is_top)
>  {
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		struct device *dev = simple_priv_to_dev(priv);
>  
>  		dev_err(dev, "too many links\n");
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon
-- 
nvpublic

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-16 14:50     ` Jon Hunter
  0 siblings, 0 replies; 20+ messages in thread
From: Jon Hunter @ 2021-04-16 14:50 UTC (permalink / raw)
  To: Thierry Reding, Mark Brown, Liam Girdwood
  Cc: linux-tegra, alsa-devel, Takashi Iwai, Kuninori Morimoto


On 16/04/2021 08:11, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> On Tegra186 and later, the number of links can go up to 72, so bump the
> maximum number of links to the next power of two (128).
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  include/sound/simple_card_utils.h    | 4 +++-
>  sound/soc/generic/audio-graph-card.c | 4 ++--
>  sound/soc/generic/simple-card.c      | 4 ++--
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
> index fac3b832d982..e318a2d4ac44 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -115,10 +115,12 @@ struct asoc_simple_priv {
>  		     ((codec) = simple_props_to_dai_codec(props, i));	\
>  	     (i)++)
>  
> +#define SNDRV_MAX_LINKS 128
> +
>  struct link_info {
>  	int link; /* number of link */
>  	int cpu;  /* turn for CPU / Codec */
> -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> +	struct prop_nums num[SNDRV_MAX_LINKS];
>  };
>  
>  int asoc_simple_parse_daifmt(struct device *dev,
> diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
> index 2401212281c2..d6d90285967c 100644
> --- a/sound/soc/generic/audio-graph-card.c
> +++ b/sound/soc/generic/audio-graph-card.c
> @@ -616,7 +616,7 @@ static int graph_count_noml(struct asoc_simple_priv *priv,
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		dev_err(dev, "too many links\n");
>  		return -EINVAL;
>  	}
> @@ -639,7 +639,7 @@ static int graph_count_dpcm(struct asoc_simple_priv *priv,
>  {
>  	struct device *dev = simple_priv_to_dev(priv);
>  
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		dev_err(dev, "too many links\n");
>  		return -EINVAL;
>  	}
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 1d1c4309b582..59b41019c65f 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -484,7 +484,7 @@ static int simple_count_noml(struct asoc_simple_priv *priv,
>  			     struct device_node *codec,
>  			     struct link_info *li, bool is_top)
>  {
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		struct device *dev = simple_priv_to_dev(priv);
>  
>  		dev_err(dev, "too many links\n");
> @@ -505,7 +505,7 @@ static int simple_count_dpcm(struct asoc_simple_priv *priv,
>  			     struct device_node *codec,
>  			     struct link_info *li, bool is_top)
>  {
> -	if (li->link >= SNDRV_MINOR_DEVICES) {
> +	if (li->link >= SNDRV_MAX_LINKS) {
>  		struct device *dev = simple_priv_to_dev(priv);
>  
>  		dev_err(dev, "too many links\n");
> 

Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Tested-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon
-- 
nvpublic

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
  2021-04-16 14:49   ` Jon Hunter
@ 2021-04-16 15:55     ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-16 15:55 UTC (permalink / raw)
  To: Jon Hunter
  Cc: Thierry Reding, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Kuninori Morimoto, alsa-devel, linux-tegra

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

On Fri, Apr 16, 2021 at 03:49:34PM +0100, Jon Hunter wrote:
> 
> On 16/04/2021 08:11, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The DAI counting code doesn't propagate errors when the number of
> > maximum links is exceeded, which causes subsequent initialization code
> > to continue to run and that eventually leads to memory corruption with
> > the code trying to access memory that is out of bounds.
> > 
> > Fix this by propgating errors when the maximum number of links is
> 
> s/propgating/propagating
> 
> > reached, which ensures that the driver fails to load and prevents the
> > memory corruption.
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
@ 2021-04-16 15:55     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-16 15:55 UTC (permalink / raw)
  To: Jon Hunter
  Cc: alsa-devel, Kuninori Morimoto, Takashi Iwai, Liam Girdwood,
	Thierry Reding, linux-tegra

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

On Fri, Apr 16, 2021 at 03:49:34PM +0100, Jon Hunter wrote:
> 
> On 16/04/2021 08:11, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The DAI counting code doesn't propagate errors when the number of
> > maximum links is exceeded, which causes subsequent initialization code
> > to continue to run and that eventually leads to memory corruption with
> > the code trying to access memory that is out of bounds.
> > 
> > Fix this by propgating errors when the maximum number of links is
> 
> s/propgating/propagating
> 
> > reached, which ensures that the driver fails to load and prevents the
> > memory corruption.
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
  2021-04-16  7:11 ` Thierry Reding
@ 2021-04-16 16:01   ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-16 16:01 UTC (permalink / raw)
  To: Liam Girdwood, Thierry Reding
  Cc: Mark Brown, Kuninori Morimoto, alsa-devel, Takashi Iwai,
	linux-tegra, Jon Hunter

On Fri, 16 Apr 2021 09:11:46 +0200, Thierry Reding wrote:
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is
> reached, which ensures that the driver fails to load and prevents the
> memory corruption.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: simple-card-utils: Propagate errors on too many links
      commit: 0f687d826736a5b4eee03170382fe54d413b912a
[2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
      commit: 343e55e71877415a23372388b3e0c59a9bba42f6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links
@ 2021-04-16 16:01   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-16 16:01 UTC (permalink / raw)
  To: Liam Girdwood, Thierry Reding
  Cc: alsa-devel, Kuninori Morimoto, Takashi Iwai, Jon Hunter,
	Mark Brown, linux-tegra

On Fri, 16 Apr 2021 09:11:46 +0200, Thierry Reding wrote:
> The DAI counting code doesn't propagate errors when the number of
> maximum links is exceeded, which causes subsequent initialization code
> to continue to run and that eventually leads to memory corruption with
> the code trying to access memory that is out of bounds.
> 
> Fix this by propgating errors when the maximum number of links is
> reached, which ensures that the driver fails to load and prevents the
> memory corruption.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: simple-card-utils: Propagate errors on too many links
      commit: 0f687d826736a5b4eee03170382fe54d413b912a
[2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
      commit: 343e55e71877415a23372388b3e0c59a9bba42f6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-16  7:11   ` Thierry Reding
@ 2021-04-19  1:42     ` Kuninori Morimoto
  -1 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2021-04-19  1:42 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mark Brown, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Jon Hunter, alsa-devel, linux-tegra


Hi Thierry

> From: Thierry Reding <treding@nvidia.com>
> 
> On Tegra186 and later, the number of links can go up to 72, so bump the
> maximum number of links to the next power of two (128).
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
(snip)
> +#define SNDRV_MAX_LINKS 128
> +
>  struct link_info {
>  	int link; /* number of link */
>  	int cpu;  /* turn for CPU / Codec */
> -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> +	struct prop_nums num[SNDRV_MAX_LINKS];
>  };


How many numbers do you really need ?

Because simple-card needs many parameters,
thus I will get below warning.
# It is not yet happen on upstream kernel, but will be

	warning: the frame size of 2280 bytes is larger than 2048 bytes [-Wframe-larger-than=]

This warning doesn't appear if SNDRV_MAX_LINKS was 64.
Can we reduce it ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-19  1:42     ` Kuninori Morimoto
  0 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2021-04-19  1:42 UTC (permalink / raw)
  To: Thierry Reding
  Cc: alsa-devel, Takashi Iwai, Liam Girdwood, Mark Brown, linux-tegra,
	Jon Hunter


Hi Thierry

> From: Thierry Reding <treding@nvidia.com>
> 
> On Tegra186 and later, the number of links can go up to 72, so bump the
> maximum number of links to the next power of two (128).
> 
> Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> Signed-off-by: Thierry Reding <treding@nvidia.com>
(snip)
> +#define SNDRV_MAX_LINKS 128
> +
>  struct link_info {
>  	int link; /* number of link */
>  	int cpu;  /* turn for CPU / Codec */
> -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> +	struct prop_nums num[SNDRV_MAX_LINKS];
>  };


How many numbers do you really need ?

Because simple-card needs many parameters,
thus I will get below warning.
# It is not yet happen on upstream kernel, but will be

	warning: the frame size of 2280 bytes is larger than 2048 bytes [-Wframe-larger-than=]

This warning doesn't appear if SNDRV_MAX_LINKS was 64.
Can we reduce it ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-19  1:42     ` Kuninori Morimoto
@ 2021-04-19 14:51       ` Thierry Reding
  -1 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-19 14:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: Mark Brown, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Jon Hunter, alsa-devel, linux-tegra

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

On Mon, Apr 19, 2021 at 10:42:55AM +0900, Kuninori Morimoto wrote:
> 
> Hi Thierry
> 
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > On Tegra186 and later, the number of links can go up to 72, so bump the
> > maximum number of links to the next power of two (128).
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> (snip)
> > +#define SNDRV_MAX_LINKS 128
> > +
> >  struct link_info {
> >  	int link; /* number of link */
> >  	int cpu;  /* turn for CPU / Codec */
> > -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> > +	struct prop_nums num[SNDRV_MAX_LINKS];
> >  };
> 
> 
> How many numbers do you really need ?

As I mentioned in the commit message, the maximum I've seen is 72.
Rounding up to the next power of two seemed like a good idea to give a
bit of extra headroom.

> Because simple-card needs many parameters,
> thus I will get below warning.
> # It is not yet happen on upstream kernel, but will be
> 
> 	warning: the frame size of 2280 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> 
> This warning doesn't appear if SNDRV_MAX_LINKS was 64.
> Can we reduce it ?

Reducing from 128 to, say, 80 should eliminate that warning, but I
wonder if perhaps a better solution for the longer term would be to
allocate this structure on the heap rather than on the stack? That
way we don't risk triggering this warning again in the future.

The data structure seems to be only used during initialization, so
something like a kzalloc()/kfree() pair doesn't seem like it would
hurt much performance-wise. Add in the devm_ variants and the code
complexity should also remain moderately low.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-19 14:51       ` Thierry Reding
  0 siblings, 0 replies; 20+ messages in thread
From: Thierry Reding @ 2021-04-19 14:51 UTC (permalink / raw)
  To: Kuninori Morimoto
  Cc: alsa-devel, Takashi Iwai, Liam Girdwood, Mark Brown, linux-tegra,
	Jon Hunter

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

On Mon, Apr 19, 2021 at 10:42:55AM +0900, Kuninori Morimoto wrote:
> 
> Hi Thierry
> 
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > On Tegra186 and later, the number of links can go up to 72, so bump the
> > maximum number of links to the next power of two (128).
> > 
> > Fixes: f2138aed231c ("ASoC: simple-card-utils: enable flexible CPU/Codec/Platform")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> (snip)
> > +#define SNDRV_MAX_LINKS 128
> > +
> >  struct link_info {
> >  	int link; /* number of link */
> >  	int cpu;  /* turn for CPU / Codec */
> > -	struct prop_nums num[SNDRV_MINOR_DEVICES];
> > +	struct prop_nums num[SNDRV_MAX_LINKS];
> >  };
> 
> 
> How many numbers do you really need ?

As I mentioned in the commit message, the maximum I've seen is 72.
Rounding up to the next power of two seemed like a good idea to give a
bit of extra headroom.

> Because simple-card needs many parameters,
> thus I will get below warning.
> # It is not yet happen on upstream kernel, but will be
> 
> 	warning: the frame size of 2280 bytes is larger than 2048 bytes [-Wframe-larger-than=]
> 
> This warning doesn't appear if SNDRV_MAX_LINKS was 64.
> Can we reduce it ?

Reducing from 128 to, say, 80 should eliminate that warning, but I
wonder if perhaps a better solution for the longer term would be to
allocate this structure on the heap rather than on the stack? That
way we don't risk triggering this warning again in the future.

The data structure seems to be only used during initialization, so
something like a kzalloc()/kfree() pair doesn't seem like it would
hurt much performance-wise. Add in the devm_ variants and the code
complexity should also remain moderately low.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-19 14:51       ` Thierry Reding
@ 2021-04-19 15:08         ` Mark Brown
  -1 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-19 15:08 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Kuninori Morimoto, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Jon Hunter, alsa-devel, linux-tegra

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

On Mon, Apr 19, 2021 at 04:51:42PM +0200, Thierry Reding wrote:

> Reducing from 128 to, say, 80 should eliminate that warning, but I
> wonder if perhaps a better solution for the longer term would be to
> allocate this structure on the heap rather than on the stack? That
> way we don't risk triggering this warning again in the future.

> The data structure seems to be only used during initialization, so
> something like a kzalloc()/kfree() pair doesn't seem like it would
> hurt much performance-wise. Add in the devm_ variants and the code
> complexity should also remain moderately low.

Yes, that'd be much better - ideally we wouldn't have a fixed limit of
any kind but that's a more involved change and quite possibly more
trouble than it's worth.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-19 15:08         ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2021-04-19 15:08 UTC (permalink / raw)
  To: Thierry Reding
  Cc: alsa-devel, Kuninori Morimoto, Takashi Iwai, Liam Girdwood,
	Jon Hunter, linux-tegra

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

On Mon, Apr 19, 2021 at 04:51:42PM +0200, Thierry Reding wrote:

> Reducing from 128 to, say, 80 should eliminate that warning, but I
> wonder if perhaps a better solution for the longer term would be to
> allocate this structure on the heap rather than on the stack? That
> way we don't risk triggering this warning again in the future.

> The data structure seems to be only used during initialization, so
> something like a kzalloc()/kfree() pair doesn't seem like it would
> hurt much performance-wise. Add in the devm_ variants and the code
> complexity should also remain moderately low.

Yes, that'd be much better - ideally we wouldn't have a fixed limit of
any kind but that's a more involved change and quite possibly more
trouble than it's worth.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
  2021-04-19 15:08         ` Mark Brown
@ 2021-04-19 22:15           ` Kuninori Morimoto
  -1 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2021-04-19 22:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: Thierry Reding, Liam Girdwood, Takashi Iwai, Jaroslav Kysela,
	Jon Hunter, alsa-devel, linux-tegra


Hi Thierry, Mark

> > The data structure seems to be only used during initialization, so
> > something like a kzalloc()/kfree() pair doesn't seem like it would
> > hurt much performance-wise. Add in the devm_ variants and the code
> > complexity should also remain moderately low.
> 
> Yes, that'd be much better - ideally we wouldn't have a fixed limit of
> any kind but that's a more involved change and quite possibly more
> trouble than it's worth.

Using "fixed array style" was very easy for me (*^o^*).

OK, let's use kzalloc()/kfree() style.
I can try it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128
@ 2021-04-19 22:15           ` Kuninori Morimoto
  0 siblings, 0 replies; 20+ messages in thread
From: Kuninori Morimoto @ 2021-04-19 22:15 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, Takashi Iwai, Liam Girdwood, Thierry Reding,
	linux-tegra, Jon Hunter


Hi Thierry, Mark

> > The data structure seems to be only used during initialization, so
> > something like a kzalloc()/kfree() pair doesn't seem like it would
> > hurt much performance-wise. Add in the devm_ variants and the code
> > complexity should also remain moderately low.
> 
> Yes, that'd be much better - ideally we wouldn't have a fixed limit of
> any kind but that's a more involved change and quite possibly more
> trouble than it's worth.

Using "fixed array style" was very easy for me (*^o^*).

OK, let's use kzalloc()/kfree() style.
I can try it.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-04-19 22:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16  7:11 [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links Thierry Reding
2021-04-16  7:11 ` Thierry Reding
2021-04-16  7:11 ` [PATCH 2/2] ASoC: simple-card-utils: Increase maximum number of links to 128 Thierry Reding
2021-04-16  7:11   ` Thierry Reding
2021-04-16 14:50   ` Jon Hunter
2021-04-16 14:50     ` Jon Hunter
2021-04-19  1:42   ` Kuninori Morimoto
2021-04-19  1:42     ` Kuninori Morimoto
2021-04-19 14:51     ` Thierry Reding
2021-04-19 14:51       ` Thierry Reding
2021-04-19 15:08       ` Mark Brown
2021-04-19 15:08         ` Mark Brown
2021-04-19 22:15         ` Kuninori Morimoto
2021-04-19 22:15           ` Kuninori Morimoto
2021-04-16 14:49 ` [PATCH 1/2] ASoC: simple-card-utils: Propagate errors on too many links Jon Hunter
2021-04-16 14:49   ` Jon Hunter
2021-04-16 15:55   ` Mark Brown
2021-04-16 15:55     ` Mark Brown
2021-04-16 16:01 ` Mark Brown
2021-04-16 16:01   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.