[alsa-devel] [PATCH 0/2 v2] ASoC: Component has suspend/resume support
Hi Mark, Lars-Peter
These patches are v2 of Component level suspend/resume support.
Current Card has has aux_comp_list and codec_dev_list. Now, this patch will add component_dev_list. Here, we can simply replace codec_dev_list -> component_dev_list. and we can replace aux_comp_list by component_dev_list and new flags.
V1 patchset fixuped Intel's broadwell and cht_bsw_rt5672 driver which is using codec_dev_list directly to uses component pointer instead of codec, but v2 keeps it as-is, just only replaced codec_dev_list to component_dev_list. These drivers are better to update in accordance with Lars's idea
Kuninori Morimoto (2): 1) ASoC: core: add component_dev_list on Card 2) ASoC: add Component level suspend/resume
include/sound/soc.h | 16 +++++++++------- sound/soc/intel/boards/broadwell.c | 10 ++++++---- sound/soc/intel/boards/cht_bsw_rt5672.c | 10 ++++++---- sound/soc/soc-core.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------- 4 files changed, 77 insertions(+), 49 deletions(-)
Best regards --- Kuninori Morimoto
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current Card has Codec list (= codec_dev_list), but Codec will be removed in the future. Because of this reason, this patch adds new Component list in Card. In the same time, current Card has AUX list (= aux_comp_list). This patch replaces it by Component list and new flag (= has_auxliary) too
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 7 ++++--- sound/soc/soc-core.c | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 1ed9371..69f2ebf 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -807,9 +807,10 @@ struct snd_soc_component {
unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ unsigned int registered_as_component:1; + unsigned int has_auxiliary:1; /* for auxiliary component of the card */
struct list_head list; - struct list_head list_aux; /* for auxiliary component of the card */ + struct list_head card_list;
struct snd_soc_dai_driver *dai_drv; int num_dai; @@ -1148,7 +1149,6 @@ struct snd_soc_card { */ struct snd_soc_aux_dev *aux_dev; int num_aux_devs; - struct list_head aux_comp_list;
const struct snd_kcontrol_new *controls; int num_controls; @@ -1171,6 +1171,7 @@ struct snd_soc_card {
/* lists of probed devices belonging to this card */ struct list_head codec_dev_list; + struct list_head component_dev_list;
struct list_head widgets; struct list_head paths; @@ -1544,7 +1545,7 @@ static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) INIT_LIST_HEAD(&card->widgets); INIT_LIST_HEAD(&card->paths); INIT_LIST_HEAD(&card->dapm_list); - INIT_LIST_HEAD(&card->aux_comp_list); + INIT_LIST_HEAD(&card->component_dev_list); }
static inline bool snd_soc_volsw_is_stereo(struct soc_mixer_control *mc) diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index c0bbcd9..2a00b5e 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1076,6 +1076,8 @@ static void soc_remove_component(struct snd_soc_component *component) if (component->codec) list_del(&component->codec->card_list);
+ list_del(&component->card_list); + if (component->remove) component->remove(component);
@@ -1448,6 +1450,8 @@ static int soc_probe_component(struct snd_soc_card *card, if (component->codec) list_add(&component->codec->card_list, &card->codec_dev_list);
+ list_add(&component->card_list, &card->component_dev_list); + return 0;
err_probe: @@ -1706,7 +1710,8 @@ static int soc_bind_aux_dev(struct snd_soc_card *card, int num) }
component->init = aux_dev->init; - list_add(&component->list_aux, &card->aux_comp_list); + component->has_auxiliary = 1; + return 0;
err_defer: @@ -1722,7 +1727,10 @@ static int soc_probe_aux_devices(struct snd_soc_card *card)
for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { - list_for_each_entry(comp, &card->aux_comp_list, list_aux) { + list_for_each_entry(comp, &card->component_dev_list, card_list) { + if (!comp->has_auxiliary) + continue; + if (comp->driver->probe_order == order) { ret = soc_probe_component(card, comp); if (ret < 0) { @@ -1746,11 +1754,14 @@ static void soc_remove_aux_devices(struct snd_soc_card *card) for (order = SND_SOC_COMP_ORDER_FIRST; order <= SND_SOC_COMP_ORDER_LAST; order++) { list_for_each_entry_safe(comp, _comp, - &card->aux_comp_list, list_aux) { + &card->component_dev_list, card_list) { + + if (!comp->has_auxiliary) + continue; + if (comp->driver->remove_order == order) { soc_remove_component(comp); - /* remove it from the card's aux_comp_list */ - list_del(&comp->list_aux); + comp->has_auxiliary = 0; } } }
On 10/25/2016 03:20 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current Card has Codec list (= codec_dev_list), but Codec will be removed in the future. Because of this reason, this patch adds new Component list in Card. In the same time, current Card has AUX list (= aux_comp_list). This patch replaces it by Component list and new flag (= has_auxliary) too
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Looks mostly good, thanks.
include/sound/soc.h | 7 ++++--- sound/soc/soc-core.c | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 1ed9371..69f2ebf 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -807,9 +807,10 @@ struct snd_soc_component {
unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ unsigned int registered_as_component:1;
- unsigned int has_auxiliary:1; /* for auxiliary component of the card */
I'd name it just auxiliary. "has" is not really the right word here.
struct list_head list;
- struct list_head list_aux; /* for auxiliary component of the card */
struct list_head card_list;
struct snd_soc_dai_driver *dai_drv; int num_dai;
Hi Lars
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current Card has Codec list (= codec_dev_list), but Codec will be removed in the future. Because of this reason, this patch adds new Component list in Card. In the same time, current Card has AUX list (= aux_comp_list). This patch replaces it by Component list and new flag (= has_auxliary) too
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Looks mostly good, thanks.
Thanks
@@ -807,9 +807,10 @@ struct snd_soc_component {
unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ unsigned int registered_as_component:1;
- unsigned int has_auxiliary:1; /* for auxiliary component of the card */
I'd name it just auxiliary. "has" is not really the right word here.
OK, will fix in v3, and post it tomorrow
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
In current ALSA SoC, Codec only has suspend/resume feature. But it should be supported on Component level. This patch adds it. This patch replaces current codec_dev_list to component_dev_list.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 9 ++-- sound/soc/intel/boards/broadwell.c | 10 +++-- sound/soc/intel/boards/cht_bsw_rt5672.c | 10 +++-- sound/soc/soc-core.c | 73 +++++++++++++++++++-------------- 4 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 69f2ebf..430bf77 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -782,6 +782,8 @@ struct snd_soc_component_driver {
int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *); + int (*suspend)(struct snd_soc_component *); + int (*resume)(struct snd_soc_component *);
/* DT */ int (*of_xlate_dai_name)(struct snd_soc_component *component, @@ -808,6 +810,7 @@ struct snd_soc_component { unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ unsigned int registered_as_component:1; unsigned int has_auxiliary:1; /* for auxiliary component of the card */ + unsigned int suspended:1; /* is in suspend PM state */
struct list_head list; struct list_head card_list; @@ -853,6 +856,8 @@ struct snd_soc_component {
int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *); + int (*suspend)(struct snd_soc_component *); + int (*resume)(struct snd_soc_component *);
/* machine specific init */ int (*init)(struct snd_soc_component *component); @@ -869,11 +874,9 @@ struct snd_soc_codec { const struct snd_soc_codec_driver *driver;
struct list_head list; - struct list_head card_list;
/* runtime */ unsigned int cache_bypass:1; /* Suppress access to the cache */ - unsigned int suspended:1; /* Codec is in suspend PM state */ unsigned int cache_init:1; /* codec cache has been initialized */
/* codec IO */ @@ -1170,7 +1173,6 @@ struct snd_soc_card { struct work_struct deferred_resume_work;
/* lists of probed devices belonging to this card */ - struct list_head codec_dev_list; struct list_head component_dev_list;
struct list_head widgets; @@ -1541,7 +1543,6 @@ static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platfo
static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) { - INIT_LIST_HEAD(&card->codec_dev_list); INIT_LIST_HEAD(&card->widgets); INIT_LIST_HEAD(&card->paths); INIT_LIST_HEAD(&card->dapm_list); diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c index 7486a00..aea8816 100644 --- a/sound/soc/intel/boards/broadwell.c +++ b/sound/soc/intel/boards/broadwell.c @@ -220,9 +220,10 @@ static int broadwell_rtd_init(struct snd_soc_pcm_runtime *rtd) };
static int broadwell_suspend(struct snd_soc_card *card){ - struct snd_soc_codec *codec; + struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) { + list_for_each_entry(component, &card->component_dev_list, card_list) { + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); if (!strcmp(codec->component.name, "i2c-INT343A:00")) { dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n"); rt286_mic_detect(codec, NULL); @@ -233,9 +234,10 @@ static int broadwell_suspend(struct snd_soc_card *card){ }
static int broadwell_resume(struct snd_soc_card *card){ - struct snd_soc_codec *codec; + struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) { + list_for_each_entry(component, &card->component_dev_list, card_list) { + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); if (!strcmp(codec->component.name, "i2c-INT343A:00")) { dev_dbg(codec->dev, "enabling jack detect for resume.\n"); rt286_mic_detect(codec, &broadwell_headset); diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index df9d254..cfadc2e 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -292,9 +292,10 @@ static int cht_aif1_startup(struct snd_pcm_substream *substream)
static int cht_suspend_pre(struct snd_soc_card *card) { - struct snd_soc_codec *codec; + struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) { + list_for_each_entry(component, &card->component_dev_list, card_list) { + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); if (!strcmp(codec->component.name, "i2c-10EC5670:00")) { dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n"); rt5670_jack_suspend(codec); @@ -306,9 +307,10 @@ static int cht_suspend_pre(struct snd_soc_card *card)
static int cht_resume_post(struct snd_soc_card *card) { - struct snd_soc_codec *codec; + struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) { + list_for_each_entry(component, &card->component_dev_list, card_list) { + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); if (!strcmp(codec->component.name, "i2c-10EC5670:00")) { dev_dbg(codec->dev, "enabling jack detect for resume.\n"); rt5670_jack_resume(codec); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 2a00b5e..41e70f1 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -626,7 +626,7 @@ static void codec2codec_close_delayed_work(struct work_struct *work) int snd_soc_suspend(struct device *dev) { struct snd_soc_card *card = dev_get_drvdata(dev); - struct snd_soc_codec *codec; + struct snd_soc_component *component; struct snd_soc_pcm_runtime *rtd; int i;
@@ -702,39 +702,39 @@ int snd_soc_suspend(struct device *dev) dapm_mark_endpoints_dirty(card); snd_soc_dapm_sync(&card->dapm);
- /* suspend all CODECs */ - list_for_each_entry(codec, &card->codec_dev_list, card_list) { - struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec); + /* suspend all COMPONENTs */ + list_for_each_entry(component, &card->component_dev_list, card_list) { + struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
- /* If there are paths active then the CODEC will be held with + /* If there are paths active then the COMPONENT will be held with * bias _ON and should not be suspended. */ - if (!codec->suspended) { + if (!component->suspended) { switch (snd_soc_dapm_get_bias_level(dapm)) { case SND_SOC_BIAS_STANDBY: /* - * If the CODEC is capable of idle + * If the COMPONENT is capable of idle * bias off then being in STANDBY * means it's doing something, * otherwise fall through. */ if (dapm->idle_bias_off) { - dev_dbg(codec->dev, + dev_dbg(component->dev, "ASoC: idle_bias_off CODEC on over suspend\n"); break; }
case SND_SOC_BIAS_OFF: - if (codec->driver->suspend) - codec->driver->suspend(codec); - codec->suspended = 1; - if (codec->component.regmap) - regcache_mark_dirty(codec->component.regmap); + if (component->suspend) + component->suspend(component); + component->suspended = 1; + if (component->regmap) + regcache_mark_dirty(component->regmap); /* deactivate pins to sleep state */ - pinctrl_pm_select_sleep_state(codec->dev); + pinctrl_pm_select_sleep_state(component->dev); break; default: - dev_dbg(codec->dev, - "ASoC: CODEC is on over suspend\n"); + dev_dbg(component->dev, + "ASoC: COMPONENT is on over suspend\n"); break; } } @@ -768,7 +768,7 @@ static void soc_resume_deferred(struct work_struct *work) struct snd_soc_card *card = container_of(work, struct snd_soc_card, deferred_resume_work); struct snd_soc_pcm_runtime *rtd; - struct snd_soc_codec *codec; + struct snd_soc_component *component; int i;
/* our power state is still SNDRV_CTL_POWER_D3hot from suspend time, @@ -794,11 +794,11 @@ static void soc_resume_deferred(struct work_struct *work) cpu_dai->driver->resume(cpu_dai); }
- list_for_each_entry(codec, &card->codec_dev_list, card_list) { - if (codec->suspended) { - if (codec->driver->resume) - codec->driver->resume(codec); - codec->suspended = 0; + list_for_each_entry(component, &card->component_dev_list, card_list) { + if (component->suspended) { + if (component->resume) + component->resume(component); + component->suspended = 0; } }
@@ -1072,10 +1072,6 @@ static void soc_remove_component(struct snd_soc_component *component) if (!component->card) return;
- /* This is a HACK and will be removed soon */ - if (component->codec) - list_del(&component->codec->card_list); - list_del(&component->card_list);
if (component->remove) @@ -1445,11 +1441,6 @@ static int soc_probe_component(struct snd_soc_card *card, component->num_dapm_routes);
list_add(&dapm->list, &card->dapm_list); - - /* This is a HACK and will be removed soon */ - if (component->codec) - list_add(&component->codec->card_list, &card->codec_dev_list); - list_add(&component->card_list, &card->component_dev_list);
return 0; @@ -2937,6 +2928,8 @@ static int snd_soc_component_initialize(struct snd_soc_component *component, component->driver = driver; component->probe = component->driver->probe; component->remove = component->driver->remove; + component->suspend = component->driver->suspend; + component->resume = component->driver->resume;
dapm = &component->dapm; dapm->dev = dev; @@ -3286,6 +3279,20 @@ static void snd_soc_codec_drv_remove(struct snd_soc_component *component) codec->driver->remove(codec); }
+static int snd_soc_codec_drv_suspend(struct snd_soc_component *component) +{ + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); + + return codec->driver->suspend(codec); +} + +static int snd_soc_codec_drv_resume(struct snd_soc_component *component) +{ + struct snd_soc_codec *codec = snd_soc_component_to_codec(component); + + return codec->driver->resume(codec); +} + static int snd_soc_codec_drv_write(struct snd_soc_component *component, unsigned int reg, unsigned int val) { @@ -3347,6 +3354,10 @@ int snd_soc_register_codec(struct device *dev, codec->component.probe = snd_soc_codec_drv_probe; if (codec_drv->remove) codec->component.remove = snd_soc_codec_drv_remove; + if (codec_drv->suspend) + codec->component.suspend = snd_soc_codec_drv_suspend; + if (codec_drv->resume) + codec->component.resume = snd_soc_codec_drv_resume; if (codec_drv->write) codec->component.write = snd_soc_codec_drv_write; if (codec_drv->read)
On 10/25/2016 03:21 AM, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
In current ALSA SoC, Codec only has suspend/resume feature. But it should be supported on Component level. This patch adds it. This patch replaces current codec_dev_list to component_dev_list.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This looks good, but can you move the removal of the codec_dev_list to a separate patch. This will make it more clear what is going on. One patch to move suspend to component, one patch to cleanup and remove codec_dev_list.
include/sound/soc.h | 9 ++-- sound/soc/intel/boards/broadwell.c | 10 +++-- sound/soc/intel/boards/cht_bsw_rt5672.c | 10 +++-- sound/soc/soc-core.c | 73 +++++++++++++++++++-------------- 4 files changed, 59 insertions(+), 43 deletions(-)
[...]
static int broadwell_suspend(struct snd_soc_card *card){
- struct snd_soc_codec *codec;
- struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) {
- list_for_each_entry(component, &card->component_dev_list, card_list) {
struct snd_soc_codec *codec = snd_soc_component_to_codec(component);
The case should happen after the name has been matched, otherwise we are casting components that are not CODECs. Same for the other similar places.
if (!strcmp(codec->component.name, "i2c-INT343A:00")) { dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n"); rt286_mic_detect(codec, NULL);@@ -233,9 +234,10 @@ static int broadwell_suspend(struct snd_soc_card *card){ }
Hi Lars
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
In current ALSA SoC, Codec only has suspend/resume feature. But it should be supported on Component level. This patch adds it. This patch replaces current codec_dev_list to component_dev_list.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
This looks good, but can you move the removal of the codec_dev_list to a separate patch. This will make it more clear what is going on. One patch to move suspend to component, one patch to cleanup and remove codec_dev_list.
OK, will do in v3
static int broadwell_suspend(struct snd_soc_card *card){
- struct snd_soc_codec *codec;
- struct snd_soc_component *component;
- list_for_each_entry(codec, &card->codec_dev_list, card_list) {
- list_for_each_entry(component, &card->component_dev_list, card_list) {
struct snd_soc_codec *codec = snd_soc_component_to_codec(component);The case should happen after the name has been matched, otherwise we are casting components that are not CODECs. Same for the other similar places.
OK
participants (2)
-
Kuninori Morimoto -
Lars-Peter Clausen