Hi Guenter, On Sun, Jul 03, 2016 at 02:28:44PM -0700, Guenter Roeck wrote: > On 07/03/2016 12:38 PM, Heikki Krogerus wrote: > > On Fri, Jul 01, 2016 at 07:33:12AM -0700, Guenter Roeck wrote: > > > On Fri, Jul 01, 2016 at 03:05:35PM +0300, Heikki Krogerus wrote: > > > > I've updated my github branch with a commit where both of these issues > > > > should be fixed. Can you give it a try? > > > > > > > > > > This is going to be very difficult. So far, calls such as > > > typec_set_data_role() were independent (asynchronous) of role change > > > requests through sysfs, meaning they happened whenever lower level > > > confirmed that a role change was complete, for whatever reason. Now > > > I have to check if a role change request through the class code > > > is pending and not call typec_set_data_role() and friends in that case. > > > > I'm sorry about the misunderstanding. > > > > What you want is basically that we only support non-blocking mode with > > this interface, and we can't do that. > > > > No, that is not what I said or asked for. The problems I am seeing are due > to locking in the class code. "Asynchronous" above referred to the state > machine vs. role change requests by the class code, which operates independent > of each other in my code. Set requests from the class code still wait for > the state machine to complete. > > The problem is that the state machine now needs to know if the class code > has a set role request pending, because in that case it can no longer report > role changes directly to the class code. This includes role changes unrelated > to the actual set role request. That code is now much more complicated, especially > since each callback into the typec code is handled differently. For example, > typec_disconnect() does not require a lock (unless I missed it), but many > other functions do. It seems that I have misunderstood your whole point. I'm really sorry. I was in a hurry to start my vacation. So let's just fix the locking. <snip> > > > > I've also removed the supports_usb_power_delivery and id_header_vdo > > > > attributes from partners and cables. We really have to put all the USB > > > > PD specific attributes to its own group, and that group can't be in > > > > control of this class. We will simply not always have access to the > > > > kind of USB PD specific details you want to show, for example details > > > > that you get from discover identity command, even when USB PD is fully > > > > supported. For example on systems that use UCSI. > > > > > > > > > > I think we should have a single unified ABI, independent of the underlying > > > driver, that informs the user about the partner device. I really don't > > > quite understand why the class code should not be able to report what device > > > it is connected to (while at the same time being able to report the alternate > > > modes it supports). > > > > OK, let's plan this more then. Maybe we can make for example a layer > > that creates the groups for the PD specific details to the class? > > > > The problem is still that we can only provide results from for example > > request identity command reliably when the PD protocol layer is > > completely inside kernel, and that is not always the case. So we > > really need to group those details in its own group, and that group > > will basically indicate is the PD stack inside the kernel or not. > > > > We should not forget also that the userspace can never rely on those > > details because of the fact that they simply will not always be > > available. > > > On the other side, not being able to rely on a well defined ABI makes the > ABI much less useful. What do we do about this? Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
