Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically
@ 2020-02-10 13:25 Alexandru Ardelean
  2020-02-10 13:25 ` [PATCH v2 2/9] iio: imu: adis16400: " Alexandru Ardelean
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:25 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
structure"). It removes the memory allocation and moves the 'adis_data'
information to be static on the chip_info struct.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
* initialize 'adis_data' statically on adis16480,adis16136 & adis16400
* split 'iio: imu: adis: Refactor adis_initial_startup' patch
  - new: 'iio: imu: adis: add unlocked __adis_initial_startup()'
  - new: 'iio: imu: adis: add support product ID check in adis_initial_startup'
  - modified: 'iio: imu: adis: Refactor adis_initial_startup'
* added 'prod_id' field together with 'prod_id_reg' on 'adis_data'
  each device that wants to use the 'prod_id_reg' must also provide an
  expected 'prod_id' value

 drivers/iio/imu/adis16480.c | 140 ++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 78 deletions(-)

diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index dac87f1001fd..4c4de1b62769 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -138,7 +138,7 @@ struct adis16480_chip_info {
 	unsigned int max_dec_rate;
 	const unsigned int *filter_freqs;
 	bool has_pps_clk_mode;
-	const struct adis_timeout *timeouts;
+	const struct adis_data adis_data;
 };
 
 enum adis16480_int_pin {
@@ -796,6 +796,55 @@ enum adis16480_variant {
 	ADIS16497_3,
 };
 
+#define ADIS16480_DIAG_STAT_XGYRO_FAIL 0
+#define ADIS16480_DIAG_STAT_YGYRO_FAIL 1
+#define ADIS16480_DIAG_STAT_ZGYRO_FAIL 2
+#define ADIS16480_DIAG_STAT_XACCL_FAIL 3
+#define ADIS16480_DIAG_STAT_YACCL_FAIL 4
+#define ADIS16480_DIAG_STAT_ZACCL_FAIL 5
+#define ADIS16480_DIAG_STAT_XMAGN_FAIL 8
+#define ADIS16480_DIAG_STAT_YMAGN_FAIL 9
+#define ADIS16480_DIAG_STAT_ZMAGN_FAIL 10
+#define ADIS16480_DIAG_STAT_BARO_FAIL 11
+
+static const char * const adis16480_status_error_msgs[] = {
+	[ADIS16480_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
+	[ADIS16480_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
+	[ADIS16480_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
+	[ADIS16480_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
+	[ADIS16480_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
+	[ADIS16480_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
+	[ADIS16480_DIAG_STAT_XMAGN_FAIL] = "X-axis magnetometer self-test failure",
+	[ADIS16480_DIAG_STAT_YMAGN_FAIL] = "Y-axis magnetometer self-test failure",
+	[ADIS16480_DIAG_STAT_ZMAGN_FAIL] = "Z-axis magnetometer self-test failure",
+	[ADIS16480_DIAG_STAT_BARO_FAIL] = "Barometer self-test failure",
+};
+
+static int adis16480_enable_irq(struct adis *adis, bool enable);
+
+#define ADIS16480_DATA(_timeouts)					\
+{									\
+	.diag_stat_reg = ADIS16480_REG_DIAG_STS,			\
+	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,				\
+	.has_paging = true,						\
+	.read_delay = 5,						\
+	.write_delay = 5,						\
+	.self_test_mask = BIT(1),					\
+	.status_error_msgs = adis16480_status_error_msgs,		\
+	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |	\
+		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_ZGYRO_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_XACCL_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_YACCL_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_ZACCL_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_XMAGN_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_YMAGN_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_ZMAGN_FAIL) |			\
+		BIT(ADIS16480_DIAG_STAT_BARO_FAIL),			\
+	.enable_irq = adis16480_enable_irq,				\
+	.timeouts = (_timeouts),					\
+}
+
 static const struct adis_timeout adis16485_timeouts = {
 	.reset_ms = 560,
 	.sw_reset_ms = 120,
@@ -838,7 +887,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.timeouts = &adis16485_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
 	},
 	[ADIS16480] = {
 		.channels = adis16480_channels,
@@ -851,7 +900,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.timeouts = &adis16480_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16480_timeouts),
 	},
 	[ADIS16485] = {
 		.channels = adis16485_channels,
@@ -864,7 +913,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.timeouts = &adis16485_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
 	},
 	[ADIS16488] = {
 		.channels = adis16480_channels,
@@ -877,7 +926,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.timeouts = &adis16485_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
 	},
 	[ADIS16490] = {
 		.channels = adis16485_channels,
@@ -891,7 +940,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_timeouts),
 	},
 	[ADIS16495_1] = {
 		.channels = adis16485_channels,
@@ -905,7 +954,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 	[ADIS16495_2] = {
 		.channels = adis16485_channels,
@@ -919,7 +968,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 	[ADIS16495_3] = {
 		.channels = adis16485_channels,
@@ -933,7 +982,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 	[ADIS16497_1] = {
 		.channels = adis16485_channels,
@@ -947,7 +996,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 	[ADIS16497_2] = {
 		.channels = adis16485_channels,
@@ -961,7 +1010,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 	[ADIS16497_3] = {
 		.channels = adis16485_channels,
@@ -975,7 +1024,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.timeouts = &adis16495_1_timeouts,
+		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
 	},
 };
 
@@ -1048,53 +1097,6 @@ static int adis16480_initial_setup(struct iio_dev *indio_dev)
 	return 0;
 }
 
-#define ADIS16480_DIAG_STAT_XGYRO_FAIL 0
-#define ADIS16480_DIAG_STAT_YGYRO_FAIL 1
-#define ADIS16480_DIAG_STAT_ZGYRO_FAIL 2
-#define ADIS16480_DIAG_STAT_XACCL_FAIL 3
-#define ADIS16480_DIAG_STAT_YACCL_FAIL 4
-#define ADIS16480_DIAG_STAT_ZACCL_FAIL 5
-#define ADIS16480_DIAG_STAT_XMAGN_FAIL 8
-#define ADIS16480_DIAG_STAT_YMAGN_FAIL 9
-#define ADIS16480_DIAG_STAT_ZMAGN_FAIL 10
-#define ADIS16480_DIAG_STAT_BARO_FAIL 11
-
-static const char * const adis16480_status_error_msgs[] = {
-	[ADIS16480_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
-	[ADIS16480_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
-	[ADIS16480_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
-	[ADIS16480_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
-	[ADIS16480_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
-	[ADIS16480_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
-	[ADIS16480_DIAG_STAT_XMAGN_FAIL] = "X-axis magnetometer self-test failure",
-	[ADIS16480_DIAG_STAT_YMAGN_FAIL] = "Y-axis magnetometer self-test failure",
-	[ADIS16480_DIAG_STAT_ZMAGN_FAIL] = "Z-axis magnetometer self-test failure",
-	[ADIS16480_DIAG_STAT_BARO_FAIL] = "Barometer self-test failure",
-};
-
-static const struct adis_data adis16480_data = {
-	.diag_stat_reg = ADIS16480_REG_DIAG_STS,
-	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,
-	.has_paging = true,
-
-	.read_delay = 5,
-	.write_delay = 5,
-
-	.status_error_msgs = adis16480_status_error_msgs,
-	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_ZGYRO_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_XACCL_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_YACCL_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_ZACCL_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_XMAGN_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_YMAGN_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_ZMAGN_FAIL) |
-		BIT(ADIS16480_DIAG_STAT_BARO_FAIL),
-
-	.enable_irq = adis16480_enable_irq,
-};
-
 static int adis16480_config_irq_pin(struct device_node *of_node,
 				    struct adis16480 *st)
 {
@@ -1245,22 +1247,6 @@ static int adis16480_get_ext_clocks(struct adis16480 *st)
 	return 0;
 }
 
-static struct adis_data *adis16480_adis_data_alloc(struct adis16480 *st,
-						   struct device *dev)
-{
-	struct adis_data *data;
-
-	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
-	if (!data)
-		return ERR_PTR(-ENOMEM);
-
-	memcpy(data, &adis16480_data, sizeof(*data));
-
-	data->timeouts = st->chip_info->timeouts;
-
-	return data;
-}
-
 static int adis16480_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
@@ -1285,9 +1271,7 @@ static int adis16480_probe(struct spi_device *spi)
 	indio_dev->info = &adis16480_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	adis16480_data = adis16480_adis_data_alloc(st, &spi->dev);
-	if (IS_ERR(adis16480_data))
-		return PTR_ERR(adis16480_data);
+	adis16480_data = &st->chip_info->adis_data;
 
 	ret = adis_init(&st->adis, indio_dev, spi, adis16480_data);
 	if (ret)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 2/9] iio: imu: adis16400: initialize adis_data statically
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
@ 2020-02-10 13:25 ` " Alexandru Ardelean
  2020-02-14 14:29   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 3/9] iio: gyro: adis16136: " Alexandru Ardelean
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:25 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
structure"). It removes the memory allocation and moves the 'adis_data'
information to be static on the chip_info struct.

This also adds a timeout structure to ADIS16334, since it was initially
omitted. This was omitted (by accident) when the change was done.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis16400.c | 139 ++++++++++++++++--------------------
 1 file changed, 63 insertions(+), 76 deletions(-)

diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index cfb1c19eb930..1c0770e03ec9 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -156,7 +156,7 @@ struct adis16400_state;
 
 struct adis16400_chip_info {
 	const struct iio_chan_spec *channels;
-	const struct adis_timeout *timeouts;
+	const struct adis_data adis_data;
 	const int num_channels;
 	const long flags;
 	unsigned int gyro_scale_micro;
@@ -930,12 +930,63 @@ static const struct iio_chan_spec adis16334_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
+static const char * const adis16400_status_error_msgs[] = {
+	[ADIS16400_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
+	[ADIS16400_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
+	[ADIS16400_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
+	[ADIS16400_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
+	[ADIS16400_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
+	[ADIS16400_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
+	[ADIS16400_DIAG_STAT_ALARM2] = "Alarm 2 active",
+	[ADIS16400_DIAG_STAT_ALARM1] = "Alarm 1 active",
+	[ADIS16400_DIAG_STAT_FLASH_CHK] = "Flash checksum error",
+	[ADIS16400_DIAG_STAT_SELF_TEST] = "Self test error",
+	[ADIS16400_DIAG_STAT_OVERFLOW] = "Sensor overrange",
+	[ADIS16400_DIAG_STAT_SPI_FAIL] = "SPI failure",
+	[ADIS16400_DIAG_STAT_FLASH_UPT] = "Flash update failed",
+	[ADIS16400_DIAG_STAT_POWER_HIGH] = "Power supply above 5.25V",
+	[ADIS16400_DIAG_STAT_POWER_LOW] = "Power supply below 4.75V",
+};
+
+#define ADIS16400_DATA(_timeouts)					\
+{									\
+	.msc_ctrl_reg = ADIS16400_MSC_CTRL,				\
+	.glob_cmd_reg = ADIS16400_GLOB_CMD,				\
+	.diag_stat_reg = ADIS16400_DIAG_STAT,				\
+	.read_delay = 50,						\
+	.write_delay = 50,						\
+	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,			\
+	.status_error_msgs = adis16400_status_error_msgs,		\
+	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |	\
+		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_XACCL_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_XGYRO_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_YGYRO_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_ZGYRO_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_ALARM2) |			\
+		BIT(ADIS16400_DIAG_STAT_ALARM1) |			\
+		BIT(ADIS16400_DIAG_STAT_FLASH_CHK) |			\
+		BIT(ADIS16400_DIAG_STAT_SELF_TEST) |			\
+		BIT(ADIS16400_DIAG_STAT_OVERFLOW) |			\
+		BIT(ADIS16400_DIAG_STAT_SPI_FAIL) |			\
+		BIT(ADIS16400_DIAG_STAT_FLASH_UPT) |			\
+		BIT(ADIS16400_DIAG_STAT_POWER_HIGH) |			\
+		BIT(ADIS16400_DIAG_STAT_POWER_LOW),			\
+	.timeouts = (_timeouts),					\
+}
+
 static const struct adis_timeout adis16300_timeouts = {
 	.reset_ms = ADIS16400_STARTUP_DELAY,
 	.sw_reset_ms = ADIS16400_STARTUP_DELAY,
 	.self_test_ms = ADIS16400_STARTUP_DELAY,
 };
 
+static const struct adis_timeout adis16334_timeouts = {
+	.reset_ms = 60,
+	.sw_reset_ms = 60,
+	.self_test_ms = 14,
+};
+
 static const struct adis_timeout adis16362_timeouts = {
 	.reset_ms = 130,
 	.sw_reset_ms = 130,
@@ -972,7 +1023,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 140000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16300_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
 	},
 	[ADIS16334] = {
 		.channels = adis16334_channels,
@@ -985,6 +1036,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 67850, /* 25 C = 0x00 */
 		.set_freq = adis16334_set_freq,
 		.get_freq = adis16334_get_freq,
+		.adis_data = ADIS16400_DATA(&adis16334_timeouts),
 	},
 	[ADIS16350] = {
 		.channels = adis16350_channels,
@@ -996,7 +1048,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.flags = ADIS16400_NO_BURST | ADIS16400_HAS_SLOW_MODE,
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16300_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
 	},
 	[ADIS16360] = {
 		.channels = adis16350_channels,
@@ -1009,7 +1061,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16300_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
 	},
 	[ADIS16362] = {
 		.channels = adis16350_channels,
@@ -1022,7 +1074,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16362_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16362_timeouts),
 	},
 	[ADIS16364] = {
 		.channels = adis16350_channels,
@@ -1035,7 +1087,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16362_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16362_timeouts),
 	},
 	[ADIS16367] = {
 		.channels = adis16350_channels,
@@ -1048,7 +1100,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16300_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
 	},
 	[ADIS16400] = {
 		.channels = adis16400_channels,
@@ -1060,7 +1112,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 25000000 / 140000, /* 25 C = 0x00 */
 		.set_freq = adis16400_set_freq,
 		.get_freq = adis16400_get_freq,
-		.timeouts = &adis16400_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16400_timeouts),
 	},
 	[ADIS16445] = {
 		.channels = adis16445_channels,
@@ -1074,7 +1126,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 31000000 / 73860, /* 31 C = 0x00 */
 		.set_freq = adis16334_set_freq,
 		.get_freq = adis16334_get_freq,
-		.timeouts = &adis16445_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16445_timeouts),
 	},
 	[ADIS16448] = {
 		.channels = adis16448_channels,
@@ -1088,7 +1140,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
 		.temp_offset = 31000000 / 73860, /* 31 C = 0x00 */
 		.set_freq = adis16334_set_freq,
 		.get_freq = adis16334_get_freq,
-		.timeouts = &adis16448_timeouts,
+		.adis_data = ADIS16400_DATA(&adis16448_timeouts),
 	}
 };
 
@@ -1099,52 +1151,6 @@ static const struct iio_info adis16400_info = {
 	.debugfs_reg_access = adis_debugfs_reg_access,
 };
 
-static const char * const adis16400_status_error_msgs[] = {
-	[ADIS16400_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
-	[ADIS16400_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
-	[ADIS16400_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
-	[ADIS16400_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
-	[ADIS16400_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
-	[ADIS16400_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
-	[ADIS16400_DIAG_STAT_ALARM2] = "Alarm 2 active",
-	[ADIS16400_DIAG_STAT_ALARM1] = "Alarm 1 active",
-	[ADIS16400_DIAG_STAT_FLASH_CHK] = "Flash checksum error",
-	[ADIS16400_DIAG_STAT_SELF_TEST] = "Self test error",
-	[ADIS16400_DIAG_STAT_OVERFLOW] = "Sensor overrange",
-	[ADIS16400_DIAG_STAT_SPI_FAIL] = "SPI failure",
-	[ADIS16400_DIAG_STAT_FLASH_UPT] = "Flash update failed",
-	[ADIS16400_DIAG_STAT_POWER_HIGH] = "Power supply above 5.25V",
-	[ADIS16400_DIAG_STAT_POWER_LOW] = "Power supply below 4.75V",
-};
-
-static const struct adis_data adis16400_data = {
-	.msc_ctrl_reg = ADIS16400_MSC_CTRL,
-	.glob_cmd_reg = ADIS16400_GLOB_CMD,
-	.diag_stat_reg = ADIS16400_DIAG_STAT,
-
-	.read_delay = 50,
-	.write_delay = 50,
-
-	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,
-
-	.status_error_msgs = adis16400_status_error_msgs,
-	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_XACCL_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_XGYRO_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_YGYRO_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_ZGYRO_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_ALARM2) |
-		BIT(ADIS16400_DIAG_STAT_ALARM1) |
-		BIT(ADIS16400_DIAG_STAT_FLASH_CHK) |
-		BIT(ADIS16400_DIAG_STAT_SELF_TEST) |
-		BIT(ADIS16400_DIAG_STAT_OVERFLOW) |
-		BIT(ADIS16400_DIAG_STAT_SPI_FAIL) |
-		BIT(ADIS16400_DIAG_STAT_FLASH_UPT) |
-		BIT(ADIS16400_DIAG_STAT_POWER_HIGH) |
-		BIT(ADIS16400_DIAG_STAT_POWER_LOW),
-};
-
 static void adis16400_setup_chan_mask(struct adis16400_state *st)
 {
 	const struct adis16400_chip_info *chip_info = st->variant;
@@ -1158,23 +1164,6 @@ static void adis16400_setup_chan_mask(struct adis16400_state *st)
 			st->avail_scan_mask[0] |= BIT(ch->scan_index);
 	}
 }
-
-static struct adis_data *adis16400_adis_data_alloc(struct adis16400_state *st,
-						   struct device *dev)
-{
-	struct adis_data *data;
-
-	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
-	if (!data)
-		return ERR_PTR(-ENOMEM);
-
-	memcpy(data, &adis16400_data, sizeof(*data));
-
-	data->timeouts = st->variant->timeouts;
-
-	return data;
-}
-
 static int adis16400_probe(struct spi_device *spi)
 {
 	struct adis16400_state *st;
@@ -1207,9 +1196,7 @@ static int adis16400_probe(struct spi_device *spi)
 			st->adis.burst->extra_len = sizeof(u16);
 	}
 
-	adis16400_data = adis16400_adis_data_alloc(st, &spi->dev);
-	if (IS_ERR(adis16400_data))
-		return PTR_ERR(adis16400_data);
+	adis16400_data = &st->variant->adis_data;
 
 	ret = adis_init(&st->adis, indio_dev, spi, adis16400_data);
 	if (ret)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 3/9] iio: gyro: adis16136: initialize adis_data statically
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
  2020-02-10 13:25 ` [PATCH v2 2/9] iio: imu: adis16400: " Alexandru Ardelean
@ 2020-02-10 13:26 ` " Alexandru Ardelean
  2020-02-14 14:30   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup() Alexandru Ardelean
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
structure"). It removes the memory allocation and moves the 'adis_data'
information to be static on the chip_info struct.

This also adds a timeout structure to ADIS16334, since it was initially
omitted. This was omitted (by accident) when the change was done.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/gyro/adis16136.c | 61 +++++++++++++-----------------------
 1 file changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
index d5e03a406d4a..1db1131e5c67 100644
--- a/drivers/iio/gyro/adis16136.c
+++ b/drivers/iio/gyro/adis16136.c
@@ -59,7 +59,7 @@
 struct adis16136_chip_info {
 	unsigned int precision;
 	unsigned int fullscale;
-	const struct adis_timeout *timeouts;
+	const struct adis_data adis_data;
 };
 
 struct adis16136 {
@@ -466,22 +466,21 @@ static const char * const adis16136_status_error_msgs[] = {
 	[ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL] = "Flash checksum error",
 };
 
-static const struct adis_data adis16136_data = {
-	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,
-	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,
-	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,
-
-	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,
-
-	.read_delay = 10,
-	.write_delay = 10,
-
-	.status_error_msgs = adis16136_status_error_msgs,
-	.status_error_mask = BIT(ADIS16136_DIAG_STAT_FLASH_UPDATE_FAIL) |
-		BIT(ADIS16136_DIAG_STAT_SPI_FAIL) |
-		BIT(ADIS16136_DIAG_STAT_SELF_TEST_FAIL) |
-		BIT(ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL),
-};
+#define ADIS16136_DATA(_timeouts)					\
+{									\
+	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,			\
+	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,				\
+	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,				\
+	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,			\
+	.read_delay = 10,						\
+	.write_delay = 10,						\
+	.status_error_msgs = adis16136_status_error_msgs,		\
+	.status_error_mask = BIT(ADIS16136_DIAG_STAT_FLASH_UPDATE_FAIL) |	\
+		BIT(ADIS16136_DIAG_STAT_SPI_FAIL) |			\
+		BIT(ADIS16136_DIAG_STAT_SELF_TEST_FAIL) |		\
+		BIT(ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL),		\
+	.timeouts = (_timeouts),					\
+}
 
 enum adis16136_id {
 	ID_ADIS16133,
@@ -506,41 +505,25 @@ static const struct adis16136_chip_info adis16136_chip_info[] = {
 	[ID_ADIS16133] = {
 		.precision = IIO_DEGREE_TO_RAD(1200),
 		.fullscale = 24000,
-		.timeouts = &adis16133_timeouts,
+		.adis_data = ADIS16136_DATA(&adis16133_timeouts),
 	},
 	[ID_ADIS16135] = {
 		.precision = IIO_DEGREE_TO_RAD(300),
 		.fullscale = 24000,
-		.timeouts = &adis16133_timeouts,
+		.adis_data = ADIS16136_DATA(&adis16133_timeouts),
 	},
 	[ID_ADIS16136] = {
 		.precision = IIO_DEGREE_TO_RAD(450),
 		.fullscale = 24623,
-		.timeouts = &adis16136_timeouts,
+		.adis_data = ADIS16136_DATA(&adis16136_timeouts),
 	},
 	[ID_ADIS16137] = {
 		.precision = IIO_DEGREE_TO_RAD(1000),
 		.fullscale = 24609,
-		.timeouts = &adis16136_timeouts,
+		.adis_data = ADIS16136_DATA(&adis16136_timeouts),
 	},
 };
 
-static struct adis_data *adis16136_adis_data_alloc(struct adis16136 *st,
-						   struct device *dev)
-{
-	struct adis_data *data;
-
-	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
-	if (!data)
-		return ERR_PTR(-ENOMEM);
-
-	memcpy(data, &adis16136_data, sizeof(*data));
-
-	data->timeouts = st->chip_info->timeouts;
-
-	return data;
-}
-
 static int adis16136_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
@@ -565,9 +548,7 @@ static int adis16136_probe(struct spi_device *spi)
 	indio_dev->info = &adis16136_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	adis16136_data = adis16136_adis_data_alloc(adis16136, &spi->dev);
-	if (IS_ERR(adis16136_data))
-		return PTR_ERR(adis16136_data);
+	adis16136_data = &adis16136->chip_info->adis_data;
 
 	ret = adis_init(&adis16136->adis, indio_dev, spi, adis16136_data);
 	if (ret)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup()
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
  2020-02-10 13:25 ` [PATCH v2 2/9] iio: imu: adis16400: " Alexandru Ardelean
  2020-02-10 13:26 ` [PATCH v2 3/9] iio: gyro: adis16136: " Alexandru Ardelean
@ 2020-02-10 13:26 ` Alexandru Ardelean
  2020-02-14 14:31   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable Alexandru Ardelean
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

This change splits the __adis_initial_startup() away from
adis_initial_startup(). The unlocked version can be used in certain calls
during probe, where races won't happen since the ADIS driver may not be
registered yet with IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis.c       | 14 ++++----------
 include/linux/iio/imu/adis.h | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 022bb54fb748..e4897dad34ab 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -365,7 +365,7 @@ static int adis_self_test(struct adis *adis)
 }
 
 /**
- * adis_inital_startup() - Performs device self-test
+ * __adis_initial_startup() - Device initial setup
  * @adis: The adis device
  *
  * Returns 0 if the device is operational, a negative error code otherwise.
@@ -373,28 +373,22 @@ static int adis_self_test(struct adis *adis)
  * This function should be called early on in the device initialization sequence
  * to ensure that the device is in a sane and known state and that it is usable.
  */
-int adis_initial_startup(struct adis *adis)
+int __adis_initial_startup(struct adis *adis)
 {
 	int ret;
 
-	mutex_lock(&adis->state_lock);
-
 	ret = adis_self_test(adis);
 	if (ret) {
 		dev_err(&adis->spi->dev, "Self-test failed, trying reset.\n");
 		__adis_reset(adis);
 		ret = adis_self_test(adis);
-		if (ret) {
+		if (ret)
 			dev_err(&adis->spi->dev, "Second self-test failed, giving up.\n");
-			goto out_unlock;
-		}
 	}
 
-out_unlock:
-	mutex_unlock(&adis->state_lock);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(adis_initial_startup);
+EXPORT_SYMBOL_GPL(__adis_initial_startup);
 
 /**
  * adis_single_conversion() - Performs a single sample conversion
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index d2fcf45b4cef..15e75670f923 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -297,6 +297,7 @@ static inline int adis_read_reg_32(struct adis *adis, unsigned int reg,
 
 int adis_enable_irq(struct adis *adis, bool enable);
 int __adis_check_status(struct adis *adis);
+int __adis_initial_startup(struct adis *adis);
 
 static inline int adis_check_status(struct adis *adis)
 {
@@ -309,7 +310,17 @@ static inline int adis_check_status(struct adis *adis)
 	return ret;
 }
 
-int adis_initial_startup(struct adis *adis);
+/* locked version of __adis_initial_startup() */
+static inline int adis_initial_startup(struct adis *adis)
+{
+	int ret;
+
+	mutex_lock(&adis->state_lock);
+	ret = __adis_initial_startup(adis);
+	mutex_unlock(&adis->state_lock);
+
+	return ret;
+}
 
 int adis_single_conversion(struct iio_dev *indio_dev,
 	const struct iio_chan_spec *chan, unsigned int error_mask,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (2 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup() Alexandru Ardelean
@ 2020-02-10 13:26 ` Alexandru Ardelean
  2020-02-14 14:33   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup Alexandru Ardelean
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

From: Nuno Sá <nuno.sa@analog.com>

This patch adds a dedicated self_test_reg variable. This is also a step
to let new drivers make use of `adis_initial_startup()`. Some devices
use MSG_CTRL reg to request a self_test command while others use the
GLOB_CMD register.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/accel/adis16201.c         | 1 +
 drivers/iio/accel/adis16209.c         | 1 +
 drivers/iio/gyro/adis16136.c          | 1 +
 drivers/iio/gyro/adis16260.c          | 1 +
 drivers/iio/imu/adis.c                | 6 +++---
 drivers/iio/imu/adis16400.c           | 1 +
 drivers/iio/imu/adis16460.c           | 2 ++
 drivers/iio/imu/adis16480.c           | 1 +
 drivers/staging/iio/accel/adis16203.c | 1 +
 drivers/staging/iio/accel/adis16240.c | 1 +
 include/linux/iio/imu/adis.h          | 2 ++
 11 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
index 0f0f27a8184e..4154e7396bbe 100644
--- a/drivers/iio/accel/adis16201.c
+++ b/drivers/iio/accel/adis16201.c
@@ -246,6 +246,7 @@ static const struct adis_data adis16201_data = {
 	.diag_stat_reg = ADIS16201_DIAG_STAT_REG,
 
 	.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
+	.self_test_reg = ADIS16201_MSC_CTRL_REG,
 	.self_test_no_autoclear = true,
 	.timeouts = &adis16201_timeouts,
 
diff --git a/drivers/iio/accel/adis16209.c b/drivers/iio/accel/adis16209.c
index c6dbd2424e10..31d45e7c5485 100644
--- a/drivers/iio/accel/adis16209.c
+++ b/drivers/iio/accel/adis16209.c
@@ -256,6 +256,7 @@ static const struct adis_data adis16209_data = {
 	.diag_stat_reg = ADIS16209_STAT_REG,
 
 	.self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN,
+	.self_test_reg = ADIS16209_MSC_CTRL_REG,
 	.self_test_no_autoclear = true,
 	.timeouts = &adis16209_timeouts,
 
diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
index 1db1131e5c67..a4c967a5fc5c 100644
--- a/drivers/iio/gyro/adis16136.c
+++ b/drivers/iio/gyro/adis16136.c
@@ -471,6 +471,7 @@ static const char * const adis16136_status_error_msgs[] = {
 	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,			\
 	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,				\
 	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,				\
+	.self_test_reg = ADIS16136_REG_MSC_CTRL,			\
 	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,			\
 	.read_delay = 10,						\
 	.write_delay = 10,						\
diff --git a/drivers/iio/gyro/adis16260.c b/drivers/iio/gyro/adis16260.c
index be09b3e5910c..9823573e811a 100644
--- a/drivers/iio/gyro/adis16260.c
+++ b/drivers/iio/gyro/adis16260.c
@@ -346,6 +346,7 @@ static const struct adis_data adis16260_data = {
 	.diag_stat_reg = ADIS16260_DIAG_STAT,
 
 	.self_test_mask = ADIS16260_MSC_CTRL_MEM_TEST,
+	.self_test_reg = ADIS16260_MSC_CTRL,
 	.timeouts = &adis16260_timeouts,
 
 	.status_error_msgs = adis1620_status_error_msgs,
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index e4897dad34ab..f7845a90f376 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -346,8 +346,8 @@ static int adis_self_test(struct adis *adis)
 	int ret;
 	const struct adis_timeout *timeouts = adis->data->timeouts;
 
-	ret = __adis_write_reg_16(adis, adis->data->msc_ctrl_reg,
-			adis->data->self_test_mask);
+	ret = __adis_write_reg_16(adis, adis->data->self_test_reg,
+				  adis->data->self_test_mask);
 	if (ret) {
 		dev_err(&adis->spi->dev, "Failed to initiate self test: %d\n",
 			ret);
@@ -359,7 +359,7 @@ static int adis_self_test(struct adis *adis)
 	ret = __adis_check_status(adis);
 
 	if (adis->data->self_test_no_autoclear)
-		__adis_write_reg_16(adis, adis->data->msc_ctrl_reg, 0x00);
+		__adis_write_reg_16(adis, adis->data->self_test_reg, 0x00);
 
 	return ret;
 }
diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index 1c0770e03ec9..05e70c1c4835 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -956,6 +956,7 @@ static const char * const adis16400_status_error_msgs[] = {
 	.read_delay = 50,						\
 	.write_delay = 50,						\
 	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,			\
+	.self_test_reg = ADIS16400_MSC_CTRL,				\
 	.status_error_msgs = adis16400_status_error_msgs,		\
 	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |	\
 		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |			\
diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index 9539cfe4a259..42fa473c6d81 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -392,6 +392,8 @@ static const struct adis_timeout adis16460_timeouts = {
 static const struct adis_data adis16460_data = {
 	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
 	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
+	.self_test_mask = BIT(2),
+	.self_test_reg = ADIS16460_REG_GLOB_CMD,
 	.has_paging = false,
 	.read_delay = 5,
 	.write_delay = 5,
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index 4c4de1b62769..acbe1701fc2d 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -830,6 +830,7 @@ static int adis16480_enable_irq(struct adis *adis, bool enable);
 	.read_delay = 5,						\
 	.write_delay = 5,						\
 	.self_test_mask = BIT(1),					\
+	.self_test_reg = ADIS16480_REG_GLOB_CMD,			\
 	.status_error_msgs = adis16480_status_error_msgs,		\
 	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |	\
 		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |			\
diff --git a/drivers/staging/iio/accel/adis16203.c b/drivers/staging/iio/accel/adis16203.c
index 39dfe3f7f254..fef52d9b5346 100644
--- a/drivers/staging/iio/accel/adis16203.c
+++ b/drivers/staging/iio/accel/adis16203.c
@@ -250,6 +250,7 @@ static const struct adis_data adis16203_data = {
 	.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,
 
diff --git a/drivers/staging/iio/accel/adis16240.c b/drivers/staging/iio/accel/adis16240.c
index 39eb8364aa95..8bd35c6c56a1 100644
--- a/drivers/staging/iio/accel/adis16240.c
+++ b/drivers/staging/iio/accel/adis16240.c
@@ -373,6 +373,7 @@ static const struct adis_data adis16240_data = {
 	.diag_stat_reg = ADIS16240_DIAG_STAT,
 
 	.self_test_mask = ADIS16240_MSC_CTRL_SELF_TEST_EN,
+	.self_test_reg = ADIS16240_MSC_CTRL,
 	.self_test_no_autoclear = true,
 	.timeouts = &adis16240_timeouts,
 
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 15e75670f923..b7feca4e5f26 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -41,6 +41,7 @@ struct adis_timeout {
  * @glob_cmd_reg: Register address of the GLOB_CMD register
  * @msc_ctrl_reg: Register address of the MSC_CTRL register
  * @diag_stat_reg: Register address of the DIAG_STAT register
+ * @self_test_reg: Register address to request self test command
  * @status_error_msgs: Array of error messgaes
  * @status_error_mask:
  * @timeouts: Chip specific delays
@@ -55,6 +56,7 @@ struct adis_data {
 	unsigned int diag_stat_reg;
 
 	unsigned int self_test_mask;
+	unsigned int self_test_reg;
 	bool self_test_no_autoclear;
 	const struct adis_timeout *timeouts;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (3 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable Alexandru Ardelean
@ 2020-02-10 13:26 ` Alexandru Ardelean
  2020-02-14 14:39   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup Alexandru Ardelean
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

From: Nuno Sá <nuno.sa@analog.com>

All the ADIS devices perform, at the beginning, a self test to make sure
the device is in a sane state. Previously, the logic was that the self-test
was performed in adis_initial_startup() and if that failed a reset was done
and then a self-test was attempted again.

This change unifies the reset mechanism under the adis_initial_startup()
call. A HW reset will be done if  GPIO is configured, or a SW reset
otherwise. This should make sure that the chip is in a sane state for
self-test. Once the reset is done, the self-test operation will be
performed. If anything goes wrong with self-test, the driver should just
bail/error-out (i.e. no second attempt). The chip would likely not be a in
a sane state state if the self-test fails after a reset.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/Kconfig |  1 +
 drivers/iio/imu/adis.c  | 29 ++++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 60bb1029e759..63036cf473c7 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -85,6 +85,7 @@ endmenu
 
 config IIO_ADIS_LIB
 	tristate
+	depends on GPIOLIB
 	help
 	  A set of IO helper functions for the Analog Devices ADIS* device family.
 
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index f7845a90f376..0bd6e32cf577 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mutex.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -368,6 +369,10 @@ static int adis_self_test(struct adis *adis)
  * __adis_initial_startup() - Device initial setup
  * @adis: The adis device
  *
+ * The function performs a HW reset via a reset pin that should be specified
+ * via GPIOLIB. If no pin is configured a SW reset will be performed.
+ * The RST pin for the ADIS devices should be configured as ACTIVE_LOW.
+ *
  * Returns 0 if the device is operational, a negative error code otherwise.
  *
  * This function should be called early on in the device initialization sequence
@@ -375,18 +380,28 @@ static int adis_self_test(struct adis *adis)
  */
 int __adis_initial_startup(struct adis *adis)
 {
+	const struct adis_timeout *timeouts = adis->data->timeouts;
+	struct gpio_desc *gpio;
 	int ret;
 
-	ret = adis_self_test(adis);
-	if (ret) {
-		dev_err(&adis->spi->dev, "Self-test failed, trying reset.\n");
-		__adis_reset(adis);
-		ret = adis_self_test(adis);
+	/* check if the device has rst pin low */
+	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(gpio))
+		return PTR_ERR(gpio);
+
+	if (gpio) {
+		gpiod_set_value_cansleep(gpio, 1);
+		msleep(10);
+		/* bring device out of reset */
+		gpiod_set_value_cansleep(gpio, 0);
+		msleep(timeouts->reset_ms);
+	} else {
+		ret = __adis_reset(adis);
 		if (ret)
-			dev_err(&adis->spi->dev, "Second self-test failed, giving up.\n");
+			return ret;
 	}
 
-	return ret;
+	return adis_self_test(adis);
 }
 EXPORT_SYMBOL_GPL(__adis_initial_startup);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (4 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup Alexandru Ardelean
@ 2020-02-10 13:26 ` Alexandru Ardelean
  2020-02-14 14:41   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup Alexandru Ardelean
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

Each driver/chip that wants to validate it's product id, can now
specify a 'prod_id_reg' and an expected 'prod_id' value.
The 'prod_id' value is intentionally left 0 (uninitialized). There aren't
(yet) any product IDs with value 0; this enforces that both 'prod_id_reg'
and 'prod_id' are specified.

At the very least, this routine validates that the SPI connection to the
ADIS chip[s] works well.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis.c       | 23 ++++++++++++++++++++++-
 include/linux/iio/imu/adis.h |  5 +++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 0bd6e32cf577..a8afd01de4f3 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -373,6 +373,10 @@ static int adis_self_test(struct adis *adis)
  * via GPIOLIB. If no pin is configured a SW reset will be performed.
  * The RST pin for the ADIS devices should be configured as ACTIVE_LOW.
  *
+ * After the self-test operation is performed, the function will also check
+ * that the product ID is as expected. This assumes that drivers providing
+ * 'prod_id_reg' will also provide the 'prod_id'.
+ *
  * Returns 0 if the device is operational, a negative error code otherwise.
  *
  * This function should be called early on in the device initialization sequence
@@ -382,6 +386,7 @@ int __adis_initial_startup(struct adis *adis)
 {
 	const struct adis_timeout *timeouts = adis->data->timeouts;
 	struct gpio_desc *gpio;
+	uint16_t prod_id;
 	int ret;
 
 	/* check if the device has rst pin low */
@@ -401,7 +406,23 @@ int __adis_initial_startup(struct adis *adis)
 			return ret;
 	}
 
-	return adis_self_test(adis);
+	ret = adis_self_test(adis);
+	if (ret)
+		return ret;
+
+	if (!adis->data->prod_id_reg)
+		return 0;
+
+	ret = adis_read_reg_16(adis, adis->data->prod_id_reg, &prod_id);
+	if (ret)
+		return ret;
+
+	if (prod_id != adis->data->prod_id)
+		dev_warn(&adis->spi->dev,
+			 "Device ID(%u) and product ID(%u) do not match.",
+			 adis->data->prod_id, prod_id);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(__adis_initial_startup);
 
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index b7feca4e5f26..ac7cfd073804 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -41,6 +41,8 @@ struct adis_timeout {
  * @glob_cmd_reg: Register address of the GLOB_CMD register
  * @msc_ctrl_reg: Register address of the MSC_CTRL register
  * @diag_stat_reg: Register address of the DIAG_STAT register
+ * @prod_id_reg: Register address of the PROD_ID register
+ * @prod_id: Product ID code that should be expected when reading @prod_id_reg
  * @self_test_reg: Register address to request self test command
  * @status_error_msgs: Array of error messgaes
  * @status_error_mask:
@@ -54,6 +56,9 @@ struct adis_data {
 	unsigned int glob_cmd_reg;
 	unsigned int msc_ctrl_reg;
 	unsigned int diag_stat_reg;
+	unsigned int prod_id_reg;
+
+	unsigned int prod_id;
 
 	unsigned int self_test_mask;
 	unsigned int self_test_reg;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (5 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup Alexandru Ardelean
@ 2020-02-10 13:26 ` Alexandru Ardelean
  2020-02-14 14:44   ` Jonathan Cameron
  2020-02-10 13:26 ` [PATCH v2 9/9] iio: adis16460: " Alexandru Ardelean
  2020-02-14 14:28 ` [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Jonathan Cameron
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

From: Nuno Sá <nuno.sa@analog.com>

All actions done in `adis16480_initial_setup()` are now done in
`__adis_initial_startup()` so, there's no need for code duplication.
Furthermore, the call to `adis16480_initial_setup()` is done before any
device configuration since the device will be reset if not already (via
rst pin). This is actually fixing a potential bug since `adis_reset()` was
being called after configuring the device which is obviously a problem.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis16480.c | 80 +++++++++++--------------------------
 1 file changed, 24 insertions(+), 56 deletions(-)

diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index acbe1701fc2d..cfae0e4476e7 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -822,10 +822,12 @@ static const char * const adis16480_status_error_msgs[] = {
 
 static int adis16480_enable_irq(struct adis *adis, bool enable);
 
-#define ADIS16480_DATA(_timeouts)					\
+#define ADIS16480_DATA(_prod_id, _timeouts)				\
 {									\
 	.diag_stat_reg = ADIS16480_REG_DIAG_STS,			\
 	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,				\
+	.prod_id_reg = ADIS16480_REG_PROD_ID,				\
+	.prod_id = (_prod_id),						\
 	.has_paging = true,						\
 	.read_delay = 5,						\
 	.write_delay = 5,						\
@@ -888,7 +890,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
+		.adis_data = ADIS16480_DATA(16375, &adis16485_timeouts),
 	},
 	[ADIS16480] = {
 		.channels = adis16480_channels,
@@ -901,7 +903,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.adis_data = ADIS16480_DATA(&adis16480_timeouts),
+		.adis_data = ADIS16480_DATA(16480, &adis16480_timeouts),
 	},
 	[ADIS16485] = {
 		.channels = adis16485_channels,
@@ -914,7 +916,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
+		.adis_data = ADIS16480_DATA(16485, &adis16485_timeouts),
 	},
 	[ADIS16488] = {
 		.channels = adis16480_channels,
@@ -927,7 +929,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.filter_freqs = adis16480_def_filter_freqs,
-		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
+		.adis_data = ADIS16480_DATA(16488, &adis16485_timeouts),
 	},
 	[ADIS16490] = {
 		.channels = adis16485_channels,
@@ -941,7 +943,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_timeouts),
+		.adis_data = ADIS16480_DATA(16490, &adis16495_timeouts),
 	},
 	[ADIS16495_1] = {
 		.channels = adis16485_channels,
@@ -955,7 +957,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
 	},
 	[ADIS16495_2] = {
 		.channels = adis16485_channels,
@@ -969,7 +971,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
 	},
 	[ADIS16495_3] = {
 		.channels = adis16485_channels,
@@ -983,7 +985,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
 	},
 	[ADIS16497_1] = {
 		.channels = adis16485_channels,
@@ -997,7 +999,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
 	},
 	[ADIS16497_2] = {
 		.channels = adis16485_channels,
@@ -1011,7 +1013,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
 	},
 	[ADIS16497_3] = {
 		.channels = adis16485_channels,
@@ -1025,7 +1027,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
 		.has_pps_clk_mode = true,
-		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
+		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
 	},
 };
 
@@ -1064,40 +1066,6 @@ static int adis16480_enable_irq(struct adis *adis, bool enable)
 	return __adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
 }
 
-static int adis16480_initial_setup(struct iio_dev *indio_dev)
-{
-	struct adis16480 *st = iio_priv(indio_dev);
-	uint16_t prod_id;
-	unsigned int device_id;
-	int ret;
-
-	adis_reset(&st->adis);
-	msleep(70);
-
-	ret = adis_write_reg_16(&st->adis, ADIS16480_REG_GLOB_CMD, BIT(1));
-	if (ret)
-		return ret;
-	msleep(30);
-
-	ret = adis_check_status(&st->adis);
-	if (ret)
-		return ret;
-
-	ret = adis_read_reg_16(&st->adis, ADIS16480_REG_PROD_ID, &prod_id);
-	if (ret)
-		return ret;
-
-	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
-	if (ret != 1)
-		return -EINVAL;
-
-	if (prod_id != device_id)
-		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
-				device_id, prod_id);
-
-	return 0;
-}
-
 static int adis16480_config_irq_pin(struct device_node *of_node,
 				    struct adis16480 *st)
 {
@@ -1278,18 +1246,22 @@ static int adis16480_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
+	ret = __adis_initial_startup(&st->adis);
 	if (ret)
 		return ret;
 
+	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
+	if (ret)
+		goto error_stop_device;
+
 	ret = adis16480_get_ext_clocks(st);
 	if (ret)
-		return ret;
+		goto error_stop_device;
 
 	if (!IS_ERR_OR_NULL(st->ext_clk)) {
 		ret = adis16480_ext_clk_config(st, spi->dev.of_node, true);
 		if (ret)
-			return ret;
+			goto error_stop_device;
 
 		st->clk_freq = clk_get_rate(st->ext_clk);
 		st->clk_freq *= 1000; /* micro */
@@ -1301,24 +1273,20 @@ static int adis16480_probe(struct spi_device *spi)
 	if (ret)
 		goto error_clk_disable_unprepare;
 
-	ret = adis16480_initial_setup(indio_dev);
-	if (ret)
-		goto error_cleanup_buffer;
-
 	ret = iio_device_register(indio_dev);
 	if (ret)
-		goto error_stop_device;
+		goto error_cleanup_buffer;
 
 	adis16480_debugfs_init(indio_dev);
 
 	return 0;
 
-error_stop_device:
-	adis16480_stop_device(indio_dev);
 error_cleanup_buffer:
 	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
 error_clk_disable_unprepare:
 	clk_disable_unprepare(st->ext_clk);
+error_stop_device:
+	adis16480_stop_device(indio_dev);
 	return ret;
 }
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 9/9] iio: adis16460: Make use of __adis_initial_startup
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (6 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup Alexandru Ardelean
@ 2020-02-10 13:26 ` " Alexandru Ardelean
  2020-02-14 14:45   ` Jonathan Cameron
  2020-02-14 14:28 ` [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Jonathan Cameron
  8 siblings, 1 reply; 18+ messages in thread
From: Alexandru Ardelean @ 2020-02-10 13:26 UTC (permalink / raw)
  To: linux-iio, linux-kernel; +Cc: nuno.sa, jic23, Alexandru Ardelean

From: Nuno Sá <nuno.sa@analog.com>

All of the actions done in `adis16460_initial_setup()` are now done in
`__adis_initial_startup()` so, there's no need for code duplication.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/imu/adis16460.c | 38 +++----------------------------------
 1 file changed, 3 insertions(+), 35 deletions(-)

diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index 42fa473c6d81..0027683d0256 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -333,40 +333,6 @@ static int adis16460_enable_irq(struct adis *adis, bool enable)
 	return 0;
 }
 
-static int adis16460_initial_setup(struct iio_dev *indio_dev)
-{
-	struct adis16460 *st = iio_priv(indio_dev);
-	uint16_t prod_id;
-	unsigned int device_id;
-	int ret;
-
-	adis_reset(&st->adis);
-	msleep(222);
-
-	ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
-	if (ret)
-		return ret;
-	msleep(75);
-
-	ret = adis_check_status(&st->adis);
-	if (ret)
-		return ret;
-
-	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
-	if (ret)
-		return ret;
-
-	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
-	if (ret != 1)
-		return -EINVAL;
-
-	if (prod_id != device_id)
-		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
-				device_id, prod_id);
-
-	return 0;
-}
-
 #define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
 #define ADIS16460_DIAG_STAT_FLASH_MEM	6
 #define ADIS16460_DIAG_STAT_SELF_TEST	5
@@ -392,6 +358,8 @@ static const struct adis_timeout adis16460_timeouts = {
 static const struct adis_data adis16460_data = {
 	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
 	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
+	.prod_id_reg = ADIS16460_REG_PROD_ID,
+	.prod_id = 16460,
 	.self_test_mask = BIT(2),
 	.self_test_reg = ADIS16460_REG_GLOB_CMD,
 	.has_paging = false,
@@ -441,7 +409,7 @@ static int adis16460_probe(struct spi_device *spi)
 
 	adis16460_enable_irq(&st->adis, 0);
 
-	ret = adis16460_initial_setup(indio_dev);
+	ret = __adis_initial_startup(&st->adis);
 	if (ret)
 		goto error_cleanup_buffer;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically
  2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
                   ` (7 preceding siblings ...)
  2020-02-10 13:26 ` [PATCH v2 9/9] iio: adis16460: " Alexandru Ardelean
@ 2020-02-14 14:28 ` Jonathan Cameron
  8 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:28 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:25:58 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
> structure"). It removes the memory allocation and moves the 'adis_data'
> information to be static on the chip_info struct.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to poke at it.

Thanks,

Jonathan

> ---
> 
> Changelog v1 -> v2:
> * initialize 'adis_data' statically on adis16480,adis16136 & adis16400
> * split 'iio: imu: adis: Refactor adis_initial_startup' patch
>   - new: 'iio: imu: adis: add unlocked __adis_initial_startup()'
>   - new: 'iio: imu: adis: add support product ID check in adis_initial_startup'
>   - modified: 'iio: imu: adis: Refactor adis_initial_startup'
> * added 'prod_id' field together with 'prod_id_reg' on 'adis_data'
>   each device that wants to use the 'prod_id_reg' must also provide an
>   expected 'prod_id' value
> 
>  drivers/iio/imu/adis16480.c | 140 ++++++++++++++++--------------------
>  1 file changed, 62 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index dac87f1001fd..4c4de1b62769 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -138,7 +138,7 @@ struct adis16480_chip_info {
>  	unsigned int max_dec_rate;
>  	const unsigned int *filter_freqs;
>  	bool has_pps_clk_mode;
> -	const struct adis_timeout *timeouts;
> +	const struct adis_data adis_data;
>  };
>  
>  enum adis16480_int_pin {
> @@ -796,6 +796,55 @@ enum adis16480_variant {
>  	ADIS16497_3,
>  };
>  
> +#define ADIS16480_DIAG_STAT_XGYRO_FAIL 0
> +#define ADIS16480_DIAG_STAT_YGYRO_FAIL 1
> +#define ADIS16480_DIAG_STAT_ZGYRO_FAIL 2
> +#define ADIS16480_DIAG_STAT_XACCL_FAIL 3
> +#define ADIS16480_DIAG_STAT_YACCL_FAIL 4
> +#define ADIS16480_DIAG_STAT_ZACCL_FAIL 5
> +#define ADIS16480_DIAG_STAT_XMAGN_FAIL 8
> +#define ADIS16480_DIAG_STAT_YMAGN_FAIL 9
> +#define ADIS16480_DIAG_STAT_ZMAGN_FAIL 10
> +#define ADIS16480_DIAG_STAT_BARO_FAIL 11
> +
> +static const char * const adis16480_status_error_msgs[] = {
> +	[ADIS16480_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
> +	[ADIS16480_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
> +	[ADIS16480_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
> +	[ADIS16480_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
> +	[ADIS16480_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
> +	[ADIS16480_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
> +	[ADIS16480_DIAG_STAT_XMAGN_FAIL] = "X-axis magnetometer self-test failure",
> +	[ADIS16480_DIAG_STAT_YMAGN_FAIL] = "Y-axis magnetometer self-test failure",
> +	[ADIS16480_DIAG_STAT_ZMAGN_FAIL] = "Z-axis magnetometer self-test failure",
> +	[ADIS16480_DIAG_STAT_BARO_FAIL] = "Barometer self-test failure",
> +};
> +
> +static int adis16480_enable_irq(struct adis *adis, bool enable);
> +
> +#define ADIS16480_DATA(_timeouts)					\
> +{									\
> +	.diag_stat_reg = ADIS16480_REG_DIAG_STS,			\
> +	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,				\
> +	.has_paging = true,						\
> +	.read_delay = 5,						\
> +	.write_delay = 5,						\
> +	.self_test_mask = BIT(1),					\
> +	.status_error_msgs = adis16480_status_error_msgs,		\
> +	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |	\
> +		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_ZGYRO_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_XACCL_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_YACCL_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_ZACCL_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_XMAGN_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_YMAGN_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_ZMAGN_FAIL) |			\
> +		BIT(ADIS16480_DIAG_STAT_BARO_FAIL),			\
> +	.enable_irq = adis16480_enable_irq,				\
> +	.timeouts = (_timeouts),					\
> +}
> +
>  static const struct adis_timeout adis16485_timeouts = {
>  	.reset_ms = 560,
>  	.sw_reset_ms = 120,
> @@ -838,7 +887,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.timeouts = &adis16485_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
>  	},
>  	[ADIS16480] = {
>  		.channels = adis16480_channels,
> @@ -851,7 +900,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.timeouts = &adis16480_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16480_timeouts),
>  	},
>  	[ADIS16485] = {
>  		.channels = adis16485_channels,
> @@ -864,7 +913,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.timeouts = &adis16485_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
>  	},
>  	[ADIS16488] = {
>  		.channels = adis16480_channels,
> @@ -877,7 +926,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.timeouts = &adis16485_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
>  	},
>  	[ADIS16490] = {
>  		.channels = adis16485_channels,
> @@ -891,7 +940,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_timeouts),
>  	},
>  	[ADIS16495_1] = {
>  		.channels = adis16485_channels,
> @@ -905,7 +954,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  	[ADIS16495_2] = {
>  		.channels = adis16485_channels,
> @@ -919,7 +968,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  	[ADIS16495_3] = {
>  		.channels = adis16485_channels,
> @@ -933,7 +982,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  	[ADIS16497_1] = {
>  		.channels = adis16485_channels,
> @@ -947,7 +996,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  	[ADIS16497_2] = {
>  		.channels = adis16485_channels,
> @@ -961,7 +1010,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  	[ADIS16497_3] = {
>  		.channels = adis16485_channels,
> @@ -975,7 +1024,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.timeouts = &adis16495_1_timeouts,
> +		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
>  	},
>  };
>  
> @@ -1048,53 +1097,6 @@ static int adis16480_initial_setup(struct iio_dev *indio_dev)
>  	return 0;
>  }
>  
> -#define ADIS16480_DIAG_STAT_XGYRO_FAIL 0
> -#define ADIS16480_DIAG_STAT_YGYRO_FAIL 1
> -#define ADIS16480_DIAG_STAT_ZGYRO_FAIL 2
> -#define ADIS16480_DIAG_STAT_XACCL_FAIL 3
> -#define ADIS16480_DIAG_STAT_YACCL_FAIL 4
> -#define ADIS16480_DIAG_STAT_ZACCL_FAIL 5
> -#define ADIS16480_DIAG_STAT_XMAGN_FAIL 8
> -#define ADIS16480_DIAG_STAT_YMAGN_FAIL 9
> -#define ADIS16480_DIAG_STAT_ZMAGN_FAIL 10
> -#define ADIS16480_DIAG_STAT_BARO_FAIL 11
> -
> -static const char * const adis16480_status_error_msgs[] = {
> -	[ADIS16480_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
> -	[ADIS16480_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
> -	[ADIS16480_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
> -	[ADIS16480_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
> -	[ADIS16480_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
> -	[ADIS16480_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
> -	[ADIS16480_DIAG_STAT_XMAGN_FAIL] = "X-axis magnetometer self-test failure",
> -	[ADIS16480_DIAG_STAT_YMAGN_FAIL] = "Y-axis magnetometer self-test failure",
> -	[ADIS16480_DIAG_STAT_ZMAGN_FAIL] = "Z-axis magnetometer self-test failure",
> -	[ADIS16480_DIAG_STAT_BARO_FAIL] = "Barometer self-test failure",
> -};
> -
> -static const struct adis_data adis16480_data = {
> -	.diag_stat_reg = ADIS16480_REG_DIAG_STS,
> -	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,
> -	.has_paging = true,
> -
> -	.read_delay = 5,
> -	.write_delay = 5,
> -
> -	.status_error_msgs = adis16480_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_ZGYRO_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_XACCL_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_YACCL_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_ZACCL_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_XMAGN_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_YMAGN_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_ZMAGN_FAIL) |
> -		BIT(ADIS16480_DIAG_STAT_BARO_FAIL),
> -
> -	.enable_irq = adis16480_enable_irq,
> -};
> -
>  static int adis16480_config_irq_pin(struct device_node *of_node,
>  				    struct adis16480 *st)
>  {
> @@ -1245,22 +1247,6 @@ static int adis16480_get_ext_clocks(struct adis16480 *st)
>  	return 0;
>  }
>  
> -static struct adis_data *adis16480_adis_data_alloc(struct adis16480 *st,
> -						   struct device *dev)
> -{
> -	struct adis_data *data;
> -
> -	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
> -	if (!data)
> -		return ERR_PTR(-ENOMEM);
> -
> -	memcpy(data, &adis16480_data, sizeof(*data));
> -
> -	data->timeouts = st->chip_info->timeouts;
> -
> -	return data;
> -}
> -
>  static int adis16480_probe(struct spi_device *spi)
>  {
>  	const struct spi_device_id *id = spi_get_device_id(spi);
> @@ -1285,9 +1271,7 @@ static int adis16480_probe(struct spi_device *spi)
>  	indio_dev->info = &adis16480_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	adis16480_data = adis16480_adis_data_alloc(st, &spi->dev);
> -	if (IS_ERR(adis16480_data))
> -		return PTR_ERR(adis16480_data);
> +	adis16480_data = &st->chip_info->adis_data;
>  
>  	ret = adis_init(&st->adis, indio_dev, spi, adis16480_data);
>  	if (ret)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/9] iio: imu: adis16400: initialize adis_data statically
  2020-02-10 13:25 ` [PATCH v2 2/9] iio: imu: adis16400: " Alexandru Ardelean
@ 2020-02-14 14:29   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:29 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:25:59 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
> structure"). It removes the memory allocation and moves the 'adis_data'
> information to be static on the chip_info struct.
> 
> This also adds a timeout structure to ADIS16334, since it was initially
> omitted. This was omitted (by accident) when the change was done.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.  Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16400.c | 139 ++++++++++++++++--------------------
>  1 file changed, 63 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
> index cfb1c19eb930..1c0770e03ec9 100644
> --- a/drivers/iio/imu/adis16400.c
> +++ b/drivers/iio/imu/adis16400.c
> @@ -156,7 +156,7 @@ struct adis16400_state;
>  
>  struct adis16400_chip_info {
>  	const struct iio_chan_spec *channels;
> -	const struct adis_timeout *timeouts;
> +	const struct adis_data adis_data;
>  	const int num_channels;
>  	const long flags;
>  	unsigned int gyro_scale_micro;
> @@ -930,12 +930,63 @@ static const struct iio_chan_spec adis16334_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
>  };
>  
> +static const char * const adis16400_status_error_msgs[] = {
> +	[ADIS16400_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
> +	[ADIS16400_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
> +	[ADIS16400_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
> +	[ADIS16400_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
> +	[ADIS16400_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
> +	[ADIS16400_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
> +	[ADIS16400_DIAG_STAT_ALARM2] = "Alarm 2 active",
> +	[ADIS16400_DIAG_STAT_ALARM1] = "Alarm 1 active",
> +	[ADIS16400_DIAG_STAT_FLASH_CHK] = "Flash checksum error",
> +	[ADIS16400_DIAG_STAT_SELF_TEST] = "Self test error",
> +	[ADIS16400_DIAG_STAT_OVERFLOW] = "Sensor overrange",
> +	[ADIS16400_DIAG_STAT_SPI_FAIL] = "SPI failure",
> +	[ADIS16400_DIAG_STAT_FLASH_UPT] = "Flash update failed",
> +	[ADIS16400_DIAG_STAT_POWER_HIGH] = "Power supply above 5.25V",
> +	[ADIS16400_DIAG_STAT_POWER_LOW] = "Power supply below 4.75V",
> +};
> +
> +#define ADIS16400_DATA(_timeouts)					\
> +{									\
> +	.msc_ctrl_reg = ADIS16400_MSC_CTRL,				\
> +	.glob_cmd_reg = ADIS16400_GLOB_CMD,				\
> +	.diag_stat_reg = ADIS16400_DIAG_STAT,				\
> +	.read_delay = 50,						\
> +	.write_delay = 50,						\
> +	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,			\
> +	.status_error_msgs = adis16400_status_error_msgs,		\
> +	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |	\
> +		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_XACCL_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_XGYRO_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_YGYRO_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_ZGYRO_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_ALARM2) |			\
> +		BIT(ADIS16400_DIAG_STAT_ALARM1) |			\
> +		BIT(ADIS16400_DIAG_STAT_FLASH_CHK) |			\
> +		BIT(ADIS16400_DIAG_STAT_SELF_TEST) |			\
> +		BIT(ADIS16400_DIAG_STAT_OVERFLOW) |			\
> +		BIT(ADIS16400_DIAG_STAT_SPI_FAIL) |			\
> +		BIT(ADIS16400_DIAG_STAT_FLASH_UPT) |			\
> +		BIT(ADIS16400_DIAG_STAT_POWER_HIGH) |			\
> +		BIT(ADIS16400_DIAG_STAT_POWER_LOW),			\
> +	.timeouts = (_timeouts),					\
> +}
> +
>  static const struct adis_timeout adis16300_timeouts = {
>  	.reset_ms = ADIS16400_STARTUP_DELAY,
>  	.sw_reset_ms = ADIS16400_STARTUP_DELAY,
>  	.self_test_ms = ADIS16400_STARTUP_DELAY,
>  };
>  
> +static const struct adis_timeout adis16334_timeouts = {
> +	.reset_ms = 60,
> +	.sw_reset_ms = 60,
> +	.self_test_ms = 14,
> +};
> +
>  static const struct adis_timeout adis16362_timeouts = {
>  	.reset_ms = 130,
>  	.sw_reset_ms = 130,
> @@ -972,7 +1023,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 140000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16300_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
>  	},
>  	[ADIS16334] = {
>  		.channels = adis16334_channels,
> @@ -985,6 +1036,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 67850, /* 25 C = 0x00 */
>  		.set_freq = adis16334_set_freq,
>  		.get_freq = adis16334_get_freq,
> +		.adis_data = ADIS16400_DATA(&adis16334_timeouts),
>  	},
>  	[ADIS16350] = {
>  		.channels = adis16350_channels,
> @@ -996,7 +1048,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.flags = ADIS16400_NO_BURST | ADIS16400_HAS_SLOW_MODE,
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16300_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
>  	},
>  	[ADIS16360] = {
>  		.channels = adis16350_channels,
> @@ -1009,7 +1061,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16300_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
>  	},
>  	[ADIS16362] = {
>  		.channels = adis16350_channels,
> @@ -1022,7 +1074,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16362_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16362_timeouts),
>  	},
>  	[ADIS16364] = {
>  		.channels = adis16350_channels,
> @@ -1035,7 +1087,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16362_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16362_timeouts),
>  	},
>  	[ADIS16367] = {
>  		.channels = adis16350_channels,
> @@ -1048,7 +1100,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 136000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16300_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16300_timeouts),
>  	},
>  	[ADIS16400] = {
>  		.channels = adis16400_channels,
> @@ -1060,7 +1112,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 25000000 / 140000, /* 25 C = 0x00 */
>  		.set_freq = adis16400_set_freq,
>  		.get_freq = adis16400_get_freq,
> -		.timeouts = &adis16400_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16400_timeouts),
>  	},
>  	[ADIS16445] = {
>  		.channels = adis16445_channels,
> @@ -1074,7 +1126,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 31000000 / 73860, /* 31 C = 0x00 */
>  		.set_freq = adis16334_set_freq,
>  		.get_freq = adis16334_get_freq,
> -		.timeouts = &adis16445_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16445_timeouts),
>  	},
>  	[ADIS16448] = {
>  		.channels = adis16448_channels,
> @@ -1088,7 +1140,7 @@ static struct adis16400_chip_info adis16400_chips[] = {
>  		.temp_offset = 31000000 / 73860, /* 31 C = 0x00 */
>  		.set_freq = adis16334_set_freq,
>  		.get_freq = adis16334_get_freq,
> -		.timeouts = &adis16448_timeouts,
> +		.adis_data = ADIS16400_DATA(&adis16448_timeouts),
>  	}
>  };
>  
> @@ -1099,52 +1151,6 @@ static const struct iio_info adis16400_info = {
>  	.debugfs_reg_access = adis_debugfs_reg_access,
>  };
>  
> -static const char * const adis16400_status_error_msgs[] = {
> -	[ADIS16400_DIAG_STAT_ZACCL_FAIL] = "Z-axis accelerometer self-test failure",
> -	[ADIS16400_DIAG_STAT_YACCL_FAIL] = "Y-axis accelerometer self-test failure",
> -	[ADIS16400_DIAG_STAT_XACCL_FAIL] = "X-axis accelerometer self-test failure",
> -	[ADIS16400_DIAG_STAT_XGYRO_FAIL] = "X-axis gyroscope self-test failure",
> -	[ADIS16400_DIAG_STAT_YGYRO_FAIL] = "Y-axis gyroscope self-test failure",
> -	[ADIS16400_DIAG_STAT_ZGYRO_FAIL] = "Z-axis gyroscope self-test failure",
> -	[ADIS16400_DIAG_STAT_ALARM2] = "Alarm 2 active",
> -	[ADIS16400_DIAG_STAT_ALARM1] = "Alarm 1 active",
> -	[ADIS16400_DIAG_STAT_FLASH_CHK] = "Flash checksum error",
> -	[ADIS16400_DIAG_STAT_SELF_TEST] = "Self test error",
> -	[ADIS16400_DIAG_STAT_OVERFLOW] = "Sensor overrange",
> -	[ADIS16400_DIAG_STAT_SPI_FAIL] = "SPI failure",
> -	[ADIS16400_DIAG_STAT_FLASH_UPT] = "Flash update failed",
> -	[ADIS16400_DIAG_STAT_POWER_HIGH] = "Power supply above 5.25V",
> -	[ADIS16400_DIAG_STAT_POWER_LOW] = "Power supply below 4.75V",
> -};
> -
> -static const struct adis_data adis16400_data = {
> -	.msc_ctrl_reg = ADIS16400_MSC_CTRL,
> -	.glob_cmd_reg = ADIS16400_GLOB_CMD,
> -	.diag_stat_reg = ADIS16400_DIAG_STAT,
> -
> -	.read_delay = 50,
> -	.write_delay = 50,
> -
> -	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,
> -
> -	.status_error_msgs = adis16400_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_XACCL_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_XGYRO_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_YGYRO_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_ZGYRO_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_ALARM2) |
> -		BIT(ADIS16400_DIAG_STAT_ALARM1) |
> -		BIT(ADIS16400_DIAG_STAT_FLASH_CHK) |
> -		BIT(ADIS16400_DIAG_STAT_SELF_TEST) |
> -		BIT(ADIS16400_DIAG_STAT_OVERFLOW) |
> -		BIT(ADIS16400_DIAG_STAT_SPI_FAIL) |
> -		BIT(ADIS16400_DIAG_STAT_FLASH_UPT) |
> -		BIT(ADIS16400_DIAG_STAT_POWER_HIGH) |
> -		BIT(ADIS16400_DIAG_STAT_POWER_LOW),
> -};
> -
>  static void adis16400_setup_chan_mask(struct adis16400_state *st)
>  {
>  	const struct adis16400_chip_info *chip_info = st->variant;
> @@ -1158,23 +1164,6 @@ static void adis16400_setup_chan_mask(struct adis16400_state *st)
>  			st->avail_scan_mask[0] |= BIT(ch->scan_index);
>  	}
>  }
> -
> -static struct adis_data *adis16400_adis_data_alloc(struct adis16400_state *st,
> -						   struct device *dev)
> -{
> -	struct adis_data *data;
> -
> -	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
> -	if (!data)
> -		return ERR_PTR(-ENOMEM);
> -
> -	memcpy(data, &adis16400_data, sizeof(*data));
> -
> -	data->timeouts = st->variant->timeouts;
> -
> -	return data;
> -}
> -
>  static int adis16400_probe(struct spi_device *spi)
>  {
>  	struct adis16400_state *st;
> @@ -1207,9 +1196,7 @@ static int adis16400_probe(struct spi_device *spi)
>  			st->adis.burst->extra_len = sizeof(u16);
>  	}
>  
> -	adis16400_data = adis16400_adis_data_alloc(st, &spi->dev);
> -	if (IS_ERR(adis16400_data))
> -		return PTR_ERR(adis16400_data);
> +	adis16400_data = &st->variant->adis_data;
>  
>  	ret = adis_init(&st->adis, indio_dev, spi, adis16400_data);
>  	if (ret)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 3/9] iio: gyro: adis16136: initialize adis_data statically
  2020-02-10 13:26 ` [PATCH v2 3/9] iio: gyro: adis16136: " Alexandru Ardelean
@ 2020-02-14 14:30   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:30 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:00 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change overrides commit 380b107bbf944 ("iio: adis: Introduce timeouts
> structure"). It removes the memory allocation and moves the 'adis_data'
> information to be static on the chip_info struct.
> 
> This also adds a timeout structure to ADIS16334, since it was initially
> omitted. This was omitted (by accident) when the change was done.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.  Thanks,

Jonathan

> ---
>  drivers/iio/gyro/adis16136.c | 61 +++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
> index d5e03a406d4a..1db1131e5c67 100644
> --- a/drivers/iio/gyro/adis16136.c
> +++ b/drivers/iio/gyro/adis16136.c
> @@ -59,7 +59,7 @@
>  struct adis16136_chip_info {
>  	unsigned int precision;
>  	unsigned int fullscale;
> -	const struct adis_timeout *timeouts;
> +	const struct adis_data adis_data;
>  };
>  
>  struct adis16136 {
> @@ -466,22 +466,21 @@ static const char * const adis16136_status_error_msgs[] = {
>  	[ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL] = "Flash checksum error",
>  };
>  
> -static const struct adis_data adis16136_data = {
> -	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,
> -	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,
> -	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,
> -
> -	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,
> -
> -	.read_delay = 10,
> -	.write_delay = 10,
> -
> -	.status_error_msgs = adis16136_status_error_msgs,
> -	.status_error_mask = BIT(ADIS16136_DIAG_STAT_FLASH_UPDATE_FAIL) |
> -		BIT(ADIS16136_DIAG_STAT_SPI_FAIL) |
> -		BIT(ADIS16136_DIAG_STAT_SELF_TEST_FAIL) |
> -		BIT(ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL),
> -};
> +#define ADIS16136_DATA(_timeouts)					\
> +{									\
> +	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,			\
> +	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,				\
> +	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,				\
> +	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,			\
> +	.read_delay = 10,						\
> +	.write_delay = 10,						\
> +	.status_error_msgs = adis16136_status_error_msgs,		\
> +	.status_error_mask = BIT(ADIS16136_DIAG_STAT_FLASH_UPDATE_FAIL) |	\
> +		BIT(ADIS16136_DIAG_STAT_SPI_FAIL) |			\
> +		BIT(ADIS16136_DIAG_STAT_SELF_TEST_FAIL) |		\
> +		BIT(ADIS16136_DIAG_STAT_FLASH_CHKSUM_FAIL),		\
> +	.timeouts = (_timeouts),					\
> +}
>  
>  enum adis16136_id {
>  	ID_ADIS16133,
> @@ -506,41 +505,25 @@ static const struct adis16136_chip_info adis16136_chip_info[] = {
>  	[ID_ADIS16133] = {
>  		.precision = IIO_DEGREE_TO_RAD(1200),
>  		.fullscale = 24000,
> -		.timeouts = &adis16133_timeouts,
> +		.adis_data = ADIS16136_DATA(&adis16133_timeouts),
>  	},
>  	[ID_ADIS16135] = {
>  		.precision = IIO_DEGREE_TO_RAD(300),
>  		.fullscale = 24000,
> -		.timeouts = &adis16133_timeouts,
> +		.adis_data = ADIS16136_DATA(&adis16133_timeouts),
>  	},
>  	[ID_ADIS16136] = {
>  		.precision = IIO_DEGREE_TO_RAD(450),
>  		.fullscale = 24623,
> -		.timeouts = &adis16136_timeouts,
> +		.adis_data = ADIS16136_DATA(&adis16136_timeouts),
>  	},
>  	[ID_ADIS16137] = {
>  		.precision = IIO_DEGREE_TO_RAD(1000),
>  		.fullscale = 24609,
> -		.timeouts = &adis16136_timeouts,
> +		.adis_data = ADIS16136_DATA(&adis16136_timeouts),
>  	},
>  };
>  
> -static struct adis_data *adis16136_adis_data_alloc(struct adis16136 *st,
> -						   struct device *dev)
> -{
> -	struct adis_data *data;
> -
> -	data = devm_kmalloc(dev, sizeof(struct adis_data), GFP_KERNEL);
> -	if (!data)
> -		return ERR_PTR(-ENOMEM);
> -
> -	memcpy(data, &adis16136_data, sizeof(*data));
> -
> -	data->timeouts = st->chip_info->timeouts;
> -
> -	return data;
> -}
> -
>  static int adis16136_probe(struct spi_device *spi)
>  {
>  	const struct spi_device_id *id = spi_get_device_id(spi);
> @@ -565,9 +548,7 @@ static int adis16136_probe(struct spi_device *spi)
>  	indio_dev->info = &adis16136_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	adis16136_data = adis16136_adis_data_alloc(adis16136, &spi->dev);
> -	if (IS_ERR(adis16136_data))
> -		return PTR_ERR(adis16136_data);
> +	adis16136_data = &adis16136->chip_info->adis_data;
>  
>  	ret = adis_init(&adis16136->adis, indio_dev, spi, adis16136_data);
>  	if (ret)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup()
  2020-02-10 13:26 ` [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup() Alexandru Ardelean
@ 2020-02-14 14:31   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:31 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:01 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> This change splits the __adis_initial_startup() away from
> adis_initial_startup(). The unlocked version can be used in certain calls
> during probe, where races won't happen since the ADIS driver may not be
> registered yet with IIO.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.  Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis.c       | 14 ++++----------
>  include/linux/iio/imu/adis.h | 13 ++++++++++++-
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 022bb54fb748..e4897dad34ab 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -365,7 +365,7 @@ static int adis_self_test(struct adis *adis)
>  }
>  
>  /**
> - * adis_inital_startup() - Performs device self-test
> + * __adis_initial_startup() - Device initial setup
>   * @adis: The adis device
>   *
>   * Returns 0 if the device is operational, a negative error code otherwise.
> @@ -373,28 +373,22 @@ static int adis_self_test(struct adis *adis)
>   * This function should be called early on in the device initialization sequence
>   * to ensure that the device is in a sane and known state and that it is usable.
>   */
> -int adis_initial_startup(struct adis *adis)
> +int __adis_initial_startup(struct adis *adis)
>  {
>  	int ret;
>  
> -	mutex_lock(&adis->state_lock);
> -
>  	ret = adis_self_test(adis);
>  	if (ret) {
>  		dev_err(&adis->spi->dev, "Self-test failed, trying reset.\n");
>  		__adis_reset(adis);
>  		ret = adis_self_test(adis);
> -		if (ret) {
> +		if (ret)
>  			dev_err(&adis->spi->dev, "Second self-test failed, giving up.\n");
> -			goto out_unlock;
> -		}
>  	}
>  
> -out_unlock:
> -	mutex_unlock(&adis->state_lock);
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(adis_initial_startup);
> +EXPORT_SYMBOL_GPL(__adis_initial_startup);
>  
>  /**
>   * adis_single_conversion() - Performs a single sample conversion
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index d2fcf45b4cef..15e75670f923 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -297,6 +297,7 @@ static inline int adis_read_reg_32(struct adis *adis, unsigned int reg,
>  
>  int adis_enable_irq(struct adis *adis, bool enable);
>  int __adis_check_status(struct adis *adis);
> +int __adis_initial_startup(struct adis *adis);
>  
>  static inline int adis_check_status(struct adis *adis)
>  {
> @@ -309,7 +310,17 @@ static inline int adis_check_status(struct adis *adis)
>  	return ret;
>  }
>  
> -int adis_initial_startup(struct adis *adis);
> +/* locked version of __adis_initial_startup() */
> +static inline int adis_initial_startup(struct adis *adis)
> +{
> +	int ret;
> +
> +	mutex_lock(&adis->state_lock);
> +	ret = __adis_initial_startup(adis);
> +	mutex_unlock(&adis->state_lock);
> +
> +	return ret;
> +}
>  
>  int adis_single_conversion(struct iio_dev *indio_dev,
>  	const struct iio_chan_spec *chan, unsigned int error_mask,


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable
  2020-02-10 13:26 ` [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable Alexandru Ardelean
@ 2020-02-14 14:33   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:33 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:02 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> This patch adds a dedicated self_test_reg variable. This is also a step
> to let new drivers make use of `adis_initial_startup()`. Some devices
> use MSG_CTRL reg to request a self_test command while others use the
> GLOB_CMD register.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.

thanks,
> ---
>  drivers/iio/accel/adis16201.c         | 1 +
>  drivers/iio/accel/adis16209.c         | 1 +
>  drivers/iio/gyro/adis16136.c          | 1 +
>  drivers/iio/gyro/adis16260.c          | 1 +
>  drivers/iio/imu/adis.c                | 6 +++---
>  drivers/iio/imu/adis16400.c           | 1 +
>  drivers/iio/imu/adis16460.c           | 2 ++
>  drivers/iio/imu/adis16480.c           | 1 +
>  drivers/staging/iio/accel/adis16203.c | 1 +
>  drivers/staging/iio/accel/adis16240.c | 1 +
>  include/linux/iio/imu/adis.h          | 2 ++
>  11 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> index 0f0f27a8184e..4154e7396bbe 100644
> --- a/drivers/iio/accel/adis16201.c
> +++ b/drivers/iio/accel/adis16201.c
> @@ -246,6 +246,7 @@ static const struct adis_data adis16201_data = {
>  	.diag_stat_reg = ADIS16201_DIAG_STAT_REG,
>  
>  	.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> +	.self_test_reg = ADIS16201_MSC_CTRL_REG,
>  	.self_test_no_autoclear = true,
>  	.timeouts = &adis16201_timeouts,
>  
> diff --git a/drivers/iio/accel/adis16209.c b/drivers/iio/accel/adis16209.c
> index c6dbd2424e10..31d45e7c5485 100644
> --- a/drivers/iio/accel/adis16209.c
> +++ b/drivers/iio/accel/adis16209.c
> @@ -256,6 +256,7 @@ static const struct adis_data adis16209_data = {
>  	.diag_stat_reg = ADIS16209_STAT_REG,
>  
>  	.self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN,
> +	.self_test_reg = ADIS16209_MSC_CTRL_REG,
>  	.self_test_no_autoclear = true,
>  	.timeouts = &adis16209_timeouts,
>  
> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
> index 1db1131e5c67..a4c967a5fc5c 100644
> --- a/drivers/iio/gyro/adis16136.c
> +++ b/drivers/iio/gyro/adis16136.c
> @@ -471,6 +471,7 @@ static const char * const adis16136_status_error_msgs[] = {
>  	.diag_stat_reg = ADIS16136_REG_DIAG_STAT,			\
>  	.glob_cmd_reg = ADIS16136_REG_GLOB_CMD,				\
>  	.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,				\
> +	.self_test_reg = ADIS16136_REG_MSC_CTRL,			\
>  	.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,			\
>  	.read_delay = 10,						\
>  	.write_delay = 10,						\
> diff --git a/drivers/iio/gyro/adis16260.c b/drivers/iio/gyro/adis16260.c
> index be09b3e5910c..9823573e811a 100644
> --- a/drivers/iio/gyro/adis16260.c
> +++ b/drivers/iio/gyro/adis16260.c
> @@ -346,6 +346,7 @@ static const struct adis_data adis16260_data = {
>  	.diag_stat_reg = ADIS16260_DIAG_STAT,
>  
>  	.self_test_mask = ADIS16260_MSC_CTRL_MEM_TEST,
> +	.self_test_reg = ADIS16260_MSC_CTRL,
>  	.timeouts = &adis16260_timeouts,
>  
>  	.status_error_msgs = adis1620_status_error_msgs,
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index e4897dad34ab..f7845a90f376 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -346,8 +346,8 @@ static int adis_self_test(struct adis *adis)
>  	int ret;
>  	const struct adis_timeout *timeouts = adis->data->timeouts;
>  
> -	ret = __adis_write_reg_16(adis, adis->data->msc_ctrl_reg,
> -			adis->data->self_test_mask);
> +	ret = __adis_write_reg_16(adis, adis->data->self_test_reg,
> +				  adis->data->self_test_mask);
>  	if (ret) {
>  		dev_err(&adis->spi->dev, "Failed to initiate self test: %d\n",
>  			ret);
> @@ -359,7 +359,7 @@ static int adis_self_test(struct adis *adis)
>  	ret = __adis_check_status(adis);
>  
>  	if (adis->data->self_test_no_autoclear)
> -		__adis_write_reg_16(adis, adis->data->msc_ctrl_reg, 0x00);
> +		__adis_write_reg_16(adis, adis->data->self_test_reg, 0x00);
>  
>  	return ret;
>  }
> diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
> index 1c0770e03ec9..05e70c1c4835 100644
> --- a/drivers/iio/imu/adis16400.c
> +++ b/drivers/iio/imu/adis16400.c
> @@ -956,6 +956,7 @@ static const char * const adis16400_status_error_msgs[] = {
>  	.read_delay = 50,						\
>  	.write_delay = 50,						\
>  	.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,			\
> +	.self_test_reg = ADIS16400_MSC_CTRL,				\
>  	.status_error_msgs = adis16400_status_error_msgs,		\
>  	.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |	\
>  		BIT(ADIS16400_DIAG_STAT_YACCL_FAIL) |			\
> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> index 9539cfe4a259..42fa473c6d81 100644
> --- a/drivers/iio/imu/adis16460.c
> +++ b/drivers/iio/imu/adis16460.c
> @@ -392,6 +392,8 @@ static const struct adis_timeout adis16460_timeouts = {
>  static const struct adis_data adis16460_data = {
>  	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
>  	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
> +	.self_test_mask = BIT(2),
> +	.self_test_reg = ADIS16460_REG_GLOB_CMD,
>  	.has_paging = false,
>  	.read_delay = 5,
>  	.write_delay = 5,
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 4c4de1b62769..acbe1701fc2d 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -830,6 +830,7 @@ static int adis16480_enable_irq(struct adis *adis, bool enable);
>  	.read_delay = 5,						\
>  	.write_delay = 5,						\
>  	.self_test_mask = BIT(1),					\
> +	.self_test_reg = ADIS16480_REG_GLOB_CMD,			\
>  	.status_error_msgs = adis16480_status_error_msgs,		\
>  	.status_error_mask = BIT(ADIS16480_DIAG_STAT_XGYRO_FAIL) |	\
>  		BIT(ADIS16480_DIAG_STAT_YGYRO_FAIL) |			\
> diff --git a/drivers/staging/iio/accel/adis16203.c b/drivers/staging/iio/accel/adis16203.c
> index 39dfe3f7f254..fef52d9b5346 100644
> --- a/drivers/staging/iio/accel/adis16203.c
> +++ b/drivers/staging/iio/accel/adis16203.c
> @@ -250,6 +250,7 @@ static const struct adis_data adis16203_data = {
>  	.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,
>  
> diff --git a/drivers/staging/iio/accel/adis16240.c b/drivers/staging/iio/accel/adis16240.c
> index 39eb8364aa95..8bd35c6c56a1 100644
> --- a/drivers/staging/iio/accel/adis16240.c
> +++ b/drivers/staging/iio/accel/adis16240.c
> @@ -373,6 +373,7 @@ static const struct adis_data adis16240_data = {
>  	.diag_stat_reg = ADIS16240_DIAG_STAT,
>  
>  	.self_test_mask = ADIS16240_MSC_CTRL_SELF_TEST_EN,
> +	.self_test_reg = ADIS16240_MSC_CTRL,
>  	.self_test_no_autoclear = true,
>  	.timeouts = &adis16240_timeouts,
>  
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index 15e75670f923..b7feca4e5f26 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -41,6 +41,7 @@ struct adis_timeout {
>   * @glob_cmd_reg: Register address of the GLOB_CMD register
>   * @msc_ctrl_reg: Register address of the MSC_CTRL register
>   * @diag_stat_reg: Register address of the DIAG_STAT register
> + * @self_test_reg: Register address to request self test command
>   * @status_error_msgs: Array of error messgaes
>   * @status_error_mask:
>   * @timeouts: Chip specific delays
> @@ -55,6 +56,7 @@ struct adis_data {
>  	unsigned int diag_stat_reg;
>  
>  	unsigned int self_test_mask;
> +	unsigned int self_test_reg;
>  	bool self_test_no_autoclear;
>  	const struct adis_timeout *timeouts;
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup
  2020-02-10 13:26 ` [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup Alexandru Ardelean
@ 2020-02-14 14:39   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:39 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:03 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> All the ADIS devices perform, at the beginning, a self test to make sure
> the device is in a sane state. Previously, the logic was that the self-test
> was performed in adis_initial_startup() and if that failed a reset was done
> and then a self-test was attempted again.
> 
> This change unifies the reset mechanism under the adis_initial_startup()
> call. A HW reset will be done if  GPIO is configured, or a SW reset
> otherwise. This should make sure that the chip is in a sane state for
> self-test. Once the reset is done, the self-test operation will be
> performed. If anything goes wrong with self-test, the driver should just
> bail/error-out (i.e. no second attempt). The chip would likely not be a in
> a sane state state if the self-test fails after a reset.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
A very small tweak in here..

Tweak made and patch applied to the togreg branch of iio.git and pushed out
as testing for the autobuilders to poke at it.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/Kconfig |  1 +
>  drivers/iio/imu/adis.c  | 29 ++++++++++++++++++++++-------
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 60bb1029e759..63036cf473c7 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -85,6 +85,7 @@ endmenu
>  
>  config IIO_ADIS_LIB
>  	tristate
> +	depends on GPIOLIB
This is needed. If we don't have GPIOLIB the gpio_get
will return NULL and we'll carry on as if one wasn't supplied in the first
place which should work fine.

As a side note, you can't do depends inside something that is selected and
expect the kernel build system to notice.  If this was actually needed
you would have gotten build errors in some configurations.


>  	help
>  	  A set of IO helper functions for the Analog Devices ADIS* device family.
>  
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index f7845a90f376..0bd6e32cf577 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/mutex.h>
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> @@ -368,6 +369,10 @@ static int adis_self_test(struct adis *adis)
>   * __adis_initial_startup() - Device initial setup
>   * @adis: The adis device
>   *
> + * The function performs a HW reset via a reset pin that should be specified
> + * via GPIOLIB. If no pin is configured a SW reset will be performed.
> + * The RST pin for the ADIS devices should be configured as ACTIVE_LOW.
> + *
>   * Returns 0 if the device is operational, a negative error code otherwise.
>   *
>   * This function should be called early on in the device initialization sequence
> @@ -375,18 +380,28 @@ static int adis_self_test(struct adis *adis)
>   */
>  int __adis_initial_startup(struct adis *adis)
>  {
> +	const struct adis_timeout *timeouts = adis->data->timeouts;
> +	struct gpio_desc *gpio;
>  	int ret;
>  
> -	ret = adis_self_test(adis);
> -	if (ret) {
> -		dev_err(&adis->spi->dev, "Self-test failed, trying reset.\n");
> -		__adis_reset(adis);
> -		ret = adis_self_test(adis);
> +	/* check if the device has rst pin low */
> +	gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
> +	if (IS_ERR(gpio))
> +		return PTR_ERR(gpio);
> +
> +	if (gpio) {
> +		gpiod_set_value_cansleep(gpio, 1);
> +		msleep(10);
> +		/* bring device out of reset */
> +		gpiod_set_value_cansleep(gpio, 0);
> +		msleep(timeouts->reset_ms);
> +	} else {
> +		ret = __adis_reset(adis);
>  		if (ret)
> -			dev_err(&adis->spi->dev, "Second self-test failed, giving up.\n");
> +			return ret;
>  	}
>  
> -	return ret;
> +	return adis_self_test(adis);
>  }
>  EXPORT_SYMBOL_GPL(__adis_initial_startup);
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup
  2020-02-10 13:26 ` [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup Alexandru Ardelean
@ 2020-02-14 14:41   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:41 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:04 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> Each driver/chip that wants to validate it's product id, can now
> specify a 'prod_id_reg' and an expected 'prod_id' value.
> The 'prod_id' value is intentionally left 0 (uninitialized). There aren't
> (yet) any product IDs with value 0; this enforces that both 'prod_id_reg'
> and 'prod_id' are specified.
> 
> At the very least, this routine validates that the SPI connection to the
> ADIS chip[s] works well.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied.  Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis.c       | 23 ++++++++++++++++++++++-
>  include/linux/iio/imu/adis.h |  5 +++++
>  2 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 0bd6e32cf577..a8afd01de4f3 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -373,6 +373,10 @@ static int adis_self_test(struct adis *adis)
>   * via GPIOLIB. If no pin is configured a SW reset will be performed.
>   * The RST pin for the ADIS devices should be configured as ACTIVE_LOW.
>   *
> + * After the self-test operation is performed, the function will also check
> + * that the product ID is as expected. This assumes that drivers providing
> + * 'prod_id_reg' will also provide the 'prod_id'.
> + *
>   * Returns 0 if the device is operational, a negative error code otherwise.
>   *
>   * This function should be called early on in the device initialization sequence
> @@ -382,6 +386,7 @@ int __adis_initial_startup(struct adis *adis)
>  {
>  	const struct adis_timeout *timeouts = adis->data->timeouts;
>  	struct gpio_desc *gpio;
> +	uint16_t prod_id;
>  	int ret;
>  
>  	/* check if the device has rst pin low */
> @@ -401,7 +406,23 @@ int __adis_initial_startup(struct adis *adis)
>  			return ret;
>  	}
>  
> -	return adis_self_test(adis);
> +	ret = adis_self_test(adis);
> +	if (ret)
> +		return ret;
> +
> +	if (!adis->data->prod_id_reg)
> +		return 0;
> +
> +	ret = adis_read_reg_16(adis, adis->data->prod_id_reg, &prod_id);
> +	if (ret)
> +		return ret;
> +
> +	if (prod_id != adis->data->prod_id)
> +		dev_warn(&adis->spi->dev,
> +			 "Device ID(%u) and product ID(%u) do not match.",
> +			 adis->data->prod_id, prod_id);
> +
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(__adis_initial_startup);
>  
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index b7feca4e5f26..ac7cfd073804 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
> @@ -41,6 +41,8 @@ struct adis_timeout {
>   * @glob_cmd_reg: Register address of the GLOB_CMD register
>   * @msc_ctrl_reg: Register address of the MSC_CTRL register
>   * @diag_stat_reg: Register address of the DIAG_STAT register
> + * @prod_id_reg: Register address of the PROD_ID register
> + * @prod_id: Product ID code that should be expected when reading @prod_id_reg
>   * @self_test_reg: Register address to request self test command
>   * @status_error_msgs: Array of error messgaes
>   * @status_error_mask:
> @@ -54,6 +56,9 @@ struct adis_data {
>  	unsigned int glob_cmd_reg;
>  	unsigned int msc_ctrl_reg;
>  	unsigned int diag_stat_reg;
> +	unsigned int prod_id_reg;
> +
> +	unsigned int prod_id;
>  
>  	unsigned int self_test_mask;
>  	unsigned int self_test_reg;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup
  2020-02-10 13:26 ` [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup Alexandru Ardelean
@ 2020-02-14 14:44   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:44 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:05 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> All actions done in `adis16480_initial_setup()` are now done in
> `__adis_initial_startup()` so, there's no need for code duplication.
> Furthermore, the call to `adis16480_initial_setup()` is done before any
> device configuration since the device will be reset if not already (via
> rst pin). This is actually fixing a potential bug since `adis_reset()` was
> being called after configuring the device which is obviously a problem.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied,

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16480.c | 80 +++++++++++--------------------------
>  1 file changed, 24 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index acbe1701fc2d..cfae0e4476e7 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -822,10 +822,12 @@ static const char * const adis16480_status_error_msgs[] = {
>  
>  static int adis16480_enable_irq(struct adis *adis, bool enable);
>  
> -#define ADIS16480_DATA(_timeouts)					\
> +#define ADIS16480_DATA(_prod_id, _timeouts)				\
>  {									\
>  	.diag_stat_reg = ADIS16480_REG_DIAG_STS,			\
>  	.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,				\
> +	.prod_id_reg = ADIS16480_REG_PROD_ID,				\
> +	.prod_id = (_prod_id),						\
>  	.has_paging = true,						\
>  	.read_delay = 5,						\
>  	.write_delay = 5,						\
> @@ -888,7 +890,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
> +		.adis_data = ADIS16480_DATA(16375, &adis16485_timeouts),
>  	},
>  	[ADIS16480] = {
>  		.channels = adis16480_channels,
> @@ -901,7 +903,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.adis_data = ADIS16480_DATA(&adis16480_timeouts),
> +		.adis_data = ADIS16480_DATA(16480, &adis16480_timeouts),
>  	},
>  	[ADIS16485] = {
>  		.channels = adis16485_channels,
> @@ -914,7 +916,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
> +		.adis_data = ADIS16480_DATA(16485, &adis16485_timeouts),
>  	},
>  	[ADIS16488] = {
>  		.channels = adis16480_channels,
> @@ -927,7 +929,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
>  		.filter_freqs = adis16480_def_filter_freqs,
> -		.adis_data = ADIS16480_DATA(&adis16485_timeouts),
> +		.adis_data = ADIS16480_DATA(16488, &adis16485_timeouts),
>  	},
>  	[ADIS16490] = {
>  		.channels = adis16485_channels,
> @@ -941,7 +943,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_timeouts),
> +		.adis_data = ADIS16480_DATA(16490, &adis16495_timeouts),
>  	},
>  	[ADIS16495_1] = {
>  		.channels = adis16485_channels,
> @@ -955,7 +957,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
>  	},
>  	[ADIS16495_2] = {
>  		.channels = adis16485_channels,
> @@ -969,7 +971,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
>  	},
>  	[ADIS16495_3] = {
>  		.channels = adis16485_channels,
> @@ -983,7 +985,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16495, &adis16495_1_timeouts),
>  	},
>  	[ADIS16497_1] = {
>  		.channels = adis16485_channels,
> @@ -997,7 +999,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
>  	},
>  	[ADIS16497_2] = {
>  		.channels = adis16485_channels,
> @@ -1011,7 +1013,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
>  	},
>  	[ADIS16497_3] = {
>  		.channels = adis16485_channels,
> @@ -1025,7 +1027,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.max_dec_rate = 4250,
>  		.filter_freqs = adis16495_def_filter_freqs,
>  		.has_pps_clk_mode = true,
> -		.adis_data = ADIS16480_DATA(&adis16495_1_timeouts),
> +		.adis_data = ADIS16480_DATA(16497, &adis16495_1_timeouts),
>  	},
>  };
>  
> @@ -1064,40 +1066,6 @@ static int adis16480_enable_irq(struct adis *adis, bool enable)
>  	return __adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
>  }
>  
> -static int adis16480_initial_setup(struct iio_dev *indio_dev)
> -{
> -	struct adis16480 *st = iio_priv(indio_dev);
> -	uint16_t prod_id;
> -	unsigned int device_id;
> -	int ret;
> -
> -	adis_reset(&st->adis);
> -	msleep(70);
> -
> -	ret = adis_write_reg_16(&st->adis, ADIS16480_REG_GLOB_CMD, BIT(1));
> -	if (ret)
> -		return ret;
> -	msleep(30);
> -
> -	ret = adis_check_status(&st->adis);
> -	if (ret)
> -		return ret;
> -
> -	ret = adis_read_reg_16(&st->adis, ADIS16480_REG_PROD_ID, &prod_id);
> -	if (ret)
> -		return ret;
> -
> -	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> -	if (ret != 1)
> -		return -EINVAL;
> -
> -	if (prod_id != device_id)
> -		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> -				device_id, prod_id);
> -
> -	return 0;
> -}
> -
>  static int adis16480_config_irq_pin(struct device_node *of_node,
>  				    struct adis16480 *st)
>  {
> @@ -1278,18 +1246,22 @@ static int adis16480_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
> +	ret = __adis_initial_startup(&st->adis);
>  	if (ret)
>  		return ret;
>  
> +	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
> +	if (ret)
> +		goto error_stop_device;
> +
>  	ret = adis16480_get_ext_clocks(st);
>  	if (ret)
> -		return ret;
> +		goto error_stop_device;
>  
>  	if (!IS_ERR_OR_NULL(st->ext_clk)) {
>  		ret = adis16480_ext_clk_config(st, spi->dev.of_node, true);
>  		if (ret)
> -			return ret;
> +			goto error_stop_device;
>  
>  		st->clk_freq = clk_get_rate(st->ext_clk);
>  		st->clk_freq *= 1000; /* micro */
> @@ -1301,24 +1273,20 @@ static int adis16480_probe(struct spi_device *spi)
>  	if (ret)
>  		goto error_clk_disable_unprepare;
>  
> -	ret = adis16480_initial_setup(indio_dev);
> -	if (ret)
> -		goto error_cleanup_buffer;
> -
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> -		goto error_stop_device;
> +		goto error_cleanup_buffer;
>  
>  	adis16480_debugfs_init(indio_dev);
>  
>  	return 0;
>  
> -error_stop_device:
> -	adis16480_stop_device(indio_dev);
>  error_cleanup_buffer:
>  	adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
>  error_clk_disable_unprepare:
>  	clk_disable_unprepare(st->ext_clk);
> +error_stop_device:
> +	adis16480_stop_device(indio_dev);
>  	return ret;
>  }
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 9/9] iio: adis16460: Make use of __adis_initial_startup
  2020-02-10 13:26 ` [PATCH v2 9/9] iio: adis16460: " Alexandru Ardelean
@ 2020-02-14 14:45   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2020-02-14 14:45 UTC (permalink / raw)
  To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel, nuno.sa

On Mon, 10 Feb 2020 15:26:06 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> All of the actions done in `adis16460_initial_setup()` are now done in
> `__adis_initial_startup()` so, there's no need for code duplication.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16460.c | 38 +++----------------------------------
>  1 file changed, 3 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> index 42fa473c6d81..0027683d0256 100644
> --- a/drivers/iio/imu/adis16460.c
> +++ b/drivers/iio/imu/adis16460.c
> @@ -333,40 +333,6 @@ static int adis16460_enable_irq(struct adis *adis, bool enable)
>  	return 0;
>  }
>  
> -static int adis16460_initial_setup(struct iio_dev *indio_dev)
> -{
> -	struct adis16460 *st = iio_priv(indio_dev);
> -	uint16_t prod_id;
> -	unsigned int device_id;
> -	int ret;
> -
> -	adis_reset(&st->adis);
> -	msleep(222);
> -
> -	ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
> -	if (ret)
> -		return ret;
> -	msleep(75);
> -
> -	ret = adis_check_status(&st->adis);
> -	if (ret)
> -		return ret;
> -
> -	ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
> -	if (ret)
> -		return ret;
> -
> -	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> -	if (ret != 1)
> -		return -EINVAL;
> -
> -	if (prod_id != device_id)
> -		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> -				device_id, prod_id);
> -
> -	return 0;
> -}
> -
>  #define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
>  #define ADIS16460_DIAG_STAT_FLASH_MEM	6
>  #define ADIS16460_DIAG_STAT_SELF_TEST	5
> @@ -392,6 +358,8 @@ static const struct adis_timeout adis16460_timeouts = {
>  static const struct adis_data adis16460_data = {
>  	.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
>  	.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
> +	.prod_id_reg = ADIS16460_REG_PROD_ID,
> +	.prod_id = 16460,
>  	.self_test_mask = BIT(2),
>  	.self_test_reg = ADIS16460_REG_GLOB_CMD,
>  	.has_paging = false,
> @@ -441,7 +409,7 @@ static int adis16460_probe(struct spi_device *spi)
>  
>  	adis16460_enable_irq(&st->adis, 0);
>  
> -	ret = adis16460_initial_setup(indio_dev);
> +	ret = __adis_initial_startup(&st->adis);
>  	if (ret)
>  		goto error_cleanup_buffer;
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-10 13:25 [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Alexandru Ardelean
2020-02-10 13:25 ` [PATCH v2 2/9] iio: imu: adis16400: " Alexandru Ardelean
2020-02-14 14:29   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 3/9] iio: gyro: adis16136: " Alexandru Ardelean
2020-02-14 14:30   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 4/9] iio: imu: adis: add unlocked __adis_initial_startup() Alexandru Ardelean
2020-02-14 14:31   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 5/9] iio: imu: adis: Add self_test_reg variable Alexandru Ardelean
2020-02-14 14:33   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 6/9] iio: imu: adis: Refactor adis_initial_startup Alexandru Ardelean
2020-02-14 14:39   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 7/9] iio: imu: adis: add support product ID check in adis_initial_startup Alexandru Ardelean
2020-02-14 14:41   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 8/9] iio: adis16480: Make use of __adis_initial_startup Alexandru Ardelean
2020-02-14 14:44   ` Jonathan Cameron
2020-02-10 13:26 ` [PATCH v2 9/9] iio: adis16460: " Alexandru Ardelean
2020-02-14 14:45   ` Jonathan Cameron
2020-02-14 14:28 ` [PATCH v2 1/9] iio: imu: adis16480: initialize adis_data statically Jonathan Cameron

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git