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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160525140429.GE27570@kuha.fi.intel.com>
Date:	Wed, 25 May 2016 17:04:29 +0300
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	Guenter Roeck <linux@...ck-us.net>,
	Oliver Neukum <oneukum@...e.com>
Cc:	Greg KH <gregkh@...uxfoundation.org>,
	Mathias Nyman <mathias.nyman@...ux.intel.com>,
	Felipe Balbi <felipe.balbi@...ux.intel.com>,
	Rajaram R <rajaram.officemail@...il.com>,
	Andy Shevchenko <andy.shevchenko@...il.com>,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [RFC PATCHv2] usb: USB Type-C Connector Class

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@...ux.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