devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree <devicetree@vger.kernel.org>,
	Rabin Vincent <rabinv@axis.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Einar Vading <einar.vading@axis.com>,
	Sascha Hauer <kernel@pengutronix.de>
Subject: Re: SV: [PATCH RFC] gpio: new driver for a gpio simulator
Date: Mon, 15 Oct 2018 22:03:31 +0200	[thread overview]
Message-ID: <20181015200331.ux5pb5cohtbjskc6@pengutronix.de> (raw)
In-Reply-To: <CAMRc=McH7taDJXaVTLYZ-1E+hkwxKon6RwjuKzBSgB9LZq7H1w@mail.gmail.com>

On Mon, Oct 15, 2018 at 11:54:22AM +0200, Bartosz Golaszewski wrote:
> pt., 12 paź 2018 o 11:47 Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> napisał(a):
> >
> > On Fri, Oct 12, 2018 at 11:27:18AM +0200, Einar Vading wrote:
> > > On Fri, Oct 12, 2018 at 11:06:12AM +0200, Uwe Kleine-König wrote:
> > > > Would it help to instanciate more than one gpio-simulator?
> > >
> > > Hmm, I don't think that would pose a problem. With DT it would be easy to name
> > > the GPIOs and get then get them by name. What gpiochip they are on should not
> > > matter... I think.
> >
> > The only downside is that you cannot atomically set GPIOs on different
> > chips. I didn't try to use naming, but maybe the gpio framework is good
> > enough that it just works.
>
> If I understand correctly the main difference between gpio-mockup and
> gpio-simulator is that simulating interrupts on input lines would
> happen by changing the value of connected output GPIO lines.

Not only for input lines. What is done with gpio-mockup via debugfs is
done for gpio-simulator with the gpio functions on the other side.

> I started using gpio-mockup for testing of both libgpiod and GPIO user
> API some time ago. I initially thought about doing the exact same
> thing with interrupts but figured that if we want to test the user API
> then we'd better not be actually using it since we won't know where
> the eventual bugs will actually come from - i.e. when testing reading
> line events, let's not be using the output API from the other side,
> but rather something not linked to GPIO in any way - debugfs in this
> case.

I wouldn't take this as a reason to not use GPIO stuff for the "other
side". On the contrary, I'd see it as an advantage as this way you test
setting an output on one side and generating an event on the other side
in a single test. And if it's broken and needs debugging there are the
normal trace mechanisms.

> That being said: I have nothing against extending gpio-mockup with
> this feature - I actually like it. I guess having a single module
> param that would create a second corresponding gpiochip for every
> standard one would be enough? I could have the numbering starting
> right after the standard chips and a special label too. However I
> don't really see a need to have two separate testing drivers for a
> single subsystem. Is there anything in the way mockup is implemented
> that stops you from reusing it?

An advantage of my simulator over gpio-mockup (as mentioned already
earlier) is that both sides are available as GPIOs in the kernel. So I
could connect a rotary-encoder input device to an device that mimics a
rotary-encoder.

> Please note that libgpiod extensively uses gpio-mockup for testing so
> there's now way we're getting rid of it anytime soon.

>From my POV in order of preference, we'd do the following:

a) take gpio-simulator in parallel to gpio-mockup
b) take gpio-simulator and drop gpio-mockup
c) merge the two

I think c) is ugly because it complicates the combined driver which
takes away much of the beauty and simplicity I see in at least the
gpio-simulator. (I don't know the mockup driver good enough to judge
here, but on my few looks it looked more complicated which might be
subjective.)

Also there are a few issues I see in the mockup driver (which are not
implied by the architecture and so are fixable):

 - I think it's racy because there is no locking (ditto for the irq
   simulator).
 - Each write to the event file generates an irq, so there is no way to
   test level sensitive irqs.
 - SPDX header claims GPL-2.0+, MODULE_LICENCE claims GPL-v2 only.
 - The debugfs interface doesn't give complete control over the gpios.

Of course there is no *need* to have two virtual gpio devices, but I
don't see a big reason not to have two anyhow.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

  reply	other threads:[~2018-10-16  3:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-08 10:13 [PATCH RFC] gpio: new driver for a gpio simulator Uwe Kleine-König
2018-10-08 13:08 ` Uwe Kleine-König
2018-10-09 12:51 ` Linus Walleij
2018-10-09 19:11   ` Uwe Kleine-König
2018-10-10 11:47     ` Linus Walleij
2018-10-11  8:16       ` Uwe Kleine-König
2018-10-11  9:49         ` Vincent Whitchurch
2018-10-12  8:02           ` SV: " Einar Vading
2018-10-12  9:06             ` Uwe Kleine-König
2018-10-12  9:27               ` Einar Vading
2018-10-12  9:47                 ` Uwe Kleine-König
2018-10-15  9:54                   ` Bartosz Golaszewski
2018-10-15 20:03                     ` Uwe Kleine-König [this message]
2018-10-18 15:03                       ` Bartosz Golaszewski
2018-10-18 19:16                         ` Uwe Kleine-König
2018-10-30 12:45                           ` Linus Walleij
2018-11-03 21:15                             ` Uwe Kleine-König

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=20181015200331.ux5pb5cohtbjskc6@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=brgl@bgdev.pl \
    --cc=devicetree@vger.kernel.org \
    --cc=einar.vading@axis.com \
    --cc=kernel@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rabinv@axis.com \
    --cc=robh+dt@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 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).