This series includes two patchs:
1.add support for case IIO_CHAN_INFO_RAW.
2.update case IIO_CHAN_INFO_PROCESSED: convert raw data to voltage.
Changes in patch v1:
1)fix typo covert to convert in patch 2/2 description.
Hui Liu (2):
iio: mtk-auxadc: add support IIO_CHAN_INFO_RAW case
iio: mtk-auxadc: update case IIO_CHAN_INFO_PROCESSED
drivers/iio/adc/mt6577_auxadc.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
--
2.18.0
Add support IIO_CHAN_INFO_RAW case.
Signed-off-by: Hui Liu <[email protected]>
---
drivers/iio/adc/mt6577_auxadc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
index 79c1dd68b909..e995d43287b2 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -60,7 +60,8 @@ static const struct mtk_auxadc_compatible mt6765_compat = {
.type = IIO_VOLTAGE, \
.indexed = 1, \
.channel = (idx), \
- .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_PROCESSED), \
}
static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
@@ -181,6 +182,19 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
struct mt6577_auxadc_device *adc_dev = iio_priv(indio_dev);
switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ *val = mt6577_auxadc_read(indio_dev, chan);
+ if (*val < 0) {
+ dev_notice(indio_dev->dev.parent,
+ "failed to sample data on channel[%d]\n",
+ chan->channel);
+ return *val;
+ }
+ if (adc_dev->dev_comp->sample_data_cali)
+ *val = mt_auxadc_get_cali_data(*val, true);
+
+ return IIO_VAL_INT;
+
case IIO_CHAN_INFO_PROCESSED:
*val = mt6577_auxadc_read(indio_dev, chan);
if (*val < 0) {
--
2.18.0
Convert raw data to processed data.
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 e995d43287b2..cda03a653269 100644
--- a/drivers/iio/adc/mt6577_auxadc.c
+++ b/drivers/iio/adc/mt6577_auxadc.c
@@ -83,6 +83,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;
@@ -205,6 +209,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.18.0
On Thu, 12 Aug 2021 13:48:43 +0800
Hui Liu <[email protected]> wrote:
> Add support IIO_CHAN_INFO_RAW case.
Why?
We almost never support both RAW and PROCESSED as userspace should be
fine to use either. There are a few reasons we've let drivers do
this but I would like know why it matters to you and it definitely
needs to be in the patch description.
>
> Signed-off-by: Hui Liu <[email protected]>
> ---
> drivers/iio/adc/mt6577_auxadc.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/mt6577_auxadc.c b/drivers/iio/adc/mt6577_auxadc.c
> index 79c1dd68b909..e995d43287b2 100644
> --- a/drivers/iio/adc/mt6577_auxadc.c
> +++ b/drivers/iio/adc/mt6577_auxadc.c
> @@ -60,7 +60,8 @@ static const struct mtk_auxadc_compatible mt6765_compat = {
> .type = IIO_VOLTAGE, \
> .indexed = 1, \
> .channel = (idx), \
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
> + BIT(IIO_CHAN_INFO_PROCESSED), \
> }
>
> static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
> @@ -181,6 +182,19 @@ static int mt6577_auxadc_read_raw(struct iio_dev *indio_dev,
> struct mt6577_auxadc_device *adc_dev = iio_priv(indio_dev);
>
> switch (info) {
> + case IIO_CHAN_INFO_RAW:
> + *val = mt6577_auxadc_read(indio_dev, chan);
> + if (*val < 0) {
> + dev_notice(indio_dev->dev.parent,
> + "failed to sample data on channel[%d]\n",
> + chan->channel);
> + return *val;
> + }
> + if (adc_dev->dev_comp->sample_data_cali)
> + *val = mt_auxadc_get_cali_data(*val, true);
> +
> + return IIO_VAL_INT;
> +
> case IIO_CHAN_INFO_PROCESSED:
> *val = mt6577_auxadc_read(indio_dev, chan);
> if (*val < 0) {
On Fri, 13 Aug 2021 11:46:24 +0800
hui.liu <[email protected]> wrote:
> On Thu, 2021-08-12 at 19:07 +0100, Jonathan Cameron wrote:
> > On Thu, 12 Aug 2021 13:48:43 +0800
> > Hui Liu <[email protected]> wrote:
> >
> > > Add support IIO_CHAN_INFO_RAW case.
> >
> > Why?
> >
> > We almost never support both RAW and PROCESSED as userspace should be
> > fine to use either. There are a few reasons we've let drivers do
> > this but I would like know why it matters to you and it definitely
> > needs to be in the patch description.
> >
> Hi Jonathan,
>
> 1. To support ADC consumers' different types of requirement: some
> consumers want to call iio_read_channel_raw to get raw data, the others
> use iio_read_channel_processed to get voltage.
Give an example of the consumer using the raw channel readback (without
acess to any scaling information?)
> 2. In our origin driver, if consumer call iio_read_channel_processed,
> read back value is raw data.
>
> Could we use SCALE instead of PROCESSED in patch for next version, or
> what's your suggestion?
That would unfortunately be a userspace ABI change. We can add interfaces
but taking them away is normally a problem :(
Your reasons here are fine, subject to information on what consumer cares
about having _RAW, please resend the patch with this information added
to the description.
Thanks,
Jonathan
>
> Thanks.
>
> > >
> > > Signed-off-by: Hui Liu <[email protected]>
> > > ---
> > > drivers/iio/adc/mt6577_auxadc.c | 16 +++++++++++++++-
> > > 1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/adc/mt6577_auxadc.c
> > > b/drivers/iio/adc/mt6577_auxadc.c
> > > index 79c1dd68b909..e995d43287b2 100644
> > > --- a/drivers/iio/adc/mt6577_auxadc.c
> > > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > > @@ -60,7 +60,8 @@ static const struct mtk_auxadc_compatible
> > > mt6765_compat = {
> > > .type = IIO_VOLTAGE,
> > > \
> > > .indexed = 1,
> > > \
> > > .channel = (idx), \
> > > - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > > \
> > > + BIT(IIO_CHAN_INFO_PROCESSED), \
> > > }
> > >
> > > static const struct iio_chan_spec mt6577_auxadc_iio_channels[] = {
> > > @@ -181,6 +182,19 @@ static int mt6577_auxadc_read_raw(struct
> > > iio_dev *indio_dev,
> > > struct mt6577_auxadc_device *adc_dev = iio_priv(indio_dev);
> > >
> > > switch (info) {
> > > + case IIO_CHAN_INFO_RAW:
> > > + *val = mt6577_auxadc_read(indio_dev, chan);
> > > + if (*val < 0) {
> > > + dev_notice(indio_dev->dev.parent,
> > > + "failed to sample data on
> > > channel[%d]\n",
> > > + chan->channel);
> > > + return *val;
> > > + }
> > > + if (adc_dev->dev_comp->sample_data_cali)
> > > + *val = mt_auxadc_get_cali_data(*val, true);
> > > +
> > > + return IIO_VAL_INT;
> > > +
> > > case IIO_CHAN_INFO_PROCESSED:
> > > *val = mt6577_auxadc_read(indio_dev, chan);
> > > if (*val < 0) {
> >
> >
On Tue, 17 Aug 2021 16:37:52 +0800
hui.liu <[email protected]> wrote:
> On Sat, 2021-08-14 at 17:10 +0100, Jonathan Cameron wrote:
> > On Fri, 13 Aug 2021 11:46:24 +0800
> > hui.liu <[email protected]> wrote:
> >
> > > On Thu, 2021-08-12 at 19:07 +0100, Jonathan Cameron wrote:
> > > > On Thu, 12 Aug 2021 13:48:43 +0800
> > > > Hui Liu <[email protected]> wrote:
> > > >
> > > > > Add support IIO_CHAN_INFO_RAW case.
> > > >
> > > > Why?
> > > >
> > > > We almost never support both RAW and PROCESSED as userspace
> > > > should be
> > > > fine to use either. There are a few reasons we've let drivers do
> > > > this but I would like know why it matters to you and it
> > > > definitely
> > > > needs to be in the patch description.
> > > >
> > >
> > > Hi Jonathan,
> > >
> > > 1. To support ADC consumers' different types of requirement: some
> > > consumers want to call iio_read_channel_raw to get raw data, the
> > > others
> > > use iio_read_channel_processed to get voltage.
> >
> > Give an example of the consumer using the raw channel readback
> > (without
> > acess to any scaling information?)
> >
>
> > > 2. In our origin driver, if consumer call
> > > iio_read_channel_processed,
> > > read back value is raw data.
> > >
> > > Could we use SCALE instead of PROCESSED in patch for next version,
> > > or
> > > what's your suggestion?
> >
> > That would unfortunately be a userspace ABI change. We can add
> > interfaces
> > but taking them away is normally a problem :(
> >
> > Your reasons here are fine, subject to information on what consumer
> > cares
> > about having _RAW, please resend the patch with this information
> > added
> > to the description.
> >
> > Thanks,
> >
> > Jonathan
>
> 1. We found afe/iio-rescale.c, dac/dpot-dac.c and multiplexer/iio-mux.c
> call iio_read_channel_raw to get raw data. If they use our ADC driver,
> I think we should support _RAW case.
> If we support _RAW case, we will add more information in v2
> description.
iio-rescale has recently gained support for processed.
https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/drivers/iio/afe/iio-rescale.c?h=testing&id=53ebee9499805add3eef630d998c40812e6a1c39
dpot-dac is a rather obscure special case, so I doubt you actually have one of those.
If iio-mux is relevant then we should add processed support to that driver as well.
I would rather see those users of the interface fixed than us
having to tweak lots of drivers to provide _raw when it isn't appropriate for
that piece of hardware.
Jonathan
>
> 2. Since we change _PROCESSED readback value from raw data to voltage,
> our consumer will make the changes synchronously.
>
> > >
> > > Thanks.
> > >
> > > > >
> > > > > Signed-off-by: Hui Liu <[email protected]>
> > > > > ---
> > > > > drivers/iio/adc/mt6577_auxadc.c | 16 +++++++++++++++-
> > > > > 1 file changed, 15 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/iio/adc/mt6577_auxadc.c
> > > > > b/drivers/iio/adc/mt6577_auxadc.c
> > > > > index 79c1dd68b909..e995d43287b2 100644
> > > > > --- a/drivers/iio/adc/mt6577_auxadc.c
> > > > > +++ b/drivers/iio/adc/mt6577_auxadc.c
> > > > > @@ -60,7 +60,8 @@ static const struct mtk_auxadc_compatible
> > > > > mt6765_compat = {
> > > > > .type = IIO_VOLTAGE,
> > > > >
> > > > > \
> > > > > .indexed = 1,
> > > > >
> > > > > \
> > > > > .channel = (idx),
> > > > > \
> > > > > - .info_mask_separate =
> > > > > BIT(IIO_CHAN_INFO_PROCESSED), \
> > > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > > > >
> > > > > \
> > > > > + BIT(IIO_CHAN_INFO_PROCESS
> > > > > ED), \
> > > > > }
> > > > >
> > > > > static const struct iio_chan_spec mt6577_auxadc_iio_channels[]
> > > > > = {
> > > > > @@ -181,6 +182,19 @@ static int mt6577_auxadc_read_raw(struct
> > > > > iio_dev *indio_dev,
> > > > > struct mt6577_auxadc_device *adc_dev =
> > > > > iio_priv(indio_dev);
> > > > >
> > > > > switch (info) {
> > > > > + case IIO_CHAN_INFO_RAW:
> > > > > + *val = mt6577_auxadc_read(indio_dev, chan);
> > > > > + if (*val < 0) {
> > > > > + dev_notice(indio_dev->dev.parent,
> > > > > + "failed to sample data on
> > > > > channel[%d]\n",
> > > > > + chan->channel);
> > > > > + return *val;
> > > > > + }
> > > > > + if (adc_dev->dev_comp->sample_data_cali)
> > > > > + *val = mt_auxadc_get_cali_data(*val,
> > > > > true);
> > > > > +
> > > > > + return IIO_VAL_INT;
> > > > > +
> > > > > case IIO_CHAN_INFO_PROCESSED:
> > > > > *val = mt6577_auxadc_read(indio_dev, chan);
> > > > > if (*val < 0) {
> > > >
> > > >
> >
> >