From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>,
Linus Walleij <linus.walleij@linaro.org>,
Bamvor Jian Zhang <bamv2005@gmail.com>
Subject: Re: [PATCH] gpio: gpio-mockup: Fix usage of new GPIO_LINE_DIRECTION
Date: Wed, 11 Dec 2019 09:16:15 +0800 [thread overview]
Message-ID: <20191211011615.GA9373@sol> (raw)
In-Reply-To: <20191210145515.GB3509@sol>
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.
next prev parent reply other threads:[~2019-12-11 1:16 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2019-12-10 17:26 ` Bartosz Golaszewski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191211011615.GA9373@sol \
--to=warthog618@gmail.com \
--cc=bamv2005@gmail.com \
--cc=bgolaszewski@baylibre.com \
--cc=brgl@bgdev.pl \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.