All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Alexander Stein <alexander.stein@ew.tq-group.com>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	Marek Vasut <marex@denx.de>, Mark Brown <broonie@kernel.org>
Subject: Re: [RFC PATCH 1/3] dt-bindings: gpio: Add optional ramp-up delay property
Date: Tue, 20 Dec 2022 01:01:23 +0200	[thread overview]
Message-ID: <Y6DtQ7PXPZ809P4C@pendragon.ideasonboard.com> (raw)
In-Reply-To: <CACRpkdZ2G=HUTBMpXJrXeSh3kYgQQc8p8zaJZPL71HWA9362ZA@mail.gmail.com>

On Thu, Dec 15, 2022 at 11:56:57AM +0100, Linus Walleij wrote:
> Hi Laurent,
> 
> thanks for the detailed brief!
> 
> On Tue, Dec 13, 2022 at 12:45 PM Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> 
> > The circuit we're looking at is
> >
> >   +----------+           +-----------+
> >   | SoC      |           |    VCC    |
> >   |          |           |     |     |
> >   |          |           |     _     |
> >   |          |           |    | | R  |
> >   |          |           |    |_|    |
> >   |          |           |     |     |
> >   |      [IOx|-----+-----|EN]--+     |
> >   |          |     |     |           |
> >   |          |     |     | SN65DSI83 |
> >   +----------+    --- C  +-----------+
> >                   ---
> >                    |
> >                    -
> >                   GND
> >
> > The IOx pin is an open-drain output, the board has a 470nF capacitor to
> > ground, and the SN65DSI83 has an internal pull-up off 200kΩ. This gives
> > an RC time constant of 94ms, far from being negligible.
> >
> > The delay is caused by the combination of the open-drain nature of the
> > output (an intrinsic property of the GPIO controller), the pull-up
> > resistor (an intrinsic property of the SN65DSI83) and the capacitor on
> > the line (a property of the board). DT is notoriously bad at modelling
> > this kind of setup.
> 
> Yeah :/
> 
> It's not like we don't model discrete electronics, we do that a lot,
> but as you say, it is really hard to know where to draw the line
> in cases like this.
> 
> > The alternative I proposed, adding a "GPIO delay" DT node to model this,
> > would also offer a centralized solution to the problem, but with
> > additional complexity both at probe time and runtime.
> 
> I have a slight preference for this, as it will be very explicit in the
> device tree and we can just put all the code inside its own file and
> depend on GPIO_OF so other HW description systems do not
> need to include it.
> 
> At the same time it feels a bit overengineered, so maybe just adding
> this delay as in the patch with some strings attached like comments
> and docs is yet the best. It feels like we need some more input to
> reach consensus.
> 
> > The regulator delays model the intrinsic delays when enabling or
> > disabling a regulator, and they should stay. They address a different
> > problem.
> 
> OK right. But someone not knowing exactly what they are doing
> will end up abusing the delay property on the delay line
> also for this delay. The risk of that is lesser with a separate
> delay box.

That may be true, but I think we can also try to catch abuses in
reviews. I would be a bit sad if we made life more difficult (and less
efficient at runtime too) for legitimate users just because we are
worried about abuses.

Another thing I've been thinking about is that we may not always want to
wait for the GPIO delay. Some consumers may not care when the GPIO line
reaches the desired state as long as it eventually does, or maybe they
need to perform multiple operations (such as enabling/disabling
regulators and/or clocks) and only need a synchronization point for a
group of operations. All that would be pretty hard to handle, and maybe
it's a problem we'll look at only when needed (and hopefully never).

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2022-12-19 23:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-12 10:35 [RFC PATCH 0/3] gpiolib: ramp-up delay support Alexander Stein
2022-12-12 10:35 ` [RFC PATCH 1/3] dt-bindings: gpio: Add optional ramp-up delay property Alexander Stein
2022-12-13  9:08   ` Linus Walleij
2022-12-13 11:45     ` Laurent Pinchart
2022-12-15 10:56       ` Linus Walleij
2022-12-19 23:01         ` Laurent Pinchart [this message]
2023-01-03 11:56           ` Alexander Stein
2023-01-03 12:34             ` Laurent Pinchart
2022-12-12 10:35 ` [RFC PATCH 2/3] gpiolib: Add support for optional ramp-up delays Alexander Stein
2022-12-13  9:11   ` Linus Walleij
2022-12-12 10:35 ` [RFC PATCH 3/3] arm64: dts: mba8mx: Add GPIO " Alexander Stein
2022-12-12 12:40 ` [RFC PATCH 0/3] gpiolib: ramp-up delay support Laurent Pinchart
2022-12-13  7:49   ` Alexander Stein
2022-12-13 11:25     ` Laurent Pinchart
2022-12-13 14:20 ` Rob Herring
2022-12-13 15:47   ` 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=Y6DtQ7PXPZ809P4C@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=alexander.stein@ew.tq-group.com \
    --cc=brgl@bgdev.pl \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=marex@denx.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.