| Message ID | 20211104082413.3681212-9-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: Use scan_type shift and realbits when processing raw data | expand |
On Thu, 4 Nov 2021 01:24:08 -0700 Gwendal Grignou <gwendal@chromium.org> wrote: > Use channel definition as root of trust and replace constant > when reading elements directly using the raw sysfs attributes. > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Hi Eugen, Gwendal's v2 crossed with your comments on this fixing an issue in 6794e23fa3fe ("iio: adc: at91-sama5d2_adc: add support for oversampling resolution") You requested a separate fix to change the value to 13 then this on top of that. I don't see why we can't go directly to this with an appropriately reworded message to say what is being fixed. Am I missing something beyond the fix being more obvious if we just change the value? Whilst this is pending I've applied the rest of this series as it's only this one with open questions. Thanks, Jonathan > --- > drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index 4c922ef634f8e..92a57cf10fba4 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > *val = st->conversion_value; > ret = at91_adc_adjust_val_osr(st, val); > if (chan->scan_type.sign == 's') > - *val = sign_extend32(*val, 11); > + *val = sign_extend32(*val, > + chan->scan_type.realbits - 1); > st->conversion_done = false; > } >
On 11/13/21 6:42 PM, Jonathan Cameron wrote: > On Thu, 4 Nov 2021 01:24:08 -0700 > Gwendal Grignou <gwendal@chromium.org> wrote: > >> Use channel definition as root of trust and replace constant >> when reading elements directly using the raw sysfs attributes. >> >> Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > Hi Eugen, > > Gwendal's v2 crossed with your comments on this fixing an issue in > 6794e23fa3fe ("iio: adc: at91-sama5d2_adc: add support for oversampling > resolution") > > You requested a separate fix to change the value to 13 then this on top > of that. I don't see why we can't go directly to this with an appropriately > reworded message to say what is being fixed. Am I missing something beyond > the fix being more obvious if we just change the value? > > Whilst this is pending I've applied the rest of this series as it's only this > one with open questions. Hi Jonathan, If you feel it's not worth fixing it in a separate commit , then feel free to apply this patch, I am happy with both ways. you can add my Reviewed-by: Eugen Hristev <eugen.hristev@microchip.com> Thanks ! Eugen > > Thanks, > > Jonathan > >> --- >> drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >> index 4c922ef634f8e..92a57cf10fba4 100644 >> --- a/drivers/iio/adc/at91-sama5d2_adc.c >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >> @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, >> *val = st->conversion_value; >> ret = at91_adc_adjust_val_osr(st, val); >> if (chan->scan_type.sign == 's') >> - *val = sign_extend32(*val, 11); >> + *val = sign_extend32(*val, >> + chan->scan_type.realbits - 1); >> st->conversion_done = false; >> } >> >
On Mon, 15 Nov 2021 09:22:58 +0000 <Eugen.Hristev@microchip.com> wrote: > On 11/13/21 6:42 PM, Jonathan Cameron wrote: > > On Thu, 4 Nov 2021 01:24:08 -0700 > > Gwendal Grignou <gwendal@chromium.org> wrote: > > > >> Use channel definition as root of trust and replace constant > >> when reading elements directly using the raw sysfs attributes. > >> > >> Signed-off-by: Gwendal Grignou <gwendal@chromium.org> > > > > Hi Eugen, > > > > Gwendal's v2 crossed with your comments on this fixing an issue in > > 6794e23fa3fe ("iio: adc: at91-sama5d2_adc: add support for oversampling > > resolution") > > > > You requested a separate fix to change the value to 13 then this on top > > of that. I don't see why we can't go directly to this with an appropriately > > reworded message to say what is being fixed. Am I missing something beyond > > the fix being more obvious if we just change the value? > > > > Whilst this is pending I've applied the rest of this series as it's only this > > one with open questions. > > Hi Jonathan, > > If you feel it's not worth fixing it in a separate commit , then feel > free to apply this patch, I am happy with both ways. > > you can add my > > Reviewed-by: Eugen Hristev <eugen.hristev@microchip.com> Thanks, Applied to the fixes-togreg branch of iio.git with the fixes tag and stable marking. Jonathan > > Thanks ! > Eugen > > > > Thanks, > > > > Jonathan > > > >> --- > >> drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > >> index 4c922ef634f8e..92a57cf10fba4 100644 > >> --- a/drivers/iio/adc/at91-sama5d2_adc.c > >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c > >> @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, > >> *val = st->conversion_value; > >> ret = at91_adc_adjust_val_osr(st, val); > >> if (chan->scan_type.sign == 's') > >> - *val = sign_extend32(*val, 11); > >> + *val = sign_extend32(*val, > >> + chan->scan_type.realbits - 1); > >> st->conversion_done = false; > >> } > >> > > >
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 4c922ef634f8e..92a57cf10fba4 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -1586,7 +1586,8 @@ static int at91_adc_read_info_raw(struct iio_dev *indio_dev, *val = st->conversion_value; ret = at91_adc_adjust_val_osr(st, val); if (chan->scan_type.sign == 's') - *val = sign_extend32(*val, 11); + *val = sign_extend32(*val, + chan->scan_type.realbits - 1); st->conversion_done = false; }
Use channel definition as root of trust and replace constant when reading elements directly using the raw sysfs attributes. Signed-off-by: Gwendal Grignou <gwendal@chromium.org> --- drivers/iio/adc/at91-sama5d2_adc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)