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

[Marek Vasut <marek.vasut@mailbox.org>]

On 7/22/25 10:55 AM, Uwe Kleine-König wrote:
> Hello,

Hi,

> On Mon, Jun 16, 2025 at 02:42:54PM +0200, Hans de Goede wrote:
>>> 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 don't have a system that triggers the problem, but there is
> https://bugs.debian.org/1106411 which I guess is the same problem.

I think the debian person is on CC here already.

> I think the suggestion to not register the triggers in the absence of an
> irq would be implemented by
> 
> diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> index be5fbb0c5d29..8d90802e65fd 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -1687,18 +1687,18 @@ int bmc150_accel_core_probe(struct device *dev, struct regmap *regmap, int irq,
>   		fifo_attrs = NULL;
>   	}
>   
> -	ret = iio_triggered_buffer_setup_ext(indio_dev,
> -					     &iio_pollfunc_store_time,
> -					     bmc150_accel_trigger_handler,
> -					     IIO_BUFFER_DIRECTION_IN,
> -					     &bmc150_accel_buffer_ops,
> -					     fifo_attrs);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed: iio triggered buffer setup\n");
> -		goto err_disable_regulators;
> -	}
> -
>   	if (irq > 0) {
> +		ret = iio_triggered_buffer_setup_ext(indio_dev,
> +						     &iio_pollfunc_store_time,
> +						     bmc150_accel_trigger_handler,
> +						     IIO_BUFFER_DIRECTION_IN,
> +						     &bmc150_accel_buffer_ops,
> +						     fifo_attrs);
> +		if (ret < 0) {
> +			dev_err(dev, "Failed: iio triggered buffer setup\n");
> +			goto err_disable_regulators;
> +		}
> +
>   		ret = devm_request_threaded_irq(dev, irq,
>   						bmc150_accel_irq_handler,
>   						bmc150_accel_irq_thread_handler,
> 
> 
> But given that bmc150_accel_write_event_config() also calls
> bmc150_accel_set_interrupt() this might not be a complete fix.
I haven't had time to get back to this, so if you want to take over, I'm 
happy to hand it over.

I'm sorry for the slow response.