Message ID | 20241111121203.3699-4-antoniu.miclaus@analog.com |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | *** Add support for AD485x DAS Family *** | expand |
On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote: > Add backend support for enabling/disabling oversampling. > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > --- > changes in v6: > - add iio backend commit for oversampling enable/disable > drivers/iio/industrialio-backend.c | 14 ++++++++++++++ > include/linux/iio/backend.h | 3 +++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > backend.c > index ea184fc2c838..6ba445ba3dd0 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back, > unsigned int size) > } > EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND); > > +/** > + * iio_backend_oversampling_en - set the data width/size in the data bus. Seems unrelated? > + * @back: Backend device > + * @en: oversampling enabled/disabled. > + * > + * Return: > + * 0 on success, negative error number on failure. > + */ > +int iio_backend_oversampling_en(struct iio_backend *back, bool en) > +{ > + return iio_backend_op_call(back, oversampling_en, en); > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND); > + There was some discussion around having APIs with a boolean parameter (actually even improving - in terms of callbacks - further with some generic getter/setter's) or having two callbacks: iio_backend_oversampling_enable() iio_backend_oversampling_disable() I'm guessing you don't really want to do any major conversion/refactoring at this point in your series so I have a slight preference for just keeping the current style of dedicated enable and disable APIs (irrespective of being the better approach or not). Please consider it, if you have to re-spin the series. - Nuno Sá
On Mon, 11 Nov 2024 16:41:02 +0100 Nuno Sá <noname.nuno@gmail.com> wrote: > On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote: > > Add backend support for enabling/disabling oversampling. > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > --- > > changes in v6: > > - add iio backend commit for oversampling enable/disable > > drivers/iio/industrialio-backend.c | 14 ++++++++++++++ > > include/linux/iio/backend.h | 3 +++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > backend.c > > index ea184fc2c838..6ba445ba3dd0 100644 > > --- a/drivers/iio/industrialio-backend.c > > +++ b/drivers/iio/industrialio-backend.c > > @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back, > > unsigned int size) > > } > > EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND); > > > > +/** > > + * iio_backend_oversampling_en - set the data width/size in the data bus. > > Seems unrelated? > > > + * @back: Backend device > > + * @en: oversampling enabled/disabled. > > + * > > + * Return: > > + * 0 on success, negative error number on failure. > > + */ > > +int iio_backend_oversampling_en(struct iio_backend *back, bool en) Odd to just be on or off vs a count of how much to oversample by with 1 meaning don't oversample, 2,4,8 etc saying how much to oversample by. > > +{ > > + return iio_backend_op_call(back, oversampling_en, en); > > +} > > +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND); > > + > > There was some discussion around having APIs with a boolean parameter (actually > even improving - in terms of callbacks - further with some generic > getter/setter's) or having two callbacks: > > iio_backend_oversampling_enable() > iio_backend_oversampling_disable() > > I'm guessing you don't really want to do any major conversion/refactoring at > this point in your series so I have a slight preference for just keeping the > current style of dedicated enable and disable APIs (irrespective of being the > better approach or not). Please consider it, if you have to re-spin the series. Agreed. Keep it consistent for now. I don't mind a proposal to refactor the lot (though not yet convinced either way on it being a good idea) but I don't want to see it inconsistent. > > - Nuno Sá > >
On Sat, 23 Nov 2024 16:02:49 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Mon, 11 Nov 2024 16:41:02 +0100 > Nuno Sá <noname.nuno@gmail.com> wrote: > > > On Mon, 2024-11-11 at 14:11 +0200, Antoniu Miclaus wrote: > > > Add backend support for enabling/disabling oversampling. > > > > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> > > > --- > > > changes in v6: > > > - add iio backend commit for oversampling enable/disable > > > drivers/iio/industrialio-backend.c | 14 ++++++++++++++ > > > include/linux/iio/backend.h | 3 +++ > > > 2 files changed, 17 insertions(+) > > > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > > backend.c > > > index ea184fc2c838..6ba445ba3dd0 100644 > > > --- a/drivers/iio/industrialio-backend.c > > > +++ b/drivers/iio/industrialio-backend.c > > > @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back, > > > unsigned int size) > > > } > > > EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND); > > > > > > +/** > > > + * iio_backend_oversampling_en - set the data width/size in the data bus. > > > > Seems unrelated? > > > > > + * @back: Backend device > > > + * @en: oversampling enabled/disabled. > > > + * > > > + * Return: > > > + * 0 on success, negative error number on failure. > > > + */ > > > +int iio_backend_oversampling_en(struct iio_backend *back, bool en) > Odd to just be on or off vs a count of how much to oversample by > with 1 meaning don't oversample, 2,4,8 etc saying how much to oversample by. Looking at the code, why does the backend care? Seems you only set the ratio on the ADC, not the in the FPGA IP. So I don't follow how that is useful Jonathan > > > > +{ > > > + return iio_backend_op_call(back, oversampling_en, en); > > > +} > > > +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND); > > > + > > > > There was some discussion around having APIs with a boolean parameter (actually > > even improving - in terms of callbacks - further with some generic > > getter/setter's) or having two callbacks: > > > > iio_backend_oversampling_enable() > > iio_backend_oversampling_disable() > > > > I'm guessing you don't really want to do any major conversion/refactoring at > > this point in your series so I have a slight preference for just keeping the > > current style of dedicated enable and disable APIs (irrespective of being the > > better approach or not). Please consider it, if you have to re-spin the series. > Agreed. Keep it consistent for now. I don't mind a proposal to refactor > the lot (though not yet convinced either way on it being a good idea) > but I don't want to see it inconsistent. > > > > > - Nuno Sá > > > > > >
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index ea184fc2c838..6ba445ba3dd0 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -681,6 +681,20 @@ int iio_backend_data_size_set(struct iio_backend *back, unsigned int size) } EXPORT_SYMBOL_NS_GPL(iio_backend_data_size_set, IIO_BACKEND); +/** + * iio_backend_oversampling_en - set the data width/size in the data bus. + * @back: Backend device + * @en: oversampling enabled/disabled. + * + * Return: + * 0 on success, negative error number on failure. + */ +int iio_backend_oversampling_en(struct iio_backend *back, bool en) +{ + return iio_backend_op_call(back, oversampling_en, en); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_oversampling_en, IIO_BACKEND); + /** * iio_backend_extend_chan_spec - Extend an IIO channel * @back: Backend device diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index 59b6651b7eaf..b8f63823b3db 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -94,6 +94,7 @@ enum iio_backend_interface_type { * @ext_info_get: Extended info getter. * @interface_type_get: Interface type. * @data_size_set: Data size. + * @oversampling_en: Oversampling. * @read_raw: Read a channel attribute from a backend device * @debugfs_print_chan_status: Print channel status into a buffer. * @debugfs_reg_access: Read or write register value of backend. @@ -132,6 +133,7 @@ struct iio_backend_ops { int (*interface_type_get)(struct iio_backend *back, enum iio_backend_interface_type *type); int (*data_size_set)(struct iio_backend *back, unsigned int size); + int (*oversampling_en)(struct iio_backend *back, bool en); int (*read_raw)(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, int *val2, long mask); @@ -183,6 +185,7 @@ ssize_t iio_backend_ext_info_get(struct iio_dev *indio_dev, uintptr_t private, int iio_backend_interface_type_get(struct iio_backend *back, enum iio_backend_interface_type *type); int iio_backend_data_size_set(struct iio_backend *back, unsigned int size); +int iio_backend_oversampling_en(struct iio_backend *back, bool en); int iio_backend_read_raw(struct iio_backend *back, struct iio_chan_spec const *chan, int *val, int *val2, long mask);
Add backend support for enabling/disabling oversampling. Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com> --- changes in v6: - add iio backend commit for oversampling enable/disable drivers/iio/industrialio-backend.c | 14 ++++++++++++++ include/linux/iio/backend.h | 3 +++ 2 files changed, 17 insertions(+)