Hi Uwe,
thank you for your work.
On Wed, Jul 20, 2022 at 05:09:33PM +0200, Uwe Kleine-König wrote:
> If for whatever reasons pm_runtime_resume_and_get() fails and .remove() is
> exited early, the i2c adapter stays around and the irq still calls its
> handler, while the driver data and the register mapping go away. So if
> later the i2c adapter is accessed or the irq triggers this results in
> havoc accessing freed memory and unmapped registers.
>
> So unregister the software resources even if resume failed, and only skip
> the hardware access in that case.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Can you please add Fixes tag. I assume this patch can got to stable
kernel version too.
Otherwise:
Acked-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> ---
> drivers/i2c/busses/i2c-imx.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index e9e2db68b9fb..7395560c13d0 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1572,9 +1572,7 @@ static int i2c_imx_remove(struct platform_device *pdev)
> struct imx_i2c_struct *i2c_imx = platform_get_drvdata(pdev);
> int irq, ret;
>
> - ret = pm_runtime_resume_and_get(&pdev->dev);
> - if (ret < 0)
> - return ret;
> + ret = pm_runtime_get_sync(&pdev->dev);
>
> hrtimer_cancel(&i2c_imx->slave_timer);
>
> @@ -1585,17 +1583,21 @@ static int i2c_imx_remove(struct platform_device *pdev)
> if (i2c_imx->dma)
> i2c_imx_dma_free(i2c_imx);
>
> - /* setup chip registers to defaults */
> - imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
> - imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
> - imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
> - imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
> + if (ret == 0) {
> + /* setup chip registers to defaults */
> + imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IADR);
> + imx_i2c_write_reg(0, i2c_imx, IMX_I2C_IFDR);
> + imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2CR);
> + imx_i2c_write_reg(0, i2c_imx, IMX_I2C_I2SR);
> + clk_disable(i2c_imx->clk);
> + }
>
> clk_notifier_unregister(i2c_imx->clk, &i2c_imx->clk_change_nb);
> irq = platform_get_irq(pdev, 0);
> if (irq >= 0)
> free_irq(irq, i2c_imx);
> - clk_disable_unprepare(i2c_imx->clk);
> +
> + clk_unprepare(i2c_imx->clk);
>
> pm_runtime_put_noidle(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> base-commit: 6014cfa5bf32cf8c5c58b3cfd5ee0e1542c8a825
> --
> 2.36.1
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
