[PATCH 0/8] ASoC: minor cleanup of warnings
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(-)
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:
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,
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 *
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,
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;
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;
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)
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,
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart