Re: [PATCH] iio: accel: bmc150: Do not configure IRQ registers if no IRQ connected

[Marek Vasut]

On 6/21/25 7:17 PM, Jonathan Cameron wrote:
> On Mon, 16 Jun 2025 14:42:54 +0200
> Hans de Goede <hansg@xxxxxxxxxx> wrote:
>
> > Hi,
> >
> > On 13-Jun-25 14:45, Marek Vasut wrote:
> > > The BMC150 on Onemix 2S does not have IRQ line described in ACPI tables,
> > > which leads to bmc150_accel_core_probe() being called with irq=0, which
> > > leads to bmc150_accel_interrupts_setup() never being called, which leads
> > > to struct bmc150_accel_data *data ->interrupts[i].info being left unset
> > > to NULL. Later, userspace can indirectly trigger bmc150_accel_set_interrupt()
> > > which depends on struct bmc150_accel_data *data ->interrupts[i].info being
> > > non-NULL, and which triggers NULL pointer dereference. This is triggered
> > > e.g. from iio-sensor-proxy.
> > >
> > > Fix this by skipping the IRQ register configuration in case there is no
> > > IRQ connected in hardware, in a manner similar to what the driver did in
> > > the very first commit which added the driver.
> >
> > ...
> >
> > > Fixes: 8e22f477e143 ("iio: bmc150: refactor interrupt enabling")
> > > Signed-off-by: Marek Vasut <marek.vasut+bmc150@xxxxxxxxxxx>
> > > ---
> > > Cc: "Nuno Sá" <nuno.sa@xxxxxxxxxx>
> > > Cc: Andy Shevchenko <andy@xxxxxxxxxx>
> > > Cc: David Lechner <dlechner@xxxxxxxxxxxx>
> > > Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
> > > Cc: Julien Stephan <jstephan@xxxxxxxxxxxx>
> > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > > Cc: Salvatore Bonaccorso <carnil@xxxxxxxxxx>
> > > Cc: linux-iio@xxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx
> > > ---
> > >   drivers/iio/accel/bmc150-accel-core.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > > index 744a034bb8b5..1c3583ade2b4 100644
> > > --- a/drivers/iio/accel/bmc150-accel-core.c
> > > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > > @@ -550,6 +550,9 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
> > >   	if (ret < 0)
> > >   		return ret;
> > >   
> > > +	if (!info)
> > > +		return 0;
> > > +
> > >   	/* map the interrupt to the appropriate pins */
> > >   	ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
> > >   				 (state ? info->map_bitmask : 0));
> >
> > AFAIK the proper fix would be to not register any IIO-triggers. This fix will
> > avoid the problem, but userspace might still try to use non-working triggers
> > which will now silently fail.
> >
> > I'm not an IIO expert, but IIRC other drivers simply skip registering their triggers
> > when there is no interrupt support.
>
> Absolutely. It is annoyingly common for devices to have some or none of the interrupt
> lines actually wired so drivers should not present the interfaces if they aren't.
> It is acceptable for a new driver to just fail to probe if handling the device with no
> interrupts is particularly complex but in general at least some functionality tends
> to be easy to implement so we prefer that.
I haven't gotten to this again ... yet ... I can try and add some sort 
of polling fallback maybe ?