Date:   Wed, 24 Aug 2016 10:56:13 -0700
From:   Guenter Roeck <groeck@...gle.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Guenter Roeck <groeck@...omium.org>,
        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 <linux-kernel@...r.kernel.org>,
        linux-usb@...r.kernel.org
Subject: Re: [RFC PATCH v3 0/2] Type-C Port Manager

Hello Heikki,

On Wed, Aug 24, 2016 at 5:52 AM, Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
> 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.
>
Makes sense, if not for anything else to reduce file size. I'll see if
I can separate it.

Thanks,
Guenter

Powered by blists - more mailing lists