[PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Badhri Jagan Sridharan]

With this CL the lower level drivers are reponsible to check and make sure
that the role swap can be performed.

This facilitates the lower level driver to attempt to perform role swap
through initial connection process.

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

Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
---
 Documentation/ABI/testing/sysfs-class-typec |  7 +++++--
 drivers/usb/typec/typec.c                   | 10 ----------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index d4a3d23eb09c..ba4ca684adb2 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -20,13 +20,16 @@ Date:		April 2017
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
 Description:
 		The supported power roles. This attribute can be used to request
-		power role swap on the port when the port supports USB Power
-		Delivery. Swapping is supported as synchronous operation, so
+		power role swap. Swapping is supported as synchronous operation, so
 		write(2) to the attribute will not return until the operation
 		has finished. The attribute is notified about role changes so
 		that poll(2) on the attribute wakes up. Change on the role will
 		also generate uevent KOBJ_CHANGE. The current role is show in
 		brackets, for example "[source] sink" when in source mode.
+		When both the port and the port-partner supports USB Power
+		Delivery, the PR_SWAP command is used to perform the role swap.
+		Otherwise, the port roles would be re-resolved by forcing
+		a disconnect and reconnect.
 
 		Valid values: source, sink
 
diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..fd2d661665fa 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -887,21 +887,11 @@ static ssize_t power_role_store(struct device *dev,
 	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;
-- 
2.13.0.303.g4ebf302169-goog

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Oliver Neukum]

Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
Sridharan:

Hi,

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

Well, as I read it, without PD once a connection is established, you
are stuck with your role. So it seems to me that blocking a later
attempt to change it makes sense.

	Regards
		Oliver

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Greg Kroah-Hartman]

On Wed, May 17, 2017 at 12:32:19AM -0700, Badhri Jagan Sridharan wrote:
> With this CL the lower level drivers are reponsible to check and make sure
> that the role swap can be performed.

What is a "CL"?

thanks,

greg k-h

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Guenter Roeck]

On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> Sridharan:
>
> Hi,
>
>> "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."
>
> Well, as I read it, without PD once a connection is established, you
> are stuck with your role. So it seems to me that blocking a later
> attempt to change it makes sense.
>

That seems to be a harsh and not very user friendly reading of the specification.

I would argue that the user doesn't care if the partner supports PD or not
when selecting a role, and I would prefer to provide an implementation which is
as user friendly as possible.

Thanks,
Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Guenter Roeck]

On 05/17/2017 01:08 AM, Greg Kroah-Hartman wrote:
> On Wed, May 17, 2017 at 12:32:19AM -0700, Badhri Jagan Sridharan wrote:
>> With this CL the lower level drivers are reponsible to check and make sure

responsible

>> that the role swap can be performed.
>
> What is a "CL"?
>

Too much Gerrit. "Change List". Took me a while to understand what people
were talking about when talking about CLs.

Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Heikki Krogerus]

Hi guys,

On Wed, May 17, 2017 at 02:36:44AM -0700, Guenter Roeck wrote:
> On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > Sridharan:
> > 
> > Hi,
> > 
> > > "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."
> > 
> > Well, as I read it, without PD once a connection is established, you
> > are stuck with your role. So it seems to me that blocking a later
> > attempt to change it makes sense.
> > 
> 
> That seems to be a harsh and not very user friendly reading of the specification.
> 
> I would argue that the user doesn't care if the partner supports PD or not
> when selecting a role, and I would prefer to provide an implementation which is
> as user friendly as possible.

I agree. But I have to point out that at least with UCSI, the role
swapping is usually not possible without USB PD connection.

So what I'm trying to say is that we can't really promise that role
swapping will ever be possible in all cases without PD, which may mean
different behavior depending on the platform. I don't know if that is
a huge problem.

How predictable do we want this interface to function with role
swapping?


Thanks,

-- 
heikki

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Guenter Roeck]

On 05/17/2017 05:38 AM, Heikki Krogerus wrote:
> Hi guys,
>
> On Wed, May 17, 2017 at 02:36:44AM -0700, Guenter Roeck wrote:
>> On 05/17/2017 12:34 AM, Oliver Neukum wrote:
>>> Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
>>> Sridharan:
>>>
>>> Hi,
>>>
>>>> "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."
>>>
>>> Well, as I read it, without PD once a connection is established, you
>>> are stuck with your role. So it seems to me that blocking a later
>>> attempt to change it makes sense.
>>>
>>
>> That seems to be a harsh and not very user friendly reading of the specification.
>>
>> I would argue that the user doesn't care if the partner supports PD or not
>> when selecting a role, and I would prefer to provide an implementation which is
>> as user friendly as possible.
>
> I agree. But I have to point out that at least with UCSI, the role
> swapping is usually not possible without USB PD connection.
>
> So what I'm trying to say is that we can't really promise that role
> swapping will ever be possible in all cases without PD, which may mean
> different behavior depending on the platform. I don't know if that is
> a huge problem.
>
> How predictable do we want this interface to function with role
> swapping?
>

We can only do as good as we can, but we should not preclude it either.
PR_SWAP and other swap operations fail a lot in practice with many dongles,
just because the PD protocol implementation isn't always stable. So even that
won't be predictable for some time to come.

As Badhri pointed out earlier, at least some low cost devices won't support PD.
Since we are talking about high volume products, we _have_ to make it as
user friendly as we can, if for nothing else to reduce the number of customer
service calls, repeated bug filings, and, last but not least, to avoid bad press.

Thanks,
Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Heikki Krogerus]

On Wed, May 17, 2017 at 06:02:47AM -0700, Guenter Roeck wrote:
> On 05/17/2017 05:38 AM, Heikki Krogerus wrote:
> > Hi guys,
> > 
> > On Wed, May 17, 2017 at 02:36:44AM -0700, Guenter Roeck wrote:
> > > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > > > Sridharan:
> > > > 
> > > > Hi,
> > > > 
> > > > > "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."
> > > > 
> > > > Well, as I read it, without PD once a connection is established, you
> > > > are stuck with your role. So it seems to me that blocking a later
> > > > attempt to change it makes sense.
> > > > 
> > > 
> > > That seems to be a harsh and not very user friendly reading of the specification.
> > > 
> > > I would argue that the user doesn't care if the partner supports PD or not
> > > when selecting a role, and I would prefer to provide an implementation which is
> > > as user friendly as possible.
> > 
> > I agree. But I have to point out that at least with UCSI, the role
> > swapping is usually not possible without USB PD connection.
> > 
> > So what I'm trying to say is that we can't really promise that role
> > swapping will ever be possible in all cases without PD, which may mean
> > different behavior depending on the platform. I don't know if that is
> > a huge problem.
> > 
> > How predictable do we want this interface to function with role
> > swapping?
> > 
> 
> We can only do as good as we can, but we should not preclude it either.
> PR_SWAP and other swap operations fail a lot in practice with many dongles,
> just because the PD protocol implementation isn't always stable. So even that
> won't be predictable for some time to come.
> 
> As Badhri pointed out earlier, at least some low cost devices won't support PD.
> Since we are talking about high volume products, we _have_ to make it as
> user friendly as we can, if for nothing else to reduce the number of customer
> service calls, repeated bug filings, and, last but not least, to avoid bad press.

OK. So I think we just need to explain in the documentation that there
are no guarantees with role swapping.


Thanks,

-- 
heikki

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Badhri Jagan Sridharan]

For non-pd case, Should I instead say that the low level driver might optionally
choose to perform a best case effort of performing a role swap by disconnect
and reconnect ?

Does the following description look better ?

Description:
                The supported power roles. This attribute can be used to request
                power role swap. Swapping is supported as synchronous
operation, so
                write(2) to the attribute will not return until the operation
                has finished. The attribute is notified about role changes so
                that poll(2) on the attribute wakes up. Change on the role will
                also generate uevent KOBJ_CHANGE. The current role is show in
                brackets, for example "[source] sink" when in source mode.
                When both the port and the port-partner supports USB Power
                Delivery, the PR_SWAP command is used to perform the role swap.
                For non-pd case, the low level driver might optionally perform
                a best effort approach to swap port roles by forcing
                a disconnect and reconnect.

Thanks,
Badhri

On Wed, May 17, 2017 at 6:51 AM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Wed, May 17, 2017 at 06:02:47AM -0700, Guenter Roeck wrote:
> > On 05/17/2017 05:38 AM, Heikki Krogerus wrote:
> > > Hi guys,
> > >
> > > On Wed, May 17, 2017 at 02:36:44AM -0700, Guenter Roeck wrote:
> > > > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > > > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > > > > Sridharan:
> > > > >
> > > > > Hi,
> > > > >
> > > > > > "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."
> > > > >
> > > > > Well, as I read it, without PD once a connection is established, you
> > > > > are stuck with your role. So it seems to me that blocking a later
> > > > > attempt to change it makes sense.
> > > > >
> > > >
> > > > That seems to be a harsh and not very user friendly reading of the specification.
> > > >
> > > > I would argue that the user doesn't care if the partner supports PD or not
> > > > when selecting a role, and I would prefer to provide an implementation which is
> > > > as user friendly as possible.
> > >
> > > I agree. But I have to point out that at least with UCSI, the role
> > > swapping is usually not possible without USB PD connection.
> > >
> > > So what I'm trying to say is that we can't really promise that role
> > > swapping will ever be possible in all cases without PD, which may mean
> > > different behavior depending on the platform. I don't know if that is
> > > a huge problem.
> > >
> > > How predictable do we want this interface to function with role
> > > swapping?
> > >
> >
> > We can only do as good as we can, but we should not preclude it either.
> > PR_SWAP and other swap operations fail a lot in practice with many dongles,
> > just because the PD protocol implementation isn't always stable. So even that
> > won't be predictable for some time to come.
> >
> > As Badhri pointed out earlier, at least some low cost devices won't support PD.
> > Since we are talking about high volume products, we _have_ to make it as
> > user friendly as we can, if for nothing else to reduce the number of customer
> > service calls, repeated bug filings, and, last but not least, to avoid bad press.
>
> OK. So I think we just need to explain in the documentation that there
> are no guarantees with role swapping.
>
>
> Thanks,
>
> --
> heikki

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Oliver Neukum]

Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
> On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > 
> > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > Sridharan:
> > 
> > Hi,
> > 
> > > 
> > > "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."
> > 
> > Well, as I read it, without PD once a connection is established, you
> > are stuck with your role. So it seems to me that blocking a later
> > attempt to change it makes sense.
> > 
> 
> That seems to be a harsh and not very user friendly reading of the specification.
> 
> I would argue that the user doesn't care if the partner supports PD or not
> when selecting a role, and I would prefer to provide an implementation which is
> as user friendly as possible.

Data role, no question, you are right.
Power role is a different question. A switch of power role with PD should
not lead to a disconnect. Any other method might. So equating them does
not look like a good idea.

	Regards
		Oliver

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Guenter Roeck]

On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
> > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> > > 
> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> > > Sridharan:
> > > 
> > > Hi,
> > > 
> > > > 
> > > > "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."
> > > 
> > > Well, as I read it, without PD once a connection is established, you
> > > are stuck with your role. So it seems to me that blocking a later
> > > attempt to change it makes sense.
> > > 
> > 
> > That seems to be a harsh and not very user friendly reading of the specification.
> > 
> > I would argue that the user doesn't care if the partner supports PD or not
> > when selecting a role, and I would prefer to provide an implementation which is
> > as user friendly as possible.
> 
> Data role, no question, you are right.
> Power role is a different question. A switch of power role with PD should
> not lead to a disconnect. Any other method might. So equating them does
> not look like a good idea.
> 

Not really sure I can follow. If a partner does not support PD, there is no
real distinction between data role and power role, or am I missing something ?

Are you saying that, if a partner does not support PD, user space should
request a data role swap instead, and that this would be acceptable for you ?

I don't really understand the difference - a data role swap doesn't cause
a disconnect either if the partner supports PD, and it would still result
in a disconnect/reconnect sequence if the partner does not support PD -
but if it works for you, fine with me.

Badhri, would that work for us ?

Thanks,
Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Badhri Jagan Sridharan]

On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
>> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
>> > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
>> > >
>> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
>> > > Sridharan:
>> > >
>> > > Hi,
>> > >
>> > > >
>> > > > "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."
>> > >
>> > > Well, as I read it, without PD once a connection is established, you
>> > > are stuck with your role. So it seems to me that blocking a later
>> > > attempt to change it makes sense.
>> > >
>> >
>> > That seems to be a harsh and not very user friendly reading of the specification.
>> >
>> > I would argue that the user doesn't care if the partner supports PD or not
>> > when selecting a role, and I would prefer to provide an implementation which is
>> > as user friendly as possible.
>>
>> Data role, no question, you are right.
>> Power role is a different question. A switch of power role with PD should
>> not lead to a disconnect. Any other method might. So equating them does
>> not look like a good idea.
>>
>
> Not really sure I can follow. If a partner does not support PD, there is no
> real distinction between data role and power role, or am I missing something ?
>
> Are you saying that, if a partner does not support PD, user space should
> request a data role swap instead, and that this would be acceptable for you ?
>
> I don't really understand the difference - a data role swap doesn't cause
> a disconnect either if the partner supports PD, and it would still result
> in a disconnect/reconnect sequence if the partner does not support PD -
> but if it works for you, fine with me.
>
> Badhri, would that work for us ?

Yes Geunter that should work as well. Requesting non-pd role swap either through
current_power_role or current_data_role is virtually the same.

>
> Thanks,
> Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Badhri Jagan Sridharan]

Not Sure whether my previous response was sent properly.
So re-sending.

On Thu, May 18, 2017 at 2:08 PM, Badhri Jagan Sridharan
<badhri@google.com> wrote:
> On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
>>> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
>>> > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
>>> > >
>>> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
>>> > > Sridharan:
>>> > >
>>> > > Hi,
>>> > >
>>> > > >
>>> > > > "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."
>>> > >
>>> > > Well, as I read it, without PD once a connection is established, you
>>> > > are stuck with your role. So it seems to me that blocking a later
>>> > > attempt to change it makes sense.
>>> > >
>>> >
>>> > That seems to be a harsh and not very user friendly reading of the specification.
>>> >
>>> > I would argue that the user doesn't care if the partner supports PD or not
>>> > when selecting a role, and I would prefer to provide an implementation which is
>>> > as user friendly as possible.
>>>
>>> Data role, no question, you are right.
>>> Power role is a different question. A switch of power role with PD should
>>> not lead to a disconnect. Any other method might. So equating them does
>>> not look like a good idea.
>>>
>>
>> Not really sure I can follow. If a partner does not support PD, there is no
>> real distinction between data role and power role, or am I missing something ?
>>
>> Are you saying that, if a partner does not support PD, user space should
>> request a data role swap instead, and that this would be acceptable for you ?
>>
>> I don't really understand the difference - a data role swap doesn't cause
>> a disconnect either if the partner supports PD, and it would still result
>> in a disconnect/reconnect sequence if the partner does not support PD -
>> but if it works for you, fine with me.
>>
>> Badhri, would that work for us ?

Yes Geunter that should work as well. Requesting non-pd role swap either
through current_power_role or current_data_role is virtually the same.

>
> Yes Geunter that should work as well. Requesting non-pd role swap either through
> current_power_role or current_data_role is virtually the same.
>
>>
>> Thanks,
>> Guenter

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Heikki Krogerus]

On Thu, May 18, 2017 at 02:08:53PM -0700, Badhri Jagan Sridharan wrote:
> On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
> >> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
> >> > On 05/17/2017 12:34 AM, Oliver Neukum wrote:
> >> > >
> >> > > Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
> >> > > Sridharan:
> >> > >
> >> > > Hi,
> >> > >
> >> > > >
> >> > > > "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."
> >> > >
> >> > > Well, as I read it, without PD once a connection is established, you
> >> > > are stuck with your role. So it seems to me that blocking a later
> >> > > attempt to change it makes sense.
> >> > >
> >> >
> >> > That seems to be a harsh and not very user friendly reading of the specification.
> >> >
> >> > I would argue that the user doesn't care if the partner supports PD or not
> >> > when selecting a role, and I would prefer to provide an implementation which is
> >> > as user friendly as possible.
> >>
> >> Data role, no question, you are right.
> >> Power role is a different question. A switch of power role with PD should
> >> not lead to a disconnect. Any other method might. So equating them does
> >> not look like a good idea.
> >>
> >
> > Not really sure I can follow. If a partner does not support PD, there is no
> > real distinction between data role and power role, or am I missing something ?
> >
> > Are you saying that, if a partner does not support PD, user space should
> > request a data role swap instead, and that this would be acceptable for you ?
> >
> > I don't really understand the difference - a data role swap doesn't cause
> > a disconnect either if the partner supports PD, and it would still result
> > in a disconnect/reconnect sequence if the partner does not support PD -
> > but if it works for you, fine with me.
> >
> > Badhri, would that work for us ?
> 
> Yes Geunter that should work as well. Requesting non-pd role swap either through
> current_power_role or current_data_role is virtually the same.

So if I understood this correctly, we'll skip this change, right?


Thanks,

-- 
heikki

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Oliver Neukum]

Am Donnerstag, den 18.05.2017, 14:08 -0700 schrieb Badhri Jagan
Sridharan:
> > Badhri, would that work for us ?
> 
> Yes Geunter that should work as well. Requesting non-pd role swap either through
> current_power_role or current_data_role is virtually the same.

Yes and that is the issue.
If you can do PD and swap power roles, power roles will be swapped
and data connection and driver assignments stay put. However without
PD drivers will be unbound. That means that user space must be made
aware that requesting a power role change will have side effects.

	Regards
		Oliver

Re: [PATCH] usb: typec: Defer checking of valid power role swap to low level drivers

[Guenter Roeck]

On 05/19/2017 03:35 AM, Heikki Krogerus wrote:
> On Thu, May 18, 2017 at 02:08:53PM -0700, Badhri Jagan Sridharan wrote:
>> On Thu, May 18, 2017 at 9:51 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>>> On Thu, May 18, 2017 at 11:13:51AM +0200, Oliver Neukum wrote:
>>>> Am Mittwoch, den 17.05.2017, 02:36 -0700 schrieb Guenter Roeck:
>>>>> On 05/17/2017 12:34 AM, Oliver Neukum wrote:
>>>>>>
>>>>>> Am Mittwoch, den 17.05.2017, 00:32 -0700 schrieb Badhri Jagan
>>>>>> Sridharan:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>>
>>>>>>> "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."
>>>>>>
>>>>>> Well, as I read it, without PD once a connection is established, you
>>>>>> are stuck with your role. So it seems to me that blocking a later
>>>>>> attempt to change it makes sense.
>>>>>>
>>>>>
>>>>> That seems to be a harsh and not very user friendly reading of the specification.
>>>>>
>>>>> I would argue that the user doesn't care if the partner supports PD or not
>>>>> when selecting a role, and I would prefer to provide an implementation which is
>>>>> as user friendly as possible.
>>>>
>>>> Data role, no question, you are right.
>>>> Power role is a different question. A switch of power role with PD should
>>>> not lead to a disconnect. Any other method might. So equating them does
>>>> not look like a good idea.
>>>>
>>>
>>> Not really sure I can follow. If a partner does not support PD, there is no
>>> real distinction between data role and power role, or am I missing something ?
>>>
>>> Are you saying that, if a partner does not support PD, user space should
>>> request a data role swap instead, and that this would be acceptable for you ?
>>>
>>> I don't really understand the difference - a data role swap doesn't cause
>>> a disconnect either if the partner supports PD, and it would still result
>>> in a disconnect/reconnect sequence if the partner does not support PD -
>>> but if it works for you, fine with me.
>>>
>>> Badhri, would that work for us ?
>>
>> Yes Geunter that should work as well. Requesting non-pd role swap either through
>> current_power_role or current_data_role is virtually the same.
> 
> So if I understood this correctly, we'll skip this change, right?
> 
Yes.

Thanks,
Guenter