diff mbox series

[1/3] gpio: mockup: fix indicated direction

Message ID 20181108165255.9940-2-brgl@bgdev.pl
State New
Headers show
Series gpio: mockup: misc updates | expand

Commit Message

Bartosz Golaszewski Nov. 8, 2018, 4:52 p.m. UTC
Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
beginning") fixed an existing issue but broke libgpiod tests by
changing the default direction of dummy lines to output.

We don't break user-space so make gpio-mockup behave as before.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Uwe Kleine-König Nov. 8, 2018, 8:35 p.m. UTC | #1
Hello Bartosz,

On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> beginning") fixed an existing issue but broke libgpiod tests by
> changing the default direction of dummy lines to output.

The indicated commit only changed what was shown in debugfs, but didn't
touch the actual direction of a GPIO, doesn't it? If someone called
gpiod_get_direction before it would have returned "output", too, unless
I miss something.

Best regards
Uwe
Bartosz Golaszewski Nov. 9, 2018, 11:13 a.m. UTC | #2
czw., 8 lis 2018 o 21:35 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> Hello Bartosz,
>
> On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > beginning") fixed an existing issue but broke libgpiod tests by
> > changing the default direction of dummy lines to output.
>
> The indicated commit only changed what was shown in debugfs, but didn't
> touch the actual direction of a GPIO, doesn't it? If someone called
> gpiod_get_direction before it would have returned "output", too, unless
> I miss something.
>

This commit (3edfb7bd76bd) sets the correct direction of the line by
actually calling get_direction() instead of assuming input if
direction_input is not NULL. It just so happened that previously the
default direction of gpio-mockup lines was output but it would be
displayed as input due to this inconsistency.

My commit fixes it by simply setting the value of GPIO_MOCKUP_DIR_IN
to 0 so that when we kzalloc the line structure, we'll get the right
direction automatically.

Bart
Uwe Kleine-König Nov. 9, 2018, 11:54 a.m. UTC | #3
Hello Bartosz,

On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > beginning") fixed an existing issue but broke libgpiod tests by
> > > changing the default direction of dummy lines to output.
> >
> > The indicated commit only changed what was shown in debugfs, but didn't
> > touch the actual direction of a GPIO, doesn't it? If someone called
> > gpiod_get_direction before it would have returned "output", too, unless
> > I miss something.
> >
> 
> This commit (3edfb7bd76bd) sets the correct direction of the line by
> actually calling get_direction() instead of assuming input if
> direction_input is not NULL. It just so happened that previously the
> default direction of gpio-mockup lines was output but it would be
> displayed as input due to this inconsistency.

Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
the direction of a gpio? I'd say it's a bad idea to depend on this as
(AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
contained in it.

I'd weaken the commit log a bit to not claim that the commit broke
libgpiod but that it only made the inconsistency visible. After all this
commit didn't "change the default direction of dummy lines to output",
they were an output already before.

Best regards
Uwe
Bartosz Golaszewski Nov. 9, 2018, 12:24 p.m. UTC | #4
pt., 9 lis 2018 o 12:54 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> Hello Bartosz,
>
> On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> > czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > > beginning") fixed an existing issue but broke libgpiod tests by
> > > > changing the default direction of dummy lines to output.
> > >
> > > The indicated commit only changed what was shown in debugfs, but didn't
> > > touch the actual direction of a GPIO, doesn't it? If someone called
> > > gpiod_get_direction before it would have returned "output", too, unless
> > > I miss something.
> > >
> >
> > This commit (3edfb7bd76bd) sets the correct direction of the line by
> > actually calling get_direction() instead of assuming input if
> > direction_input is not NULL. It just so happened that previously the
> > default direction of gpio-mockup lines was output but it would be
> > displayed as input due to this inconsistency.
>
> Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
> the direction of a gpio? I'd say it's a bad idea to depend on this as
> (AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
> contained in it.
>

No, this is what we get from the LINEINFO ioctl(). I only use debugfs
for triggering dummy interrupts.

> I'd weaken the commit log a bit to not claim that the commit broke
> libgpiod but that it only made the inconsistency visible. After all this
> commit didn't "change the default direction of dummy lines to output",
> they were an output already before.
>

Well it did break the tests you know. ;)

I guess no harm in rewording the message.

Bart
Uwe Kleine-König Nov. 9, 2018, 1:10 p.m. UTC | #5
Hello,

On Fri, Nov 09, 2018 at 01:24:36PM +0100, Bartosz Golaszewski wrote:
> pt., 9 lis 2018 o 12:54 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> > On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> > > czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > > > beginning") fixed an existing issue but broke libgpiod tests by
> > > > > changing the default direction of dummy lines to output.
> > > >
> > > > The indicated commit only changed what was shown in debugfs, but didn't
> > > > touch the actual direction of a GPIO, doesn't it? If someone called
> > > > gpiod_get_direction before it would have returned "output", too, unless
> > > > I miss something.
> > > >
> > >
> > > This commit (3edfb7bd76bd) sets the correct direction of the line by
> > > actually calling get_direction() instead of assuming input if
> > > direction_input is not NULL. It just so happened that previously the
> > > default direction of gpio-mockup lines was output but it would be
> > > displayed as input due to this inconsistency.
> >
> > Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
> > the direction of a gpio? I'd say it's a bad idea to depend on this as
> > (AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
> > contained in it.
> 
> No, this is what we get from the LINEINFO ioctl(). I only use debugfs
> for triggering dummy interrupts.

A right, gpio_ioctl uses these flags, too.

> > I'd weaken the commit log a bit to not claim that the commit broke
> > libgpiod but that it only made the inconsistency visible. After all this
> > commit didn't "change the default direction of dummy lines to output",
> > they were an output already before.
> 
> Well it did break the tests you know. ;)

Which test failed exactly?

Best regards
Uwe
Bartosz Golaszewski Nov. 9, 2018, 1:53 p.m. UTC | #6
pt., 9 lis 2018 o 14:10 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> Hello,
>
> On Fri, Nov 09, 2018 at 01:24:36PM +0100, Bartosz Golaszewski wrote:
> > pt., 9 lis 2018 o 12:54 Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> > > > czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > > > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > > > > beginning") fixed an existing issue but broke libgpiod tests by
> > > > > > changing the default direction of dummy lines to output.
> > > > >
> > > > > The indicated commit only changed what was shown in debugfs, but didn't
> > > > > touch the actual direction of a GPIO, doesn't it? If someone called
> > > > > gpiod_get_direction before it would have returned "output", too, unless
> > > > > I miss something.
> > > > >
> > > >
> > > > This commit (3edfb7bd76bd) sets the correct direction of the line by
> > > > actually calling get_direction() instead of assuming input if
> > > > direction_input is not NULL. It just so happened that previously the
> > > > default direction of gpio-mockup lines was output but it would be
> > > > displayed as input due to this inconsistency.
> > >
> > > Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
> > > the direction of a gpio? I'd say it's a bad idea to depend on this as
> > > (AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
> > > contained in it.
> >
> > No, this is what we get from the LINEINFO ioctl(). I only use debugfs
> > for triggering dummy interrupts.
>
> A right, gpio_ioctl uses these flags, too.
>
> > > I'd weaken the commit log a bit to not claim that the commit broke
> > > libgpiod but that it only made the inconsistency visible. After all this
> > > commit didn't "change the default direction of dummy lines to output",
> > > they were an output already before.
> >
> > Well it did break the tests you know. ;)
>
> Which test failed exactly?
>

All gpioinfo tests that check the output of this command and expect to
see input as line direction.

Bart
Uwe Kleine-König Nov. 9, 2018, 2:39 p.m. UTC | #7
On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> >
> > Hello,
> >
> > On Fri, Nov 09, 2018 at 01:24:36PM +0100, Bartosz Golaszewski wrote:
> > > pt., 9 lis 2018 o 12:54 Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> > > > > czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> > > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > > > > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > > > > > beginning") fixed an existing issue but broke libgpiod tests by
> > > > > > > changing the default direction of dummy lines to output.
> > > > > >
> > > > > > The indicated commit only changed what was shown in debugfs, but didn't
> > > > > > touch the actual direction of a GPIO, doesn't it? If someone called
> > > > > > gpiod_get_direction before it would have returned "output", too, unless
> > > > > > I miss something.
> > > > > >
> > > > >
> > > > > This commit (3edfb7bd76bd) sets the correct direction of the line by
> > > > > actually calling get_direction() instead of assuming input if
> > > > > direction_input is not NULL. It just so happened that previously the
> > > > > default direction of gpio-mockup lines was output but it would be
> > > > > displayed as input due to this inconsistency.
> > > >
> > > > Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
> > > > the direction of a gpio? I'd say it's a bad idea to depend on this as
> > > > (AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
> > > > contained in it.
> > >
> > > No, this is what we get from the LINEINFO ioctl(). I only use debugfs
> > > for triggering dummy interrupts.
> >
> > A right, gpio_ioctl uses these flags, too.
> >
> > > > I'd weaken the commit log a bit to not claim that the commit broke
> > > > libgpiod but that it only made the inconsistency visible. After all this
> > > > commit didn't "change the default direction of dummy lines to output",
> > > > they were an output already before.
> > >
> > > Well it did break the tests you know. ;)
> >
> > Which test failed exactly?
> >
> 
> All gpioinfo tests that check the output of this command and expect to
> see input as line direction.

This wasn't the answer I expected. The background of my question was:
This failing test seems to expect that a given GPIO is an input. If that
expectation already exists after the gpio is only requested, then the
test is broken and a fix is necessary there.

Best regards
Uwe
Bartosz Golaszewski Nov. 9, 2018, 3:24 p.m. UTC | #8
pt., 9 lis 2018 o 15:39 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> > pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> napisał(a):
> > >
> > > Hello,
> > >
> > > On Fri, Nov 09, 2018 at 01:24:36PM +0100, Bartosz Golaszewski wrote:
> > > > pt., 9 lis 2018 o 12:54 Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > On Fri, Nov 09, 2018 at 12:13:44PM +0100, Bartosz Golaszewski wrote:
> > > > > > czw., 8 lis 2018 o 21:35 Uwe Kleine-König
> > > > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > > > On Thu, Nov 08, 2018 at 05:52:53PM +0100, Bartosz Golaszewski wrote:
> > > > > > > > Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> > > > > > > > beginning") fixed an existing issue but broke libgpiod tests by
> > > > > > > > changing the default direction of dummy lines to output.
> > > > > > >
> > > > > > > The indicated commit only changed what was shown in debugfs, but didn't
> > > > > > > touch the actual direction of a GPIO, doesn't it? If someone called
> > > > > > > gpiod_get_direction before it would have returned "output", too, unless
> > > > > > > I miss something.
> > > > > > >
> > > > > >
> > > > > > This commit (3edfb7bd76bd) sets the correct direction of the line by
> > > > > > actually calling get_direction() instead of assuming input if
> > > > > > direction_input is not NULL. It just so happened that previously the
> > > > > > default direction of gpio-mockup lines was output but it would be
> > > > > > displayed as input due to this inconsistency.
> > > > >
> > > > > Does the test that fails since 3edfb7bd76bd use debugfs/gpio to find out
> > > > > the direction of a gpio? I'd say it's a bad idea to depend on this as
> > > > > (AFAICT) debugfs is only a debug aid and you shouldn't depend on stuff
> > > > > contained in it.
> > > >
> > > > No, this is what we get from the LINEINFO ioctl(). I only use debugfs
> > > > for triggering dummy interrupts.
> > >
> > > A right, gpio_ioctl uses these flags, too.
> > >
> > > > > I'd weaken the commit log a bit to not claim that the commit broke
> > > > > libgpiod but that it only made the inconsistency visible. After all this
> > > > > commit didn't "change the default direction of dummy lines to output",
> > > > > they were an output already before.
> > > >
> > > > Well it did break the tests you know. ;)
> > >
> > > Which test failed exactly?
> > >
> >
> > All gpioinfo tests that check the output of this command and expect to
> > see input as line direction.
>
> This wasn't the answer I expected. The background of my question was:
> This failing test seems to expect that a given GPIO is an input. If that
> expectation already exists after the gpio is only requested, then the
> test is broken and a fix is necessary there.
>

The test is only expected to work with gpio-mockup which is a dummy
testing module. I believe its behavior should be as deterministic as
possible to the point where newly created chips always have the same
direction.

Bart
Uwe Kleine-König Nov. 9, 2018, 5:03 p.m. UTC | #9
Hello Bartosz,

On Fri, Nov 09, 2018 at 04:24:10PM +0100, Bartosz Golaszewski wrote:
> pt., 9 lis 2018 o 15:39 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> > On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> > > pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > Which test failed exactly?
> > >
> > > All gpioinfo tests that check the output of this command and expect to
> > > see input as line direction.
> >
> > This wasn't the answer I expected. The background of my question was:
> > This failing test seems to expect that a given GPIO is an input. If that
> > expectation already exists after the gpio is only requested, then the
> > test is broken and a fix is necessary there.
> 
> The test is only expected to work with gpio-mockup which is a dummy
> testing module. I believe its behavior should be as deterministic as
> possible to the point where newly created chips always have the same
> direction.

Given that the initial direction of a GPIO isn't fixed in my eyes the
test should be able to cope with both possibilities. I'd say it's a bug
in the test if it doesn't.

Best regards
Uwe
Bartosz Golaszewski Nov. 9, 2018, 5:23 p.m. UTC | #10
pt., 9 lis 2018 o 18:03 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> Hello Bartosz,
>
> On Fri, Nov 09, 2018 at 04:24:10PM +0100, Bartosz Golaszewski wrote:
> > pt., 9 lis 2018 o 15:39 Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> > > > pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > Which test failed exactly?
> > > >
> > > > All gpioinfo tests that check the output of this command and expect to
> > > > see input as line direction.
> > >
> > > This wasn't the answer I expected. The background of my question was:
> > > This failing test seems to expect that a given GPIO is an input. If that
> > > expectation already exists after the gpio is only requested, then the
> > > test is broken and a fix is necessary there.
> >
> > The test is only expected to work with gpio-mockup which is a dummy
> > testing module. I believe its behavior should be as deterministic as
> > possible to the point where newly created chips always have the same
> > direction.
>
> Given that the initial direction of a GPIO isn't fixed in my eyes the
> test should be able to cope with both possibilities. I'd say it's a bug
> in the test if it doesn't.
>

As I said before: it is and should be fixed in this specific case.
This isn't real hardware.

Bart
Uwe Kleine-König Nov. 11, 2018, 4:59 p.m. UTC | #11
On Fri, Nov 09, 2018 at 06:23:01PM +0100, Bartosz Golaszewski wrote:
> pt., 9 lis 2018 o 18:03 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> >
> > Hello Bartosz,
> >
> > On Fri, Nov 09, 2018 at 04:24:10PM +0100, Bartosz Golaszewski wrote:
> > > pt., 9 lis 2018 o 15:39 Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> > > > > pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> > > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > > Which test failed exactly?
> > > > >
> > > > > All gpioinfo tests that check the output of this command and expect to
> > > > > see input as line direction.
> > > >
> > > > This wasn't the answer I expected. The background of my question was:
> > > > This failing test seems to expect that a given GPIO is an input. If that
> > > > expectation already exists after the gpio is only requested, then the
> > > > test is broken and a fix is necessary there.
> > >
> > > The test is only expected to work with gpio-mockup which is a dummy
> > > testing module. I believe its behavior should be as deterministic as
> > > possible to the point where newly created chips always have the same
> > > direction.
> >
> > Given that the initial direction of a GPIO isn't fixed in my eyes the
> > test should be able to cope with both possibilities. I'd say it's a bug
> > in the test if it doesn't.
> >
> 
> As I said before: it is and should be fixed in this specific case.
> This isn't real hardware.

Not sure if we agree here yet. What do you want to fix? The driver or
the test?

In my eyes test driven development is great. But if something breaks
because the test is wrong, please don't "fix" the system to repair the
test, but modify the test to be able to handle reality.

Best regards
Uwe
Bartosz Golaszewski Nov. 12, 2018, 2:14 p.m. UTC | #12
niedz., 11 lis 2018 o 17:59 Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> napisał(a):
>
> On Fri, Nov 09, 2018 at 06:23:01PM +0100, Bartosz Golaszewski wrote:
> > pt., 9 lis 2018 o 18:03 Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> napisał(a):
> > >
> > > Hello Bartosz,
> > >
> > > On Fri, Nov 09, 2018 at 04:24:10PM +0100, Bartosz Golaszewski wrote:
> > > > pt., 9 lis 2018 o 15:39 Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > On Fri, Nov 09, 2018 at 02:53:16PM +0100, Bartosz Golaszewski wrote:
> > > > > > pt., 9 lis 2018 o 14:10 Uwe Kleine-König
> > > > > > <u.kleine-koenig@pengutronix.de> napisał(a):
> > > > > > > Which test failed exactly?
> > > > > >
> > > > > > All gpioinfo tests that check the output of this command and expect to
> > > > > > see input as line direction.
> > > > >
> > > > > This wasn't the answer I expected. The background of my question was:
> > > > > This failing test seems to expect that a given GPIO is an input. If that
> > > > > expectation already exists after the gpio is only requested, then the
> > > > > test is broken and a fix is necessary there.
> > > >
> > > > The test is only expected to work with gpio-mockup which is a dummy
> > > > testing module. I believe its behavior should be as deterministic as
> > > > possible to the point where newly created chips always have the same
> > > > direction.
> > >
> > > Given that the initial direction of a GPIO isn't fixed in my eyes the
> > > test should be able to cope with both possibilities. I'd say it's a bug
> > > in the test if it doesn't.
> > >
> >
> > As I said before: it is and should be fixed in this specific case.
> > This isn't real hardware.
>
> Not sure if we agree here yet. What do you want to fix? The driver or
> the test?
>
> In my eyes test driven development is great. But if something breaks
> because the test is wrong, please don't "fix" the system to repair the
> test, but modify the test to be able to handle reality.
>

No, we don't have an agreement. You think I should fix the test, I
think the dummy driver should continue behaving like before.

Given that there's no real hardware behind, the direction of newly
created dummy lines has always been deterministic - input. Certain
tests have been relying on it. I want to keep on doing it. There's no
harm. It's not broken logic as the very purpose of this module is to
allow for easy testing of the UAPI.

So unless something *else* is wrong with this patch, I intend to push
it upstream.

Best regards,
Bartosz Golaszewski
Uwe Kleine-König Nov. 13, 2018, 6:57 a.m. UTC | #13
Hello Bartosz,

On Mon, Nov 12, 2018 at 03:14:31PM +0100, Bartosz Golaszewski wrote:
> > > As I said before: it is and should be fixed in this specific case.
> > > This isn't real hardware.

Not being "real hardware" is hardly an argument that matters here.

> > Not sure if we agree here yet. What do you want to fix? The driver or
> > the test?
> >
> > In my eyes test driven development is great. But if something breaks
> > because the test is wrong, please don't "fix" the system to repair the
> > test, but modify the test to be able to handle reality.
> >
> 
> No, we don't have an agreement. You think I should fix the test, I
> think the dummy driver should continue behaving like before.

It's arguable if after your patch the driver behaves as before. IMHO the
initial direction was output from the start before. This was just
shadowed by the an inconsitency that was fixed some time ago.

> Given that there's no real hardware behind, the direction of newly
> created dummy lines has always been deterministic - input. Certain
> tests have been relying on it. I want to keep on doing it. There's no
> harm. It's not broken logic as the very purpose of this module is to
> allow for easy testing of the UAPI.
> 
> So unless something *else* is wrong with this patch, I intend to push
> it upstream.

Note I don't oppose to the patch as is. Just the motiviation is wrong
and I'd do both: Modify the mockup driver to start with direction=input
and modify the tests to not expect this.

Best regards
Uwe
Linus Walleij Nov. 16, 2018, 9:38 p.m. UTC | #14
On Thu, Nov 8, 2018 at 5:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Commit 3edfb7bd76bd ("gpiolib: Show correct direction from the
> beginning") fixed an existing issue but broke libgpiod tests by
> changing the default direction of dummy lines to output.
>
> We don't break user-space so make gpio-mockup behave as before.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

Patch applied for fixes.

I browsed the debate here. I think this should be applied for
fixes, then if the test should be changed, we can do that for
devel in that case.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 8269cffc2967..6a50f9f59c90 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -35,8 +35,8 @@ 
 #define gpio_mockup_err(...)	pr_err(GPIO_MOCKUP_NAME ": " __VA_ARGS__)
 
 enum {
-	GPIO_MOCKUP_DIR_OUT = 0,
-	GPIO_MOCKUP_DIR_IN = 1,
+	GPIO_MOCKUP_DIR_IN = 0,
+	GPIO_MOCKUP_DIR_OUT = 1,
 };
 
 /*
@@ -131,7 +131,7 @@  static int gpio_mockup_get_direction(struct gpio_chip *gc, unsigned int offset)
 {
 	struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
 
-	return chip->lines[offset].dir;
+	return !chip->lines[offset].dir;
 }
 
 static int gpio_mockup_to_irq(struct gpio_chip *gc, unsigned int offset)