* [PATCH] iio/axp288_adc: add missing channel info mask
@ 2015-03-16 17:34 Jacob Pan
2015-03-21 12:07 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Jacob Pan @ 2015-03-16 17:34 UTC (permalink / raw)
To: IIO, Jonathan Cameron, LKML
Cc: Lee Jones, Lars-Peter Clausen, Hartmut Knaack, Aaron Lu,
Todd Brandt, Jacob Pan
Commit 65de7654d39c70c2b ("iio: iio: Fix iio_channel_read return if
channel havn't info") added a check for valid info masks.
This patch adds missing channel info masks for all ADC channels.
Otherwise, iio_read_channel_raw() would return -EINVAL when called
by consumer drivers.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iio/adc/axp288_adc.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
index 08bcfb0..7ce361c 100644
--- a/drivers/iio/adc/axp288_adc.c
+++ b/drivers/iio/adc/axp288_adc.c
@@ -53,39 +53,45 @@ static const struct iio_chan_spec const axp288_adc_channels[] = {
.channel = 0,
.address = AXP288_TS_ADC_H,
.datasheet_name = "TS_PIN",
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
}, {
.indexed = 1,
.type = IIO_TEMP,
.channel = 1,
.address = AXP288_PMIC_ADC_H,
.datasheet_name = "PMIC_TEMP",
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
}, {
.indexed = 1,
.type = IIO_TEMP,
.channel = 2,
.address = AXP288_GP_ADC_H,
.datasheet_name = "GPADC",
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
}, {
.indexed = 1,
.type = IIO_CURRENT,
.channel = 3,
.address = AXP20X_BATT_CHRG_I_H,
.datasheet_name = "BATT_CHG_I",
- .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+ | BIT(IIO_CHAN_INFO_RAW),
}, {
.indexed = 1,
.type = IIO_CURRENT,
.channel = 4,
.address = AXP20X_BATT_DISCHRG_I_H,
.datasheet_name = "BATT_DISCHRG_I",
- .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+ | BIT(IIO_CHAN_INFO_RAW),
}, {
.indexed = 1,
.type = IIO_VOLTAGE,
.channel = 5,
.address = AXP20X_BATT_V_H,
.datasheet_name = "BATT_V",
- .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
+ | BIT(IIO_CHAN_INFO_RAW),
},
};
@@ -140,20 +146,19 @@ static int axp288_adc_read_raw(struct iio_dev *indio_dev,
mutex_lock(&indio_dev->mlock);
switch (mask) {
case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_PROCESSED:
if (axp288_adc_set_ts(info->regmap, AXP288_ADC_TS_PIN_GPADC,
chan->address)) {
dev_err(&indio_dev->dev, "GPADC mode\n");
ret = -EINVAL;
break;
}
+
ret = axp288_adc_read_channel(val, chan->address, info->regmap);
if (axp288_adc_set_ts(info->regmap, AXP288_ADC_TS_PIN_ON,
chan->address))
dev_err(&indio_dev->dev, "TS pin restore\n");
break;
- case IIO_CHAN_INFO_PROCESSED:
- ret = axp288_adc_read_channel(val, chan->address, info->regmap);
- break;
default:
ret = -EINVAL;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] iio/axp288_adc: add missing channel info mask
2015-03-16 17:34 [PATCH] iio/axp288_adc: add missing channel info mask Jacob Pan
@ 2015-03-21 12:07 ` Jonathan Cameron
2015-04-06 18:39 ` Jacob Pan
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2015-03-21 12:07 UTC (permalink / raw)
To: Jacob Pan, IIO, LKML
Cc: Lee Jones, Lars-Peter Clausen, Hartmut Knaack, Aaron Lu, Todd Brandt
On 16/03/15 17:34, Jacob Pan wrote:
> Commit 65de7654d39c70c2b ("iio: iio: Fix iio_channel_read return if
> channel havn't info") added a check for valid info masks.
>
> This patch adds missing channel info masks for all ADC channels.
> Otherwise, iio_read_channel_raw() would return -EINVAL when called
> by consumer drivers.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
> drivers/iio/adc/axp288_adc.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/axp288_adc.c b/drivers/iio/adc/axp288_adc.c
> index 08bcfb0..7ce361c 100644
> --- a/drivers/iio/adc/axp288_adc.c
> +++ b/drivers/iio/adc/axp288_adc.c
> @@ -53,39 +53,45 @@ static const struct iio_chan_spec const axp288_adc_channels[] = {
> .channel = 0,
> .address = AXP288_TS_ADC_H,
> .datasheet_name = "TS_PIN",
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> }, {
> .indexed = 1,
> .type = IIO_TEMP,
> .channel = 1,
> .address = AXP288_PMIC_ADC_H,
> .datasheet_name = "PMIC_TEMP",
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> }, {
> .indexed = 1,
> .type = IIO_TEMP,
> .channel = 2,
> .address = AXP288_GP_ADC_H,
> .datasheet_name = "GPADC",
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
These additions are fair enough given otherwise the driver is claiming not to provide
the raw read. I suppose we might want to hide these from direct userspace exposure
but I doubt anyone cares.
> }, {
> .indexed = 1,
> .type = IIO_CURRENT,
> .channel = 3,
> .address = AXP20X_BATT_CHRG_I_H,
> .datasheet_name = "BATT_CHG_I",
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> + | BIT(IIO_CHAN_INFO_RAW),
> }, {
> .indexed = 1,
> .type = IIO_CURRENT,
> .channel = 4,
> .address = AXP20X_BATT_DISCHRG_I_H,
> .datasheet_name = "BATT_DISCHRG_I",
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> + | BIT(IIO_CHAN_INFO_RAW),
> }, {
> .indexed = 1,
> .type = IIO_VOLTAGE,
> .channel = 5,
> .address = AXP20X_BATT_V_H,
> .datasheet_name = "BATT_V",
> - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> + | BIT(IIO_CHAN_INFO_RAW),
> },
These cases strike me as problematic. You should not need both. If the consumer
is reading _raw from a channel that only supplies PROCESSED then that is the bug,
not the lack of support for a raw read.
> };
>
> @@ -140,20 +146,19 @@ static int axp288_adc_read_raw(struct iio_dev *indio_dev,
> mutex_lock(&indio_dev->mlock);
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> + case IIO_CHAN_INFO_PROCESSED:
> if (axp288_adc_set_ts(info->regmap, AXP288_ADC_TS_PIN_GPADC,
> chan->address)) {
> dev_err(&indio_dev->dev, "GPADC mode\n");
> ret = -EINVAL;
> break;
> }
> +
> ret = axp288_adc_read_channel(val, chan->address, info->regmap);
> if (axp288_adc_set_ts(info->regmap, AXP288_ADC_TS_PIN_ON,
> chan->address))
> dev_err(&indio_dev->dev, "TS pin restore\n");
> break;
> - case IIO_CHAN_INFO_PROCESSED:
> - ret = axp288_adc_read_channel(val, chan->address, info->regmap);
> - break;
> default:
> ret = -EINVAL;
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] iio/axp288_adc: add missing channel info mask
2015-03-21 12:07 ` Jonathan Cameron
@ 2015-04-06 18:39 ` Jacob Pan
0 siblings, 0 replies; 3+ messages in thread
From: Jacob Pan @ 2015-04-06 18:39 UTC (permalink / raw)
To: Jonathan Cameron
Cc: IIO, LKML, Lee Jones, Lars-Peter Clausen, Hartmut Knaack,
Aaron Lu, Todd Brandt
On Sat, 21 Mar 2015 12:07:03 +0000
Jonathan Cameron <jic23@kernel.org> wrote:
> > - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED)
> > + | BIT(IIO_CHAN_INFO_RAW),
> > },
> These cases strike me as problematic. You should not need both. If
> the consumer is reading _raw from a channel that only supplies
> PROCESSED then that is the bug, not the lack of support for a raw
> read.
> > };
Very sorry about the delay, you are right that we should just need RAW.
V2 patch just sent.
Thanks,
Jacob
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-04-06 18:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-16 17:34 [PATCH] iio/axp288_adc: add missing channel info mask Jacob Pan
2015-03-21 12:07 ` Jonathan Cameron
2015-04-06 18:39 ` Jacob Pan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).