| Message ID | 20170502133303.19622-3-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9f3b777f1de9ff5d17f7259b8f7da5e9d4303e87 |
| Headers | show |
On Tue, May 02, 2017 at 10:33:01PM +0900, Takashi Sakamoto wrote: > This commit fixes this bug by using unsigned int type, instead of bool. > This bug is detected by sparse: > smsm8916-wcd-analog.c:809:43: warning: odd constant _Bool cast (40 becomes 1) > smsm8916-wcd-analog.c:814:43: warning: odd constant _Bool cast (40 becomes 1) This looks like a bug in sparse - the use of integers in a boolean context is totally valid and especially the fact that it is claiming there is a cast when clearly there is no cast is an obvious red flag, at the very least the message it reports is bogus.
On Sun, 14 May 2017 11:59:45 +0200, Mark Brown wrote: > > On Tue, May 02, 2017 at 10:33:01PM +0900, Takashi Sakamoto wrote: > > > This commit fixes this bug by using unsigned int type, instead of bool. > > This bug is detected by sparse: > > > smsm8916-wcd-analog.c:809:43: warning: odd constant _Bool cast (40 becomes 1) > > smsm8916-wcd-analog.c:814:43: warning: odd constant _Bool cast (40 becomes 1) > > This looks like a bug in sparse - the use of integers in a boolean > context is totally valid and especially the fact that it is claiming > there is a cast when clearly there is no cast is an obvious red flag, at > the very least the message it reports is bogus. Well, the function pm8916_wcd_analog_parse_dt() assigns a non-boolean value (BIT(6)) to a boolean field, and the value is supposed to be kept and passed to snd_soc_update_bits(), instead of dealing as a boolean. It looks rather like a buggy code to me. thanks, Takashi
On Mon, May 15, 2017 at 10:14:28AM +0200, Takashi Iwai wrote: > Mark Brown wrote: > > On Tue, May 02, 2017 at 10:33:01PM +0900, Takashi Sakamoto wrote: > > > This commit fixes this bug by using unsigned int type, instead of bool. > > > This bug is detected by sparse: > > > > > smsm8916-wcd-analog.c:809:43: warning: odd constant _Bool cast (40 becomes 1) > > > smsm8916-wcd-analog.c:814:43: warning: odd constant _Bool cast (40 becomes 1) > > This looks like a bug in sparse - the use of integers in a boolean > > context is totally valid and especially the fact that it is claiming > > there is a cast when clearly there is no cast is an obvious red flag, at > > the very least the message it reports is bogus. > Well, the function pm8916_wcd_analog_parse_dt() assigns a non-boolean > value (BIT(6)) to a boolean field, and the value is supposed to be > kept and passed to snd_soc_update_bits(), instead of dealing as a > boolean. It looks rather like a buggy code to me. The code is buggy (which is why I applied the patch) but the error message is talking about casts that simply aren't there which is also buggy.
On Mon, 15 May 2017 10:17:48 +0200, Mark Brown wrote: > > On Mon, May 15, 2017 at 10:14:28AM +0200, Takashi Iwai wrote: > > Mark Brown wrote: > > > > On Tue, May 02, 2017 at 10:33:01PM +0900, Takashi Sakamoto wrote: > > > > > This commit fixes this bug by using unsigned int type, instead of bool. > > > > This bug is detected by sparse: > > > > > > > smsm8916-wcd-analog.c:809:43: warning: odd constant _Bool cast (40 becomes 1) > > > > smsm8916-wcd-analog.c:814:43: warning: odd constant _Bool cast (40 becomes 1) > > > > This looks like a bug in sparse - the use of integers in a boolean > > > context is totally valid and especially the fact that it is claiming > > > there is a cast when clearly there is no cast is an obvious red flag, at > > > the very least the message it reports is bogus. > > > Well, the function pm8916_wcd_analog_parse_dt() assigns a non-boolean > > value (BIT(6)) to a boolean field, and the value is supposed to be > > kept and passed to snd_soc_update_bits(), instead of dealing as a > > boolean. It looks rather like a buggy code to me. > > The code is buggy (which is why I applied the patch) but the error > message is talking about casts that simply aren't there which is also > buggy. It's still better than TeX :) Takashi
diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c index d8e8590..a788029 100644 --- a/sound/soc/codecs/msm8916-wcd-analog.c +++ b/sound/soc/codecs/msm8916-wcd-analog.c @@ -223,8 +223,8 @@ struct pm8916_wcd_analog_priv { u16 codec_version; struct clk *mclk; struct regulator_bulk_data supplies[ARRAY_SIZE(supply_names)]; - bool micbias1_cap_mode; - bool micbias2_cap_mode; + unsigned int micbias1_cap_mode; + unsigned int micbias2_cap_mode; }; static const char *const adc2_mux_text[] = { "ZERO", "INP2", "INP3" }; @@ -285,7 +285,7 @@ static void pm8916_wcd_analog_micbias_enable(struct snd_soc_codec *codec) static int pm8916_wcd_analog_enable_micbias_ext(struct snd_soc_codec *codec, int event, - int reg, u32 cap_mode) + int reg, unsigned int cap_mode) { switch (event) { case SND_SOC_DAPM_POST_PMU:
A function snd_soc_update_bits() is an application of regmap_update_bits_base(). This function takes some arguments for bitmask and new value, thus the arguments should be a type which has width. However bool is used to variable for the argument. This brings truncation and results in invalid operation. This commit fixes this bug by using unsigned int type, instead of bool. This bug is detected by sparse: smsm8916-wcd-analog.c:809:43: warning: odd constant _Bool cast (40 becomes 1) smsm8916-wcd-analog.c:814:43: warning: odd constant _Bool cast (40 becomes 1) Fixes: 585e881e5b9e ("ASoC: codecs: Add msm8916-wcd analog codec") Cc: <stable@vger.kernel.org> # 4.10+ Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/soc/codecs/msm8916-wcd-analog.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)