[alsa-devel] [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 lars@metafoo.de --- 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;
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 lars@metafoo.de --- 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);
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 lars@metafoo.de
There's already a call for this in post_component_init(), it just needs to be moved later in the sequence now.
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 lars@metafoo.de
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
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.
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
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 lars@metafoo.de --- 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;
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 lars@metafoo.de --- 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);
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 lars@metafoo.de --- 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);
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 lars@metafoo.de
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 lrg@ti.com
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 lars@metafoo.de
Applied 1, 3-5.
participants (3)
-
Lars-Peter Clausen -
Liam Girdwood -
Mark Brown