Met vriendelijke groet / kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxx
W: www.topic.nl
Please consider the environment before printing this e-mail
On 04-02-2024 16:54, Jonathan Cameron wrote:
On Fri, 2 Feb 2024 11:59:01 +0100
Mike Looijmans<mike.looijmans@xxxxxxxx> wrote:
Skeleton driver for the TI ADS1298 medical ADC. This device is
typically used for ECG and similar measurements. Supports data
acquisition at configurable scale and sampling frequency.
Signed-off-by: Mike Looijmans<mike.looijmans@xxxxxxxx>
Hi Mike,
A few minor things I'd missed before.
I'm still interested in why more standard interrupt handling isn't
good enough here (see reply to v1 thread) but if we can't get to the bottom
of that (or do figure it out and we can't fix it) then this doesn't look
too bad so I'll accept the complex handling.
I think one of the key elements was the IRQF_ONESHOT usage. The DRDY
signal on this chip isn't a "level" signal as most chips have, it will
de-assert at any rising edge of the SPI clock, without regarding
chip-select. There's no other indication of "data ready", so the only
way is to keep the interrupt active on edge detect.
Keeping things in hard IRQ handlers reduces the number of context
switches, and the amount of work done is minimal. A worker thread would
wake at every DRDY signal, and after the corresponding SPI transaction.
This doesn't account for much overhead, but the interrupt rate is double
the sampling frequency. Most importantly, the device doesn't have to
compete with other threads in the system.
If I have time and hardware available, I try to get some timing info
with an oscilloscope...
Assume "yes" to all other suggestions...
+ ret = devm_request_irq(dev, spi->irq, &ads1298_interrupt,
+ IRQF_TRIGGER_FALLING, indio_dev->name,
I missed this before (and we've gotten it wrong a bunch of times in the past
so plenty of bad examples to copy that we can't fix without possible
regressions) but we generally now leave irq direction to the firmware description.
People have an annoying habit of putting not gates and similar in the path
to interrupt pins. Fine to have the binding state the expected form though
(as you do). So basically not flags here.
I'd be happy to leave the IRQ direction to firmware (indeed, inverters
happen...), but afaik that wasn't possible with interrupts. I'll dig
into the docs to see if that has changed recently.
I'm still curious to understand more of where the delays that lead to
needing to do this complex handling came from, but I guess it's not too bad
if we can't get to the bottom of that so I'll take the driver anyway
(after a little more time on list for others to review!)
+ indio_dev);
(PS - sorry for sending HTML, consider me appropriately punisched for that)
--
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E:mike.looijmans@xxxxxxxx
W:www.topic.nl
