All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree
@ 2016-03-15 12:06 Takashi Sakamoto
  2016-03-15 12:06 ` Takashi Sakamoto
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Sakamoto @ 2016-03-15 12:06 UTC (permalink / raw)
  To: brian.austin, broonie; +Cc: alsa-devel

Hi,

When compiling recent kernel, I got a warning caused by a codec in ASoC.

sound/soc/codecs/cs35l32.c: In function ‘cs35l32_i2c_probe’:
sound/soc/codecs/cs35l32.c:278:2: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  switch (val) {
  ^
sound/soc/codecs/cs35l32.c:272:15: note: ‘val’ was declared here
  unsigned int val;
               ^

I'm not an ASoC developer, furthermore, have no knowledge about this Cirrus
codec. Although, as a quick glance it may include a logical bug to assign wrong
information to structure member as a reult of parsing device tree.

This patch is my proposal to fix it. I'm happy to receive your comments.

Regards


Takashi Sakamoto (1):
  ASoC: cs35l32: fix unexpected parse result of device tree blob

 sound/soc/codecs/cs35l32.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.7.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree
  2016-03-15 12:06 [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree Takashi Sakamoto
@ 2016-03-15 12:06 ` Takashi Sakamoto
  2016-03-15 14:30   ` Mark Brown
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Sakamoto @ 2016-03-15 12:06 UTC (permalink / raw)
  To: brian.austin, broonie; +Cc: alsa-devel

When device tree blob has no node properties requested by callers,
of_property_read_u32() returns minus value, then given argument is
not changed.

In current code of cs35l32 codec, the function is used with no condition
statement for evaluation of the return value. Thus, the value of 'val'
argument can be evaluated in following switch statement without
initialization or assignment.

This causes a bug that the property retrieved by previous function calls
can be evaluated in later switch statements when some properties are
missing.

This commit fixes the bug. Unfortunately, these switch statements include
cases with zero value. I initialize the argument with maximum number for
argument type so that it apparently matches default cases and generates
debug messages when missing.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/soc/codecs/cs35l32.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/cs35l32.c b/sound/soc/codecs/cs35l32.c
index 44c30fe..e464107 100644
--- a/sound/soc/codecs/cs35l32.c
+++ b/sound/soc/codecs/cs35l32.c
@@ -274,6 +274,7 @@ static int cs35l32_handle_of_data(struct i2c_client *i2c_client,
 	if (of_property_read_u32(np, "cirrus,sdout-share", &val) >= 0)
 		pdata->sdout_share = val;
 
+	val = UINT_MAX;
 	of_property_read_u32(np, "cirrus,boost-manager", &val);
 	switch (val) {
 	case CS35L32_BOOST_MGR_AUTO:
@@ -288,6 +289,7 @@ static int cs35l32_handle_of_data(struct i2c_client *i2c_client,
 		pdata->boost_mng = CS35L32_BOOST_MGR_BYPASS;
 	}
 
+	val = UINT_MAX;
 	of_property_read_u32(np, "cirrus,sdout-datacfg", &val);
 	switch (val) {
 	case CS35L32_DATA_CFG_LR_VP:
@@ -302,6 +304,7 @@ static int cs35l32_handle_of_data(struct i2c_client *i2c_client,
 		pdata->sdout_datacfg = CS35L32_DATA_CFG_LR;
 	}
 
+	val = UINT_MAX;
 	of_property_read_u32(np, "cirrus,battery-threshold", &val);
 	switch (val) {
 	case CS35L32_BATT_THRESH_3_1V:
@@ -316,6 +319,7 @@ static int cs35l32_handle_of_data(struct i2c_client *i2c_client,
 		pdata->batt_thresh = CS35L32_BATT_THRESH_3_3V;
 	}
 
+	val = UINT_MAX;
 	of_property_read_u32(np, "cirrus,battery-recovery", &val);
 	switch (val) {
 	case CS35L32_BATT_RECOV_3_1V:
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree
  2016-03-15 12:06 ` Takashi Sakamoto
@ 2016-03-15 14:30   ` Mark Brown
  2016-03-16  3:51     ` Takashi Sakamoto
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2016-03-15 14:30 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: brian.austin, alsa-devel


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

On Tue, Mar 15, 2016 at 09:06:14PM +0900, Takashi Sakamoto wrote:
> When device tree blob has no node properties requested by callers,
> of_property_read_u32() returns minus value, then given argument is
> not changed.

This is probably fixed by a recent patch, please check with current code.

Please don't send cover latters for single patches, if they're needed
they indicate that you need to improve the commit log for the change.

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

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree
  2016-03-15 14:30   ` Mark Brown
@ 2016-03-16  3:51     ` Takashi Sakamoto
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2016-03-16  3:51 UTC (permalink / raw)
  To: Mark Brown; +Cc: brian.austin, alsa-devel

On Mar 15 2016 23:30, Mark Brown wrote:
> On Tue, Mar 15, 2016 at 09:06:14PM +0900, Takashi Sakamoto wrote:
>> When device tree blob has no node properties requested by callers,
>> of_property_read_u32() returns minus value, then given argument is
>> not changed.
>
> This is probably fixed by a recent patch, please check with current code.

Aha, exactly. I should have sought mailing list archive.

http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105419.html

> Please don't send cover latters for single patches, if they're needed
> they indicate that you need to improve the commit log for the change.

I used for this purpose. Anyway, OK.


Thanks

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-03-16  3:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-15 12:06 [PATCH] ASoC: cs35l32: prevent unexpected parse result of device tree Takashi Sakamoto
2016-03-15 12:06 ` Takashi Sakamoto
2016-03-15 14:30   ` Mark Brown
2016-03-16  3:51     ` Takashi Sakamoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.