Merge series "ASoC: topology: fix error handling flow" from Pierre-Louis Bossart...
authorMark Brown <broonie@kernel.org>
Wed, 8 Jul 2020 15:50:36 +0000 (16:50 +0100)
committerMark Brown <broonie@kernel.org>
Wed, 8 Jul 2020 15:50:36 +0000 (16:50 +0100)
While experimenting and introducing errors in Baytrail topology files
until I got them right, I encountered multiple kernel oopses and
memory leaks. This is a first batch to harden the code, but we should
probably think of a tool to fuzz the topology...

Pierre-Louis Bossart (5):
  ASoC: topology: fix kernel oops on route addition error
  ASoC: topology: fix tlvs in error handling for widget_dmixer
  ASoC: topology: use break on errors, not continue
  ASoC: topology: factor kfree(se) in error handling
  ASoC: topology: add more logs when topology load fails.

 sound/soc/soc-topology.c | 97 ++++++++++++++++++++++++----------------
 1 file changed, 58 insertions(+), 39 deletions(-)

base-commit: a5911ac5790acaf98c929b826b3f7b4a438f9759
--
2.25.1

MAINTAINERS
include/sound/soc.h
sound/soc/amd/raven/pci-acp3x.c
sound/soc/codecs/rt5682.c
sound/soc/rockchip/rk3399_gru_sound.c
sound/soc/soc-core.c
sound/soc/soc-devres.c
sound/soc/soc-generic-dmaengine-pcm.c
sound/soc/soc-topology.c

index 68f21d46614c46ce8e21ea80507610eb47a9ff30..1bcd50b24dcae8d5f6d9f2783982af48ec0b49ab 100644 (file)
@@ -6956,6 +6956,7 @@ M:        Timur Tabi <timur@kernel.org>
 M:     Nicolin Chen <nicoleotsuka@gmail.com>
 M:     Xiubo Li <Xiubo.Lee@gmail.com>
 R:     Fabio Estevam <festevam@gmail.com>
+R:     Shengjiu Wang <shengjiu.wang@gmail.com>
 L:     alsa-devel@alsa-project.org (moderated for non-subscribers)
 L:     linuxppc-dev@lists.ozlabs.org
 S:     Maintained
@@ -11333,17 +11334,17 @@ F:    drivers/iio/adc/at91-sama5d2_adc.c
 F:     include/dt-bindings/iio/adc/at91-sama5d2_adc.h
 
 MICROCHIP SAMA5D2-COMPATIBLE SHUTDOWN CONTROLLER
-M:     Nicolas Ferre <nicolas.ferre@microchip.com>
+M:     Claudiu Beznea <claudiu.beznea@microchip.com>
 S:     Supported
 F:     drivers/power/reset/at91-sama5d2_shdwc.c
 
 MICROCHIP SPI DRIVER
-M:     Nicolas Ferre <nicolas.ferre@microchip.com>
+M:     Tudor Ambarus <tudor.ambarus@microchip.com>
 S:     Supported
 F:     drivers/spi/spi-atmel.*
 
 MICROCHIP SSC DRIVER
-M:     Nicolas Ferre <nicolas.ferre@microchip.com>
+M:     Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
 L:     linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:     Supported
 F:     drivers/misc/atmel-ssc.c
index 6791b7570a67a9ae8adc568ed02ef49857c6d01f..59235e553630654a25d566226b1c4b371c5ad09a 100644 (file)
@@ -426,6 +426,8 @@ int devm_snd_soc_register_component(struct device *dev,
                         const struct snd_soc_component_driver *component_driver,
                         struct snd_soc_dai_driver *dai_drv, int num_dai);
 void snd_soc_unregister_component(struct device *dev);
+void snd_soc_unregister_component_by_driver(struct device *dev,
+                        const struct snd_soc_component_driver *component_driver);
 struct snd_soc_component *snd_soc_lookup_component_nolocked(struct device *dev,
                                                            const char *driver_name);
 struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
index f25ce50f1a901609b51aeb2d2cc86bd6de51d689..ebf4388b626215ffb0bc76ea22fb63624dbcea98 100644 (file)
@@ -232,9 +232,7 @@ static int snd_acp3x_probe(struct pci_dev *pci,
        }
        pm_runtime_set_autosuspend_delay(&pci->dev, 2000);
        pm_runtime_use_autosuspend(&pci->dev);
-       pm_runtime_set_active(&pci->dev);
        pm_runtime_put_noidle(&pci->dev);
-       pm_runtime_enable(&pci->dev);
        pm_runtime_allow(&pci->dev);
        return 0;
 
@@ -303,7 +301,7 @@ static void snd_acp3x_remove(struct pci_dev *pci)
        ret = acp3x_deinit(adata->acp3x_base);
        if (ret)
                dev_err(&pci->dev, "ACP de-init failed\n");
-       pm_runtime_disable(&pci->dev);
+       pm_runtime_forbid(&pci->dev);
        pm_runtime_get_noresume(&pci->dev);
        pci_disable_msi(pci);
        pci_release_regions(pci);
index e9514c81b9ba444ff87cd7eaffd1ccb3d944a173..de40b6cd16cf23ae02814fec2b8bc1762904e85d 100644 (file)
@@ -992,16 +992,17 @@ static int rt5682_set_jack_detect(struct snd_soc_component *component,
 
        rt5682->hs_jack = hs_jack;
 
-       if (!rt5682->is_sdw) {
-               if (!hs_jack) {
-                       regmap_update_bits(rt5682->regmap, RT5682_IRQ_CTRL_2,
-                               RT5682_JD1_EN_MASK, RT5682_JD1_DIS);
-                       regmap_update_bits(rt5682->regmap, RT5682_RC_CLK_CTRL,
-                               RT5682_POW_JDH | RT5682_POW_JDL, 0);
-                       cancel_delayed_work_sync(&rt5682->jack_detect_work);
-                       return 0;
-               }
+       if (!hs_jack) {
+               regmap_update_bits(rt5682->regmap, RT5682_IRQ_CTRL_2,
+                       RT5682_JD1_EN_MASK, RT5682_JD1_DIS);
+               regmap_update_bits(rt5682->regmap, RT5682_RC_CLK_CTRL,
+                       RT5682_POW_JDH | RT5682_POW_JDL, 0);
+               cancel_delayed_work_sync(&rt5682->jack_detect_work);
 
+               return 0;
+       }
+
+       if (!rt5682->is_sdw) {
                switch (rt5682->pdata.jd_src) {
                case RT5682_JD1:
                        snd_soc_component_update_bits(component,
index f45e5aaa4b3028f21e53e5514bea4be856459927..9539b0d024fed224577fb000a8077fda874a74f3 100644 (file)
@@ -219,19 +219,32 @@ static int rockchip_sound_dmic_hw_params(struct snd_pcm_substream *substream,
        return 0;
 }
 
+static int rockchip_sound_startup(struct snd_pcm_substream *substream)
+{
+       struct snd_pcm_runtime *runtime = substream->runtime;
+
+       runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
+       return snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE,
+                       8000, 96000);
+}
+
 static const struct snd_soc_ops rockchip_sound_max98357a_ops = {
+       .startup = rockchip_sound_startup,
        .hw_params = rockchip_sound_max98357a_hw_params,
 };
 
 static const struct snd_soc_ops rockchip_sound_rt5514_ops = {
+       .startup = rockchip_sound_startup,
        .hw_params = rockchip_sound_rt5514_hw_params,
 };
 
 static const struct snd_soc_ops rockchip_sound_da7219_ops = {
+       .startup = rockchip_sound_startup,
        .hw_params = rockchip_sound_da7219_hw_params,
 };
 
 static const struct snd_soc_ops rockchip_sound_dmic_ops = {
+       .startup = rockchip_sound_startup,
        .hw_params = rockchip_sound_dmic_hw_params,
 };
 
index adedadcb0efb4dea4acfd5bf849a6c30d0dd7791..7c58e45c1c3f243afcfa6f1ca0f4fd5f80ee9c0f 100644 (file)
@@ -2513,6 +2513,33 @@ int snd_soc_register_component(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_component);
 
+/**
+ * snd_soc_unregister_component_by_driver - Unregister component using a given driver
+ * from the ASoC core
+ *
+ * @dev: The device to unregister
+ * @component_driver: The component driver to unregister
+ */
+void snd_soc_unregister_component_by_driver(struct device *dev,
+                                           const struct snd_soc_component_driver *component_driver)
+{
+       struct snd_soc_component *component;
+
+       if (!component_driver)
+               return;
+
+       mutex_lock(&client_mutex);
+       component = snd_soc_lookup_component_nolocked(dev, component_driver->name);
+       if (!component)
+               goto out;
+
+       snd_soc_del_component_unlocked(component);
+
+out:
+       mutex_unlock(&client_mutex);
+}
+EXPORT_SYMBOL_GPL(snd_soc_unregister_component_by_driver);
+
 /**
  * snd_soc_unregister_component - Unregister all related component
  * from the ASoC core
index 11e5d79623707a7b36803323056bc179a09dab5d..4534a1c03e8e5e85f9f5e4e889197edd88652649 100644 (file)
@@ -48,7 +48,9 @@ EXPORT_SYMBOL_GPL(devm_snd_soc_register_dai);
 
 static void devm_component_release(struct device *dev, void *res)
 {
-       snd_soc_unregister_component(*(struct device **)res);
+       const struct snd_soc_component_driver **cmpnt_drv = res;
+
+       snd_soc_unregister_component_by_driver(dev, *cmpnt_drv);
 }
 
 /**
@@ -65,7 +67,7 @@ int devm_snd_soc_register_component(struct device *dev,
                         const struct snd_soc_component_driver *cmpnt_drv,
                         struct snd_soc_dai_driver *dai_drv, int num_dai)
 {
-       struct device **ptr;
+       const struct snd_soc_component_driver **ptr;
        int ret;
 
        ptr = devres_alloc(devm_component_release, sizeof(*ptr), GFP_KERNEL);
@@ -74,7 +76,7 @@ int devm_snd_soc_register_component(struct device *dev,
 
        ret = snd_soc_register_component(dev, cmpnt_drv, dai_drv, num_dai);
        if (ret == 0) {
-               *ptr = dev;
+               *ptr = cmpnt_drv;
                devres_add(dev, ptr);
        } else {
                devres_free(ptr);
index 80a4e71f2d95d6254a6e45b8ffe4600c3a7151bb..61844403f1817fd2bdc7c781f0d1c545e5f16f97 100644 (file)
@@ -478,7 +478,7 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
 
        pcm = soc_component_to_pcm(component);
 
-       snd_soc_unregister_component(dev);
+       snd_soc_unregister_component_by_driver(dev, component->driver);
        dmaengine_pcm_release_chan(pcm);
        kfree(pcm);
 }
index 43e5745b06aa7fe91e96472f5289cf547bcf799a..cee99867131879620d292045b9837950e0c03a0d 100644 (file)
@@ -741,7 +741,8 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
        struct snd_soc_tplg_bytes_control *be;
        struct soc_bytes_ext *sbe;
        struct snd_kcontrol_new kc;
-       int i, err;
+       int i;
+       int err = 0;
 
        if (soc_tplg_check_elem_count(tplg,
                sizeof(struct snd_soc_tplg_bytes_control), count,
@@ -786,7 +787,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
                if (err) {
                        soc_control_err(tplg, &be->hdr, be->hdr.name);
                        kfree(sbe);
-                       continue;
+                       break;
                }
 
                /* pass control to driver for optional further init */
@@ -796,7 +797,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
                        dev_err(tplg->dev, "ASoC: failed to init %s\n",
                                be->hdr.name);
                        kfree(sbe);
-                       continue;
+                       break;
                }
 
                /* register control here */
@@ -806,12 +807,12 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count,
                        dev_err(tplg->dev, "ASoC: failed to add %s\n",
                                be->hdr.name);
                        kfree(sbe);
-                       continue;
+                       break;
                }
 
                list_add(&sbe->dobj.list, &tplg->comp->dobj_list);
        }
-       return 0;
+       return err;
 
 }
 
@@ -821,7 +822,8 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
        struct snd_soc_tplg_mixer_control *mc;
        struct soc_mixer_control *sm;
        struct snd_kcontrol_new kc;
-       int i, err;
+       int i;
+       int err = 0;
 
        if (soc_tplg_check_elem_count(tplg,
                sizeof(struct snd_soc_tplg_mixer_control),
@@ -880,7 +882,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
                if (err) {
                        soc_control_err(tplg, &mc->hdr, mc->hdr.name);
                        kfree(sm);
-                       continue;
+                       break;
                }
 
                /* create any TLV data */
@@ -889,7 +891,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
                        dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
                                mc->hdr.name);
                        kfree(sm);
-                       continue;
+                       break;
                }
 
                /* pass control to driver for optional further init */
@@ -900,7 +902,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
                                mc->hdr.name);
                        soc_tplg_free_tlv(tplg, &kc);
                        kfree(sm);
-                       continue;
+                       break;
                }
 
                /* register control here */
@@ -911,13 +913,13 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count,
                                mc->hdr.name);
                        soc_tplg_free_tlv(tplg, &kc);
                        kfree(sm);
-                       continue;
+                       break;
                }
 
                list_add(&sm->dobj.list, &tplg->comp->dobj_list);
        }
 
-       return 0;
+       return err;
 }
 
 static int soc_tplg_denum_create_texts(struct soc_enum *se,
@@ -997,7 +999,8 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
        struct snd_soc_tplg_enum_control *ec;
        struct soc_enum *se;
        struct snd_kcontrol_new kc;
-       int i, ret, err;
+       int i;
+       int err = 0;
 
        if (soc_tplg_check_elem_count(tplg,
                sizeof(struct snd_soc_tplg_enum_control),
@@ -1052,8 +1055,7 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
                                dev_err(tplg->dev,
                                        "ASoC: could not create values for %s\n",
                                        ec->hdr.name);
-                               kfree(se);
-                               continue;
+                               goto err_denum;
                        }
                        /* fall through */
                case SND_SOC_TPLG_CTL_ENUM:
@@ -1064,24 +1066,22 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
                                dev_err(tplg->dev,
                                        "ASoC: could not create texts for %s\n",
                                        ec->hdr.name);
-                               kfree(se);
-                               continue;
+                               goto err_denum;
                        }
                        break;
                default:
+                       err = -EINVAL;
                        dev_err(tplg->dev,
                                "ASoC: invalid enum control type %d for %s\n",
                                ec->hdr.ops.info, ec->hdr.name);
-                       kfree(se);
-                       continue;
+                       goto err_denum;
                }
 
                /* map io handlers */
                err = soc_tplg_kcontrol_bind_io(&ec->hdr, &kc, tplg);
                if (err) {
                        soc_control_err(tplg, &ec->hdr, ec->hdr.name);
-                       kfree(se);
-                       continue;
+                       goto err_denum;
                }
 
                /* pass control to driver for optional further init */
@@ -1090,24 +1090,25 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count,
                if (err < 0) {
                        dev_err(tplg->dev, "ASoC: failed to init %s\n",
                                ec->hdr.name);
-                       kfree(se);
-                       continue;
+                       goto err_denum;
                }
 
                /* register control here */
-               ret = soc_tplg_add_kcontrol(tplg,
-                       &kc, &se->dobj.control.kcontrol);
-               if (ret < 0) {
+               err = soc_tplg_add_kcontrol(tplg,
+                                           &kc, &se->dobj.control.kcontrol);
+               if (err < 0) {
                        dev_err(tplg->dev, "ASoC: could not add kcontrol %s\n",
                                ec->hdr.name);
-                       kfree(se);
-                       continue;
+                       goto err_denum;
                }
 
                list_add(&se->dobj.list, &tplg->comp->dobj_list);
        }
-
        return 0;
+
+err_denum:
+       kfree(se);
+       return err;
 }
 
 static int soc_tplg_kcontrol_elems_load(struct soc_tplg *tplg,
@@ -1261,17 +1262,30 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
                list_add(&routes[i]->dobj.list, &tplg->comp->dobj_list);
 
                ret = soc_tplg_add_route(tplg, routes[i]);
-               if (ret < 0)
+               if (ret < 0) {
+                       dev_err(tplg->dev, "ASoC: topology: add_route failed: %d\n", ret);
+                       /*
+                        * this route was added to the list, it will
+                        * be freed in remove_route() so increment the
+                        * counter to skip it in the error handling
+                        * below.
+                        */
+                       i++;
                        break;
+               }
 
                /* add route, but keep going if some fail */
                snd_soc_dapm_add_routes(dapm, routes[i], 1);
        }
 
-       /* free memory allocated for all dapm routes in case of error */
-       if (ret < 0)
-               for (i = 0; i < count ; i++)
-                       kfree(routes[i]);
+       /*
+        * free memory allocated for all dapm routes not added to the
+        * list in case of error
+        */
+       if (ret < 0) {
+               while (i < count)
+                       kfree(routes[i++]);
+       }
 
        /*
         * free pointer to array of dapm routes as this is no longer needed.
@@ -1349,8 +1363,7 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
                if (err < 0) {
                        dev_err(tplg->dev, "ASoC: failed to create TLV %s\n",
                                mc->hdr.name);
-                       kfree(sm);
-                       continue;
+                       goto err_sm;
                }
 
                /* pass control to driver for optional further init */
@@ -1359,7 +1372,6 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
                if (err < 0) {
                        dev_err(tplg->dev, "ASoC: failed to init %s\n",
                                mc->hdr.name);
-                       soc_tplg_free_tlv(tplg, &kc[i]);
                        goto err_sm;
                }
        }
@@ -1367,6 +1379,7 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 
 err_sm:
        for (; i >= 0; i--) {
+               soc_tplg_free_tlv(tplg, &kc[i]);
                sm = (struct soc_mixer_control *)kc[i].private_value;
                kfree(sm);
                kfree(kc[i].name);
@@ -2731,15 +2744,21 @@ static int soc_tplg_process_headers(struct soc_tplg *tplg)
 
                        /* make sure header is valid before loading */
                        ret = soc_valid_header(tplg, hdr);
-                       if (ret < 0)
+                       if (ret < 0) {
+                               dev_err(tplg->dev,
+                                       "ASoC: topology: invalid header: %d\n", ret);
                                return ret;
-                       else if (ret == 0)
+                       } else if (ret == 0) {
                                break;
+                       }
 
                        /* load the header object */
                        ret = soc_tplg_load_header(tplg, hdr);
-                       if (ret < 0)
+                       if (ret < 0) {
+                               dev_err(tplg->dev,
+                                       "ASoC: topology: could not load header: %d\n", ret);
                                return ret;
+                       }
 
                        /* goto next header */
                        tplg->hdr_pos += le32_to_cpu(hdr->payload_size) +