On Fri, Feb 9, 2024 at 5:26 PM Nuno Sa <nuno.sa@xxxxxxxxxx> wrote:
...
> +struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name)
> +{
> + struct fwnode_handle *fwnode;
> + struct iio_backend *back;
> + unsigned int index;
> + int ret;
> +
> + if (name) {
> + ret = device_property_match_string(dev, "io-backend-names",
> + name);
> + if (ret < 0)
> + return ERR_PTR(ret);
> + index = ret;
> + } else {
> + index = 0;
> + }
> +
> + fwnode = fwnode_find_reference(dev_fwnode(dev), "io-backends", index);
> + if (IS_ERR(fwnode)) {
> + dev_err_probe(dev, PTR_ERR(fwnode),
> + "Cannot get Firmware reference\n");
> + return ERR_CAST(fwnode);
> + }
> +
> + guard(mutex)(&iio_back_lock);
> + list_for_each_entry(back, &iio_back_list, entry) {
> + if (!device_match_fwnode(back->dev, fwnode))
> + continue;
> + fwnode_handle_put(fwnode);
> + ret = __devm_iio_backend_get(dev, back);
This order makes me think about the reference counting. So, fwnode is
the one of the backend devices to which the property points to.
Another piece is the local (to this framework) list that keeps backend
devices. So, fwnode reference can be dropped earlier, while the usual
pattern to interleave gets and puts in a chain. Dunno if above needs a
comment, reordering or nothing.
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return back;
> + }
> +
> + fwnode_handle_put(fwnode);
> + return ERR_PTR(-EPROBE_DEFER);
While thinking about the above, I noticed the room to refactor.
list_for_each_entry(...) {
if (...)
break;
}
fwnode_handle_put(...);
// Yes, we may use the below macro as the (global) pointers are
protected by a mutex.
if (list_entry_is_head(...))
return ERR_PTR(...);
ret = __devm_iio_backend_get(...);
...
> +}
--
With Best Regards,
Andy Shevchenko
