Message-ID: <20160824125224.GD12117@kuha.fi.intel.com>
Date:   Wed, 24 Aug 2016 15:52:24 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Guenter Roeck <groeck@...omium.org>
Cc:     Felipe Balbi <felipe.balbi@...ux.intel.com>,
        Chandra Sekhar Anagani <chandra.sekhar.anagani@...el.com>,
        Bruce Ashfield <bruce.ashfield@...driver.com>,
        Bin Gao <bin.gao@...el.com>,
        Pranav Tipnis <pranav.tipnis@...el.com>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [RFC PATCH v3 0/2] Type-C Port Manager

Hi Guenter,

On Tue, Aug 23, 2016 at 02:10:49PM -0700, Guenter Roeck wrote:
> The following series of patches implements a USB Type-C Port Manager
> using the pending USB Type-C class code as basis. The code is still WIP,
> but I think it is important to get feedback from the community at this point.
> 
> There are two patches in the series. The first patch implements the Type-C
> Port Manager state machine. The forth patch is an interface between the
> Type-C Port Manager and a TCPCI (Type-C Port Controller Interface) compliant
> USB Type-C Port Controller.
> 
> Patch 2/2 (the interface to a TCPCI compliant chip) is currently untested
> since I don't have the necessary hardware available. The port manager code
> was tested connecting to an Embedded Controller on a Chromebook, bypassing
> the Port Manager implementation in the EC.
> 
> Both Source and Sink operation was tested with various Type-C chargers, docks,
> and connectors. Alternate mode support is partially implemented (Alternate mode
> support is requested from the partner), but alternate modes are not actually
> selected. Implementing this will require more thought, since the actual
> alternate mode support has to be implemented elsewhere, such as in a dedicated
> Phy driver. It should be possible to implement the interface between phy driver
> and Type-C Port Controller driver using extcon, but I have not further explored
> the possibilities, and other options might be possible and/or better.
> 
> v3:
> - Improve TCPM state machine resiliency if there are spurious CC line changes
>   while the state machine is in a transient change (waiting for a timeout)
> - Update current limit after CC voltage level changes on a port which is not
>   PD capable.
> - Applies to v6 of "USB Type-C Connector class" patch series.
> 
> v2:
> - Class code no longer uses locking, so the patch to remove it is no longer
>   necessary.
> - tcpm: Only update polarity if setting it was successful
>   If setting the CC line polarity in the driver was not successful,
>   don't update the internal polarity state.
> - tcpm: All PD messages are little endian; convert to and from CPU endianness.
> - tcpm: Avoid comparisons against NULL.
> - tcpm: Use u8/u16/u32 instead of uint8_t/uint16_t/uint32_t consistently.
> - tcpm/tcpc: Callbacks into tcpm need to be lockless to avoid timing problems
>   in low level drivers.
> - tcpm/tcpc: Simplify callbacks; tcpm can request the current state of cc/vbus
>   when it is ready to use it.
> - Applies to v5 of "USB Type-C Connector class" patch series.

I have not reviewed these yet completely, but so far the series looks
really good. Nice job!

There are a lot of things that look generic and not tied to TCPCs, for
example the USB PD message handling. Couldn't those already be
separated from TCPM code or made otherwise available for non-TCPC
PHYs? The struct tcpc_dev looks to me like it would be usable for also
non-TCPC PHYs as is and enough for most cases.


Thanks,

-- 
heikki

Powered by blists - more mailing lists