Message-ID: <20170428105235.GG14071@kuha.fi.intel.com>
Date:   Fri, 28 Apr 2017 13:52:35 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Guenter Roeck <linux@...ck-us.net>,
        Rajaram R <rajaram.officemail@...il.com>
Cc:     Badhri Jagan Sridharan <badhri@...gle.com>,
        Oliver Neukum <oneukum@...e.com>,
        Mats Karrman <mats.dev.list@...il.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Felipe Balbi <felipe.balbi@...ux.intel.com>,
        LKML <linux-kernel@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>
Subject: Re: [PATCH v17 2/3] usb: USB Type-C connector class

On Thu, Apr 27, 2017 at 11:10:55AM -0700, Guenter Roeck wrote:
> On Thu, Apr 27, 2017 at 11:50:12AM +0530, Rajaram R wrote:
> > On Tue, Apr 25, 2017 at 7:40 PM, Guenter Roeck <linux@...ck-us.net> wrote:
> > > On 04/25/2017 01:26 AM, Rajaram R wrote:
> > >>
> > >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> > >> <badhri@...gle.com> wrote:
> > >>>
> > >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R <rajaram.officemail@...il.com>
> > >>> wrote:
> > >>>>
> > >>>> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck <linux@...ck-us.net>
> > >>>> wrote:
> > >>>>>
> > >>>>> On Fri, Apr 21, 2017 at 07:57:52PM +0530, Rajaram R wrote:
> > >>>>>>
> > >>>>>> On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan
> > >>>>>> <badhri@...gle.com> wrote:
> > >>>>>>>
> > >>>>>>> Thanks for the responses :)
> > >>>>>>>
> > >>>>>>> So seems like we have a plan.
> > >>>>>>>
> > >>>>>>> In Type-C connector class the checks for TYPEC_PWR_MODE_PD
> > >>>>>>> and pd_revision for both the port and the partner will be removed in
> > >>>>>>> power_role_store and the data_role_store and will be delegated
> > >>>>>>> to the low level drivers.
> > >>>>>>
> > >>>>>>
> > >>>>>> It is important to remember what USB Type-C provide is mechanisms for
> > >>>>>> "TRYing" to become a particular role and not guaranteeing.
> > >>>>>>
> > >>>>>> With what device combination do you fore see we could get the desired
> > >>>>>> role with this change ?
> > >>>>>>
> > >>>>>
> > >>>>> If the partner is not PD capable, if a preferred role is specified,
> > >>>>> if the current cole does not match the preferred role, and if the
> > >>>>> request
> > >>>>> is to set the role to match the preferred role, I think it is
> > >>>>> reasonable
> > >>>>> to expect that re-establishing the connection would accomplish that if
> > >>>>> the
> > >>>>> partner supports it.
> > >>>>>
> > >>>> In this context I believe we have two different inputs as follows:
> > >>>>
> > >>>> /sys/class/typec/<port>/supported_power_roles
> > >>>> /sys/class/typec/<port>/preferred_role
> > >>>>
> > >>>> The need of preferred role is required when DRP is set in
> > >>>> supported_power_roles option.
> > >>>> Ideally a battery powered device will TRY to be SNK and a a/c plugged
> > >>>> device will TRY to be SRC
> > >>>>
> > >>>> We need to understand which non-PD device will set to DRP? In the
> > >>>
> > >>>
> > >>> Android Phones (actually it could be any phone which has a type-c port)
> > >>> since it can act as usb gadget (when connected to PC) or Usb host
> > >>> when connected to peripherals such as thumb drives, keyboard etc.
> > >>> Phones with smaller form factors might be thermally limited to charge
> > >>> above 15W, therefore supporting PD might be an overkill for them.
> > >>>
> > >>>> current ecosystem all legacy devices
> > >>>> will sit behind adapters which either present an Rp or Rd.
> > >>>>
> > >>>> If it is a power adapter in 5V range can either present Rp or DRP with
> > >>>> TRY.SRC and there is no role swap requirement.
> > >>>>
> > >>>> If it is a laptop port or similar with non-PD (??) DRP  there is no
> > >>>> guaranteed role swap in a non-PD mode.
> > >>>
> > >>>
> > >>> This is true, but following a Try.SRC or Try.SNK state machine can
> > >>> increase the chances of landing in the desired role/preferred role.
> > >>
> > >>
> > >> Agree and as indicated it increases only chances.
> > >>
> > >
> > > FWIW, this is pretty much the same as requesting a role change using PD.
> > > Based on my experience with various devices, the chance for that to succeed
> > > isn't really that high either.
> > >
> > > I am not really sure I understand your problem with using Try.{SRC,SNK}
> > > to trigger (or attempt to trigger) a role change. Can we take a step back,
> > > and can you explain ?
> > 
> > The parameters required for a type-c connection is defined as follows
> > and will have a default value.
> > 
> > /sys/class/typec/<port>/supported_power_roles
> 
> I don't see that attribute (it is implied in the power_role attribute).
> It only existed in an earlier version of the driver.
> 
> Also, the attribute (when it existed) listed the roles supported
> by the port, not an administrative restriction of supported roles.
> 
> > /sys/class/typec/<port>/preferred_role
> > 
> > When two DRP devices are connected and for which we have
> > preferred_role which provides input on the preference, In a DRP mode
> > we have randomness in the mode of connection and hence we require role
> > swap mechanisms. A Type-C only device cannot role swap as this is
> > valid only in PD operation.
> > 
> 
> The same is true for non-PD connections. Also, PD doesn't solve the problem
> if both ends have the same source/sink preference. The result is still
> randomness. To resolve that randomness, one of the connection partners
> would have to give up its preference. That is quite independent of PD
> support.
> 
> > #  Question was how to choose a particular role in non-PD mode. Only
> > way to have a deterministic role in a non-PD mode is to set expected
> > supported_role of choice rather than DRP.
> > 
> > # As part of the solution suggested, checking of roles and triggering
> > role swaps has to be done by the policy manager(PM) and delinked from
> > Policy Engine. I guess the policy manager is at user space?.
> > 
> > I do not have the complete git repo and may be i could be missing
> > something. If this is in any public git please let me know
> > 
> 
> Even if a device does support PD, there is no guarantee that a power role
> swap is successful. That depends on the policy set by the partner, and
> for all practical purposes it also depends on the quality and stability
> of the PD protocol implementation on both ends. As mentioned above, if
> both ends have the same preferred role, the result is still randomness
> and requires manual intervention by the user if the "other" role is
> desired.
> 
> It seems to me that we are discussing if we should do as good as we can
> or if we should only implement what can be guaranteed. I strongly favor
> the first approach. I also think we should not preclude that approach in
> the kernel implementation. It should be left to a specific policy manager
> implementation to decide if a given device should be a DRP or source-only
> or sink-only, independent of the question if PD is supported on either
> end of a connection.
> 
> If a given implementation wants to restrict available port modes, it
> can do so by providing acceptable port capabilities (ie DRP or source
> or sink) when registering the port with the infrastructure.
> 
> I think you are suggesting that there should be a means to override port
> capabilities (more specifically, permitted roles) after port registration.
> That may be a matter for discussion, but it is a separate issue.

That is how also I see it.

> Also, I am not sure if it was already discussed. Does anyone remember ?

Yes, I did propose an attribute that would allow fixing the role of a
port to either sink or source at one point. I don't remember what was
the reason I ended up dropping that proposal - I need to go through my
archives - but I think it was seen as a feature that, if needed, can be
added later by adding a new attribute file for it.

So if that is the functionality you want, please start a new thread
and propose the new attribute. At least don't try to modify the
behaviour of the existing ones because of it.


Thank you,

-- 
heikki

Powered by blists - more mailing lists