Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965179AbdDSPOR (ORCPT ); Wed, 19 Apr 2017 11:14:17 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:54776 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964894AbdDSPOK (ORCPT ); Wed, 19 Apr 2017 11:14:10 -0400 Date: Wed, 19 Apr 2017 08:14:01 -0700 From: Guenter Roeck To: Badhri Jagan Sridharan Cc: Heikki Krogerus , Oliver Neukum , Mats Karrman , Greg KH , Felipe Balbi , LKML , USB Subject: Re: [PATCH v17 2/3] usb: USB Type-C connector class Message-ID: <20170419151401.GA14036@roeck-us.net> References: <20170303125940.GA6999@kuha.fi.intel.com> <6ddb2eac-03d5-127e-df1e-ad189968e6b2@gmail.com> <20170306131442.GC6999@kuha.fi.intel.com> <696552a7-c36a-1d73-9517-543907e9da39@gmail.com> <20170308135853.GH6999@kuha.fi.intel.com> <68817c44-d880-581a-e9f5-12845b9215eb@gmail.com>

<20170419112323.GD24062@kuha.fi.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4346 Lines: 109 On Wed, Apr 19, 2017 at 07:45:00AM -0700, Badhri Jagan Sridharan wrote: > On Wed, Apr 19, 2017 at 4:23 AM, Heikki Krogerus > wrote: > > Hi, > > > > On Tue, Apr 18, 2017 at 11:52:33AM -0700, Badhri Jagan Sridharan wrote: > >> Hi Heikki, > >> > >> I have a question regarding the preferred_role node. > >> > >> +What: /sys/class/typec//preferred_role > >> +Date: March 2017 > >> +Contact: Heikki Krogerus > >> +Description: > >> + The user space can notify the driver about the preferred role. > >> + It should be handled as enabling of Try.SRC or Try.SNK, as > >> + defined in USB Type-C specification, in the port drivers. By > >> + default the preferred role should come from the platform. > >> + > >> + Valid values: source, sink, none (to remove preference) > >> > >> What is the expected behavior when the userspace changes the > >> preferred_role node when the port is in connected state ? > >> > >> 1. the state machine re-resolves the port roles right away based on > >> the new state machine in place ? (or) > > > > No! There are separate attributes for sending role swap requests. > > Right. But, that might not be helpful in cases when PD is not implemented. > and Implementing PD is not mandatory according the spec :/ > > FYI quoting from the Type-C specification release(page 24), > role swaps are not limited to devices that only support PD. > > "Two independent set of mechanisms are defined to allow a USB Type-C > DRP to functionally swap power and data roles. When USB PD is > supported, power and data role swapping is performed as a subsequent > step following the initial connection process. For non-PD implementations, > power/data role swapping can optionally be dealt with as part of the initial > connection process." > > But, the current interface definition actually prevents current/data role > swaps for non-pd devices. > This is correct for the attribute definition, but it is not implemented that way. Writing the attribute is only read-only for non-DRP ports. Given the standard, I would consider that to be intentional; it might make sense to update the description accordingly. How about implementing a mechanism in the dr_set and pr_set code in tcpm which would handle that situation ? Something along the line of if (!port->pd_capable && connected && current role != desired role) { reset_port(); goto done; } My current code doesn't handle the !pd_capable state, so I'll need to do something anyway. Thanks, Guenter > > > > The attribute will "enable" Try.SRC/SNK states, i.e. next time the > > state machine is executed, those states need to be considered. > > Changing the value of this attribute must not affect the current > > connection. > > > >> 2. Wait till the subsequent connect for resolving port roles based on the > >> new state machine. > > > > Yes. > > > >> For #1 to happen the policy_engine layer would have to reset the port > >> to resolve the port roles based on the (Try.SRC /Try.SNK/ Default) > >> new state machine preference. > >> > >> Say for example when two non-PD devices following none (default state > >> machine) are connected, the port role resolution is going to be random. > >> But, if the userspace in one of the devices later changes the > >> preferred_role to source, then that device is most likely to become source > >> if the Try.SRC state-machine is re-run. > >> > >> Does the above question fall under a policy decision ? If so, should there > >> be another node to say if the port roles have to re-resolved based on the > >> new state machine right away ? > > > > I don't think we should even consider option #1, but just to be sure, > > Oliver, what do you say? > > Can we at least consider exposing a port_reset field so that the userspace > at least has an option to make the state machine to kick in right away with > a hard reset ? > > Please do consider. We can't expect all low-end phones and devices with > smaller form factors then phones to implement PD as it might be an overkill > for them. > > > > > I guess we need to say in the documentation explicitly that changing > > the value will not affect the current connection. > > > > > > Thanks, > > > > -- > > heikki