linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [libgpiod][PATCH 0/6] treewide: remove more cruft and
@ 2021-01-11 13:34 Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 1/6] treewide: remove helpers for opening chips by name & number Bartosz Golaszewski
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This is another batch of improvements to libgpiod before we overhaul the
data structure model.

The last patch adds the kernel uapi header to the repository so that we
no longer depend on its presence on the host system.

Bartosz Golaszewski (6):
  treewide: remove helpers for opening chips by name & number
  treewide: simplify the active-low line property
  treewide: rename BIAS_AS_IS to BIAS_NONE
  treewide: make drive settings an enum
  bindings: cxx: line: reorder bias mapping entries
  core: add the kernel uapi header to the repository

 bindings/cxx/chip.cpp                  |  41 +-
 bindings/cxx/examples/gpioinfocxx.cpp  |   3 +-
 bindings/cxx/gpiod.hpp                 |  57 +--
 bindings/cxx/line.cpp                  |  28 +-
 bindings/cxx/tests/tests-chip.cpp      |  97 +----
 bindings/cxx/tests/tests-event.cpp     |  14 +-
 bindings/cxx/tests/tests-iter.cpp      |   2 +-
 bindings/cxx/tests/tests-line.cpp      |  93 ++---
 bindings/python/examples/gpioinfo.py   |   4 +-
 bindings/python/gpiodmodule.c          | 167 +++-----
 bindings/python/tests/gpiod_py_test.py | 173 ++++----
 configure.ac                           |  12 +-
 include/gpiod.h                        |  65 +--
 lib/Makefile.am                        |   2 +-
 lib/core.c                             |  29 +-
 lib/helpers.c                          |  57 ---
 lib/uapi/gpio.h                        | 522 +++++++++++++++++++++++++
 tests/tests-chip.c                     |  41 --
 tests/tests-line.c                     | 100 ++---
 tools/gpiodetect.c                     |   2 +-
 tools/gpiofind.c                       |   2 +-
 tools/gpioget.c                        |   2 +-
 tools/gpioinfo.c                       |  28 +-
 tools/gpiomon.c                        |   2 +-
 tools/gpioset.c                        |   2 +-
 tools/tools-common.c                   |  57 +++
 tools/tools-common.h                   |   3 +
 27 files changed, 903 insertions(+), 702 deletions(-)
 create mode 100644 lib/uapi/gpio.h

-- 
2.29.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 1/6] treewide: remove helpers for opening chips by name & number
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 2/6] treewide: simplify the active-low line property Bartosz Golaszewski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Helper wrappers around gpiod_chip_open() shouldn't really be part of
the core, low-level library. They assume that devtmpfs is mounted at
/dev and that GPIO chips follow the gpiochipX naming convention.

This changeset removes all variants of gpiod_chip_open() other than
the one taking the path as argument. We're doing this treewide so
C++ and Python bindings lose the 'how' argument that currently allows
to change the way the chips are opened by the chip's constructors or
the open() method in C++.

The gpio-tools programs still support opening chips by number, name
and path but they implement this functionality locally rather than
using the library code.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 bindings/cxx/chip.cpp                  |  41 +-------
 bindings/cxx/gpiod.hpp                 |  25 +----
 bindings/cxx/tests/tests-chip.cpp      |  97 +++---------------
 bindings/cxx/tests/tests-event.cpp     |  14 +--
 bindings/cxx/tests/tests-iter.cpp      |   2 +-
 bindings/cxx/tests/tests-line.cpp      |  16 +--
 bindings/python/gpiodmodule.c          |  57 ++---------
 bindings/python/tests/gpiod_py_test.py | 133 +++++++++++--------------
 include/gpiod.h                        |  29 ------
 lib/helpers.c                          |  57 -----------
 tests/tests-chip.c                     |  41 --------
 tools/gpiodetect.c                     |   2 +-
 tools/gpiofind.c                       |   2 +-
 tools/gpioget.c                        |   2 +-
 tools/gpioinfo.c                       |   4 +-
 tools/gpiomon.c                        |   2 +-
 tools/gpioset.c                        |   2 +-
 tools/tools-common.c                   |  57 +++++++++++
 tools/tools-common.h                   |   3 +
 19 files changed, 167 insertions(+), 419 deletions(-)

diff --git a/bindings/cxx/chip.cpp b/bindings/cxx/chip.cpp
index 107088e..59351dd 100644
--- a/bindings/cxx/chip.cpp
+++ b/bindings/cxx/chip.cpp
@@ -15,35 +15,6 @@ namespace gpiod {
 
 namespace {
 
-::gpiod_chip* open_lookup(const ::std::string& device)
-{
-	return ::gpiod_chip_open_lookup(device.c_str());
-}
-
-::gpiod_chip* open_by_path(const ::std::string& device)
-{
-	return ::gpiod_chip_open(device.c_str());
-}
-
-::gpiod_chip* open_by_name(const ::std::string& device)
-{
-	return ::gpiod_chip_open_by_name(device.c_str());
-}
-
-::gpiod_chip* open_by_number(const ::std::string& device)
-{
-	return ::gpiod_chip_open_by_number(::std::stoul(device));
-}
-
-using open_func = ::std::function<::gpiod_chip* (const ::std::string&)>;
-
-const ::std::map<int, open_func> open_funcs = {
-	{ chip::OPEN_LOOKUP,	open_lookup,	},
-	{ chip::OPEN_BY_PATH,	open_by_path,	},
-	{ chip::OPEN_BY_NAME,	open_by_name,	},
-	{ chip::OPEN_BY_NUMBER,	open_by_number,	},
-};
-
 void chip_deleter(::gpiod_chip* chip)
 {
 	::gpiod_chip_close(chip);
@@ -56,10 +27,10 @@ bool is_gpiochip_device(const ::std::string& path)
 	return ::gpiod_is_gpiochip_device(path.c_str());
 }
 
-chip::chip(const ::std::string& device, int how)
+chip::chip(const ::std::string& path)
 	: _m_chip()
 {
-	this->open(device, how);
+	this->open(path);
 }
 
 chip::chip(::gpiod_chip* chip)
@@ -74,14 +45,12 @@ chip::chip(const ::std::weak_ptr<::gpiod_chip>& chip_ptr)
 
 }
 
-void chip::open(const ::std::string& device, int how)
+void chip::open(const ::std::string& path)
 {
-	auto func = open_funcs.at(how);
-
-	::gpiod_chip *chip = func(device);
+	::gpiod_chip *chip = ::gpiod_chip_open(path.c_str());
 	if (!chip)
 		throw ::std::system_error(errno, ::std::system_category(),
-					  "cannot open GPIO device " + device);
+					  "cannot open GPIO device " + path);
 
 	this->_m_chip.reset(chip, chip_deleter);
 }
diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
index d81ee30..6c9ccd6 100644
--- a/bindings/cxx/gpiod.hpp
+++ b/bindings/cxx/gpiod.hpp
@@ -58,10 +58,9 @@ public:
 
 	/**
 	 * @brief Constructor. Opens the chip using chip::open.
-	 * @param device String describing the GPIO chip.
-	 * @param how Indicates how the chip should be opened.
+	 * @param path Path to the GPIO chip device.
 	 */
-	GPIOD_API chip(const ::std::string& device, int how = OPEN_LOOKUP);
+	GPIOD_API chip(const ::std::string& path);
 
 	/**
 	 * @brief Copy constructor. References the object held by other.
@@ -96,13 +95,12 @@ public:
 
 	/**
 	 * @brief Open a GPIO chip.
-	 * @param device String describing the GPIO chip.
-	 * @param how Indicates how the chip should be opened.
+	 * @param path Path to the GPIO chip device.
 	 *
 	 * If the object already holds a reference to an open chip, it will be
 	 * closed and the reference reset.
 	 */
-	GPIOD_API void open(const ::std::string &device, int how = OPEN_LOOKUP);
+	GPIOD_API void open(const ::std::string &path);
 
 	/**
 	 * @brief Reset the internal smart pointer owned by this object.
@@ -184,21 +182,6 @@ public:
 	 */
 	GPIOD_API bool operator!(void) const noexcept;
 
-	/**
-	 * @brief Affect the way in which chip::chip and chip::open will try
-	 *        to open a GPIO chip character device.
-	 */
-	enum : int {
-		OPEN_LOOKUP = 1,
-		/**< Open based on the best guess what the supplied string is. */
-		OPEN_BY_PATH,
-		/**< Assume the string is a path to the GPIO chardev. */
-		OPEN_BY_NAME,
-		/**< Assume the string is the name of the chip */
-		OPEN_BY_NUMBER,
-		/**< Assume the string is the number of the GPIO chip. */
-	};
-
 private:
 
 	chip(::gpiod_chip* chip);
diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
index 2492b42..1b16b65 100644
--- a/bindings/cxx/tests/tests-chip.cpp
+++ b/bindings/cxx/tests/tests-chip.cpp
@@ -32,72 +32,22 @@ TEST_CASE("GPIO chip device can be verified with is_gpiochip_device()", "[chip]"
 	}
 }
 
-TEST_CASE("GPIO chip device can be opened in different modes", "[chip]")
+TEST_CASE("GPIO chip device can be opened", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8, 8, 8 });
 
-	SECTION("open by name")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip(mockup::instance().chip_name(1),
-				::gpiod::chip::OPEN_BY_NAME));
-	}
-
-	SECTION("open by path")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip(mockup::instance().chip_path(1),
-				::gpiod::chip::OPEN_BY_PATH));
-	}
-
-	SECTION("open by number")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip(::std::to_string(mockup::instance().chip_num(1)),
-				::gpiod::chip::OPEN_BY_NUMBER));
-	}
-}
-
-TEST_CASE("GPIO chip device can be opened with implicit lookup", "[chip]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8 });
-
-	SECTION("lookup by name")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip(mockup::instance().chip_name(1)));
-	}
-
-	SECTION("lookup by path")
+	SECTION("open from constructor")
 	{
 		REQUIRE_NOTHROW(::gpiod::chip(mockup::instance().chip_path(1)));
 	}
 
-	SECTION("lookup by number")
+	SECTION("open from open() method")
 	{
-		REQUIRE_NOTHROW(::gpiod::chip(::std::to_string(mockup::instance().chip_num(1))));
-	}
-}
-
-TEST_CASE("GPIO chip can be opened with the open() method in different modes", "[chip]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8 });
-	::gpiod::chip chip;
-
-	REQUIRE_FALSE(!!chip);
+		::gpiod::chip chip;
 
-	SECTION("open by name")
-	{
-		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_name(1),
-					  ::gpiod::chip::OPEN_BY_NAME));
-	}
-
-	SECTION("open by path")
-	{
-		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_path(1),
-					  ::gpiod::chip::OPEN_BY_PATH));
-	}
+		REQUIRE_FALSE(!!chip);
 
-	SECTION("open by number")
-	{
-		REQUIRE_NOTHROW(chip.open(::std::to_string(mockup::instance().chip_num(1)),
-					  ::gpiod::chip::OPEN_BY_NUMBER));
+		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_path(1)));
 	}
 }
 
@@ -116,27 +66,6 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
 	}
 }
 
-TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup", "[chip]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8 });
-	::gpiod::chip chip;
-
-	SECTION("lookup by name")
-	{
-		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_name(1)));
-	}
-
-	SECTION("lookup by path")
-	{
-		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_path(1)));
-	}
-
-	SECTION("lookup by number")
-	{
-		REQUIRE_NOTHROW(chip.open(::std::to_string(mockup::instance().chip_num(1))));
-	}
-}
-
 TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
 {
 	REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
@@ -146,7 +75,7 @@ TEST_CASE("Chip object can be reset", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
 
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	REQUIRE(chip);
 	chip.reset();
 	REQUIRE_FALSE(!!chip);
@@ -156,7 +85,7 @@ TEST_CASE("Chip info can be correctly retrieved", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8, 16, 8 });
 
-	::gpiod::chip chip(mockup::instance().chip_name(1));
+	::gpiod::chip chip(mockup::instance().chip_path(1));
 	REQUIRE(chip.name() == mockup::instance().chip_name(1));
 	REQUIRE(chip.label() == "gpio-mockup-B");
 	REQUIRE(chip.num_lines() == 16);
@@ -166,11 +95,11 @@ TEST_CASE("Chip object can be copied and compared", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8, 8 });
 
-	::gpiod::chip chip1(mockup::instance().chip_name(0));
+	::gpiod::chip chip1(mockup::instance().chip_path(0));
 	auto chip2 = chip1;
 	REQUIRE(chip1 == chip2);
 	REQUIRE_FALSE(chip1 != chip2);
-	::gpiod::chip chip3(mockup::instance().chip_name(1));
+	::gpiod::chip chip3(mockup::instance().chip_path(1));
 	REQUIRE(chip1 != chip3);
 	REQUIRE_FALSE(chip2 == chip3);
 }
@@ -178,7 +107,7 @@ TEST_CASE("Chip object can be copied and compared", "[chip]")
 TEST_CASE("Lines can be retrieved from chip objects", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8, 8, 8 }, mockup::FLAG_NAMED_LINES);
-	::gpiod::chip chip(mockup::instance().chip_name(1));
+	::gpiod::chip chip(mockup::instance().chip_path(1));
 
 	SECTION("get single line by offset")
 	{
@@ -216,7 +145,7 @@ TEST_CASE("Lines can be retrieved from chip objects", "[chip]")
 TEST_CASE("All lines can be retrieved from a chip at once", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 4 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 
 	auto lines = chip.get_all_lines();
 	REQUIRE(lines.size() == 4);
@@ -229,7 +158,7 @@ TEST_CASE("All lines can be retrieved from a chip at once", "[chip]")
 TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8 }, mockup::FLAG_NAMED_LINES);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 
 	SECTION("invalid offset (single line)")
 	{
diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
index 6d1c069..97ea670 100644
--- a/bindings/cxx/tests/tests-event.cpp
+++ b/bindings/cxx/tests/tests-event.cpp
@@ -24,7 +24,7 @@ TEST_CASE("Line events can be detected", "[event][line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
 	mockup::event_thread events(0, 4, 200);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 	::gpiod::line_request config;
 
@@ -105,7 +105,7 @@ TEST_CASE("Watching line_bulk objects for events works", "[event][bulk]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
 	mockup::event_thread events(0, 2, 200);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto lines = chip.get_lines({ 0, 1, 2, 3 });
 	::gpiod::line_request config;
 
@@ -133,7 +133,7 @@ TEST_CASE("Watching line_bulk objects for events works", "[event][bulk]")
 TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 	::gpiod::line_request config;
 
@@ -165,7 +165,7 @@ TEST_CASE("Event file descriptors can be used for polling", "[event]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
 	mockup::event_thread events(0, 3, 200);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto lines = chip.get_lines({ 0, 1, 2, 3, 4, 5 });
 
 	::gpiod::line_request config;
@@ -196,7 +196,7 @@ TEST_CASE("Event file descriptors can be used for polling", "[event]")
 TEST_CASE("It's possible to read a value from a line requested for events", "[event][line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 	::gpiod::line_request config;
 
@@ -222,7 +222,7 @@ TEST_CASE("It's possible to read a value from a line requested for events", "[ev
 TEST_CASE("It's possible to read values from lines requested for events", "[event][bulk]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto lines = chip.get_lines({ 0, 1, 2, 3, 4 });
 	::gpiod::line_request config;
 
@@ -256,7 +256,7 @@ TEST_CASE("It's possible to read values from lines requested for events", "[even
 TEST_CASE("It's possible to read more than one line event", "[event][line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 	::gpiod::line_request config;
 
diff --git a/bindings/cxx/tests/tests-iter.cpp b/bindings/cxx/tests/tests-iter.cpp
index 708709c..0dee754 100644
--- a/bindings/cxx/tests/tests-iter.cpp
+++ b/bindings/cxx/tests/tests-iter.cpp
@@ -15,7 +15,7 @@ using ::gpiod::test::mockup;
 TEST_CASE("Line iterator works", "[iter][line]")
 {
 	mockup::probe_guard mockup_chips({ 4 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	unsigned int count = 0;
 
 	for (auto& it: ::gpiod::line_iter(chip))
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index a5499ac..53b1d03 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -21,7 +21,7 @@ const ::std::string consumer = "line-test";
 TEST_CASE("Line information can be correctly retrieved", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 }, mockup::FLAG_NAMED_LINES);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 
 	SECTION("unexported line")
@@ -167,7 +167,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 TEST_CASE("Line bulk object works correctly", "[line][bulk]")
 {
 	mockup::probe_guard mockup_chips({ 8 }, mockup::FLAG_NAMED_LINES);
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 
 	SECTION("lines can be added, accessed and cleared")
 	{
@@ -210,7 +210,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
 TEST_CASE("Line values can be set and read", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	::gpiod::line_request config;
 
 	config.consumer = consumer.c_str();
@@ -308,7 +308,7 @@ TEST_CASE("Line values can be set and read", "[line]")
 TEST_CASE("Line can be reconfigured", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	::gpiod::line_request config;
 
 	config.consumer = consumer.c_str();
@@ -436,7 +436,7 @@ TEST_CASE("Line can be reconfigured", "[line]")
 TEST_CASE("Exported line can be released", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(4);
 	::gpiod::line_request config;
 
@@ -487,7 +487,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
 TEST_CASE("Cannot request the same line twice", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	::gpiod::line_request config;
 
 	config.consumer = consumer.c_str();
@@ -516,7 +516,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
 TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line = chip.get_line(3);
 
 	SECTION("get value")
@@ -533,7 +533,7 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
 TEST_CASE("Line objects can be compared")
 {
 	mockup::probe_guard mockup_chips({ 8 });
-	::gpiod::chip chip(mockup::instance().chip_name(0));
+	::gpiod::chip chip(mockup::instance().chip_path(0));
 	auto line1 = chip.get_line(3);
 	auto line2 = chip.get_line(3);
 	auto line3 = chip.get_line(4);
diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
index af37df2..e8641f1 100644
--- a/bindings/python/gpiodmodule.c
+++ b/bindings/python/gpiodmodule.c
@@ -1952,44 +1952,19 @@ static gpiod_LineBulkObject *gpiod_LineToLineBulk(gpiod_LineObject *line)
 	return ret;
 }
 
-enum {
-	gpiod_OPEN_LOOKUP = 1,
-	gpiod_OPEN_BY_NAME,
-	gpiod_OPEN_BY_PATH,
-	gpiod_OPEN_BY_NUMBER,
-};
-
 static int gpiod_Chip_init(gpiod_ChipObject *self,
 			   PyObject *args, PyObject *Py_UNUSED(ignored))
 {
-	int rv, how = gpiod_OPEN_LOOKUP;
-	PyThreadState *thread;
-	char *descr;
+	char *path;
+	int rv;
 
-	rv = PyArg_ParseTuple(args, "s|i", &descr, &how);
+	rv = PyArg_ParseTuple(args, "s", &path);
 	if (!rv)
 		return -1;
 
-	thread = PyEval_SaveThread();
-	switch (how) {
-	case gpiod_OPEN_LOOKUP:
-		self->chip = gpiod_chip_open_lookup(descr);
-		break;
-	case gpiod_OPEN_BY_NAME:
-		self->chip = gpiod_chip_open_by_name(descr);
-		break;
-	case gpiod_OPEN_BY_PATH:
-		self->chip = gpiod_chip_open(descr);
-		break;
-	case gpiod_OPEN_BY_NUMBER:
-		self->chip = gpiod_chip_open_by_number(atoi(descr));
-		break;
-	default:
-		PyEval_RestoreThread(thread);
-		PyErr_BadArgument();
-		return -1;
-	}
-	PyEval_RestoreThread(thread);
+	Py_BEGIN_ALLOW_THREADS;
+	self->chip = gpiod_chip_open(path);
+	Py_END_ALLOW_THREADS;
 	if (!self->chip) {
 		PyErr_SetFromErrno(PyExc_OSError);
 		return -1;
@@ -2553,26 +2528,6 @@ typedef struct {
 } gpiod_ConstDescr;
 
 static gpiod_ConstDescr gpiod_ConstList[] = {
-	{
-		.typeobj = &gpiod_ChipType,
-		.name = "OPEN_LOOKUP",
-		.val = gpiod_OPEN_LOOKUP,
-	},
-	{
-		.typeobj = &gpiod_ChipType,
-		.name = "OPEN_BY_PATH",
-		.val = gpiod_OPEN_BY_PATH,
-	},
-	{
-		.typeobj = &gpiod_ChipType,
-		.name = "OPEN_BY_NAME",
-		.val = gpiod_OPEN_BY_NAME,
-	},
-	{
-		.typeobj = &gpiod_ChipType,
-		.name = "OPEN_BY_NUMBER",
-		.val = gpiod_OPEN_BY_NUMBER,
-	},
 	{
 		.typeobj = &gpiod_LineType,
 		.name = "DIRECTION_INPUT",
diff --git a/bindings/python/tests/gpiod_py_test.py b/bindings/python/tests/gpiod_py_test.py
index f116657..79294bc 100755
--- a/bindings/python/tests/gpiod_py_test.py
+++ b/bindings/python/tests/gpiod_py_test.py
@@ -96,31 +96,10 @@ class ChipOpen(MockupTestCase):
 
     chip_sizes = ( 8, 8, 8 )
 
-    def test_open_chip_by_name(self):
-        with gpiod.Chip(mockup.chip_name(1), gpiod.Chip.OPEN_BY_NAME) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
-    def test_open_chip_by_path(self):
-        with gpiod.Chip(mockup.chip_path(1), gpiod.Chip.OPEN_BY_PATH) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
-    def test_open_chip_by_num(self):
-        with gpiod.Chip('{}'.format(mockup.chip_num(1)),
-                        gpiod.Chip.OPEN_BY_NUMBER) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
-    def test_lookup_chip_by_name(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
-    def test_lookup_chip_by_path(self):
+    def test_open_good(self):
         with gpiod.Chip(mockup.chip_path(1)) as chip:
             self.assertEqual(chip.name(), mockup.chip_name(1))
 
-    def test_lookup_chip_by_num(self):
-        with gpiod.Chip('{}'.format(mockup.chip_num(1))) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
     def test_nonexistent_chip(self):
         with self.assertRaises(FileNotFoundError):
             chip = gpiod.Chip('nonexistent-chip')
@@ -134,7 +113,7 @@ class ChipClose(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_use_chip_after_close(self):
-        chip = gpiod.Chip(mockup.chip_name(0))
+        chip = gpiod.Chip(mockup.chip_path(0))
         self.assertEqual(chip.name(), mockup.chip_name(0))
         chip.close()
         with self.assertRaises(ValueError):
@@ -145,7 +124,7 @@ class ChipInfo(MockupTestCase):
     chip_sizes = ( 16, )
 
     def test_chip_get_info(self):
-        chip = gpiod.Chip(mockup.chip_name(0))
+        chip = gpiod.Chip(mockup.chip_path(0))
         self.assertEqual(chip.name(), mockup.chip_name(0))
         self.assertEqual(chip.label(), 'gpio-mockup-A')
         self.assertEqual(chip.num_lines(), 16)
@@ -156,29 +135,29 @@ class ChipGetLines(MockupTestCase):
     flags = gpiomockup.Mockup.FLAG_NAMED_LINES
 
     def test_get_single_line_by_offset(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             line = chip.get_line(4)
             self.assertEqual(line.name(), 'gpio-mockup-B-4')
 
     def test_find_single_line_by_name(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             lines = chip.find_line('gpio-mockup-B-4', unique=True).to_list()
             self.assertEqual(lines[0].offset(), 4)
 
     def test_get_single_line_invalid_offset(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             with self.assertRaises(OSError) as err_ctx:
                 line = chip.get_line(11)
 
             self.assertEqual(err_ctx.exception.errno, errno.EINVAL)
 
     def test_find_single_line_nonexistent(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             lines = chip.find_line('nonexistent-line', unique=True)
             self.assertEqual(lines, None)
 
     def test_get_multiple_lines_by_offsets_in_tuple(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             lines = chip.get_lines(( 1, 3, 6, 7 )).to_list()
             self.assertEqual(len(lines), 4)
             self.assertEqual(lines[0].name(), 'gpio-mockup-B-1')
@@ -187,7 +166,7 @@ class ChipGetLines(MockupTestCase):
             self.assertEqual(lines[3].name(), 'gpio-mockup-B-7')
 
     def test_get_multiple_lines_by_offsets_in_list(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             lines = chip.get_lines([ 1, 3, 6, 7 ]).to_list()
             self.assertEqual(len(lines), 4)
             self.assertEqual(lines[0].name(), 'gpio-mockup-B-1')
@@ -196,14 +175,14 @@ class ChipGetLines(MockupTestCase):
             self.assertEqual(lines[3].name(), 'gpio-mockup-B-7')
 
     def test_get_multiple_lines_invalid_offset(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
+        with gpiod.Chip(mockup.chip_path(1)) as chip:
             with self.assertRaises(OSError) as err_ctx:
                 line = chip.get_lines(( 1, 3, 11, 7 ))
 
             self.assertEqual(err_ctx.exception.errno, errno.EINVAL)
 
     def test_get_all_lines(self):
-        with gpiod.Chip(mockup.chip_name(2)) as chip:
+        with gpiod.Chip(mockup.chip_path(2)) as chip:
             lines = chip.get_all_lines().to_list()
             self.assertEqual(len(lines), 4)
             self.assertEqual(lines[0].name(), 'gpio-mockup-C-0')
@@ -221,7 +200,7 @@ class LineInfo(MockupTestCase):
     flags = gpiomockup.Mockup.FLAG_NAMED_LINES
 
     def test_unexported_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
@@ -232,7 +211,7 @@ class LineInfo(MockupTestCase):
             self.assertFalse(line.is_requested())
 
     def test_exported_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -246,7 +225,7 @@ class LineInfo(MockupTestCase):
             self.assertTrue(line.is_requested())
 
     def test_exported_line_with_flags(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = (gpiod.LINE_REQ_FLAG_ACTIVE_LOW |
                      gpiod.LINE_REQ_FLAG_OPEN_DRAIN)
@@ -265,7 +244,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
 
     def test_exported_open_drain_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = gpiod.LINE_REQ_FLAG_OPEN_DRAIN
             line.request(consumer=default_consumer,
@@ -283,7 +262,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
 
     def test_exported_open_source_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = gpiod.LINE_REQ_FLAG_OPEN_SOURCE
             line.request(consumer=default_consumer,
@@ -301,7 +280,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
 
     def test_exported_bias_disable_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = gpiod.LINE_REQ_FLAG_BIAS_DISABLE
             line.request(consumer=default_consumer,
@@ -319,7 +298,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_DISABLE)
 
     def test_exported_bias_pull_down_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = gpiod.LINE_REQ_FLAG_BIAS_PULL_DOWN
             line.request(consumer=default_consumer,
@@ -337,7 +316,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_PULL_DOWN)
 
     def test_exported_bias_pull_up_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             flags = gpiod.LINE_REQ_FLAG_BIAS_PULL_UP
             line.request(consumer=default_consumer,
@@ -355,7 +334,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.bias(), gpiod.Line.BIAS_PULL_UP)
 
     def test_update_line_info(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.update()
 
@@ -364,7 +343,7 @@ class LineValues(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_get_value_single_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -373,7 +352,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(line.get_value(), 1)
 
     def test_set_value_single_line(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT)
@@ -383,7 +362,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 0)
 
     def test_set_value_with_default_value_argument(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -391,7 +370,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 1)
 
     def test_get_value_multiple_lines(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                           type=gpiod.LINE_REQ_DIR_IN)
@@ -402,7 +381,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(lines.get_values(), [ 1, 0, 1, 1 ])
 
     def test_set_value_multiple_lines(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                           type=gpiod.LINE_REQ_DIR_OUT)
@@ -418,7 +397,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 6), 0)
 
     def test_set_multiple_values_with_default_vals_argument(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -429,7 +408,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 6), 1)
 
     def test_get_value_active_low(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN,
@@ -439,7 +418,7 @@ class LineValues(MockupTestCase):
             self.assertEqual(line.get_value(), 0)
 
     def test_set_value_active_low(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -454,7 +433,7 @@ class LineConfig(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_set_config_direction(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -465,7 +444,7 @@ class LineConfig(MockupTestCase):
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
 
     def test_set_config_flags(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT)
@@ -476,7 +455,7 @@ class LineConfig(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 0)
 
     def test_set_config_output_value(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -486,7 +465,7 @@ class LineConfig(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 0)
 
     def test_set_config_output_no_value(self):
-         with gpiod.Chip(mockup.chip_name(0)) as chip:
+         with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -496,7 +475,7 @@ class LineConfig(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 0)
 
     def test_set_config_bulk_output_no_values(self):
-         with gpiod.Chip(mockup.chip_name(0)) as chip:
+         with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                           type=gpiod.LINE_REQ_DIR_OUT,
@@ -516,7 +495,7 @@ class LineFlags(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_set_flags(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT,
@@ -528,7 +507,7 @@ class LineFlags(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 1)
 
     def test_set_flags_bulk(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                           type=gpiod.LINE_REQ_DIR_OUT,
@@ -553,7 +532,7 @@ class LineDirection(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_set_direction(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_OUT)
@@ -571,7 +550,7 @@ class LineDirection(MockupTestCase):
             self.assertEqual(mockup.chip_get_value(0, 3), 0)
 
     def test_set_direction_bulk(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 0, 3, 4, 6 ))
             lines.request(consumer=default_consumer,
                           type=gpiod.LINE_REQ_DIR_OUT)
@@ -637,7 +616,7 @@ class LineRequestBehavior(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_line_export_release(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -647,7 +626,7 @@ class LineRequestBehavior(MockupTestCase):
             self.assertFalse(line.is_requested())
 
     def test_line_request_twice_two_calls(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -658,7 +637,7 @@ class LineRequestBehavior(MockupTestCase):
             self.assertEqual(err_ctx.exception.errno, errno.EBUSY)
 
     def test_line_request_twice_in_bulk(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_lines(( 2, 3, 6, 6 ))
             with self.assertRaises(OSError) as err_ctx:
                 lines.request(consumer=default_consumer,
@@ -667,7 +646,7 @@ class LineRequestBehavior(MockupTestCase):
             self.assertEqual(err_ctx.exception.errno, errno.EBUSY)
 
     def test_use_value_unrequested(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             with self.assertRaises(OSError) as err_ctx:
                 line.get_value()
@@ -675,7 +654,7 @@ class LineRequestBehavior(MockupTestCase):
             self.assertEqual(err_ctx.exception.errno, errno.EPERM)
 
     def test_request_with_no_kwds(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(2)
             line.request(default_consumer)
             self.assertTrue(line.is_requested())
@@ -692,7 +671,7 @@ class LineIterator(MockupTestCase):
     chip_sizes = ( 4, )
 
     def test_iterate_over_lines(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             count = 0
 
             for line in gpiod.LineIter(chip):
@@ -706,7 +685,7 @@ class LineBulkIter(MockupTestCase):
     chip_sizes = ( 4, )
 
     def test_line_bulk_iterator(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             lines = chip.get_all_lines()
             count = 0
 
@@ -726,7 +705,7 @@ class EventSingleLine(MockupTestCase):
 
     def test_single_line_rising_edge_event(self):
         with EventThread(0, 4, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 line = chip.get_line(4)
                 line.request(consumer=default_consumer,
                              type=gpiod.LINE_REQ_EV_RISING_EDGE)
@@ -737,7 +716,7 @@ class EventSingleLine(MockupTestCase):
 
     def test_single_line_falling_edge_event(self):
         with EventThread(0, 4, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 line = chip.get_line(4)
                 line.request(consumer=default_consumer,
                              type=gpiod.LINE_REQ_EV_FALLING_EDGE)
@@ -748,7 +727,7 @@ class EventSingleLine(MockupTestCase):
 
     def test_single_line_both_edges_events(self):
         with EventThread(0, 4, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 line = chip.get_line(4)
                 line.request(consumer=default_consumer,
                              type=gpiod.LINE_REQ_EV_BOTH_EDGES)
@@ -763,7 +742,7 @@ class EventSingleLine(MockupTestCase):
 
     def test_single_line_both_edges_events_active_low(self):
         with EventThread(0, 4, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 line = chip.get_line(4)
                 line.request(consumer=default_consumer,
                              type=gpiod.LINE_REQ_EV_BOTH_EDGES,
@@ -778,7 +757,7 @@ class EventSingleLine(MockupTestCase):
                 self.assertEqual(event.source.offset(), 4)
 
     def test_single_line_read_multiple_events(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(4)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_EV_BOTH_EDGES)
@@ -804,7 +783,7 @@ class EventBulk(MockupTestCase):
 
     def test_watch_multiple_lines_for_events(self):
         with EventThread(0, 2, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 lines = chip.get_lines(( 0, 1, 2, 3, 4 ))
                 lines.request(consumer=default_consumer,
                               type=gpiod.LINE_REQ_EV_BOTH_EDGES)
@@ -826,7 +805,7 @@ class EventValues(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_request_for_events_get_value(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_EV_BOTH_EDGES)
@@ -835,7 +814,7 @@ class EventValues(MockupTestCase):
             self.assertEqual(line.get_value(), 1)
 
     def test_request_for_events_get_value_active_low(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_EV_BOTH_EDGES,
@@ -849,7 +828,7 @@ class EventFileDescriptor(MockupTestCase):
     chip_sizes = ( 8, )
 
     def test_event_get_fd(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_EV_BOTH_EDGES)
@@ -857,7 +836,7 @@ class EventFileDescriptor(MockupTestCase):
             self.assertGreaterEqual(fd, 0)
 
     def test_event_get_fd_not_requested(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             with self.assertRaises(OSError) as err_ctx:
                 fd = line.event_get_fd();
@@ -865,7 +844,7 @@ class EventFileDescriptor(MockupTestCase):
             self.assertEqual(err_ctx.exception.errno, errno.EPERM)
 
     def test_event_get_fd_requested_for_values(self):
-        with gpiod.Chip(mockup.chip_name(0)) as chip:
+        with gpiod.Chip(mockup.chip_path(0)) as chip:
             line = chip.get_line(3)
             line.request(consumer=default_consumer,
                          type=gpiod.LINE_REQ_DIR_IN)
@@ -876,7 +855,7 @@ class EventFileDescriptor(MockupTestCase):
 
     def test_event_fd_polling(self):
         with EventThread(0, 2, 200):
-            with gpiod.Chip(mockup.chip_name(0)) as chip:
+            with gpiod.Chip(mockup.chip_path(0)) as chip:
                 lines = chip.get_lines(( 0, 1, 2, 3, 4, 5, 6 ))
                 lines.request(consumer=default_consumer,
                               type=gpiod.LINE_REQ_EV_BOTH_EDGES)
diff --git a/include/gpiod.h b/include/gpiod.h
index b82d507..b073b3b 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -84,35 +84,6 @@ bool gpiod_is_gpiochip_device(const char *path) GPIOD_API;
  */
 struct gpiod_chip *gpiod_chip_open(const char *path) GPIOD_API;
 
-/**
- * @brief Open a gpiochip by name.
- * @param name Name of the gpiochip to open.
- * @return GPIO chip handle or NULL if an error occurred.
- *
- * This routine appends name to '/dev/' to create the path.
- */
-struct gpiod_chip *gpiod_chip_open_by_name(const char *name) GPIOD_API;
-
-/**
- * @brief Open a gpiochip by number.
- * @param num Number of the gpiochip.
- * @return GPIO chip handle or NULL if an error occurred.
- *
- * This routine appends num to '/dev/gpiochip' to create the path.
- */
-struct gpiod_chip *gpiod_chip_open_by_number(unsigned int num) GPIOD_API;
-
-/**
- * @brief Open a gpiochip based on the best guess what the path is.
- * @param descr String describing the gpiochip.
- * @return GPIO chip handle or NULL if an error occurred.
- *
- * This routine tries to figure out whether the user passed it the path to the
- * GPIO chip, its name or number as a string. Then it tries to open it using
- * one of the gpiod_chip_open** variants.
- */
-struct gpiod_chip *gpiod_chip_open_lookup(const char *descr) GPIOD_API;
-
 /**
  * @brief Close a GPIO chip handle and release all allocated resources.
  * @param chip The GPIO chip object.
diff --git a/lib/helpers.c b/lib/helpers.c
index ec2575d..ac6325d 100644
--- a/lib/helpers.c
+++ b/lib/helpers.c
@@ -10,68 +10,11 @@
  * access to neither the internal library data structures nor the kernel UAPI.
  */
 
-#include <ctype.h>
 #include <errno.h>
 #include <gpiod.h>
 #include <stdio.h>
 #include <string.h>
 
-static bool isuint(const char *str)
-{
-	for (; *str && isdigit(*str); str++)
-		;
-
-	return *str == '\0';
-}
-
-struct gpiod_chip *gpiod_chip_open_by_name(const char *name)
-{
-	struct gpiod_chip *chip;
-	char *path;
-	int rv;
-
-	rv = asprintf(&path, "/dev/%s", name);
-	if (rv < 0)
-		return NULL;
-
-	chip = gpiod_chip_open(path);
-	free(path);
-
-	return chip;
-}
-
-struct gpiod_chip *gpiod_chip_open_by_number(unsigned int num)
-{
-	struct gpiod_chip *chip;
-	char *path;
-	int rv;
-
-	rv = asprintf(&path, "/dev/gpiochip%u", num);
-	if (!rv)
-		return NULL;
-
-	chip = gpiod_chip_open(path);
-	free(path);
-
-	return chip;
-}
-
-struct gpiod_chip *gpiod_chip_open_lookup(const char *descr)
-{
-	struct gpiod_chip *chip;
-
-	if (isuint(descr)) {
-		chip = gpiod_chip_open_by_number(strtoul(descr, NULL, 10));
-	} else {
-		if (strncmp(descr, "/dev/", 5))
-			chip = gpiod_chip_open_by_name(descr);
-		else
-			chip = gpiod_chip_open(descr);
-	}
-
-	return chip;
-}
-
 struct gpiod_line_bulk *
 gpiod_chip_get_lines(struct gpiod_chip *chip,
 		     unsigned int *offsets, unsigned int num_offsets)
diff --git a/tests/tests-chip.c b/tests/tests-chip.c
index 1c365b9..c18bd5d 100644
--- a/tests/tests-chip.c
+++ b/tests/tests-chip.c
@@ -52,47 +52,6 @@ GPIOD_TEST_CASE(open_notty, 0, { 8 })
 	g_assert_cmpint(errno, ==, ENOTTY);
 }
 
-GPIOD_TEST_CASE(open_by_name_good, 0, { 8 })
-{
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-
-	chip = gpiod_chip_open_by_name(gpiod_test_chip_name(0));
-	g_assert_nonnull(chip);
-}
-
-GPIOD_TEST_CASE(open_by_number_good, 0, { 8 })
-{
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-
-	chip = gpiod_chip_open_by_number(gpiod_test_chip_num(0));
-	g_assert_nonnull(chip);
-}
-
-GPIOD_TEST_CASE(open_lookup_good, 0, { 8, 8, 8})
-{
-	g_autoptr(gpiod_chip_struct) chip_by_name = NULL;
-	g_autoptr(gpiod_chip_struct) chip_by_path = NULL;
-	g_autoptr(gpiod_chip_struct) chip_by_num = NULL;
-	g_autofree gchar *chip_num_str = g_strdup_printf(
-						"%u", gpiod_test_chip_num(1));
-
-	chip_by_name = gpiod_chip_open_lookup(gpiod_test_chip_name(1));
-	chip_by_path = gpiod_chip_open_lookup(gpiod_test_chip_path(1));
-	chip_by_num = gpiod_chip_open_lookup(chip_num_str);
-
-	g_assert_nonnull(chip_by_name);
-	g_assert_nonnull(chip_by_path);
-	g_assert_nonnull(chip_by_num);
-	gpiod_test_return_if_failed();
-
-	g_assert_cmpstr(gpiod_chip_name(chip_by_name),
-			==, gpiod_test_chip_name(1));
-	g_assert_cmpstr(gpiod_chip_name(chip_by_path),
-			==, gpiod_test_chip_name(1));
-	g_assert_cmpstr(gpiod_chip_name(chip_by_num),
-			==, gpiod_test_chip_name(1));
-}
-
 GPIOD_TEST_CASE(get_name, 0, { 8, 8, 8})
 {
 	g_autoptr(gpiod_chip_struct) chip0 = NULL;
diff --git a/tools/gpiodetect.c b/tools/gpiodetect.c
index 8e067f7..6f94f91 100644
--- a/tools/gpiodetect.c
+++ b/tools/gpiodetect.c
@@ -69,7 +69,7 @@ int main(int argc, char **argv)
 		die_perror("unable to scan /dev");
 
 	for (i = 0; i < num_chips; i++) {
-		chip = gpiod_chip_open_by_name(entries[i]->d_name);
+		chip = chip_open_by_name(entries[i]->d_name);
 		if (!chip) {
 			if (errno == EACCES)
 				printf("%s Permission denied\n",
diff --git a/tools/gpiofind.c b/tools/gpiofind.c
index 4acf621..3760ada 100644
--- a/tools/gpiofind.c
+++ b/tools/gpiofind.c
@@ -70,7 +70,7 @@ int main(int argc, char **argv)
 		die_perror("unable to scan /dev");
 
 	for (i = 0; i < num_chips; i++) {
-		chip = gpiod_chip_open_by_name(entries[i]->d_name);
+		chip = chip_open_by_name(entries[i]->d_name);
 		if (!chip) {
 			if (errno == EACCES)
 				continue;
diff --git a/tools/gpioget.c b/tools/gpioget.c
index 11a9c9f..efbacce 100644
--- a/tools/gpioget.c
+++ b/tools/gpioget.c
@@ -97,7 +97,7 @@ int main(int argc, char **argv)
 			die("invalid GPIO offset: %s", argv[i + 1]);
 	}
 
-	chip = gpiod_chip_open_lookup(device);
+	chip = chip_open_lookup(device);
 	if (!chip)
 		die_perror("unable to open %s", device);
 
diff --git a/tools/gpioinfo.c b/tools/gpioinfo.c
index e3dbde7..057a19f 100644
--- a/tools/gpioinfo.c
+++ b/tools/gpioinfo.c
@@ -214,7 +214,7 @@ int main(int argc, char **argv)
 			die_perror("unable to scan /dev");
 
 		for (i = 0; i < num_chips; i++) {
-			chip = gpiod_chip_open_by_name(entries[i]->d_name);
+			chip = chip_open_by_name(entries[i]->d_name);
 			if (!chip) {
 				if (errno == EACCES)
 					printf("%s Permission denied\n",
@@ -230,7 +230,7 @@ int main(int argc, char **argv)
 		}
 	} else {
 		for (i = 0; i < argc; i++) {
-			chip = gpiod_chip_open_lookup(argv[i]);
+			chip = chip_open_lookup(argv[i]);
 			if (!chip)
 				die_perror("looking up chip %s", argv[i]);
 
diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index c271913..3586cb4 100644
--- a/tools/gpiomon.c
+++ b/tools/gpiomon.c
@@ -253,7 +253,7 @@ int main(int argc, char **argv)
 		num_lines++;
 	}
 
-	chip = gpiod_chip_open_lookup(argv[0]);
+	chip = chip_open_lookup(argv[0]);
 	if (!chip)
 		die_perror("unable to open %s", argv[0]);
 
diff --git a/tools/gpioset.c b/tools/gpioset.c
index 7f4615a..20a7d3b 100644
--- a/tools/gpioset.c
+++ b/tools/gpioset.c
@@ -288,7 +288,7 @@ int main(int argc, char **argv)
 			die("invalid offset: %s", argv[i + 1]);
 	}
 
-	chip = gpiod_chip_open_lookup(device);
+	chip = chip_open_lookup(device);
 	if (!chip)
 		die_perror("unable to open %s", device);
 
diff --git a/tools/tools-common.c b/tools/tools-common.c
index b64ef93..bb6cde6 100644
--- a/tools/tools-common.c
+++ b/tools/tools-common.c
@@ -7,6 +7,7 @@
 
 /* Common code for GPIO tools. */
 
+#include <ctype.h>
 #include <errno.h>
 #include <gpiod.h>
 #include <libgen.h>
@@ -116,3 +117,59 @@ int chip_dir_filter(const struct dirent *entry)
 	free(path);
 	return !!is_chip;
 }
+
+struct gpiod_chip *chip_open_by_name(const char *name)
+{
+	struct gpiod_chip *chip;
+	char *path;
+	int ret;
+
+	ret = asprintf(&path, "/dev/%s", name);
+	if (ret < 0)
+		return NULL;
+
+	chip = gpiod_chip_open(path);
+	free(path);
+
+	return chip;
+}
+
+static struct gpiod_chip *chip_open_by_number(unsigned int num)
+{
+	struct gpiod_chip *chip;
+	char *path;
+	int ret;
+
+	ret = asprintf(&path, "/dev/gpiochip%u", num);
+	if (!ret)
+		return NULL;
+
+	chip = gpiod_chip_open(path);
+	free(path);
+
+	return chip;
+}
+
+static bool isuint(const char *str)
+{
+	for (; *str && isdigit(*str); str++)
+		;
+
+	return *str == '\0';
+}
+
+struct gpiod_chip *chip_open_lookup(const char *device)
+{
+	struct gpiod_chip *chip;
+
+	if (isuint(device)) {
+		chip = chip_open_by_number(strtoul(device, NULL, 10));
+	} else {
+		if (strncmp(device, "/dev/", 5))
+			chip = chip_open_by_name(device);
+		else
+			chip = gpiod_chip_open(device);
+	}
+
+	return chip;
+}
diff --git a/tools/tools-common.h b/tools/tools-common.h
index 4148dd8..866f809 100644
--- a/tools/tools-common.h
+++ b/tools/tools-common.h
@@ -9,6 +9,7 @@
 #define __GPIOD_TOOLS_COMMON_H__
 
 #include <dirent.h>
+#include <gpiod.h>
 
 /*
  * Various helpers for the GPIO tools.
@@ -32,5 +33,7 @@ int bias_flags(const char *option);
 void print_bias_help(void);
 int make_signalfd(void);
 int chip_dir_filter(const struct dirent *entry);
+struct gpiod_chip *chip_open_by_name(const char *name);
+struct gpiod_chip *chip_open_lookup(const char *device);
 
 #endif /* __GPIOD_TOOLS_COMMON_H__ */
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 2/6] treewide: simplify the active-low line property
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 1/6] treewide: remove helpers for opening chips by name & number Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE Bartosz Golaszewski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Unlike line direction - where input and output modes are equal, no signal
inversion (active-high) is the natural state of the line while active-low
is less likely. Let's drop the ACTIVE_STATE enum treewide and provide a
boolean property for lines - is_active_low() - to reflect that fact. This
function returning false means the line is "active-high".

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 bindings/cxx/examples/gpioinfocxx.cpp  |  3 +-
 bindings/cxx/gpiod.hpp                 | 16 ++------
 bindings/cxx/line.cpp                  |  6 +--
 bindings/cxx/tests/tests-line.cpp      | 24 ++++++------
 bindings/python/examples/gpioinfo.py   |  4 +-
 bindings/python/gpiodmodule.c          | 45 ++++++----------------
 bindings/python/tests/gpiod_py_test.py | 16 ++++----
 include/gpiod.h                        | 16 ++------
 lib/core.c                             | 12 +++---
 tests/tests-line.c                     | 52 +++++++++-----------------
 tools/gpioinfo.c                       | 10 ++---
 11 files changed, 69 insertions(+), 135 deletions(-)

diff --git a/bindings/cxx/examples/gpioinfocxx.cpp b/bindings/cxx/examples/gpioinfocxx.cpp
index 2490abd..384286e 100644
--- a/bindings/cxx/examples/gpioinfocxx.cpp
+++ b/bindings/cxx/examples/gpioinfocxx.cpp
@@ -44,8 +44,7 @@ int main(int argc, char **argv)
 				::std::cout << " ";
 
 				::std::cout.width(10);
-				::std::cout << (lit.active_state() == ::gpiod::line::ACTIVE_LOW
-									? "active-low" : "active-high");
+				::std::cout << (lit.is_active_low() ? "active-low" : "active-high");
 
 				::std::cout << ::std::endl;
 			}
diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
index 6c9ccd6..8b4a8f9 100644
--- a/bindings/cxx/gpiod.hpp
+++ b/bindings/cxx/gpiod.hpp
@@ -313,10 +313,10 @@ public:
 	GPIOD_API int direction(void) const;
 
 	/**
-	 * @brief Get current active state of this line.
-	 * @return Current active state setting.
+	 * @brief Check if this line's signal is inverted.
+	 * @return True if this line is "active-low", false otherwise.
 	 */
-	GPIOD_API int active_state(void) const;
+	GPIOD_API bool is_active_low(void) const;
 
 	/**
 	 * @brief Get current bias of this line.
@@ -482,16 +482,6 @@ public:
 		/**< Line's direction setting is output. */
 	};
 
-	/**
-	 * @brief Possible active state settings.
-	 */
-	enum : int {
-		ACTIVE_LOW = 1,
-		/**< Line's active state is low. */
-		ACTIVE_HIGH,
-		/**< Line's active state is high. */
-	};
-
 	/**
 	 * @brief Possible bias settings.
 	 */
diff --git a/bindings/cxx/line.cpp b/bindings/cxx/line.cpp
index 54382e2..5a907db 100644
--- a/bindings/cxx/line.cpp
+++ b/bindings/cxx/line.cpp
@@ -74,14 +74,12 @@ int line::direction(void) const
 	return dir == GPIOD_LINE_DIRECTION_INPUT ? DIRECTION_INPUT : DIRECTION_OUTPUT;
 }
 
-int line::active_state(void) const
+bool line::is_active_low(void) const
 {
 	this->throw_if_null();
 	line::chip_guard lock_chip(*this);
 
-	int active = ::gpiod_line_active_state(this->_m_line);
-
-	return active == GPIOD_LINE_ACTIVE_STATE_HIGH ? ACTIVE_HIGH : ACTIVE_LOW;
+	return ::gpiod_line_is_active_low(this->_m_line);
 }
 
 int line::bias(void) const
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index 53b1d03..3c7ea39 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -29,7 +29,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_INPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.consumer().empty());
 		REQUIRE_FALSE(line.is_requested());
 		REQUIRE_FALSE(line.is_used());
@@ -49,7 +49,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
@@ -70,7 +70,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_LOW);
+		REQUIRE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE(line.is_open_drain());
@@ -90,7 +90,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
@@ -110,7 +110,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
@@ -130,7 +130,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());;
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
@@ -150,7 +150,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.offset() == 4);
 		REQUIRE(line.name() == "gpio-mockup-A-4");
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
@@ -320,19 +320,19 @@ TEST_CASE("Line can be reconfigured", "[line]")
 		config.flags = 0;
 		line.request(config);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_INPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 
 		line.set_config(::gpiod::line_request::DIRECTION_OUTPUT,
 			::gpiod::line_request::FLAG_ACTIVE_LOW,1);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_LOW);
+		REQUIRE(line.is_active_low());
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 0);
 		line.set_value(0);
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 1);
 
 		line.set_config(::gpiod::line_request::DIRECTION_OUTPUT, 0);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 0);
 		line.set_value(1);
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 1);
@@ -348,12 +348,12 @@ TEST_CASE("Line can be reconfigured", "[line]")
 
 		line.set_flags(::gpiod::line_request::FLAG_ACTIVE_LOW);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_LOW);
+		REQUIRE(line.is_active_low());
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 0);
 
 		line.set_flags(0);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.active_state() == ::gpiod::line::ACTIVE_HIGH);
+		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(mockup::instance().chip_get_value(0, 3) == 1);
 	}
 
diff --git a/bindings/python/examples/gpioinfo.py b/bindings/python/examples/gpioinfo.py
index 6a47b66..1593337 100755
--- a/bindings/python/examples/gpioinfo.py
+++ b/bindings/python/examples/gpioinfo.py
@@ -23,11 +23,11 @@ if __name__ == '__main__':
                     name = line.name()
                     consumer = line.consumer()
                     direction = line.direction()
-                    active_state = line.active_state()
+                    active_low = line.is_active_low()
 
                     print('\tline {:>3}: {:>18} {:>12} {:>8} {:>10}'.format(
                           offset,
                           'unnamed' if name is None else name,
                           'unused' if consumer is None else consumer,
                           'input' if direction == gpiod.Line.DIRECTION_INPUT else 'output',
-                          'active-low' if active_state == gpiod.Line.ACTIVE_LOW else 'active-high'))
+                          'active-low' if active_low else 'active-high'))
diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
index e8641f1..b48a83a 100644
--- a/bindings/python/gpiodmodule.c
+++ b/bindings/python/gpiodmodule.c
@@ -67,11 +67,6 @@ enum {
 	gpiod_DIRECTION_OUTPUT,
 };
 
-enum {
-	gpiod_ACTIVE_HIGH = 1,
-	gpiod_ACTIVE_LOW,
-};
-
 enum {
 	gpiod_BIAS_AS_IS = 1,
 	gpiod_BIAS_DISABLE,
@@ -341,28 +336,20 @@ static PyObject *gpiod_Line_direction(gpiod_LineObject *self,
 	return ret;
 }
 
-PyDoc_STRVAR(gpiod_Line_active_state_doc,
-"active_state() -> integer\n"
+PyDoc_STRVAR(gpiod_Line_is_active_low_doc,
+"is_active_low() -> boolean\n"
 "\n"
-"Get the active state setting of this GPIO line.");
+"Check if this line's signal is inverted");
 
-static PyObject *gpiod_Line_active_state(gpiod_LineObject *self,
-					 PyObject *Py_UNUSED(ignored))
+static PyObject *gpiod_Line_is_active_low(gpiod_LineObject *self,
+					  PyObject *Py_UNUSED(ignored))
 {
-	PyObject *ret;
-	int active;
-
 	if (gpiod_ChipIsClosed(self->owner))
 		return NULL;
 
-	active = gpiod_line_active_state(self->line);
-
-	if (active == GPIOD_LINE_ACTIVE_STATE_HIGH)
-		ret = Py_BuildValue("I", gpiod_ACTIVE_HIGH);
-	else
-		ret = Py_BuildValue("I", gpiod_ACTIVE_LOW);
-
-	return ret;
+	if (gpiod_line_is_active_low(self->line))
+		Py_RETURN_TRUE;
+	Py_RETURN_FALSE;
 }
 
 PyDoc_STRVAR(gpiod_Line_bias_doc,
@@ -973,10 +960,10 @@ static PyMethodDef gpiod_Line_methods[] = {
 		.ml_doc = gpiod_Line_direction_doc,
 	},
 	{
-		.ml_name = "active_state",
-		.ml_meth = (PyCFunction)gpiod_Line_active_state,
+		.ml_name = "is_active_low",
+		.ml_meth = (PyCFunction)gpiod_Line_is_active_low,
 		.ml_flags = METH_NOARGS,
-		.ml_doc = gpiod_Line_active_state_doc,
+		.ml_doc = gpiod_Line_is_active_low_doc,
 	},
 	{
 		.ml_name = "bias",
@@ -2538,16 +2525,6 @@ static gpiod_ConstDescr gpiod_ConstList[] = {
 		.name = "DIRECTION_OUTPUT",
 		.val = gpiod_DIRECTION_OUTPUT,
 	},
-	{
-		.typeobj = &gpiod_LineType,
-		.name = "ACTIVE_HIGH",
-		.val = gpiod_ACTIVE_HIGH,
-	},
-	{
-		.typeobj = &gpiod_LineType,
-		.name = "ACTIVE_LOW",
-		.val = gpiod_ACTIVE_LOW,
-	},
 	{
 		.typeobj = &gpiod_LineType,
 		.name = "BIAS_AS_IS",
diff --git a/bindings/python/tests/gpiod_py_test.py b/bindings/python/tests/gpiod_py_test.py
index 79294bc..3093a1c 100755
--- a/bindings/python/tests/gpiod_py_test.py
+++ b/bindings/python/tests/gpiod_py_test.py
@@ -205,7 +205,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_INPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), None)
             self.assertFalse(line.is_used())
             self.assertFalse(line.is_requested())
@@ -219,7 +219,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_LOW)
+            self.assertTrue(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -235,7 +235,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_LOW)
+            self.assertTrue(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -253,7 +253,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -271,7 +271,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -289,7 +289,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -307,7 +307,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
@@ -325,7 +325,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.offset(), 4)
             self.assertEqual(line.name(), 'gpio-mockup-A-4')
             self.assertEqual(line.direction(), gpiod.Line.DIRECTION_OUTPUT)
-            self.assertEqual(line.active_state(), gpiod.Line.ACTIVE_HIGH)
+            self.assertFalse(line.is_active_low())
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
diff --git a/include/gpiod.h b/include/gpiod.h
index b073b3b..75da84c 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -280,16 +280,6 @@ enum {
 	/**< Direction is output - we're driving the GPIO line. */
 };
 
-/**
- * @brief Possible active state settings.
- */
-enum {
-	GPIOD_LINE_ACTIVE_STATE_HIGH = 1,
-	/**< The active state of a GPIO is active-high. */
-	GPIOD_LINE_ACTIVE_STATE_LOW,
-	/**< The active state of a GPIO is active-low. */
-};
-
 /**
  * @brief Possible internal bias settings.
  */
@@ -337,11 +327,11 @@ const char *gpiod_line_consumer(struct gpiod_line *line) GPIOD_API;
 int gpiod_line_direction(struct gpiod_line *line) GPIOD_API;
 
 /**
- * @brief Read the GPIO line active state setting.
+ * @brief Check if the signal of this line is inverted.
  * @param line GPIO line object.
- * @return Returns GPIOD_LINE_ACTIVE_STATE_HIGH or GPIOD_LINE_ACTIVE_STATE_LOW.
+ * @return True if this line is "active-low", false otherwise.
  */
-int gpiod_line_active_state(struct gpiod_line *line) GPIOD_API;
+bool gpiod_line_is_active_low(struct gpiod_line *line) GPIOD_API;
 
 /**
  * @brief Read the GPIO line bias setting.
diff --git a/lib/core.c b/lib/core.c
index d96e6cf..c6fb474 100644
--- a/lib/core.c
+++ b/lib/core.c
@@ -41,8 +41,8 @@ struct gpiod_line {
 	/* The direction of the GPIO line. */
 	int direction;
 
-	/* The active-state configuration. */
-	int active_state;
+	/* Is this line active-low?. */
+	bool active_low;
 
 	/* The logical value last written to the line. */
 	int output_value;
@@ -471,9 +471,9 @@ int gpiod_line_direction(struct gpiod_line *line)
 	return line->direction;
 }
 
-int gpiod_line_active_state(struct gpiod_line *line)
+bool gpiod_line_is_active_low(struct gpiod_line *line)
 {
-	return line->active_state;
+	return line->active_low;
 }
 
 int gpiod_line_bias(struct gpiod_line *line)
@@ -541,9 +541,7 @@ int gpiod_line_update(struct gpiod_line *line)
 						? GPIOD_LINE_DIRECTION_OUTPUT
 						: GPIOD_LINE_DIRECTION_INPUT;
 
-	line->active_state = info.flags & GPIO_V2_LINE_FLAG_ACTIVE_LOW
-						? GPIOD_LINE_ACTIVE_STATE_LOW
-						: GPIOD_LINE_ACTIVE_STATE_HIGH;
+	line->active_low = !!(info.flags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
 
 	line->info_flags = line_info_v2_to_info_flags(&info);
 
diff --git a/tests/tests-line.c b/tests/tests-line.c
index cc66fcc..d6264af 100644
--- a/tests/tests-line.c
+++ b/tests/tests-line.c
@@ -340,12 +340,10 @@ GPIOD_TEST_CASE(set_config_bulk_null_values, 0, { 8 })
 	ret = gpiod_line_request_bulk_output(bulk, GPIOD_TEST_CONSUMER, 0);
 	g_assert_cmpint(ret, ==, 0);
 	gpiod_test_return_if_failed();
-	g_assert_cmpint(gpiod_line_active_state(line0), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
-	g_assert_cmpint(gpiod_line_active_state(line1), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
-	g_assert_cmpint(gpiod_line_active_state(line2), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
+	g_assert_false(gpiod_line_is_active_low(line0));
+	g_assert_false(gpiod_line_is_active_low(line1));
+	g_assert_false(gpiod_line_is_active_low(line2));
+
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 0), ==, 0);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 1), ==, 0);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
@@ -354,12 +352,9 @@ GPIOD_TEST_CASE(set_config_bulk_null_values, 0, { 8 })
 			GPIOD_LINE_REQUEST_DIRECTION_OUTPUT,
 			GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW, NULL);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_active_state(line0), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
-	g_assert_cmpint(gpiod_line_active_state(line1), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
-	g_assert_cmpint(gpiod_line_active_state(line2), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line0));
+	g_assert_true(gpiod_line_is_active_low(line1));
+	g_assert_true(gpiod_line_is_active_low(line2));
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 0), ==, 1);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 1), ==, 1);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 1);
@@ -367,12 +362,9 @@ GPIOD_TEST_CASE(set_config_bulk_null_values, 0, { 8 })
 	ret = gpiod_line_set_config_bulk(bulk,
 			GPIOD_LINE_REQUEST_DIRECTION_OUTPUT, 0, NULL);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_active_state(line0), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
-	g_assert_cmpint(gpiod_line_active_state(line1), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
-	g_assert_cmpint(gpiod_line_active_state(line2), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
+	g_assert_false(gpiod_line_is_active_low(line0));
+	g_assert_false(gpiod_line_is_active_low(line1));
+	g_assert_false(gpiod_line_is_active_low(line2));
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 0), ==, 0);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 1), ==, 0);
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
@@ -395,20 +387,17 @@ GPIOD_TEST_CASE(set_flags_active_state, 0, { 8 })
 	ret = gpiod_line_request_output(line, GPIOD_TEST_CONSUMER, 1);
 	g_assert_cmpint(ret, ==, 0);
 	gpiod_test_return_if_failed();
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
+	g_assert_false(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 1);
 
 	ret = gpiod_line_set_flags(line, GPIOD_LINE_REQUEST_FLAG_ACTIVE_LOW);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
 
 	ret = gpiod_line_set_flags(line, 0);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
+	g_assert_false(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 1);
 }
 
@@ -719,8 +708,7 @@ GPIOD_TEST_CASE(active_state, 0, { 8 })
 	g_assert_cmpint(ret, ==, 0);
 	gpiod_test_return_if_failed();
 
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_HIGH);
+	g_assert_false(gpiod_line_is_active_low(line));
 
 	gpiod_line_release(line);
 
@@ -842,8 +830,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_true(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
 
@@ -860,8 +847,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_true(gpiod_line_is_open_source(line));
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line));
 
 	gpiod_line_release(line);
 
@@ -882,8 +868,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_PULL_DOWN);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_INPUT);
 
@@ -903,8 +888,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_PULL_UP);
-	g_assert_cmpint(gpiod_line_active_state(line), ==,
-			GPIOD_LINE_ACTIVE_STATE_LOW);
+	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_INPUT);
 
diff --git a/tools/gpioinfo.c b/tools/gpioinfo.c
index 057a19f..8d228ab 100644
--- a/tools/gpioinfo.c
+++ b/tools/gpioinfo.c
@@ -117,11 +117,11 @@ static PRINTF(3, 4) void prinfo(bool *of,
 
 static void list_lines(struct gpiod_chip *chip)
 {
-	int direction, active_state;
+	bool flag_printed, of, active_low;
 	const char *name, *consumer;
 	struct gpiod_line *line;
 	unsigned int i, offset;
-	bool flag_printed, of;
+	int direction;
 
 	printf("%s - %u lines:\n",
 	       gpiod_chip_name(chip), gpiod_chip_num_lines(chip));
@@ -134,7 +134,7 @@ static void list_lines(struct gpiod_chip *chip)
 		name = gpiod_line_name(line);
 		consumer = gpiod_line_consumer(line);
 		direction = gpiod_line_direction(line);
-		active_state = gpiod_line_active_state(line);
+		active_low = gpiod_line_is_active_low(line);
 
 		of = false;
 
@@ -157,9 +157,7 @@ static void list_lines(struct gpiod_chip *chip)
 		prinfo(&of, 8, "%s ", direction == GPIOD_LINE_DIRECTION_INPUT
 							? "input" : "output");
 		prinfo(&of, 13, "%s ",
-		       active_state == GPIOD_LINE_ACTIVE_STATE_LOW
-							? "active-low"
-							: "active-high");
+		       active_low ? "active-low" : "active-high");
 
 		flag_printed = false;
 		for (i = 0; i < ARRAY_SIZE(flags); i++) {
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 1/6] treewide: remove helpers for opening chips by name & number Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 2/6] treewide: simplify the active-low line property Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 14:31   ` Kent Gibson
  2021-01-11 13:34 ` [libgpiod][PATCH 4/6] treewide: make drive settings an enum Bartosz Golaszewski
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

When inspecting the current bias setting of a GPIO line, the AS_IS name
of one of the possible values really means that there's no bias so it
should be renamed to NONE for clarity.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 bindings/cxx/gpiod.hpp                 |  2 +-
 bindings/cxx/line.cpp                  |  2 +-
 bindings/cxx/tests/tests-line.cpp      |  8 ++++----
 bindings/python/gpiodmodule.c          | 10 +++++-----
 bindings/python/tests/gpiod_py_test.py |  6 +++---
 include/gpiod.h                        |  4 ++--
 lib/core.c                             |  2 +-
 tests/tests-line.c                     | 12 ++++++------
 8 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
index 8b4a8f9..fb675fc 100644
--- a/bindings/cxx/gpiod.hpp
+++ b/bindings/cxx/gpiod.hpp
@@ -486,7 +486,7 @@ public:
 	 * @brief Possible bias settings.
 	 */
 	enum : int {
-		BIAS_AS_IS = 1,
+		BIAS_NONE = 1,
 		/**< Line's bias state is unknown. */
 		BIAS_DISABLE,
 		/**< Line's internal bias is disabled. */
diff --git a/bindings/cxx/line.cpp b/bindings/cxx/line.cpp
index 5a907db..19c974d 100644
--- a/bindings/cxx/line.cpp
+++ b/bindings/cxx/line.cpp
@@ -17,7 +17,7 @@ const ::std::map<int, int> bias_mapping = {
 	{ GPIOD_LINE_BIAS_PULL_UP,	line::BIAS_PULL_UP, },
 	{ GPIOD_LINE_BIAS_PULL_DOWN,	line::BIAS_PULL_DOWN, },
 	{ GPIOD_LINE_BIAS_DISABLE,	line::BIAS_DISABLE, },
-	{ GPIOD_LINE_BIAS_AS_IS,	line::BIAS_AS_IS, },
+	{ GPIOD_LINE_BIAS_NONE,		line::BIAS_NONE, },
 };
 
 } /* namespace */
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index 3c7ea39..2a6cdf8 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -35,7 +35,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
 		REQUIRE_FALSE(line.is_open_source());
-		REQUIRE(line.bias() == ::gpiod::line::BIAS_AS_IS);
+		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
 	SECTION("exported line")
@@ -54,7 +54,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
 		REQUIRE_FALSE(line.is_open_source());
-		REQUIRE(line.bias() == ::gpiod::line::BIAS_AS_IS);
+		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
 	SECTION("exported line with flags")
@@ -75,7 +75,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.is_used());
 		REQUIRE(line.is_open_drain());
 		REQUIRE_FALSE(line.is_open_source());
-		REQUIRE(line.bias() == ::gpiod::line::BIAS_AS_IS);
+		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
 	SECTION("exported open source line")
@@ -95,7 +95,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.is_used());
 		REQUIRE_FALSE(line.is_open_drain());
 		REQUIRE(line.is_open_source());
-		REQUIRE(line.bias() == ::gpiod::line::BIAS_AS_IS);
+		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
 	SECTION("exported bias disable line")
diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
index b48a83a..228ac73 100644
--- a/bindings/python/gpiodmodule.c
+++ b/bindings/python/gpiodmodule.c
@@ -68,7 +68,7 @@ enum {
 };
 
 enum {
-	gpiod_BIAS_AS_IS = 1,
+	gpiod_BIAS_NONE = 1,
 	gpiod_BIAS_DISABLE,
 	gpiod_BIAS_PULL_UP,
 	gpiod_BIAS_PULL_DOWN,
@@ -374,9 +374,9 @@ static PyObject *gpiod_Line_bias(gpiod_LineObject *self,
 		return Py_BuildValue("I", gpiod_BIAS_PULL_DOWN);
 	case GPIOD_LINE_BIAS_DISABLE:
 		return Py_BuildValue("I", gpiod_BIAS_DISABLE);
-	case GPIOD_LINE_BIAS_AS_IS:
+	case GPIOD_LINE_BIAS_NONE:
 	default:
-		return Py_BuildValue("I", gpiod_BIAS_AS_IS);
+		return Py_BuildValue("I", gpiod_BIAS_NONE);
 	}
 }
 
@@ -2527,8 +2527,8 @@ static gpiod_ConstDescr gpiod_ConstList[] = {
 	},
 	{
 		.typeobj = &gpiod_LineType,
-		.name = "BIAS_AS_IS",
-		.val = gpiod_BIAS_AS_IS,
+		.name = "BIAS_NONE",
+		.val = gpiod_BIAS_NONE,
 	},
 	{
 		.typeobj = &gpiod_LineType,
diff --git a/bindings/python/tests/gpiod_py_test.py b/bindings/python/tests/gpiod_py_test.py
index 3093a1c..d9521de 100755
--- a/bindings/python/tests/gpiod_py_test.py
+++ b/bindings/python/tests/gpiod_py_test.py
@@ -241,7 +241,7 @@ class LineInfo(MockupTestCase):
             self.assertTrue(line.is_requested())
             self.assertTrue(line.is_open_drain())
             self.assertFalse(line.is_open_source())
-            self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
+            self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_open_drain_line(self):
         with gpiod.Chip(mockup.chip_path(0)) as chip:
@@ -259,7 +259,7 @@ class LineInfo(MockupTestCase):
             self.assertTrue(line.is_requested())
             self.assertTrue(line.is_open_drain())
             self.assertFalse(line.is_open_source())
-            self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
+            self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_open_source_line(self):
         with gpiod.Chip(mockup.chip_path(0)) as chip:
@@ -277,7 +277,7 @@ class LineInfo(MockupTestCase):
             self.assertTrue(line.is_requested())
             self.assertFalse(line.is_open_drain())
             self.assertTrue(line.is_open_source())
-            self.assertEqual(line.bias(), gpiod.Line.BIAS_AS_IS)
+            self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_bias_disable_line(self):
         with gpiod.Chip(mockup.chip_path(0)) as chip:
diff --git a/include/gpiod.h b/include/gpiod.h
index 75da84c..18017d5 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -284,7 +284,7 @@ enum {
  * @brief Possible internal bias settings.
  */
 enum {
-	GPIOD_LINE_BIAS_AS_IS = 1,
+	GPIOD_LINE_BIAS_NONE = 1,
 	/**< The internal bias state is unknown. */
 	GPIOD_LINE_BIAS_DISABLE,
 	/**< The internal bias is disabled. */
@@ -337,7 +337,7 @@ bool gpiod_line_is_active_low(struct gpiod_line *line) GPIOD_API;
  * @brief Read the GPIO line bias setting.
  * @param line GPIO line object.
  * @return Returns GPIOD_LINE_BIAS_PULL_UP, GPIOD_LINE_BIAS_PULL_DOWN,
- *         GPIOD_LINE_BIAS_DISABLE or GPIOD_LINE_BIAS_AS_IS.
+ *         GPIOD_LINE_BIAS_DISABLE or GPIOD_LINE_BIAS_NONE.
  */
 int gpiod_line_bias(struct gpiod_line *line) GPIOD_API;
 
diff --git a/lib/core.c b/lib/core.c
index c6fb474..60b39c6 100644
--- a/lib/core.c
+++ b/lib/core.c
@@ -485,7 +485,7 @@ int gpiod_line_bias(struct gpiod_line *line)
 	if (line->info_flags & GPIOLINE_FLAG_BIAS_PULL_DOWN)
 		return GPIOD_LINE_BIAS_PULL_DOWN;
 
-	return GPIOD_LINE_BIAS_AS_IS;
+	return GPIOD_LINE_BIAS_NONE;
 }
 
 bool gpiod_line_is_used(struct gpiod_line *line)
diff --git a/tests/tests-line.c b/tests/tests-line.c
index d6264af..07d5601 100644
--- a/tests/tests-line.c
+++ b/tests/tests-line.c
@@ -418,7 +418,7 @@ GPIOD_TEST_CASE(set_flags_bias, 0, { 8 })
 	ret = gpiod_line_request_input(line, GPIOD_TEST_CONSUMER);
 	g_assert_cmpint(ret, ==, 0);
 	gpiod_test_return_if_failed();
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 
 	ret = gpiod_line_set_flags(line,
 		GPIOD_LINE_REQUEST_FLAG_BIAS_DISABLE);
@@ -762,7 +762,7 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	g_assert_false(gpiod_line_is_used(line));
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 
 	config.request_type = GPIOD_LINE_REQUEST_DIRECTION_OUTPUT;
 	config.consumer = GPIOD_TEST_CONSUMER;
@@ -775,7 +775,7 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	g_assert_true(gpiod_line_is_used(line));
 	g_assert_true(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
 
@@ -790,7 +790,7 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	g_assert_true(gpiod_line_is_used(line));
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_true(gpiod_line_is_open_source(line));
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
 
@@ -829,7 +829,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_true(gpiod_line_is_used(line));
 	g_assert_true(gpiod_line_is_open_drain(line));
 	g_assert_false(gpiod_line_is_open_source(line));
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
@@ -846,7 +846,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	g_assert_true(gpiod_line_is_used(line));
 	g_assert_false(gpiod_line_is_open_drain(line));
 	g_assert_true(gpiod_line_is_open_source(line));
-	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_AS_IS);
+	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_true(gpiod_line_is_active_low(line));
 
 	gpiod_line_release(line);
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 4/6] treewide: make drive settings an enum
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2021-01-11 13:34 ` [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 14:39   ` Kent Gibson
  2021-01-11 13:34 ` [libgpiod][PATCH 5/6] bindings: cxx: line: reorder bias mapping entries Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository Bartosz Golaszewski
  5 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Open-source and open-drain drive settings are mutually exclusive just like
the bias settings. Make them into an enum so that becomes clear.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 bindings/cxx/gpiod.hpp                 | 24 +++++----
 bindings/cxx/line.cpp                  | 18 +++----
 bindings/cxx/tests/tests-line.cpp      | 45 ++++++----------
 bindings/python/gpiodmodule.c          | 73 ++++++++++++++------------
 bindings/python/tests/gpiod_py_test.py | 18 +++----
 include/gpiod.h                        | 26 +++++----
 lib/core.c                             | 12 ++---
 tests/tests-line.c                     | 36 ++++++-------
 tools/gpioinfo.c                       | 14 ++++-
 9 files changed, 134 insertions(+), 132 deletions(-)

diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
index fb675fc..f9c341d 100644
--- a/bindings/cxx/gpiod.hpp
+++ b/bindings/cxx/gpiod.hpp
@@ -332,16 +332,10 @@ public:
 	GPIOD_API bool is_used(void) const;
 
 	/**
-	 * @brief Check if this line represents an open-drain GPIO.
-	 * @return True if the line is an open-drain GPIO, false otherwise.
+	 * @brief Get current drive setting of this line.
+	 * @return Current drive setting.
 	 */
-	GPIOD_API bool is_open_drain(void) const;
-
-	/**
-	 * @brief Check if this line represents an open-source GPIO.
-	 * @return True if the line is an open-source GPIO, false otherwise.
-	 */
-	GPIOD_API bool is_open_source(void) const;
+	GPIOD_API int drive(void) const;
 
 	/**
 	 * @brief Request this line.
@@ -482,6 +476,18 @@ public:
 		/**< Line's direction setting is output. */
 	};
 
+	/**
+	 * @brief Possible drive settings.
+	 */
+	enum : int {
+		DRIVE_NONE = 1,
+		/**< Drive setting is unknown. */
+		DRIVE_OPEN_DRAIN,
+		/**< Line output is open-drain. */
+		DRIVE_OPEN_SOURCE,
+		/**< Line output is open-source. */
+	};
+
 	/**
 	 * @brief Possible bias settings.
 	 */
diff --git a/bindings/cxx/line.cpp b/bindings/cxx/line.cpp
index 19c974d..04b15f7 100644
--- a/bindings/cxx/line.cpp
+++ b/bindings/cxx/line.cpp
@@ -13,6 +13,12 @@ namespace gpiod {
 
 namespace {
 
+const ::std::map<int, int> drive_mapping = {
+	{ GPIOD_LINE_DRIVE_NONE,	line::DRIVE_NONE, },
+	{ GPIOD_LINE_DRIVE_OPEN_DRAIN,	line::DRIVE_OPEN_DRAIN, },
+	{ GPIOD_LINE_DRIVE_OPEN_SOURCE,	line::DRIVE_OPEN_SOURCE, },
+};
+
 const ::std::map<int, int> bias_mapping = {
 	{ GPIOD_LINE_BIAS_PULL_UP,	line::BIAS_PULL_UP, },
 	{ GPIOD_LINE_BIAS_PULL_DOWN,	line::BIAS_PULL_DOWN, },
@@ -98,20 +104,12 @@ bool line::is_used(void) const
 	return ::gpiod_line_is_used(this->_m_line);
 }
 
-bool line::is_open_drain(void) const
-{
-	this->throw_if_null();
-	line::chip_guard lock_chip(*this);
-
-	return ::gpiod_line_is_open_drain(this->_m_line);
-}
-
-bool line::is_open_source(void) const
+int line::drive(void) const
 {
 	this->throw_if_null();
 	line::chip_guard lock_chip(*this);
 
-	return ::gpiod_line_is_open_source(this->_m_line);
+	return drive_mapping.at(::gpiod_line_drive(this->_m_line));
 }
 
 void line::request(const line_request& config, int default_val) const
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index 2a6cdf8..852b8a0 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -33,8 +33,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.consumer().empty());
 		REQUIRE_FALSE(line.is_requested());
 		REQUIRE_FALSE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
@@ -52,8 +51,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
@@ -73,8 +71,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_DRAIN);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
@@ -93,8 +90,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_SOURCE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_NONE);
 	}
 
@@ -113,8 +109,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_DISABLE);
 	}
 
@@ -133,8 +128,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_active_low());;
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_PULL_DOWN);
 	}
 
@@ -153,8 +147,7 @@ TEST_CASE("Line information can be correctly retrieved", "[line]")
 		REQUIRE_FALSE(line.is_active_low());
 		REQUIRE(line.is_requested());
 		REQUIRE(line.is_used());
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 		REQUIRE(line.bias() == ::gpiod::line::BIAS_PULL_UP);
 	}
 
@@ -364,23 +357,19 @@ TEST_CASE("Line can be reconfigured", "[line]")
 		config.flags = 0;
 		line.request(config);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 
 		line.set_flags(::gpiod::line_request::FLAG_OPEN_DRAIN);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_DRAIN);
 
 		line.set_flags(::gpiod::line_request::FLAG_OPEN_SOURCE);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_SOURCE);
 
 		line.set_flags(0);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 	}
 
 	SECTION("set flags (single line, bias)")
@@ -390,23 +379,19 @@ TEST_CASE("Line can be reconfigured", "[line]")
 		config.flags = 0;
 		line.request(config);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 
 		line.set_flags(::gpiod::line_request::FLAG_OPEN_DRAIN);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_DRAIN);
 
 		line.set_flags(::gpiod::line_request::FLAG_OPEN_SOURCE);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_OPEN_SOURCE);
 
 		line.set_flags(0);
 		REQUIRE(line.direction() == ::gpiod::line::DIRECTION_OUTPUT);
-		REQUIRE_FALSE(line.is_open_drain());
-		REQUIRE_FALSE(line.is_open_source());
+		REQUIRE(line.drive() == ::gpiod::line::DRIVE_NONE);
 	}
 
 	SECTION("set direction input (single line)")
diff --git a/bindings/python/gpiodmodule.c b/bindings/python/gpiodmodule.c
index 228ac73..a1a7f98 100644
--- a/bindings/python/gpiodmodule.c
+++ b/bindings/python/gpiodmodule.c
@@ -67,6 +67,12 @@ enum {
 	gpiod_DIRECTION_OUTPUT,
 };
 
+enum {
+	gpiod_DRIVE_NONE = 1,
+	gpiod_DRIVE_OPEN_DRAIN,
+	gpiod_DRIVE_OPEN_SOURCE,
+};
+
 enum {
 	gpiod_BIAS_NONE = 1,
 	gpiod_BIAS_DISABLE,
@@ -397,38 +403,30 @@ static PyObject *gpiod_Line_is_used(gpiod_LineObject *self,
 	Py_RETURN_FALSE;
 }
 
-PyDoc_STRVAR(gpiod_Line_is_open_drain_doc,
-"is_open_drain() -> boolean\n"
+PyDoc_STRVAR(gpiod_Line_drive_doc,
+"drive() -> integer\n"
 "\n"
-"Check if this line represents an open-drain GPIO.");
+"Get the current drive setting of this GPIO line.");
 
-static PyObject *gpiod_Line_is_open_drain(gpiod_LineObject *self,
-					  PyObject *Py_UNUSED(ignored))
+static PyObject *gpiod_Line_drive(gpiod_LineObject *self,
+				  PyObject *Py_UNUSED(ignored))
 {
-	if (gpiod_ChipIsClosed(self->owner))
-		return NULL;
-
-	if (gpiod_line_is_open_drain(self->line))
-		Py_RETURN_TRUE;
-
-	Py_RETURN_FALSE;
-}
+	int drive;
 
-PyDoc_STRVAR(gpiod_Line_is_open_source_doc,
-"is_open_source() -> boolean\n"
-"\n"
-"Check if this line represents an open-source GPIO.");
-
-static PyObject *gpiod_Line_is_open_source(gpiod_LineObject *self,
-					   PyObject *Py_UNUSED(ignored))
-{
 	if (gpiod_ChipIsClosed(self->owner))
 		return NULL;
 
-	if (gpiod_line_is_open_source(self->line))
-		Py_RETURN_TRUE;
+	drive = gpiod_line_drive(self->line);
 
-	Py_RETURN_FALSE;
+	switch (drive) {
+	case GPIOD_LINE_DRIVE_OPEN_DRAIN:
+		return Py_BuildValue("I", gpiod_DRIVE_OPEN_DRAIN);
+	case GPIOD_LINE_DRIVE_OPEN_SOURCE:
+		return Py_BuildValue("I", gpiod_DRIVE_OPEN_SOURCE);
+	case GPIOD_LINE_DRIVE_NONE:
+	default:
+		return Py_BuildValue("I", gpiod_DRIVE_NONE);
+	}
 }
 
 PyDoc_STRVAR(gpiod_Line_request_doc,
@@ -978,16 +976,10 @@ static PyMethodDef gpiod_Line_methods[] = {
 		.ml_doc = gpiod_Line_is_used_doc,
 	},
 	{
-		.ml_name = "is_open_drain",
-		.ml_meth = (PyCFunction)gpiod_Line_is_open_drain,
+		.ml_name = "drive",
+		.ml_meth = (PyCFunction)gpiod_Line_drive,
 		.ml_flags = METH_NOARGS,
-		.ml_doc = gpiod_Line_is_open_drain_doc,
-	},
-	{
-		.ml_name = "is_open_source",
-		.ml_meth = (PyCFunction)gpiod_Line_is_open_source,
-		.ml_flags = METH_NOARGS,
-		.ml_doc = gpiod_Line_is_open_source_doc,
+		.ml_doc = gpiod_Line_drive_doc,
 	},
 	{
 		.ml_name = "request",
@@ -2525,6 +2517,21 @@ static gpiod_ConstDescr gpiod_ConstList[] = {
 		.name = "DIRECTION_OUTPUT",
 		.val = gpiod_DIRECTION_OUTPUT,
 	},
+	{
+		.typeobj = &gpiod_LineType,
+		.name = "DRIVE_NONE",
+		.val = gpiod_DRIVE_NONE,
+	},
+	{
+		.typeobj = &gpiod_LineType,
+		.name = "DRIVE_OPEN_DRAIN",
+		.val = gpiod_DRIVE_OPEN_DRAIN,
+	},
+	{
+		.typeobj = &gpiod_LineType,
+		.name = "DRIVE_OPEN_SOURCE",
+		.val = gpiod_DRIVE_OPEN_SOURCE,
+	},
 	{
 		.typeobj = &gpiod_LineType,
 		.name = "BIAS_NONE",
diff --git a/bindings/python/tests/gpiod_py_test.py b/bindings/python/tests/gpiod_py_test.py
index d9521de..4b05f01 100755
--- a/bindings/python/tests/gpiod_py_test.py
+++ b/bindings/python/tests/gpiod_py_test.py
@@ -239,8 +239,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertTrue(line.is_open_drain())
-            self.assertFalse(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_OPEN_DRAIN)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_open_drain_line(self):
@@ -257,8 +256,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertTrue(line.is_open_drain())
-            self.assertFalse(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_OPEN_DRAIN)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_open_source_line(self):
@@ -275,8 +273,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertFalse(line.is_open_drain())
-            self.assertTrue(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_OPEN_SOURCE)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_NONE)
 
     def test_exported_bias_disable_line(self):
@@ -293,8 +290,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertFalse(line.is_open_drain())
-            self.assertFalse(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_NONE)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_DISABLE)
 
     def test_exported_bias_pull_down_line(self):
@@ -311,8 +307,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertFalse(line.is_open_drain())
-            self.assertFalse(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_NONE)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_PULL_DOWN)
 
     def test_exported_bias_pull_up_line(self):
@@ -329,8 +324,7 @@ class LineInfo(MockupTestCase):
             self.assertEqual(line.consumer(), default_consumer)
             self.assertTrue(line.is_used())
             self.assertTrue(line.is_requested())
-            self.assertFalse(line.is_open_drain())
-            self.assertFalse(line.is_open_source())
+            self.assertEqual(line.drive(), gpiod.Line.DRIVE_NONE)
             self.assertEqual(line.bias(), gpiod.Line.BIAS_PULL_UP)
 
     def test_update_line_info(self):
diff --git a/include/gpiod.h b/include/gpiod.h
index 18017d5..4e8e47f 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -280,6 +280,18 @@ enum {
 	/**< Direction is output - we're driving the GPIO line. */
 };
 
+/**
+ * @brief Possible drive settings.
+ */
+enum {
+	GPIOD_LINE_DRIVE_NONE = 1,
+	/**< Drive setting is unknown. */
+	GPIOD_LINE_DRIVE_OPEN_DRAIN,
+	/**< Line output is open-drain. */
+	GPIOD_LINE_DRIVE_OPEN_SOURCE,
+	/**< Line output is open-source. */
+};
+
 /**
  * @brief Possible internal bias settings.
  */
@@ -353,18 +365,12 @@ int gpiod_line_bias(struct gpiod_line *line) GPIOD_API;
 bool gpiod_line_is_used(struct gpiod_line *line) GPIOD_API;
 
 /**
- * @brief Check if the line is an open-drain GPIO.
- * @param line GPIO line object.
- * @return True if the line is an open-drain GPIO, false otherwise.
- */
-bool gpiod_line_is_open_drain(struct gpiod_line *line) GPIOD_API;
-
-/**
- * @brief Check if the line is an open-source GPIO.
+ * @brief Read the GPIO line drive setting.
  * @param line GPIO line object.
- * @return True if the line is an open-source GPIO, false otherwise.
+ * @return Returns GPIOD_LINE_DRIVE_NONE, GPIOD_LINE_DRIVE_OPEN_DRAIN or
+ *         GPIOD_LINE_DRIVE_OPEN_SOURCE.
  */
-bool gpiod_line_is_open_source(struct gpiod_line *line) GPIOD_API;
+int gpiod_line_drive(struct gpiod_line *line) GPIOD_API;
 
 /**
  * @brief Re-read the line info.
diff --git a/lib/core.c b/lib/core.c
index 60b39c6..9629a4f 100644
--- a/lib/core.c
+++ b/lib/core.c
@@ -493,14 +493,14 @@ bool gpiod_line_is_used(struct gpiod_line *line)
 	return line->info_flags & GPIOLINE_FLAG_KERNEL;
 }
 
-bool gpiod_line_is_open_drain(struct gpiod_line *line)
+int gpiod_line_drive(struct gpiod_line *line)
 {
-	return line->info_flags & GPIOLINE_FLAG_OPEN_DRAIN;
-}
+	if (line->info_flags & GPIOLINE_FLAG_OPEN_DRAIN)
+		return GPIOD_LINE_DRIVE_OPEN_DRAIN;
+	if (line->info_flags & GPIOLINE_FLAG_OPEN_SOURCE)
+		return GPIOD_LINE_DRIVE_OPEN_SOURCE;
 
-bool gpiod_line_is_open_source(struct gpiod_line *line)
-{
-	return line->info_flags & GPIOLINE_FLAG_OPEN_SOURCE;
+	return GPIOD_LINE_DRIVE_NONE;
 }
 
 static int line_info_v2_to_info_flags(struct gpio_v2_line_info *info)
diff --git a/tests/tests-line.c b/tests/tests-line.c
index 07d5601..e6f0b69 100644
--- a/tests/tests-line.c
+++ b/tests/tests-line.c
@@ -455,20 +455,19 @@ GPIOD_TEST_CASE(set_flags_drive, 0, { 8 })
 	ret = gpiod_line_request_output(line, GPIOD_TEST_CONSUMER, 0);
 	g_assert_cmpint(ret, ==, 0);
 	gpiod_test_return_if_failed();
-	g_assert_cmpint(gpiod_line_is_open_drain(line), ==, false);
-	g_assert_cmpint(gpiod_line_is_open_source(line), ==, false);
+	g_assert_cmpint(gpiod_line_drive(line), ==, GPIOD_LINE_DRIVE_NONE);
 
 	ret = gpiod_line_set_flags(line,
 		GPIOD_LINE_REQUEST_FLAG_OPEN_DRAIN);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_is_open_drain(line), ==, true);
-	g_assert_cmpint(gpiod_line_is_open_source(line), ==, false);
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_DRAIN);
 
 	ret = gpiod_line_set_flags(line,
 		GPIOD_LINE_REQUEST_FLAG_OPEN_SOURCE);
 	g_assert_cmpint(ret, ==, 0);
-	g_assert_cmpint(gpiod_line_is_open_drain(line), ==, false);
-	g_assert_cmpint(gpiod_line_is_open_source(line), ==, true);
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_SOURCE);
 }
 
 GPIOD_TEST_CASE(set_direction, 0, { 8 })
@@ -760,8 +759,7 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_false(gpiod_line_is_used(line));
-	g_assert_false(gpiod_line_is_open_drain(line));
-	g_assert_false(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==, GPIOD_LINE_DRIVE_NONE);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 
 	config.request_type = GPIOD_LINE_REQUEST_DIRECTION_OUTPUT;
@@ -773,8 +771,8 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_true(gpiod_line_is_open_drain(line));
-	g_assert_false(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_DRAIN);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
@@ -788,8 +786,8 @@ GPIOD_TEST_CASE(misc_flags, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_false(gpiod_line_is_open_drain(line));
-	g_assert_true(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_SOURCE);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_cmpint(gpiod_line_direction(line), ==,
 			GPIOD_LINE_DIRECTION_OUTPUT);
@@ -827,8 +825,8 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_true(gpiod_line_is_open_drain(line));
-	g_assert_false(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_DRAIN);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
@@ -844,8 +842,8 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_false(gpiod_line_is_open_drain(line));
-	g_assert_true(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==,
+			GPIOD_LINE_DRIVE_OPEN_SOURCE);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_NONE);
 	g_assert_true(gpiod_line_is_active_low(line));
 
@@ -865,8 +863,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_false(gpiod_line_is_open_drain(line));
-	g_assert_false(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==, GPIOD_LINE_DRIVE_NONE);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_PULL_DOWN);
 	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
@@ -885,8 +882,7 @@ GPIOD_TEST_CASE(misc_flags_work_together, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	g_assert_true(gpiod_line_is_used(line));
-	g_assert_false(gpiod_line_is_open_drain(line));
-	g_assert_false(gpiod_line_is_open_source(line));
+	g_assert_cmpint(gpiod_line_drive(line), ==, GPIOD_LINE_DRIVE_NONE);
 	g_assert_cmpint(gpiod_line_bias(line), ==, GPIOD_LINE_BIAS_PULL_UP);
 	g_assert_true(gpiod_line_is_active_low(line));
 	g_assert_cmpint(gpiod_line_direction(line), ==,
diff --git a/tools/gpioinfo.c b/tools/gpioinfo.c
index 8d228ab..7da9b03 100644
--- a/tools/gpioinfo.c
+++ b/tools/gpioinfo.c
@@ -37,6 +37,16 @@ static bool line_bias_is_disabled(struct gpiod_line *line)
 	return gpiod_line_bias(line) == GPIOD_LINE_BIAS_DISABLE;
 }
 
+static bool line_drive_is_open_drain(struct gpiod_line *line)
+{
+	return gpiod_line_drive(line) == GPIOD_LINE_DRIVE_OPEN_DRAIN;
+}
+
+static bool line_drive_is_open_source(struct gpiod_line *line)
+{
+	return gpiod_line_drive(line) == GPIOD_LINE_DRIVE_OPEN_SOURCE;
+}
+
 static const struct flag flags[] = {
 	{
 		.name = "used",
@@ -44,11 +54,11 @@ static const struct flag flags[] = {
 	},
 	{
 		.name = "open-drain",
-		.is_set = gpiod_line_is_open_drain,
+		.is_set = line_drive_is_open_drain,
 	},
 	{
 		.name = "open-source",
-		.is_set = gpiod_line_is_open_source,
+		.is_set = line_drive_is_open_source,
 	},
 	{
 		.name = "pull-up",
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 5/6] bindings: cxx: line: reorder bias mapping entries
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2021-01-11 13:34 ` [libgpiod][PATCH 4/6] treewide: make drive settings an enum Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 13:34 ` [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository Bartosz Golaszewski
  5 siblings, 0 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Reorder the definitions so that they match the order in which they're
declared in the class. This is cosmetic only.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 bindings/cxx/line.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bindings/cxx/line.cpp b/bindings/cxx/line.cpp
index 04b15f7..cb9b6dc 100644
--- a/bindings/cxx/line.cpp
+++ b/bindings/cxx/line.cpp
@@ -20,10 +20,10 @@ const ::std::map<int, int> drive_mapping = {
 };
 
 const ::std::map<int, int> bias_mapping = {
+	{ GPIOD_LINE_BIAS_NONE,		line::BIAS_NONE, },
+	{ GPIOD_LINE_BIAS_DISABLE,	line::BIAS_DISABLE, },
 	{ GPIOD_LINE_BIAS_PULL_UP,	line::BIAS_PULL_UP, },
 	{ GPIOD_LINE_BIAS_PULL_DOWN,	line::BIAS_PULL_DOWN, },
-	{ GPIOD_LINE_BIAS_DISABLE,	line::BIAS_DISABLE, },
-	{ GPIOD_LINE_BIAS_NONE,		line::BIAS_NONE, },
 };
 
 } /* namespace */
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
                   ` (4 preceding siblings ...)
  2021-01-11 13:34 ` [libgpiod][PATCH 5/6] bindings: cxx: line: reorder bias mapping entries Bartosz Golaszewski
@ 2021-01-11 13:34 ` Bartosz Golaszewski
  2021-01-11 13:46   ` Andy Shevchenko
  5 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 13:34 UTC (permalink / raw)
  To: Kent Gibson, Andy Shevchenko, Linus Walleij
  Cc: linux-gpio, Bartosz Golaszewski

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

In order to avoid any problems with symbols missing from the host linux
kernel headers (for example: if current version of libgpiod supports
features that were added recently to the kernel but the host headers are
outdated and don't export required symbols) let's add the uapi header to
the repository and include it instead of the one in /usr/include/linux.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 configure.ac    |  12 +-
 lib/Makefile.am |   2 +-
 lib/core.c      |   3 +-
 lib/uapi/gpio.h | 522 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 528 insertions(+), 11 deletions(-)
 create mode 100644 lib/uapi/gpio.h

diff --git a/configure.ac b/configure.ac
index ddb9dc2..f6db096 100644
--- a/configure.ac
+++ b/configure.ac
@@ -93,16 +93,10 @@ AC_CHECK_HEADERS([getopt.h], [], [HEADER_NOT_FOUND_LIB([getopt.h])])
 AC_CHECK_HEADERS([dirent.h], [], [HEADER_NOT_FOUND_LIB([dirent.h])])
 AC_CHECK_HEADERS([sys/poll.h], [], [HEADER_NOT_FOUND_LIB([sys/poll.h])])
 AC_CHECK_HEADERS([sys/sysmacros.h], [], [HEADER_NOT_FOUND_LIB([sys/sysmacros.h])])
-AC_CHECK_HEADERS([linux/gpio.h], [], [HEADER_NOT_FOUND_LIB([linux/gpio.h])])
 AC_CHECK_HEADERS([linux/version.h], [], [HEADER_NOT_FOUND_LIB([linux/version.h])])
-
-AC_COMPILE_IFELSE([AC_LANG_SOURCE(
-#include <linux/version.h>
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0)
-#error
-#endif
-)],
-[], [AC_MSG_ERROR(["libgpiod needs linux headers version >= v5.10.0"])])
+AC_CHECK_HEADERS([linux/const.h], [], [HEADER_NOT_FOUND_LIB([linux/const.h])])
+AC_CHECK_HEADERS([linux/ioctl.h], [], [HEADER_NOT_FOUND_LIB([linux/ioctl.h])])
+AC_CHECK_HEADERS([linux/types.h], [], [HEADER_NOT_FOUND_LIB([linux/types.h])])
 
 AC_ARG_ENABLE([tools],
 	[AS_HELP_STRING([--enable-tools],[enable libgpiod command-line tools [default=no]])],
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 43ebf76..fb0dae9 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -7,7 +7,7 @@
 #
 
 lib_LTLIBRARIES = libgpiod.la
-libgpiod_la_SOURCES = core.c helpers.c misc.c
+libgpiod_la_SOURCES = core.c helpers.c misc.c uapi/gpio.h
 libgpiod_la_CFLAGS = -Wall -Wextra -g -std=gnu89
 libgpiod_la_CFLAGS += -fvisibility=hidden -I$(top_srcdir)/include/
 libgpiod_la_CFLAGS += -include $(top_builddir)/config.h
diff --git a/lib/core.c b/lib/core.c
index 9629a4f..5e19c67 100644
--- a/lib/core.c
+++ b/lib/core.c
@@ -11,7 +11,6 @@
 #include <fcntl.h>
 #include <gpiod.h>
 #include <limits.h>
-#include <linux/gpio.h>
 #include <poll.h>
 #include <stdint.h>
 #include <stdio.h>
@@ -22,6 +21,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "uapi/gpio.h"
+
 #define LINE_REQUEST_MAX_LINES	64
 
 enum {
diff --git a/lib/uapi/gpio.h b/lib/uapi/gpio.h
new file mode 100644
index 0000000..b8d2687
--- /dev/null
+++ b/lib/uapi/gpio.h
@@ -0,0 +1,522 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * <linux/gpio.h> - userspace ABI for the GPIO character devices
+ *
+ * Copyright (C) 2016 Linus Walleij
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#ifndef _GPIO_H_
+#define _GPIO_H_
+
+#include <linux/const.h>
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+/*
+ * The maximum size of name and label arrays.
+ *
+ * Must be a multiple of 8 to ensure 32/64-bit alignment of structs.
+ */
+#define GPIO_MAX_NAME_SIZE 32
+
+/**
+ * struct gpiochip_info - Information about a certain GPIO chip
+ * @name: the Linux kernel name of this GPIO chip
+ * @label: a functional name for this GPIO chip, such as a product
+ * number, may be empty (i.e. label[0] == '\0')
+ * @lines: number of GPIO lines on this chip
+ */
+struct gpiochip_info {
+	char name[GPIO_MAX_NAME_SIZE];
+	char label[GPIO_MAX_NAME_SIZE];
+	__u32 lines;
+};
+
+/*
+ * Maximum number of requested lines.
+ *
+ * Must be no greater than 64, as bitmaps are restricted here to 64-bits
+ * for simplicity, and a multiple of 2 to ensure 32/64-bit alignment of
+ * structs.
+ */
+#define GPIO_V2_LINES_MAX 64
+
+/*
+ * The maximum number of configuration attributes associated with a line
+ * request.
+ */
+#define GPIO_V2_LINE_NUM_ATTRS_MAX 10
+
+/**
+ * enum gpio_v2_line_flag - &struct gpio_v2_line_attribute.flags values
+ * @GPIO_V2_LINE_FLAG_USED: line is not available for request
+ * @GPIO_V2_LINE_FLAG_ACTIVE_LOW: line active state is physical low
+ * @GPIO_V2_LINE_FLAG_INPUT: line is an input
+ * @GPIO_V2_LINE_FLAG_OUTPUT: line is an output
+ * @GPIO_V2_LINE_FLAG_EDGE_RISING: line detects rising (inactive to active)
+ * edges
+ * @GPIO_V2_LINE_FLAG_EDGE_FALLING: line detects falling (active to
+ * inactive) edges
+ * @GPIO_V2_LINE_FLAG_OPEN_DRAIN: line is an open drain output
+ * @GPIO_V2_LINE_FLAG_OPEN_SOURCE: line is an open source output
+ * @GPIO_V2_LINE_FLAG_BIAS_PULL_UP: line has pull-up bias enabled
+ * @GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN: line has pull-down bias enabled
+ * @GPIO_V2_LINE_FLAG_BIAS_DISABLED: line has bias disabled
+ */
+enum gpio_v2_line_flag {
+	GPIO_V2_LINE_FLAG_USED			= _BITULL(0),
+	GPIO_V2_LINE_FLAG_ACTIVE_LOW		= _BITULL(1),
+	GPIO_V2_LINE_FLAG_INPUT			= _BITULL(2),
+	GPIO_V2_LINE_FLAG_OUTPUT		= _BITULL(3),
+	GPIO_V2_LINE_FLAG_EDGE_RISING		= _BITULL(4),
+	GPIO_V2_LINE_FLAG_EDGE_FALLING		= _BITULL(5),
+	GPIO_V2_LINE_FLAG_OPEN_DRAIN		= _BITULL(6),
+	GPIO_V2_LINE_FLAG_OPEN_SOURCE		= _BITULL(7),
+	GPIO_V2_LINE_FLAG_BIAS_PULL_UP		= _BITULL(8),
+	GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN	= _BITULL(9),
+	GPIO_V2_LINE_FLAG_BIAS_DISABLED		= _BITULL(10),
+};
+
+/**
+ * struct gpio_v2_line_values - Values of GPIO lines
+ * @bits: a bitmap containing the value of the lines, set to 1 for active
+ * and 0 for inactive.
+ * @mask: a bitmap identifying the lines to get or set, with each bit
+ * number corresponding to the index into &struct
+ * gpio_v2_line_request.offsets.
+ */
+struct gpio_v2_line_values {
+	__aligned_u64 bits;
+	__aligned_u64 mask;
+};
+
+/**
+ * enum gpio_v2_line_attr_id - &struct gpio_v2_line_attribute.id values
+ * identifying which field of the attribute union is in use.
+ * @GPIO_V2_LINE_ATTR_ID_FLAGS: flags field is in use
+ * @GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES: values field is in use
+ * @GPIO_V2_LINE_ATTR_ID_DEBOUNCE: debounce_period_us field is in use
+ */
+enum gpio_v2_line_attr_id {
+	GPIO_V2_LINE_ATTR_ID_FLAGS		= 1,
+	GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES	= 2,
+	GPIO_V2_LINE_ATTR_ID_DEBOUNCE		= 3,
+};
+
+/**
+ * struct gpio_v2_line_attribute - a configurable attribute of a line
+ * @id: attribute identifier with value from &enum gpio_v2_line_attr_id
+ * @padding: reserved for future use and must be zero filled
+ * @flags: if id is %GPIO_V2_LINE_ATTR_ID_FLAGS, the flags for the GPIO
+ * line, with values from &enum gpio_v2_line_flag, such as
+ * %GPIO_V2_LINE_FLAG_ACTIVE_LOW, %GPIO_V2_LINE_FLAG_OUTPUT etc, added
+ * together.  This overrides the default flags contained in the &struct
+ * gpio_v2_line_config for the associated line.
+ * @values: if id is %GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES, a bitmap
+ * containing the values to which the lines will be set, with each bit
+ * number corresponding to the index into &struct
+ * gpio_v2_line_request.offsets.
+ * @debounce_period_us: if id is %GPIO_V2_LINE_ATTR_ID_DEBOUNCE, the
+ * desired debounce period, in microseconds
+ */
+struct gpio_v2_line_attribute {
+	__u32 id;
+	__u32 padding;
+	union {
+		__aligned_u64 flags;
+		__aligned_u64 values;
+		__u32 debounce_period_us;
+	};
+};
+
+/**
+ * struct gpio_v2_line_config_attribute - a configuration attribute
+ * associated with one or more of the requested lines.
+ * @attr: the configurable attribute
+ * @mask: a bitmap identifying the lines to which the attribute applies,
+ * with each bit number corresponding to the index into &struct
+ * gpio_v2_line_request.offsets.
+ */
+struct gpio_v2_line_config_attribute {
+	struct gpio_v2_line_attribute attr;
+	__aligned_u64 mask;
+};
+
+/**
+ * struct gpio_v2_line_config - Configuration for GPIO lines
+ * @flags: flags for the GPIO lines, with values from &enum
+ * gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
+ * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together.  This is the default for
+ * all requested lines but may be overridden for particular lines using
+ * @attrs.
+ * @num_attrs: the number of attributes in @attrs
+ * @padding: reserved for future use and must be zero filled
+ * @attrs: the configuration attributes associated with the requested
+ * lines.  Any attribute should only be associated with a particular line
+ * once.  If an attribute is associated with a line multiple times then the
+ * first occurrence (i.e. lowest index) has precedence.
+ */
+struct gpio_v2_line_config {
+	__aligned_u64 flags;
+	__u32 num_attrs;
+	/* Pad to fill implicit padding and reserve space for future use. */
+	__u32 padding[5];
+	struct gpio_v2_line_config_attribute attrs[GPIO_V2_LINE_NUM_ATTRS_MAX];
+};
+
+/**
+ * struct gpio_v2_line_request - Information about a request for GPIO lines
+ * @offsets: an array of desired lines, specified by offset index for the
+ * associated GPIO chip
+ * @consumer: a desired consumer label for the selected GPIO lines such as
+ * "my-bitbanged-relay"
+ * @config: requested configuration for the lines.
+ * @num_lines: number of lines requested in this request, i.e. the number
+ * of valid fields in the %GPIO_V2_LINES_MAX sized arrays, set to 1 to
+ * request a single line
+ * @event_buffer_size: a suggested minimum number of line events that the
+ * kernel should buffer.  This is only relevant if edge detection is
+ * enabled in the configuration. Note that this is only a suggested value
+ * and the kernel may allocate a larger buffer or cap the size of the
+ * buffer. If this field is zero then the buffer size defaults to a minimum
+ * of @num_lines * 16.
+ * @padding: reserved for future use and must be zero filled
+ * @fd: if successful this field will contain a valid anonymous file handle
+ * after a %GPIO_GET_LINE_IOCTL operation, zero or negative value means
+ * error
+ */
+struct gpio_v2_line_request {
+	__u32 offsets[GPIO_V2_LINES_MAX];
+	char consumer[GPIO_MAX_NAME_SIZE];
+	struct gpio_v2_line_config config;
+	__u32 num_lines;
+	__u32 event_buffer_size;
+	/* Pad to fill implicit padding and reserve space for future use. */
+	__u32 padding[5];
+	__s32 fd;
+};
+
+/**
+ * struct gpio_v2_line_info - Information about a certain GPIO line
+ * @name: the name of this GPIO line, such as the output pin of the line on
+ * the chip, a rail or a pin header name on a board, as specified by the
+ * GPIO chip, may be empty (i.e. name[0] == '\0')
+ * @consumer: a functional name for the consumer of this GPIO line as set
+ * by whatever is using it, will be empty if there is no current user but
+ * may also be empty if the consumer doesn't set this up
+ * @offset: the local offset on this GPIO chip, fill this in when
+ * requesting the line information from the kernel
+ * @num_attrs: the number of attributes in @attrs
+ * @flags: flags for the GPIO lines, with values from &enum
+ * gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
+ * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together.
+ * @attrs: the configuration attributes associated with the line
+ * @padding: reserved for future use
+ */
+struct gpio_v2_line_info {
+	char name[GPIO_MAX_NAME_SIZE];
+	char consumer[GPIO_MAX_NAME_SIZE];
+	__u32 offset;
+	__u32 num_attrs;
+	__aligned_u64 flags;
+	struct gpio_v2_line_attribute attrs[GPIO_V2_LINE_NUM_ATTRS_MAX];
+	/* Space reserved for future use. */
+	__u32 padding[4];
+};
+
+/**
+ * enum gpio_v2_line_changed_type - &struct gpio_v2_line_changed.event_type
+ * values
+ * @GPIO_V2_LINE_CHANGED_REQUESTED: line has been requested
+ * @GPIO_V2_LINE_CHANGED_RELEASED: line has been released
+ * @GPIO_V2_LINE_CHANGED_CONFIG: line has been reconfigured
+ */
+enum gpio_v2_line_changed_type {
+	GPIO_V2_LINE_CHANGED_REQUESTED	= 1,
+	GPIO_V2_LINE_CHANGED_RELEASED	= 2,
+	GPIO_V2_LINE_CHANGED_CONFIG	= 3,
+};
+
+/**
+ * struct gpio_v2_line_info_changed - Information about a change in status
+ * of a GPIO line
+ * @info: updated line information
+ * @timestamp_ns: estimate of time of status change occurrence, in nanoseconds
+ * @event_type: the type of change with a value from &enum
+ * gpio_v2_line_changed_type
+ * @padding: reserved for future use
+ */
+struct gpio_v2_line_info_changed {
+	struct gpio_v2_line_info info;
+	__aligned_u64 timestamp_ns;
+	__u32 event_type;
+	/* Pad struct to 64-bit boundary and reserve space for future use. */
+	__u32 padding[5];
+};
+
+/**
+ * enum gpio_v2_line_event_id - &struct gpio_v2_line_event.id values
+ * @GPIO_V2_LINE_EVENT_RISING_EDGE: event triggered by a rising edge
+ * @GPIO_V2_LINE_EVENT_FALLING_EDGE: event triggered by a falling edge
+ */
+enum gpio_v2_line_event_id {
+	GPIO_V2_LINE_EVENT_RISING_EDGE	= 1,
+	GPIO_V2_LINE_EVENT_FALLING_EDGE	= 2,
+};
+
+/**
+ * struct gpio_v2_line_event - The actual event being pushed to userspace
+ * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds.
+ * The @timestamp_ns is read from %CLOCK_MONOTONIC and is intended to allow
+ * the accurate measurement of the time between events. It does not provide
+ * the wall-clock time.
+ * @id: event identifier with value from &enum gpio_v2_line_event_id
+ * @offset: the offset of the line that triggered the event
+ * @seqno: the sequence number for this event in the sequence of events for
+ * all the lines in this line request
+ * @line_seqno: the sequence number for this event in the sequence of
+ * events on this particular line
+ * @padding: reserved for future use
+ */
+struct gpio_v2_line_event {
+	__aligned_u64 timestamp_ns;
+	__u32 id;
+	__u32 offset;
+	__u32 seqno;
+	__u32 line_seqno;
+	/* Space reserved for future use. */
+	__u32 padding[6];
+};
+
+/*
+ * ABI v1
+ *
+ * This version of the ABI is deprecated.
+ * Use the latest version of the ABI, defined above, instead.
+ */
+
+/* Informational flags */
+#define GPIOLINE_FLAG_KERNEL		(1UL << 0) /* Line used by the kernel */
+#define GPIOLINE_FLAG_IS_OUT		(1UL << 1)
+#define GPIOLINE_FLAG_ACTIVE_LOW	(1UL << 2)
+#define GPIOLINE_FLAG_OPEN_DRAIN	(1UL << 3)
+#define GPIOLINE_FLAG_OPEN_SOURCE	(1UL << 4)
+#define GPIOLINE_FLAG_BIAS_PULL_UP	(1UL << 5)
+#define GPIOLINE_FLAG_BIAS_PULL_DOWN	(1UL << 6)
+#define GPIOLINE_FLAG_BIAS_DISABLE	(1UL << 7)
+
+/**
+ * struct gpioline_info - Information about a certain GPIO line
+ * @line_offset: the local offset on this GPIO device, fill this in when
+ * requesting the line information from the kernel
+ * @flags: various flags for this line
+ * @name: the name of this GPIO line, such as the output pin of the line on the
+ * chip, a rail or a pin header name on a board, as specified by the gpio
+ * chip, may be empty (i.e. name[0] == '\0')
+ * @consumer: a functional name for the consumer of this GPIO line as set by
+ * whatever is using it, will be empty if there is no current user but may
+ * also be empty if the consumer doesn't set this up
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_info instead.
+ */
+struct gpioline_info {
+	__u32 line_offset;
+	__u32 flags;
+	char name[GPIO_MAX_NAME_SIZE];
+	char consumer[GPIO_MAX_NAME_SIZE];
+};
+
+/* Maximum number of requested handles */
+#define GPIOHANDLES_MAX 64
+
+/* Possible line status change events */
+enum {
+	GPIOLINE_CHANGED_REQUESTED = 1,
+	GPIOLINE_CHANGED_RELEASED,
+	GPIOLINE_CHANGED_CONFIG,
+};
+
+/**
+ * struct gpioline_info_changed - Information about a change in status
+ * of a GPIO line
+ * @info: updated line information
+ * @timestamp: estimate of time of status change occurrence, in nanoseconds
+ * @event_type: one of %GPIOLINE_CHANGED_REQUESTED,
+ * %GPIOLINE_CHANGED_RELEASED and %GPIOLINE_CHANGED_CONFIG
+ * @padding: reserved for future use
+ *
+ * The &struct gpioline_info embedded here has 32-bit alignment on its own,
+ * but it works fine with 64-bit alignment too. With its 72 byte size, we can
+ * guarantee there are no implicit holes between it and subsequent members.
+ * The 20-byte padding at the end makes sure we don't add any implicit padding
+ * at the end of the structure on 64-bit architectures.
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_info_changed instead.
+ */
+struct gpioline_info_changed {
+	struct gpioline_info info;
+	__u64 timestamp;
+	__u32 event_type;
+	__u32 padding[5]; /* for future use */
+};
+
+/* Linerequest flags */
+#define GPIOHANDLE_REQUEST_INPUT	(1UL << 0)
+#define GPIOHANDLE_REQUEST_OUTPUT	(1UL << 1)
+#define GPIOHANDLE_REQUEST_ACTIVE_LOW	(1UL << 2)
+#define GPIOHANDLE_REQUEST_OPEN_DRAIN	(1UL << 3)
+#define GPIOHANDLE_REQUEST_OPEN_SOURCE	(1UL << 4)
+#define GPIOHANDLE_REQUEST_BIAS_PULL_UP	(1UL << 5)
+#define GPIOHANDLE_REQUEST_BIAS_PULL_DOWN	(1UL << 6)
+#define GPIOHANDLE_REQUEST_BIAS_DISABLE	(1UL << 7)
+
+/**
+ * struct gpiohandle_request - Information about a GPIO handle request
+ * @lineoffsets: an array of desired lines, specified by offset index for the
+ * associated GPIO device
+ * @flags: desired flags for the desired GPIO lines, such as
+ * %GPIOHANDLE_REQUEST_OUTPUT, %GPIOHANDLE_REQUEST_ACTIVE_LOW etc, added
+ * together. Note that even if multiple lines are requested, the same flags
+ * must be applicable to all of them, if you want lines with individual
+ * flags set, request them one by one. It is possible to select
+ * a batch of input or output lines, but they must all have the same
+ * characteristics, i.e. all inputs or all outputs, all active low etc
+ * @default_values: if the %GPIOHANDLE_REQUEST_OUTPUT is set for a requested
+ * line, this specifies the default output value, should be 0 (low) or
+ * 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)
+ * @consumer_label: a desired consumer label for the selected GPIO line(s)
+ * such as "my-bitbanged-relay"
+ * @lines: number of lines requested in this request, i.e. the number of
+ * valid fields in the above arrays, set to 1 to request a single line
+ * @fd: if successful this field will contain a valid anonymous file handle
+ * after a %GPIO_GET_LINEHANDLE_IOCTL operation, zero or negative value
+ * means error
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_request instead.
+ */
+struct gpiohandle_request {
+	__u32 lineoffsets[GPIOHANDLES_MAX];
+	__u32 flags;
+	__u8 default_values[GPIOHANDLES_MAX];
+	char consumer_label[GPIO_MAX_NAME_SIZE];
+	__u32 lines;
+	int fd;
+};
+
+/**
+ * struct gpiohandle_config - Configuration for a GPIO handle request
+ * @flags: updated flags for the requested GPIO lines, such as
+ * %GPIOHANDLE_REQUEST_OUTPUT, %GPIOHANDLE_REQUEST_ACTIVE_LOW etc, added
+ * together
+ * @default_values: if the %GPIOHANDLE_REQUEST_OUTPUT is set in flags,
+ * this specifies the default output value, should be 0 (low) or
+ * 1 (high), anything else than 0 or 1 will be interpreted as 1 (high)
+ * @padding: reserved for future use and should be zero filled
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_config instead.
+ */
+struct gpiohandle_config {
+	__u32 flags;
+	__u8 default_values[GPIOHANDLES_MAX];
+	__u32 padding[4]; /* padding for future use */
+};
+
+/**
+ * struct gpiohandle_data - Information of values on a GPIO handle
+ * @values: when getting the state of lines this contains the current
+ * state of a line, when setting the state of lines these should contain
+ * the desired target state
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_values instead.
+ */
+struct gpiohandle_data {
+	__u8 values[GPIOHANDLES_MAX];
+};
+
+/* Eventrequest flags */
+#define GPIOEVENT_REQUEST_RISING_EDGE	(1UL << 0)
+#define GPIOEVENT_REQUEST_FALLING_EDGE	(1UL << 1)
+#define GPIOEVENT_REQUEST_BOTH_EDGES	((1UL << 0) | (1UL << 1))
+
+/**
+ * struct gpioevent_request - Information about a GPIO event request
+ * @lineoffset: the desired line to subscribe to events from, specified by
+ * offset index for the associated GPIO device
+ * @handleflags: desired handle flags for the desired GPIO line, such as
+ * %GPIOHANDLE_REQUEST_ACTIVE_LOW or %GPIOHANDLE_REQUEST_OPEN_DRAIN
+ * @eventflags: desired flags for the desired GPIO event line, such as
+ * %GPIOEVENT_REQUEST_RISING_EDGE or %GPIOEVENT_REQUEST_FALLING_EDGE
+ * @consumer_label: a desired consumer label for the selected GPIO line(s)
+ * such as "my-listener"
+ * @fd: if successful this field will contain a valid anonymous file handle
+ * after a %GPIO_GET_LINEEVENT_IOCTL operation, zero or negative value
+ * means error
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_request instead.
+ */
+struct gpioevent_request {
+	__u32 lineoffset;
+	__u32 handleflags;
+	__u32 eventflags;
+	char consumer_label[GPIO_MAX_NAME_SIZE];
+	int fd;
+};
+
+/*
+ * GPIO event types
+ */
+#define GPIOEVENT_EVENT_RISING_EDGE 0x01
+#define GPIOEVENT_EVENT_FALLING_EDGE 0x02
+
+/**
+ * struct gpioevent_data - The actual event being pushed to userspace
+ * @timestamp: best estimate of time of event occurrence, in nanoseconds
+ * @id: event identifier
+ *
+ * Note: This struct is part of ABI v1 and is deprecated.
+ * Use &struct gpio_v2_line_event instead.
+ */
+struct gpioevent_data {
+	__u64 timestamp;
+	__u32 id;
+};
+
+/*
+ * v1 and v2 ioctl()s
+ */
+#define GPIO_GET_CHIPINFO_IOCTL _IOR(0xB4, 0x01, struct gpiochip_info)
+#define GPIO_GET_LINEINFO_UNWATCH_IOCTL _IOWR(0xB4, 0x0C, __u32)
+
+/*
+ * v2 ioctl()s
+ */
+#define GPIO_V2_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x05, struct gpio_v2_line_info)
+#define GPIO_V2_GET_LINEINFO_WATCH_IOCTL _IOWR(0xB4, 0x06, struct gpio_v2_line_info)
+#define GPIO_V2_GET_LINE_IOCTL _IOWR(0xB4, 0x07, struct gpio_v2_line_request)
+#define GPIO_V2_LINE_SET_CONFIG_IOCTL _IOWR(0xB4, 0x0D, struct gpio_v2_line_config)
+#define GPIO_V2_LINE_GET_VALUES_IOCTL _IOWR(0xB4, 0x0E, struct gpio_v2_line_values)
+#define GPIO_V2_LINE_SET_VALUES_IOCTL _IOWR(0xB4, 0x0F, struct gpio_v2_line_values)
+
+/*
+ * v1 ioctl()s
+ *
+ * These ioctl()s are deprecated.  Use the v2 equivalent instead.
+ */
+#define GPIO_GET_LINEINFO_IOCTL _IOWR(0xB4, 0x02, struct gpioline_info)
+#define GPIO_GET_LINEHANDLE_IOCTL _IOWR(0xB4, 0x03, struct gpiohandle_request)
+#define GPIO_GET_LINEEVENT_IOCTL _IOWR(0xB4, 0x04, struct gpioevent_request)
+#define GPIOHANDLE_GET_LINE_VALUES_IOCTL _IOWR(0xB4, 0x08, struct gpiohandle_data)
+#define GPIOHANDLE_SET_LINE_VALUES_IOCTL _IOWR(0xB4, 0x09, struct gpiohandle_data)
+#define GPIOHANDLE_SET_CONFIG_IOCTL _IOWR(0xB4, 0x0A, struct gpiohandle_config)
+#define GPIO_GET_LINEINFO_WATCH_IOCTL _IOWR(0xB4, 0x0B, struct gpioline_info)
+
+#endif /* _GPIO_H_ */
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 13:34 ` [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository Bartosz Golaszewski
@ 2021-01-11 13:46   ` Andy Shevchenko
  2021-01-11 14:06     ` Bartosz Golaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Andy Shevchenko @ 2021-01-11 13:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Kent Gibson, Andy Shevchenko, Linus Walleij,
	open list:GPIO SUBSYSTEM, Bartosz Golaszewski

On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> In order to avoid any problems with symbols missing from the host linux
> kernel headers (for example: if current version of libgpiod supports
> features that were added recently to the kernel but the host headers are
> outdated and don't export required symbols) let's add the uapi header to
> the repository and include it instead of the one in /usr/include/linux.

I doubt this is a good decision. First of all if the host (or rather
target, because host should not influence build of libgpiod) has
outdated header it may be for a reason (it runs old kernel).
When you run new library on outdated kernel it might produce various
of interesting errors (in general, I haven't investigated libgpiod
case).
On top of that you make a copy'n'paste source code which is against
the Unix way.

Sorry, but I'm in favour of dropping this one.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 13:46   ` Andy Shevchenko
@ 2021-01-11 14:06     ` Bartosz Golaszewski
  2021-01-11 14:46       ` Andy Shevchenko
  0 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 14:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Kent Gibson, Andy Shevchenko, Linus Walleij,
	open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thomas Petazzoni

On Mon, Jan 11, 2021 at 2:45 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > In order to avoid any problems with symbols missing from the host linux
> > kernel headers (for example: if current version of libgpiod supports
> > features that were added recently to the kernel but the host headers are
> > outdated and don't export required symbols) let's add the uapi header to
> > the repository and include it instead of the one in /usr/include/linux.
>
> I doubt this is a good decision. First of all if the host (or rather
> target, because host should not influence build of libgpiod) has

I meant the host as in: the machine on which you build and which
contains the headers for the target as well but I see what you mean.

> outdated header it may be for a reason (it runs old kernel).
> When you run new library on outdated kernel it might produce various
> of interesting errors (in general, I haven't investigated libgpiod
> case).
> On top of that you make a copy'n'paste source code which is against
> the Unix way.
>
> Sorry, but I'm in favour of dropping this one.
>

Cc: Thomas

This problem has been raised by the buildroot people when we started
requiring different versions of kernel headers to build v1.4 and v1.6.
It turns out most projects simply package the uapi headers together
with their sources (e.g. wpa_supplicant, libnl, iproute2) to avoid
complicated dependencies. It's true that now the library can fail at
runtime but I'm fine with that. Also: if we add new features between
two kernel versions, we still allow to build the new library version
except that these new features won't work on older kernels.

Bartosz

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE
  2021-01-11 13:34 ` [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE Bartosz Golaszewski
@ 2021-01-11 14:31   ` Kent Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: Kent Gibson @ 2021-01-11 14:31 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Linus Walleij, linux-gpio, Bartosz Golaszewski

On Mon, Jan 11, 2021 at 02:34:23PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> When inspecting the current bias setting of a GPIO line, the AS_IS name
> of one of the possible values really means that there's no bias so it
> should be renamed to NONE for clarity.
> 

I disagree this one, as the BIAS_NONE can be confused with BIAS_DISABLED.

Its meaning depends on whether you are setting or reading the value.
On write, for backwards compatibility, it means don't change the bias
from what it was previously, hence the AS-IS naming.
On read it means unknown, as the uAPI can't determine bias setting
unless it set it itself - so if it is set by DT then the cdev can't tell.

If you are going to change it I'd go with BIAS_UNKNOWN.

Cheers,
Kent.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 4/6] treewide: make drive settings an enum
  2021-01-11 13:34 ` [libgpiod][PATCH 4/6] treewide: make drive settings an enum Bartosz Golaszewski
@ 2021-01-11 14:39   ` Kent Gibson
  2021-01-11 14:48     ` Bartosz Golaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Kent Gibson @ 2021-01-11 14:39 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Linus Walleij, linux-gpio, Bartosz Golaszewski

On Mon, Jan 11, 2021 at 02:34:24PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Open-source and open-drain drive settings are mutually exclusive just like
> the bias settings. Make them into an enum so that becomes clear.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  bindings/cxx/gpiod.hpp                 | 24 +++++----
>  bindings/cxx/line.cpp                  | 18 +++----
>  bindings/cxx/tests/tests-line.cpp      | 45 ++++++----------
>  bindings/python/gpiodmodule.c          | 73 ++++++++++++++------------
>  bindings/python/tests/gpiod_py_test.py | 18 +++----
>  include/gpiod.h                        | 26 +++++----
>  lib/core.c                             | 12 ++---
>  tests/tests-line.c                     | 36 ++++++-------
>  tools/gpioinfo.c                       | 14 ++++-
>  9 files changed, 134 insertions(+), 132 deletions(-)
> 
> diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
> index fb675fc..f9c341d 100644
> --- a/bindings/cxx/gpiod.hpp
> +++ b/bindings/cxx/gpiod.hpp
> @@ -332,16 +332,10 @@ public:
>  	GPIOD_API bool is_used(void) const;
>  
>  	/**
> -	 * @brief Check if this line represents an open-drain GPIO.
> -	 * @return True if the line is an open-drain GPIO, false otherwise.
> +	 * @brief Get current drive setting of this line.
> +	 * @return Current drive setting.
>  	 */
> -	GPIOD_API bool is_open_drain(void) const;
> -
> -	/**
> -	 * @brief Check if this line represents an open-source GPIO.
> -	 * @return True if the line is an open-source GPIO, false otherwise.
> -	 */
> -	GPIOD_API bool is_open_source(void) const;
> +	GPIOD_API int drive(void) const;
>  
>  	/**
>  	 * @brief Request this line.
> @@ -482,6 +476,18 @@ public:
>  		/**< Line's direction setting is output. */
>  	};
>  
> +	/**
> +	 * @brief Possible drive settings.
> +	 */
> +	enum : int {
> +		DRIVE_NONE = 1,
> +		/**< Drive setting is unknown. */

Unlike bias, the drive setting is never unknown - if zero the pin is
assumed push-pull.

And to me DRIVE_NONE implies a high impedance state.

Cheers,
Kent.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 14:06     ` Bartosz Golaszewski
@ 2021-01-11 14:46       ` Andy Shevchenko
  2021-01-11 15:15         ` Bartosz Golaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Andy Shevchenko @ 2021-01-11 14:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Kent Gibson, Linus Walleij, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Thomas Petazzoni

On Mon, Jan 11, 2021 at 03:06:28PM +0100, Bartosz Golaszewski wrote:
> On Mon, Jan 11, 2021 at 2:45 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > In order to avoid any problems with symbols missing from the host linux
> > > kernel headers (for example: if current version of libgpiod supports
> > > features that were added recently to the kernel but the host headers are
> > > outdated and don't export required symbols) let's add the uapi header to
> > > the repository and include it instead of the one in /usr/include/linux.
> >
> > I doubt this is a good decision. First of all if the host (or rather
> > target, because host should not influence build of libgpiod) has
> 
> I meant the host as in: the machine on which you build and which
> contains the headers for the target as well but I see what you mean.
> 
> > outdated header it may be for a reason (it runs old kernel).
> > When you run new library on outdated kernel it might produce various
> > of interesting errors (in general, I haven't investigated libgpiod
> > case).
> > On top of that you make a copy'n'paste source code which is against
> > the Unix way.
> >
> > Sorry, but I'm in favour of dropping this one.
> >
> 
> Cc: Thomas
> 
> This problem has been raised by the buildroot people when we started
> requiring different versions of kernel headers to build v1.4 and v1.6.
> It turns out most projects simply package the uapi headers together
> with their sources (e.g. wpa_supplicant, libnl, iproute2) to avoid
> complicated dependencies. It's true that now the library can fail at
> runtime but I'm fine with that. Also: if we add new features between
> two kernel versions, we still allow to build the new library version
> except that these new features won't work on older kernels.

I see.

So known ways to solve this are
 - provide a header with source tree (see above)
 - modify code with ifdeffery against specific kernel versions
 - ...something else... ?

Second item is what ALSA used (not sure if they provide a standalone driver
anymore). Ugly, but won't require header which may be staled.

Any other solutions in mind?


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 4/6] treewide: make drive settings an enum
  2021-01-11 14:39   ` Kent Gibson
@ 2021-01-11 14:48     ` Bartosz Golaszewski
  0 siblings, 0 replies; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 14:48 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski

On Mon, Jan 11, 2021 at 3:39 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 02:34:24PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Open-source and open-drain drive settings are mutually exclusive just like
> > the bias settings. Make them into an enum so that becomes clear.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  bindings/cxx/gpiod.hpp                 | 24 +++++----
> >  bindings/cxx/line.cpp                  | 18 +++----
> >  bindings/cxx/tests/tests-line.cpp      | 45 ++++++----------
> >  bindings/python/gpiodmodule.c          | 73 ++++++++++++++------------
> >  bindings/python/tests/gpiod_py_test.py | 18 +++----
> >  include/gpiod.h                        | 26 +++++----
> >  lib/core.c                             | 12 ++---
> >  tests/tests-line.c                     | 36 ++++++-------
> >  tools/gpioinfo.c                       | 14 ++++-
> >  9 files changed, 134 insertions(+), 132 deletions(-)
> >
> > diff --git a/bindings/cxx/gpiod.hpp b/bindings/cxx/gpiod.hpp
> > index fb675fc..f9c341d 100644
> > --- a/bindings/cxx/gpiod.hpp
> > +++ b/bindings/cxx/gpiod.hpp
> > @@ -332,16 +332,10 @@ public:
> >       GPIOD_API bool is_used(void) const;
> >
> >       /**
> > -      * @brief Check if this line represents an open-drain GPIO.
> > -      * @return True if the line is an open-drain GPIO, false otherwise.
> > +      * @brief Get current drive setting of this line.
> > +      * @return Current drive setting.
> >        */
> > -     GPIOD_API bool is_open_drain(void) const;
> > -
> > -     /**
> > -      * @brief Check if this line represents an open-source GPIO.
> > -      * @return True if the line is an open-source GPIO, false otherwise.
> > -      */
> > -     GPIOD_API bool is_open_source(void) const;
> > +     GPIOD_API int drive(void) const;
> >
> >       /**
> >        * @brief Request this line.
> > @@ -482,6 +476,18 @@ public:
> >               /**< Line's direction setting is output. */
> >       };
> >
> > +     /**
> > +      * @brief Possible drive settings.
> > +      */
> > +     enum : int {
> > +             DRIVE_NONE = 1,
> > +             /**< Drive setting is unknown. */
>
> Unlike bias, the drive setting is never unknown - if zero the pin is
> assumed push-pull.
>
> And to me DRIVE_NONE implies a high impedance state.
>

Indeed, I'll change it to DRIVE_PUSH_PULL then.

Bart

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 14:46       ` Andy Shevchenko
@ 2021-01-11 15:15         ` Bartosz Golaszewski
  2021-01-25  5:55           ` Kent Gibson
  0 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-11 15:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Bartosz Golaszewski, Kent Gibson, Linus Walleij,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Mon, Jan 11, 2021 at 3:45 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 03:06:28PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Jan 11, 2021 at 2:45 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > >
> > > > In order to avoid any problems with symbols missing from the host linux
> > > > kernel headers (for example: if current version of libgpiod supports
> > > > features that were added recently to the kernel but the host headers are
> > > > outdated and don't export required symbols) let's add the uapi header to
> > > > the repository and include it instead of the one in /usr/include/linux.
> > >
> > > I doubt this is a good decision. First of all if the host (or rather
> > > target, because host should not influence build of libgpiod) has
> >
> > I meant the host as in: the machine on which you build and which
> > contains the headers for the target as well but I see what you mean.
> >
> > > outdated header it may be for a reason (it runs old kernel).
> > > When you run new library on outdated kernel it might produce various
> > > of interesting errors (in general, I haven't investigated libgpiod
> > > case).
> > > On top of that you make a copy'n'paste source code which is against
> > > the Unix way.
> > >
> > > Sorry, but I'm in favour of dropping this one.
> > >
> >
> > Cc: Thomas
> >
> > This problem has been raised by the buildroot people when we started
> > requiring different versions of kernel headers to build v1.4 and v1.6.
> > It turns out most projects simply package the uapi headers together
> > with their sources (e.g. wpa_supplicant, libnl, iproute2) to avoid
> > complicated dependencies. It's true that now the library can fail at
> > runtime but I'm fine with that. Also: if we add new features between
> > two kernel versions, we still allow to build the new library version
> > except that these new features won't work on older kernels.
>
> I see.
>
> So known ways to solve this are
>  - provide a header with source tree (see above)
>  - modify code with ifdeffery against specific kernel versions
>  - ...something else... ?
>
> Second item is what ALSA used (not sure if they provide a standalone driver
> anymore). Ugly, but won't require header which may be staled.
>
> Any other solutions in mind?
>

I tried to go the third way and just ignore the problem but I've
received too many emails about that. :)

I don't like the ifdef hell so I prefer to bundle the header. I'm open
to other suggestions, although I can't come up with anything else.

Bart

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-11 15:15         ` Bartosz Golaszewski
@ 2021-01-25  5:55           ` Kent Gibson
  2021-01-25 10:54             ` Andy Shevchenko
  2021-01-26 15:07             ` Bartosz Golaszewski
  0 siblings, 2 replies; 22+ messages in thread
From: Kent Gibson @ 2021-01-25  5:55 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Andy Shevchenko, Bartosz Golaszewski, Linus Walleij,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Mon, Jan 11, 2021 at 04:15:21PM +0100, Bartosz Golaszewski wrote:
> On Mon, Jan 11, 2021 at 3:45 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Jan 11, 2021 at 03:06:28PM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Jan 11, 2021 at 2:45 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > >
> > > > On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > >
> > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > >
> > > > > In order to avoid any problems with symbols missing from the host linux
> > > > > kernel headers (for example: if current version of libgpiod supports
> > > > > features that were added recently to the kernel but the host headers are
> > > > > outdated and don't export required symbols) let's add the uapi header to
> > > > > the repository and include it instead of the one in /usr/include/linux.
> > > >
> > > > I doubt this is a good decision. First of all if the host (or rather
> > > > target, because host should not influence build of libgpiod) has
> > >
> > > I meant the host as in: the machine on which you build and which
> > > contains the headers for the target as well but I see what you mean.
> > >
> > > > outdated header it may be for a reason (it runs old kernel).
> > > > When you run new library on outdated kernel it might produce various
> > > > of interesting errors (in general, I haven't investigated libgpiod
> > > > case).
> > > > On top of that you make a copy'n'paste source code which is against
> > > > the Unix way.
> > > >
> > > > Sorry, but I'm in favour of dropping this one.
> > > >
> > >
> > > Cc: Thomas
> > >
> > > This problem has been raised by the buildroot people when we started
> > > requiring different versions of kernel headers to build v1.4 and v1.6.
> > > It turns out most projects simply package the uapi headers together
> > > with their sources (e.g. wpa_supplicant, libnl, iproute2) to avoid
> > > complicated dependencies. It's true that now the library can fail at
> > > runtime but I'm fine with that. Also: if we add new features between
> > > two kernel versions, we still allow to build the new library version
> > > except that these new features won't work on older kernels.
> >
> > I see.
> >
> > So known ways to solve this are
> >  - provide a header with source tree (see above)
> >  - modify code with ifdeffery against specific kernel versions
> >  - ...something else... ?
> >
> > Second item is what ALSA used (not sure if they provide a standalone driver
> > anymore). Ugly, but won't require header which may be staled.
> >
> > Any other solutions in mind?
> >
> 
> I tried to go the third way and just ignore the problem but I've
> received too many emails about that. :)
> 
> I don't like the ifdef hell so I prefer to bundle the header. I'm open
> to other suggestions, although I can't come up with anything else.
> 

Going off on a bit of a tangent, but I'm trying to add support for
decoding the GPIO ioctls into strace and am running up against a similar
issue.

The way strace does it is to check the uAPI header on the host and use
it if possible.  To handle where it may be stale, local types are
defined that mirror any types that may have been added since the header
was originally released.  If the corresponding type is available in the
linux header then it is used, else the local type.

This obviously creates a lot of pointless boilerplate code and
preprocessor chicanery so I floated the idea of just including the latest
header in the strace tree, as you are doing here for libgpiod.
But that raised the issue of licencing, specifically if you copy the
linux/gpio.h into a source tree does that mean that the whole project
becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
libgpiod.

The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
which is also not totally clear on this point[1].

My gut feeling was that using and even copying API headers doesn't
constitute a derived work, as per the FSF view quoted in [1], and
ethically might even be less of a violation than copying and re-defining
individual types, but I'd rather not rely on a gut feeling.

Is there some clear opinion or precedent on this point?
i.e. are libgpiod and strace in legal licence jeopardy if they include
gpio.h in their source tree?

Cheers,
Kent.

[1] https://lkml.org/lkml/2020/2/21/2193

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-25  5:55           ` Kent Gibson
@ 2021-01-25 10:54             ` Andy Shevchenko
  2021-01-26 15:07             ` Bartosz Golaszewski
  1 sibling, 0 replies; 22+ messages in thread
From: Andy Shevchenko @ 2021-01-25 10:54 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Bartosz Golaszewski, Bartosz Golaszewski, Linus Walleij,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Mon, Jan 25, 2021 at 7:55 AM Kent Gibson <warthog618@gmail.com> wrote:
> On Mon, Jan 11, 2021 at 04:15:21PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Jan 11, 2021 at 3:45 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Mon, Jan 11, 2021 at 03:06:28PM +0100, Bartosz Golaszewski wrote:
> > > > On Mon, Jan 11, 2021 at 2:45 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > > On Mon, Jan 11, 2021 at 3:37 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > >
> > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > > >
> > > > > > In order to avoid any problems with symbols missing from the host linux
> > > > > > kernel headers (for example: if current version of libgpiod supports
> > > > > > features that were added recently to the kernel but the host headers are
> > > > > > outdated and don't export required symbols) let's add the uapi header to
> > > > > > the repository and include it instead of the one in /usr/include/linux.
> > > > >
> > > > > I doubt this is a good decision. First of all if the host (or rather
> > > > > target, because host should not influence build of libgpiod) has
> > > >
> > > > I meant the host as in: the machine on which you build and which
> > > > contains the headers for the target as well but I see what you mean.
> > > >
> > > > > outdated header it may be for a reason (it runs old kernel).
> > > > > When you run new library on outdated kernel it might produce various
> > > > > of interesting errors (in general, I haven't investigated libgpiod
> > > > > case).
> > > > > On top of that you make a copy'n'paste source code which is against
> > > > > the Unix way.
> > > > >
> > > > > Sorry, but I'm in favour of dropping this one.
> > > > >
> > > >
> > > > Cc: Thomas
> > > >
> > > > This problem has been raised by the buildroot people when we started
> > > > requiring different versions of kernel headers to build v1.4 and v1.6.
> > > > It turns out most projects simply package the uapi headers together
> > > > with their sources (e.g. wpa_supplicant, libnl, iproute2) to avoid
> > > > complicated dependencies. It's true that now the library can fail at
> > > > runtime but I'm fine with that. Also: if we add new features between
> > > > two kernel versions, we still allow to build the new library version
> > > > except that these new features won't work on older kernels.
> > >
> > > I see.
> > >
> > > So known ways to solve this are
> > >  - provide a header with source tree (see above)
> > >  - modify code with ifdeffery against specific kernel versions
> > >  - ...something else... ?
> > >
> > > Second item is what ALSA used (not sure if they provide a standalone driver
> > > anymore). Ugly, but won't require header which may be staled.
> > >
> > > Any other solutions in mind?
> > >
> >
> > I tried to go the third way and just ignore the problem but I've
> > received too many emails about that. :)
> >
> > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > to other suggestions, although I can't come up with anything else.
> >
>
> Going off on a bit of a tangent, but I'm trying to add support for
> decoding the GPIO ioctls into strace and am running up against a similar
> issue.
>
> The way strace does it is to check the uAPI header on the host and use
> it if possible.  To handle where it may be stale, local types are
> defined that mirror any types that may have been added since the header
> was originally released.  If the corresponding type is available in the
> linux header then it is used, else the local type.
>
> This obviously creates a lot of pointless boilerplate code and
> preprocessor chicanery so I floated the idea of just including the latest
> header in the strace tree, as you are doing here for libgpiod.
> But that raised the issue of licencing, specifically if you copy the
> linux/gpio.h into a source tree does that mean that the whole project
> becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> libgpiod.

Very good point!

> The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> which is also not totally clear on this point[1].
>
> My gut feeling was that using and even copying API headers doesn't
> constitute a derived work, as per the FSF view quoted in [1], and
> ethically might even be less of a violation than copying and re-defining
> individual types, but I'd rather not rely on a gut feeling.

This reminds me of the Google vs. Oracle case where they pointed out
the header files (IIRC!).

> Is there some clear opinion or precedent on this point?
> i.e. are libgpiod and strace in legal licence jeopardy if they include
> gpio.h in their source tree?

> [1] https://lkml.org/lkml/2020/2/21/2193



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-25  5:55           ` Kent Gibson
  2021-01-25 10:54             ` Andy Shevchenko
@ 2021-01-26 15:07             ` Bartosz Golaszewski
  2021-01-26 17:11               ` Greg Kroah-Hartman
  1 sibling, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-26 15:07 UTC (permalink / raw)
  To: Kent Gibson, Linus Walleij, Greg Kroah-Hartman
  Cc: Bartosz Golaszewski, Andy Shevchenko, open list:GPIO SUBSYSTEM,
	Thomas Petazzoni

On Mon, Jan 25, 2021 at 6:55 AM Kent Gibson <warthog618@gmail.com> wrote:
>

[snip!]

> >
> > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > to other suggestions, although I can't come up with anything else.
> >
>
> Going off on a bit of a tangent, but I'm trying to add support for
> decoding the GPIO ioctls into strace and am running up against a similar
> issue.
>
> The way strace does it is to check the uAPI header on the host and use
> it if possible.  To handle where it may be stale, local types are
> defined that mirror any types that may have been added since the header
> was originally released.  If the corresponding type is available in the
> linux header then it is used, else the local type.
>
> This obviously creates a lot of pointless boilerplate code and
> preprocessor chicanery so I floated the idea of just including the latest
> header in the strace tree, as you are doing here for libgpiod.
> But that raised the issue of licencing, specifically if you copy the
> linux/gpio.h into a source tree does that mean that the whole project
> becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> libgpiod.
>
> The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> which is also not totally clear on this point[1].
>
> My gut feeling was that using and even copying API headers doesn't
> constitute a derived work, as per the FSF view quoted in [1], and
> ethically might even be less of a violation than copying and re-defining
> individual types, but I'd rather not rely on a gut feeling.
>
> Is there some clear opinion or precedent on this point?
> i.e. are libgpiod and strace in legal licence jeopardy if they include
> gpio.h in their source tree?
>
> Cheers,
> Kent.
>
> [1] https://lkml.org/lkml/2020/2/21/2193

Thanks for pointing that out. I lack the legal knowledge to have an
opinion of my own on this.

Cc'ing Greg KH for help.

Greg: do you know if it's fine to bundle a 'GPL-2.0 WITH
Linux-syscall-note' uAPI header together with an LGPL-v2.1-or-later
user-space shared library?

Best Regards,
Bartosz Golaszewski

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-26 15:07             ` Bartosz Golaszewski
@ 2021-01-26 17:11               ` Greg Kroah-Hartman
  2021-01-26 19:08                 ` Bartosz Golaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-26 17:11 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Kent Gibson, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Tue, Jan 26, 2021 at 04:07:47PM +0100, Bartosz Golaszewski wrote:
> On Mon, Jan 25, 2021 at 6:55 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> 
> [snip!]
> 
> > >
> > > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > > to other suggestions, although I can't come up with anything else.
> > >
> >
> > Going off on a bit of a tangent, but I'm trying to add support for
> > decoding the GPIO ioctls into strace and am running up against a similar
> > issue.
> >
> > The way strace does it is to check the uAPI header on the host and use
> > it if possible.  To handle where it may be stale, local types are
> > defined that mirror any types that may have been added since the header
> > was originally released.  If the corresponding type is available in the
> > linux header then it is used, else the local type.
> >
> > This obviously creates a lot of pointless boilerplate code and
> > preprocessor chicanery so I floated the idea of just including the latest
> > header in the strace tree, as you are doing here for libgpiod.
> > But that raised the issue of licencing, specifically if you copy the
> > linux/gpio.h into a source tree does that mean that the whole project
> > becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> > libgpiod.
> >
> > The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> > which is also not totally clear on this point[1].
> >
> > My gut feeling was that using and even copying API headers doesn't
> > constitute a derived work, as per the FSF view quoted in [1], and
> > ethically might even be less of a violation than copying and re-defining
> > individual types, but I'd rather not rely on a gut feeling.
> >
> > Is there some clear opinion or precedent on this point?
> > i.e. are libgpiod and strace in legal licence jeopardy if they include
> > gpio.h in their source tree?
> >
> > Cheers,
> > Kent.
> >
> > [1] https://lkml.org/lkml/2020/2/21/2193
> 
> Thanks for pointing that out. I lack the legal knowledge to have an
> opinion of my own on this.
> 
> Cc'ing Greg KH for help.
> 
> Greg: do you know if it's fine to bundle a 'GPL-2.0 WITH
> Linux-syscall-note' uAPI header together with an LGPL-v2.1-or-later
> user-space shared library?

How would you "bundle" such a thing as that is not what is in the kernel
source tree?  If you are going to copy files out of the kernel and do
other things with them, well, I recommend asking a lawyer as I am not
one :)

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-26 17:11               ` Greg Kroah-Hartman
@ 2021-01-26 19:08                 ` Bartosz Golaszewski
  2021-01-27 11:43                   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 22+ messages in thread
From: Bartosz Golaszewski @ 2021-01-26 19:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kent Gibson, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Tue, Jan 26, 2021 at 6:11 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jan 26, 2021 at 04:07:47PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Jan 25, 2021 at 6:55 AM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> >
> > [snip!]
> >
> > > >
> > > > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > > > to other suggestions, although I can't come up with anything else.
> > > >
> > >
> > > Going off on a bit of a tangent, but I'm trying to add support for
> > > decoding the GPIO ioctls into strace and am running up against a similar
> > > issue.
> > >
> > > The way strace does it is to check the uAPI header on the host and use
> > > it if possible.  To handle where it may be stale, local types are
> > > defined that mirror any types that may have been added since the header
> > > was originally released.  If the corresponding type is available in the
> > > linux header then it is used, else the local type.
> > >
> > > This obviously creates a lot of pointless boilerplate code and
> > > preprocessor chicanery so I floated the idea of just including the latest
> > > header in the strace tree, as you are doing here for libgpiod.
> > > But that raised the issue of licencing, specifically if you copy the
> > > linux/gpio.h into a source tree does that mean that the whole project
> > > becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> > > libgpiod.
> > >
> > > The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> > > which is also not totally clear on this point[1].
> > >
> > > My gut feeling was that using and even copying API headers doesn't
> > > constitute a derived work, as per the FSF view quoted in [1], and
> > > ethically might even be less of a violation than copying and re-defining
> > > individual types, but I'd rather not rely on a gut feeling.
> > >
> > > Is there some clear opinion or precedent on this point?
> > > i.e. are libgpiod and strace in legal licence jeopardy if they include
> > > gpio.h in their source tree?
> > >
> > > Cheers,
> > > Kent.
> > >
> > > [1] https://lkml.org/lkml/2020/2/21/2193
> >
> > Thanks for pointing that out. I lack the legal knowledge to have an
> > opinion of my own on this.
> >
> > Cc'ing Greg KH for help.
> >
> > Greg: do you know if it's fine to bundle a 'GPL-2.0 WITH
> > Linux-syscall-note' uAPI header together with an LGPL-v2.1-or-later
> > user-space shared library?
>
> How would you "bundle" such a thing as that is not what is in the kernel
> source tree?  If you are going to copy files out of the kernel and do
> other things with them, well, I recommend asking a lawyer as I am not
> one :)
>
> good luck!
>
> greg k-h

By "bundling" I mean - copying the kernel uAPI header verbatim from
the kernel tree into the project repository. The reason for that is
the fact that always relying on the toolchain kernel headers leads to
build issues if we want to support more recent kernel features in the
library while the supplied headers don't define all required symbols.

We can either make the latest supported version of linux headers a
hard requirement for building (I did that and buildroot folks yelled
at me because two stable versions of the library had different kernel
headers requirements) or redefine certain symbols (new symbols since
the oldest supported kernel version) or - and this is preferred unless
it's against the linux license - include the kernel headers in the
source tarball of the library.

I hope this is not a stupid question but obviously I don't know any
lawyer well versed in software copyright: can we direct this question
to anyone at the Linux Foundation maybe?

Bartosz

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-26 19:08                 ` Bartosz Golaszewski
@ 2021-01-27 11:43                   ` Greg Kroah-Hartman
  2021-01-28  3:26                     ` Kent Gibson
  0 siblings, 1 reply; 22+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-27 11:43 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Kent Gibson, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Tue, Jan 26, 2021 at 08:08:01PM +0100, Bartosz Golaszewski wrote:
> On Tue, Jan 26, 2021 at 6:11 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Jan 26, 2021 at 04:07:47PM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Jan 25, 2021 at 6:55 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > >
> > > [snip!]
> > >
> > > > >
> > > > > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > > > > to other suggestions, although I can't come up with anything else.
> > > > >
> > > >
> > > > Going off on a bit of a tangent, but I'm trying to add support for
> > > > decoding the GPIO ioctls into strace and am running up against a similar
> > > > issue.
> > > >
> > > > The way strace does it is to check the uAPI header on the host and use
> > > > it if possible.  To handle where it may be stale, local types are
> > > > defined that mirror any types that may have been added since the header
> > > > was originally released.  If the corresponding type is available in the
> > > > linux header then it is used, else the local type.
> > > >
> > > > This obviously creates a lot of pointless boilerplate code and
> > > > preprocessor chicanery so I floated the idea of just including the latest
> > > > header in the strace tree, as you are doing here for libgpiod.
> > > > But that raised the issue of licencing, specifically if you copy the
> > > > linux/gpio.h into a source tree does that mean that the whole project
> > > > becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> > > > libgpiod.
> > > >
> > > > The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> > > > which is also not totally clear on this point[1].
> > > >
> > > > My gut feeling was that using and even copying API headers doesn't
> > > > constitute a derived work, as per the FSF view quoted in [1], and
> > > > ethically might even be less of a violation than copying and re-defining
> > > > individual types, but I'd rather not rely on a gut feeling.
> > > >
> > > > Is there some clear opinion or precedent on this point?
> > > > i.e. are libgpiod and strace in legal licence jeopardy if they include
> > > > gpio.h in their source tree?
> > > >
> > > > Cheers,
> > > > Kent.
> > > >
> > > > [1] https://lkml.org/lkml/2020/2/21/2193
> > >
> > > Thanks for pointing that out. I lack the legal knowledge to have an
> > > opinion of my own on this.
> > >
> > > Cc'ing Greg KH for help.
> > >
> > > Greg: do you know if it's fine to bundle a 'GPL-2.0 WITH
> > > Linux-syscall-note' uAPI header together with an LGPL-v2.1-or-later
> > > user-space shared library?
> >
> > How would you "bundle" such a thing as that is not what is in the kernel
> > source tree?  If you are going to copy files out of the kernel and do
> > other things with them, well, I recommend asking a lawyer as I am not
> > one :)
> >
> > good luck!
> >
> > greg k-h
> 
> By "bundling" I mean - copying the kernel uAPI header verbatim from
> the kernel tree into the project repository. The reason for that is
> the fact that always relying on the toolchain kernel headers leads to
> build issues if we want to support more recent kernel features in the
> library while the supplied headers don't define all required symbols.
> 
> We can either make the latest supported version of linux headers a
> hard requirement for building (I did that and buildroot folks yelled
> at me because two stable versions of the library had different kernel
> headers requirements) or redefine certain symbols (new symbols since
> the oldest supported kernel version) or - and this is preferred unless
> it's against the linux license - include the kernel headers in the
> source tarball of the library.
> 
> I hope this is not a stupid question but obviously I don't know any
> lawyer well versed in software copyright: can we direct this question
> to anyone at the Linux Foundation maybe?

Ok, first off, I am not a lawyer so don't take legal advice from me.

But, if you copy the .h file directly, and keep the same license on the
file, that should be fine as you would be using it under the "GPLv2 with
syscall note" license for your userspace program, right?

So there shouldn't be an issue there that I can determine, as we want
userspace programs to be free to use those headers to interact with the
kernel.

It's come up in the past that we might want to somehow make this much
more obvious, and we have talked about this with the legal community,
but that's only in the context of making it more obvious that we want
people to write programs of any license to talk to the kernel, not that
we would want to keep anyone from doing that :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-27 11:43                   ` Greg Kroah-Hartman
@ 2021-01-28  3:26                     ` Kent Gibson
  2021-01-28  7:51                       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 22+ messages in thread
From: Kent Gibson @ 2021-01-28  3:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Bartosz Golaszewski, Dmitry V. Levin
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, Thomas Petazzoni

On Wed, Jan 27, 2021 at 12:43:25PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Jan 26, 2021 at 08:08:01PM +0100, Bartosz Golaszewski wrote:
> > On Tue, Jan 26, 2021 at 6:11 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, Jan 26, 2021 at 04:07:47PM +0100, Bartosz Golaszewski wrote:
> > > > On Mon, Jan 25, 2021 at 6:55 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > >
> > > > [snip!]
> > > >
> > > > > >
> > > > > > I don't like the ifdef hell so I prefer to bundle the header. I'm open
> > > > > > to other suggestions, although I can't come up with anything else.
> > > > > >
> > > > >
> > > > > Going off on a bit of a tangent, but I'm trying to add support for
> > > > > decoding the GPIO ioctls into strace and am running up against a similar
> > > > > issue.
> > > > >
> > > > > The way strace does it is to check the uAPI header on the host and use
> > > > > it if possible.  To handle where it may be stale, local types are
> > > > > defined that mirror any types that may have been added since the header
> > > > > was originally released.  If the corresponding type is available in the
> > > > > linux header then it is used, else the local type.
> > > > >
> > > > > This obviously creates a lot of pointless boilerplate code and
> > > > > preprocessor chicanery so I floated the idea of just including the latest
> > > > > header in the strace tree, as you are doing here for libgpiod.
> > > > > But that raised the issue of licencing, specifically if you copy the
> > > > > linux/gpio.h into a source tree does that mean that the whole project
> > > > > becomes GPL 2.0?  That is an issue for strace as it is LGPL 2.1 - as is
> > > > > libgpiod.
> > > > >
> > > > > The Linux uAPI headers are under the GPL-2.0 WITH Linux-syscall-note,
> > > > > which is also not totally clear on this point[1].
> > > > >
> > > > > My gut feeling was that using and even copying API headers doesn't
> > > > > constitute a derived work, as per the FSF view quoted in [1], and
> > > > > ethically might even be less of a violation than copying and re-defining
> > > > > individual types, but I'd rather not rely on a gut feeling.
> > > > >
> > > > > Is there some clear opinion or precedent on this point?
> > > > > i.e. are libgpiod and strace in legal licence jeopardy if they include
> > > > > gpio.h in their source tree?
> > > > >
> > > > > Cheers,
> > > > > Kent.
> > > > >
> > > > > [1] https://lkml.org/lkml/2020/2/21/2193
> > > >
> > > > Thanks for pointing that out. I lack the legal knowledge to have an
> > > > opinion of my own on this.
> > > >
> > > > Cc'ing Greg KH for help.
> > > >
> > > > Greg: do you know if it's fine to bundle a 'GPL-2.0 WITH
> > > > Linux-syscall-note' uAPI header together with an LGPL-v2.1-or-later
> > > > user-space shared library?
> > >
> > > How would you "bundle" such a thing as that is not what is in the kernel
> > > source tree?  If you are going to copy files out of the kernel and do
> > > other things with them, well, I recommend asking a lawyer as I am not
> > > one :)
> > >
> > > good luck!
> > >
> > > greg k-h
> > 
> > By "bundling" I mean - copying the kernel uAPI header verbatim from
> > the kernel tree into the project repository. The reason for that is
> > the fact that always relying on the toolchain kernel headers leads to
> > build issues if we want to support more recent kernel features in the
> > library while the supplied headers don't define all required symbols.
> > 
> > We can either make the latest supported version of linux headers a
> > hard requirement for building (I did that and buildroot folks yelled
> > at me because two stable versions of the library had different kernel
> > headers requirements) or redefine certain symbols (new symbols since
> > the oldest supported kernel version) or - and this is preferred unless
> > it's against the linux license - include the kernel headers in the
> > source tarball of the library.
> > 
> > I hope this is not a stupid question but obviously I don't know any
> > lawyer well versed in software copyright: can we direct this question
> > to anyone at the Linux Foundation maybe?
> 
> Ok, first off, I am not a lawyer so don't take legal advice from me.
> 
> But, if you copy the .h file directly, and keep the same license on the
> file, that should be fine as you would be using it under the "GPLv2 with
> syscall note" license for your userspace program, right?
> 
> So there shouldn't be an issue there that I can determine, as we want
> userspace programs to be free to use those headers to interact with the
> kernel.
> 
> It's come up in the past that we might want to somehow make this much
> more obvious, and we have talked about this with the legal community,
> but that's only in the context of making it more obvious that we want
> people to write programs of any license to talk to the kernel, not that
> we would want to keep anyone from doing that :)
> 
> thanks,
> 
> greg k-h

Including Dmitry as he orignally raised the issue and has an interest in
its resolution.

Greg: thanks for providing your view.  You may not be a lawyer (which is
a very good thing, btw), but your opinion carries a lot of weight, and
combined with what we already knew I think we're on very solid ground
distributing the kernel headers in the libgpiod and strace repositories.

Bart and Dmitry: I submit that we are good to copy the headers into the
repositories, but we should take a few steps just to make clear that we
are in full compliance with the GPL v2.

Firstly, we are distributing the headers under Section 1 (distribution)
of the GPL, so we should keep the headers in a separate directory that
contains its own COPYING file as well as the GPL v2 and Linux syscall note
- unless they are already available elsewhere in the repo.

The headers must be copied verbatim so as to not trigger Section 2
(modification).  And it is probably good to include in the commit
comment what kernel version or commit they were drawn from so that can
be easily confirmed.

Section 3 still doesn't apply, as any resulting object code or
executables are no more a derived work due to the availability of the
header than they were previously.  And I don't think anyone is claiming
that the repo itself is a derived work - in this context it is just a
distribution medium.

The COPYING file, or equivalent, for the project should explicitly
exclude any claim on the kernel header directory to make clear we are
not trying to sublicense the headers as LGPL - which could breach
Section 4.

Other than those points, I don't see anywhere we may be in breach.

As with Greg, IANAL so this is not legal advice.  Feel free to
disregard any or all of the above if you still consider the legal
standing uncertain or too risky.

Cheers,
Kent.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository
  2021-01-28  3:26                     ` Kent Gibson
@ 2021-01-28  7:51                       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 22+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-28  7:51 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Bartosz Golaszewski, Dmitry V. Levin, Linus Walleij,
	Bartosz Golaszewski, Andy Shevchenko, open list:GPIO SUBSYSTEM,
	Thomas Petazzoni

On Thu, Jan 28, 2021 at 11:26:41AM +0800, Kent Gibson wrote:
> Bart and Dmitry: I submit that we are good to copy the headers into the
> repositories, but we should take a few steps just to make clear that we
> are in full compliance with the GPL v2.
> 
> Firstly, we are distributing the headers under Section 1 (distribution)
> of the GPL, so we should keep the headers in a separate directory that
> contains its own COPYING file as well as the GPL v2 and Linux syscall note
> - unless they are already available elsewhere in the repo.
> 
> The headers must be copied verbatim so as to not trigger Section 2
> (modification).  And it is probably good to include in the commit
> comment what kernel version or commit they were drawn from so that can
> be easily confirmed.
> 
> Section 3 still doesn't apply, as any resulting object code or
> executables are no more a derived work due to the availability of the
> header than they were previously.  And I don't think anyone is claiming
> that the repo itself is a derived work - in this context it is just a
> distribution medium.
> 
> The COPYING file, or equivalent, for the project should explicitly
> exclude any claim on the kernel header directory to make clear we are
> not trying to sublicense the headers as LGPL - which could breach
> Section 4.
> 
> Other than those points, I don't see anywhere we may be in breach.

That looks good, you should also consider following the REUSE
specification:
	https://reuse.software/
which recommends using a LICENSES/ directory for the different licenses
in your project and use SPDX lines at the top of your files to make
everything explicit.  The `reuse lint` command line tool should give you
lots of hints on good things to fix up in this area.

Good luck!

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-01-28  7:53 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11 13:34 [libgpiod][PATCH 0/6] treewide: remove more cruft and Bartosz Golaszewski
2021-01-11 13:34 ` [libgpiod][PATCH 1/6] treewide: remove helpers for opening chips by name & number Bartosz Golaszewski
2021-01-11 13:34 ` [libgpiod][PATCH 2/6] treewide: simplify the active-low line property Bartosz Golaszewski
2021-01-11 13:34 ` [libgpiod][PATCH 3/6] treewide: rename BIAS_AS_IS to BIAS_NONE Bartosz Golaszewski
2021-01-11 14:31   ` Kent Gibson
2021-01-11 13:34 ` [libgpiod][PATCH 4/6] treewide: make drive settings an enum Bartosz Golaszewski
2021-01-11 14:39   ` Kent Gibson
2021-01-11 14:48     ` Bartosz Golaszewski
2021-01-11 13:34 ` [libgpiod][PATCH 5/6] bindings: cxx: line: reorder bias mapping entries Bartosz Golaszewski
2021-01-11 13:34 ` [libgpiod][PATCH 6/6] core: add the kernel uapi header to the repository Bartosz Golaszewski
2021-01-11 13:46   ` Andy Shevchenko
2021-01-11 14:06     ` Bartosz Golaszewski
2021-01-11 14:46       ` Andy Shevchenko
2021-01-11 15:15         ` Bartosz Golaszewski
2021-01-25  5:55           ` Kent Gibson
2021-01-25 10:54             ` Andy Shevchenko
2021-01-26 15:07             ` Bartosz Golaszewski
2021-01-26 17:11               ` Greg Kroah-Hartman
2021-01-26 19:08                 ` Bartosz Golaszewski
2021-01-27 11:43                   ` Greg Kroah-Hartman
2021-01-28  3:26                     ` Kent Gibson
2021-01-28  7:51                       ` Greg Kroah-Hartman

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