Re: [PATCH 0/4] RFC: USB C extcon patchset for the tps6598x

[Angus Ainslie]

Hi Heikki,

On 2020-09-21 07:37, Heikki Krogerus wrote:
> On Mon, Sep 14, 2020 at 09:46:35AM -0700, Angus Ainslie wrote:
> > We have a complex set of hardware components to manage our USB C data 
> > and
> > power. For these to work together we decided to use extcon to 
> > communicate
> > the system changes as various cables and devices are plugged in/out. 
> > We did
> > look at usb_roleswitch and the charging framework but thought it would 
> > be
> > preferable to keep all of the information together in one system.
> >
> > The components we have in the system are:
> >
> > 1) TPS65982 type USB type C controller
> > 2) dwc3 IP in the imx8mq
> > 3) BQ25895 battery charger
> >
> > I'll break this into 2 parts the data role and the power role.
> >
> > For the data role the TPS65982 senses connect and disconnect as well 
> > as data
> > source/sink. It is also controlling the USB 3 data lanes. The display 
> > port and
> > USB 3 muxing is handled by a different chip and we'll submit patches 
> > for that
> > later on. The dwc3 controls the USB 2 data lanes.
> >
> > On the power side there are even more moving pieces. The TPS65982 
> > negotiates
> > the power delivery contract, the dwc3 senses the BC1.2 charging 
> > current and the
> > BQ25895 sets whether we are sinking or sourcing current and what the 
> > current
> > limit is of the sink and source.
> >
> > For both the data and power roles no single chip has all of the 
> > required
> > information. Is using extcon the correct way of doing this and if not 
> > what
> > are the alternatives ?
>
> Do not use extcon with the Type-C drivers unless you have some really
> good reason for not using the dedicated frameworks for each thing. The
> reason why we even have some of the dedicated frameworks in the first
> place, for example the USB role switch class, is because extcon simply
> could not be made to work on every type of hardware architecture.
>
> So you will need to register a power supply in tps6598x.c just like
> the other USB Type-C drivers like tcpm.c and ucsi.c if the TPS65982
> does not communicated directly with the BQ25895. That can be one
> of "supplied_from" (and also "supplied_to" if needed for sourcing) for
> the bq25890_changer. You probable only need to implement the
> external_power_changed() hook for it if it's missing in order to make
> it work. You can also register a power supply in dwc3 and use it as a
> second supply for bq25890 if you still really need to handle BC1.2.
>
> The data role should already be handled for you. dwc3 already
> registers an USB role switch, and tps6598x.c already configures one.
> For data role you should not need any additional code.
>
> Please note that there is also framework for the alt mode muxes.

Thanks for looking this over. I'll investigate the power supply 
framework.

Angus

> > The extcon extensions allow us to communicate the the power roles 
> > amongst
> > the various chips.
> >
> > This patch series has been tested with the 5.9-rc4 kernel on the 
> > Purism
> > Librem5 HW. Assuming this is the correct way to use extcon there will 
> > be
> > follow on patches to the BQ25895 and dwc3 drivers.
> >
> > Strictly speaking only the first 3 patches are needed for extcon 
> > support, the
> > forth patch decodes the power delivery contracts which makes use of 
> > the extcon
> > system.
> >
> >
> > Angus Ainslie (4):
> >   extcon: Add USB VBUS properties
> >   usb: typec: tps6589x: register as an extcon provider
> >   usb: typec: tps6598x: Add the extcon USB chargers
> >   usb: typec: tps6598x: Add the power delivery irq
> >
> >  drivers/usb/typec/tps6598x.c | 488 
> > ++++++++++++++++++++++++++++++++++-
> >  include/linux/extcon.h       |  17 +-
> >  2 files changed, 503 insertions(+), 2 deletions(-)
> >
> > --
> > 2.25.1
>
> thanks,