Re: [PATCH v6 3/9] 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:
- [Medium] Hardware-Software Polarity Mismatch

Pre-existing issues:
- [High] PM runtime usage counter leaks permanently on I2C read/write errors.
- [High] Hardware state is accessed over I2C before acquiring runtime PM references, failing if the device is suspended.
--

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

iio: accel: mma8452: Only apply trigger type when not set by firmware

This patch modifies the mma8452 driver to only apply a default active-low
trigger type if the firmware does not specify one. This aims to improve
backward compatibility while respecting firmware configuration.

> 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);
> +		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);

[Severity: Medium]
Does this configuration leave the hardware and software out of sync?

The code reads the firmware's IRQ trigger type and requests the IRQ with
it, but the actual MMA8452 sensor hardware polarity doesn't appear to be
updated. If the firmware requests an active-high or rising-edge trigger,
the host interrupt controller is configured for active-high, but the sensor
would continue to drive its default active-low signal.

Unless there is an external hardware inverter on the board, could this 
mismatch break interrupts? Should the driver read the requested trigger type
and configure the IPOL bit in CTRL_REG3 to match?

>  		if (ret)
>  			goto buffer_cleanup;
>  	}


While reviewing the I2C handling in this driver, I also noticed a couple of
other issues.

[Severity: High]
This is a pre-existing issue, but I noticed a potential leak of the runtime
PM usage counter when handling I2C errors.

In mma8452_read(), the device is woken up, incrementing the PM counter:

mma8452_read() {
    ...
    ret = mma8452_set_runtime_pm_state(data->client, true);
    ...
    ret = i2c_smbus_read_i2c_block_data(data->client, ...);
    if (ret < 0)
        return ret;
}

If the I2C read fails, the function returns immediately and bypasses the
matching mma8452_set_runtime_pm_state() false call. Could this permanently
prevent the device from re-entering autosuspend?

A similar pattern appears to exist in mma8452_write_event_config() where
an early return on a failed I2C read skips restoring the PM state.


[Severity: High]
This is a pre-existing issue, but could some I2C accesses fail if the
device is currently suspended?

In mma8452_read(), the code checks for data readiness before waking up the
device:

mma8452_read() {
    ...
    ret = mma8452_drdy(data);
    if (ret < 0)
        return ret;

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

Because mma8452_drdy() executes an I2C read against the MMA8452_STATUS
register, it happens before the power regulators are enabled. If the device
has already autosuspended (after 2 seconds of inactivity), will this I2C
read hit powered-off silicon and fail?

A similar direct I2C read without a runtime PM reference seems to happen
in mma8452_read_raw() for IIO_CHAN_INFO_CALIBBIAS.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825-mma8452-open-drain-v6-0-9b252804ee80@geanix.com?part=3