Quoting Doug Anderson (2018-06-28 11:45:30) > Hi, > > On Thu, Jun 28, 2018 at 10:14 AM, Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote: > > Quoting Linus Walleij (2018-06-28 07:25:46) > >> On Fri, Jun 22, 2018 at 8:29 PM Bjorn Andersson > >> <bjorn.andersson@xxxxxxxxxx> wrote: > >> > On Fri 22 Jun 10:58 PDT 2018, Bjorn Andersson wrote: > >> > > On Mon 18 Jun 13:52 PDT 2018, Stephen Boyd wrote: > >> > > > >> > > > We rely on devices to use pinmuxing configurations in DT to select the > >> > > > GPIO function (function 0) if they're going to use the gpio in GPIO > >> > > > mode. Let's simplify things for driver authors by implementing > >> > > > gpio_request_enable() for this pinctrl driver to mux out the GPIO > >> > > > function when the gpio is use from gpiolib. > >> > > > > >> > > > Cc: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > >> > > > >> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > >> (...) > >> > While both patch 2 and 3 are convenient ways to get around the annoyance > >> > of having to specify a pinmux state both patches then ends up relying on > >> > some default pinconf state; which I think is bad. > > > > What default state are we relying on? The reset state of the pins? I'm > > very confused by this statement. These last two patches are making sure > > that when a GPIO is requested as a GPIO or IRQ, it's really in GPIO > > mode. If this code is not in place, then we'll allow drivers to request > > a GPIO but pinmuxing won't be called to actually mux that pin to GPIO > > mode unless they have a DT conf for function = "gpio". That seems > > entirely unexpected and easy to mess up. > > > >> > >> Nothing stops you from setting up a default conf in > >> this callback though. > >> > >> But admittedly this call was added for simpler hardware. > >> > >> > Further more in situations like i2c-qup (downstream), where the pins are > >> > requested as gpios in order to "bitbang" a reset this would mean that > >> > the driver has to counter the convenience; by either switching in the > >> > default pinmux at the end of probe or postponing the gpio_request() to > >> > the invocation of reset and then, after issuing the gpio_release, > >> > switching in the default pinmux explicitly again. > >> > >> That's a bigger problem. If the system is using device and GPIO > >> mode orthogonally, it'd be good to leave like this. > >> > > > > Doesn't that driver need to explicitly mux GPIO mode vs. device mode > > right now? So having gpio_request() do the muxing to GPIO mode and then > > explicit muxing of the pin to device mode would be what we have after > > this patch, while before this patch we would have mux to GPIO, request > > GPIO (nop), mux to device. We saved an explicit pinmux call? > > After reading these replies I'm slightly worried about some of the > stuff Bjorn is worried about too. In Bjorn's example I think that the > "default" state of the pins is probably i2c-mode and that it might > switch to GPIO mode only when it needs to do the special unwedge of > the i2c port. > > So maybe the i2c driver does this: > > 1. "Default" pinmux state of i2c is i2c-mode, "unwedge" pinmux state > is gpio-mode. > > 2. In probe, request the GPIOs for use later in case we need to > unwedge. Don't use them right now since we don't need to unwedge. > Assumes pinmux state stays as "default". > > 3. When you decide you need to unwedge the bus, pinmux to the special > i2c mode and drive the pins you requested in probe. Then pinmux back. > > > ...I think your patch will break this because when you request the > GPIOs you'll have an implicit (and unexpected?) pinmux transition. > > What do you think? > > I could do with some more clarity from Linus in the "Drivers needing both pin control and GPIOs" section of Documentation/driver-api/pinctl.rst but I read that section as stating that the GPIO driver needs to mux the pin as a GPIO by requesting the pinctrl backend to do so, unless the hardware overrides the muxed function selection when the GPIO is used, without involving pinctrl software. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html