On 25.01.19 12:42, Philippe Gerum wrote:
On 1/25/19 12:32 PM, Jan Kiszka wrote:
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.
Please avoid the "I know best" non-argument. I don't think it is
particularly relevant in this context, and the last thing we want is to
lose relevance when discussing technical matters.
I just wanted to clarify the author's intention with and the usage of this type.
It seems you interpreted those differently, and we never talked about
assumptions regarding this detail. That's all.
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.
Ok, you don't seem to understand my point. I'll switch to nanosecs*,
time is a scarce resource for me too.
I thought you were referring to how nanosecs_*_t is implemented. If you concern
is how it's being used: We can clarify / state explicitly that it is today and
will remain tomorrow part of the RTDM ABI. But the fact that it is already ABI
will not change by implementing things differently only in the GPIO driver.
Thanks for following my change request.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
