Re: [PATCH 2/2] iio: accel: mma8452: Allow open drain interrupt pin configuration

[Esben Haabendal <esben@geanix.com>]

"Andy Shevchenko" <andriy.shevchenko@intel.com> writes:

> On Wed, Jul 15, 2026 at 01:35:41PM +0200, Esben Haabendal wrote:
>> "Andy Shevchenko" <andriy.shevchenko@intel.com> writes:
>> > On Wed, Jul 15, 2026 at 10:07:39AM +0200, Esben Haabendal wrote:
>
> ...
>
>> >>  	if (client->irq) {
>> >>  		ret = request_threaded_irq(client->irq, NULL, mma8452_interrupt,
>> >> -					   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
>> >> +					   IRQF_TRIGGER_LOW | IRQF_ONESHOT |
>> >> +					   data->open_drain ? IRQF_SHARED : 0,
>> >>  					   client->name, indio_dev);
>> >
>> > Why do we care?
>>
>> Care about what exactly?
>
> About exclusivity of the interrupt.

Ok.

>> We need to add IRQF_SHARED flag in order to allow shared interrupt, and
>> we should not add it when using (the default) push-pull mode.
>
> Why not? How would it make any difference from SW perspective?

Not adding the IRQF_SHARED flag prevents use with shared interrupts. I
think we are on the same page on that.

Unconditional adding IRQF_SHARED flag would allow configurations where
other devices share interrupt line with mma8452 compatible chip
configured with push-pull, resulting in broken or unpredictable results.
I don't see why we should not care about that.

> Yes, I understand the HW case.
>
>> > The (hidden) problem this will have in the future is that the IRQ core
>> > will splat a warning in case that other shared IRQs might be
>> > configured with different flags. Putting that flag conditionally makes
>> > it a mine field for the users. Instead just unconditionally add that
>> > flag and we will get reports as soon as there will be a user that
>> > shares the same interrupt pin with some other devices which drivers do
>> > not use the same settings.
>>
>> If we add the IRQF_SHARED flag unconditionally, it will be set also when
>> push-pull mode is enabled. I don't see how the kernel will be able to
>> notice that that is not going to work. If you have another device that
>> uses IRQF_TRIGGER_LOW|IRF_ONESHOT|IRQF_SHARED, it will not work with the
>> MMA8452 device when configured as push-pull.
>
> Right, and why do we care (again)?

Why we care that the system as a whole (SW on top of HW) will not work?

If we don't care about that, why do we even have this IRQF_SHARED flag?
The only purpose of that is to tell the kernel that this particular
device / interrupt will work with shared interrupt or not.

Isn't that exactly what I do with this change? Nothing more, nothing less.

> It's pure DT/FW/HW issue, not an SW issue.
> Otherwise it will become a carefully placed mine for the poor user who will
> use these flags and try to share an interrupt with the mma8452 device which
> has no set property and uses push-pull mode.

I don't get how you see it like that. Adding IRQF_SHARED unconditionally
would create exactly the mine field you are talking about. Poor users
can specify a system configuration (DT) that tries to use a shared
interrupt line, but configures the mma8452 compatible chip in push-pull.
The poor user will not only be poor, but also unhappy.

By applying the IRQF_SHARED dynamically, the kernel will be able to fail
in a controlled manner instead, making it much less painful to create a
working system configuration (DT)

Going back to your poor user story above, if the poor user tries to
share the interrupt pin with the mma8452 device, it will get an error
(just as it is with the kernel today). The device does not support
shared interrupts. Reading the DT bindings documentation, the user
should be able to find the drive-open-drain property, and add that to
the device-tree to make things work. Maybe even consider if that is
compatible with the hardware being used.

If IRQF_SHARED was set unconditionally, the user would not get an error,
but most likely would get a system where no irq's were raised for the
other chips. I sincerely believe that debugging this is much more
painful than reading device-tree bindings.

> Did I miss anything?

I don't know. Maybe I am missing the obvious here.

/Esben