All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Fixes and improvements to the ADIS lib/devices
@ 2021-09-03 14:14 Nuno Sá
  2021-09-03 14:14 ` [PATCH 1/5] iio: adis: do not disabe IRQs in 'adis_init()' Nuno Sá
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

This series adds some fixes and improvements for the ADIS library and
devices. There are two fixes:
 
1) On the library 'adis_init()' function, a call to 'adis_enable_irq()'
was being done. That does not make sense because
'__adis_initial_startup()' will reset the device and put the data ready
pin in the default state. For some drivers, these could mean that we
were leaving probe in a state different from the desired one.
2) The adis16480 driver was registering a managed reset action to put
the device into sleep mode in the unbinding path. Well, not all devices
supported by the driver support sleep mode.
 
The rest of the series is a minor improvement about passing the handling of
enabling/disabling IRQs (for devices that cannot unmask the data ready
pin) inside the library.

Nuno Sá (5):
  iio: adis: do not disabe IRQs in 'adis_init()'
  iio: adis: handle devices that cannot unmask the drdy pin
  iio: adis16475: make use of the new unmasked_drdy flag
  iio: adis16460: make use of the new unmasked_drdy flag
  iio: adis16480: fix devices that do not support sleep mode

 drivers/iio/imu/adis.c         | 17 ++++++++++++++++-
 drivers/iio/imu/adis16460.c    | 18 +-----------------
 drivers/iio/imu/adis16475.c    | 19 +------------------
 drivers/iio/imu/adis16480.c    | 14 +++++++++++---
 drivers/iio/imu/adis_trigger.c |  4 ++++
 include/linux/iio/imu/adis.h   |  2 ++
 6 files changed, 35 insertions(+), 39 deletions(-)

-- 
2.33.0


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

* [PATCH 1/5] iio: adis: do not disabe IRQs in 'adis_init()'
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
@ 2021-09-03 14:14 ` Nuno Sá
  2021-09-03 14:14 ` [PATCH 2/5] iio: adis: handle devices that cannot unmask the drdy pin Nuno Sá
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

With commit ecb010d441088 ("iio: imu: adis: Refactor adis_initial_startup")
we are doing a HW or SW reset to the device which means that we'll get
the default state of the data ready pin (which is enabled). Hence there's
no point in disabling the IRQ in the init function. Moreover, this
function is intended to initialize internal data structures and not
really do anything on the device.

As a result of this, some devices were left with the data ready pin enabled
after probe which was not the desired behavior. Thus, we move the call to
'adis_enable_irq()' to the initial startup function where it makes more
sense for it to be.

Note that for devices that cannot mask/unmask the pin, it makes no sense
to call the function at this point since the IRQ should not have been
yet requested. This will be improved in a follow up change.

Fixes: ecb010d441088 ("iio: imu: adis: Refactor adis_initial_startup")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index b9a06ca29bee..d4e692b187cd 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -430,6 +430,8 @@ int __adis_initial_startup(struct adis *adis)
 	if (ret)
 		return ret;
 
+	adis_enable_irq(adis, false);
+
 	if (!adis->data->prod_id_reg)
 		return 0;
 
@@ -526,7 +528,7 @@ int adis_init(struct adis *adis, struct iio_dev *indio_dev,
 		adis->current_page = 0;
 	}
 
-	return adis_enable_irq(adis, false);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(adis_init);
 
-- 
2.33.0


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

* [PATCH 2/5] iio: adis: handle devices that cannot unmask the drdy pin
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
  2021-09-03 14:14 ` [PATCH 1/5] iio: adis: do not disabe IRQs in 'adis_init()' Nuno Sá
@ 2021-09-03 14:14 ` Nuno Sá
  2021-09-03 14:14 ` [PATCH 3/5] iio: adis16475: make use of the new unmasked_drdy flag Nuno Sá
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

Some devices can't mask/unmask the data ready pin and in those cases
each driver was just calling '{dis}enable_irq()' to control the trigger
state. This change, moves that handling into the library by introducing
a new boolean in the data structure that tells the library that the
device cannot unmask the pin.

On top of controlling the trigger state, we can also use this flag to
automatically request the IRQ with 'IRQF_NO_AUTOEN' in case it is set.
So far, all users of the library want to start operation with IRQs/DRDY
pin disabled so it should be fairly safe to do this inside the library.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis.c         | 15 ++++++++++++++-
 drivers/iio/imu/adis_trigger.c |  4 ++++
 include/linux/iio/imu/adis.h   |  2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index d4e692b187cd..cb0d66bf6561 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -286,6 +286,13 @@ int adis_enable_irq(struct adis *adis, bool enable)
 	if (adis->data->enable_irq) {
 		ret = adis->data->enable_irq(adis, enable);
 		goto out_unlock;
+	} else if (adis->data->unmasked_drdy) {
+		if (enable)
+			enable_irq(adis->spi->irq);
+		else
+			disable_irq(adis->spi->irq);
+
+		goto out_unlock;
 	}
 
 	ret = __adis_read_reg_16(adis, adis->data->msc_ctrl_reg, &msc);
@@ -430,7 +437,13 @@ int __adis_initial_startup(struct adis *adis)
 	if (ret)
 		return ret;
 
-	adis_enable_irq(adis, false);
+	/*
+	 * don't bother calling this if we can't unmask the IRQ as in this case
+	 * the IRQ is most likely not yet requested and we will request it
+	 * with 'IRQF_NO_AUTOEN' anyways.
+	 */
+	if (!adis->data->unmasked_drdy)
+		adis_enable_irq(adis, false);
 
 	if (!adis->data->prod_id_reg)
 		return 0;
diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
index 48eedc29b28a..c461bd1e8e69 100644
--- a/drivers/iio/imu/adis_trigger.c
+++ b/drivers/iio/imu/adis_trigger.c
@@ -30,6 +30,10 @@ static const struct iio_trigger_ops adis_trigger_ops = {
 static int adis_validate_irq_flag(struct adis *adis)
 {
 	unsigned long direction = adis->irq_flag & IRQF_TRIGGER_MASK;
+
+	/* We cannot mask the interrupt so ensure it's not enabled at request */
+	if (adis->data->unmasked_drdy)
+		adis->irq_flag |= IRQF_NO_AUTOEN;
 	/*
 	 * Typically this devices have data ready either on the rising edge or
 	 * on the falling edge of the data ready pin. This checks enforces that
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index cf49997d5903..7c02f5292eea 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -49,6 +49,7 @@ struct adis_timeout {
  * @status_error_mask: Bitmask of errors supported by the device
  * @timeouts: Chip specific delays
  * @enable_irq: Hook for ADIS devices that have a special IRQ enable/disable
+ * @unmasked_drdy: True for devices that cannot mask/unmask the data ready pin
  * @has_paging: True if ADIS device has paged registers
  * @burst_reg_cmd:	Register command that triggers burst
  * @burst_len:		Burst size in the SPI RX buffer. If @burst_max_len is defined,
@@ -78,6 +79,7 @@ struct adis_data {
 	unsigned int status_error_mask;
 
 	int (*enable_irq)(struct adis *adis, bool enable);
+	bool unmasked_drdy;
 
 	bool has_paging;
 
-- 
2.33.0


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

* [PATCH 3/5] iio: adis16475: make use of the new unmasked_drdy flag
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
  2021-09-03 14:14 ` [PATCH 1/5] iio: adis: do not disabe IRQs in 'adis_init()' Nuno Sá
  2021-09-03 14:14 ` [PATCH 2/5] iio: adis: handle devices that cannot unmask the drdy pin Nuno Sá
@ 2021-09-03 14:14 ` Nuno Sá
  2021-09-03 14:14 ` [PATCH 4/5] iio: adis16460: " Nuno Sá
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

The library can now handle enabling/disabling IRQs for devices that
cannot unmask the data ready pin. Hence there's no need to provide an
'enable_irq' callback anymore.

The library will also automatically request the IRQ with 'IRQF_NO_AUTOEN'
so that we can also remove that from the driver.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16475.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index eb48102f9424..b76d8482bbd5 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -606,20 +606,6 @@ static const char * const adis16475_status_error_msgs[] = {
 	[ADIS16475_DIAG_STAT_CLK] = "Clock error",
 };
 
-static int adis16475_enable_irq(struct adis *adis, bool enable)
-{
-	/*
-	 * There is no way to gate the data-ready signal internally inside the
-	 * ADIS16475. We can only control it's polarity...
-	 */
-	if (enable)
-		enable_irq(adis->spi->irq);
-	else
-		disable_irq(adis->spi->irq);
-
-	return 0;
-}
-
 #define ADIS16475_DATA(_prod_id, _timeouts)				\
 {									\
 	.msc_ctrl_reg = ADIS16475_REG_MSG_CTRL,				\
@@ -640,7 +626,7 @@ static int adis16475_enable_irq(struct adis *adis, bool enable)
 		BIT(ADIS16475_DIAG_STAT_SENSOR) |			\
 		BIT(ADIS16475_DIAG_STAT_MEMORY) |			\
 		BIT(ADIS16475_DIAG_STAT_CLK),				\
-	.enable_irq = adis16475_enable_irq,				\
+	.unmasked_drdy = true,						\
 	.timeouts = (_timeouts),					\
 	.burst_reg_cmd = ADIS16475_REG_GLOB_CMD,			\
 	.burst_len = ADIS16475_BURST_MAX_DATA,				\
@@ -1254,9 +1240,6 @@ static int adis16475_config_irq_pin(struct adis16475 *st)
 		return -EINVAL;
 	}
 
-	/* We cannot mask the interrupt so ensure it's not enabled at request */
-	st->adis.irq_flag |= IRQF_NO_AUTOEN;
-
 	val = ADIS16475_MSG_CTRL_DR_POL(polarity);
 	ret = __adis_update_bits(&st->adis, ADIS16475_REG_MSG_CTRL,
 				 ADIS16475_MSG_CTRL_DR_POL_MASK, val);
-- 
2.33.0


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

* [PATCH 4/5] iio: adis16460: make use of the new unmasked_drdy flag
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
                   ` (2 preceding siblings ...)
  2021-09-03 14:14 ` [PATCH 3/5] iio: adis16475: make use of the new unmasked_drdy flag Nuno Sá
@ 2021-09-03 14:14 ` Nuno Sá
  2021-09-26 15:25   ` Jonathan Cameron
  2021-09-03 14:14 ` [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode Nuno Sá
  2021-09-05 13:39 ` [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Jonathan Cameron
  5 siblings, 1 reply; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

The library can now handle enabling/disabling IRQs for devices that
cannot unmask the data ready pin. Hence there's no need to provide an
'enable_irq' callback anymore.

The library will also automatically request the IRQ with 'IRQF_NO_AUTOEN'
so that we can also remove that from the driver.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16460.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index a6f9fba3e03f..b01988170118 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -319,20 +319,6 @@ static const struct iio_info adis16460_info = {
 	.debugfs_reg_access = adis_debugfs_reg_access,
 };
 
-static int adis16460_enable_irq(struct adis *adis, bool enable)
-{
-	/*
-	 * There is no way to gate the data-ready signal internally inside the
-	 * ADIS16460 :(
-	 */
-	if (enable)
-		enable_irq(adis->spi->irq);
-	else
-		disable_irq(adis->spi->irq);
-
-	return 0;
-}
-
 #define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
 #define ADIS16460_DIAG_STAT_FLASH_MEM	6
 #define ADIS16460_DIAG_STAT_SELF_TEST	5
@@ -373,7 +359,7 @@ static const struct adis_data adis16460_data = {
 		BIT(ADIS16460_DIAG_STAT_OVERRANGE) |
 		BIT(ADIS16460_DIAG_STAT_SPI_COMM) |
 		BIT(ADIS16460_DIAG_STAT_FLASH_UPT),
-	.enable_irq = adis16460_enable_irq,
+	.unmasked_drdy = true,
 	.timeouts = &adis16460_timeouts,
 };
 
@@ -400,8 +386,6 @@ static int adis16460_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	/* We cannot mask the interrupt, so ensure it isn't auto enabled */
-	st->adis.irq_flag |= IRQF_NO_AUTOEN;
 	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
 	if (ret)
 		return ret;
-- 
2.33.0


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

* [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
                   ` (3 preceding siblings ...)
  2021-09-03 14:14 ` [PATCH 4/5] iio: adis16460: " Nuno Sá
@ 2021-09-03 14:14 ` Nuno Sá
  2021-09-26 15:23   ` Jonathan Cameron
  2021-09-05 13:39 ` [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Jonathan Cameron
  5 siblings, 1 reply; 10+ messages in thread
From: Nuno Sá @ 2021-09-03 14:14 UTC (permalink / raw)
  To: linux-iio
  Cc: Jonathan Cameron, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

Not all devices supported by this driver support being put to sleep
mode. For those devices, when calling 'adis16480_stop_device()' on the
unbind path, we where actually writing in the SYNC_SCALE register.

Fixes: 80cbc848c4fa0 ("iio: imu: adis16480: Add support for ADIS16490")
Fixes: 82e7a1b250170 ("iio: imu: adis16480: Add support for ADIS1649x family of devices")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/imu/adis16480.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index a869a6e52a16..ed129321a14d 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -144,6 +144,7 @@ struct adis16480_chip_info {
 	unsigned int max_dec_rate;
 	const unsigned int *filter_freqs;
 	bool has_pps_clk_mode;
+	bool has_sleep_cnt;
 	const struct adis_data adis_data;
 };
 
@@ -939,6 +940,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
+		.has_sleep_cnt = true,
 		.filter_freqs = adis16480_def_filter_freqs,
 		.adis_data = ADIS16480_DATA(16375, &adis16485_timeouts, 0),
 	},
@@ -952,6 +954,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
+		.has_sleep_cnt = true,
 		.filter_freqs = adis16480_def_filter_freqs,
 		.adis_data = ADIS16480_DATA(16480, &adis16480_timeouts, 0),
 	},
@@ -965,6 +968,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
+		.has_sleep_cnt = true,
 		.filter_freqs = adis16480_def_filter_freqs,
 		.adis_data = ADIS16480_DATA(16485, &adis16485_timeouts, 0),
 	},
@@ -978,6 +982,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
+		.has_sleep_cnt = true,
 		.filter_freqs = adis16480_def_filter_freqs,
 		.adis_data = ADIS16480_DATA(16488, &adis16485_timeouts, 0),
 	},
@@ -1425,9 +1430,12 @@ static int adis16480_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = devm_add_action_or_reset(&spi->dev, adis16480_stop, indio_dev);
-	if (ret)
-		return ret;
+	if (st->chip_info->has_sleep_cnt) {
+		ret = devm_add_action_or_reset(&spi->dev, adis16480_stop,
+					       indio_dev);
+		if (ret)
+			return ret;
+	}
 
 	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
 	if (ret)
-- 
2.33.0


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

* Re: [PATCH 0/5] Fixes and improvements to the ADIS lib/devices
  2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
                   ` (4 preceding siblings ...)
  2021-09-03 14:14 ` [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode Nuno Sá
@ 2021-09-05 13:39 ` Jonathan Cameron
  2021-09-06  8:56   ` Sa, Nuno
  5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2021-09-05 13:39 UTC (permalink / raw)
  To: Nuno Sá
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

On Fri, 3 Sep 2021 16:14:18 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> This series adds some fixes and improvements for the ADIS library and
> devices. There are two fixes:
>  
> 1) On the library 'adis_init()' function, a call to 'adis_enable_irq()'
> was being done. That does not make sense because
> '__adis_initial_startup()' will reset the device and put the data ready
> pin in the default state. For some drivers, these could mean that we
> were leaving probe in a state different from the desired one.
> 2) The adis16480 driver was registering a managed reset action to put
> the device into sleep mode in the unbinding path. Well, not all devices
> supported by the driver support sleep mode.
>  
> The rest of the series is a minor improvement about passing the handling of
> enabling/disabling IRQs (for devices that cannot unmask the data ready
> pin) inside the library.
> 
> Nuno Sá (5):
>   iio: adis: do not disabe IRQs in 'adis_init()'
>   iio: adis: handle devices that cannot unmask the drdy pin
>   iio: adis16475: make use of the new unmasked_drdy flag
>   iio: adis16460: make use of the new unmasked_drdy flag
>   iio: adis16480: fix devices that do not support sleep mode

Hi Nuno.

Series looks good to me but I'd like it to sit on list a little longer before
I take any of it.

Would have been 'nice' to have had the two fixes at the start of the series
as they should probably go via stable whereas the 3 patches moving things
into the core are 5.16 material.

Looks like I should be fine picking two fixes up even in this order though so
not a big issue.

Thanks,

Jonathan

> 
>  drivers/iio/imu/adis.c         | 17 ++++++++++++++++-
>  drivers/iio/imu/adis16460.c    | 18 +-----------------
>  drivers/iio/imu/adis16475.c    | 19 +------------------
>  drivers/iio/imu/adis16480.c    | 14 +++++++++++---
>  drivers/iio/imu/adis_trigger.c |  4 ++++
>  include/linux/iio/imu/adis.h   |  2 ++
>  6 files changed, 35 insertions(+), 39 deletions(-)
> 


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

* RE: [PATCH 0/5] Fixes and improvements to the ADIS lib/devices
  2021-09-05 13:39 ` [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Jonathan Cameron
@ 2021-09-06  8:56   ` Sa, Nuno
  0 siblings, 0 replies; 10+ messages in thread
From: Sa, Nuno @ 2021-09-06  8:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio, Lars-Peter Clausen, Hennerich, Michael, Bogdan, Dragos



> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, September 5, 2021 3:40 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>
> Cc: linux-iio@vger.kernel.org; Lars-Peter Clausen <lars@metafoo.de>;
> Hennerich, Michael <Michael.Hennerich@analog.com>; Bogdan,
> Dragos <Dragos.Bogdan@analog.com>
> Subject: Re: [PATCH 0/5] Fixes and improvements to the ADIS
> lib/devices
> 
> On Fri, 3 Sep 2021 16:14:18 +0200
> Nuno Sá <nuno.sa@analog.com> wrote:
> 
> > This series adds some fixes and improvements for the ADIS library
> and
> > devices. There are two fixes:
> >
> > 1) On the library 'adis_init()' function, a call to 'adis_enable_irq()'
> > was being done. That does not make sense because
> > '__adis_initial_startup()' will reset the device and put the data ready
> > pin in the default state. For some drivers, these could mean that we
> > were leaving probe in a state different from the desired one.
> > 2) The adis16480 driver was registering a managed reset action to put
> > the device into sleep mode in the unbinding path. Well, not all
> devices
> > supported by the driver support sleep mode.
> >
> > The rest of the series is a minor improvement about passing the
> handling of
> > enabling/disabling IRQs (for devices that cannot unmask the data
> ready
> > pin) inside the library.
> >
> > Nuno Sá (5):
> >   iio: adis: do not disabe IRQs in 'adis_init()'
> >   iio: adis: handle devices that cannot unmask the drdy pin
> >   iio: adis16475: make use of the new unmasked_drdy flag
> >   iio: adis16460: make use of the new unmasked_drdy flag
> >   iio: adis16480: fix devices that do not support sleep mode
> 
> Hi Nuno.
> 
> Series looks good to me but I'd like it to sit on list a little longer before
> I take any of it.
> 
> Would have been 'nice' to have had the two fixes at the start of the
> series
> as they should probably go via stable whereas the 3 patches moving
> things
> into the core are 5.16 material.

I actually wondered about this but since the adis16480 fix has no
dependencies on the lib stuff I ended up doing the lib patches first
and then the drivers. Next time I will send fixes together...

- Nuno Sá


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

* Re: [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode
  2021-09-03 14:14 ` [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode Nuno Sá
@ 2021-09-26 15:23   ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2021-09-26 15:23 UTC (permalink / raw)
  To: Nuno Sá
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

On Fri, 3 Sep 2021 16:14:23 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> Not all devices supported by this driver support being put to sleep
> mode. For those devices, when calling 'adis16480_stop_device()' on the
> unbind path, we where actually writing in the SYNC_SCALE register.
> 
> Fixes: 80cbc848c4fa0 ("iio: imu: adis16480: Add support for ADIS16490")
> Fixes: 82e7a1b250170 ("iio: imu: adis16480: Add support for ADIS1649x family of devices")
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16480.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index a869a6e52a16..ed129321a14d 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -144,6 +144,7 @@ struct adis16480_chip_info {
>  	unsigned int max_dec_rate;
>  	const unsigned int *filter_freqs;
>  	bool has_pps_clk_mode;
> +	bool has_sleep_cnt;
>  	const struct adis_data adis_data;
>  };
>  
> @@ -939,6 +940,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.temp_scale = 5650, /* 5.65 milli degree Celsius */
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
> +		.has_sleep_cnt = true,
>  		.filter_freqs = adis16480_def_filter_freqs,
>  		.adis_data = ADIS16480_DATA(16375, &adis16485_timeouts, 0),
>  	},
> @@ -952,6 +954,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.temp_scale = 5650, /* 5.65 milli degree Celsius */
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
> +		.has_sleep_cnt = true,
>  		.filter_freqs = adis16480_def_filter_freqs,
>  		.adis_data = ADIS16480_DATA(16480, &adis16480_timeouts, 0),
>  	},
> @@ -965,6 +968,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.temp_scale = 5650, /* 5.65 milli degree Celsius */
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
> +		.has_sleep_cnt = true,
>  		.filter_freqs = adis16480_def_filter_freqs,
>  		.adis_data = ADIS16480_DATA(16485, &adis16485_timeouts, 0),
>  	},
> @@ -978,6 +982,7 @@ static const struct adis16480_chip_info adis16480_chip_info[] = {
>  		.temp_scale = 5650, /* 5.65 milli degree Celsius */
>  		.int_clk = 2460000,
>  		.max_dec_rate = 2048,
> +		.has_sleep_cnt = true,
>  		.filter_freqs = adis16480_def_filter_freqs,
>  		.adis_data = ADIS16480_DATA(16488, &adis16485_timeouts, 0),
>  	},
> @@ -1425,9 +1430,12 @@ static int adis16480_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_add_action_or_reset(&spi->dev, adis16480_stop, indio_dev);
> -	if (ret)
> -		return ret;
> +	if (st->chip_info->has_sleep_cnt) {
> +		ret = devm_add_action_or_reset(&spi->dev, adis16480_stop,
> +					       indio_dev);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	ret = adis16480_config_irq_pin(spi->dev.of_node, st);
>  	if (ret)


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

* Re: [PATCH 4/5] iio: adis16460: make use of the new unmasked_drdy flag
  2021-09-03 14:14 ` [PATCH 4/5] iio: adis16460: " Nuno Sá
@ 2021-09-26 15:25   ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2021-09-26 15:25 UTC (permalink / raw)
  To: Nuno Sá
  Cc: linux-iio, Lars-Peter Clausen, Michael Hennerich, Dragos Bogdan

On Fri, 3 Sep 2021 16:14:22 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> The library can now handle enabling/disabling IRQs for devices that
> cannot unmask the data ready pin. Hence there's no need to provide an
> 'enable_irq' callback anymore.
> 
> The library will also automatically request the IRQ with 'IRQF_NO_AUTOEN'
> so that we can also remove that from the driver.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Hi Nuno,

1-4 applied to the togreg branch of iio.git and pushed out as testing for 0-day
to see if it can find anything we missed.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/adis16460.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
> index a6f9fba3e03f..b01988170118 100644
> --- a/drivers/iio/imu/adis16460.c
> +++ b/drivers/iio/imu/adis16460.c
> @@ -319,20 +319,6 @@ static const struct iio_info adis16460_info = {
>  	.debugfs_reg_access = adis_debugfs_reg_access,
>  };
>  
> -static int adis16460_enable_irq(struct adis *adis, bool enable)
> -{
> -	/*
> -	 * There is no way to gate the data-ready signal internally inside the
> -	 * ADIS16460 :(
> -	 */
> -	if (enable)
> -		enable_irq(adis->spi->irq);
> -	else
> -		disable_irq(adis->spi->irq);
> -
> -	return 0;
> -}
> -
>  #define ADIS16460_DIAG_STAT_IN_CLK_OOS	7
>  #define ADIS16460_DIAG_STAT_FLASH_MEM	6
>  #define ADIS16460_DIAG_STAT_SELF_TEST	5
> @@ -373,7 +359,7 @@ static const struct adis_data adis16460_data = {
>  		BIT(ADIS16460_DIAG_STAT_OVERRANGE) |
>  		BIT(ADIS16460_DIAG_STAT_SPI_COMM) |
>  		BIT(ADIS16460_DIAG_STAT_FLASH_UPT),
> -	.enable_irq = adis16460_enable_irq,
> +	.unmasked_drdy = true,
>  	.timeouts = &adis16460_timeouts,
>  };
>  
> @@ -400,8 +386,6 @@ static int adis16460_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	/* We cannot mask the interrupt, so ensure it isn't auto enabled */
> -	st->adis.irq_flag |= IRQF_NO_AUTOEN;
>  	ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL);
>  	if (ret)
>  		return ret;


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

end of thread, other threads:[~2021-09-26 15:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 14:14 [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Nuno Sá
2021-09-03 14:14 ` [PATCH 1/5] iio: adis: do not disabe IRQs in 'adis_init()' Nuno Sá
2021-09-03 14:14 ` [PATCH 2/5] iio: adis: handle devices that cannot unmask the drdy pin Nuno Sá
2021-09-03 14:14 ` [PATCH 3/5] iio: adis16475: make use of the new unmasked_drdy flag Nuno Sá
2021-09-03 14:14 ` [PATCH 4/5] iio: adis16460: " Nuno Sá
2021-09-26 15:25   ` Jonathan Cameron
2021-09-03 14:14 ` [PATCH 5/5] iio: adis16480: fix devices that do not support sleep mode Nuno Sá
2021-09-26 15:23   ` Jonathan Cameron
2021-09-05 13:39 ` [PATCH 0/5] Fixes and improvements to the ADIS lib/devices Jonathan Cameron
2021-09-06  8:56   ` Sa, Nuno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.