Re: [PATCH v8 4/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 1 potential issue(s) to consider:
- [High] Hardware configuration mismatch for active-high interrupts
--

commit cc23812be1f638568d4c39e30f713017f6ba0b47
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, and should only potentially cause
problems if systems exist where firmware specifies an incorrect trigger
type. With a bit of luck, there are no such systems.

> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 6c5a8d65c8a42..05851274596e5 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1665,9 +1665,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: High]
Will this cause a hardware configuration mismatch if the firmware specifies
an active-high or edge-rising interrupt?

In mma8452_probe(), when the firmware specifies an active-high interrupt
(e.g., IRQ_TYPE_LEVEL_HIGH or IRQ_TYPE_EDGE_RISING), this code passes the
active-high flag to request_threaded_irq() and configures the host
interrupt controller accordingly.

However, the MMA8452 sensor hardware defaults to an active-low output.
Without explicitly configuring the IPOL (Interrupt Polarity) bit in the
sensor's CTRL_REG3 register (0x2C) to match the requested firmware trigger
type, the sensor will continue driving an active-low signal.

This mismatch will break interrupts for systems that correctly specify an
active-high trigger.

It may also regress systems with incorrectly specified active-high device
trees that previously worked because the driver hardcoded IRQF_TRIGGER_LOW.

>  		if (ret)
>  			goto buffer_cleanup;
>  	}

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