linux-kernel - Re: [PATCHv11 2/3] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20161122133753.GG18501@kuha.fi.intel.com>
Date:   Tue, 22 Nov 2016 15:37:53 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        Oliver Neukum <oneukum@...e.com>, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCHv11 2/3] usb: USB Type-C connector class

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