Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1591677 > unrolled thread

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

Started byGuenter Roeck <linux@roeck-us.net>
First post2017-03-03 07:10 +0100
Last post2017-03-06 02:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v17 2/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2017-03-03 07:10 +0100
    Re: [PATCH v17 2/3] usb: USB Type-C connector class Peter Chen <hzpeterchen@gmail.com> - 2017-03-03 11:20 +0100
      Re: [PATCH v17 2/3] usb: USB Type-C connector class Guenter Roeck <linux@roeck-us.net> - 2017-03-03 15:40 +0100
        Re: [PATCH v17 2/3] usb: USB Type-C connector class Peter Chen <hzpeterchen@gmail.com> - 2017-03-06 02:30 +0100

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

FromGuenter Roeck <linux@roeck-us.net>
Date2017-03-03 07:10 +0100
SubjectRe: [PATCH v17 2/3] usb: USB Type-C connector class
Message-ID<tgOzD-5ms-5@gated-at.bofh.it>
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/
>

[toc] | [next] | [standalone]


#1591828

FromPeter Chen <hzpeterchen@gmail.com>
Date2017-03-03 11:20 +0100
Message-ID<tgStA-8ce-23@gated-at.bofh.it>
In reply to#1591677
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

[toc] | [prev] | [next] | [standalone]


#1592009

FromGuenter Roeck <linux@roeck-us.net>
Date2017-03-03 15:40 +0100
Message-ID<tgWxd-2tg-45@gated-at.bofh.it>
In reply to#1591828
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

[toc] | [prev] | [next] | [standalone]


#1592876

FromPeter Chen <hzpeterchen@gmail.com>
Date2017-03-06 02:30 +0100
Message-ID<thPDj-ro-1@gated-at.bofh.it>
In reply to#1592009
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web