| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150218175314.GA25273@psi-dev26.jf.intel.com>
Date: Wed, 18 Feb 2015 09:53:14 -0800
From: David Cohen <david.a.cohen@...ux.intel.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Felipe Balbi <balbi@...com>,
Linus Walleij <linus.walleij@...aro.org>,
myungjoo.ham@...sung.com, cw00.choi@...sung.com,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
baolu.lu@...ux.intel.com,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [RFC/PATCH] extcon: otg_gpio: add driver for USB OTG port
controlled by GPIO(s)
Hi,
On Wed, Feb 18, 2015 at 12:17:06PM +0200, Mika Westerberg wrote:
> On Tue, Feb 17, 2015 at 11:35:23AM -0800, David Cohen wrote:
> > 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@...ux.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?
>
> Isn't this exactly what Heikki suggested in his GPIO forwarding series
> here:
>
> https://lkml.org/lkml/2014/12/18/82
Yes, that's pretty much the case. It's a dead end for case 2 (and 4).
If we split this driver into smaller pieces, we'll have to force kernel
to hack GPIO ownership.
I get back to the original proposal in this case. I can send a second
version addressing all comments I received.
Comments? :)
BR, David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists