Re: [PATCH v2] mux: convert to use fwnode interface

[Fabio]

> > @@ -566,39 +569,40 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> >   	}
> >   
> >   	if (state)
> > -		ret = of_parse_phandle_with_args(np,
> > -						 "mux-states", "#mux-state-cells",
> > -						 index, &args);
> > +		ret = fwnode_property_get_reference_args(fwnode, "mux-states",
> > +							 "#mux-state-cells", 0,
> > +							 index, &args);
> >   	else
> > -		ret = of_parse_phandle_with_args(np,
> > -						 "mux-controls", "#mux-control-cells",
> > -						 index, &args);
> > +		ret = fwnode_property_get_reference_args(fwnode,
> > +							 "mux-controls", "#mux-control-cells",
> > +							 0, index, &args);
> > +
> >   	if (ret) {
> >   		if (optional && ret == -ENOENT)
> >   			return NULL;
> >   
> > -		dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> > -			np, state ? "state" : "control",
> > -			mux_name ?: "", index);
> > +		dev_err(dev, "%pfw: failed to get mux-%s %s(%i)\n",
> > +			fwnode, state ? "state" : "control", mux_name ?: "",
> > +			index);
> >   		return ERR_PTR(ret);
> >   	}
> >   
> > -	mux_chip = of_find_mux_chip_by_node(args.np);
> > -	of_node_put(args.np);
> > +	mux_chip = mux_chip_find_by_fwnode(args.fwnode);
> > +	fwnode_handle_put(args.fwnode);
>
> I'm actually not sure why there was an of_node_put() here in the first
> place. It seems wrong to me. Any idea what it's for?
>
> I want to give a Reviewed-by here for the rest of the code, but if the
> above _put() erroneous, it might be better to add a patch before this
> one to remove it (and cc the stable list with that patch).

I don't think it's wrong, because fwnode_property_get_reference_args() documents that:

    The caller is responsible for calling fwnode_handle_put() on the returned
    @args->fwnode pointer.

What is wrong though, is passing args.fwnode to the dev_err() calls below after it was freed.
Or that's what I'm inclined to say; I doubt put() would free everything but fwnode's name.
If I had to guess, fwnode_handle_put is (erroneously) where it is to avoid gotos/calling mux_chip_find_by_fwnode()
in the sad path.

If you agree with the above, I'll be happy to write a patch to fix it.

Regards,
Fabio.