linux-kernel - Re: [PATCH 4/8] usb: typec: mux: Find the muxes by also matching against the device node

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]

Date:   Mon, 28 Jan 2019 17:46:18 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Chen Yu <chenyu56@...wei.com>, Jun Li <jun.li@....com>,
        Hans de Goede <hdegoede@...hat.com>,
        USB <linux-usb@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/8] usb: typec: mux: Find the muxes by also matching
 against the device node

On Mon, Jan 28, 2019 at 11:53:54AM +0200, Andy Shevchenko wrote:
> On Fri, Jan 25, 2019 at 3:17 PM Heikki Krogerus
> <heikki.krogerus@...ux.intel.com> wrote:
> >
> > When the connections are defined in firmware, struct
> > device_connection will have the fwnode member pointing to
> > the device node (struct fwnode_handle) of the requested
> > device, and the endpoint will not be used at all in that
> > case.
> 
> >  static void *typec_mux_match(struct device_connection *con, int ep, void *data)
> >  {
> > +       const struct typec_altmode_desc *desc = data;
> >         struct typec_mux *mux;
> 
> > +       bool match = !con->id;
> 
> I don't see how this assignment is used.

That definitely deserves a comment at least.

This series adds support for the "device graph" to the device
connection API, but it is also possible to describe connections with
normal references (fwnode_property_get_reference_args()). With normal
references we don't need to do any extra identification like we do
with device graph.

The idea is to supply the connection id only if it has not been
"consumed" already (device graph case). If it has been "consumed", we
know there is no need for any extra connection identification (normal
references), and we can jump straight to the next stage -> find the
mux..

> > +       size_t nval;
> > +       u16 *val;
> > +       int i;
> > +
> > +       if (!con->fwnode) {
> > +               list_for_each_entry(mux, &mux_list, entry)
> > +                       if (!strcmp(con->endpoint[ep], dev_name(mux->dev)))
> > +                               return mux;
> > +               return ERR_PTR(-EPROBE_DEFER);
> > +       }
> > +
> > +       if (match)
> > +               goto find_mux;

..here.

I'll change this so that the match variable is introduced without
setting default value. Instead, how about this:

        /*
         * Check has the identifier already been "consumed". If it
         * has, no need to do any extra connection identification.
         */
        match = !con->id;
        if (match)
                goto find_mux;

thanks,

-- 
heikki