On 1 August 2012 04:51, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote: > On Wednesday, August 01, 2012 1:39 AM Damien Cassou wrote: >> >> From: Damien Cassou <damien.cassou@xxxxxxx> >> >> 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 <damien.cassou@xxxxxxx> >> >> --- >> 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. 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@xxxxxxxxxxxxxxx > 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 -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html