[PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Pierre-Louis Bossart]

No functional changes except for patch 2 and 3 where missing error
checks were added for consistency.

Pierre-Louis Bossart (6):
  ASoC: samsung: i2s: remove unassigned variable
  ASoC: samsung: s3c24xx_simtec: add missing error check
  ASoC: samsung: smdk_wm8994: add missing return
  ASoC: samsung: snow: remove useless test
  ASoC: samsung: tm2_wm5110: check of_parse return value
  ASoC: samsung: tm2_wm5510: remove shadowing variable

 sound/soc/samsung/i2s.c            | 3 +--
 sound/soc/samsung/s3c24xx_simtec.c | 5 +++++
 sound/soc/samsung/smdk_wm8994.c    | 1 +
 sound/soc/samsung/snow.c           | 5 +----
 sound/soc/samsung/tm2_wm5110.c     | 3 +--
 5 files changed, 9 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/6] ASoC: samsung: i2s: remove unassigned variable

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/i2s.c:1159:18: style: Variable 'dai' is not assigned
a value. [unassignedVariable]
 struct i2s_dai *dai;
                 ^

This variable is only used for a sizeof(*dai).

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/i2s.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index b043183174b2..c632842d42eb 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -1156,11 +1156,10 @@ static int i2s_alloc_dais(struct samsung_i2s_priv *priv,
 	static const char *stream_names[] = { "Primary Playback",
 					      "Secondary Playback" };
 	struct snd_soc_dai_driver *dai_drv;
-	struct i2s_dai *dai;
 	int i;
 
 	priv->dai = devm_kcalloc(&priv->pdev->dev, num_dais,
-				     sizeof(*dai), GFP_KERNEL);
+				     sizeof(struct i2s_dai), GFP_KERNEL);
 	if (!priv->dai)
 		return -ENOMEM;
 
-- 
2.25.1

[PATCH 2/6] ASoC: samsung: s3c24xx_simtec: add missing error check

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/s3c24xx_simtec.c:191:7: style: Variable 'ret' is
assigned a value that is never used. [unreadVariable]
  ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C24XX_DIV_PRESCALER,
      ^

Looking at the code, it's not clear why the return value is checked in
the two other cases but not here, so mirror the behavior and add a
check.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/s3c24xx_simtec.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/samsung/s3c24xx_simtec.c b/sound/soc/samsung/s3c24xx_simtec.c
index 3cddd11344ac..81a29d12c57d 100644
--- a/sound/soc/samsung/s3c24xx_simtec.c
+++ b/sound/soc/samsung/s3c24xx_simtec.c
@@ -190,6 +190,11 @@ static int simtec_hw_params(struct snd_pcm_substream *substream,
 
 		ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C24XX_DIV_PRESCALER,
 					     cdclk_scale);
+		if (ret) {
+			pr_err("%s: failed to set clock div\n",
+			       __func__);
+			return ret;
+		}
 	}
 
 	return 0;
-- 
2.25.1

[PATCH 3/6] ASoC: samsung: smdk_wm8994: add missing return

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/smdk_wm8994.c:179:6: style: Variable 'ret' is
reassigned a value before the old one has been
used. [redundantAssignment]
 ret = devm_snd_soc_register_card(&pdev->dev, card);
     ^
sound/soc/samsung/smdk_wm8994.c:166:8: note: ret is assigned
   ret = -EINVAL;
       ^
sound/soc/samsung/smdk_wm8994.c:179:6: note: ret is overwritten
 ret = devm_snd_soc_register_card(&pdev->dev, card);
     ^

The initial authors bothered to set ret to -EINVAL and throw a
dev_err() message, so it looks like there is a missing return to avoid
continuing if the property is missing.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/smdk_wm8994.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/samsung/smdk_wm8994.c b/sound/soc/samsung/smdk_wm8994.c
index 681b244d5312..39a7a449f554 100644
--- a/sound/soc/samsung/smdk_wm8994.c
+++ b/sound/soc/samsung/smdk_wm8994.c
@@ -164,6 +164,7 @@ static int smdk_audio_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev,
 			   "Property 'samsung,i2s-controller' missing or invalid\n");
 			ret = -EINVAL;
+			return ret;
 		}
 
 		smdk_dai[0].platforms->name = NULL;
-- 
2.25.1

[PATCH 4/6] ASoC: samsung: snow: remove useless test

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/snow.c:112:2: style:inconclusive: Found duplicate
branches for 'if' and 'else'. [duplicateBranch]
 if (rtd->num_codecs > 1)
 ^
sound/soc/samsung/snow.c:114:2: note: Found duplicate branches for
'if' and 'else'.
 else
 ^
sound/soc/samsung/snow.c:112:2: note: Found duplicate branches for
'if' and 'else'.
 if (rtd->num_codecs > 1)
 ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/snow.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c
index 989af624dd11..6da674e901ca 100644
--- a/sound/soc/samsung/snow.c
+++ b/sound/soc/samsung/snow.c
@@ -109,10 +109,7 @@ static int snow_late_probe(struct snd_soc_card *card)
 	rtd = snd_soc_get_pcm_runtime(card, &card->dai_link[0]);
 
 	/* In the multi-codec case codec_dais 0 is MAX98095 and 1 is HDMI. */
-	if (rtd->num_codecs > 1)
-		codec_dai = asoc_rtd_to_codec(rtd, 0);
-	else
-		codec_dai = asoc_rtd_to_codec(rtd, 0);
+	codec_dai = asoc_rtd_to_codec(rtd, 0);
 
 	/* Set the MCLK rate for the codec */
 	return snd_soc_dai_set_sysclk(codec_dai, 0,
-- 
2.25.1

[PATCH 5/6] ASoC: samsung: tm2_wm5110: check of_parse return value

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/tm2_wm5110.c:605:6: style: Variable 'ret' is
reassigned a value before the old one has been
used. [redundantAssignment]
 ret = devm_snd_soc_register_component(dev, &tm2_component,
     ^
sound/soc/samsung/tm2_wm5110.c:554:7: note: ret is assigned
  ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
      ^
sound/soc/samsung/tm2_wm5110.c:605:6: note: ret is overwritten
 ret = devm_snd_soc_register_component(dev, &tm2_component,
     ^

it seems wise to first test the return value before checking if the
returned argument is not null?

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/tm2_wm5110.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
index 9300fef9bf26..da6204248f82 100644
--- a/sound/soc/samsung/tm2_wm5110.c
+++ b/sound/soc/samsung/tm2_wm5110.c
@@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
 
 		ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
 						 cells_name, i, &args);
-		if (!args.np) {
+		if (ret || !args.np) {
 			dev_err(dev, "i2s-controller property parse error: %d\n", i);
 			ret = -EINVAL;
 			goto dai_node_put;
-- 
2.25.1

[PATCH 6/6] ASoC: samsung: tm2_wm5510: remove shadowing variable

[Pierre-Louis Bossart]

cppcheck warning:

sound/soc/samsung/tm2_wm5110.c:552:26: style: Local variable 'args'
shadows outer variable [shadowVariable]
  struct of_phandle_args args;
                         ^
sound/soc/samsung/tm2_wm5110.c:504:25: note: Shadowed declaration
 struct of_phandle_args args;
                        ^
sound/soc/samsung/tm2_wm5110.c:552:26: note: Shadow variable
  struct of_phandle_args args;
                         ^

it's not clear why there was a need for a local variable at a lower
scope, remove it and share the same variable between scopes.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/samsung/tm2_wm5110.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
index da6204248f82..b9c17fdd168d 100644
--- a/sound/soc/samsung/tm2_wm5110.c
+++ b/sound/soc/samsung/tm2_wm5110.c
@@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev)
 	}
 
 	for (i = 0; i < num_codecs; i++) {
-		struct of_phandle_args args;
 
 		ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
 						 cells_name, i, &args);
-- 
2.25.1

Re: [PATCH 1/6] ASoC: samsung: i2s: remove unassigned variable

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:13PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/i2s.c:1159:18: style: Variable 'dai' is not assigned
> a value. [unassignedVariable]
>  struct i2s_dai *dai;
>                  ^
> 
> This variable is only used for a sizeof(*dai).
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/i2s.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 2/6] ASoC: samsung: s3c24xx_simtec: add missing error check

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:14PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/s3c24xx_simtec.c:191:7: style: Variable 'ret' is
> assigned a value that is never used. [unreadVariable]
>   ret = snd_soc_dai_set_clkdiv(cpu_dai, S3C24XX_DIV_PRESCALER,
>       ^
> 
> Looking at the code, it's not clear why the return value is checked in
> the two other cases but not here, so mirror the behavior and add a
> check.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/s3c24xx_simtec.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 3/6] ASoC: samsung: smdk_wm8994: add missing return

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:15PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/smdk_wm8994.c:179:6: style: Variable 'ret' is
> reassigned a value before the old one has been
> used. [redundantAssignment]
>  ret = devm_snd_soc_register_card(&pdev->dev, card);
>      ^
> sound/soc/samsung/smdk_wm8994.c:166:8: note: ret is assigned
>    ret = -EINVAL;
>        ^
> sound/soc/samsung/smdk_wm8994.c:179:6: note: ret is overwritten
>  ret = devm_snd_soc_register_card(&pdev->dev, card);
>      ^
> 
> The initial authors bothered to set ret to -EINVAL and throw a
> dev_err() message, so it looks like there is a missing return to avoid
> continuing if the property is missing.

Good catch. It's a required property.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 4/6] ASoC: samsung: snow: remove useless test

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:16PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/snow.c:112:2: style:inconclusive: Found duplicate
> branches for 'if' and 'else'. [duplicateBranch]
>  if (rtd->num_codecs > 1)
>  ^
> sound/soc/samsung/snow.c:114:2: note: Found duplicate branches for
> 'if' and 'else'.
>  else
>  ^
> sound/soc/samsung/snow.c:112:2: note: Found duplicate branches for
> 'if' and 'else'.
>  if (rtd->num_codecs > 1)
>  ^
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/snow.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 

Fixes: 7de6b6bc1a58 ("ASoC: samsung: use asoc_rtd_to_cpu() / asoc_rtd_to_codec() macro for DAI pointer")
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 5/6] ASoC: samsung: tm2_wm5110: check of_parse return value

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:17PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/tm2_wm5110.c:605:6: style: Variable 'ret' is
> reassigned a value before the old one has been
> used. [redundantAssignment]
>  ret = devm_snd_soc_register_component(dev, &tm2_component,
>      ^
> sound/soc/samsung/tm2_wm5110.c:554:7: note: ret is assigned
>   ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
>       ^
> sound/soc/samsung/tm2_wm5110.c:605:6: note: ret is overwritten
>  ret = devm_snd_soc_register_component(dev, &tm2_component,
>      ^
> 
> it seems wise to first test the return value before checking if the
> returned argument is not null?

Actually this is real bug. The args is a stack variable, so it could
have junk (uninitialized) therefore args.np could have a non-NULL and
random value even though property was missing. Later could trigger
invalid pointer dereference.

  Fixes: 8d1513cef51a ("ASoC: samsung: Add support for HDMI audio on TM2 board")
  Cc: <stable@vger.kernel.org>

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/tm2_wm5110.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
> index 9300fef9bf26..da6204248f82 100644
> --- a/sound/soc/samsung/tm2_wm5110.c
> +++ b/sound/soc/samsung/tm2_wm5110.c
> @@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
>  
>  		ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
>  						 cells_name, i, &args);
> -		if (!args.np) {
> +		if (ret || !args.np) {

Only "if (ret)" because args.np won't be initialized on errors.

Best regards,
Krzysztof

Re: [PATCH 6/6] ASoC: samsung: tm2_wm5510: remove shadowing variable

[Krzysztof Kozlowski]

On Fri, Feb 19, 2021 at 05:09:18PM -0600, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/tm2_wm5110.c:552:26: style: Local variable 'args'
> shadows outer variable [shadowVariable]
>   struct of_phandle_args args;
>                          ^
> sound/soc/samsung/tm2_wm5110.c:504:25: note: Shadowed declaration
>  struct of_phandle_args args;
>                         ^
> sound/soc/samsung/tm2_wm5110.c:552:26: note: Shadow variable
>   struct of_phandle_args args;
>                          ^
> 
> it's not clear why there was a need for a local variable at a lower
> scope, remove it and share the same variable between scopes.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  sound/soc/samsung/tm2_wm5110.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
> index da6204248f82..b9c17fdd168d 100644
> --- a/sound/soc/samsung/tm2_wm5110.c
> +++ b/sound/soc/samsung/tm2_wm5110.c
> @@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev)
>  	}
>  
>  	for (i = 0; i < num_codecs; i++) {
> -		struct of_phandle_args args;

I would prefer to have them more local, so:
1. Remove args in tm2_probe() scope.
2. Add args around line 588 in "if (num_codecs > 1) {" block.

Best regards,
Krzysztof

Re: [PATCH 5/6] ASoC: samsung: tm2_wm5110: check of_parse return value

[Pierre-Louis Bossart]

>> diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
>> index 9300fef9bf26..da6204248f82 100644
>> --- a/sound/soc/samsung/tm2_wm5110.c
>> +++ b/sound/soc/samsung/tm2_wm5110.c
>> @@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
>>   
>>   		ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
>>   						 cells_name, i, &args);
>> -		if (!args.np) {
>> +		if (ret || !args.np) {
> 
> Only "if (ret)" because args.np won't be initialized on errors.

Thanks Krzysztof for the review, I will make that change in a v2.
But just to be clear, there's no need to test args.np then?

Re: [PATCH 6/6] ASoC: samsung: tm2_wm5510: remove shadowing variable

[Pierre-Louis Bossart]

>> diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
>> index da6204248f82..b9c17fdd168d 100644
>> --- a/sound/soc/samsung/tm2_wm5110.c
>> +++ b/sound/soc/samsung/tm2_wm5110.c
>> @@ -549,7 +549,6 @@ static int tm2_probe(struct platform_device *pdev)
>>   	}
>>   
>>   	for (i = 0; i < num_codecs; i++) {
>> -		struct of_phandle_args args;
> 
> I would prefer to have them more local, so:
> 1. Remove args in tm2_probe() scope.
> 2. Add args around line 588 in "if (num_codecs > 1) {" block.

Will change this in a v2.

Re: (subset) [PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Mark Brown]

On Fri, 19 Feb 2021 17:09:12 -0600, Pierre-Louis Bossart wrote:
> No functional changes except for patch 2 and 3 where missing error
> checks were added for consistency.
> 
> Pierre-Louis Bossart (6):
>   ASoC: samsung: i2s: remove unassigned variable
>   ASoC: samsung: s3c24xx_simtec: add missing error check
>   ASoC: samsung: smdk_wm8994: add missing return
>   ASoC: samsung: snow: remove useless test
>   ASoC: samsung: tm2_wm5110: check of_parse return value
>   ASoC: samsung: tm2_wm5510: remove shadowing variable
> 
> [...]

Applied to

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

Thanks!

[5/6] ASoC: samsung: tm2_wm5110: check of_parse return value
      commit: 75fa6833aef349fce1b315eaa96c9611a227014b

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

Re: [PATCH 5/6] ASoC: samsung: tm2_wm5110: check of_parse return value

[Krzysztof Kozlowski]

On Mon, 22 Feb 2021 at 17:45, Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> >> diff --git a/sound/soc/samsung/tm2_wm5110.c b/sound/soc/samsung/tm2_wm5110.c
> >> index 9300fef9bf26..da6204248f82 100644
> >> --- a/sound/soc/samsung/tm2_wm5110.c
> >> +++ b/sound/soc/samsung/tm2_wm5110.c
> >> @@ -553,7 +553,7 @@ static int tm2_probe(struct platform_device *pdev)
> >>
> >>              ret = of_parse_phandle_with_args(dev->of_node, "i2s-controller",
> >>                                               cells_name, i, &args);
> >> -            if (!args.np) {
> >> +            if (ret || !args.np) {
> >
> > Only "if (ret)" because args.np won't be initialized on errors.
>
> Thanks Krzysztof for the review, I will make that change in a v2.
> But just to be clear, there's no need to test args.np then?

Correct, no need to test args.np.

Best regards,
Krzysztof

Re: (subset) [PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Krzysztof Kozlowski]

On Mon, 22 Feb 2021 at 17:08, Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, 19 Feb 2021 17:09:12 -0600, Pierre-Louis Bossart wrote:
> > No functional changes except for patch 2 and 3 where missing error
> > checks were added for consistency.
> >
> > Pierre-Louis Bossart (6):
> >   ASoC: samsung: i2s: remove unassigned variable
> >   ASoC: samsung: s3c24xx_simtec: add missing error check
> >   ASoC: samsung: smdk_wm8994: add missing return
> >   ASoC: samsung: snow: remove useless test
> >   ASoC: samsung: tm2_wm5110: check of_parse return value
> >   ASoC: samsung: tm2_wm5510: remove shadowing variable
> >
> > [...]
>
> Applied to
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
>
> Thanks!
>
> [5/6] ASoC: samsung: tm2_wm5110: check of_parse return value
>       commit: 75fa6833aef349fce1b315eaa96c9611a227014b

Hi Mark,

Hmmm, I had comments about this one so it should not have been
applied. The check if (ret || !args.np) is still not good (or
confusing) because args is an uninitialized stack value.

Best regards,
Krzysztof

Re: (subset) [PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Mark Brown]

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

On Mon, Feb 22, 2021 at 06:31:15PM +0100, Krzysztof Kozlowski wrote:

> Hmmm, I had comments about this one so it should not have been
> applied. The check if (ret || !args.np) is still not good (or
> confusing) because args is an uninitialized stack value.

Ah, I saw the "this is actually a fix CC stable" bit, the bit saying
there were issues was hidden - it looked like you'd just not deleted
context.

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

Re: (subset) [PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Pierre-Louis Bossart]

On 2/22/21 2:19 PM, Mark Brown wrote:
> On Mon, Feb 22, 2021 at 06:31:15PM +0100, Krzysztof Kozlowski wrote:
> 
>> Hmmm, I had comments about this one so it should not have been
>> applied. The check if (ret || !args.np) is still not good (or
>> confusing) because args is an uninitialized stack value.
> 
> Ah, I saw the "this is actually a fix CC stable" bit, the bit saying
> there were issues was hidden - it looked like you'd just not deleted
> context.

If you give me about an hour, I can resend a v2 series that includes 
Krzysztof's tags and provides the correct fix for this patch5.

Re: [PATCH 6/6] ASoC: samsung: tm2_wm5510: remove shadowing variable

[Sylwester Nawrocki]

On 20.02.2021 00:09, Pierre-Louis Bossart wrote:
> cppcheck warning:
> 
> sound/soc/samsung/tm2_wm5110.c:552:26: style: Local variable 'args'
> shadows outer variable [shadowVariable]
>   struct of_phandle_args args;

> it's not clear why there was a need for a local variable at a lower
> scope, remove it and share the same variable between scopes.

s/tm2_wm5510/tm2_wm5110

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
 

Re: (subset) [PATCH 0/6] ASoC: samsung: remove cppcheck warnings

[Mark Brown]

On Fri, 19 Feb 2021 17:09:12 -0600, Pierre-Louis Bossart wrote:
> No functional changes except for patch 2 and 3 where missing error
> checks were added for consistency.
> 
> Pierre-Louis Bossart (6):
>   ASoC: samsung: i2s: remove unassigned variable
>   ASoC: samsung: s3c24xx_simtec: add missing error check
>   ASoC: samsung: smdk_wm8994: add missing return
>   ASoC: samsung: snow: remove useless test
>   ASoC: samsung: tm2_wm5110: check of_parse return value
>   ASoC: samsung: tm2_wm5510: remove shadowing variable
> 
> [...]

Applied to

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

Thanks!

[1/6] ASoC: samsung: i2s: remove unassigned variable
      commit: b832fa1ce0826a915a9e1fe533fc86a1cf5ae8cd
[2/6] ASoC: samsung: s3c24xx_simtec: add missing error check
      commit: feb45eb2ecafdfaca5b82f27997e717ae3c70323
[3/6] ASoC: samsung: smdk_wm8994: add missing return
      commit: 1e4a9fcffd56b73acf4e706465be2df261da83de
[4/6] ASoC: samsung: snow: remove useless test
      commit: 4ff97b8dc7e6a3c5caf733ebad4efaf018829142

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