Re: [PATCH v17 2/3] usb: USB Type-C connector class

[Guenter Roeck]

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@xxxxxxxxxxxx> wrote:
> > On 04/25/2017 01:26 AM, Rajaram R wrote:
> >>
> >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> >> <badhri@xxxxxxxxxx> wrote:
> >>>
> >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R <rajaram.officemail@xxxxxxxxx>
> >>> wrote:
> >>>>
> >>>> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck <linux@xxxxxxxxxxxx>
> >>>> 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@xxxxxxxxxx> 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.
Also, I am not sure if it was already discussed. Does anyone remember ?

Thanks,
Guenter