* [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio
2011-12-04 11:35 [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Axel Lin
@ 2011-12-04 11:37 ` Axel Lin
2011-12-04 13:02 ` Mark Brown
2011-12-04 18:55 ` Vasily
2011-12-04 11:38 ` [PATCH 3/3] ASoC: uda1380: Convert to gpio_request_one() Axel Lin
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Axel Lin @ 2011-12-04 11:37 UTC (permalink / raw)
To: alsa-devel; +Cc: Philipp Zabel, Mark Brown, Liam Girdwood, Lars-Peter Clausen
Current code does not call uda1380_reset() in uda1380_probe() if
pdata->gpio_power is a valid gpio. Fix it.
Also remove a redundant "Failed to issue reset" error message.
We already show error message in uda1380_reset() error path.
Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
I don't has this hardware, I'd appreciate if someone can test this patch.
Axel
sound/soc/codecs/uda1380.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/uda1380.c b/sound/soc/codecs/uda1380.c
index 3a238cb..7ebfbeb 100644
--- a/sound/soc/codecs/uda1380.c
+++ b/sound/soc/codecs/uda1380.c
@@ -747,14 +747,12 @@ static int uda1380_probe(struct snd_soc_codec *codec)
ret = gpio_direction_output(pdata->gpio_power, 0);
if (ret)
goto err_gpio_power_conf;
- } else {
- ret = uda1380_reset(codec);
- if (ret) {
- dev_err(codec->dev, "Failed to issue reset\n");
- goto err_reset;
- }
}
+ ret = uda1380_reset(codec);
+ if (ret)
+ goto err_reset;
+
INIT_WORK(&uda1380->work, uda1380_flush_work);
/* power on device */
--
1.7.5.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio
2011-12-04 11:37 ` [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio Axel Lin
@ 2011-12-04 13:02 ` Mark Brown
2011-12-04 18:55 ` Vasily
1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2011-12-04 13:02 UTC (permalink / raw)
To: Axel Lin; +Cc: alsa-devel, Lars-Peter Clausen, Liam Girdwood, Philipp Zabel
On Sun, Dec 04, 2011 at 07:37:32PM +0800, Axel Lin wrote:
> Current code does not call uda1380_reset() in uda1380_probe() if
> pdata->gpio_power is a valid gpio. Fix it.
This doesn't look like a problem - what's happening is that when the
driver has control over the CODEC power it's powering it off which has
the effect of restoring the device to the power on defaults. It only
needs to issue a reset if the device is powered up.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio
2011-12-04 11:37 ` [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio Axel Lin
2011-12-04 13:02 ` Mark Brown
@ 2011-12-04 18:55 ` Vasily
1 sibling, 0 replies; 9+ messages in thread
From: Vasily @ 2011-12-04 18:55 UTC (permalink / raw)
To: Axel Lin
Cc: alsa-devel, Mark Brown, Liam Girdwood, Philipp Zabel,
Lars-Peter Clausen
2011/12/4 Axel Lin <axel.lin@gmail.com>:
> Current code does not call uda1380_reset() in uda1380_probe() if
> pdata->gpio_power is a valid gpio. Fix it.
>
> Also remove a redundant "Failed to issue reset" error message.
> We already show error message in uda1380_reset() error path.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
NAK. Issuing reset command when codec is powered off does not look as
a sane thing
to me.
> ---
> I don't has this hardware, I'd appreciate if someone can test this patch.
> Axel
> sound/soc/codecs/uda1380.c | 10 ++++------
> 1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/codecs/uda1380.c b/sound/soc/codecs/uda1380.c
> index 3a238cb..7ebfbeb 100644
> --- a/sound/soc/codecs/uda1380.c
> +++ b/sound/soc/codecs/uda1380.c
> @@ -747,14 +747,12 @@ static int uda1380_probe(struct snd_soc_codec *codec)
> ret = gpio_direction_output(pdata->gpio_power, 0);
> if (ret)
> goto err_gpio_power_conf;
> - } else {
> - ret = uda1380_reset(codec);
> - if (ret) {
> - dev_err(codec->dev, "Failed to issue reset\n");
> - goto err_reset;
> - }
> }
>
> + ret = uda1380_reset(codec);
> + if (ret)
> + goto err_reset;
> +
> INIT_WORK(&uda1380->work, uda1380_flush_work);
>
> /* power on device */
> --
> 1.7.5.4
>
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] ASoC: uda1380: Convert to gpio_request_one()
2011-12-04 11:35 [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Axel Lin
2011-12-04 11:37 ` [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio Axel Lin
@ 2011-12-04 11:38 ` Axel Lin
2011-12-04 23:46 ` Mark Brown
2011-12-04 13:00 ` [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Mark Brown
2011-12-04 23:39 ` Mark Brown
3 siblings, 1 reply; 9+ messages in thread
From: Axel Lin @ 2011-12-04 11:38 UTC (permalink / raw)
To: alsa-devel; +Cc: Philipp Zabel, Mark Brown, Liam Girdwood, Lars-Peter Clausen
Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
sound/soc/codecs/uda1380.c | 14 ++++----------
1 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/sound/soc/codecs/uda1380.c b/sound/soc/codecs/uda1380.c
index 7ebfbeb..17ae60f 100644
--- a/sound/soc/codecs/uda1380.c
+++ b/sound/soc/codecs/uda1380.c
@@ -732,21 +732,17 @@ static int uda1380_probe(struct snd_soc_codec *codec)
return -EINVAL;
if (gpio_is_valid(pdata->gpio_reset)) {
- ret = gpio_request(pdata->gpio_reset, "uda1380 reset");
+ ret = gpio_request_one(pdata->gpio_reset, GPIOF_OUT_INIT_LOW,
+ "uda1380 reset");
if (ret)
goto err_out;
- ret = gpio_direction_output(pdata->gpio_reset, 0);
- if (ret)
- goto err_gpio_reset_conf;
}
if (gpio_is_valid(pdata->gpio_power)) {
- ret = gpio_request(pdata->gpio_power, "uda1380 power");
+ ret = gpio_request_one(pdata->gpio_power, GPIOF_OUT_INIT_LOW,
+ "uda1380 power");
if (ret)
goto err_gpio;
- ret = gpio_direction_output(pdata->gpio_power, 0);
- if (ret)
- goto err_gpio_power_conf;
}
ret = uda1380_reset(codec);
@@ -775,11 +771,9 @@ static int uda1380_probe(struct snd_soc_codec *codec)
return 0;
err_reset:
-err_gpio_power_conf:
if (gpio_is_valid(pdata->gpio_power))
gpio_free(pdata->gpio_power);
-err_gpio_reset_conf:
err_gpio:
if (gpio_is_valid(pdata->gpio_reset))
gpio_free(pdata->gpio_reset);
--
1.7.5.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path
2011-12-04 11:35 [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Axel Lin
2011-12-04 11:37 ` [PATCH 2/3] ASoC: uda1380: Fix missed uda1380_reset call if pdata->gpio_power is a valid gpio Axel Lin
2011-12-04 11:38 ` [PATCH 3/3] ASoC: uda1380: Convert to gpio_request_one() Axel Lin
@ 2011-12-04 13:00 ` Mark Brown
2011-12-04 13:44 ` Axel Lin
2011-12-04 23:39 ` Mark Brown
3 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2011-12-04 13:00 UTC (permalink / raw)
To: Axel Lin; +Cc: alsa-devel, Lars-Peter Clausen, Liam Girdwood, Philipp Zabel
On Sun, Dec 04, 2011 at 07:35:20PM +0800, Axel Lin wrote:
> + int ret = 0;
> #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
> ret = i2c_add_driver(&uda1380_i2c_driver);
> if (ret != 0)
> pr_err("Failed to register UDA1380 I2C driver: %d\n", ret);
> #endif
> - return 0;
> + return ret;
In cases like this where only I2C is supported it's best to just remove
all the ifdefs for I2C completely.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path
2011-12-04 13:00 ` [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Mark Brown
@ 2011-12-04 13:44 ` Axel Lin
0 siblings, 0 replies; 9+ messages in thread
From: Axel Lin @ 2011-12-04 13:44 UTC (permalink / raw)
To: Mark Brown; +Cc: alsa-devel, Lars-Peter Clausen, Liam Girdwood, Philipp Zabel
2011/12/4 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Sun, Dec 04, 2011 at 07:35:20PM +0800, Axel Lin wrote:
>
>> + int ret = 0;
>> #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
>> ret = i2c_add_driver(&uda1380_i2c_driver);
>> if (ret != 0)
>> pr_err("Failed to register UDA1380 I2C driver: %d\n", ret);
>> #endif
>> - return 0;
>> + return ret;
>
> In cases like this where only I2C is supported it's best to just remove
> all the ifdefs for I2C completely.
The datasheet says:
The UDA1380 supports I2C-bus microcontroller interface
mode as well as the L3-bus mode.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path
2011-12-04 11:35 [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Axel Lin
` (2 preceding siblings ...)
2011-12-04 13:00 ` [PATCH 1/3] ASoC: uda1380: Return proper error in uda1380_modinit failure path Mark Brown
@ 2011-12-04 23:39 ` Mark Brown
3 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2011-12-04 23:39 UTC (permalink / raw)
To: Axel Lin; +Cc: alsa-devel, Lars-Peter Clausen, Liam Girdwood, Philipp Zabel
On Sun, Dec 04, 2011 at 07:35:20PM +0800, Axel Lin wrote:
> Return proper error for uda1380_modinit if i2c_add_driver() fails.
Applied, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread