On Sat, Dec 14, 2013 at 10:26:10PM +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx>
>
> Add the missing clk_disable_unprepare() before return in
> the error handling cases.
>
> Signed-off-by: Wei Yongjun <yongjun_wei@xxxxxxxxxxxxxxxxx>
> ---
> drivers/i2c/busses/i2c-imx.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index d0cfbb4..e62f3be 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -328,8 +328,10 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
> temp |= I2CR_MSTA;
> imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> result = i2c_imx_bus_busy(i2c_imx, 1);
> - if (result)
> + if (result) {
> + clk_disable_unprepare(i2c_imx->clk);
> return result;
> + }
> i2c_imx->stopped = 0;
Nice catch, but that is not enough. The logic between i2x_imx_start and
*_stop is still broken. *_start may return with clocks enabled or not
while stop will unconditionally disables them.
>
> temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
> @@ -654,7 +656,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
> pdev->name, i2c_imx);
> if (ret) {
> dev_err(&pdev->dev, "can't claim irq %d\n", irq);
> - return ret;
> + goto err_out;
> }
>
> /* Init queue */
> @@ -680,7 +682,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
> ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
> if (ret < 0) {
> dev_err(&pdev->dev, "registration failed\n");
> - return ret;
> + goto err_out;
> }
>
> /* Set up platform driver data */
> @@ -697,6 +699,9 @@ static int i2c_imx_probe(struct platform_device *pdev)
> dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
>
> return 0; /* Return OK */
> +err_out:
> + clk_disable_unprepare(i2c_imx->clk);
> + return ret;
Why not just enabling the clocks around the small codeblock that needs
them?
Attachment:
signature.asc
Description: Digital signature
