| Message ID | 20250324090813.2775011-2-pop.ioan-daniel@analog.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for AD7405/ADUM770x | expand |
On Mon, Mar 24, 2025 at 11:07:56AM +0200, Pop Ioan Daniel wrote: > Add backend support for setting the decimation ratio used. ... > +/** > + * iio_backend_set_dec_rate - set decimation ratio > + * @back: Backend device > + * @rate: Rate in decimal > + Something is missing here... > + * Return: > + * 0 on success, negative error number on failure. Please, double check that the style of Return section is the same as for the rest of the functions in this file. If there are different styles choose one which is simultaneously more recent and has more common sense in it (like Returns: vs. Return, the preferred is the latter). > + */ > + Here is an uneeded blank line. > +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate) > +{ > + if (!rate) > + return -EINVAL; Yeah, besides missing style comment above, the function is undescribed. You really need to add something meaningful in the kernel-doc and esp. explain corner cases like this and choices made (why we fail it, what's wrong with 0?). > + return iio_backend_op_call(back, set_dec_rate, rate); > +}
On 3/24/25 4:07 AM, Pop Ioan Daniel wrote: > Add backend support for setting the decimation ratio used. > > Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com> > --- > drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++ > include/linux/iio/backend.h | 3 +++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c > index 363281272035..f4db6514944a 100644 > --- a/drivers/iio/industrialio-backend.c > +++ b/drivers/iio/industrialio-backend.c > @@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back, > } > EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND"); > > +/** > + * iio_backend_set_dec_rate - set decimation ratio In [1], we decided that for a similar chip we could use "decimation rate" and "oversampling ratio" interchangeably. And in [2], we recently added a backend op for setting the oversampling ratio. Would it make sense to use that here as well instead of introducing a new function? If not, we'll want more of an explanation here on what the difference is. But from what I can tell, this sounds very similar to the other case where they are essentially the same thing. On the other hand, this is internal API and not userspace ABI, so having a separate name might not be so bad, especially if there is a chance we would ever have both at the same time. [1]: https://lore.kernel.org/linux-iio/20250112122047.1e1978e0@jic23-huawei/ [2]: https://web.git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/industrialio-backend.c?h=testing&id=22894e0be908791e3df011cdfac02589c2f340bd > + * @back: Backend device > + * @rate: Rate in decimal > + > + * Return: > + * 0 on success, negative error number on failure. > + */ > + > +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate) > +{ > + if (!rate) > + return -EINVAL; > + > + return iio_backend_op_call(back, set_dec_rate, rate); > +} > +EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND"); > +
On Mon, 2025-03-24 at 13:53 -0500, David Lechner wrote: > On 3/24/25 4:07 AM, Pop Ioan Daniel wrote: > > Add backend support for setting the decimation ratio used. > > > > Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com> > > --- > > drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++ > > include/linux/iio/backend.h | 3 +++ > > 2 files changed, 21 insertions(+) > > > > diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio- > > backend.c > > index 363281272035..f4db6514944a 100644 > > --- a/drivers/iio/industrialio-backend.c > > +++ b/drivers/iio/industrialio-backend.c > > @@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back, > > } > > EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND"); > > > > +/** > > + * iio_backend_set_dec_rate - set decimation ratio > > In [1], we decided that for a similar chip we could use "decimation rate" and > "oversampling ratio" interchangeably. And in [2], we recently added a backend > op for setting the oversampling ratio. Would it make sense to use that here > as well instead of introducing a new function? If not, we'll want more of an > explanation here on what the difference is. But from what I can tell, this > sounds very similar to the other case where they are essentially the same thing. > On the other hand, this is internal API and not userspace ABI, so having a > separate name might not be so bad, especially if there is a chance we would ever > have both at the same time. If it fits, I have preference for using existent interfaces... - Nuno Sá > > [1]: https://lore.kernel.org/linux-iio/20250112122047.1e1978e0@jic23-huawei/ > [2]: > https://web.git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/industrialio-backend.c?h=testing&id=22894e0be908791e3df011cdfac02589c2f340bd > > > + * @back: Backend device > > + * @rate: Rate in decimal > > + > > + * Return: > > + * 0 on success, negative error number on failure. > > + */ > > + > > +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate) > > +{ > > + if (!rate) > > + return -EINVAL; > > + > > + return iio_backend_op_call(back, set_dec_rate, rate); > > +} > > +EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND"); > > +
diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c index 363281272035..f4db6514944a 100644 --- a/drivers/iio/industrialio-backend.c +++ b/drivers/iio/industrialio-backend.c @@ -417,6 +417,24 @@ int iio_backend_test_pattern_set(struct iio_backend *back, } EXPORT_SYMBOL_NS_GPL(iio_backend_test_pattern_set, "IIO_BACKEND"); +/** + * iio_backend_set_dec_rate - set decimation ratio + * @back: Backend device + * @rate: Rate in decimal + + * Return: + * 0 on success, negative error number on failure. + */ + +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate) +{ + if (!rate) + return -EINVAL; + + return iio_backend_op_call(back, set_dec_rate, rate); +} +EXPORT_SYMBOL_NS_GPL(iio_backend_set_dec_rate, "IIO_BACKEND"); + /** * iio_backend_chan_status - Get the channel status * @back: Backend device diff --git a/include/linux/iio/backend.h b/include/linux/iio/backend.h index 10be00f3b120..e73d7d265a16 100644 --- a/include/linux/iio/backend.h +++ b/include/linux/iio/backend.h @@ -80,6 +80,7 @@ enum iio_backend_sample_trigger { * @data_source_set: Configure the data source for a specific channel. * @set_sample_rate: Configure the sampling rate for a specific channel. * @test_pattern_set: Configure a test pattern. + * @set_dec_rate: Set decimation ratio * @chan_status: Get the channel status. * @iodelay_set: Set digital I/O delay. * @data_sample_trigger: Control when to sample data. @@ -111,6 +112,7 @@ struct iio_backend_ops { int (*test_pattern_set)(struct iio_backend *back, unsigned int chan, enum iio_backend_test_pattern pattern); + int (*set_dec_rate)(struct iio_backend *back, unsigned int rate); int (*chan_status)(struct iio_backend *back, unsigned int chan, bool *error); int (*iodelay_set)(struct iio_backend *back, unsigned int chan, @@ -167,6 +169,7 @@ int iio_backend_set_sampling_freq(struct iio_backend *back, unsigned int chan, int iio_backend_test_pattern_set(struct iio_backend *back, unsigned int chan, enum iio_backend_test_pattern pattern); +int iio_backend_set_dec_rate(struct iio_backend *back, unsigned int rate); int iio_backend_chan_status(struct iio_backend *back, unsigned int chan, bool *error); int iio_backend_iodelay_set(struct iio_backend *back, unsigned int lane,
Add backend support for setting the decimation ratio used. Signed-off-by: Pop Ioan Daniel <pop.ioan-daniel@analog.com> --- drivers/iio/industrialio-backend.c | 18 ++++++++++++++++++ include/linux/iio/backend.h | 3 +++ 2 files changed, 21 insertions(+)