Re: [PATCH v10 5/7] iio: add the IIO backend framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux