diff mbox series

[libgpiod,v2] treewide: add support for hardware timestamp engine

Message ID 20220923081223.25851-1-brgl@bgdev.pl
State New
Headers show
Series [libgpiod,v2] treewide: add support for hardware timestamp engine | expand

Commit Message

Bartosz Golaszewski Sept. 23, 2022, 8:12 a.m. UTC 
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(-)

Comments

Andy Shevchenko Sept. 23, 2022, 10:30 a.m. UTC | #1 
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?)?
Bartosz Golaszewski Sept. 23, 2022, 5:30 p.m. UTC | #2 
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
diff mbox series

Patch

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);