[PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The kernel GPIO subsystem can emulate open-drain and open-source by not
actively driving the line for active and inactive output values
respectively. The kernel does it by setting the line to input in these
cases but this still must be reported as output to user-space. Add new
test-cases that verify this behavior.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Andy's comment on a GPIOLIB patch made me realize it's a good idea to
add tests for open-drain and open-source emulation in the kernel where
we don't actively drive the line for active and inactive values
respectively.
---
 tests/tests-kernel-uapi.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)

diff --git a/tests/tests-kernel-uapi.c b/tests/tests-kernel-uapi.c
index ff220fc..5955fac 100644
--- a/tests/tests-kernel-uapi.c
+++ b/tests/tests-kernel-uapi.c
@@ -110,3 +110,90 @@ GPIOD_TEST_CASE(enable_debounce_then_edge_detection)
 
 	g_assert_cmpuint(ts_falling, >, ts_rising);
 }
+
+GPIOD_TEST_CASE(open_drain_emulation)
+{
+	static const guint offset = 2;
+
+	g_autoptr(GPIOSimChip) sim = g_gpiosim_chip_new("num-lines", 8, NULL);
+	g_autoptr(struct_gpiod_chip) chip = NULL;
+	g_autoptr(struct_gpiod_line_settings) settings = NULL;
+	g_autoptr(struct_gpiod_line_config) line_cfg = NULL;
+	g_autoptr(struct_gpiod_line_request) request = NULL;
+	g_autoptr(struct_gpiod_line_info) info = NULL;
+	gint ret;
+
+	chip = gpiod_test_open_chip_or_fail(g_gpiosim_chip_get_dev_path(sim));
+	settings = gpiod_test_create_line_settings_or_fail();
+	line_cfg = gpiod_test_create_line_config_or_fail();
+
+	gpiod_line_settings_set_direction(settings,
+					  GPIOD_LINE_DIRECTION_OUTPUT);
+	gpiod_line_settings_set_drive(settings, GPIOD_LINE_DRIVE_OPEN_DRAIN);
+	gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
+							 settings);
+	request = gpiod_test_chip_request_lines_or_fail(chip, NULL, line_cfg);
+
+	ret = gpiod_line_request_set_value(request, offset,
+					   GPIOD_LINE_VALUE_ACTIVE);
+	g_assert_cmpint(ret, ==, 0);
+	gpiod_test_return_if_failed();
+
+	/*
+	 * The open-drain emulation in the kernel will set the line's direction
+	 * to input but NOT set FLAG_IS_OUT. Let's verify the direction is
+	 * still reported as output.
+	 */
+	info = gpiod_test_chip_get_line_info_or_fail(chip, offset);
+	g_assert_cmpint(gpiod_line_info_get_direction(info), ==,
+			GPIOD_LINE_DIRECTION_OUTPUT);
+	g_assert_cmpint(gpiod_line_info_get_drive(info), ==,
+			GPIOD_LINE_DRIVE_OPEN_DRAIN);
+
+	/*
+	 * The actual line is not being actively driven, so check that too on
+	 * the gpio-sim end.
+	 */
+	g_assert_cmpint(g_gpiosim_chip_get_value(sim, offset), ==,
+			G_GPIOSIM_VALUE_INACTIVE);
+}
+
+GPIOD_TEST_CASE(open_source_emulation)
+{
+	static const guint offset = 2;
+
+	g_autoptr(GPIOSimChip) sim = g_gpiosim_chip_new("num-lines", 8, NULL);
+	g_autoptr(struct_gpiod_chip) chip = NULL;
+	g_autoptr(struct_gpiod_line_settings) settings = NULL;
+	g_autoptr(struct_gpiod_line_config) line_cfg = NULL;
+	g_autoptr(struct_gpiod_line_request) request = NULL;
+	g_autoptr(struct_gpiod_line_info) info = NULL;
+	gint ret;
+
+	chip = gpiod_test_open_chip_or_fail(g_gpiosim_chip_get_dev_path(sim));
+	settings = gpiod_test_create_line_settings_or_fail();
+	line_cfg = gpiod_test_create_line_config_or_fail();
+
+	gpiod_line_settings_set_direction(settings,
+					  GPIOD_LINE_DIRECTION_OUTPUT);
+	gpiod_line_settings_set_drive(settings, GPIOD_LINE_DRIVE_OPEN_SOURCE);
+	gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
+							 settings);
+	request = gpiod_test_chip_request_lines_or_fail(chip, NULL, line_cfg);
+
+	ret = gpiod_line_request_set_value(request, offset,
+					   GPIOD_LINE_VALUE_INACTIVE);
+	g_assert_cmpint(ret, ==, 0);
+	gpiod_test_return_if_failed();
+
+	/*
+	 * The open-source emulation in the kernel will set the line's direction
+	 * to input but NOT set FLAG_IS_OUT. Let's verify the direction is
+	 * still reported as output.
+	 */
+	info = gpiod_test_chip_get_line_info_or_fail(chip, offset);
+	g_assert_cmpint(gpiod_line_info_get_direction(info), ==,
+			GPIOD_LINE_DIRECTION_OUTPUT);
+	g_assert_cmpint(gpiod_line_info_get_drive(info), ==,
+			GPIOD_LINE_DRIVE_OPEN_SOURCE);
+}

---
base-commit: 9f0eca2d7260de1ae22fed3795280bdb14b62e57
change-id: 20250410-open-drain-source-tests-908e55ac8bec

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Andy Shevchenko]

On Thu, Apr 10, 2025 at 12:17 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> The kernel GPIO subsystem can emulate open-drain and open-source by not
> actively driving the line for active and inactive output values
> respectively. The kernel does it by setting the line to input in these
> cases but this still must be reported as output to user-space. Add new
> test-cases that verify this behavior.

Thanks, that's indeed a good idea!
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Kent Gibson]

On Thu, Apr 10, 2025 at 08:34:37PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 10, 2025 at 12:17 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The kernel GPIO subsystem can emulate open-drain and open-source by not
> > actively driving the line for active and inactive output values
> > respectively. The kernel does it by setting the line to input in these
> > cases but this still must be reported as output to user-space. Add new
> > test-cases that verify this behavior.
>
> Thanks, that's indeed a good idea!
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
>

FWIW, the test suite[1] I wrote for my Go library along with the uAPI v2
also covers these cases, and more - particularly error cases.
Not that I've run it over Bart's changes yet, but it is my first stop when
checking for uAPI breakage.
It is written in Go, as there obviously was no libgpiod v2 at the time.

You can run the whole uapi test suite yourself by grabbing that repo and Go and...

~/go-gpiocdev/uapi$ go test -c
~/go-gpiocdev/uapi$ sudo ./uapi.test --test.v

I believe the cases above are covered (mostly) by

$ sudo ./uapi.test --test.v --test.run "TestGetLine$/output_[drain|source]"
=== RUN   TestGetLine
=== RUN   TestGetLine/output_drain
=== RUN   TestGetLine/output_source
--- PASS: TestGetLine (0.01s)
    --- PASS: TestGetLine/output_drain (0.00s)
    --- PASS: TestGetLine/output_source (0.00s)
PASS

which request the line then check that the reported info corresponds.

I don't seem to have any tests that cover setting the value of
open-drain/open-source outputs yet, so I might have to add some.
Though my existing set value tests check the value reported by the
gpio-sim, which will behave differently for emulated outputs...
Which probably explains why I didn't test that already - it is getting
into the bowels of gpiolib so it is really testing gpiolib behaviour,
not just the uAPI.  (I do have a few kernel specific tests, but they have
generally been added to test specific bugs that were visible though the
uAPI.)

On that point, it might also be handy if we could turn open-drain/open-source
support on and off in the gpio-sim (per sim), so tests can control if they
are getting the emulation, or not.

Cheers,
Kent.

[1] https://github.com/warthog618/go-gpiocdev

Re: [PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Kent Gibson]

On Thu, Apr 10, 2025 at 11:17:47AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> The kernel GPIO subsystem can emulate open-drain and open-source by not
> actively driving the line for active and inactive output values
> respectively. The kernel does it by setting the line to input in these
> cases but this still must be reported as output to user-space. Add new
> test-cases that verify this behavior.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> Andy's comment on a GPIOLIB patch made me realize it's a good idea to
> add tests for open-drain and open-source emulation in the kernel where
> we don't actively drive the line for active and inactive values
> respectively.
> ---
>  tests/tests-kernel-uapi.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>
> diff --git a/tests/tests-kernel-uapi.c b/tests/tests-kernel-uapi.c
> index ff220fc..5955fac 100644
> --- a/tests/tests-kernel-uapi.c
> +++ b/tests/tests-kernel-uapi.c
> @@ -110,3 +110,90 @@ GPIOD_TEST_CASE(enable_debounce_then_edge_detection)
>
>  	g_assert_cmpuint(ts_falling, >, ts_rising);
>  }
> +
> +GPIOD_TEST_CASE(open_drain_emulation)
> +{
> +	static const guint offset = 2;
> +
> +	g_autoptr(GPIOSimChip) sim = g_gpiosim_chip_new("num-lines", 8, NULL);
> +	g_autoptr(struct_gpiod_chip) chip = NULL;
> +	g_autoptr(struct_gpiod_line_settings) settings = NULL;
> +	g_autoptr(struct_gpiod_line_config) line_cfg = NULL;
> +	g_autoptr(struct_gpiod_line_request) request = NULL;
> +	g_autoptr(struct_gpiod_line_info) info = NULL;
> +	gint ret;
> +
> +	chip = gpiod_test_open_chip_or_fail(g_gpiosim_chip_get_dev_path(sim));
> +	settings = gpiod_test_create_line_settings_or_fail();
> +	line_cfg = gpiod_test_create_line_config_or_fail();
> +
> +	gpiod_line_settings_set_direction(settings,
> +					  GPIOD_LINE_DIRECTION_OUTPUT);
> +	gpiod_line_settings_set_drive(settings, GPIOD_LINE_DRIVE_OPEN_DRAIN);
> +	gpiod_test_line_config_add_line_settings_or_fail(line_cfg, &offset, 1,
> +							 settings);
> +	request = gpiod_test_chip_request_lines_or_fail(chip, NULL, line_cfg);
> +
> +	ret = gpiod_line_request_set_value(request, offset,
> +					   GPIOD_LINE_VALUE_ACTIVE);
> +	g_assert_cmpint(ret, ==, 0);
> +	gpiod_test_return_if_failed();
> +
> +	/*
> +	 * The open-drain emulation in the kernel will set the line's direction
> +	 * to input but NOT set FLAG_IS_OUT. Let's verify the direction is
> +	 * still reported as output.
> +	 */

My understanding is that FLAG_IS_OUT is always set for output lines,
even if the direction is set to input for the emulation.

To quote gpiod_direction_output():

set_output_flag:
	/*
	 * When emulating open-source or open-drain functionalities by not
	 * actively driving the line (setting mode to input) we still need to
	 * set the IS_OUT flag or otherwise we won't be able to set the line
	 * value anymore.
	 */
	if (ret == 0)
		set_bit(FLAG_IS_OUT, &desc->flags);
	return ret;

Cheers,
Kent.

Re: [PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Bartosz Golaszewski]

On Fri, 11 Apr 2025 at 03:34, Kent Gibson <warthog618@gmail.com> wrote:
>
> On Thu, Apr 10, 2025 at 11:17:47AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The kernel GPIO subsystem can emulate open-drain and open-source by not
> > actively driving the line for active and inactive output values
> > respectively. The kernel does it by setting the line to input in these
> > cases but this still must be reported as output to user-space. Add new
> > test-cases that verify this behavior.
> >
> > +     ret = gpiod_line_request_set_value(request, offset,
> > +                                        GPIOD_LINE_VALUE_ACTIVE);
> > +     g_assert_cmpint(ret, ==, 0);
> > +     gpiod_test_return_if_failed();
> > +
> > +     /*
> > +      * The open-drain emulation in the kernel will set the line's direction
> > +      * to input but NOT set FLAG_IS_OUT. Let's verify the direction is
> > +      * still reported as output.
> > +      */
>
> My understanding is that FLAG_IS_OUT is always set for output lines,
> even if the direction is set to input for the emulation.
>

Of course, it's a typo. It should have said: does NOT clear FLAG_IS_OUT.

Bartosz

Re: [PATCH libgpiod] tests: uapi: add test-cases for open-drain and open-source emulation

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Thu, 10 Apr 2025 11:17:47 +0200, Bartosz Golaszewski wrote:
> The kernel GPIO subsystem can emulate open-drain and open-source by not
> actively driving the line for active and inactive output values
> respectively. The kernel does it by setting the line to input in these
> cases but this still must be reported as output to user-space. Add new
> test-cases that verify this behavior.
> 
> 
> [...]

Applied, thanks!

[1/1] tests: uapi: add test-cases for open-drain and open-source emulation
      commit: 41231df28c9aecacaaae9e6493d31161023733d6

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>