From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1033770AbdAJRft (ORCPT ); Tue, 10 Jan 2017 12:35:49 -0500 Received: from bh-25.webhostbox.net ([208.91.199.152]:57088 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1033658AbdAJRfp (ORCPT ); Tue, 10 Jan 2017 12:35:45 -0500 Date: Tue, 10 Jan 2017 09:35:42 -0800 From: Guenter Roeck To: Heikki Krogerus Cc: Greg KH , Oliver Neukum , Mika Westerberg , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [PATCHv14 2/3] usb: USB Type-C connector class Message-ID: <20170110173542.GA29970@roeck-us.net> References: <20170105110119.87401-1-heikki.krogerus@linux.intel.com> <20170105110119.87401-3-heikki.krogerus@linux.intel.com> <20170109165932.GA16253@roeck-us.net> <20170110085422.GA18258@kuha.fi.intel.com> <20170110144612.GC18258@kuha.fi.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170110144612.GC18258@kuha.fi.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 10, 2017 at 04:46:12PM +0200, Heikki Krogerus wrote: > On Tue, Jan 10, 2017 at 05:50:04AM -0800, Guenter Roeck wrote: > > On 01/10/2017 12:54 AM, Heikki Krogerus wrote: > > > Hi Guenter, > > > > > > On Mon, Jan 09, 2017 at 08:59:32AM -0800, Guenter Roeck wrote: > > > > > +/** > > > > > + * typec_register_partner - Register a USB Type-C Partner > > > > > + * @port: The USB Type-C Port the partner is connected to > > > > > + * @desc: Description of the partner > > > > > + * > > > > > + * Registers a device for USB Type-C Partner described in @desc. > > > > > + * > > > > > + * Returns handle to the partner on success or NULL on failure. > > > > > + */ > > > > > +struct typec_partner *typec_register_partner(struct typec_port *port, > > > > > + struct typec_partner_desc *desc) > > > > > +{ > > > > > > > > With the changes to hide the actual partner structure, this looks at first > > > > glance like a minor API change, but it is substantial. > > > > > > > > Reason is that the vdo as required by typec_partner_desc is provided by a VDM > > > > command reply, which is completely orthogonal to the PD registration process. > > > > So far I was able to set the vdo later, after registering the connection, > > > > and after (and if) the vdo was received. > > > > > > If the identity vdo value is updated after the creation of the device, > > > then the user space needs to be notified separately. > > > > > > > Since the partner may not even respond to the DISCOVER_IDENT message, or not > > > > support PD at all, this means that I would have to disconnect partner > > > > registration from the PD protocol itself and tie it to the VDO message > > > > exchange, with appropriate timeouts to register anyway even if the identity > > > > was not received after some period of time or if the partner does not support > > > > PD. > > > > > > > > This in turn means that I'll have to re-implement and possibly re-architect > > > > a substantial amount of code. > > > > > > We don't need to protect the structures like this, we can change this > > > back. But how about we introduce driver callback function for updating > > > the value instead, which would also notify the uses space? > > > > > > > That would work. > > OK, cool. > > I guess we might as well then split the VDO into header, cert stat and > product parts. What do you think? > > If it's OK, then should we change that file to "identity" and dump the > whole response from Discover Identity command in hex (minus VDM > Header), separate the parts in the output, or simply provide separate > attribute files for each part? > Makes me wonder what you expect in the vdo attribute today. Right now I copy the value from the identity header into it. Personally I would prefer separate attributes. identity, cert_stat, product, maybe ? > Just as a reminder, the user space can't rely on that attribute file. > We still can't get any of that information from UCSI or for example > the Thunderbolt controllers, which is annoying, but I guess it does > not matter. > > An other question: > I would like to hide the attribute file(s) when the partner does not > support USB Power Delivery. Is it OK with you guys? > Ok with me. Thanks, Guenter