Re: [RFC PATCHv2] usb: USB Type-C Connector Class

From: Oliver Neukum
Date: Mon May 30 2016 - 10:03:31 EST


On Mon, 2016-05-30 at 16:19 +0300, Heikki Krogerus wrote:
> Hi guys,
>
> I'm attaching a diff instead of full v3. I'm not yet adding attributes
> for the reset and cable_reset. I still don't understand what is the
> case where the userspace would need to be able to tricker reset? Why
> isn't it enough for the userspace to be able to enter/exit modes?
> Oliver! Can you please comment?

1. Because we need error handling.
Devices crash. Cables will crash. We will get out of sync.
You never put yourself in a place where you cannot handle an
IO error.
2. Because it is in the spec. We do not second guess the spec.
We implement it.

> I also made a change to the partner devices so that they always have
> the port as their parent. That will have an effect on the location
> where the partner device is exposed in sysfs (so now always under the
> port). And because of that, I would like to get an OK from you guys.

It is not very important. i am fine with it.

> I'm a bit concerned about the current API for adding the alternate
> modes. Since we are passing the parent for an alternate mode as
> struct device, it makes it possible for the caller to place it into
> some wrong place. But I guess we can change the API even later.
>
> We also need to decide how the alternate modes a port support are
> exposed to the userspace. Do we just assume the port drivers will
> create them as devices under the port device itself, just like
> alternate modes of partners and cable plugs are exposed under the
> partners and cable plugs? That works for me, but again, the class does
> not have any effect on that, and it will be just a guideline. Maybe
> we can add some kind of helpers and force the port drivers to use
> them.

What are the alternatives?

> And finally, mostly as a reminder for myself. I didn't yet add
> attributes for Try.SRC/SNK. So can we make it something like
> "preferred_role" like I think you proposed Guenter? The
> "current_data_role" I'm dropping.

That sounds good.

Regards
Oliver