2022-11-18 09:42:49

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v3 00/10] Add '__adis_enable_irq()'

Added implementation for '__adis_enable_irq()' and called it instead of
'adis_enable_irq()' inside '__adis_initial_startup()'.
Replaced 'adis_initial_startup()' calls in probe, with its unlocked
variant.
changes in v3:
new patches: 1,2
changed commit message for patches 2-9

Ramona Bolboaca (10):
iio: adis: add '__adis_enable_irq()' implementation
iio: adis: Call '__adis_enable_irq()' in '__adis_initial_startup()'
iio: accel: adis16201: Call '__adis_initial_startup()' in probe
iio: accel: adis16209: Call '__adis_initial_startup()' in probe
iio: gyro: adis16136: Call '__adis_initial_startup()' in probe
iio: gyro: adis16260: Call '__adis_initial_startup()' in probe
iio: imu: adis16400: Call '__adis_initial_startup()' in probe
staging: iio: accel: adis16203: Call '__adis_initial_startup()'
staging: iio: accel: adis16240: Call '__adis_initial_startup()'
iio: imu: adis: Remove adis_initial_startup function

drivers/iio/accel/adis16201.c | 2 +-
drivers/iio/accel/adis16209.c | 2 +-
drivers/iio/gyro/adis16136.c | 2 +-
drivers/iio/gyro/adis16260.c | 2 +-
drivers/iio/imu/adis.c | 28 ++++++++++-----------------
drivers/iio/imu/adis16400.c | 2 +-
drivers/staging/iio/accel/adis16203.c | 2 +-
drivers/staging/iio/accel/adis16240.c | 2 +-
include/linux/iio/imu/adis.h | 16 +++++++--------
9 files changed, 24 insertions(+), 34 deletions(-)

--
2.25.1



2022-11-18 09:43:31

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v3 01/10] iio: adis: add '__adis_enable_irq()' implementation

Add '__adis_enable_irq()' implementation which is the unlocked
version of 'adis_enable_irq()'.

Signed-off-by: Ramona Bolboaca <[email protected]>
---
changes in v3:
- new patch
drivers/iio/imu/adis.c | 26 +++++++++-----------------
include/linux/iio/imu/adis.h | 12 +++++++++++-
2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index f7fcfd04f659..cfcb4abeb5db 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -270,23 +270,19 @@ EXPORT_SYMBOL_NS(adis_debugfs_reg_access, IIO_ADISLIB);
#endif

/**
- * adis_enable_irq() - Enable or disable data ready IRQ
+ * __adis_enable_irq() - Enable or disable data ready IRQ (unlocked)
* @adis: The adis device
* @enable: Whether to enable the IRQ
*
* Returns 0 on success, negative error code otherwise
*/
-int adis_enable_irq(struct adis *adis, bool enable)
+int __adis_enable_irq(struct adis *adis, bool enable)
{
- int ret = 0;
+ int ret;
u16 msc;

- mutex_lock(&adis->state_lock);
-
- if (adis->data->enable_irq) {
- ret = adis->data->enable_irq(adis, enable);
- goto out_unlock;
- }
+ if (adis->data->enable_irq)
+ return adis->data->enable_irq(adis, enable);

if (adis->data->unmasked_drdy) {
if (enable)
@@ -294,12 +290,12 @@ int adis_enable_irq(struct adis *adis, bool enable)
else
disable_irq(adis->spi->irq);

- goto out_unlock;
+ return 0;
}

ret = __adis_read_reg_16(adis, adis->data->msc_ctrl_reg, &msc);
if (ret)
- goto out_unlock;
+ return ret;

msc |= ADIS_MSC_CTRL_DATA_RDY_POL_HIGH;
msc &= ~ADIS_MSC_CTRL_DATA_RDY_DIO2;
@@ -308,13 +304,9 @@ int adis_enable_irq(struct adis *adis, bool enable)
else
msc &= ~ADIS_MSC_CTRL_DATA_RDY_EN;

- ret = __adis_write_reg_16(adis, adis->data->msc_ctrl_reg, msc);
-
-out_unlock:
- mutex_unlock(&adis->state_lock);
- return ret;
+ return __adis_write_reg_16(adis, adis->data->msc_ctrl_reg, msc);
}
-EXPORT_SYMBOL_NS(adis_enable_irq, IIO_ADISLIB);
+EXPORT_SYMBOL_NS(__adis_enable_irq, IIO_ADISLIB);

/**
* __adis_check_status() - Check the device for error conditions (unlocked)
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 515ca09764fe..f5432f3b2b22 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -402,7 +402,17 @@ static inline int adis_update_bits_base(struct adis *adis, unsigned int reg,
__adis_update_bits_base(adis, reg, mask, val, sizeof(val)); \
})

-int adis_enable_irq(struct adis *adis, bool enable);
+static inline int adis_enable_irq(struct adis *adis, bool enable)
+{
+ int ret;
+
+ mutex_lock(&adis->state_lock);
+ ret = __adis_enable_irq(adis);
+ mutex_unlock(&adis->state_lock);
+
+ return ret;
+}
+
int __adis_check_status(struct adis *adis);
int __adis_initial_startup(struct adis *adis);

--
2.25.1


2022-11-18 10:19:04

by Nuno Sa

[permalink] [raw]
Subject: RE: [PATCH v3 01/10] iio: adis: add '__adis_enable_irq()' implementation



> -----Original Message-----
> From: Ramona Bolboaca <[email protected]>
> Sent: Friday, November 18, 2022 9:31 AM
> To: [email protected]; Sa, Nuno <[email protected]>; linux-
> [email protected]; [email protected]
> Cc: Bolboaca, Ramona <[email protected]>
> Subject: [PATCH v3 01/10] iio: adis: add '__adis_enable_irq()' implementation
>
> Add '__adis_enable_irq()' implementation which is the unlocked
> version of 'adis_enable_irq()'.
>
> Signed-off-by: Ramona Bolboaca <[email protected]>
> ---
> changes in v3:
> - new patch
> drivers/iio/imu/adis.c | 26 +++++++++-----------------
> include/linux/iio/imu/adis.h | 12 +++++++++++-
> 2 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index f7fcfd04f659..cfcb4abeb5db 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -270,23 +270,19 @@ EXPORT_SYMBOL_NS(adis_debugfs_reg_access,
> IIO_ADISLIB);
> #endif
>
> /**
> - * adis_enable_irq() - Enable or disable data ready IRQ
> + * __adis_enable_irq() - Enable or disable data ready IRQ (unlocked)
> * @adis: The adis device
> * @enable: Whether to enable the IRQ
> *
> * Returns 0 on success, negative error code otherwise
> */
> -int adis_enable_irq(struct adis *adis, bool enable)
> +int __adis_enable_irq(struct adis *adis, bool enable)
> {
> - int ret = 0;
> + int ret;

This change was not really needed as long as we 'return ret'
In the ' unmasked_drdy' case. Anyways, not a big deal for me...

- Nuno S?

2022-11-18 10:57:12

by Nuno Sa

[permalink] [raw]
Subject: RE: [PATCH v3 00/10] Add '__adis_enable_irq()'



> -----Original Message-----
> From: Ramona Bolboaca <[email protected]>
> Sent: Friday, November 18, 2022 9:31 AM
> To: [email protected]; Sa, Nuno <[email protected]>; linux-
> [email protected]; [email protected]
> Cc: Bolboaca, Ramona <[email protected]>
> Subject: [PATCH v3 00/10] Add '__adis_enable_irq()'
>
> Added implementation for '__adis_enable_irq()' and called it instead of
> 'adis_enable_irq()' inside '__adis_initial_startup()'.
> Replaced 'adis_initial_startup()' calls in probe, with its unlocked
> variant.
> changes in v3:
> new patches: 1,2
> changed commit message for patches 2-9
>
> Ramona Bolboaca (10):
> iio: adis: add '__adis_enable_irq()' implementation
> iio: adis: Call '__adis_enable_irq()' in '__adis_initial_startup()'
> iio: accel: adis16201: Call '__adis_initial_startup()' in probe
> iio: accel: adis16209: Call '__adis_initial_startup()' in probe
> iio: gyro: adis16136: Call '__adis_initial_startup()' in probe
> iio: gyro: adis16260: Call '__adis_initial_startup()' in probe
> iio: imu: adis16400: Call '__adis_initial_startup()' in probe
> staging: iio: accel: adis16203: Call '__adis_initial_startup()'
> staging: iio: accel: adis16240: Call '__adis_initial_startup()'
> iio: imu: adis: Remove adis_initial_startup function
>
> drivers/iio/accel/adis16201.c | 2 +-
> drivers/iio/accel/adis16209.c | 2 +-
> drivers/iio/gyro/adis16136.c | 2 +-
> drivers/iio/gyro/adis16260.c | 2 +-
> drivers/iio/imu/adis.c | 28 ++++++++++-----------------
> drivers/iio/imu/adis16400.c | 2 +-
> drivers/staging/iio/accel/adis16203.c | 2 +-
> drivers/staging/iio/accel/adis16240.c | 2 +-
> include/linux/iio/imu/adis.h | 16 +++++++--------
> 9 files changed, 24 insertions(+), 34 deletions(-)
>

I would squash patch 1 & 2 as we need to backport them both
and adding ''__adis_enable_irq()' is a direct consequence of fixing the
deadlock. Other than that:

Reviewed-by: Nuno S? <[email protected]>