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

From: Guenter Roeck
Date: Wed May 25 2016 - 09:22:15 EST

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@xxxxxxxxxxxxxxx>

[ ... ]

+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.


+void typec_unregister_altmodes(struct typec_altmode *alt_modes)
+ struct typec_altmode *alt;
This will crash if alt_modes is NULL, which will happen if
partner->alt_modes is NULL at connection time. Semantically
this is different to typec_register_altmodes(), which does
have a NULL check.

Yes, need to fix that.

Thanks Guenter,