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

[Fabio <development@redaril.me>]

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