Re: [PATCH v8 7/9] iio: accel: mma8452: Allow open drain interrupt pin configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware configuration context is lost across runtime suspend/resume cycles involving power loss.
- [High] Raw reads fail when the device is idle due to waiting for data ready before resuming the device.
--

commit 74793c7cf1159fd96130d4ae4c8de1b7780ff4b3
Author: Esben Haabendal <esben@geanix.com>

    iio: accel: mma8452: Allow open drain interrupt pin configuration
    
    When designing systems sharing the interrupt for mma8452 chips, it is
    helpful to be able to configure the irq pin in open-drain mode (default is
    push-pull).

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 2c1b97b77bc19..b5ab85a86e72a 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c

[ ... ]

> @@ -1792,6 +1816,10 @@ static int mma8452_runtime_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	ret = mma8452_set_interrupt_pin_mode(data);
> +	if (ret)
> +		goto runtime_resume_failed;
> +
>  	ret = mma8452_active(data);

[Severity: High]
This isn't a bug introduced by this patch, but does mma8452_runtime_resume()
fail to restore the hardware configuration after regulators are turned off
during suspend?

The device autosuspends after 2 seconds of inactivity, which disables the
VDD/VDDIO regulators via regulator_bulk_disable(). When it resumes,
mma8452_runtime_resume() appears to only restore CTRL_REG1 (via
mma8452_active()), and now CTRL_REG3, permanently dropping the rest of the
configuration.

Will the sensor lose settings like full-scale range (DATA_CFG), interrupt
routing (CTRL_REG5), and threshold configurations (TRANSIENT_THS) since
they revert to factory defaults upon power loss?

[ ... ]

While reviewing this driver, I noticed another pre-existing issue:

drivers/iio/accel/mma8452.c:mma8452_read() {
...
	int ret = mma8452_drdy(data);

	if (ret < 0)
		return ret;

	ret = mma8452_set_runtime_pm_state(data->client, true);
...
}

[Severity: High]
This is a pre-existing issue, but will raw sysfs reads fail when the device
is idle?

The mma8452_set_runtime_pm_state() call to wake the device is placed after
the polling loop for data ready in mma8452_drdy().

If the device has been idle for 2 seconds and entered autosuspend, the
chip is in STANDBY (or completely unpowered). Because of this, the Data
Ready (DRDY) bit is never set. The function times out after polling and
aborts the read before it ever attempts to wake up the device.

Should the device be resumed before polling for data ready?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-mma8452-open-drain-v8-0-c17407e22118@geanix.com?part=7