[PATCH 0/8] ASoC: minor cleanup of warnings

[PATCH 0/8] ASoC: minor cleanup of warnings

[Pierre-Louis Bossart]

Sparse, make W=1 and cppcheck all report minor warnings.

The only functional change is in patch7 where the error code is now
returned to the caller.

Pierre-Louis Bossart (8):
  ASoC: topology: handle endianness warning
  ASoC: rt5682s: use 'static' qualifier
  ASoC: nau8821: fix kernel-doc
  ASoC: nau8821: clarify out-of-bounds check
  ASoC: mediatek: remove unnecessary initialization
  ASoC: mediatek: mt8195: rename shadowed array
  ASoC: mediatek: mt8195: fix return value
  ASoC: rockchip: i2s_tdm: improve return value handling

 sound/soc/codecs/nau8821.c                  | 6 ++++--
 sound/soc/codecs/rt5682s.c                  | 6 +++---
 sound/soc/mediatek/common/mtk-afe-fe-dai.c  | 2 +-
 sound/soc/mediatek/mt8195/mt8195-afe-pcm.c  | 4 ++--
 sound/soc/mediatek/mt8195/mt8195-dai-etdm.c | 2 +-
 sound/soc/rockchip/rockchip_i2s_tdm.c       | 2 +-
 sound/soc/soc-topology.c                    | 2 +-
 7 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.25.1

[PATCH 1/8] ASoC: topology: handle endianness warning

[Pierre-Louis Bossart]

Sparse reports the following warning:

sound/soc/soc-topology.c:1488:26: error: restricted __le32 degrades to
integer

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

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 88f849b119da..7a4559ddf903 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1485,7 +1485,7 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	if (!kcontrol_type)
 		goto err;
 
-	for (i = 0; i < w->num_kcontrols; i++) {
+	for (i = 0; i < le32_to_cpu(w->num_kcontrols); i++) {
 		control_hdr = (struct snd_soc_tplg_ctl_hdr *)tplg->pos;
 		switch (le32_to_cpu(control_hdr->ops.info)) {
 		case SND_SOC_TPLG_CTL_VOLSW:
-- 
2.25.1

[PATCH 2/8] ASoC: rt5682s: use 'static' qualifier

[Pierre-Louis Bossart]

Sparse reports the following warnings:

sound/soc/codecs/rt5682s.c:44:12: error: symbol 'rt5682s_supply_names'
was not declared. Should it be static?

sound/soc/codecs/rt5682s.c:74:26: error: symbol 'rt5682s_reg' was not
declared. Should it be static?

sound/soc/codecs/rt5682s.c:2841:30: error: symbol
'rt5682s_aif1_dai_ops' was not declared. Should it be static?

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

diff --git a/sound/soc/codecs/rt5682s.c b/sound/soc/codecs/rt5682s.c
index f6435b233a51..470957fcad6b 100644
--- a/sound/soc/codecs/rt5682s.c
+++ b/sound/soc/codecs/rt5682s.c
@@ -41,7 +41,7 @@ static const struct rt5682s_platform_data i2s_default_platform_data = {
 	.dai_clk_names[RT5682S_DAI_BCLK_IDX] = "rt5682-dai-bclk",
 };
 
-const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = {
+static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = {
 	"AVDD",
 	"MICVDD",
 };
@@ -71,7 +71,7 @@ static void rt5682s_apply_patch_list(struct rt5682s_priv *rt5682s,
 		dev_warn(dev, "Failed to apply regmap patch: %d\n", ret);
 }
 
-const struct reg_default rt5682s_reg[] = {
+static const struct reg_default rt5682s_reg[] = {
 	{0x0002, 0x8080},
 	{0x0003, 0x0001},
 	{0x0005, 0x0000},
@@ -2838,7 +2838,7 @@ static int rt5682s_resume(struct snd_soc_component *component)
 #define rt5682s_resume NULL
 #endif
 
-const struct snd_soc_dai_ops rt5682s_aif1_dai_ops = {
+static const struct snd_soc_dai_ops rt5682s_aif1_dai_ops = {
 	.hw_params = rt5682s_hw_params,
 	.set_fmt = rt5682s_set_dai_fmt,
 	.set_tdm_slot = rt5682s_set_tdm_slot,
-- 
2.25.1

[PATCH 3/8] ASoC: nau8821: fix kernel-doc

[Pierre-Louis Bossart]

make W=1 reports warnings:

sound/soc/codecs/nau8821.c:1192: warning: Function parameter or member
'component' not described in 'nau8821_set_fll'

sound/soc/codecs/nau8821.c:1192: warning: Function parameter or member
'pll_id' not described in 'nau8821_set_fll'

sound/soc/codecs/nau8821.c:1192: warning: Function parameter or member
'source' not described in 'nau8821_set_fll'

sound/soc/codecs/nau8821.c:1192: warning: Excess function parameter
'codec' description in 'nau8821_set_fll'

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/codecs/nau8821.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/nau8821.c b/sound/soc/codecs/nau8821.c
index 4ea56d2f9509..2757d2eeb48a 100644
--- a/sound/soc/codecs/nau8821.c
+++ b/sound/soc/codecs/nau8821.c
@@ -1178,7 +1178,9 @@ static void nau8821_fll_apply(struct nau8821 *nau8821,
 
 /**
  * nau8821_set_fll - FLL configuration of nau8821
- * @codec:  codec component
+ * @component:  codec component
+ * @pll_id:  PLL requested
+ * @source:  clock source
  * @freq_in:  frequency of input clock source
  * @freq_out:  must be 256*Fs in order to achieve the best performance
  *
-- 
2.25.1

[PATCH 4/8] ASoC: nau8821: clarify out-of-bounds check

[Pierre-Louis Bossart]

cppcheck reports a false positive

sound/soc/codecs/nau8821.c:390:17: error: Array 'dmic_speed_sel[4]'
accessed at index 4, which is out of bounds. [arrayIndexOutOfBounds]

  dmic_speed_sel[i].param, dmic_speed_sel[i].val);
                ^
sound/soc/codecs/nau8821.c:378:2: note: After for loop, i has value 4
 for (i = 0 ; i < 4 ; i++)
 ^
sound/soc/codecs/nau8821.c:390:17: note: Array index out of bounds
  dmic_speed_sel[i].param, dmic_speed_sel[i].val);
                ^

While the code is not incorrect, we can deal with the out-of-bounds
check in a clearer way that makes static analysis happy.

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

diff --git a/sound/soc/codecs/nau8821.c b/sound/soc/codecs/nau8821.c
index 2757d2eeb48a..2de818377484 100644
--- a/sound/soc/codecs/nau8821.c
+++ b/sound/soc/codecs/nau8821.c
@@ -381,7 +381,7 @@ static int dmic_clock_control(struct snd_soc_dapm_widget *w,
 			speed_selection = dmic_speed_sel[i].val;
 			break;
 		}
-	if (speed_selection < 0)
+	if (i == 4)
 		return -EINVAL;
 
 	dev_dbg(nau8821->dev,
-- 
2.25.1

[PATCH 5/8] ASoC: mediatek: remove unnecessary initialization

[Pierre-Louis Bossart]

Cppcheck warning:

sound/soc/mediatek/common/mtk-afe-fe-dai.c:353:8: style: Variable 'i'
is assigned a value that is never used. [unreadVariable]
 int i = 0;
       ^

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

diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.c b/sound/soc/mediatek/common/mtk-afe-fe-dai.c
index 4f2c2379531b..395be97f13ae 100644
--- a/sound/soc/mediatek/common/mtk-afe-fe-dai.c
+++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.c
@@ -350,7 +350,7 @@ int mtk_afe_resume(struct snd_soc_component *component)
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 	struct device *dev = afe->dev;
 	struct regmap *regmap = afe->regmap;
-	int i = 0;
+	int i;
 
 	if (pm_runtime_status_suspended(dev) || !afe->suspended)
 		return 0;
-- 
2.25.1

[PATCH 6/8] ASoC: mediatek: mt8195: rename shadowed array

[Pierre-Louis Bossart]

cppcheck warning:

Checking sound/soc/mediatek/mt8195/mt8195-afe-pcm.c ...
sound/soc/mediatek/mt8195/mt8195-afe-pcm.c:2884:35: style: Local
variable 'irq_data' shadows outer variable [shadowVariable]
  struct mtk_base_irq_data const *irq_data;
                                  ^
sound/soc/mediatek/mt8195/mt8195-afe-pcm.c:2235:39: note: Shadowed declaration
static const struct mtk_base_irq_data irq_data[MT8195_AFE_IRQ_NUM] = {
                                      ^
sound/soc/mediatek/mt8195/mt8195-afe-pcm.c:2884:35: note: Shadow variable
  struct mtk_base_irq_data const *irq_data;
                                  ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/mediatek/mt8195/mt8195-afe-pcm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c b/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
index df8b90baf981..2bb05a828e8d 100644
--- a/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
+++ b/sound/soc/mediatek/mt8195/mt8195-afe-pcm.c
@@ -2232,7 +2232,7 @@ static const struct mtk_base_memif_data memif_data[MT8195_AFE_MEMIF_NUM] = {
 	},
 };
 
-static const struct mtk_base_irq_data irq_data[MT8195_AFE_IRQ_NUM] = {
+static const struct mtk_base_irq_data irq_data_array[MT8195_AFE_IRQ_NUM] = {
 	[MT8195_AFE_IRQ_1] = {
 		.id = MT8195_AFE_IRQ_1,
 		.irq_cnt_reg = -1,
@@ -3100,7 +3100,7 @@ static int mt8195_afe_pcm_dev_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for (i = 0; i < afe->irqs_size; i++)
-		afe->irqs[i].irq_data = &irq_data[i];
+		afe->irqs[i].irq_data = &irq_data_array[i];
 
 	/* init memif */
 	afe->memif_size = MT8195_AFE_MEMIF_NUM;
-- 
2.25.1

[PATCH 7/8] ASoC: mediatek: mt8195: fix return value

[Pierre-Louis Bossart]

cppcheck reports the following warning:

sound/soc/mediatek/mt8195/mt8195-dai-etdm.c:1299:10: style: Variable
'ret' is assigned a value that is never used. [unreadVariable]
 int ret = 0;
         ^

The suggested change aligns the implementation of
mt8195_afe_disable_etdm() with mt8195_afe_enable_etdm() - same
negative return value upon error.

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

diff --git a/sound/soc/mediatek/mt8195/mt8195-dai-etdm.c b/sound/soc/mediatek/mt8195/mt8195-dai-etdm.c
index ac591d453e1e..c02c10da3600 100644
--- a/sound/soc/mediatek/mt8195/mt8195-dai-etdm.c
+++ b/sound/soc/mediatek/mt8195/mt8195-dai-etdm.c
@@ -1316,7 +1316,7 @@ static int mt8195_afe_disable_etdm(struct mtk_base_afe *afe, int dai_id)
 	}
 out:
 	spin_unlock_irqrestore(&afe_priv->afe_ctrl_lock, flags);
-	return 0;
+	return ret;
 }
 
 static int etdm_cowork_slv_sel(int id, int slave_mode)
-- 
2.25.1

[PATCH 8/8] ASoC: rockchip: i2s_tdm: improve return value handling

[Pierre-Louis Bossart]

cppcheck reports the following warning:

sound/soc/rockchip/rockchip_i2s_tdm.c:599:9: warning: Identical
condition and return expression 'ret', return value is always 0
[identicalConditionAfterEarlyExit]

 return ret;
        ^
sound/soc/rockchip/rockchip_i2s_tdm.c:594:6: note: If condition 'ret'
is true, the function will return/exit
 if (ret)
     ^
sound/soc/rockchip/rockchip_i2s_tdm.c:599:9: note: Returning identical
expression 'ret'

 return ret;
        ^

While the code is not wrong, it's clearer to return 0 directly.

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

diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
index e8dee1f95d85..17b9b287853a 100644
--- a/sound/soc/rockchip/rockchip_i2s_tdm.c
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
@@ -596,7 +596,7 @@ static int rockchip_i2s_tdm_clk_set_rate(struct rk_i2s_tdm_dev *i2s_tdm,
 
 	i2s_tdm->clk_ppm = ppm;
 
-	return ret;
+	return 0;
 }
 
 static int rockchip_i2s_tdm_calibrate_mclk(struct rk_i2s_tdm_dev *i2s_tdm,
-- 
2.25.1

Re: [PATCH 0/8] ASoC: minor cleanup of warnings

[Mark Brown]

On Mon, 25 Oct 2021 13:59:25 -0500, Pierre-Louis Bossart wrote:
> Sparse, make W=1 and cppcheck all report minor warnings.
> 
> The only functional change is in patch7 where the error code is now
> returned to the caller.
> 
> Pierre-Louis Bossart (8):
>   ASoC: topology: handle endianness warning
>   ASoC: rt5682s: use 'static' qualifier
>   ASoC: nau8821: fix kernel-doc
>   ASoC: nau8821: clarify out-of-bounds check
>   ASoC: mediatek: remove unnecessary initialization
>   ASoC: mediatek: mt8195: rename shadowed array
>   ASoC: mediatek: mt8195: fix return value
>   ASoC: rockchip: i2s_tdm: improve return value handling
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: topology: handle endianness warning
      commit: 1baad7dad115ea3976fb5e5d0e3f3bec83dfd7ca
[2/8] ASoC: rt5682s: use 'static' qualifier
      commit: 49ba5e936e1512d4c7812d433048f8909234fca0
[3/8] ASoC: nau8821: fix kernel-doc
      commit: 765e08bdc7faa44b13bf96df4663a580d68a1c49
[4/8] ASoC: nau8821: clarify out-of-bounds check
      commit: 46ae0b3f554a323322a770c0edee50aa8019a655
[5/8] ASoC: mediatek: remove unnecessary initialization
      commit: 33fb790fcc02a717c1cac90958f203f06da14f7e
[6/8] ASoC: mediatek: mt8195: rename shadowed array
      commit: 73983ad922764e747d40b486ec7c2526e0355db1
[7/8] ASoC: mediatek: mt8195: fix return value
      commit: 439c06f341aa1f09ad7def774998db1076946c98
[8/8] ASoC: rockchip: i2s_tdm: improve return value handling
      commit: f913582190ddfe2380ecf8ee87b4ff2c8dcb5d48

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