| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Apr 2014 15:18:36 -0700
From: Doug Anderson <dianders@...omium.org>
To: Naveen Krishna Chatradhi <ch.naveen@...sung.com>
Cc: linux-iio <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Naveen Krishna <naveenkrishna.ch@...il.com>,
Lars-Peter Clausen <lars@...afoo.de>,
"cpgs ." <cpgs@...sung.com>,
Grant Grundler <grundler@...omium.org>,
Jonathan Cameron <jic23@...nel.org>
Subject: Re: [PATCH 2/5 v2] iio: exynos_adc: rearrange clk and regulator
enable/disable calls
Naveen,
On Sat, Apr 26, 2014 at 4:37 AM, Naveen Krishna Chatradhi
<ch.naveen@...sung.com> wrote:
> From: Naveen Krishna Ch <ch.naveen@...sung.com>
>
> This patch maintains the following order in
> probe(), remove(), resume() and suspend() calls
>
> regulator enable, clk prepare enable
> ...
> clk disable unprepare, regulator disable
>
> While at it,
> 1. enable the regulator before the iio_device_register()
> 2. handle the return values for enable/disable calls
>
> Signed-off-by: Naveen Krishna Ch <ch.naveen@...sung.com>
> ---
> Changes since v1:
> corrected the register/unregister and enabling/disbaling sequences
>
> drivers/iio/adc/exynos_adc.c | 63 +++++++++++++++++++++++-------------------
> 1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index affa93f..0df8916 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -290,32 +290,30 @@ static int exynos_adc_probe(struct platform_device *pdev)
>
> init_completion(&info->completion);
>
> - ret = request_irq(info->irq, exynos_adc_isr,
> - 0, dev_name(&pdev->dev), info);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> - info->irq);
> - return ret;
> - }
> -
> - writel(1, info->enable_reg);
> -
> info->clk = devm_clk_get(&pdev->dev, "adc");
> if (IS_ERR(info->clk)) {
> dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> PTR_ERR(info->clk));
> - ret = PTR_ERR(info->clk);
> - goto err_irq;
> + return PTR_ERR(info->clk);
> }
>
> info->vdd = devm_regulator_get(&pdev->dev, "vdd");
> if (IS_ERR(info->vdd)) {
> dev_err(&pdev->dev, "failed getting regulator, err = %ld\n",
> PTR_ERR(info->vdd));
> - ret = PTR_ERR(info->vdd);
> - goto err_irq;
> + return PTR_ERR(info->vdd);
> }
>
> + ret = regulator_enable(info->vdd);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(info->clk);
> + if (ret)
> + goto err_disable_reg;
> +
> + writel(1, info->enable_reg);
> +
> info->version = exynos_adc_get_version(pdev);
>
> platform_set_drvdata(pdev, indio_dev);
> @@ -332,16 +330,18 @@ static int exynos_adc_probe(struct platform_device *pdev)
> else
> indio_dev->num_channels = MAX_ADC_V2_CHANNELS;
>
> + ret = request_irq(info->irq, exynos_adc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq, irq = %d\n",
> + info->irq);
> + goto err_disable_clk;
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret)
> goto err_irq;
>
> - ret = regulator_enable(info->vdd);
> - if (ret)
> - goto err_iio_dev;
> -
> - clk_prepare_enable(info->clk);
> -
> exynos_adc_hw_init(info);
>
> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
> @@ -355,12 +355,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
> err_of_populate:
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> - regulator_disable(info->vdd);
> - clk_disable_unprepare(info->clk);
> -err_iio_dev:
> iio_device_unregister(indio_dev);
> err_irq:
> free_irq(info->irq, info);
> +err_disable_clk:
> + writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> +err_disable_reg:
> + regulator_disable(info->vdd);
> return ret;
> }
>
> @@ -371,11 +373,12 @@ static int exynos_adc_remove(struct platform_device *pdev)
>
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> - regulator_disable(info->vdd);
> - clk_disable_unprepare(info->clk);
> - writel(0, info->enable_reg);
> iio_device_unregister(indio_dev);
> free_irq(info->irq, info);
> + writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> + regulator_disable(info->vdd);
> +
nit: one too many blank lines here
> return 0;
> }
> @@ -397,8 +400,8 @@ static int exynos_adc_suspend(struct device *dev)
> writel(con, ADC_V1_CON(info->regs));
> }
>
> - clk_disable_unprepare(info->clk);
> writel(0, info->enable_reg);
> + clk_disable_unprepare(info->clk);
> regulator_disable(info->vdd);
>
> return 0;
> @@ -414,9 +417,11 @@ static int exynos_adc_resume(struct device *dev)
> if (ret)
> return ret;
>
> - writel(1, info->enable_reg);
> - clk_prepare_enable(info->clk);
> + ret = clk_prepare_enable(info->clk);
> + if (ret)
> + return ret;
>
> + writel(1, info->enable_reg);
> exynos_adc_hw_init(info);
>
> return 0;
> --
> 1.7.9.5
>
Other than nit, looks good to me.
Reviewed-by: Doug Anderson <dianders@...omium.org>
--
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