Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

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 <heikki.krogerus@linux.intel.com>
> > > 
> > > [ ... ]
> > > 
> > > > +
> > > > +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