[alsa-devel] [PATCH 1/2] ASoC: uda134x: replace a macro with a value in platform struct.
This change wipes out a hardcoded macro, which enables codec bias level control. Now is_powered_on_standby value shall be used instead.
Signed-off-by: Vladimir Zapolskiy vzapolskiy@gmail.com --- include/sound/uda134x.h | 12 ++++++++++++ sound/soc/codecs/uda134x.c | 21 +++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/include/sound/uda134x.h b/include/sound/uda134x.h index 509efb0..e475659 100644 --- a/include/sound/uda134x.h +++ b/include/sound/uda134x.h @@ -18,6 +18,18 @@ struct uda134x_platform_data { struct l3_pins l3; void (*power) (int); int model; + /* + ALSA SOC usually puts the device in standby mode when it's not used + for sometime. If you unset is_powered_on_standby the driver will + turn off the ADC/DAC when this callback is invoked and turn it back + on when needed. Unfortunately this will result in a very light bump + (it can be audible only with good earphones). If this bothers you + set is_powered_on_standby, you will have slightly higher power + consumption. Please note that sending the L3 command for ADC is + enough to make the bump, so it doesn't make difference if you + completely take off power from the codec. + */ + int is_powered_on_standby; #define UDA134X_UDA1340 1 #define UDA134X_UDA1341 2 #define UDA134X_UDA1344 3 diff --git a/sound/soc/codecs/uda134x.c b/sound/soc/codecs/uda134x.c index e8066b6..5c7b054 100644 --- a/sound/soc/codecs/uda134x.c +++ b/sound/soc/codecs/uda134x.c @@ -27,19 +27,6 @@ #include "uda134x.h"
-#define POWER_OFF_ON_STANDBY 1 -/* - ALSA SOC usually puts the device in standby mode when it's not used - for sometime. If you define POWER_OFF_ON_STANDBY the driver will - turn off the ADC/DAC when this callback is invoked and turn it back - on when needed. Unfortunately this will result in a very light bump - (it can be audible only with good earphones). If this bothers you - just comment this line, you will have slightly higher power - consumption . Please note that sending the L3 command for ADC is - enough to make the bump, so it doesn't make difference if you - completely take off power from the codec. - */ - #define UDA134X_RATES SNDRV_PCM_RATE_8000_48000 #define UDA134X_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | \ SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S20_3LE) @@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY - codec->set_bias_level = uda134x_set_bias_level; -#endif + + if (!pd->is_powered_on_standby) { + codec->set_bias_level = uda134x_set_bias_level; + } + INIT_LIST_HEAD(&codec->dapm_widgets); INIT_LIST_HEAD(&codec->dapm_paths);
On initialization ADC/DAC are enabled only for UDA1341, that's why bias_level shall be set to off explicitly, otherwise dapm is misinformed about bias_level on startup.
Signed-off-by: Vladimir Zapolskiy vzapolskiy@gmail.com --- sound/soc/codecs/uda134x.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/uda134x.c b/sound/soc/codecs/uda134x.c index 5c7b054..ed13515 100644 --- a/sound/soc/codecs/uda134x.c +++ b/sound/soc/codecs/uda134x.c @@ -547,10 +547,6 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->read = uda134x_read_reg_cache; codec->write = uda134x_write;
- if (!pd->is_powered_on_standby) { - codec->set_bias_level = uda134x_set_bias_level; - } - INIT_LIST_HEAD(&codec->dapm_widgets); INIT_LIST_HEAD(&codec->dapm_paths);
@@ -561,6 +557,14 @@ static int uda134x_soc_probe(struct platform_device *pdev)
uda134x_reset(codec);
+ if (pd->is_powered_on_standby) { + codec->set_bias_level = NULL; + uda134x_set_bias_level(codec, SND_SOC_BIAS_ON); + } else { + codec->set_bias_level = uda134x_set_bias_level; + uda134x_set_bias_level(codec, SND_SOC_BIAS_STANDBY); + } + /* register pcms */ ret = snd_soc_new_pcms(socdev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1); if (ret < 0) {
On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
This change wipes out a hardcoded macro, which enables codec bias level control. Now is_powered_on_standby value shall be used instead.
Signed-off-by: Vladimir Zapolskiy vzapolskiy@gmail.com
include/sound/uda134x.h | 12 ++++++++++++ sound/soc/codecs/uda134x.c | 21 +++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/include/sound/uda134x.h b/include/sound/uda134x.h index 509efb0..e475659 100644 --- a/include/sound/uda134x.h +++ b/include/sound/uda134x.h @@ -18,6 +18,18 @@ struct uda134x_platform_data { struct l3_pins l3; void (*power) (int); int model;
- /*
ALSA SOC usually puts the device in standby mode when it's not usedfor sometime. If you unset is_powered_on_standby the driver willturn off the ADC/DAC when this callback is invoked and turn it backon when needed. Unfortunately this will result in a very light bump(it can be audible only with good earphones). If this bothers youset is_powered_on_standby, you will have slightly higher powerconsumption. Please note that sending the L3 command for ADC isenough to make the bump, so it doesn't make difference if youcompletely take off power from the codec.- */
- int is_powered_on_standby;
#define UDA134X_UDA1340 1 #define UDA134X_UDA1341 2 #define UDA134X_UDA1344 3 diff --git a/sound/soc/codecs/uda134x.c b/sound/soc/codecs/uda134x.c index e8066b6..5c7b054 100644 --- a/sound/soc/codecs/uda134x.c +++ b/sound/soc/codecs/uda134x.c @@ -27,19 +27,6 @@ #include "uda134x.h"
-#define POWER_OFF_ON_STANDBY 1 -/*
- ALSA SOC usually puts the device in standby mode when it's not used
- for sometime. If you define POWER_OFF_ON_STANDBY the driver will
- turn off the ADC/DAC when this callback is invoked and turn it back
- on when needed. Unfortunately this will result in a very light bump
- (it can be audible only with good earphones). If this bothers you
- just comment this line, you will have slightly higher power
- consumption . Please note that sending the L3 command for ADC is
- enough to make the bump, so it doesn't make difference if you
- completely take off power from the codec.
- */
#define UDA134X_RATES SNDRV_PCM_RATE_8000_48000 #define UDA134X_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | \ SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S20_3LE) @@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY
- codec->set_bias_level = uda134x_set_bias_level;
-#endif
- if (!pd->is_powered_on_standby) {
codec->set_bias_level = uda134x_set_bias_level;
The codec ops pointers have moved in multi-component. Can you put this check in your uda134x_set_bias_level() instead.
Thanks
Liam
Hi,
On Thu, Jun 24, 2010 at 5:49 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
This change wipes out a hardcoded macro, which enables codec bias level control. Now is_powered_on_standby value shall be used instead.
Signed-off-by: Vladimir Zapolskiy vzapolskiy@gmail.com
include/sound/uda134x.h | 12 ++++++++++++ sound/soc/codecs/uda134x.c | 21 +++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/include/sound/uda134x.h b/include/sound/uda134x.h index 509efb0..e475659 100644 --- a/include/sound/uda134x.h +++ b/include/sound/uda134x.h @@ -18,6 +18,18 @@ struct uda134x_platform_data { struct l3_pins l3; void (*power) (int); int model;
- /*
- ALSA SOC usually puts the device in standby mode when it's not used
- for sometime. If you unset is_powered_on_standby the driver will
- turn off the ADC/DAC when this callback is invoked and turn it back
- on when needed. Unfortunately this will result in a very light bump
- (it can be audible only with good earphones). If this bothers you
- set is_powered_on_standby, you will have slightly higher power
- consumption. Please note that sending the L3 command for ADC is
- enough to make the bump, so it doesn't make difference if you
- completely take off power from the codec.
- */
- int is_powered_on_standby;
#define UDA134X_UDA1340 1 #define UDA134X_UDA1341 2 #define UDA134X_UDA1344 3 diff --git a/sound/soc/codecs/uda134x.c b/sound/soc/codecs/uda134x.c index e8066b6..5c7b054 100644 --- a/sound/soc/codecs/uda134x.c +++ b/sound/soc/codecs/uda134x.c @@ -27,19 +27,6 @@ #include "uda134x.h"
-#define POWER_OFF_ON_STANDBY 1 -/*
- ALSA SOC usually puts the device in standby mode when it's not used
- for sometime. If you define POWER_OFF_ON_STANDBY the driver will
- turn off the ADC/DAC when this callback is invoked and turn it back
- on when needed. Unfortunately this will result in a very light bump
- (it can be audible only with good earphones). If this bothers you
- just comment this line, you will have slightly higher power
- consumption . Please note that sending the L3 command for ADC is
- enough to make the bump, so it doesn't make difference if you
- completely take off power from the codec.
- */
#define UDA134X_RATES SNDRV_PCM_RATE_8000_48000 #define UDA134X_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | \ SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S20_3LE) @@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY
- codec->set_bias_level = uda134x_set_bias_level;
-#endif
- if (!pd->is_powered_on_standby) {
- codec->set_bias_level = uda134x_set_bias_level;
The codec ops pointers have moved in multi-component. Can you put this check in your uda134x_set_bias_level() instead.
Not sure I understand the problem, and I do not see a simple solution for actual bugfix in patch 2/2, if this check is placed in uda134x_set_bias_level(). Please could you provide me with a hint or link to some codec source file?
Thank you, Vladimir
On Thu, 2010-06-24 at 19:54 +0400, Vladimir Zapolskiy wrote:
Hi,
On Thu, Jun 24, 2010 at 5:49 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
@@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY
codec->set_bias_level = uda134x_set_bias_level;-#endif
if (!pd->is_powered_on_standby) {codec->set_bias_level = uda134x_set_bias_level;The codec ops pointers have moved in multi-component. Can you put this check in your uda134x_set_bias_level() instead.
Not sure I understand the problem, and I do not see a simple solution for actual bugfix in patch 2/2, if this check is placed in uda134x_set_bias_level(). Please could you provide me with a hint or link to some codec source file?
I was meaning something like :-
uda134x_set_bias_level() { if (pd->is_powered_on_standby) return
/* rest of func */
}
The multi-component branch makes codec->set_bias_level a driver level operation (that can be shared amongst other uda134x codec devices so we cant change this at runtime)
Thanks
Liam
On Thu, Jun 24, 2010 at 8:05 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 19:54 +0400, Vladimir Zapolskiy wrote:
Hi,
On Thu, Jun 24, 2010 at 5:49 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
@@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY
- codec->set_bias_level = uda134x_set_bias_level;
-#endif
- if (!pd->is_powered_on_standby) {
- codec->set_bias_level = uda134x_set_bias_level;
The codec ops pointers have moved in multi-component. Can you put this check in your uda134x_set_bias_level() instead.
Not sure I understand the problem, and I do not see a simple solution for actual bugfix in patch 2/2, if this check is placed in uda134x_set_bias_level(). Please could you provide me with a hint or link to some codec source file?
I was meaning something like :-
uda134x_set_bias_level() { if (pd->is_powered_on_standby) return
/* rest of func */
}
Thank you, good that I meant the same. But that makes quite impossible to set a mandatory bias level on initialization without additional code complication.
The multi-component branch makes codec->set_bias_level a driver level operation (that can be shared amongst other uda134x codec devices so we cant change this at runtime)
Looks like I miss something, because in codec drivers I observe codec->set_bias_level is set in probe function, like it is in uda134x.
Would it be better just to drop a condition of pd->is_powered_on_standby? What do you think?
Vladimir
On Thu, 2010-06-24 at 20:21 +0400, Vladimir Zapolskiy wrote:
On Thu, Jun 24, 2010 at 8:05 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 19:54 +0400, Vladimir Zapolskiy wrote:
Hi,
On Thu, Jun 24, 2010 at 5:49 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Thu, 2010-06-24 at 17:38 +0400, Vladimir Zapolskiy wrote:
@@ -559,9 +546,11 @@ static int uda134x_soc_probe(struct platform_device *pdev) codec->num_dai = 1; codec->read = uda134x_read_reg_cache; codec->write = uda134x_write; -#ifdef POWER_OFF_ON_STANDBY
codec->set_bias_level = uda134x_set_bias_level;-#endif
if (!pd->is_powered_on_standby) {codec->set_bias_level = uda134x_set_bias_level;The codec ops pointers have moved in multi-component. Can you put this check in your uda134x_set_bias_level() instead.
Not sure I understand the problem, and I do not see a simple solution for actual bugfix in patch 2/2, if this check is placed in uda134x_set_bias_level(). Please could you provide me with a hint or link to some codec source file?
I was meaning something like :-
uda134x_set_bias_level() { if (pd->is_powered_on_standby) return
/* rest of func */}
Thank you, good that I meant the same. But that makes quite impossible to set a mandatory bias level on initialization without additional code complication.
Sorry, I'm just trying to make the multi-component merge simpler here.
The multi-component branch is here :-
http://git.kernel.org/?p=linux/kernel/git/lrg/asoc-2.6.git;a=shortlog;h=refs...
I'll merge this myself later.
The multi-component branch makes codec->set_bias_level a driver level operation (that can be shared amongst other uda134x codec devices so we cant change this at runtime)
Looks like I miss something, because in codec drivers I observe codec->set_bias_level is set in probe function, like it is in uda134x.
Would it be better just to drop a condition of pd->is_powered_on_standby? What do you think?
Both looks fine.
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Thu, Jun 24, 2010 at 05:38:50PM +0400, Vladimir Zapolskiy wrote:
This change wipes out a hardcoded macro, which enables codec bias level control. Now is_powered_on_standby value shall be used instead.
Signed-off-by: Vladimir Zapolskiy vzapolskiy@gmail.com
Applied both, thanks. Like I say, the first patch really should be done by using DAPM rather than this custom code but it's still an improvement on the existing code.
participants (3)
-
Liam Girdwood -
Mark Brown -
Vladimir Zapolskiy