* [libgpiod v2][PATCH] treewide: add support for hardware timestamp engine
@ 2022-09-23 8:12 Bartosz Golaszewski
2022-09-23 10:30 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Bartosz Golaszewski @ 2022-09-23 8:12 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar
Cc: linux-gpio, Bartosz Golaszewski
Since v5.19 the linux GPIO uAPI exposes a new request flag for making
the hardware timestamp engine be the source of edge event timestamps.
Add support for it to libgpiod.
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
bindings/cxx/gpiodcxx/line-info.hpp | 2 +-
bindings/cxx/gpiodcxx/line.hpp | 4 +++-
bindings/cxx/line-info.cpp | 3 ++-
bindings/cxx/line-settings.cpp | 3 ++-
bindings/cxx/line.cpp | 3 ++-
bindings/cxx/tests/check-kernel.cpp | 2 +-
bindings/cxx/tests/tests-line-settings.cpp | 2 ++
bindings/cxx/tests/tests-line.cpp | 2 ++
include/gpiod.h | 2 ++
lib/line-config.c | 3 +++
lib/line-info.c | 2 ++
lib/line-settings.c | 1 +
lib/uapi/gpio.h | 3 +++
tests/gpiod-test.c | 4 ++--
tests/tests-line-info.c | 10 ++++++++++
tests/tests-line-settings.c | 6 ++++++
16 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/bindings/cxx/gpiodcxx/line-info.hpp b/bindings/cxx/gpiodcxx/line-info.hpp
index cbc8156..189d305 100644
--- a/bindings/cxx/gpiodcxx/line-info.hpp
+++ b/bindings/cxx/gpiodcxx/line-info.hpp
@@ -143,7 +143,7 @@ public:
/**
* @brief Read the current event clock setting used for edge event
* timestamps.
- * @return Returns MONOTONIC or REALTIME.
+ * @return Returns MONOTONIC, REALTIME or HTE.
*/
line::clock event_clock() const;
diff --git a/bindings/cxx/gpiodcxx/line.hpp b/bindings/cxx/gpiodcxx/line.hpp
index 32c4136..a8aae57 100644
--- a/bindings/cxx/gpiodcxx/line.hpp
+++ b/bindings/cxx/gpiodcxx/line.hpp
@@ -156,8 +156,10 @@ enum class clock
{
MONOTONIC = 1,
/**< Line uses the monotonic clock for edge event timestamps. */
- REALTIME
+ REALTIME,
/**< Line uses the realtime clock for edge event timestamps. */
+ HTE,
+ /*<< Line uses the hardware timestamp engine for event timestamps. */
};
/**
diff --git a/bindings/cxx/line-info.cpp b/bindings/cxx/line-info.cpp
index 2ad0baf..a6b6dfa 100644
--- a/bindings/cxx/line-info.cpp
+++ b/bindings/cxx/line-info.cpp
@@ -38,7 +38,8 @@ const ::std::map<int, line::edge> edge_mapping = {
const ::std::map<int, line::clock> clock_mapping = {
{ GPIOD_LINE_EVENT_CLOCK_MONOTONIC, line::clock::MONOTONIC },
- { GPIOD_LINE_EVENT_CLOCK_REALTIME, line::clock::REALTIME }
+ { GPIOD_LINE_EVENT_CLOCK_REALTIME, line::clock::REALTIME },
+ { GPIOD_LINE_EVENT_CLOCK_HTE, line::clock::HTE }
};
} /* namespace */
diff --git a/bindings/cxx/line-settings.cpp b/bindings/cxx/line-settings.cpp
index dbbe30e..22655e2 100644
--- a/bindings/cxx/line-settings.cpp
+++ b/bindings/cxx/line-settings.cpp
@@ -57,7 +57,8 @@ const ::std::map<int, line::drive> reverse_drive_mapping = make_reverse_maping(d
const ::std::map<line::clock, int> clock_mapping = {
{ line::clock::MONOTONIC, GPIOD_LINE_EVENT_CLOCK_MONOTONIC },
- { line::clock::REALTIME, GPIOD_LINE_EVENT_CLOCK_REALTIME }
+ { line::clock::REALTIME, GPIOD_LINE_EVENT_CLOCK_REALTIME },
+ { line::clock::HTE, GPIOD_LINE_EVENT_CLOCK_HTE }
};
const ::std::map<int, line::clock> reverse_clock_mapping = make_reverse_maping(clock_mapping);
diff --git a/bindings/cxx/line.cpp b/bindings/cxx/line.cpp
index a9caedd..c2750a8 100644
--- a/bindings/cxx/line.cpp
+++ b/bindings/cxx/line.cpp
@@ -45,7 +45,8 @@ const ::std::map<line::edge, ::std::string> edge_names = {
const ::std::map<line::clock, ::std::string> clock_names = {
{ line::clock::MONOTONIC, "MONOTONIC" },
- { line::clock::REALTIME, "REALTIME" }
+ { line::clock::REALTIME, "REALTIME" },
+ { line::clock::HTE, "HTE" }
};
} /* namespace */
diff --git a/bindings/cxx/tests/check-kernel.cpp b/bindings/cxx/tests/check-kernel.cpp
index 5d128a0..e10fb5d 100644
--- a/bindings/cxx/tests/check-kernel.cpp
+++ b/bindings/cxx/tests/check-kernel.cpp
@@ -43,6 +43,6 @@ public:
}
};
-kernel_checker require_kernel(5, 17, 4);
+kernel_checker require_kernel(5, 19, 0);
} /* namespace */
diff --git a/bindings/cxx/tests/tests-line-settings.cpp b/bindings/cxx/tests/tests-line-settings.cpp
index a7801a4..a3f4bc5 100644
--- a/bindings/cxx/tests/tests-line-settings.cpp
+++ b/bindings/cxx/tests/tests-line-settings.cpp
@@ -107,6 +107,8 @@ TEST_CASE("line_settings mutators work", "[line-settings]")
REQUIRE(settings.event_clock() == clock_type::REALTIME);
settings.set_event_clock(clock_type::MONOTONIC);
REQUIRE(settings.event_clock() == clock_type::MONOTONIC);
+ settings.set_event_clock(clock_type::HTE);
+ REQUIRE(settings.event_clock() == clock_type::HTE);
REQUIRE_THROWS_AS(settings.set_event_clock(static_cast<clock_type>(999)),
::std::invalid_argument);
}
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index c17122c..319012a 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -92,9 +92,11 @@ TEST_CASE("stream insertion operators for types in gpiod::line work", "[line]")
{
auto monotonic = clock_type::MONOTONIC;
auto realtime = clock_type::REALTIME;
+ auto hte = clock_type::HTE;
REQUIRE_THAT(monotonic, stringify_matcher<clock_type>("MONOTONIC"));
REQUIRE_THAT(realtime, stringify_matcher<clock_type>("REALTIME"));
+ REQUIRE_THAT(hte, stringify_matcher<clock_type>("HTE"));
}
SECTION("offsets")
diff --git a/include/gpiod.h b/include/gpiod.h
index b60a177..a8e002b 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -306,6 +306,8 @@ enum {
/**< Line uses the monotonic clock for edge event timestamps. */
GPIOD_LINE_EVENT_CLOCK_REALTIME,
/**< Line uses the realtime clock for edge event timestamps. */
+ GPIOD_LINE_EVENT_CLOCK_HTE,
+ /**< Line uses the hardware timestamp engine for event timestamps. */
};
/**
diff --git a/lib/line-config.c b/lib/line-config.c
index 114d40c..5ee7390 100644
--- a/lib/line-config.c
+++ b/lib/line-config.c
@@ -341,6 +341,9 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings)
case GPIOD_LINE_EVENT_CLOCK_REALTIME:
flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME;
break;
+ case GPIOD_LINE_EVENT_CLOCK_HTE:
+ flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE;
+ break;
}
return flags;
diff --git a/lib/line-info.c b/lib/line-info.c
index 65eca48..9809c43 100644
--- a/lib/line-info.c
+++ b/lib/line-info.c
@@ -160,6 +160,8 @@ gpiod_line_info_from_uapi(struct gpio_v2_line_info *uapi_info)
if (uapi_info->flags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME)
info->event_clock = GPIOD_LINE_EVENT_CLOCK_REALTIME;
+ else if (uapi_info->flags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE)
+ info->event_clock = GPIOD_LINE_EVENT_CLOCK_HTE;
else
info->event_clock = GPIOD_LINE_EVENT_CLOCK_MONOTONIC;
diff --git a/lib/line-settings.c b/lib/line-settings.c
index 7125124..f97a90e 100644
--- a/lib/line-settings.c
+++ b/lib/line-settings.c
@@ -195,6 +195,7 @@ gpiod_line_settings_set_event_clock(struct gpiod_line_settings *settings,
switch (event_clock) {
case GPIOD_LINE_EVENT_CLOCK_MONOTONIC:
case GPIOD_LINE_EVENT_CLOCK_REALTIME:
+ case GPIOD_LINE_EVENT_CLOCK_HTE:
settings->event_clock = event_clock;
break;
default:
diff --git a/lib/uapi/gpio.h b/lib/uapi/gpio.h
index eaaea3d..cb9966d 100644
--- a/lib/uapi/gpio.h
+++ b/lib/uapi/gpio.h
@@ -66,6 +66,8 @@ struct gpiochip_info {
* @GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN: line has pull-down bias enabled
* @GPIO_V2_LINE_FLAG_BIAS_DISABLED: line has bias disabled
* @GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME: line events contain REALTIME timestamps
+ * @GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE: line events contain timestamps from
+ * hardware timestamp engine
*/
enum gpio_v2_line_flag {
GPIO_V2_LINE_FLAG_USED = _BITULL(0),
@@ -80,6 +82,7 @@ enum gpio_v2_line_flag {
GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN = _BITULL(9),
GPIO_V2_LINE_FLAG_BIAS_DISABLED = _BITULL(10),
GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME = _BITULL(11),
+ GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE = _BITULL(12),
};
/**
diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c
index 38d80a4..39a1f40 100644
--- a/tests/gpiod-test.c
+++ b/tests/gpiod-test.c
@@ -10,8 +10,8 @@
#include "gpiod-test.h"
#define MIN_KERNEL_MAJOR 5
-#define MIN_KERNEL_MINOR 17
-#define MIN_KERNEL_RELEASE 4
+#define MIN_KERNEL_MINOR 19
+#define MIN_KERNEL_RELEASE 0
#define MIN_KERNEL_VERSION KERNEL_VERSION(MIN_KERNEL_MAJOR, \
MIN_KERNEL_MINOR, \
MIN_KERNEL_RELEASE)
diff --git a/tests/tests-line-info.c b/tests/tests-line-info.c
index ffc4586..45b14ff 100644
--- a/tests/tests-line-info.c
+++ b/tests/tests-line-info.c
@@ -362,6 +362,7 @@ GPIOD_TEST_CASE(event_clock)
g_autoptr(struct_gpiod_line_request) request = NULL;
g_autoptr(struct_gpiod_line_info) info0 = NULL;
g_autoptr(struct_gpiod_line_info) info1 = NULL;
+ g_autoptr(struct_gpiod_line_info) info2 = NULL;
guint offset;
chip = gpiod_test_open_chip_or_fail(g_gpiosim_chip_get_dev_path(sim));
@@ -377,13 +378,22 @@ GPIOD_TEST_CASE(event_clock)
gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
settings);
+ gpiod_line_settings_set_event_clock(settings,
+ GPIOD_LINE_EVENT_CLOCK_HTE);
+ offset = 2;
+ gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
+ settings);
+
request = gpiod_test_request_lines_or_fail(chip, NULL, line_cfg);
info0 = gpiod_test_get_line_info_or_fail(chip, 0);
info1 = gpiod_test_get_line_info_or_fail(chip, 1);
+ info2 = gpiod_test_get_line_info_or_fail(chip, 2);
g_assert_cmpint(gpiod_line_info_get_event_clock(info0), ==,
GPIOD_LINE_EVENT_CLOCK_MONOTONIC);
g_assert_cmpint(gpiod_line_info_get_event_clock(info1), ==,
GPIOD_LINE_EVENT_CLOCK_REALTIME);
+ g_assert_cmpint(gpiod_line_info_get_event_clock(info2), ==,
+ GPIOD_LINE_EVENT_CLOCK_HTE);
}
diff --git a/tests/tests-line-settings.c b/tests/tests-line-settings.c
index d074063..bdf932d 100644
--- a/tests/tests-line-settings.c
+++ b/tests/tests-line-settings.c
@@ -222,6 +222,12 @@ GPIOD_TEST_CASE(set_event_clock)
g_assert_cmpint(gpiod_line_settings_get_event_clock(settings), ==,
GPIOD_LINE_EVENT_CLOCK_REALTIME);
+ ret = gpiod_line_settings_set_event_clock(settings,
+ GPIOD_LINE_EVENT_CLOCK_HTE);
+ g_assert_cmpint(ret, ==, 0);
+ g_assert_cmpint(gpiod_line_settings_get_event_clock(settings), ==,
+ GPIOD_LINE_EVENT_CLOCK_HTE);
+
ret = gpiod_line_settings_set_event_clock(settings, 999);
g_assert_cmpint(ret, <, 0);
g_assert_cmpint(errno, ==, EINVAL);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [libgpiod v2][PATCH] treewide: add support for hardware timestamp engine
2022-09-23 8:12 [libgpiod v2][PATCH] treewide: add support for hardware timestamp engine Bartosz Golaszewski
@ 2022-09-23 10:30 ` Andy Shevchenko
2022-09-23 17:30 ` Bartosz Golaszewski
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2022-09-23 10:30 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: Kent Gibson, Linus Walleij, Viresh Kumar, linux-gpio
On Fri, Sep 23, 2022 at 10:12:23AM +0200, Bartosz Golaszewski wrote:
> Since v5.19 the linux GPIO uAPI exposes a new request flag for making
> the hardware timestamp engine be the source of edge event timestamps.
> Add support for it to libgpiod.
...
> /**
> * @brief Read the current event clock setting used for edge event
> * timestamps.
> - * @return Returns MONOTONIC or REALTIME.
> + * @return Returns MONOTONIC, REALTIME or HTE.
Is it possible to refer to these with % so doxygen (or whatever you are using)
make the links out of them?
Because HTE is way too cryptic TLA (the same concern was also during submission
of that framework into the Linux kernel).
> */
...
> { line::clock::MONOTONIC, "MONOTONIC" },
> - { line::clock::REALTIME, "REALTIME" }
> + { line::clock::REALTIME, "REALTIME" },
> + { line::clock::HTE, "HTE" }
Similar here and hey, have you got the idea of the trailing comma (or poor C++
forbids that?)?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [libgpiod v2][PATCH] treewide: add support for hardware timestamp engine
2022-09-23 10:30 ` Andy Shevchenko
@ 2022-09-23 17:30 ` Bartosz Golaszewski
0 siblings, 0 replies; 3+ messages in thread
From: Bartosz Golaszewski @ 2022-09-23 17:30 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Kent Gibson, Linus Walleij, Viresh Kumar, linux-gpio
On Fri, Sep 23, 2022 at 12:30 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 23, 2022 at 10:12:23AM +0200, Bartosz Golaszewski wrote:
> > Since v5.19 the linux GPIO uAPI exposes a new request flag for making
> > the hardware timestamp engine be the source of edge event timestamps.
> > Add support for it to libgpiod.
>
> ...
>
> > /**
> > * @brief Read the current event clock setting used for edge event
> > * timestamps.
> > - * @return Returns MONOTONIC or REALTIME.
> > + * @return Returns MONOTONIC, REALTIME or HTE.
>
> Is it possible to refer to these with % so doxygen (or whatever you are using)
> make the links out of them?
>
Yeah I couldn't get that to work with enums. If I can make it work,
then I'll do a sweeping fix across the entire tree.
> Because HTE is way too cryptic TLA (the same concern was also during submission
> of that framework into the Linux kernel).
>
> > */
>
> ...
>
> > { line::clock::MONOTONIC, "MONOTONIC" },
> > - { line::clock::REALTIME, "REALTIME" }
> > + { line::clock::REALTIME, "REALTIME" },
> > + { line::clock::HTE, "HTE" }
>
> Similar here and hey, have you got the idea of the trailing comma (or poor C++
> forbids that?)?
>
I just made it consistent with the rest of the code. I will fix it
treewide too, already have that on my TODO.
Bart
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-23 17:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-23 8:12 [libgpiod v2][PATCH] treewide: add support for hardware timestamp engine Bartosz Golaszewski
2022-09-23 10:30 ` Andy Shevchenko
2022-09-23 17:30 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).