* [PATCH 1/7] dt-bindings: iio: imu: Add ADIS16501 compatibles
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-26 16:36 ` Conor Dooley
2024-04-26 13:53 ` [PATCH 2/7] drivers: iio: imu: Add support for ADIS16501 Ramona Gradinariu
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Add ADIS16501 compatible. Similarly to other ADIS1650X devices,
ADIS16501 supports sync-mode values [0,2].
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
index 9b7ad609f7db..db52e7063116 100644
--- a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
@@ -30,6 +30,7 @@ properties:
- adi,adis16467-2
- adi,adis16467-3
- adi,adis16500
+ - adi,adis16501
- adi,adis16505-1
- adi,adis16505-2
- adi,adis16505-3
@@ -90,6 +91,7 @@ allOf:
contains:
enum:
- adi,adis16500
+ - adi,adis16501
- adi,adis16505-1
- adi,adis16505-2
- adi,adis16505-3
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/7] dt-bindings: iio: imu: Add ADIS16501 compatibles
2024-04-26 13:53 ` [PATCH 1/7] dt-bindings: iio: imu: Add ADIS16501 compatibles Ramona Gradinariu
@ 2024-04-26 16:36 ` Conor Dooley
0 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2024-04-26 16:36 UTC (permalink / raw)
To: Ramona Gradinariu
Cc: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]
On Fri, Apr 26, 2024 at 04:53:33PM +0300, Ramona Gradinariu wrote:
> Add ADIS16501 compatible. Similarly to other ADIS1650X devices,
> ADIS16501 supports sync-mode values [0,2].
To avoid me having to check the driver patch, you should point out what
makes this device different from the existing drivers and therefore
justify not having a fallback compatible.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
>
> Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
> ---
> Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> index 9b7ad609f7db..db52e7063116 100644
> --- a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> @@ -30,6 +30,7 @@ properties:
> - adi,adis16467-2
> - adi,adis16467-3
> - adi,adis16500
> + - adi,adis16501
> - adi,adis16505-1
> - adi,adis16505-2
> - adi,adis16505-3
> @@ -90,6 +91,7 @@ allOf:
> contains:
> enum:
> - adi,adis16500
> + - adi,adis16501
> - adi,adis16505-1
> - adi,adis16505-2
> - adi,adis16505-3
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/7] drivers: iio: imu: Add support for ADIS16501
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 1/7] dt-bindings: iio: imu: Add ADIS16501 compatibles Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA Ramona Gradinariu
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Add support for ADIS16501 device in already existing ADIS16475
driver.
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
drivers/iio/imu/Kconfig | 4 ++--
drivers/iio/imu/adis16475.c | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 52a155ff3250..782fb80e44c2 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -36,8 +36,8 @@ config ADIS16475
select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
help
Say yes here to build support for Analog Devices ADIS16470, ADIS16475,
- ADIS16477, ADIS16465, ADIS16467, ADIS16500, ADIS16505, ADIS16507 inertial
- sensors.
+ ADIS16477, ADIS16465, ADIS16467, ADIS16500, ADIS16501, ADIS16505,
+ ADIS16507 inertial sensors.
To compile this driver as a module, choose M here: the module will be
called adis16475.
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 01f55cc902fa..7fab40daa659 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -661,6 +661,7 @@ enum adis16475_variant {
ADIS16467_2,
ADIS16467_3,
ADIS16500,
+ ADIS16501,
ADIS16505_1,
ADIS16505_2,
ADIS16505_3,
@@ -980,6 +981,25 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
.adis_data = ADIS16475_DATA(16500, &adis1650x_timeouts),
},
+ [ADIS16501] = {
+ .name = "adis16501",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
+ .accel_max_val = 1,
+ .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(720),
+ .deltvel_max_val = 125,
+ .int_clk = 2000,
+ .max_dec = 1999,
+ .sync = adis16475_sync_mode,
+ /* pulse sync not supported */
+ .num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
+ .flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
+ .adis_data = ADIS16475_DATA(16501, &adis1650x_timeouts),
+ },
[ADIS16505_1] = {
.name = "adis16505-1",
.num_channels = ARRAY_SIZE(adis16477_channels),
@@ -1482,6 +1502,8 @@ static const struct of_device_id adis16475_of_match[] = {
.data = &adis16475_chip_info[ADIS16467_3] },
{ .compatible = "adi,adis16500",
.data = &adis16475_chip_info[ADIS16500] },
+ { .compatible = "adi,adis16501",
+ .data = &adis16475_chip_info[ADIS16501] },
{ .compatible = "adi,adis16505-1",
.data = &adis16475_chip_info[ADIS16505_1] },
{ .compatible = "adi,adis16505-2",
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 1/7] dt-bindings: iio: imu: Add ADIS16501 compatibles Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 2/7] drivers: iio: imu: Add support for ADIS16501 Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-27 2:00 ` kernel test robot
2024-04-27 2:43 ` kernel test robot
2024-04-26 13:53 ` [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes Ramona Gradinariu
` (3 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Re-define ADIS16475_DATA such that it takes _burst_max_len and
_burst_max_speed_hz as parameters.
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
drivers/iio/imu/adis16475.c | 136 +++++++++++++++++++++++-------------
1 file changed, 89 insertions(+), 47 deletions(-)
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 7fab40daa659..52333772fbe0 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -690,32 +690,32 @@ static const char * const adis16475_status_error_msgs[] = {
[ADIS16475_DIAG_STAT_CLK] = "Clock error",
};
-#define ADIS16475_DATA(_prod_id, _timeouts) \
-{ \
- .msc_ctrl_reg = ADIS16475_REG_MSG_CTRL, \
- .glob_cmd_reg = ADIS16475_REG_GLOB_CMD, \
- .diag_stat_reg = ADIS16475_REG_DIAG_STAT, \
- .prod_id_reg = ADIS16475_REG_PROD_ID, \
- .prod_id = (_prod_id), \
- .self_test_mask = BIT(2), \
- .self_test_reg = ADIS16475_REG_GLOB_CMD, \
- .cs_change_delay = 16, \
- .read_delay = 5, \
- .write_delay = 5, \
- .status_error_msgs = adis16475_status_error_msgs, \
- .status_error_mask = BIT(ADIS16475_DIAG_STAT_DATA_PATH) | \
- BIT(ADIS16475_DIAG_STAT_FLASH_MEM) | \
- BIT(ADIS16475_DIAG_STAT_SPI) | \
- BIT(ADIS16475_DIAG_STAT_STANDBY) | \
- BIT(ADIS16475_DIAG_STAT_SENSOR) | \
- BIT(ADIS16475_DIAG_STAT_MEMORY) | \
- BIT(ADIS16475_DIAG_STAT_CLK), \
- .unmasked_drdy = true, \
- .timeouts = (_timeouts), \
- .burst_reg_cmd = ADIS16475_REG_GLOB_CMD, \
- .burst_len = ADIS16475_BURST_MAX_DATA, \
- .burst_max_len = ADIS16475_BURST32_MAX_DATA, \
- .burst_max_speed_hz = ADIS16475_BURST_MAX_SPEED \
+#define ADIS16475_DATA(_prod_id, _timeouts, _burst_max_len, _burst_max_speed_hz) \
+{ \
+ .msc_ctrl_reg = ADIS16475_REG_MSG_CTRL, \
+ .glob_cmd_reg = ADIS16475_REG_GLOB_CMD, \
+ .diag_stat_reg = ADIS16475_REG_DIAG_STAT, \
+ .prod_id_reg = ADIS16475_REG_PROD_ID, \
+ .prod_id = (_prod_id), \
+ .self_test_mask = BIT(2), \
+ .self_test_reg = ADIS16475_REG_GLOB_CMD, \
+ .cs_change_delay = 16, \
+ .read_delay = 5, \
+ .write_delay = 5, \
+ .status_error_msgs = adis16475_status_error_msgs, \
+ .status_error_mask = BIT(ADIS16475_DIAG_STAT_DATA_PATH) | \
+ BIT(ADIS16475_DIAG_STAT_FLASH_MEM) | \
+ BIT(ADIS16475_DIAG_STAT_SPI) | \
+ BIT(ADIS16475_DIAG_STAT_STANDBY) | \
+ BIT(ADIS16475_DIAG_STAT_SENSOR) | \
+ BIT(ADIS16475_DIAG_STAT_MEMORY) | \
+ BIT(ADIS16475_DIAG_STAT_CLK), \
+ .unmasked_drdy = true, \
+ .timeouts = (_timeouts), \
+ .burst_reg_cmd = ADIS16475_REG_GLOB_CMD, \
+ .burst_len = ADIS16475_BURST_MAX_DATA, \
+ .burst_max_len = _burst_max_len, \
+ .burst_max_speed_hz = _burst_max_speed_hz \
}
static const struct adis16475_sync adis16475_sync_mode[] = {
@@ -753,7 +753,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16470, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16470, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16475_1] = {
.name = "adis16475-1",
@@ -770,7 +772,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16475_2] = {
.name = "adis16475-2",
@@ -787,7 +791,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16475_3] = {
.name = "adis16475-3",
@@ -804,7 +810,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16475, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16477_1] = {
.name = "adis16477-1",
@@ -822,7 +830,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16477_2] = {
.name = "adis16477-2",
@@ -840,7 +850,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16477_3] = {
.name = "adis16477-3",
@@ -858,7 +870,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16465_1] = {
.name = "adis16465-1",
@@ -875,7 +889,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16465_2] = {
.name = "adis16465-2",
@@ -892,7 +908,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16465_3] = {
.name = "adis16465-3",
@@ -909,7 +927,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16465, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16467_1] = {
.name = "adis16467-1",
@@ -926,7 +946,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16467_2] = {
.name = "adis16467-2",
@@ -943,7 +965,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16467_3] = {
.name = "adis16467-3",
@@ -960,7 +984,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
- .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts),
+ .adis_data = ADIS16475_DATA(16467, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16500] = {
.name = "adis16500",
@@ -979,7 +1005,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16500, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16500, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16501] = {
.name = "adis16501",
@@ -998,7 +1026,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16501, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16501, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16505_1] = {
.name = "adis16505-1",
@@ -1017,7 +1047,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16505, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16505, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16505_2] = {
.name = "adis16505-2",
@@ -1036,7 +1068,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16505, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16505, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16505_3] = {
.name = "adis16505-3",
@@ -1055,7 +1089,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16505, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16505, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16507_1] = {
.name = "adis16507-1",
@@ -1074,7 +1110,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16507, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16507, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16507_2] = {
.name = "adis16507-2",
@@ -1093,7 +1131,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16507, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16507, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
[ADIS16507_3] = {
.name = "adis16507-3",
@@ -1112,7 +1152,9 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
/* pulse sync not supported */
.num_sync = ARRAY_SIZE(adis16475_sync_mode) - 1,
.flags = ADIS16475_HAS_BURST32 | ADIS16475_HAS_BURST_DELTA_DATA,
- .adis_data = ADIS16475_DATA(16507, &adis1650x_timeouts),
+ .adis_data = ADIS16475_DATA(16507, &adis16475_timeouts,
+ ADIS16475_BURST32_MAX_DATA,
+ ADIS16475_BURST_MAX_SPEED),
},
};
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
2024-04-26 13:53 ` [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA Ramona Gradinariu
@ 2024-04-27 2:00 ` kernel test robot
2024-04-28 13:49 ` Jonathan Cameron
2024-04-27 2:43 ` kernel test robot
1 sibling, 1 reply; 17+ messages in thread
From: kernel test robot @ 2024-04-27 2:00 UTC (permalink / raw)
To: Ramona Gradinariu, linux-kernel, jic23, linux-iio, devicetree,
conor+dt, krzysztof.kozlowski+dt, robh, nuno.sa
Cc: oe-kbuild-all, Ramona Gradinariu
Hi Ramona,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.9-rc5 next-20240426]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ramona-Gradinariu/dt-bindings-iio-imu-Add-ADIS16501-compatibles/20240426-215728
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240426135339.185602-4-ramona.bolboaca13%40gmail.com
patch subject: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240427/202404270958.43fSMp4J-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404270958.43fSMp4J-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404270958.43fSMp4J-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/iio/imu/adis16475.c:734:34: warning: 'adis1650x_timeouts' defined but not used [-Wunused-const-variable=]
734 | static const struct adis_timeout adis1650x_timeouts = {
| ^~~~~~~~~~~~~~~~~~
vim +/adis1650x_timeouts +734 drivers/iio/imu/adis16475.c
fff7352bf7a3ce Nuno Sá 2020-04-13 733
fff7352bf7a3ce Nuno Sá 2020-04-13 @734 static const struct adis_timeout adis1650x_timeouts = {
fff7352bf7a3ce Nuno Sá 2020-04-13 735 .reset_ms = 260,
fff7352bf7a3ce Nuno Sá 2020-04-13 736 .sw_reset_ms = 260,
fff7352bf7a3ce Nuno Sá 2020-04-13 737 .self_test_ms = 30,
fff7352bf7a3ce Nuno Sá 2020-04-13 738 };
fff7352bf7a3ce Nuno Sá 2020-04-13 739
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
2024-04-27 2:00 ` kernel test robot
@ 2024-04-28 13:49 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2024-04-28 13:49 UTC (permalink / raw)
To: kernel test robot
Cc: Ramona Gradinariu, linux-kernel, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa, oe-kbuild-all
On Sat, 27 Apr 2024 10:00:49 +0800
kernel test robot <lkp@intel.com> wrote:
> Hi Ramona,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on jic23-iio/togreg]
> [also build test WARNING on linus/master v6.9-rc5 next-20240426]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Ramona-Gradinariu/dt-bindings-iio-imu-Add-ADIS16501-compatibles/20240426-215728
> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link: https://lore.kernel.org/r/20240426135339.185602-4-ramona.bolboaca13%40gmail.com
> patch subject: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
> config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240427/202404270958.43fSMp4J-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404270958.43fSMp4J-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202404270958.43fSMp4J-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/iio/imu/adis16475.c:734:34: warning: 'adis1650x_timeouts' defined but not used [-Wunused-const-variable=]
> 734 | static const struct adis_timeout adis1650x_timeouts = {
> | ^~~~~~~~~~~~~~~~~~
>
I missed that entirely when reading. Indeed, looks like a cut and paste
issue where some entries should still be using this structure and got
accidentally modified.
A case of robots saving the day :)
Jonathan
>
> vim +/adis1650x_timeouts +734 drivers/iio/imu/adis16475.c
>
> fff7352bf7a3ce Nuno Sá 2020-04-13 733
> fff7352bf7a3ce Nuno Sá 2020-04-13 @734 static const struct adis_timeout adis1650x_timeouts = {
> fff7352bf7a3ce Nuno Sá 2020-04-13 735 .reset_ms = 260,
> fff7352bf7a3ce Nuno Sá 2020-04-13 736 .sw_reset_ms = 260,
> fff7352bf7a3ce Nuno Sá 2020-04-13 737 .self_test_ms = 30,
> fff7352bf7a3ce Nuno Sá 2020-04-13 738 };
> fff7352bf7a3ce Nuno Sá 2020-04-13 739
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
2024-04-26 13:53 ` [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA Ramona Gradinariu
2024-04-27 2:00 ` kernel test robot
@ 2024-04-27 2:43 ` kernel test robot
1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-04-27 2:43 UTC (permalink / raw)
To: Ramona Gradinariu, linux-kernel, jic23, linux-iio, devicetree,
conor+dt, krzysztof.kozlowski+dt, robh, nuno.sa
Cc: llvm, oe-kbuild-all, Ramona Gradinariu
Hi Ramona,
kernel test robot noticed the following build warnings:
[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.9-rc5 next-20240426]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ramona-Gradinariu/dt-bindings-iio-imu-Add-ADIS16501-compatibles/20240426-215728
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240426135339.185602-4-ramona.bolboaca13%40gmail.com
patch subject: [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA
config: arm-randconfig-004-20240427 (https://download.01.org/0day-ci/archive/20240427/202404271034.fsyc7qe2-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 5ef5eb66fb428aaf61fb51b709f065c069c11242)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404271034.fsyc7qe2-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404271034.fsyc7qe2-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/iio/imu/adis16475.c:16:
In file included from include/linux/iio/imu/adis.h:12:
In file included from include/linux/spi/spi.h:17:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2208:
include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
522 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/iio/imu/adis16475.c:734:34: warning: unused variable 'adis1650x_timeouts' [-Wunused-const-variable]
734 | static const struct adis_timeout adis1650x_timeouts = {
| ^~~~~~~~~~~~~~~~~~
2 warnings generated.
vim +/adis1650x_timeouts +734 drivers/iio/imu/adis16475.c
fff7352bf7a3ce Nuno Sá 2020-04-13 733
fff7352bf7a3ce Nuno Sá 2020-04-13 @734 static const struct adis_timeout adis1650x_timeouts = {
fff7352bf7a3ce Nuno Sá 2020-04-13 735 .reset_ms = 260,
fff7352bf7a3ce Nuno Sá 2020-04-13 736 .sw_reset_ms = 260,
fff7352bf7a3ce Nuno Sá 2020-04-13 737 .self_test_ms = 30,
fff7352bf7a3ce Nuno Sá 2020-04-13 738 };
fff7352bf7a3ce Nuno Sá 2020-04-13 739
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
` (2 preceding siblings ...)
2024-04-26 13:53 ` [PATCH 3/7] iio: imu: adis16475: Re-define ADIS16475_DATA Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-27 3:05 ` kernel test robot
2024-04-26 13:53 ` [PATCH 5/7] iio: imu: adis16475: Create push single sample API Ramona Gradinariu
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Add new API called devm_adis_setup_buffer_and_trigger_with_attrs() which
also takes buffer attributes as a parameter.
Rewrite devm_adis_setup_buffer_and_trigger() implementation such that it
calls devm_adis_setup_buffer_and_trigger_with_attrs() with buffer
attributes parameter NULL
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
drivers/iio/imu/adis_buffer.c | 32 ++++++++++++++++++--------------
include/linux/iio/imu/adis.h | 18 ++++++++++++++----
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c
index 928933027ae3..b6c0d8690070 100644
--- a/drivers/iio/imu/adis_buffer.c
+++ b/drivers/iio/imu/adis_buffer.c
@@ -175,31 +175,36 @@ static void adis_buffer_cleanup(void *arg)
}
/**
- * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for
- * the managed adis device
+ * devm_adis_setup_buffer_and_trigger_with_attrs() - Sets up buffer and trigger
+ * for the managed adis device with buffer attributes.
* @adis: The adis device
* @indio_dev: The IIO device
- * @trigger_handler: Optional trigger handler, may be NULL.
+ * @trigger_handler: Trigger handler: should handle the buffer readings.
+ * @ops: Optional buffer setup functions, may be NULL.
+ * @buffer_attrs: Extra buffer attributes.
*
* Returns 0 on success, a negative error code otherwise.
*
- * This function sets up the buffer and trigger for a adis devices. If
- * 'trigger_handler' is NULL the default trigger handler will be used. The
- * default trigger handler will simply read the registers assigned to the
- * currently active channels.
+ * This function sets up the buffer (with buffer setup functions and extra
+ * buffer attributes) and trigger for a adis devices with buffer attributes.
*/
int
-devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
- irq_handler_t trigger_handler)
+devm_adis_setup_buffer_and_trigger_with_attrs(struct adis *adis, struct iio_dev *indio_dev,
+ irq_handler_t trigger_handler,
+ const struct iio_buffer_setup_ops *ops,
+ const struct attribute **buffer_attrs)
{
int ret;
if (!trigger_handler)
trigger_handler = adis_trigger_handler;
- ret = devm_iio_triggered_buffer_setup(&adis->spi->dev, indio_dev,
- &iio_pollfunc_store_time,
- trigger_handler, NULL);
+ ret = devm_iio_triggered_buffer_setup_ext(&adis->spi->dev, indio_dev,
+ &iio_pollfunc_store_time,
+ trigger_handler,
+ IIO_BUFFER_DIRECTION_IN,
+ ops,
+ buffer_attrs);
if (ret)
return ret;
@@ -212,5 +217,4 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
return devm_add_action_or_reset(&adis->spi->dev, adis_buffer_cleanup,
adis);
}
-EXPORT_SYMBOL_NS_GPL(devm_adis_setup_buffer_and_trigger, IIO_ADISLIB);
-
+EXPORT_SYMBOL_NS_GPL(devm_adis_setup_buffer_and_trigger_with_attrs, IIO_ADISLIB);
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 8898966bc0f0..20978374527f 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -515,11 +515,19 @@ int adis_single_conversion(struct iio_dev *indio_dev,
#define ADIS_ROT_CHAN(mod, addr, si, info_sep, info_all, bits) \
ADIS_MOD_CHAN(IIO_ROT, mod, addr, si, info_sep, info_all, bits)
+#define devm_adis_setup_buffer_and_trigger(adis, indio_dev, trigger_handler) \
+ devm_adis_setup_buffer_and_trigger_with_attrs((adis), (indio_dev), \
+ (trigger_handler), NULL, \
+ NULL)
+
#ifdef CONFIG_IIO_ADIS_LIB_BUFFER
int
-devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
- irq_handler_t trigger_handler);
+devm_adis_setup_buffer_and_trigger_with_attrs(struct adis *adis,
+ struct iio_dev *indio_dev,
+ irq_handler_t trigger_handler,
+ const struct iio_buffer_setup_ops *ops,
+ const struct attribute **buffer_attrs);
int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev);
@@ -529,8 +537,10 @@ int adis_update_scan_mode(struct iio_dev *indio_dev,
#else /* CONFIG_IIO_BUFFER */
static inline int
-devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev,
- irq_handler_t trigger_handler)
+devm_adis_setup_buffer_and_trigger_with_attrs(struct adis *adis, struct iio_dev *indio_dev,
+ irq_handler_t trigger_handler,
+ const struct iio_buffer_setup_ops *ops,
+ const struct attribute **buffer_attrs)
{
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes
2024-04-26 13:53 ` [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes Ramona Gradinariu
@ 2024-04-27 3:05 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-04-27 3:05 UTC (permalink / raw)
To: Ramona Gradinariu, linux-kernel, jic23, linux-iio, devicetree,
conor+dt, krzysztof.kozlowski+dt, robh, nuno.sa
Cc: oe-kbuild-all, Ramona Gradinariu
Hi Ramona,
kernel test robot noticed the following build errors:
[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v6.9-rc5 next-20240426]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ramona-Gradinariu/dt-bindings-iio-imu-Add-ADIS16501-compatibles/20240426-215728
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link: https://lore.kernel.org/r/20240426135339.185602-5-ramona.bolboaca13%40gmail.com
patch subject: [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240427/202404271027.ySHRhuOH-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240427/202404271027.ySHRhuOH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404271027.ySHRhuOH-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/iio/imu/adis_buffer.c: In function 'devm_adis_setup_buffer_and_trigger_with_attrs':
>> drivers/iio/imu/adis_buffer.c:207:51: error: passing argument 7 of 'devm_iio_triggered_buffer_setup_ext' from incompatible pointer type [-Werror=incompatible-pointer-types]
207 | buffer_attrs);
| ^~~~~~~~~~~~
| |
| const struct attribute **
In file included from drivers/iio/imu/adis_buffer.c:19:
include/linux/iio/triggered_buffer.h:31:69: note: expected 'const struct iio_dev_attr **' but argument is of type 'const struct attribute **'
31 | const struct iio_dev_attr **buffer_attrs);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/devm_iio_triggered_buffer_setup_ext +207 drivers/iio/imu/adis_buffer.c
176
177 /**
178 * devm_adis_setup_buffer_and_trigger_with_attrs() - Sets up buffer and trigger
179 * for the managed adis device with buffer attributes.
180 * @adis: The adis device
181 * @indio_dev: The IIO device
182 * @trigger_handler: Trigger handler: should handle the buffer readings.
183 * @ops: Optional buffer setup functions, may be NULL.
184 * @buffer_attrs: Extra buffer attributes.
185 *
186 * Returns 0 on success, a negative error code otherwise.
187 *
188 * This function sets up the buffer (with buffer setup functions and extra
189 * buffer attributes) and trigger for a adis devices with buffer attributes.
190 */
191 int
192 devm_adis_setup_buffer_and_trigger_with_attrs(struct adis *adis, struct iio_dev *indio_dev,
193 irq_handler_t trigger_handler,
194 const struct iio_buffer_setup_ops *ops,
195 const struct attribute **buffer_attrs)
196 {
197 int ret;
198
199 if (!trigger_handler)
200 trigger_handler = adis_trigger_handler;
201
202 ret = devm_iio_triggered_buffer_setup_ext(&adis->spi->dev, indio_dev,
203 &iio_pollfunc_store_time,
204 trigger_handler,
205 IIO_BUFFER_DIRECTION_IN,
206 ops,
> 207 buffer_attrs);
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] iio: imu: adis16475: Create push single sample API
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
` (3 preceding siblings ...)
2024-04-26 13:53 ` [PATCH 4/7] iio: imu: adis_buffer: Add buffer setup API with buffer attributes Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 6/7] dt-bindings: iio: imu: Add ADIS1657X family devices compatibles Ramona Gradinariu
2024-04-26 13:53 ` [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family Ramona Gradinariu
6 siblings, 0 replies; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Create push single sample API reposnsible for pushing a single
sample into the buffer.
This is a preparation patch for FIFO support where more than
one sample has to be pushed in the trigger handler.
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
drivers/iio/imu/adis16475.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 52333772fbe0..7cfef553f298 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -1249,9 +1249,8 @@ static void adis16475_burst32_check(struct adis16475 *st)
}
}
-static irqreturn_t adis16475_trigger_handler(int irq, void *p)
+static int adis16475_push_single_sample(struct iio_poll_func *pf)
{
- struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
struct adis16475 *st = iio_priv(indio_dev);
struct adis *adis = &st->adis;
@@ -1340,6 +1339,15 @@ static irqreturn_t adis16475_trigger_handler(int irq, void *p)
* array.
*/
adis16475_burst32_check(st);
+ return ret;
+}
+
+static irqreturn_t adis16475_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+
+ adis16475_push_single_sample(pf);
iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/7] dt-bindings: iio: imu: Add ADIS1657X family devices compatibles
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
` (4 preceding siblings ...)
2024-04-26 13:53 ` [PATCH 5/7] iio: imu: adis16475: Create push single sample API Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-26 16:38 ` Conor Dooley
2024-04-26 13:53 ` [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family Ramona Gradinariu
6 siblings, 1 reply; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Add ADIS1657X family devices compatibles and specify the according
maximum SPI baudrate.
Similarly to other ADIS1650X devices, ADIS1657X supports sync-mode
values [0,2].
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
.../bindings/iio/imu/adi,adis16475.yaml | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
index db52e7063116..9d185f7bfdcb 100644
--- a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
@@ -37,6 +37,12 @@ properties:
- adi,adis16507-1
- adi,adis16507-2
- adi,adis16507-3
+ - adi,adis16575-2
+ - adi,adis16575-3
+ - adi,adis16576-2
+ - adi,adis16576-3
+ - adi,adis16577-2
+ - adi,adis16577-3
reg:
maxItems: 1
@@ -98,6 +104,12 @@ allOf:
- adi,adis16507-1
- adi,adis16507-2
- adi,adis16507-3
+ - adi,adis16575-2
+ - adi,adis16575-3
+ - adi,adis16576-2
+ - adi,adis16576-3
+ - adi,adis16577-2
+ - adi,adis16577-3
then:
properties:
@@ -114,6 +126,23 @@ allOf:
dependencies:
adi,sync-mode: [ clocks ]
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,adis16575-2
+ - adi,adis16575-3
+ - adi,adis16576-2
+ - adi,adis16576-3
+ - adi,adis16577-2
+ - adi,adis16577-3
+
+ then:
+ properties:
+ spi-max-frequency:
+ maximum: 15000000
+
unevaluatedProperties: false
examples:
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/7] dt-bindings: iio: imu: Add ADIS1657X family devices compatibles
2024-04-26 13:53 ` [PATCH 6/7] dt-bindings: iio: imu: Add ADIS1657X family devices compatibles Ramona Gradinariu
@ 2024-04-26 16:38 ` Conor Dooley
0 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2024-04-26 16:38 UTC (permalink / raw)
To: Ramona Gradinariu
Cc: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]
On Fri, Apr 26, 2024 at 04:53:38PM +0300, Ramona Gradinariu wrote:
> Add ADIS1657X family devices compatibles and specify the according
> maximum SPI baudrate.
> Similarly to other ADIS1650X devices, ADIS1657X supports sync-mode
> values [0,2].
>
> Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
> ---
> .../bindings/iio/imu/adi,adis16475.yaml | 29 +++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> index db52e7063116..9d185f7bfdcb 100644
> --- a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
> @@ -37,6 +37,12 @@ properties:
> - adi,adis16507-1
> - adi,adis16507-2
> - adi,adis16507-3
> + - adi,adis16575-2
> + - adi,adis16575-3
> + - adi,adis16576-2
> + - adi,adis16576-3
> + - adi,adis16577-2
> + - adi,adis16577-3
I'm not checking all of the structs for these ones against eachother,
please explain why fallback compatible are not suitable.
Thanks,
Conor.
>
> reg:
> maxItems: 1
> @@ -98,6 +104,12 @@ allOf:
> - adi,adis16507-1
> - adi,adis16507-2
> - adi,adis16507-3
> + - adi,adis16575-2
> + - adi,adis16575-3
> + - adi,adis16576-2
> + - adi,adis16576-3
> + - adi,adis16577-2
> + - adi,adis16577-3
>
> then:
> properties:
> @@ -114,6 +126,23 @@ allOf:
> dependencies:
> adi,sync-mode: [ clocks ]
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - adi,adis16575-2
> + - adi,adis16575-3
> + - adi,adis16576-2
> + - adi,adis16576-3
> + - adi,adis16577-2
> + - adi,adis16577-3
> +
> + then:
> + properties:
> + spi-max-frequency:
> + maximum: 15000000
> +
> unevaluatedProperties: false
>
> examples:
> --
> 2.34.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family
2024-04-26 13:53 [PATCH 0/7] adis16501 and adis1657x support Ramona Gradinariu
` (5 preceding siblings ...)
2024-04-26 13:53 ` [PATCH 6/7] dt-bindings: iio: imu: Add ADIS1657X family devices compatibles Ramona Gradinariu
@ 2024-04-26 13:53 ` Ramona Gradinariu
2024-04-28 14:45 ` Jonathan Cameron
6 siblings, 1 reply; 17+ messages in thread
From: Ramona Gradinariu @ 2024-04-26 13:53 UTC (permalink / raw)
To: linux-kernel, jic23, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Cc: Ramona Gradinariu
Add support for ADIS1657X family devices in already exiting ADIS16475
driver.
Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
---
drivers/iio/imu/adis16475.c | 536 +++++++++++++++++++++++++++++++++---
1 file changed, 499 insertions(+), 37 deletions(-)
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 7cfef553f298..c759ec456258 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -14,6 +14,7 @@
#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
#include <linux/iio/imu/adis.h>
+#include <linux/iio/sysfs.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/irq.h>
#include <linux/lcm.h>
@@ -52,6 +53,8 @@
FIELD_PREP(ADIS16475_MSG_CTRL_DR_POL_MASK, x)
#define ADIS16475_SYNC_MODE_MASK GENMASK(4, 2)
#define ADIS16475_SYNC_MODE(x) FIELD_PREP(ADIS16475_SYNC_MODE_MASK, x)
+#define ADIS16475_SYNC_4KHZ_MASK BIT(11)
+#define ADIS16475_SYNC_4KHZ(x) FIELD_PREP(ADIS16475_SYNC_4KHZ_MASK, x)
#define ADIS16475_REG_UP_SCALE 0x62
#define ADIS16475_REG_DEC_RATE 0x64
#define ADIS16475_REG_GLOB_CMD 0x68
@@ -66,14 +69,31 @@
#define ADIS16500_BURST32(x) FIELD_PREP(ADIS16500_BURST32_MASK, x)
/* number of data elements in burst mode */
#define ADIS16475_BURST32_MAX_DATA 32
+#define ADIS16575_BURST32_MAX_DATA 34
#define ADIS16475_BURST_MAX_DATA 20
#define ADIS16475_MAX_SCAN_DATA 20
/* spi max speed in brust mode */
#define ADIS16475_BURST_MAX_SPEED 1000000
+#define ADIS16575_BURST_MAX_SPEED 8000000
#define ADIS16475_LSB_DEC_MASK 0
#define ADIS16475_LSB_FIR_MASK 1
#define ADIS16500_BURST_DATA_SEL_0_CHN_MASK GENMASK(5, 0)
#define ADIS16500_BURST_DATA_SEL_1_CHN_MASK GENMASK(12, 7)
+#define ADIS16475_MAX_FIFO_WM 511
+#define ADIS16475_REG_FIFO_CTRL 0x5A
+#define ADIS16475_WM_LVL_MASK GENMASK(15, 4)
+#define ADIS16475_WM_LVL(x) FIELD_PREP(ADIS16475_WM_LVL_MASK, x)
+#define ADIS16475_WM_POL_MASK BIT(3)
+#define ADIS16475_WM_POL(x) FIELD_PREP(ADIS16475_WM_POL_MASK, x)
+#define ADIS16475_WM_EN_MASK BIT(2)
+#define ADIS16475_WM_EN(x) FIELD_PREP(ADIS16475_WM_EN_MASK, x)
+#define ADIS16475_OVERFLOW_MASK BIT(1)
+#define ADIS16475_STOP_ENQUEUE FIELD_PREP(ADIS16475_OVERFLOW_MASK, 0)
+#define ADIS16475_OVERWRITE_OLDEST FIELD_PREP(ADIS16475_OVERFLOW_MASK, 1)
+#define ADIS16475_FIFO_EN_MASK BIT(0)
+#define ADIS16475_FIFO_EN(x) FIELD_PREP(ADIS16475_FIFO_EN_MASK, x)
+#define ADIS16475_FIFO_FLUSH_CMD BIT(5)
+#define ADIS16475_REG_FIFO_CNT 0x3C
enum {
ADIS16475_SYNC_DIRECT = 1,
@@ -95,6 +115,9 @@ struct adis16475_chip_info {
const char *name;
#define ADIS16475_HAS_BURST32 BIT(0)
#define ADIS16475_HAS_BURST_DELTA_DATA BIT(1)
+#define ADIS16475_HAS_TIMESTAMP32 BIT(2)
+#define ADIS16475_NEEDS_BURST_REQUEST BIT(3)
+#define ADIS16475_HAS_FIFO BIT(4)
const long flags;
u32 num_channels;
u32 gyro_max_val;
@@ -116,6 +139,7 @@ struct adis16475 {
bool burst32;
unsigned long lsb_flag;
u16 sync_mode;
+ u16 fifo_watermark;
/* Alignment needed for the timestamp */
__be16 data[ADIS16475_MAX_SCAN_DATA] __aligned(8);
};
@@ -310,6 +334,9 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
u16 dec;
int ret;
u32 sample_rate = st->clk_freq;
+ /* The optimal sample rate for the supported IMUs is between int_clk - 100 and int_clk + 100. */
+ u32 max_sample_rate = st->info->int_clk * 1000 + 100000;
+ u32 min_sample_rate = st->info->int_clk * 1000 - 100000;
if (!freq)
return -EINVAL;
@@ -317,8 +344,9 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
adis_dev_lock(&st->adis);
/*
* When using sync scaled mode, the input clock needs to be scaled so that we have
- * an IMU sample rate between (optimally) 1900 and 2100. After this, we can use the
- * decimation filter to lower the sampling rate in order to get what the user wants.
+ * an IMU sample rate between (optimally) int_clk - 100 and int_clk + 100.
+ * After this, we can use the decimation filter to lower the sampling rate in order
+ * to get what the user wants.
* Optimally, the user sample rate is a multiple of both the IMU sample rate and
* the input clock. Hence, calculating the sync_scale dynamically gives us better
* chances of achieving a perfect/integer value for DEC_RATE. The math here is:
@@ -336,23 +364,25 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
* solution. In this case, we get the highest multiple of the input clock
* lower than the IMU max sample rate.
*/
- if (scaled_rate > 2100000)
- scaled_rate = 2100000 / st->clk_freq * st->clk_freq;
+ if (scaled_rate > max_sample_rate)
+ scaled_rate = max_sample_rate / st->clk_freq * st->clk_freq;
+
else
- scaled_rate = 2100000 / scaled_rate * scaled_rate;
+ scaled_rate = max_sample_rate / scaled_rate * scaled_rate;
/*
* This is not an hard requirement but it's not advised to run the IMU
- * with a sample rate lower than 1900Hz due to possible undersampling
- * issues. However, there are users that might really want to take the risk.
- * Hence, we provide a module parameter for them. If set, we allow sample
- * rates lower than 1.9KHz. By default, we won't allow this and we just roundup
- * the rate to the next multiple of the input clock bigger than 1.9KHz. This
- * is done like this as in some cases (when DEC_RATE is 0) might give
- * us the closest value to the one desired by the user...
+ * with a sample rate lower than internal clock frequency, due to possible
+ * undersampling issues. However, there are users that might really want
+ * to take the risk. Hence, we provide a module parameter for them. If set,
+ * we allow sample rates lower than internal clock frequency.
+ * By default, we won't allow this and we just roundup the rate to the next
+ * multiple of the input clock. This is done like this as in some cases
+ * (when DEC_RATE is 0) might give us the closest value to the one desired
+ * by the user...
*/
- if (scaled_rate < 1900000 && !low_rate_allow)
- scaled_rate = roundup(1900000, st->clk_freq);
+ if (scaled_rate < min_sample_rate && !low_rate_allow)
+ scaled_rate = roundup(min_sample_rate, st->clk_freq);
sync_scale = scaled_rate / st->clk_freq;
ret = __adis_write_reg_16(&st->adis, ADIS16475_REG_UP_SCALE, sync_scale);
@@ -437,6 +467,130 @@ static int adis16475_set_filter(struct adis16475 *st, const u32 filter)
return 0;
}
+static ssize_t adis16475_get_fifo_enabled(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct adis16475 *st = iio_priv(indio_dev);
+ int ret;
+ u16 val;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
+ if (ret)
+ return ret;
+ val = FIELD_GET(ADIS16475_FIFO_EN_MASK, val);
+
+ return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t adis16475_get_fifo_watermark(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct adis16475 *st = iio_priv(indio_dev);
+
+ int ret;
+ u16 val;
+
+ ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
+ if (ret)
+ return ret;
+ val = FIELD_GET(ADIS16475_WM_LVL_MASK, val) + 1;
+
+ return sysfs_emit(buf, "%d\n", val);
+}
+
+static ssize_t hwfifo_watermark_min_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%s\n", "1");
+}
+
+static ssize_t hwfifo_watermark_max_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%s\n", __stringify(ADIS16475_MAX_FIFO_WM));
+}
+
+static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0);
+static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
+static IIO_DEVICE_ATTR(hwfifo_watermark, 0444,
+ adis16475_get_fifo_watermark, NULL, 0);
+static IIO_DEVICE_ATTR(hwfifo_enabled, 0444,
+ adis16475_get_fifo_enabled, NULL, 0);
+
+static const struct attribute *adis16475_fifo_attributes[] = {
+ &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr,
+ &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
+ &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
+ &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
+ NULL,
+};
+
+static int adis16475_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct adis16475 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ u16 val;
+
+ val = ADIS16475_FIFO_EN(1);
+ return adis_update_bits(adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_FIFO_EN_MASK, val);
+}
+
+static int adis16475_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adis16475 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ int ret;
+ u16 val;
+
+ adis_dev_lock(&st->adis);
+
+ val = ADIS16475_FIFO_EN(0);
+ ret = __adis_update_bits(adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_FIFO_EN_MASK, val);
+ if (ret)
+ goto unlock;
+
+ ret = __adis_write_reg_16(adis, ADIS16475_REG_GLOB_CMD,
+ ADIS16475_FIFO_FLUSH_CMD);
+
+unlock:
+ adis_dev_unlock(&st->adis);
+ return ret;
+}
+
+static const struct iio_buffer_setup_ops adis16475_buffer_ops = {
+ .postenable = adis16475_buffer_postenable,
+ .postdisable = adis16475_buffer_postdisable,
+};
+
+static int adis16475_set_watermark(struct iio_dev *indio_dev, unsigned int val)
+{
+ struct adis16475 *st = iio_priv(indio_dev);
+ int ret;
+ u16 wm_lvl;
+
+ adis_dev_lock(&st->adis);
+
+ if (val > ADIS16475_MAX_FIFO_WM)
+ val = ADIS16475_MAX_FIFO_WM;
+
+ wm_lvl = ADIS16475_WM_LVL(val - 1);
+ ret = __adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_WM_LVL_MASK, wm_lvl);
+ if (ret)
+ goto unlock;
+
+ st->fifo_watermark = val;
+
+unlock:
+ adis_dev_unlock(&st->adis);
+ return ret;
+}
+
static const u32 adis16475_calib_regs[] = {
[ADIS16475_SCAN_GYRO_X] = ADIS16475_REG_X_GYRO_BIAS_L,
[ADIS16475_SCAN_GYRO_Y] = ADIS16475_REG_Y_GYRO_BIAS_L,
@@ -668,6 +822,12 @@ enum adis16475_variant {
ADIS16507_1,
ADIS16507_2,
ADIS16507_3,
+ ADIS16575_2,
+ ADIS16575_3,
+ ADIS16576_2,
+ ADIS16576_3,
+ ADIS16577_2,
+ ADIS16577_3,
};
enum {
@@ -725,6 +885,12 @@ static const struct adis16475_sync adis16475_sync_mode[] = {
{ ADIS16475_SYNC_PULSE, 1000, 2100 },
};
+static const struct adis16475_sync adis16575_sync_mode[] = {
+ { ADIS16475_SYNC_OUTPUT },
+ { ADIS16475_SYNC_DIRECT, 1900, 4100 },
+ { ADIS16475_SYNC_SCALED, 1, 400 },
+};
+
static const struct adis_timeout adis16475_timeouts = {
.reset_ms = 200,
.sw_reset_ms = 200,
@@ -1156,6 +1322,150 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
ADIS16475_BURST32_MAX_DATA,
ADIS16475_BURST_MAX_SPEED),
},
+ [ADIS16575_2] = {
+ .name = "adis16575-2",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
+ .accel_max_val = 8,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(450),
+ .deltvel_max_val = 100,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16575, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
+ [ADIS16575_3] = {
+ .name = "adis16575-3",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
+ .accel_max_val = 8,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
+ .deltvel_max_val = 100,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16575, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
+ [ADIS16576_2] = {
+ .name = "adis16576-2",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
+ .accel_max_val = 40,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(450),
+ .deltvel_max_val = 125,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16576, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
+ [ADIS16576_3] = {
+ .name = "adis16576-3",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
+ .accel_max_val = 40,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
+ .deltvel_max_val = 125,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16576, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
+ [ADIS16577_2] = {
+ .name = "adis16577-2",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
+ .accel_max_val = 40,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(450),
+ .deltvel_max_val = 400,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16577, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
+ [ADIS16577_3] = {
+ .name = "adis16577-3",
+ .num_channels = ARRAY_SIZE(adis16477_channels),
+ .channels = adis16477_channels,
+ .gyro_max_val = 1,
+ .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
+ .accel_max_val = 40,
+ .accel_max_scale = IIO_M_S_2_TO_G(32000 << 16),
+ .temp_scale = 100,
+ .deltang_max_val = IIO_DEGREE_TO_RAD(2000),
+ .deltvel_max_val = 400,
+ .int_clk = 4000,
+ .max_dec = 3999,
+ .sync = adis16575_sync_mode,
+ .num_sync = ARRAY_SIZE(adis16575_sync_mode),
+ .flags = ADIS16475_HAS_BURST32 |
+ ADIS16475_HAS_BURST_DELTA_DATA |
+ ADIS16475_NEEDS_BURST_REQUEST |
+ ADIS16475_HAS_TIMESTAMP32 |
+ ADIS16475_HAS_FIFO,
+ .adis_data = ADIS16475_DATA(16577, &adis16475_timeouts,
+ ADIS16575_BURST32_MAX_DATA,
+ ADIS16575_BURST_MAX_SPEED),
+ },
};
static int adis16475_update_scan_mode(struct iio_dev *indio_dev,
@@ -1188,17 +1498,14 @@ static const struct iio_info adis16475_info = {
.write_raw = &adis16475_write_raw,
.update_scan_mode = adis16475_update_scan_mode,
.debugfs_reg_access = adis_debugfs_reg_access,
+ .hwfifo_set_watermark = adis16475_set_watermark,
};
static bool adis16475_validate_crc(const u8 *buffer, u16 crc,
- const bool burst32)
+ u16 burst_size, u16 start_idx)
{
int i;
- /* extra 6 elements for low gyro and accel */
- const u16 sz = burst32 ? ADIS16475_BURST32_MAX_DATA :
- ADIS16475_BURST_MAX_DATA;
-
- for (i = 0; i < sz - 2; i++)
+ for (i = start_idx; i < burst_size - 2; i++)
crc -= buffer[i];
return crc == 0;
@@ -1226,8 +1533,11 @@ static void adis16475_burst32_check(struct adis16475 *st)
* In 32-bit mode we need extra 2 bytes for all gyro
* and accel channels.
*/
- adis->burst_extra_len = 6 * sizeof(u16);
- adis->xfer[1].len += 6 * sizeof(u16);
+ adis->burst_extra_len = (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
+ sizeof(u16);
+ adis->xfer[1].len += (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
+ sizeof(u16);
+
dev_dbg(&adis->spi->dev, "Enable burst32 mode, xfer:%d",
adis->xfer[1].len);
@@ -1243,7 +1553,8 @@ static void adis16475_burst32_check(struct adis16475 *st)
/* Remove the extra bits */
adis->burst_extra_len = 0;
- adis->xfer[1].len -= 6 * sizeof(u16);
+ adis->xfer[1].len -= (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
+ sizeof(u16);
dev_dbg(&adis->spi->dev, "Disable burst32 mode, xfer:%d\n",
adis->xfer[1].len);
}
@@ -1258,20 +1569,31 @@ static int adis16475_push_single_sample(struct iio_poll_func *pf)
__be16 *buffer;
u16 crc;
bool valid;
+ u8 crc_offset = 9;
+ u16 burst_size = ADIS16475_BURST_MAX_DATA;
+ u16 start_idx = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ? 2 : 0;
+
/* offset until the first element after gyro and accel */
const u8 offset = st->burst32 ? 13 : 7;
+ if (st->burst32) {
+ crc_offset = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ? 16 : 15;
+ burst_size = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ?
+ ADIS16575_BURST32_MAX_DATA : ADIS16475_BURST32_MAX_DATA;
+ }
+
ret = spi_sync(adis->spi, &adis->msg);
if (ret)
- goto check_burst32;
+ return ret;
buffer = adis->buffer;
- crc = be16_to_cpu(buffer[offset + 2]);
- valid = adis16475_validate_crc(adis->buffer, crc, st->burst32);
+ crc = be16_to_cpu(buffer[crc_offset]);
+ valid = adis16475_validate_crc(adis->buffer, crc, burst_size, start_idx);
+
if (!valid) {
dev_err(&adis->spi->dev, "Invalid crc\n");
- goto check_burst32;
+ return ret;
}
for_each_set_bit(bit, indio_dev->active_scan_mask,
@@ -1332,22 +1654,111 @@ static int adis16475_push_single_sample(struct iio_poll_func *pf)
}
iio_push_to_buffers_with_timestamp(indio_dev, st->data, pf->timestamp);
-check_burst32:
+
+ return 0;
+}
+
+static irqreturn_t adis16475_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adis16475 *st = iio_priv(indio_dev);
+
+ adis16475_push_single_sample(pf);
/*
* We only check the burst mode at the end of the current capture since
* it takes a full data ready cycle for the device to update the burst
* array.
*/
adis16475_burst32_check(st);
- return ret;
+
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
}
-static irqreturn_t adis16475_trigger_handler(int irq, void *p)
+static int adis16575_custom_burst_read(struct iio_poll_func *pf, u8 burst_req)
+{
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adis16475 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ unsigned int burst_length, burst_max_length;
+ u8 *tx;
+
+ burst_length = adis->data->burst_len + adis->burst_extra_len;
+
+ if (adis->data->burst_max_len)
+ burst_max_length = adis->data->burst_max_len;
+ else
+ burst_max_length = burst_length;
+
+ tx = adis->buffer + burst_max_length;
+ tx[0] = ADIS_READ_REG(burst_req);
+
+ if (burst_req)
+ return spi_sync(adis->spi, &adis->msg);
+
+ return adis16475_push_single_sample(pf);
+}
+
+/*
+ * This handler is meant to be used for devices which support burst readings
+ * from FIFO (namely devices from adis1657x family).
+ * In order to pop the FIFO the 0x68 0x00 FIFO pop burst request has to be sent.
+ * If the previous device command was not a FIFO pop burst request, the FIFO pop
+ * burst request will simply pop the FIFO without returning valid data.
+ * For the nth consecutive burst request, the
+ * device will send the data popped with the (n-1)th consecutive burst request.
+ * In order to read the data which was popped previously, without popping the FIFO,
+ * the 0x00 0x00 burst request has to be sent.
+ * If after a 0x68 0x00 FIFO pop burst request, there is any other device access
+ * different from a 0x68 0x00 or a 0x00 0x00 burst request, the FIFO data popped
+ * previously will be lost.
+ */
+static irqreturn_t adis16475_trigger_handler_with_fifo(int irq, void *p)
{
struct iio_poll_func *pf = p;
struct iio_dev *indio_dev = pf->indio_dev;
+ struct adis16475 *st = iio_priv(indio_dev);
+ struct adis *adis = &st->adis;
+ int ret;
+ u16 fifo_cnt, i;
- adis16475_push_single_sample(pf);
+ adis_dev_lock(&st->adis);
+
+ ret = __adis_read_reg_16(adis, ADIS16475_REG_FIFO_CNT, &fifo_cnt);
+ if (ret || fifo_cnt < 2)
+ goto unlock;
+
+ if (fifo_cnt > st->fifo_watermark)
+ fifo_cnt = st->fifo_watermark;
+
+ /*
+ * First burst request - FIFO pop: popped data will be returned in the
+ * next burst request.
+ */
+ ret = adis16575_custom_burst_read(pf, adis->data->burst_reg_cmd);
+ if (ret)
+ goto unlock;
+
+ for (i = 0; i < fifo_cnt - 1; i++) {
+ ret = adis16475_push_single_sample(pf);
+ if (ret)
+ goto unlock;
+ }
+
+ /* FIFO read without popping */
+ ret = adis16575_custom_burst_read(pf, 0);
+ if (ret)
+ goto unlock;
+
+unlock:
+ /*
+ * We only check the burst mode at the end of the current capture since
+ * reading data from registers will impact the FIFO reading.
+ */
+ adis16475_burst32_check(st);
+ adis_dev_unlock(&st->adis);
iio_trigger_notify_done(indio_dev->trig);
return IRQ_HANDLED;
@@ -1359,6 +1770,17 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
struct device *dev = &st->adis.spi->dev;
const struct adis16475_sync *sync;
u32 sync_mode;
+ u16 max_sample_rate = st->info->int_clk + 100;
+ u16 val;
+
+ /* if available, enable 4khz internal clock */
+ if (st->info->int_clk == 4000) {
+ val = ADIS16475_SYNC_4KHZ(1);
+ ret = __adis_update_bits(&st->adis, ADIS16475_REG_MSG_CTRL,
+ ADIS16475_SYNC_4KHZ_MASK, val);
+ if (ret)
+ return ret;
+ }
/* default to internal clk */
st->clk_freq = st->info->int_clk * 1000;
@@ -1398,10 +1820,9 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
/*
* In sync scaled mode, the IMU sample rate is the clk_freq * sync_scale.
* Hence, default the IMU sample rate to the highest multiple of the input
- * clock lower than the IMU max sample rate. The optimal range is
- * 1900-2100 sps...
+ * clock lower than the IMU max sample rate.
*/
- up_scale = 2100 / st->clk_freq;
+ up_scale = max_sample_rate / st->clk_freq;
ret = __adis_write_reg_16(&st->adis,
ADIS16475_REG_UP_SCALE,
@@ -1467,7 +1888,23 @@ static int adis16475_config_irq_pin(struct adis16475 *st)
*/
usleep_range(250, 260);
- return 0;
+ /*
+ * If the device has FIFO support, configure the watermark polarity
+ * pin as well.
+ */
+ if (st->info->flags & ADIS16475_HAS_FIFO) {
+ val = ADIS16475_WM_POL(polarity);
+ ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL,
+ ADIS16475_WM_POL_MASK, val);
+ if (ret)
+ return ret;
+
+ /* Enable watermark interrupt pin. */
+ val = ADIS16475_WM_EN(1);
+ ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_WM_EN_MASK, val);
+ }
+
+ return ret;
}
@@ -1509,8 +1946,21 @@ static int adis16475_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
- adis16475_trigger_handler);
+ if (st->info->flags & ADIS16475_HAS_FIFO) {
+ ret = devm_adis_setup_buffer_and_trigger_with_attrs(&st->adis, indio_dev,
+ adis16475_trigger_handler_with_fifo,
+ &adis16475_buffer_ops,
+ adis16475_fifo_attributes);
+ if (ret)
+ return ret;
+ /* Update overflow behavior to always overwrite the oldest sample. */
+ u16 val = ADIS16475_OVERWRITE_OLDEST;
+
+ ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_OVERFLOW_MASK, val);
+ } else {
+ ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
+ adis16475_trigger_handler);
+ }
if (ret)
return ret;
@@ -1566,6 +2016,18 @@ static const struct of_device_id adis16475_of_match[] = {
.data = &adis16475_chip_info[ADIS16507_2] },
{ .compatible = "adi,adis16507-3",
.data = &adis16475_chip_info[ADIS16507_3] },
+ { .compatible = "adi,adis16575-2",
+ .data = &adis16475_chip_info[ADIS16575_2] },
+ { .compatible = "adi,adis16575-3",
+ .data = &adis16475_chip_info[ADIS16575_3] },
+ { .compatible = "adi,adis16576-2",
+ .data = &adis16475_chip_info[ADIS16576_2] },
+ { .compatible = "adi,adis16576-3",
+ .data = &adis16475_chip_info[ADIS16576_3] },
+ { .compatible = "adi,adis16577-2",
+ .data = &adis16475_chip_info[ADIS16577_2] },
+ { .compatible = "adi,adis16577-3",
+ .data = &adis16475_chip_info[ADIS16577_3] },
{ },
};
MODULE_DEVICE_TABLE(of, adis16475_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family
2024-04-26 13:53 ` [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family Ramona Gradinariu
@ 2024-04-28 14:45 ` Jonathan Cameron
2024-05-08 14:32 ` Ramona Gradinariu
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2024-04-28 14:45 UTC (permalink / raw)
To: Ramona Gradinariu
Cc: linux-kernel, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
On Fri, 26 Apr 2024 16:53:39 +0300
Ramona Gradinariu <ramona.bolboaca13@gmail.com> wrote:
> Add support for ADIS1657X family devices in already exiting ADIS16475
> driver.
>
> Signed-off-by: Ramona Gradinariu <ramona.bolboaca13@gmail.com>
Hi Ramona,
Various comments inline,
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16475.c | 536 +++++++++++++++++++++++++++++++++---
> 1 file changed, 499 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> index 7cfef553f298..c759ec456258 100644
> --- a/drivers/iio/imu/adis16475.c
> +++ b/drivers/iio/imu/adis16475.c
> @@ -14,6 +14,7 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/imu/adis.h>
> +#include <linux/iio/sysfs.h>
> #include <linux/iio/trigger_consumer.h>
> #include <linux/irq.h>
> #include <linux/lcm.h>
> @@ -52,6 +53,8 @@
> FIELD_PREP(ADIS16475_MSG_CTRL_DR_POL_MASK, x)
> #define ADIS16475_SYNC_MODE_MASK GENMASK(4, 2)
> #define ADIS16475_SYNC_MODE(x) FIELD_PREP(ADIS16475_SYNC_MODE_MASK, x)
> +#define ADIS16475_SYNC_4KHZ_MASK BIT(11)
> +#define ADIS16475_SYNC_4KHZ(x) FIELD_PREP(ADIS16475_SYNC_4KHZ_MASK, x)
When a field doesn't exist for the original device, it's fine to name it after the
first part where it does.
#define ADIS16575_SYNC_4KHZ() etc.
> #define ADIS16475_REG_UP_SCALE 0x62
> #define ADIS16475_REG_DEC_RATE 0x64
> #define ADIS16475_REG_GLOB_CMD 0x68
> @@ -66,14 +69,31 @@
> #define ADIS16500_BURST32(x) FIELD_PREP(ADIS16500_BURST32_MASK, x)
> /* number of data elements in burst mode */
> #define ADIS16475_BURST32_MAX_DATA 32
> +#define ADIS16575_BURST32_MAX_DATA 34
As below, I wonder if this is better referred to as with and without the timestamp
rather than by part number?
> #define ADIS16475_BURST_MAX_DATA 20
> #define ADIS16475_MAX_SCAN_DATA 20
> /* spi max speed in brust mode */
> #define ADIS16475_BURST_MAX_SPEED 1000000
> +#define ADIS16575_BURST_MAX_SPEED 8000000
> #define ADIS16475_LSB_DEC_MASK 0
> #define ADIS16475_LSB_FIR_MASK 1
> #define ADIS16500_BURST_DATA_SEL_0_CHN_MASK GENMASK(5, 0)
> #define ADIS16500_BURST_DATA_SEL_1_CHN_MASK GENMASK(12, 7)
> +#define ADIS16475_MAX_FIFO_WM 511
> +#define ADIS16475_REG_FIFO_CTRL 0x5A
Name these after first part to be added to the driver that supports this register.
> +#define ADIS16475_WM_LVL_MASK GENMASK(15, 4)
> +#define ADIS16475_WM_LVL(x) FIELD_PREP(ADIS16475_WM_LVL_MASK, x)
> +#define ADIS16475_WM_POL_MASK BIT(3)
> +#define ADIS16475_WM_POL(x) FIELD_PREP(ADIS16475_WM_POL_MASK, x)
> +#define ADIS16475_WM_EN_MASK BIT(2)
> +#define ADIS16475_WM_EN(x) FIELD_PREP(ADIS16475_WM_EN_MASK, x)
> +#define ADIS16475_OVERFLOW_MASK BIT(1)
> +#define ADIS16475_STOP_ENQUEUE FIELD_PREP(ADIS16475_OVERFLOW_MASK, 0)
> +#define ADIS16475_OVERWRITE_OLDEST FIELD_PREP(ADIS16475_OVERFLOW_MASK, 1)
> +#define ADIS16475_FIFO_EN_MASK BIT(0)
> +#define ADIS16475_FIFO_EN(x) FIELD_PREP(ADIS16475_FIFO_EN_MASK, x)
> +#define ADIS16475_FIFO_FLUSH_CMD BIT(5)
> +#define ADIS16475_REG_FIFO_CNT 0x3C
...
> @@ -437,6 +467,130 @@ static int adis16475_set_filter(struct adis16475 *st, const u32 filter)
> return 0;
> }
>
> +static ssize_t adis16475_get_fifo_enabled(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct adis16475 *st = iio_priv(indio_dev);
> + int ret;
> + u16 val;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
> + if (ret)
> + return ret;
> + val = FIELD_GET(ADIS16475_FIFO_EN_MASK, val);
> +
> + return sysfs_emit(buf, "%d\n", val);
As below, might as well put the FIELD_GET() in the sysfs_emit rather than
writing the local parameter.
> +}
> +
> +static ssize_t adis16475_get_fifo_watermark(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct adis16475 *st = iio_priv(indio_dev);
> +
Unusual to have a line break here. I'd drop it.
> + int ret;
> + u16 val;
> +
> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
> + if (ret)
> + return ret;
> + val = FIELD_GET(ADIS16475_WM_LVL_MASK, val) + 1;
Might as well put this maths inline in the sysfs_emit()
> +
> + return sysfs_emit(buf, "%d\n", val);
> +}
> +
> +static ssize_t hwfifo_watermark_min_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%s\n", "1");
> +}
> +
> +static ssize_t hwfifo_watermark_max_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "%s\n", __stringify(ADIS16475_MAX_FIFO_WM));
> +}
> +
> +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_min, 0);
> +static IIO_DEVICE_ATTR_RO(hwfifo_watermark_max, 0);
> +static IIO_DEVICE_ATTR(hwfifo_watermark, 0444,
> + adis16475_get_fifo_watermark, NULL, 0);
> +static IIO_DEVICE_ATTR(hwfifo_enabled, 0444,
> + adis16475_get_fifo_enabled, NULL, 0);
> +
> +static const struct attribute *adis16475_fifo_attributes[] = {
> + &iio_dev_attr_hwfifo_watermark_min.dev_attr.attr,
> + &iio_dev_attr_hwfifo_watermark_max.dev_attr.attr,
> + &iio_dev_attr_hwfifo_watermark.dev_attr.attr,
> + &iio_dev_attr_hwfifo_enabled.dev_attr.attr,
> + NULL,
No comma needed on 'null terminators' as we must never add anything after them.
> +};
> +
> +static int adis16475_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct adis16475 *st = iio_priv(indio_dev);
> + struct adis *adis = &st->adis;
> + u16 val;
> +
> + val = ADIS16475_FIFO_EN(1);
> + return adis_update_bits(adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_FIFO_EN_MASK, val);
Put the value in val inline, that local variable isn't helping readability.
> +}
> +
> +static int adis16475_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct adis16475 *st = iio_priv(indio_dev);
> + struct adis *adis = &st->adis;
> + int ret;
> + u16 val;
> +
> + adis_dev_lock(&st->adis);
> +
> + val = ADIS16475_FIFO_EN(0);
> + ret = __adis_update_bits(adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_FIFO_EN_MASK, val);
As above, val isn't adding much.
> + if (ret)
> + goto unlock;
> +
> + ret = __adis_write_reg_16(adis, ADIS16475_REG_GLOB_CMD,
> + ADIS16475_FIFO_FLUSH_CMD);
> +
> +unlock:
> + adis_dev_unlock(&st->adis);
> + return ret;
> +}
> +
> +static const struct iio_buffer_setup_ops adis16475_buffer_ops = {
> + .postenable = adis16475_buffer_postenable,
> + .postdisable = adis16475_buffer_postdisable,
> +};
> +
> +static int adis16475_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> + struct adis16475 *st = iio_priv(indio_dev);
> + int ret;
> + u16 wm_lvl;
> +
> + adis_dev_lock(&st->adis);
> +
> + if (val > ADIS16475_MAX_FIFO_WM)
> + val = ADIS16475_MAX_FIFO_WM;
val = min(val, ADIS16475_MAX_FIFO_WM);
> +
> + wm_lvl = ADIS16475_WM_LVL(val - 1);
> + ret = __adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_WM_LVL_MASK, wm_lvl);
> + if (ret)
> + goto unlock;
> +
> + st->fifo_watermark = val;
> +
> +unlock:
> + adis_dev_unlock(&st->adis);
> + return ret;
> +}
> +
>
> static int adis16475_update_scan_mode(struct iio_dev *indio_dev,
> @@ -1188,17 +1498,14 @@ static const struct iio_info adis16475_info = {
> .write_raw = &adis16475_write_raw,
> .update_scan_mode = adis16475_update_scan_mode,
> .debugfs_reg_access = adis_debugfs_reg_access,
> + .hwfifo_set_watermark = adis16475_set_watermark,
Odd to have this for devices that don't have a watermark. I would split
this into separate info structures to allow with fifo and without fifo.
It may make no difference in practice, but it will maintain clean separation
of the fifo support from non fifo devices.
> };
>
> static bool adis16475_validate_crc(const u8 *buffer, u16 crc,
> - const bool burst32)
> + u16 burst_size, u16 start_idx)
> {
> int i;
> - /* extra 6 elements for low gyro and accel */
> - const u16 sz = burst32 ? ADIS16475_BURST32_MAX_DATA :
> - ADIS16475_BURST_MAX_DATA;
> -
> - for (i = 0; i < sz - 2; i++)
> + for (i = start_idx; i < burst_size - 2; i++)
> crc -= buffer[i];
>
> return crc == 0;
> @@ -1226,8 +1533,11 @@ static void adis16475_burst32_check(struct adis16475 *st)
> * In 32-bit mode we need extra 2 bytes for all gyro
> * and accel channels.
> */
> - adis->burst_extra_len = 6 * sizeof(u16);
> - adis->xfer[1].len += 6 * sizeof(u16);
> + adis->burst_extra_len = (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
> + sizeof(u16);
> + adis->xfer[1].len += (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
> + sizeof(u16);
> +
As below, the !! is hard to read so better to have an if / else block here.
However, that second value is += adis->burst_extra_len so use that to avoid duplication.
Or set a local variable and use that to set both up.
> dev_dbg(&adis->spi->dev, "Enable burst32 mode, xfer:%d",
> adis->xfer[1].len);
>
> @@ -1243,7 +1553,8 @@ static void adis16475_burst32_check(struct adis16475 *st)
>
> /* Remove the extra bits */
> adis->burst_extra_len = 0;
> - adis->xfer[1].len -= 6 * sizeof(u16);
> + adis->xfer[1].len -= (6 + !!(st->info->flags & ADIS16475_HAS_TIMESTAMP32)) *
> + sizeof(u16);
I'd write this one as an if / else as the !! is hard to read.
> dev_dbg(&adis->spi->dev, "Disable burst32 mode, xfer:%d\n",
> adis->xfer[1].len);
> }
> @@ -1258,20 +1569,31 @@ static int adis16475_push_single_sample(struct iio_poll_func *pf)
> __be16 *buffer;
> u16 crc;
> bool valid;
> + u8 crc_offset = 9;
> + u16 burst_size = ADIS16475_BURST_MAX_DATA;
> + u16 start_idx = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ? 2 : 0;
> +
> /* offset until the first element after gyro and accel */
> const u8 offset = st->burst32 ? 13 : 7;
>
> + if (st->burst32) {
> + crc_offset = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ? 16 : 15;
> + burst_size = (st->info->flags & ADIS16475_HAS_TIMESTAMP32) ?
> + ADIS16575_BURST32_MAX_DATA : ADIS16475_BURST32_MAX_DATA;
I'd rename these to reflect the flag.
ADIS16475_BURST32_MAX_DATA_NO_TS32 vs ADIS16475_BURST32_DATA_TS32
or something like that. Picking two random part numbers won't scale long term.
> + }
> +
> ret = spi_sync(adis->spi, &adis->msg);
> if (ret)
> - goto check_burst32;
> + return ret;
>
> buffer = adis->buffer;
>
> - crc = be16_to_cpu(buffer[offset + 2]);
> - valid = adis16475_validate_crc(adis->buffer, crc, st->burst32);
> + crc = be16_to_cpu(buffer[crc_offset]);
> + valid = adis16475_validate_crc(adis->buffer, crc, burst_size, start_idx);
> +
No blank line here - !valid check is closely coupled to the block above so reflect
that by not having a blank line before it.
> if (!valid) {
> dev_err(&adis->spi->dev, "Invalid crc\n");
> - goto check_burst32;
> + return ret;
> }
>
> for_each_set_bit(bit, indio_dev->active_scan_mask,
> @@ -1332,22 +1654,111 @@ static int adis16475_push_single_sample(struct iio_poll_func *pf)
> }
>
> iio_push_to_buffers_with_timestamp(indio_dev, st->data, pf->timestamp);
> -check_burst32:
> +
> + return 0;
> +}
> +
> +static irqreturn_t adis16475_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adis16475 *st = iio_priv(indio_dev);
> +
> + adis16475_push_single_sample(pf);
> /*
> * We only check the burst mode at the end of the current capture since
> * it takes a full data ready cycle for the device to update the burst
> * array.
> */
> adis16475_burst32_check(st);
> - return ret;
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> }
>
> -static irqreturn_t adis16475_trigger_handler(int irq, void *p)
> +static int adis16575_custom_burst_read(struct iio_poll_func *pf, u8 burst_req)
> +{
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adis16475 *st = iio_priv(indio_dev);
> + struct adis *adis = &st->adis;
> + unsigned int burst_length, burst_max_length;
> + u8 *tx;
> +
> + burst_length = adis->data->burst_len + adis->burst_extra_len;
> +
Could be simplified as this is only used if !burst_max_len.
if (adis->data->burst_max_len)
burst_max_length = adis->data->burst_max_lent;
else
burst_max_length = adis->data->burst_len + adis->burst_extra_len;
> + if (adis->data->burst_max_len)
> + burst_max_length = adis->data->burst_max_len;
> + else
> + burst_max_length = burst_length;
> +
> + tx = adis->buffer + burst_max_length;
> + tx[0] = ADIS_READ_REG(burst_req);
> +
> + if (burst_req)
If !burst_req does the rest of this do anything at all?
If so flip the logic as
if (!burst_req)
return adis16475_push_single_sample(pf);
the rest...
return spi_sync(adis->spi, &adis->msg);
> + return spi_sync(adis->spi, &adis->msg);
> +
> + return adis16475_push_single_sample(pf);
> +}
> +
> +/*
> + * This handler is meant to be used for devices which support burst readings
> + * from FIFO (namely devices from adis1657x family).
> + * In order to pop the FIFO the 0x68 0x00 FIFO pop burst request has to be sent.
> + * If the previous device command was not a FIFO pop burst request, the FIFO pop
> + * burst request will simply pop the FIFO without returning valid data.
> + * For the nth consecutive burst request, the
> + * device will send the data popped with the (n-1)th consecutive burst request.
> + * In order to read the data which was popped previously, without popping the FIFO,
> + * the 0x00 0x00 burst request has to be sent.
> + * If after a 0x68 0x00 FIFO pop burst request, there is any other device access
> + * different from a 0x68 0x00 or a 0x00 0x00 burst request, the FIFO data popped
> + * previously will be lost.
> + */
> +static irqreturn_t adis16475_trigger_handler_with_fifo(int irq, void *p)
> {
> struct iio_poll_func *pf = p;
> struct iio_dev *indio_dev = pf->indio_dev;
> + struct adis16475 *st = iio_priv(indio_dev);
> + struct adis *adis = &st->adis;
> + int ret;
> + u16 fifo_cnt, i;
>
> - adis16475_push_single_sample(pf);
> + adis_dev_lock(&st->adis);
> +
> + ret = __adis_read_reg_16(adis, ADIS16475_REG_FIFO_CNT, &fifo_cnt);
> + if (ret || fifo_cnt < 2)
> + goto unlock;
I would break these conditions and add a comment on why fifo_cnt < 2 is
a reason to just return 0;
> +
> + if (fifo_cnt > st->fifo_watermark)
> + fifo_cnt = st->fifo_watermark;
fifo_cnt = min(fifo_cnt, st->fifo_watermark);
This confuses me though as normally overreading after a fifo watermark is
both safe and the right thing to do (as reduces chance of overflow etc).
If we need to clamp to the watermark for some reason, add a comment.
> +
> + /*
> + * First burst request - FIFO pop: popped data will be returned in the
> + * next burst request.
> + */
> + ret = adis16575_custom_burst_read(pf, adis->data->burst_reg_cmd);
> + if (ret)
> + goto unlock;
> +
> + for (i = 0; i < fifo_cnt - 1; i++) {
> + ret = adis16475_push_single_sample(pf);
> + if (ret)
> + goto unlock;
> + }
> +
> + /* FIFO read without popping */
> + ret = adis16575_custom_burst_read(pf, 0);
> + if (ret)
> + goto unlock;
> +
> +unlock:
> + /*
> + * We only check the burst mode at the end of the current capture since
> + * reading data from registers will impact the FIFO reading.
> + */
> + adis16475_burst32_check(st);
> + adis_dev_unlock(&st->adis);
> iio_trigger_notify_done(indio_dev->trig);
>
> return IRQ_HANDLED;
> @@ -1359,6 +1770,17 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
> struct device *dev = &st->adis.spi->dev;
> const struct adis16475_sync *sync;
> u32 sync_mode;
> + u16 max_sample_rate = st->info->int_clk + 100;
This changes seems unconnected to the rest. Pull it out as a precursor patch where
you can explain why this is the right value etc.
> + u16 val;
> +
> + /* if available, enable 4khz internal clock */
> + if (st->info->int_clk == 4000) {
> + val = ADIS16475_SYNC_4KHZ(1);
Use the value inline rather than adding a local variable write.
> + ret = __adis_update_bits(&st->adis, ADIS16475_REG_MSG_CTRL,
> + ADIS16475_SYNC_4KHZ_MASK, val);
> + if (ret)
> + return ret;
> + }
>
> /* default to internal clk */
> st->clk_freq = st->info->int_clk * 1000;
> @@ -1398,10 +1820,9 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
> /*
> * In sync scaled mode, the IMU sample rate is the clk_freq * sync_scale.
> * Hence, default the IMU sample rate to the highest multiple of the input
> - * clock lower than the IMU max sample rate. The optimal range is
> - * 1900-2100 sps...
> + * clock lower than the IMU max sample rate.
> */
> - up_scale = 2100 / st->clk_freq;
> + up_scale = max_sample_rate / st->clk_freq;
>
> ret = __adis_write_reg_16(&st->adis,
> ADIS16475_REG_UP_SCALE,
> @@ -1467,7 +1888,23 @@ static int adis16475_config_irq_pin(struct adis16475 *st)
> */
> usleep_range(250, 260);
>
> - return 0;
> + /*
> + * If the device has FIFO support, configure the watermark polarity
> + * pin as well.
The pin is for polarity or the polarity is for the watermark signalling on that
pin? I'm not seeing a datasheet yet for these parts so I couldn't check.
> + */
> + if (st->info->flags & ADIS16475_HAS_FIFO) {
> + val = ADIS16475_WM_POL(polarity);
> + ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL,
> + ADIS16475_WM_POL_MASK, val);
> + if (ret)
> + return ret;
> +
> + /* Enable watermark interrupt pin. */
> + val = ADIS16475_WM_EN(1);
> + ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_WM_EN_MASK, val);
Similar to below, what does val give us? Just use the values
inline.
> + }
> +
> + return ret;
For general code extensibility in the long run I'd prefer you kept the form as
ret = adis_update...
if (ret)
return ret;
}
return 0;
> }
>
>
> @@ -1509,8 +1946,21 @@ static int adis16475_probe(struct spi_device *spi)
> if (ret)
> return ret;
>
> - ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
> - adis16475_trigger_handler);
> + if (st->info->flags & ADIS16475_HAS_FIFO) {
> + ret = devm_adis_setup_buffer_and_trigger_with_attrs(&st->adis, indio_dev,
> + adis16475_trigger_handler_with_fifo,
> + &adis16475_buffer_ops,
> + adis16475_fifo_attributes);
> + if (ret)
> + return ret;
> + /* Update overflow behavior to always overwrite the oldest sample. */
> + u16 val = ADIS16475_OVERWRITE_OLDEST;
Mostly we still don't do inline variable declaration. So move this to top of scope.
Or better yet, just use the value inline.
> +
> + ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_OVERFLOW_MASK, val);
> + } else {
> + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev,
> + adis16475_trigger_handler);
> + }
> if (ret)
> return ret;
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family
2024-04-28 14:45 ` Jonathan Cameron
@ 2024-05-08 14:32 ` Ramona Gradinariu
2024-05-11 11:27 ` Jonathan Cameron
0 siblings, 1 reply; 17+ messages in thread
From: Ramona Gradinariu @ 2024-05-08 14:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-kernel, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
Hello Jonathan,
Some explanations from my side.
>> @@ -437,6 +467,130 @@ static int adis16475_set_filter(struct adis16475 *st, const u32 filter)
>> return 0;
>> }
>>
>> +static ssize_t adis16475_get_fifo_enabled(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>> + struct adis16475 *st = iio_priv(indio_dev);
>> + int ret;
>> + u16 val;
>> +
>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
>> + if (ret)
>> + return ret;
>> + val = FIELD_GET(ADIS16475_FIFO_EN_MASK, val);
>> +
>> + return sysfs_emit(buf, "%d\n", val);
> As below, might as well put the FIELD_GET() in the sysfs_emit rather than
> writing the local parameter.
In all instances where I did, I did it to avoid casting. v2 inlines the values and
the cast is needed to avoid compilation errors.
>
>> + if (adis->data->burst_max_len)
>> + burst_max_length = adis->data->burst_max_len;
>> + else
>> + burst_max_length = burst_length;
>> +
>> + tx = adis->buffer + burst_max_length;
>> + tx[0] = ADIS_READ_REG(burst_req);
>> +
>> + if (burst_req)
> If !burst_req does the rest of this do anything at all?
> If so flip the logic as
> if (!burst_req)
> return adis16475_push_single_sample(pf);
>
> the rest...
> return spi_sync(adis->spi, &adis->msg);
The update is needed even if burst_req is false. The adis message has to be updated
based on the burst request value, which is then used either in
adis16475_push_single_sample or in spi_sync call.
>
>
>> + return spi_sync(adis->spi, &adis->msg);
>> +
>> + return adis16475_push_single_sample(pf);
>> +}
>> +
>> +/*
>> + * This handler is meant to be used for devices which support burst readings
>> + * from FIFO (namely devices from adis1657x family).
>> + * In order to pop the FIFO the 0x68 0x00 FIFO pop burst request has to be sent.
>> + * If the previous device command was not a FIFO pop burst request, the FIFO pop
>> + * burst request will simply pop the FIFO without returning valid data.
>> + * For the nth consecutive burst request, the
>> + * device will send the data popped with the (n-1)th consecutive burst request.
>> + * In order to read the data which was popped previously, without popping the FIFO,
>> + * the 0x00 0x00 burst request has to be sent.
>> + * If after a 0x68 0x00 FIFO pop burst request, there is any other device access
>> + * different from a 0x68 0x00 or a 0x00 0x00 burst request, the FIFO data popped
>> + * previously will be lost.
>> + */
>> +static irqreturn_t adis16475_trigger_handler_with_fifo(int irq, void *p)
>> {
>> struct iio_poll_func *pf = p;
>> struct iio_dev *indio_dev = pf->indio_dev;
>> + struct adis16475 *st = iio_priv(indio_dev);
>> + struct adis *adis = &st->adis;
>> + int ret;
>> + u16 fifo_cnt, i;
>>
>> - adis16475_push_single_sample(pf);
>> + adis_dev_lock(&st->adis);
>> +
>> + ret = __adis_read_reg_16(adis, ADIS16475_REG_FIFO_CNT, &fifo_cnt);
>> + if (ret || fifo_cnt < 2)
>> + goto unlock;
> I would break these conditions and add a comment on why fifo_cnt < 2 is
> a reason to just return 0;
Updated this in v2, and actually fifo_cnt can also be 1 so the code simply
verifies if !fifo_cnt.
>
>> +
>> + if (fifo_cnt > st->fifo_watermark)
>> + fifo_cnt = st->fifo_watermark;
> fifo_cnt = min(fifo_cnt, st->fifo_watermark);
>
> This confuses me though as normally overreading after a fifo watermark is
> both safe and the right thing to do (as reduces chance of overflow etc).
> If we need to clamp to the watermark for some reason, add a comment.
Removed this in v2.
>>
>> ret = __adis_write_reg_16(&st->adis,
>> ADIS16475_REG_UP_SCALE,
>> @@ -1467,7 +1888,23 @@ static int adis16475_config_irq_pin(struct adis16475 *st)
>> */
>> usleep_range(250, 260);
>>
>> - return 0;
>> + /*
>> + * If the device has FIFO support, configure the watermark polarity
>> + * pin as well.
> The pin is for polarity or the polarity is for the watermark signalling on that
> pin? I'm not seeing a datasheet yet for these parts so I couldn't check.
The device has a watermark pin, which can be used as a trigger source.
In this case we set the polarity for the watermark pin.
>
>> + */
>> + if (st->info->flags & ADIS16475_HAS_FIFO) {
>> + val = ADIS16475_WM_POL(polarity);
>> + ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL,
>> + ADIS16475_WM_POL_MASK, val);
>> + if (ret)
>> + return ret;
>> +
>> + /* Enable watermark interrupt pin. */
>> + val = ADIS16475_WM_EN(1);
>> + ret = adis_update_bits(&st->adis, ADIS16475_REG_FIFO_CTRL, ADIS16475_WM_EN_MASK, val);
Best Regards,
Ramona
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 7/7] drivers: iio: imu: Add support for adis1657x family
2024-05-08 14:32 ` Ramona Gradinariu
@ 2024-05-11 11:27 ` Jonathan Cameron
0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2024-05-11 11:27 UTC (permalink / raw)
To: Ramona Gradinariu
Cc: linux-kernel, linux-iio, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh, nuno.sa
On Wed, 8 May 2024 17:32:34 +0300
Ramona Gradinariu <ramona.bolboaca13@gmail.com> wrote:
> Hello Jonathan,
>
> Some explanations from my side.
>
> >> @@ -437,6 +467,130 @@ static int adis16475_set_filter(struct adis16475 *st, const u32 filter)
> >> return 0;
> >> }
> >>
> >> +static ssize_t adis16475_get_fifo_enabled(struct device *dev,
> >> + struct device_attribute *attr,
> >> + char *buf)
> >> +{
> >> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> >> + struct adis16475 *st = iio_priv(indio_dev);
> >> + int ret;
> >> + u16 val;
> >> +
> >> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIFO_CTRL, &val);
> >> + if (ret)
> >> + return ret;
> >> + val = FIELD_GET(ADIS16475_FIFO_EN_MASK, val);
> >> +
> >> + return sysfs_emit(buf, "%d\n", val);
> > As below, might as well put the FIELD_GET() in the sysfs_emit rather than
> > writing the local parameter.
>
> In all instances where I did, I did it to avoid casting. v2 inlines the values and
> the cast is needed to avoid compilation errors.
I'd guess that is because the type in the sysfs_emit isn't the same as FIELD_GET()
returns? That will correspond to the type of the ADIS16475_FIFO_EN_MASK
which is defined as UL I think. So just use %lu here instead and need to cast goes
away.
>
> >
> >> + if (adis->data->burst_max_len)
> >> + burst_max_length = adis->data->burst_max_len;
> >> + else
> >> + burst_max_length = burst_length;
> >> +
> >> + tx = adis->buffer + burst_max_length;
> >> + tx[0] = ADIS_READ_REG(burst_req);
> >> +
> >> + if (burst_req)
> > If !burst_req does the rest of this do anything at all?
> > If so flip the logic as
> > if (!burst_req)
> > return adis16475_push_single_sample(pf);
> >
> > the rest...
> > return spi_sync(adis->spi, &adis->msg);
>
> The update is needed even if burst_req is false. The adis message has to be updated
> based on the burst request value, which is then used either in
> adis16475_push_single_sample or in spi_sync call.
Which update? The content of tx[0]? If so that is non obvious so definitely
need a comment. Perhaps even wrap up the update in a function with a name
that makes it clear it's changing the adis->msg.
adis_update_msg_burst() or something like that.
>
> >
> >
> >> + return spi_sync(adis->spi, &adis->msg);
> >> +
> >> + return adis16475_push_single_sample(pf);
> >> +}
> >> +
Jonathan
^ permalink raw reply [flat|nested] 17+ messages in thread