[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAVeFuKT1st8E1fxcDM5CV_OzUcTmvFw8UuU5oh15J1PiFJ0vA@mail.gmail.com>
Date: Mon, 17 Aug 2015 16:17:27 +0900
From: Alexandre Courbot <gnurou@...il.com>
To: Vaishali Thakkar <vthakkar1994@...il.com>
Cc: Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
Stephen Warren <swarren@...dotorg.org>,
Thierry Reding <thierry.reding@...il.com>,
Wolfram Sang <wsa@...-dreams.de>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ASoC: tegra: Convert to managed resources
On Thu, Aug 13, 2015 at 4:29 PM, Vaishali Thakkar
<vthakkar1994@...il.com> wrote:
> Use managed resource functions devm_clk_put and
> devm_snd_soc_register_component to simplify error handling.
>
> To be compatible with the change various gotos are replaced
> with direct returns, and unneeded labels are dropped.
>
> Signed-off-by: Vaishali Thakkar <vthakkar1994@...il.com>
> ---
> sound/soc/tegra/tegra20_spdif.c | 37 +++++++++++++------------------------
> 1 file changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra20_spdif.c b/sound/soc/tegra/tegra20_spdif.c
> index 9141477..f69b2e4 100644
> --- a/sound/soc/tegra/tegra20_spdif.c
> +++ b/sound/soc/tegra/tegra20_spdif.c
> @@ -273,45 +273,40 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
> GFP_KERNEL);
> if (!spdif) {
> dev_err(&pdev->dev, "Can't allocate tegra20_spdif\n");
> - ret = -ENOMEM;
> - goto err;
> + return -ENOMEM;
> }
> dev_set_drvdata(&pdev->dev, spdif);
>
> - spdif->clk_spdif_out = clk_get(&pdev->dev, "spdif_out");
> + spdif->clk_spdif_out = devm_clk_get(&pdev->dev, "spdif_out");
> if (IS_ERR(spdif->clk_spdif_out)) {
> pr_err("Can't retrieve spdif clock\n");
> ret = PTR_ERR(spdif->clk_spdif_out);
> - goto err;
> + return ret;
Maybe do "return PTR_ERR(spdif->clk_spdif_out);" for consistency with
the other error cases of this function?
> }
>
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!mem) {
> dev_err(&pdev->dev, "No memory resource\n");
> - ret = -ENODEV;
> - goto err_clk_put;
> + return -ENODEV;
> }
>
> dmareq = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> if (!dmareq) {
> dev_err(&pdev->dev, "No DMA resource\n");
> - ret = -ENODEV;
> - goto err_clk_put;
> + return -ENODEV;
> }
>
> memregion = devm_request_mem_region(&pdev->dev, mem->start,
> resource_size(mem), DRV_NAME);
> if (!memregion) {
> dev_err(&pdev->dev, "Memory region already claimed\n");
> - ret = -EBUSY;
> - goto err_clk_put;
> + return -EBUSY;
> }
>
> regs = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
> if (!regs) {
> dev_err(&pdev->dev, "ioremap failed\n");
> - ret = -ENOMEM;
> - goto err_clk_put;
> + return -ENOMEM;
> }
>
> spdif->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> @@ -319,7 +314,7 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
> if (IS_ERR(spdif->regmap)) {
> dev_err(&pdev->dev, "regmap init failed\n");
> ret = PTR_ERR(spdif->regmap);
> - goto err_clk_put;
> + return ret;
Same here.
> }
>
> spdif->playback_dma_data.addr = mem->start + TEGRA20_SPDIF_DATA_OUT;
> @@ -334,8 +329,9 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
> goto err_pm_disable;
> }
>
> - ret = snd_soc_register_component(&pdev->dev, &tegra20_spdif_component,
> - &tegra20_spdif_dai, 1);
> + ret = devm_snd_soc_register_component(&pdev->dev,
> + &tegra20_spdif_component,
> + &tegra20_spdif_dai, 1);
> if (ret) {
> dev_err(&pdev->dev, "Could not register DAI: %d\n", ret);
> ret = -ENOMEM;
> @@ -345,21 +341,17 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
> ret = tegra_pcm_platform_register(&pdev->dev);
> if (ret) {
> dev_err(&pdev->dev, "Could not register PCM: %d\n", ret);
> - goto err_unregister_component;
> + return ret;
In the previous code, PM cleanup was also performed after the
component was unregistered. If you return directly, this is not
performed anymore - I think you should "goto err_suspend;" here.
This will change the ordering of cleanup operations though - e.g.
snd_soc_unregister_component() will now be called *after* PM cleanup.
Are things still working ok after this? (I suppose they do considering
how simple the PM ops are, but it might be worth testing).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists