| Message ID | 20240625150717.1038212-2-olivier.moysan@foss.st.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: dfsdm: add scaling support | expand |
On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote: > Add iio_backend_read_raw() service to support attributes read > from an IIO backend. > > Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com> > Reviewed-by: Nuno Sa <nuno.sa@analog.com> > --- > drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++ > include/linux/iio/backend.h | 6 +++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > backend.c > index 929aff4040ed..0e2653de1956 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev, > } > EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND); > > +/** > + * iio_backend_read_raw - Request a channel attribute from the IIO backend. > + * @back: Backend device > + * @chan: IIO channel reference > + * @val: First element of the returned value > + * @val2: Second element of the returned value > + * @mask: Specify value to retrieve > + * > + * This callback replicates the read_raw callback of the IIO framework, and > is intended to > + * request miscellaneous channel attributes from the backend device. > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const > *chan, int *val, > + int *val2, long mask) > +{ > + return iio_backend_op_call(back, read_raw, chan, val, val2, mask); > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND); I actually got an idea when looking at this for my existential crisis between dedicated APIs and a catch all .read_raw() :). What we can do is just provide the .read_raw() or write_raw() ops to backends (so we minimize the number of ops) and then we build on top of them for providing more readable (depending on the case; some cases it does make sense to just call iio_backend_read_raw()) APIs to frontends. So in your case you could have in backend.h static inline int iio_backend_read_scale(...) { return iio_backend_read_raw(..., IIO_CHAN_INFO_SCALE); } Naturally no need for you to do this right now in your series. Just wanted to write it down before I go into other stuff and forget about this :) - Nuno Sá
Hi Nuno, On 6/26/24 10:50, Nuno Sá wrote: > On Tue, 2024-06-25 at 17:07 +0200, Olivier Moysan wrote: >> Add iio_backend_read_raw() service to support attributes read >> from an IIO backend. >> >> Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com> >> Reviewed-by: Nuno Sa <nuno.sa@analog.com> >> --- >> drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++ >> include/linux/iio/backend.h | 6 +++++- >> 2 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- >> backend.c >> index 929aff4040ed..0e2653de1956 100644 >> --- a/drivers/iio/industrialio-backend.c >> +++ b/drivers/iio/industrialio-backend.c >> @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev, >> } >> EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND); >> >> +/** >> + * iio_backend_read_raw - Request a channel attribute from the IIO backend. >> + * @back: Backend device >> + * @chan: IIO channel reference >> + * @val: First element of the returned value >> + * @val2: Second element of the returned value >> + * @mask: Specify value to retrieve >> + * >> + * This callback replicates the read_raw callback of the IIO framework, and >> is intended to >> + * request miscellaneous channel attributes from the backend device. >> + * >> + * RETURNS: >> + * 0 on success, negative error number on failure. >> + */ >> +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const >> *chan, int *val, >> + int *val2, long mask) >> +{ >> + return iio_backend_op_call(back, read_raw, chan, val, val2, mask); >> +} >> +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND); > > I actually got an idea when looking at this for my existential crisis between > dedicated APIs and a catch all .read_raw() :). What we can do is just provide > the .read_raw() or write_raw() ops to backends (so we minimize the number of > ops) and then we build on top of them for providing more readable (depending on > the case; some cases it does make sense to just call iio_backend_read_raw()) > APIs to frontends. > > So in your case you could have in backend.h > > static inline int iio_backend_read_scale(...) > { > return iio_backend_read_raw(..., IIO_CHAN_INFO_SCALE); > } > > Naturally no need for you to do this right now in your series. Just wanted to > write it down before I go into other stuff and forget about this :) > Yes, this is a good compromise. Such helpers are more user-friendly for the consumer. As long as I have to push a v3, I might take the opportunity to add this. BRs Olivier > - Nuno Sá > >
On Tue, 25 Jun 2024 17:07:09 +0200 Olivier Moysan <olivier.moysan@foss.st.com> wrote: > Add iio_backend_read_raw() service to support attributes read > from an IIO backend. > > Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com> > Reviewed-by: Nuno Sa <nuno.sa@analog.com> Other than line wrapping moans this looks good to me. J > --- > drivers/iio/industrialio-backend.c | 21 +++++++++++++++++++++ > include/linux/iio/backend.h | 6 +++++- > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c > index 929aff4040ed..0e2653de1956 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev, > } > EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND); > > +/** > + * iio_backend_read_raw - Request a channel attribute from the IIO backend. > + * @back: Backend device > + * @chan: IIO channel reference > + * @val: First element of the returned value > + * @val2: Second element of the returned value > + * @mask: Specify value to retrieve > + * > + * This callback replicates the read_raw callback of the IIO framework, and is intended to > + * request miscellaneous channel attributes from the backend device. For IIO code, please still wrap at 80 chars unless there is a good reason to got longer. > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, Likewise. Wrap this shorter. > + int *val2, long mask) > +{ > + return iio_backend_op_call(back, read_raw, chan, val, val2, mask); > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND); > + > static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev) > { > struct iio_backend *back = ERR_PTR(-ENODEV), *iter; > diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h > index 8099759d7242..24185718b20d 100644 > --- a/include/linux/iio/backend.h > +++ b/include/linux/iio/backend.h > @@ -81,6 +81,7 @@ enum iio_backend_sample_trigger { > * @extend_chan_spec: Extend an IIO channel. > * @ext_info_set: Extended info setter. > * @ext_info_get: Extended info getter. > + * @read_raw: Read value from a backend device > **/ > struct iio_backend_ops { > int (*enable)(struct iio_backend *back); > @@ -113,6 +114,8 @@ struct iio_backend_ops { > 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 (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, And here. > + int *val2, long mask); > }; > > int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan); > @@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, > 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_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, and here. > + int *val2, long mask); > int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, > struct iio_backend *back, > struct iio_chan_spec *chan);
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 929aff4040ed..0e2653de1956 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -357,6 +357,27 @@ int devm_iio_backend_request_buffer(struct device *dev, } EXPORT_SYMBOL_NS_GPL(devm_iio_backend_request_buffer, IIO_BACKEND); +/** + * iio_backend_read_raw - Request a channel attribute from the IIO backend. + * @back: Backend device + * @chan: IIO channel reference + * @val: First element of the returned value + * @val2: Second element of the returned value + * @mask: Specify value to retrieve + * + * This callback replicates the read_raw callback of the IIO framework, and is intended to + * request miscellaneous channel attributes from the backend device. + * + * RETURNS: + * 0 on success, negative error number on failure. + */ +int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + return iio_backend_op_call(back, read_raw, chan, val, val2, mask); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_read_raw, IIO_BACKEND); + static struct iio_backend *iio_backend_from_indio_dev_parent(const struct device *dev) { struct iio_backend *back = ERR_PTR(-ENODEV), *iter; diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index 8099759d7242..24185718b20d 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -81,6 +81,7 @@ enum iio_backend_sample_trigger { * @extend_chan_spec: Extend an IIO channel. * @ext_info_set: Extended info setter. * @ext_info_get: Extended info getter. + * @read_raw: Read value from a backend device **/ struct iio_backend_ops { int (*enable)(struct iio_backend *back); @@ -113,6 +114,8 @@ struct iio_backend_ops { 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 (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, + int *val2, long mask); }; int iio_backend_chan_enable(struct iio_backend *back, unsigned int chan); @@ -141,7 +144,8 @@ ssize_t iio_backend_ext_info_set(struct iio_dev *indio_dev, uintptr_t private, 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_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, + int *val2, long mask); int iio_backend_extend_chan_spec(struct iio_dev *indio_dev, struct iio_backend *back, struct iio_chan_spec *chan);