iio/axp288_adc: add missing channel info mask

PATCH Superseded Jacob Pan 2 participants 3 messages Mar 16, 2015 – Apr 6, 2015

Subsystems (3)

IIO X-powers multifunction PMIC device drivers X-powers AXP288 PMIC drivers

lore

Share & tools

lore

Jacob Pan

author

2015-03-16 17:34 UTC #0 lore raw

PATCH 1/1 iio/axp288_adc: add missing channel info mask

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 <[email protected]>

---
 drivers/iio/adc/axp288_adc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

drivers/iio/adc/axp288_adc.c +11 -6

index 08bcfb0..7ce361c 100644
@@ -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;
 	}

Signature · 1 line

1.9.1

Jonathan Cameron

maintainer

2015-03-21 12:07 UTC #1 lore raw

Quoted 25 lines · 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 <[email protected]>
---
 drivers/iio/adc/axp288_adc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
drivers/iio/adc/axp288_adc.c +1 -0
index 08bcfb0..7ce361c 100644
@@ -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,

drivers/iio/adc/axp288_adc.c static const struct iio_chan_spec const axp288_adc_channels[] = { · line 70

Quoted 10 lines

drivers/iio/adc/axp288_adc.c +2 -0

 		.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.

Quoted 18 lines

drivers/iio/adc/axp288_adc.c +4 -2

 	}, {
 		.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),

drivers/iio/adc/axp288_adc.c static const struct iio_chan_spec const axp288_adc_channels[] = { · line 95

Quoted 10 lines

drivers/iio/adc/axp288_adc.c +2 -1

 	}, {
 		.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.

Quoted 26 lines

drivers/iio/adc/axp288_adc.c +2 -3

};

@@ -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;
 	}

Jacob Pan

author

2015-04-06 18:39 UTC #2 lore raw

Quoted 9 lines · On Sat, 21 Mar 2015 12:07:03 +0000 Jonathan Cameron <[email protected]> wrote:

drivers/iio/adc/axp288_adc.c static const struct iio_chan_spec const axp288_adc_channels[] = { · line 95
Quoted 4 lines
drivers/iio/adc/axp288_adc.c +2 -1
-		.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.
Quoted 1 line
};

Very sorry about the delay, you are right that we should just need RAW. V2 patch just sent.

Thanks,

Jacob