[PATCH 0/3] iio: accel: merge adis16203 into mainline adis16201 and remove from staging

[PATCH 0/3] iio: accel: merge adis16203 into mainline adis16201 and remove from staging

[Shehryar Ahmad]

This series merges the adis16203 staging driver into the mainline
adis16201 driver and removes staging copy. Following discussion at [1].

Register addresses are identical between both parts (SUPPLY_OUT,
AUX_ADC, TEMP_OUT, inclination regs all match). Pin config is identical
too, all 16 pins, same names/types (Table 5, both datasheets). SPI
protocol is identical as well (Figure 3, both datasheets, Phase = 1,
Polarity = 1). adis16203's second (180-degree) inclination channel is
dropped, since userspace can trivially compute it from the first channel
anyway.[2]

Differences between chips selected per-device via a new struct
differ_info, picked in probe() off of_device_id match, same pattern as
adis16475.c, similar thing in adis16480 but different mechanism.

Don't have hardware for either part, only tested locally with KUnit that
the sign_extend32 and SCALE/OFFSET paths of real read_raw behave
correctly for both chips. Register read/write and probe aren't tested.

Patch 3's binding content is unchanged from [3].

If this isn't the right direction, guidance would be appreciated.

Link: https://lore.kernel.org/all/20250308144239.0442f1a7@jic23-huawei/
Link: https://lore.kernel.org/linux-iio/20180204163124.67234ae5@archlinux/
Link: https://lore.kernel.org/all/20260812043506.456ced6d@jic23-huawei/

Shehryar Ahmad (3):
  iio: accel: adis16201: merge adis16203 support from staging
  staging: iio: accel: remove adis16203, merged into mainline adis16201
    driver
  dt-bindings: iio: accel: adi,adis16201: add adis16203 compatible

 .../bindings/iio/accel/adi,adis16201.yaml     |   6 +-
 drivers/iio/accel/adis16201.c                 | 289 +++++++++-------
 drivers/staging/iio/Kconfig                   |   1 -
 drivers/staging/iio/Makefile                  |   1 -
 drivers/staging/iio/accel/Kconfig             |  19 --
 drivers/staging/iio/accel/Makefile            |   6 -
 drivers/staging/iio/accel/adis16203.c         | 315 ------------------
 7 files changed, 178 insertions(+), 459 deletions(-)
 delete mode 100644 drivers/staging/iio/accel/Kconfig
 delete mode 100644 drivers/staging/iio/accel/Makefile
 delete mode 100644 drivers/staging/iio/accel/adis16203.c

-- 
2.43.0

[PATCH 1/3] iio: accel: adis16201: merge adis16203 support from staging

[Shehryar Ahmad]

Merge the ADIS16203 360 degree inclinometer driver from staging to
mainline ADIS16201 driver. Register addresses and external SPI interface
are identical between both. Some things that differ like write mask, and
calibbias bit width are handled by per chip struct differ_info, handled
by of_device_id match data in probe, following the same pattern used in
adis16475 which selects per-chip data directly via of_device_id.data and
spi_get_device_match_data(), adis16480 does similar but via a different
mechanism (index into an array via the SPI ID table).

Channels arrays are kept separate to avoid dropping const and kmemdup
would be extra failure point if memory allocation fails and is memory
inefficient.

GENMASK is still same instead of range check to keep adis16201 behaviour
unchanged

Signed-off-by: Shehryar Ahmad <shehryar.amd@gmail.com>
---
 drivers/iio/accel/adis16201.c | 289 ++++++++++++++++++++--------------
 1 file changed, 174 insertions(+), 115 deletions(-)

diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
index ba0f97944..ee8efeed2 100644
--- a/drivers/iio/accel/adis16201.c
+++ b/drivers/iio/accel/adis16201.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
+ * Analog Devices ADIS16201 Inclinometer/Accelerometer and ADIS16203 Inclinometer Driver
  *
  * Copyright 2010 Analog Devices Inc.
  */
@@ -13,93 +13,187 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/imu/adis.h>
 
-#define ADIS16201_STARTUP_DELAY_MS			220
-#define ADIS16201_FLASH_CNT				0x00
+#define ADIS16201_STARTUP_DELAY_MS			        220
+#define ADIS16201_FLASH_CNT					        0x00
 
 /* Data Output Register Information */
-#define ADIS16201_SUPPLY_OUT_REG			0x02
-#define ADIS16201_XACCL_OUT_REG				0x04
-#define ADIS16201_YACCL_OUT_REG				0x06
-#define ADIS16201_AUX_ADC_REG				0x08
-#define ADIS16201_TEMP_OUT_REG				0x0A
-#define ADIS16201_XINCL_OUT_REG				0x0C
-#define ADIS16201_YINCL_OUT_REG				0x0E
+#define ADIS16201_SUPPLY_OUT_REG					0x02
+#define ADIS16201_XACCL_OUT_REG				        0x04
+#define ADIS16201_YACCL_OUT_REG				        0x06
+#define ADIS16201_AUX_ADC_REG				        0x08
+#define ADIS16201_TEMP_OUT_REG				        0x0A
+#define ADIS16201_INCL_OUT_REG		                0x0C
+#define ADIS16201_YINCL_OUT_REG				        0x0E
 
 /* Calibration Register Definition */
-#define ADIS16201_XACCL_OFFS_REG			0x10
-#define ADIS16201_YACCL_OFFS_REG			0x12
-#define ADIS16201_XACCL_SCALE_REG			0x14
-#define ADIS16201_YACCL_SCALE_REG			0x16
-#define ADIS16201_XINCL_OFFS_REG			0x18
-#define ADIS16201_YINCL_OFFS_REG			0x1A
-#define ADIS16201_XINCL_SCALE_REG			0x1C
-#define ADIS16201_YINCL_SCALE_REG			0x1E
+#define ADIS16201_XACCL_OFFS_REG			        0x10
+#define ADIS16201_YACCL_OFFS_REG			        0x12
+#define ADIS16201_XACCL_SCALE_REG			        0x14
+#define ADIS16201_YACCL_SCALE_REG			        0x16
+#define ADIS16201_INCL_OFFS_REG			            0x18
+#define ADIS16201_YINCL_OFFS_REG			        0x1A
+#define ADIS16201_XINCL_SCALE_REG			        0x1C
+#define ADIS16201_YINCL_SCALE_REG			        0x1E
 
 /* Alarm Register Definition */
-#define ADIS16201_ALM_MAG1_REG				0x20
-#define ADIS16201_ALM_MAG2_REG				0x22
-#define ADIS16201_ALM_SMPL1_REG				0x24
-#define ADIS16201_ALM_SMPL2_REG				0x26
-#define ADIS16201_ALM_CTRL_REG				0x28
-
-#define ADIS16201_AUX_DAC_REG				0x30
-#define ADIS16201_GPIO_CTRL_REG				0x32
-#define ADIS16201_SMPL_PRD_REG				0x36
+#define ADIS16201_ALM_MAG1_REG				        0x20
+#define ADIS16201_ALM_MAG2_REG				        0x22
+#define ADIS16201_ALM_SMPL1_REG				        0x24
+#define ADIS16201_ALM_SMPL2_REG				        0x26
+#define ADIS16201_ALM_CTRL_REG				        0x28
+
+#define ADIS16201_AUX_DAC_REG						0x30
+#define ADIS16201_GPIO_CTRL_REG						0x32
+#define ADIS16201_SMPL_PRD_REG						0x36
+
 /* Operation, filter configuration */
-#define ADIS16201_AVG_CNT_REG				0x38
-#define ADIS16201_SLP_CNT_REG				0x3A
+#define ADIS16201_AVG_CNT_REG				        0x38
+#define ADIS16201_SLP_CNT_REG				        0x3A
 
 /* Miscellaneous Control Register Definition */
-#define ADIS16201_MSC_CTRL_REG				0x34
-#define  ADIS16201_MSC_CTRL_SELF_TEST_EN		BIT(8)
+#define  ADIS16201_MSC_CTRL_REG				        0x34
+#define  ADIS16201_MSC_CTRL_SELF_TEST_EN		    BIT(8)
 /* Data-ready enable: 1 = enabled, 0 = disabled */
-#define  ADIS16201_MSC_CTRL_DATA_RDY_EN			BIT(2)
+#define  ADIS16201_MSC_CTRL_DATA_RDY_EN			    BIT(2)
 /* Data-ready polarity: 1 = active high, 0 = active low */
 #define  ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH	BIT(1)
 /* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
-#define  ADIS16201_MSC_CTRL_DATA_RDY_DIO1		BIT(0)
+#define  ADIS16201_MSC_CTRL_DATA_RDY_DIO1		    BIT(0)
 
 /* Diagnostics System Status Register Definition */
-#define ADIS16201_DIAG_STAT_REG				0x3C
-#define  ADIS16201_DIAG_STAT_ALARM2			BIT(9)
-#define  ADIS16201_DIAG_STAT_ALARM1			BIT(8)
-#define  ADIS16201_DIAG_STAT_SPI_FAIL_BIT		3
+#define  ADIS16201_DIAG_STAT_REG				    0x3C
+#define  ADIS16201_DIAG_STAT_ALARM2			        BIT(9)
+#define  ADIS16201_DIAG_STAT_ALARM1			        BIT(8)
+#define  ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT      5
+#define  ADIS16201_DIAG_STAT_SPI_FAIL_BIT			3
 #define  ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT		2
 /* Power supply above 3.625 V */
-#define  ADIS16201_DIAG_STAT_POWER_HIGH_BIT		1
+#define  ADIS16201_DIAG_STAT_POWER_HIGH_BIT		    1
 /* Power supply below 2.975 V */
-#define  ADIS16201_DIAG_STAT_POWER_LOW_BIT		0
+#define  ADIS16201_DIAG_STAT_POWER_LOW_BIT		    0
 
 /* System Command Register Definition */
-#define ADIS16201_GLOB_CMD_REG				0x3E
-#define  ADIS16201_GLOB_CMD_SW_RESET			BIT(7)
-#define  ADIS16201_GLOB_CMD_FACTORY_RESET		BIT(1)
+#define ADIS16201_GLOB_CMD_REG				        0x3E
+#define ADIS16201_GLOB_CMD_SW_RESET			        BIT(7)
+#define ADIS16203_GLOB_CMD_CLEAR_STAT               BIT(4)
+#define ADIS16201_GLOB_CMD_FACTORY_RESET		    BIT(1)
 
-#define ADIS16201_ERROR_ACTIVE				BIT(14)
+#define ADIS16201_ERROR_ACTIVE				        BIT(14)
 
 enum adis16201_scan {
-	ADIS16201_SCAN_ACC_X,
-	ADIS16201_SCAN_ACC_Y,
-	ADIS16201_SCAN_INCLI_X,
-	ADIS16201_SCAN_INCLI_Y,
 	ADIS16201_SCAN_SUPPLY,
-	ADIS16201_SCAN_AUX_ADC,
 	ADIS16201_SCAN_TEMP,
+	ADIS16201_SCAN_AUX_ADC,
+	ADIS16201_SCAN_INCLI,
+	ADIS16201_SCAN_INCLI_Y,
+	ADIS16201_SCAN_ACC_X,
+	ADIS16201_SCAN_ACC_Y,
 };
 
 static const u8 adis16201_addresses[] = {
 	[ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
 	[ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
-	[ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
+	[ADIS16201_SCAN_INCLI] = ADIS16201_INCL_OFFS_REG,
 	[ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
 };
 
+static const struct iio_chan_spec adis16201_channels[] = {
+	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY,
+	0, 12),
+	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+	ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
+		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+	ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
+		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+	ADIS_INCLI_CHAN(X, ADIS16201_INCL_OUT_REG, ADIS16201_SCAN_INCLI,
+	BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
+	ADIS_INCLI_CHAN(Y, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
+		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
+	IIO_CHAN_SOFT_TIMESTAMP(7)
+};
+
+static const struct iio_chan_spec adis16203_channels[] = {
+	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
+	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+	ADIS_INCLI_CHAN(X, ADIS16201_INCL_OUT_REG, ADIS16201_SCAN_INCLI,
+		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+	IIO_CHAN_SOFT_TIMESTAMP(4)
+};
+
+struct differ_info {
+	u16 write_mask_incli;
+	unsigned int incli_scale_val2;
+	unsigned int read_bits_incli;
+	const struct iio_chan_spec *arr_chans;
+	u16 diag_stat_mask;
+	unsigned int num_chans;
+};
+
+static const struct differ_info adis16201_diff = {
+	.write_mask_incli = GENMASK(8, 0),
+	.incli_scale_val2 = 100000,
+	.read_bits_incli = 9,
+	.arr_chans = adis16201_channels,
+	.diag_stat_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
+		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
+		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
+		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
+	.num_chans = ARRAY_SIZE(adis16201_channels)
+};
+
+static const struct differ_info adis16203_diff = {
+	.write_mask_incli = GENMASK(13, 0),
+	.incli_scale_val2 = 25000,
+	.read_bits_incli = 14,
+	.arr_chans = adis16203_channels,
+	.diag_stat_mask = BIT(ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT) |
+		BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
+		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
+		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
+		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
+	.num_chans = ARRAY_SIZE(adis16203_channels)
+};
+
+struct adis16201_state {
+	struct adis adis;
+	const struct differ_info *info;
+};
+
+static int adis16201_write_raw(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int val,
+			       int val2,
+			       long mask)
+{
+	struct adis16201_state *st = iio_priv(indio_dev);
+	int m;
+
+	if (mask != IIO_CHAN_INFO_CALIBBIAS)
+		return -EINVAL;
+
+	switch (chan->type) {
+	case IIO_ACCEL:
+		m = GENMASK(11, 0);
+		break;
+	case IIO_INCLI:
+		m = st->info->write_mask_incli;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return adis_write_reg_16(&st->adis, adis16201_addresses[chan->scan_index],
+				 val & m);
+}
+
 static int adis16201_read_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan,
-			      int *val, int *val2,
-			      long mask)
+			      int *val,
+			      int *val2, long mask)
 {
-	struct adis *st = iio_priv(indio_dev);
+	struct adis16201_state *st = iio_priv(indio_dev);
 	int ret;
 	int bits;
 	u8 addr;
@@ -126,6 +220,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
 			*val = -470;
 			*val2 = 0;
 			return IIO_VAL_INT_PLUS_MICRO;
+
+		case IIO_INCLI:
+			*val = 0;
+			*val2 = st->info->incli_scale_val2;
+			return IIO_VAL_INT_PLUS_MICRO;
 		case IIO_ACCEL:
 			/*
 			 * IIO base unit for sensitivity of accelerometer
@@ -135,10 +234,6 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
 			*val = 0;
 			*val2 = IIO_G_TO_M_S_2(462400);
 			return IIO_VAL_INT_PLUS_NANO;
-		case IIO_INCLI:
-			*val = 0;
-			*val2 = 100000;
-			return IIO_VAL_INT_PLUS_MICRO;
 		default:
 			return -EINVAL;
 		}
@@ -157,13 +252,13 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
 			bits = 12;
 			break;
 		case IIO_INCLI:
-			bits = 9;
+			bits = st->info->read_bits_incli;
 			break;
 		default:
 			return -EINVAL;
 		}
 		addr = adis16201_addresses[chan->scan_index];
-		ret = adis_read_reg_16(st, addr, &val16);
+		ret = adis_read_reg_16(&st->adis, addr, &val16);
 		if (ret)
 			return ret;
 
@@ -174,49 +269,6 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int adis16201_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val,
-			       int val2,
-			       long mask)
-{
-	struct adis *st = iio_priv(indio_dev);
-	int m;
-
-	if (mask != IIO_CHAN_INFO_CALIBBIAS)
-		return -EINVAL;
-
-	switch (chan->type) {
-	case IIO_ACCEL:
-		m = GENMASK(11, 0);
-		break;
-	case IIO_INCLI:
-		m = GENMASK(8, 0);
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return adis_write_reg_16(st, adis16201_addresses[chan->scan_index],
-				 val & m);
-}
-
-static const struct iio_chan_spec adis16201_channels[] = {
-	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0,
-			 12),
-	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
-	ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
-			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
-	ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
-			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
-	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
-	ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
-			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
-	ADIS_INCLI_CHAN(Y, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
-			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(7)
-};
-
 static const struct iio_info adis16201_info = {
 	.read_raw = adis16201_read_raw,
 	.write_raw = adis16201_write_raw,
@@ -224,6 +276,7 @@ static const struct iio_info adis16201_info = {
 };
 
 static const char * const adis16201_status_error_msgs[] = {
+	[ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT] = "Self test failure",
 	[ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
 	[ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
 	[ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
@@ -248,56 +301,62 @@ static const struct adis_data adis16201_data = {
 	.timeouts = &adis16201_timeouts,
 
 	.status_error_msgs = adis16201_status_error_msgs,
-	.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
-		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
-		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
-		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
 };
 
 static int adis16201_probe(struct spi_device *spi)
 {
 	struct iio_dev *indio_dev;
-	struct adis *st;
+	struct adis16201_state *st;
+	struct adis_data data = adis16201_data;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
+	st->info = spi_get_device_match_data(spi);
 
 	indio_dev->name = spi->dev.driver->name;
 	indio_dev->info = &adis16201_info;
-
-	indio_dev->channels = adis16201_channels;
-	indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
+	indio_dev->channels = st->info->arr_chans;
+	indio_dev->num_channels = st->info->num_chans;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = adis_init(st, indio_dev, spi, &adis16201_data);
+	data.status_error_mask = st->info->diag_stat_mask;
+	ret = adis_init(&st->adis, indio_dev, spi, &data);
 	if (ret)
 		return ret;
 
-	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
 	if (ret)
 		return ret;
 
-	ret = __adis_initial_startup(st);
+	ret = __adis_initial_startup(&st->adis);
 	if (ret)
 		return ret;
 
 	return devm_iio_device_register(&spi->dev, indio_dev);
 }
 
+static const struct of_device_id adis16201_of_match[] = {
+	{ .compatible = "adi,adis16201", .data = &adis16201_diff },
+	{ .compatible = "adi,adis16203", .data = &adis16203_diff },
+	{ }
+};
+
 static struct spi_driver adis16201_driver = {
 	.driver = {
-		.name = "adis16201",
+		.name = "adis16201_adis16203",
+		.of_match_table = adis16201_of_match,
 	},
 	.probe = adis16201_probe,
 };
 module_spi_driver(adis16201_driver);
 
 MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
+MODULE_DESCRIPTION("Analog Devices ADIS16201 Inclinometer/Accelerometer and ADIS16203 Inclinometer Driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("spi:adis16201");
+MODULE_ALIAS("spi:adis16203");
 MODULE_IMPORT_NS("IIO_ADISLIB");
-- 
2.43.0

Re: [PATCH 1/3] iio: accel: adis16201: merge adis16203 support from staging

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Stack variable address is passed to persistent state, leading to a Use-After-Free.
- [Critical] IIO device name is set to the combined driver name, breaking userspace ABI.
- [High] Missing `spi_device_id` table causes a NULL pointer dereference in the SPI core when matched via fallback.
- [High] Missing NULL check on `spi_get_device_match_data()` return value leads to NULL pointer dereference.
--

commit 4a7d80a7646bf1890c13a1b5b3c99c166b2c3898
Author: Shehryar Ahmad <shehryar.amd@gmail.com>

iio: accel: adis16201: merge adis16203 support from staging
    
This commit merges support for the ADIS16203 inclinometer into the mainline
ADIS16201 driver. It leverages of_device_id match data to handle differences
in write masks and calibration bit widths between the two chips.

> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index 5127e58eebc7d..293655fb00531 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c
[ ... ]
> @@ -248,56 +301,62 @@ static const struct adis_data adis16201_data = {
[ ... ]
>  static int adis16201_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
> -	struct adis *st;
> +	struct adis16201_state *st;
> +	struct adis_data data = adis16201_data;

[Severity: Critical]
Is it safe to allocate this struct adis_data on the stack here?

Later in this function, the address of this stack variable is passed to
adis_init(), which stores it in adis->data:

drivers/iio/imu/adis.c:adis_init() {
    ...
    adis->data = data;
    ...
}

Since the stack frame is destroyed when adis16201_probe() returns, won't any
subsequent device operation (such as reading raw values or trigger handler
execution) dereference adis->data and read unallocated stack memory?

>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	st->info = spi_get_device_match_data(spi);
>  
>  	indio_dev->name = spi->dev.driver->name;

[Severity: Critical]
Will this change break backward compatibility for existing userspace
applications?

Previously, spi->dev.driver->name was "adis16201". Since the driver name
is updated later in this patch to "adis16201_adis16203", the exported sysfs
name attribute will change. Any userspace applications or udev rules matching
on the exact sensor name might fail.

>  	indio_dev->info = &adis16201_info;
> -
> -	indio_dev->channels = adis16201_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> +	indio_dev->channels = st->info->arr_chans;

[Severity: High]
Could st->info be NULL when it is dereferenced here?

If the driver is bound without match data, for example via manual sysfs
binding or legacy board files, spi_get_device_match_data() will return NULL.
This would result in a kernel oops when accessing st->info->arr_chans.

> +	indio_dev->num_channels = st->info->num_chans;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	ret = adis_init(st, indio_dev, spi, &adis16201_data);
> +	data.status_error_mask = st->info->diag_stat_mask;
> +	ret = adis_init(&st->adis, indio_dev, spi, &data);
>  	if (ret)
>  		return ret;
[ ... ]
> +static const struct of_device_id adis16201_of_match[] = {
> +	{ .compatible = "adi,adis16201", .data = &adis16201_diff },
> +	{ .compatible = "adi,adis16203", .data = &adis16203_diff },
> +	{ }
> +};
> +
>  static struct spi_driver adis16201_driver = {
>  	.driver = {
> -		.name = "adis16201",
> +		.name = "adis16201_adis16203",
> +		.of_match_table = adis16201_of_match,
>  	},
>  	.probe = adis16201_probe,
>  };

[Severity: High]
Should this struct include an id_table?

If the device is instantiated without OF/ACPI (like legacy spi_board_info or
sysfs driver_override), device_get_match_data() returns NULL. When the probe
function then calls spi_get_device_match_data(), it falls back to
spi_get_device_id().

Since the id_table is missing here, spi_get_device_id() passes a NULL pointer
to spi_match_id(), which can cause a kernel oops when it tries to access
id->name[0]. Local users with sysfs access or board initialization code could
trigger this crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831184201.34416-1-shehryar.amd@gmail.com?part=1

Re: [PATCH 1/3] iio: accel: adis16201: merge adis16203 support from staging

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 11:41:59PM +0500, Shehryar Ahmad wrote:
> Merge the ADIS16203 360 degree inclinometer driver from staging to
> mainline ADIS16201 driver. Register addresses and external SPI interface
> are identical between both. Some things that differ like write mask, and
> calibbias bit width are handled by per chip struct differ_info, handled
> by of_device_id match data in probe, following the same pattern used in
> adis16475 which selects per-chip data directly via of_device_id.data and
> spi_get_device_match_data(), adis16480 does similar but via a different
> mechanism (index into an array via the SPI ID table).
> 
> Channels arrays are kept separate to avoid dropping const and kmemdup
> would be extra failure point if memory allocation fails and is memory
> inefficient.
> 
> GENMASK is still same instead of range check to keep adis16201 behaviour

"GENMASK():s are..." ?

> unchanged

Missing period.

...

Yes, it's a good change, but please split it to 3+ patches. See below.

...

> - * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + * Analog Devices ADIS16201 Inclinometer/Accelerometer and ADIS16203 Inclinometer Driver

This needs to be written in the more generic form. Also the same text should be
in MODULE_DESCRIPTION() and in the Summary of the Kconfig help text (which is
missing to update). This will be part of the patch that adds the new HW.

...

>   * Copyright 2010 Analog Devices Inc.

This probably needs to be merged as well. I haven't checked the actual lines in
the other driver, though.

...

> -#define ADIS16201_STARTUP_DELAY_MS			220
> -#define ADIS16201_FLASH_CNT				0x00
> +#define ADIS16201_STARTUP_DELAY_MS			        220
> +#define ADIS16201_FLASH_CNT					        0x00
>  
>  /* Data Output Register Information */
> -#define ADIS16201_SUPPLY_OUT_REG			0x02
> -#define ADIS16201_XACCL_OUT_REG				0x04
> -#define ADIS16201_YACCL_OUT_REG				0x06
> -#define ADIS16201_AUX_ADC_REG				0x08
> -#define ADIS16201_TEMP_OUT_REG				0x0A
> -#define ADIS16201_XINCL_OUT_REG				0x0C
> -#define ADIS16201_YINCL_OUT_REG				0x0E
> +#define ADIS16201_SUPPLY_OUT_REG					0x02
> +#define ADIS16201_XACCL_OUT_REG				        0x04
> +#define ADIS16201_YACCL_OUT_REG				        0x06
> +#define ADIS16201_AUX_ADC_REG				        0x08
> +#define ADIS16201_TEMP_OUT_REG				        0x0A
> +#define ADIS16201_INCL_OUT_REG		                0x0C
> +#define ADIS16201_YINCL_OUT_REG				        0x0E
>  
>  /* Calibration Register Definition */
> -#define ADIS16201_XACCL_OFFS_REG			0x10
> -#define ADIS16201_YACCL_OFFS_REG			0x12
> -#define ADIS16201_XACCL_SCALE_REG			0x14
> -#define ADIS16201_YACCL_SCALE_REG			0x16
> -#define ADIS16201_XINCL_OFFS_REG			0x18
> -#define ADIS16201_YINCL_OFFS_REG			0x1A
> -#define ADIS16201_XINCL_SCALE_REG			0x1C
> -#define ADIS16201_YINCL_SCALE_REG			0x1E
> +#define ADIS16201_XACCL_OFFS_REG			        0x10
> +#define ADIS16201_YACCL_OFFS_REG			        0x12
> +#define ADIS16201_XACCL_SCALE_REG			        0x14
> +#define ADIS16201_YACCL_SCALE_REG			        0x16
> +#define ADIS16201_INCL_OFFS_REG			            0x18
> +#define ADIS16201_YINCL_OFFS_REG			        0x1A
> +#define ADIS16201_XINCL_SCALE_REG			        0x1C
> +#define ADIS16201_YINCL_SCALE_REG			        0x1E
>  
>  /* Alarm Register Definition */
> -#define ADIS16201_ALM_MAG1_REG				0x20
> -#define ADIS16201_ALM_MAG2_REG				0x22
> -#define ADIS16201_ALM_SMPL1_REG				0x24
> -#define ADIS16201_ALM_SMPL2_REG				0x26
> -#define ADIS16201_ALM_CTRL_REG				0x28
> -
> -#define ADIS16201_AUX_DAC_REG				0x30
> -#define ADIS16201_GPIO_CTRL_REG				0x32
> -#define ADIS16201_SMPL_PRD_REG				0x36
> +#define ADIS16201_ALM_MAG1_REG				        0x20
> +#define ADIS16201_ALM_MAG2_REG				        0x22
> +#define ADIS16201_ALM_SMPL1_REG				        0x24
> +#define ADIS16201_ALM_SMPL2_REG				        0x26
> +#define ADIS16201_ALM_CTRL_REG				        0x28
> +
> +#define ADIS16201_AUX_DAC_REG						0x30
> +#define ADIS16201_GPIO_CTRL_REG						0x32
> +#define ADIS16201_SMPL_PRD_REG						0x36
> +
>  /* Operation, filter configuration */
> -#define ADIS16201_AVG_CNT_REG				0x38
> -#define ADIS16201_SLP_CNT_REG				0x3A
> +#define ADIS16201_AVG_CNT_REG				        0x38
> +#define ADIS16201_SLP_CNT_REG				        0x3A

Make indentation changes either separate or do not touch these lines at all,
it's very hard to check if there were any real changes (in the offsets
and/or names).

...

>  enum adis16201_scan {
> -	ADIS16201_SCAN_ACC_X,
> -	ADIS16201_SCAN_ACC_Y,
> -	ADIS16201_SCAN_INCLI_X,
> -	ADIS16201_SCAN_INCLI_Y,
>  	ADIS16201_SCAN_SUPPLY,
> -	ADIS16201_SCAN_AUX_ADC,
>  	ADIS16201_SCAN_TEMP,
> +	ADIS16201_SCAN_AUX_ADC,
> +	ADIS16201_SCAN_INCLI,
> +	ADIS16201_SCAN_INCLI_Y,
> +	ADIS16201_SCAN_ACC_X,
> +	ADIS16201_SCAN_ACC_Y,
>  };

Why do you need to reshuffle the enum? This needs a good justification and
explanation why it's not a problem for the existing support.

...

> +static const struct iio_chan_spec adis16201_channels[] = {
> +	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY,
> +	0, 12),
> +	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> +	ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> +	ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> +	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> +	ADIS_INCLI_CHAN(X, ADIS16201_INCL_OUT_REG, ADIS16201_SCAN_INCLI,
> +	BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> +	ADIS_INCLI_CHAN(Y, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> +	IIO_CHAN_SOFT_TIMESTAMP(7)

Keep trailing commas in non-termination entries (yes, while it looks like
the end of the list, strictly speaking it's not). Same for the rest of the
similar cases.

> +};

...

> +struct differ_info {
> +	u16 write_mask_incli;
> +	unsigned int incli_scale_val2;
> +	unsigned int read_bits_incli;
> +	const struct iio_chan_spec *arr_chans;
> +	u16 diag_stat_mask;
> +	unsigned int num_chans;

Always run `pahole` when do some data type changes or introduction. This one
may have unneeded gaps.

> +};

...

> +static const struct differ_info adis16201_diff = {
> +	.write_mask_incli = GENMASK(8, 0),
> +	.incli_scale_val2 = 100000,

Would it make sense to use multipliers from units.h or elsewhere?

> +	.read_bits_incli = 9,
> +	.arr_chans = adis16201_channels,
> +	.diag_stat_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),

Make the BIT() indented in the same way, I recommend to have

	.diag_stat_mask =
		BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |

> +	.num_chans = ARRAY_SIZE(adis16201_channels)
> +};

...

> +static const struct differ_info adis16203_diff = {

Same comments as per above.

> +};

...

> +static int adis16201_write_raw(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,

> +			       int val,
> +			       int val2,
> +			       long mask)

There is room for all three on the one line.

> +{
> +	struct adis16201_state *st = iio_priv(indio_dev);
> +	int m;

Why signed?

> +	if (mask != IIO_CHAN_INFO_CALIBBIAS)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_ACCEL:
> +		m = GENMASK(11, 0);
> +		break;
> +	case IIO_INCLI:
> +		m = st->info->write_mask_incli;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return adis_write_reg_16(&st->adis, adis16201_addresses[chan->scan_index],
> +				 val & m);

Apply mask to the valu directly in the switch case, drop 'm'.

With

	struct adis *adis = ...;

and the above suggestion this becomes

	return adis_write_reg_16(adis, adis16201_addresses[chan->scan_index], val);

> +}

...

>  static int adis16201_read_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan,
> -			      int *val, int *val2,
> -			      long mask)
> +			      int *val,
> +			      int *val2, long mask)

This is stray change. Why?!

>  {

...

> +		case IIO_INCLI:
> +			*val = 0;
> +			*val2 = st->info->incli_scale_val2;
> +			return IIO_VAL_INT_PLUS_MICRO;


> -		case IIO_INCLI:
> -			*val = 0;
> -			*val2 = 100000;
> -			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;

Same Q.

...

Now, looking at the above I see an additional preparatory patch, id est
the new data type (struct adis16201_state) that wraps existing one. In
the following changes you may extend it as required.

...

> -static int adis16201_write_raw(struct iio_dev *indio_dev,
> -			       struct iio_chan_spec const *chan,
> -			       int val,
> -			       int val2,
> -			       long mask)
> -{
> -	struct adis *st = iio_priv(indio_dev);
> -	int m;
> -
> -	if (mask != IIO_CHAN_INFO_CALIBBIAS)
> -		return -EINVAL;
> -
> -	switch (chan->type) {
> -	case IIO_ACCEL:
> -		m = GENMASK(11, 0);
> -		break;
> -	case IIO_INCLI:
> -		m = GENMASK(8, 0);
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	return adis_write_reg_16(st, adis16201_addresses[chan->scan_index],
> -				 val & m);
> -}

Ah, this code is moved up and being modified. So, split this to a few patches:

- move this function up (no changes, explain why you will need it up)
- modify as I suggested above (apply mast directly to the value)
- add your changes in the final (big) patch

...

>  	struct iio_dev *indio_dev;
> -	struct adis *st;
> +	struct adis16201_state *st;
> +	struct adis_data data = adis16201_data;
>  	int ret;

> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));

Why?!

>  	if (!indio_dev)
>  		return -ENOMEM;

...

> +	st->info = spi_get_device_match_data(spi);

This should be checked against NULL and ENODATA to be returned otherwise.

Also switching to chip_info should go separately.

...

>  	indio_dev->name = spi->dev.driver->name;

This needs to be part of the chip_info structure.

...

> +static const struct of_device_id adis16201_of_match[] = {
> +	{ .compatible = "adi,adis16201", .data = &adis16201_diff },
> +	{ .compatible = "adi,adis16203", .data = &adis16203_diff },
> +	{ }
> +};

Missing MODULE_DEVICE_TABLE() (IIRC the macro name). Also this needs to be
added in a separate patch.

...

>  static struct spi_driver adis16201_driver = {
>  	.driver = {
> -		.name = "adis16201",
> +		.name = "adis16201_adis16203",
> +		.of_match_table = adis16201_of_match,
>  	},
>  	.probe = adis16201_probe,
>  };
>  module_spi_driver(adis16201_driver);

...

>  MODULE_ALIAS("spi:adis16201");
> +MODULE_ALIAS("spi:adis16203");

No, make proper SPI ID table to be present instead (in a separate patch)

...

Overall I counted something like 7 patches this one has to be split into.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] iio: accel: adis16201: merge adis16203 support from staging

[Jonathan Cameron]

On Mon, 31 Aug 2026 23:41:59 +0500
Shehryar Ahmad <shehryar.amd@gmail.com> wrote:

> Merge the ADIS16203 360 degree inclinometer driver from staging to
> mainline ADIS16201 driver. Register addresses and external SPI interface
> are identical between both. Some things that differ like write mask, and
> calibbias bit width are handled by per chip struct differ_info, handled
> by of_device_id match data in probe, following the same pattern used in
> adis16475 which selects per-chip data directly via of_device_id.data and
> spi_get_device_match_data(), adis16480 does similar but via a different
> mechanism (index into an array via the SPI ID table).
> 
> Channels arrays are kept separate to avoid dropping const and kmemdup
> would be extra failure point if memory allocation fails and is memory
> inefficient.
> 
> GENMASK is still same instead of range check to keep adis16201 behaviour
> unchanged
> 
> Signed-off-by: Shehryar Ahmad <shehryar.amd@gmail.com>
Hi Shehryar,

I suspect I repeated quite a bit of what Andy raised, but various comments
inline.  Spend some time on minimizing the diff and splitting this up
into a series of patches. Each of the following could be multiple
patches.

First tidy ups etc / introduction of device id tables etc.
Second real changes to enable multi parts, but no new parts yet.
Third add the new device support with any stuff unique to that device.

> ---
>  drivers/iio/accel/adis16201.c | 289 ++++++++++++++++++++--------------
>  1 file changed, 174 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index ba0f97944..ee8efeed2 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + * Analog Devices ADIS16201 Inclinometer/Accelerometer and ADIS16203 Inclinometer Driver

Break this into one line per part. Ends up a lot less churn heavy if we add
more in future.

>   *
>   * Copyright 2010 Analog Devices Inc.
>   */
> @@ -13,93 +13,187 @@
>  #include <linux/iio/iio.h>
...

>  
>  /* System Command Register Definition */
> -#define ADIS16201_GLOB_CMD_REG				0x3E
> -#define  ADIS16201_GLOB_CMD_SW_RESET			BIT(7)
> -#define  ADIS16201_GLOB_CMD_FACTORY_RESET		BIT(1)
> +#define ADIS16201_GLOB_CMD_REG				        0x3E
> +#define ADIS16201_GLOB_CMD_SW_RESET			        BIT(7)
> +#define ADIS16203_GLOB_CMD_CLEAR_STAT               BIT(4)
> +#define ADIS16201_GLOB_CMD_FACTORY_RESET		    BIT(1)
>  
> -#define ADIS16201_ERROR_ACTIVE				BIT(14)
> +#define ADIS16201_ERROR_ACTIVE				        BIT(14)

You need a very good reason to change existing driver white space in
register definitions.  Particularly as what you have is worse as it
removes the clear separation between register address and field masks that
those extra little bits of white space where givving.

>  
>  enum adis16201_scan {
> -	ADIS16201_SCAN_ACC_X,
> -	ADIS16201_SCAN_ACC_Y,
> -	ADIS16201_SCAN_INCLI_X,
> -	ADIS16201_SCAN_INCLI_Y,
>  	ADIS16201_SCAN_SUPPLY,
> -	ADIS16201_SCAN_AUX_ADC,
>  	ADIS16201_SCAN_TEMP,
> +	ADIS16201_SCAN_AUX_ADC,
> +	ADIS16201_SCAN_INCLI,
> +	ADIS16201_SCAN_INCLI_Y,
> +	ADIS16201_SCAN_ACC_X,
> +	ADIS16201_SCAN_ACC_Y,
>  };
>  
>  static const u8 adis16201_addresses[] = {
>  	[ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
>  	[ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
> -	[ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
> +	[ADIS16201_SCAN_INCLI] = ADIS16201_INCL_OFFS_REG,
>  	[ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
>  };
>  
> +static const struct iio_chan_spec adis16201_channels[] = {
> +	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY,
> +	0, 12),
> +	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> +	ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> +	ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> +	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> +	ADIS_INCLI_CHAN(X, ADIS16201_INCL_OUT_REG, ADIS16201_SCAN_INCLI,
> +	BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> +	ADIS_INCLI_CHAN(Y, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> +	IIO_CHAN_SOFT_TIMESTAMP(7)
> +};
> +
> +static const struct iio_chan_spec adis16203_channels[] = {
> +	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
> +	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> +	ADIS_INCLI_CHAN(X, ADIS16201_INCL_OUT_REG, ADIS16201_SCAN_INCLI,
> +		BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> +	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> +	IIO_CHAN_SOFT_TIMESTAMP(4)
> +};
> +
> +struct differ_info {

adis16201_chip_info or something like that. differ_info is not
a naming convention we use in other drivers and is not immediately
obvious to me.  Each instance is data about a chip, not the difference
in anything.


> +	u16 write_mask_incli;
> +	unsigned int incli_scale_val2;
> +	unsigned int read_bits_incli;
> +	const struct iio_chan_spec *arr_chans;
> +	u16 diag_stat_mask;
> +	unsigned int num_chans;
> +};
> +
> +static const struct differ_info adis16201_diff = {
> +	.write_mask_incli = GENMASK(8, 0),
> +	.incli_scale_val2 = 100000,
> +	.read_bits_incli = 9,
> +	.arr_chans = adis16201_channels,
> +	.diag_stat_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> +	.num_chans = ARRAY_SIZE(adis16201_channels)
> +};
> +
> +static const struct differ_info adis16203_diff = {
> +	.write_mask_incli = GENMASK(13, 0),
> +	.incli_scale_val2 = 25000,
> +	.read_bits_incli = 14,
> +	.arr_chans = adis16203_channels,
> +	.diag_stat_mask = BIT(ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> +		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> +	.num_chans = ARRAY_SIZE(adis16203_channels)
> +};
I'd be tempted to shift all this stuff down to where the chan_spec
array originally was. Should give you a more readable diff.
Only the structure definitions should need to be up here, not the
instances.

> +
> +struct adis16201_state {
> +	struct adis adis;
> +	const struct differ_info *info;
> +};
> +
> +static int adis16201_write_raw(struct iio_dev *indio_dev,

If this needs to move, do it in a patch that has a description of why.
Also rewrap parameters to have similar ones on same line if that
line ends up fairly short.

> +			       struct iio_chan_spec const *chan,
> +			       int val,
> +			       int val2,
> +			       long mask)
> +{
> +	struct adis16201_state *st = iio_priv(indio_dev);
> +	int m;
> +
> +	if (mask != IIO_CHAN_INFO_CALIBBIAS)
> +		return -EINVAL;
> +
> +	switch (chan->type) {
> +	case IIO_ACCEL:
> +		m = GENMASK(11, 0);
> +		break;
> +	case IIO_INCLI:
> +		m = st->info->write_mask_incli;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return adis_write_reg_16(&st->adis, adis16201_addresses[chan->scan_index],
> +				 val & m);
> +}
> +
>  static int adis16201_read_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan,
> -			      int *val, int *val2,
> -			      long mask)
> +			      int *val,
> +			      int *val2, long mask)
>  {
> -	struct adis *st = iio_priv(indio_dev);
> +	struct adis16201_state *st = iio_priv(indio_dev);
>  	int ret;
>  	int bits;
>  	u8 addr;
> @@ -126,6 +220,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>  			*val = -470;
>  			*val2 = 0;
>  			return IIO_VAL_INT_PLUS_MICRO;
> +
> +		case IIO_INCLI:
> +			*val = 0;
> +			*val2 = st->info->incli_scale_val2;
> +			return IIO_VAL_INT_PLUS_MICRO;

Keep the ordering the same. No obvious reason to move it and it will
make the diff more obvious.

>  		case IIO_ACCEL:
>  			/*
>  			 * IIO base unit for sensitivity of accelerometer
> @@ -135,10 +234,6 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>  			*val = 0;
>  			*val2 = IIO_G_TO_M_S_2(462400);
>  			return IIO_VAL_INT_PLUS_NANO;
> -		case IIO_INCLI:
> -			*val = 0;
> -			*val2 = 100000;
> -			return IIO_VAL_INT_PLUS_MICRO;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -157,13 +252,13 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>  			bits = 12;
>  			break;
>  		case IIO_INCLI:
> -			bits = 9;
> +			bits = st->info->read_bits_incli;
>  			break;
>  		default:
>  			return -EINVAL;
>  		}
>  		addr = adis16201_addresses[chan->scan_index];
> -		ret = adis_read_reg_16(st, addr, &val16);
> +		ret = adis_read_reg_16(&st->adis, addr, &val16);
>  		if (ret)
>  			return ret;
>  
> @@ -174,49 +269,6 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
>  	return -EINVAL;
>  }
>  
> -static int adis16201_write_raw(struct iio_dev *indio_dev,
> -			       struct iio_chan_spec const *chan,
> -			       int val,
> -			       int val2,
> -			       long mask)
> -{
> -	struct adis *st = iio_priv(indio_dev);
> -	int m;
> -
> -	if (mask != IIO_CHAN_INFO_CALIBBIAS)
> -		return -EINVAL;
> -
> -	switch (chan->type) {
> -	case IIO_ACCEL:
> -		m = GENMASK(11, 0);
> -		break;
> -	case IIO_INCLI:
> -		m = GENMASK(8, 0);
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	return adis_write_reg_16(st, adis16201_addresses[chan->scan_index],
> -				 val & m);
> -}
> -
> -static const struct iio_chan_spec adis16201_channels[] = {
> -	ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0,
> -			 12),
> -	ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> -	ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> -			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> -	ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> -			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> -	ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> -	ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
> -			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> -	ADIS_INCLI_CHAN(Y, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> -			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 12),
> -	IIO_CHAN_SOFT_TIMESTAMP(7)
> -};
> -

See above. I think all this code movement can be easily avoided
but maybe I'm missing something.  If it does need moving, separate
patch that makes it clear why.

>  static const struct iio_info adis16201_info = {
>  	.read_raw = adis16201_read_raw,
>  	.write_raw = adis16201_write_raw,
> @@ -224,6 +276,7 @@ static const struct iio_info adis16201_info = {
>  };
>  
>  static const char * const adis16201_status_error_msgs[] = {
> +	[ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT] = "Self test failure",

Bring this in only in the patch (after split) that adds adis16203 support.

>  	[ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
>  	[ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
>  	[ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> @@ -248,56 +301,62 @@ static const struct adis_data adis16201_data = {
>  	.timeouts = &adis16201_timeouts,
>  
>  	.status_error_msgs = adis16201_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
>  };
>  
>  static int adis16201_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
> -	struct adis *st;
> +	struct adis16201_state *st;
> +	struct adis_data data = adis16201_data;
Pull the initialization down here rather than copying the const copy
above. Even better do it where you know the remaining value.

	data = (struct adis_data) {
		.whatever =,
	};

>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	st->info = spi_get_device_match_data(spi);

Annoyingly there are baths where that fails because the user is forcing
binding to a part without a matching firmware description so check for NULL and
fail probe if you get it.

>  
>  	indio_dev->name = spi->dev.driver->name;
>  	indio_dev->info = &adis16201_info;
> -
> -	indio_dev->channels = adis16201_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> +	indio_dev->channels = st->info->arr_chans;
> +	indio_dev->num_channels = st->info->num_chans;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	ret = adis_init(st, indio_dev, spi, &adis16201_data);
> +	data.status_error_mask = st->info->diag_stat_mask;

This is where the adis = bit above belongs.

> +	ret = adis_init(&st->adis, indio_dev, spi, &data);
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> +	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
>  	if (ret)
>  		return ret;
>  
> -	ret = __adis_initial_startup(st);
> +	ret = __adis_initial_startup(&st->adis);
>  	if (ret)
>  		return ret;
>  
>  	return devm_iio_device_register(&spi->dev, indio_dev);
>  }
>  
> +static const struct of_device_id adis16201_of_match[] = {
> +	{ .compatible = "adi,adis16201", .data = &adis16201_diff },
> +	{ .compatible = "adi,adis16203", .data = &adis16203_diff },
> +	{ }
> +};
> +
>  static struct spi_driver adis16201_driver = {
>  	.driver = {
> -		.name = "adis16201",
> +		.name = "adis16201_adis16203",
> +		.of_match_table = adis16201_of_match,

Add an of table in a separate patch - including the MODULE_DEVICE_TABLE() stuff
mentioned below.


>  	},
>  	.probe = adis16201_probe,
>  };
>  module_spi_driver(adis16201_driver);
>  
>  MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
> -MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
> +MODULE_DESCRIPTION("Analog Devices ADIS16201 Inclinometer/Accelerometer and ADIS16203 Inclinometer Driver");

We normally just add 'and similar' for cases like this.   Once there are two
parts the chances of a third become too high to have an ever extending description.
Do that as part of a refactor only patch. As Andy mentioned, this needs
breaking up into no op changes with no new support (in appropriate
sized parts) then introduction of new stuff such as the adis16203 support.

>  MODULE_LICENSE("GPL v2");
>  MODULE_ALIAS("spi:adis16201");
> +MODULE_ALIAS("spi:adis16203");

I'm not sure why we need any explicit MODULE_ALIAS() in here in the first
places vs MODULE_DEVICE_TABLE() calls for the two tables.

Make that change as a precursor patch having verified it should generate the same
effective module alias for the parts already supported (I don't care about the
one you are bringing out of staging - though it should work anyway).

>  MODULE_IMPORT_NS("IIO_ADISLIB");

[PATCH 2/3] staging: iio: accel: remove adis16203, merged into mainline adis16201 driver

[Shehryar Ahmad]

The ADIS16203 is merged with mainline ADIS16201 in previous patch of
series, remove the staging copy and its build system entries.

Signed-off-by: Shehryar Ahmad <shehryar.amd@gmail.com>
---
 drivers/staging/iio/Kconfig           |   1 -
 drivers/staging/iio/Makefile          |   1 -
 drivers/staging/iio/accel/Kconfig     |  19 --
 drivers/staging/iio/accel/Makefile    |   6 -
 drivers/staging/iio/accel/adis16203.c | 315 --------------------------
 5 files changed, 342 deletions(-)
 delete mode 100644 drivers/staging/iio/accel/Kconfig
 delete mode 100644 drivers/staging/iio/accel/Makefile
 delete mode 100644 drivers/staging/iio/accel/adis16203.c

diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index a60631c1f..bf6addf29 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -5,7 +5,6 @@
 menu "IIO staging drivers"
 	depends on IIO
 
-source "drivers/staging/iio/accel/Kconfig"
 source "drivers/staging/iio/adc/Kconfig"
 source "drivers/staging/iio/addac/Kconfig"
 source "drivers/staging/iio/frequency/Kconfig"
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index 628583535..739e4cbf9 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -3,7 +3,6 @@
 # Makefile for the industrial I/O core.
 #
 
-obj-y += accel/
 obj-y += adc/
 obj-y += addac/
 obj-y += frequency/
diff --git a/drivers/staging/iio/accel/Kconfig b/drivers/staging/iio/accel/Kconfig
deleted file mode 100644
index cee51f64b..000000000
--- a/drivers/staging/iio/accel/Kconfig
+++ /dev/null
@@ -1,19 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Accelerometer drivers
-#
-menu "Accelerometers"
-
-config ADIS16203
-	tristate "Analog Devices ADIS16203 Programmable 360 Degrees Inclinometer"
-	depends on SPI
-	select IIO_ADIS_LIB
-	select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
-	help
-	  Say Y here to build support for Analog Devices adis16203 Programmable
-	  360 Degrees Inclinometer.
-
-	  To compile this driver as a module, say M here: the module will be
-	  called adis16203.
-
-endmenu
diff --git a/drivers/staging/iio/accel/Makefile b/drivers/staging/iio/accel/Makefile
deleted file mode 100644
index acac7bc9b..000000000
--- a/drivers/staging/iio/accel/Makefile
+++ /dev/null
@@ -1,6 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-#
-# Makefile for industrial I/O accelerometer drivers
-#
-
-obj-$(CONFIG_ADIS16203) += adis16203.o
diff --git a/drivers/staging/iio/accel/adis16203.c b/drivers/staging/iio/accel/adis16203.c
deleted file mode 100644
index 830ff38fd..000000000
--- a/drivers/staging/iio/accel/adis16203.c
+++ /dev/null
@@ -1,315 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * ADIS16203 Programmable 360 Degrees Inclinometer
- *
- * Copyright 2010 Analog Devices Inc.
- */
-
-#include <linux/device.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/imu/adis.h>
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/spi/spi.h>
-
-#define ADIS16203_STARTUP_DELAY 220 /* ms */
-
-/* Flash memory write count */
-#define ADIS16203_FLASH_CNT      0x00
-
-/* Output, power supply */
-#define ADIS16203_SUPPLY_OUT     0x02
-
-/* Output, auxiliary ADC input */
-#define ADIS16203_AUX_ADC        0x08
-
-/* Output, temperature */
-#define ADIS16203_TEMP_OUT       0x0A
-
-/* Output, x-axis inclination */
-#define ADIS16203_XINCL_OUT      0x0C
-
-/* Output, y-axis inclination */
-#define ADIS16203_YINCL_OUT      0x0E
-
-/* Incline null calibration */
-#define ADIS16203_INCL_NULL      0x18
-
-/* Alarm 1 amplitude threshold */
-#define ADIS16203_ALM_MAG1       0x20
-
-/* Alarm 2 amplitude threshold */
-#define ADIS16203_ALM_MAG2       0x22
-
-/* Alarm 1, sample period */
-#define ADIS16203_ALM_SMPL1      0x24
-
-/* Alarm 2, sample period */
-#define ADIS16203_ALM_SMPL2      0x26
-
-/* Alarm control */
-#define ADIS16203_ALM_CTRL       0x28
-
-/* Auxiliary DAC data */
-#define ADIS16203_AUX_DAC        0x30
-
-/* General-purpose digital input/output control */
-#define ADIS16203_GPIO_CTRL      0x32
-
-/* Miscellaneous control */
-#define ADIS16203_MSC_CTRL       0x34
-
-/* Internal sample period (rate) control */
-#define ADIS16203_SMPL_PRD       0x36
-
-/* Operation, filter configuration */
-#define ADIS16203_AVG_CNT        0x38
-
-/* Operation, sleep mode control */
-#define ADIS16203_SLP_CNT        0x3A
-
-/* Diagnostics, system status register */
-#define ADIS16203_DIAG_STAT      0x3C
-
-/* Operation, system command register */
-#define ADIS16203_GLOB_CMD       0x3E
-
-/* MSC_CTRL */
-
-/* Self-test at power-on: 1 = disabled, 0 = enabled */
-#define ADIS16203_MSC_CTRL_PWRUP_SELF_TEST      BIT(10)
-
-/* Reverses rotation of both inclination outputs */
-#define ADIS16203_MSC_CTRL_REVERSE_ROT_EN       BIT(9)
-
-/* Self-test enable */
-#define ADIS16203_MSC_CTRL_SELF_TEST_EN         BIT(8)
-
-/* Data-ready enable: 1 = enabled, 0 = disabled */
-#define ADIS16203_MSC_CTRL_DATA_RDY_EN          BIT(2)
-
-/* Data-ready polarity: 1 = active high, 0 = active low */
-#define ADIS16203_MSC_CTRL_ACTIVE_HIGH          BIT(1)
-
-/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
-#define ADIS16203_MSC_CTRL_DATA_RDY_DIO1        BIT(0)
-
-/* DIAG_STAT */
-
-/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16203_DIAG_STAT_ALARM2        BIT(9)
-
-/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
-#define ADIS16203_DIAG_STAT_ALARM1        BIT(8)
-
-/* Self-test diagnostic error flag */
-#define ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT 5
-
-/* SPI communications failure */
-#define ADIS16203_DIAG_STAT_SPI_FAIL_BIT      3
-
-/* Flash update failure */
-#define ADIS16203_DIAG_STAT_FLASH_UPT_BIT     2
-
-/* Power supply above 3.625 V */
-#define ADIS16203_DIAG_STAT_POWER_HIGH_BIT    1
-
-/* Power supply below 2.975 V */
-#define ADIS16203_DIAG_STAT_POWER_LOW_BIT     0
-
-/* GLOB_CMD */
-
-#define ADIS16203_GLOB_CMD_SW_RESET     BIT(7)
-#define ADIS16203_GLOB_CMD_CLEAR_STAT   BIT(4)
-#define ADIS16203_GLOB_CMD_FACTORY_CAL  BIT(1)
-
-#define ADIS16203_ERROR_ACTIVE          BIT(14)
-
-enum adis16203_scan {
-	 ADIS16203_SCAN_INCLI_X,
-	 ADIS16203_SCAN_INCLI_Y,
-	 ADIS16203_SCAN_SUPPLY,
-	 ADIS16203_SCAN_AUX_ADC,
-	 ADIS16203_SCAN_TEMP,
-};
-
-#define DRIVER_NAME		"adis16203"
-
-static const u8 adis16203_addresses[] = {
-	[ADIS16203_SCAN_INCLI_X] = ADIS16203_INCL_NULL,
-};
-
-static int adis16203_write_raw(struct iio_dev *indio_dev,
-			       struct iio_chan_spec const *chan,
-			       int val,
-			       int val2,
-			       long mask)
-{
-	struct adis *st = iio_priv(indio_dev);
-	/* currently only one writable parameter which keeps this simple */
-	u8 addr = adis16203_addresses[chan->scan_index];
-
-	return adis_write_reg_16(st, addr, val & 0x3FFF);
-}
-
-static int adis16203_read_raw(struct iio_dev *indio_dev,
-			      struct iio_chan_spec const *chan,
-			      int *val, int *val2,
-			      long mask)
-{
-	struct adis *st = iio_priv(indio_dev);
-	int ret;
-	u8 addr;
-	s16 val16;
-
-	switch (mask) {
-	case IIO_CHAN_INFO_RAW:
-		return adis_single_conversion(indio_dev, chan,
-				ADIS16203_ERROR_ACTIVE, val);
-	case IIO_CHAN_INFO_SCALE:
-		switch (chan->type) {
-		case IIO_VOLTAGE:
-			if (chan->channel == 0) {
-				*val = 1;
-				*val2 = 220000; /* 1.22 mV */
-			} else {
-				*val = 0;
-				*val2 = 610000; /* 0.61 mV */
-			}
-			return IIO_VAL_INT_PLUS_MICRO;
-		case IIO_TEMP:
-			*val = -470; /* -0.47 C */
-			*val2 = 0;
-			return IIO_VAL_INT_PLUS_MICRO;
-		case IIO_INCLI:
-			*val = 0;
-			*val2 = 25000; /* 0.025 degree */
-			return IIO_VAL_INT_PLUS_MICRO;
-		default:
-			return -EINVAL;
-		}
-	case IIO_CHAN_INFO_OFFSET:
-		*val = 25000 / -470 - 1278; /* 25 C = 1278 */
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_CALIBBIAS:
-		addr = adis16203_addresses[chan->scan_index];
-		ret = adis_read_reg_16(st, addr, &val16);
-		if (ret)
-			return ret;
-		*val = sign_extend32(val16, 13);
-		return IIO_VAL_INT;
-	default:
-		return -EINVAL;
-	}
-}
-
-static const struct iio_chan_spec adis16203_channels[] = {
-	ADIS_SUPPLY_CHAN(ADIS16203_SUPPLY_OUT, ADIS16203_SCAN_SUPPLY, 0, 12),
-	ADIS_AUX_ADC_CHAN(ADIS16203_AUX_ADC, ADIS16203_SCAN_AUX_ADC, 0, 12),
-	ADIS_INCLI_CHAN(X, ADIS16203_XINCL_OUT, ADIS16203_SCAN_INCLI_X,
-			BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
-	/* Fixme: Not what it appears to be - see data sheet */
-	ADIS_INCLI_CHAN(Y, ADIS16203_YINCL_OUT, ADIS16203_SCAN_INCLI_Y,
-			0, 0, 14),
-	ADIS_TEMP_CHAN(ADIS16203_TEMP_OUT, ADIS16203_SCAN_TEMP, 0, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(5),
-};
-
-static const struct iio_info adis16203_info = {
-	.read_raw = adis16203_read_raw,
-	.write_raw = adis16203_write_raw,
-	.update_scan_mode = adis_update_scan_mode,
-};
-
-static const char * const adis16203_status_error_msgs[] = {
-	[ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT] = "Self test failure",
-	[ADIS16203_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
-	[ADIS16203_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
-	[ADIS16203_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
-	[ADIS16203_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 2.975V",
-};
-
-static const struct adis_timeout adis16203_timeouts = {
-	.reset_ms = ADIS16203_STARTUP_DELAY,
-	.sw_reset_ms = ADIS16203_STARTUP_DELAY,
-	.self_test_ms = ADIS16203_STARTUP_DELAY
-};
-
-static const struct adis_data adis16203_data = {
-	.read_delay = 20,
-	.msc_ctrl_reg = ADIS16203_MSC_CTRL,
-	.glob_cmd_reg = ADIS16203_GLOB_CMD,
-	.diag_stat_reg = ADIS16203_DIAG_STAT,
-
-	.self_test_mask = ADIS16203_MSC_CTRL_SELF_TEST_EN,
-	.self_test_reg = ADIS16203_MSC_CTRL,
-	.self_test_no_autoclear = true,
-	.timeouts = &adis16203_timeouts,
-
-	.status_error_msgs = adis16203_status_error_msgs,
-	.status_error_mask = BIT(ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT) |
-		BIT(ADIS16203_DIAG_STAT_SPI_FAIL_BIT) |
-		BIT(ADIS16203_DIAG_STAT_FLASH_UPT_BIT) |
-		BIT(ADIS16203_DIAG_STAT_POWER_HIGH_BIT) |
-		BIT(ADIS16203_DIAG_STAT_POWER_LOW_BIT),
-};
-
-static int adis16203_probe(struct spi_device *spi)
-{
-	int ret;
-	struct iio_dev *indio_dev;
-	struct adis *st;
-
-	/* setup the industrialio driver allocated elements */
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
-	if (!indio_dev)
-		return -ENOMEM;
-	st = iio_priv(indio_dev);
-	/* this is only used for removal purposes */
-	spi_set_drvdata(spi, indio_dev);
-
-	indio_dev->name = spi->dev.driver->name;
-	indio_dev->channels = adis16203_channels;
-	indio_dev->num_channels = ARRAY_SIZE(adis16203_channels);
-	indio_dev->info = &adis16203_info;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-
-	ret = adis_init(st, indio_dev, spi, &adis16203_data);
-	if (ret)
-		return ret;
-
-	ret = devm_adis_setup_buffer_and_trigger(st, indio_dev, NULL);
-	if (ret)
-		return ret;
-
-	/* Get the device into a sane initial state */
-	ret = __adis_initial_startup(st);
-	if (ret)
-		return ret;
-
-	return devm_iio_device_register(&spi->dev, indio_dev);
-}
-
-static const struct of_device_id adis16203_of_match[] = {
-	{ .compatible = "adi,adis16203" },
-	{ }
-};
-
-MODULE_DEVICE_TABLE(of, adis16203_of_match);
-
-static struct spi_driver adis16203_driver = {
-	.driver = {
-		.name = "adis16203",
-		.of_match_table = adis16203_of_match,
-	},
-	.probe = adis16203_probe,
-};
-module_spi_driver(adis16203_driver);
-
-MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADIS16203 Programmable 360 Degrees Inclinometer");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("spi:adis16203");
-MODULE_IMPORT_NS("IIO_ADISLIB");
-- 
2.43.0

Re: [PATCH 2/3] staging: iio: accel: remove adis16203, merged into mainline adis16201 driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The IIO device name is incorrectly set to the combined SPI driver name ('adis16201_adis16203'), breaking userspace ABI.
- [High] Missing check for spi_get_device_match_data() returning NULL leads to a crash on non-OF probes.
--

commit 4a7d80a7646bf1890c13a1b5b3c99c166b2c3898
Author: Shehryar Ahmad <shehryar.amd@gmail.com>

iio: accel: adis16201: merge adis16203 support from staging
    
This commit merges the ADIS16203 inclinometer driver from staging into the
mainline ADIS16201 driver. It handles chip differences using a differ_info
struct passed via of_device_id match data and keeps the channel arrays separate.
The SPI driver name is also updated to reflect the combined driver support.

> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index 5127e58eebc7d..293655fb00531 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c

[ ... ]

> @@ -248,56 +301,62 @@ static const struct adis_data adis16201_data = {
>  	.timeouts = &adis16201_timeouts,
>  
>  	.status_error_msgs = adis16201_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> -		BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
>  };
>  
>  static int adis16201_probe(struct spi_device *spi)
>  {
>  	struct iio_dev *indio_dev;
> -	struct adis *st;
> +	struct adis16201_state *st;
> +	struct adis_data data = adis16201_data;
>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(struct adis16201_state));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	st->info = spi_get_device_match_data(spi);
>  
>  	indio_dev->name = spi->dev.driver->name;

[Severity: Critical]
Does this code assign the combined SPI driver name "adis16201_adis16203" to
the IIO device name? The IIO name attribute is relied upon by applications
and libraries (like libiio) to identify the specific hardware part.

Since the driver name was changed from "adis16201" to "adis16201_adis16203",
the sysfs name attribute now statically exposes "adis16201_adis16203" for all
instantiated devices regardless of the actual hardware, breaking the userspace
ABI.

>  	indio_dev->info = &adis16201_info;
> -
> -	indio_dev->channels = adis16201_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> +	indio_dev->channels = st->info->arr_chans;

[Severity: High]
Can this lead to a NULL pointer dereference during non-OF probes? If the device
is instantiated via legacy SPI board info or sysfs new_device with the driver
name "adis16201_adis16203", the SPI core matches it and calls adis16201_probe()
in drivers/iio/accel/adis16201.c.

Because it's not an OF/ACPI match, spi_get_device_match_data() falls back to
checking the driver's id_table. Since the id_table is missing from
adis16201_driver, spi_get_device_match_data() returns NULL, and dereferencing
it via st->info->arr_chans causes a kernel crash.

> +	indio_dev->num_channels = st->info->num_chans;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	ret = adis_init(st, indio_dev, spi, &adis16201_data);
> +	data.status_error_mask = st->info->diag_stat_mask;
> +	ret = adis_init(&st->adis, indio_dev, spi, &data);

[ ... ]

> @@ -318,10 +372,11 @@ static int adis16201_probe(struct spi_device *spi)
>  
>  static struct spi_driver adis16201_driver = {
>  	.driver = {
> -		.name = "adis16201",
> +		.name = "adis16201_adis16203",
> +		.of_match_table = adis16201_of_match,
>  	},
>  	.probe = adis16201_probe,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831184201.34416-1-shehryar.amd@gmail.com?part=2

Re: [PATCH 2/3] staging: iio: accel: remove adis16203, merged into mainline adis16201 driver

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 11:42:00PM +0500, Shehryar Ahmad wrote:
> The ADIS16203 is merged with mainline ADIS16201 in previous patch of
> series, remove the staging copy and its build system entries.

This is the ideal patch!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/3] dt-bindings: iio: accel: adi,adis16201: add adis16203 compatible

[Shehryar Ahmad]

The ADIS16203 has the same external interface as the already documented
ADIS16201/ADIS16209, for the properties covered by this binding
(compatible/reg, the DIO-based interrupt pin, and VDD supply), verified
by datasheets Table 5, Pin Function Descriptions, in the respective
Analog Devices datasheets. Some Auxiliary pins differ (e.g. adis16209
marks a few pins DNC where adis16201/adis16203 use them as AUX COM), but
these are not represented in this binding.  Add adis16203 to the
existing enum rather than creating a separate file.

Signed-off-by: Shehryar Ahmad <shehryar.amd@gmail.com>
---
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Previously reviewed at:
https://lore.kernel.org/all/20260812043506.456ced6d@jic23-huawei/

 .../devicetree/bindings/iio/accel/adi,adis16201.yaml        | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
index b6ba7ad1a..3ce5fb1be 100644
--- a/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
@@ -4,21 +4,23 @@
 $id: http://devicetree.org/schemas/iio/accel/adi,adis16201.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: ADIS16201 Dual Axis Inclinometer and similar
+title: ADIS16201 Inclinometer and similar
 
 maintainers:
   - Jonathan Cameron <Jonathan.Cameron@huawei.com>
 
 description: |
-  Two similar parts from external interface point of view.
+  Three similar parts from external interface point of view.
   SPI interface.
     https://www.analog.com/en/products/adis16201.html
+    https://www.analog.com/en/products/adis16203.html
     https://www.analog.com/en/products/adis16209.html
 
 properties:
   compatible:
     enum:
       - adi,adis16201
+      - adi,adis16203
       - adi,adis16209
 
   reg:
-- 
2.43.0

Re: [PATCH 3/3] dt-bindings: iio: accel: adi,adis16201: add adis16203 compatible

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2240 bytes --]

On Mon, Aug 31, 2026 at 11:42:01PM +0500, Shehryar Ahmad wrote:
> The ADIS16203 has the same external interface as the already documented
> ADIS16201/ADIS16209, for the properties covered by this binding
> (compatible/reg, the DIO-based interrupt pin, and VDD supply), verified
> by datasheets Table 5, Pin Function Descriptions, in the respective
> Analog Devices datasheets. Some Auxiliary pins differ (e.g. adis16209
> marks a few pins DNC where adis16201/adis16203 use them as AUX COM), but
> these are not represented in this binding.  Add adis16203 to the
> existing enum rather than creating a separate file.
> 
> Signed-off-by: Shehryar Ahmad <shehryar.amd@gmail.com>
> ---
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Why is this below the --- line (IOW it won't appear when the patch is
applied)?

> 
> Previously reviewed at:
> https://lore.kernel.org/all/20260812043506.456ced6d@jic23-huawei/
> 
>  .../devicetree/bindings/iio/accel/adi,adis16201.yaml        | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
> index b6ba7ad1a..3ce5fb1be 100644
> --- a/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml
> @@ -4,21 +4,23 @@
>  $id: http://devicetree.org/schemas/iio/accel/adi,adis16201.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: ADIS16201 Dual Axis Inclinometer and similar
> +title: ADIS16201 Inclinometer and similar
>  
>  maintainers:
>    - Jonathan Cameron <Jonathan.Cameron@huawei.com>
>  
>  description: |
> -  Two similar parts from external interface point of view.
> +  Three similar parts from external interface point of view.
>    SPI interface.
>      https://www.analog.com/en/products/adis16201.html
> +    https://www.analog.com/en/products/adis16203.html
>      https://www.analog.com/en/products/adis16209.html
>  
>  properties:
>    compatible:
>      enum:
>        - adi,adis16201
> +      - adi,adis16203
>        - adi,adis16209
>  
>    reg:
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/3] iio: accel: merge adis16203 into mainline adis16201 and remove from staging

[Andy Shevchenko]

On Mon, Aug 31, 2026 at 11:41:58PM +0500, Shehryar Ahmad wrote:
> This series merges the adis16203 staging driver into the mainline
> adis16201 driver and removes staging copy. Following discussion at [1].
> 
> Register addresses are identical between both parts (SUPPLY_OUT,
> AUX_ADC, TEMP_OUT, inclination regs all match). Pin config is identical
> too, all 16 pins, same names/types (Table 5, both datasheets). SPI
> protocol is identical as well (Figure 3, both datasheets, Phase = 1,
> Polarity = 1). adis16203's second (180-degree) inclination channel is
> dropped, since userspace can trivially compute it from the first channel
> anyway.[2]
> 
> Differences between chips selected per-device via a new struct
> differ_info, picked in probe() off of_device_id match, same pattern as
> adis16475.c, similar thing in adis16480 but different mechanism.
> 
> Don't have hardware for either part, only tested locally with KUnit that
> the sign_extend32 and SCALE/OFFSET paths of real read_raw behave
> correctly for both chips. Register read/write and probe aren't tested.
> 
> Patch 3's binding content is unchanged from [3].
> 
> If this isn't the right direction, guidance would be appreciated.
> 
> Link: https://lore.kernel.org/all/20250308144239.0442f1a7@jic23-huawei/
> Link: https://lore.kernel.org/linux-iio/20180204163124.67234ae5@archlinux/
> Link: https://lore.kernel.org/all/20260812043506.456ced6d@jic23-huawei/

Yeah, in the cover letter it's better to avoid Link tags unless you want
them to be present in the each of the patches (here I believe it's not
the case). Also you forgot the reference numbers. I assume you wanted
this:

 [1] https://lore.kernel.org/all/20250308144239.0442f1a7@jic23-huawei/
 [2] https://lore.kernel.org/linux-iio/20180204163124.67234ae5@archlinux/
 [3] https://lore.kernel.org/all/20260812043506.456ced6d@jic23-huawei/

-- 
With Best Regards,
Andy Shevchenko