[alsa-devel] [PATCH v3 0/4] ASoC: fsl: audmix: remove "model" attribute and fix ref leaks
The latest audmix patch-set (v5) had the "model" attribute removed as requested by Nicolin Chen, but looks like (v4) version of DAI driver reached "for-next" branch - fix this by removing "model" attribute. Asside of this fix object reference leaks in machine probe reported by Julia Lawall.
Viorel Suman (4): ASoC: fsl_audmix: remove "model" attribute dt-bindings: fsl,audmix: remove "model" attribute ASoC: imx-audmix: fix object reference leaks in probe ASoC: fsl_audmix: cache pdev->dev pointer
Changes since V1: a) Removed "model" attribute from dt-bindings documentation b) Adressed Daniel's comments
Changes since V2: a) Cache pdev->dev pointer in fsl_audmix probe as suggested by Nicolin
.../devicetree/bindings/sound/fsl,audmix.txt | 4 -- sound/soc/fsl/fsl_audmix.c | 61 ++++++++++++---------- sound/soc/fsl/imx-audmix.c | 4 ++ 3 files changed, 36 insertions(+), 33 deletions(-)
Use "of_device_id.data" to specify the machine driver instead of "model" DTS attribute.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Acked-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/fsl_audmix.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c index dabde03..dc802d5 100644 --- a/sound/soc/fsl/fsl_audmix.c +++ b/sound/soc/fsl/fsl_audmix.c @@ -445,13 +445,29 @@ static const struct regmap_config fsl_audmix_regmap_config = { .cache_type = REGCACHE_FLAT, };
+static const struct of_device_id fsl_audmix_ids[] = { + { + .compatible = "fsl,imx8qm-audmix", + .data = "imx-audmix", + }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, fsl_audmix_ids); + static int fsl_audmix_probe(struct platform_device *pdev) { struct fsl_audmix *priv; struct resource *res; + const char *mdrv; + const struct of_device_id *of_id; void __iomem *regs; int ret; - const char *sprop; + + of_id = of_match_device(fsl_audmix_ids, &pdev->dev); + if (!of_id || !of_id->data) + return -EINVAL; + + mdrv = of_id->data;
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); if (!priv) @@ -487,19 +503,12 @@ static int fsl_audmix_probe(struct platform_device *pdev) return ret; }
- sprop = of_get_property(pdev->dev.of_node, "model", NULL); - if (sprop) { - priv->pdev = platform_device_register_data(&pdev->dev, sprop, 0, - NULL, 0); - if (IS_ERR(priv->pdev)) { - ret = PTR_ERR(priv->pdev); - dev_err(&pdev->dev, - "failed to register platform %s: %d\n", sprop, - ret); - } - } else { - dev_err(&pdev->dev, "[model] attribute missing.\n"); - ret = -EINVAL; + priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL, + 0); + if (IS_ERR(priv->pdev)) { + ret = PTR_ERR(priv->pdev); + dev_err(&pdev->dev, "failed to register platform %s: %d\n", + mdrv, ret); }
return ret; @@ -553,12 +562,6 @@ static const struct dev_pm_ops fsl_audmix_pm = { pm_runtime_force_resume) };
-static const struct of_device_id fsl_audmix_ids[] = { - { .compatible = "fsl,imx8qm-audmix", }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(of, fsl_audmix_ids); - static struct platform_driver fsl_audmix_driver = { .probe = fsl_audmix_probe, .remove = fsl_audmix_remove,
Remove "model" attribute.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Acked-by: Nicolin Chen nicoleotsuka@gmail.com --- Documentation/devicetree/bindings/sound/fsl,audmix.txt | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl,audmix.txt b/Documentation/devicetree/bindings/sound/fsl,audmix.txt index 45f807e..840b7e0 100644 --- a/Documentation/devicetree/bindings/sound/fsl,audmix.txt +++ b/Documentation/devicetree/bindings/sound/fsl,audmix.txt @@ -38,9 +38,6 @@ Device driver required properties: to SAI interfaces to be provided, the first SAI in the list being used to route the AUDMIX output.
- - model : Must contain machine driver name which will configure - and instantiate the appropriate audio card. - Device driver configuration example: ====================================== audmix: audmix@59840000 { @@ -50,5 +47,4 @@ Device driver configuration example: clock-names = "ipg"; power-domains = <&pd_audmix>; dais = <&sai4>, <&sai5>; - model = "imx-audmix"; };
On Wed, Apr 10, 2019 at 10:37:30AM +0000, Viorel Suman wrote:
Remove "model" attribute.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Acked-by: Nicolin Chen nicoleotsuka@gmail.com
Please use subject lines matching the style for the subsystem. This makes it easier for people to identify relevant patches.
Hi Mark,
On Mi, 2019-04-10 at 11:39 +0100, Mark Brown wrote:
On Wed, Apr 10, 2019 at 10:37:30AM +0000, Viorel Suman wrote:
Remove "model" attribute.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Acked-by: Nicolin Chen nicoleotsuka@gmail.com
Please use subject lines matching the style for the subsystem. This makes it easier for people to identify relevant patches.
Sure, thank you, do I have to send V4 with subject fixed ?
Release the reference to the underlying device taken by of_find_device_by_node() call.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Reported-by: Julia Lawall Julia.Lawall@lip6.fr Acked-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/imx-audmix.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/fsl/imx-audmix.c b/sound/soc/fsl/imx-audmix.c index 7983bd3..9aaf3e5 100644 --- a/sound/soc/fsl/imx-audmix.c +++ b/sound/soc/fsl/imx-audmix.c @@ -171,6 +171,7 @@ static int imx_audmix_probe(struct platform_device *pdev) np->full_name); return -EINVAL; } + put_device(&audmix_pdev->dev);
num_dai = of_count_phandle_with_args(audmix_np, "dais", NULL); if (num_dai != FSL_AUDMIX_MAX_DAIS) { @@ -216,6 +217,7 @@ static int imx_audmix_probe(struct platform_device *pdev) dev_err(&pdev->dev, "failed to find SAI platform device\n"); return -EINVAL; } + put_device(&cpu_pdev->dev);
dai_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s%s", fe_name_pref, args.np->full_name + 1); @@ -280,6 +282,8 @@ static int imx_audmix_probe(struct platform_device *pdev) dev_err(&pdev->dev, "failed to find SAI platform device\n"); return -EINVAL; } + put_device(&cpu_pdev->dev); + priv->cpu_mclk = devm_clk_get(&cpu_pdev->dev, "mclk1"); if (IS_ERR(priv->cpu_mclk)) { ret = PTR_ERR(priv->cpu_mclk);
There should be no trouble to understand dev = pdev->dev. This can save some space to have more print info or save some wrapped lines.
Signed-off-by: Viorel Suman viorel.suman@nxp.com Suggested-by: Nicolin Chen nicoleotsuka@gmail.com --- sound/soc/fsl/fsl_audmix.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/sound/soc/fsl/fsl_audmix.c b/sound/soc/fsl/fsl_audmix.c index dc802d5..2d10d8b 100644 --- a/sound/soc/fsl/fsl_audmix.c +++ b/sound/soc/fsl/fsl_audmix.c @@ -456,6 +456,7 @@ MODULE_DEVICE_TABLE(of, fsl_audmix_ids);
static int fsl_audmix_probe(struct platform_device *pdev) { + struct device *dev = &pdev->dev; struct fsl_audmix *priv; struct resource *res; const char *mdrv; @@ -463,51 +464,50 @@ static int fsl_audmix_probe(struct platform_device *pdev) void __iomem *regs; int ret;
- of_id = of_match_device(fsl_audmix_ids, &pdev->dev); + of_id = of_match_device(fsl_audmix_ids, dev); if (!of_id || !of_id->data) return -EINVAL;
mdrv = of_id->data;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) return -ENOMEM;
/* Get the addresses */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - regs = devm_ioremap_resource(&pdev->dev, res); + regs = devm_ioremap_resource(dev, res); if (IS_ERR(regs)) return PTR_ERR(regs);
- priv->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "ipg", regs, + priv->regmap = devm_regmap_init_mmio_clk(dev, "ipg", regs, &fsl_audmix_regmap_config); if (IS_ERR(priv->regmap)) { - dev_err(&pdev->dev, "failed to init regmap\n"); + dev_err(dev, "failed to init regmap\n"); return PTR_ERR(priv->regmap); }
- priv->ipg_clk = devm_clk_get(&pdev->dev, "ipg"); + priv->ipg_clk = devm_clk_get(dev, "ipg"); if (IS_ERR(priv->ipg_clk)) { - dev_err(&pdev->dev, "failed to get ipg clock\n"); + dev_err(dev, "failed to get ipg clock\n"); return PTR_ERR(priv->ipg_clk); }
platform_set_drvdata(pdev, priv); - pm_runtime_enable(&pdev->dev); + pm_runtime_enable(dev);
- ret = devm_snd_soc_register_component(&pdev->dev, &fsl_audmix_component, + ret = devm_snd_soc_register_component(dev, &fsl_audmix_component, fsl_audmix_dai, ARRAY_SIZE(fsl_audmix_dai)); if (ret) { - dev_err(&pdev->dev, "failed to register ASoC DAI\n"); + dev_err(dev, "failed to register ASoC DAI\n"); return ret; }
- priv->pdev = platform_device_register_data(&pdev->dev, mdrv, 0, NULL, - 0); + priv->pdev = platform_device_register_data(dev, mdrv, 0, NULL, 0); if (IS_ERR(priv->pdev)) { ret = PTR_ERR(priv->pdev); - dev_err(&pdev->dev, "failed to register platform %s: %d\n", + dev_err(dev, "failed to register platform %s: %d\n", mdrv, ret); }
participants (2)
-
Mark Brown -
Viorel Suman