Re: [alsa-devel] Applied "ASoC: Make aux_dev more like a generic component" to the asoc tree

Hi,

2016-01-10 13:50 GMT+01:00 Mark Brown broonie@kernel.org:

The patch

ASoC: Make aux_dev more like a generic component

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

From f2ed6b07645ed29c1e090ead2e41066385cba3ea Mon Sep 17 00:00:00 2001 From: Mengdong Lin mengdong.lin@linux.intel.com Date: Wed, 6 Jan 2016 13:29:31 +0800 Subject: [PATCH] ASoC: Make aux_dev more like a generic component

aux_dev is mainly used by the machine driver to specify analog devices, which are registered as codecs. Making it more like a generic component can help the machine driver to use it to specify any component with topology info by name.

Details:

• Remove the stub 'rtd_aux' array from the soc card.
• Add a list 'aux_comp_list' to store the components of aux_devs. And add a list head 'list_aux' to struct snd_soc_component, for adding such components to the above list.
• Add a 'init' ops to a component for machine specific init. soc_bind_aux_dev() will set it to be aux_dev's init. And it will be called when probing the component.
• soc_bind_aux_dev() will also search components by name of an aux_dev, since it may not be a codec.
• Move probing of aux_devs before checking new DAI links brought by topology.
• Move removal of aux_devs later than removal of links. Because topology of aux components may register DAIs and the DAI drivers will go with removal of the aux components, we want soc_remove_link_dais() to remove the DAIs at first.

Signed-off-by: Mengdong Lin mengdong.lin@linux.intel.com Signed-off-by: Mark Brown broonie@kernel.org

---

include/sound/soc.h | 8 ++- sound/soc/soc-core.c | 146 +++++++++++++++++++++++++++------------------------ 2 files changed, 83 insertions(+), 71 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h index 9d1383e8d039..5bc5def6af02 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -787,6 +787,7 @@ struct snd_soc_component { unsigned int registered_as_component:1;

    struct list_head list;

•   struct list_head list_aux; /* for auxiliary component of the card */
  
    struct snd_soc_dai_driver *dai_drv;
    int num_dai;
  

@@ -830,6 +831,9 @@ struct snd_soc_component { int (*probe)(struct snd_soc_component *); void (*remove)(struct snd_soc_component *);

•   /* machine specific init */
  

•   int (*init)(struct snd_soc_component *component);
  

#ifdef CONFIG_DEBUG_FS void (*init_debugfs)(struct snd_soc_component *component); const char *debugfs_prefix; @@ -1130,8 +1134,7 @@ struct snd_soc_card { */ struct snd_soc_aux_dev *aux_dev; int num_aux_devs;

•   struct snd_soc_pcm_runtime *rtd_aux;
  

•   int num_aux_rtd;
  

•   struct list_head aux_comp_list;
  
    const struct snd_kcontrol_new *controls;
    int num_controls;
  

@@ -1537,6 +1540,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);
  

}

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 c572673a5a24..c10bd668659c 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1413,6 +1413,16 @@ static int soc_probe_component(struct snd_soc_card *card, component->name); }

•   /* machine specific init */
  

•   if (component->init) {
  

•           ret = component->init(component);
  

•           if (ret < 0) {
  

•                   dev_err(component->dev,
  

•                           "Failed to do machine specific init %d\n", ret);
  

•                   goto err_probe;
  

•           }
  

•   }
  

•   if (component->controls)
            snd_soc_add_component_controls(component, component->controls,
                                 component->num_controls);
  

@@ -1657,65 +1667,81 @@ static int soc_probe_link_dais(struct snd_soc_card *card,

static int soc_bind_aux_dev(struct snd_soc_card *card, int num) {

•   struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
    struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
  

•   const char *name = aux_dev->codec_name;
  

•   rtd->component = soc_find_component(aux_dev->codec_of_node, name);
  

•   if (!rtd->component) {
  

•           if (aux_dev->codec_of_node)
  

•                   name = of_node_full_name(aux_dev->codec_of_node);
  

•           dev_err(card->dev, "ASoC: %s not registered\n", name);
  

•           return -EPROBE_DEFER;
  

•   struct snd_soc_component *component;
  

•   const char *name;
  

•   struct device_node *codec_of_node;
  

•   if (aux_dev->codec_of_node || aux_dev->codec_name) {
  

•           /* codecs, usually analog devices */
  

•           name = aux_dev->codec_name;
  

•           codec_of_node = aux_dev->codec_of_node;
  

•           component = soc_find_component(codec_of_node, name);
  

•           if (!component) {
  

•                   if (codec_of_node)
  

•                           name = of_node_full_name(codec_of_node);
  

•                   goto err_defer;
  

•           }
  

•   } else if (aux_dev->name) {
  

•           /* generic components */
  

•           name = aux_dev->name;
  

•           component = soc_find_component(NULL, name);
  

•           if (!component)
  

•                   goto err_defer;
  

•   } else {
  

•           dev_err(card->dev, "ASoC: Invalid auxiliary device\n");
  

•           return -EINVAL;
    }
  
  

•   /*
  

•    * Some places still reference rtd->codec, so we have to keep that
  

•    * initialized if the component is a CODEC. Once all those references
  

•    * have been removed, this code can be removed as well.
  

•    */
  

•    rtd->codec = rtd->component->codec;
  

•   component->init = aux_dev->init;
  

•   list_add(&component->list_aux, &card->aux_comp_list);
    return 0;
  

+err_defer:

•   dev_err(card->dev, "ASoC: %s not registered\n", name);
  

•   return -EPROBE_DEFER;
  

}

-static int soc_probe_aux_dev(struct snd_soc_card *card, int num) +static int soc_probe_aux_devices(struct snd_soc_card *card) {

•   struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
  

•   struct snd_soc_aux_dev *aux_dev = &card->aux_dev[num];
  

•   struct snd_soc_component *comp;
  

•   int order;
    int ret;
  
  

•   ret = soc_probe_component(card, rtd->component);
  

•   if (ret < 0)
  

•           return ret;
  

•   /* do machine specific initialization */
  

•   if (aux_dev->init) {
  

•           ret = aux_dev->init(rtd->component);
  

•           if (ret < 0) {
  

•                   dev_err(card->dev, "ASoC: failed to init %s: %d\n",
  

•                           aux_dev->name, ret);
  

•                   return ret;
  

•   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) {
  

•                   if (comp->driver->probe_order == order) {
  

•                           ret = soc_probe_component(card, comp);
  

•                           if (ret < 0) {
  

•                                   dev_err(card->dev,
  

•                                           "ASoC: failed to probe aux component %s %d\n",
  

•                                           comp->name, ret);
  

•                                   return ret;
  

•                           }
  

•                   }
            }
    }
  
  

•   return soc_post_component_init(rtd, aux_dev->name);
  

•   return 0;
  

}

-static void soc_remove_aux_dev(struct snd_soc_card *card, int num) +static void soc_remove_aux_devices(struct snd_soc_card *card) {

•   struct snd_soc_pcm_runtime *rtd = &card->rtd_aux[num];
  

•   struct snd_soc_component *component = rtd->component;
  

•   struct snd_soc_component *comp, *_comp;
  

•   int order;
  
  

•   /* unregister the rtd device */
  

•   if (rtd->dev_registered) {
  

•           device_unregister(rtd->dev);
  

•           rtd->dev_registered = 0;
  

•   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) {
  

•                   if (comp->driver->remove_order == order) {
  

•                           soc_remove_component(comp);
  

•                           /* remove it from the card's aux_comp_list */
  

•                           list_del(&comp->list_aux);
  

•                   }
  

•           }
    }
  

•   if (component)
  

•           soc_remove_component(component);
  

}

static int snd_soc_init_codec_cache(struct snd_soc_codec *codec) @@ -1894,6 +1920,11 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) } }

•   /* probe auxiliary components */
  

•   ret = soc_probe_aux_devices(card);
  

•   if (ret < 0)
  

•           goto probe_dai_err;
  

•   /* Find new DAI links added during probing components and bind them.
     * Components with topology may bring new DAIs and DAI links.
     */
  

@@ -1923,16 +1954,6 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) } }

•   for (i = 0; i < card->num_aux_devs; i++) {
  

•           ret = soc_probe_aux_dev(card, i);
  

•           if (ret < 0) {
  

•                   dev_err(card->dev,
  

•                           "ASoC: failed to add auxiliary devices %d\n",
  

•                           ret);
  

•                   goto probe_aux_dev_err;
  

•           }
  

•   }
  

•   snd_soc_dapm_link_dai_widgets(card);
    snd_soc_dapm_connect_dai_link_widgets(card);
  
  

@@ -1992,8 +2013,7 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card) return 0;

probe_aux_dev_err:

•   for (i = 0; i < card->num_aux_devs; i++)
  

•           soc_remove_aux_dev(card, i);
  

•   soc_remove_aux_devices(card);
  
  

probe_dai_err: soc_remove_dai_links(card); @@ -2039,20 +2059,18 @@ static int soc_probe(struct platform_device *pdev) static int soc_cleanup_card_resources(struct snd_soc_card *card) { struct snd_soc_pcm_runtime *rtd;

•   int i;
  
    /* make sure any delayed work runs */
    list_for_each_entry(rtd, &card->rtd_list, list)
            flush_delayed_work(&rtd->delayed_work);
  
  

•   /* remove auxiliary devices */
  

•   for (i = 0; i < card->num_aux_devs; i++)
  

•           soc_remove_aux_dev(card, i);
  

•   /* remove and free each DAI */
    soc_remove_dai_links(card);
    soc_remove_pcm_runtimes(card);
  
  

•   /* remove auxiliary devices */
  

•   soc_remove_aux_devices(card);
  

•   soc_cleanup_card_debugfs(card);
  
    /* remove the card */
  

@@ -2608,16 +2626,6 @@ int snd_soc_register_card(struct snd_soc_card *card) INIT_LIST_HEAD(&card->rtd_list); card->num_rtd = 0;

•   card->rtd_aux = devm_kzalloc(card->dev,
  

•                            sizeof(struct snd_soc_pcm_runtime) *
  

•                            card->num_aux_devs,
  

•                            GFP_KERNEL);
  

•   if (card->rtd_aux == NULL)
  

•           return -ENOMEM;
  

•   for (i = 0; i < card->num_aux_devs; i++)
  

•           card->rtd_aux[i].card = card;
  

•   INIT_LIST_HEAD(&card->dapm_dirty);
    INIT_LIST_HEAD(&card->dobj_list);
    card->instantiated = 0;
  

-- 2.7.0.rc3

---

Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

I just found that this change produces a kernel NULL pointer dereference [1]. Reverting the patch avoids the kernel oops. From a quick look seems that moving the probing of aux_devs before checking new DAI links is what causes the problem. I'm not really familiar with the API so before send a patch I prefer share my thoughts.

The sound/soc/rockchip/rockchip_max98090.c registers a headset chip as a snd_soc_aux_dev, and the problem is that the init function calls ts3a227e_enable_jack_detect and snd_jack_set_key before snd_soc_card_jack_new is called. I think similar problem can happen with this driver: sound/soc/intel/boards/cht_bsw_max98090_ti.c

Makes sense what I'm saying for you? And is the proper fix delay the call of ts3a227e_enable_jack_detect to be later?

[1] dmesg.log

[ 132.091648] max98090 2-0010: MAX98090 REVID=0x43 [ 132.094658] max98090 2-0010: use default 2.8v micbias [ 132.103997] Unable to handle kernel NULL pointer dereference at virtual address 00000014 [ 132.104396] pgd = eba34000 [ 132.104552] [00000014] *pgd=00000000 [ 132.104780] Internal error: Oops: 5 [#1] SMP ARM [ 132.105005] Modules linked in: snd_soc_rockchip_max98090(+) snd_soc_ts3a227e smsc95xx mwifiex_sdio mwifiex btmrv l_sdio btmrvl bluetooth rk_crypto sha256_generic nvmem_rockchip_efuse snd_soc_max98090 snd_soc_rockchip_i2s nvmem_c ore sha1_generic joydev rockchip_thermal [last unloaded: snd_soc_ts3a227e] [ 132.106579] CPU: 0 PID: 708 Comm: insmod Tainted: G W 4.6.0-rc5+ #1 [ 132.106920] Hardware name: Rockchip (Device Tree) [ 132.107147] task: ec1c8d80 ti: ec3e8000 task.ti: ec3e8000 [ 132.107411] PC is at snd_jack_set_key+0x14/0x78 [ 132.107643] LR is at ts3a227e_enable_jack_detect+0x38/0x8c [snd_soc_ts3a227e] [ 132.107979] pc : [<c06a3c10>] lr : [<bf166150>] psr: 60010013 sp : ec3e9bb0 ip : ec3e9bd0 fp : ec3e9bcc [ 132.108494] r10: 00000000 r9 : ec56a038 r8 : ec56a0a0 [ 132.108742] r7 : ec569ff0 r6 : bf16bb30 r5 : eb8a7c58 r4 : bf16c080 [ 132.109051] r3 : 00000000 r2 : 000000e2 r1 : 00004000 r0 : 00000000 [ 132.109357] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 132.109688] Control: 10c5387d Table: 2ba3406a DAC: 00000051 [ 132.109958] Process insmod (pid: 708, stack limit = 0xec3e8218) [ 132.110236] Stack: (0xec3e9bb0 to 0xec3ea000) [ 132.110450] 9ba0: bf16c080 eb8a7c58 bf16bb30 ec569ff0 [ 132.110832] 9bc0: ec3e9be4 ec3e9bd0 bf166150 c06a3c08 ec56a000 00000000 ec3e9bf4 ec3e9be8

...

[ 132.290280] 9fe0: bee024f0 bee024e0 7f6546c3 b6f57b42 80010030 00000003 00000000 00000000 [ 132.299958] [<c06a3c10>] (snd_jack_set_key) from [<bf166150>] (ts3a227e_enable_jack_detect+0x38/0x8c [snd_soc_ts 3a227e]) [ 132.309961] [<bf166150>] (ts3a227e_enable_jack_detect [snd_soc_ts3a227e]) from [<bf16b124>] (rk_98090_headset_in it+0x1c/0x24 [snd_soc_rockchip_max98090]) [ 132.330170] [<bf16b124>] (rk_98090_headset_init [snd_soc_rockchip_max98090]) from [<c06b771c>] (soc_probe_compon ent+0x22c/0x334) [ 132.351063] [<c06b771c>] (soc_probe_component) from [<c06ba6a0>] (snd_soc_register_card+0x560/0xd14) [ 132.362015] [<c06ba6a0>] (snd_soc_register_card) from [<c06c7e60>] (devm_snd_soc_register_card+0x50/0x8c) [ 132.373129] [<c06c7e60>] (devm_snd_soc_register_card) from [<bf16b0c0>] (snd_rk_mc_probe+0xc0/0x108 [snd_soc_roc kchip_max98090]) [ 132.395594] [<bf16b0c0>] (snd_rk_mc_probe [snd_soc_rockchip_max98090]) from [<c04af5bc>] (platform_drv_probe+0x6 0/0xb0) [ 132.407335] [<c04af5bc>] (platform_drv_probe) from [<c04ad03c>] (driver_probe_device+0x1dc/0x41c) [ 132.419192] [<c04ad03c>] (driver_probe_device) from [<c04ad364>] (__driver_attach+0xe8/0x11c) [ 132.431112] [<c04ad364>] (__driver_attach) from [<c04aaf24>] (bus_for_each_dev+0x7c/0xa0) [ 132.443033] [<c04aaf24>] (bus_for_each_dev) from [<c04ac900>] (driver_attach+0x28/0x30) [ 132.454945] [<c04ac900>] (driver_attach) from [<c04ac340>] (bus_add_driver+0x128/0x254) [ 132.466853] [<c04ac340>] (bus_add_driver) from [<c04ae3a4>] (driver_register+0xac/0xf0) [ 132.478765] [<c04ae3a4>] (driver_register) from [<c04af4fc>] (__platform_driver_register+0x40/0x54) [ 132.490793] [<c04af4fc>] (__platform_driver_register) from [<bf16e018>] (snd_rk_mc_driver_init+0x18/0x24 [snd_so c_rockchip_max98090]) [ 132.515020] [<bf16e018>] (snd_rk_mc_driver_init [snd_soc_rockchip_max98090]) from [<c0101de8>] (do_one_initcall+ 0x128/0x1e4) [ 132.527685] [<c0101de8>] (do_one_initcall) from [<c02172f4>] (do_init_module+0x6c/0x3ac) [ 132.540430] [<c02172f4>] (do_init_module) from [<c01bb1b4>] (load_module+0x1a40/0x1f64) [ 132.553282] [<c01bb1b4>] (load_module) from [<c01bb93c>] (SyS_finit_module+0xc4/0xd4) [ 132.566158] [<c01bb93c>] (SyS_finit_module) from [<c0108540>] (ret_fast_syscall+0x0/0x1c) [ 132.579069] Code: e92dd8f0 e24cb004 e52de004 e8bd4000 (e5903014) [ 132.592242] ---[ end trace ca16e398efcae7d0 ]---