2017-03-03 18:41:41

by Peter Chen

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

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(&port->dev.kobj, NULL, "data_role");
> + kobject_uevent(&port->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


2017-03-03 06:03:39

by Guenter Roeck

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

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(&port->dev.kobj, NULL, "data_role");
>> + kobject_uevent(&port->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/
>

2017-03-03 10:18:30

by Peter Chen

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

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(&port->dev.kobj, NULL, "data_role");
> >>+ kobject_uevent(&port->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

2017-03-03 14:37:05

by Guenter Roeck

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

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(&port->dev.kobj, NULL, "data_role");
>>>> + kobject_uevent(&port->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

2017-03-03 14:40:32

by Heikki Krogerus

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

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(&port->dev.kobj, NULL, "data_role");
> > + kobject_uevent(&port->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

2017-03-06 01:16:51

by Peter Chen

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

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(&port->dev.kobj, NULL, "data_role");
> > > + kobject_uevent(&port->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

2017-03-06 01:25:09

by Peter Chen

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

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(&port->dev.kobj, NULL, "data_role");
> >>>>+ kobject_uevent(&port->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

2017-03-06 13:25:49

by Heikki Krogerus

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

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

2017-03-07 10:47:48

by Heikki Krogerus

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

On Tue, Mar 07, 2017 at 01:36:29AM +0000, 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

2017-03-07 13:00:55

by Peter Chen

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


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

2017-03-08 04:06:14

by Guenter Roeck

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

On 03/07/2017 12:57 AM, Heikki Krogerus wrote:
> On Tue, Mar 07, 2017 at 01:36:29AM +0000, 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

2017-03-08 06:50:54

by Peter Chen

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


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

2017-03-08 14:45:21

by Guenter Roeck

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

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

2017-03-09 02:00:50

by Peter Chen

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

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