subject:"\[PATCH\] iio\: accel\: mma8452\: code improvements to handle more than one event"

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Harinath Nampally


[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

ok I will resend it with patch version and change history.


This just missed my pull request for today anyway.  There may be time for
another pull towards the end of this week, depending on how busy things get.
If not I'm afraid this will now be the next merge window.

Sure no problem. Thanks.

On 08/20/2017 11:07 AM, Jonathan Cameron wrote:

[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Harinath Nampally


[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

ok I will resend it with patch version and change history.


This just missed my pull request for today anyway.  There may be time for
another pull towards the end of this week, depending on how busy things get.
If not I'm afraid this will now be the next merge window.

Sure no problem. Thanks.

On 08/20/2017 11:07 AM, Jonathan Cameron wrote:

[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Jonathan Cameron

On Sun, 20 Aug 2017 10:42:54 -0400
Harinath Nampally  wrote:

[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

> This driver supports multiple devices like mma8653,
> mma8652, mma8452, mma8453 and fxls8471. Almost all
> these devices have more than one event.
> 
> Current driver design hardcodes the event specific
> information, so only one event can be supported by this
> driver at any given time.
> Also current design doesn't have the flexibility to
> add more events.
> 
> This patch improves by detaching the event related
> information from chip_info struct,and based on channel
> type and event direction the corresponding event
> configuration registers are picked dynamically.
> Hence both transient and freefall events can be
> handled in read/write callbacks.
> 
> Changes are thoroughly tested on fxls8471 device on imx6UL
> Eval board using iio_event_monitor user space program.
> 
> After this fix both Freefall and Transient events are
> handled by the driver without any conflicts.
> 
> Signed-off-by: Harinath Nampally 
> ---

Should be a version history here to let us know what has changed between each
revision.

Otherwise this looks fine, but I'd like to let it sit for a day or two for
other reviews. 

This just missed my pull request for today anyway.  There may be time for
another pull towards the end of this week, depending on how busy things get.
If not I'm afraid this will now be the next merge window.

Thanks,

Jonathan
>  drivers/iio/accel/mma8452.c | 277 
> 
>  1 file changed, 123 insertions(+), 154 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index eb6e3dc..7bfc257 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -59,7 +59,9 @@
>  #define MMA8452_FF_MT_THS0x17
>  #define  MMA8452_FF_MT_THS_MASK  0x7f
>  #define MMA8452_FF_MT_COUNT  0x18
> +#define MMA8452_FF_MT_CHAN_SHIFT 3
>  #define MMA8452_TRANSIENT_CFG0x1d
> +#define  MMA8452_TRANSIENT_CFG_CHAN(chan)BIT(chan + 1)
>  #define  MMA8452_TRANSIENT_CFG_HPF_BYP   BIT(0)
>  #define  MMA8452_TRANSIENT_CFG_ELE   BIT(4)
>  #define MMA8452_TRANSIENT_SRC0x1e
> @@ -69,6 +71,7 @@
>  #define MMA8452_TRANSIENT_THS0x1f
>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT  0x20
> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1
>  #define MMA8452_CTRL_REG10x2a
>  #define  MMA8452_CTRL_ACTIVE BIT(0)
>  #define  MMA8452_CTRL_DR_MASKGENMASK(5, 3)
> @@ -107,6 +110,42 @@ struct mma8452_data {
>   const struct mma_chip_info *chip_info;
>  };
>  
> + /**
> +  * struct mma8452_event_regs - chip specific data related to events
> +  * @ev_cfg: event config register address
> +  * @ev_src: event source register address
> +  * @ev_ths: event threshold register address
> +  * @ev_ths_mask:mask for the threshold value
> +  * @ev_count:   event count (period) register address
> +  *
> +  * Since not all chips supported by the driver support comparing high pass
> +  * filtered data for events (interrupts), different interrupt sources are
> +  * used for different chips and the relevant registers are included here.
> +  */
> +struct mma8452_event_regs {
> + u8 ev_cfg;
> + u8 ev_src;
> + u8 ev_ths;
> + u8 ev_ths_mask;
> + u8 ev_count;
> +};
> +
> +static const struct mma8452_event_regs ev_regs_accel_falling = {
> + .ev_cfg = MMA8452_FF_MT_CFG,
> + .ev_src = MMA8452_FF_MT_SRC,
> + .ev_ths = MMA8452_FF_MT_THS,
> + .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
> + .ev_count = MMA8452_FF_MT_COUNT
> +};
> +
> +static const struct mma8452_event_regs ev_regs_accel_rising = {
> + .ev_cfg = MMA8452_TRANSIENT_CFG,
> + .ev_src = MMA8452_TRANSIENT_SRC,
> + .ev_ths = MMA8452_TRANSIENT_THS,
> + .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> + .ev_count = MMA8452_TRANSIENT_COUNT,
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id: WHO_AM_I register's value
> @@ -116,40 +155,12 @@ struct mma8452_data {
>   * @mma_scales:  scale factors for converting register 
> values
>   *   to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
>   *   per mode: m/s^2 and micro m/s^2
> - * @ev_cfg:  event config register address
> - * @ev_cfg_ele:  latch

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Jonathan Cameron

On Sun, 20 Aug 2017 10:42:54 -0400
Harinath Nampally  wrote:

[PATCH V3]...
(I think, I've lost track - but this needs to be in the patch title
so we know which is latest.

> This driver supports multiple devices like mma8653,
> mma8652, mma8452, mma8453 and fxls8471. Almost all
> these devices have more than one event.
> 
> Current driver design hardcodes the event specific
> information, so only one event can be supported by this
> driver at any given time.
> Also current design doesn't have the flexibility to
> add more events.
> 
> This patch improves by detaching the event related
> information from chip_info struct,and based on channel
> type and event direction the corresponding event
> configuration registers are picked dynamically.
> Hence both transient and freefall events can be
> handled in read/write callbacks.
> 
> Changes are thoroughly tested on fxls8471 device on imx6UL
> Eval board using iio_event_monitor user space program.
> 
> After this fix both Freefall and Transient events are
> handled by the driver without any conflicts.
> 
> Signed-off-by: Harinath Nampally 
> ---

Should be a version history here to let us know what has changed between each
revision.

Otherwise this looks fine, but I'd like to let it sit for a day or two for
other reviews. 

This just missed my pull request for today anyway.  There may be time for
another pull towards the end of this week, depending on how busy things get.
If not I'm afraid this will now be the next merge window.

Thanks,

Jonathan
>  drivers/iio/accel/mma8452.c | 277 
> 
>  1 file changed, 123 insertions(+), 154 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index eb6e3dc..7bfc257 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -59,7 +59,9 @@
>  #define MMA8452_FF_MT_THS0x17
>  #define  MMA8452_FF_MT_THS_MASK  0x7f
>  #define MMA8452_FF_MT_COUNT  0x18
> +#define MMA8452_FF_MT_CHAN_SHIFT 3
>  #define MMA8452_TRANSIENT_CFG0x1d
> +#define  MMA8452_TRANSIENT_CFG_CHAN(chan)BIT(chan + 1)
>  #define  MMA8452_TRANSIENT_CFG_HPF_BYP   BIT(0)
>  #define  MMA8452_TRANSIENT_CFG_ELE   BIT(4)
>  #define MMA8452_TRANSIENT_SRC0x1e
> @@ -69,6 +71,7 @@
>  #define MMA8452_TRANSIENT_THS0x1f
>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT  0x20
> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1
>  #define MMA8452_CTRL_REG10x2a
>  #define  MMA8452_CTRL_ACTIVE BIT(0)
>  #define  MMA8452_CTRL_DR_MASKGENMASK(5, 3)
> @@ -107,6 +110,42 @@ struct mma8452_data {
>   const struct mma_chip_info *chip_info;
>  };
>  
> + /**
> +  * struct mma8452_event_regs - chip specific data related to events
> +  * @ev_cfg: event config register address
> +  * @ev_src: event source register address
> +  * @ev_ths: event threshold register address
> +  * @ev_ths_mask:mask for the threshold value
> +  * @ev_count:   event count (period) register address
> +  *
> +  * Since not all chips supported by the driver support comparing high pass
> +  * filtered data for events (interrupts), different interrupt sources are
> +  * used for different chips and the relevant registers are included here.
> +  */
> +struct mma8452_event_regs {
> + u8 ev_cfg;
> + u8 ev_src;
> + u8 ev_ths;
> + u8 ev_ths_mask;
> + u8 ev_count;
> +};
> +
> +static const struct mma8452_event_regs ev_regs_accel_falling = {
> + .ev_cfg = MMA8452_FF_MT_CFG,
> + .ev_src = MMA8452_FF_MT_SRC,
> + .ev_ths = MMA8452_FF_MT_THS,
> + .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
> + .ev_count = MMA8452_FF_MT_COUNT
> +};
> +
> +static const struct mma8452_event_regs ev_regs_accel_rising = {
> + .ev_cfg = MMA8452_TRANSIENT_CFG,
> + .ev_src = MMA8452_TRANSIENT_SRC,
> + .ev_ths = MMA8452_TRANSIENT_THS,
> + .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
> + .ev_count = MMA8452_TRANSIENT_COUNT,
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id: WHO_AM_I register's value
> @@ -116,40 +155,12 @@ struct mma8452_data {
>   * @mma_scales:  scale factors for converting register 
> values
>   *   to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
>   *   per mode: m/s^2 and micro m/s^2
> - * @ev_cfg:  event config register address
> - * @ev_cfg_ele:  latch bit in event config register
> - *

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Harinath Nampally

This driver supports multiple devices like mma8653,
mma8652, mma8452, mma8453 and fxls8471. Almost all
these devices have more than one event.

Current driver design hardcodes the event specific
information, so only one event can be supported by this
driver at any given time.
Also current design doesn't have the flexibility to
add more events.

This patch improves by detaching the event related
information from chip_info struct,and based on channel
type and event direction the corresponding event
configuration registers are picked dynamically.
Hence both transient and freefall events can be
handled in read/write callbacks.

Changes are thoroughly tested on fxls8471 device on imx6UL
Eval board using iio_event_monitor user space program.

After this fix both Freefall and Transient events are
handled by the driver without any conflicts.

Signed-off-by: Harinath Nampally 
---
 drivers/iio/accel/mma8452.c | 277 
 1 file changed, 123 insertions(+), 154 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..7bfc257 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -59,7 +59,9 @@
 #define MMA8452_FF_MT_THS  0x17
 #define  MMA8452_FF_MT_THS_MASK0x7f
 #define MMA8452_FF_MT_COUNT0x18
+#define MMA8452_FF_MT_CHAN_SHIFT   3
 #define MMA8452_TRANSIENT_CFG  0x1d
+#define  MMA8452_TRANSIENT_CFG_CHAN(chan)  BIT(chan + 1)
 #define  MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
 #define  MMA8452_TRANSIENT_CFG_ELE BIT(4)
 #define MMA8452_TRANSIENT_SRC  0x1e
@@ -69,6 +71,7 @@
 #define MMA8452_TRANSIENT_THS  0x1f
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
+#define MMA8452_TRANSIENT_CHAN_SHIFT 1
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -107,6 +110,42 @@ struct mma8452_data {
const struct mma_chip_info *chip_info;
 };
 
+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @ev_cfg:   event config register address
+  * @ev_src:   event source register address
+  * @ev_ths:   event threshold register address
+  * @ev_ths_mask:  mask for the threshold value
+  * @ev_count: event count (period) register address
+  *
+  * Since not all chips supported by the driver support comparing high pass
+  * filtered data for events (interrupts), different interrupt sources are
+  * used for different chips and the relevant registers are included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_src;
+   u8 ev_ths;
+   u8 ev_ths_mask;
+   u8 ev_count;
+};
+
+static const struct mma8452_event_regs ev_regs_accel_falling = {
+   .ev_cfg = MMA8452_FF_MT_CFG,
+   .ev_src = MMA8452_FF_MT_SRC,
+   .ev_ths = MMA8452_FF_MT_THS,
+   .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+   .ev_count = MMA8452_FF_MT_COUNT
+};
+
+static const struct mma8452_event_regs ev_regs_accel_rising = {
+   .ev_cfg = MMA8452_TRANSIENT_CFG,
+   .ev_src = MMA8452_TRANSIENT_SRC,
+   .ev_ths = MMA8452_TRANSIENT_THS,
+   .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+   .ev_count = MMA8452_TRANSIENT_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -116,40 +155,12 @@ struct mma8452_data {
  * @mma_scales:scale factors for converting register 
values
  * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
  * per mode: m/s^2 and micro m/s^2
- * @ev_cfg:event config register address
- * @ev_cfg_ele:latch bit in event config register
- * @ev_cfg_chan_shift: number of the bit to enable events in X
- * direction; in event config register
- * @ev_src:event source register address
- * @ev_src_xe: bit in event source register that indicates
- * an event in X direction
- * @ev_src_ye: bit in event source register that indicates
- * an event in Y direction
- * @ev_src_ze: bit in event source register that indicates
- * an event in Z direction
- * @ev_ths:event threshold register address
- * @ev_ths_mask:   mask for the threshold value
- * @ev_count:

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Harinath Nampally

This driver supports multiple devices like mma8653,
mma8652, mma8452, mma8453 and fxls8471. Almost all
these devices have more than one event.

Current driver design hardcodes the event specific
information, so only one event can be supported by this
driver at any given time.
Also current design doesn't have the flexibility to
add more events.

This patch improves by detaching the event related
information from chip_info struct,and based on channel
type and event direction the corresponding event
configuration registers are picked dynamically.
Hence both transient and freefall events can be
handled in read/write callbacks.

Changes are thoroughly tested on fxls8471 device on imx6UL
Eval board using iio_event_monitor user space program.

After this fix both Freefall and Transient events are
handled by the driver without any conflicts.

Signed-off-by: Harinath Nampally 
---
 drivers/iio/accel/mma8452.c | 277 
 1 file changed, 123 insertions(+), 154 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..7bfc257 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -59,7 +59,9 @@
 #define MMA8452_FF_MT_THS  0x17
 #define  MMA8452_FF_MT_THS_MASK0x7f
 #define MMA8452_FF_MT_COUNT0x18
+#define MMA8452_FF_MT_CHAN_SHIFT   3
 #define MMA8452_TRANSIENT_CFG  0x1d
+#define  MMA8452_TRANSIENT_CFG_CHAN(chan)  BIT(chan + 1)
 #define  MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
 #define  MMA8452_TRANSIENT_CFG_ELE BIT(4)
 #define MMA8452_TRANSIENT_SRC  0x1e
@@ -69,6 +71,7 @@
 #define MMA8452_TRANSIENT_THS  0x1f
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
+#define MMA8452_TRANSIENT_CHAN_SHIFT 1
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -107,6 +110,42 @@ struct mma8452_data {
const struct mma_chip_info *chip_info;
 };
 
+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @ev_cfg:   event config register address
+  * @ev_src:   event source register address
+  * @ev_ths:   event threshold register address
+  * @ev_ths_mask:  mask for the threshold value
+  * @ev_count: event count (period) register address
+  *
+  * Since not all chips supported by the driver support comparing high pass
+  * filtered data for events (interrupts), different interrupt sources are
+  * used for different chips and the relevant registers are included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_src;
+   u8 ev_ths;
+   u8 ev_ths_mask;
+   u8 ev_count;
+};
+
+static const struct mma8452_event_regs ev_regs_accel_falling = {
+   .ev_cfg = MMA8452_FF_MT_CFG,
+   .ev_src = MMA8452_FF_MT_SRC,
+   .ev_ths = MMA8452_FF_MT_THS,
+   .ev_ths_mask = MMA8452_FF_MT_THS_MASK,
+   .ev_count = MMA8452_FF_MT_COUNT
+};
+
+static const struct mma8452_event_regs ev_regs_accel_rising = {
+   .ev_cfg = MMA8452_TRANSIENT_CFG,
+   .ev_src = MMA8452_TRANSIENT_SRC,
+   .ev_ths = MMA8452_TRANSIENT_THS,
+   .ev_ths_mask = MMA8452_TRANSIENT_THS_MASK,
+   .ev_count = MMA8452_TRANSIENT_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -116,40 +155,12 @@ struct mma8452_data {
  * @mma_scales:scale factors for converting register 
values
  * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
  * per mode: m/s^2 and micro m/s^2
- * @ev_cfg:event config register address
- * @ev_cfg_ele:latch bit in event config register
- * @ev_cfg_chan_shift: number of the bit to enable events in X
- * direction; in event config register
- * @ev_src:event source register address
- * @ev_src_xe: bit in event source register that indicates
- * an event in X direction
- * @ev_src_ye: bit in event source register that indicates
- * an event in Y direction
- * @ev_src_ze: bit in event source register that indicates
- * an event in Z direction
- * @ev_ths:event threshold register address
- * @ev_ths_mask:   mask for the threshold value
- * @ev_count:  event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Jonathan Cameron

On Fri, 18 Aug 2017 22:08:10 -0400
Harinath Nampally  wrote:

> This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 
> and
> fxls8471. Almost all these devices have more than one event. Current driver 
> design
> hardcodes the event specific information, so only one event can be supported 
> by this
> driver and current design doesn't have the flexibility to add more events.
> 
> This patch improves by detaching the event related information from chip_info 
> struct,
> and based on channel type and event direction the corresponding event 
> configuration registers
> are picked dynamically. Hence both transient and freefall events can be 
> handled in read/write callbacks.
Please keep description to sub 75 characters (it tends to get indented by some
tools so best to leave it a bit shorter than the 80 char limit).
> 
> Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using 
> iio_event_monitor user space program.
> 
> After this fix both Freefall and Transient events are handled by the driver 
> without any conflicts.
> 
> Changes since v2 -> v3
> -Fix typo in commit message
> -Replace word 'Bugfix' with 'Improvements'
> -Describe more accurate commit message
> -Replace breaks with returns
> -Initialise transient event threshold mask
> -Remove unrelated change of IIO_ACCEL channel type check in read/write event 
> callbacks
> 
> Changes since v1 -> v2
> -Fix indentations
> -Remove unused fields in mma8452_event_regs struct
> -Remove redundant return statement
> -Remove unrelated changes like checkpatch.pl warning fixes
> 
> Signed-off-by: Harinath Nampally 

For some reason this appears to be a reply to an earlier series.  Please
always start a fresh thread for a fresh version of a patch.  I almost
missed it entirely.

Anyhow, one suggestion inline.  We never tend to like functions that fill
in what is basically constant data.  If it's constant then make it so as
there are lots of advantages (including XIP).

Jonathan

> ---
>  drivers/iio/accel/mma8452.c | 272 
> +++-
>  1 file changed, 118 insertions(+), 154 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index eb6e3dc..8de9928 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -59,7 +59,9 @@
>  #define MMA8452_FF_MT_THS0x17
>  #define  MMA8452_FF_MT_THS_MASK  0x7f
>  #define MMA8452_FF_MT_COUNT  0x18
> +#define MMA8452_FF_MT_CHAN_SHIFT 3
>  #define MMA8452_TRANSIENT_CFG0x1d
> +#define  MMA8452_TRANSIENT_CFG_CHAN(chan)BIT(chan + 1)
>  #define  MMA8452_TRANSIENT_CFG_HPF_BYP   BIT(0)
>  #define  MMA8452_TRANSIENT_CFG_ELE   BIT(4)
>  #define MMA8452_TRANSIENT_SRC0x1e
> @@ -69,6 +71,7 @@
>  #define MMA8452_TRANSIENT_THS0x1f
>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT  0x20
> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1
>  #define MMA8452_CTRL_REG10x2a
>  #define  MMA8452_CTRL_ACTIVE BIT(0)
>  #define  MMA8452_CTRL_DR_MASKGENMASK(5, 3)
> @@ -107,6 +110,26 @@ struct mma8452_data {
>   const struct mma_chip_info *chip_info;
>  };
>  
> + /**
> +  * struct mma8452_event_regs - chip specific data related to events
> +  * @ev_cfg: event config register address
> +  * @ev_src: event source register address
> +  * @ev_ths: event threshold register address
> +  * @ev_ths_mask:mask for the threshold value
> +  * @ev_count:   event count (period) register address
> +  *
> +  * Since not all chips supported by the driver support comparing high pass
> +  * filtered data for events (interrupts), different interrupt sources are
> +  * used for different chips and the relevant registers are included here.
> +  */
> +struct mma8452_event_regs {
> + u8 ev_cfg;
> + u8 ev_src;
> + u8 ev_ths;
> + u8 ev_ths_mask;
> + u8 ev_count;
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id: WHO_AM_I register's value
> @@ -116,40 +139,12 @@ struct mma8452_data {
>   * @mma_scales:  scale factors for converting register 
> values
>   *   to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
>   *   per mode: m/s^2 and micro m/s^2
> - * @ev_cfg:  event config register address
> - * @ev_cfg_ele:  latch bit in event config register
> - * @ev_cfg_chan_shift:   number of the bit to enable events in X
> - *   direction; in event config register
> - * @ev_src:  event source register

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-20 Thread Jonathan Cameron

On Fri, 18 Aug 2017 22:08:10 -0400
Harinath Nampally  wrote:

> This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 
> and
> fxls8471. Almost all these devices have more than one event. Current driver 
> design
> hardcodes the event specific information, so only one event can be supported 
> by this
> driver and current design doesn't have the flexibility to add more events.
> 
> This patch improves by detaching the event related information from chip_info 
> struct,
> and based on channel type and event direction the corresponding event 
> configuration registers
> are picked dynamically. Hence both transient and freefall events can be 
> handled in read/write callbacks.
Please keep description to sub 75 characters (it tends to get indented by some
tools so best to leave it a bit shorter than the 80 char limit).
> 
> Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using 
> iio_event_monitor user space program.
> 
> After this fix both Freefall and Transient events are handled by the driver 
> without any conflicts.
> 
> Changes since v2 -> v3
> -Fix typo in commit message
> -Replace word 'Bugfix' with 'Improvements'
> -Describe more accurate commit message
> -Replace breaks with returns
> -Initialise transient event threshold mask
> -Remove unrelated change of IIO_ACCEL channel type check in read/write event 
> callbacks
> 
> Changes since v1 -> v2
> -Fix indentations
> -Remove unused fields in mma8452_event_regs struct
> -Remove redundant return statement
> -Remove unrelated changes like checkpatch.pl warning fixes
> 
> Signed-off-by: Harinath Nampally 

For some reason this appears to be a reply to an earlier series.  Please
always start a fresh thread for a fresh version of a patch.  I almost
missed it entirely.

Anyhow, one suggestion inline.  We never tend to like functions that fill
in what is basically constant data.  If it's constant then make it so as
there are lots of advantages (including XIP).

Jonathan

> ---
>  drivers/iio/accel/mma8452.c | 272 
> +++-
>  1 file changed, 118 insertions(+), 154 deletions(-)
> 
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index eb6e3dc..8de9928 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -59,7 +59,9 @@
>  #define MMA8452_FF_MT_THS0x17
>  #define  MMA8452_FF_MT_THS_MASK  0x7f
>  #define MMA8452_FF_MT_COUNT  0x18
> +#define MMA8452_FF_MT_CHAN_SHIFT 3
>  #define MMA8452_TRANSIENT_CFG0x1d
> +#define  MMA8452_TRANSIENT_CFG_CHAN(chan)BIT(chan + 1)
>  #define  MMA8452_TRANSIENT_CFG_HPF_BYP   BIT(0)
>  #define  MMA8452_TRANSIENT_CFG_ELE   BIT(4)
>  #define MMA8452_TRANSIENT_SRC0x1e
> @@ -69,6 +71,7 @@
>  #define MMA8452_TRANSIENT_THS0x1f
>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT  0x20
> +#define MMA8452_TRANSIENT_CHAN_SHIFT 1
>  #define MMA8452_CTRL_REG10x2a
>  #define  MMA8452_CTRL_ACTIVE BIT(0)
>  #define  MMA8452_CTRL_DR_MASKGENMASK(5, 3)
> @@ -107,6 +110,26 @@ struct mma8452_data {
>   const struct mma_chip_info *chip_info;
>  };
>  
> + /**
> +  * struct mma8452_event_regs - chip specific data related to events
> +  * @ev_cfg: event config register address
> +  * @ev_src: event source register address
> +  * @ev_ths: event threshold register address
> +  * @ev_ths_mask:mask for the threshold value
> +  * @ev_count:   event count (period) register address
> +  *
> +  * Since not all chips supported by the driver support comparing high pass
> +  * filtered data for events (interrupts), different interrupt sources are
> +  * used for different chips and the relevant registers are included here.
> +  */
> +struct mma8452_event_regs {
> + u8 ev_cfg;
> + u8 ev_src;
> + u8 ev_ths;
> + u8 ev_ths_mask;
> + u8 ev_count;
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id: WHO_AM_I register's value
> @@ -116,40 +139,12 @@ struct mma8452_data {
>   * @mma_scales:  scale factors for converting register 
> values
>   *   to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
>   *   per mode: m/s^2 and micro m/s^2
> - * @ev_cfg:  event config register address
> - * @ev_cfg_ele:  latch bit in event config register
> - * @ev_cfg_chan_shift:   number of the bit to enable events in X
> - *   direction; in event config register
> - * @ev_src:  event source register address
> - * @ev_src_xe:   bit

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-18 Thread Harinath Nampally

This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 
and
fxls8471. Almost all these devices have more than one event. Current driver 
design
hardcodes the event specific information, so only one event can be supported by 
this
driver and current design doesn't have the flexibility to add more events.

This patch improves by detaching the event related information from chip_info 
struct,
and based on channel type and event direction the corresponding event 
configuration registers
are picked dynamically. Hence both transient and freefall events can be handled 
in read/write callbacks.

Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using 
iio_event_monitor user space program.

After this fix both Freefall and Transient events are handled by the driver 
without any conflicts.

Changes since v2 -> v3
-Fix typo in commit message
-Replace word 'Bugfix' with 'Improvements'
-Describe more accurate commit message
-Replace breaks with returns
-Initialise transient event threshold mask
-Remove unrelated change of IIO_ACCEL channel type check in read/write event 
callbacks

Changes since v1 -> v2
-Fix indentations
-Remove unused fields in mma8452_event_regs struct
-Remove redundant return statement
-Remove unrelated changes like checkpatch.pl warning fixes

Signed-off-by: Harinath Nampally 
---
 drivers/iio/accel/mma8452.c | 272 +++-
 1 file changed, 118 insertions(+), 154 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..8de9928 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -59,7 +59,9 @@
 #define MMA8452_FF_MT_THS  0x17
 #define  MMA8452_FF_MT_THS_MASK0x7f
 #define MMA8452_FF_MT_COUNT0x18
+#define MMA8452_FF_MT_CHAN_SHIFT   3
 #define MMA8452_TRANSIENT_CFG  0x1d
+#define  MMA8452_TRANSIENT_CFG_CHAN(chan)  BIT(chan + 1)
 #define  MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
 #define  MMA8452_TRANSIENT_CFG_ELE BIT(4)
 #define MMA8452_TRANSIENT_SRC  0x1e
@@ -69,6 +71,7 @@
 #define MMA8452_TRANSIENT_THS  0x1f
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
+#define MMA8452_TRANSIENT_CHAN_SHIFT 1
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -107,6 +110,26 @@ struct mma8452_data {
const struct mma_chip_info *chip_info;
 };
 
+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @ev_cfg:   event config register address
+  * @ev_src:   event source register address
+  * @ev_ths:   event threshold register address
+  * @ev_ths_mask:  mask for the threshold value
+  * @ev_count: event count (period) register address
+  *
+  * Since not all chips supported by the driver support comparing high pass
+  * filtered data for events (interrupts), different interrupt sources are
+  * used for different chips and the relevant registers are included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_src;
+   u8 ev_ths;
+   u8 ev_ths_mask;
+   u8 ev_count;
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -116,40 +139,12 @@ struct mma8452_data {
  * @mma_scales:scale factors for converting register 
values
  * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
  * per mode: m/s^2 and micro m/s^2
- * @ev_cfg:event config register address
- * @ev_cfg_ele:latch bit in event config register
- * @ev_cfg_chan_shift: number of the bit to enable events in X
- * direction; in event config register
- * @ev_src:event source register address
- * @ev_src_xe: bit in event source register that indicates
- * an event in X direction
- * @ev_src_ye: bit in event source register that indicates
- * an event in Y direction
- * @ev_src_ze: bit in event source register that indicates
- * an event in Z direction
- * @ev_ths:event threshold register address
- * @ev_ths_mask:   mask for the threshold value
- * @ev_count:  event count (period) register address
- *
- * Since not all chips supported by the driver support comparing high pass
- * filtered data for events (interrupts), different interrupt sources are
- * used for different chips and the

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

2017-08-18 Thread Harinath Nampally

This driver supports multiple devices like mma8653, mma8652, mma8452, mma8453 
and
fxls8471. Almost all these devices have more than one event. Current driver 
design
hardcodes the event specific information, so only one event can be supported by 
this
driver and current design doesn't have the flexibility to add more events.

This patch improves by detaching the event related information from chip_info 
struct,
and based on channel type and event direction the corresponding event 
configuration registers
are picked dynamically. Hence both transient and freefall events can be handled 
in read/write callbacks.

Changes are thoroughly tested on fxls8471 device on imx6UL Eval board using 
iio_event_monitor user space program.

After this fix both Freefall and Transient events are handled by the driver 
without any conflicts.

Changes since v2 -> v3
-Fix typo in commit message
-Replace word 'Bugfix' with 'Improvements'
-Describe more accurate commit message
-Replace breaks with returns
-Initialise transient event threshold mask
-Remove unrelated change of IIO_ACCEL channel type check in read/write event 
callbacks

Changes since v1 -> v2
-Fix indentations
-Remove unused fields in mma8452_event_regs struct
-Remove redundant return statement
-Remove unrelated changes like checkpatch.pl warning fixes

Signed-off-by: Harinath Nampally 
---
 drivers/iio/accel/mma8452.c | 272 +++-
 1 file changed, 118 insertions(+), 154 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..8de9928 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -59,7 +59,9 @@
 #define MMA8452_FF_MT_THS  0x17
 #define  MMA8452_FF_MT_THS_MASK0x7f
 #define MMA8452_FF_MT_COUNT0x18
+#define MMA8452_FF_MT_CHAN_SHIFT   3
 #define MMA8452_TRANSIENT_CFG  0x1d
+#define  MMA8452_TRANSIENT_CFG_CHAN(chan)  BIT(chan + 1)
 #define  MMA8452_TRANSIENT_CFG_HPF_BYP BIT(0)
 #define  MMA8452_TRANSIENT_CFG_ELE BIT(4)
 #define MMA8452_TRANSIENT_SRC  0x1e
@@ -69,6 +71,7 @@
 #define MMA8452_TRANSIENT_THS  0x1f
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
+#define MMA8452_TRANSIENT_CHAN_SHIFT 1
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -107,6 +110,26 @@ struct mma8452_data {
const struct mma_chip_info *chip_info;
 };
 
+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @ev_cfg:   event config register address
+  * @ev_src:   event source register address
+  * @ev_ths:   event threshold register address
+  * @ev_ths_mask:  mask for the threshold value
+  * @ev_count: event count (period) register address
+  *
+  * Since not all chips supported by the driver support comparing high pass
+  * filtered data for events (interrupts), different interrupt sources are
+  * used for different chips and the relevant registers are included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_src;
+   u8 ev_ths;
+   u8 ev_ths_mask;
+   u8 ev_count;
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -116,40 +139,12 @@ struct mma8452_data {
  * @mma_scales:scale factors for converting register 
values
  * to m/s^2; 3 modes: 2g, 4g, 8g; 2 integers
  * per mode: m/s^2 and micro m/s^2
- * @ev_cfg:event config register address
- * @ev_cfg_ele:latch bit in event config register
- * @ev_cfg_chan_shift: number of the bit to enable events in X
- * direction; in event config register
- * @ev_src:event source register address
- * @ev_src_xe: bit in event source register that indicates
- * an event in X direction
- * @ev_src_ye: bit in event source register that indicates
- * an event in Y direction
- * @ev_src_ze: bit in event source register that indicates
- * an event in Z direction
- * @ev_ths:event threshold register address
- * @ev_ths_mask:   mask for the threshold value
- * @ev_count:  event count (period) register address
- *
- * Since not all chips supported by the driver support comparing high pass
- * filtered data for events (interrupts), different interrupt sources are
- * used for different chips and the relevant registers are

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

Re: [PATCH] iio: accel: mma8452: code improvements to handle more than one event

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

[PATCH] iio: accel: mma8452: code improvements to handle more than one event

10 matches

Site Navigation

Mail list logo

Footer information