Re: [PATCH] i2c: imx-lpi2c: reset controller in probe stage

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving devm_request_irq() after PM runtime initialization causes a PM runtime usage counter and clock leak if the IRQ request fails.
--

commit 3005ae3d3fdbe3e9115c8d88256efdcb9d2fcde9
Author: Carlos Song <carlos.song@nxp.com>

i2c: imx-lpi2c: reset controller in probe stage

This commit resets the I2C controller in the probe stage to avoid an
unexpected LPI2C controller state left from previous stages that could hang
system boot.

> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index a01c236964819..da40b365a07f0 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
[ ... ]
> @@ -1535,10 +1530,23 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	/*
> +	 * Reset all internal controller logic and registers to avoid effects of previous status
> +	 * The reset takes effect immediately and there is no minimum delay required before
> +	 * clearing the software reset.
> +	 */
> +	writel(MCR_RST, lpi2c_imx->base + LPI2C_MCR);
> +	writel(0, lpi2c_imx->base + LPI2C_MCR);
> +
>  	temp = readl(lpi2c_imx->base + LPI2C_PARAM);
>  	lpi2c_imx->txfifosize = 1 << (temp & 0x0f);
>  	lpi2c_imx->rxfifosize = 1 << ((temp >> 8) & 0x0f);
>  
> +	ret = devm_request_irq(&pdev->dev, lpi2c_imx->irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
> +			       pdev->name, lpi2c_imx);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", lpi2c_imx->irq);

Does this code leak the PM runtime usage counter and hardware clocks if the
IRQ request fails?

Because this block was moved after pm_runtime_enable() and
pm_runtime_set_active(), a direct return bypasses the rpm_disable cleanup
path.

Should this instead jump to rpm_disable rather than returning directly, so
that pm_runtime_put_sync() and pm_runtime_disable() can run and disable the
clocks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519104214.2794176-1-carlos.song@oss.nxp.com?part=1