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

[Heikki Krogerus]

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@xxxxxxxxxxxxxxx> 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