* [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
@ 2024-09-03 16:33 Gyeyoung Baek
2024-09-05 9:29 ` Jean-Baptiste Maneyrol
0 siblings, 1 reply; 5+ messages in thread
From: Gyeyoung Baek @ 2024-09-03 16:33 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, linux-kernel, Gyeyoung Baek
'wom_bits' variable is defined by chip type,
and chip type is statically defined by device tree.
so 'wom_bits' need to be set once during probe function.
but before code set it every time using 'switch statement' during
threaded irq handler, so i move that to probe function.
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 +++++++++++++++
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 +
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 20 ++-----------------
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 14d95f34e981..322ae664adc0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -2076,6 +2076,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
return result;
}
+ switch (chip_type) {
+ case INV_MPU6050:
+ case INV_MPU6500:
+ case INV_MPU6515:
+ case INV_MPU6880:
+ case INV_MPU6000:
+ case INV_MPU9150:
+ case INV_MPU9250:
+ case INV_MPU9255:
+ st->wom_bits = INV_MPU6500_BIT_WOM_INT;
+ break;
+ default:
+ st->wom_bits = INV_ICM20608_BIT_WOM_INT;
+ break;
+ }
+
return 0;
error_power_off:
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e1c0c5146876..a91b9c2b26e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -212,6 +212,7 @@ struct inv_mpu6050_state {
bool level_shifter;
u8 *data;
s64 it_timestamp;
+ unsigned int wom_bits;
};
/*register and associated bit definition*/
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 84273660ca2e..b19556df1801 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -243,26 +243,10 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
{
struct iio_dev *indio_dev = p;
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- unsigned int int_status, wom_bits;
+ unsigned int int_status;
u64 ev_code;
int result;
- switch (st->chip_type) {
- case INV_MPU6050:
- case INV_MPU6500:
- case INV_MPU6515:
- case INV_MPU6880:
- case INV_MPU6000:
- case INV_MPU9150:
- case INV_MPU9250:
- case INV_MPU9255:
- wom_bits = INV_MPU6500_BIT_WOM_INT;
- break;
- default:
- wom_bits = INV_ICM20608_BIT_WOM_INT;
- break;
- }
-
scoped_guard(mutex, &st->lock) {
/* ack interrupt and check status */
result = regmap_read(st->map, st->reg->int_status, &int_status);
@@ -272,7 +256,7 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
}
/* handle WoM event */
- if (st->chip_config.wom_en && (int_status & wom_bits)) {
+ if (st->chip_config.wom_en && (int_status & st->wom_bits)) {
ev_code = IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING);
iio_push_event(indio_dev, ev_code, st->it_timestamp);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
2024-09-03 16:33 [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function Gyeyoung Baek
@ 2024-09-05 9:29 ` Jean-Baptiste Maneyrol
2024-09-05 14:32 ` gyeyoung
2024-09-07 16:22 ` Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Jean-Baptiste Maneyrol @ 2024-09-05 9:29 UTC (permalink / raw)
To: Gyeyoung Baek, jic23@kernel.org
Cc: linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Hello,
nice improvement, thanks.
But beware there is a fix pending in fixes-togreg branch and missing in testing branch that is changing this part of code.
To avoid a painful merge, it should be better to wait for the fix to be integrated inside testing.
Is that correct Jonathan?
And I would prefer the wom_bits being inside the inv_mpu6050_reg_map structure.
Thanks,
JB
________________________________________
From: Gyeyoung Baek <gye976@gmail.com>
Sent: Tuesday, September 3, 2024 18:33
To: jic23@kernel.org <jic23@kernel.org>
Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Gyeyoung Baek <gye976@gmail.com>
Subject: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
This Message Is From an Untrusted Sender
You have not previously corresponded with this sender.
'wom_bits' variable is defined by chip type,
and chip type is statically defined by device tree.
so 'wom_bits' need to be set once during probe function.
but before code set it every time using 'switch statement' during
threaded irq handler, so i move that to probe function.
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 +++++++++++++++
drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 +
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 20 ++-----------------
3 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 14d95f34e981..322ae664adc0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -2076,6 +2076,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
return result;
}
+ switch (chip_type) {
+ case INV_MPU6050:
+ case INV_MPU6500:
+ case INV_MPU6515:
+ case INV_MPU6880:
+ case INV_MPU6000:
+ case INV_MPU9150:
+ case INV_MPU9250:
+ case INV_MPU9255:
+ st->wom_bits = INV_MPU6500_BIT_WOM_INT;
+ break;
+ default:
+ st->wom_bits = INV_ICM20608_BIT_WOM_INT;
+ break;
+ }
+
return 0;
error_power_off:
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e1c0c5146876..a91b9c2b26e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -212,6 +212,7 @@ struct inv_mpu6050_state {
bool level_shifter;
u8 *data;
s64 it_timestamp;
+ unsigned int wom_bits;
};
/*register and associated bit definition*/
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 84273660ca2e..b19556df1801 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -243,26 +243,10 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
{
struct iio_dev *indio_dev = p;
struct inv_mpu6050_state *st = iio_priv(indio_dev);
- unsigned int int_status, wom_bits;
+ unsigned int int_status;
u64 ev_code;
int result;
- switch (st->chip_type) {
- case INV_MPU6050:
- case INV_MPU6500:
- case INV_MPU6515:
- case INV_MPU6880:
- case INV_MPU6000:
- case INV_MPU9150:
- case INV_MPU9250:
- case INV_MPU9255:
- wom_bits = INV_MPU6500_BIT_WOM_INT;
- break;
- default:
- wom_bits = INV_ICM20608_BIT_WOM_INT;
- break;
- }
-
scoped_guard(mutex, &st->lock) {
/* ack interrupt and check status */
result = regmap_read(st->map, st->reg->int_status, &int_status);
@@ -272,7 +256,7 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
}
/* handle WoM event */
- if (st->chip_config.wom_en && (int_status & wom_bits)) {
+ if (st->chip_config.wom_en && (int_status & st->wom_bits)) {
ev_code = IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING);
iio_push_event(indio_dev, ev_code, st->it_timestamp);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
2024-09-05 9:29 ` Jean-Baptiste Maneyrol
@ 2024-09-05 14:32 ` gyeyoung
2024-09-07 16:27 ` Jonathan Cameron
2024-09-07 16:22 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: gyeyoung @ 2024-09-05 14:32 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol
Cc: jic23@kernel.org, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Thank you for your response. I understand what you mean.
But I think it's better to have 'wom_bits' in 'inv_mpu6050_state',
because the 'wom_bits' variable is only a variable for bit operation
with the 'INT_STATUS' register, not an actual register in register
manual.
Isn't it?
Thanks.
On Thu, Sep 5, 2024 at 6:30 PM Jean-Baptiste Maneyrol
<Jean-Baptiste.Maneyrol@tdk.com> wrote:
>
> Hello,
>
> nice improvement, thanks.
>
> But beware there is a fix pending in fixes-togreg branch and missing in testing branch that is changing this part of code.
> To avoid a painful merge, it should be better to wait for the fix to be integrated inside testing.
>
> Is that correct Jonathan?
>
> And I would prefer the wom_bits being inside the inv_mpu6050_reg_map structure.
>
> Thanks,
> JB
>
> ________________________________________
> From: Gyeyoung Baek <gye976@gmail.com>
> Sent: Tuesday, September 3, 2024 18:33
> To: jic23@kernel.org <jic23@kernel.org>
> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Gyeyoung Baek <gye976@gmail.com>
> Subject: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
>
> This Message Is From an Untrusted Sender
> You have not previously corresponded with this sender.
>
> 'wom_bits' variable is defined by chip type,
> and chip type is statically defined by device tree.
> so 'wom_bits' need to be set once during probe function.
>
> but before code set it every time using 'switch statement' during
> threaded irq handler, so i move that to probe function.
>
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 +++++++++++++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 +
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 20 ++-----------------
> 3 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 14d95f34e981..322ae664adc0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -2076,6 +2076,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> return result;
> }
>
> + switch (chip_type) {
> + case INV_MPU6050:
> + case INV_MPU6500:
> + case INV_MPU6515:
> + case INV_MPU6880:
> + case INV_MPU6000:
> + case INV_MPU9150:
> + case INV_MPU9250:
> + case INV_MPU9255:
> + st->wom_bits = INV_MPU6500_BIT_WOM_INT;
> + break;
> + default:
> + st->wom_bits = INV_ICM20608_BIT_WOM_INT;
> + break;
> + }
> +
> return 0;
>
> error_power_off:
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e1c0c5146876..a91b9c2b26e4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -212,6 +212,7 @@ struct inv_mpu6050_state {
> bool level_shifter;
> u8 *data;
> s64 it_timestamp;
> + unsigned int wom_bits;
> };
>
> /*register and associated bit definition*/
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 84273660ca2e..b19556df1801 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -243,26 +243,10 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> {
> struct iio_dev *indio_dev = p;
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> - unsigned int int_status, wom_bits;
> + unsigned int int_status;
> u64 ev_code;
> int result;
>
> - switch (st->chip_type) {
> - case INV_MPU6050:
> - case INV_MPU6500:
> - case INV_MPU6515:
> - case INV_MPU6880:
> - case INV_MPU6000:
> - case INV_MPU9150:
> - case INV_MPU9250:
> - case INV_MPU9255:
> - wom_bits = INV_MPU6500_BIT_WOM_INT;
> - break;
> - default:
> - wom_bits = INV_ICM20608_BIT_WOM_INT;
> - break;
> - }
> -
> scoped_guard(mutex, &st->lock) {
> /* ack interrupt and check status */
> result = regmap_read(st->map, st->reg->int_status, &int_status);
> @@ -272,7 +256,7 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> }
>
> /* handle WoM event */
> - if (st->chip_config.wom_en && (int_status & wom_bits)) {
> + if (st->chip_config.wom_en && (int_status & st->wom_bits)) {
> ev_code = IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
> IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING);
> iio_push_event(indio_dev, ev_code, st->it_timestamp);
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
2024-09-05 14:32 ` gyeyoung
@ 2024-09-07 16:27 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2024-09-07 16:27 UTC (permalink / raw)
To: gyeyoung
Cc: Jean-Baptiste Maneyrol, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
On Thu, 5 Sep 2024 23:32:47 +0900
gyeyoung <gye976@gmail.com> wrote:
> Thank you for your response. I understand what you mean.
>
> But I think it's better to have 'wom_bits' in 'inv_mpu6050_state',
> because the 'wom_bits' variable is only a variable for bit operation
> with the 'INT_STATUS' register, not an actual register in register
> manual.
> Isn't it?
I'd like it somewhere static const. Either in the register map
(which indeed currently only has register addresses) or in the
hw_info structures. I'd prefer the reg_map.
The current handling as 'code' based on the ID should
be replaced with data.
Jonathan
>
> Thanks.
>
> On Thu, Sep 5, 2024 at 6:30 PM Jean-Baptiste Maneyrol
> <Jean-Baptiste.Maneyrol@tdk.com> wrote:
> >
> > Hello,
> >
> > nice improvement, thanks.
> >
> > But beware there is a fix pending in fixes-togreg branch and missing in testing branch that is changing this part of code.
> > To avoid a painful merge, it should be better to wait for the fix to be integrated inside testing.
> >
> > Is that correct Jonathan?
> >
> > And I would prefer the wom_bits being inside the inv_mpu6050_reg_map structure.
> >
> > Thanks,
> > JB
> >
> > ________________________________________
> > From: Gyeyoung Baek <gye976@gmail.com>
> > Sent: Tuesday, September 3, 2024 18:33
> > To: jic23@kernel.org <jic23@kernel.org>
> > Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Gyeyoung Baek <gye976@gmail.com>
> > Subject: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
> >
> > This Message Is From an Untrusted Sender
> > You have not previously corresponded with this sender.
> >
> > 'wom_bits' variable is defined by chip type,
> > and chip type is statically defined by device tree.
> > so 'wom_bits' need to be set once during probe function.
> >
> > but before code set it every time using 'switch statement' during
> > threaded irq handler, so i move that to probe function.
> >
> > Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> > ---
> > drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 +++++++++++++++
> > drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 +
> > drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 20 ++-----------------
> > 3 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 14d95f34e981..322ae664adc0 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -2076,6 +2076,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> > return result;
> > }
> >
> > + switch (chip_type) {
> > + case INV_MPU6050:
> > + case INV_MPU6500:
> > + case INV_MPU6515:
> > + case INV_MPU6880:
> > + case INV_MPU6000:
> > + case INV_MPU9150:
> > + case INV_MPU9250:
> > + case INV_MPU9255:
> > + st->wom_bits = INV_MPU6500_BIT_WOM_INT;
> > + break;
> > + default:
> > + st->wom_bits = INV_ICM20608_BIT_WOM_INT;
> > + break;
> > + }
> > +
> > return 0;
> >
> > error_power_off:
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index e1c0c5146876..a91b9c2b26e4 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -212,6 +212,7 @@ struct inv_mpu6050_state {
> > bool level_shifter;
> > u8 *data;
> > s64 it_timestamp;
> > + unsigned int wom_bits;
> > };
> >
> > /*register and associated bit definition*/
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > index 84273660ca2e..b19556df1801 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> > @@ -243,26 +243,10 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> > {
> > struct iio_dev *indio_dev = p;
> > struct inv_mpu6050_state *st = iio_priv(indio_dev);
> > - unsigned int int_status, wom_bits;
> > + unsigned int int_status;
> > u64 ev_code;
> > int result;
> >
> > - switch (st->chip_type) {
> > - case INV_MPU6050:
> > - case INV_MPU6500:
> > - case INV_MPU6515:
> > - case INV_MPU6880:
> > - case INV_MPU6000:
> > - case INV_MPU9150:
> > - case INV_MPU9250:
> > - case INV_MPU9255:
> > - wom_bits = INV_MPU6500_BIT_WOM_INT;
> > - break;
> > - default:
> > - wom_bits = INV_ICM20608_BIT_WOM_INT;
> > - break;
> > - }
> > -
> > scoped_guard(mutex, &st->lock) {
> > /* ack interrupt and check status */
> > result = regmap_read(st->map, st->reg->int_status, &int_status);
> > @@ -272,7 +256,7 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> > }
> >
> > /* handle WoM event */
> > - if (st->chip_config.wom_en && (int_status & wom_bits)) {
> > + if (st->chip_config.wom_en && (int_status & st->wom_bits)) {
> > ev_code = IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
> > IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING);
> > iio_push_event(indio_dev, ev_code, st->it_timestamp);
> > --
> > 2.34.1
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
2024-09-05 9:29 ` Jean-Baptiste Maneyrol
2024-09-05 14:32 ` gyeyoung
@ 2024-09-07 16:22 ` Jonathan Cameron
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2024-09-07 16:22 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol
Cc: Gyeyoung Baek, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
On Thu, 5 Sep 2024 09:29:58 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:
> Hello,
>
> nice improvement, thanks.
>
> But beware there is a fix pending in fixes-togreg branch and missing in testing branch that is changing this part of code.
> To avoid a painful merge, it should be better to wait for the fix to be integrated inside testing.
>
> Is that correct Jonathan?
Yes. It is too late for new cleanups to hit this cycle anyway, so better to just wait
for that fix to be in my togreg branch (6.12-rc1 probably)
>
> And I would prefer the wom_bits being inside the inv_mpu6050_reg_map structure.
>
> Thanks,
> JB
>
> ________________________________________
> From: Gyeyoung Baek <gye976@gmail.com>
> Sent: Tuesday, September 3, 2024 18:33
> To: jic23@kernel.org <jic23@kernel.org>
> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; Gyeyoung Baek <gye976@gmail.com>
> Subject: [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function
>
> This Message Is From an Untrusted Sender
> You have not previously corresponded with this sender.
>
> 'wom_bits' variable is defined by chip type,
> and chip type is statically defined by device tree.
> so 'wom_bits' need to be set once during probe function.
>
> but before code set it every time using 'switch statement' during
> threaded irq handler, so i move that to probe function.
>
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 16 +++++++++++++++
> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 1 +
> drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 20 ++-----------------
> 3 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 14d95f34e981..322ae664adc0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -2076,6 +2076,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> return result;
> }
>
> + switch (chip_type) {
> + case INV_MPU6050:
> + case INV_MPU6500:
> + case INV_MPU6515:
> + case INV_MPU6880:
> + case INV_MPU6000:
> + case INV_MPU9150:
> + case INV_MPU9250:
> + case INV_MPU9255:
> + st->wom_bits = INV_MPU6500_BIT_WOM_INT;
> + break;
> + default:
> + st->wom_bits = INV_ICM20608_BIT_WOM_INT;
> + break;
> + }
> +
> return 0;
>
> error_power_off:
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e1c0c5146876..a91b9c2b26e4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -212,6 +212,7 @@ struct inv_mpu6050_state {
> bool level_shifter;
> u8 *data;
> s64 it_timestamp;
> + unsigned int wom_bits;
> };
>
> /*register and associated bit definition*/
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> index 84273660ca2e..b19556df1801 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
> @@ -243,26 +243,10 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> {
> struct iio_dev *indio_dev = p;
> struct inv_mpu6050_state *st = iio_priv(indio_dev);
> - unsigned int int_status, wom_bits;
> + unsigned int int_status;
> u64 ev_code;
> int result;
>
> - switch (st->chip_type) {
> - case INV_MPU6050:
> - case INV_MPU6500:
> - case INV_MPU6515:
> - case INV_MPU6880:
> - case INV_MPU6000:
> - case INV_MPU9150:
> - case INV_MPU9250:
> - case INV_MPU9255:
> - wom_bits = INV_MPU6500_BIT_WOM_INT;
> - break;
> - default:
> - wom_bits = INV_ICM20608_BIT_WOM_INT;
> - break;
> - }
> -
> scoped_guard(mutex, &st->lock) {
> /* ack interrupt and check status */
> result = regmap_read(st->map, st->reg->int_status, &int_status);
> @@ -272,7 +256,7 @@ static irqreturn_t inv_mpu6050_interrupt_handle(int irq, void *p)
> }
>
> /* handle WoM event */
> - if (st->chip_config.wom_en && (int_status & wom_bits)) {
> + if (st->chip_config.wom_en && (int_status & st->wom_bits)) {
> ev_code = IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, IIO_MOD_X_OR_Y_OR_Z,
> IIO_EV_TYPE_ROC, IIO_EV_DIR_RISING);
> iio_push_event(indio_dev, ev_code, st->it_timestamp);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-07 16:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-03 16:33 [PATCH] iio: imu: inv_mpu6050: Move setting 'wom_bits' to probe function Gyeyoung Baek
2024-09-05 9:29 ` Jean-Baptiste Maneyrol
2024-09-05 14:32 ` gyeyoung
2024-09-07 16:27 ` Jonathan Cameron
2024-09-07 16:22 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox