On Tue, Feb 6, 2024 at 12:08 PM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote:
>
> From: Nuno Sa <nuno.sa@xxxxxxxxxx>
>
> This is a Framework to handle complex IIO aggregate devices.
>
> The typical architecture is to have one device as the frontend device which
> can be "linked" against one or multiple backend devices. All the IIO and
> userspace interface is expected to be registers/managed by the frontend
> device which will callback into the backends when needed (to get/set
> some configuration that it does not directly control).
>
> The basic framework interface is pretty simple:
> - Backends should register themselves with @devm_iio_backend_register()
> - Frontend devices should get backends with @devm_iio_backend_get()
Not sure what the meaning of @ in the above lines.
...
> +/*
> + * Framework to handle complex IIO aggregate devices.
> + *
> + * The typical architecture is to have one device as the frontend device which
> + * can be "linked" against one or multiple backend devices.
Can we have an ASCII art with an example?
> All the IIO and
> + * userspace interface is expected to be registers/managed by the frontend
> + * device which will callback into the backends when needed (to get/set some
> + * configuration that it does not directly control).
> + *
> + * The framework interface is pretty simple:
> + * - Backends should register themselves with @devm_iio_backend_register()
> + * - Frontend devices should get backends with @devm_iio_backend_get()
> + *
> + * Also to note that the primary target for this framework are converters like
> + * ADC/DACs so @iio_backend_ops will have some operations typical of converter
> + * devices. On top of that, this is "generic" for all IIO which means any kind
> + * of device can make use of the framework. That said, If the @iio_backend_ops
> + * struct begins to grow out of control, we can always refactor things so that
> + * the industrialio-backend.c is only left with the really generic stuff. Then,
> + * we can build on top of it depending on the needs.
> + *
> + * Copyright (C) 2023-2024 Analog Devices Inc.
> + */
...
> +/*
> + * Helper macros to call backend ops. Makes sure the option is supported
Missing period.
> + */
...
> +/**
> + * iio_backend_chan_enable - Enable a backend channel
> + * @back: Backend device
> + * @chan: Channel number
> + *
> + * RETURNS:
Not sure if this is the style used in other IIO core files...
> + * 0 on success, negative error number on failure.
> + */
...
> + if (!try_module_get(back->owner)) {
> + dev_err(dev, "Cannot get module reference\n");
> + return -ENODEV;
devm_*() are supposed to be used only at ->probe() stage, hence
dev_err_probe() is fine (and eventually would be nice to have for the
sake of making messaging uniform).
> + }
...
> + link = device_link_add(dev, back->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
> + if (!link) {
> + dev_err(dev, "Could not link to supplier(%s)\n",
> + dev_name(back->dev));
> + return -EINVAL;
Ditto.
> + }
...
> + if (name) {
> + ret = device_property_match_string(dev, "io-backend-names",
> + name);
One line?
> + 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(dev, "Cannot get Firmware reference\n");
> + return ERR_CAST(fwnode);
dev_err_probe() ?
> + }
...
> + if (!ops) {
> + dev_err(dev, "No backend ops given\n");
> + return -EINVAL;
dev_err_probe() ?
> + }
--
With Best Regards,
Andy Shevchenko
