Re: Requesting as a GPIO a pin already used through pinctrl

[Maxime Ripard <maxime.ripard@free-electrons.com>]

[-- Attachment #1: Type: text/plain, Size: 6376 bytes --]

On Fri, Sep 23, 2016 at 06:24:04PM +0300, Vladimir Zapolskiy wrote:
> On 09/23/2016 04:22 PM, Linus Walleij wrote:
> >On Wed, Sep 21, 2016 at 9:51 PM, Maxime Ripard
> ><maxime.ripard@free-electrons.com> wrote:
> >>On Sun, Sep 18, 2016 at 01:30:24PM +0200, Linus Walleij wrote:
> >
> >>>>I have the feeling that the core should prevent that, making sure that
> >>>>the gpiod_request returns EBUSY in such a case, but I'm not really
> >>>>sure whether it's the case or not, and if it is, where that check is
> >>>>happening.
> >>>
> >>>- Did you try specifying .strict for the pinmux?
> >>>
> >>>- Did you read Documentation/pinctrl.txt, section titled
> >>>  "GPIO mode pitfalls"?
> >>
> >>Sigh. Sorry for that, I should learn to read the documentation. This
> >>is obviously the right thing to do.
> >
> >Guessed so.
> >
> >>However, it does have an unexpected side-effect. On our DT, for the
> >>GPIOs, we also set up a pinctrl node (which seem to be along the lines
> >>of the doc recommandations, section "Drivers needing both pin control
> >>and GPIOs").
> >>
> >>However, when pinctrl_select_default is called by the core, which in
> >>turns ends up calling pinmux_enable_setting, which builds the owner
> >>name using the dev_name. However, when we call gpiod_request, it ends
> >>up in pinmux_request_gpio, which build the owner string using the
> >>pinctrl device name and the pin number.
> >>
> >>This results in a mismatch of owners, and the gpiod_request fails,
> >>while the device really is the same.
> >
> >Yeah, needing both GPIO and pinctrl on the same pin kind of
> >implies that the subsystems are poking at the same hardware and
> >that is !=strict.
> 
> recently I worked on GPIO to pinctrl connection for i.MX SoCs, so
> let me share some details until I forget them while time goes by.
> 
> IMHO i.MX* IOMUX controllers are type A strict pinctrl controllers
> described in Documentation/pinctrl.txt, however practically it won't
> be possible to convert them to the strict type due to the countless
> runtime issues on boards when DTBs are not updated at the same time,
> in other words backward compatibility with the preexisting firmware
> is hardly reachable,

I'm not sure marking an optional, generic, property (pinctrl-0) as
exclusive with an often mandatory, if not optional, usually
binding-specific property (*-gpio) would be considered an ABI
breakage, even more so if it was wrong the whole time, and it prevents
us from fixing an actual bug.

Rob? Any opinion on that one?

> however for any next added i.MX pinctrl drivers (and probably many
> other pinctrl drivers) I would suggest to set the type to strict, at
> least it might be nice to
> 
> a) more precisely describe what is the strict pinctrl controller
>    beast,
> b) ask every pinctrl driver submitter for clear confirmation that
>    their controller is strict or non-strict.
> 
> This should allow to avoid some degree of pain in the future, when
> it accidentally happens that users (e.g. fossilized in DTBs) of
> a strict pinctrl controller abuse the freedom given exclusively
> for non-strict controllers.
> 
> Next are some issues I've observed, due to our specifics we have to
> control pad muxing in runtime between GPIO and functional modes.
> This may imply not only the change of mux setting but as well pin
> config changes, hence limiting the discussion to pin descriptions
> in OF there should be two or more pinctrl tuples (mux function,
> pin config) for the same pin. Usually on practice this case is
> resolved by providing pinctrl-[0-9]+ properties, but this works
> well only if there is no pad ownership transfer, otherwise one of
> two independent users of the same pad resourse will fail to be
> registered during boot time due to a conflict for the resourse.
> When a device is unbound in runtime, a pad resourse is released
> and it can be captured by another device (repeat the procedure to
> restore the original state), and when one of the rivals is active
> another shall not get a chance to grab the pads. I didn't manage
> to find a good description of this possibility in DTS though,
> the best one we came across is adding a pinctrl-* property pointing
> to GPIO mux/config settings of wanted GPIOs right into the
> correspondent GPIO controller device node and set it as non-default
> to avoid boot time conflict. When turn for GPIOs begins in runtime
> the mux and config values for a pinctrl hardware are taken from
> that phandle (by an overly hackish in-kernel pinctrl API extension,
> which accounts that non-"default" pinctrl-names values).
> 
> And here are some conclusions, as I can see often potentially
> dangling pads for GPIOs are described in a hog group (owned by
> pinctrl), for strict controllers there is a conflict between
> the pinctrl and an end-point GPIO user, probably it might be better
> to neutralize this conflict by moving pad groups with GPIO
> functioning pads into exclusive usage (including formal runtime
> ownership) by GPIO controller drivers and their backends in
> pinctrl subsystem. There should be an anchor in DTS, which says
> that a pad is functioning as a GPIO, it is given if a pad
> belongs to a "gpio-controller" device node. Alternatively hog
> groups may be reviewed, all non-GPIO pads placed there are wiped
> out or moved to more appropriate groups and hog group ownership
> is transferred to the class of GPIO controllers, but this is
> a more questionable path.

Basing that on whether the pinctrl controller has gpio-controller is a
pretty good idea.

I think the only we would need then is some kind of callback in the
pintrl driver to query for whether a particular configuration is a
gpio, and we can have some neat handover without reducing the
strictness of the whole driver, and without basing it on the owner
string.

Something like "if the configuration is a gpio, and my pinctrl driver
is also a gpio controller, and that gpiod_request is being called on
that pin, hand over the reference"

We'd probably need some additional check on the refcount too (for
example only the first gpiod_request gets the handover, and the next
one are rejected.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]