On 25.01.19 11:11, Philippe Gerum wrote:
On 1/25/19 10:48 AM, Jan Kiszka wrote:
On 25.01.19 10:38, Philippe Gerum wrote:
On 1/25/19 10:33 AM, Jan Kiszka wrote:
On 25.01.19 10:31, Philippe Gerum wrote:
On 1/25/19 10:23 AM, Jan Kiszka wrote:
On 25.01.19 10:15, Philippe Gerum wrote:
On 1/24/19 7:17 PM, Jan Kiszka wrote:
On 24.01.19 16:34, Philippe Gerum wrote:
In timestamping mode, read() returns the timestamp of the latest
event
receipt on the pin based on CLOCK_MONOTONIC, along with the pin
state. This is an optional pin readout mode controlled by the
GPIO_RTIOC_TS request, e.g.:
struct rtdm_gpio_readout rdo;
int ret, on, val;
on = 1;
ret = ioctl(pinfd, GPIO_RTIOC_TS, &on);
ret = read(pinfd, &rdo, sizeof(rdo));
/* pin state changed to rdo.value at time rdo.timestamp */
on = 0;
ret = ioctl(pinfd, GPIO_RTIOC_TS, &on);
ret = read(pinfd, &val, sizeof(val));
/* pin state changed to value (time of change unspecified) */
By default, timestamping mode is disabled, which corresponds to the
original behavior.
Signed-off-by: Philippe Gerum <[email protected]>
---
include/cobalt/kernel/rtdm/gpio.h | 1 +
include/rtdm/uapi/gpio.h | 18 +++++++----
kernel/drivers/gpio/gpio-core.c | 54
+++++++++++++++++++++++--------
3 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/include/cobalt/kernel/rtdm/gpio.h
b/include/cobalt/kernel/rtdm/gpio.h
index cdb472f8a..00055ec0a 100644
--- a/include/cobalt/kernel/rtdm/gpio.h
+++ b/include/cobalt/kernel/rtdm/gpio.h
@@ -33,6 +33,7 @@ struct rtdm_gpio_pin {
rtdm_event_t event;
char *name;
struct gpio_desc *desc;
+ nanosecs_abs_t timestamp;
};
struct rtdm_gpio_chip {
diff --git a/include/rtdm/uapi/gpio.h b/include/rtdm/uapi/gpio.h
index b745f156c..ac14be66c 100644
--- a/include/rtdm/uapi/gpio.h
+++ b/include/rtdm/uapi/gpio.h
@@ -18,12 +18,18 @@
#ifndef _RTDM_UAPI_GPIO_H
#define _RTDM_UAPI_GPIO_H
-#define GPIO_RTIOC_DIR_OUT _IOW(RTDM_CLASS_GPIO, 0,
int)
-#define GPIO_RTIOC_DIR_IN _IO(RTDM_CLASS_GPIO, 1)
-#define GPIO_RTIOC_IRQEN _IOW(RTDM_CLASS_GPIO, 2, int) /*
GPIO
trigger */
-#define GPIO_RTIOC_IRQDIS _IO(RTDM_CLASS_GPIO, 3)
-#define GPIO_RTIOC_REQS _IO(RTDM_CLASS_GPIO, 4)
-#define GPIO_RTIOC_RELS _IO(RTDM_CLASS_GPIO, 5)
+struct rtdm_gpio_readout {
+ __u64 timestamp;
nanosecs_abs_t - we use this type also in to userspace interface.
Nope. This is ABI stuff, nanosecs_abs_t is not guaranteed to have the
same size regardless of the ABI models, which is unsafe in mixed
32/64
model configurations. OTOH, __u64 is safe.
If nanosecs_abs_t is alread an ABI, and it is used as such for a long
time (see include/rtdm/uapi/serial.h). If you see a bug in its
definition, that needs to be fixed. We should not resolve that by
choosing open-coded local workarounds.
nanosecs_t is not defined in the Xenomai ABI, never has been. Using
nanosecs_t in structs actually abuses such ABI. So we may either keep
adding even more nanosecs, or fix the spots where it should be replaced
with __u64.
Xenomai ABI includes the driver ABI - so your assumption might have been
it's not while it was de facto.
Again, let's address the issue, not work around it.
__u64 is the fix, using nanosecs*_t in uapi/ is wrong. So please merge
this fix, and we'll see how we can progress fixing other abuses.
Nope: Adjust nanosecs_*_t if it has a problem ([u]int64_t vs. __u/s64 -
what's the problem exactly?), and use that. That will both address
existing users and future ones. No special solution for the GPIO driver
here.
I'm not pushing for a GPIO-specific solution, I'm pushing for using
ABI-specific types which state their size explicitly in uapi/ data as
much as possible, which is safer with mixed ABI models and would have
spared me a lot of trouble back when I implemented the 32/64 ABI support
for Xenomai. nanosecs_* is originally a kernel-side type which slipped
in uapi/ structs, which does not state its size.
No, it was designed and used for both side. I know best as I wrote and first
used that abstraction.
The fact that nanosecs_* is defined as uint64_t but the kernel today
does not address the issue at stake: there would be no obvious way to
detect that a discrepancy might exist with what userland expects once
such definition is modified for a different with if ever (which might be
highly unlikely for nanosecs, but could happen for other types).
So the good news is that a) we do not have a problem today and b) we can safely
prevent any hypothetical future by changing the definition of nanosecs_*_t
towards __u64/__s64 backing. So, if you are concerned about the stability of
standard-int, let's switch to the kernel uapi types for nanosecs, use that also
for the GPIO ABI, and call it a day.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
