Skip to contentSkip to conversation

staging: iio: accel: adis16203 Inclinometer cleanup and graduation

PATCH Superseded Jonathan Cameron 5 participants 18 messages Jan 23, 2023 – Feb 1, 2023

Subsystems (5)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jonathan Cameron

author maintainer
#0 lore raw

cover staging: iio: accel: adis16203 Inclinometer cleanup and graduation

From: Jonathan Cameron <[email protected]>

Whilst taking a look at what we still have in staging, I noticed that this driver was (probably) only still there due to the need to define some ABI for the odd 180 degree offset channel (same as the other channel minus 180 degrees).

Rather than leaving the driver in staging for ever, let us just drop that channel and move on. The rest of the series is trivial driver cleanup.

Note this is only build tested, so would be great if people can take a good look. If anyone happens to have one of these that would be even better (they are still marked for production use more than 12 years after the driver was submitted).

There are lots of features we could support if anyone wanted to, but it should be functional for basic channel reading without the bells and whistles. I've tried to incorporate the relevant device tree bindings however (multiple interrupt lines, supplies and reset gpio).

Cc: Barry Song <[email protected]>
(mostly for comedy value, not expecting you to remember the driver :)

Jonathan Cameron (12):
staging: iio: accel: adis16203: More conventional header ordering
staging: iio: accel: adis16203: Drop excessive register comments
staging: iio: accel: adis16203: Rename incli channel registers
staging: iio: accel: adis16203: Drop the 180 degree offset channel
staging: iio: accel: adis16203: Simplify handling of calibbias
staging: iio: accel: adis16203: Drop driver name define
staging: iio: accel: adis16203: Make units explicit in name of delay
define
staging: iio: accel: adis16203: Trivial whitespace cleanup
staging: iio: accel: adis16203: Drop setting drvdata as nothing gets
it
staging: iio: accel: adis16203: Trivial style cleanup
staging: iio: accel: adis16203: Move out of staging
dt-bindings: iio: accel: Add ADIS16203 Inclinometer

 .../bindings/iio/accel/adi,adis16203.yaml     |  59 +++++
 drivers/iio/accel/Kconfig                     |  12 +
 drivers/iio/accel/Makefile                    |   1 +
 drivers/{staging => }/iio/accel/adis16203.c   | 212 ++++++------------
 drivers/staging/iio/accel/Kconfig             |  12 -
 drivers/staging/iio/accel/Makefile            |   1 -
 6 files changed, 136 insertions(+), 161 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
 rename drivers/{staging => }/iio/accel/adis16203.c (51%)
Signature · 1 line
2.39.1
author maintainer
#1 lore raw

PATCH 1/12 staging: iio: accel: adis16203: More conventional header ordering

From: Jonathan Cameron <[email protected]>

A common practice in IIO drivers it to have the headers in alphabetical order with the exception of the more specific IIO headers which come in their own block at the end.

---
 drivers/staging/iio/accel/adis16203.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/staging/iio/accel/adis16203.c +3 -4
index c0e4c9266b5f..dd02ceb00d40 100644
@@ -6,14 +6,13 @@
  */
 
 #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>
 
+#include <linux/iio/iio.h>
+#include <linux/iio/imu/adis.h>
+
 #define ADIS16203_STARTUP_DELAY 220 /* ms */
 
 /* Flash memory write count */
Signature · 1 line
2.39.1
author maintainer
#2 lore raw

PATCH 2/12 staging: iio: accel: adis16203: Drop excessive register comments

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

A lot of these comments added very little that was not obvious from the register/field name. In one case the polarity was the opposite of what would be expected so rename that field (currently unused in the driver)

---
 drivers/staging/iio/accel/adis16203.c | 71 +--------------------------
 1 file changed, 1 insertion(+), 70 deletions(-)
drivers/staging/iio/accel/adis16203.c +1 -70
index dd02ceb00d40..60117aaf56d4 100644
@@ -15,111 +15,42 @@
 
 #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_PWRUP_SELF_TEST_DIS  BIT(10)
 #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)
Signature · 1 line
2.39.1
author maintainer
#3 lore raw

PATCH 3/12 staging: iio: accel: adis16203: Rename incli channel registers

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

This device has two such channels that are effectively reading the same thing, just with a 180 degree offset. First rename them (later we will drop one of them as it is easier to apply an offset in software).

---
 drivers/staging/iio/accel/adis16203.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/iio/accel/adis16203.c +4 -4
index 60117aaf56d4..8e5d1d2ea8e9 100644
@@ -19,8 +19,8 @@
 #define ADIS16203_SUPPLY_OUT     0x02
 #define ADIS16203_AUX_ADC        0x08
 #define ADIS16203_TEMP_OUT       0x0A
-#define ADIS16203_XINCL_OUT      0x0C
-#define ADIS16203_YINCL_OUT      0x0E
+#define ADIS16203_INCL_OUT       0x0C
+#define ADIS16203_INCL_OUT_180   0x0E
 #define ADIS16203_INCL_NULL      0x18
 #define ADIS16203_ALM_MAG1       0x20
 #define ADIS16203_ALM_MAG2       0x22
@@ -138,10 +138,10 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
 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,
+	ADIS_INCLI_CHAN(X, ADIS16203_INCL_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,
+	ADIS_INCLI_CHAN(Y, ADIS16203_INCL_OUT_180, ADIS16203_SCAN_INCLI_Y,
 			0, 0, 14),
 	ADIS_TEMP_CHAN(ADIS16203_TEMP_OUT, ADIS16203_SCAN_TEMP, 0, 12),
 	IIO_CHAN_SOFT_TIMESTAMP(5),
Signature · 1 line
2.39.1
author maintainer
#4 lore raw

PATCH 4/12 staging: iio: accel: adis16203: Drop the 180 degree offset channel

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

There is no standard ABI suitable for this channel, which is blocking the graduation of this driver from staging. It provides no obvious benefit over applying a -180 degree offset in software. So drop the channel.

---
 drivers/staging/iio/accel/adis16203.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/staging/iio/accel/adis16203.c +1 -5
index 8e5d1d2ea8e9..b59a003f3e26 100644
@@ -59,7 +59,6 @@
 
 enum adis16203_scan {
 	 ADIS16203_SCAN_INCLI_X,
-	 ADIS16203_SCAN_INCLI_Y,
 	 ADIS16203_SCAN_SUPPLY,
 	 ADIS16203_SCAN_AUX_ADC,
 	 ADIS16203_SCAN_TEMP,
@@ -140,11 +139,8 @@ static const struct iio_chan_spec adis16203_channels[] = {
 	ADIS_AUX_ADC_CHAN(ADIS16203_AUX_ADC, ADIS16203_SCAN_AUX_ADC, 0, 12),
 	ADIS_INCLI_CHAN(X, ADIS16203_INCL_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_INCL_OUT_180, ADIS16203_SCAN_INCLI_Y,
-			0, 0, 14),
 	ADIS_TEMP_CHAN(ADIS16203_TEMP_OUT, ADIS16203_SCAN_TEMP, 0, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(5),
+	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
 static const struct iio_info adis16203_info = {
Signature · 1 line
2.39.1
author maintainer
#5 lore raw

PATCH 5/12 staging: iio: accel: adis16203: Simplify handling of calibbias

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Dropping the indirection on address lookup as there was only one address enables several other minor cleanups.

---
 drivers/staging/iio/accel/adis16203.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/staging/iio/accel/adis16203.c +10 -10
index b59a003f3e26..67c0340d7097 100644
@@ -5,6 +5,7 @@
  * Copyright 2010 Analog Devices Inc.
  */
 
+#include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -57,6 +58,8 @@
 
 #define ADIS16203_ERROR_ACTIVE          BIT(14)
 
+#define ADIS16203_INCLI_NULL_MSK     GENMASK(13, 0)
+
 enum adis16203_scan {
 	 ADIS16203_SCAN_INCLI_X,
 	 ADIS16203_SCAN_SUPPLY,
@@ -66,10 +69,6 @@ enum adis16203_scan {
 
 #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,
@@ -77,10 +76,12 @@ static int adis16203_write_raw(struct iio_dev *indio_dev,
 			       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);
+	if (mask != IIO_CHAN_INFO_CALIBBIAS)
+		return -EINVAL;
+
+	return adis_write_reg_16(st, ADIS16203_INCL_NULL,
+				 FIELD_PREP(ADIS16203_INCLI_NULL_MSK, val));
 }
 
 static int adis16203_read_raw(struct iio_dev *indio_dev,
@@ -90,7 +91,7 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
 {
 	struct adis *st = iio_priv(indio_dev);
 	int ret;
-	u8 addr;
+
 	s16 val16;
 
 	switch (mask) {
@@ -123,8 +124,7 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
 		*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);
+		ret = adis_read_reg_16(st, ADIS16203_INCL_NULL, &val16);
 		if (ret)
 			return ret;
 		*val = sign_extend32(val16, 13);
Signature · 1 line
2.39.1
author maintainer
#6 lore raw

PATCH 6/12 staging: iio: accel: adis16203: Drop driver name define

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

These defines just make it harder to see what strings are set for which driver structure fields. Much better to just have the string inline. In this case it wasn't even used.

---
 drivers/staging/iio/accel/adis16203.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/staging/iio/accel/adis16203.c +1 -3
index 67c0340d7097..25342717539b 100644
@@ -67,8 +67,6 @@ enum adis16203_scan {
 	 ADIS16203_SCAN_TEMP,
 };
 
-#define DRIVER_NAME		"adis16203"
-
 static int adis16203_write_raw(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int val,
@@ -196,7 +194,7 @@ static int adis16203_probe(struct spi_device *spi)
 	/* this is only used for removal purposes */
 	spi_set_drvdata(spi, indio_dev);
 
-	indio_dev->name = spi->dev.driver->name;
+	indio_dev->name = "adis16203";
 	indio_dev->channels = adis16203_channels;
 	indio_dev->num_channels = ARRAY_SIZE(adis16203_channels);
 	indio_dev->info = &adis16203_info;
Signature · 1 line
2.39.1
author maintainer
#7 lore raw

PATCH 7/12 staging: iio: accel: adis16203: Make units explicit in name of delay define

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

A postfix of _MS avoids the need for a comment that might bit rot.

---
 drivers/staging/iio/accel/adis16203.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/iio/accel/adis16203.c +4 -4
index 25342717539b..271e560c41c0 100644
@@ -14,7 +14,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/imu/adis.h>
 
-#define ADIS16203_STARTUP_DELAY 220 /* ms */
+#define ADIS16203_STARTUP_DELAY_MS		220
 
 #define ADIS16203_FLASH_CNT      0x00
 #define ADIS16203_SUPPLY_OUT     0x02
@@ -156,9 +156,9 @@ static const char * const adis16203_status_error_msgs[] = {
 };
 
 static const struct adis_timeout adis16203_timeouts = {
-	.reset_ms = ADIS16203_STARTUP_DELAY,
-	.sw_reset_ms = ADIS16203_STARTUP_DELAY,
-	.self_test_ms = ADIS16203_STARTUP_DELAY
+	.reset_ms = ADIS16203_STARTUP_DELAY_MS,
+	.sw_reset_ms = ADIS16203_STARTUP_DELAY_MS,
+	.self_test_ms = ADIS16203_STARTUP_DELAY_MS
 };
 
 static const struct adis_data adis16203_data = {
Signature · 1 line
2.39.1
author maintainer
#8 lore raw

PATCH 8/12 staging: iio: accel: adis16203: Trivial whitespace cleanup

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Identation etc in this driver was inconsistent. Fix that.

---
 drivers/staging/iio/accel/adis16203.c | 97 +++++++++++++--------------
 1 file changed, 46 insertions(+), 51 deletions(-)
drivers/staging/iio/accel/adis16203.c +46 -51
index 271e560c41c0..20e9c9eb9436 100644
@@ -16,49 +16,49 @@
 
 #define ADIS16203_STARTUP_DELAY_MS		220
 
-#define ADIS16203_FLASH_CNT      0x00
-#define ADIS16203_SUPPLY_OUT     0x02
-#define ADIS16203_AUX_ADC        0x08
-#define ADIS16203_TEMP_OUT       0x0A
-#define ADIS16203_INCL_OUT       0x0C
-#define ADIS16203_INCL_OUT_180   0x0E
-#define ADIS16203_INCL_NULL      0x18
-#define ADIS16203_ALM_MAG1       0x20
-#define ADIS16203_ALM_MAG2       0x22
-#define ADIS16203_ALM_SMPL1      0x24
-#define ADIS16203_ALM_SMPL2      0x26
-#define ADIS16203_ALM_CTRL       0x28
-#define ADIS16203_AUX_DAC        0x30
-#define ADIS16203_GPIO_CTRL      0x32
-#define ADIS16203_MSC_CTRL       0x34
-#define ADIS16203_SMPL_PRD       0x36
-#define ADIS16203_AVG_CNT        0x38
-#define ADIS16203_SLP_CNT        0x3A
-#define ADIS16203_DIAG_STAT      0x3C
-#define ADIS16203_GLOB_CMD       0x3E
-
-#define ADIS16203_MSC_CTRL_PWRUP_SELF_TEST_DIS  BIT(10)
-#define ADIS16203_MSC_CTRL_REVERSE_ROT_EN       BIT(9)
-#define ADIS16203_MSC_CTRL_SELF_TEST_EN         BIT(8)
-#define ADIS16203_MSC_CTRL_DATA_RDY_EN          BIT(2)
-#define ADIS16203_MSC_CTRL_ACTIVE_HIGH          BIT(1)
-#define ADIS16203_MSC_CTRL_DATA_RDY_DIO1        BIT(0)
-
-#define ADIS16203_DIAG_STAT_ALARM2        BIT(9)
-#define ADIS16203_DIAG_STAT_ALARM1        BIT(8)
-#define ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT 5
-#define ADIS16203_DIAG_STAT_SPI_FAIL_BIT      3
-#define ADIS16203_DIAG_STAT_FLASH_UPT_BIT     2
-#define ADIS16203_DIAG_STAT_POWER_HIGH_BIT    1
-#define ADIS16203_DIAG_STAT_POWER_LOW_BIT     0
-
-#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)
-
-#define ADIS16203_INCLI_NULL_MSK     GENMASK(13, 0)
+#define ADIS16203_FLASH_CNT			0x00
+#define ADIS16203_SUPPLY_OUT			0x02
+#define ADIS16203_AUX_ADC			0x08
+#define ADIS16203_TEMP_OUT			0x0A
+#define ADIS16203_INCL_OUT			0x0C
+#define ADIS16203_INCL_OUT_180			0x0E
+#define ADIS16203_INCL_NULL			0x18
+#define ADIS16203_ALM_MAG1			0x20
+#define ADIS16203_ALM_MAG2			0x22
+#define ADIS16203_ALM_SMPL1			0x24
+#define ADIS16203_ALM_SMPL2			0x26
+#define ADIS16203_ALM_CTRL			0x28
+#define ADIS16203_AUX_DAC			0x30
+#define ADIS16203_GPIO_CTRL			0x32
+#define ADIS16203_MSC_CTRL			0x34
+#define ADIS16203_SMPL_PRD			0x36
+#define ADIS16203_AVG_CNT			0x38
+#define ADIS16203_SLP_CNT			0x3A
+#define ADIS16203_DIAG_STAT			0x3C
+#define ADIS16203_GLOB_CMD			0x3E
+
+#define ADIS16203_MSC_CTRL_PWRUP_SELF_TEST_DIS	BIT(10)
+#define ADIS16203_MSC_CTRL_REVERSE_ROT_EN	BIT(9)
+#define ADIS16203_MSC_CTRL_SELF_TEST_EN		BIT(8)
+#define ADIS16203_MSC_CTRL_DATA_RDY_EN		BIT(2)
+#define ADIS16203_MSC_CTRL_ACTIVE_HIGH		BIT(1)
+#define ADIS16203_MSC_CTRL_DATA_RDY_DIO1	BIT(0)
+
+#define ADIS16203_DIAG_STAT_ALARM2		BIT(9)
+#define ADIS16203_DIAG_STAT_ALARM1		BIT(8)
+#define ADIS16203_DIAG_STAT_SELFTEST_FAIL_BIT	5
+#define ADIS16203_DIAG_STAT_SPI_FAIL_BIT	3
+#define ADIS16203_DIAG_STAT_FLASH_UPT_BIT	2
+#define ADIS16203_DIAG_STAT_POWER_HIGH_BIT	1
+#define ADIS16203_DIAG_STAT_POWER_LOW_BIT	0
+
+#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)
+
+#define ADIS16203_INCLI_NULL_MSK		GENMASK(13, 0)
 
 enum adis16203_scan {
 	 ADIS16203_SCAN_INCLI_X,
@@ -69,9 +69,7 @@ enum adis16203_scan {
 
 static int adis16203_write_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);
 
@@ -84,18 +82,16 @@ static int adis16203_write_raw(struct iio_dev *indio_dev,
 
 static int adis16203_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);
 	int ret;
-
 	s16 val16;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		return adis_single_conversion(indio_dev, chan,
-				ADIS16203_ERROR_ACTIVE, val);
+					      ADIS16203_ERROR_ACTIVE, val);
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
 		case IIO_VOLTAGE:
@@ -220,7 +216,6 @@ 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 = {
Signature · 1 line
2.39.1
author maintainer
#9 lore raw

PATCH 9/12 staging: iio: accel: adis16203: Drop setting drvdata as nothing gets it

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

As commented, it is only useful for .remove() which doesn't exist, so drop setting it and the comment.

---
 drivers/staging/iio/accel/adis16203.c | 2 --
 1 file changed, 2 deletions(-)
drivers/staging/iio/accel/adis16203.c +0 -2
index 20e9c9eb9436..9915b73454d2 100644
@@ -187,8 +187,6 @@ static int adis16203_probe(struct spi_device *spi)
 	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 = "adis16203";
 	indio_dev->channels = adis16203_channels;
Signature · 1 line
2.39.1
author maintainer
#10 lore raw

PATCH 10/12 staging: iio: accel: adis16203: Trivial style cleanup

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

Drop a training comma after a null terminator and a comment that says nothing beyond the obvious.

---
 drivers/staging/iio/accel/adis16203.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/accel/adis16203.c +1 -2
index 9915b73454d2..1aaf9815aa66 100644
@@ -182,7 +182,6 @@ static int adis16203_probe(struct spi_device *spi)
 	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;
@@ -212,7 +211,7 @@ static int adis16203_probe(struct spi_device *spi)
 
 static const struct of_device_id adis16203_of_match[] = {
 	{ .compatible = "adi,adis16203" },
-	{ },
+	{ }
 };
 MODULE_DEVICE_TABLE(of, adis16203_of_match);
 
Signature · 1 line
2.39.1
author maintainer
#11 lore raw

PATCH 11/12 staging: iio: accel: adis16203: Move out of staging

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

The main blocker of this driver moving previously was non standard ABI for the 180 degree offset channel. Now support for that channel has been dropped, this simple driver can move out of staging.

---
 drivers/iio/accel/Kconfig                   | 12 ++++++++++++
 drivers/iio/accel/Makefile                  |  1 +
 drivers/{staging => }/iio/accel/adis16203.c |  0
 drivers/staging/iio/accel/Kconfig           | 12 ------------
 drivers/staging/iio/accel/Makefile          |  1 -
 5 files changed, 13 insertions(+), 13 deletions(-)
drivers/iio/accel/Kconfig +12 -0
index b6b45d359f28..88d4b18bd5e8 100644
@@ -18,6 +18,18 @@ config ADIS16201
 	  To compile this driver as a module, say M here: the module will
 	  be called adis16201.
 
+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.
+
 config ADIS16209
 	tristate "Analog Devices ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer"
 	depends on SPI
drivers/iio/accel/Makefile +1 -0
index 311ead9c3ef1..b6f0cee52659 100644
@@ -5,6 +5,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ADIS16201) += adis16201.o
+obj-$(CONFIG_ADIS16203) += adis16203.o
 obj-$(CONFIG_ADIS16209) += adis16209.o
 obj-$(CONFIG_ADXL313) += adxl313_core.o
 obj-$(CONFIG_ADXL313_I2C) += adxl313_i2c.o
drivers/iio/accel/adis16203.c renamed from drivers/staging/iio/accel/adis16203.c +0 -0
similarity index 100%
rename from drivers/staging/iio/accel/adis16203.c
rename to drivers/iio/accel/adis16203.c
drivers/staging/iio/accel/Kconfig +0 -12
index 3318997a7009..3769af8719f3 100644
@@ -4,18 +4,6 @@
 #
 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.
-
 config ADIS16240
 	tristate "Analog Devices ADIS16240 Programmable Impact Sensor and Recorder"
 	depends on SPI
drivers/staging/iio/accel/Makefile +0 -1
index 094cc9be35bd..b0beec471814 100644
@@ -3,5 +3,4 @@
 # Makefile for industrial I/O accelerometer drivers
 #
 
-obj-$(CONFIG_ADIS16203) += adis16203.o
 obj-$(CONFIG_ADIS16240) += adis16240.o
Signature · 1 line
2.39.1
#12 lore raw

Re: [PATCH 11/12] staging: iio: accel: adis16203: Move out of staging

Quoted 7 lines · On Mon, 23 Jan 2023 21:17:57 +0000 Jonathan Cameron <[email protected]> wrote:

From: Jonathan Cameron <[email protected]>

The main blocker of this driver moving previously was non standard ABI for the 180 degree offset channel. Now support for that channel has been dropped, this simple driver can move out of staging.

I forgot my own rules of posting graduations with move detection turned off. I'm going to be doing a v2 anyway to fix the issues Rob's bot and Krysztof pointed out in the binding so will hopefully remember to turn move detection off for that.

Quoted 80 lines
---
 drivers/iio/accel/Kconfig                   | 12 ++++++++++++
 drivers/iio/accel/Makefile                  |  1 +
 drivers/{staging => }/iio/accel/adis16203.c |  0
 drivers/staging/iio/accel/Kconfig           | 12 ------------
 drivers/staging/iio/accel/Makefile          |  1 -
 5 files changed, 13 insertions(+), 13 deletions(-)
drivers/iio/accel/Kconfig +12 -0
index b6b45d359f28..88d4b18bd5e8 100644
@@ -18,6 +18,18 @@ config ADIS16201
 	  To compile this driver as a module, say M here: the module will
 	  be called adis16201.
 
+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.
+
 config ADIS16209
 	tristate "Analog Devices ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer"
 	depends on SPI
drivers/iio/accel/Makefile +1 -0
index 311ead9c3ef1..b6f0cee52659 100644
@@ -5,6 +5,7 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ADIS16201) += adis16201.o
+obj-$(CONFIG_ADIS16203) += adis16203.o
 obj-$(CONFIG_ADIS16209) += adis16209.o
 obj-$(CONFIG_ADXL313) += adxl313_core.o
 obj-$(CONFIG_ADXL313_I2C) += adxl313_i2c.o
drivers/iio/accel/adis16203.c renamed from drivers/staging/iio/accel/adis16203.c +0 -0
similarity index 100%
rename from drivers/staging/iio/accel/adis16203.c
rename to drivers/iio/accel/adis16203.c
drivers/staging/iio/accel/Kconfig +0 -12
index 3318997a7009..3769af8719f3 100644
@@ -4,18 +4,6 @@
 #
 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.
-
 config ADIS16240
 	tristate "Analog Devices ADIS16240 Programmable Impact Sensor and Recorder"
 	depends on SPI
drivers/staging/iio/accel/Makefile +0 -1
index 094cc9be35bd..b0beec471814 100644
@@ -3,5 +3,4 @@
 # Makefile for industrial I/O accelerometer drivers
 #
 
-obj-$(CONFIG_ADIS16203) += adis16203.o
 obj-$(CONFIG_ADIS16240) += adis16240.o
author maintainer
#13 lore raw

PATCH 12/12 dt-bindings: iio: accel: Add ADIS16203 Inclinometer

In reply to Jonathan Cameron

From: Jonathan Cameron <[email protected]>

There has been a driver in staging for quite a while. Given we are now moving it to the main tree, time to make sure it has binding documentation.

---
 .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +59 -0
new file mode 100644
index 000000000000..05c095247e10
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADIS16203 Programmable 360 degree inclinometer
+
+maintainers:
+  - Jonathan Cameron <[email protected]>
+
+properties:
+  compatible:
+    const: adi,adis16203
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    description:
+      Device has two configurable outputs, both of which may be used
+      as interrupt sources.
+    enum:
+      - dio0
+      - dio1
+
+  reset-gpios: true
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        accelerometer@0 {
+            compatible = "adi,adis16201";
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dio0";
+        };
+    };
+...
+
Signature · 1 line
2.39.1
maintainer
#14 lore raw

Re: [PATCH 12/12] dt-bindings: iio: accel: Add ADIS16203 Inclinometer

Quoted 11 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

There has been a driver in staging for quite a while. Given we are now moving it to the main tree, time to make sure it has binding documentation.

---
 .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/accel/adi,adis16203.example.dtb: accelerometer@0: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

#15 lore raw

Re: [PATCH 12/12] dt-bindings: iio: accel: Add ADIS16203 Inclinometer

In reply to Jonathan Cameron
Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

There has been a driver in staging for quite a while. Given we are now moving it to the main tree, time to make sure it has binding documentation.

---
 .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +12 -0
new file mode 100644
index 000000000000..05c095247e10
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADIS16203 Programmable 360 degree inclinometer
+
+maintainers:
+  - Jonathan Cameron <[email protected]>
+
+properties:

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 22

Quoted 10 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +10 -0
+  compatible:
+    const: adi,adis16203
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:

maxItems (and probably minItems)

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 26

Quoted 4 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+    description:
+      Device has two configurable outputs, both of which may be used
+      as interrupt sources.
+    enum:

This won't work. It's an list. You need:
items:
enum:

     .....

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 30

Quoted 4 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+      - dio0
+      - dio1
+
+  reset-gpios: true

maxItems: 1

Quoted 15 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +15 -0
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 55

Quoted 10 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +10 -0
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        accelerometer@0 {
+            compatible = "adi,adis16201";
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dio0";

You require here two items, according to interrupts. Don't you miss minItems?

Also... if you tested the binding it would complain here.

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 59

Quoted 4 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+        };
+    };
+...
+

Best regards,
Krzysztof

#16 lore raw

Re: [PATCH 12/12] dt-bindings: iio: accel: Add ADIS16203 Inclinometer

Quoted 48 lines · On Tue, 24 Jan 2023 09:39:10 +0100 Krzysztof Kozlowski…
Quoted 29 lines · Jonathan Cameron

From: Jonathan Cameron <[email protected]>

There has been a driver in staging for quite a while. Given we are now moving it to the main tree, time to make sure it has binding documentation.

---
 .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +12 -0
new file mode 100644
index 000000000000..05c095247e10
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADIS16203 Programmable 360 degree inclinometer
+
+maintainers:
+  - Jonathan Cameron <[email protected]>
+
+properties:

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 22

Quoted 10 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +10 -0
+  compatible:
+    const: adi,adis16203
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:  

maxItems (and probably minItems

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 26

Quoted 4 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+    description:
+      Device has two configurable outputs, both of which may be used
+      as interrupt sources.
+    enum:  
Quoted 4 lines

This won't work. It's an list. You need:
items:
enum:

     .....

Gah. I'm still rubbish at writing these things. I guess I just don't do them often enough.

Quoted 29 lines

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 30

Quoted 5 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+      - dio0
+      - dio1
+
+  reset-gpios: true  

maxItems: 1

Quoted 20 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +20 -0
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        accelerometer@0 {
+            compatible = "adi,adis16201";
Quoted 8 lines

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 55

Quoted 5 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +5 -0
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dio0";  

You require here two items, according to interrupts. Don't you miss minItems?

Indeed.

Quoted 2 lines

Also... if you tested the binding it would complain here.

I thought I had tested it (and fixed a few issues that showed up). I guess something went wrong with with the final test. Will investigate.

Thanks and sorry for the waste of time with the silly errors!

Jonathan

Quoted 9 lines

Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml line 59

Quoted 5 lines
Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml new file +4 -0
+        };
+    };
+...
+  

Best regards,
Krzysztof

#17 lore raw

Re: [PATCH 12/12] dt-bindings: iio: accel: Add ADIS16203 Inclinometer

Quoted 7 lines · Jonathan Cameron
Quoted 4 lines

This won't work. It's an list. You need:
items:
enum:

     .....

Gah. I'm still rubbish at writing these things. I guess I just don't do them often enough.

No the dtschema YAML is just really hard. Don't beat yourself up about it.

Yours,
Linus Walleij