linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines()
@ 2024-06-26  5:38 Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 1/3] bindings: python: tests: extend reconfiguration tests Kent Gibson
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Kent Gibson @ 2024-06-26  5:38 UTC (permalink / raw)
  To: linux-gpio, brgl; +Cc: Kent Gibson

This series addresses issue #54[1], making reconfigure_lines() less
restrictive in the configurations it will accept, allowing for
misordered configurations and reconfiguring a subset of lines.

Patch 1 adds a set of tests for the new behaviour.  These all fail with
the existing bindings, but pass after patch 2 is applied.

Patch 2 is the change to reconfigure_lines() itself.

The reconfiguration of a subset of lines works better with a change to the
kernel to ignore reconfiguration of lines without a direction set,
i.e. a default LineSettings.
With existing kernels, if a line has been requested with flags set to
non-default values those flags will be reset to default values, though
that may not become evident electrically until subsequent operations are
performed on the line.

Patch 3 extends the tests to demonstrate that kernel issue.  A kernel
patch addressing that issue has been submitted[2], and the test passes
with that patch applied.

Cheers,
Kent.

[1] https://github.com/brgl/libgpiod/issues/54
[2] https://lore.kernel.org/linux-gpio/20240626052925.174272-3-warthog618@gmail.com

Kent Gibson (3):
  bindings: python: tests: extend reconfiguration tests
  bindings: python: more flexible reconfigure_lines()
  bindings: python: tests: add coverage of kernel reconfigure as-is
    behaviour

 bindings/python/gpiod/line_request.py       | 17 +++---
 bindings/python/tests/tests_line_request.py | 64 ++++++++++++++++++++-
 2 files changed, 72 insertions(+), 9 deletions(-)

--
2.39.2


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

* [libgpiod][PATCH 1/3] bindings: python: tests: extend reconfiguration tests
  2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
@ 2024-06-26  5:38 ` Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 2/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kent Gibson @ 2024-06-26  5:38 UTC (permalink / raw)
  To: linux-gpio, brgl; +Cc: Kent Gibson

A number of the corner cases for reconfiguration are untested, including
 - using None for default settings
 - missing settings for some lines
 - jumbled line ordering relative to the request
 - extra settings for non-requested lines

Add tests for these corner cases.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 bindings/python/tests/tests_line_request.py | 50 +++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
index f99b93d..2f375d6 100644
--- a/bindings/python/tests/tests_line_request.py
+++ b/bindings/python/tests/tests_line_request.py
@@ -490,6 +490,56 @@ class ReconfigureRequestedLines(TestCase):
         info = self.chip.get_line_info(2)
         self.assertEqual(info.direction, Direction.INPUT)
 
+    def test_reconfigure_by_misordered_offsets(self):
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+        self.req.reconfigure_lines(
+            {(6, 0, 3, 2): gpiod.LineSettings(direction=Direction.INPUT)}
+        )
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.INPUT)
+
+    def test_reconfigure_by_misordered_names(self):
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+        self.req.reconfigure_lines(
+            {(0, "baz", 2, "foo"): gpiod.LineSettings(direction=Direction.INPUT)}
+        )
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.INPUT)
+
+    def test_reconfigure_with_default(self):
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+        self.req.reconfigure_lines({
+            0: gpiod.LineSettings(direction=Direction.INPUT),
+            2: None,
+            ("baz", "foo"): gpiod.LineSettings(direction=Direction.INPUT)
+        })
+        info = self.chip.get_line_info(0)
+        self.assertEqual(info.direction, Direction.INPUT)
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+
+    def test_reconfigure_missing_offsets(self):
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+        self.req.reconfigure_lines(
+                {(6, 0): gpiod.LineSettings(direction=Direction.INPUT)}
+            )
+        info = self.chip.get_line_info(0)
+        self.assertEqual(info.direction, Direction.INPUT)
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+
+    def test_reconfigure_extra_offsets(self):
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.OUTPUT)
+        self.req.reconfigure_lines(
+            {(0, 2, 3, 6, 5): gpiod.LineSettings(direction=Direction.INPUT)}
+            )
+        info = self.chip.get_line_info(2)
+        self.assertEqual(info.direction, Direction.INPUT)
 
 class ReleasedLineRequestCannotBeUsed(TestCase):
     def test_using_released_line_request(self):
-- 
2.39.2


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

* [libgpiod][PATCH 2/3] bindings: python: more flexible reconfigure_lines()
  2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 1/3] bindings: python: tests: extend reconfiguration tests Kent Gibson
@ 2024-06-26  5:38 ` Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour Kent Gibson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Kent Gibson @ 2024-06-26  5:38 UTC (permalink / raw)
  To: linux-gpio, brgl; +Cc: Kent Gibson

The C API requires the configuration passed to reconfigure_lines()
to contain the lines in the same order as they were requested.  This is
problematic for the Python bindings which accepts the configuration in
the form of a dict.  For versions prior to Python 3.6, dicts do not
maintain insertion order, so iterating over the dict emits lines in
unreliable order.

Even with later Python versions, the ordering requirement makes
reconfigure_lines() awkward to use as subsequent configurations may
group line settings quite differently to the request, yet the user must
go out of their way to reproduce the original ordering.
This is a task better performed by the bindings.

Further, while the documentation for reconfigure_lines() states that
None settings values are treated as default values, the current
implementation raises an error when it tries to dereference the None,
thinking it is an actual object.

Similarly, providing default values for lines for which no settings
are provided would allow support for reconfiguring a subset of
requested lines.

Rework reconfigure_lines() to remove the ordering requirement and
construct the configuration provided to the C API in request order.
Populate missing or None line settings with default values to satisfy
the requirements of the C API that all requested lines must be
reconfigured.

Closes: https://github.com/brgl/libgpiod/issues/54
Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 bindings/python/gpiod/line_request.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
index cde298f..51e600a 100644
--- a/bindings/python/gpiod/line_request.py
+++ b/bindings/python/gpiod/line_request.py
@@ -151,23 +151,26 @@ class LineRequest:
         Args:
           config
             Dictionary mapping offsets or names (or tuples thereof) to
-            LineSettings. If None is passed as the value of the mapping,
-            default settings are used.
+            LineSettings. If no entry exists, or a None is passed as the
+            settings, then the configuration for that line is not changed.
+            Any settings for non-requested lines are ignored.
         """
         self._check_released()

         line_cfg = _ext.LineConfig()
+        line_settings = {}

         for lines, settings in config.items():
             if isinstance(lines, int) or isinstance(lines, str):
                 lines = [lines]

-            offsets = [
-                self._name_map[line] if self._check_line_name(line) else line
-                for line in lines
-            ]
+            for line in lines:
+                offset = self._name_map[line] if self._check_line_name(line) else line
+                line_settings[offset] = settings

-            line_cfg.add_line_settings(offsets, _line_settings_to_ext(settings))
+        for offset in self.offsets:
+            settings = line_settings.get(offset) or LineSettings()
+            line_cfg.add_line_settings([offset], _line_settings_to_ext(settings))

         self._req.reconfigure_lines(line_cfg)

--
2.39.2


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

* [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour
  2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 1/3] bindings: python: tests: extend reconfiguration tests Kent Gibson
  2024-06-26  5:38 ` [libgpiod][PATCH 2/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
@ 2024-06-26  5:38 ` Kent Gibson
  2024-06-27 15:06   ` Bartosz Golaszewski
  2024-07-05  7:34 ` [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Bartosz Golaszewski
  2024-07-08  9:54 ` Bartosz Golaszewski
  4 siblings, 1 reply; 10+ messages in thread
From: Kent Gibson @ 2024-06-26  5:38 UTC (permalink / raw)
  To: linux-gpio, brgl; +Cc: Kent Gibson

The kernel's handling of reconfigure with default values, as is the
case for providing a None value as the settings to the Python bindings'
reconfigure_lines(), resets any flags set to non-default values when the
line is requested to their default values.  While the flags are cleared,
the kernel makes no corresponding change to the electrical settings -
though subsequent calls to get and set values will apply the updated
flags.

The tests for missing or None settings are extended to demonstrate the
issue for active_low and drive flags, though the issue applies to all
flags.

The tests fail unless the kernel is patched to ignore reconfiguration
of lines without direction set.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 bindings/python/tests/tests_line_request.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
index 2f375d6..79167f1 100644
--- a/bindings/python/tests/tests_line_request.py
+++ b/bindings/python/tests/tests_line_request.py
@@ -5,7 +5,7 @@ import errno
 import gpiod
 
 from . import gpiosim
-from gpiod.line import Direction, Edge, Value
+from gpiod.line import Direction, Drive, Edge, Value
 from unittest import TestCase
 
 Pull = gpiosim.Chip.Pull
@@ -462,7 +462,9 @@ class ReconfigureRequestedLines(TestCase):
         self.sim = gpiosim.Chip(num_lines=8, line_names={3: "foo", 4: "bar", 6: "baz"})
         self.chip = gpiod.Chip(self.sim.dev_path)
         self.req = self.chip.request_lines(
-            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT)}
+            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT,
+                                                      active_low=True,
+                                                      drive=Drive.OPEN_DRAIN)}
         )
 
     def tearDown(self):
@@ -511,6 +513,8 @@ class ReconfigureRequestedLines(TestCase):
     def test_reconfigure_with_default(self):
         info = self.chip.get_line_info(2)
         self.assertEqual(info.direction, Direction.OUTPUT)
+        self.assertTrue(info.active_low)
+        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
         self.req.reconfigure_lines({
             0: gpiod.LineSettings(direction=Direction.INPUT),
             2: None,
@@ -520,10 +524,14 @@ class ReconfigureRequestedLines(TestCase):
         self.assertEqual(info.direction, Direction.INPUT)
         info = self.chip.get_line_info(2)
         self.assertEqual(info.direction, Direction.OUTPUT)
+        self.assertTrue(info.active_low)
+        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
 
     def test_reconfigure_missing_offsets(self):
         info = self.chip.get_line_info(2)
         self.assertEqual(info.direction, Direction.OUTPUT)
+        self.assertTrue(info.active_low)
+        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
         self.req.reconfigure_lines(
                 {(6, 0): gpiod.LineSettings(direction=Direction.INPUT)}
             )
@@ -531,6 +539,8 @@ class ReconfigureRequestedLines(TestCase):
         self.assertEqual(info.direction, Direction.INPUT)
         info = self.chip.get_line_info(2)
         self.assertEqual(info.direction, Direction.OUTPUT)
+        self.assertTrue(info.active_low)
+        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
 
     def test_reconfigure_extra_offsets(self):
         info = self.chip.get_line_info(2)
-- 
2.39.2


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

* Re: [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour
  2024-06-26  5:38 ` [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour Kent Gibson
@ 2024-06-27 15:06   ` Bartosz Golaszewski
  2024-06-27 15:20     ` Bartosz Golaszewski
  0 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2024-06-27 15:06 UTC (permalink / raw)
  To: Kent Gibson; +Cc: linux-gpio

On Wed, Jun 26, 2024 at 7:39 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> The kernel's handling of reconfigure with default values, as is the
> case for providing a None value as the settings to the Python bindings'
> reconfigure_lines(), resets any flags set to non-default values when the
> line is requested to their default values.  While the flags are cleared,
> the kernel makes no corresponding change to the electrical settings -
> though subsequent calls to get and set values will apply the updated
> flags.
>
> The tests for missing or None settings are extended to demonstrate the
> issue for active_low and drive flags, though the issue applies to all
> flags.
>
> The tests fail unless the kernel is patched to ignore reconfiguration
> of lines without direction set.
>

Does it mean the kernel patches (at least the first two in the series)
are meant to be backported?

Bart

> Signed-off-by: Kent Gibson <warthog618@gmail.com>
> ---
>  bindings/python/tests/tests_line_request.py | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
> index 2f375d6..79167f1 100644
> --- a/bindings/python/tests/tests_line_request.py
> +++ b/bindings/python/tests/tests_line_request.py
> @@ -5,7 +5,7 @@ import errno
>  import gpiod
>
>  from . import gpiosim
> -from gpiod.line import Direction, Edge, Value
> +from gpiod.line import Direction, Drive, Edge, Value
>  from unittest import TestCase
>
>  Pull = gpiosim.Chip.Pull
> @@ -462,7 +462,9 @@ class ReconfigureRequestedLines(TestCase):
>          self.sim = gpiosim.Chip(num_lines=8, line_names={3: "foo", 4: "bar", 6: "baz"})
>          self.chip = gpiod.Chip(self.sim.dev_path)
>          self.req = self.chip.request_lines(
> -            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT)}
> +            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT,
> +                                                      active_low=True,
> +                                                      drive=Drive.OPEN_DRAIN)}
>          )
>
>      def tearDown(self):
> @@ -511,6 +513,8 @@ class ReconfigureRequestedLines(TestCase):
>      def test_reconfigure_with_default(self):
>          info = self.chip.get_line_info(2)
>          self.assertEqual(info.direction, Direction.OUTPUT)
> +        self.assertTrue(info.active_low)
> +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
>          self.req.reconfigure_lines({
>              0: gpiod.LineSettings(direction=Direction.INPUT),
>              2: None,
> @@ -520,10 +524,14 @@ class ReconfigureRequestedLines(TestCase):
>          self.assertEqual(info.direction, Direction.INPUT)
>          info = self.chip.get_line_info(2)
>          self.assertEqual(info.direction, Direction.OUTPUT)
> +        self.assertTrue(info.active_low)
> +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
>
>      def test_reconfigure_missing_offsets(self):
>          info = self.chip.get_line_info(2)
>          self.assertEqual(info.direction, Direction.OUTPUT)
> +        self.assertTrue(info.active_low)
> +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
>          self.req.reconfigure_lines(
>                  {(6, 0): gpiod.LineSettings(direction=Direction.INPUT)}
>              )
> @@ -531,6 +539,8 @@ class ReconfigureRequestedLines(TestCase):
>          self.assertEqual(info.direction, Direction.INPUT)
>          info = self.chip.get_line_info(2)
>          self.assertEqual(info.direction, Direction.OUTPUT)
> +        self.assertTrue(info.active_low)
> +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
>
>      def test_reconfigure_extra_offsets(self):
>          info = self.chip.get_line_info(2)
> --
> 2.39.2
>

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

* Re: [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour
  2024-06-27 15:06   ` Bartosz Golaszewski
@ 2024-06-27 15:20     ` Bartosz Golaszewski
  2024-06-28  1:26       ` Kent Gibson
  0 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2024-06-27 15:20 UTC (permalink / raw)
  To: Kent Gibson; +Cc: linux-gpio

On Thu, Jun 27, 2024 at 5:06 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Wed, Jun 26, 2024 at 7:39 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > The kernel's handling of reconfigure with default values, as is the
> > case for providing a None value as the settings to the Python bindings'
> > reconfigure_lines(), resets any flags set to non-default values when the
> > line is requested to their default values.  While the flags are cleared,
> > the kernel makes no corresponding change to the electrical settings -
> > though subsequent calls to get and set values will apply the updated
> > flags.
> >
> > The tests for missing or None settings are extended to demonstrate the
> > issue for active_low and drive flags, though the issue applies to all
> > flags.
> >
> > The tests fail unless the kernel is patched to ignore reconfiguration
> > of lines without direction set.
> >
>
> Does it mean the kernel patches (at least the first two in the series)
> are meant to be backported?
>
> Bart

Well, that was a stupid question, they both have the Fixes: tag...

Bart

>
> > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > ---
> >  bindings/python/tests/tests_line_request.py | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
> > index 2f375d6..79167f1 100644
> > --- a/bindings/python/tests/tests_line_request.py
> > +++ b/bindings/python/tests/tests_line_request.py
> > @@ -5,7 +5,7 @@ import errno
> >  import gpiod
> >
> >  from . import gpiosim
> > -from gpiod.line import Direction, Edge, Value
> > +from gpiod.line import Direction, Drive, Edge, Value
> >  from unittest import TestCase
> >
> >  Pull = gpiosim.Chip.Pull
> > @@ -462,7 +462,9 @@ class ReconfigureRequestedLines(TestCase):
> >          self.sim = gpiosim.Chip(num_lines=8, line_names={3: "foo", 4: "bar", 6: "baz"})
> >          self.chip = gpiod.Chip(self.sim.dev_path)
> >          self.req = self.chip.request_lines(
> > -            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT)}
> > +            {(0, 2, "foo", "baz"): gpiod.LineSettings(direction=Direction.OUTPUT,
> > +                                                      active_low=True,
> > +                                                      drive=Drive.OPEN_DRAIN)}
> >          )
> >
> >      def tearDown(self):
> > @@ -511,6 +513,8 @@ class ReconfigureRequestedLines(TestCase):
> >      def test_reconfigure_with_default(self):
> >          info = self.chip.get_line_info(2)
> >          self.assertEqual(info.direction, Direction.OUTPUT)
> > +        self.assertTrue(info.active_low)
> > +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
> >          self.req.reconfigure_lines({
> >              0: gpiod.LineSettings(direction=Direction.INPUT),
> >              2: None,
> > @@ -520,10 +524,14 @@ class ReconfigureRequestedLines(TestCase):
> >          self.assertEqual(info.direction, Direction.INPUT)
> >          info = self.chip.get_line_info(2)
> >          self.assertEqual(info.direction, Direction.OUTPUT)
> > +        self.assertTrue(info.active_low)
> > +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
> >
> >      def test_reconfigure_missing_offsets(self):
> >          info = self.chip.get_line_info(2)
> >          self.assertEqual(info.direction, Direction.OUTPUT)
> > +        self.assertTrue(info.active_low)
> > +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
> >          self.req.reconfigure_lines(
> >                  {(6, 0): gpiod.LineSettings(direction=Direction.INPUT)}
> >              )
> > @@ -531,6 +539,8 @@ class ReconfigureRequestedLines(TestCase):
> >          self.assertEqual(info.direction, Direction.INPUT)
> >          info = self.chip.get_line_info(2)
> >          self.assertEqual(info.direction, Direction.OUTPUT)
> > +        self.assertTrue(info.active_low)
> > +        self.assertEqual(info.drive, Drive.OPEN_DRAIN)
> >
> >      def test_reconfigure_extra_offsets(self):
> >          info = self.chip.get_line_info(2)
> > --
> > 2.39.2
> >

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

* Re: [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour
  2024-06-27 15:20     ` Bartosz Golaszewski
@ 2024-06-28  1:26       ` Kent Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: Kent Gibson @ 2024-06-28  1:26 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: linux-gpio

On Thu, Jun 27, 2024 at 05:20:13PM +0200, Bartosz Golaszewski wrote:
> On Thu, Jun 27, 2024 at 5:06 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Wed, Jun 26, 2024 at 7:39 AM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > The kernel's handling of reconfigure with default values, as is the
> > > case for providing a None value as the settings to the Python bindings'
> > > reconfigure_lines(), resets any flags set to non-default values when the
> > > line is requested to their default values.  While the flags are cleared,
> > > the kernel makes no corresponding change to the electrical settings -
> > > though subsequent calls to get and set values will apply the updated
> > > flags.
> > >
> > > The tests for missing or None settings are extended to demonstrate the
> > > issue for active_low and drive flags, though the issue applies to all
> > > flags.
> > >
> > > The tests fail unless the kernel is patched to ignore reconfiguration
> > > of lines without direction set.
> > >
> >
> > Does it mean the kernel patches (at least the first two in the series)
> > are meant to be backported?
> >
> > Bart
>
> Well, that was a stupid question, they both have the Fixes: tag...
>

I split them up and added the Fixes in case you do want to backport
them.  It would be good to backport the second as the Python bindings
now become the first use case I am aware of that uses a directionless
reconfig for subsets.  It would be great if that always worked as
expected on stable kernels.

Backporting the first, for uAPI v1, is less pressing as I'm not aware
of anyone actually using it that way, but your call.

Cheers,
Kent.

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

* Re: [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines()
  2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
                   ` (2 preceding siblings ...)
  2024-06-26  5:38 ` [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour Kent Gibson
@ 2024-07-05  7:34 ` Bartosz Golaszewski
  2024-07-06  2:35   ` Kent Gibson
  2024-07-08  9:54 ` Bartosz Golaszewski
  4 siblings, 1 reply; 10+ messages in thread
From: Bartosz Golaszewski @ 2024-07-05  7:34 UTC (permalink / raw)
  To: Kent Gibson; +Cc: linux-gpio

On Wed, Jun 26, 2024 at 7:38 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> This series addresses issue #54[1], making reconfigure_lines() less
> restrictive in the configurations it will accept, allowing for
> misordered configurations and reconfiguring a subset of lines.
>
> Patch 1 adds a set of tests for the new behaviour.  These all fail with
> the existing bindings, but pass after patch 2 is applied.
>
> Patch 2 is the change to reconfigure_lines() itself.
>
> The reconfiguration of a subset of lines works better with a change to the
> kernel to ignore reconfiguration of lines without a direction set,
> i.e. a default LineSettings.
> With existing kernels, if a line has been requested with flags set to
> non-default values those flags will be reset to default values, though
> that may not become evident electrically until subsequent operations are
> performed on the line.
>
> Patch 3 extends the tests to demonstrate that kernel issue.  A kernel
> patch addressing that issue has been submitted[2], and the test passes
> with that patch applied.
>
> Cheers,
> Kent.
>
> [1] https://github.com/brgl/libgpiod/issues/54
> [2] https://lore.kernel.org/linux-gpio/20240626052925.174272-3-warthog618@gmail.com
>
> Kent Gibson (3):
>   bindings: python: tests: extend reconfiguration tests
>   bindings: python: more flexible reconfigure_lines()
>   bindings: python: tests: add coverage of kernel reconfigure as-is
>     behaviour
>
>  bindings/python/gpiod/line_request.py       | 17 +++---
>  bindings/python/tests/tests_line_request.py | 64 ++++++++++++++++++++-
>  2 files changed, 72 insertions(+), 9 deletions(-)
>
> --
> 2.39.2
>

FYI I did not forget about this series - I'm just waiting for the next
stable kernel release so that changes required for the new test cases
to pass become available.

Bart

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

* Re: [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines()
  2024-07-05  7:34 ` [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Bartosz Golaszewski
@ 2024-07-06  2:35   ` Kent Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: Kent Gibson @ 2024-07-06  2:35 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: linux-gpio

On Fri, Jul 05, 2024 at 09:34:52AM +0200, Bartosz Golaszewski wrote:
> On Wed, Jun 26, 2024 at 7:38 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > This series addresses issue #54[1], making reconfigure_lines() less
> > restrictive in the configurations it will accept, allowing for
> > misordered configurations and reconfiguring a subset of lines.
> >
> > Patch 1 adds a set of tests for the new behaviour.  These all fail with
> > the existing bindings, but pass after patch 2 is applied.
> >
> > Patch 2 is the change to reconfigure_lines() itself.
> >
> > The reconfiguration of a subset of lines works better with a change to the
> > kernel to ignore reconfiguration of lines without a direction set,
> > i.e. a default LineSettings.
> > With existing kernels, if a line has been requested with flags set to
> > non-default values those flags will be reset to default values, though
> > that may not become evident electrically until subsequent operations are
> > performed on the line.
> >
> > Patch 3 extends the tests to demonstrate that kernel issue.  A kernel
> > patch addressing that issue has been submitted[2], and the test passes
> > with that patch applied.
> >
> > Cheers,
> > Kent.
> >
> > [1] https://github.com/brgl/libgpiod/issues/54
> > [2] https://lore.kernel.org/linux-gpio/20240626052925.174272-3-warthog618@gmail.com
> >
> > Kent Gibson (3):
> >   bindings: python: tests: extend reconfiguration tests
> >   bindings: python: more flexible reconfigure_lines()
> >   bindings: python: tests: add coverage of kernel reconfigure as-is
> >     behaviour
> >
> >  bindings/python/gpiod/line_request.py       | 17 +++---
> >  bindings/python/tests/tests_line_request.py | 64 ++++++++++++++++++++-
> >  2 files changed, 72 insertions(+), 9 deletions(-)
> >
> > --
> > 2.39.2
> >
>
> FYI I did not forget about this series - I'm just waiting for the next
> stable kernel release so that changes required for the new test cases
> to pass become available.
>

No problem - I assumed that was the case.

Cheers,
Kent.

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

* Re: [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines()
  2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
                   ` (3 preceding siblings ...)
  2024-07-05  7:34 ` [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Bartosz Golaszewski
@ 2024-07-08  9:54 ` Bartosz Golaszewski
  4 siblings, 0 replies; 10+ messages in thread
From: Bartosz Golaszewski @ 2024-07-08  9:54 UTC (permalink / raw)
  To: linux-gpio, brgl, Kent Gibson; +Cc: Bartosz Golaszewski

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


On Wed, 26 Jun 2024 13:38:05 +0800, Kent Gibson wrote:
> This series addresses issue #54[1], making reconfigure_lines() less
> restrictive in the configurations it will accept, allowing for
> misordered configurations and reconfiguring a subset of lines.
> 
> Patch 1 adds a set of tests for the new behaviour.  These all fail with
> the existing bindings, but pass after patch 2 is applied.
> 
> [...]

Applied, thanks!

[1/3] bindings: python: tests: extend reconfiguration tests
      commit: 40db20eec045b026fb76089fbf46417bae20026b
[2/3] bindings: python: more flexible reconfigure_lines()
      commit: fd57153ab2ba0d05f359f21eb72765ac1ede8fb5
[3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour
      commit: d2466f5ae2b4541ec6d231e1cb5f00e86ec51195

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

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

end of thread, other threads:[~2024-07-08  9:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-26  5:38 [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
2024-06-26  5:38 ` [libgpiod][PATCH 1/3] bindings: python: tests: extend reconfiguration tests Kent Gibson
2024-06-26  5:38 ` [libgpiod][PATCH 2/3] bindings: python: more flexible reconfigure_lines() Kent Gibson
2024-06-26  5:38 ` [libgpiod][PATCH 3/3] bindings: python: tests: add coverage of kernel reconfigure as-is behaviour Kent Gibson
2024-06-27 15:06   ` Bartosz Golaszewski
2024-06-27 15:20     ` Bartosz Golaszewski
2024-06-28  1:26       ` Kent Gibson
2024-07-05  7:34 ` [libgpiod][PATCH 0/3] bindings: python: more flexible reconfigure_lines() Bartosz Golaszewski
2024-07-06  2:35   ` Kent Gibson
2024-07-08  9:54 ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).