[PATCH v2 3/6] iio: adc: ti-ads1298: fix scale, ID check, regmap and Kconfig

[Stephan Costa Barros <stephanbrrs8@gmail.com>]

Fix four smaller issues:

- The scale divides the reference, in whole millivolts, by the gain
  before it is reported. With the internal 4 V reference, it is 0.099 %
  low at gain 6, 0.101 % at gain 12 and 0.025 % at gain 3. Report
  VREF / (gain * 2^23) as an IIO_VAL_FRACTIONAL; sysfs still shows it
  with nine decimals, now without the error.

- Only the channel codes 0, 1 and 2 of the ID register are defined; an
  ID of 0x93, for example, gives 10 channels. The IIO core then reads
  past ads1298_channels, and the RDATA transfer writes past rx_buffer.
  Refuse the reserved codes as an unknown ID.

- The driver uses regmap but does not select REGMAP, and fails to build
  where nothing else selects it, such as tinyconfig with SPI, IIO and
  TI_ADS1298 on x86_64.

- Bit 0 of CONFIG3 is a read-only status bit and the GPIO register
  reads the pins, but both are cached, so a read through debugfs returns
  a stale value; debugfs can also write ID, LOFF_STATP and LOFF_STATN.
  Mark CONFIG3 and GPIO volatile, and the other three not writeable.

The ID check was found by review and reproduced on an emulated device
only, where KASAN reports the read past ads1298_channels.

Fixes: 00ef7708fa60 ("iio: adc: ti-ads1298: Add driver")
Signed-off-by: Stephan Costa Barros <stephanbrrs8@gmail.com>
---
 drivers/iio/adc/Kconfig      |  1 +
 drivers/iio/adc/ti-ads1298.c | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index f32f678f7..a29ed8f45 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1924,6 +1924,7 @@ config TI_ADS1298
 	depends on SPI
 	select IIO_BUFFER
 	select IIO_KFIFO_BUF
+	select REGMAP
 	help
 	  If you say yes here you get support for Texas Instruments ADS1298
 	  medical ADC chips
diff --git a/drivers/iio/adc/ti-ads1298.c b/drivers/iio/adc/ti-ads1298.c
index f5965b994..b390c13ae 100644
--- a/drivers/iio/adc/ti-ads1298.c
+++ b/drivers/iio/adc/ti-ads1298.c
@@ -75,6 +75,7 @@
 
 #define ADS1298_REG_LOFF_STATP	0x12
 #define ADS1298_REG_LOFF_STATN	0x13
+#define ADS1298_REG_GPIO	0x14
 #define ADS1298_REG_CONFIG4	0x17
 #define ADS1298_MASK_CONFIG4_SINGLE_SHOT	BIT(3)
 
@@ -373,11 +374,10 @@ static int ads1298_get_scale(struct ads1298_private *priv,
 		return -EINVAL;
 
 	gain = ads1298_pga_settings[pga_idx];
-	*val /= gain; /* Full scale is VREF / gain */
+	/* Full scale is VREF / gain, signed, hence the -1 */
+	*val2 = gain << (ADS1298_BITS_PER_SAMPLE - 1);
 
-	*val2 = ADS1298_BITS_PER_SAMPLE - 1; /* Signed, hence the -1 */
-
-	return IIO_VAL_FRACTIONAL_LOG2;
+	return IIO_VAL_FRACTIONAL;
 }
 
 static int ads1298_read_raw(struct iio_dev *indio_dev,
@@ -680,8 +680,11 @@ static void ads1298_quiesce(void *data)
 	ads1298_wait_rdata_idle(priv);
 }
 
+/* CONFIG3 holds the read-only RLD_STAT; GPIO reads the pins */
 static const struct regmap_range ads1298_regmap_volatile_range[] = {
+	regmap_reg_range(ADS1298_REG_CONFIG3, ADS1298_REG_CONFIG3),
 	regmap_reg_range(ADS1298_REG_LOFF_STATP, ADS1298_REG_LOFF_STATN),
+	regmap_reg_range(ADS1298_REG_GPIO, ADS1298_REG_GPIO),
 };
 
 static const struct regmap_access_table ads1298_regmap_volatile = {
@@ -689,12 +692,23 @@ static const struct regmap_access_table ads1298_regmap_volatile = {
 	.n_yes_ranges = ARRAY_SIZE(ads1298_regmap_volatile_range),
 };
 
+static const struct regmap_range ads1298_regmap_read_only_range[] = {
+	regmap_reg_range(ADS1298_REG_ID, ADS1298_REG_ID),
+	regmap_reg_range(ADS1298_REG_LOFF_STATP, ADS1298_REG_LOFF_STATN),
+};
+
+static const struct regmap_access_table ads1298_regmap_writeable = {
+	.no_ranges = ads1298_regmap_read_only_range,
+	.n_no_ranges = ARRAY_SIZE(ads1298_regmap_read_only_range),
+};
+
 static const struct regmap_config ads1298_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.reg_read = ads1298_reg_read,
 	.reg_write = ads1298_reg_write,
 	.max_register = ADS1298_REG_WCT2,
+	.wr_table = &ads1298_regmap_writeable,
 	.volatile_table = &ads1298_regmap_volatile,
 	.cache_type = REGCACHE_MAPLE,
 };
@@ -704,6 +718,7 @@ static int ads1298_init(struct iio_dev *indio_dev)
 	struct ads1298_private *priv = iio_priv(indio_dev);
 	struct device *dev = &priv->spi->dev;
 	const char *suffix;
+	unsigned int nu_ch;
 	unsigned int val;
 	int ret;
 
@@ -717,7 +732,11 @@ static int ads1298_init(struct iio_dev *indio_dev)
 		return ret;
 
 	/* Fill in name and channel count based on what the chip told us */
-	indio_dev->num_channels = 4 + 2 * (val & ADS1298_MASK_ID_CHANNELS);
+	nu_ch = FIELD_GET(ADS1298_MASK_ID_CHANNELS, val);
+	/* 0, 1 and 2 are 4, 6 and 8 channels; the other codes are reserved */
+	if (nu_ch > 2)
+		return dev_err_probe(dev, -ENODEV, "Unknown ID: 0x%x\n", val);
+	indio_dev->num_channels = 4 + 2 * nu_ch;
 	switch (val & ADS1298_MASK_ID_FAMILY) {
 	case ADS1298_ID_FAMILY_ADS129X:
 		suffix = "";
-- 
2.43.0