linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
@ 2019-12-10  2:15 Kent Gibson
  2019-12-10 14:11 ` Bartosz Golaszewski
  2019-12-10 17:26 ` Bartosz Golaszewski
  0 siblings, 2 replies; 7+ messages in thread
From: Kent Gibson @ 2019-12-10  2:15 UTC (permalink / raw)
  To: linux-kernel, linux-gpio, bgolaszewski, linus.walleij, bamv2005
  Cc: Kent Gibson

Restore the external behavior of gpio-mockup to what it was prior to the
change to using GPIO_LINE_DIRECTION.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---

Fix a regression introduced in v5.5-rc1.

The change to GPIO_LINE_DIRECTION reversed the polarity of the
dir field within gpio-mockup.c, but overlooked inverting the value on
initialization and when returned by gpio_mockup_get_direction.
The latter is a bug.
The former is a problem for tests which assume initial conditions,
specifically the mockup used to initialize chips with all lines as inputs.
That superficially appeared to be the case after the previous patch due
to the bug in gpio_mockup_get_direction.

 drivers/gpio/gpio-mockup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 56d647a30e3e..c4fdc192ea4e 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
 	int direction;
 
 	mutex_lock(&chip->lock);
-	direction = !chip->lines[offset].dir;
+	direction = chip->lines[offset].dir;
 	mutex_unlock(&chip->lock);
 
 	return direction;
@@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	struct gpio_chip *gc;
 	struct device *dev;
 	const char *name;
-	int rv, base;
+	int rv, base, i;
 	u16 ngpio;
 
 	dev = &pdev->dev;
@@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 	if (!chip->lines)
 		return -ENOMEM;
 
+	for (i = 0; i < gc->ngpio; i++)
+		chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
+
 	if (device_property_read_bool(dev, "named-gpio-lines")) {
 		rv = gpio_mockup_name_lines(dev, chip);
 		if (rv)
-- 
2.24.0


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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10  2:15 [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION Kent Gibson
@ 2019-12-10 14:11 ` Bartosz Golaszewski
  2019-12-10 14:55   ` Kent Gibson
  2019-12-10 17:26 ` Bartosz Golaszewski
  1 sibling, 1 reply; 7+ messages in thread
From: Bartosz Golaszewski @ 2019-12-10 14:11 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang

wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> Restore the external behavior of gpio-mockup to what it was prior to the
> change to using GPIO_LINE_DIRECTION.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
> ---
>
> Fix a regression introduced in v5.5-rc1.
>
> The change to GPIO_LINE_DIRECTION reversed the polarity of the
> dir field within gpio-mockup.c, but overlooked inverting the value on
> initialization and when returned by gpio_mockup_get_direction.
> The latter is a bug.
> The former is a problem for tests which assume initial conditions,
> specifically the mockup used to initialize chips with all lines as inputs.
> That superficially appeared to be the case after the previous patch due
> to the bug in gpio_mockup_get_direction.
>
>  drivers/gpio/gpio-mockup.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index 56d647a30e3e..c4fdc192ea4e 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
>         int direction;
>
>         mutex_lock(&chip->lock);
> -       direction = !chip->lines[offset].dir;
> +       direction = chip->lines[offset].dir;
>         mutex_unlock(&chip->lock);
>
>         return direction;
> @@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>         struct gpio_chip *gc;
>         struct device *dev;
>         const char *name;
> -       int rv, base;
> +       int rv, base, i;
>         u16 ngpio;
>
>         dev = &pdev->dev;
> @@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
>         if (!chip->lines)
>                 return -ENOMEM;
>
> +       for (i = 0; i < gc->ngpio; i++)
> +               chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
> +
>         if (device_property_read_bool(dev, "named-gpio-lines")) {
>                 rv = gpio_mockup_name_lines(dev, chip);
>                 if (rv)
> --
> 2.24.0
>

Hi Kent,

I was applying and testing your libgpiod series and noticed that the
gpio-tools tests fail after applying patches 16 & 17 (with linux
v5.5-rc1). Is this fix related to this?

Bart

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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10 14:11 ` Bartosz Golaszewski
@ 2019-12-10 14:55   ` Kent Gibson
  2019-12-10 15:34     ` Bartosz Golaszewski
  2019-12-11  1:16     ` Kent Gibson
  0 siblings, 2 replies; 7+ messages in thread
From: Kent Gibson @ 2019-12-10 14:55 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang

On Tue, Dec 10, 2019 at 03:11:12PM +0100, Bartosz Golaszewski wrote:
> wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > Restore the external behavior of gpio-mockup to what it was prior to the
> > change to using GPIO_LINE_DIRECTION.
> >
> > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > ---
> >
> > Fix a regression introduced in v5.5-rc1.
> >
> > The change to GPIO_LINE_DIRECTION reversed the polarity of the
> > dir field within gpio-mockup.c, but overlooked inverting the value on
> > initialization and when returned by gpio_mockup_get_direction.
> > The latter is a bug.
> > The former is a problem for tests which assume initial conditions,
> > specifically the mockup used to initialize chips with all lines as inputs.
> > That superficially appeared to be the case after the previous patch due
> > to the bug in gpio_mockup_get_direction.
> >
> >  drivers/gpio/gpio-mockup.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> > index 56d647a30e3e..c4fdc192ea4e 100644
> > --- a/drivers/gpio/gpio-mockup.c
> > +++ b/drivers/gpio/gpio-mockup.c
> > @@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
> >         int direction;
> >
> >         mutex_lock(&chip->lock);
> > -       direction = !chip->lines[offset].dir;
> > +       direction = chip->lines[offset].dir;
> >         mutex_unlock(&chip->lock);
> >
> >         return direction;
> > @@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> >         struct gpio_chip *gc;
> >         struct device *dev;
> >         const char *name;
> > -       int rv, base;
> > +       int rv, base, i;
> >         u16 ngpio;
> >
> >         dev = &pdev->dev;
> > @@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> >         if (!chip->lines)
> >                 return -ENOMEM;
> >
> > +       for (i = 0; i < gc->ngpio; i++)
> > +               chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
> > +
> >         if (device_property_read_bool(dev, "named-gpio-lines")) {
> >                 rv = gpio_mockup_name_lines(dev, chip);
> >                 if (rv)
> > --
> > 2.24.0
> >
> 
> Hi Kent,
> 
> I was applying and testing your libgpiod series and noticed that the
> gpio-tools tests fail after applying patches 16 & 17 (with linux
> v5.5-rc1). Is this fix related to this?
> 

I don't think so.  I've only been able to trip this problem with a
couple of corner cases in my Go uapi test suite.
I have been unable to reproduce it with the tools as it requires
multiple requests with the same chip fd, including an as-is, to trip.

And running the libgpiod tests against v5.5-rc1 works for me.
Can you provide more details as to the errors you are seeing?

Btw, I was writing tests for your LINEINFO_WATCH patch v2, which I was
applying to v5.5-rc1, when I ran across this.  That works ok if I
__packed the changed struct.
And I can confirm that patch v2 doesn't isolate watches on different
chip fds.

Kent.

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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10 14:55   ` Kent Gibson
@ 2019-12-10 15:34     ` Bartosz Golaszewski
  2019-12-10 23:36       ` Kent Gibson
  2019-12-11  1:16     ` Kent Gibson
  1 sibling, 1 reply; 7+ messages in thread
From: Bartosz Golaszewski @ 2019-12-10 15:34 UTC (permalink / raw)
  To: Kent Gibson
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang

wt., 10 gru 2019 o 15:55 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> On Tue, Dec 10, 2019 at 03:11:12PM +0100, Bartosz Golaszewski wrote:
> > wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > >
> > > Restore the external behavior of gpio-mockup to what it was prior to the
> > > change to using GPIO_LINE_DIRECTION.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > ---
> > >
> > > Fix a regression introduced in v5.5-rc1.
> > >
> > > The change to GPIO_LINE_DIRECTION reversed the polarity of the
> > > dir field within gpio-mockup.c, but overlooked inverting the value on
> > > initialization and when returned by gpio_mockup_get_direction.
> > > The latter is a bug.
> > > The former is a problem for tests which assume initial conditions,
> > > specifically the mockup used to initialize chips with all lines as inputs.
> > > That superficially appeared to be the case after the previous patch due
> > > to the bug in gpio_mockup_get_direction.
> > >
> > >  drivers/gpio/gpio-mockup.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> > > index 56d647a30e3e..c4fdc192ea4e 100644
> > > --- a/drivers/gpio/gpio-mockup.c
> > > +++ b/drivers/gpio/gpio-mockup.c
> > > @@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
> > >         int direction;
> > >
> > >         mutex_lock(&chip->lock);
> > > -       direction = !chip->lines[offset].dir;
> > > +       direction = chip->lines[offset].dir;
> > >         mutex_unlock(&chip->lock);
> > >
> > >         return direction;
> > > @@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > >         struct gpio_chip *gc;
> > >         struct device *dev;
> > >         const char *name;
> > > -       int rv, base;
> > > +       int rv, base, i;
> > >         u16 ngpio;
> > >
> > >         dev = &pdev->dev;
> > > @@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > >         if (!chip->lines)
> > >                 return -ENOMEM;
> > >
> > > +       for (i = 0; i < gc->ngpio; i++)
> > > +               chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
> > > +
> > >         if (device_property_read_bool(dev, "named-gpio-lines")) {
> > >                 rv = gpio_mockup_name_lines(dev, chip);
> > >                 if (rv)
> > > --
> > > 2.24.0
> > >
> >
> > Hi Kent,
> >
> > I was applying and testing your libgpiod series and noticed that the
> > gpio-tools tests fail after applying patches 16 & 17 (with linux
> > v5.5-rc1). Is this fix related to this?
> >
>
> I don't think so.  I've only been able to trip this problem with a
> couple of corner cases in my Go uapi test suite.
> I have been unable to reproduce it with the tools as it requires
> multiple requests with the same chip fd, including an as-is, to trip.
>
> And running the libgpiod tests against v5.5-rc1 works for me.
> Can you provide more details as to the errors you are seeing?
>

Hmm whatever that was, it's gone now. Must have been some leftovers
from previous builds. All works now.

> Btw, I was writing tests for your LINEINFO_WATCH patch v2, which I was
> applying to v5.5-rc1, when I ran across this.  That works ok if I
> __packed the changed struct.

These things can still change, so don't spend too much time on it yet. :)

Since the lineinfo struct is not __packed, I'd prefer to not use it
for any struct embedding it. I'll just add appropriate padding for
64-bit alignment.

> And I can confirm that patch v2 doesn't isolate watches on different
> chip fds.
>

Yeah, I'll fix this.

Bart

> Kent.

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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10  2:15 [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION Kent Gibson
  2019-12-10 14:11 ` Bartosz Golaszewski
@ 2019-12-10 17:26 ` Bartosz Golaszewski
  1 sibling, 0 replies; 7+ messages in thread
From: Bartosz Golaszewski @ 2019-12-10 17:26 UTC (permalink / raw)
  To: Kent Gibson; +Cc: LKML, linux-gpio, Linus Walleij, Bamvor Jian Zhang

wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
>
> Restore the external behavior of gpio-mockup to what it was prior to the
> change to using GPIO_LINE_DIRECTION.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>

Please add the Fixes: tag with the commit this patch fixes. See other
commits that Cc the stable branch.

Bart

> ---

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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10 15:34     ` Bartosz Golaszewski
@ 2019-12-10 23:36       ` Kent Gibson
  0 siblings, 0 replies; 7+ messages in thread
From: Kent Gibson @ 2019-12-10 23:36 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang

On Tue, Dec 10, 2019 at 04:34:21PM +0100, Bartosz Golaszewski wrote:
> wt., 10 gru 2019 o 15:55 Kent Gibson <warthog618@gmail.com> napisał(a):
> >
> > On Tue, Dec 10, 2019 at 03:11:12PM +0100, Bartosz Golaszewski wrote:
> > > wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > > >
> > > > Restore the external behavior of gpio-mockup to what it was prior to the
> > > > change to using GPIO_LINE_DIRECTION.
> > > >
> > > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > > ---
> > > >
> > > > Fix a regression introduced in v5.5-rc1.
> > > >
> > > > The change to GPIO_LINE_DIRECTION reversed the polarity of the
> > > > dir field within gpio-mockup.c, but overlooked inverting the value on
> > > > initialization and when returned by gpio_mockup_get_direction.
> > > > The latter is a bug.
> > > > The former is a problem for tests which assume initial conditions,
> > > > specifically the mockup used to initialize chips with all lines as inputs.
> > > > That superficially appeared to be the case after the previous patch due
> > > > to the bug in gpio_mockup_get_direction.
> > > >
> > > >  drivers/gpio/gpio-mockup.c | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> > > > index 56d647a30e3e..c4fdc192ea4e 100644
> > > > --- a/drivers/gpio/gpio-mockup.c
> > > > +++ b/drivers/gpio/gpio-mockup.c
> > > > @@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
> > > >         int direction;
> > > >
> > > >         mutex_lock(&chip->lock);
> > > > -       direction = !chip->lines[offset].dir;
> > > > +       direction = chip->lines[offset].dir;
> > > >         mutex_unlock(&chip->lock);
> > > >
> > > >         return direction;
> > > > @@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > > >         struct gpio_chip *gc;
> > > >         struct device *dev;
> > > >         const char *name;
> > > > -       int rv, base;
> > > > +       int rv, base, i;
> > > >         u16 ngpio;
> > > >
> > > >         dev = &pdev->dev;
> > > > @@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > > >         if (!chip->lines)
> > > >                 return -ENOMEM;
> > > >
> > > > +       for (i = 0; i < gc->ngpio; i++)
> > > > +               chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
> > > > +
> > > >         if (device_property_read_bool(dev, "named-gpio-lines")) {
> > > >                 rv = gpio_mockup_name_lines(dev, chip);
> > > >                 if (rv)
> > > > --
> > > > 2.24.0
> > > >
> > >
> > > Hi Kent,
> > >
> > > I was applying and testing your libgpiod series and noticed that the
> > > gpio-tools tests fail after applying patches 16 & 17 (with linux
> > > v5.5-rc1). Is this fix related to this?
> > >
> >
> > I don't think so.  I've only been able to trip this problem with a
> > couple of corner cases in my Go uapi test suite.
> > I have been unable to reproduce it with the tools as it requires
> > multiple requests with the same chip fd, including an as-is, to trip.
> >
> > And running the libgpiod tests against v5.5-rc1 works for me.
> > Can you provide more details as to the errors you are seeing?
> >
> 
> Hmm whatever that was, it's gone now. Must have been some leftovers
> from previous builds. All works now.
> 
> > Btw, I was writing tests for your LINEINFO_WATCH patch v2, which I was
> > applying to v5.5-rc1, when I ran across this.  That works ok if I
> > __packed the changed struct.
> 
> These things can still change, so don't spend too much time on it yet. :)
> 

Absolutely.  But as this is an ABI addition I wanted to have something
to give it a decent workout before it gets applied.
So far the only problems I've found are the alignment and isolation
issues already mentioned.

> Since the lineinfo struct is not __packed, I'd prefer to not use it
> for any struct embedding it. I'll just add appropriate padding for
> 64-bit alignment.
> 

Explicit padding for 64-bit alignment makes sense to me.

> > And I can confirm that patch v2 doesn't isolate watches on different
> > chip fds.
> >
> 
> Yeah, I'll fix this.

One more thing - since it is possible to lose change events due to fifo
overflow, how about adding a seqnum?  And if you do end up doing a v2 of
the event ABI to fix its alignment, adding one there as well.

Kent.


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

* Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
  2019-12-10 14:55   ` Kent Gibson
  2019-12-10 15:34     ` Bartosz Golaszewski
@ 2019-12-11  1:16     ` Kent Gibson
  1 sibling, 0 replies; 7+ messages in thread
From: Kent Gibson @ 2019-12-11  1:16 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM,
	Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang

On Tue, Dec 10, 2019 at 10:55:15PM +0800, Kent Gibson wrote:
> On Tue, Dec 10, 2019 at 03:11:12PM +0100, Bartosz Golaszewski wrote:
> > wt., 10 gru 2019 o 03:15 Kent Gibson <warthog618@gmail.com> napisał(a):
> > >
> > > Restore the external behavior of gpio-mockup to what it was prior to the
> > > change to using GPIO_LINE_DIRECTION.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > ---
> > >
> > > Fix a regression introduced in v5.5-rc1.
> > >
> > > The change to GPIO_LINE_DIRECTION reversed the polarity of the
> > > dir field within gpio-mockup.c, but overlooked inverting the value on
> > > initialization and when returned by gpio_mockup_get_direction.
> > > The latter is a bug.
> > > The former is a problem for tests which assume initial conditions,
> > > specifically the mockup used to initialize chips with all lines as inputs.
> > > That superficially appeared to be the case after the previous patch due
> > > to the bug in gpio_mockup_get_direction.
> > >
> > >  drivers/gpio/gpio-mockup.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> > > index 56d647a30e3e..c4fdc192ea4e 100644
> > > --- a/drivers/gpio/gpio-mockup.c
> > > +++ b/drivers/gpio/gpio-mockup.c
> > > @@ -226,7 +226,7 @@ static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
> > >         int direction;
> > >
> > >         mutex_lock(&chip->lock);
> > > -       direction = !chip->lines[offset].dir;
> > > +       direction = chip->lines[offset].dir;
> > >         mutex_unlock(&chip->lock);
> > >
> > >         return direction;
> > > @@ -395,7 +395,7 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > >         struct gpio_chip *gc;
> > >         struct device *dev;
> > >         const char *name;
> > > -       int rv, base;
> > > +       int rv, base, i;
> > >         u16 ngpio;
> > >
> > >         dev = &pdev->dev;
> > > @@ -447,6 +447,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
> > >         if (!chip->lines)
> > >                 return -ENOMEM;
> > >
> > > +       for (i = 0; i < gc->ngpio; i++)
> > > +               chip->lines[i].dir = GPIO_LINE_DIRECTION_IN;
> > > +
> > >         if (device_property_read_bool(dev, "named-gpio-lines")) {
> > >                 rv = gpio_mockup_name_lines(dev, chip);
> > >                 if (rv)
> > > --
> > > 2.24.0
> > >
> > 
> > Hi Kent,
> > 
> > I was applying and testing your libgpiod series and noticed that the
> > gpio-tools tests fail after applying patches 16 & 17 (with linux
> > v5.5-rc1). Is this fix related to this?
> > 
> 
> I don't think so.  I've only been able to trip this problem with a
> couple of corner cases in my Go uapi test suite.
> I have been unable to reproduce it with the tools as it requires
> multiple requests with the same chip fd, including an as-is, to trip.
> 

It turns out that I can reproduce the bug with my gpiod tools:

root@firefly:/home/kent/gpiod/cmd/gpiodctl# uname -a
Linux firefly 5.5.0-rc1 #23 SMP Mon Dec 9 16:26:33 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
root@firefly:/home/kent/gpiod/cmd/gpiodctl# modprobe gpio-mockup gpio_mockup_ranges=-1,4
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl get gpiochip0 1
0
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl info
gpiochip0 - 4 lines:
	line   0:     unnamed      unused   input  active-high
	line   1:     unnamed      unused   input  active-high
	line   2:     unnamed      unused   input  active-high
	line   3:     unnamed      unused   input  active-high
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl get --as-is gpiochip0 1
0
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl info
gpiochip0 - 4 lines:
	line   0:     unnamed      unused   input  active-high
	line   1:     unnamed      unused  output  active-high
	line   2:     unnamed      unused   input  active-high
	line   3:     unnamed      unused   input  active-high

Note that the line 1 direction has flipped for no reason.

With the patched kernel that doesn't happen:

root@firefly:/home/kent/gpiod/cmd/gpiodctl# uname -a
Linux firefly 5.5.0-rc1+ #27 SMP Tue Dec 10 01:07:59 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
root@firefly:/home/kent/gpiod/cmd/gpiodctl# modprobe gpio-mockup gpio_mockup_ranges=-1,4
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl get gpiochip0 1
0
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl info
gpiochip0 - 4 lines:
	line   0:     unnamed      unused   input  active-high
	line   1:     unnamed      unused   input  active-high
	line   2:     unnamed      unused   input  active-high
	line   3:     unnamed      unused   input  active-high
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl get --as-is gpiochip0 1
0
root@firefly:/home/kent/gpiod/cmd/gpiodctl# ./gpiodctl info
gpiochip0 - 4 lines:
	line   0:     unnamed      unused   input  active-high
	line   1:     unnamed      unused   input  active-high
	line   2:     unnamed      unused   input  active-high
	line   3:     unnamed      unused   input  active-high


I would prefer to demonstrate this with the libgpiod tools, but they
don't support as-is on gets.  I recall suggesting adding it and you
asking why - who would need it.  This is a concrete example of my
response at the time - so you can exercise the full API for testing.

Cheers,
Kent.


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

end of thread, other threads:[~2019-12-11  1:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10  2:15 [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION Kent Gibson
2019-12-10 14:11 ` Bartosz Golaszewski
2019-12-10 14:55   ` Kent Gibson
2019-12-10 15:34     ` Bartosz Golaszewski
2019-12-10 23:36       ` Kent Gibson
2019-12-11  1:16     ` Kent Gibson
2019-12-10 17:26 ` 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).