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

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

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

On Sat, Sep 24, 2016 at 12:05:49AM +0300, Maxime Ripard wrote:
> On Fri, Sep 23, 2016 at 03:22:53PM +0200, Linus Walleij wrote:
> > > 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.
> 
> My understanding was that GPIO and pinctrl were pretty much
> orthogonal, a pinctrl property would set up the muxing, and mark the
> pin as in use, while the GPIO property would just say what we use the
> pin for. In a way, that was similar to what any other controller would
> behave. You would mark the pins as exclusive, mux them, and leave the
> controller deal with its pin.
> 
> Anyway, I'll remove those properties from our DTs, and add the .strict
> flag.

So I discussed that with Mark at ELCE.

In order not to break the DT, we looked at the code of pin_request
(which is the one using the strict flag), and went to the conclusion
that it needs to be amended to check the owner based on the device
structure pointer.

Which would need just to add an extra parameter to the pin_request
function, right?

It should be quite easy, because there's basically two users of that
function: pinmux_request_gpio, and pinmux_enable_setting, which in
turn are called by pinmux_request_gpio and pinctrl_select_state,
respectively. Which are exported, and used in a significant number of
callers.

... None of them having access to the struct device
directly. pinctrl_select_state is used in 13 different
drivers. pinctrl_request_gpio by 16 of them, but in a gpio hook
(gpio_request) that do not have the calling device structure.

Which means that in order to avoid removing one property from a DT to
fix an actual bug that can cause real stability issues to Linux, we
end up reworking the gpiolib API and fixing all the users.

Mark, Rob, do you really think this is a reasonable request?
Especially when the feature in question was added more than 2 years
after our driver, leaving us no chance to actually benefit from it
without breaking that ABI?

Thanks,
Maxime

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

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