Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Eugeniu Rosca <roscaeugeniu@gmail.com>
Cc: Eugeniu Rosca <erosca@de.adit-jv.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Harish Jenny K N <harish_kandiga@mentor.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Balasubramani Vivekanandan 
	<balasubramani_vivekanandan@mentor.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Stephen Warren <swarren@nvidia.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Phil Reid <preid@electromag.com.au>,
	Enrico Weigelt <info@metux.net>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
Date: Mon, 7 Oct 2019 10:18:40 +0200
Message-ID: <CAMuHMdViwrqg48t2Pc2JtZKLGzLPy0cVfzcnqctGo9oaDpC9Wg@mail.gmail.com> (raw)
In-Reply-To: <20191005130740.GA22620@x230>

Hi Eugeniu,

On Sat, Oct 5, 2019 at 3:08 PM Eugeniu Rosca <roscaeugeniu@gmail.com> wrote:
> On Fri, Sep 27, 2019 at 11:07:20AM +0200, Geert Uytterhoeven wrote:
> > My standard reply would be: describe the device connected to the GPIO(s)
> > in DT.  The GPIO line polarities are specified in the device's "gpios"
> > properties.

> > Next step would be to use the device from Linux.  For that to work, you
> > need a dedicated driver (for the complex case), or something generic
> > (for the simple case).
> > The latter is not unlike e.g. spidev.  Once you have a generic driver,
> > you can use "driver_override" in sysfs to bind the generic driver to
> > your device.  See e.g. commit 5039563e7c25eccd ("spi: Add
> > driver_override SPI device attribute").
>
> We have passed your suggestions along. Many thanks.
>
> > Currently we don't have a "generic" driver for GPIOs. We do have the
> > GPIO chardev interface, which exports a full gpio_chip.
> > It indeed looks like this "gpio-inverter" could be used as a generic
> > driver.  But it is limited to GPIOs that are inverted, which rules out
> > some use cases.
> >
> > So what about making it more generic, and dropping the "inverter" from
> > its name, and the "inverted" from the "inverted-gpios" property? After
> > all the inversion can be specified by the polarity of the GPIO cells in
> > the "gpios" property, and the GPIO core will take care of it[*]?
> > Which boils down to adding a simple DT interface to my gpio-aggregator
> > ("[PATCH/RFC v2 0/5] gpio: Add GPIO Aggregator Driver",
> >  https://lore.kernel.org/lkml/20190911143858.13024-1-geert+renesas@glider.be/).
> > And now I have realized[*], we probably no longer need the GPIO
> > Forwarder Helper, as there is no need to add inversion on top.
>
> After having a look at the gpio aggregator (and giving it a try on
> R-Car3 H3ULCB), here is how I interpret the above comment:
>
> If there is still a compelling reason for having gpio-inverter, then it
> probably makes sense to strip it from its "inverter" function (hence,
> transforming it into some kind of "repeater") on the basis that the
> inverting function is more of a collateral/secondary feature, rather
> than its primary one. Just like in the case of gpio aggregator, the
> primary function of gpio inverter is to accept a bunch of GPIO lines and
> to expose those via a dedicated gpiochip. I hope this is a proper
> summary of the first point in your comment. In any case, this is the
> understanding I get based on my experiments with both drivers.

Yes, the inverter is basically a "repeater" (or "aggregator", when it has
multiple GPIOs connected), hardcoded to invert.

> What I also infer is that, assuming gpio-inverter will stay (potentially
> renamed and stripped of its non-essential inverting function), the gpio
> aggregator will need to keep its Forwarder Helper (supposed to act as a
> common foundation for both drivers).

What I meant is that if the inverter and aggregator would be combinoed
into a single driver, there would no longer be a need[*] for a separate
helper, and it could be incorporated into the single driver.

[*] The individual helper functions may still be useful for some other
     driver, though.

> The second point which I extract from your comment is that the "gpio
> aggregator" could alternatively acquire the role of "gpio-inverter"
> (hence superseding it) by adding a "simple DT interface". I actually
> tend to like this proposal, since (as said above) both drivers are
> essentially doing the same thing, i.e. they cluster a number of gpio
> lines and expose this cluster as a new gpiochip (keeping the
> reserved/used gpio lines on hold). That looks like a huge overlap in
> the functionalities of the two drivers.

Yes, both drivers are very similar.  The difference lies in how they
acquire the list of GPIO descriptors.

> The only difference which I see is that "gpio-inverter" is getting its
> input from DT and generates the gpiochips at probe time, while
> "gpio aggregator" is getting its input from sysfs and generates the
> gpiochips at runtime, post-probe.

Exactly.

For my virtualization use case, I need to create the list of GPIO
descriptors at run-time, hence the sysfs interface. This is
polarity-agnostic (i.e. the end user needs to care about polarity).

For Harish use case, he needs to describe the list from DT, with
polarity inverted, which can be done by specifying the GPIO_ACTIVE_LOW
flag in the node's"gpios" property.

For your use case, you want to describe the list in DT, with line-names,
and polarity specified.

> So, assuming no objections from Harish and other reviewers, I would be
> very happy to review and test the DT-based gpio inversion functionality
> as part of gpio aggregator. Thanks!

Thanks, adding to my list ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-28  9:30 Harish Jenny K N
2019-07-04  5:01 ` Harish Jenny K N
2019-07-08 22:36 ` Rob Herring
2019-07-09  5:25   ` Harish Jenny K N
2019-07-09 16:08     ` Rob Herring
2019-07-10  8:28       ` Harish Jenny K N
2019-07-17 13:51         ` Harish Jenny K N
2019-07-29 11:07           ` Harish Jenny K N
2019-08-05 11:15         ` Linus Walleij
2019-08-09 14:08           ` Rob Herring
2019-08-10  8:51             ` Linus Walleij
2019-08-19  9:36               ` Harish Jenny K N
2019-08-27  7:47                 ` Harish Jenny K N
2019-08-30  5:21                   ` Harish Jenny K N
2019-09-04  4:58                     ` Harish Jenny K N
2019-09-10  7:47                       ` Rob Herring
2019-09-11 12:52                         ` Harish Jenny K N
2019-09-25 16:51 ` Eugeniu Rosca
2019-09-27  5:52   ` Phil Reid
2019-09-27  9:07   ` Geert Uytterhoeven
2019-10-05 13:07     ` Eugeniu Rosca
2019-10-07  8:18       ` Geert Uytterhoeven [this message]
2019-10-11  4:35         ` Harish Jenny K N
2019-11-12 11:52           ` Harish Jenny K N
2019-11-12 12:19             ` Geert Uytterhoeven
2019-10-04 19:07   ` Stephen Warren
2019-10-05 17:50     ` Eugeniu Rosca
2019-10-07 15:36       ` Stephen Warren
  -- strict thread matches above, loose matches on Subject: below --
2019-06-28  5:20 [PATCH V4 0/2] Add Inverter controller for gpio configuration Harish Jenny K N
2019-06-28  5:20 ` [PATCH V4 2/2] gpio: inverter: document the inverter bindings Harish Jenny K N

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=CAMuHMdViwrqg48t2Pc2JtZKLGzLPy0cVfzcnqctGo9oaDpC9Wg@mail.gmail.com \
    --to=geert@linux-m68k.org \
    --cc=balasubramani_vivekanandan@mentor.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=erosca@de.adit-jv.com \
    --cc=harish_kandiga@mentor.com \
    --cc=info@metux.net \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=preid@electromag.com.au \
    --cc=robh+dt@kernel.org \
    --cc=roscaeugeniu@gmail.com \
    --cc=swarren@nvidia.com \
    --cc=swarren@wwwdotorg.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

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org
	public-inbox-index linux-gpio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git