| Message ID | 20240328-iio-backend-axi-dac-v1-8-afc808b3fde3@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: dac: support IIO backends on the output direction | expand |
On Thu, 28 Mar 2024 14:22:32 +0100 Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > From: Nuno Sa <nuno.sa@analog.com> > > This adds the needed backend ops for supporting a backend inerfacing > with an high speed dac. The new ops are: > > * data_source_set(); > * set_sampling_freq(); > * extend_chan_spec(); > * ext_info_set(); > * ext_info_get(). > > Also to note the new helpers that are meant to be used by the backends > when extending an IIO channel (adding extended info): > > * iio_backend_ext_info_set(); > * iio_backend_ext_info_get(). > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> I'm pretty flexible on this so far as I think we are still learning how front ends and backends should interact. Maybe we'll figure that out in the medium term and rework this stuff. For now it looks fine. A few minor things inline. > > +/** > + * iio_backend_ext_info_get - IIO ext_info read callback > + * @indio_dev: IIO device > + * @private: Data private to the driver > + * @chan: IIO channel > + * @buf: Buffer where to place the attribute data > + * > + * This helper is intended to be used by backends that extend an IIO channel > + * (trough iio_backend_extend_chan_spec()) with extended info. In that case, > + * backends are not supposed to give their own callbacks (as they would not > + * a way to get te backend from indio_dev). This is the getter. te->the? > +/** > + * iio_backend_extend_chan_spec - Extend an IIO channel > + * @indio_dev: IIO device > + * @back: Backend device > + * @chan: IIO channel > + * > + * Some backends may have their own functionalities and hence capable of > + * extending a frontend's channel. > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, > + struct iio_backend *back, > + struct iio_chan_spec *chan) > +{ > + const struct iio_chan_spec_ext_info *ext_info = chan->ext_info; This is getting confusing. So this one is the front end value of ext_info? Name it as such frontend_ext_info > + int ret; > + > + ret = iio_backend_op_call(back, extend_chan_spec, chan); > + if (ret) > + return ret; > + /* > + * Let's keep things simple for now. Don't allow to overwrite the > + * frontend's extended info. If ever needed, we can support appending > + * it. > + */ > + if (ext_info && chan->ext_info != ext_info) > + return -EOPNOTSUPP; > + if (!chan->ext_info) This is checking if the backend added anything? Perhaps a comment on that as we don't need a backend_ext_info local variable... > + return 0; > + /* > + * !\NOTE: this will break as soon as we have multiple backends on one > + * frontend and all of them extend channels. In that case, the core > + * backend code has no way to get the correct backend given the > + * iio device. > + * > + * One solution for this could be introducing a new backend > + * dedicated callback in struct iio_info so we can callback into the > + * frontend so it can give us the right backend given a chan_spec. > + */ Hmm. This is indeed messy. Could we associate it with the buffer as presuably a front end with multiple backends is using multiple IIO buffers? As you say a dance via the front end would work fine. > + iio_device_set_drvdata(indio_dev, back); > + > + /* Don't allow backends to get creative and force their own handlers */ > + for (ext_info = chan->ext_info; ext_info->name; ext_info++) { > + if (ext_info->read != iio_backend_ext_info_get) > + return -EINVAL; > + if (ext_info->write != iio_backend_ext_info_set) > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_extend_chan_spec, IIO_BACKEND); > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > index a6d79381866e..09ff2f8f9fd8 100644 > --- a/include/linux/iio/backend.h > +++ b/include/linux/iio/backend.h > @@ -4,6 +4,7 @@ > > #include <linux/types.h> > > +struct iio_chan_spec; > struct fwnode_handle; > struct iio_backend; > struct device; > @@ -15,6 +16,26 @@ enum iio_backend_data_type { > IIO_BACKEND_DATA_TYPE_MAX > }; > > +enum iio_backend_data_source { > + IIO_BACKEND_INTERNAL_CW, CW? Either expand out what ever that is in definition of add a comment at least. > + IIO_BACKEND_EXTERNAL, What does external mean in this case? > + IIO_BACKEND_DATA_SOURCE_MAX > +}; > + > +/** > + * IIO_BACKEND_EX_INFO - Helper for an IIO extended channel attribute > + * @_name: Attribute name > + * @_shared: Whether the attribute is shared between all channels > + * @_what: Data private to the driver > + */ > +#define IIO_BACKEND_EX_INFO(_name, _shared, _what) { \ > + .name = (_name), \ > + .shared = (_shared), \ > + .read = iio_backend_ext_info_get, \ > + .write = iio_backend_ext_info_set, \ > + .private = (_what), \ > +} > + > /** > * struct iio_backend_data_fmt - Backend data format > * @type: Data type. > @@ -35,8 +56,13 @@ struct iio_backend_data_fmt { > * @chan_enable: Enable one channel. > * @chan_disable: Disable one channel. > * @data_format_set: Configure the data format for a specific channel. > + * @data_source_set: Configure the data source for a specific channel. > + * @set_sample_rate: Configure the sampling rate for a specific channel. > * @request_buffer: Request an IIO buffer. > * @free_buffer: Free an IIO buffer. > + * @extend_chan_spec: Extend an IIO channel. > + * @ext_info_set: Extended info setter. > + * @ext_info_get: Extended info getter. > **/ > struct iio_backend_ops { > int (*enable)(struct iio_backend *back); > @@ -45,10 +71,21 @@ struct iio_backend_ops { > int (*chan_disable)(struct iio_backend *back, unsigned int chan); > int (*data_format_set)(struct iio_backend *back, unsigned int chan, > const struct iio_backend_data_fmt *data); > + int (*data_source_set)(struct iio_backend *back, unsigned int chan, > + enum iio_backend_data_source data); > + int (*set_sample_rate)(struct iio_backend *back, unsigned int chan, > + u64 sample_rate); Name the parameter that so we know the units. _hz?
On Thu, 2024-03-28 at 15:16 +0000, Jonathan Cameron wrote: > On Thu, 28 Mar 2024 14:22:32 +0100 > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > > > From: Nuno Sa <nuno.sa@analog.com> > > > > This adds the needed backend ops for supporting a backend inerfacing > > with an high speed dac. The new ops are: > > > > * data_source_set(); > > * set_sampling_freq(); > > * extend_chan_spec(); > > * ext_info_set(); > > * ext_info_get(). > > > > Also to note the new helpers that are meant to be used by the backends > > when extending an IIO channel (adding extended info): > > > > * iio_backend_ext_info_set(); > > * iio_backend_ext_info_get(). > > > > Signed-off-by: Nuno Sa <nuno.sa@analog.com> > I'm pretty flexible on this so far as I think we are still learning how front > ends and backends should interact. Maybe we'll figure that out in the medium > term and rework this stuff. For now it looks fine. A few minor things inline. > > > > +/** > > + * iio_backend_ext_info_get - IIO ext_info read callback > > + * @indio_dev: IIO device > > + * @private: Data private to the driver > > + * @chan: IIO channel > > + * @buf: Buffer where to place the attribute data > > + * > > + * This helper is intended to be used by backends that extend an IIO channel > > + * (trough iio_backend_extend_chan_spec()) with extended info. In that case, > > + * backends are not supposed to give their own callbacks (as they would not > > + * a way to get te backend from indio_dev). This is the getter. > > te->the? Yes and some more typos :). > > > > +/** > > + * iio_backend_extend_chan_spec - Extend an IIO channel > > + * @indio_dev: IIO device > > + * @back: Backend device > > + * @chan: IIO channel > > + * > > + * Some backends may have their own functionalities and hence capable of > > + * extending a frontend's channel. > > + * > > + * RETURNS: > > + * 0 on success, negative error number on failure. > > + */ > > +int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, > > + struct iio_backend *back, > > + struct iio_chan_spec *chan) > > +{ > > + const struct iio_chan_spec_ext_info *ext_info = chan->ext_info; > This is getting confusing. So this one is the front end value of ext_info? > Name it as such frontend_ext_info Yes, it's the frontend pointer. Just to enforce the below constrain. Will rename as suggested. > > > + int ret; > > + > > + ret = iio_backend_op_call(back, extend_chan_spec, chan); > > + if (ret) > > + return ret; > > + /* > > + * Let's keep things simple for now. Don't allow to overwrite the > > + * frontend's extended info. If ever needed, we can support appending > > + * it. > > + */ > > + if (ext_info && chan->ext_info != ext_info) > > + return -EOPNOTSUPP; > > + if (!chan->ext_info) > > This is checking if the backend added anything? Perhaps a comment on that > as we don't need a backend_ext_info local variable... Yes, but just regarding ext_info as that is the only thing we're handling below (doing some sanity checks). Note that (as you said above) I'm keeping things a bit "open" in that the backend is open to extend whatever it wants. With time we may learn better what do we want constrain or not. > > > + return 0; > > + /* > > + * !\NOTE: this will break as soon as we have multiple backends on one > > + * frontend and all of them extend channels. In that case, the core > > + * backend code has no way to get the correct backend given the > > + * iio device. > > + * > > + * One solution for this could be introducing a new backend > > + * dedicated callback in struct iio_info so we can callback into the > > + * frontend so it can give us the right backend given a chan_spec. > > + */ > > Hmm. This is indeed messy. Could we associate it with the buffer as presuably > a front end with multiple backends is using multiple IIO buffers? > Hmm, the assumption of having multiple buffers seems plausible to me but considering the example we have in hands it would be cumbersome to get the backend. Considering iio_backend_ext_info_get(), how could we get the backend if it was associated to one of the IIO buffers? I think we would need more "intrusive" changes to make that work or do you have something in mind= > As you say a dance via the front end would work fine. I'm happy you're also open for a proper solution already. I mention this in the cover. My idea was something like (consider the iio_backend_ext_info_get()): if (!indio_dev->info->get_iio_backend()) return -EOPNOTSUPP; back = indio_dev->info->get_iio_backend(indio_dev, chan_spec); It would be nice to have some "default/generic" implementation for cases where we only have one backend per frontend so that the frontend would not need to define the callback. > > > > + iio_device_set_drvdata(indio_dev, back); > > + > > + /* Don't allow backends to get creative and force their own handlers */ > > + for (ext_info = chan->ext_info; ext_info->name; ext_info++) { > > + if (ext_info->read != iio_backend_ext_info_get) > > + return -EINVAL; > > + if (ext_info->write != iio_backend_ext_info_set) > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_NS_GPL(iio_backend_extend_chan_spec, IIO_BACKEND); > > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > > index a6d79381866e..09ff2f8f9fd8 100644 > > --- a/include/linux/iio/backend.h > > +++ b/include/linux/iio/backend.h > > @@ -4,6 +4,7 @@ > > > > #include <linux/types.h> > > > > +struct iio_chan_spec; > > struct fwnode_handle; > > struct iio_backend; > > struct device; > > @@ -15,6 +16,26 @@ enum iio_backend_data_type { > > IIO_BACKEND_DATA_TYPE_MAX > > }; > > > > +enum iio_backend_data_source { > > + IIO_BACKEND_INTERNAL_CW, > > CW? Either expand out what ever that is in definition of add a comment > at least. Continuous wave :) > > > + IIO_BACKEND_EXTERNAL, > What does external mean in this case? In this particular case comes from a DMA source (IP). I thought external to be more generic but if you prefer, I can do something like IIO_BACKEND_DMA? > > + IIO_BACKEND_DATA_SOURCE_MAX > > +}; > > + > > +/** > > + * IIO_BACKEND_EX_INFO - Helper for an IIO extended channel attribute > > + * @_name: Attribute name > > + * @_shared: Whether the attribute is shared between all channels > > + * @_what: Data private to the driver > > + */ > > +#define IIO_BACKEND_EX_INFO(_name, _shared, _what) { \ > > + .name = (_name), \ > > + .shared = (_shared), \ > > + .read = iio_backend_ext_info_get, \ > > + .write = iio_backend_ext_info_set, \ > > + .private = (_what), \ > > +} > > + > > /** > > * struct iio_backend_data_fmt - Backend data format > > * @type: Data type. > > @@ -35,8 +56,13 @@ struct iio_backend_data_fmt { > > * @chan_enable: Enable one channel. > > * @chan_disable: Disable one channel. > > * @data_format_set: Configure the data format for a specific channel. > > + * @data_source_set: Configure the data source for a specific channel. > > + * @set_sample_rate: Configure the sampling rate for a specific channel. > > * @request_buffer: Request an IIO buffer. > > * @free_buffer: Free an IIO buffer. > > + * @extend_chan_spec: Extend an IIO channel. > > + * @ext_info_set: Extended info setter. > > + * @ext_info_get: Extended info getter. > > **/ > > struct iio_backend_ops { > > int (*enable)(struct iio_backend *back); > > @@ -45,10 +71,21 @@ struct iio_backend_ops { > > int (*chan_disable)(struct iio_backend *back, unsigned int chan); > > int (*data_format_set)(struct iio_backend *back, unsigned int chan, > > const struct iio_backend_data_fmt *data); > > + int (*data_source_set)(struct iio_backend *back, unsigned int chan, > > + enum iio_backend_data_source data); > > + int (*set_sample_rate)(struct iio_backend *back, unsigned int chan, > > + u64 sample_rate); > > Name the parameter that so we know the units. _hz? yes. And u64 to not fall in the CCF problem for 32bits :) - Nuno Sá
On Thu, 28 Mar 2024 16:42:38 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Thu, 2024-03-28 at 15:16 +0000, Jonathan Cameron wrote: > > On Thu, 28 Mar 2024 14:22:32 +0100 > > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > > > > > From: Nuno Sa <nuno.sa@analog.com> > > > > > > This adds the needed backend ops for supporting a backend inerfacing > > > with an high speed dac. The new ops are: > > > > > > * data_source_set(); > > > * set_sampling_freq(); > > > * extend_chan_spec(); > > > * ext_info_set(); > > > * ext_info_get(). > > > > > > Also to note the new helpers that are meant to be used by the backends > > > + return 0; > > > + /* > > > + * !\NOTE: this will break as soon as we have multiple backends on one > > > + * frontend and all of them extend channels. In that case, the core > > > + * backend code has no way to get the correct backend given the > > > + * iio device. > > > + * > > > + * One solution for this could be introducing a new backend > > > + * dedicated callback in struct iio_info so we can callback into the > > > + * frontend so it can give us the right backend given a chan_spec. > > > + */ > > > > Hmm. This is indeed messy. Could we associate it with the buffer as presuably > > a front end with multiple backends is using multiple IIO buffers? > > > > Hmm, the assumption of having multiple buffers seems plausible to me but considering > the example we have in hands it would be cumbersome to get the backend. Considering > iio_backend_ext_info_get(), how could we get the backend if it was associated to one > of the IIO buffers? I think we would need more "intrusive" changes to make that work > or do you have something in mind= Nope. Just trying to get my head around the associations. I hadn't thought about how to make that visible in the code. Probably a callabck anyway. > > > As you say a dance via the front end would work fine. > > I'm happy you're also open for a proper solution already. I mention this in the > cover. My idea was something like (consider the iio_backend_ext_info_get()): > > if (!indio_dev->info->get_iio_backend()) > return -EOPNOTSUPP; > > back = indio_dev->info->get_iio_backend(indio_dev, chan_spec); > > It would be nice to have some "default/generic" implementation for cases where we > only have one backend per frontend so that the frontend would not need to define the > callback. Agreed - either a default that means if the callback isn't provided we get the single backend or if that proves fiddly at least a standard callback we can use in all such cases. > > > > > > > > + iio_device_set_drvdata(indio_dev, back); > > > + > > > + /* Don't allow backends to get creative and force their own handlers */ > > > + for (ext_info = chan->ext_info; ext_info->name; ext_info++) { > > > + if (ext_info->read != iio_backend_ext_info_get) > > > + return -EINVAL; > > > + if (ext_info->write != iio_backend_ext_info_set) > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iio_backend_extend_chan_spec, IIO_BACKEND); > > > > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > > > index a6d79381866e..09ff2f8f9fd8 100644 > > > --- a/include/linux/iio/backend.h > > > +++ b/include/linux/iio/backend.h > > > @@ -4,6 +4,7 @@ > > > > > > #include <linux/types.h> > > > > > > +struct iio_chan_spec; > > > struct fwnode_handle; > > > struct iio_backend; > > > struct device; > > > @@ -15,6 +16,26 @@ enum iio_backend_data_type { > > > IIO_BACKEND_DATA_TYPE_MAX > > > }; > > > > > > +enum iio_backend_data_source { > > > + IIO_BACKEND_INTERNAL_CW, > > > > CW? Either expand out what ever that is in definition of add a comment > > at least. > > Continuous wave :) Spell that out. > > > > > > + IIO_BACKEND_EXTERNAL, > > What does external mean in this case? > > In this particular case comes from a DMA source (IP). I thought external to be more > generic but if you prefer, I can do something like IIO_BACKEND_DMA? So from another IP block? For that to be reasonably 'generic' we'd need a way to known where it was coming from. Now I remember advantage of reviewing on weekends - fewer replies during the reviews :) Jonathan
On Thu, 2024-03-28 at 15:59 +0000, Jonathan Cameron wrote: > On Thu, 28 Mar 2024 16:42:38 +0100 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Thu, 2024-03-28 at 15:16 +0000, Jonathan Cameron wrote: > > > On Thu, 28 Mar 2024 14:22:32 +0100 > > > Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote: > > > > > > > From: Nuno Sa <nuno.sa@analog.com> > > > > > > > > This adds the needed backend ops for supporting a backend inerfacing > > > > with an high speed dac. The new ops are: > > > > > > > > * data_source_set(); > > > > * set_sampling_freq(); > > > > * extend_chan_spec(); > > > > * ext_info_set(); > > > > * ext_info_get(). > > > > > > > > Also to note the new helpers that are meant to be used by the backends > > > > + return 0; > > > > + /* > > > > + * !\NOTE: this will break as soon as we have multiple backends on > > > > one > > > > + * frontend and all of them extend channels. In that case, the core > > > > + * backend code has no way to get the correct backend given the > > > > + * iio device. > > > > + * > > > > + * One solution for this could be introducing a new backend > > > > + * dedicated callback in struct iio_info so we can callback into the > > > > + * frontend so it can give us the right backend given a chan_spec. > > > > + */ > > > > > > Hmm. This is indeed messy. Could we associate it with the buffer as presuably > > > a front end with multiple backends is using multiple IIO buffers? > > > > > > > Hmm, the assumption of having multiple buffers seems plausible to me but > > considering > > the example we have in hands it would be cumbersome to get the backend. > > Considering > > iio_backend_ext_info_get(), how could we get the backend if it was associated to > > one > > of the IIO buffers? I think we would need more "intrusive" changes to make that > > work > > or do you have something in mind= > > Nope. Just trying to get my head around the associations. I hadn't thought about > how to make that visible in the code. Probably a callabck anyway. > > > > > > As you say a dance via the front end would work fine. > > > > I'm happy you're also open for a proper solution already. I mention this in the > > cover. My idea was something like (consider the iio_backend_ext_info_get()): > > > > if (!indio_dev->info->get_iio_backend()) > > return -EOPNOTSUPP; > > > > back = indio_dev->info->get_iio_backend(indio_dev, chan_spec); > > > > It would be nice to have some "default/generic" implementation for cases where we > > only have one backend per frontend so that the frontend would not need to define > > the > > callback. > Agreed - either a default that means if the callback isn't provided we get the > single backend or if that proves fiddly at least a standard callback we can > use in all such cases. > I'll have to think a bit about it. We may need some association/link between iio_dev and iio_backend in order to "if the callback isn't provided we get the single backend". The easiest that comes to my mind without much thinking would be to use iio_device_set_drvdata()/iio_device_get_drvdata() in case the frontend does not provide a callback. This would already force callers to assign the indio_dev->info pointer before this call. Not that nice but acceptable if properly documented I guess. Anyways, I'll see if I can think of something better... > > > > > > > > > > > > + iio_device_set_drvdata(indio_dev, back); > > > > + > > > > + /* Don't allow backends to get creative and force their own handlers > > > > */ > > > > + for (ext_info = chan->ext_info; ext_info->name; ext_info++) { > > > > + if (ext_info->read != iio_backend_ext_info_get) > > > > + return -EINVAL; > > > > + if (ext_info->write != iio_backend_ext_info_set) > > > > + return -EINVAL; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > +EXPORT_SYMBOL_NS_GPL(iio_backend_extend_chan_spec, IIO_BACKEND); > > > > > > > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > > > > index a6d79381866e..09ff2f8f9fd8 100644 > > > > --- a/include/linux/iio/backend.h > > > > +++ b/include/linux/iio/backend.h > > > > @@ -4,6 +4,7 @@ > > > > > > > > #include <linux/types.h> > > > > > > > > +struct iio_chan_spec; > > > > struct fwnode_handle; > > > > struct iio_backend; > > > > struct device; > > > > @@ -15,6 +16,26 @@ enum iio_backend_data_type { > > > > IIO_BACKEND_DATA_TYPE_MAX > > > > }; > > > > > > > > +enum iio_backend_data_source { > > > > + IIO_BACKEND_INTERNAL_CW, > > > > > > CW? Either expand out what ever that is in definition of add a comment > > > at least. > > > > Continuous wave :) > > Spell that out. > > > > > > > > > > + IIO_BACKEND_EXTERNAL, > > > What does external mean in this case? > > > > In this particular case comes from a DMA source (IP). I thought external to be > > more > > generic but if you prefer, I can do something like IIO_BACKEND_DMA? > > So from another IP block? For that to be reasonably 'generic' we'd need a way > to known where it was coming from. > Yeps, in this case comes from the IIO DMA buffer which in HW means a DMA IP core... > Now I remember advantage of reviewing on weekends - fewer replies during the > reviews :) > :) - Nuno Sá
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 2fea2bbbe47f..0a26dd8c6343 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -43,6 +43,7 @@ #include <linux/types.h> #include <linux/iio/backend.h> +#include <linux/iio/iio.h> struct iio_backend { struct list_head entry; @@ -186,6 +187,44 @@ int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan, } EXPORT_SYMBOL_NS_GPL(iio_backend_data_format_set, IIO_BACKEND); +/** + * iio_backend_data_source_set - Select data source + * @back: Backend device + * @chan: Channel number + * @data: Data source + * + * A given backend may have different sources to stream/sync data. This allows + * to choose that source. + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_data_source_set(struct iio_backend *back, unsigned int chan, + enum iio_backend_data_source data) +{ + if (data >= IIO_BACKEND_DATA_SOURCE_MAX) + return -EINVAL; + + return iio_backend_op_call(back, data_source_set, chan, data); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_data_source_set, IIO_BACKEND); + +/** + * iio_backend_set_sampling_freq - Set channel sampling rate + * @back: Backend device + * @chan: Channel number + * @sample_rate: Sample rate + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_set_sampling_freq(struct iio_backend *back, unsigned int chan, + u64 sample_rate) +{ + return iio_backend_op_call(back, set_sample_rate, chan, sample_rate); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_set_sampling_freq, IIO_BACKEND); + static void iio_backend_free_buffer(void *arg) { struct iio_backend_buffer_pair *pair = arg; @@ -231,6 +270,111 @@ int devm_iio_backend_request_buffer(struct device *dev, } EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND); +/** + * iio_backend_ext_info_get - IIO ext_info read callback + * @indio_dev: IIO device + * @private: Data private to the driver + * @chan: IIO channel + * @buf: Buffer where to place the attribute data + * + * This helper is intended to be used by backends that extend an IIO channel + * (trough iio_backend_extend_chan_spec()) with extended info. In that case, + * backends are not supposed to give their own callbacks (as they would not + * a way to get te backend from indio_dev). This is the getter. + * + * RETURNS: + * Number of bytes written to buf, negative error number on failure. + */ +ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, char *buf) +{ + struct iio_backend *back = iio_device_get_drvdata(indio_dev); + + return iio_backend_op_call(back, ext_info_get, private, chan, buf); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_get, IIO_BACKEND); + +/** + * iio_backend_ext_info_set - IIO ext_info write callback + * @indio_dev: IIO device + * @private: Data private to the driver + * @chan: IIO channel + * @buf: Buffer holding the sysfs attribute + * @len: Buffer length + * + * This helper is intended to be used by backends that extend an IIO channel + * (trough iio_backend_extend_chan_spec()) with extended info. In that case, + * backends are not supposed to give their own callbacks (as they would not + * a way to get te backend from indio_dev). This is the setter. + * + * RETURNS: + * Buffer length on success, negative error number on failure. + */ +ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len) +{ + struct iio_backend *back = iio_device_get_drvdata(indio_dev); + + return iio_backend_op_call(back, ext_info_set, private, chan, buf, len); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_ext_info_set, IIO_BACKEND); + +/** + * iio_backend_extend_chan_spec - Extend an IIO channel + * @indio_dev: IIO device + * @back: Backend device + * @chan: IIO channel + * + * Some backends may have their own functionalities and hence capable of + * extending a frontend's channel. + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, + struct iio_backend *back, + struct iio_chan_spec *chan) +{ + const struct iio_chan_spec_ext_info *ext_info = chan->ext_info; + int ret; + + ret = iio_backend_op_call(back, extend_chan_spec, chan); + if (ret) + return ret; + /* + * Let's keep things simple for now. Don't allow to overwrite the + * frontend's extended info. If ever needed, we can support appending + * it. + */ + if (ext_info && chan->ext_info != ext_info) + return -EOPNOTSUPP; + if (!chan->ext_info) + return 0; + /* + * !\NOTE: this will break as soon as we have multiple backends on one + * frontend and all of them extend channels. In that case, the core + * backend code has no way to get the correct backend given the + * iio device. + * + * One solution for this could be introducing a new backend + * dedicated callback in struct iio_info so we can callback into the + * frontend so it can give us the right backend given a chan_spec. + */ + iio_device_set_drvdata(indio_dev, back); + + /* Don't allow backends to get creative and force their own handlers */ + for (ext_info = chan->ext_info; ext_info->name; ext_info++) { + if (ext_info->read != iio_backend_ext_info_get) + return -EINVAL; + if (ext_info->write != iio_backend_ext_info_set) + return -EINVAL; + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(iio_backend_extend_chan_spec, IIO_BACKEND); + static void iio_backend_release(void *arg) { struct iio_backend *back = arg; diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index a6d79381866e..09ff2f8f9fd8 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -4,6 +4,7 @@ #include <linux/types.h> +struct iio_chan_spec; struct fwnode_handle; struct iio_backend; struct device; @@ -15,6 +16,26 @@ enum iio_backend_data_type { IIO_BACKEND_DATA_TYPE_MAX }; +enum iio_backend_data_source { + IIO_BACKEND_INTERNAL_CW, + IIO_BACKEND_EXTERNAL, + IIO_BACKEND_DATA_SOURCE_MAX +}; + +/** + * IIO_BACKEND_EX_INFO - Helper for an IIO extended channel attribute + * @_name: Attribute name + * @_shared: Whether the attribute is shared between all channels + * @_what: Data private to the driver + */ +#define IIO_BACKEND_EX_INFO(_name, _shared, _what) { \ + .name = (_name), \ + .shared = (_shared), \ + .read = iio_backend_ext_info_get, \ + .write = iio_backend_ext_info_set, \ + .private = (_what), \ +} + /** * struct iio_backend_data_fmt - Backend data format * @type: Data type. @@ -35,8 +56,13 @@ struct iio_backend_data_fmt { * @chan_enable: Enable one channel. * @chan_disable: Disable one channel. * @data_format_set: Configure the data format for a specific channel. + * @data_source_set: Configure the data source for a specific channel. + * @set_sample_rate: Configure the sampling rate for a specific channel. * @request_buffer: Request an IIO buffer. * @free_buffer: Free an IIO buffer. + * @extend_chan_spec: Extend an IIO channel. + * @ext_info_set: Extended info setter. + * @ext_info_get: Extended info getter. **/ struct iio_backend_ops { int (*enable)(struct iio_backend *back); @@ -45,10 +71,21 @@ struct iio_backend_ops { int (*chan_disable)(struct iio_backend *back, unsigned int chan); int (*data_format_set)(struct iio_backend *back, unsigned int chan, const struct iio_backend_data_fmt *data); + int (*data_source_set)(struct iio_backend *back, unsigned int chan, + enum iio_backend_data_source data); + int (*set_sample_rate)(struct iio_backend *back, unsigned int chan, + u64 sample_rate); struct iio_buffer *(*request_buffer)(struct iio_backend *back, struct iio_dev *indio_dev); void (*free_buffer)(struct iio_backend *back, struct iio_buffer *buffer); + int (*extend_chan_spec)(struct iio_backend *back, + struct iio_chan_spec *chan); + int (*ext_info_set)(struct iio_backend *back, uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len); + int (*ext_info_get)(struct iio_backend *back, uintptr_t private, + const struct iio_chan_spec *chan, char *buf); }; int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan); @@ -56,10 +93,22 @@ int iio_backend_chan_disable(struct iio_backend *back, unsigned int chan); int devm_iio_backend_enable(struct device *dev, struct iio_backend *back); int iio_backend_data_format_set(struct iio_backend *back, unsigned int chan, const struct iio_backend_data_fmt *data); +int iio_backend_data_source_set(struct iio_backend *back, unsigned int chan, + enum iio_backend_data_source data); +int iio_backend_set_sampling_freq(struct iio_backend *back, unsigned int chan, + u64 sample_rate); int devm_iio_backend_request_buffer(struct device *dev, struct iio_backend *back, struct iio_dev *indio_dev); +ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, + const char *buf, size_t len); +ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private, + const struct iio_chan_spec *chan, char *buf); +int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, + struct iio_backend *back, + struct iio_chan_spec *chan); void *iio_backend_get_priv(const struct iio_backend *conv); struct iio_backend *devm_iio_backend_get(struct device *dev, const char *name); struct iio_backend *