From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753626AbcGDRMA (ORCPT ); Mon, 4 Jul 2016 13:12:00 -0400 Received: from mga03.intel.com ([134.134.136.65]:47950 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752844AbcGDRL6 (ORCPT ); Mon, 4 Jul 2016 13:11:58 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,575,1459839600"; d="scan'208";a="989011898" Date: Mon, 4 Jul 2016 20:11:53 +0300 From: Heikki Krogerus To: Guenter Roeck Cc: Greg KH , Oliver Neukum , Felipe Balbi , Roger Quadros , Rajaram R , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCHv4 1/2] usb: USB Type-C connector class Message-ID: <20160704171153.GA26797@kuha.fi.intel.com> References: <1467207518-55953-1-git-send-email-heikki.krogerus@linux.intel.com> <1467207518-55953-2-git-send-email-heikki.krogerus@linux.intel.com> <20160630220220.GA3707@roeck-us.net> <20160701071348.GA21901@kuha.fi.intel.com> <20160701120535.GA7673@kuha.fi.intel.com> <20160701143312.GA14350@roeck-us.net> <20160703193828.GA30987@kuha.fi.intel.com> <5779838C.9050008@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5779838C.9050008@roeck-us.net> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > > 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