On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote: > Hi, > > On 24-04-17 15:12, Guenter Roeck wrote: > >On 04/24/2017 04:02 AM, Heikki Krogerus wrote: > >>+Guenter > >> > >>On Fri, Apr 21, 2017 at 09:51:50PM +0300, Andy Shevchenko wrote: > >>>+Cc: Heikki. > >>> > >>>He might comment on this. > >> > >>Thanks Andy. > >> > >>>On Fri, Apr 21, 2017 at 4:01 PM, Hans de Goede <[email protected]> wrote: > >>>>Add support for USB TYPE-C cable detection on systems using a > >>>>FUSB302 USB TYPE-C controller. > >>>> > >>>>Signed-off-by: Hans de Goede <[email protected]> > >>>>--- > >>>> drivers/extcon/Kconfig | 8 + > >>>> drivers/extcon/Makefile | 1 + > >>>> drivers/extcon/extcon-fusb302.c | 782 > >>>> ++++++++++++++++++++++++++++++++++++++++ > >>>> 3 files changed, 791 insertions(+) > >>>> create mode 100644 drivers/extcon/extcon-fusb302.c > >> > >>There is now the typec class in linux-next that really needs to be > >>used with all USB Type-C PHYs like fusb302. > >> > >>Unless I'm mistaken, Guenter has also written a driver for fusb302. I > > > >Not me; it was Nathan. > > > >>have not seen it, but if I understood correctly, that driver will > >>register itself with the upcoming tcpm (USB Type-C Port Manager) [1], > >>which in practice would mean we can take properly advantage of the USB > >>PD transceiver on fusb302. > >> > >>Guenter! Can you publish the fusb302 driver? > >> > > > >Working on it. > > Ok, I see this has landed in staging. So lets go with this driver > then and not mine (I did not get around to the PD stuff yet, so > that is fine). > > Question, how is this supposed to interface with the rest of the > kernel ? Specifically the commit message for the tcpm frameworks > says: > > "This driver only implements the state machine. Lower level drivers are > responsible for > - Reporting VBUS status and activating VBUS > - Setting CC lines and providing CC line status > - Setting line polarity > - Activating and deactivating VCONN > - Setting the current limit > - Activating and deactivating PD message transfers > - Sending and receiving PD messages" > > But the FUSB302 cannot set the current limit... >
There is an underlying driver which I didn't pay enough attention to. Badhri, Yueyao, can we publish the pd engine code as well ? If that doesn't solve the problem, it might at least give a starting point. Thanks, Guenter > The device I'm working on: > http://www.gpd.hk/gpdwin.asp > > Has the following more or less relevant components: > > -Intel Cherry Trail Z8700 SoC > -Intel Cherry Trail Whiskey Cove PMIC (which is different from the Broxton > Whiskey Cove PMIC), > this PMIC uses an external charger and fuel-gauge: > -TI bq24292i charger > -Maxim MAX17047 fuel-gauge > -PI3USB30532 USB TYPE-C mux > > Currently I only have the USB-C connector on the > device working in USB-2 mode (I did not realize > the USB-3 bits were wired up at first). > > The device ships with a charger with an USB-A > receptacle and an USB-2 only USB-A to USB-C > cable. > > The way I've hooked up USB-2 charging is like this: > The Whiskey Cove PMIC has built in USB-2 BC detection, and > I've written an extcon driver for this which reports one > of the following cables depended on the BC detection: > > EXTCON_CHG_USB_SDP, > EXTCON_CHG_USB_CDP, > EXTCON_CHG_USB_DCP, > > And I've modified the bq24290_charger driver to (optionally) > receive an extcon name through device-properties and if > it does, then set the input current based on the detected > charger type (0.5 A for SDP, 1.5A for CDP, 2A for DCP). > > Most of the patches for this are upstream. But once we > hookup the FUSB302 driver we need to propagate the current > limit it has negotiated to the bq24290_charger driver. > > Which is part of the reason why I wrote the 5th patch of > my original series, let me reply to Heikki's question about > that one here as it is related: > > On 24-04-17 16:25, Heikki Krogerus wrote: > > > Doesn't this mean you have several extcon_dev registered for a > > single port? > > Yes this is unfortunate, but the primary consumer of extcon > info is the kernel itself and we can teach it to look at the > right one. > > > I'm not completely sure how extcon framework is meant to > > be used, but shouldn't a single extcon_dev represent all the types of > > connectors a port is meant to support? > > Ideally, yes. But here we've 2 chips / drivers connected > to the same connector (more even, but 2 which provide extcon > related info). > > As explained above the device ships with an USB-2 charger + > USB-A to USB-C cable, that cable correctly gets seen by the > FUSB302 as a TYPEC_CC_RP_DEF cable, which means limiting input > current to 0.5A, but as the FUSB302 datasheet says: > > "The host software is expected to follow the USB Type-C > specification for charging current priority based on > feedback from the FUSB302 detection, external BC1.2 detection > and any USB Power Delivery communication. > > The FUSB302 does not integrate BC1.2 charger > detection which is assumed available in the USB > transceiver or USB charger in the system." > > So when we see TYPEC_CC_RP_DEF we need to ask the > Cherry Trail PMIC (in my case) to do BC detection > and use the result of that, otherwise we end up > setting input current limit way too low. > > >> Since the Type-C controller info is incomplete and needs to be > >> supplemented with the BC1.2 detection result in some cases, this > >> commit makes the intel-cht-wc extcon driver monitor the extcon device > >> registered by the Type-C controller and report that as our extcon state > >> except when BC-1.2 detection should be used. This allows extcon > >> consumers on these boards to keep monitoring only the intel-cht-wc extcon > >> and then get complete extcon info from that, which combines the Type-C > >> and BC-1.2 info. > > > > I don't really understand why does this need to be done in such a > > complex manner? Both components should just report the result and be > > done with it, and there is no need for any coupling between the two. > > If there is a consumer for the power, the driver for it can decide on > > its own the limits and maximum power from the two sources. > > To me making the combination of the 2 sources the problem > of the consumer seems just wrong, as you mentioned > above there should really be only one extcon for the > connector, likewise their should be only one definitive > source on what the input current limit is. > > TL;DR: We need some way to combine the current limit info > from TYPE-C and USB-2 BC detection into a single source and > to propagate that current to drivers which can actually set > the current-limit. > > Note I'm happy to use something else then extcon for this, > but we do need some way to combine + propagate the > current-limit. > > Likewise we need a way to tell another driver to drive VBus > on the USB-C connector. In my case this is again done by the > bq24290_charger driver, which currently does this based on the > combination of EXTCON_CHG_USB_* not being set on the extcon + > EXTCON_USB_HOST being set. > > One possible solution would be for the > drivers/extcon/extcon-intel-cht-wc.c to somehow hook into > the tpmc device and to get notifications of state changes, > then it could reflect the info from the FUSB302 in its > extcon info, not sure if that is the best design though. > > What also seems to missing is for a way to hookup a > mux driver to deal with upside-down plug-ins and > alt-mode switching. > > Regards, > > Hans
