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