On Fri, 12 Jan 2024 17:40:20 +0100
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()
>
> Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
A few minor comments inline.
...
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> new file mode 100644
> index 000000000000..994bc68c2679
> --- /dev/null
> +++ b/drivers/iio/industrialio-backend.c
> @@ -0,0 +1,411 @@
...
> +
> +/*
> + * Helper struct for requesting buffers. Allows for multiple buffers per
> + * backend.
Only seems to be used to ensure we have all the data needed to free it...
So comment seems less than obviously connected to that.
> + */
> +struct iio_backend_buffer_pair {
> + struct iio_backend *back;
> + struct iio_buffer *buffer;
> +};
> +
> +/**
> + * iio_backend_chan_enable - Enable a backend channel.
> + * @back: Backend device.
> + * @chan: Channel number.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan)
> +{
> + return iio_backend_op_call(back, chan_enable, chan);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_chan_enable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_chan_disable - Disable a backend channel.
> + * @back: Backend device.
> + * @chan: Channel number.
Would be good to be consistent on . or not.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan)
> +{
> + return iio_backend_op_call(back, chan_disable, chan);
> +}
> +EXPORT_SYMBOL_NS_GPL(iio_backend_chan_disable, IIO_BACKEND);
> +
> +/**
> + * iio_backend_chan_enable - Enable the backend.
> + * @back: Backend device
...
> +/**
> + * devm_iio_backend_get_from_fwnode_lookup
Not valid kernel doc + name is wrong. Make sure you run
the kernel-doc script over this and fix any errors or warnings
reported.
> + * @dev: Device where to bind the backend lifetime.
> + * @fwnode: Firmware node of the backend device.
> + *
> + * Search the backend list for a device matching @fwnode.
> + * This API should not be used and it's only present for preventing the first
> + * user of this framework to break it's DT ABI.
> + *
> + * RETURNS:
> + * A backend pointer, negative error pointer otherwise.
> + */
> +struct iio_backend *
> +__devm_iio_backend_get_from_fwnode_lookup(struct device *dev,
> + struct fwnode_handle *fwnode)
> +{
> + struct iio_backend *back;
> + int ret;
> +
> + guard(mutex)(&iio_back_lock);
> + list_for_each_entry(back, &iio_back_list, entry) {
> + if (!device_match_fwnode(back->dev, fwnode))
> + continue;
> +
> + ret = __devm_iio_backend_get(dev, back);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return back;
> + }
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL_NS_GPL(__devm_iio_backend_get_from_fwnode_lookup, IIO_BACKEND);
>
> +/**
> + * devm_iio_backend_register - Register a new backend device
> + * @dev: Backend device being registered.
> + * @ops: Backend ops
> + * @priv: Device private data.
> + *
> + * @ops and @priv are both mandatory. Not providing them results in -EINVAL.
It's unusual to 'insist' on the private data.
Whilst it's highly likely it will always be there from a core point of view
we don't mind it being NULL. This is different from the ops as we want
to be able to call those without checking they are there.
> + *
> + * RETURNS:
> + * 0 on success, negative error number on failure.
> + */
> +int devm_iio_backend_register(struct device *dev,
> + const struct iio_backend_ops *ops, void *priv)
> +{
> + struct iio_backend *back;
> +
> + if (!ops || !priv) {
> + pr_err("%s: No backend ops or private data given\n",
> + dev_name(dev));
> + return -EINVAL;
> + }
> +
> + /*
> + * Through device_links, we guarantee that a frontend device cannot be
> + * bound/exist if the backend driver is not around. Hence, we can bind
> + * the backend object lifetime with the device being passed since
> + * removing it will torn the frontend down.
"will have torn" or "will tear the"
> + */
> + back = devm_kzalloc(dev, sizeof(*back), GFP_KERNEL);
> + if (!back)
> + return -ENOMEM;
> +
> + back->ops = ops;
> + back->owner = dev->driver->owner;
> + back->dev = dev;
> + back->priv = priv;
> + mutex_lock(&iio_back_lock);
> + list_add(&back->entry, &iio_back_list);
> + mutex_unlock(&iio_back_lock);
> +
> + return devm_add_action_or_reset(dev, iio_backend_unregister, back);
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_iio_backend_register, IIO_BACKEND);
> +
> +MODULE_AUTHOR("Nuno Sa <nuno.sa@xxxxxxxxxx>");
> +MODULE_DESCRIPTION("Framework to handle complex IIO aggregate devices");
> +MODULE_LICENSE("GPL");
