Re: [PATCHv11 2/3] usb: USB Type-C connector class

From: Heikki Krogerus
Date: Tue Nov 22 2016 - 03:54:44 EST


On Mon, Nov 21, 2016 at 07:33:45AM -0800, Guenter Roeck wrote:
> On 11/21/2016 06:23 AM, Heikki Krogerus wrote:
> > On Mon, Nov 21, 2016 at 03:11:03PM +0200, Heikki Krogerus wrote:
> > > Hi Greg,
> > >
> > > On Mon, Nov 21, 2016 at 11:35:28AM +0100, Greg KH wrote:
> > > > > +static void typec_partner_release(struct device *dev)
> > > > > +{
> > > > > + struct typec_port *port = to_typec_port(dev->parent);
> > > > > +
> > > > > + typec_unregister_altmodes(dev);
> > > > > + port->partner = NULL;
> > > > > +}
> > > >
> > > > This doesn't feel right, why are you both exporting
> > > > typec_unregister_altmodes() and also calling it from release callbacks?
> > > > That implies there is two way to clean stuff up, so what is a driver
> > > > writer to use? Please simplify and force it to be one way or the other.
> > >
> > > OK.
> >
> > Guenter did you need to also remove the alternate modes in drivers, or
> > can we just do it here?
> >
>
> It is currently called explicitly on a data role change, when executing
> a hard reset, on detach, and during error recovery. Most of those would
> also unregister the partner, so I should be able to drop those calls
> (or maybe I'll have to - I will see when testing), but I am not sure
> how to handle data role changes.

Let's keep this in the drivers. Actually, we can't unregister them
here. The partner is the parent of the alternate modes devices. Sorry
about the hassle.


Thanks,

--
heikki