On 2014-02-06 23:48, H Hartley Sweeten wrote:
Introduce a comedi core helper function to handle the boilerplate
needed by the drivers to wait for a condition to occur. Typically
this condition is the analog input/output end-of-conversion used
with the comedi (*insn_read) and (*insn_write) operations.
To use this function, the drivers just need to provide a callback
that checks for the desired condition. The callback should return
0 if the condition is met or -EBUSY if it is still waiting. Any
other errno will be returned to the caller. If the timeout occurs
before the condition is met -ETIMEDOUT will be returned.
The parameters to the callback function are the comedi_device,
comedi_subdevice, and comedi_insn pointers that were passed to the
(*insn_read) or (*insn_write) as well as an unsigned long, driver
specific, 'context' that can be used to pass any other information
that might be needed in the callback. This 'context' could be anything
such as the register offset to read the status or the bits needed
to check the status. The comedi_timeout() function itself does not
use any of these parameters.
This will help remove all the crazy "wait this many loops" used by
some of the drivers. It also creates a common errno for comedi to
detect when a timeout occurs.
ADC/DAC conversion times are typically pretty fast. A conservative
timeout of 1 second (HZ) is used in comedi_timeout().
Perhaps it could be made tunable? Or at least the 1 second
comedi_timeout(blah, blah, blah) could call comedi_timeout_ms(blah,
blah, blah, 1000) or whatever.
More comments below....
Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/staging/comedi/comedidev.h | 6 ++++++
drivers/staging/comedi/drivers.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
index f82bd42..d27510c 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -353,6 +353,12 @@ void comedi_buf_memcpy_from(struct comedi_async *async, unsigned int offset,
/* drivers.c - general comedi driver functions */
+int comedi_timeout(struct comedi_device *, struct comedi_subdevice *,
+ struct comedi_insn *,
+ int (*cb)(struct comedi_device *, struct comedi_subdevice *,
+ struct comedi_insn *, unsigned long context),
+ unsigned long context);
+
int comedi_dio_insn_config(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *data,
unsigned int mask);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 2460803..ceec408 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -18,6 +18,7 @@
#include <linux/device.h>
#include <linux/module.h>
+#include <linux/delay.h>
#include <linux/errno.h>
#include <linux/kconfig.h>
#include <linux/kernel.h>
@@ -155,6 +156,38 @@ int insn_inval(struct comedi_device *dev, struct comedi_subdevice *s,
}
/**
+ * comedi_timeout() - wait up to 1 sec for a driver condition to occur.
+ * @dev: comedi_device struct
+ * @s: comedi_subdevice struct
+ * @insn: comedi_insn struct
+ * @cb: callback to check for the condition
+ * @context: private context from the driver
+ */
+int comedi_timeout(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ int (*cb)(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned long context),
+ unsigned long context)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(HZ);
HZ is the number of jiffies in one second, so that should be:
unsigned long timeout = jiffies + HZ;
or:
unsigned long timeout = jiffies + msecs_to_jiffies(1000);
(If the time-out is to be made tunable in milliseconds, the second form
would be preferred.)
+ int ret;
+
+ while (time_before(jiffies, timeout)) {
+ ret = cb(dev, s, insn, context);
+ if (ret == 0)
+ return 0;
That test is redundant as 0 != -EBUSY below.
+ if (ret != -EBUSY)
+ return ret;
+ udelay(1);
It could just do a cpu_relax() instead of udelay(1).
+ }
+ return -ETIMEDOUT;
+}
If the function is pre-empted for a long while after calling cb(), the
time_before() test could fail without giving the cb() a final chance to
return 0.
How about:
int ret;
for (;;) {
bool going = time_before(jiffies, timeout);
ret = cb(dev, s, insn, context);
if (ret != -EBUSY)
return ret;
if (!going)
return -ETIMEDOUT;
cpu_relax();
}
(N.B. #include <asm/processor.h> for cpu_relax().)
+EXPORT_SYMBOL_GPL(comedi_timeout);
+
+/**
* comedi_dio_insn_config() - boilerplate (*insn_config) for DIO subdevices.
* @dev: comedi_device struct
* @s: comedi_subdevice struct
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
