Message ID | 20240626053808.179457-1-warthog618@gmail.com |
---|---|
Headers | show |
Series | bindings: python: more flexible reconfigure_lines() | expand |
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 >
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 > >
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.