[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_Jsq+V0oAdVCaW+S12CUa4grCJhZD8OGDeu=0ohcGgxOkPVg@mail.gmail.com>
Date: Fri, 1 Nov 2019 08:46:06 -0500
From: Rob Herring <robh+dt@...nel.org>
To: Peter Ujfalusi <peter.ujfalusi@...com>
Cc: Mark Brown <broonie@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Tero Kristo <t-kristo@...com>,
Maxime Ripard <mripard@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
devicetree@...r.kernel.org
Subject: Re: [RFC v2 0/2] gpio: Support for shared GPIO lines on boards
On Thu, Oct 31, 2019 at 3:00 AM Peter Ujfalusi <peter.ujfalusi@...com> wrote:
>
>
>
> On 30/10/2019 20.49, Rob Herring wrote:
> > On Wed, Oct 30, 2019 at 9:30 AM Peter Ujfalusi <peter.ujfalusi@...com> wrote:
> >>
> >>
> >>
> >> On 30/10/2019 16.17, Mark Brown wrote:
> >>> On Wed, Oct 30, 2019 at 03:32:09PM +0200, Peter Ujfalusi wrote:
> >>>> On 30/10/2019 15.12, Rob Herring wrote:
> >>>
> >>>>> Why can't we just add a shared flag like we have for interrupts?
> >>>>> Effectively, we have that for resets too, it's just hardcoded in the
> >>>>> the drivers.
> >>>
> >>>> This would be kind of the same thing what the
> >>>> GPIOD_FLAGS_BIT_NONEXCLUSIVE does, which was a quick workaround for
> >>>> fixed-regulators afaik.
> >>>
> >>> The theory with that was that any usage of this would need the
> >>> higher level code using the GPIO to cooperate so they didn't step
> >>> on each other's toes so the GPIO code should just punt to it.
> >>
> >> But from the client driver point of view a GPIO is still GPIO and if the
> >> components are unrelated then it is hard to patch things together from
> >> the top.
> >
> > You can't escape a driver being aware. If a driver depends on that
> > GPIO to actually be set to states the driver says, then it can't be
> > guaranteed to work. For example, maybe the driver assumes the device
> > is in reset state after toggling reset and doesn't work if not in
> > reset state. The driver has to be aware no matter what you do in DT.
>
> That's true for some device, but it is also true that some can not
> tolerate being reset without them knowing it.
You mean a reset when the driver is not loaded would not work? How
could that ever work? I don't think you can have any reset control in
the drivers in that case.
> If all users of the shared GPIO have full control over it then they can
> just toggle it whatever way they want. How would a regulator, codec,
> amplifier would negotiate on what to do with the shared GPIO?
>
> Another not uncommon setup is when the two components needs different level:
> C1: ENABLE is high active
> C2: RESET is high active
>
> To enable C1, the GPIO should be high. To enable C2 the GPIO must be low.
> In the board one of the branch of the shared GPIO needs (and have) a
> logic inverter.
>
> If they both control the same GPIO then they must have requested it with
> different GPIO_ACTIVE_ since the drivers are written according to chip
> spec, so C1 sets the GPIO to 1, C2 sets it to 0, the inversion for one
> of them must happen in gpio core, right?
No, drivers are written to set the state to active/inactive. The DT
GPIO_ACTIVE_ flags can depend on an inverter being present (BTW, there
was a recent attempt to do an inverter binding).
> It should be possible to add pass-through mode for gpio-shared so that
> all requests would propagate to the root GPIO if that's what needed for
> some setups.
>
> That way the gpio-shared would nicely handle the GPIO inversions, would
> be able to handle cases to avoid unwanted reset/enable of components or
> allow components to be ninja-reset.
What does ninja-reset mean?
> I think it would be possible to add gpiod_is_shared(struct gpio_desc
> *desc) so users can check if the GPIO is shared - it would only return
> true if the gpio-shared is not in pass-through mode so they can know
> that the state they see on their gpio desc is not necessary matching
> with reality.
> Probably another gpiod_shared_get_root_value() to fetch the root's state?
>
> I intentionally not returning that in the driver as clients might skip a
> gpio_set_value() seeing that the GPIO line is already in a state they
> would want it, but that would not register their needs for the level.
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Powered by blists - more mailing lists