2011-04-12 17:37:13

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 1/5] ASoC: Make struct snd_soc_card's dapm_widgets and dapm_routes const

Those should not be modified (and are not) by the core code, so make them const.
This also makes them consistent with the same members of snd_soc_codec.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
include/sound/soc.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 435cb83..cb6b18b 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -755,9 +755,9 @@ struct snd_soc_card {
/*
* Card-specific routes and widgets.
*/
- struct snd_soc_dapm_widget *dapm_widgets;
+ const struct snd_soc_dapm_widget *dapm_widgets;
int num_dapm_widgets;
- struct snd_soc_dapm_route *dapm_routes;
+ const struct snd_soc_dapm_route *dapm_routes;
int num_dapm_routes;

struct work_struct deferred_resume_work;
--
1.7.2.5


2011-04-12 17:37:44

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 4/5] ASoC: JZ4740: qi_lb60: Use gpio_request_array to request and setup gpios

This patch changes the qi_lb60 setup code to use gpio_request_array instead of
manually calling gpio_request and gpio_direction_output for each gpio.
Doing so makes the code a bit more compact.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/soc/jz4740/qi_lb60.c | 29 ++++++++++-------------------
1 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/sound/soc/jz4740/qi_lb60.c b/sound/soc/jz4740/qi_lb60.c
index 875abc9..8c4e84b 100644
--- a/sound/soc/jz4740/qi_lb60.c
+++ b/sound/soc/jz4740/qi_lb60.c
@@ -96,6 +96,11 @@ static struct snd_soc_card qi_lb60 = {

static struct platform_device *qi_lb60_snd_device;

+static const struct gpio qi_lb60_gpios[] = {
+ { QI_LB60_SND_GPIO, GPIOF_OUT_INIT_LOW, "SND" },
+ { QI_LB60_AMP_GPIO, GPIOF_OUT_INIT_LOW, "AMP" },
+};
+
static int __init qi_lb60_init(void)
{
int ret;
@@ -105,23 +110,12 @@ static int __init qi_lb60_init(void)
if (!qi_lb60_snd_device)
return -ENOMEM;

- ret = gpio_request(QI_LB60_SND_GPIO, "SND");
+ ret = gpio_request_array(qi_lb60_gpios, ARRAY_SIZE(qi_lb60_gpios));
if (ret) {
- pr_err("qi_lb60 snd: Failed to request SND GPIO(%d): %d\n",
- QI_LB60_SND_GPIO, ret);
+ pr_err("qi_lb60 snd: Failed to request gpios: %d\n", ret);
goto err_device_put;
}

- ret = gpio_request(QI_LB60_AMP_GPIO, "AMP");
- if (ret) {
- pr_err("qi_lb60 snd: Failed to request AMP GPIO(%d): %d\n",
- QI_LB60_AMP_GPIO, ret);
- goto err_gpio_free_snd;
- }
-
- gpio_direction_output(QI_LB60_SND_GPIO, 0);
- gpio_direction_output(QI_LB60_AMP_GPIO, 0);
-
platform_set_drvdata(qi_lb60_snd_device, &qi_lb60);

ret = platform_device_add(qi_lb60_snd_device);
@@ -134,10 +128,8 @@ static int __init qi_lb60_init(void)

err_unset_pdata:
platform_set_drvdata(qi_lb60_snd_device, NULL);
-/*err_gpio_free_amp:*/
- gpio_free(QI_LB60_AMP_GPIO);
-err_gpio_free_snd:
- gpio_free(QI_LB60_SND_GPIO);
+/*err_gpio_free_array:*/
+ gpio_free_array(qi_lb60_gpios, ARRAY_SIZE(qi_lb60_gpios));
err_device_put:
platform_device_put(qi_lb60_snd_device);

@@ -147,9 +139,8 @@ module_init(qi_lb60_init);

static void __exit qi_lb60_exit(void)
{
- gpio_free(QI_LB60_AMP_GPIO);
- gpio_free(QI_LB60_SND_GPIO);
platform_device_unregister(qi_lb60_snd_device);
+ gpio_free_array(qi_lb60_gpios, ARRAY_SIZE(qi_lb60_gpios));
}
module_exit(qi_lb60_exit);

--
1.7.2.5

2011-04-12 17:37:24

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 2/5] ASoC: Call snd_soc_new_widgets() after registering cards DAPM widgets

After registering new DAPM widgets nd_soc_new_widgets() must be called,
otherwise DAPM for those new widgets will not work.

snd_soc_new_widgets() is placed after the card late_probe call, so cards
can register new DAPM widgets in there.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/soc/soc-core.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f75f139..b6a78d9 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1932,6 +1932,8 @@ static void snd_soc_instantiate_card(struct snd_soc_card *card)
}
}

+ snd_soc_dapm_new_widgets(&card->dapm);
+
ret = snd_card_register(card->snd_card);
if (ret < 0) {
printk(KERN_ERR "asoc: failed to register soundcard for %s\n", card->name);
--
1.7.2.5

2011-04-12 17:37:42

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 3/5] ASoC: JZ4740: Convert qi_lb60 codec to table based DAPM setup

Use the newly introduced dapm_widgets, dpam_routes and fields of the
snd_soc_card struct to setup DAPM.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/soc/jz4740/qi_lb60.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/sound/soc/jz4740/qi_lb60.c b/sound/soc/jz4740/qi_lb60.c
index 49723e3..875abc9 100644
--- a/sound/soc/jz4740/qi_lb60.c
+++ b/sound/soc/jz4740/qi_lb60.c
@@ -70,12 +70,6 @@ static int qi_lb60_codec_init(struct snd_soc_pcm_runtime *rtd)
return ret;
}

- snd_soc_dapm_new_controls(dapm, qi_lb60_widgets,
- ARRAY_SIZE(qi_lb60_widgets));
- snd_soc_dapm_add_routes(dapm, qi_lb60_routes,
- ARRAY_SIZE(qi_lb60_routes));
- snd_soc_dapm_sync(dapm);
-
return 0;
}

@@ -93,6 +87,11 @@ static struct snd_soc_card qi_lb60 = {
.name = "QI LB60",
.dai_link = &qi_lb60_dai,
.num_links = 1,
+
+ .dapm_widgets = qi_lb60_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(qi_lb60_widgets),
+ .dapm_routes = qi_lb60_routes,
+ .num_dapm_routes = ARRAY_SIZE(qi_lb60_routes),
};

static struct platform_device *qi_lb60_snd_device;
--
1.7.2.5

2011-04-12 17:38:00

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 5/5] ASoC: JZ4740: qi_lb60: Use the SND_SOC_DAPM_EVENT_OFF for the speakers status

Use SND_SOC_DAPM_EVENT_OFF for determining whether the speaker should be turned
on or off instead of open coding it.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
sound/soc/jz4740/qi_lb60.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/sound/soc/jz4740/qi_lb60.c b/sound/soc/jz4740/qi_lb60.c
index 8c4e84b..c5fc339 100644
--- a/sound/soc/jz4740/qi_lb60.c
+++ b/sound/soc/jz4740/qi_lb60.c
@@ -27,11 +27,7 @@
static int qi_lb60_spk_event(struct snd_soc_dapm_widget *widget,
struct snd_kcontrol *ctrl, int event)
{
- int on = 0;
- if (event & SND_SOC_DAPM_POST_PMU)
- on = 1;
- else if (event & SND_SOC_DAPM_PRE_PMD)
- on = 0;
+ int on = !SND_SOC_DAPM_EVENT_OFF(event);

gpio_set_value(QI_LB60_SND_GPIO, on);
gpio_set_value(QI_LB60_AMP_GPIO, on);
--
1.7.2.5

2011-04-13 00:38:40

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/5] ASoC: Call snd_soc_new_widgets() after registering cards DAPM widgets

On Tue, Apr 12, 2011 at 07:31:02PM +0200, Lars-Peter Clausen wrote:
> After registering new DAPM widgets nd_soc_new_widgets() must be called,
> otherwise DAPM for those new widgets will not work.
>
> snd_soc_new_widgets() is placed after the card late_probe call, so cards
> can register new DAPM widgets in there.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>

There's already a call for this in post_component_init(), it just needs
to be moved later in the sequence now.

2011-04-13 07:39:10

by Liam Girdwood

[permalink] [raw]
Subject: Re: [alsa-devel] [PATCH 1/5] ASoC: Make struct snd_soc_card's dapm_widgets and dapm_routes const

On Tue, 2011-04-12 at 19:31 +0200, Lars-Peter Clausen wrote:
> Those should not be modified (and are not) by the core code, so make them const.
> This also makes them consistent with the same members of snd_soc_codec.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> include/sound/soc.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 435cb83..cb6b18b 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -755,9 +755,9 @@ struct snd_soc_card {
> /*
> * Card-specific routes and widgets.
> */
> - struct snd_soc_dapm_widget *dapm_widgets;
> + const struct snd_soc_dapm_widget *dapm_widgets;
> int num_dapm_widgets;
> - struct snd_soc_dapm_route *dapm_routes;
> + const struct snd_soc_dapm_route *dapm_routes;
> int num_dapm_routes;
>
> struct work_struct deferred_resume_work;

Patch 1,3,4 & 5

Acked-by: Liam Girdwood <[email protected]>

2011-04-13 12:32:13

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 2/5] ASoC: Call snd_soc_new_widgets() after registering cards DAPM widgets

On 04/13/2011 02:38 AM, Mark Brown wrote:
> On Tue, Apr 12, 2011 at 07:31:02PM +0200, Lars-Peter Clausen wrote:
>> After registering new DAPM widgets nd_soc_new_widgets() must be called,
>> otherwise DAPM for those new widgets will not work.
>>
>> snd_soc_new_widgets() is placed after the card late_probe call, so cards
>> can register new DAPM widgets in there.
>>
>> Signed-off-by: Lars-Peter Clausen <[email protected]>
>
> There's already a call for this in post_component_init(), it just needs
> to be moved later in the sequence now.

post_component_init() is called for each registered codec (and auxdev).
Since all the DAPM widgets are kept in one list now it probably makes sense to
traverse that list only once, when all components have been registered.
But snd_soc_dapm_new_widgets also calls dapm_power_widgets which operates on
the passed dapm_context, will it be without any effect on the DAPM context,
when it is called on a newly allocated context?

Otherwise I would say call snd_soc_dapm_new_widgets once for the cards
dapm_context and snd_soc_dapm_sync for each codec context.

- Lars

2011-04-13 17:32:40

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/5] ASoC: Call snd_soc_new_widgets() after registering cards DAPM widgets

On Wed, Apr 13, 2011 at 02:32:35PM +0200, Lars-Peter Clausen wrote:

> But snd_soc_dapm_new_widgets also calls dapm_power_widgets which operates on
> the passed dapm_context, will it be without any effect on the DAPM context,
> when it is called on a newly allocated context?

Yup.

> Otherwise I would say call snd_soc_dapm_new_widgets once for the cards
> dapm_context and snd_soc_dapm_sync for each codec context.

There's no point in repeatedly syncing - it may cause audible issues if
we power things off due to incomplete information. If the sync isn't
propagating over the entire system we should fix that.

2011-04-13 17:35:51

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/5] ASoC: Make struct snd_soc_card's dapm_widgets and dapm_routes const

On Tue, Apr 12, 2011 at 07:31:01PM +0200, Lars-Peter Clausen wrote:
> Those should not be modified (and are not) by the core code, so make them const.
> This also makes them consistent with the same members of snd_soc_codec.
>
> Signed-off-by: Lars-Peter Clausen <[email protected]>

Applied 1, 3-5.

2011-04-13 19:26:31

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [PATCH 2/5] ASoC: Call snd_soc_new_widgets() after registering cards DAPM widgets

On 04/13/2011 07:32 PM, Mark Brown wrote:
> On Wed, Apr 13, 2011 at 02:32:35PM +0200, Lars-Peter Clausen wrote:
>
>> But snd_soc_dapm_new_widgets also calls dapm_power_widgets which operates on
>> the passed dapm_context, will it be without any effect on the DAPM context,
>> when it is called on a newly allocated context?
>
> Yup.

Ok. I'll add a snd_soc_dapm_init_widgets(struct snd_soc_card *card) which will
basically do what snd_soc_dapm_new_widgets does except for calling
dapm_power_widgets.


>
>> Otherwise I would say call snd_soc_dapm_new_widgets once for the cards
>> dapm_context and snd_soc_dapm_sync for each codec context.
>
> There's no point in repeatedly syncing - it may cause audible issues if
> we power things off due to incomplete information. If the sync isn't
> propagating over the entire system we should fix that.

The dapm context is really only used to see if it is widget-less context. In
which case the event type is used to see whether it should be powered or not.
But I wonder if that shouldn't really be done for all widget less contexts.
Since widget-less contexts dev_power is not set to 0 at the beginning of
dapm_power_widgets and all dapm contexts are forced into the same power state,
effectively that cause a system with a widget-less context to be stuck in a
powered state.
Since right now the cards dapm context is almost always widget-less this would
affect almost all systems.
This is what happens on such a system:
Playback starts, codec is powered up, card dapm context is forced to on,
playback stops, codec is powered down but card context is still on and will
force the codec on again.

And it is also used for passing the update struct around. But is there any
reason as to why to pass as a member of the dapm context instead of as a simple
parameter for dapm_power_widgets()? If not I would like to change it to the latter.

- Lars