Re: [PATCH 6/6] i2c: xiic: remove redundant pm_runtime_dont_use_autosuspend() calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] i2c: xiic: premature IRQ registration in xiic_i2c_probe() [1]
--

commit 188337d85ab0ac1fad4560f785b36f1dce4f451f
Author: Joshua Crofts <crofts@kernel.org>

i2c: xiic: remove redundant pm_runtime_dont_use_autosuspend() calls

This commit removes redundant pm_runtime_dont_use_autosuspend() calls from
the probe error path and remove callback in the xiic I2C driver.

> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index ab7916745ad39..4e06fa4f73d77 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -1580,7 +1580,6 @@ static int xiic_i2c_probe(struct platform_device *pdev)
>  err_pm_disable:
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
> -	pm_runtime_dont_use_autosuspend(dev);
>  
>  	return ret;
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can registering the IRQ
in xiic_i2c_probe() before detecting hardware endianness cause an
interrupt storm [1]?

drivers/i2c/busses/i2c-xiic.c:xiic_i2c_probe() {
    ...
	ret = request_threaded_irq(irq, NULL, xiic_process, IRQF_ONESHOT,
				   pdev->name, i2c);
	if (ret)
		goto err_pm_disable;

	i2c->singlemaster = device_property_read_bool(dev, "single-master");

	/*
	 * Detect endianness
	 * Try to reset the TX FIFO. Then check the EMPTY flag. If it is not
	 * set, assume that the endianness was wrong and swap.
	 */
	i2c->endianness = LITTLE;
	xiic_setreg32(i2c, XIIC_CR_REG_OFFSET, XIIC_CR_TX_FIFO_RESET_MASK);
	/* Reset is cleared in xiic_reinit */
	sr = xiic_getreg32(i2c, XIIC_SR_REG_OFFSET);
	if (!(sr & XIIC_SR_TX_FIFO_EMPTY_MASK))
		i2c->endianness = BIG;
    ...
}

If a pending interrupt remains from an earlier boot stage on a big-endian
system, xiic_process() can fire immediately after request_threaded_irq()
while i2c->endianness is still zero-initialized to LITTLE.  xiic_getreg32()
and xiic_setreg32() then use little-endian accessors, failing to clear the
interrupt and hanging the system in an unhandled IRQ storm.  Detecting
endianness and running xiic_reinit() before requesting the IRQ would avoid
this.

[1] https://sashiko.dev/#/bug/linux-6466439c-c62b-471e-ba38-0219e368df95

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-i2c-remove-dont-use-autosuspend-v1-0-49a08b8daf36@kernel.org?part=6