On Sat, Dec 14, 2013 at 10:26:10PM +0800, Wei Yongjun wrote: > From: Wei Yongjun <[email protected]> > > Add the missing clk_disable_unprepare() before return in > the error handling cases. > > Signed-off-by: Wei Yongjun <[email protected]> > --- > 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?
signature.asc
Description: Digital signature
