Re: [RFC/PATCH] extcon: otg_gpio: add driver for USB OTG port controlled by GPIO(s)

[David Cohen <david.a.cohen@linux.intel.com>]

Hi,

Adding Mika.

On Tue, Feb 17, 2015 at 01:25:00PM -0600, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Feb 17, 2015 at 11:18:44AM -0800, David Cohen wrote:
> > > > > > (3) Platform has 2 USB controllers connected to same port: one for
> > > > > >     device and one for host role. D+/- are switched between phys
> > > > > >     by GPIO.
> > > > > 
> > > > > so you have discrete mux with a GPIO select signal, like below ?
> > > > > 
> > > > > 
> > > > >  .-------.----------------.  .--------.
> > > > >  |       |                |  |        |      D+
> > > > >  |       |                |  |        |<-------------.
> > > > >  |       |                |  |        |              |
> > > > >  |       |    USB Host    -->|    P   |              |
> > > > >  |       |                |  |    H   |              |
> > > > >  |       |                |  |    Y   |    D-        |
> > > > >  |       '----------------'  |    0   |<--------.    |
> > > > >  |                       |   |        |         |    |
> > > > >  |                       |   '--------'      .--------.  D+
> > > > >  |                       |                   |        |------>
> > > > >  |       SOC        GPIO | ----------------->|        |
> > > > >  |                       |                   |   MUX  |
> > > > >  |                       |                   |        |------>
> > > > >  |                       |   .--------.      '--------'  D-
> > > > >  |       .----------------.  |        |   D-  |      |
> > > > >  |       |                |  |    P   |<------'      |
> > > > >  |       |                |  |    H   |              |
> > > > >  |       |                |  |    Y   |              |
> > > > >  |       |   USB Device   -->|    1   |              |
> > > > >  |       |                |  |        |      D+      |
> > > > >  |       |                |  |        |<-------------'
> > > > >  |       |                |  |        |
> > > > >  '-------'----------------'  '--------'
> > > > 
> > > > Nice ASCII pic :)
> > > 
> > > asciio ftw \o/
> > > 
> > > > Yes, that's the case.
> > > 
> > > alright
> > > 
> > > > > I have been on and off about writing a pinctrl-gpio.c driver which would
> > > > > allow us to hide this detail from users. It shouldn't really matter
> > > > > which modes are available behind the mux, but we should be able to tell
> > > > > the mux to go into mode 0 or mode 1 by toggling its select signal. And
> > > > > it shouldn't really matter that we have a GPIO pin. The problem is: I
> > > > > don't really know if pinctrl would be able to handle discrete muxes.
> > > > > 
> > > > > Adding Linus W to ask. Linus, what do you think ? Should we have a
> > > > > pinctrl-gpio.c for such cases ? In TI we too have quite a few boards
> > > > > which different modes hidden behind discrete muxes.
> > > > 
> > > > An input from Linus would fine in this case.
> > > > 
> > > > > 
> > > > > > As per initial version, this driver has the duty to control whether
> > > > > > USB-Host cable is plugged in or not:
> > > > > >  - If yes, OTG port is configured for host role
> > > > > >  - If no, by standard, the OTG port is configured for device role
> > > > > 
> > > > > correct, this default-B is mandated by OTG spec anyway.
> > > > > 
> > > > > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > > > > ---
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > Some Intel Bay Trail boards have an unusual way to handle the USB OTG port:
> > > > > >  - The USB ID pin is connected directly to GPIO on SoC
> > > > > >  - When in host role, VBUS is provided by enabling a GPIO
> > > > > >  - Device and host roles are supported by 2 independent controllers with D+/-
> > > > > >    pins from port switched between different phys according a GPIO level.
> > > > > > 
> > > > > > The ACPI table describes this USB port as a (virtual) device with all the
> > > > > > necessary GPIOs. This driver implements support to this virtual device as an
> > > > > > extcon class driver. All drivers that depend on the USB OTG port state (USB phy,
> > > > > > PMIC, charge detection) will listen to extcon events.
> > > > > 
> > > > > Right I think you're almost there, but I still think that this needs to
> > > > > be a bit broken down. First, we need some generic DRD library under
> > > > > drivers/usb/common, and that should be the one listening to extcon cable
> > > > > events. But your extcon driver should be doing only that: checking which
> > > > > cable was attached, it shouldn't be doing the switch by itself. That
> > > > > should be part of the DRD library.
> > > > > 
> > > > > That DRD library would also be the one enabling the (optional) VBUS
> > > > > regulator.
> > > > > 
> > > > > George Cherian tried to implement a generic DRD library but I think
> > > > > there are still too many changes happening on usbcore and udc-core. Most
> > > > > of the pieces are already there (usb_del_hcd() and usb_del_gadget_udc()
> > > > > know how to properly unload an hcd/udc), but there are details missing,
> > > > > no doubt.
> > > > > 
> > > > > If we can find a way to broadcast (probably not the best term, but
> > > > > whatever) a "Hey ID pin was just grounded" message, we can get things
> > > > > working.
> > > > > 
> > > > > That message, btw, shouldn't really depend on extcon-gpio alone because
> > > > > other platforms might use non-gpio methods to verify ID pin level. In
> > > > > any case, we need to have generic ID_PIN_LOW and ID_PIN_HIGH messages
> > > > > that a generic DRD library can listen to and load/unload the correct
> > > > > drivers by means of usb_{add,del}_{hcd,gadget_udc}().
> > > > 
> > > > IMHO extcon is the correct way to broadcast it, as long as we define a
> > > > standard for the cable names. E.g. DRD library could listen to
> > > > "USB-HOST" cable state. Then it doesn't matter how ID pin is grounded,
> > > > it just matters that whoever is controlling it broadcast via this cable.
> > > 
> > > right, the likelyhood that someone would not use a GPIO is also quite
> > > minimal and for such cases, the controller would likely switch roles
> > > automatically (like with MUSB).
> > > 
> > > > > With that in mind, I think you can use extcon-gpio.c for your purposes
> > > > > if the other pieces can be fullfilled by regulator and pinctrl.
> > > > 
> > > > In my case we have all gpios listed in a single ACPI device. In order to
> > > > be backwards compatible with products already on market, we'd need
> > > > something like a single mfd to register platform devices for this
> > > > smaller pieces (extcon gpio, possible pintrl gpio, maybe vbus as regulator??).
> > > 
> > > correct.
> > 
> > Getting back to this case :)
> > Guess I need to get back my words.
> > 
> > extcon-gpio.c cannot work out-of-the-box with my case. There is no clean
> > way to get the GPIO given to this device via ACPI and refer it to another
> > device (i.e. extcon-gpio).
> 
> add what's missing ?
> 
> > Here's my scenario:
> > This platform has only one ACPI device representing the USB port with 3
> > gpios controlling it. As GPIO consumer, there is no clean interface
> > where I could get a GPIO descriptor via ACPI without requesting it.
> > After request it, I cannot give it to extcon-gpio.c. Same would happen
> > for a possible pinctrl gpio and regulator controller by gpio.
> > 
> > So my choices:
> > 1) request GPIO locally, give it to other drivers and somehow inform
> > them they should not request, but just to handle it (ugly)
> > 
> > 2) implement a way to pass this GPIO resource to another device without
> > requesting locally
> > 
> > 3) stick with this driver fully handling the GPIOs which control this
> > virtual "USB OTG port" device
> 
> 4) grab gpio via ACPI, gpio_free() it, pass to this driver. Would that
> work ?

That works. But I feel it'd be same as 2. In case we don't want to
implement 2, the same reasons would apply to not implement 4.

Linus, Mika, would you have any thoughts about case 4?

Br, David