This series includes one patch:
1.update case IIO_CHAN_INFO_PROCESSED: convert raw data to voltage.
Changes in patch v3:
1)add Fixes tags inforamtion.
Changes in patch v2:
1)abandon support case IIO_CHAN_INFO_RAW.
Changes in patch v1:
1)fix typo covert to convert in patch 2/2 description.
Hui Liu (1):
iio: mtk-auxadc: update case IIO_CHAN_INFO_PROCESSED
drivers/iio/adc/mt6577_auxadc.c | 8 ++++++++
1 file changed, 8 insertions(+)
--
2.18.0
Convert raw data to processed data.
Fixes: ace4cdfe67be ("iio: adc: mt2701: Add Mediatek auxadc driver for
mt2701.")
Signed-off-by: Hui Liu <[email protected]>
---
drivers/iio/adc/mt6577_auxadc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 79c1dd68b909..d4fccd52ef08 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -82,6 +82,10 @@ static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
MT6577_AUXADC_CHANNEL(15),
};
+/* For Voltage calculation */
+#define VOLTAGE_FULL_RANGE 1500 /* VA voltage */
+#define AUXADC_PRECISE 4096 /* 12 bits */
+
static int mt_auxadc_get_cali_data(int rawdata, bool enable_cali)
{
return rawdata;
@@ -191,6 +195,10 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
}
if (adc_dev->dev_comp->sample_data_cali)
*val = mt_auxadc_get_cali_data(*val, true);
+
+ /* Convert adc raw data to voltage: 0 - 1500 mV */
+ *val = *val * VOLTAGE_FULL_RANGE / AUXADC_PRECISE;
+
return IIO_VAL_INT;
default:
--
2.25.1
On Fri, 24 Sep 2021 16:30:09 +0800
hui.liu <[email protected]> wrote:
> Hi Jonathan,
>
> In the previous mail(maybe 2021/08/15), we want to support two case in
> our driver: _RAW and _SCALE. In _SCALE case:
_RAW and _PROCESSED I think? (scale is something else entirely)
> *val = *val * VOLTAGE_FULL_RANGE;
> *val2 = AUXADC_PRECISE;
> return IIO_VAL_FRACTIONAL_LOG2;
>
> If user call read_raw, will get raw data; If user call read_processed,
> will get processed voltage.
>
> What's your opinion?
I'm sorry, but I don't understand the question.
I looked back at that earlier discussion and the discussion was about also
adding _RAW. I think we concluded that, as we already had _PROCESSED and
generally do not support both, we would just continue to have processed.
The suggestion in this review is to use a different choice from the various
ways that IIO can represent values in order to potentially maintain slightly
higher precision. Whether that actually helps depends a bit of how the
value is being used in your out of tree driver. If that driver wants
a different scaling (perhaps micro volts rather than millivolts) then,
if I recall correctly slightly higher precision is maintained
https://elixir.bootlin.com/linux/latest/source/drivers/iio/inkern.c#L625
If you prefer to keep the code as is, just address the request for a little
more information in the patch description.
Thanks,
Jonathan
>
> Thanks.
>
>
> On Sat, 2021-09-18 at 19:20 +0100, Jonathan Cameron wrote:
> > On Tue, 14 Sep 2021 21:09:01 +0800
> > Hui Liu <[email protected]> wrote:
> >
> > > Convert raw data to processed data.
> > >
> > > Fixes: ace4cdfe67be ("iio: adc: mt2701: Add Mediatek auxadc driver
> > > for
> > > mt2701.")
> > > Signed-off-by: Hui Liu <[email protected]>
> >
> > Hi Hui Liu
> >
> > This fix is obviously correct but I think we can improve it a little.
> >
> > 1) Add a bit more detail to the patch description. Perhaps change it
> > to something like
> > Previously the driver did not apply the scaling necessary to take the
> > voltage range of this ADC into account.
> >
> > 2) If you change to
> >
> > *val = *val * VOLTAGE_FULL_RANGE;
> > *val2 = 12;
> > return IIO_VAL_FRACTIONAL_LOG2;
> >
> > then you should get a more precise answer. (Please check that
> > though!)
> > This might be an issue if you have consumers drivers though that can
> > not
> > cope with this particular type. If so please state that in the patch
> > description
> > and add a comment to the code to say that so we don't end up
> > 'improving' this
> > in future without taking those consumers into account.
> >
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > drivers/iio/adc/mt6577_auxadc.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/iio/adc/mt6577_auxadc.c
> > > b/drivers/iio/adc/mt6577_auxadc.c
> > > index 79c1dd68b909..d4fccd52ef08 100644
> > > --- a/drivers/iio/adc/mt6577_auxadc.c
> > > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > > @@ -82,6 +82,10 @@ static const struct iio_chan_spec
> > > mt6577_auxadc_iio_channels[] = {
> > > MT6577_AUXADC_CHANNEL(15),
> > > };
> > >
> > > +/* For Voltage calculation */
> > > +#define VOLTAGE_FULL_RANGE 1500 /* VA voltage */
> > > +#define AUXADC_PRECISE 4096 /* 12 bits */
> > > +
> > > static int mt_auxadc_get_cali_data(int rawdata, bool enable_cali)
> > > {
> > > return rawdata;
> > > @@ -191,6 +195,10 @@ static int mt6577_auxadc_read_raw(struct
> > > iio_dev *indio_dev,
> > > }
> > > if (adc_dev->dev_comp->sample_data_cali)
> > > *val = mt_auxadc_get_cali_data(*val, true);
> > > +
> > > + /* Convert adc raw data to voltage: 0 - 1500 mV */
> > > + *val = *val * VOLTAGE_FULL_RANGE / AUXADC_PRECISE;
> > > +
> > > return IIO_VAL_INT;
> > >
> > > default:
> >
> >