On Fri, Jun 03, 2016 at 06:51:54AM -0700, Guenter Roeck wrote: > On 06/03/2016 06:21 AM, Heikki Krogerus wrote: > > Hi, > > > > On Thu, Jun 02, 2016 at 09:12:19AM -0700, Guenter Roeck wrote: > > > On Thu, Jun 02, 2016 at 01:18:53PM +0300, Heikki Krogerus wrote: > > > > On Wed, Jun 01, 2016 at 04:29:26PM -0700, Guenter Roeck wrote: > > > > > On Wed, Jun 01, 2016 at 11:26:09AM +0200, Oliver Neukum wrote: > > > > > > On Thu, 2016-05-19 at 15:44 +0300, Heikki Krogerus wrote: > > > > > > > Just noticed that the "active" file is for now read only, but it > > > > > > > needs > > > > > > > to be changed to writable. That file will of course provide means > > > > > > > for > > > > > > > the userspace to Exit and Enter modes. But please note that the > > > > > > > responsibility of the dependencies between the modes, say, if a > > > > > > > plug > > > > > > > needs to be in one mode or the other in order for the partner to > > > > > > > enter > > > > > > > some specific mode, will fall on the Alternate Mode specific > > > > > > > drivers > > > > > > > once we have the altmode bus. I remember there were concerns about > > > > > > > this in the original thread. > > > > > > > > > > > > There's one thing we haven't touched upon yet. And I cannot really > > > > > > find > > > > > > an answer in the spec. > > > > > > > > > > > > What do we do if we return from S4 or S3? I think we need to restore > > > > > > the ALternate Mode because our display may be running over that > > > > > > Alternate Mode. > > > > > > If we want to support USB persist we also need to restore data role > > > > > > after S4. > > > > > > > > > > > I don't have an answer ... but another interesting question. > > > > > > > > > > How do we distinguish between alternate modes supported by a host vs. > > > > > alternate modes supported by a sink ? typec_capability includes a > > > > > pointer > > > > > to alternate modes supportedf by the connector, but it is not clear if > > > > > those are alternate modes supported as host, or alternate modes > > > > > supported > > > > > as device, or alternate modes supported by both. > > > > > > > > > > This doesn't matter much if only a fixed role is supported, but it > > > > > does matter > > > > > for dual role ports. A laptop will typically only support DisplayPort > > > > > as host, > > > > > for example. > > > > > > > > The DP alternate mode spec actually separates the display role from > > > > Type-C role. A laptop most likely would only support the modes for > > > > display host roles, but if the port was DRP port then it would still > > > > do so in both Type-C roles. > > > > > > > > So basically, even if the display was Type-C host, it would still work > > > > as a display when attached to the laptop. > > > > > > > > > Any idea ? > > > > > > > > I'm actually not sure this is a problem. > > > > > > > Yes, this was a bad example, since the DisplayPort mode vdo includes a > > > flag > > > indicating if the port supports source, sink, or both. > > > > I meant that in case of DP alternate mode, there should not be a > > problem. > > > > > Let's use a different example: > > > Google devices (such as power adapters) have mode '1' for firmware > > > upgrades. > > > Obviously hosts will support that, but what should the host advertise if > > > it > > > is configured as sink ? > > > > > > Maybe this is just my personal confusion, and there is no real problem. > > > It might as well be that the Google mode VDO _should_ include a flag > > > indicating if the port supports updating the partner, and/or if it > > > supports > > > being updated. For now I'll just assume that this is the case. > > > > Well, do you think we can rely on always being able to get this detail > > from VDO? > > > > Not really. Like in the Google case, one end will implement sending the > firmware, > the other end will implement receiving it and writing it to flash (or > whatever). > Which is which isn't currently made visible to user space. I suspect that > other "intelligent" devices like Apple's multi-function adapter do the same, > though obviously I don't really know what the two VDO modes do on the apple > adapter. > > I'll have to have some ABI to user space for the alternate mode, for example > to send the firmware file to the kernel. I am not there yet, though, so I > don't really know exactly how that will look like. Most likely it is going to > be > added sysfs attributes in the mode device (eg usbc0.svid:18d1/mode0/firmware). > > > > Something else, which goes back into the symlink question. If I create the > > > alternate mode devices before calling typec_register_port(), the devices > > > won't > > > have a parent and don't show up in the class directory. You previously > > > solved > > > that with the symlink. I am trying to solve it in my current code by > > > calling > > > typec_register_altmodes() from typec_register_port() - primarily because I > > > don't really want to duplicate all the device creation code in my driver. > > > > > > In my test case, this gives me > > > /sys/class/type-c/usbc0/ > > > usbc0.svid:18d1 > > > usbc0.svid:18d1/mode0 > > > usbc0.svid:18d1/mode0/vdo > > > usbc0.svid:18d1/mode0/description > > > usbc0.svid:18d1/mode0/active > > > ... > > > usbc0.svid:ff01 > > > usbc0.svid:ff01/mode0/vdo > > > usbc0.svid:ff01/mode0/description > > > usbc0.svid:ff01/mode0/active > > Side note: I didn't provide a description/name for the modes, because that > would result in something like usbc0.DisplayPort/ instead of usbc0.svid:ff01/, > and I prefer a consistent ABI. Since this _is_ part of the ABI, would it make > sense to standardize on names for modes in sysfs ? For example, how should > a "Display Port" mode directory be named ? It doesn't sound good if I > use "usbc0.svid:ff01", someone else uses "usbc0.DisplayPort", and yet > someone else uses "usbc0.displayport".
Yeah, let's make them standard. > > Also, do we at some point need to standardize the ABI for the standard > alternate modes such as DisplayPort (if there are any - again I am not > there yet) ? I don't have an answer to that. > > > > > > in addition to > > > /sys/class/type-c/usbc0/ > > > usbc0-partner/usbc0-partner.svid:05ac > > > usbc0-partner/usbc0-partner.svid:05ac/mode0 > > > usbc0-partner/usbc0-partner.svid:05ac/mode0/vdo > > > usbc0-partner/usbc0-partner.svid:05ac/mode0/description > > > usbc0-partner/usbc0-partner.svid:05ac/mode0/active > > > usbc0-partner/usbc0-partner.svid:05ac/mode1 > > > usbc0-partner/usbc0-partner.svid:05ac/mode1/vdo > > > usbc0-partner/usbc0-partner.svid:05ac/mode1/description > > > usbc0-partner/usbc0-partner.svid:05ac/mode1/active > > > ... > > > usbc0-partner/usbc0-partner.svid:ff01 > > > usbc0-partner/usbc0-partner.svid:ff01/mode0 > > > usbc0-partner/usbc0-partner.svid:ff01/mode0/vdo > > > usbc0-partner/usbc0-partner.svid:ff01/mode0/description > > > usbc0-partner/usbc0-partner.svid:ff01/mode0/active > > > > > > (when connecting the Apple adapter), which is exactly what I would expect > > > to see. > > > > > > Is this sensible ? Do we have a reason for expecting the alternate mode > > > _devices_ to be created (without parent) when calling > > > typec_register_port() ? > > > > So if you would prefer that the class code takes care of creating the > > alternate modes when typec_register_port() is called, I'm fine with > > that too. Let's make it so. > > > > Sounds good to me. Many other subsystems do the same, ie create the subsystem > device(s) during registration with the subsystem, so this is in line with > other > kernel code. > > Should I send you a follow-up patch on top of yours ? Sure. I'm a little bit stuck with an other tasks, so let's keep this thing rolling. Thanks, -- heikki