Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751636Ab2HAEib (ORCPT ); Wed, 1 Aug 2012 00:38:31 -0400 Received: from mail-vb0-f46.google.com ([209.85.212.46]:58572 "EHLO mail-vb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751157Ab2HAEi3 (ORCPT ); Wed, 1 Aug 2012 00:38:29 -0400 MIME-Version: 1.0 In-Reply-To: <003e01cd6f9e$5a1146d0$0e33d470$%han@samsung.com> References: <1343752762-16861-1-git-send-email-damien.cassou@lifl.fr> <1343752762-16861-6-git-send-email-damien.cassou@lifl.fr> <002c01cd6f73$4252b090$c6f811b0$%han@samsung.com> <003e01cd6f9e$5a1146d0$0e33d470$%han@samsung.com> Date: Wed, 1 Aug 2012 10:08:28 +0530 Message-ID: Subject: Re: [PATCH 5/5] drivers/video/exynos/exynos_dp_core.c: use devm_ functions From: Sachin Kamat To: Jingoo Han Cc: Damien Cassou , kernel-janitors@vger.kernel.org, Florian Tobias Schandinat , linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6118 Lines: 184 On 1 August 2012 10:00, Jingoo Han wrote: > On Wednesday, August 01, 2012 1:00 PMSachin Kamat wrote: >> >> On 1 August 2012 04:51, Jingoo Han wrote: >> > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote: >> >> >> >> From: Damien Cassou >> >> >> >> The various devm_ functions allocate memory that is released when a driver >> >> detaches. This patch uses these functions for data that is allocated in >> >> the probe function of a platform device and is only freed in the remove >> >> function. >> >> >> >> Signed-off-by: Damien Cassou >> >> >> >> --- >> >> drivers/video/exynos/exynos_dp_core.c | 27 +++++++-------------------- >> >> 1 file changed, 7 insertions(+), 20 deletions(-) >> >> >> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c >> >> index c6c016a..00fe4f0 100644 >> >> --- a/drivers/video/exynos/exynos_dp_core.c >> >> +++ b/drivers/video/exynos/exynos_dp_core.c >> >> @@ -872,7 +872,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev) >> >> >> >> dp->dev = &pdev->dev; >> >> >> >> - dp->clock = clk_get(&pdev->dev, "dp"); >> >> + dp->clock = devm_clk_get(&pdev->dev, "dp"); >> >> if (IS_ERR(dp->clock)) { >> >> dev_err(&pdev->dev, "failed to get clock\n"); >> >> return PTR_ERR(dp->clock); >> >> @@ -881,31 +881,24 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev) >> >> clk_enable(dp->clock); >> >> >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> >> - if (!res) { >> >> - dev_err(&pdev->dev, "failed to get registers\n"); >> >> - ret = -EINVAL; >> >> - goto err_clock; >> >> - } >> > >> > Why do you remove this return check? >> > If there is no reason, please, do it as follows: >> > >> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> > if (!res) { >> > dev_err(&pdev->dev, "failed to get registers\n"); >> > - ret = -EINVAL; >> > - goto err_clock; >> > + return -EINVAL; >> > } >> > >> > >> >> devm_request_and_ioremap function checks the validity of res. Hence >> this check above is redundant and can be removed. > > > I don't think so. > Even though function called next checks the NULL value, > for robustness, the return value of platform_get_resource() should be > checked. > > It is possible that devm_request_and_ioremap() can be changed in the future, > as request_mem_region() & ioremap() were changed to devm_request_and_ioremap(). They are not changed. They still exist. devm_request_and_ioremap() is an additional function provided for device managed resources. > > > Best regards, > Jingoo Han > > >> >> Damien, >> This patch only adds devm_clk_get() function. Hence you could make the >> subject line more specific. >> >> >> >> >> > Best regards, >> > Jingoo Han >> > >> > >> >> >> >> dp->reg_base = devm_request_and_ioremap(&pdev->dev, res); >> >> if (!dp->reg_base) { >> >> dev_err(&pdev->dev, "failed to ioremap\n"); >> >> - ret = -ENOMEM; >> >> - goto err_clock; >> >> + return -ENOMEM; >> >> } >> >> >> >> dp->irq = platform_get_irq(pdev, 0); >> >> if (!dp->irq) { >> >> dev_err(&pdev->dev, "failed to get irq\n"); >> >> - ret = -ENODEV; >> >> - goto err_clock; >> >> + return -ENODEV; >> >> } >> >> >> >> ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler, 0, >> >> "exynos-dp", dp); >> >> if (ret) { >> >> dev_err(&pdev->dev, "failed to request irq\n"); >> >> - goto err_clock; >> >> + return ret; >> >> } >> >> >> >> dp->video_info = pdata->video_info; >> >> @@ -917,7 +910,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev) >> >> ret = exynos_dp_detect_hpd(dp); >> >> if (ret) { >> >> dev_err(&pdev->dev, "unable to detect hpd\n"); >> >> - goto err_clock; >> >> + return ret; >> >> } >> >> >> >> exynos_dp_handle_edid(dp); >> >> @@ -926,7 +919,7 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev) >> >> dp->video_info->link_rate); >> >> if (ret) { >> >> dev_err(&pdev->dev, "unable to do link train\n"); >> >> - goto err_clock; >> >> + return ret; >> >> } >> >> >> >> exynos_dp_enable_scramble(dp, 1); >> >> @@ -940,17 +933,12 @@ static int __devinit exynos_dp_probe(struct platform_device *pdev) >> >> ret = exynos_dp_config_video(dp, dp->video_info); >> >> if (ret) { >> >> dev_err(&pdev->dev, "unable to config video\n"); >> >> - goto err_clock; >> >> + return ret; >> >> } >> >> >> >> platform_set_drvdata(pdev, dp); >> >> >> >> return 0; >> >> - >> >> -err_clock: >> >> - clk_put(dp->clock); >> >> - >> >> - return ret; >> >> } >> >> >> >> static int __devexit exynos_dp_remove(struct platform_device *pdev) >> >> @@ -962,7 +950,6 @@ static int __devexit exynos_dp_remove(struct platform_device *pdev) >> >> pdata->phy_exit(); >> >> >> >> clk_disable(dp->clock); >> >> - clk_put(dp->clock); >> >> >> >> return 0; >> >> } >> > >> > -- >> > 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/ >> >> >> >> -- >> With warm regards, >> Sachin > -- With warm regards, Sachin -- 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/