Re: [PATCH RESEND v5 3/6] iio: accel: mma8452: Only apply trigger type when not set by firmware

[sashiko-bot@kernel.org]

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

New issues:
- [High] The patch dynamically trusts the firmware's IRQ trigger type but fails to configure the sensor hardware's interrupt polarity to match, causing an interrupt storm.

Pre-existing issues:
- [High] I2C register accesses are performed while the device is physically powered off in PM runtime suspend.
- [High] Unhandled latched motion events (`INT_FF_MT`) cause an infinite IRQ storm.
--

--- Patch [3]: [PATCH RESEND v5 3/6] iio: accel: mma8452: Only apply trigger type when not set by firmware ---
commit ad034b1db299a86a70db53d5a2b4a52f2a134eee
Author: Esben Haabendal <esben@geanix.com>
iio: accel: mma8452: Only apply trigger type when not set by firmware

Instead of unconditionally overriding the trigger type, it is better to
only apply a default when no trigger type is set by firmware. This should
be reasonably backward compatible.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index f645a5c6fd1c2..1fb43c5b0b72f 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1683,9 +1683,16 @@ static int mma8452_probe(struct i2c_client *client)
>  		goto trigger_cleanup;
>  
>  	if (client->irq) {
> +		unsigned long irq_flags;
> +
> +		irq_flags = irq_get_trigger_type(client->irq);

[Severity: High]
If the firmware specifies an active-high interrupt, will this cause an
interrupt storm?

The irq_get_trigger_type function fetches the interrupt polarity from the
firmware (e.g., active-high) and passes it to request_threaded_irq.
However, the driver forces the sensor into its power-on default state
via mma8452_reset, which hardcodes the interrupt pin to active-low.

Because the driver never writes to CTRL_REG3 to set the IPOL bit
to active-high to match the requested IRQ type, the sensor's idle-high
line could be seen as a permanently asserted interrupt.

> +		if (irq_flags == IRQ_TYPE_NONE) {
> +			dev_info(dev, "invalid irq type, setting default active low\n");
> +			irq_flags = IRQF_TRIGGER_LOW;
> +		}
> +		irq_flags |= IRQF_ONESHOT;
>  		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
> -					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -					   client->name, indio_dev);
> +					   irq_flags, client->name, indio_dev);
>  		if (ret)
>  			goto buffer_cleanup;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but are there I2C register
accesses occurring while the device is physically powered off?

If userspace attempts to read or write sysfs attributes after the
autosuspend timeout, the regulators are disabled. Direct calls are
made without resuming the device:

drivers/iio/accel/mma8452.c:mma8452_read_raw() {
    ...
    ret = i2c_smbus_read_byte_data(data->client,
                                   MMA8452_OFF_X + chan->scan_index);
    ...
}

A similar pattern happens in __mma8452_write_raw calling
mma8452_change_config. Any unprivileged userspace read/write to the
configuration attributes 2 seconds after the last activity will result
in an I2C NACK (-EREMOTEIO or -ENXIO).

[Severity: High]
This is a pre-existing issue, but can unhandled latched motion events
cause an infinite IRQ storm?

For chips that lack the TRANS event (like mma8652), rising-edge motion
events are mapped to the ff_mt registers. When the latch is explicitly
enabled for rising events, the hardware will hold the interrupt until
read:

drivers/iio/accel/mma8452.c:mma8452_write_event_config() {
    ...
    val |= ev_regs->ev_cfg_ele;
    ...
}

When the interrupt fires, the handler evaluates freefall mode and skips
handling, never reading the MMA8452_FF_MT_SRC register to clear the latch:

drivers/iio/accel/mma8452.c:mma8452_interrupt() {
    ...
    if (src & MMA8452_INT_FF_MT) {
        if (mma8452_freefall_mode_enabled(data)) {
            ...
        }
        ret = IRQ_HANDLED;
    }
    ...
}

Remotely triggering a physical motion event after enabling the rising
threshold via sysfs could permanently lock up the interrupt line.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-mma8452-open-drain-v5-0-d8aa590d7c36@geanix.com?part=3