linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Alexandre Courbot <gnurou@gmail.com>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>
Subject: Re: Correct meaning of the GPIO active low flag
Date: Sat, 15 Feb 2014 01:20:50 +0100	[thread overview]
Message-ID: <1774231.xdMd4d6ShW@avalon> (raw)
In-Reply-To: <52FEAFC4.1010700@wwwdotorg.org>

Hi Stephen,

On Friday 14 February 2014 17:07:32 Stephen Warren wrote:
> On 02/14/2014 04:48 PM, Laurent Pinchart wrote:
> > On Thursday 13 February 2014 09:49:57 Stephen Warren wrote:
> >> On 02/13/2014 07:43 AM, Laurent Pinchart wrote:
> >>> On Wednesday 12 February 2014 09:50:37 Stephen Warren wrote:
> >>>> On 02/10/2014 04:21 PM, Laurent Pinchart wrote:
> >>>>> On Monday 10 February 2014 16:04:30 Stephen Warren wrote:
> >>>>>> On 02/10/2014 10:52 AM, Laurent Pinchart wrote:
> >>>>>>> On Monday 10 February 2014 09:57:43 Stephen Warren wrote:
> >>>>>>>> On 02/10/2014 09:56 AM, Stephen Warren wrote:
> >>>>>> ...
> >>>>>> 
> >>>>>>>>> I think the flag should represent the physical level of the signal
> >>>>>>>>> on the board at the device pin. I'm pretty sure that's what's most
> >>>>>>>>> consistent with existing DT properties.
> >>>>>>>> 
> >>>>>>>> (That would have to be the GPIO source device, in order to account
> >>>>>>>> for any board-induced inversion)
> >>>>>>> 
> >>>>>>> Would that be the physical level at the GPIO source device output to
> >>>>>>> achieve a high level at the target device input pin, or the physical
> >>>>>>> level at the GPIO source device output to assert the signal at the
> >>>>>>> target device input pin ? The first case wouldn't take the receiver
> >>>>>>> device internal inverter into account while the second case would.
> >>>>>>> In the second case, how should we handle receiver devices that have
> >>>>>>> configurable signal polarities (essentially enabling/disabling the
> >>>>>>> internal inverter from a software-controller configuration) ?
> >>>>>> 
> >>>>>> I would expect the flag to represent the physical level that achieves
> >>>>>> (or represents, for inputs) a logically asserted value at the device.
> >>>>> 
> >>>>> I assume you mean "the physical level at the GPIO controller output".
> >>>> 
> >>>> Yes.
> >>>> 
> >>>>>> I don't think we should make the level flag influence any kind of
> >>>>>> configurable level within the device; that's a separate orthogonal,
> >>>>>> but related, concept. It'd be best if the DT binding for the device
> >>>>>> either (a) provided a separate property to configure that, or (b)
> >>>>>> picked a single one of the configurable values, and documented that
> >>>>>> all DTs should assume that value.
> >>>>> 
> >>>>> Agreed. I've phrased my question incorrectly though.
> >>>>> 
> >>>>> My concern with devices that have configurable input polarities is
> >>>>> that the
> >>>> 
> >>>> s/input/output/ I assume?
> >>> 
> >>> No, I mean input.
> >> 
> >> OK, I guess I was thinking about GPIO inputs then; the same discussion
> >> applies in reverse.
> >> 
> >>> Think about video vertical/horizontal sync inputs, they usually have
> >>> configurable polarities on the receiver side. In that case the physical
> >>> level at the GPIO controller output that achieves a logically asserted
> >>> value at the device depends on how the device is configured at runtime.
> >> 
> >> Sure.
> >> 
> >> I think the GPIO specifier should specify the signal polarity required
> >> to get a logically asserted signal to the device. If the device can be
> >> configured to accept different signal polarities as logically asserted,
> >> then that must indeed be a separate DT property to the GPIO specifier,
> >> since the GPIO specifier's format and semantics are only meaningful (and
> >> parsable/interpretable) to the GPIO controller, and not the GPIO
> >> consumer.
> > 
> > Agreed, but that's not my point (or maybe I've just not understood that
> > you
> > got my point). The small detail that made me concerned in the first place
> > is that, when the device input polarity is configurable, the "logically
> > asserted signal" state becomes dynamic. How do we define the DT GPIO
> > polarity in that case ?
> 
> Pick whichever polarity you want. Write that polarity into the GPIO
> specifier's flags. If there's an inverter on the board between the GPIO
> controller pin and the remote device pin, you need an extra property in
> the device to specify how to program the polarity of the signal at the
> device end too.
> 
> Perhaps the following will make my thoughts clearer:
> 
> http://www.spinics.net/lists/arm-kernel/msg307927.html
> [PATCH 1/2] mfd: palmas: support IRQ inversion at the board level
> 
> http://www.spinics.net/lists/arm-kernel/msg307926.html
> [PATCH 2/2] ARM: tegra: fix Dalmore PMIC IRQ polarity
> 
> > Let me take a (slightly made up) example. Let's assume a chip with a
> > control input signal connected to a GPIO output of an SoC without any
> > inverter on the board. The input polarity is runtime configurable
> > (through I2C for instance). The chip has two modes of operation (USB host
> > or USB function for instance, or ethernet link speed, ...), and for some
> > reason, we need the input to be active high in mode A and active low in
> > mode B. This is handled in the chip device driver that configures the
> > input polarity based on the mode.
> 
> Oh, if you're talking about fiddling around at run-time, then that's
> just something the driver has to deal with internally. In that case,
> let's just make the GPIO active-high in DT. When the driver programs the
> device into whatever mode requires it to be active-low, the driver needs
> to be written to set that GPIO value correctly. I don't think DT has any
> influence on this at all, since DT is about a static setup, whereas your
> use-case is dynamic.

Except that there could be an inverter, which would need to be expressed in 
DT.

I propose wording the documentation as follows.

The GPIO polarity flag should represent the physical level that achieves (or 
represents, for inputs) a logically asserted value at the device. When the 
device signal polarity is dynamically configurable (as opposed to the 
statically configured case where the polarity is set based on DT properties 
only) the flag should bet set to the polarity required by the default 
logically asserted value, and that default logically asserted value should be 
documented in the device DT bindings.

> > The chip DT bindings will have a property that contains a reference to the
> > GPIO connected to the control input, and a flag to set the GPIO polarity.
> > We want that flag to express the physical level at the GPIO source to
> > achieve an asserted level at the chip input. That's active high in mode A
> > and active low in mode B (and would be the opposite if we had an inverter
> > on the board). Now, as the mode of operation is fully dynamic, what value
> > should the DT flag have (assuming there's no default mode from a chip
> > point of view) ?
> > 
> >> Similarly, for inputs to the GPIO controller, the GPIO specifier should
> >> specify the value at the GPIO controller too, and any configuration of
> >> the output polarity of the device should be a separate property of that
> >> device.
> >> 
> >> The same argument applies to IRQs.

-- 
Regards,

Laurent Pinchart


  reply	other threads:[~2014-02-15  0:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-10 14:33 Correct meaning of the GPIO active low flag Laurent Pinchart
2014-02-10 14:50 ` Alexandre Courbot
2014-02-10 15:13   ` Laurent Pinchart
2014-02-10 16:56     ` Stephen Warren
2014-02-10 16:57       ` Stephen Warren
2014-02-10 17:52         ` Laurent Pinchart
2014-02-10 23:04           ` Stephen Warren
2014-02-10 23:21             ` Laurent Pinchart
2014-02-12 16:50               ` Stephen Warren
2014-02-13 14:43                 ` Laurent Pinchart
2014-02-13 16:49                   ` Stephen Warren
2014-02-14 23:48                     ` Laurent Pinchart
2014-02-15  0:07                       ` Stephen Warren
2014-02-15  0:20                         ` Laurent Pinchart [this message]
2014-02-18 17:58                           ` Stephen Warren
2014-02-19  0:19                             ` Laurent Pinchart

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=1774231.xdMd4d6ShW@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=gnurou@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --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
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).