* [PATCH 00/11] staging: iio: accel: adis16201 driver cleanup
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
The following patch series cleans up miscellaneous code fragments and
then the driver is moved from staging to mainline IIO subsytem directory.
Note that the last patch to move driver is *not* generated using '-M' flag,
which is used for detecting renames, since it may help reviewers to
suggest more cleanups/fix while pointing inline to the patch, the
necessary changes. The framing of patch is made consistent with the datasheet
naming where possible.
For all the patches:
Suggested-by: Jonathan Cameron <jic23@kernel.org>
https://marc.info/?l=linux-iio&m=151775804702998&w=2
All the patches have been tested using 0-day test service with success.
Himanshu Jha (11):
Staging: iio: accel: Prefer alphabetical sequence of header files
Staging: iio: accel: Add a blank space before returns
Staging: iio: accel: Remove unnecessary comments
Staging: iio: accel: Rename few macro definitions
Staging: iio: accel: Add _REG suffix to registers
Staging: iio: accel: Reverse christmas tree
Staging: iio: accel: Adjust arguments to match open parentheses
Staging: iio: accel: Use switch statement than if-else
Staging: iio: accel: Use sign_extend32 function
Staging: iio: accel: Add comments about units in data read function
Staging: iio: accel: Move adis16201 driver out of staging subsystem
drivers/iio/accel/Kconfig | 12 ++
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++
drivers/staging/iio/accel/Kconfig | 12 --
drivers/staging/iio/accel/Makefile | 1 -
drivers/staging/iio/accel/adis16201.c | 381 ----------------------------------
6 files changed, 336 insertions(+), 394 deletions(-)
create mode 100644 drivers/iio/accel/adis16201.c
delete mode 100644 drivers/staging/iio/accel/adis16201.c
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 00/11] staging: iio: accel: adis16201 driver cleanup
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
The following patch series cleans up miscellaneous code fragments and
then the driver is moved from staging to mainline IIO subsytem directory.
Note that the last patch to move driver is *not* generated using '-M' flag,
which is used for detecting renames, since it may help reviewers to
suggest more cleanups/fix while pointing inline to the patch, the
necessary changes. The framing of patch is made consistent with the datasheet
naming where possible.
For all the patches:
Suggested-by: Jonathan Cameron <jic23@kernel.org>
https://marc.info/?l=linux-iio&m=151775804702998&w=2
All the patches have been tested using 0-day test service with success.
Himanshu Jha (11):
Staging: iio: accel: Prefer alphabetical sequence of header files
Staging: iio: accel: Add a blank space before returns
Staging: iio: accel: Remove unnecessary comments
Staging: iio: accel: Rename few macro definitions
Staging: iio: accel: Add _REG suffix to registers
Staging: iio: accel: Reverse christmas tree
Staging: iio: accel: Adjust arguments to match open parentheses
Staging: iio: accel: Use switch statement than if-else
Staging: iio: accel: Use sign_extend32 function
Staging: iio: accel: Add comments about units in data read function
Staging: iio: accel: Move adis16201 driver out of staging subsystem
drivers/iio/accel/Kconfig | 12 ++
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++
drivers/staging/iio/accel/Kconfig | 12 --
drivers/staging/iio/accel/Makefile | 1 -
drivers/staging/iio/accel/adis16201.c | 381 ----------------------------------
6 files changed, 336 insertions(+), 394 deletions(-)
create mode 100644 drivers/iio/accel/adis16201.c
delete mode 100644 drivers/staging/iio/accel/adis16201.c
--
2.7.4
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files
2018-03-05 7:49 ` Himanshu Jha
(?)
@ 2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:29 ` Jonathan Cameron
-1 siblings, 1 reply; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Arrange header files in alphabetical sequence to improve readability.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 2ebd275..0f6a204 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -7,13 +7,13 @@
*/
#include <linux/delay.h>
-#include <linux/mutex.h>
#include <linux/device.h>
#include <linux/kernel.h>
-#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/spi/spi.h>
#include <linux/sysfs.h>
-#include <linux/module.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 02/11] Staging: iio: accel: Add a blank space before returns
2018-03-05 7:49 ` Himanshu Jha
(?)
(?)
@ 2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:32 ` Jonathan Cameron
-1 siblings, 1 reply; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Adding a blank space before/after some returns improves readability.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 0f6a204..0fae8aa 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -232,6 +232,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
*val = val16;
return IIO_VAL_INT;
}
+
return -EINVAL;
}
@@ -262,6 +263,7 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
addr = adis16201_addresses[chan->scan_index];
return adis_write_reg_16(st, addr, val16);
}
+
return -EINVAL;
}
@@ -336,6 +338,7 @@ static int adis16201_probe(struct spi_device *spi)
ret = adis_init(st, indio_dev, spi, &adis16201_data);
if (ret)
return ret;
+
ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
if (ret)
return ret;
@@ -348,6 +351,7 @@ static int adis16201_probe(struct spi_device *spi)
ret = iio_device_register(indio_dev);
if (ret < 0)
goto error_cleanup_buffer_trigger;
+
return 0;
error_cleanup_buffer_trigger:
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Remove unnecessary comments since the definitions are pretty clear
with their macro names.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 82 +++++------------------------------
1 file changed, 10 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 0fae8aa..59c1166 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,87 +20,42 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY 220 /* ms */
+#define ADIS16201_STARTUP_DELAY 220
-/* Flash memory write count */
#define ADIS16201_FLASH_CNT 0x00
-/* Output, power supply */
+/* Data Output Register Information */
#define ADIS16201_SUPPLY_OUT 0x02
-
-/* Output, x-axis accelerometer */
#define ADIS16201_XACCL_OUT 0x04
-
-/* Output, y-axis accelerometer */
#define ADIS16201_YACCL_OUT 0x06
-
-/* Output, auxiliary ADC input */
#define ADIS16201_AUX_ADC 0x08
-
-/* Output, temperature */
#define ADIS16201_TEMP_OUT 0x0A
-
-/* Output, x-axis inclination */
#define ADIS16201_XINCL_OUT 0x0C
-
-/* Output, y-axis inclination */
#define ADIS16201_YINCL_OUT 0x0E
-/* Calibration, x-axis acceleration offset */
+/* Calibration Register Definition */
#define ADIS16201_XACCL_OFFS 0x10
-
-/* Calibration, y-axis acceleration offset */
#define ADIS16201_YACCL_OFFS 0x12
-
-/* x-axis acceleration scale factor */
#define ADIS16201_XACCL_SCALE 0x14
-
-/* y-axis acceleration scale factor */
#define ADIS16201_YACCL_SCALE 0x16
-
-/* Calibration, x-axis inclination offset */
#define ADIS16201_XINCL_OFFS 0x18
-
-/* Calibration, y-axis inclination offset */
#define ADIS16201_YINCL_OFFS 0x1A
-
-/* x-axis inclination scale factor */
#define ADIS16201_XINCL_SCALE 0x1C
-
-/* y-axis inclination scale factor */
#define ADIS16201_YINCL_SCALE 0x1E
-/* Alarm 1 amplitude threshold */
+/* Alarm Register Definition */
#define ADIS16201_ALM_MAG1 0x20
-
-/* Alarm 2 amplitude threshold */
#define ADIS16201_ALM_MAG2 0x22
-
-/* Alarm 1, sample period */
#define ADIS16201_ALM_SMPL1 0x24
-
-/* Alarm 2, sample period */
#define ADIS16201_ALM_SMPL2 0x26
-
-/* Alarm control */
#define ADIS16201_ALM_CTRL 0x28
-/* Auxiliary DAC data */
#define ADIS16201_AUX_DAC 0x30
-
-/* General-purpose digital input/output control */
#define ADIS16201_GPIO_CTRL 0x32
-
-/* Miscellaneous control */
#define ADIS16201_MSC_CTRL 0x34
-/* Internal sample period (rate) control */
#define ADIS16201_SMPL_PRD 0x36
-
-/* Operation, filter configuration */
#define ADIS16201_AVG_CNT 0x38
-
-/* Operation, sleep mode control */
#define ADIS16201_SLP_CNT 0x3A
/* Diagnostics, system status register */
@@ -109,42 +64,28 @@
/* Operation, system command register */
#define ADIS16201_GLOB_CMD 0x3E
-/* MSC_CTRL */
-/* Self-test enable */
#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-/* Data-ready enable: 1 = enabled, 0 = disabled */
#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-/* Data-ready polarity: 1 = active high, 0 = active low */
#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
-/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-/* DIAG_STAT */
-/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-/* SPI communications failure */
#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-/* Flash update failure */
#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
-/* Power supply above 3.625 V */
#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-/* Power supply below 3.15 V */
#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-/* GLOB_CMD */
-
#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
@@ -187,30 +128,30 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
case IIO_VOLTAGE:
if (chan->channel == 0) {
*val = 1;
- *val2 = 220000; /* 1.22 mV */
+ *val2 = 220000;
} else {
*val = 0;
- *val2 = 610000; /* 0.610 mV */
+ *val2 = 610000;
}
return IIO_VAL_INT_PLUS_MICRO;
case IIO_TEMP:
- *val = -470; /* 0.47 C */
+ *val = -470;
*val2 = 0;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_ACCEL:
*val = 0;
- *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg */
+ *val2 = IIO_G_TO_M_S_2(462400);
return IIO_VAL_INT_PLUS_NANO;
case IIO_INCLI:
*val = 0;
- *val2 = 100000; /* 0.1 degree */
+ *val2 = 100000;
return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
break;
case IIO_CHAN_INFO_OFFSET:
- *val = 25000 / -470 - 1278; /* 25 C = 1278 */
+ *val = 25000 / -470 - 1278;
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
switch (chan->type) {
@@ -318,13 +259,11 @@ static int adis16201_probe(struct spi_device *spi)
struct adis *st;
struct iio_dev *indio_dev;
- /* setup the industrialio driver allocated elements */
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
st = iio_priv(indio_dev);
- /* this is only used for removal purposes */
spi_set_drvdata(spi, indio_dev);
indio_dev->name = spi->dev.driver->name;
@@ -343,7 +282,6 @@ static int adis16201_probe(struct spi_device *spi)
if (ret)
return ret;
- /* Get the device into a sane initial state */
ret = adis_initial_startup(st);
if (ret)
goto error_cleanup_buffer_trigger;
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Remove unnecessary comments since the definitions are pretty clear
with their macro names.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 82 +++++------------------------------
1 file changed, 10 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 0fae8aa..59c1166 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,87 +20,42 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY 220 /* ms */
+#define ADIS16201_STARTUP_DELAY 220
-/* Flash memory write count */
#define ADIS16201_FLASH_CNT 0x00
-/* Output, power supply */
+/* Data Output Register Information */
#define ADIS16201_SUPPLY_OUT 0x02
-
-/* Output, x-axis accelerometer */
#define ADIS16201_XACCL_OUT 0x04
-
-/* Output, y-axis accelerometer */
#define ADIS16201_YACCL_OUT 0x06
-
-/* Output, auxiliary ADC input */
#define ADIS16201_AUX_ADC 0x08
-
-/* Output, temperature */
#define ADIS16201_TEMP_OUT 0x0A
-
-/* Output, x-axis inclination */
#define ADIS16201_XINCL_OUT 0x0C
-
-/* Output, y-axis inclination */
#define ADIS16201_YINCL_OUT 0x0E
-/* Calibration, x-axis acceleration offset */
+/* Calibration Register Definition */
#define ADIS16201_XACCL_OFFS 0x10
-
-/* Calibration, y-axis acceleration offset */
#define ADIS16201_YACCL_OFFS 0x12
-
-/* x-axis acceleration scale factor */
#define ADIS16201_XACCL_SCALE 0x14
-
-/* y-axis acceleration scale factor */
#define ADIS16201_YACCL_SCALE 0x16
-
-/* Calibration, x-axis inclination offset */
#define ADIS16201_XINCL_OFFS 0x18
-
-/* Calibration, y-axis inclination offset */
#define ADIS16201_YINCL_OFFS 0x1A
-
-/* x-axis inclination scale factor */
#define ADIS16201_XINCL_SCALE 0x1C
-
-/* y-axis inclination scale factor */
#define ADIS16201_YINCL_SCALE 0x1E
-/* Alarm 1 amplitude threshold */
+/* Alarm Register Definition */
#define ADIS16201_ALM_MAG1 0x20
-
-/* Alarm 2 amplitude threshold */
#define ADIS16201_ALM_MAG2 0x22
-
-/* Alarm 1, sample period */
#define ADIS16201_ALM_SMPL1 0x24
-
-/* Alarm 2, sample period */
#define ADIS16201_ALM_SMPL2 0x26
-
-/* Alarm control */
#define ADIS16201_ALM_CTRL 0x28
-/* Auxiliary DAC data */
#define ADIS16201_AUX_DAC 0x30
-
-/* General-purpose digital input/output control */
#define ADIS16201_GPIO_CTRL 0x32
-
-/* Miscellaneous control */
#define ADIS16201_MSC_CTRL 0x34
-/* Internal sample period (rate) control */
#define ADIS16201_SMPL_PRD 0x36
-
-/* Operation, filter configuration */
#define ADIS16201_AVG_CNT 0x38
-
-/* Operation, sleep mode control */
#define ADIS16201_SLP_CNT 0x3A
/* Diagnostics, system status register */
@@ -109,42 +64,28 @@
/* Operation, system command register */
#define ADIS16201_GLOB_CMD 0x3E
-/* MSC_CTRL */
-/* Self-test enable */
#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-/* Data-ready enable: 1 = enabled, 0 = disabled */
#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-/* Data-ready polarity: 1 = active high, 0 = active low */
#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
-/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-/* DIAG_STAT */
-/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-/* SPI communications failure */
#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-/* Flash update failure */
#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
-/* Power supply above 3.625 V */
#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-/* Power supply below 3.15 V */
#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-/* GLOB_CMD */
-
#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
@@ -187,30 +128,30 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
case IIO_VOLTAGE:
if (chan->channel == 0) {
*val = 1;
- *val2 = 220000; /* 1.22 mV */
+ *val2 = 220000;
} else {
*val = 0;
- *val2 = 610000; /* 0.610 mV */
+ *val2 = 610000;
}
return IIO_VAL_INT_PLUS_MICRO;
case IIO_TEMP:
- *val = -470; /* 0.47 C */
+ *val = -470;
*val2 = 0;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_ACCEL:
*val = 0;
- *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg */
+ *val2 = IIO_G_TO_M_S_2(462400);
return IIO_VAL_INT_PLUS_NANO;
case IIO_INCLI:
*val = 0;
- *val2 = 100000; /* 0.1 degree */
+ *val2 = 100000;
return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
break;
case IIO_CHAN_INFO_OFFSET:
- *val = 25000 / -470 - 1278; /* 25 C = 1278 */
+ *val = 25000 / -470 - 1278;
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
switch (chan->type) {
@@ -318,13 +259,11 @@ static int adis16201_probe(struct spi_device *spi)
struct adis *st;
struct iio_dev *indio_dev;
- /* setup the industrialio driver allocated elements */
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
st = iio_priv(indio_dev);
- /* this is only used for removal purposes */
spi_set_drvdata(spi, indio_dev);
indio_dev->name = spi->dev.driver->name;
@@ -343,7 +282,6 @@ static int adis16201_probe(struct spi_device *spi)
if (ret)
return ret;
- /* Get the device into a sane initial state */
ret = adis_initial_startup(st);
if (ret)
goto error_cleanup_buffer_trigger;
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 04/11] Staging: iio: accel: Rename few macro definitions
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Rename few macros with appropriate names specifying their usage/function.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 59c1166..445cb56 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,9 +20,8 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY 220
-
-#define ADIS16201_FLASH_CNT 0x00
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
/* Data Output Register Information */
#define ADIS16201_SUPPLY_OUT 0x02
@@ -69,7 +68,7 @@
#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
@@ -80,14 +79,14 @@
#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
#define ADIS16201_ERROR_ACTIVE BIT(14)
@@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = {
static const char * const adis16201_status_error_msgs[] = {
[ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
- [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
+ [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
[ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
[ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
};
@@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = {
.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
.self_test_no_autoclear = true,
- .startup_delay = ADIS16201_STARTUP_DELAY,
+ .startup_delay = ADIS16201_STARTUP_DELAY_MS,
.status_error_msgs = adis16201_status_error_msgs,
.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
+ BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
};
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 04/11] Staging: iio: accel: Rename few macro definitions
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Rename few macros with appropriate names specifying their usage/function.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 59c1166..445cb56 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,9 +20,8 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY 220
-
-#define ADIS16201_FLASH_CNT 0x00
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
/* Data Output Register Information */
#define ADIS16201_SUPPLY_OUT 0x02
@@ -69,7 +68,7 @@
#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
@@ -80,14 +79,14 @@
#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
#define ADIS16201_ERROR_ACTIVE BIT(14)
@@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = {
static const char * const adis16201_status_error_msgs[] = {
[ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
- [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
+ [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
[ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
[ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
};
@@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = {
.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
.self_test_no_autoclear = true,
- .startup_delay = ADIS16201_STARTUP_DELAY,
+ .startup_delay = ADIS16201_STARTUP_DELAY_MS,
.status_error_msgs = adis16201_status_error_msgs,
.status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
+ BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
};
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Addition of _REG suffix to the register definitions allows a distinction
between registers and register fields. The various registers and its field
bits are grouped together to improve readability and easy indentification.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 133 ++++++++++++++++------------------
1 file changed, 61 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 445cb56..476b1c3 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,75 +20,64 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY_MS 220
-#define ADIS16201_FLASH_CNT 0x00
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
/* Data Output Register Information */
-#define ADIS16201_SUPPLY_OUT 0x02
-#define ADIS16201_XACCL_OUT 0x04
-#define ADIS16201_YACCL_OUT 0x06
-#define ADIS16201_AUX_ADC 0x08
-#define ADIS16201_TEMP_OUT 0x0A
-#define ADIS16201_XINCL_OUT 0x0C
-#define ADIS16201_YINCL_OUT 0x0E
+#define ADIS16201_SUPPLY_OUT_REG 0x02
+#define ADIS16201_XACCL_OUT_REG 0x04
+#define ADIS16201_YACCL_OUT_REG 0x06
+#define ADIS16201_AUX_ADC_REG 0x08
+#define ADIS16201_TEMP_OUT_REG 0x0A
+#define ADIS16201_XINCL_OUT_REG 0x0C
+#define ADIS16201_YINCL_OUT_REG 0x0E
/* Calibration Register Definition */
-#define ADIS16201_XACCL_OFFS 0x10
-#define ADIS16201_YACCL_OFFS 0x12
-#define ADIS16201_XACCL_SCALE 0x14
-#define ADIS16201_YACCL_SCALE 0x16
-#define ADIS16201_XINCL_OFFS 0x18
-#define ADIS16201_YINCL_OFFS 0x1A
-#define ADIS16201_XINCL_SCALE 0x1C
-#define ADIS16201_YINCL_SCALE 0x1E
+#define ADIS16201_XACCL_OFFS_REG 0x10
+#define ADIS16201_YACCL_OFFS_REG 0x12
+#define ADIS16201_XACCL_SCALE_REG 0x14
+#define ADIS16201_YACCL_SCALE_REG 0x16
+#define ADIS16201_XINCL_OFFS_REG 0x18
+#define ADIS16201_YINCL_OFFS_REG 0x1A
+#define ADIS16201_XINCL_SCALE_REG 0x1C
+#define ADIS16201_YINCL_SCALE_REG 0x1E
/* Alarm Register Definition */
-#define ADIS16201_ALM_MAG1 0x20
-#define ADIS16201_ALM_MAG2 0x22
-#define ADIS16201_ALM_SMPL1 0x24
-#define ADIS16201_ALM_SMPL2 0x26
-#define ADIS16201_ALM_CTRL 0x28
-
-#define ADIS16201_AUX_DAC 0x30
-#define ADIS16201_GPIO_CTRL 0x32
-#define ADIS16201_MSC_CTRL 0x34
-
-#define ADIS16201_SMPL_PRD 0x36
-#define ADIS16201_AVG_CNT 0x38
-#define ADIS16201_SLP_CNT 0x3A
+#define ADIS16201_ALM_MAG1_REG 0x20
+#define ADIS16201_ALM_MAG2_REG 0x22
+#define ADIS16201_ALM_SMPL1_REG 0x24
+#define ADIS16201_ALM_SMPL2_REG 0x26
+#define ADIS16201_ALM_CTRL_REG 0x28
+
+#define ADIS16201_AUX_DAC_REG 0x30
+#define ADIS16201_GPIO_CTRL_REG 0x32
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_SMPL_PRD_REG 0x36
+#define ADIS16201_AVG_CNT_REG 0x38
+#define ADIS16201_SLP_CNT_REG 0x3A
+
+/* Miscellaneous Control Register Definition */
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
+#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT 0x3C
+#define ADIS16201_DIAG_STAT_REG 0x3C
+#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
+#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
+#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
+#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
+#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
/* Operation, system command register */
-#define ADIS16201_GLOB_CMD 0x3E
-
-
-#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-
-#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
-
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-
-
-#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-
-#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-
-#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
-
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-
-#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
+#define ADIS16201_GLOB_CMD_REG 0x3E
+#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
-#define ADIS16201_ERROR_ACTIVE BIT(14)
+#define ADIS16201_ERROR_ACTIVE BIT(14)
enum adis16201_scan {
ADIS16201_SCAN_ACC_X,
@@ -101,10 +90,10 @@ enum adis16201_scan {
};
static const u8 adis16201_addresses[] = {
- [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS,
- [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS,
- [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS,
- [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS,
+ [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
+ [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
};
static int adis16201_read_raw(struct iio_dev *indio_dev,
@@ -208,16 +197,16 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
}
static const struct iio_chan_spec adis16201_channels[] = {
- ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT, ADIS16201_SCAN_SUPPLY, 0, 12),
- ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT, ADIS16201_SCAN_TEMP, 0, 12),
- ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT, ADIS16201_SCAN_ACC_X,
+ ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
+ ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+ ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT, ADIS16201_SCAN_ACC_Y,
+ ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC, ADIS16201_SCAN_AUX_ADC, 0, 12),
- ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT, ADIS16201_SCAN_INCLI_X,
+ ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+ ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT, ADIS16201_SCAN_INCLI_Y,
+ ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
IIO_CHAN_SOFT_TIMESTAMP(7)
};
@@ -237,9 +226,9 @@ static const char * const adis16201_status_error_msgs[] = {
static const struct adis_data adis16201_data = {
.read_delay = 20,
- .msc_ctrl_reg = ADIS16201_MSC_CTRL,
- .glob_cmd_reg = ADIS16201_GLOB_CMD,
- .diag_stat_reg = ADIS16201_DIAG_STAT,
+ .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
+ .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
+ .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
.self_test_no_autoclear = true,
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Addition of _REG suffix to the register definitions allows a distinction
between registers and register fields. The various registers and its field
bits are grouped together to improve readability and easy indentification.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 133 ++++++++++++++++------------------
1 file changed, 61 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 445cb56..476b1c3 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -20,75 +20,64 @@
#include <linux/iio/buffer.h>
#include <linux/iio/imu/adis.h>
-#define ADIS16201_STARTUP_DELAY_MS 220
-#define ADIS16201_FLASH_CNT 0x00
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
/* Data Output Register Information */
-#define ADIS16201_SUPPLY_OUT 0x02
-#define ADIS16201_XACCL_OUT 0x04
-#define ADIS16201_YACCL_OUT 0x06
-#define ADIS16201_AUX_ADC 0x08
-#define ADIS16201_TEMP_OUT 0x0A
-#define ADIS16201_XINCL_OUT 0x0C
-#define ADIS16201_YINCL_OUT 0x0E
+#define ADIS16201_SUPPLY_OUT_REG 0x02
+#define ADIS16201_XACCL_OUT_REG 0x04
+#define ADIS16201_YACCL_OUT_REG 0x06
+#define ADIS16201_AUX_ADC_REG 0x08
+#define ADIS16201_TEMP_OUT_REG 0x0A
+#define ADIS16201_XINCL_OUT_REG 0x0C
+#define ADIS16201_YINCL_OUT_REG 0x0E
/* Calibration Register Definition */
-#define ADIS16201_XACCL_OFFS 0x10
-#define ADIS16201_YACCL_OFFS 0x12
-#define ADIS16201_XACCL_SCALE 0x14
-#define ADIS16201_YACCL_SCALE 0x16
-#define ADIS16201_XINCL_OFFS 0x18
-#define ADIS16201_YINCL_OFFS 0x1A
-#define ADIS16201_XINCL_SCALE 0x1C
-#define ADIS16201_YINCL_SCALE 0x1E
+#define ADIS16201_XACCL_OFFS_REG 0x10
+#define ADIS16201_YACCL_OFFS_REG 0x12
+#define ADIS16201_XACCL_SCALE_REG 0x14
+#define ADIS16201_YACCL_SCALE_REG 0x16
+#define ADIS16201_XINCL_OFFS_REG 0x18
+#define ADIS16201_YINCL_OFFS_REG 0x1A
+#define ADIS16201_XINCL_SCALE_REG 0x1C
+#define ADIS16201_YINCL_SCALE_REG 0x1E
/* Alarm Register Definition */
-#define ADIS16201_ALM_MAG1 0x20
-#define ADIS16201_ALM_MAG2 0x22
-#define ADIS16201_ALM_SMPL1 0x24
-#define ADIS16201_ALM_SMPL2 0x26
-#define ADIS16201_ALM_CTRL 0x28
-
-#define ADIS16201_AUX_DAC 0x30
-#define ADIS16201_GPIO_CTRL 0x32
-#define ADIS16201_MSC_CTRL 0x34
-
-#define ADIS16201_SMPL_PRD 0x36
-#define ADIS16201_AVG_CNT 0x38
-#define ADIS16201_SLP_CNT 0x3A
+#define ADIS16201_ALM_MAG1_REG 0x20
+#define ADIS16201_ALM_MAG2_REG 0x22
+#define ADIS16201_ALM_SMPL1_REG 0x24
+#define ADIS16201_ALM_SMPL2_REG 0x26
+#define ADIS16201_ALM_CTRL_REG 0x28
+
+#define ADIS16201_AUX_DAC_REG 0x30
+#define ADIS16201_GPIO_CTRL_REG 0x32
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_SMPL_PRD_REG 0x36
+#define ADIS16201_AVG_CNT_REG 0x38
+#define ADIS16201_SLP_CNT_REG 0x3A
+
+/* Miscellaneous Control Register Definition */
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
+#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT 0x3C
+#define ADIS16201_DIAG_STAT_REG 0x3C
+#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
+#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
+#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
+#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
+#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
/* Operation, system command register */
-#define ADIS16201_GLOB_CMD 0x3E
-
-
-#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-
-#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
-
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-
-
-#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-
-#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-
-#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
-
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-
-#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
+#define ADIS16201_GLOB_CMD_REG 0x3E
+#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
-#define ADIS16201_ERROR_ACTIVE BIT(14)
+#define ADIS16201_ERROR_ACTIVE BIT(14)
enum adis16201_scan {
ADIS16201_SCAN_ACC_X,
@@ -101,10 +90,10 @@ enum adis16201_scan {
};
static const u8 adis16201_addresses[] = {
- [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS,
- [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS,
- [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS,
- [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS,
+ [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
+ [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
};
static int adis16201_read_raw(struct iio_dev *indio_dev,
@@ -208,16 +197,16 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
}
static const struct iio_chan_spec adis16201_channels[] = {
- ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT, ADIS16201_SCAN_SUPPLY, 0, 12),
- ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT, ADIS16201_SCAN_TEMP, 0, 12),
- ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT, ADIS16201_SCAN_ACC_X,
+ ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
+ ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+ ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT, ADIS16201_SCAN_ACC_Y,
+ ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC, ADIS16201_SCAN_AUX_ADC, 0, 12),
- ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT, ADIS16201_SCAN_INCLI_X,
+ ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+ ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT, ADIS16201_SCAN_INCLI_Y,
+ ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
IIO_CHAN_SOFT_TIMESTAMP(7)
};
@@ -237,9 +226,9 @@ static const char * const adis16201_status_error_msgs[] = {
static const struct adis_data adis16201_data = {
.read_delay = 20,
- .msc_ctrl_reg = ADIS16201_MSC_CTRL,
- .glob_cmd_reg = ADIS16201_GLOB_CMD,
- .diag_stat_reg = ADIS16201_DIAG_STAT,
+ .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
+ .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
+ .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
.self_test_no_autoclear = true,
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 06/11] Staging: iio: accel: Reverse christmas tree
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Prefer reverse christmas tree ordering of declarations to improve
readability.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 476b1c3..907fb85 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -243,9 +243,9 @@ static const struct adis_data adis16201_data = {
static int adis16201_probe(struct spi_device *spi)
{
- int ret;
- struct adis *st;
struct iio_dev *indio_dev;
+ struct adis *st;
+ int ret;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 06/11] Staging: iio: accel: Reverse christmas tree
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Prefer reverse christmas tree ordering of declarations to improve
readability.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 476b1c3..907fb85 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -243,9 +243,9 @@ static const struct adis_data adis16201_data = {
static int adis16201_probe(struct spi_device *spi)
{
- int ret;
- struct adis *st;
struct iio_dev *indio_dev;
+ struct adis *st;
+ int ret;
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev)
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 07/11] Staging: iio: accel: Adjust arguments to match open parentheses
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
In adis16201_read_raw() adjust an argument to match an open parentheses
using tabs.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 907fb85..1737708 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -110,7 +110,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
return adis_single_conversion(indio_dev, chan,
- ADIS16201_ERROR_ACTIVE, val);
+ ADIS16201_ERROR_ACTIVE, val);
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_VOLTAGE:
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 07/11] Staging: iio: accel: Adjust arguments to match open parentheses
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
In adis16201_read_raw() adjust an argument to match an open parentheses
using tabs.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 907fb85..1737708 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -110,7 +110,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_RAW:
return adis_single_conversion(indio_dev, chan,
- ADIS16201_ERROR_ACTIVE, val);
+ ADIS16201_ERROR_ACTIVE, val);
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_VOLTAGE:
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Use switch statement instead of if-else pair to explicitly match
the only two channels present.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 1737708..307d4ab 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_VOLTAGE:
- if (chan->channel == 0) {
+ switch (chan->channel) {
+ case 0:
*val = 1;
*val2 = 220000;
- } else {
+ break;
+ case 1:
*val = 0;
*val2 = 610000;
+ break;
}
return IIO_VAL_INT_PLUS_MICRO;
case IIO_TEMP:
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Use switch statement instead of if-else pair to explicitly match
the only two channels present.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 1737708..307d4ab 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_VOLTAGE:
- if (chan->channel == 0) {
+ switch (chan->channel) {
+ case 0:
*val = 1;
*val2 = 220000;
- } else {
+ break;
+ case 1:
*val = 0;
*val2 = 610000;
+ break;
}
return IIO_VAL_INT_PLUS_MICRO;
case IIO_TEMP:
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 09/11] Staging: iio: accel: Use sign_extend32 function
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Use sign_extned32() for 32 bit sign extending rather than hard coding the
same.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 307d4ab..8d795e2 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -159,9 +159,8 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
ret = adis_read_reg_16(st, addr, &val16);
if (ret)
return ret;
- val16 &= (1 << bits) - 1;
- val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
- *val = val16;
+
+ *val = sign_extend32(val16, bits - 1);
return IIO_VAL_INT;
}
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 09/11] Staging: iio: accel: Use sign_extend32 function
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Use sign_extned32() for 32 bit sign extending rather than hard coding the
same.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 307d4ab..8d795e2 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -159,9 +159,8 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
ret = adis_read_reg_16(st, addr, &val16);
if (ret)
return ret;
- val16 &= (1 << bits) - 1;
- val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
- *val = val16;
+
+ *val = sign_extend32(val16, bits - 1);
return IIO_VAL_INT;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Clarify the conversion and formation of resultant data in the
adis16201_read_raw() with sufficient comments.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 8d795e2..946c7b1 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
*val2 = 0;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_ACCEL:
+ /*
+ * IIO base unit for sensitivity of accelerometer
+ * is milli g.
+ * 1 LSB represents 0.244 mg.
+ */
*val = 0;
*val2 = IIO_G_TO_M_S_2(462400);
return IIO_VAL_INT_PLUS_NANO;
@@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
}
break;
case IIO_CHAN_INFO_OFFSET:
+ /*
+ * The raw ADC value is 0x4FE when the temperature
+ * is 25 degrees and the scale factor per milli
+ * degree celcius is -470.
+ */
*val = 25000 / -470 - 1278;
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Clarify the conversion and formation of resultant data in the
adis16201_read_raw() with sufficient comments.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/staging/iio/accel/adis16201.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
index 8d795e2..946c7b1 100644
--- a/drivers/staging/iio/accel/adis16201.c
+++ b/drivers/staging/iio/accel/adis16201.c
@@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
*val2 = 0;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_ACCEL:
+ /*
+ * IIO base unit for sensitivity of accelerometer
+ * is milli g.
+ * 1 LSB represents 0.244 mg.
+ */
*val = 0;
*val2 = IIO_G_TO_M_S_2(462400);
return IIO_VAL_INT_PLUS_NANO;
@@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
}
break;
case IIO_CHAN_INFO_OFFSET:
+ /*
+ * The raw ADC value is 0x4FE when the temperature
+ * is 25 degrees and the scale factor per milli
+ * degree celcius is -470.
+ */
*val = 25000 / -470 - 1278;
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 7:49 ` Himanshu Jha
-1 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, Himanshu Jha, pmeerw, knaack.h
Move the adis16201 driver out of staging directory and merge to the
mainline IIO subsystem.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/iio/accel/Kconfig | 12 ++
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++++++++
drivers/staging/iio/accel/Kconfig | 12 --
drivers/staging/iio/accel/Makefile | 1 -
drivers/staging/iio/accel/adis16201.c | 323 ----------------------------------
6 files changed, 336 insertions(+), 336 deletions(-)
create mode 100644 drivers/iio/accel/adis16201.c
delete mode 100644 drivers/staging/iio/accel/adis16201.c
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index c6d9517..9416c6f 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -5,6 +5,18 @@
menu "Accelerometers"
+config ADIS16201
+ tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
+ depends on SPI
+ select IIO_ADIS_LIB
+ select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
+ help
+ Say Y here to build support for Analog Devices adis16201 dual-axis
+ digital inclinometer and accelerometer.
+
+ To compile this driver as a module, say M here: the module will
+ be called adis16201.
+
config ADXL345
tristate
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 368aedb..7832ec9 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -4,6 +4,7 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_ADIS16201) += adis16201.o
obj-$(CONFIG_ADXL345) += adxl345_core.o
obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
new file mode 100644
index 0000000..946c7b1
--- /dev/null
+++ b/drivers/iio/accel/adis16201.c
@@ -0,0 +1,323 @@
+/*
+ * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/imu/adis.h>
+
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
+
+/* Data Output Register Information */
+#define ADIS16201_SUPPLY_OUT_REG 0x02
+#define ADIS16201_XACCL_OUT_REG 0x04
+#define ADIS16201_YACCL_OUT_REG 0x06
+#define ADIS16201_AUX_ADC_REG 0x08
+#define ADIS16201_TEMP_OUT_REG 0x0A
+#define ADIS16201_XINCL_OUT_REG 0x0C
+#define ADIS16201_YINCL_OUT_REG 0x0E
+
+/* Calibration Register Definition */
+#define ADIS16201_XACCL_OFFS_REG 0x10
+#define ADIS16201_YACCL_OFFS_REG 0x12
+#define ADIS16201_XACCL_SCALE_REG 0x14
+#define ADIS16201_YACCL_SCALE_REG 0x16
+#define ADIS16201_XINCL_OFFS_REG 0x18
+#define ADIS16201_YINCL_OFFS_REG 0x1A
+#define ADIS16201_XINCL_SCALE_REG 0x1C
+#define ADIS16201_YINCL_SCALE_REG 0x1E
+
+/* Alarm Register Definition */
+#define ADIS16201_ALM_MAG1_REG 0x20
+#define ADIS16201_ALM_MAG2_REG 0x22
+#define ADIS16201_ALM_SMPL1_REG 0x24
+#define ADIS16201_ALM_SMPL2_REG 0x26
+#define ADIS16201_ALM_CTRL_REG 0x28
+
+#define ADIS16201_AUX_DAC_REG 0x30
+#define ADIS16201_GPIO_CTRL_REG 0x32
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_SMPL_PRD_REG 0x36
+#define ADIS16201_AVG_CNT_REG 0x38
+#define ADIS16201_SLP_CNT_REG 0x3A
+
+/* Miscellaneous Control Register Definition */
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
+#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
+
+/* Diagnostics, system status register */
+#define ADIS16201_DIAG_STAT_REG 0x3C
+#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
+#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
+#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
+#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
+#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
+
+/* Operation, system command register */
+#define ADIS16201_GLOB_CMD_REG 0x3E
+#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
+
+#define ADIS16201_ERROR_ACTIVE BIT(14)
+
+enum adis16201_scan {
+ ADIS16201_SCAN_ACC_X,
+ ADIS16201_SCAN_ACC_Y,
+ ADIS16201_SCAN_INCLI_X,
+ ADIS16201_SCAN_INCLI_Y,
+ ADIS16201_SCAN_SUPPLY,
+ ADIS16201_SCAN_AUX_ADC,
+ ADIS16201_SCAN_TEMP,
+};
+
+static const u8 adis16201_addresses[] = {
+ [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
+ [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
+};
+
+static int adis16201_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2,
+ long mask)
+{
+ struct adis *st = iio_priv(indio_dev);
+ int ret;
+ int bits;
+ u8 addr;
+ s16 val16;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return adis_single_conversion(indio_dev, chan,
+ ADIS16201_ERROR_ACTIVE, val);
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ switch (chan->channel) {
+ case 0:
+ *val = 1;
+ *val2 = 220000;
+ break;
+ case 1:
+ *val = 0;
+ *val2 = 610000;
+ break;
+ }
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_TEMP:
+ *val = -470;
+ *val2 = 0;
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_ACCEL:
+ /*
+ * IIO base unit for sensitivity of accelerometer
+ * is milli g.
+ * 1 LSB represents 0.244 mg.
+ */
+ *val = 0;
+ *val2 = IIO_G_TO_M_S_2(462400);
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_INCLI:
+ *val = 0;
+ *val2 = 100000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ break;
+ case IIO_CHAN_INFO_OFFSET:
+ /*
+ * The raw ADC value is 0x4FE when the temperature
+ * is 25 degrees and the scale factor per milli
+ * degree celcius is -470.
+ */
+ *val = 25000 / -470 - 1278;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ bits = 12;
+ break;
+ case IIO_INCLI:
+ bits = 9;
+ break;
+ default:
+ return -EINVAL;
+ }
+ addr = adis16201_addresses[chan->scan_index];
+ ret = adis_read_reg_16(st, addr, &val16);
+ if (ret)
+ return ret;
+
+ *val = sign_extend32(val16, bits - 1);
+ return IIO_VAL_INT;
+ }
+
+ return -EINVAL;
+}
+
+static int adis16201_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct adis *st = iio_priv(indio_dev);
+ int bits;
+ s16 val16;
+ u8 addr;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBBIAS:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ bits = 12;
+ break;
+ case IIO_INCLI:
+ bits = 9;
+ break;
+ default:
+ return -EINVAL;
+ }
+ val16 = val & ((1 << bits) - 1);
+ addr = adis16201_addresses[chan->scan_index];
+ return adis_write_reg_16(st, addr, val16);
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_chan_spec adis16201_channels[] = {
+ ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
+ ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+ ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+ ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ IIO_CHAN_SOFT_TIMESTAMP(7)
+};
+
+static const struct iio_info adis16201_info = {
+ .read_raw = adis16201_read_raw,
+ .write_raw = adis16201_write_raw,
+ .update_scan_mode = adis_update_scan_mode,
+};
+
+static const char * const adis16201_status_error_msgs[] = {
+ [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
+ [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
+ [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
+ [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
+};
+
+static const struct adis_data adis16201_data = {
+ .read_delay = 20,
+ .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
+ .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
+ .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
+
+ .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
+ .self_test_no_autoclear = true,
+ .startup_delay = ADIS16201_STARTUP_DELAY_MS,
+
+ .status_error_msgs = adis16201_status_error_msgs,
+ .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
+ BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
+ BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
+ BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
+};
+
+static int adis16201_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct adis *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ indio_dev->name = spi->dev.driver->name;
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->info = &adis16201_info;
+
+ indio_dev->channels = adis16201_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = adis_init(st, indio_dev, spi, &adis16201_data);
+ if (ret)
+ return ret;
+
+ ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+ if (ret)
+ return ret;
+
+ ret = adis_initial_startup(st);
+ if (ret)
+ goto error_cleanup_buffer_trigger;
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0)
+ goto error_cleanup_buffer_trigger;
+
+ return 0;
+
+error_cleanup_buffer_trigger:
+ adis_cleanup_buffer_and_trigger(st, indio_dev);
+ return ret;
+}
+
+static int adis16201_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct adis *st = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ adis_cleanup_buffer_and_trigger(st, indio_dev);
+
+ return 0;
+}
+
+static struct spi_driver adis16201_driver = {
+ .driver = {
+ .name = "adis16201",
+ },
+ .probe = adis16201_probe,
+ .remove = adis16201_remove,
+};
+module_spi_driver(adis16201_driver);
+
+MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:adis16201");
diff --git a/drivers/staging/iio/accel/Kconfig b/drivers/staging/iio/accel/Kconfig
index c6b0f5e..2f61e21 100644
--- a/drivers/staging/iio/accel/Kconfig
+++ b/drivers/staging/iio/accel/Kconfig
@@ -3,18 +3,6 @@
#
menu "Accelerometers"
-config ADIS16201
- tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
- depends on SPI
- select IIO_ADIS_LIB
- select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
- help
- Say Y here to build support for Analog Devices adis16201 dual-axis
- digital inclinometer and accelerometer.
-
- To compile this driver as a module, say M here: the module will
- be called adis16201.
-
config ADIS16203
tristate "Analog Devices ADIS16203 Programmable 360 Degrees Inclinometer"
depends on SPI
diff --git a/drivers/staging/iio/accel/Makefile b/drivers/staging/iio/accel/Makefile
index 5d8ad21..7dd55410 100644
--- a/drivers/staging/iio/accel/Makefile
+++ b/drivers/staging/iio/accel/Makefile
@@ -2,7 +2,6 @@
# Makefile for industrial I/O accelerometer drivers
#
-obj-$(CONFIG_ADIS16201) += adis16201.o
obj-$(CONFIG_ADIS16203) += adis16203.o
obj-$(CONFIG_ADIS16209) += adis16209.o
obj-$(CONFIG_ADIS16240) += adis16240.o
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
deleted file mode 100644
index 946c7b1..0000000
--- a/drivers/staging/iio/accel/adis16201.c
+++ /dev/null
@@ -1,323 +0,0 @@
-/*
- * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
- *
- * Copyright 2010 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- */
-
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/slab.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/imu/adis.h>
-
-#define ADIS16201_STARTUP_DELAY_MS 220
-#define ADIS16201_FLASH_CNT 0x00
-
-/* Data Output Register Information */
-#define ADIS16201_SUPPLY_OUT_REG 0x02
-#define ADIS16201_XACCL_OUT_REG 0x04
-#define ADIS16201_YACCL_OUT_REG 0x06
-#define ADIS16201_AUX_ADC_REG 0x08
-#define ADIS16201_TEMP_OUT_REG 0x0A
-#define ADIS16201_XINCL_OUT_REG 0x0C
-#define ADIS16201_YINCL_OUT_REG 0x0E
-
-/* Calibration Register Definition */
-#define ADIS16201_XACCL_OFFS_REG 0x10
-#define ADIS16201_YACCL_OFFS_REG 0x12
-#define ADIS16201_XACCL_SCALE_REG 0x14
-#define ADIS16201_YACCL_SCALE_REG 0x16
-#define ADIS16201_XINCL_OFFS_REG 0x18
-#define ADIS16201_YINCL_OFFS_REG 0x1A
-#define ADIS16201_XINCL_SCALE_REG 0x1C
-#define ADIS16201_YINCL_SCALE_REG 0x1E
-
-/* Alarm Register Definition */
-#define ADIS16201_ALM_MAG1_REG 0x20
-#define ADIS16201_ALM_MAG2_REG 0x22
-#define ADIS16201_ALM_SMPL1_REG 0x24
-#define ADIS16201_ALM_SMPL2_REG 0x26
-#define ADIS16201_ALM_CTRL_REG 0x28
-
-#define ADIS16201_AUX_DAC_REG 0x30
-#define ADIS16201_GPIO_CTRL_REG 0x32
-#define ADIS16201_MSC_CTRL_REG 0x34
-#define ADIS16201_SMPL_PRD_REG 0x36
-#define ADIS16201_AVG_CNT_REG 0x38
-#define ADIS16201_SLP_CNT_REG 0x3A
-
-/* Miscellaneous Control Register Definition */
-#define ADIS16201_MSC_CTRL_REG 0x34
-#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-
-/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT_REG 0x3C
-#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-
-/* Operation, system command register */
-#define ADIS16201_GLOB_CMD_REG 0x3E
-#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
-
-#define ADIS16201_ERROR_ACTIVE BIT(14)
-
-enum adis16201_scan {
- ADIS16201_SCAN_ACC_X,
- ADIS16201_SCAN_ACC_Y,
- ADIS16201_SCAN_INCLI_X,
- ADIS16201_SCAN_INCLI_Y,
- ADIS16201_SCAN_SUPPLY,
- ADIS16201_SCAN_AUX_ADC,
- ADIS16201_SCAN_TEMP,
-};
-
-static const u8 adis16201_addresses[] = {
- [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
- [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
- [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
- [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
-};
-
-static int adis16201_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2,
- long mask)
-{
- struct adis *st = iio_priv(indio_dev);
- int ret;
- int bits;
- u8 addr;
- s16 val16;
-
- switch (mask) {
- case IIO_CHAN_INFO_RAW:
- return adis_single_conversion(indio_dev, chan,
- ADIS16201_ERROR_ACTIVE, val);
- case IIO_CHAN_INFO_SCALE:
- switch (chan->type) {
- case IIO_VOLTAGE:
- switch (chan->channel) {
- case 0:
- *val = 1;
- *val2 = 220000;
- break;
- case 1:
- *val = 0;
- *val2 = 610000;
- break;
- }
- return IIO_VAL_INT_PLUS_MICRO;
- case IIO_TEMP:
- *val = -470;
- *val2 = 0;
- return IIO_VAL_INT_PLUS_MICRO;
- case IIO_ACCEL:
- /*
- * IIO base unit for sensitivity of accelerometer
- * is milli g.
- * 1 LSB represents 0.244 mg.
- */
- *val = 0;
- *val2 = IIO_G_TO_M_S_2(462400);
- return IIO_VAL_INT_PLUS_NANO;
- case IIO_INCLI:
- *val = 0;
- *val2 = 100000;
- return IIO_VAL_INT_PLUS_MICRO;
- default:
- return -EINVAL;
- }
- break;
- case IIO_CHAN_INFO_OFFSET:
- /*
- * The raw ADC value is 0x4FE when the temperature
- * is 25 degrees and the scale factor per milli
- * degree celcius is -470.
- */
- *val = 25000 / -470 - 1278;
- return IIO_VAL_INT;
- case IIO_CHAN_INFO_CALIBBIAS:
- switch (chan->type) {
- case IIO_ACCEL:
- bits = 12;
- break;
- case IIO_INCLI:
- bits = 9;
- break;
- default:
- return -EINVAL;
- }
- addr = adis16201_addresses[chan->scan_index];
- ret = adis_read_reg_16(st, addr, &val16);
- if (ret)
- return ret;
-
- *val = sign_extend32(val16, bits - 1);
- return IIO_VAL_INT;
- }
-
- return -EINVAL;
-}
-
-static int adis16201_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
-{
- struct adis *st = iio_priv(indio_dev);
- int bits;
- s16 val16;
- u8 addr;
-
- switch (mask) {
- case IIO_CHAN_INFO_CALIBBIAS:
- switch (chan->type) {
- case IIO_ACCEL:
- bits = 12;
- break;
- case IIO_INCLI:
- bits = 9;
- break;
- default:
- return -EINVAL;
- }
- val16 = val & ((1 << bits) - 1);
- addr = adis16201_addresses[chan->scan_index];
- return adis_write_reg_16(st, addr, val16);
- }
-
- return -EINVAL;
-}
-
-static const struct iio_chan_spec adis16201_channels[] = {
- ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
- ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
- ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
- ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- IIO_CHAN_SOFT_TIMESTAMP(7)
-};
-
-static const struct iio_info adis16201_info = {
- .read_raw = adis16201_read_raw,
- .write_raw = adis16201_write_raw,
- .update_scan_mode = adis_update_scan_mode,
-};
-
-static const char * const adis16201_status_error_msgs[] = {
- [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
- [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
- [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
- [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
-};
-
-static const struct adis_data adis16201_data = {
- .read_delay = 20,
- .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
- .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
- .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
-
- .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
- .self_test_no_autoclear = true,
- .startup_delay = ADIS16201_STARTUP_DELAY_MS,
-
- .status_error_msgs = adis16201_status_error_msgs,
- .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
- BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
-};
-
-static int adis16201_probe(struct spi_device *spi)
-{
- struct iio_dev *indio_dev;
- struct adis *st;
- int ret;
-
- indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
- if (!indio_dev)
- return -ENOMEM;
-
- st = iio_priv(indio_dev);
- spi_set_drvdata(spi, indio_dev);
-
- indio_dev->name = spi->dev.driver->name;
- indio_dev->dev.parent = &spi->dev;
- indio_dev->info = &adis16201_info;
-
- indio_dev->channels = adis16201_channels;
- indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
- indio_dev->modes = INDIO_DIRECT_MODE;
-
- ret = adis_init(st, indio_dev, spi, &adis16201_data);
- if (ret)
- return ret;
-
- ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
- if (ret)
- return ret;
-
- ret = adis_initial_startup(st);
- if (ret)
- goto error_cleanup_buffer_trigger;
-
- ret = iio_device_register(indio_dev);
- if (ret < 0)
- goto error_cleanup_buffer_trigger;
-
- return 0;
-
-error_cleanup_buffer_trigger:
- adis_cleanup_buffer_and_trigger(st, indio_dev);
- return ret;
-}
-
-static int adis16201_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
- struct adis *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- adis_cleanup_buffer_and_trigger(st, indio_dev);
-
- return 0;
-}
-
-static struct spi_driver adis16201_driver = {
- .driver = {
- .name = "adis16201",
- },
- .probe = adis16201_probe,
- .remove = adis16201_remove,
-};
-module_spi_driver(adis16201_driver);
-
-MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("spi:adis16201");
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem
@ 2018-03-05 7:49 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-05 7:49 UTC (permalink / raw)
To: jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta, Himanshu Jha
Move the adis16201 driver out of staging directory and merge to the
mainline IIO subsystem.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/iio/accel/Kconfig | 12 ++
drivers/iio/accel/Makefile | 1 +
drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++++++++
drivers/staging/iio/accel/Kconfig | 12 --
drivers/staging/iio/accel/Makefile | 1 -
drivers/staging/iio/accel/adis16201.c | 323 ----------------------------------
6 files changed, 336 insertions(+), 336 deletions(-)
create mode 100644 drivers/iio/accel/adis16201.c
delete mode 100644 drivers/staging/iio/accel/adis16201.c
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index c6d9517..9416c6f 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -5,6 +5,18 @@
menu "Accelerometers"
+config ADIS16201
+ tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
+ depends on SPI
+ select IIO_ADIS_LIB
+ select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
+ help
+ Say Y here to build support for Analog Devices adis16201 dual-axis
+ digital inclinometer and accelerometer.
+
+ To compile this driver as a module, say M here: the module will
+ be called adis16201.
+
config ADXL345
tristate
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 368aedb..7832ec9 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -4,6 +4,7 @@
#
# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_ADIS16201) += adis16201.o
obj-$(CONFIG_ADXL345) += adxl345_core.o
obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
new file mode 100644
index 0000000..946c7b1
--- /dev/null
+++ b/drivers/iio/accel/adis16201.c
@@ -0,0 +1,323 @@
+/*
+ * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/imu/adis.h>
+
+#define ADIS16201_STARTUP_DELAY_MS 220
+#define ADIS16201_FLASH_CNT 0x00
+
+/* Data Output Register Information */
+#define ADIS16201_SUPPLY_OUT_REG 0x02
+#define ADIS16201_XACCL_OUT_REG 0x04
+#define ADIS16201_YACCL_OUT_REG 0x06
+#define ADIS16201_AUX_ADC_REG 0x08
+#define ADIS16201_TEMP_OUT_REG 0x0A
+#define ADIS16201_XINCL_OUT_REG 0x0C
+#define ADIS16201_YINCL_OUT_REG 0x0E
+
+/* Calibration Register Definition */
+#define ADIS16201_XACCL_OFFS_REG 0x10
+#define ADIS16201_YACCL_OFFS_REG 0x12
+#define ADIS16201_XACCL_SCALE_REG 0x14
+#define ADIS16201_YACCL_SCALE_REG 0x16
+#define ADIS16201_XINCL_OFFS_REG 0x18
+#define ADIS16201_YINCL_OFFS_REG 0x1A
+#define ADIS16201_XINCL_SCALE_REG 0x1C
+#define ADIS16201_YINCL_SCALE_REG 0x1E
+
+/* Alarm Register Definition */
+#define ADIS16201_ALM_MAG1_REG 0x20
+#define ADIS16201_ALM_MAG2_REG 0x22
+#define ADIS16201_ALM_SMPL1_REG 0x24
+#define ADIS16201_ALM_SMPL2_REG 0x26
+#define ADIS16201_ALM_CTRL_REG 0x28
+
+#define ADIS16201_AUX_DAC_REG 0x30
+#define ADIS16201_GPIO_CTRL_REG 0x32
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_SMPL_PRD_REG 0x36
+#define ADIS16201_AVG_CNT_REG 0x38
+#define ADIS16201_SLP_CNT_REG 0x3A
+
+/* Miscellaneous Control Register Definition */
+#define ADIS16201_MSC_CTRL_REG 0x34
+#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
+#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
+#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
+#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
+
+/* Diagnostics, system status register */
+#define ADIS16201_DIAG_STAT_REG 0x3C
+#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
+#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
+#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
+#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
+#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
+#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
+
+/* Operation, system command register */
+#define ADIS16201_GLOB_CMD_REG 0x3E
+#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
+#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
+
+#define ADIS16201_ERROR_ACTIVE BIT(14)
+
+enum adis16201_scan {
+ ADIS16201_SCAN_ACC_X,
+ ADIS16201_SCAN_ACC_Y,
+ ADIS16201_SCAN_INCLI_X,
+ ADIS16201_SCAN_INCLI_Y,
+ ADIS16201_SCAN_SUPPLY,
+ ADIS16201_SCAN_AUX_ADC,
+ ADIS16201_SCAN_TEMP,
+};
+
+static const u8 adis16201_addresses[] = {
+ [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
+ [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
+ [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
+};
+
+static int adis16201_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2,
+ long mask)
+{
+ struct adis *st = iio_priv(indio_dev);
+ int ret;
+ int bits;
+ u8 addr;
+ s16 val16;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return adis_single_conversion(indio_dev, chan,
+ ADIS16201_ERROR_ACTIVE, val);
+ case IIO_CHAN_INFO_SCALE:
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ switch (chan->channel) {
+ case 0:
+ *val = 1;
+ *val2 = 220000;
+ break;
+ case 1:
+ *val = 0;
+ *val2 = 610000;
+ break;
+ }
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_TEMP:
+ *val = -470;
+ *val2 = 0;
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_ACCEL:
+ /*
+ * IIO base unit for sensitivity of accelerometer
+ * is milli g.
+ * 1 LSB represents 0.244 mg.
+ */
+ *val = 0;
+ *val2 = IIO_G_TO_M_S_2(462400);
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_INCLI:
+ *val = 0;
+ *val2 = 100000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+ break;
+ case IIO_CHAN_INFO_OFFSET:
+ /*
+ * The raw ADC value is 0x4FE when the temperature
+ * is 25 degrees and the scale factor per milli
+ * degree celcius is -470.
+ */
+ *val = 25000 / -470 - 1278;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ bits = 12;
+ break;
+ case IIO_INCLI:
+ bits = 9;
+ break;
+ default:
+ return -EINVAL;
+ }
+ addr = adis16201_addresses[chan->scan_index];
+ ret = adis_read_reg_16(st, addr, &val16);
+ if (ret)
+ return ret;
+
+ *val = sign_extend32(val16, bits - 1);
+ return IIO_VAL_INT;
+ }
+
+ return -EINVAL;
+}
+
+static int adis16201_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct adis *st = iio_priv(indio_dev);
+ int bits;
+ s16 val16;
+ u8 addr;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBBIAS:
+ switch (chan->type) {
+ case IIO_ACCEL:
+ bits = 12;
+ break;
+ case IIO_INCLI:
+ bits = 9;
+ break;
+ default:
+ return -EINVAL;
+ }
+ val16 = val & ((1 << bits) - 1);
+ addr = adis16201_addresses[chan->scan_index];
+ return adis_write_reg_16(st, addr, val16);
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_chan_spec adis16201_channels[] = {
+ ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
+ ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
+ ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
+ ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
+ BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
+ IIO_CHAN_SOFT_TIMESTAMP(7)
+};
+
+static const struct iio_info adis16201_info = {
+ .read_raw = adis16201_read_raw,
+ .write_raw = adis16201_write_raw,
+ .update_scan_mode = adis_update_scan_mode,
+};
+
+static const char * const adis16201_status_error_msgs[] = {
+ [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
+ [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
+ [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
+ [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
+};
+
+static const struct adis_data adis16201_data = {
+ .read_delay = 20,
+ .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
+ .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
+ .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
+
+ .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
+ .self_test_no_autoclear = true,
+ .startup_delay = ADIS16201_STARTUP_DELAY_MS,
+
+ .status_error_msgs = adis16201_status_error_msgs,
+ .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
+ BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
+ BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
+ BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
+};
+
+static int adis16201_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct adis *st;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ st = iio_priv(indio_dev);
+ spi_set_drvdata(spi, indio_dev);
+
+ indio_dev->name = spi->dev.driver->name;
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->info = &adis16201_info;
+
+ indio_dev->channels = adis16201_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = adis_init(st, indio_dev, spi, &adis16201_data);
+ if (ret)
+ return ret;
+
+ ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
+ if (ret)
+ return ret;
+
+ ret = adis_initial_startup(st);
+ if (ret)
+ goto error_cleanup_buffer_trigger;
+
+ ret = iio_device_register(indio_dev);
+ if (ret < 0)
+ goto error_cleanup_buffer_trigger;
+
+ return 0;
+
+error_cleanup_buffer_trigger:
+ adis_cleanup_buffer_and_trigger(st, indio_dev);
+ return ret;
+}
+
+static int adis16201_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct adis *st = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ adis_cleanup_buffer_and_trigger(st, indio_dev);
+
+ return 0;
+}
+
+static struct spi_driver adis16201_driver = {
+ .driver = {
+ .name = "adis16201",
+ },
+ .probe = adis16201_probe,
+ .remove = adis16201_remove,
+};
+module_spi_driver(adis16201_driver);
+
+MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:adis16201");
diff --git a/drivers/staging/iio/accel/Kconfig b/drivers/staging/iio/accel/Kconfig
index c6b0f5e..2f61e21 100644
--- a/drivers/staging/iio/accel/Kconfig
+++ b/drivers/staging/iio/accel/Kconfig
@@ -3,18 +3,6 @@
#
menu "Accelerometers"
-config ADIS16201
- tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
- depends on SPI
- select IIO_ADIS_LIB
- select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
- help
- Say Y here to build support for Analog Devices adis16201 dual-axis
- digital inclinometer and accelerometer.
-
- To compile this driver as a module, say M here: the module will
- be called adis16201.
-
config ADIS16203
tristate "Analog Devices ADIS16203 Programmable 360 Degrees Inclinometer"
depends on SPI
diff --git a/drivers/staging/iio/accel/Makefile b/drivers/staging/iio/accel/Makefile
index 5d8ad21..7dd55410 100644
--- a/drivers/staging/iio/accel/Makefile
+++ b/drivers/staging/iio/accel/Makefile
@@ -2,7 +2,6 @@
# Makefile for industrial I/O accelerometer drivers
#
-obj-$(CONFIG_ADIS16201) += adis16201.o
obj-$(CONFIG_ADIS16203) += adis16203.o
obj-$(CONFIG_ADIS16209) += adis16209.o
obj-$(CONFIG_ADIS16240) += adis16240.o
diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
deleted file mode 100644
index 946c7b1..0000000
--- a/drivers/staging/iio/accel/adis16201.c
+++ /dev/null
@@ -1,323 +0,0 @@
-/*
- * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
- *
- * Copyright 2010 Analog Devices Inc.
- *
- * Licensed under the GPL-2 or later.
- */
-
-#include <linux/delay.h>
-#include <linux/device.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/slab.h>
-#include <linux/spi/spi.h>
-#include <linux/sysfs.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/sysfs.h>
-#include <linux/iio/buffer.h>
-#include <linux/iio/imu/adis.h>
-
-#define ADIS16201_STARTUP_DELAY_MS 220
-#define ADIS16201_FLASH_CNT 0x00
-
-/* Data Output Register Information */
-#define ADIS16201_SUPPLY_OUT_REG 0x02
-#define ADIS16201_XACCL_OUT_REG 0x04
-#define ADIS16201_YACCL_OUT_REG 0x06
-#define ADIS16201_AUX_ADC_REG 0x08
-#define ADIS16201_TEMP_OUT_REG 0x0A
-#define ADIS16201_XINCL_OUT_REG 0x0C
-#define ADIS16201_YINCL_OUT_REG 0x0E
-
-/* Calibration Register Definition */
-#define ADIS16201_XACCL_OFFS_REG 0x10
-#define ADIS16201_YACCL_OFFS_REG 0x12
-#define ADIS16201_XACCL_SCALE_REG 0x14
-#define ADIS16201_YACCL_SCALE_REG 0x16
-#define ADIS16201_XINCL_OFFS_REG 0x18
-#define ADIS16201_YINCL_OFFS_REG 0x1A
-#define ADIS16201_XINCL_SCALE_REG 0x1C
-#define ADIS16201_YINCL_SCALE_REG 0x1E
-
-/* Alarm Register Definition */
-#define ADIS16201_ALM_MAG1_REG 0x20
-#define ADIS16201_ALM_MAG2_REG 0x22
-#define ADIS16201_ALM_SMPL1_REG 0x24
-#define ADIS16201_ALM_SMPL2_REG 0x26
-#define ADIS16201_ALM_CTRL_REG 0x28
-
-#define ADIS16201_AUX_DAC_REG 0x30
-#define ADIS16201_GPIO_CTRL_REG 0x32
-#define ADIS16201_MSC_CTRL_REG 0x34
-#define ADIS16201_SMPL_PRD_REG 0x36
-#define ADIS16201_AVG_CNT_REG 0x38
-#define ADIS16201_SLP_CNT_REG 0x3A
-
-/* Miscellaneous Control Register Definition */
-#define ADIS16201_MSC_CTRL_REG 0x34
-#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
-#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
-#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
-#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
-
-/* Diagnostics, system status register */
-#define ADIS16201_DIAG_STAT_REG 0x3C
-#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
-#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
-#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
-#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
-#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
-#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
-
-/* Operation, system command register */
-#define ADIS16201_GLOB_CMD_REG 0x3E
-#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
-#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
-
-#define ADIS16201_ERROR_ACTIVE BIT(14)
-
-enum adis16201_scan {
- ADIS16201_SCAN_ACC_X,
- ADIS16201_SCAN_ACC_Y,
- ADIS16201_SCAN_INCLI_X,
- ADIS16201_SCAN_INCLI_Y,
- ADIS16201_SCAN_SUPPLY,
- ADIS16201_SCAN_AUX_ADC,
- ADIS16201_SCAN_TEMP,
-};
-
-static const u8 adis16201_addresses[] = {
- [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
- [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
- [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
- [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
-};
-
-static int adis16201_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2,
- long mask)
-{
- struct adis *st = iio_priv(indio_dev);
- int ret;
- int bits;
- u8 addr;
- s16 val16;
-
- switch (mask) {
- case IIO_CHAN_INFO_RAW:
- return adis_single_conversion(indio_dev, chan,
- ADIS16201_ERROR_ACTIVE, val);
- case IIO_CHAN_INFO_SCALE:
- switch (chan->type) {
- case IIO_VOLTAGE:
- switch (chan->channel) {
- case 0:
- *val = 1;
- *val2 = 220000;
- break;
- case 1:
- *val = 0;
- *val2 = 610000;
- break;
- }
- return IIO_VAL_INT_PLUS_MICRO;
- case IIO_TEMP:
- *val = -470;
- *val2 = 0;
- return IIO_VAL_INT_PLUS_MICRO;
- case IIO_ACCEL:
- /*
- * IIO base unit for sensitivity of accelerometer
- * is milli g.
- * 1 LSB represents 0.244 mg.
- */
- *val = 0;
- *val2 = IIO_G_TO_M_S_2(462400);
- return IIO_VAL_INT_PLUS_NANO;
- case IIO_INCLI:
- *val = 0;
- *val2 = 100000;
- return IIO_VAL_INT_PLUS_MICRO;
- default:
- return -EINVAL;
- }
- break;
- case IIO_CHAN_INFO_OFFSET:
- /*
- * The raw ADC value is 0x4FE when the temperature
- * is 25 degrees and the scale factor per milli
- * degree celcius is -470.
- */
- *val = 25000 / -470 - 1278;
- return IIO_VAL_INT;
- case IIO_CHAN_INFO_CALIBBIAS:
- switch (chan->type) {
- case IIO_ACCEL:
- bits = 12;
- break;
- case IIO_INCLI:
- bits = 9;
- break;
- default:
- return -EINVAL;
- }
- addr = adis16201_addresses[chan->scan_index];
- ret = adis_read_reg_16(st, addr, &val16);
- if (ret)
- return ret;
-
- *val = sign_extend32(val16, bits - 1);
- return IIO_VAL_INT;
- }
-
- return -EINVAL;
-}
-
-static int adis16201_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
-{
- struct adis *st = iio_priv(indio_dev);
- int bits;
- s16 val16;
- u8 addr;
-
- switch (mask) {
- case IIO_CHAN_INFO_CALIBBIAS:
- switch (chan->type) {
- case IIO_ACCEL:
- bits = 12;
- break;
- case IIO_INCLI:
- bits = 9;
- break;
- default:
- return -EINVAL;
- }
- val16 = val & ((1 << bits) - 1);
- addr = adis16201_addresses[chan->scan_index];
- return adis_write_reg_16(st, addr, val16);
- }
-
- return -EINVAL;
-}
-
-static const struct iio_chan_spec adis16201_channels[] = {
- ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
- ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
- ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
- ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
- BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
- IIO_CHAN_SOFT_TIMESTAMP(7)
-};
-
-static const struct iio_info adis16201_info = {
- .read_raw = adis16201_read_raw,
- .write_raw = adis16201_write_raw,
- .update_scan_mode = adis_update_scan_mode,
-};
-
-static const char * const adis16201_status_error_msgs[] = {
- [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
- [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
- [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
- [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
-};
-
-static const struct adis_data adis16201_data = {
- .read_delay = 20,
- .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
- .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
- .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
-
- .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
- .self_test_no_autoclear = true,
- .startup_delay = ADIS16201_STARTUP_DELAY_MS,
-
- .status_error_msgs = adis16201_status_error_msgs,
- .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
- BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
- BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
-};
-
-static int adis16201_probe(struct spi_device *spi)
-{
- struct iio_dev *indio_dev;
- struct adis *st;
- int ret;
-
- indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
- if (!indio_dev)
- return -ENOMEM;
-
- st = iio_priv(indio_dev);
- spi_set_drvdata(spi, indio_dev);
-
- indio_dev->name = spi->dev.driver->name;
- indio_dev->dev.parent = &spi->dev;
- indio_dev->info = &adis16201_info;
-
- indio_dev->channels = adis16201_channels;
- indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
- indio_dev->modes = INDIO_DIRECT_MODE;
-
- ret = adis_init(st, indio_dev, spi, &adis16201_data);
- if (ret)
- return ret;
-
- ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
- if (ret)
- return ret;
-
- ret = adis_initial_startup(st);
- if (ret)
- goto error_cleanup_buffer_trigger;
-
- ret = iio_device_register(indio_dev);
- if (ret < 0)
- goto error_cleanup_buffer_trigger;
-
- return 0;
-
-error_cleanup_buffer_trigger:
- adis_cleanup_buffer_and_trigger(st, indio_dev);
- return ret;
-}
-
-static int adis16201_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
- struct adis *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- adis_cleanup_buffer_and_trigger(st, indio_dev);
-
- return 0;
-}
-
-static struct spi_driver adis16201_driver = {
- .driver = {
- .name = "adis16201",
- },
- .probe = adis16201_probe,
- .remove = adis16201_remove,
-};
-module_spi_driver(adis16201_driver);
-
-MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
-MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("spi:adis16201");
--
2.7.4
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 07/11] Staging: iio: accel: Adjust arguments to match open parentheses
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-05 10:16 ` Dan Carpenter
-1 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2018-03-05 10:16 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h, jic23
On Mon, Mar 05, 2018 at 01:19:26PM +0530, Himanshu Jha wrote:
> In adis16201_read_raw() adjust an argument to match an open parentheses
> using tabs.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/staging/iio/accel/adis16201.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 907fb85..1737708 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -110,7 +110,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> return adis_single_conversion(indio_dev, chan,
> - ADIS16201_ERROR_ACTIVE, val);
> + ADIS16201_ERROR_ACTIVE, val);
It still doesn't line up. You need to use some spaces.
return adis_single_conversion(indio_dev, chan,
ADIS16201_ERROR_ACTIVE, val);
[tab][tab][tab][tab][tab][space][space][space][space][space][space]ADIS...
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/11] Staging: iio: accel: Adjust arguments to match open parentheses
@ 2018-03-05 10:16 ` Dan Carpenter
0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2018-03-05 10:16 UTC (permalink / raw)
To: Himanshu Jha
Cc: jic23, devel, daniel.baluta, lars, Michael.Hennerich, linux-iio,
gregkh, linux-kernel, pmeerw, knaack.h
On Mon, Mar 05, 2018 at 01:19:26PM +0530, Himanshu Jha wrote:
> In adis16201_read_raw() adjust an argument to match an open parentheses
> using tabs.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/staging/iio/accel/adis16201.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 907fb85..1737708 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -110,7 +110,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> switch (mask) {
> case IIO_CHAN_INFO_RAW:
> return adis_single_conversion(indio_dev, chan,
> - ADIS16201_ERROR_ACTIVE, val);
> + ADIS16201_ERROR_ACTIVE, val);
It still doesn't line up. You need to use some spaces.
return adis_single_conversion(indio_dev, chan,
ADIS16201_ERROR_ACTIVE, val);
[tab][tab][tab][tab][tab][space][space][space][space][space][space]ADIS...
regards,
dan carpenter
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
2018-03-05 7:49 ` Himanshu Jha
(?)
@ 2018-03-06 21:26 ` Shreeya Patel
2018-03-06 22:25 ` Himanshu Jha
-1 siblings, 1 reply; 43+ messages in thread
From: Shreeya Patel @ 2018-03-06 21:26 UTC (permalink / raw)
To: Himanshu Jha, jic23
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 2018-03-05 at 13:19 +0530, Himanshu Jha wrote:
> Remove unnecessary comments since the definitions are pretty clear
> with their macro names.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/staging/iio/accel/adis16201.c | 82 +++++------------------
> ------------
> 1 file changed, 10 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c
> b/drivers/staging/iio/accel/adis16201.c
> index 0fae8aa..59c1166 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,87 +20,42 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY 220 /* ms */
> +#define ADIS16201_STARTUP_DELAY 220
>
> -/* Flash memory write count */
> #define ADIS16201_FLASH_CNT 0x00
>
> -/* Output, power supply */
> +/* Data Output Register Information */
> #define ADIS16201_SUPPLY_OUT 0x02
> -
> -/* Output, x-axis accelerometer */
> #define ADIS16201_XACCL_OUT 0x04
> -
> -/* Output, y-axis accelerometer */
> #define ADIS16201_YACCL_OUT 0x06
> -
> -/* Output, auxiliary ADC input */
> #define ADIS16201_AUX_ADC 0x08
> -
> -/* Output, temperature */
> #define ADIS16201_TEMP_OUT 0x0A
> -
> -/* Output, x-axis inclination */
> #define ADIS16201_XINCL_OUT 0x0C
> -
> -/* Output, y-axis inclination */
> #define ADIS16201_YINCL_OUT 0x0E
>
> -/* Calibration, x-axis acceleration offset */
> +/* Calibration Register Definition */
> #define ADIS16201_XACCL_OFFS 0x10
> -
> -/* Calibration, y-axis acceleration offset */
> #define ADIS16201_YACCL_OFFS 0x12
> -
> -/* x-axis acceleration scale factor */
> #define ADIS16201_XACCL_SCALE 0x14
> -
> -/* y-axis acceleration scale factor */
> #define ADIS16201_YACCL_SCALE 0x16
> -
> -/* Calibration, x-axis inclination offset */
> #define ADIS16201_XINCL_OFFS 0x18
> -
> -/* Calibration, y-axis inclination offset */
> #define ADIS16201_YINCL_OFFS 0x1A
> -
> -/* x-axis inclination scale factor */
> #define ADIS16201_XINCL_SCALE 0x1C
> -
> -/* y-axis inclination scale factor */
> #define ADIS16201_YINCL_SCALE 0x1E
>
> -/* Alarm 1 amplitude threshold */
> +/* Alarm Register Definition */
> #define ADIS16201_ALM_MAG1 0x20
> -
> -/* Alarm 2 amplitude threshold */
> #define ADIS16201_ALM_MAG2 0x22
> -
> -/* Alarm 1, sample period */
> #define ADIS16201_ALM_SMPL1 0x24
> -
> -/* Alarm 2, sample period */
> #define ADIS16201_ALM_SMPL2 0x26
> -
> -/* Alarm control */
> #define ADIS16201_ALM_CTRL 0x28
>
> -/* Auxiliary DAC data */
> #define ADIS16201_AUX_DAC 0x30
> -
> -/* General-purpose digital input/output control */
> #define ADIS16201_GPIO_CTRL 0x32
> -
> -/* Miscellaneous control */
> #define ADIS16201_MSC_CTRL 0x34
>
> -/* Internal sample period (rate) control */
> #define ADIS16201_SMPL_PRD 0x36
> -
> -/* Operation, filter configuration */
> #define ADIS16201_AVG_CNT 0x38
> -
> -/* Operation, sleep mode control */
> #define ADIS16201_SLP_CNT 0x3A
>
> /* Diagnostics, system status register */
> @@ -109,42 +64,28 @@
> /* Operation, system command register */
> #define ADIS16201_GLOB_CMD 0x3E
>
> -/* MSC_CTRL */
>
> -/* Self-test enable */
> #define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
>
> -/* Data-ready enable: 1 = enabled, 0 = disabled */
> #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
>
> -/* Data-ready polarity: 1 = active high, 0 = active low */
> #define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
>
> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
> #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> -/* DIAG_STAT */
>
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
>
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
>
> -/* SPI communications failure */
> #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
>
> -/* Flash update failure */
> #define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
>
> -/* Power supply above 3.625 V */
> #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>
> -/* Power supply below 3.15 V */
> #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
>
> -/* GLOB_CMD */
> -
> #define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> #define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
I was just going through your patch for giving myself
a habit of reading patches.
I see here that there are many comments which are necessary
have also been removed.
Jonathan told that some of the names do not explain
much about the how registers are related to the orientation.
So it is necessary for some comments to be there here.
I saw your next patch too, in which you are changing some of the
names for betterment, but again, that doesn't cover everything which
was told by Jonathan.
Here is the link to the patch where Jonathan had given detailed
description
Just sharing this information so in case if Jonathan agrees with
this then he will not have to explain it all again :)
https://lkml.org/lkml/2018/3/3/104
Thanks,
Shreeya
>
> @@ -187,30 +128,30 @@ static int adis16201_read_raw(struct iio_dev
> *indio_dev,
> case IIO_VOLTAGE:
> if (chan->channel == 0) {
> *val = 1;
> - *val2 = 220000; /* 1.22 mV */
> + *val2 = 220000;
> } else {
> *val = 0;
> - *val2 = 610000; /* 0.610 mV */
> + *val2 = 610000;
> }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> - *val = -470; /* 0.47 C */
> + *val = -470;
> *val2 = 0;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_ACCEL:
> *val = 0;
> - *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg
> */
> + *val2 = IIO_G_TO_M_S_2(462400);
> return IIO_VAL_INT_PLUS_NANO;
> case IIO_INCLI:
> *val = 0;
> - *val2 = 100000; /* 0.1 degree */
> + *val2 = 100000;
> return IIO_VAL_INT_PLUS_MICRO;
> default:
> return -EINVAL;
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> - *val = 25000 / -470 - 1278; /* 25 C = 1278 */
> + *val = 25000 / -470 - 1278;
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
> switch (chan->type) {
> @@ -318,13 +259,11 @@ static int adis16201_probe(struct spi_device
> *spi)
> struct adis *st;
> struct iio_dev *indio_dev;
>
> - /* setup the industrialio driver allocated elements */
> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> if (!indio_dev)
> return -ENOMEM;
>
> st = iio_priv(indio_dev);
> - /* this is only used for removal purposes */
> spi_set_drvdata(spi, indio_dev);
>
> indio_dev->name = spi->dev.driver->name;
> @@ -343,7 +282,6 @@ static int adis16201_probe(struct spi_device
> *spi)
> if (ret)
> return ret;
>
> - /* Get the device into a sane initial state */
> ret = adis_initial_startup(st);
> if (ret)
> goto error_cleanup_buffer_trigger;
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
2018-03-06 21:26 ` Shreeya Patel
@ 2018-03-06 22:25 ` Himanshu Jha
0 siblings, 0 replies; 43+ messages in thread
From: Himanshu Jha @ 2018-03-06 22:25 UTC (permalink / raw)
To: Shreeya Patel
Cc: jic23, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel, daniel.baluta
Hi Shreeya,
> I was just going through your patch for giving myself
> a habit of reading patches.
Great!
> I see here that there are many comments which are necessary
> have also been removed.
> Jonathan told that some of the names do not explain
> much about the how registers are related to the orientation.
> So it is necessary for some comments to be there here.
>
> I saw your next patch too, in which you are changing some of the
> names for betterment, but again, that doesn't cover everything which
> was told by Jonathan.
>
> Here is the link to the patch where Jonathan had given detailed
> description
>
> Just sharing this information so in case if Jonathan agrees with
> this then he will not have to explain it all again :)
>
> https://lkml.org/lkml/2018/3/3/104
Well, the naming of macros is debatable as Jonathan pointed out[1] and
the unusual/odd naming pointed to you was for rotation registers
#define ADIS16209_ROT_OUT_REG 0x10
I renamed the unusual/odd naming macros for eg
#define ADIS16209_DIAG_STAT_FLASH_UPT_BIT
to
#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT
as it didn't look like a failure bit address for which I consulted Jonathan to which
he agreed. Also, *DIAG_STAT* seems like a good name since these status registers
are serving the purpose of diagnosing the device behavior. Again, GLOB_CMD stands
for Global Command register for controlling the deivce operation such as
Fatory Reset, Software Reset, etc.
See, it is difficult to point the perfect names than the suitable ones!
And let's just leave these *bikeshedding* ;-)
--
Thanks
Himanshu Jha
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files
2018-03-05 7:49 ` [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files Himanshu Jha
@ 2018-03-07 20:29 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:29 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:20 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Arrange header files in alphabetical sequence to improve readability.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
One general comment - when naming a patch it will form the description
in the git short log or on the various web interfaces etc. It's
really useful to actually say which driver it is.
I've fixed that up by adding adis16201: in the appropriate place.
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 2ebd275..0f6a204 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -7,13 +7,13 @@
> */
>
> #include <linux/delay.h>
> -#include <linux/mutex.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> -#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/slab.h>
> +#include <linux/spi/spi.h>
> #include <linux/sysfs.h>
> -#include <linux/module.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/11] Staging: iio: accel: Add a blank space before returns
2018-03-05 7:49 ` [PATCH 02/11] Staging: iio: accel: Add a blank space before returns Himanshu Jha
@ 2018-03-07 20:32 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:32 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:21 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Adding a blank space before/after some returns improves readability.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
Applied - patch title adjusted.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 0f6a204..0fae8aa 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -232,6 +232,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> *val = val16;
> return IIO_VAL_INT;
> }
> +
> return -EINVAL;
> }
>
> @@ -262,6 +263,7 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
> addr = adis16201_addresses[chan->scan_index];
> return adis_write_reg_16(st, addr, val16);
> }
> +
> return -EINVAL;
> }
>
> @@ -336,6 +338,7 @@ static int adis16201_probe(struct spi_device *spi)
> ret = adis_init(st, indio_dev, spi, &adis16201_data);
> if (ret)
> return ret;
> +
> ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> if (ret)
> return ret;
> @@ -348,6 +351,7 @@ static int adis16201_probe(struct spi_device *spi)
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> goto error_cleanup_buffer_trigger;
> +
> return 0;
>
> error_cleanup_buffer_trigger:
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/11] Staging: iio: accel: Add a blank space before returns
@ 2018-03-07 20:32 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:32 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:21 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Adding a blank space before/after some returns improves readability.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
Applied - patch title adjusted.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 0f6a204..0fae8aa 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -232,6 +232,7 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> *val = val16;
> return IIO_VAL_INT;
> }
> +
> return -EINVAL;
> }
>
> @@ -262,6 +263,7 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
> addr = adis16201_addresses[chan->scan_index];
> return adis_write_reg_16(st, addr, val16);
> }
> +
> return -EINVAL;
> }
>
> @@ -336,6 +338,7 @@ static int adis16201_probe(struct spi_device *spi)
> ret = adis_init(st, indio_dev, spi, &adis16201_data);
> if (ret)
> return ret;
> +
> ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> if (ret)
> return ret;
> @@ -348,6 +351,7 @@ static int adis16201_probe(struct spi_device *spi)
> ret = iio_device_register(indio_dev);
> if (ret < 0)
> goto error_cleanup_buffer_trigger;
> +
> return 0;
>
> error_cleanup_buffer_trigger:
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-07 20:40 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:40 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:22 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Remove unnecessary comments since the definitions are pretty clear
> with their macro names.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
Hi,
The art of commenting (and indeed removing comments) is never a clear one.
I agree with the vast majority of what you have here, but as with other
drivers in this set, I think there are a couple of places comments are useful
and even in one case where the comment should be expanded to become useful.
I'd also like to see the patches reordered so the renames occur before this
one. That will make some of the changes you have made in here a lot more
obviously good!
Thanks,
Jonathan.
> ---
> drivers/staging/iio/accel/adis16201.c | 82 +++++------------------------------
> 1 file changed, 10 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 0fae8aa..59c1166 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,87 +20,42 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY 220 /* ms */
> +#define ADIS16201_STARTUP_DELAY 220
As below, this would have made a lot more sense if you had
the rename of the macro patch (I assume the unit will be added
to the name) before this one.
>
> -/* Flash memory write count */
> #define ADIS16201_FLASH_CNT 0x00
>
> -/* Output, power supply */
> +/* Data Output Register Information */
> #define ADIS16201_SUPPLY_OUT 0x02
> -
> -/* Output, x-axis accelerometer */
> #define ADIS16201_XACCL_OUT 0x04
> -
> -/* Output, y-axis accelerometer */
> #define ADIS16201_YACCL_OUT 0x06
> -
> -/* Output, auxiliary ADC input */
> #define ADIS16201_AUX_ADC 0x08
> -
> -/* Output, temperature */
> #define ADIS16201_TEMP_OUT 0x0A
> -
> -/* Output, x-axis inclination */
> #define ADIS16201_XINCL_OUT 0x0C
> -
> -/* Output, y-axis inclination */
> #define ADIS16201_YINCL_OUT 0x0E
>
> -/* Calibration, x-axis acceleration offset */
> +/* Calibration Register Definition */
> #define ADIS16201_XACCL_OFFS 0x10
> -
> -/* Calibration, y-axis acceleration offset */
> #define ADIS16201_YACCL_OFFS 0x12
> -
> -/* x-axis acceleration scale factor */
> #define ADIS16201_XACCL_SCALE 0x14
> -
> -/* y-axis acceleration scale factor */
> #define ADIS16201_YACCL_SCALE 0x16
> -
> -/* Calibration, x-axis inclination offset */
> #define ADIS16201_XINCL_OFFS 0x18
> -
> -/* Calibration, y-axis inclination offset */
> #define ADIS16201_YINCL_OFFS 0x1A
> -
> -/* x-axis inclination scale factor */
> #define ADIS16201_XINCL_SCALE 0x1C
> -
> -/* y-axis inclination scale factor */
> #define ADIS16201_YINCL_SCALE 0x1E
>
> -/* Alarm 1 amplitude threshold */
> +/* Alarm Register Definition */
> #define ADIS16201_ALM_MAG1 0x20
> -
> -/* Alarm 2 amplitude threshold */
> #define ADIS16201_ALM_MAG2 0x22
> -
> -/* Alarm 1, sample period */
> #define ADIS16201_ALM_SMPL1 0x24
> -
> -/* Alarm 2, sample period */
> #define ADIS16201_ALM_SMPL2 0x26
> -
> -/* Alarm control */
> #define ADIS16201_ALM_CTRL 0x28
>
> -/* Auxiliary DAC data */
> #define ADIS16201_AUX_DAC 0x30
> -
> -/* General-purpose digital input/output control */
> #define ADIS16201_GPIO_CTRL 0x32
> -
> -/* Miscellaneous control */
> #define ADIS16201_MSC_CTRL 0x34
>
> -/* Internal sample period (rate) control */
> #define ADIS16201_SMPL_PRD 0x36
> -
> -/* Operation, filter configuration */
> #define ADIS16201_AVG_CNT 0x38
> -
> -/* Operation, sleep mode control */
> #define ADIS16201_SLP_CNT 0x3A
>
> /* Diagnostics, system status register */
> @@ -109,42 +64,28 @@
> /* Operation, system command register */
> #define ADIS16201_GLOB_CMD 0x3E
>
> -/* MSC_CTRL */
>
> -/* Self-test enable */
> #define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
>
> -/* Data-ready enable: 1 = enabled, 0 = disabled */
> #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
>
> -/* Data-ready polarity: 1 = active high, 0 = active low */
> #define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
>
> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
> #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> -/* DIAG_STAT */
>
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
>
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
>
> -/* SPI communications failure */
> #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
>
> -/* Flash update failure */
> #define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
I would have preferred this and the next patch to be
in the opposite order as then it would have been obvious when
reading this patch on why the comment here is no longer needed.
>
> -/* Power supply above 3.625 V */
> #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>
> -/* Power supply below 3.15 V */
> #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
These two comments are providing information that isn't
available from the naming so are useful and should stay.
>
> -/* GLOB_CMD */
> -
> #define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> #define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
>
> @@ -187,30 +128,30 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> case IIO_VOLTAGE:
> if (chan->channel == 0) {
> *val = 1;
> - *val2 = 220000; /* 1.22 mV */
> + *val2 = 220000;
> } else {
> *val = 0;
> - *val2 = 610000; /* 0.610 mV */
> + *val2 = 610000;
> }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> - *val = -470; /* 0.47 C */
> + *val = -470;
The interesting point here is that the original comment
was actually wrong. It is clearly -0.47!
> *val2 = 0;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_ACCEL:
> *val = 0;
> - *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg */
> + *val2 = IIO_G_TO_M_S_2(462400);
> return IIO_VAL_INT_PLUS_NANO;
> case IIO_INCLI:
> *val = 0;
> - *val2 = 100000; /* 0.1 degree */
> + *val2 = 100000;
> return IIO_VAL_INT_PLUS_MICRO;
> default:
> return -EINVAL;
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> - *val = 25000 / -470 - 1278; /* 25 C = 1278 */
> + *val = 25000 / -470 - 1278;
The original comment wasn't great but we probably would benefit from some
information here. Something like:
/*
* When temperature is 25 C sensor raw reading is 1278.
* The scale is -470 milli degree C per LSB.
*/
Hopefully that would let people know where the maths comes
from (please check I got it right!)
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
> switch (chan->type) {
> @@ -318,13 +259,11 @@ static int adis16201_probe(struct spi_device *spi)
> struct adis *st;
> struct iio_dev *indio_dev;
>
> - /* setup the industrialio driver allocated elements */
> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> if (!indio_dev)
> return -ENOMEM;
>
> st = iio_priv(indio_dev);
> - /* this is only used for removal purposes */
> spi_set_drvdata(spi, indio_dev);
>
> indio_dev->name = spi->dev.driver->name;
> @@ -343,7 +282,6 @@ static int adis16201_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - /* Get the device into a sane initial state */
> ret = adis_initial_startup(st);
> if (ret)
> goto error_cleanup_buffer_trigger;
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments
@ 2018-03-07 20:40 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:40 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:22 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Remove unnecessary comments since the definitions are pretty clear
> with their macro names.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
Hi,
The art of commenting (and indeed removing comments) is never a clear one.
I agree with the vast majority of what you have here, but as with other
drivers in this set, I think there are a couple of places comments are useful
and even in one case where the comment should be expanded to become useful.
I'd also like to see the patches reordered so the renames occur before this
one. That will make some of the changes you have made in here a lot more
obviously good!
Thanks,
Jonathan.
> ---
> drivers/staging/iio/accel/adis16201.c | 82 +++++------------------------------
> 1 file changed, 10 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 0fae8aa..59c1166 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,87 +20,42 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY 220 /* ms */
> +#define ADIS16201_STARTUP_DELAY 220
As below, this would have made a lot more sense if you had
the rename of the macro patch (I assume the unit will be added
to the name) before this one.
>
> -/* Flash memory write count */
> #define ADIS16201_FLASH_CNT 0x00
>
> -/* Output, power supply */
> +/* Data Output Register Information */
> #define ADIS16201_SUPPLY_OUT 0x02
> -
> -/* Output, x-axis accelerometer */
> #define ADIS16201_XACCL_OUT 0x04
> -
> -/* Output, y-axis accelerometer */
> #define ADIS16201_YACCL_OUT 0x06
> -
> -/* Output, auxiliary ADC input */
> #define ADIS16201_AUX_ADC 0x08
> -
> -/* Output, temperature */
> #define ADIS16201_TEMP_OUT 0x0A
> -
> -/* Output, x-axis inclination */
> #define ADIS16201_XINCL_OUT 0x0C
> -
> -/* Output, y-axis inclination */
> #define ADIS16201_YINCL_OUT 0x0E
>
> -/* Calibration, x-axis acceleration offset */
> +/* Calibration Register Definition */
> #define ADIS16201_XACCL_OFFS 0x10
> -
> -/* Calibration, y-axis acceleration offset */
> #define ADIS16201_YACCL_OFFS 0x12
> -
> -/* x-axis acceleration scale factor */
> #define ADIS16201_XACCL_SCALE 0x14
> -
> -/* y-axis acceleration scale factor */
> #define ADIS16201_YACCL_SCALE 0x16
> -
> -/* Calibration, x-axis inclination offset */
> #define ADIS16201_XINCL_OFFS 0x18
> -
> -/* Calibration, y-axis inclination offset */
> #define ADIS16201_YINCL_OFFS 0x1A
> -
> -/* x-axis inclination scale factor */
> #define ADIS16201_XINCL_SCALE 0x1C
> -
> -/* y-axis inclination scale factor */
> #define ADIS16201_YINCL_SCALE 0x1E
>
> -/* Alarm 1 amplitude threshold */
> +/* Alarm Register Definition */
> #define ADIS16201_ALM_MAG1 0x20
> -
> -/* Alarm 2 amplitude threshold */
> #define ADIS16201_ALM_MAG2 0x22
> -
> -/* Alarm 1, sample period */
> #define ADIS16201_ALM_SMPL1 0x24
> -
> -/* Alarm 2, sample period */
> #define ADIS16201_ALM_SMPL2 0x26
> -
> -/* Alarm control */
> #define ADIS16201_ALM_CTRL 0x28
>
> -/* Auxiliary DAC data */
> #define ADIS16201_AUX_DAC 0x30
> -
> -/* General-purpose digital input/output control */
> #define ADIS16201_GPIO_CTRL 0x32
> -
> -/* Miscellaneous control */
> #define ADIS16201_MSC_CTRL 0x34
>
> -/* Internal sample period (rate) control */
> #define ADIS16201_SMPL_PRD 0x36
> -
> -/* Operation, filter configuration */
> #define ADIS16201_AVG_CNT 0x38
> -
> -/* Operation, sleep mode control */
> #define ADIS16201_SLP_CNT 0x3A
>
> /* Diagnostics, system status register */
> @@ -109,42 +64,28 @@
> /* Operation, system command register */
> #define ADIS16201_GLOB_CMD 0x3E
>
> -/* MSC_CTRL */
>
> -/* Self-test enable */
> #define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
>
> -/* Data-ready enable: 1 = enabled, 0 = disabled */
> #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
>
> -/* Data-ready polarity: 1 = active high, 0 = active low */
> #define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
>
> -/* Data-ready line selection: 1 = DIO1, 0 = DIO0 */
> #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> -/* DIAG_STAT */
>
> -/* Alarm 2 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
>
> -/* Alarm 1 status: 1 = alarm active, 0 = alarm inactive */
> #define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
>
> -/* SPI communications failure */
> #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
>
> -/* Flash update failure */
> #define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
I would have preferred this and the next patch to be
in the opposite order as then it would have been obvious when
reading this patch on why the comment here is no longer needed.
>
> -/* Power supply above 3.625 V */
> #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>
> -/* Power supply below 3.15 V */
> #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
These two comments are providing information that isn't
available from the naming so are useful and should stay.
>
> -/* GLOB_CMD */
> -
> #define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> #define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
>
> @@ -187,30 +128,30 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> case IIO_VOLTAGE:
> if (chan->channel == 0) {
> *val = 1;
> - *val2 = 220000; /* 1.22 mV */
> + *val2 = 220000;
> } else {
> *val = 0;
> - *val2 = 610000; /* 0.610 mV */
> + *val2 = 610000;
> }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> - *val = -470; /* 0.47 C */
> + *val = -470;
The interesting point here is that the original comment
was actually wrong. It is clearly -0.47!
> *val2 = 0;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_ACCEL:
> *val = 0;
> - *val2 = IIO_G_TO_M_S_2(462400); /* 0.4624 mg */
> + *val2 = IIO_G_TO_M_S_2(462400);
> return IIO_VAL_INT_PLUS_NANO;
> case IIO_INCLI:
> *val = 0;
> - *val2 = 100000; /* 0.1 degree */
> + *val2 = 100000;
> return IIO_VAL_INT_PLUS_MICRO;
> default:
> return -EINVAL;
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> - *val = 25000 / -470 - 1278; /* 25 C = 1278 */
> + *val = 25000 / -470 - 1278;
The original comment wasn't great but we probably would benefit from some
information here. Something like:
/*
* When temperature is 25 C sensor raw reading is 1278.
* The scale is -470 milli degree C per LSB.
*/
Hopefully that would let people know where the maths comes
from (please check I got it right!)
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
> switch (chan->type) {
> @@ -318,13 +259,11 @@ static int adis16201_probe(struct spi_device *spi)
> struct adis *st;
> struct iio_dev *indio_dev;
>
> - /* setup the industrialio driver allocated elements */
> indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> if (!indio_dev)
> return -ENOMEM;
>
> st = iio_priv(indio_dev);
> - /* this is only used for removal purposes */
> spi_set_drvdata(spi, indio_dev);
>
> indio_dev->name = spi->dev.driver->name;
> @@ -343,7 +282,6 @@ static int adis16201_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - /* Get the device into a sane initial state */
> ret = adis_initial_startup(st);
> if (ret)
> goto error_cleanup_buffer_trigger;
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 04/11] Staging: iio: accel: Rename few macro definitions
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-07 20:42 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:42 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:23 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Rename few macros with appropriate names specifying their usage/function.
Most of these are obviously good, but for one I didn't understand your
reasoning.
Given there are only a few of them, I'd put some more description here
to explain why you changed each one.
Thanks,
Jonathan
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/staging/iio/accel/adis16201.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 59c1166..445cb56 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,9 +20,8 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY 220
> -
> -#define ADIS16201_FLASH_CNT 0x00
> +#define ADIS16201_STARTUP_DELAY_MS 220
> +#define ADIS16201_FLASH_CNT 0x00
>
> /* Data Output Register Information */
> #define ADIS16201_SUPPLY_OUT 0x02
> @@ -69,7 +68,7 @@
>
> #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
>
> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
>
> #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> @@ -80,14 +79,14 @@
>
> #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
>
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
>
> #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>
> #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
>
> #define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
> +#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
What was the logic behind this particular change?
>
> #define ADIS16201_ERROR_ACTIVE BIT(14)
>
> @@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = {
>
> static const char * const adis16201_status_error_msgs[] = {
> [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> - [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
> [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
> };
> @@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = {
>
> .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> .self_test_no_autoclear = true,
> - .startup_delay = ADIS16201_STARTUP_DELAY,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
>
> .status_error_msgs = adis16201_status_error_msgs,
> .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> - BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> };
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 04/11] Staging: iio: accel: Rename few macro definitions
@ 2018-03-07 20:42 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:42 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:23 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Rename few macros with appropriate names specifying their usage/function.
Most of these are obviously good, but for one I didn't understand your
reasoning.
Given there are only a few of them, I'd put some more description here
to explain why you changed each one.
Thanks,
Jonathan
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/staging/iio/accel/adis16201.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 59c1166..445cb56 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,9 +20,8 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY 220
> -
> -#define ADIS16201_FLASH_CNT 0x00
> +#define ADIS16201_STARTUP_DELAY_MS 220
> +#define ADIS16201_FLASH_CNT 0x00
>
> /* Data Output Register Information */
> #define ADIS16201_SUPPLY_OUT 0x02
> @@ -69,7 +68,7 @@
>
> #define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
>
> -#define ADIS16201_MSC_CTRL_ACTIVE_HIGH BIT(1)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
>
> #define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> @@ -80,14 +79,14 @@
>
> #define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
>
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_BIT 2
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
>
> #define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
>
> #define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
>
> #define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY_CAL BIT(1)
> +#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
What was the logic behind this particular change?
>
> #define ADIS16201_ERROR_ACTIVE BIT(14)
>
> @@ -231,7 +230,7 @@ static const struct iio_info adis16201_info = {
>
> static const char * const adis16201_status_error_msgs[] = {
> [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> - [ADIS16201_DIAG_STAT_FLASH_UPT_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
> [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
> };
> @@ -244,11 +243,11 @@ static const struct adis_data adis16201_data = {
>
> .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> .self_test_no_autoclear = true,
> - .startup_delay = ADIS16201_STARTUP_DELAY,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
>
> .status_error_msgs = adis16201_status_error_msgs,
> .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> - BIT(ADIS16201_DIAG_STAT_FLASH_UPT_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> };
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers
2018-03-05 7:49 ` Himanshu Jha
(?)
@ 2018-03-07 20:44 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:44 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:24 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Addition of _REG suffix to the register definitions allows a distinction
> between registers and register fields. The various registers and its field
> bits are grouped together to improve readability and easy indentification.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
This one looks good. I will pick it up once you've tweaked the
predecessor patches.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 133 ++++++++++++++++------------------
> 1 file changed, 61 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 445cb56..476b1c3 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -20,75 +20,64 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/imu/adis.h>
>
> -#define ADIS16201_STARTUP_DELAY_MS 220
> -#define ADIS16201_FLASH_CNT 0x00
> +#define ADIS16201_STARTUP_DELAY_MS 220
> +#define ADIS16201_FLASH_CNT 0x00
>
> /* Data Output Register Information */
> -#define ADIS16201_SUPPLY_OUT 0x02
> -#define ADIS16201_XACCL_OUT 0x04
> -#define ADIS16201_YACCL_OUT 0x06
> -#define ADIS16201_AUX_ADC 0x08
> -#define ADIS16201_TEMP_OUT 0x0A
> -#define ADIS16201_XINCL_OUT 0x0C
> -#define ADIS16201_YINCL_OUT 0x0E
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG 0x04
> +#define ADIS16201_YACCL_OUT_REG 0x06
> +#define ADIS16201_AUX_ADC_REG 0x08
> +#define ADIS16201_TEMP_OUT_REG 0x0A
> +#define ADIS16201_XINCL_OUT_REG 0x0C
> +#define ADIS16201_YINCL_OUT_REG 0x0E
>
> /* Calibration Register Definition */
> -#define ADIS16201_XACCL_OFFS 0x10
> -#define ADIS16201_YACCL_OFFS 0x12
> -#define ADIS16201_XACCL_SCALE 0x14
> -#define ADIS16201_YACCL_SCALE 0x16
> -#define ADIS16201_XINCL_OFFS 0x18
> -#define ADIS16201_YINCL_OFFS 0x1A
> -#define ADIS16201_XINCL_SCALE 0x1C
> -#define ADIS16201_YINCL_SCALE 0x1E
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG 0x14
> +#define ADIS16201_YACCL_SCALE_REG 0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG 0x1C
> +#define ADIS16201_YINCL_SCALE_REG 0x1E
>
> /* Alarm Register Definition */
> -#define ADIS16201_ALM_MAG1 0x20
> -#define ADIS16201_ALM_MAG2 0x22
> -#define ADIS16201_ALM_SMPL1 0x24
> -#define ADIS16201_ALM_SMPL2 0x26
> -#define ADIS16201_ALM_CTRL 0x28
> -
> -#define ADIS16201_AUX_DAC 0x30
> -#define ADIS16201_GPIO_CTRL 0x32
> -#define ADIS16201_MSC_CTRL 0x34
> -
> -#define ADIS16201_SMPL_PRD 0x36
> -#define ADIS16201_AVG_CNT 0x38
> -#define ADIS16201_SLP_CNT 0x3A
> +#define ADIS16201_ALM_MAG1_REG 0x20
> +#define ADIS16201_ALM_MAG2_REG 0x22
> +#define ADIS16201_ALM_SMPL1_REG 0x24
> +#define ADIS16201_ALM_SMPL2_REG 0x26
> +#define ADIS16201_ALM_CTRL_REG 0x28
> +
> +#define ADIS16201_AUX_DAC_REG 0x30
> +#define ADIS16201_GPIO_CTRL_REG 0x32
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_SMPL_PRD_REG 0x36
> +#define ADIS16201_AVG_CNT_REG 0x38
> +#define ADIS16201_SLP_CNT_REG 0x3A
> +
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
>
> /* Diagnostics, system status register */
> -#define ADIS16201_DIAG_STAT 0x3C
> +#define ADIS16201_DIAG_STAT_REG 0x3C
> +#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
> +#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
> +#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
> +#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> +#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
>
> /* Operation, system command register */
> -#define ADIS16201_GLOB_CMD 0x3E
> -
> -
> -#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
> -
> -#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
> -
> -#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
> -
> -#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
> -
> -
> -#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
> -
> -#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
> -
> -#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
> -
> -#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
> -
> -#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> -
> -#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
> -
> -#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> -#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
> +#define ADIS16201_GLOB_CMD_REG 0x3E
> +#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> +#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
>
> -#define ADIS16201_ERROR_ACTIVE BIT(14)
> +#define ADIS16201_ERROR_ACTIVE BIT(14)
>
> enum adis16201_scan {
> ADIS16201_SCAN_ACC_X,
> @@ -101,10 +90,10 @@ enum adis16201_scan {
> };
>
> static const u8 adis16201_addresses[] = {
> - [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS,
> - [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS,
> - [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS,
> - [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS,
> + [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
> + [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
> };
>
> static int adis16201_read_raw(struct iio_dev *indio_dev,
> @@ -208,16 +197,16 @@ static int adis16201_write_raw(struct iio_dev *indio_dev,
> }
>
> static const struct iio_chan_spec adis16201_channels[] = {
> - ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT, ADIS16201_SCAN_SUPPLY, 0, 12),
> - ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT, ADIS16201_SCAN_TEMP, 0, 12),
> - ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT, ADIS16201_SCAN_ACC_X,
> + ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
> + ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> + ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> - ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT, ADIS16201_SCAN_ACC_Y,
> + ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> - ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC, ADIS16201_SCAN_AUX_ADC, 0, 12),
> - ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT, ADIS16201_SCAN_INCLI_X,
> + ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> + ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
> BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> - ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT, ADIS16201_SCAN_INCLI_Y,
> + ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> IIO_CHAN_SOFT_TIMESTAMP(7)
> };
> @@ -237,9 +226,9 @@ static const char * const adis16201_status_error_msgs[] = {
>
> static const struct adis_data adis16201_data = {
> .read_delay = 20,
> - .msc_ctrl_reg = ADIS16201_MSC_CTRL,
> - .glob_cmd_reg = ADIS16201_GLOB_CMD,
> - .diag_stat_reg = ADIS16201_DIAG_STAT,
> + .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
> + .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
> + .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
>
> .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> .self_test_no_autoclear = true,
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-07 20:48 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:48 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:27 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Use switch statement instead of if-else pair to explicitly match
> the only two channels present.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
I think this is going to generate some warnings in the various static
analysers as they will point out there are lots of values channel can
take that aren't handled by the switch statement.. You should have
a default.
(This is what made Dan less than convinced of whether this was a good
change when I originally suggested it) I still think it's a marginal
improvement in making it explicit that we only have two valid choices
though - and Dan didn't care strongly about it.
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 1737708..307d4ab 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
> case IIO_VOLTAGE:
> - if (chan->channel == 0) {
> + switch (chan->channel) {
> + case 0:
> *val = 1;
> *val2 = 220000;
> - } else {
> + break;
> + case 1:
> *val = 0;
> *val2 = 610000;
> + break;
> }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else
@ 2018-03-07 20:48 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:48 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:27 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Use switch statement instead of if-else pair to explicitly match
> the only two channels present.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
I think this is going to generate some warnings in the various static
analysers as they will point out there are lots of values channel can
take that aren't handled by the switch statement.. You should have
a default.
(This is what made Dan less than convinced of whether this was a good
change when I originally suggested it) I still think it's a marginal
improvement in making it explicit that we only have two valid choices
though - and Dan didn't care strongly about it.
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 1737708..307d4ab 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -114,12 +114,15 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_SCALE:
> switch (chan->type) {
> case IIO_VOLTAGE:
> - if (chan->channel == 0) {
> + switch (chan->channel) {
> + case 0:
> *val = 1;
> *val2 = 220000;
> - } else {
> + break;
> + case 1:
> *val = 0;
> *val2 = 610000;
> + break;
> }
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-07 20:50 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:50 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:29 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Clarify the conversion and formation of resultant data in the
> adis16201_read_raw() with sufficient comments.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
This is fine but it needs to be in the original comment changing patch
rather than removing the comments first then a few patches later putting
back a different version.
So good change but in the wrong place in the series. Learning to reorder
a series and merge down multiple patches into one is very useful when
working with git.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 8d795e2..946c7b1 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> *val2 = 0;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_ACCEL:
> + /*
> + * IIO base unit for sensitivity of accelerometer
> + * is milli g.
> + * 1 LSB represents 0.244 mg.
> + */
> *val = 0;
> *val2 = IIO_G_TO_M_S_2(462400);
> return IIO_VAL_INT_PLUS_NANO;
> @@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> + /*
> + * The raw ADC value is 0x4FE when the temperature
> + * is 25 degrees and the scale factor per milli
> + * degree celcius is -470.
> + */
> *val = 25000 / -470 - 1278;
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
@ 2018-03-07 20:50 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:50 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:29 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Clarify the conversion and formation of resultant data in the
> adis16201_read_raw() with sufficient comments.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
This is fine but it needs to be in the original comment changing patch
rather than removing the comments first then a few patches later putting
back a different version.
So good change but in the wrong place in the series. Learning to reorder
a series and merge down multiple patches into one is very useful when
working with git.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16201.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/staging/iio/accel/adis16201.c b/drivers/staging/iio/accel/adis16201.c
> index 8d795e2..946c7b1 100644
> --- a/drivers/staging/iio/accel/adis16201.c
> +++ b/drivers/staging/iio/accel/adis16201.c
> @@ -130,6 +130,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> *val2 = 0;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_ACCEL:
> + /*
> + * IIO base unit for sensitivity of accelerometer
> + * is milli g.
> + * 1 LSB represents 0.244 mg.
> + */
> *val = 0;
> *val2 = IIO_G_TO_M_S_2(462400);
> return IIO_VAL_INT_PLUS_NANO;
> @@ -142,6 +147,11 @@ static int adis16201_read_raw(struct iio_dev *indio_dev,
> }
> break;
> case IIO_CHAN_INFO_OFFSET:
> + /*
> + * The raw ADC value is 0x4FE when the temperature
> + * is 25 degrees and the scale factor per milli
> + * degree celcius is -470.
> + */
> *val = 25000 / -470 - 1278;
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem
2018-03-05 7:49 ` Himanshu Jha
@ 2018-03-07 20:58 ` Jonathan Cameron
-1 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:58 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Mon, 5 Mar 2018 13:19:30 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Move the adis16201 driver out of staging directory and merge to the
> mainline IIO subsystem.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
One comment inline (that I should have noticed in one of your earlier patches but
missed).
Michael, do you have the time to look through these at the moment?
The changes are not likely to break anything so if you are too busy I'm happy to
move them out of staging if you are fine with that.
Thanks,
Jonathan
> ---
> drivers/iio/accel/Kconfig | 12 ++
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++++++++
> drivers/staging/iio/accel/Kconfig | 12 --
> drivers/staging/iio/accel/Makefile | 1 -
> drivers/staging/iio/accel/adis16201.c | 323 ----------------------------------
> 6 files changed, 336 insertions(+), 336 deletions(-)
> create mode 100644 drivers/iio/accel/adis16201.c
> delete mode 100644 drivers/staging/iio/accel/adis16201.c
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index c6d9517..9416c6f 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -5,6 +5,18 @@
>
> menu "Accelerometers"
>
> +config ADIS16201
> + tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
> + depends on SPI
> + select IIO_ADIS_LIB
> + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> + help
> + Say Y here to build support for Analog Devices adis16201 dual-axis
> + digital inclinometer and accelerometer.
> +
> + To compile this driver as a module, say M here: the module will
> + be called adis16201.
> +
> config ADXL345
> tristate
>
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 368aedb..7832ec9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -4,6 +4,7 @@
> #
>
> # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ADIS16201) += adis16201.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> new file mode 100644
> index 0000000..946c7b1
> --- /dev/null
> +++ b/drivers/iio/accel/adis16201.c
> @@ -0,0 +1,323 @@
> +/*
> + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/imu/adis.h>
> +
> +#define ADIS16201_STARTUP_DELAY_MS 220
> +#define ADIS16201_FLASH_CNT 0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG 0x04
> +#define ADIS16201_YACCL_OUT_REG 0x06
> +#define ADIS16201_AUX_ADC_REG 0x08
> +#define ADIS16201_TEMP_OUT_REG 0x0A
> +#define ADIS16201_XINCL_OUT_REG 0x0C
> +#define ADIS16201_YINCL_OUT_REG 0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG 0x14
> +#define ADIS16201_YACCL_SCALE_REG 0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG 0x1C
> +#define ADIS16201_YINCL_SCALE_REG 0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG 0x20
> +#define ADIS16201_ALM_MAG2_REG 0x22
> +#define ADIS16201_ALM_SMPL1_REG 0x24
> +#define ADIS16201_ALM_SMPL2_REG 0x26
> +#define ADIS16201_ALM_CTRL_REG 0x28
> +
> +#define ADIS16201_AUX_DAC_REG 0x30
> +#define ADIS16201_GPIO_CTRL_REG 0x32
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_SMPL_PRD_REG 0x36
> +#define ADIS16201_AVG_CNT_REG 0x38
> +#define ADIS16201_SLP_CNT_REG 0x3A
> +
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
> +
> +/* Diagnostics, system status register */
> +#define ADIS16201_DIAG_STAT_REG 0x3C
> +#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
> +#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
> +#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
> +#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> +#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
> +
> +/* Operation, system command register */
> +#define ADIS16201_GLOB_CMD_REG 0x3E
> +#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> +#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
> +
> +#define ADIS16201_ERROR_ACTIVE BIT(14)
> +
> +enum adis16201_scan {
> + ADIS16201_SCAN_ACC_X,
> + ADIS16201_SCAN_ACC_Y,
> + ADIS16201_SCAN_INCLI_X,
> + ADIS16201_SCAN_INCLI_Y,
> + ADIS16201_SCAN_SUPPLY,
> + ADIS16201_SCAN_AUX_ADC,
> + ADIS16201_SCAN_TEMP,
> +};
> +
> +static const u8 adis16201_addresses[] = {
> + [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
> + [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
> +};
> +
> +static int adis16201_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2,
> + long mask)
> +{
> + struct adis *st = iio_priv(indio_dev);
> + int ret;
> + int bits;
> + u8 addr;
> + s16 val16;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return adis_single_conversion(indio_dev, chan,
> + ADIS16201_ERROR_ACTIVE, val);
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + switch (chan->channel) {
> + case 0:
> + *val = 1;
> + *val2 = 220000;
> + break;
> + case 1:
> + *val = 0;
> + *val2 = 610000;
> + break;
> + }
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_TEMP:
> + *val = -470;
> + *val2 = 0;
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_ACCEL:
> + /*
> + * IIO base unit for sensitivity of accelerometer
> + * is milli g.
> + * 1 LSB represents 0.244 mg.
> + */
> + *val = 0;
> + *val2 = IIO_G_TO_M_S_2(462400);
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_INCLI:
> + *val = 0;
> + *val2 = 100000;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case IIO_CHAN_INFO_OFFSET:
> + /*
> + * The raw ADC value is 0x4FE when the temperature
Ah, I missed this earlier. Why have it in hex here in the comment
and decimal in the maths. Make them the same (either way round
is fine)
> + * is 25 degrees and the scale factor per milli
> + * degree celcius is -470.
> + */
> + *val = 25000 / -470 - 1278;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + bits = 12;
> + break;
> + case IIO_INCLI:
> + bits = 9;
> + break;
> + default:
> + return -EINVAL;
> + }
> + addr = adis16201_addresses[chan->scan_index];
> + ret = adis_read_reg_16(st, addr, &val16);
> + if (ret)
> + return ret;
> +
> + *val = sign_extend32(val16, bits - 1);
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int adis16201_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask)
> +{
> + struct adis *st = iio_priv(indio_dev);
> + int bits;
> + s16 val16;
> + u8 addr;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBBIAS:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + bits = 12;
> + break;
> + case IIO_INCLI:
> + bits = 9;
> + break;
> + default:
> + return -EINVAL;
> + }
> + val16 = val & ((1 << bits) - 1);
> + addr = adis16201_addresses[chan->scan_index];
> + return adis_write_reg_16(st, addr, val16);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec adis16201_channels[] = {
> + ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
> + ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> + ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> + ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + IIO_CHAN_SOFT_TIMESTAMP(7)
> +};
> +
> +static const struct iio_info adis16201_info = {
> + .read_raw = adis16201_read_raw,
> + .write_raw = adis16201_write_raw,
> + .update_scan_mode = adis_update_scan_mode,
> +};
> +
> +static const char * const adis16201_status_error_msgs[] = {
> + [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> + [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
> +};
> +
> +static const struct adis_data adis16201_data = {
> + .read_delay = 20,
> + .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
> + .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
> + .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
> +
> + .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> + .self_test_no_autoclear = true,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
> +
> + .status_error_msgs = adis16201_status_error_msgs,
> + .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> + BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> + BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> +};
> +
> +static int adis16201_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct adis *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + spi_set_drvdata(spi, indio_dev);
> +
> + indio_dev->name = spi->dev.driver->name;
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->info = &adis16201_info;
> +
> + indio_dev->channels = adis16201_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = adis_init(st, indio_dev, spi, &adis16201_data);
> + if (ret)
> + return ret;
> +
> + ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> + if (ret)
> + return ret;
> +
> + ret = adis_initial_startup(st);
> + if (ret)
> + goto error_cleanup_buffer_trigger;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0)
> + goto error_cleanup_buffer_trigger;
> +
> + return 0;
> +
> +error_cleanup_buffer_trigger:
> + adis_cleanup_buffer_and_trigger(st, indio_dev);
> + return ret;
> +}
> +
> +static int adis16201_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct adis *st = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + adis_cleanup_buffer_and_trigger(st, indio_dev);
> +
> + return 0;
> +}
> +
> +static struct spi_driver adis16201_driver = {
> + .driver = {
> + .name = "adis16201",
> + },
> + .probe = adis16201_probe,
> + .remove = adis16201_remove,
> +};
> +module_spi_driver(adis16201_driver);
> +
> +MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("spi:adis16201");
<snip>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem
@ 2018-03-07 20:58 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-07 20:58 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Mon, 5 Mar 2018 13:19:30 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> Move the adis16201 driver out of staging directory and merge to the
> mainline IIO subsystem.
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
One comment inline (that I should have noticed in one of your earlier patches but
missed).
Michael, do you have the time to look through these at the moment?
The changes are not likely to break anything so if you are too busy I'm happy to
move them out of staging if you are fine with that.
Thanks,
Jonathan
> ---
> drivers/iio/accel/Kconfig | 12 ++
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adis16201.c | 323 ++++++++++++++++++++++++++++++++++
> drivers/staging/iio/accel/Kconfig | 12 --
> drivers/staging/iio/accel/Makefile | 1 -
> drivers/staging/iio/accel/adis16201.c | 323 ----------------------------------
> 6 files changed, 336 insertions(+), 336 deletions(-)
> create mode 100644 drivers/iio/accel/adis16201.c
> delete mode 100644 drivers/staging/iio/accel/adis16201.c
>
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index c6d9517..9416c6f 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -5,6 +5,18 @@
>
> menu "Accelerometers"
>
> +config ADIS16201
> + tristate "Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer"
> + depends on SPI
> + select IIO_ADIS_LIB
> + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
> + help
> + Say Y here to build support for Analog Devices adis16201 dual-axis
> + digital inclinometer and accelerometer.
> +
> + To compile this driver as a module, say M here: the module will
> + be called adis16201.
> +
> config ADXL345
> tristate
>
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 368aedb..7832ec9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -4,6 +4,7 @@
> #
>
> # When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_ADIS16201) += adis16201.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
> new file mode 100644
> index 0000000..946c7b1
> --- /dev/null
> +++ b/drivers/iio/accel/adis16201.c
> @@ -0,0 +1,323 @@
> +/*
> + * ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer
> + *
> + * Copyright 2010 Analog Devices Inc.
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/imu/adis.h>
> +
> +#define ADIS16201_STARTUP_DELAY_MS 220
> +#define ADIS16201_FLASH_CNT 0x00
> +
> +/* Data Output Register Information */
> +#define ADIS16201_SUPPLY_OUT_REG 0x02
> +#define ADIS16201_XACCL_OUT_REG 0x04
> +#define ADIS16201_YACCL_OUT_REG 0x06
> +#define ADIS16201_AUX_ADC_REG 0x08
> +#define ADIS16201_TEMP_OUT_REG 0x0A
> +#define ADIS16201_XINCL_OUT_REG 0x0C
> +#define ADIS16201_YINCL_OUT_REG 0x0E
> +
> +/* Calibration Register Definition */
> +#define ADIS16201_XACCL_OFFS_REG 0x10
> +#define ADIS16201_YACCL_OFFS_REG 0x12
> +#define ADIS16201_XACCL_SCALE_REG 0x14
> +#define ADIS16201_YACCL_SCALE_REG 0x16
> +#define ADIS16201_XINCL_OFFS_REG 0x18
> +#define ADIS16201_YINCL_OFFS_REG 0x1A
> +#define ADIS16201_XINCL_SCALE_REG 0x1C
> +#define ADIS16201_YINCL_SCALE_REG 0x1E
> +
> +/* Alarm Register Definition */
> +#define ADIS16201_ALM_MAG1_REG 0x20
> +#define ADIS16201_ALM_MAG2_REG 0x22
> +#define ADIS16201_ALM_SMPL1_REG 0x24
> +#define ADIS16201_ALM_SMPL2_REG 0x26
> +#define ADIS16201_ALM_CTRL_REG 0x28
> +
> +#define ADIS16201_AUX_DAC_REG 0x30
> +#define ADIS16201_GPIO_CTRL_REG 0x32
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_SMPL_PRD_REG 0x36
> +#define ADIS16201_AVG_CNT_REG 0x38
> +#define ADIS16201_SLP_CNT_REG 0x3A
> +
> +/* Miscellaneous Control Register Definition */
> +#define ADIS16201_MSC_CTRL_REG 0x34
> +#define ADIS16201_MSC_CTRL_SELF_TEST_EN BIT(8)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_EN BIT(2)
> +#define ADIS16201_MSC_CTRL_ACTIVE_DATA_RDY_HIGH BIT(1)
> +#define ADIS16201_MSC_CTRL_DATA_RDY_DIO1 BIT(0)
> +
> +/* Diagnostics, system status register */
> +#define ADIS16201_DIAG_STAT_REG 0x3C
> +#define ADIS16201_DIAG_STAT_ALARM2 BIT(9)
> +#define ADIS16201_DIAG_STAT_ALARM1 BIT(8)
> +#define ADIS16201_DIAG_STAT_SPI_FAIL_BIT 3
> +#define ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT 2
> +#define ADIS16201_DIAG_STAT_POWER_HIGH_BIT 1
> +#define ADIS16201_DIAG_STAT_POWER_LOW_BIT 0
> +
> +/* Operation, system command register */
> +#define ADIS16201_GLOB_CMD_REG 0x3E
> +#define ADIS16201_GLOB_CMD_SW_RESET BIT(7)
> +#define ADIS16201_GLOB_CMD_FACTORY BIT(1)
> +
> +#define ADIS16201_ERROR_ACTIVE BIT(14)
> +
> +enum adis16201_scan {
> + ADIS16201_SCAN_ACC_X,
> + ADIS16201_SCAN_ACC_Y,
> + ADIS16201_SCAN_INCLI_X,
> + ADIS16201_SCAN_INCLI_Y,
> + ADIS16201_SCAN_SUPPLY,
> + ADIS16201_SCAN_AUX_ADC,
> + ADIS16201_SCAN_TEMP,
> +};
> +
> +static const u8 adis16201_addresses[] = {
> + [ADIS16201_SCAN_ACC_X] = ADIS16201_XACCL_OFFS_REG,
> + [ADIS16201_SCAN_ACC_Y] = ADIS16201_YACCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_X] = ADIS16201_XINCL_OFFS_REG,
> + [ADIS16201_SCAN_INCLI_Y] = ADIS16201_YINCL_OFFS_REG,
> +};
> +
> +static int adis16201_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2,
> + long mask)
> +{
> + struct adis *st = iio_priv(indio_dev);
> + int ret;
> + int bits;
> + u8 addr;
> + s16 val16;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + return adis_single_conversion(indio_dev, chan,
> + ADIS16201_ERROR_ACTIVE, val);
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_VOLTAGE:
> + switch (chan->channel) {
> + case 0:
> + *val = 1;
> + *val2 = 220000;
> + break;
> + case 1:
> + *val = 0;
> + *val2 = 610000;
> + break;
> + }
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_TEMP:
> + *val = -470;
> + *val2 = 0;
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_ACCEL:
> + /*
> + * IIO base unit for sensitivity of accelerometer
> + * is milli g.
> + * 1 LSB represents 0.244 mg.
> + */
> + *val = 0;
> + *val2 = IIO_G_TO_M_S_2(462400);
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_INCLI:
> + *val = 0;
> + *val2 = 100000;
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> + break;
> + case IIO_CHAN_INFO_OFFSET:
> + /*
> + * The raw ADC value is 0x4FE when the temperature
Ah, I missed this earlier. Why have it in hex here in the comment
and decimal in the maths. Make them the same (either way round
is fine)
> + * is 25 degrees and the scale factor per milli
> + * degree celcius is -470.
> + */
> + *val = 25000 / -470 - 1278;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_CALIBBIAS:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + bits = 12;
> + break;
> + case IIO_INCLI:
> + bits = 9;
> + break;
> + default:
> + return -EINVAL;
> + }
> + addr = adis16201_addresses[chan->scan_index];
> + ret = adis_read_reg_16(st, addr, &val16);
> + if (ret)
> + return ret;
> +
> + *val = sign_extend32(val16, bits - 1);
> + return IIO_VAL_INT;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int adis16201_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask)
> +{
> + struct adis *st = iio_priv(indio_dev);
> + int bits;
> + s16 val16;
> + u8 addr;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_CALIBBIAS:
> + switch (chan->type) {
> + case IIO_ACCEL:
> + bits = 12;
> + break;
> + case IIO_INCLI:
> + bits = 9;
> + break;
> + default:
> + return -EINVAL;
> + }
> + val16 = val & ((1 << bits) - 1);
> + addr = adis16201_addresses[chan->scan_index];
> + return adis_write_reg_16(st, addr, val16);
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec adis16201_channels[] = {
> + ADIS_SUPPLY_CHAN(ADIS16201_SUPPLY_OUT_REG, ADIS16201_SCAN_SUPPLY, 0, 12),
> + ADIS_TEMP_CHAN(ADIS16201_TEMP_OUT_REG, ADIS16201_SCAN_TEMP, 0, 12),
> + ADIS_ACCEL_CHAN(X, ADIS16201_XACCL_OUT_REG, ADIS16201_SCAN_ACC_X,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_ACCEL_CHAN(Y, ADIS16201_YACCL_OUT_REG, ADIS16201_SCAN_ACC_Y,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_AUX_ADC_CHAN(ADIS16201_AUX_ADC_REG, ADIS16201_SCAN_AUX_ADC, 0, 12),
> + ADIS_INCLI_CHAN(X, ADIS16201_XINCL_OUT_REG, ADIS16201_SCAN_INCLI_X,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + ADIS_INCLI_CHAN(X, ADIS16201_YINCL_OUT_REG, ADIS16201_SCAN_INCLI_Y,
> + BIT(IIO_CHAN_INFO_CALIBBIAS), 0, 14),
> + IIO_CHAN_SOFT_TIMESTAMP(7)
> +};
> +
> +static const struct iio_info adis16201_info = {
> + .read_raw = adis16201_read_raw,
> + .write_raw = adis16201_write_raw,
> + .update_scan_mode = adis_update_scan_mode,
> +};
> +
> +static const char * const adis16201_status_error_msgs[] = {
> + [ADIS16201_DIAG_STAT_SPI_FAIL_BIT] = "SPI failure",
> + [ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT] = "Flash update failed",
> + [ADIS16201_DIAG_STAT_POWER_HIGH_BIT] = "Power supply above 3.625V",
> + [ADIS16201_DIAG_STAT_POWER_LOW_BIT] = "Power supply below 3.15V",
> +};
> +
> +static const struct adis_data adis16201_data = {
> + .read_delay = 20,
> + .msc_ctrl_reg = ADIS16201_MSC_CTRL_REG,
> + .glob_cmd_reg = ADIS16201_GLOB_CMD_REG,
> + .diag_stat_reg = ADIS16201_DIAG_STAT_REG,
> +
> + .self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
> + .self_test_no_autoclear = true,
> + .startup_delay = ADIS16201_STARTUP_DELAY_MS,
> +
> + .status_error_msgs = adis16201_status_error_msgs,
> + .status_error_mask = BIT(ADIS16201_DIAG_STAT_SPI_FAIL_BIT) |
> + BIT(ADIS16201_DIAG_STAT_FLASH_UPT_FAIL_BIT) |
> + BIT(ADIS16201_DIAG_STAT_POWER_HIGH_BIT) |
> + BIT(ADIS16201_DIAG_STAT_POWER_LOW_BIT),
> +};
> +
> +static int adis16201_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct adis *st;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + st = iio_priv(indio_dev);
> + spi_set_drvdata(spi, indio_dev);
> +
> + indio_dev->name = spi->dev.driver->name;
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->info = &adis16201_info;
> +
> + indio_dev->channels = adis16201_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adis16201_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = adis_init(st, indio_dev, spi, &adis16201_data);
> + if (ret)
> + return ret;
> +
> + ret = adis_setup_buffer_and_trigger(st, indio_dev, NULL);
> + if (ret)
> + return ret;
> +
> + ret = adis_initial_startup(st);
> + if (ret)
> + goto error_cleanup_buffer_trigger;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret < 0)
> + goto error_cleanup_buffer_trigger;
> +
> + return 0;
> +
> +error_cleanup_buffer_trigger:
> + adis_cleanup_buffer_and_trigger(st, indio_dev);
> + return ret;
> +}
> +
> +static int adis16201_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> + struct adis *st = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + adis_cleanup_buffer_and_trigger(st, indio_dev);
> +
> + return 0;
> +}
> +
> +static struct spi_driver adis16201_driver = {
> + .driver = {
> + .name = "adis16201",
> + },
> + .probe = adis16201_probe,
> + .remove = adis16201_remove,
> +};
> +module_spi_driver(adis16201_driver);
> +
> +MODULE_AUTHOR("Barry Song <21cnbao@gmail.com>");
> +MODULE_DESCRIPTION("Analog Devices ADIS16201 Dual-Axis Digital Inclinometer and Accelerometer");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("spi:adis16201");
<snip>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
2018-03-07 20:50 ` Jonathan Cameron
(?)
@ 2018-03-08 7:28 ` Himanshu Jha
2018-03-10 15:05 ` Jonathan Cameron
-1 siblings, 1 reply; 43+ messages in thread
From: Himanshu Jha @ 2018-03-08 7:28 UTC (permalink / raw)
To: Jonathan Cameron
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Wed, Mar 07, 2018 at 08:50:30PM +0000, Jonathan Cameron wrote:
> On Mon, 5 Mar 2018 13:19:29 +0530
> Himanshu Jha <himanshujha199640@gmail.com> wrote:
>
> > Clarify the conversion and formation of resultant data in the
> > adis16201_read_raw() with sufficient comments.
> >
> > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> This is fine but it needs to be in the original comment changing patch
> rather than removing the comments first then a few patches later putting
> back a different version.
>
> So good change but in the wrong place in the series. Learning to reorder
> a series and merge down multiple patches into one is very useful when
> working with git.
I will send v2 with all the updates!
What about sign_extend32 & reverse christmas ordering patch ? Will you
pick that up or shall I send them again ?
--
Thanks
Himanshu Jha
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
2018-03-08 7:28 ` Himanshu Jha
@ 2018-03-10 15:05 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-10 15:05 UTC (permalink / raw)
To: Himanshu Jha
Cc: devel, daniel.baluta, lars, Michael.Hennerich, linux-iio, gregkh,
linux-kernel, pmeerw, knaack.h
On Thu, 8 Mar 2018 12:58:19 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> On Wed, Mar 07, 2018 at 08:50:30PM +0000, Jonathan Cameron wrote:
> > On Mon, 5 Mar 2018 13:19:29 +0530
> > Himanshu Jha <himanshujha199640@gmail.com> wrote:
> >
> > > Clarify the conversion and formation of resultant data in the
> > > adis16201_read_raw() with sufficient comments.
> > >
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > This is fine but it needs to be in the original comment changing patch
> > rather than removing the comments first then a few patches later putting
> > back a different version.
> >
> > So good change but in the wrong place in the series. Learning to reorder
> > a series and merge down multiple patches into one is very useful when
> > working with git.
>
> I will send v2 with all the updates!
>
> What about sign_extend32 & reverse christmas ordering patch ? Will you
> pick that up or shall I send them again ?
Please repost anything not yet applied in V2 of the series.
Makes it much easier for me to avoid missing patches!
Thanks,
Jonathan
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function
@ 2018-03-10 15:05 ` Jonathan Cameron
0 siblings, 0 replies; 43+ messages in thread
From: Jonathan Cameron @ 2018-03-10 15:05 UTC (permalink / raw)
To: Himanshu Jha
Cc: lars, Michael.Hennerich, knaack.h, pmeerw, gregkh, linux-iio,
devel, linux-kernel, daniel.baluta
On Thu, 8 Mar 2018 12:58:19 +0530
Himanshu Jha <himanshujha199640@gmail.com> wrote:
> On Wed, Mar 07, 2018 at 08:50:30PM +0000, Jonathan Cameron wrote:
> > On Mon, 5 Mar 2018 13:19:29 +0530
> > Himanshu Jha <himanshujha199640@gmail.com> wrote:
> >
> > > Clarify the conversion and formation of resultant data in the
> > > adis16201_read_raw() with sufficient comments.
> > >
> > > Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> > This is fine but it needs to be in the original comment changing patch
> > rather than removing the comments first then a few patches later putting
> > back a different version.
> >
> > So good change but in the wrong place in the series. Learning to reorder
> > a series and merge down multiple patches into one is very useful when
> > working with git.
>
> I will send v2 with all the updates!
>
> What about sign_extend32 & reverse christmas ordering patch ? Will you
> pick that up or shall I send them again ?
Please repost anything not yet applied in V2 of the series.
Makes it much easier for me to avoid missing patches!
Thanks,
Jonathan
>
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2018-03-10 15:05 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-05 7:49 [PATCH 00/11] staging: iio: accel: adis16201 driver cleanup Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-05 7:49 ` [PATCH 01/11] Staging: iio: accel: Prefer alphabetical sequence of header files Himanshu Jha
2018-03-07 20:29 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 02/11] Staging: iio: accel: Add a blank space before returns Himanshu Jha
2018-03-07 20:32 ` Jonathan Cameron
2018-03-07 20:32 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 03/11] Staging: iio: accel: Remove unnecessary comments Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-06 21:26 ` Shreeya Patel
2018-03-06 22:25 ` Himanshu Jha
2018-03-07 20:40 ` Jonathan Cameron
2018-03-07 20:40 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 04/11] Staging: iio: accel: Rename few macro definitions Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:42 ` Jonathan Cameron
2018-03-07 20:42 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 05/11] Staging: iio: accel: Add _REG suffix to registers Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:44 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 06/11] Staging: iio: accel: Reverse christmas tree Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-05 7:49 ` [PATCH 07/11] Staging: iio: accel: Adjust arguments to match open parentheses Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-05 10:16 ` Dan Carpenter
2018-03-05 10:16 ` Dan Carpenter
2018-03-05 7:49 ` [PATCH 08/11] Staging: iio: accel: Use switch statement than if-else Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:48 ` Jonathan Cameron
2018-03-07 20:48 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 09/11] Staging: iio: accel: Use sign_extend32 function Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-05 7:49 ` [PATCH 10/11] Staging: iio: accel: Add comments about units in data read function Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:50 ` Jonathan Cameron
2018-03-07 20:50 ` Jonathan Cameron
2018-03-08 7:28 ` Himanshu Jha
2018-03-10 15:05 ` Jonathan Cameron
2018-03-10 15:05 ` Jonathan Cameron
2018-03-05 7:49 ` [PATCH 11/11] Staging: iio: accel: Move adis16201 driver out of staging subsystem Himanshu Jha
2018-03-05 7:49 ` Himanshu Jha
2018-03-07 20:58 ` Jonathan Cameron
2018-03-07 20:58 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.