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

From: Heikki Krogerus
Date: Tue Nov 22 2016 - 08:40:11 EST


On Tue, Nov 22, 2016 at 12:51:11PM +0200, Heikki Krogerus wrote:
> Hi Greg,
>
> On Mon, Nov 21, 2016 at 03:45:06PM +0100, Greg KH wrote:
> > > We could allocate an extra structure for the partner when
> > > typec_connect() is called, but we would do that just for the sake of
> > > having something to free in the release hook. It would not be useful
> > > for anything. It would not help us increase/decrease the reference
> > > count of the device, and the port driver would still have to provide
> > > details about the partner capabilities the moment it tells us the
> > > partner was connected.
> >
> > Again, free the device for which this release function is being called
> > for, that is why it is there.
>
> The struct device is now member of struct typec_partner. This is what
> a typical port driver would have (I hope it's readable):
>
> struct my_port {
> /* This structure is provided by the class */
> struct typec_port *port;
>
> /*
> * Don't forget, there can only be one partner at a time
> */
> struct typec_partner partner; /* NOTE: this is not a pointer */
> };
>
> int my_interrupt(...)
> {
> ...
> /*
> * Connection happened (I'm skipping the typec_connection
> * wrapper in this example)
> */
> my_port->partner.usb_pd = ...
> ...
> ret = typec_connect(my_port->port, &my_port->partner);
> ...
> /*
> * Disconnect
> */
> typec_disconnect(my_port->port);
> memset(&my_port->partner, 0, sizeof(struct typec_partner));
> ...
> }
>
> int my_probe(...)
> {
> struct my_port *my_port;
> ...
> my_port = devm_kzalloc(...
> ...
> my_port->port = typec_register_port(...
> ...
> }
>
> To have something to free when the partner device's reference counter
> goes to zero and release is called (this happens after all the
> alternate modes, so the children, and the device are unregistered), we
> will need an extra structure, just for the fun of having something to
> free in release.
>
> struct internal_partner_structure {
> struct device dev;
> struct typec_partner *partner_capabilities; /* port driver provides */
> };
>
> Why is this necessary in this case? It is just something extra we have
> to do, just so we can allocate that when connection happens and the
> partner device is generated, and so we can then free that thing when
> release gets called? It does not give us anything. It does not affect
> anything.

Blah, ignore that message. I'm talking about the wrong thing here.

Sorry about that.

--
heikki