| Message ID | 20211222034646.222189-5-liambeguin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: afe: add temperature rescaling support | expand |
On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: > > From: Liam Beguin <lvb@xiphos.com> > > In preparation for the addition of kunit tests, expose the logic > responsible for combining channel scales. ... > #include <linux/gcd.h> > #include <linux/iio/consumer.h> > #include <linux/iio/iio.h> > +#include <linux/iio/afe/rescale.h> It should go before the consumer.h, no? And I would rather move the entire IIO group of headers... > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/property.h> ... somewhere here (with blank line above). > > -struct rescale; ... > +#ifndef __IIO_RESCALE_H__ > +#define __IIO_RESCALE_H__ > + > +#include <linux/iio/iio.h> Missed types.h and forward declarations like struct device;
Hi Andy, On Wed, Dec 22, 2021 at 12:21:01PM +0200, Andy Shevchenko wrote: > On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: > > > > From: Liam Beguin <lvb@xiphos.com> > > > > In preparation for the addition of kunit tests, expose the logic > > responsible for combining channel scales. > > ... > > > #include <linux/gcd.h> > > #include <linux/iio/consumer.h> > > #include <linux/iio/iio.h> > > +#include <linux/iio/afe/rescale.h> > > It should go before the consumer.h, no? I don't mind making the change, but why should it go before consumer.h? > And I would rather move the entire IIO group of headers... I can do that too. Do we have a convention for the ordering of #includes? What's usually the rule/guideline for this? > > #include <linux/module.h> > > #include <linux/of.h> > > #include <linux/of_device.h> > > #include <linux/platform_device.h> > > #include <linux/property.h> > > ... somewhere here (with blank line above). > > > > > -struct rescale; > > ... > > > +#ifndef __IIO_RESCALE_H__ > > +#define __IIO_RESCALE_H__ > > + > > +#include <linux/iio/iio.h> > > Missed types.h and forward declarations like > struct device; Okay. will add linux/types.h Cheers, Liam > -- > With Best Regards, > Andy Shevchenko
On Wed, Dec 22, 2021 at 8:20 PM Liam Beguin <liambeguin@gmail.com> wrote: > On Wed, Dec 22, 2021 at 12:21:01PM +0200, Andy Shevchenko wrote: > > On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: ... > > > #include <linux/iio/consumer.h> > > > #include <linux/iio/iio.h> > > > +#include <linux/iio/afe/rescale.h> > > > > It should go before the consumer.h, no? > > I don't mind making the change, but why should it go before consumer.h? 'a' is earlier than 'c' in the alphabet, no? ... > > And I would rather move the entire IIO group of headers... > > I can do that too. Do we have a convention for the ordering of #includes? > What's usually the rule/guideline for this? Guidelines suggest sorting without clear instructions. But in IIO and pin control I suggest people use this kind of grouping. > > > #include <linux/module.h> > > > #include <linux/of.h> > > > #include <linux/of_device.h> > > > #include <linux/platform_device.h> > > > #include <linux/property.h> > > > > ... somewhere here (with blank line above). > > > > > -struct rescale; ... > > Missed types.h and forward declarations like > > struct device; > > Okay. will add linux/types.h What about forward declaration?
On Wed, Dec 22, 2021 at 08:52:30PM +0200, Andy Shevchenko wrote: > On Wed, Dec 22, 2021 at 8:20 PM Liam Beguin <liambeguin@gmail.com> wrote: > > On Wed, Dec 22, 2021 at 12:21:01PM +0200, Andy Shevchenko wrote: > > > On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: > > ... > > > > > #include <linux/iio/consumer.h> > > > > #include <linux/iio/iio.h> > > > > +#include <linux/iio/afe/rescale.h> > > > > > > It should go before the consumer.h, no? > > > > I don't mind making the change, but why should it go before consumer.h? > > 'a' is earlier than 'c' in the alphabet, no? > > ... > > > > And I would rather move the entire IIO group of headers... > > > > I can do that too. Do we have a convention for the ordering of #includes? > > What's usually the rule/guideline for this? > > Guidelines suggest sorting without clear instructions. But in IIO and > pin control I suggest people use this kind of grouping. Understood, will update. > > > > #include <linux/module.h> > > > > #include <linux/of.h> > > > > #include <linux/of_device.h> > > > > #include <linux/platform_device.h> > > > > #include <linux/property.h> > > > > > > ... somewhere here (with blank line above). > > > > > > > -struct rescale; > > ... > > > > Missed types.h and forward declarations like > > > struct device; > > > > Okay. will add linux/types.h > > What about forward declaration? I'm not sure I understand what you mean here. Cheers, Liam > -- > With Best Regards, > Andy Shevchenko
On Wed, Dec 22, 2021 at 9:42 PM Liam Beguin <liambeguin@gmail.com> wrote: > On Wed, Dec 22, 2021 at 08:52:30PM +0200, Andy Shevchenko wrote: > > On Wed, Dec 22, 2021 at 8:20 PM Liam Beguin <liambeguin@gmail.com> wrote: > > > On Wed, Dec 22, 2021 at 12:21:01PM +0200, Andy Shevchenko wrote: > > > > On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: ... > > > > Missed types.h and forward declarations like ^^^^ (1) > > > > struct device; > > > > > > Okay. will add linux/types.h ^^^^ (2) > > What about forward declaration? > > I'm not sure I understand what you mean here. In (1) I have mentioned header and forward declaration. You agreed in (2) to add a header. What about forward declaration?
On Wed, Dec 22, 2021 at 09:50:28PM +0200, Andy Shevchenko wrote: > On Wed, Dec 22, 2021 at 9:42 PM Liam Beguin <liambeguin@gmail.com> wrote: > > On Wed, Dec 22, 2021 at 08:52:30PM +0200, Andy Shevchenko wrote: > > > On Wed, Dec 22, 2021 at 8:20 PM Liam Beguin <liambeguin@gmail.com> wrote: > > > > On Wed, Dec 22, 2021 at 12:21:01PM +0200, Andy Shevchenko wrote: > > > > > On Wed, Dec 22, 2021 at 5:46 AM Liam Beguin <liambeguin@gmail.com> wrote: > > ... > > > > > > Missed types.h and forward declarations like > > ^^^^ (1) > > > > > > struct device; > > > > > > > > Okay. will add linux/types.h > > ^^^^ (2) > > > > What about forward declaration? > > > > I'm not sure I understand what you mean here. > > In (1) I have mentioned header and forward declaration. You agreed in > (2) to add a header. What about forward declaration? Oh, understood, sorry I misread your message. I'll add `struct device;` above `struct rescale;`. Cheers, Liam > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c index 774eb3044edd..d0669fd8eac5 100644 --- a/drivers/iio/afe/iio-rescale.c +++ b/drivers/iio/afe/iio-rescale.c @@ -11,35 +11,46 @@ #include <linux/gcd.h> #include <linux/iio/consumer.h> #include <linux/iio/iio.h> +#include <linux/iio/afe/rescale.h> #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/property.h> -struct rescale; - -struct rescale_cfg { - enum iio_chan_type type; - int (*props)(struct device *dev, struct rescale *rescale); -}; +int rescale_process_scale(struct rescale *rescale, int scale_type, + int *val, int *val2) +{ + unsigned long long tmp; -struct rescale { - const struct rescale_cfg *cfg; - struct iio_channel *source; - struct iio_chan_spec chan; - struct iio_chan_spec_ext_info *ext_info; - bool chan_processed; - s32 numerator; - s32 denominator; -}; + switch (scale_type) { + case IIO_VAL_FRACTIONAL: + *val *= rescale->numerator; + *val2 *= rescale->denominator; + return scale_type; + case IIO_VAL_INT: + *val *= rescale->numerator; + if (rescale->denominator == 1) + return scale_type; + *val2 = rescale->denominator; + return IIO_VAL_FRACTIONAL; + case IIO_VAL_FRACTIONAL_LOG2: + tmp = *val * 1000000000LL; + do_div(tmp, rescale->denominator); + tmp *= rescale->numerator; + do_div(tmp, 1000000000LL); + *val = tmp; + return scale_type; + default: + return -EOPNOTSUPP; + } +} static int rescale_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) { struct rescale *rescale = iio_priv(indio_dev); - unsigned long long tmp; int ret; switch (mask) { @@ -65,27 +76,7 @@ static int rescale_read_raw(struct iio_dev *indio_dev, } else { ret = iio_read_channel_scale(rescale->source, val, val2); } - switch (ret) { - case IIO_VAL_FRACTIONAL: - *val *= rescale->numerator; - *val2 *= rescale->denominator; - return ret; - case IIO_VAL_INT: - *val *= rescale->numerator; - if (rescale->denominator == 1) - return ret; - *val2 = rescale->denominator; - return IIO_VAL_FRACTIONAL; - case IIO_VAL_FRACTIONAL_LOG2: - tmp = *val * 1000000000LL; - do_div(tmp, rescale->denominator); - tmp *= rescale->numerator; - do_div(tmp, 1000000000LL); - *val = tmp; - return ret; - default: - return -EOPNOTSUPP; - } + return rescale_process_scale(rescale, ret, val, val2); default: return -EINVAL; } diff --git a/include/linux/iio/afe/rescale.h b/include/linux/iio/afe/rescale.h new file mode 100644 index 000000000000..14d4ee1227c6 --- /dev/null +++ b/include/linux/iio/afe/rescale.h @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2021 Liam Beguin <liambeguin@gmail.com> + */ + +#ifndef __IIO_RESCALE_H__ +#define __IIO_RESCALE_H__ + +#include <linux/iio/iio.h> + +struct rescale; + +struct rescale_cfg { + enum iio_chan_type type; + int (*props)(struct device *dev, struct rescale *rescale); +}; + +struct rescale { + const struct rescale_cfg *cfg; + struct iio_channel *source; + struct iio_chan_spec chan; + struct iio_chan_spec_ext_info *ext_info; + bool chan_processed; + s32 numerator; + s32 denominator; +}; + +int rescale_process_scale(struct rescale *rescale, int scale_type, + int *val, int *val2); +#endif /* __IIO_RESCALE_H__ */