* [PATCH v3 0/2] staging: iio: ad7606: oversampling_ratio clean-up
@ 2016-10-08 6:50 Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 1/2] staging: iio: ad7606: implement IIO_CHAN_INFO_OVERSAMPLING_RATIO Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins Eva Rachel Retuya
0 siblings, 2 replies; 6+ messages in thread
From: Eva Rachel Retuya @ 2016-10-08 6:50 UTC (permalink / raw)
To: linux-iio, outreachy-kernel
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
Eva Rachel Retuya
Implement the oversampling_ratio attribute using standard global define
as well as fix bug about incorrect setting of oversampling pins
Changes in v3:
* Add follow-up patch fixing bug about incorrect setting of oversampling
pins
* Add Fixes tag
Changes in v2:
* Add Lars' Acked-by tag
* Update dev_err() to use proper arguments
Eva Rachel Retuya (2):
staging: iio: ad7606: implement IIO_CHAN_INFO_OVERSAMPLING_RATIO
staging: iio: ad7606: fix improper setting of oversampling pins
drivers/staging/iio/adc/ad7606_core.c | 67 ++++++++++++++++-------------------
1 file changed, 31 insertions(+), 36 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] staging: iio: ad7606: implement IIO_CHAN_INFO_OVERSAMPLING_RATIO
2016-10-08 6:50 [PATCH v3 0/2] staging: iio: ad7606: oversampling_ratio clean-up Eva Rachel Retuya
@ 2016-10-08 6:50 ` Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins Eva Rachel Retuya
1 sibling, 0 replies; 6+ messages in thread
From: Eva Rachel Retuya @ 2016-10-08 6:50 UTC (permalink / raw)
To: linux-iio, outreachy-kernel
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
Eva Rachel Retuya
This driver predates the availability of IIO_CHAN_INFO_OVERSAMPLING_RATIO
attribute wherein usage has some advantages like it can be accessed by
in-kernel consumers as well as reduces the code size.
Therefore, use IIO_CHAN_INFO_OVERSAMPLING_RATIO to implement the
oversampling_ratio attribute instead of using IIO_DEVICE_ATTR() macro.
Move code from the functions associated with IIO_DEVICE_ATTR() into
the read_raw hook as well as add the write_raw hook with both masks set
to IIO_CHAN_INFO_OVERSAMPLING_RATIO.
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/staging/iio/adc/ad7606_core.c | 67 ++++++++++++++++-------------------
1 file changed, 31 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index f79ee61..437c7d0 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -103,6 +103,9 @@ static int ad7606_read_raw(struct iio_dev *indio_dev,
*val = st->range * 2;
*val2 = st->chip_info->channels[0].scan_type.realbits;
return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *val = st->oversampling;
+ return IIO_VAL_INT;
}
return -EINVAL;
}
@@ -145,16 +148,6 @@ static IIO_DEVICE_ATTR(in_voltage_range, S_IRUGO | S_IWUSR,
ad7606_show_range, ad7606_store_range, 0);
static IIO_CONST_ATTR(in_voltage_range_available, "5000 10000");
-static ssize_t ad7606_show_oversampling_ratio(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
- struct ad7606_state *st = iio_priv(indio_dev);
-
- return sprintf(buf, "%u\n", st->oversampling);
-}
-
static int ad7606_oversampling_get_index(unsigned int val)
{
unsigned char supported[] = {0, 2, 4, 8, 16, 32, 64};
@@ -167,44 +160,43 @@ static int ad7606_oversampling_get_index(unsigned int val)
return -EINVAL;
}
-static ssize_t ad7606_store_oversampling_ratio(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
+static int ad7606_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
{
- struct iio_dev *indio_dev = dev_to_iio_dev(dev);
struct ad7606_state *st = iio_priv(indio_dev);
- unsigned long lval;
int ret;
- ret = kstrtoul(buf, 10, &lval);
- if (ret)
- return ret;
+ switch (mask) {
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ if (val2)
+ return -EINVAL;
+ ret = ad7606_oversampling_get_index(val);
+ if (ret < 0) {
+ dev_err(st->dev, "oversampling %d is not supported\n",
+ val);
+ return ret;
+ }
- ret = ad7606_oversampling_get_index(lval);
- if (ret < 0) {
- dev_err(dev, "oversampling %lu is not supported\n", lval);
- return ret;
+ mutex_lock(&indio_dev->mlock);
+ gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
+ gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
+ gpio_set_value(st->pdata->gpio_os1, (ret >> 2) & 1);
+ st->oversampling = val;
+ mutex_unlock(&indio_dev->mlock);
+ return 0;
+ default:
+ return -EINVAL;
}
-
- mutex_lock(&indio_dev->mlock);
- gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
- gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
- gpio_set_value(st->pdata->gpio_os1, (ret >> 2) & 1);
- st->oversampling = lval;
- mutex_unlock(&indio_dev->mlock);
-
- return count;
}
-static IIO_DEVICE_ATTR(oversampling_ratio, S_IRUGO | S_IWUSR,
- ad7606_show_oversampling_ratio,
- ad7606_store_oversampling_ratio, 0);
static IIO_CONST_ATTR(oversampling_ratio_available, "0 2 4 8 16 32 64");
static struct attribute *ad7606_attributes_os_and_range[] = {
&iio_dev_attr_in_voltage_range.dev_attr.attr,
&iio_const_attr_in_voltage_range_available.dev_attr.attr,
- &iio_dev_attr_oversampling_ratio.dev_attr.attr,
&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
NULL,
};
@@ -214,7 +206,6 @@ static const struct attribute_group ad7606_attribute_group_os_and_range = {
};
static struct attribute *ad7606_attributes_os[] = {
- &iio_dev_attr_oversampling_ratio.dev_attr.attr,
&iio_const_attr_oversampling_ratio_available.dev_attr.attr,
NULL,
};
@@ -241,6 +232,8 @@ static const struct attribute_group ad7606_attribute_group_range = {
.address = num, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ .info_mask_shared_by_all = \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
.scan_index = num, \
.scan_type = { \
.sign = 's', \
@@ -429,12 +422,14 @@ static const struct iio_info ad7606_info_no_os_or_range = {
static const struct iio_info ad7606_info_os_and_range = {
.driver_module = THIS_MODULE,
.read_raw = &ad7606_read_raw,
+ .write_raw = &ad7606_write_raw,
.attrs = &ad7606_attribute_group_os_and_range,
};
static const struct iio_info ad7606_info_os = {
.driver_module = THIS_MODULE,
.read_raw = &ad7606_read_raw,
+ .write_raw = &ad7606_write_raw,
.attrs = &ad7606_attribute_group_os,
};
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins
2016-10-08 6:50 [PATCH v3 0/2] staging: iio: ad7606: oversampling_ratio clean-up Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 1/2] staging: iio: ad7606: implement IIO_CHAN_INFO_OVERSAMPLING_RATIO Eva Rachel Retuya
@ 2016-10-08 6:50 ` Eva Rachel Retuya
2016-10-08 12:21 ` Lars-Peter Clausen
1 sibling, 1 reply; 6+ messages in thread
From: Eva Rachel Retuya @ 2016-10-08 6:50 UTC (permalink / raw)
To: linux-iio, outreachy-kernel
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
Eva Rachel Retuya
The oversampling ratio is controlled using the oversampling pins,
OS [2:0] with OS2 being the MSB control bit, and OS0 the LSB control
bit.
The gpio connected to the OS2 pin is not being set correctly, only OS0
and OS1 pins are being set. Fix the typo to allow proper control of the
oversampling pins.
Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
Fixes: b9618c0 ("staging: IIO: ADC: New driver for AD7606/AD7606-6/AD7606-4")
---
drivers/staging/iio/adc/ad7606_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
index 437c7d0..2042225 100644
--- a/drivers/staging/iio/adc/ad7606_core.c
+++ b/drivers/staging/iio/adc/ad7606_core.c
@@ -183,7 +183,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
mutex_lock(&indio_dev->mlock);
gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
- gpio_set_value(st->pdata->gpio_os1, (ret >> 2) & 1);
+ gpio_set_value(st->pdata->gpio_os2, (ret >> 2) & 1);
st->oversampling = val;
mutex_unlock(&indio_dev->mlock);
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins
2016-10-08 6:50 ` [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins Eva Rachel Retuya
@ 2016-10-08 12:21 ` Lars-Peter Clausen
2016-10-12 12:42 ` [Outreachy kernel] " Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Lars-Peter Clausen @ 2016-10-08 12:21 UTC (permalink / raw)
To: Eva Rachel Retuya, linux-iio, outreachy-kernel
Cc: Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh
On 10/08/2016 08:50 AM, Eva Rachel Retuya wrote:
> The oversampling ratio is controlled using the oversampling pins,
> OS [2:0] with OS2 being the MSB control bit, and OS0 the LSB control
> bit.
>
> The gpio connected to the OS2 pin is not being set correctly, only OS0
> and OS1 pins are being set. Fix the typo to allow proper control of the
> oversampling pins.
>
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> Fixes: b9618c0 ("staging: IIO: ADC: New driver for AD7606/AD7606-6/AD7606-4")
Looks good, thanks.
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Ideally you'd have but the patches in the opposite order, fix first, then
rework. As it is right now with the fix after the rework the fix no longer
applies cleanly to older versions.
> ---
> drivers/staging/iio/adc/ad7606_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/adc/ad7606_core.c b/drivers/staging/iio/adc/ad7606_core.c
> index 437c7d0..2042225 100644
> --- a/drivers/staging/iio/adc/ad7606_core.c
> +++ b/drivers/staging/iio/adc/ad7606_core.c
> @@ -183,7 +183,7 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,
> mutex_lock(&indio_dev->mlock);
> gpio_set_value(st->pdata->gpio_os0, (ret >> 0) & 1);
> gpio_set_value(st->pdata->gpio_os1, (ret >> 1) & 1);
> - gpio_set_value(st->pdata->gpio_os1, (ret >> 2) & 1);
> + gpio_set_value(st->pdata->gpio_os2, (ret >> 2) & 1);
> st->oversampling = val;
> mutex_unlock(&indio_dev->mlock);
> return 0;
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins
2016-10-08 12:21 ` Lars-Peter Clausen
@ 2016-10-12 12:42 ` Greg KH
2016-10-12 13:03 ` Eva Rachel Retuya
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2016-10-12 12:42 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Eva Rachel Retuya, linux-iio, outreachy-kernel,
Michael.Hennerich, jic23, knaack.h, pmeerw
On Sat, Oct 08, 2016 at 02:21:46PM +0200, Lars-Peter Clausen wrote:
> On 10/08/2016 08:50 AM, Eva Rachel Retuya wrote:
> > The oversampling ratio is controlled using the oversampling pins,
> > OS [2:0] with OS2 being the MSB control bit, and OS0 the LSB control
> > bit.
> >
> > The gpio connected to the OS2 pin is not being set correctly, only OS0
> > and OS1 pins are being set. Fix the typo to allow proper control of the
> > oversampling pins.
> >
> > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> > Fixes: b9618c0 ("staging: IIO: ADC: New driver for AD7606/AD7606-6/AD7606-4")
>
> Looks good, thanks.
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
> Ideally you'd have but the patches in the opposite order, fix first, then
> rework. As it is right now with the fix after the rework the fix no longer
> applies cleanly to older versions.
Yes, please do that, so it can be applied to older kernels properly.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins
2016-10-12 12:42 ` [Outreachy kernel] " Greg KH
@ 2016-10-12 13:03 ` Eva Rachel Retuya
0 siblings, 0 replies; 6+ messages in thread
From: Eva Rachel Retuya @ 2016-10-12 13:03 UTC (permalink / raw)
To: Greg KH
Cc: Lars-Peter Clausen, linux-iio, outreachy-kernel,
Michael.Hennerich, jic23, knaack.h, pmeerw
On Wed, Oct 12, 2016 at 02:42:17PM +0200, Greg KH wrote:
> On Sat, Oct 08, 2016 at 02:21:46PM +0200, Lars-Peter Clausen wrote:
> > On 10/08/2016 08:50 AM, Eva Rachel Retuya wrote:
> > > The oversampling ratio is controlled using the oversampling pins,
> > > OS [2:0] with OS2 being the MSB control bit, and OS0 the LSB control
> > > bit.
> > >
> > > The gpio connected to the OS2 pin is not being set correctly, only OS0
> > > and OS1 pins are being set. Fix the typo to allow proper control of the
> > > oversampling pins.
> > >
> > > Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> > > Fixes: b9618c0 ("staging: IIO: ADC: New driver for AD7606/AD7606-6/AD7606-4")
> >
> > Looks good, thanks.
> >
> > Acked-by: Lars-Peter Clausen <lars@metafoo.de>
> >
> > Ideally you'd have but the patches in the opposite order, fix first, then
> > rework. As it is right now with the fix after the rework the fix no longer
> > applies cleanly to older versions.
>
> Yes, please do that, so it can be applied to older kernels properly.
>
> thanks,
>
> greg k-h
Lars and Greg:
Thanks for the feedback. I got the v4 accepted using the proper order,
fix first then rework as suggested.
Eva
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-12 13:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-08 6:50 [PATCH v3 0/2] staging: iio: ad7606: oversampling_ratio clean-up Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 1/2] staging: iio: ad7606: implement IIO_CHAN_INFO_OVERSAMPLING_RATIO Eva Rachel Retuya
2016-10-08 6:50 ` [PATCH v3 2/2] staging: iio: ad7606: fix improper setting of oversampling pins Eva Rachel Retuya
2016-10-08 12:21 ` Lars-Peter Clausen
2016-10-12 12:42 ` [Outreachy kernel] " Greg KH
2016-10-12 13:03 ` Eva Rachel Retuya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.