[bug report] ASoC: tegra: Add Tegra210 based OPE driver
Hello Sameer Pujar,
The patch 7358a803c778: "ASoC: tegra: Add Tegra210 based OPE driver" from Jun 3, 2022, leads to the following Smatch static checker warning:
sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false here
sound/soc/tegra/tegra210_mbdrc.c 769 int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) 770 { 771 struct tegra210_ope *ope = snd_soc_component_get_drvdata(cmpnt); 772 const struct tegra210_mbdrc_config *conf = &mbdrc_init_config; 773 u32 val = 0; 774 unsigned int i; 775 776 regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); 777 --> 778 if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS is zero so this can't be true.
#define TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS (0 << TEGRA210_MBDRC_CFG_MBDRC_MODE_SHIFT)
The common false positive with this warning is that the macro is something which depends on the .config and in that case I just add it to the list in smatch_data/kernel.unconstant_macros. But in this case the macro is just always zero... Is there a plan to make it configurable or something?
779 return 0; 780 781 for (i = 0; i < MBDRC_NUM_BAND; i++) { 782 const struct tegra210_mbdrc_band_params *params = 783 &conf->band_params[i]; 784 785 u32 reg_off = i * TEGRA210_MBDRC_FILTER_PARAM_STRIDE; 786 787 tegra210_mbdrc_write_ram(ope->mbdrc_regmap, 788 reg_off + TEGRA210_MBDRC_CFG_RAM_CTRL, 789 reg_off + TEGRA210_MBDRC_CFG_RAM_DATA, 790 0, (u32 *)¶ms->biquad_params[0], 791 TEGRA210_MBDRC_MAX_BIQUAD_STAGES * 5); 792 } 793 return 0; 794 }
regards, dan carpenter
On 13-06-2022 19:40, Dan Carpenter wrote:
Hello Sameer Pujar,
The patch 7358a803c778: "ASoC: tegra: Add Tegra210 based OPE driver" from Jun 3, 2022, leads to the following Smatch static checker warning:
sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false heresound/soc/tegra/tegra210_mbdrc.c 769 int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) 770 { 771 struct tegra210_ope *ope = snd_soc_component_get_drvdata(cmpnt); 772 const struct tegra210_mbdrc_config *conf = &mbdrc_init_config; 773 u32 val = 0; 774 unsigned int i; 775 776 regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); 777 --> 778 if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS is zero so this can't be true.
#define TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS (0 << TEGRA210_MBDRC_CFG_MBDRC_MODE_SHIFT)
The common false positive with this warning is that the macro is something which depends on the .config and in that case I just add it to the list in smatch_data/kernel.unconstant_macros. But in this case the macro is just always zero... Is there a plan to make it configurable or something?
Thanks Dan for reporting this. The device is actually configurable. I will provide a patch to fix above condition.
On Tue, Jun 14, 2022 at 09:47:21AM +0530, Sameer Pujar wrote:
On 13-06-2022 19:40, Dan Carpenter wrote:
Hello Sameer Pujar,
The patch 7358a803c778: "ASoC: tegra: Add Tegra210 based OPE driver" from Jun 3, 2022, leads to the following Smatch static checker warning:
sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false heresound/soc/tegra/tegra210_mbdrc.c 769 int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) 770 { 771 struct tegra210_ope *ope = snd_soc_component_get_drvdata(cmpnt); 772 const struct tegra210_mbdrc_config *conf = &mbdrc_init_config; 773 u32 val = 0; 774 unsigned int i; 775 776 regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); 777 --> 778 if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS is zero so this can't be true.
#define TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS (0 << TEGRA210_MBDRC_CFG_MBDRC_MODE_SHIFT)
The common false positive with this warning is that the macro is something which depends on the .config and in that case I just add it to the list in smatch_data/kernel.unconstant_macros. But in this case the macro is just always zero... Is there a plan to make it configurable or something?
Thanks Dan for reporting this. The device is actually configurable. I will provide a patch to fix above condition.
What I meant by configurable is:
#ifdef CONFIG_FOO #define MASK 0x30 #else #define MASK 0 #endif
Smatch works on the preprocessed source so it doesn't see that there are two definitions of MASK.
regards, dan carpenter
On 14-06-2022 11:39, Dan Carpenter wrote:
Hello Sameer Pujar,
The patch 7358a803c778: "ASoC: tegra: Add Tegra210 based OPE driver" from Jun 3, 2022, leads to the following Smatch static checker warning:
sound/soc/tegra/tegra210_mbdrc.c:778 tegra210_mbdrc_hw_params() warn: bitwise AND condition is false heresound/soc/tegra/tegra210_mbdrc.c 769 int tegra210_mbdrc_hw_params(struct snd_soc_component *cmpnt) 770 { 771 struct tegra210_ope *ope = snd_soc_component_get_drvdata(cmpnt); 772 const struct tegra210_mbdrc_config *conf = &mbdrc_init_config; 773 u32 val = 0; 774 unsigned int i; 775 776 regmap_read(ope->mbdrc_regmap, TEGRA210_MBDRC_CFG, &val); 777 --> 778 if (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS is zero so this can't be true.
#define TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS (0 << TEGRA210_MBDRC_CFG_MBDRC_MODE_SHIFT)
The common false positive with this warning is that the macro is something which depends on the .config and in that case I just add it to the list in smatch_data/kernel.unconstant_macros. But in this case the macro is just always zero... Is there a plan to make it configurable or something?
Thanks Dan for reporting this. The device is actually configurable. I will provide a patch to fix above condition.
What I meant by configurable is:
#ifdef CONFIG_FOO #define MASK 0x30 #else #define MASK 0 #endif
Smatch works on the preprocessed source so it doesn't see that there are two definitions of MASK.
By configurable I meant, the device (OPE in this case) supports different configurations driven by user space control settings. Above if condition (val & TEGRA210_MBDRC_CFG_MBDRC_MODE_BYPASS) is wrong, instead proper mask needs to be used. I have sent a patch to fix this now.
participants (2)
-
Dan Carpenter -
Sameer Pujar