Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Rob Herring <robh+dt@kernel.org>
Cc: Harish Jenny K N <harish_kandiga@mentor.com>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Balasubramani Vivekanandan 
	<balasubramani_vivekanandan@mentor.com>
Subject: Re: [PATCH V4 2/2] gpio: inverter: document the inverter bindings
Date: Sat, 10 Aug 2019 10:51:43 +0200
Message-ID: <CACRpkdbmyc9LsJ2xiX=zAQR9FZ9dmwu-nPrNbt1Tgud9+rBGpw@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqLp___2O-naU+2PPQy0QmJX6+aN3hByz-OB9+qFvWgN9Q@mail.gmail.com>

On Fri, Aug 9, 2019 at 4:08 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Mon, Aug 5, 2019 at 5:15 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> > There is some level of ambition here which is inherently a bit fuzzy
> > around the edges. ("How long is the coast of Britain?" comes to mind.)
> >
> > Surely the intention of device tree is not to recreate the schematic
> > in all detail. What we want is a model of the hardware that will
> > suffice for the operating system usecases.
> >
> > But sometimes the DTS files will become confusing: why is this
> > component using GPIO_ACTIVE_LOW when another system
> > doesn't have that flag? If there is an explicit inverter, the
> > DTS gets more readable for a human.
> >
> > But arguable that is case for adding inverters as syntactic
> > sugar in the DTS compiler instead...
>
> If you really want something more explicit, then add a new GPIO
> 'inverted' flag. Then a device can always have the same HIGH/LOW flag.
> That also solves the abstract it for userspace problem.

I think there are some intricate ontologies at work here.

Consider this example: a GPIO is controlling a chip select
regulator, say Acme Foo. The chip select
has a pin named CSN. We know from convention that the
"N" at the end of that pin name means "negative" i.e. active
low, and that is how the electronics engineers think about
that chip select line: it activates the IC when
the line goes low.

The regulator subsystem and I think all subsystems in the
Linux kernel say the consumer pin should be named and
tagged after the datsheet of the regulator.

So it has for example:

foo {
    compatible = "acme,foo";
    cs-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
};

(It would be inappropriate to name it "csn-gpios" since
we have an established flag for active low. But it is another
of these syntactic choices where people likely do mistakes.)

I think it would be appropriate for the DT binding to say
that this flag must always be GPIO_ACTIVE_LOW since
the bindings are seen from the component point of view,
and thus this is always active low.

It would even be reasonable for a yaml schema to enfore
this, if it could. It is defined as active low after all.

Now if someone adds an inverter on that line between
gpio0 and Acme Foo it looks like this:

foo {
    compatible = "acme,foo";
    cs-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
};

And now we get cognitive dissonance or whatever I should
call it: someone reading this DTS sheet and the data
sheet for the component Acme Foo to troubleshoot
this will be confused: this component has CS active
low and still it is specified as active high? Unless they
also look at the schematic or the board and find the
inverter things are pretty muddy and they will likely curse
and solve the situation with the usual trial-and-error,
inserting some random cursewords as a comment.

With an intermediate inverter node, the cs-gpios
can go back to GPIO_ACTIVE_LOW and follow
the bindings:

inv0: inverter {
    compatible = "gpio-inverter";
    gpio-controller;
    #gpio-cells = <1>;
    inverted-gpios = <&gpio0 6 GPIO_ACTIVE_HIGH>;
};

foo {
    compatible = "acme,foo";
    cs-gpios = <&inv0 0 GPIO_ACTIVE_LOW>;
};

And now Acme Foo bindings can keep enforcing cs-gpios
to always be tagged GPIO_ACTIVE_LOW.

Yours,
Linus Walleij

  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 [this message]
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
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='CACRpkdbmyc9LsJ2xiX=zAQR9FZ9dmwu-nPrNbt1Tgud9+rBGpw@mail.gmail.com' \
    --to=linus.walleij@linaro.org \
    --cc=balasubramani_vivekanandan@mentor.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=devicetree@vger.kernel.org \
    --cc=harish_kandiga@mentor.com \
    --cc=linux-gpio@vger.kernel.org \
    --cc=mark.rutland@arm.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

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