* [bug report] ASoC: tegra: Add Tegra210 based OPE driver
@ 2022-06-13 14:10 Dan Carpenter
2022-06-14 4:17 ` Sameer Pujar
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2022-06-13 14:10 UTC (permalink / raw)
To: spujar; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] ASoC: tegra: Add Tegra210 based OPE driver
2022-06-13 14:10 [bug report] ASoC: tegra: Add Tegra210 based OPE driver Dan Carpenter
@ 2022-06-14 4:17 ` Sameer Pujar
2022-06-14 6:09 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Sameer Pujar @ 2022-06-14 4:17 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
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 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?
Thanks Dan for reporting this. The device is actually configurable. I
will provide a patch to fix above condition.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] ASoC: tegra: Add Tegra210 based OPE driver
2022-06-14 4:17 ` Sameer Pujar
@ 2022-06-14 6:09 ` Dan Carpenter
2022-06-15 4:46 ` Sameer Pujar
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2022-06-14 6:09 UTC (permalink / raw)
To: Sameer Pujar; +Cc: alsa-devel
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 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?
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] ASoC: tegra: Add Tegra210 based OPE driver
2022-06-14 6:09 ` Dan Carpenter
@ 2022-06-15 4:46 ` Sameer Pujar
0 siblings, 0 replies; 4+ messages in thread
From: Sameer Pujar @ 2022-06-15 4:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: alsa-devel
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 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?
>> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-15 4:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-13 14:10 [bug report] ASoC: tegra: Add Tegra210 based OPE driver Dan Carpenter
2022-06-14 4:17 ` Sameer Pujar
2022-06-14 6:09 ` Dan Carpenter
2022-06-15 4:46 ` Sameer Pujar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox