[PATCH 0/4] ALSA/ASoC: fix after merging ASoC PR for v4.12-rc1

[PATCH 0/4] ALSA/ASoC: fix after merging ASoC PR for v4.12-rc1

[Takashi Sakamoto]

Hi,

This patchset is for current 'for-linus' remote branch[1], after merging
PR from ALSA SoC part for v4.12-rc1[2].

The first two patch includes bug fixes introduced at former kernel
releases, the rest is to suppress sparse.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?h=for-linus&id=a5c3b32a1146e44f6b38fdfdfffc27842953420c
[2] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120308.html

Takashi Sakamoto (4):
  ASoC: codecs: da7213: fix invalid usage of bitwise operation in loop
    condition
  ASoC: codecs: msm8916: fix invalid cast to bool type
  ASoC: hisilicon: localize functions without external linkage
  ASoC: intel: atom: localize variable without external linkage

 sound/soc/codecs/da7213.c             | 14 +++++---------
 sound/soc/codecs/msm8916-wcd-analog.c |  6 +++---
 sound/soc/hisilicon/hi6210-i2s.c      | 11 ++++++-----
 sound/soc/intel/atom/sst/sst.c        |  2 +-
 4 files changed, 15 insertions(+), 18 deletions(-)

-- 
2.9.3

[PATCH 1/4] ASoC: codecs: da7213: fix invalid usage of bitwise operation in loop condition

[Takashi Sakamoto]

AND bitwise operation is used for retry counter and lock status, however in
this case, it's invalid as condition statement.

This commit fixes this bug with simpler representation. This bug is
detected by sparse with below warning:

sound/soc/codecs/da7213.c:775:57: warning: dubious: x & !y

Fixes: d575b0b0f01a ("ASoC: da7213: Add checking of SRM lock status before enabling DAI")
Cc: <stable@vger.kernel.org> # 4.4+
Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/soc/codecs/da7213.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
index 6dd7578..11d257c 100644
--- a/sound/soc/codecs/da7213.c
+++ b/sound/soc/codecs/da7213.c
@@ -737,7 +737,6 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,
 	struct da7213_priv *da7213 = snd_soc_codec_get_drvdata(codec);
 	u8 pll_ctrl, pll_status;
 	int i = 0;
-	bool srm_lock = false;
 
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
@@ -766,15 +765,12 @@ static int da7213_dai_event(struct snd_soc_dapm_widget *w,
 		/* Check SRM has locked */
 		do {
 			pll_status = snd_soc_read(codec, DA7213_PLL_STATUS);
-			if (pll_status & DA7219_PLL_SRM_LOCK) {
-				srm_lock = true;
-			} else {
-				++i;
-				msleep(50);
-			}
-		} while ((i < DA7213_SRM_CHECK_RETRIES) & (!srm_lock));
+			if (pll_status & DA7219_PLL_SRM_LOCK)
+				break;
+			msleep(50);
+		} while (++i < DA7213_SRM_CHECK_RETRIES);
 
-		if (!srm_lock)
+		if (i >= DA7213_SRM_CHECK_RETRIES)
 			dev_warn(codec->dev, "SRM failed to lock\n");
 
 		return 0;
-- 
2.9.3

[PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Takashi Sakamoto]

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(-)

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:
-- 
2.9.3

[PATCH 3/4] ASoC: hisilicon: localize functions without external linkage

[Takashi Sakamoto]

A driver for hi6210 sound interface on hi6220 boards includes some
functions which has no external linkage. These functions should have
static qualifier.

This commit adds the qualifier to localize the functions. This issue is
detected by sparse:

hi6210-i2s.c:100:5: warning: symbol 'hi6210_i2s_startup' was not declared. Should it be static?
hi6210-i2s.c:178:6: warning: symbol 'hi6210_i2s_shutdown' was not declared. Should it be static?
hi6210-i2s.c:527:27: warning: symbol 'hi6210_i2s_dai_init' was not declared. Should it be static?

Cc: John Stultz <john.stultz@linaro.org>
Cc: Andy Green <andy.green@linaro.org>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/soc/hisilicon/hi6210-i2s.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/hisilicon/hi6210-i2s.c b/sound/soc/hisilicon/hi6210-i2s.c
index 45163e5..b193d3b 100644
--- a/sound/soc/hisilicon/hi6210-i2s.c
+++ b/sound/soc/hisilicon/hi6210-i2s.c
@@ -97,8 +97,8 @@ static inline u32 hi6210_read_reg(struct hi6210_i2s *i2s, int reg)
 	return readl(i2s->base + reg);
 }
 
-int hi6210_i2s_startup(struct snd_pcm_substream *substream,
-		     struct snd_soc_dai *cpu_dai)
+static int hi6210_i2s_startup(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *cpu_dai)
 {
 	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
 	int ret, n;
@@ -175,8 +175,9 @@ int hi6210_i2s_startup(struct snd_pcm_substream *substream,
 
 	return 0;
 }
-void hi6210_i2s_shutdown(struct snd_pcm_substream *substream,
-		       struct snd_soc_dai *cpu_dai)
+
+static void hi6210_i2s_shutdown(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *cpu_dai)
 {
 	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
 	int n;
@@ -524,7 +525,7 @@ static struct snd_soc_dai_ops hi6210_i2s_dai_ops = {
 	.shutdown	= hi6210_i2s_shutdown,
 };
 
-struct snd_soc_dai_driver hi6210_i2s_dai_init = {
+static const struct snd_soc_dai_driver hi6210_i2s_dai_init = {
 	.probe		= hi6210_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-- 
2.9.3

[PATCH 4/4] ASoC: intel: atom: localize variable without external linkage

[Takashi Sakamoto]

A driver for Intel SST driver for old atom platform includes a variable
which has no external linkage. These functions should have static
qualifier.

This commit adds the qualifier to localize the variable. This issue is
detected by sparse:

sst.c:261:1: warning: symbol 'dev_attr_firmware_version' was not declared. Should it be static?

Cc: Sebastien Guiriec <sebastien.guiriec@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/soc/intel/atom/sst/sst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/atom/sst/sst.c b/sound/soc/intel/atom/sst/sst.c
index f9ba713..d97556a 100644
--- a/sound/soc/intel/atom/sst/sst.c
+++ b/sound/soc/intel/atom/sst/sst.c
@@ -258,7 +258,7 @@ static ssize_t firmware_version_show(struct device *dev,
 
 }
 
-DEVICE_ATTR_RO(firmware_version);
+static DEVICE_ATTR_RO(firmware_version);
 
 static const struct attribute *sst_fw_version_attrs[] = {
 	&dev_attr_firmware_version.attr,
-- 
2.9.3

RE: [PATCH 1/4] ASoC: codecs: da7213: fix invalid usage of bitwise operation in loop condition

[Adam Thomson]

On 02 May 2017 14:33, Takashi Sakamoto wrote:

> AND bitwise operation is used for retry counter and lock status, however in
> this case, it's invalid as condition statement.
> 
> This commit fixes this bug with simpler representation. This bug is
> detected by sparse with below warning:
> 
> sound/soc/codecs/da7213.c:775:57: warning: dubious: x & !y
> 
> Fixes: d575b0b0f01a ("ASoC: da7213: Add checking of SRM lock status before
> enabling DAI")

Agreed that the current '&' operator usage is technically invalid although the
code does operate correctly. Would rather you just change '&' to '&&' here
though rather than refactoring as I think the existing representation is clearer
to read in my opinion, and it's just a 1 line change. Please take that approach.

> Cc: <stable@vger.kernel.org> # 4.4+
> Cc: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/soc/codecs/da7213.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7213.c b/sound/soc/codecs/da7213.c
> index 6dd7578..11d257c 100644
> --- a/sound/soc/codecs/da7213.c
> +++ b/sound/soc/codecs/da7213.c
> @@ -737,7 +737,6 @@ static int da7213_dai_event(struct snd_soc_dapm_widget
> *w,
>  	struct da7213_priv *da7213 = snd_soc_codec_get_drvdata(codec);
>  	u8 pll_ctrl, pll_status;
>  	int i = 0;
> -	bool srm_lock = false;
> 
>  	switch (event) {
>  	case SND_SOC_DAPM_PRE_PMU:
> @@ -766,15 +765,12 @@ static int da7213_dai_event(struct snd_soc_dapm_widget
> *w,
>  		/* Check SRM has locked */
>  		do {
>  			pll_status = snd_soc_read(codec, DA7213_PLL_STATUS);
> -			if (pll_status & DA7219_PLL_SRM_LOCK) {
> -				srm_lock = true;
> -			} else {
> -				++i;
> -				msleep(50);
> -			}
> -		} while ((i < DA7213_SRM_CHECK_RETRIES) & (!srm_lock));
> +			if (pll_status & DA7219_PLL_SRM_LOCK)
> +				break;
> +			msleep(50);
> +		} while (++i < DA7213_SRM_CHECK_RETRIES);
> 
> -		if (!srm_lock)
> +		if (i >= DA7213_SRM_CHECK_RETRIES)
>  			dev_warn(codec->dev, "SRM failed to lock\n");
> 
>  		return 0;
> --
> 2.9.3

Re: [PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Applied "ASoC: intel: atom: localize variable without external linkage" to the asoc tree

[Mark Brown]

The patch

   ASoC: intel: atom: localize variable without external linkage

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

>From 65db85fba1df213ff80d6f3cbafee244c58f6ec3 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Tue, 2 May 2017 22:33:03 +0900
Subject: [PATCH] ASoC: intel: atom: localize variable without external linkage

A driver for Intel SST driver for old atom platform includes a variable
which has no external linkage. These functions should have static
qualifier.

This commit adds the qualifier to localize the variable. This issue is
detected by sparse:

sst.c:261:1: warning: symbol 'dev_attr_firmware_version' was not declared. Should it be static?

Cc: Sebastien Guiriec <sebastien.guiriec@intel.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/atom/sst/sst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/atom/sst/sst.c b/sound/soc/intel/atom/sst/sst.c
index f9ba71315e33..d97556a3772c 100644
--- a/sound/soc/intel/atom/sst/sst.c
+++ b/sound/soc/intel/atom/sst/sst.c
@@ -258,7 +258,7 @@ static ssize_t firmware_version_show(struct device *dev,
 
 }
 
-DEVICE_ATTR_RO(firmware_version);
+static DEVICE_ATTR_RO(firmware_version);
 
 static const struct attribute *sst_fw_version_attrs[] = {
 	&dev_attr_firmware_version.attr,
-- 
2.11.0

Applied "ASoC: hisilicon: localize functions without external linkage" to the asoc tree

[Mark Brown]

The patch

   ASoC: hisilicon: localize functions without external linkage

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

>From 2a54e845f6e5069666e1749bd952abdc0413910d Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Tue, 2 May 2017 22:33:02 +0900
Subject: [PATCH] ASoC: hisilicon: localize functions without external linkage

A driver for hi6210 sound interface on hi6220 boards includes some
functions which has no external linkage. These functions should have
static qualifier.

This commit adds the qualifier to localize the functions. This issue is
detected by sparse:

hi6210-i2s.c:100:5: warning: symbol 'hi6210_i2s_startup' was not declared. Should it be static?
hi6210-i2s.c:178:6: warning: symbol 'hi6210_i2s_shutdown' was not declared. Should it be static?
hi6210-i2s.c:527:27: warning: symbol 'hi6210_i2s_dai_init' was not declared. Should it be static?

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/hisilicon/hi6210-i2s.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/hisilicon/hi6210-i2s.c b/sound/soc/hisilicon/hi6210-i2s.c
index 45163e5202f5..b193d3beb253 100644
--- a/sound/soc/hisilicon/hi6210-i2s.c
+++ b/sound/soc/hisilicon/hi6210-i2s.c
@@ -97,8 +97,8 @@ static inline u32 hi6210_read_reg(struct hi6210_i2s *i2s, int reg)
 	return readl(i2s->base + reg);
 }
 
-int hi6210_i2s_startup(struct snd_pcm_substream *substream,
-		     struct snd_soc_dai *cpu_dai)
+static int hi6210_i2s_startup(struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *cpu_dai)
 {
 	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
 	int ret, n;
@@ -175,8 +175,9 @@ int hi6210_i2s_startup(struct snd_pcm_substream *substream,
 
 	return 0;
 }
-void hi6210_i2s_shutdown(struct snd_pcm_substream *substream,
-		       struct snd_soc_dai *cpu_dai)
+
+static void hi6210_i2s_shutdown(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *cpu_dai)
 {
 	struct hi6210_i2s *i2s = dev_get_drvdata(cpu_dai->dev);
 	int n;
@@ -524,7 +525,7 @@ static struct snd_soc_dai_ops hi6210_i2s_dai_ops = {
 	.shutdown	= hi6210_i2s_shutdown,
 };
 
-struct snd_soc_dai_driver hi6210_i2s_dai_init = {
+static const struct snd_soc_dai_driver hi6210_i2s_dai_init = {
 	.probe		= hi6210_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-- 
2.11.0

Applied "ASoC: codecs: msm8916: fix invalid cast to bool type" to the asoc tree

[Mark Brown]

The patch

   ASoC: codecs: msm8916: fix invalid cast to bool type

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

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

>From 9f3b777f1de9ff5d17f7259b8f7da5e9d4303e87 Mon Sep 17 00:00:00 2001
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Date: Tue, 2 May 2017 22:33:01 +0900
Subject: [PATCH] ASoC: codecs: msm8916: fix invalid cast to bool type

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")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/msm8916-wcd-analog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c
index d8e8590746af..a78802920c3c 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:
-- 
2.11.0

Re: [PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Takashi Iwai]

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

Re: [PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1149 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/4] ASoC: codecs: msm8916: fix invalid cast to bool type

[Takashi Iwai]

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