From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754254AbcEYOGQ (ORCPT ); Wed, 25 May 2016 10:06:16 -0400 Received: from mga01.intel.com ([192.55.52.88]:52597 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754005AbcEYOGO (ORCPT ); Wed, 25 May 2016 10:06:14 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,364,1459839600"; d="scan'208";a="984457978" Date: Wed, 25 May 2016 17:04:29 +0300 From: Heikki Krogerus To: Guenter Roeck , Oliver Neukum Cc: Greg KH , Mathias Nyman , Felipe Balbi , Rajaram R , Andy Shevchenko , linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [RFC PATCHv2] usb: USB Type-C Connector Class Message-ID: <20160525140429.GE27570@kuha.fi.intel.com> References: <1463661894-22820-1-git-send-email-heikki.krogerus@linux.intel.com> <20160524192826.GA28453@roeck-us.net> <20160525115135.GD27570@kuha.fi.intel.com> <5745A6F2.6000406@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5745A6F2.6000406@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 On Wed, May 25, 2016 at 06:21:54AM -0700, Guenter Roeck wrote: > On 05/25/2016 04:51 AM, Heikki Krogerus wrote: > > On Tue, May 24, 2016 at 12:28:26PM -0700, Guenter Roeck wrote: > > > On Thu, May 19, 2016 at 03:44:54PM +0300, Heikki Krogerus wrote: > > > > The purpose of this class is to provide unified interface for user > > > > space to get the status and basic information about USB Type-C > > > > Connectors in the system, control data role swapping, and when USB PD > > > > is available, also power role swapping and Alternate Modes. > > > > > > > > Signed-off-by: Heikki Krogerus > > > > > > [ ... ] > > > > > > > + > > > > +static void typec_remove_partner(struct typec_port *port) > > > > +{ > > > > + sysfs_remove_link(&port->dev.kobj, "partner"); > > > > + typec_unregister_altmodes(port->partner->alt_modes); > > > > > > This only unregisters alternate modes registered through typec_add_partner(), > > > but not alternate modes registered separately. Or is the calling code expected > > > to set port->partner->alt_modes when calling typec_register_altmodes() > > > directly ? > > > > The altmodes for the partner are not meant to be registered > > separately. With the partners and also cable plugs the class is in > > control of registering and unregistering of the altmode devices after > > typec_connect() is called. > > > > The idea was that only the ports will register the alternate modes > > they support separately, but I think we have to change that too. So I > > don't think we'll export the typec_un/register_altmodes() at all. > > > > We will have to prevent any drivers from being bound to the port > > alternate mode devices when we add the alternate mode bus, and I had > > some idea where by making the port drivers themselves in charge of > > registering the port alternate modes, we could prevent it easily. But > > it's probable easier to just handle those in the class driver as well. > > > > Alternate mode discovery is an orthogonal process to the connection > state machine, and may take a while to complete. Are you saying > that the call to typec_connect() should be delayed until after > alternate mode discovery completes or times out ? > > So far I call typec_connect() in SRC.Ready and SNK.Ready, and > typec_register_altmodes() after mode discovery is complete. > It is also orthogonal, meaning it is only called if and when alternate > mode discovery completes, and the alternate mode discovery state machine > is separate to the port state machine. > > No problem for me to change that, just making sure that the registration > delay is understood and accepted. I'm not against leaving the responsibility of registering the alternate modes to the drivers. I'm a little bit worried about relying then on the drivers to also handle the unregistering accordingly, but I can live with that. But we just shouldn't share the responsibility of un/registering them between the class and the drivers, so the driver should then handle the registration always. Oliver, what do you think? Thanks, -- heikki