Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752048AbbHQHRt (ORCPT ); Mon, 17 Aug 2015 03:17:49 -0400 Received: from mail-ob0-f169.google.com ([209.85.214.169]:36638 "EHLO mail-ob0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750847AbbHQHRr (ORCPT ); Mon, 17 Aug 2015 03:17:47 -0400 MIME-Version: 1.0 In-Reply-To: <20150813072920.GA17181@localhost> References: <20150813072920.GA17181@localhost> From: Alexandre Courbot Date: Mon, 17 Aug 2015 16:17:27 +0900 Message-ID: Subject: Re: [PATCH] ASoC: tegra: Convert to managed resources To: Vaishali Thakkar Cc: Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , Stephen Warren , Thierry Reding , Wolfram Sang , "alsa-devel@alsa-project.org" , "linux-tegra@vger.kernel.org" , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4894 Lines: 118 On Thu, Aug 13, 2015 at 4:29 PM, Vaishali Thakkar 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 > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/