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

[Heikki Krogerus]

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  wrote:
> > > On 04/25/2017 01:26 AM, Rajaram R wrote:
> > >>
> > >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> > >>  wrote:
> > >>>
> > >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
> > >>> 
> > >>> wrote:
> > 
> >  On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
> >  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
> > >>  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//supported_power_roles
> >  /sys/class/typec//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//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//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 

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

[Heikki Krogerus]

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  wrote:
> > > On 04/25/2017 01:26 AM, Rajaram R wrote:
> > >>
> > >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> > >>  wrote:
> > >>>
> > >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
> > >>> 
> > >>> wrote:
> > 
> >  On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
> >  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
> > >>  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//supported_power_roles
> >  /sys/class/typec//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//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//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.

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  wrote:
> > On 04/25/2017 01:26 AM, Rajaram R wrote:
> >>
> >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> >>  wrote:
> >>>
> >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
> >>> wrote:
> 
>  On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
>  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
> >>  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//supported_power_roles
>  /sys/class/typec//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//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//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 

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  wrote:
> > On 04/25/2017 01:26 AM, Rajaram R wrote:
> >>
> >> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
> >>  wrote:
> >>>
> >>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
> >>> wrote:
> 
>  On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
>  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
> >>  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//supported_power_roles
>  /sys/class/typec//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//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//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 

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

[Rajaram R]

On Tue, Apr 25, 2017 at 7:40 PM, Guenter Roeck  wrote:
> On 04/25/2017 01:26 AM, Rajaram R wrote:
>>
>> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
>>  wrote:
>>>
>>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
>>> wrote:

 On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
 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
>>  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//supported_power_roles
 /sys/class/typec//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//supported_power_roles
/sys/class/typec//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.

#  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

>
> Thanks,
> Guenter
>

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

[Rajaram R]

On Tue, Apr 25, 2017 at 7:40 PM, Guenter Roeck  wrote:
> On 04/25/2017 01:26 AM, Rajaram R wrote:
>>
>> On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
>>  wrote:
>>>
>>> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R 
>>> wrote:

 On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck 
 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
>>  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//supported_power_roles
 /sys/class/typec//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//supported_power_roles
/sys/class/typec//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.

#  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

>
> Thanks,
> Guenter
>

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

[Guenter Roeck]

On 04/25/2017 01:26 AM, Rajaram R wrote:

On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
 wrote:

On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  wrote:

On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
 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//supported_power_roles
/sys/class/typec//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 ?

Thanks,
Guenter

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

[Guenter Roeck]

On 04/25/2017 01:26 AM, Rajaram R wrote:

On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
 wrote:

On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  wrote:

On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
 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//supported_power_roles
/sys/class/typec//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 ?

Thanks,
Guenter

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

[Rajaram R]

On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
 wrote:
> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  
> wrote:
>> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
  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//supported_power_roles
>> /sys/class/typec//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.

>
>> So we need to understand what non PD device will fit into this scenario ?
> Answered above.
>
>>
>>> Of course, that won't change anything if the partner does not support the
>>> desired role, but it is better than doing nothing. This is also comparable
>>> to requesting a role change from the partner if it does support PD.
>>
>> All I am highlighting is that we can only TRY and there is no
>> guaranteed role swap with Type-C
>>
>>> Do you have a better idea ?
>>>
>> If need a guaranteed role in a non-PD mode we need to set the required
>> role in supported_power_roles.
>> An understanding of scenario will help take better approach.
>
> The current Type-c connector class interface defines the support_*_roles as
> read-only nodes. Leaving that apart, I think what you are trying to say is 
> that
> instead of running through the state machine again by switching to
> Try.SRC or Try.SNK, you are suggesting that switch from DRP to source/host
> (or) sink/device to make sure that CC is either pulled up through Rp or
> grounded through Rd so that it increases the chances of settling in the 
> desired
> role. I do agree this, but, there is a pitfall here. Say when a DRP is
> connected to
> a pure sink/device, when the DRP switches to being a pure sink as well, then
> the port roles would not resolve at all as both would be asserting Rd on CC 
> and

If it is a pure SNK(presenting Rd), there is no conflict and a DRP be
it TRY SRC or TRY SNK
will endup as SRC.


> therefore it might not be possible to detect a disconnect unless we have
> a VCONN powered cable. Following Try.SRC, Try.SNK state machine actually

VConn does not play any role in detection.

> takes care of this for you. When in Try.SRC or Try.SNK state, CC would either
> be pulled up or down for a specific amount of time (tCCDebounce) to check if 
> the
> port partner is capable of switching to another role. If no port
> resolution happens
> within the timer expiry, the state machine forces the port into the
> other role and
> port resolution would eventually happen. IMHO So in short it is more safer to
> switch to between Try.SRC and Try.SNK state machine to land in a preferred 
> role
> rather than switching a DRP to source or sink.

The only scenario i can think of now is  when connecting two mobiles
back to back. In this case if both devices
uses 

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

[Rajaram R]

On Mon, Apr 24, 2017 at 11:20 PM, Badhri Jagan Sridharan
 wrote:
> On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  
> wrote:
>> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
  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//supported_power_roles
>> /sys/class/typec//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.

>
>> So we need to understand what non PD device will fit into this scenario ?
> Answered above.
>
>>
>>> Of course, that won't change anything if the partner does not support the
>>> desired role, but it is better than doing nothing. This is also comparable
>>> to requesting a role change from the partner if it does support PD.
>>
>> All I am highlighting is that we can only TRY and there is no
>> guaranteed role swap with Type-C
>>
>>> Do you have a better idea ?
>>>
>> If need a guaranteed role in a non-PD mode we need to set the required
>> role in supported_power_roles.
>> An understanding of scenario will help take better approach.
>
> The current Type-c connector class interface defines the support_*_roles as
> read-only nodes. Leaving that apart, I think what you are trying to say is 
> that
> instead of running through the state machine again by switching to
> Try.SRC or Try.SNK, you are suggesting that switch from DRP to source/host
> (or) sink/device to make sure that CC is either pulled up through Rp or
> grounded through Rd so that it increases the chances of settling in the 
> desired
> role. I do agree this, but, there is a pitfall here. Say when a DRP is
> connected to
> a pure sink/device, when the DRP switches to being a pure sink as well, then
> the port roles would not resolve at all as both would be asserting Rd on CC 
> and

If it is a pure SNK(presenting Rd), there is no conflict and a DRP be
it TRY SRC or TRY SNK
will endup as SRC.


> therefore it might not be possible to detect a disconnect unless we have
> a VCONN powered cable. Following Try.SRC, Try.SNK state machine actually

VConn does not play any role in detection.

> takes care of this for you. When in Try.SRC or Try.SNK state, CC would either
> be pulled up or down for a specific amount of time (tCCDebounce) to check if 
> the
> port partner is capable of switching to another role. If no port
> resolution happens
> within the timer expiry, the state machine forces the port into the
> other role and
> port resolution would eventually happen. IMHO So in short it is more safer to
> switch to between Try.SRC and Try.SNK state machine to land in a preferred 
> role
> rather than switching a DRP to source or sink.

The only scenario i can think of now is  when connecting two mobiles
back to back. In this case if both devices
uses the same logic of switch to between Try.SRC and Try.SNK you will
not get desired result.

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

[Badhri Jagan Sridharan]

On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  wrote:
> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
>>>  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//supported_power_roles
> /sys/class/typec//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.

> So we need to understand what non PD device will fit into this scenario ?
Answered above.

>
>> Of course, that won't change anything if the partner does not support the
>> desired role, but it is better than doing nothing. This is also comparable
>> to requesting a role change from the partner if it does support PD.
>
> All I am highlighting is that we can only TRY and there is no
> guaranteed role swap with Type-C
>
>> Do you have a better idea ?
>>
> If need a guaranteed role in a non-PD mode we need to set the required
> role in supported_power_roles.
> An understanding of scenario will help take better approach.

The current Type-c connector class interface defines the support_*_roles as
read-only nodes. Leaving that apart, I think what you are trying to say is that
instead of running through the state machine again by switching to
Try.SRC or Try.SNK, you are suggesting that switch from DRP to source/host
(or) sink/device to make sure that CC is either pulled up through Rp or
grounded through Rd so that it increases the chances of settling in the desired
role. I do agree this, but, there is a pitfall here. Say when a DRP is
connected to
a pure sink/device, when the DRP switches to being a pure sink as well, then
the port roles would not resolve at all as both would be asserting Rd on CC and
therefore it might not be possible to detect a disconnect unless we have
a VCONN powered cable. Following Try.SRC, Try.SNK state machine actually
takes care of this for you. When in Try.SRC or Try.SNK state, CC would either
be pulled up or down for a specific amount of time (tCCDebounce) to check if the
port partner is capable of switching to another role. If no port
resolution happens
within the timer expiry, the state machine forces the port into the
other role and
port resolution would eventually happen. IMHO So in short it is more safer to
switch to between Try.SRC and Try.SNK state machine to land in a preferred role
rather than switching a DRP to source or sink.

>
>> Thanks,
>> Guenter
>>
>>>
>>> >
>>> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
>>> > current_role is not same as the preferred_role.
>>> >
>>
>> ... if the partner is not PD capable.
>>
>>> > I am going to make changes in my local kernel code base to start
>>> > making the corresponding changes in userspace.
>>> > Should I post-back the local kernel changes or Heikki and Geunter
>>> > you are planning to upload them ?
>>> >
>>> > Thanks for the support !!
>>> > Badhri.
>>> >
>>> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki 

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

[Badhri Jagan Sridharan]

On Sat, Apr 22, 2017 at 2:23 AM, Rajaram R  wrote:
> On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
>>>  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//supported_power_roles
> /sys/class/typec//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.

> So we need to understand what non PD device will fit into this scenario ?
Answered above.

>
>> Of course, that won't change anything if the partner does not support the
>> desired role, but it is better than doing nothing. This is also comparable
>> to requesting a role change from the partner if it does support PD.
>
> All I am highlighting is that we can only TRY and there is no
> guaranteed role swap with Type-C
>
>> Do you have a better idea ?
>>
> If need a guaranteed role in a non-PD mode we need to set the required
> role in supported_power_roles.
> An understanding of scenario will help take better approach.

The current Type-c connector class interface defines the support_*_roles as
read-only nodes. Leaving that apart, I think what you are trying to say is that
instead of running through the state machine again by switching to
Try.SRC or Try.SNK, you are suggesting that switch from DRP to source/host
(or) sink/device to make sure that CC is either pulled up through Rp or
grounded through Rd so that it increases the chances of settling in the desired
role. I do agree this, but, there is a pitfall here. Say when a DRP is
connected to
a pure sink/device, when the DRP switches to being a pure sink as well, then
the port roles would not resolve at all as both would be asserting Rd on CC and
therefore it might not be possible to detect a disconnect unless we have
a VCONN powered cable. Following Try.SRC, Try.SNK state machine actually
takes care of this for you. When in Try.SRC or Try.SNK state, CC would either
be pulled up or down for a specific amount of time (tCCDebounce) to check if the
port partner is capable of switching to another role. If no port
resolution happens
within the timer expiry, the state machine forces the port into the
other role and
port resolution would eventually happen. IMHO So in short it is more safer to
switch to between Try.SRC and Try.SNK state machine to land in a preferred role
rather than switching a DRP to source or sink.

>
>> Thanks,
>> Guenter
>>
>>>
>>> >
>>> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
>>> > current_role is not same as the preferred_role.
>>> >
>>
>> ... if the partner is not PD capable.
>>
>>> > I am going to make changes in my local kernel code base to start
>>> > making the corresponding changes in userspace.
>>> > Should I post-back the local kernel changes or Heikki and Geunter
>>> > you are planning to upload them ?
>>> >
>>> > Thanks for the support !!
>>> > Badhri.
>>> >
>>> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
>>> >  wrote:
>>> >> On Wed, Apr 19, 2017 at 10:22:47AM 

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

[Rajaram R]

On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
>>  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//supported_power_roles
/sys/class/typec//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
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.
So we need to understand what non PD device will fit into this scenario ?

> Of course, that won't change anything if the partner does not support the
> desired role, but it is better than doing nothing. This is also comparable
> to requesting a role change from the partner if it does support PD.

All I am highlighting is that we can only TRY and there is no
guaranteed role swap with Type-C

> Do you have a better idea ?
>
If need a guaranteed role in a non-PD mode we need to set the required
role in supported_power_roles.
An understanding of scenario will help take better approach.

> Thanks,
> Guenter
>
>>
>> >
>> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
>> > current_role is not same as the preferred_role.
>> >
>
> ... if the partner is not PD capable.
>
>> > I am going to make changes in my local kernel code base to start
>> > making the corresponding changes in userspace.
>> > Should I post-back the local kernel changes or Heikki and Geunter
>> > you are planning to upload them ?
>> >
>> > Thanks for the support !!
>> > Badhri.
>> >
>> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
>> >  wrote:
>> >> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>> >>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  
>> >>> wrote:
>> >>> > 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 

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

[Rajaram R]

On Fri, Apr 21, 2017 at 10:13 PM, Guenter Roeck  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
>>  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//supported_power_roles
/sys/class/typec//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
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.
So we need to understand what non PD device will fit into this scenario ?

> Of course, that won't change anything if the partner does not support the
> desired role, but it is better than doing nothing. This is also comparable
> to requesting a role change from the partner if it does support PD.

All I am highlighting is that we can only TRY and there is no
guaranteed role swap with Type-C

> Do you have a better idea ?
>
If need a guaranteed role in a non-PD mode we need to set the required
role in supported_power_roles.
An understanding of scenario will help take better approach.

> Thanks,
> Guenter
>
>>
>> >
>> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
>> > current_role is not same as the preferred_role.
>> >
>
> ... if the partner is not PD capable.
>
>> > I am going to make changes in my local kernel code base to start
>> > making the corresponding changes in userspace.
>> > Should I post-back the local kernel changes or Heikki and Geunter
>> > you are planning to upload them ?
>> >
>> > Thanks for the support !!
>> > Badhri.
>> >
>> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
>> >  wrote:
>> >> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>> >>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  
>> >>> wrote:
>> >>> > 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 

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

[Guenter Roeck]

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
>  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.

Of course, that won't change anything if the partner does not support the
desired role, but it is better than doing nothing. This is also comparable
to requesting a role change from the partner if it does support PD.

Do you have a better idea ?

Thanks,
Guenter

> 
> >
> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> > current_role is not same as the preferred_role.
> >

... if the partner is not PD capable.

> > I am going to make changes in my local kernel code base to start
> > making the corresponding changes in userspace.
> > Should I post-back the local kernel changes or Heikki and Geunter
> > you are planning to upload them ?
> >
> > Thanks for the support !!
> > Badhri.
> >
> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
> >  wrote:
> >> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> >>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> >>> > 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.
> >>>
> >>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> >>> is not TYPEC_PORT_DRP, is that what you are referring to ?
> >>>
> >>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> >>> ret = -EINVAL;
> >>> goto port_unlock;
> >>> }
> >>>
> >>> I do agree that this is actually correct. I am referring to the case
> >>> where port is
> 

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

[Guenter Roeck]

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
>  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.

Of course, that won't change anything if the partner does not support the
desired role, but it is better than doing nothing. This is also comparable
to requesting a role change from the partner if it does support PD.

Do you have a better idea ?

Thanks,
Guenter

> 
> >
> > TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> > current_role is not same as the preferred_role.
> >

... if the partner is not PD capable.

> > I am going to make changes in my local kernel code base to start
> > making the corresponding changes in userspace.
> > Should I post-back the local kernel changes or Heikki and Geunter
> > you are planning to upload them ?
> >
> > Thanks for the support !!
> > Badhri.
> >
> > On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
> >  wrote:
> >> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> >>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> >>> > 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.
> >>>
> >>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> >>> is not TYPEC_PORT_DRP, is that what you are referring to ?
> >>>
> >>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> >>> ret = -EINVAL;
> >>> goto port_unlock;
> >>> }
> >>>
> >>> I do agree that this is actually correct. I am referring to the case
> >>> where port is
> >>> dual-role-power and dual-role-data but NOT PD capable.
> >>>
> >>> > Given the standard, I would consider that to be intentional; it 

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

[Rajaram R]

On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan
 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 ?


>
> TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> current_role is not same as the preferred_role.
>
> I am going to make changes in my local kernel code base to start
> making the corresponding changes in userspace.
> Should I post-back the local kernel changes or Heikki and Geunter
> you are planning to upload them ?
>
> Thanks for the support !!
> Badhri.
>
> On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
>  wrote:
>> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
>>> > 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.
>>>
>>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
>>> is not TYPEC_PORT_DRP, is that what you are referring to ?
>>>
>>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
>>> ret = -EINVAL;
>>> goto port_unlock;
>>> }
>>>
>>> I do agree that this is actually correct. I am referring to the case
>>> where port is
>>> dual-role-power and dual-role-data but NOT PD capable.
>>>
>>> > 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;
>>> > }
>>>
>>> By "desired role" you are referring to preferred_role right ?
>>>
>>> If so yes, That's a good idea as well and it might work as long as
>>> type-c connector
>>> class allows the call to reach tcpm code :) But the current connector
>>> class code does
>>> not allow that because the power_role and data_role nodes are defined that 
>>> way.
>>
>> Well, the 

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

[Rajaram R]

On Fri, Apr 21, 2017 at 1:16 AM, Badhri Jagan Sridharan
 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 ?


>
> TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> current_role is not same as the preferred_role.
>
> I am going to make changes in my local kernel code base to start
> making the corresponding changes in userspace.
> Should I post-back the local kernel changes or Heikki and Geunter
> you are planning to upload them ?
>
> Thanks for the support !!
> Badhri.
>
> On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
>  wrote:
>> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
>>> > 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.
>>>
>>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
>>> is not TYPEC_PORT_DRP, is that what you are referring to ?
>>>
>>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
>>> ret = -EINVAL;
>>> goto port_unlock;
>>> }
>>>
>>> I do agree that this is actually correct. I am referring to the case
>>> where port is
>>> dual-role-power and dual-role-data but NOT PD capable.
>>>
>>> > 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;
>>> > }
>>>
>>> By "desired role" you are referring to preferred_role right ?
>>>
>>> If so yes, That's a good idea as well and it might work as long as
>>> type-c connector
>>> class allows the call to reach tcpm code :) But the current connector
>>> class code does
>>> not allow that because the power_role and data_role nodes are defined that 
>>> way.
>>
>> Well, the data_role does not limit the requests from reaching the low
>> level drivers, but..
>>
>>> port->cap->pd_revision and the port->pwr_opmode check 

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

[Guenter Roeck]

On 04/21/2017 05:12 AM, Heikki Krogerus wrote:

Hi,

On Thu, Apr 20, 2017 at 12:46:59PM -0700, Badhri Jagan Sridharan 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.

TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
current_role is not same as the preferred_role.

I am going to make changes in my local kernel code base to start
making the corresponding changes in userspace.
Should I post-back the local kernel changes or Heikki and Geunter
you are planning to upload them ?


Please send a patch for the class at least. I think the changes to the
tcpm can just be amended still at this point. That is of course up to
Guenter.



I'll try to make the changes and post a new version today.

Guenter

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

[Guenter Roeck]

On 04/21/2017 05:12 AM, Heikki Krogerus wrote:

Hi,

On Thu, Apr 20, 2017 at 12:46:59PM -0700, Badhri Jagan Sridharan 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.

TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
current_role is not same as the preferred_role.

I am going to make changes in my local kernel code base to start
making the corresponding changes in userspace.
Should I post-back the local kernel changes or Heikki and Geunter
you are planning to upload them ?


Please send a patch for the class at least. I think the changes to the
tcpm can just be amended still at this point. That is of course up to
Guenter.



I'll try to make the changes and post a new version today.

Guenter

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

[Heikki Krogerus]

Hi,

On Thu, Apr 20, 2017 at 12:46:59PM -0700, Badhri Jagan Sridharan 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.
> 
> TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> current_role is not same as the preferred_role.
> 
> I am going to make changes in my local kernel code base to start
> making the corresponding changes in userspace.
> Should I post-back the local kernel changes or Heikki and Geunter
> you are planning to upload them ?

Please send a patch for the class at least. I think the changes to the
tcpm can just be amended still at this point. That is of course up to
Guenter.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi,

On Thu, Apr 20, 2017 at 12:46:59PM -0700, Badhri Jagan Sridharan 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.
> 
> TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
> current_role is not same as the preferred_role.
> 
> I am going to make changes in my local kernel code base to start
> making the corresponding changes in userspace.
> Should I post-back the local kernel changes or Heikki and Geunter
> you are planning to upload them ?

Please send a patch for the class at least. I think the changes to the
tcpm can just be amended still at this point. That is of course up to
Guenter.


Thanks,

-- 
heikki

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

[Badhri Jagan Sridharan]

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.

TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
current_role is not same as the preferred_role.

I am going to make changes in my local kernel code base to start
making the corresponding changes in userspace.
Should I post-back the local kernel changes or Heikki and Geunter
you are planning to upload them ?

Thanks for the support !!
Badhri.

On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
 wrote:
> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
>> > 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.
>>
>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
>> is not TYPEC_PORT_DRP, is that what you are referring to ?
>>
>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
>> ret = -EINVAL;
>> goto port_unlock;
>> }
>>
>> I do agree that this is actually correct. I am referring to the case
>> where port is
>> dual-role-power and dual-role-data but NOT PD capable.
>>
>> > 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;
>> > }
>>
>> By "desired role" you are referring to preferred_role right ?
>>
>> If so yes, That's a good idea as well and it might work as long as
>> type-c connector
>> class allows the call to reach tcpm code :) But the current connector
>> class code does
>> not allow that because the power_role and data_role nodes are defined that 
>> way.
>
> Well, the data_role does not limit the requests from reaching the low
> level drivers, but..
>
>> port->cap->pd_revision and the port->pwr_opmode check in the below code
>> stub have to removed/refactored to make current_role/data_role writes to
>> reach the tcpm code.
>>
>> +static ssize_t power_role_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t size)
>> +{
>> + struct typec_port *port = to_typec_port(dev);
>> + int ret = size;
>> +

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

[Badhri Jagan Sridharan]

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.

TCPM code will issue hard reset in tcpm_dr_set and tcpm_pr_set if
current_role is not same as the preferred_role.

I am going to make changes in my local kernel code base to start
making the corresponding changes in userspace.
Should I post-back the local kernel changes or Heikki and Geunter
you are planning to upload them ?

Thanks for the support !!
Badhri.

On Thu, Apr 20, 2017 at 5:24 AM, Heikki Krogerus
 wrote:
> On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
>> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
>> > 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.
>>
>> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
>> is not TYPEC_PORT_DRP, is that what you are referring to ?
>>
>> if (port->typec_caps.type != TYPEC_PORT_DRP) {
>> ret = -EINVAL;
>> goto port_unlock;
>> }
>>
>> I do agree that this is actually correct. I am referring to the case
>> where port is
>> dual-role-power and dual-role-data but NOT PD capable.
>>
>> > 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;
>> > }
>>
>> By "desired role" you are referring to preferred_role right ?
>>
>> If so yes, That's a good idea as well and it might work as long as
>> type-c connector
>> class allows the call to reach tcpm code :) But the current connector
>> class code does
>> not allow that because the power_role and data_role nodes are defined that 
>> way.
>
> Well, the data_role does not limit the requests from reaching the low
> level drivers, but..
>
>> port->cap->pd_revision and the port->pwr_opmode check in the below code
>> stub have to removed/refactored to make current_role/data_role writes to
>> reach the tcpm code.
>>
>> +static ssize_t power_role_store(struct device *dev,
>> + struct device_attribute *attr,
>> + const char *buf, size_t size)
>> +{
>> + struct typec_port *port = to_typec_port(dev);
>> + int ret = size;
>> +
>> + if (!port->cap->pd_revision) {
>> + dev_dbg(dev, "USB Power Delivery not supported\n");
>> + return -EOPNOTSUPP;

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

[Heikki Krogerus]

On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> > 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.
> 
> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> is not TYPEC_PORT_DRP, is that what you are referring to ?
> 
> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> ret = -EINVAL;
> goto port_unlock;
> }
> 
> I do agree that this is actually correct. I am referring to the case
> where port is
> dual-role-power and dual-role-data but NOT PD capable.
> 
> > 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;
> > }
> 
> By "desired role" you are referring to preferred_role right ?
> 
> If so yes, That's a good idea as well and it might work as long as
> type-c connector
> class allows the call to reach tcpm code :) But the current connector
> class code does
> not allow that because the power_role and data_role nodes are defined that 
> way.

Well, the data_role does not limit the requests from reaching the low
level drivers, but..

> port->cap->pd_revision and the port->pwr_opmode check in the below code
> stub have to removed/refactored to make current_role/data_role writes to
> reach the tcpm code.
> 
> +static ssize_t power_role_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + int ret = size;
> +
> + if (!port->cap->pd_revision) {
> + dev_dbg(dev, "USB Power Delivery not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->cap->pr_set) {
> + dev_dbg(dev, "power role swapping not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
> + dev_dbg(dev, "partner unable to swap power role\n");
> + return -EIO;
> + }
> +
> + ret = sysfs_match_string(typec_roles, buf);
> + if (ret < 0)
> + return ret;
> +
> + ret = port->cap->pr_set(port->cap, ret);
> + if (ret)
> + return ret;
> +
> + return size;
> +}

.. yes. The power_role_store() does indeed need to be refactored. The
PD requirement should only be applied to Type-C spec versions < 1.2,
or removed completely. I would be happy to leave the checks to the low
level drivers.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> > 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.
> 
> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> is not TYPEC_PORT_DRP, is that what you are referring to ?
> 
> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> ret = -EINVAL;
> goto port_unlock;
> }
> 
> I do agree that this is actually correct. I am referring to the case
> where port is
> dual-role-power and dual-role-data but NOT PD capable.
> 
> > 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;
> > }
> 
> By "desired role" you are referring to preferred_role right ?
> 
> If so yes, That's a good idea as well and it might work as long as
> type-c connector
> class allows the call to reach tcpm code :) But the current connector
> class code does
> not allow that because the power_role and data_role nodes are defined that 
> way.

Well, the data_role does not limit the requests from reaching the low
level drivers, but..

> port->cap->pd_revision and the port->pwr_opmode check in the below code
> stub have to removed/refactored to make current_role/data_role writes to
> reach the tcpm code.
> 
> +static ssize_t power_role_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + int ret = size;
> +
> + if (!port->cap->pd_revision) {
> + dev_dbg(dev, "USB Power Delivery not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->cap->pr_set) {
> + dev_dbg(dev, "power role swapping not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
> + dev_dbg(dev, "partner unable to swap power role\n");
> + return -EIO;
> + }
> +
> + ret = sysfs_match_string(typec_roles, buf);
> + if (ret < 0)
> + return ret;
> +
> + ret = port->cap->pr_set(port->cap, ret);
> + if (ret)
> + return ret;
> +
> + return size;
> +}

.. yes. The power_role_store() does indeed need to be refactored. The
PD requirement should only be applied to Type-C spec versions < 1.2,
or removed completely. I would be happy to leave the checks to the low
level drivers.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi,

On Wed, Apr 19, 2017 at 07:45:00AM -0700, Badhri Jagan Sridharan wrote:
> >> 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.

No it doesn't. When USB PD is not supported, you can still use
data_role to swap the role. Of course, the lower level driver may
not support any role swapping in that case.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi,

On Wed, Apr 19, 2017 at 07:45:00AM -0700, Badhri Jagan Sridharan wrote:
> >> 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.

No it doesn't. When USB PD is not supported, you can still use
data_role to swap the role. Of course, the lower level driver may
not support any role swapping in that case.


Thanks,

-- 
heikki

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

[Guenter Roeck]

On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> > 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.
> 
> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> is not TYPEC_PORT_DRP, is that what you are referring to ?
> 
> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> ret = -EINVAL;
> goto port_unlock;
> }
> 
> I do agree that this is actually correct. I am referring to the case
> where port is
> dual-role-power and dual-role-data but NOT PD capable.
> 
At least in the current implementation of tcpm, the port type is reported
from low level drivers and does not change if the partner is not PD
capable. Port capabilities don't change if the partner is not PD capable,
and both are reported separately to the typec infrastructure.

> > 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;
> > }
> 
> By "desired role" you are referring to preferred_role right ?
> 
That is what it would boil down to, but that is really up to user space
to decide. User space would check if the reported role is the desired role,
and request a role change if it is not.
 
> If so yes, That's a good idea as well and it might work as long as
> type-c connector
> class allows the call to reach tcpm code :) But the current connector
> class code does
> not allow that because the power_role and data_role nodes are defined that 
> way.
> 
> port->cap->pd_revision and the port->pwr_opmode check in the below code
> stub have to removed/refactored to make current_role/data_role writes to
> reach the tcpm code.
> 
> +static ssize_t power_role_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + int ret = size;
> +
> + if (!port->cap->pd_revision) {
> + dev_dbg(dev, "USB Power Delivery not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->cap->pr_set) {
> + dev_dbg(dev, "power role swapping not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
> + dev_dbg(dev, "partner unable to swap power role\n");
> + return -EIO;
> + }
> +
Ah yes, you are right. Of course, you could request a data role 

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

[Guenter Roeck]

On Wed, Apr 19, 2017 at 10:22:47AM -0700, Badhri Jagan Sridharan wrote:
> On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> > 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.
> 
> i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
> is not TYPEC_PORT_DRP, is that what you are referring to ?
> 
> if (port->typec_caps.type != TYPEC_PORT_DRP) {
> ret = -EINVAL;
> goto port_unlock;
> }
> 
> I do agree that this is actually correct. I am referring to the case
> where port is
> dual-role-power and dual-role-data but NOT PD capable.
> 
At least in the current implementation of tcpm, the port type is reported
from low level drivers and does not change if the partner is not PD
capable. Port capabilities don't change if the partner is not PD capable,
and both are reported separately to the typec infrastructure.

> > 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;
> > }
> 
> By "desired role" you are referring to preferred_role right ?
> 
That is what it would boil down to, but that is really up to user space
to decide. User space would check if the reported role is the desired role,
and request a role change if it is not.
 
> If so yes, That's a good idea as well and it might work as long as
> type-c connector
> class allows the call to reach tcpm code :) But the current connector
> class code does
> not allow that because the power_role and data_role nodes are defined that 
> way.
> 
> port->cap->pd_revision and the port->pwr_opmode check in the below code
> stub have to removed/refactored to make current_role/data_role writes to
> reach the tcpm code.
> 
> +static ssize_t power_role_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + struct typec_port *port = to_typec_port(dev);
> + int ret = size;
> +
> + if (!port->cap->pd_revision) {
> + dev_dbg(dev, "USB Power Delivery not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (!port->cap->pr_set) {
> + dev_dbg(dev, "power role swapping not supported\n");
> + return -EOPNOTSUPP;
> + }
> +
> + if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
> + dev_dbg(dev, "partner unable to swap power role\n");
> + return -EIO;
> + }
> +
Ah yes, you are right. Of course, you could request a data role change,
which doesn't have that restriction, at least not right now, but that
would not 

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

[Badhri Jagan Sridharan]

On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> 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.

i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
is not TYPEC_PORT_DRP, is that what you are referring to ?

if (port->typec_caps.type != TYPEC_PORT_DRP) {
ret = -EINVAL;
goto port_unlock;
}

I do agree that this is actually correct. I am referring to the case
where port is
dual-role-power and dual-role-data but NOT PD capable.

> 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;
> }

By "desired role" you are referring to preferred_role right ?

If so yes, That's a good idea as well and it might work as long as
type-c connector
class allows the call to reach tcpm code :) But the current connector
class code does
not allow that because the power_role and data_role nodes are defined that way.

port->cap->pd_revision and the port->pwr_opmode check in the below code
stub have to removed/refactored to make current_role/data_role writes to
reach the tcpm code.

+static ssize_t power_role_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct typec_port *port = to_typec_port(dev);
+ int ret = size;
+
+ if (!port->cap->pd_revision) {
+ dev_dbg(dev, "USB Power Delivery not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (!port->cap->pr_set) {
+ dev_dbg(dev, "power role swapping not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
+ dev_dbg(dev, "partner unable to swap power role\n");
+ return -EIO;
+ }
+
+ ret = sysfs_match_string(typec_roles, buf);
+ if (ret < 0)
+ return ret;
+
+ ret = port->cap->pr_set(port->cap, ret);
+ if (ret)
+ return ret;
+
+ return size;
+}

Thanks,
Badhri

>
> 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.
>> >>
>> >> 

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

[Badhri Jagan Sridharan]

On Wed, Apr 19, 2017 at 8:14 AM, Guenter Roeck  wrote:
> 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.

i.e. tcpm_dr_set/tcpm_pr_set at tcpm.c would return EINVAL when type
is not TYPEC_PORT_DRP, is that what you are referring to ?

if (port->typec_caps.type != TYPEC_PORT_DRP) {
ret = -EINVAL;
goto port_unlock;
}

I do agree that this is actually correct. I am referring to the case
where port is
dual-role-power and dual-role-data but NOT PD capable.

> 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;
> }

By "desired role" you are referring to preferred_role right ?

If so yes, That's a good idea as well and it might work as long as
type-c connector
class allows the call to reach tcpm code :) But the current connector
class code does
not allow that because the power_role and data_role nodes are defined that way.

port->cap->pd_revision and the port->pwr_opmode check in the below code
stub have to removed/refactored to make current_role/data_role writes to
reach the tcpm code.

+static ssize_t power_role_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ struct typec_port *port = to_typec_port(dev);
+ int ret = size;
+
+ if (!port->cap->pd_revision) {
+ dev_dbg(dev, "USB Power Delivery not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (!port->cap->pr_set) {
+ dev_dbg(dev, "power role swapping not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
+ dev_dbg(dev, "partner unable to swap power role\n");
+ return -EIO;
+ }
+
+ ret = sysfs_match_string(typec_roles, buf);
+ if (ret < 0)
+ return ret;
+
+ ret = port->cap->pr_set(port->cap, ret);
+ if (ret)
+ return ret;
+
+ return size;
+}

Thanks,
Badhri

>
> 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) 

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

[Guenter Roeck]

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

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

[Guenter Roeck]

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

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

[Badhri Jagan Sridharan]

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.

>
> 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

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

[Badhri Jagan Sridharan]

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.

>
> 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

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

[Heikki Krogerus]

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.

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?

I guess we need to say in the documentation explicitly that changing
the value will not affect the current connection.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

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.

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?

I guess we need to say in the documentation explicitly that changing
the value will not affect the current connection.


Thanks,

-- 
heikki

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

[Badhri Jagan Sridharan]

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)
2. Wait till the subsequent connect for resolving port roles based on the
new state machine.

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 ?

Thanks !!

On Fri, Mar 10, 2017 at 3:41 PM, Guenter Roeck  wrote:
> On 03/10/2017 02:22 PM, Mats Karrman wrote:
>>
>> On 2017-03-08 14:58, Heikki Krogerus wrote:
>>
>>> On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

 If I read Heikki's original suggestion I understand it like the DP
 driver would be
 responsible for AM specific USB PD/VDM communication. But wouldn't that
 lead
 to a lot of code duplication since the AM protocol is the same for all
 drivers of
 a kind?
>>>
>>> No that's not what I mean. I'm still mixing your PD controller with
>>> something else above, sorry about that. Your PD controller driver
>>> should not ideally even need to be aware of Type-C connector, right?
>>> It definitely does not need to do any USB PD communication.
>>
>>
>> Right.
>>
>>> I would imagine you have on top of the DP controller, a mux (which
>>> could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
>>> Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
>>> PD controller. The bus would be tying the mux to the Type-C port (PHY
>>> or PD controller) and its partner (note that it does not tie the mux
>>> to the DP controller). Please correct me if I'm wrong about your
>>> hardware.
>>
>>
>> No, you're correct, a discrete mux and a fusb302.
>>
>>> Assuming that is how your board roughly looks like, the driver for the
>>> mux would be the driver for the DP altmode devices. That driver would
>>> be the one converting things like the Attention messages notifying
>>> about HPD into toggling of GPIOs, or what ever is needed on your
>>> board, etc.
>>
>>
>> OK.
>>
>>> The actual PD communication with VDMs should be considered as just the
>>> protocol, so we probable should have "protocol drivers". For example
>>> DP alternate mode VDMs and communication will always be the same
>>> despite of the hardware. The DP alternate mode "protocol driver" would
>>> then be tied to the alternate mode device for the partner, and that
>>> driver could have its own hooks for what ever is needed, like HPD
>>> signal handling, configuration changes, whatever. In any case,
>>> hopefully making things easy and straightforward for the "mux driver",
>>> _so that it does not need to care about the actual PD communication_.
>>
>>
>> I'm digesting your and Guenter's replies and patches.
>> I will try getting something up and running too soon and hopefully the
>> foggy parts will
>> dissolve. As for now I find it a lot easier to grok Guenter's drivers than
>> to see the
>> advantages and/or disadvantages of an altmode bus :-)
>>
>> @Guenter: There _is_ interest for your fusb302 driver, thank you
>>
>
> Ok, I'll see what I need to do to publish it.
>
> Guenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Badhri Jagan Sridharan]

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)
2. Wait till the subsequent connect for resolving port roles based on the
new state machine.

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 ?

Thanks !!

On Fri, Mar 10, 2017 at 3:41 PM, Guenter Roeck  wrote:
> On 03/10/2017 02:22 PM, Mats Karrman wrote:
>>
>> On 2017-03-08 14:58, Heikki Krogerus wrote:
>>
>>> On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

 If I read Heikki's original suggestion I understand it like the DP
 driver would be
 responsible for AM specific USB PD/VDM communication. But wouldn't that
 lead
 to a lot of code duplication since the AM protocol is the same for all
 drivers of
 a kind?
>>>
>>> No that's not what I mean. I'm still mixing your PD controller with
>>> something else above, sorry about that. Your PD controller driver
>>> should not ideally even need to be aware of Type-C connector, right?
>>> It definitely does not need to do any USB PD communication.
>>
>>
>> Right.
>>
>>> I would imagine you have on top of the DP controller, a mux (which
>>> could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
>>> Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
>>> PD controller. The bus would be tying the mux to the Type-C port (PHY
>>> or PD controller) and its partner (note that it does not tie the mux
>>> to the DP controller). Please correct me if I'm wrong about your
>>> hardware.
>>
>>
>> No, you're correct, a discrete mux and a fusb302.
>>
>>> Assuming that is how your board roughly looks like, the driver for the
>>> mux would be the driver for the DP altmode devices. That driver would
>>> be the one converting things like the Attention messages notifying
>>> about HPD into toggling of GPIOs, or what ever is needed on your
>>> board, etc.
>>
>>
>> OK.
>>
>>> The actual PD communication with VDMs should be considered as just the
>>> protocol, so we probable should have "protocol drivers". For example
>>> DP alternate mode VDMs and communication will always be the same
>>> despite of the hardware. The DP alternate mode "protocol driver" would
>>> then be tied to the alternate mode device for the partner, and that
>>> driver could have its own hooks for what ever is needed, like HPD
>>> signal handling, configuration changes, whatever. In any case,
>>> hopefully making things easy and straightforward for the "mux driver",
>>> _so that it does not need to care about the actual PD communication_.
>>
>>
>> I'm digesting your and Guenter's replies and patches.
>> I will try getting something up and running too soon and hopefully the
>> foggy parts will
>> dissolve. As for now I find it a lot easier to grok Guenter's drivers than
>> to see the
>> advantages and/or disadvantages of an altmode bus :-)
>>
>> @Guenter: There _is_ interest for your fusb302 driver, thank you
>>
>
> Ok, I'll see what I need to do to publish it.
>
> Guenter
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Guenter Roeck]

On 03/10/2017 02:22 PM, Mats Karrman wrote:

On 2017-03-08 14:58, Heikki Krogerus wrote:


On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.


Right.


I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.


No, you're correct, a discrete mux and a fusb302.


Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.


OK.


The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


I'm digesting your and Guenter's replies and patches.
I will try getting something up and running too soon and hopefully the foggy 
parts will
dissolve. As for now I find it a lot easier to grok Guenter's drivers than to 
see the
advantages and/or disadvantages of an altmode bus :-)

@Guenter: There _is_ interest for your fusb302 driver, thank you



Ok, I'll see what I need to do to publish it.

Guenter

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

[Guenter Roeck]

On 03/10/2017 02:22 PM, Mats Karrman wrote:

On 2017-03-08 14:58, Heikki Krogerus wrote:


On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.


Right.


I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.


No, you're correct, a discrete mux and a fusb302.


Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.


OK.


The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


I'm digesting your and Guenter's replies and patches.
I will try getting something up and running too soon and hopefully the foggy 
parts will
dissolve. As for now I find it a lot easier to grok Guenter's drivers than to 
see the
advantages and/or disadvantages of an altmode bus :-)

@Guenter: There _is_ interest for your fusb302 driver, thank you



Ok, I'll see what I need to do to publish it.

Guenter

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

[Mats Karrman]

On 2017-03-08 14:58, Heikki Krogerus wrote:


On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.


Right.


I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.


No, you're correct, a discrete mux and a fusb302.


Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.


OK.


The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


I'm digesting your and Guenter's replies and patches.
I will try getting something up and running too soon and hopefully the foggy 
parts will
dissolve. As for now I find it a lot easier to grok Guenter's drivers than to 
see the
advantages and/or disadvantages of an altmode bus :-)

@Guenter: There _is_ interest for your fusb302 driver, thank you

BR // Mats

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

[Mats Karrman]

On 2017-03-08 14:58, Heikki Krogerus wrote:


On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.


Right.


I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.


No, you're correct, a discrete mux and a fusb302.


Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.


OK.


The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


I'm digesting your and Guenter's replies and patches.
I will try getting something up and running too soon and hopefully the foggy 
parts will
dissolve. As for now I find it a lot easier to grok Guenter's drivers than to 
see the
advantages and/or disadvantages of an altmode bus :-)

@Guenter: There _is_ interest for your fusb302 driver, thank you

BR // Mats

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

[Peter Chen]

On Wed, Mar 08, 2017 at 06:44:47AM -0800, Guenter Roeck wrote:
> On 03/07/2017 10:50 PM, Peter Chen wrote:
> >
> You mean type-C trigger an ACPI event, and this ACPI event can notify
> related USB controller driver doing role switch?
> >>>
> >>>No (firmware programs the dual-role hw/registers), but never mind.
> >>>That could be the case.
> >>>
> If it is correct, there is a notifier between type-C and USB
> controller driver, how to define this notifier for non-ACPI platform?
> >>>
> >>>Once there is a platform with Type-C like that, the problem needs to
> >>>be solved. However..
> >>>
> >I'm not commenting on Roger's dual role patch series, but I don't
> >really think it should be mixed with Type-C. USB Type-C and USB
> >Power Delivery define their own ways of handling the roles, and they
> >are not limited to the data role only. Things like OTG for example
> >will, and actually can not be supported. With Type-C we will have
> >competing state machines compared to OTG. The dual-role framework
> >may be useful on systems that provide more traditional connectors,
> >which possibly have the ID-pin like micro-AB, and possibly also
> >support OTG. It can also be something that exist in parallel with the 
> >Type-C
> >>class, but there just can not be any dependencies between the two.
> >
> 
> Yes, there are two independent things. But if the kernel doesn't have
> a notifier between type-C message sender (type-c class) and message
> receiver (like USB controller driver for role switch or other drivers
> for alternate mode message), we had to find some ways at userspace.
> >>>
> >>>..what ever the solution is, it really can't rely on user space.
> >>>
> >>
> >>... and, at least for our application, using extcon for the necessary 
> >>notifications works
> >>just fine.
> >>
> >
> >I see, that means you have a hardware signal to notify role switch.
> >
> 
> In our case the Type-C protocol manager (including alternate mode handling)
> is implemented in an EC. The EC signals the extcon-cros_ec driver, which
> in turn signals the phy driver as well as the DP driver. The Type-C class
> is orthogonal; extcon-cros_ec will also register with the Type-C class code
> once that is upstream.
> 
> As mentioned earlier, using extcon for signaling was the most convenient means
> for us to pass events around. I am more than open to change it to a bus,
> if that can be made to work - we'd have to keep in mind though that this code
> already works without Type-C infrastructure and is for the most part already
> upstream (the rk3399 code it ties into is upstream, and extcon-cros_ec has 
> been
> submitted as https://patchwork.kernel.org/patch/9583045/).
> 

I am clear your implementation now, thank, Guenter.

> As for to how to handle alternate mode if the Type-C protocol manager
> (TCPM) is implemented in the kernel - I have not yet implemented it yet,
> but my thinking goes along the line described by Heikki in his last e-mail.
> 
> Note that we also have a kernel driver for FUSB302 which ties into my tcpm
> driver. I'll have to check if that is public yet and if I or someone
> else can publish it if there is interest.
> 
> Guenter
> 

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Wed, Mar 08, 2017 at 06:44:47AM -0800, Guenter Roeck wrote:
> On 03/07/2017 10:50 PM, Peter Chen wrote:
> >
> You mean type-C trigger an ACPI event, and this ACPI event can notify
> related USB controller driver doing role switch?
> >>>
> >>>No (firmware programs the dual-role hw/registers), but never mind.
> >>>That could be the case.
> >>>
> If it is correct, there is a notifier between type-C and USB
> controller driver, how to define this notifier for non-ACPI platform?
> >>>
> >>>Once there is a platform with Type-C like that, the problem needs to
> >>>be solved. However..
> >>>
> >I'm not commenting on Roger's dual role patch series, but I don't
> >really think it should be mixed with Type-C. USB Type-C and USB
> >Power Delivery define their own ways of handling the roles, and they
> >are not limited to the data role only. Things like OTG for example
> >will, and actually can not be supported. With Type-C we will have
> >competing state machines compared to OTG. The dual-role framework
> >may be useful on systems that provide more traditional connectors,
> >which possibly have the ID-pin like micro-AB, and possibly also
> >support OTG. It can also be something that exist in parallel with the 
> >Type-C
> >>class, but there just can not be any dependencies between the two.
> >
> 
> Yes, there are two independent things. But if the kernel doesn't have
> a notifier between type-C message sender (type-c class) and message
> receiver (like USB controller driver for role switch or other drivers
> for alternate mode message), we had to find some ways at userspace.
> >>>
> >>>..what ever the solution is, it really can't rely on user space.
> >>>
> >>
> >>... and, at least for our application, using extcon for the necessary 
> >>notifications works
> >>just fine.
> >>
> >
> >I see, that means you have a hardware signal to notify role switch.
> >
> 
> In our case the Type-C protocol manager (including alternate mode handling)
> is implemented in an EC. The EC signals the extcon-cros_ec driver, which
> in turn signals the phy driver as well as the DP driver. The Type-C class
> is orthogonal; extcon-cros_ec will also register with the Type-C class code
> once that is upstream.
> 
> As mentioned earlier, using extcon for signaling was the most convenient means
> for us to pass events around. I am more than open to change it to a bus,
> if that can be made to work - we'd have to keep in mind though that this code
> already works without Type-C infrastructure and is for the most part already
> upstream (the rk3399 code it ties into is upstream, and extcon-cros_ec has 
> been
> submitted as https://patchwork.kernel.org/patch/9583045/).
> 

I am clear your implementation now, thank, Guenter.

> As for to how to handle alternate mode if the Type-C protocol manager
> (TCPM) is implemented in the kernel - I have not yet implemented it yet,
> but my thinking goes along the line described by Heikki in his last e-mail.
> 
> Note that we also have a kernel driver for FUSB302 which ties into my tcpm
> driver. I'll have to check if that is public yet and if I or someone
> else can publish it if there is interest.
> 
> Guenter
> 

-- 

Best Regards,
Peter Chen

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

[Guenter Roeck]

On 03/07/2017 10:50 PM, Peter Chen wrote:



You mean type-C trigger an ACPI event, and this ACPI event can notify
related USB controller driver doing role switch?


No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.


If it is correct, there is a notifier between type-C and USB
controller driver, how to define this notifier for non-ACPI platform?


Once there is a platform with Type-C like that, the problem needs to
be solved. However..


I'm not commenting on Roger's dual role patch series, but I don't
really think it should be mixed with Type-C. USB Type-C and USB
Power Delivery define their own ways of handling the roles, and they
are not limited to the data role only. Things like OTG for example
will, and actually can not be supported. With Type-C we will have
competing state machines compared to OTG. The dual-role framework
may be useful on systems that provide more traditional connectors,
which possibly have the ID-pin like micro-AB, and possibly also
support OTG. It can also be something that exist in parallel with the Type-C

class, but there just can not be any dependencies between the two.




Yes, there are two independent things. But if the kernel doesn't have
a notifier between type-C message sender (type-c class) and message
receiver (like USB controller driver for role switch or other drivers
for alternate mode message), we had to find some ways at userspace.


..what ever the solution is, it really can't rely on user space.



... and, at least for our application, using extcon for the necessary 
notifications works
just fine.



I see, that means you have a hardware signal to notify role switch.



In our case the Type-C protocol manager (including alternate mode handling)
is implemented in an EC. The EC signals the extcon-cros_ec driver, which
in turn signals the phy driver as well as the DP driver. The Type-C class
is orthogonal; extcon-cros_ec will also register with the Type-C class code
once that is upstream.

As mentioned earlier, using extcon for signaling was the most convenient means
for us to pass events around. I am more than open to change it to a bus,
if that can be made to work - we'd have to keep in mind though that this code
already works without Type-C infrastructure and is for the most part already
upstream (the rk3399 code it ties into is upstream, and extcon-cros_ec has been
submitted as https://patchwork.kernel.org/patch/9583045/).

As for to how to handle alternate mode if the Type-C protocol manager
(TCPM) is implemented in the kernel - I have not yet implemented it yet,
but my thinking goes along the line described by Heikki in his last e-mail.

Note that we also have a kernel driver for FUSB302 which ties into my tcpm
driver. I'll have to check if that is public yet and if I or someone
else can publish it if there is interest.

Guenter

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

[Guenter Roeck]

On 03/07/2017 10:50 PM, Peter Chen wrote:



You mean type-C trigger an ACPI event, and this ACPI event can notify
related USB controller driver doing role switch?


No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.


If it is correct, there is a notifier between type-C and USB
controller driver, how to define this notifier for non-ACPI platform?


Once there is a platform with Type-C like that, the problem needs to
be solved. However..


I'm not commenting on Roger's dual role patch series, but I don't
really think it should be mixed with Type-C. USB Type-C and USB
Power Delivery define their own ways of handling the roles, and they
are not limited to the data role only. Things like OTG for example
will, and actually can not be supported. With Type-C we will have
competing state machines compared to OTG. The dual-role framework
may be useful on systems that provide more traditional connectors,
which possibly have the ID-pin like micro-AB, and possibly also
support OTG. It can also be something that exist in parallel with the Type-C

class, but there just can not be any dependencies between the two.




Yes, there are two independent things. But if the kernel doesn't have
a notifier between type-C message sender (type-c class) and message
receiver (like USB controller driver for role switch or other drivers
for alternate mode message), we had to find some ways at userspace.


..what ever the solution is, it really can't rely on user space.



... and, at least for our application, using extcon for the necessary 
notifications works
just fine.



I see, that means you have a hardware signal to notify role switch.



In our case the Type-C protocol manager (including alternate mode handling)
is implemented in an EC. The EC signals the extcon-cros_ec driver, which
in turn signals the phy driver as well as the DP driver. The Type-C class
is orthogonal; extcon-cros_ec will also register with the Type-C class code
once that is upstream.

As mentioned earlier, using extcon for signaling was the most convenient means
for us to pass events around. I am more than open to change it to a bus,
if that can be made to work - we'd have to keep in mind though that this code
already works without Type-C infrastructure and is for the most part already
upstream (the rk3399 code it ties into is upstream, and extcon-cros_ec has been
submitted as https://patchwork.kernel.org/patch/9583045/).

As for to how to handle alternate mode if the Type-C protocol manager
(TCPM) is implemented in the kernel - I have not yet implemented it yet,
but my thinking goes along the line described by Heikki in his last e-mail.

Note that we also have a kernel driver for FUSB302 which ties into my tcpm
driver. I'll have to check if that is public yet and if I or someone
else can publish it if there is interest.

Guenter

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

[Heikki Krogerus]

On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:
> > > So, both the DP controller and the USB PD phy are I2C devices, and now I 
> > > have to make them both
> > > attach to the AM bus as well?
> > The DP controller would provide the driver and the USB PD phy
> > (actually, the typec class) the device.
> > 
> > Would it be a problem to register these I2C devices with some other
> > subsystem, was it extcon or something like AM bus? It really would not
> > be that uncommon. Or have I misunderstood your question?
> 
> OK, so a bus could be used for drivers to find each other but it still does 
> not say
> anything about how those drivers are supposed to communicate so that must be 
> prescribed
> separately, right?

Yes.

> If I read Heikki's original suggestion I understand it like the DP driver 
> would be
> responsible for AM specific USB PD/VDM communication. But wouldn't that lead
> to a lot of code duplication since the AM protocol is the same for all 
> drivers of
> a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.

I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.

Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.

The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

On Tue, Mar 07, 2017 at 11:30:54PM +0100, Mats Karrman wrote:
> > > So, both the DP controller and the USB PD phy are I2C devices, and now I 
> > > have to make them both
> > > attach to the AM bus as well?
> > The DP controller would provide the driver and the USB PD phy
> > (actually, the typec class) the device.
> > 
> > Would it be a problem to register these I2C devices with some other
> > subsystem, was it extcon or something like AM bus? It really would not
> > be that uncommon. Or have I misunderstood your question?
> 
> OK, so a bus could be used for drivers to find each other but it still does 
> not say
> anything about how those drivers are supposed to communicate so that must be 
> prescribed
> separately, right?

Yes.

> If I read Heikki's original suggestion I understand it like the DP driver 
> would be
> responsible for AM specific USB PD/VDM communication. But wouldn't that lead
> to a lot of code duplication since the AM protocol is the same for all 
> drivers of
> a kind?

No that's not what I mean. I'm still mixing your PD controller with
something else above, sorry about that. Your PD controller driver
should not ideally even need to be aware of Type-C connector, right?
It definitely does not need to do any USB PD communication.

I would imagine you have on top of the DP controller, a mux (which
could be a DP/USB3 PHY like on Rockchip RK3399, discrete mux like
Pericom PI3USB30532, or something else), and a USB Type-C PHY or USB
PD controller. The bus would be tying the mux to the Type-C port (PHY
or PD controller) and its partner (note that it does not tie the mux
to the DP controller). Please correct me if I'm wrong about your
hardware.

Assuming that is how your board roughly looks like, the driver for the
mux would be the driver for the DP altmode devices. That driver would
be the one converting things like the Attention messages notifying
about HPD into toggling of GPIOs, or what ever is needed on your
board, etc.

The actual PD communication with VDMs should be considered as just the
protocol, so we probable should have "protocol drivers". For example
DP alternate mode VDMs and communication will always be the same
despite of the hardware. The DP alternate mode "protocol driver" would
then be tied to the alternate mode device for the partner, and that
driver could have its own hooks for what ever is needed, like HPD
signal handling, configuration changes, whatever. In any case,
hopefully making things easy and straightforward for the "mux driver",
_so that it does not need to care about the actual PD communication_.


Thanks,

-- 
heikki

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

[Peter Chen]

 
>>> You mean type-C trigger an ACPI event, and this ACPI event can notify
>>> related USB controller driver doing role switch?
>>
>> No (firmware programs the dual-role hw/registers), but never mind.
>> That could be the case.
>>
>>> If it is correct, there is a notifier between type-C and USB
>>> controller driver, how to define this notifier for non-ACPI platform?
>>
>> Once there is a platform with Type-C like that, the problem needs to
>> be solved. However..
>>
 I'm not commenting on Roger's dual role patch series, but I don't
 really think it should be mixed with Type-C. USB Type-C and USB
 Power Delivery define their own ways of handling the roles, and they
 are not limited to the data role only. Things like OTG for example
 will, and actually can not be supported. With Type-C we will have
 competing state machines compared to OTG. The dual-role framework
 may be useful on systems that provide more traditional connectors,
 which possibly have the ID-pin like micro-AB, and possibly also
 support OTG. It can also be something that exist in parallel with the 
 Type-C
>class, but there just can not be any dependencies between the two.

>>>
>>> Yes, there are two independent things. But if the kernel doesn't have
>>> a notifier between type-C message sender (type-c class) and message
>>> receiver (like USB controller driver for role switch or other drivers
>>> for alternate mode message), we had to find some ways at userspace.
>>
>> ..what ever the solution is, it really can't rely on user space.
>>
>
>... and, at least for our application, using extcon for the necessary 
>notifications works
>just fine.
>

I see, that means you have a hardware signal to notify role switch.

Peter

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

[Peter Chen]

 
>>> You mean type-C trigger an ACPI event, and this ACPI event can notify
>>> related USB controller driver doing role switch?
>>
>> No (firmware programs the dual-role hw/registers), but never mind.
>> That could be the case.
>>
>>> If it is correct, there is a notifier between type-C and USB
>>> controller driver, how to define this notifier for non-ACPI platform?
>>
>> Once there is a platform with Type-C like that, the problem needs to
>> be solved. However..
>>
 I'm not commenting on Roger's dual role patch series, but I don't
 really think it should be mixed with Type-C. USB Type-C and USB
 Power Delivery define their own ways of handling the roles, and they
 are not limited to the data role only. Things like OTG for example
 will, and actually can not be supported. With Type-C we will have
 competing state machines compared to OTG. The dual-role framework
 may be useful on systems that provide more traditional connectors,
 which possibly have the ID-pin like micro-AB, and possibly also
 support OTG. It can also be something that exist in parallel with the 
 Type-C
>class, but there just can not be any dependencies between the two.

>>>
>>> Yes, there are two independent things. But if the kernel doesn't have
>>> a notifier between type-C message sender (type-c class) and message
>>> receiver (like USB controller driver for role switch or other drivers
>>> for alternate mode message), we had to find some ways at userspace.
>>
>> ..what ever the solution is, it really can't rely on user space.
>>
>
>... and, at least for our application, using extcon for the necessary 
>notifications works
>just fine.
>

I see, that means you have a hardware signal to notify role switch.

Peter

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

[Guenter Roeck]

On 03/07/2017 02:30 PM, Mats Karrman wrote:
[ ... ]



I'm still struggling to catch up on what you guys have been up to during the
last year or so :-) and came across some patches of Guenter from last October:

http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1243527.html

What happened to them? Has there been any progress since then?



Updates to keep in sync with API changes, bug fixes, and minor improvements,
for the most part. I can post a current version if there is interest.
The latest version is also available from
https://chromium-review.googlesource.com/#/c/389917/

Guenter

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

[Guenter Roeck]

On 03/07/2017 02:30 PM, Mats Karrman wrote:
[ ... ]



I'm still struggling to catch up on what you guys have been up to during the
last year or so :-) and came across some patches of Guenter from last October:

http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1243527.html

What happened to them? Has there been any progress since then?



Updates to keep in sync with API changes, bug fixes, and minor improvements,
for the most part. I can post a current version if there is interest.
The latest version is also available from
https://chromium-review.googlesource.com/#/c/389917/

Guenter

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

[Guenter Roeck]

On 03/07/2017 12:57 AM, Heikki Krogerus wrote:

On Tue, Mar 07, 2017 at 01:36:29AM +, Peter Chen wrote:

On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:

What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.

Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


No. If the partner executes successfully for example DR_Swap
message, the kernel has to take care everything that is needed for
the role to be what ever was negotiated on its own. User space can't
be involved with that.



Would you give me an example how kernel handle this? How type-C event
triggers role switch?


On our boards, the firmware or EC (or ACPI) configures the hardware as needed
and also notifies the components using ACPI if needed. It's often not even 
possible to
directly configure the components/hardware for a particular role.



You mean type-C trigger an ACPI event, and this ACPI event can notify related
USB controller driver doing role switch?


No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.


If it is correct, there is a notifier between type-C
and USB controller driver, how to define this notifier for non-ACPI platform?


Once there is a platform with Type-C like that, the problem needs to
be solved. However..


I'm not commenting on Roger's dual role patch series, but I don't really think 
it should
be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
of handling the roles, and they are not limited to the data role only. Things 
like OTG
for example will, and actually can not be supported. With Type-C we will have
competing state machines compared to OTG. The dual-role framework may be
useful on systems that provide more traditional connectors, which possibly have 
the
ID-pin like micro-AB, and possibly also support OTG. It can also be something 
that
exist in parallel with the Type-C class, but there just can not be any 
dependencies
between the two.



Yes, there are two independent things. But if the kernel doesn't have a 
notifier between
type-C message sender (type-c class) and message receiver (like USB controller 
driver
for role switch or other drivers for alternate mode message), we had to find 
some ways at
userspace.


..what ever the solution is, it really can't rely on user space.



... and, at least for our application, using extcon for the necessary 
notifications
works just fine.

Guenter

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

[Guenter Roeck]

On 03/07/2017 12:57 AM, Heikki Krogerus wrote:

On Tue, Mar 07, 2017 at 01:36:29AM +, Peter Chen wrote:

On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:

What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.

Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


No. If the partner executes successfully for example DR_Swap
message, the kernel has to take care everything that is needed for
the role to be what ever was negotiated on its own. User space can't
be involved with that.



Would you give me an example how kernel handle this? How type-C event
triggers role switch?


On our boards, the firmware or EC (or ACPI) configures the hardware as needed
and also notifies the components using ACPI if needed. It's often not even 
possible to
directly configure the components/hardware for a particular role.



You mean type-C trigger an ACPI event, and this ACPI event can notify related
USB controller driver doing role switch?


No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.


If it is correct, there is a notifier between type-C
and USB controller driver, how to define this notifier for non-ACPI platform?


Once there is a platform with Type-C like that, the problem needs to
be solved. However..


I'm not commenting on Roger's dual role patch series, but I don't really think 
it should
be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
of handling the roles, and they are not limited to the data role only. Things 
like OTG
for example will, and actually can not be supported. With Type-C we will have
competing state machines compared to OTG. The dual-role framework may be
useful on systems that provide more traditional connectors, which possibly have 
the
ID-pin like micro-AB, and possibly also support OTG. It can also be something 
that
exist in parallel with the Type-C class, but there just can not be any 
dependencies
between the two.



Yes, there are two independent things. But if the kernel doesn't have a 
notifier between
type-C message sender (type-c class) and message receiver (like USB controller 
driver
for role switch or other drivers for alternate mode message), we had to find 
some ways at
userspace.


..what ever the solution is, it really can't rely on user space.



... and, at least for our application, using extcon for the necessary 
notifications
works just fine.

Guenter

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

[Mats Karrman]

On 2017-03-06 14:14, Heikki Krogerus wrote:


Hi Mats,

On Fri, Mar 03, 2017 at 08:27:08PM +0100, Mats Karrman wrote:

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?

No, the "DP controller" just talks DP and knows nothing about Type-C or USB PD.
It takes a video stream from the SoC and turns it into a DP link, set up and 
orchestrated
by the corresponding driver. And all the driver needs from Type-C is the 
plugged in / interrupt /
plugged out events.

Got it.


The analog switching between USB / safe / DP signal levels in the Type-C 
connector is, I think,
best handled by the software doing the USB PD negotiation / Altmode handling 
(using some GPIOs).


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.

So, both the DP controller and the USB PD phy are I2C devices, and now I have 
to make them both
attach to the AM bus as well?

The DP controller would provide the driver and the USB PD phy
(actually, the typec class) the device.

Would it be a problem to register these I2C devices with some other
subsystem, was it extcon or something like AM bus? It really would not
be that uncommon. Or have I misunderstood your question?


OK, so a bus could be used for drivers to find each other but it still does not 
say
anything about how those drivers are supposed to communicate so that must be 
prescribed
separately, right?

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

I'm still struggling to catch up on what you guys have been up to during the
last year or so :-) and came across some patches of Guenter from last October:

http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1243527.html

What happened to them? Has there been any progress since then?

BR // Mats

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

[Mats Karrman]

On 2017-03-06 14:14, Heikki Krogerus wrote:


Hi Mats,

On Fri, Mar 03, 2017 at 08:27:08PM +0100, Mats Karrman wrote:

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?

No, the "DP controller" just talks DP and knows nothing about Type-C or USB PD.
It takes a video stream from the SoC and turns it into a DP link, set up and 
orchestrated
by the corresponding driver. And all the driver needs from Type-C is the 
plugged in / interrupt /
plugged out events.

Got it.


The analog switching between USB / safe / DP signal levels in the Type-C 
connector is, I think,
best handled by the software doing the USB PD negotiation / Altmode handling 
(using some GPIOs).


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.

So, both the DP controller and the USB PD phy are I2C devices, and now I have 
to make them both
attach to the AM bus as well?

The DP controller would provide the driver and the USB PD phy
(actually, the typec class) the device.

Would it be a problem to register these I2C devices with some other
subsystem, was it extcon or something like AM bus? It really would not
be that uncommon. Or have I misunderstood your question?


OK, so a bus could be used for drivers to find each other but it still does not 
say
anything about how those drivers are supposed to communicate so that must be 
prescribed
separately, right?

If I read Heikki's original suggestion I understand it like the DP driver would 
be
responsible for AM specific USB PD/VDM communication. But wouldn't that lead
to a lot of code duplication since the AM protocol is the same for all drivers 
of
a kind?

I'm still struggling to catch up on what you guys have been up to during the
last year or so :-) and came across some patches of Guenter from last October:

http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1243527.html

What happened to them? Has there been any progress since then?

BR // Mats

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

[Peter Chen]

 
>
>On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
>> > > What interface you use when you receive this event to handle
>> > > dual-role switch? I am wonder if a common dual-role class is
>> > > needed, then we can have a common user utility.
>> > >
>> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
>> > > /sys/class/usb-dual-role/role
>> >
>> > No. If the partner executes successfully for example DR_Swap
>> > message, the kernel has to take care everything that is needed for
>> > the role to be what ever was negotiated on its own. User space can't
>> > be involved with that.
>> >
>>
>> Would you give me an example how kernel handle this? How type-C event
>> triggers role switch?
>
>On our boards, the firmware or EC (or ACPI) configures the hardware as needed
>and also notifies the components using ACPI if needed. It's often not even 
>possible to
>directly configure the components/hardware for a particular role.
>

You mean type-C trigger an ACPI event, and this ACPI event can notify related
USB controller driver doing role switch? If it is correct, there is a notifier 
between type-C
and USB controller driver, how to define this notifier for non-ACPI platform? 

>I'm not commenting on Roger's dual role patch series, but I don't really think 
>it should
>be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
>of handling the roles, and they are not limited to the data role only. Things 
>like OTG
>for example will, and actually can not be supported. With Type-C we will have
>competing state machines compared to OTG. The dual-role framework may be
>useful on systems that provide more traditional connectors, which possibly 
>have the
>ID-pin like micro-AB, and possibly also support OTG. It can also be something 
>that
>exist in parallel with the Type-C class, but there just can not be any 
>dependencies
>between the two.
>

Yes, there are two independent things. But if the kernel doesn't have a 
notifier between
type-C message sender (type-c class) and message receiver (like USB controller 
driver
for role switch or other drivers for alternate mode message), we had to find 
some ways at
userspace.

Peter

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

[Peter Chen]

 
>
>On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
>> > > What interface you use when you receive this event to handle
>> > > dual-role switch? I am wonder if a common dual-role class is
>> > > needed, then we can have a common user utility.
>> > >
>> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
>> > > /sys/class/usb-dual-role/role
>> >
>> > No. If the partner executes successfully for example DR_Swap
>> > message, the kernel has to take care everything that is needed for
>> > the role to be what ever was negotiated on its own. User space can't
>> > be involved with that.
>> >
>>
>> Would you give me an example how kernel handle this? How type-C event
>> triggers role switch?
>
>On our boards, the firmware or EC (or ACPI) configures the hardware as needed
>and also notifies the components using ACPI if needed. It's often not even 
>possible to
>directly configure the components/hardware for a particular role.
>

You mean type-C trigger an ACPI event, and this ACPI event can notify related
USB controller driver doing role switch? If it is correct, there is a notifier 
between type-C
and USB controller driver, how to define this notifier for non-ACPI platform? 

>I'm not commenting on Roger's dual role patch series, but I don't really think 
>it should
>be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
>of handling the roles, and they are not limited to the data role only. Things 
>like OTG
>for example will, and actually can not be supported. With Type-C we will have
>competing state machines compared to OTG. The dual-role framework may be
>useful on systems that provide more traditional connectors, which possibly 
>have the
>ID-pin like micro-AB, and possibly also support OTG. It can also be something 
>that
>exist in parallel with the Type-C class, but there just can not be any 
>dependencies
>between the two.
>

Yes, there are two independent things. But if the kernel doesn't have a 
notifier between
type-C message sender (type-c class) and message receiver (like USB controller 
driver
for role switch or other drivers for alternate mode message), we had to find 
some ways at
userspace.

Peter

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

[Heikki Krogerus]

On Tue, Mar 07, 2017 at 01:36:29AM +, Peter Chen wrote:
> >On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
> >> > > What interface you use when you receive this event to handle
> >> > > dual-role switch? I am wonder if a common dual-role class is
> >> > > needed, then we can have a common user utility.
> >> > >
> >> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
> >> > > /sys/class/usb-dual-role/role
> >> >
> >> > No. If the partner executes successfully for example DR_Swap
> >> > message, the kernel has to take care everything that is needed for
> >> > the role to be what ever was negotiated on its own. User space can't
> >> > be involved with that.
> >> >
> >>
> >> Would you give me an example how kernel handle this? How type-C event
> >> triggers role switch?
> >
> >On our boards, the firmware or EC (or ACPI) configures the hardware as needed
> >and also notifies the components using ACPI if needed. It's often not even 
> >possible to
> >directly configure the components/hardware for a particular role.
> >
> 
> You mean type-C trigger an ACPI event, and this ACPI event can notify related
> USB controller driver doing role switch?

No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.

> If it is correct, there is a notifier between type-C
> and USB controller driver, how to define this notifier for non-ACPI platform? 

Once there is a platform with Type-C like that, the problem needs to
be solved. However..

> >I'm not commenting on Roger's dual role patch series, but I don't really 
> >think it should
> >be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
> >of handling the roles, and they are not limited to the data role only. 
> >Things like OTG
> >for example will, and actually can not be supported. With Type-C we will have
> >competing state machines compared to OTG. The dual-role framework may be
> >useful on systems that provide more traditional connectors, which possibly 
> >have the
> >ID-pin like micro-AB, and possibly also support OTG. It can also be 
> >something that
> >exist in parallel with the Type-C class, but there just can not be any 
> >dependencies
> >between the two.
> >
> 
> Yes, there are two independent things. But if the kernel doesn't have a 
> notifier between
> type-C message sender (type-c class) and message receiver (like USB 
> controller driver
> for role switch or other drivers for alternate mode message), we had to find 
> some ways at
> userspace.

..what ever the solution is, it really can't rely on user space.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

On Tue, Mar 07, 2017 at 01:36:29AM +, Peter Chen wrote:
> >On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
> >> > > What interface you use when you receive this event to handle
> >> > > dual-role switch? I am wonder if a common dual-role class is
> >> > > needed, then we can have a common user utility.
> >> > >
> >> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
> >> > > /sys/class/usb-dual-role/role
> >> >
> >> > No. If the partner executes successfully for example DR_Swap
> >> > message, the kernel has to take care everything that is needed for
> >> > the role to be what ever was negotiated on its own. User space can't
> >> > be involved with that.
> >> >
> >>
> >> Would you give me an example how kernel handle this? How type-C event
> >> triggers role switch?
> >
> >On our boards, the firmware or EC (or ACPI) configures the hardware as needed
> >and also notifies the components using ACPI if needed. It's often not even 
> >possible to
> >directly configure the components/hardware for a particular role.
> >
> 
> You mean type-C trigger an ACPI event, and this ACPI event can notify related
> USB controller driver doing role switch?

No (firmware programs the dual-role hw/registers), but never mind.
That could be the case.

> If it is correct, there is a notifier between type-C
> and USB controller driver, how to define this notifier for non-ACPI platform? 

Once there is a platform with Type-C like that, the problem needs to
be solved. However..

> >I'm not commenting on Roger's dual role patch series, but I don't really 
> >think it should
> >be mixed with Type-C. USB Type-C and USB Power Delivery define their own ways
> >of handling the roles, and they are not limited to the data role only. 
> >Things like OTG
> >for example will, and actually can not be supported. With Type-C we will have
> >competing state machines compared to OTG. The dual-role framework may be
> >useful on systems that provide more traditional connectors, which possibly 
> >have the
> >ID-pin like micro-AB, and possibly also support OTG. It can also be 
> >something that
> >exist in parallel with the Type-C class, but there just can not be any 
> >dependencies
> >between the two.
> >
> 
> Yes, there are two independent things. But if the kernel doesn't have a 
> notifier between
> type-C message sender (type-c class) and message receiver (like USB 
> controller driver
> for role switch or other drivers for alternate mode message), we had to find 
> some ways at
> userspace.

..what ever the solution is, it really can't rely on user space.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Mats,

On Fri, Mar 03, 2017 at 08:27:08PM +0100, Mats Karrman wrote:
> On 2017-03-03 13:59, Heikki Krogerus wrote:
> 
> > On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> > 
> 
> > How would something like that sound to you guys?
> 
> Complicated... Need to marinate on that for a while ;)

Sorry about the bad explanation :-). Let me try again..  I'm simply
looking for a method that is as scalable as possible to handle the
alternate modes, basically how to couple the different components
involved. Bus would feel like the best approach at the moment.

> > > My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
> > > controller connected
> > > directly to the SoC and it's using DTB/OF.
> > Is this "DP controller" a controller that is capable of taking care of
> > the USB Power Delivery communication with the partner regarding
> > DisplayPort alternate mode?
> 
> No, the "DP controller" just talks DP and knows nothing about Type-C or USB 
> PD.
> It takes a video stream from the SoC and turns it into a DP link, set up and 
> orchestrated
> by the corresponding driver. And all the driver needs from Type-C is the 
> plugged in / interrupt /
> plugged out events.

Got it.

> The analog switching between USB / safe / DP signal levels in the Type-C 
> connector is, I think,
> best handled by the software doing the USB PD negotiation / Altmode handling 
> (using some GPIOs).
> 
> > > Do we need to further standardize attributes under (each) specific 
> > > alternate mode to
> > > include things such as HPD for the DP mode?
> > I'm not completely sure what kind of system you have, but I would
> > imagine that if we had the bus, your DP controller driver would be the
> > port (and partner) alternate mode driver. The bus would bind you to
> > the typec phy.
> 
> So, both the DP controller and the USB PD phy are I2C devices, and now I have 
> to make them both
> attach to the AM bus as well?

The DP controller would provide the driver and the USB PD phy
(actually, the typec class) the device.

Would it be a problem to register these I2C devices with some other
subsystem, was it extcon or something like AM bus? It really would not
be that uncommon. Or have I misunderstood your question?


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Mats,

On Fri, Mar 03, 2017 at 08:27:08PM +0100, Mats Karrman wrote:
> On 2017-03-03 13:59, Heikki Krogerus wrote:
> 
> > On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> > 
> 
> > How would something like that sound to you guys?
> 
> Complicated... Need to marinate on that for a while ;)

Sorry about the bad explanation :-). Let me try again..  I'm simply
looking for a method that is as scalable as possible to handle the
alternate modes, basically how to couple the different components
involved. Bus would feel like the best approach at the moment.

> > > My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
> > > controller connected
> > > directly to the SoC and it's using DTB/OF.
> > Is this "DP controller" a controller that is capable of taking care of
> > the USB Power Delivery communication with the partner regarding
> > DisplayPort alternate mode?
> 
> No, the "DP controller" just talks DP and knows nothing about Type-C or USB 
> PD.
> It takes a video stream from the SoC and turns it into a DP link, set up and 
> orchestrated
> by the corresponding driver. And all the driver needs from Type-C is the 
> plugged in / interrupt /
> plugged out events.

Got it.

> The analog switching between USB / safe / DP signal levels in the Type-C 
> connector is, I think,
> best handled by the software doing the USB PD negotiation / Altmode handling 
> (using some GPIOs).
> 
> > > Do we need to further standardize attributes under (each) specific 
> > > alternate mode to
> > > include things such as HPD for the DP mode?
> > I'm not completely sure what kind of system you have, but I would
> > imagine that if we had the bus, your DP controller driver would be the
> > port (and partner) alternate mode driver. The bus would bind you to
> > the typec phy.
> 
> So, both the DP controller and the USB PD phy are I2C devices, and now I have 
> to make them both
> attach to the AM bus as well?

The DP controller would provide the driver and the USB PD phy
(actually, the typec class) the device.

Would it be a problem to register these I2C devices with some other
subsystem, was it extcon or something like AM bus? It really would not
be that uncommon. Or have I misunderstood your question?


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Peter,

On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
> > > What interface you use when you receive this event to handle
> > > dual-role switch? I am wonder if a common dual-role class is
> > > needed, then we can have a common user utility.
> > > 
> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
> > > /sys/class/usb-dual-role/role
> > 
> > No. If the partner executes successfully for example DR_Swap message,
> > the kernel has to take care everything that is needed for the role to
> > be what ever was negotiated on its own. User space can't be involved
> > with that.
> > 
> 
> Would you give me an example how kernel handle this? How type-C event
> triggers role switch?

On our boards, the firmware or EC (or ACPI) configures the hardware as
needed and also notifies the components using ACPI if needed. It's
often not even possible to directly configure the components/hardware
for a particular role.

I'm not commenting on Roger's dual role patch series, but I don't
really think it should be mixed with Type-C. USB Type-C and USB Power
Delivery define their own ways of handling the roles, and they are not
limited to the data role only. Things like OTG for example will, and
actually can not be supported. With Type-C we will have competing
state machines compared to OTG. The dual-role framework may be useful
on systems that provide more traditional connectors, which possibly
have the ID-pin like micro-AB, and possibly also support OTG. It can
also be something that exist in parallel with the Type-C class, but
there just can not be any dependencies between the two.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Peter,

On Mon, Mar 06, 2017 at 09:15:51AM +0800, Peter Chen wrote:
> > > What interface you use when you receive this event to handle
> > > dual-role switch? I am wonder if a common dual-role class is
> > > needed, then we can have a common user utility.
> > > 
> > > Eg, if "data_role" has changed, the udev can echo "data_role" to
> > > /sys/class/usb-dual-role/role
> > 
> > No. If the partner executes successfully for example DR_Swap message,
> > the kernel has to take care everything that is needed for the role to
> > be what ever was negotiated on its own. User space can't be involved
> > with that.
> > 
> 
> Would you give me an example how kernel handle this? How type-C event
> triggers role switch?

On our boards, the firmware or EC (or ACPI) configures the hardware as
needed and also notifies the components using ACPI if needed. It's
often not even possible to directly configure the components/hardware
for a particular role.

I'm not commenting on Roger's dual role patch series, but I don't
really think it should be mixed with Type-C. USB Type-C and USB Power
Delivery define their own ways of handling the roles, and they are not
limited to the data role only. Things like OTG for example will, and
actually can not be supported. With Type-C we will have competing
state machines compared to OTG. The dual-role framework may be useful
on systems that provide more traditional connectors, which possibly
have the ID-pin like micro-AB, and possibly also support OTG. It can
also be something that exist in parallel with the Type-C class, but
there just can not be any dependencies between the two.


Thanks,

-- 
heikki

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

[Oliver Neukum]

Am Freitag, den 03.03.2017, 20:27 +0100 schrieb Mats Karrman:
> On 2017-03-03 13:59, Heikki Krogerus wrote:
> 
> > 
> > On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> > 
> 
> > 
> > How would something like that sound to you guys?
> 
> Complicated... Need to marinate on that for a while ;)

The thing is that a type C plug has operations that affect every
device on it and need to be synchronized. Reset and power
requirements would be the obvious examples.

It seems to me that the natural way to represent this is a bus.
Like SCSI where you have a bus reset operation.

Regards
Oliver

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

[Oliver Neukum]

Am Freitag, den 03.03.2017, 20:27 +0100 schrieb Mats Karrman:
> On 2017-03-03 13:59, Heikki Krogerus wrote:
> 
> > 
> > On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> > 
> 
> > 
> > How would something like that sound to you guys?
> 
> Complicated... Need to marinate on that for a while ;)

The thing is that a type C plug has operations that affect every
device on it and need to be synchronized. Reset and power
requirements would be the obvious examples.

It seems to me that the natural way to represent this is a bus.
Like SCSI where you have a bus reset operation.

Regards
Oliver

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

[Peter Chen]

On Fri, Mar 03, 2017 at 06:36:50AM -0800, Guenter Roeck wrote:
> On 03/02/2017 08:52 PM, Peter Chen wrote:
> >On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:
> >>On 03/02/2017 07:35 PM, Peter Chen wrote:
> >>>On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> +/* --- */
> +/* Driver callbacks to report role updates */
> +
> +/**
> + * typec_set_data_role - Report data role change
> + * @port: The USB Type-C Port where the role was changed
> + * @role: The new data role
> + *
> + * This routine is used by the port drivers to report data role changes.
> + */
> +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> role)
> +{
> + if (port->data_role == role)
> + return;
> +
> + port->data_role = role;
> + sysfs_notify(>dev.kobj, NULL, "data_role");
> + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_set_data_role);
> +
> >>>
> >>>Hi Keikki,
> >>>
> >>>Have you tested this interface with real dual-role controller/board?
> >>
> >>If it helps, my primary test system is a HP Chromebook 13 G1.
> >>
> >>>What interface you use when you receive this event to handle
> >>>dual-role switch? I am wonder if a common dual-role class is
> >>>needed, then we can have a common user utility.
> >>
> >>I don't really understand "What interface you use when you receive
> >>this event". Can you explain ?
> >>
> >
> >I mean "How to trigger kernel USB controller driver do role switch?"
> >
> 
> I think this should be handled by the lower level driver. I am wide open
> to other ideas, though.
> 

Would you show me how it works at your test system if it is not
a private thing?

> >>>
> >>>Eg, if "data_role" has changed, the udev can echo "data_role" to
> >>>/sys/class/usb-dual-role/role
> >>>
> >>That sounds like a kernel event delivered to user space via udev or
> >>sysfs notification and returned back into the kernel through a sysfs
> >>attribute. Do I understand that correctly ?
> >>
> >
> >Yes.
> >
> 
> That doesn't sound like a good idea to me, and I don't see a technical reason
> to require it.
> 

I don't like it either, but current kernel has no ability to handle it
except you use hardware signal like extcon-gpio or controller id as
input pin to SoC handle this event.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Fri, Mar 03, 2017 at 06:36:50AM -0800, Guenter Roeck wrote:
> On 03/02/2017 08:52 PM, Peter Chen wrote:
> >On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:
> >>On 03/02/2017 07:35 PM, Peter Chen wrote:
> >>>On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> +/* --- */
> +/* Driver callbacks to report role updates */
> +
> +/**
> + * typec_set_data_role - Report data role change
> + * @port: The USB Type-C Port where the role was changed
> + * @role: The new data role
> + *
> + * This routine is used by the port drivers to report data role changes.
> + */
> +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> role)
> +{
> + if (port->data_role == role)
> + return;
> +
> + port->data_role = role;
> + sysfs_notify(>dev.kobj, NULL, "data_role");
> + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_set_data_role);
> +
> >>>
> >>>Hi Keikki,
> >>>
> >>>Have you tested this interface with real dual-role controller/board?
> >>
> >>If it helps, my primary test system is a HP Chromebook 13 G1.
> >>
> >>>What interface you use when you receive this event to handle
> >>>dual-role switch? I am wonder if a common dual-role class is
> >>>needed, then we can have a common user utility.
> >>
> >>I don't really understand "What interface you use when you receive
> >>this event". Can you explain ?
> >>
> >
> >I mean "How to trigger kernel USB controller driver do role switch?"
> >
> 
> I think this should be handled by the lower level driver. I am wide open
> to other ideas, though.
> 

Would you show me how it works at your test system if it is not
a private thing?

> >>>
> >>>Eg, if "data_role" has changed, the udev can echo "data_role" to
> >>>/sys/class/usb-dual-role/role
> >>>
> >>That sounds like a kernel event delivered to user space via udev or
> >>sysfs notification and returned back into the kernel through a sysfs
> >>attribute. Do I understand that correctly ?
> >>
> >
> >Yes.
> >
> 
> That doesn't sound like a good idea to me, and I don't see a technical reason
> to require it.
> 

I don't like it either, but current kernel has no ability to handle it
except you use hardware signal like extcon-gpio or controller id as
input pin to SoC handle this event.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Fri, Mar 03, 2017 at 04:31:33PM +0200, Heikki Krogerus wrote:
> Hi Peter,
> 
> On Fri, Mar 03, 2017 at 11:35:29AM +0800, Peter Chen wrote:
> > On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> > > +/* --- */
> > > +/* Driver callbacks to report role updates */
> > > +
> > > +/**
> > > + * typec_set_data_role - Report data role change
> > > + * @port: The USB Type-C Port where the role was changed
> > > + * @role: The new data role
> > > + *
> > > + * This routine is used by the port drivers to report data role changes.
> > > + */
> > > +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> > > role)
> > > +{
> > > + if (port->data_role == role)
> > > + return;
> > > +
> > > + port->data_role = role;
> > > + sysfs_notify(>dev.kobj, NULL, "data_role");
> > > + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_set_data_role);
> > > +
> > 
> > Hi Keikki,
> > 
> > Have you tested this interface with real dual-role controller/board?
> 
> Yes. Our boards are mostly USB dual-role capable.
> 
> > What interface you use when you receive this event to handle
> > dual-role switch? I am wonder if a common dual-role class is
> > needed, then we can have a common user utility.
> > 
> > Eg, if "data_role" has changed, the udev can echo "data_role" to
> > /sys/class/usb-dual-role/role
> 
> No. If the partner executes successfully for example DR_Swap message,
> the kernel has to take care everything that is needed for the role to
> be what ever was negotiated on its own. User space can't be involved
> with that.
> 

Would you give me an example how kernel handle this? How type-C event
triggers role switch?

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Fri, Mar 03, 2017 at 04:31:33PM +0200, Heikki Krogerus wrote:
> Hi Peter,
> 
> On Fri, Mar 03, 2017 at 11:35:29AM +0800, Peter Chen wrote:
> > On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> > > +/* --- */
> > > +/* Driver callbacks to report role updates */
> > > +
> > > +/**
> > > + * typec_set_data_role - Report data role change
> > > + * @port: The USB Type-C Port where the role was changed
> > > + * @role: The new data role
> > > + *
> > > + * This routine is used by the port drivers to report data role changes.
> > > + */
> > > +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> > > role)
> > > +{
> > > + if (port->data_role == role)
> > > + return;
> > > +
> > > + port->data_role = role;
> > > + sysfs_notify(>dev.kobj, NULL, "data_role");
> > > + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > > +}
> > > +EXPORT_SYMBOL_GPL(typec_set_data_role);
> > > +
> > 
> > Hi Keikki,
> > 
> > Have you tested this interface with real dual-role controller/board?
> 
> Yes. Our boards are mostly USB dual-role capable.
> 
> > What interface you use when you receive this event to handle
> > dual-role switch? I am wonder if a common dual-role class is
> > needed, then we can have a common user utility.
> > 
> > Eg, if "data_role" has changed, the udev can echo "data_role" to
> > /sys/class/usb-dual-role/role
> 
> No. If the partner executes successfully for example DR_Swap message,
> the kernel has to take care everything that is needed for the role to
> be what ever was negotiated on its own. User space can't be involved
> with that.
> 

Would you give me an example how kernel handle this? How type-C event
triggers role switch?

-- 

Best Regards,
Peter Chen

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

[Mats Karrman]

On 2017-03-03 13:59, Heikki Krogerus wrote:


On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:




How would something like that sound to you guys?


Complicated... Need to marinate on that for a while ;)


My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?


No, the "DP controller" just talks DP and knows nothing about Type-C or USB PD.
It takes a video stream from the SoC and turns it into a DP link, set up and 
orchestrated
by the corresponding driver. And all the driver needs from Type-C is the 
plugged in / interrupt /
plugged out events.

The analog switching between USB / safe / DP signal levels in the Type-C 
connector is, I think,
best handled by the software doing the USB PD negotiation / Altmode handling 
(using some GPIOs).


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.


So, both the DP controller and the USB PD phy are I2C devices, and now I have 
to make them both
attach to the AM bus as well?

BR,
Mats

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

[Mats Karrman]

On 2017-03-03 13:59, Heikki Krogerus wrote:


On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:




How would something like that sound to you guys?


Complicated... Need to marinate on that for a while ;)


My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?


No, the "DP controller" just talks DP and knows nothing about Type-C or USB PD.
It takes a video stream from the SoC and turns it into a DP link, set up and 
orchestrated
by the corresponding driver. And all the driver needs from Type-C is the 
plugged in / interrupt /
plugged out events.

The analog switching between USB / safe / DP signal levels in the Type-C 
connector is, I think,
best handled by the software doing the USB PD negotiation / Altmode handling 
(using some GPIOs).


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.


So, both the DP controller and the USB PD phy are I2C devices, and now I have 
to make them both
attach to the AM bus as well?

BR,
Mats

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

[Guenter Roeck]

On 03/03/2017 04:59 AM, Heikki Krogerus wrote:

Hi,

On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?


My idea was to have something like the altmode "bus" at one point.
We create a device for every alternate mode registered in typec class,
so the alternate modes registered for the ports and partners would
simply be attached to the altmode bus. There would be a bus per port
of course.

The drivers for the port alternate modes would take care of things
like muxing and other platform specific stuff as needed, and they
would be tied to the underlying subsystems and drivers, graphics in
case of DisplayPort. The drivers for the partner alternate modes would
take care of the actual communication with the alternate mode with
VDMs if needed (but not necessarily), and they would need to be tied
to the port alternate modes. In practice the driver for both the port
and the partner alternate modes will be the same (in the same
location) obviously, at least in most cases.

I think a bus would allow us to support several ways of handling the
alternate modes on different platforms. It would work fine also on
platforms that had no use for it of course, like platforms where
firmware or EC takes care of most things related to Type-C.

But please note that since this is just a high level idea still, we
wouldn't for example need to create an actual bus if there is no use
for it, but since we have the SVIDs that can be used for matching,
then why not try take advantage of them, right.

How would something like that sound to you guys?


That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


In this case I think you would only need to register a driver with the
bus in case you want the handle to the device for the alternate mode.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.


Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?


Using extcon I would have a driver that is both typec class and extcon driver 
at the same time
since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?
I don't know much about the wcove PMIC and what alternate modes it might 
support but I
guess that driver would end up in the same place.


What alternate modes systems with WhiskeyCove supports depends on
the platform. WhiskeyCove PMIC (as in Power Management IC) is
available on a few Intel Atom platforms. The USB Type-C PHY in it
provides a simple USB PD transceiver that does not touch the actual
communication with the partners. The communication needs to be done in
software, including dealing with alternate modes.

I'm not planning on using extcon for anything with WhiskeyCove. I
don't have any use for it. It looks to me that extcon is used just as
a tool to create software couplings in many cases, and I'm not
completely comfortable with that.



For rk3399 we went back and forth trying other approaches, but using extcon
turned out to be the most straightforward mechanism; everything else would
just have replicated what extcon already provides. Having said that,
I am open to other approaches. The rk3399 implementation may be seen as an
example on what is required, specifically when it comes to the interaction
between cable, DP, and the Type-C phy.

Guenter


In my case with DP altmode, if we had for example muxes to take care
of, I don't think it would be a problem to tie the driver for the mux
to the graphics directly, so basically make it part of the graphics
stack. That driver would be the port (and partner) altmode driver.


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?


I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) 

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

[Guenter Roeck]

On 03/03/2017 04:59 AM, Heikki Krogerus wrote:

Hi,

On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?


My idea was to have something like the altmode "bus" at one point.
We create a device for every alternate mode registered in typec class,
so the alternate modes registered for the ports and partners would
simply be attached to the altmode bus. There would be a bus per port
of course.

The drivers for the port alternate modes would take care of things
like muxing and other platform specific stuff as needed, and they
would be tied to the underlying subsystems and drivers, graphics in
case of DisplayPort. The drivers for the partner alternate modes would
take care of the actual communication with the alternate mode with
VDMs if needed (but not necessarily), and they would need to be tied
to the port alternate modes. In practice the driver for both the port
and the partner alternate modes will be the same (in the same
location) obviously, at least in most cases.

I think a bus would allow us to support several ways of handling the
alternate modes on different platforms. It would work fine also on
platforms that had no use for it of course, like platforms where
firmware or EC takes care of most things related to Type-C.

But please note that since this is just a high level idea still, we
wouldn't for example need to create an actual bus if there is no use
for it, but since we have the SVIDs that can be used for matching,
then why not try take advantage of them, right.

How would something like that sound to you guys?


That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


In this case I think you would only need to register a driver with the
bus in case you want the handle to the device for the alternate mode.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected
directly to the SoC and it's using DTB/OF.


Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?


Using extcon I would have a driver that is both typec class and extcon driver 
at the same time
since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?
I don't know much about the wcove PMIC and what alternate modes it might 
support but I
guess that driver would end up in the same place.


What alternate modes systems with WhiskeyCove supports depends on
the platform. WhiskeyCove PMIC (as in Power Management IC) is
available on a few Intel Atom platforms. The USB Type-C PHY in it
provides a simple USB PD transceiver that does not touch the actual
communication with the partners. The communication needs to be done in
software, including dealing with alternate modes.

I'm not planning on using extcon for anything with WhiskeyCove. I
don't have any use for it. It looks to me that extcon is used just as
a tool to create software couplings in many cases, and I'm not
completely comfortable with that.



For rk3399 we went back and forth trying other approaches, but using extcon
turned out to be the most straightforward mechanism; everything else would
just have replicated what extcon already provides. Having said that,
I am open to other approaches. The rk3399 implementation may be seen as an
example on what is required, specifically when it comes to the interaction
between cable, DP, and the Type-C phy.

Guenter


In my case with DP altmode, if we had for example muxes to take care
of, I don't think it would be a problem to tie the driver for the mux
to the graphics directly, so basically make it part of the graphics
stack. That driver would be the port (and partner) altmode driver.


Do we need to further standardize attributes under (each) specific alternate 
mode to
include things such as HPD for the DP mode?


I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) 

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

[Peter Chen]

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> +/* --- */
> +/* Driver callbacks to report role updates */
> +
> +/**
> + * typec_set_data_role - Report data role change
> + * @port: The USB Type-C Port where the role was changed
> + * @role: The new data role
> + *
> + * This routine is used by the port drivers to report data role changes.
> + */
> +void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
> +{
> + if (port->data_role == role)
> + return;
> +
> + port->data_role = role;
> + sysfs_notify(>dev.kobj, NULL, "data_role");
> + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_set_data_role);
> +

Hi Keikki,

Have you tested this interface with real dual-role controller/board?
What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.

Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role

Maybe we can enhance Roger's drd framework [1] to fulfill that.

[1] https://lwn.net/Articles/682531/
-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> +/* --- */
> +/* Driver callbacks to report role updates */
> +
> +/**
> + * typec_set_data_role - Report data role change
> + * @port: The USB Type-C Port where the role was changed
> + * @role: The new data role
> + *
> + * This routine is used by the port drivers to report data role changes.
> + */
> +void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
> +{
> + if (port->data_role == role)
> + return;
> +
> + port->data_role = role;
> + sysfs_notify(>dev.kobj, NULL, "data_role");
> + kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> +}
> +EXPORT_SYMBOL_GPL(typec_set_data_role);
> +

Hi Keikki,

Have you tested this interface with real dual-role controller/board?
What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.

Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role

Maybe we can enhance Roger's drd framework [1] to fulfill that.

[1] https://lwn.net/Articles/682531/
-- 

Best Regards,
Peter Chen

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

[Guenter Roeck]

On 03/02/2017 11:29 PM, Mats Karrman wrote:

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected

directly to the SoC and it's using DTB/OF.



Sounds quite similar to rk3399 (Samsung Chromebook Plus). I have not ported the 
Type-C class code
to that system yet, but it does use the extcon cros_ec driver, and DP 
connectivity is signaled
to the DP code through extcon events.


Using extcon I would have a driver that is both typec class and extcon driver 
at the same time

since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?


See above. The code is available in the repository mentioned above.

Guenter


I don't know much about the wcove PMIC and what alternate modes it might 
support but I

guess that driver would end up in the same place.

Do we need to further standardize attributes under (each) specific alternate 
mode to

include things such as HPD for the DP mode?

BR // Mats

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

[Guenter Roeck]

On 03/02/2017 11:29 PM, Mats Karrman wrote:

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected

directly to the SoC and it's using DTB/OF.



Sounds quite similar to rk3399 (Samsung Chromebook Plus). I have not ported the 
Type-C class code
to that system yet, but it does use the extcon cros_ec driver, and DP 
connectivity is signaled
to the DP code through extcon events.


Using extcon I would have a driver that is both typec class and extcon driver 
at the same time

since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?


See above. The code is available in the repository mentioned above.

Guenter


I don't know much about the wcove PMIC and what alternate modes it might 
support but I

guess that driver would end up in the same place.

Do we need to further standardize attributes under (each) specific alternate 
mode to

include things such as HPD for the DP mode?

BR // Mats

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

[Heikki Krogerus]

Hi,

On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> On 2017-03-03 04:13, Guenter Roeck wrote:
> 
> > On 03/02/2017 07:22 AM, Mats Karrman wrote:
> > > 
> > > Looking forward, one thing I have run into is how to connect the typec 
> > > driver with a
> > > driver for an alternate mode. E.g. the DisplayPort Alternate Mode 
> > > specification
> > > includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as 
> > > bits in the
> > > Attention message. These signals are needed by the DisplayPort driver to 
> > > know when to
> > > start negotiation etc.
> > > Have you got any thoughts on how to standardize such interfaces?

My idea was to have something like the altmode "bus" at one point.
We create a device for every alternate mode registered in typec class,
so the alternate modes registered for the ports and partners would
simply be attached to the altmode bus. There would be a bus per port
of course.

The drivers for the port alternate modes would take care of things
like muxing and other platform specific stuff as needed, and they
would be tied to the underlying subsystems and drivers, graphics in
case of DisplayPort. The drivers for the partner alternate modes would
take care of the actual communication with the alternate mode with
VDMs if needed (but not necessarily), and they would need to be tied
to the port alternate modes. In practice the driver for both the port
and the partner alternate modes will be the same (in the same
location) obviously, at least in most cases.

I think a bus would allow us to support several ways of handling the
alternate modes on different platforms. It would work fine also on
platforms that had no use for it of course, like platforms where
firmware or EC takes care of most things related to Type-C.

But please note that since this is just a high level idea still, we
wouldn't for example need to create an actual bus if there is no use
for it, but since we have the SVIDs that can be used for matching,
then why not try take advantage of them, right.

How would something like that sound to you guys?

> > That really depends on the lower level driver. For Chromebooks, where the 
> > Type-C
> > Protocol Manager runs on the EC, we have an extcon driver which reports the 
> > pin states
> > to the graphics drivers and connects to the Type-C class code using the 
> > Type-C class
> > API. I still need to update, re-test, and publish that code. The published 
> > code in
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
> > chromeos-4.4,
> > shows how it can be done, though that code currently still uses the Android 
> > Type-C
> > infrastructure.

In this case I think you would only need to register a driver with the
bus in case you want the handle to the device for the alternate mode.

> OK, thanks!
> 
> My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
> controller connected
> directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?

> Using extcon I would have a driver that is both typec class and extcon driver 
> at the same time
> since I can't share the access to the typec phy. Is this done elsewhere in 
> the kernel?
> I don't know much about the wcove PMIC and what alternate modes it might 
> support but I
> guess that driver would end up in the same place.

What alternate modes systems with WhiskeyCove supports depends on
the platform. WhiskeyCove PMIC (as in Power Management IC) is
available on a few Intel Atom platforms. The USB Type-C PHY in it
provides a simple USB PD transceiver that does not touch the actual
communication with the partners. The communication needs to be done in
software, including dealing with alternate modes.

I'm not planning on using extcon for anything with WhiskeyCove. I
don't have any use for it. It looks to me that extcon is used just as
a tool to create software couplings in many cases, and I'm not
completely comfortable with that.

In my case with DP altmode, if we had for example muxes to take care
of, I don't think it would be a problem to tie the driver for the mux
to the graphics directly, so basically make it part of the graphics
stack. That driver would be the port (and partner) altmode driver.

> Do we need to further standardize attributes under (each) specific alternate 
> mode to
> include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi,

On Fri, Mar 03, 2017 at 08:29:18AM +0100, Mats Karrman wrote:
> On 2017-03-03 04:13, Guenter Roeck wrote:
> 
> > On 03/02/2017 07:22 AM, Mats Karrman wrote:
> > > 
> > > Looking forward, one thing I have run into is how to connect the typec 
> > > driver with a
> > > driver for an alternate mode. E.g. the DisplayPort Alternate Mode 
> > > specification
> > > includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as 
> > > bits in the
> > > Attention message. These signals are needed by the DisplayPort driver to 
> > > know when to
> > > start negotiation etc.
> > > Have you got any thoughts on how to standardize such interfaces?

My idea was to have something like the altmode "bus" at one point.
We create a device for every alternate mode registered in typec class,
so the alternate modes registered for the ports and partners would
simply be attached to the altmode bus. There would be a bus per port
of course.

The drivers for the port alternate modes would take care of things
like muxing and other platform specific stuff as needed, and they
would be tied to the underlying subsystems and drivers, graphics in
case of DisplayPort. The drivers for the partner alternate modes would
take care of the actual communication with the alternate mode with
VDMs if needed (but not necessarily), and they would need to be tied
to the port alternate modes. In practice the driver for both the port
and the partner alternate modes will be the same (in the same
location) obviously, at least in most cases.

I think a bus would allow us to support several ways of handling the
alternate modes on different platforms. It would work fine also on
platforms that had no use for it of course, like platforms where
firmware or EC takes care of most things related to Type-C.

But please note that since this is just a high level idea still, we
wouldn't for example need to create an actual bus if there is no use
for it, but since we have the SVIDs that can be used for matching,
then why not try take advantage of them, right.

How would something like that sound to you guys?

> > That really depends on the lower level driver. For Chromebooks, where the 
> > Type-C
> > Protocol Manager runs on the EC, we have an extcon driver which reports the 
> > pin states
> > to the graphics drivers and connects to the Type-C class code using the 
> > Type-C class
> > API. I still need to update, re-test, and publish that code. The published 
> > code in
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
> > chromeos-4.4,
> > shows how it can be done, though that code currently still uses the Android 
> > Type-C
> > infrastructure.

In this case I think you would only need to register a driver with the
bus in case you want the handle to the device for the alternate mode.

> OK, thanks!
> 
> My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
> controller connected
> directly to the SoC and it's using DTB/OF.

Is this "DP controller" a controller that is capable of taking care of
the USB Power Delivery communication with the partner regarding
DisplayPort alternate mode?

> Using extcon I would have a driver that is both typec class and extcon driver 
> at the same time
> since I can't share the access to the typec phy. Is this done elsewhere in 
> the kernel?
> I don't know much about the wcove PMIC and what alternate modes it might 
> support but I
> guess that driver would end up in the same place.

What alternate modes systems with WhiskeyCove supports depends on
the platform. WhiskeyCove PMIC (as in Power Management IC) is
available on a few Intel Atom platforms. The USB Type-C PHY in it
provides a simple USB PD transceiver that does not touch the actual
communication with the partners. The communication needs to be done in
software, including dealing with alternate modes.

I'm not planning on using extcon for anything with WhiskeyCove. I
don't have any use for it. It looks to me that extcon is used just as
a tool to create software couplings in many cases, and I'm not
completely comfortable with that.

In my case with DP altmode, if we had for example muxes to take care
of, I don't think it would be a problem to tie the driver for the mux
to the graphics directly, so basically make it part of the graphics
stack. That driver would be the port (and partner) altmode driver.

> Do we need to further standardize attributes under (each) specific alternate 
> mode to
> include things such as HPD for the DP mode?

I'm not completely sure what kind of system you have, but I would
imagine that if we had the bus, your DP controller driver would be the
port (and partner) alternate mode driver. The bus would bind you to
the typec phy.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Peter,

On Fri, Mar 03, 2017 at 11:35:29AM +0800, Peter Chen wrote:
> On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> > +/* --- */
> > +/* Driver callbacks to report role updates */
> > +
> > +/**
> > + * typec_set_data_role - Report data role change
> > + * @port: The USB Type-C Port where the role was changed
> > + * @role: The new data role
> > + *
> > + * This routine is used by the port drivers to report data role changes.
> > + */
> > +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> > role)
> > +{
> > +   if (port->data_role == role)
> > +   return;
> > +
> > +   port->data_role = role;
> > +   sysfs_notify(>dev.kobj, NULL, "data_role");
> > +   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_set_data_role);
> > +
> 
> Hi Keikki,
> 
> Have you tested this interface with real dual-role controller/board?

Yes. Our boards are mostly USB dual-role capable.

> What interface you use when you receive this event to handle
> dual-role switch? I am wonder if a common dual-role class is
> needed, then we can have a common user utility.
> 
> Eg, if "data_role" has changed, the udev can echo "data_role" to
> /sys/class/usb-dual-role/role

No. If the partner executes successfully for example DR_Swap message,
the kernel has to take care everything that is needed for the role to
be what ever was negotiated on its own. User space can't be involved
with that.


Thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Peter,

On Fri, Mar 03, 2017 at 11:35:29AM +0800, Peter Chen wrote:
> On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> > +/* --- */
> > +/* Driver callbacks to report role updates */
> > +
> > +/**
> > + * typec_set_data_role - Report data role change
> > + * @port: The USB Type-C Port where the role was changed
> > + * @role: The new data role
> > + *
> > + * This routine is used by the port drivers to report data role changes.
> > + */
> > +void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> > role)
> > +{
> > +   if (port->data_role == role)
> > +   return;
> > +
> > +   port->data_role = role;
> > +   sysfs_notify(>dev.kobj, NULL, "data_role");
> > +   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> > +}
> > +EXPORT_SYMBOL_GPL(typec_set_data_role);
> > +
> 
> Hi Keikki,
> 
> Have you tested this interface with real dual-role controller/board?

Yes. Our boards are mostly USB dual-role capable.

> What interface you use when you receive this event to handle
> dual-role switch? I am wonder if a common dual-role class is
> needed, then we can have a common user utility.
> 
> Eg, if "data_role" has changed, the udev can echo "data_role" to
> /sys/class/usb-dual-role/role

No. If the partner executes successfully for example DR_Swap message,
the kernel has to take care everything that is needed for the role to
be what ever was negotiated on its own. User space can't be involved
with that.


Thanks,

-- 
heikki

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

[Guenter Roeck]

On 03/02/2017 08:52 PM, Peter Chen wrote:

On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:

On 03/02/2017 07:35 PM, Peter Chen wrote:

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:

+/* --- */
+/* Driver callbacks to report role updates */
+
+/**
+ * typec_set_data_role - Report data role change
+ * @port: The USB Type-C Port where the role was changed
+ * @role: The new data role
+ *
+ * This routine is used by the port drivers to report data role changes.
+ */
+void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
+{
+   if (port->data_role == role)
+   return;
+
+   port->data_role = role;
+   sysfs_notify(>dev.kobj, NULL, "data_role");
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
+}
+EXPORT_SYMBOL_GPL(typec_set_data_role);
+


Hi Keikki,

Have you tested this interface with real dual-role controller/board?


If it helps, my primary test system is a HP Chromebook 13 G1.


What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.


I don't really understand "What interface you use when you receive
this event". Can you explain ?



I mean "How to trigger kernel USB controller driver do role switch?"



I think this should be handled by the lower level driver. I am wide open
to other ideas, though.



Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


That sounds like a kernel event delivered to user space via udev or
sysfs notification and returned back into the kernel through a sysfs
attribute. Do I understand that correctly ?



Yes.



That doesn't sound like a good idea to me, and I don't see a technical reason
to require it.

Thanks,
Guenter

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

[Guenter Roeck]

On 03/02/2017 08:52 PM, Peter Chen wrote:

On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:

On 03/02/2017 07:35 PM, Peter Chen wrote:

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:

+/* --- */
+/* Driver callbacks to report role updates */
+
+/**
+ * typec_set_data_role - Report data role change
+ * @port: The USB Type-C Port where the role was changed
+ * @role: The new data role
+ *
+ * This routine is used by the port drivers to report data role changes.
+ */
+void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
+{
+   if (port->data_role == role)
+   return;
+
+   port->data_role = role;
+   sysfs_notify(>dev.kobj, NULL, "data_role");
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
+}
+EXPORT_SYMBOL_GPL(typec_set_data_role);
+


Hi Keikki,

Have you tested this interface with real dual-role controller/board?


If it helps, my primary test system is a HP Chromebook 13 G1.


What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.


I don't really understand "What interface you use when you receive
this event". Can you explain ?



I mean "How to trigger kernel USB controller driver do role switch?"



I think this should be handled by the lower level driver. I am wide open
to other ideas, though.



Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


That sounds like a kernel event delivered to user space via udev or
sysfs notification and returned back into the kernel through a sysfs
attribute. Do I understand that correctly ?



Yes.



That doesn't sound like a good idea to me, and I don't see a technical reason
to require it.

Thanks,
Guenter

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

[Peter Chen]

On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:
> On 03/02/2017 07:35 PM, Peter Chen wrote:
> >On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> >>+/* --- */
> >>+/* Driver callbacks to report role updates */
> >>+
> >>+/**
> >>+ * typec_set_data_role - Report data role change
> >>+ * @port: The USB Type-C Port where the role was changed
> >>+ * @role: The new data role
> >>+ *
> >>+ * This routine is used by the port drivers to report data role changes.
> >>+ */
> >>+void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> >>role)
> >>+{
> >>+   if (port->data_role == role)
> >>+   return;
> >>+
> >>+   port->data_role = role;
> >>+   sysfs_notify(>dev.kobj, NULL, "data_role");
> >>+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> >>+}
> >>+EXPORT_SYMBOL_GPL(typec_set_data_role);
> >>+
> >
> >Hi Keikki,
> >
> >Have you tested this interface with real dual-role controller/board?
> 
> If it helps, my primary test system is a HP Chromebook 13 G1.
> 
> >What interface you use when you receive this event to handle
> >dual-role switch? I am wonder if a common dual-role class is
> >needed, then we can have a common user utility.
> 
> I don't really understand "What interface you use when you receive
> this event". Can you explain ?
> 

I mean "How to trigger kernel USB controller driver do role switch?"

> >
> >Eg, if "data_role" has changed, the udev can echo "data_role" to
> >/sys/class/usb-dual-role/role
> >
> That sounds like a kernel event delivered to user space via udev or
> sysfs notification and returned back into the kernel through a sysfs
> attribute. Do I understand that correctly ?
> 

Yes.

-- 

Best Regards,
Peter Chen

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

[Peter Chen]

On Thu, Mar 02, 2017 at 08:29:07PM -0800, Guenter Roeck wrote:
> On 03/02/2017 07:35 PM, Peter Chen wrote:
> >On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:
> >>+/* --- */
> >>+/* Driver callbacks to report role updates */
> >>+
> >>+/**
> >>+ * typec_set_data_role - Report data role change
> >>+ * @port: The USB Type-C Port where the role was changed
> >>+ * @role: The new data role
> >>+ *
> >>+ * This routine is used by the port drivers to report data role changes.
> >>+ */
> >>+void typec_set_data_role(struct typec_port *port, enum typec_data_role 
> >>role)
> >>+{
> >>+   if (port->data_role == role)
> >>+   return;
> >>+
> >>+   port->data_role = role;
> >>+   sysfs_notify(>dev.kobj, NULL, "data_role");
> >>+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
> >>+}
> >>+EXPORT_SYMBOL_GPL(typec_set_data_role);
> >>+
> >
> >Hi Keikki,
> >
> >Have you tested this interface with real dual-role controller/board?
> 
> If it helps, my primary test system is a HP Chromebook 13 G1.
> 
> >What interface you use when you receive this event to handle
> >dual-role switch? I am wonder if a common dual-role class is
> >needed, then we can have a common user utility.
> 
> I don't really understand "What interface you use when you receive
> this event". Can you explain ?
> 

I mean "How to trigger kernel USB controller driver do role switch?"

> >
> >Eg, if "data_role" has changed, the udev can echo "data_role" to
> >/sys/class/usb-dual-role/role
> >
> That sounds like a kernel event delivered to user space via udev or
> sysfs notification and returned back into the kernel through a sysfs
> attribute. Do I understand that correctly ?
> 

Yes.

-- 

Best Regards,
Peter Chen

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

[Enric Balletbo Serra]

2017-03-03 8:29 GMT+01:00 Mats Karrman :
> On 2017-03-03 04:13, Guenter Roeck wrote:
>
>> On 03/02/2017 07:22 AM, Mats Karrman wrote:
>>>
>>> 
>>> Looking forward, one thing I have run into is how to connect the typec
>>> driver with a
>>> driver for an alternate mode. E.g. the DisplayPort Alternate Mode
>>> specification
>>> includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as
>>> bits in the
>>> Attention message. These signals are needed by the DisplayPort driver to
>>> know when to
>>> start negotiation etc.
>>> Have you got any thoughts on how to standardize such interfaces?
>>
>> That really depends on the lower level driver. For Chromebooks, where the
>> Type-C
>> Protocol Manager runs on the EC, we have an extcon driver which reports
>> the pin states
>> to the graphics drivers and connects to the Type-C class code using the
>> Type-C class
>> API. I still need to update, re-test, and publish that code. The published
>> code in
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch
>> chromeos-4.4,
>> shows how it can be done, though that code currently still uses the
>> Android Type-C
>> infrastructure.
>

On a side note, the cros-ec extcon part for DP is currently discussed
here [1]. This driver works together with the cdn-dp driver that just
landed in mainline. So comments and feedback are welcome

[1] https://lkml.org/lkml/2017/3/1/301

>
> OK, thanks!
>
> My system is a bit different. It's an i.MX6 SoC with the typec phy and DP
> controller connected
>
> directly to the SoC and it's using DTB/OF.
>
> Using extcon I would have a driver that is both typec class and extcon
> driver at the same time
>
> since I can't share the access to the typec phy. Is this done elsewhere in
> the kernel?
>
> I don't know much about the wcove PMIC and what alternate modes it might
> support but I
>
> guess that driver would end up in the same place.
>
> Do we need to further standardize attributes under (each) specific alternate
> mode to
>
> include things such as HPD for the DP mode?
>
> BR // Mats
>

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

[Enric Balletbo Serra]

2017-03-03 8:29 GMT+01:00 Mats Karrman :
> On 2017-03-03 04:13, Guenter Roeck wrote:
>
>> On 03/02/2017 07:22 AM, Mats Karrman wrote:
>>>
>>> 
>>> Looking forward, one thing I have run into is how to connect the typec
>>> driver with a
>>> driver for an alternate mode. E.g. the DisplayPort Alternate Mode
>>> specification
>>> includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as
>>> bits in the
>>> Attention message. These signals are needed by the DisplayPort driver to
>>> know when to
>>> start negotiation etc.
>>> Have you got any thoughts on how to standardize such interfaces?
>>
>> That really depends on the lower level driver. For Chromebooks, where the
>> Type-C
>> Protocol Manager runs on the EC, we have an extcon driver which reports
>> the pin states
>> to the graphics drivers and connects to the Type-C class code using the
>> Type-C class
>> API. I still need to update, re-test, and publish that code. The published
>> code in
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch
>> chromeos-4.4,
>> shows how it can be done, though that code currently still uses the
>> Android Type-C
>> infrastructure.
>

On a side note, the cros-ec extcon part for DP is currently discussed
here [1]. This driver works together with the cdn-dp driver that just
landed in mainline. So comments and feedback are welcome

[1] https://lkml.org/lkml/2017/3/1/301

>
> OK, thanks!
>
> My system is a bit different. It's an i.MX6 SoC with the typec phy and DP
> controller connected
>
> directly to the SoC and it's using DTB/OF.
>
> Using extcon I would have a driver that is both typec class and extcon
> driver at the same time
>
> since I can't share the access to the typec phy. Is this done elsewhere in
> the kernel?
>
> I don't know much about the wcove PMIC and what alternate modes it might
> support but I
>
> guess that driver would end up in the same place.
>
> Do we need to further standardize attributes under (each) specific alternate
> mode to
>
> include things such as HPD for the DP mode?
>
> BR // Mats
>

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

[Mats Karrman]

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected

directly to the SoC and it's using DTB/OF.

Using extcon I would have a driver that is both typec class and extcon driver 
at the same time

since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?

I don't know much about the wcove PMIC and what alternate modes it might 
support but I

guess that driver would end up in the same place.

Do we need to further standardize attributes under (each) specific alternate 
mode to

include things such as HPD for the DP mode?

BR // Mats

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

[Mats Karrman]

On 2017-03-03 04:13, Guenter Roeck wrote:


On 03/02/2017 07:22 AM, Mats Karrman wrote:


Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.


OK, thanks!

My system is a bit different. It's an i.MX6 SoC with the typec phy and DP 
controller connected

directly to the SoC and it's using DTB/OF.

Using extcon I would have a driver that is both typec class and extcon driver 
at the same time

since I can't share the access to the typec phy. Is this done elsewhere in the 
kernel?

I don't know much about the wcove PMIC and what alternate modes it might 
support but I

guess that driver would end up in the same place.

Do we need to further standardize attributes under (each) specific alternate 
mode to

include things such as HPD for the DP mode?

BR // Mats

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

[Guenter Roeck]

On 03/02/2017 07:35 PM, Peter Chen wrote:

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:

+/* --- */
+/* Driver callbacks to report role updates */
+
+/**
+ * typec_set_data_role - Report data role change
+ * @port: The USB Type-C Port where the role was changed
+ * @role: The new data role
+ *
+ * This routine is used by the port drivers to report data role changes.
+ */
+void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
+{
+   if (port->data_role == role)
+   return;
+
+   port->data_role = role;
+   sysfs_notify(>dev.kobj, NULL, "data_role");
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
+}
+EXPORT_SYMBOL_GPL(typec_set_data_role);
+


Hi Keikki,

Have you tested this interface with real dual-role controller/board?


If it helps, my primary test system is a HP Chromebook 13 G1.


What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.


I don't really understand "What interface you use when you receive
this event". Can you explain ?



Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


That sounds like a kernel event delivered to user space via udev or
sysfs notification and returned back into the kernel through a sysfs
attribute. Do I understand that correctly ?

Thanks,
Guenter


Maybe we can enhance Roger's drd framework [1] to fulfill that.

[1] https://lwn.net/Articles/682531/

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

[Guenter Roeck]

On 03/02/2017 07:35 PM, Peter Chen wrote:

On Tue, Feb 21, 2017 at 05:24:04PM +0300, Heikki Krogerus wrote:

+/* --- */
+/* Driver callbacks to report role updates */
+
+/**
+ * typec_set_data_role - Report data role change
+ * @port: The USB Type-C Port where the role was changed
+ * @role: The new data role
+ *
+ * This routine is used by the port drivers to report data role changes.
+ */
+void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
+{
+   if (port->data_role == role)
+   return;
+
+   port->data_role = role;
+   sysfs_notify(>dev.kobj, NULL, "data_role");
+   kobject_uevent(>dev.kobj, KOBJ_CHANGE);
+}
+EXPORT_SYMBOL_GPL(typec_set_data_role);
+


Hi Keikki,

Have you tested this interface with real dual-role controller/board?


If it helps, my primary test system is a HP Chromebook 13 G1.


What interface you use when you receive this event to handle
dual-role switch? I am wonder if a common dual-role class is
needed, then we can have a common user utility.


I don't really understand "What interface you use when you receive
this event". Can you explain ?



Eg, if "data_role" has changed, the udev can echo "data_role" to
/sys/class/usb-dual-role/role


That sounds like a kernel event delivered to user space via udev or
sysfs notification and returned back into the kernel through a sysfs
attribute. Do I understand that correctly ?

Thanks,
Guenter


Maybe we can enhance Roger's drd framework [1] to fulfill that.

[1] https://lwn.net/Articles/682531/

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

[Guenter Roeck]

On 03/02/2017 07:22 AM, Mats Karrman wrote:

Hi Heikki,

Good to see things are happening with Type-C!

On 2017-02-21 15:24, Heikki Krogerus wrote:


...
+When connected, the partner will be presented also as its own device under
+/sys/class/typec/. The parent of the partner device will always be the port it
+is attached to. The partner attached to port "port0" will be named
+"port0-partner". Full path to the device would be
+/sys/class/typec/port0/port0-partner/.


A "/port0" too much?


+
+The cable and the two plugs on it may also be optionally presented as their own
+devices under /sys/class/typec/. The cable attached to the port "port0" port
+will be named port0-cable and the plug on the SOP Prime end (see USB Power
+Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP
+Double Prime end "port0-plug1". The parent of a cable will always be the port,
+and the parent of the cable plugs will always be the cable.
+
+If the port, partner or cable plug support Alternate Modes, every supported
+Alternate Mode SVID will have their own device describing them. The Alternate
+Modes will not be attached to the typec class. The parent of an alternate mode
+will be the device that supports it, so for example an alternate mode of
+port0-partner will bees presented under /sys/class/typec/port0-partner/. Every


bees?


+mode that is supported will have its own group under the Alternate Mode device
+named "mode", for example /sys/class/typec/port0//mode1/.
+The requests for entering/exiting a mode can be done with "active" attribute
+file in that group.
+
...


I'm hoping to find time to upgrade the kernel and try these patches in my 
system.

Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?



That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.

Guenter

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

[Guenter Roeck]

On 03/02/2017 07:22 AM, Mats Karrman wrote:

Hi Heikki,

Good to see things are happening with Type-C!

On 2017-02-21 15:24, Heikki Krogerus wrote:


...
+When connected, the partner will be presented also as its own device under
+/sys/class/typec/. The parent of the partner device will always be the port it
+is attached to. The partner attached to port "port0" will be named
+"port0-partner". Full path to the device would be
+/sys/class/typec/port0/port0-partner/.


A "/port0" too much?


+
+The cable and the two plugs on it may also be optionally presented as their own
+devices under /sys/class/typec/. The cable attached to the port "port0" port
+will be named port0-cable and the plug on the SOP Prime end (see USB Power
+Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP
+Double Prime end "port0-plug1". The parent of a cable will always be the port,
+and the parent of the cable plugs will always be the cable.
+
+If the port, partner or cable plug support Alternate Modes, every supported
+Alternate Mode SVID will have their own device describing them. The Alternate
+Modes will not be attached to the typec class. The parent of an alternate mode
+will be the device that supports it, so for example an alternate mode of
+port0-partner will bees presented under /sys/class/typec/port0-partner/. Every


bees?


+mode that is supported will have its own group under the Alternate Mode device
+named "mode", for example /sys/class/typec/port0//mode1/.
+The requests for entering/exiting a mode can be done with "active" attribute
+file in that group.
+
...


I'm hoping to find time to upgrade the kernel and try these patches in my 
system.

Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?



That really depends on the lower level driver. For Chromebooks, where the Type-C
Protocol Manager runs on the EC, we have an extcon driver which reports the pin 
states
to the graphics drivers and connects to the Type-C class code using the Type-C 
class
API. I still need to update, re-test, and publish that code. The published code 
in
https://chromium.googlesource.com/chromiumos/third_party/kernel/, branch 
chromeos-4.4,
shows how it can be done, though that code currently still uses the Android 
Type-C
infrastructure.

Guenter

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

[Mats Karrman]

Hi Heikki,

Good to see things are happening with Type-C!

On 2017-02-21 15:24, Heikki Krogerus wrote:


...
+When connected, the partner will be presented also as its own device under
+/sys/class/typec/. The parent of the partner device will always be the port it
+is attached to. The partner attached to port "port0" will be named
+"port0-partner". Full path to the device would be
+/sys/class/typec/port0/port0-partner/.


A "/port0" too much?


+
+The cable and the two plugs on it may also be optionally presented as their own
+devices under /sys/class/typec/. The cable attached to the port "port0" port
+will be named port0-cable and the plug on the SOP Prime end (see USB Power
+Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP
+Double Prime end "port0-plug1". The parent of a cable will always be the port,
+and the parent of the cable plugs will always be the cable.
+
+If the port, partner or cable plug support Alternate Modes, every supported
+Alternate Mode SVID will have their own device describing them. The Alternate
+Modes will not be attached to the typec class. The parent of an alternate mode
+will be the device that supports it, so for example an alternate mode of
+port0-partner will bees presented under /sys/class/typec/port0-partner/. Every


bees?


+mode that is supported will have its own group under the Alternate Mode device
+named "mode", for example /sys/class/typec/port0//mode1/.
+The requests for entering/exiting a mode can be done with "active" attribute
+file in that group.
+
...


I'm hoping to find time to upgrade the kernel and try these patches in my 
system.

Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

BR // Mats

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

[Mats Karrman]

Hi Heikki,

Good to see things are happening with Type-C!

On 2017-02-21 15:24, Heikki Krogerus wrote:


...
+When connected, the partner will be presented also as its own device under
+/sys/class/typec/. The parent of the partner device will always be the port it
+is attached to. The partner attached to port "port0" will be named
+"port0-partner". Full path to the device would be
+/sys/class/typec/port0/port0-partner/.


A "/port0" too much?


+
+The cable and the two plugs on it may also be optionally presented as their own
+devices under /sys/class/typec/. The cable attached to the port "port0" port
+will be named port0-cable and the plug on the SOP Prime end (see USB Power
+Delivery Specification ch. 2.4) will be named "port0-plug0" and on the SOP
+Double Prime end "port0-plug1". The parent of a cable will always be the port,
+and the parent of the cable plugs will always be the cable.
+
+If the port, partner or cable plug support Alternate Modes, every supported
+Alternate Mode SVID will have their own device describing them. The Alternate
+Modes will not be attached to the typec class. The parent of an alternate mode
+will be the device that supports it, so for example an alternate mode of
+port0-partner will bees presented under /sys/class/typec/port0-partner/. Every


bees?


+mode that is supported will have its own group under the Alternate Mode device
+named "mode", for example /sys/class/typec/port0//mode1/.
+The requests for entering/exiting a mode can be done with "active" attribute
+file in that group.
+
...


I'm hoping to find time to upgrade the kernel and try these patches in my 
system.

Looking forward, one thing I have run into is how to connect the typec driver 
with a
driver for an alternate mode. E.g. the DisplayPort Alternate Mode specification
includes the HPD (hot plug) and HPD-INT (hot plug interrupt) signals as bits in 
the
Attention message. These signals are needed by the DisplayPort driver to know 
when to
start negotiation etc.
Have you got any thoughts on how to standardize such interfaces?

BR // Mats