Re: [PATCH v4 2/3] iio: accel: Support Kionix/ROHM KX022A accelerometer

From: Vaittinen, Matti
Date: Mon Oct 31 2022 - 02:25:55 EST


On 10/29/22 14:35, Jonathan Cameron wrote:
> On Mon, 24 Oct 2022 05:44:21 +0000
> "Vaittinen, Matti" <Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:
>
>>>> +
>>>> + if (fwnode_irq_get_byname(fwnode, "INT2") > 0)
>>>> + dev_warn(dev, "Only one IRQ supported\n");
>>>
>>> Why? If you get the both, only the first one will be used by the driver.
>>> Not really worth warning about the lack of features...
>>
>> My thinking regarding developing new device went along the lines:
>>
>> Precondition: The HW (and data-sheet) explain how there is two INT pins.
>> 1. Board designer reads the data-sheet and uses both INT pins.
>> 2. SW engineer finds the driver and reads the DT-binding description.
>> 3. SW engineer writes the DT-description and hopes everything "just
>> works". (Amount of hope is probably inversely proportional to the amount
>> of experience XD).
>> 4) SW engineer gives a first go to the sensor SW and notices everything
>> does not just magically work.
>
> Ah but it does "work". We simply don't use one of the IRQs. That's not
> normally a problem as there are lots of other features we don't fully
> support. Not using something is not normally considered a problem.

I guess you're correct...

> So far I'm not seeing anything that doesn't work because we only support
> one IRQ. There may be combinations of interrupts that are tricky to handle
> on one IRQ line (or may not be supported at the same time on a single line)
> but so far you don't support those anyway.. Adding a more informative warning
> when adding those features would be reasonable
>
> "Feature X not supported as only a single IRQ line available".

... and I also fully agree with this. My thinking was stuck with each
IRQ line having a purpose fixed by HW - which is not the case with this
sensor.

Yours
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~