Re: [PATCH] iio: mma8452: Fix ignoring MMA8452_INT_DRDY

[harinath Nampally]

On 2018-06-07 20:52, Leonard Crestez wrote:
> Interrupts are ignored if no event bit is set in the status status
> register and this breaks the buffer interface. No data is shown when
> running "iio_generic_buffer -n mma8451 -a" and interrupt counts go
> crazy.
>
> Fix by not returning IRQ_NONE if DRDY is set.
>
> Fixes: 605f72de137a ("iio: accel: mma8452: improvements to handle
> multiple events")
>
> Signed-off-by: Leonard Crestez 
Thanks for catching it. Looks good to me.
Acked-by: Harinath Nampally 

Thanks,
Harinath


On Wed, Jun 20, 2018 at 5:15 AM, Martin Kepplinger  wrote:
> On 2018-06-07 20:52, Leonard Crestez wrote:
>> Interrupts are ignored if no event bit is set in the status status
>> register and this breaks the buffer interface. No data is shown when
>> running "iio_generic_buffer -n mma8451 -a" and interrupt counts go
>> crazy.
>>
>> Fix by not returning IRQ_NONE if DRDY is set.
>>
>> Fixes: 605f72de137a ("iio: accel: mma8452: improvements to handle
>> multiple events")
>>
>> Signed-off-by: Leonard Crestez 
>
> At least this does no harm to events. So if this solves your problem:
>
> Acked-by: Martin Kepplinger 
>
> thanks,
>
> martin
>
>>
>> ---
>>  drivers/iio/accel/mma8452.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Perhaps this whole early-exit check could be dropped? It is not clear
>> how it helps.
>>
>> If for some models we want to ignore unsupported events then maybe this
>> should be checked for each individual bit. Instead of
>>
>> if (src & MMA8452_INT_FF_MT) {
>>
>> Check for:
>>
>> if ((src & MMA8452_INT_FF_MT) && (data->chip_info->enabled_events & 
>> MMA8452_INT_FF_MT))
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 7a2da7f9d4dc..5485b35fe553 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1032,11 +1032,11 @@ static irqreturn_t mma8452_interrupt(int irq, void 
>> *p)
>>
>>   src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
>>   if (src < 0)
>>   return IRQ_NONE;
>>
>> - if (!(src & data->chip_info->enabled_events))
>> + if (!(src & (data->chip_info->enabled_events | MMA8452_INT_DRDY)))
>>   return IRQ_NONE;
>>
>>   if (src & MMA8452_INT_DRDY) {
>>   iio_trigger_poll_chained(indio_dev->trig);
>>   ret = IRQ_HANDLED;
>>
>

Re: [PATCH] iio: mma8452: Fix ignoring MMA8452_INT_DRDY

[harinath Nampally]

On 2018-06-07 20:52, Leonard Crestez wrote:
> Interrupts are ignored if no event bit is set in the status status
> register and this breaks the buffer interface. No data is shown when
> running "iio_generic_buffer -n mma8451 -a" and interrupt counts go
> crazy.
>
> Fix by not returning IRQ_NONE if DRDY is set.
>
> Fixes: 605f72de137a ("iio: accel: mma8452: improvements to handle
> multiple events")
>
> Signed-off-by: Leonard Crestez 
Thanks for catching it. Looks good to me.
Acked-by: Harinath Nampally 

Thanks,
Harinath


On Wed, Jun 20, 2018 at 5:15 AM, Martin Kepplinger  wrote:
> On 2018-06-07 20:52, Leonard Crestez wrote:
>> Interrupts are ignored if no event bit is set in the status status
>> register and this breaks the buffer interface. No data is shown when
>> running "iio_generic_buffer -n mma8451 -a" and interrupt counts go
>> crazy.
>>
>> Fix by not returning IRQ_NONE if DRDY is set.
>>
>> Fixes: 605f72de137a ("iio: accel: mma8452: improvements to handle
>> multiple events")
>>
>> Signed-off-by: Leonard Crestez 
>
> At least this does no harm to events. So if this solves your problem:
>
> Acked-by: Martin Kepplinger 
>
> thanks,
>
> martin
>
>>
>> ---
>>  drivers/iio/accel/mma8452.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Perhaps this whole early-exit check could be dropped? It is not clear
>> how it helps.
>>
>> If for some models we want to ignore unsupported events then maybe this
>> should be checked for each individual bit. Instead of
>>
>> if (src & MMA8452_INT_FF_MT) {
>>
>> Check for:
>>
>> if ((src & MMA8452_INT_FF_MT) && (data->chip_info->enabled_events & 
>> MMA8452_INT_FF_MT))
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 7a2da7f9d4dc..5485b35fe553 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -1032,11 +1032,11 @@ static irqreturn_t mma8452_interrupt(int irq, void 
>> *p)
>>
>>   src = i2c_smbus_read_byte_data(data->client, MMA8452_INT_SRC);
>>   if (src < 0)
>>   return IRQ_NONE;
>>
>> - if (!(src & data->chip_info->enabled_events))
>> + if (!(src & (data->chip_info->enabled_events | MMA8452_INT_DRDY)))
>>   return IRQ_NONE;
>>
>>   if (src & MMA8452_INT_DRDY) {
>>   iio_trigger_poll_chained(indio_dev->trig);
>>   ret = IRQ_HANDLED;
>>
>

Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[harinath Nampally]

> > This patch adds following related changes:
> > - defines pulse event related registers
> > - enables and handles single pulse interrupt for fxls8471
> > - handles IIO_EV_DIR_EITHER in read/write callbacks (because
> >   event direction for pulse is either rising or falling)
> > - configures read/write event value for pulse latency register
> >   using IIO_EV_INFO_HYSTERESIS
> > - adds multiple events like pulse and tranient event spec
> >   as elements of event_spec array named 'mma8452_accel_events'
> >
> > Except mma8653 chip all other chips like mma845x and
> > fxls8471 have single tap detection feature.
> > Tested thoroughly using iio_event_monitor application on
> > imx6ul-evk board which has fxls8471.
> >
> > Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>
> The use of an either direction magnitude event is misleading.
> Tap detection requires a timed sequence of events - a rapid
> increase in acceleration followed by a rapid decrease.
Yes I agree.

> So.. I'd propose (and I want as much feedback on this as possible)
>
> IIO_EV_TYPE_TAP
>
> Then abuse iio_event_direction to specify single or double (the concept
> of direction doesn't really apply to these in that sense so we'd always
> have them set to none).
Sure, I think this would be very useful as there are lot of modern
accelerometers
with tap event support.

Thanks,
Harinath

On Sun, Nov 19, 2017 at 9:47 AM, Jonathan Cameron <ji...@kernel.org> wrote:
> On Wed,  8 Nov 2017 22:12:57 -0500
> Harinath Nampally <harinath...@gmail.com> wrote:
>
>> This patch adds following related changes:
>> - defines pulse event related registers
>> - enables and handles single pulse interrupt for fxls8471
>> - handles IIO_EV_DIR_EITHER in read/write callbacks (because
>>   event direction for pulse is either rising or falling)
>> - configures read/write event value for pulse latency register
>>   using IIO_EV_INFO_HYSTERESIS
>> - adds multiple events like pulse and tranient event spec
>>   as elements of event_spec array named 'mma8452_accel_events'
>>
>> Except mma8653 chip all other chips like mma845x and
>> fxls8471 have single tap detection feature.
>> Tested thoroughly using iio_event_monitor application on
>> imx6ul-evk board which has fxls8471.
>>
>> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>
> The use of an either direction magnitude event is misleading.
> Tap detection requires a timed sequence of events - a rapid
> increase in acceleration followed by a rapid decrease.
>
> I don't think we can fit tap detection into the existing
> event types (which is annoying but there we are).
>
> So.. I'd propose (and I want as much feedback on this as possible)
>
> IIO_EV_TYPE_TAP
>
> Then abuse iio_event_direction to specify single or double (the concept
> of direction doesn't really apply to these in that sense so we'd always
> have them set to none).
>
> The alternative would be to full out for an abstract representation of
> these events (like step detection) and define a channel type
> IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but
> given we have directional taps we would need the modifier for that.
> Hence no means of describing whether it is a single or double tap
> short of burning channel taps.  I'm also not sure we want to
> remove the association with a particular sensor channel.
>
> Hence I think I prefer option 1 but would like other's input on this...
>
> Jonathan
>
>> ---
>>  drivers/iio/accel/mma8452.c | 156 
>> ++--
>>  1 file changed, 151 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 43c3a6b..36f1b56 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -72,6 +72,19 @@
>>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>>  #define MMA8452_TRANSIENT_COUNT  0x20
>>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
>> +#define MMA8452_PULSE_CFG0x21
>> +#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2)
>> +#define MMA8452_PULSE_CFG_ELEBIT(6)
>> +#define MMA8452_PULSE_SRC0x22
>> +#define MMA8452_PULSE_SRC_XPULSE BIT(4)
>> +#define MMA8452_PULSE_SRC_YPULSE BIT(5)
>> +#define MMA8452_PULSE_SRC_ZPULSE BIT(6)
>> +#define MMA8452_PULSE_THS0x23
>> +#define MMA8452_PULSE_THS_MASK   GENMASK(6, 0)
>> +#define M

Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[harinath Nampally]

> > This patch adds following related changes:
> > - defines pulse event related registers
> > - enables and handles single pulse interrupt for fxls8471
> > - handles IIO_EV_DIR_EITHER in read/write callbacks (because
> >   event direction for pulse is either rising or falling)
> > - configures read/write event value for pulse latency register
> >   using IIO_EV_INFO_HYSTERESIS
> > - adds multiple events like pulse and tranient event spec
> >   as elements of event_spec array named 'mma8452_accel_events'
> >
> > Except mma8653 chip all other chips like mma845x and
> > fxls8471 have single tap detection feature.
> > Tested thoroughly using iio_event_monitor application on
> > imx6ul-evk board which has fxls8471.
> >
> > Signed-off-by: Harinath Nampally 
>
> The use of an either direction magnitude event is misleading.
> Tap detection requires a timed sequence of events - a rapid
> increase in acceleration followed by a rapid decrease.
Yes I agree.

> So.. I'd propose (and I want as much feedback on this as possible)
>
> IIO_EV_TYPE_TAP
>
> Then abuse iio_event_direction to specify single or double (the concept
> of direction doesn't really apply to these in that sense so we'd always
> have them set to none).
Sure, I think this would be very useful as there are lot of modern
accelerometers
with tap event support.

Thanks,
Harinath

On Sun, Nov 19, 2017 at 9:47 AM, Jonathan Cameron  wrote:
> On Wed,  8 Nov 2017 22:12:57 -0500
> Harinath Nampally  wrote:
>
>> This patch adds following related changes:
>> - defines pulse event related registers
>> - enables and handles single pulse interrupt for fxls8471
>> - handles IIO_EV_DIR_EITHER in read/write callbacks (because
>>   event direction for pulse is either rising or falling)
>> - configures read/write event value for pulse latency register
>>   using IIO_EV_INFO_HYSTERESIS
>> - adds multiple events like pulse and tranient event spec
>>   as elements of event_spec array named 'mma8452_accel_events'
>>
>> Except mma8653 chip all other chips like mma845x and
>> fxls8471 have single tap detection feature.
>> Tested thoroughly using iio_event_monitor application on
>> imx6ul-evk board which has fxls8471.
>>
>> Signed-off-by: Harinath Nampally 
>
> The use of an either direction magnitude event is misleading.
> Tap detection requires a timed sequence of events - a rapid
> increase in acceleration followed by a rapid decrease.
>
> I don't think we can fit tap detection into the existing
> event types (which is annoying but there we are).
>
> So.. I'd propose (and I want as much feedback on this as possible)
>
> IIO_EV_TYPE_TAP
>
> Then abuse iio_event_direction to specify single or double (the concept
> of direction doesn't really apply to these in that sense so we'd always
> have them set to none).
>
> The alternative would be to full out for an abstract representation of
> these events (like step detection) and define a channel type
> IIO_TAP then detect a change in the number of taps (IIO_DIR_NONE), but
> given we have directional taps we would need the modifier for that.
> Hence no means of describing whether it is a single or double tap
> short of burning channel taps.  I'm also not sure we want to
> remove the association with a particular sensor channel.
>
> Hence I think I prefer option 1 but would like other's input on this...
>
> Jonathan
>
>> ---
>>  drivers/iio/accel/mma8452.c | 156 
>> ++--
>>  1 file changed, 151 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 43c3a6b..36f1b56 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -72,6 +72,19 @@
>>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>>  #define MMA8452_TRANSIENT_COUNT  0x20
>>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
>> +#define MMA8452_PULSE_CFG0x21
>> +#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2)
>> +#define MMA8452_PULSE_CFG_ELEBIT(6)
>> +#define MMA8452_PULSE_SRC0x22
>> +#define MMA8452_PULSE_SRC_XPULSE BIT(4)
>> +#define MMA8452_PULSE_SRC_YPULSE BIT(5)
>> +#define MMA8452_PULSE_SRC_ZPULSE BIT(6)
>> +#define MMA8452_PULSE_THS0x23
>> +#define MMA8452_PULSE_THS_MASK   GENMASK(6, 0)
>> +#define MMA8452_PULSE_COUNT  0x26
>> +#define MMA8452_PULSE_CHAN_SHIFT 2
>> +#define MMA8452

Re: [PATCH v2] iio: mma8452: replace license description with SPDX specifier

[harinath Nampally]

> This replaces the custom license information text with the appropriate
> SPDX identifier. While the information here stays the same, it is easier
> to read.
> Signed-off-by: Martin Kepplinger <mart...@posteo.de>
> Acked-by: Peter Meerwald-Stadler <pme...@pmeerw.net>
Acked-by: Harinath Nampally <harinath...@gmail.com>

On Sat, Nov 18, 2017 at 11:29 AM, Philippe Ombredanne
<pombreda...@nexb.com> wrote:
> On Sat, Nov 18, 2017 at 4:53 PM, Jonathan Cameron <ji...@kernel.org> wrote:
>> On Sat, 18 Nov 2017 10:10:11 +0100
>> Martin Kepplinger <mart...@posteo.de> wrote:
>>
>>> This replaces the custom license information text with the appropriate
>>> SPDX identifier. While the information here stays the same, it is easier
>>> to read.
>>>
>>> Signed-off-by: Martin Kepplinger <mart...@posteo.de>
>>> Acked-by: Peter Meerwald-Stadler <pme...@pmeerw.net>
>>
>> I'm not 100% sure the intent of the SPDX work is to remove
>> existing licence text.  So far the big sets have only been
>> adding tags to files missing their licenses entirely...
>>
>> Anyone found any specific guidance on this?
>
> Jonathan:
> you might want to check the doc patches from tglx [1] as well as
> several related patches from greg k-h such as these  [2] and his
> initial pull [3]
>
> To get a lot of details you can check all the recent SPDX-related posts too 
> [4]
>
> [1] https://marc.info/?l=linux-kernel=151051532322831=2
> [2] https://marc.info/?l=linux-kernel=151068111802610=2
> [3] https://marc.info/?l=linux-kernel=150963579219623=2
> [4] https://marc.info/?l=linux-kernel=2=1=spdx=b
> --
> Cordially
> Philippe Ombredanne

Re: [PATCH v2] iio: mma8452: replace license description with SPDX specifier

[harinath Nampally]

> This replaces the custom license information text with the appropriate
> SPDX identifier. While the information here stays the same, it is easier
> to read.
> Signed-off-by: Martin Kepplinger 
> Acked-by: Peter Meerwald-Stadler 
Acked-by: Harinath Nampally 

On Sat, Nov 18, 2017 at 11:29 AM, Philippe Ombredanne
 wrote:
> On Sat, Nov 18, 2017 at 4:53 PM, Jonathan Cameron  wrote:
>> On Sat, 18 Nov 2017 10:10:11 +0100
>> Martin Kepplinger  wrote:
>>
>>> This replaces the custom license information text with the appropriate
>>> SPDX identifier. While the information here stays the same, it is easier
>>> to read.
>>>
>>> Signed-off-by: Martin Kepplinger 
>>> Acked-by: Peter Meerwald-Stadler 
>>
>> I'm not 100% sure the intent of the SPDX work is to remove
>> existing licence text.  So far the big sets have only been
>> adding tags to files missing their licenses entirely...
>>
>> Anyone found any specific guidance on this?
>
> Jonathan:
> you might want to check the doc patches from tglx [1] as well as
> several related patches from greg k-h such as these  [2] and his
> initial pull [3]
>
> To get a lot of details you can check all the recent SPDX-related posts too 
> [4]
>
> [1] https://marc.info/?l=linux-kernel=151051532322831=2
> [2] https://marc.info/?l=linux-kernel=151068111802610=2
> [3] https://marc.info/?l=linux-kernel=150963579219623=2
> [4] https://marc.info/?l=linux-kernel=2=1=spdx=b
> --
> Cordially
> Philippe Ombredanne

Re: [PATCH] iio: mma8452: add power_mode sysfs configuration

[harinath Nampally]

Hi Martin,

> But given your concerns, I would strip down this patch to only offer the
> already documented "low_noise" and "low_power" modes. It wouldn't be
> worth it to extend the ABI just because of this!
OK then we can map 'low_noise' to high resolution mode. But I am afraid
I can't test the functionality because I don't have proper instruments to
measure the current draw(in microAmps) accurately.

> I would like "oversampling" more than this "power_mode" too. For this
> driver it would be far more complicated to implement though. I doubt
> that it'll be done. power_mode is basically already there implicitely,
> and given that there *is* the ABI, we could offer it for free.
I think 'oversampling' is already implemented, as I see
'case IIO_CHAN_INFO_OVERSAMPLING_RATIO:'
being handled which is basically setting the all 4 different power modes.
If we also add 'power_mode', I think it would be like having two
different user interfaces for
same functionality. So I don't see much of value adding 'power_mode' as well.
Please correct me if I am wrong.

Thanks,
Harinath

On Sun, Nov 12, 2017 at 7:28 AM, Martin Kepplinger  wrote:
> On 2017-11-11 01:33, Jonathan Cameron wrote:
>> On Mon, 6 Nov 2017 08:19:58 +0100
>> Martin Kepplinger  wrote:
>>
>>> This adds the power_mode sysfs interface to the device as documented in
>>> sysfs-bus-iio.
>>>
>>> ---
>>>
>>> Note that I explicitely don't sign off on this.
>>>
>>> This is a starting point for anybody who can test it and check for correct
>>> API usage, and ABI correctness, as documented in 
>>> Documentation/ABI/testing/sys-bus-iio
>>> (grep it for "power_mode"). The ABI doc probably would need an addition
>>> too, if the 4 power modes here seem generally useful (there are only
>>>  2 listed there)!
>>>
>>> So, if you can test this, feel free to set up a proper patch or
>>> two, and I'm happy to review.
>>>
>>> Please note that this patch is quite old. It really should be that simple
>>> as far as my understanding back then. We always list the available 
>>> frequencies
>>> of the given power mode we are in, for example, already, and everything
>>> basically is in place except for the user interface.
>>
>> Hmm. A lot of devices support something along these lines.  The issue
>> has always been - how is userspace to figure out what to do with it?
>> It's all very vague...
>>
>> Funnily enough - this used to be really common, but is becoming less so
>> now - presumably because no one was using it much (or maybe I am reading
>> too much into that ;)
>>
>> Now the question is whether it can be tied to better defined things?
>>
>> Here low noise restricts the range to 4g.  Issue is that we don't actually
>> have writeable _available attributes (which correspond to range in this 
>> case).
>>
>
> Does it? Isn't it merely less oversampling.
>
>> Low power mode... This one is apparently oversampling.  If possible support
>> it as that as we have well defined interfaces for that.
>>
>> Jonathan.
>
> Ah, I remember; the oversampling settings was actually a reason why I
> hadn't submitted the patch :) The oversampling API would definitely be
> more accurate.
>
> I would like "oversampling" more than this "power_mode" too. For this
> driver it would be far more complicated to implement though. I doubt
> that it'll be done. power_mode is basically already there implicitely,
> and given that there *is* the ABI, we could offer it for free.
>
> But given your concerns, I would strip down this patch to only offer the
> already documented "low_noise" and "low_power" modes. It wouldn't be
> worth it to extend the ABI just because of this!
>
> Users would have a simple switch if they don't really *want* to know the
> details. I think it can be useful to just say "I don't care about power
> consuption. Be as accurate as possible" or "I just want this think to
> work. Use a little power as possible." Sure it's vage, but would it be
> useless?

Re: [PATCH] iio: mma8452: add power_mode sysfs configuration

[harinath Nampally]

Hi Martin,

> But given your concerns, I would strip down this patch to only offer the
> already documented "low_noise" and "low_power" modes. It wouldn't be
> worth it to extend the ABI just because of this!
OK then we can map 'low_noise' to high resolution mode. But I am afraid
I can't test the functionality because I don't have proper instruments to
measure the current draw(in microAmps) accurately.

> I would like "oversampling" more than this "power_mode" too. For this
> driver it would be far more complicated to implement though. I doubt
> that it'll be done. power_mode is basically already there implicitely,
> and given that there *is* the ABI, we could offer it for free.
I think 'oversampling' is already implemented, as I see
'case IIO_CHAN_INFO_OVERSAMPLING_RATIO:'
being handled which is basically setting the all 4 different power modes.
If we also add 'power_mode', I think it would be like having two
different user interfaces for
same functionality. So I don't see much of value adding 'power_mode' as well.
Please correct me if I am wrong.

Thanks,
Harinath

On Sun, Nov 12, 2017 at 7:28 AM, Martin Kepplinger  wrote:
> On 2017-11-11 01:33, Jonathan Cameron wrote:
>> On Mon, 6 Nov 2017 08:19:58 +0100
>> Martin Kepplinger  wrote:
>>
>>> This adds the power_mode sysfs interface to the device as documented in
>>> sysfs-bus-iio.
>>>
>>> ---
>>>
>>> Note that I explicitely don't sign off on this.
>>>
>>> This is a starting point for anybody who can test it and check for correct
>>> API usage, and ABI correctness, as documented in 
>>> Documentation/ABI/testing/sys-bus-iio
>>> (grep it for "power_mode"). The ABI doc probably would need an addition
>>> too, if the 4 power modes here seem generally useful (there are only
>>>  2 listed there)!
>>>
>>> So, if you can test this, feel free to set up a proper patch or
>>> two, and I'm happy to review.
>>>
>>> Please note that this patch is quite old. It really should be that simple
>>> as far as my understanding back then. We always list the available 
>>> frequencies
>>> of the given power mode we are in, for example, already, and everything
>>> basically is in place except for the user interface.
>>
>> Hmm. A lot of devices support something along these lines.  The issue
>> has always been - how is userspace to figure out what to do with it?
>> It's all very vague...
>>
>> Funnily enough - this used to be really common, but is becoming less so
>> now - presumably because no one was using it much (or maybe I am reading
>> too much into that ;)
>>
>> Now the question is whether it can be tied to better defined things?
>>
>> Here low noise restricts the range to 4g.  Issue is that we don't actually
>> have writeable _available attributes (which correspond to range in this 
>> case).
>>
>
> Does it? Isn't it merely less oversampling.
>
>> Low power mode... This one is apparently oversampling.  If possible support
>> it as that as we have well defined interfaces for that.
>>
>> Jonathan.
>
> Ah, I remember; the oversampling settings was actually a reason why I
> hadn't submitted the patch :) The oversampling API would definitely be
> more accurate.
>
> I would like "oversampling" more than this "power_mode" too. For this
> driver it would be far more complicated to implement though. I doubt
> that it'll be done. power_mode is basically already there implicitely,
> and given that there *is* the ABI, we could offer it for free.
>
> But given your concerns, I would strip down this patch to only offer the
> already documented "low_noise" and "low_power" modes. It wouldn't be
> worth it to extend the ABI just because of this!
>
> Users would have a simple switch if they don't really *want* to know the
> details. I think it can be useful to just say "I don't care about power
> consuption. Be as accurate as possible" or "I just want this think to
> work. Use a little power as possible." Sure it's vage, but would it be
> useless?

Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[harinath Nampally]

> > This patch adds following related changes:
> > - defines pulse event related registers
> > - enables and handles single pulse interrupt for fxls8471
> > - handles IIO_EV_DIR_EITHER in read/write callbacks (because
> >   event direction for pulse is either rising or falling)
> > - configures read/write event value for pulse latency register
> >   using IIO_EV_INFO_HYSTERESIS
> > - adds multiple events like pulse and tranient event spec
> >   as elements of event_spec array named 'mma8452_accel_events'
> >
> > Except mma8653 chip all other chips like mma845x and
> > fxls8471 have single tap detection feature.
> > Tested thoroughly using iio_event_monitor application on
> > imx6ul-evk board which has fxls8471.
> >
> > Signed-off-by: Harinath Nampally <harinath...@gmail.com>
> > ---
> What tree is this written against? It doesn't apply to the current -next
> anyways.
Thanks for the review.
It is actually against 'testing' branch, I think two of my earlier
patches are not yet applied to
any branch, that might be reason this patch is not good against
current -next or 'togreg'.

> I think the defintions would deserve to be in a separate patch, but
> that's debatable.
Yes, I would argue that definitions are not a logical change.

> >   .type = IIO_EV_TYPE_MAG,
> >   .dir = IIO_EV_DIR_RISING,
> >   .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > @@ -1139,6 +1274,15 @@ static const struct iio_event_spec 
> > mma8452_transient_event[] = {
> >   BIT(IIO_EV_INFO_PERIOD) |
> >   BIT(IIO_EV_INFO_HIGH_PASS_FILTER_3DB)
> >   },
> > + {
> > + //pulse event
> > + .type = IIO_EV_TYPE_MAG,
> > + .dir = IIO_EV_DIR_EITHER,
> > + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> > + BIT(IIO_EV_INFO_PERIOD) |
> > + BIT(IIO_EV_INFO_HYSTERESIS)
> > + },
> >  };
> >
> >  static const struct iio_event_spec mma8452_motion_event[] = {
> > @@ -1202,8 +1346,8 @@ static struct attribute_group 
> > mma8452_event_attribute_group = {
> >   .shift = 16 - (bits), \
> >   .endianness = IIO_BE, \
> >   }, \
> > - .event_spec = mma8452_transient_event, \
> > - .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> > + .event_spec = mma8452_accel_events, \
> > + .num_event_specs = ARRAY_SIZE(mma8452_accel_events), \
> that would go in the mentioned separate renaming-patch
OK so I will make a patch set; patch 1/2 to just rename
'mma8452_transient_event[]'
to 'mma8452_accel_events[]'(without adding pulse event).
and everything else would go in 2/2. Does that makes sense?

Thanks,
Harinath

On Fri, Nov 10, 2017 at 5:35 PM, Martin Kepplinger <mart...@posteo.de> wrote:
> On 2017-11-09 04:12, Harinath Nampally wrote:
>> This patch adds following related changes:
>> - defines pulse event related registers
>> - enables and handles single pulse interrupt for fxls8471
>> - handles IIO_EV_DIR_EITHER in read/write callbacks (because
>>   event direction for pulse is either rising or falling)
>> - configures read/write event value for pulse latency register
>>   using IIO_EV_INFO_HYSTERESIS
>> - adds multiple events like pulse and tranient event spec
>>   as elements of event_spec array named 'mma8452_accel_events'
>>
>> Except mma8653 chip all other chips like mma845x and
>> fxls8471 have single tap detection feature.
>> Tested thoroughly using iio_event_monitor application on
>> imx6ul-evk board which has fxls8471.
>>
>> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>> ---
>
> What tree is this written against? It doesn't apply to the current -next
> anyways.
>
>>  drivers/iio/accel/mma8452.c | 156 
>> ++--
>>  1 file changed, 151 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 43c3a6b..36f1b56 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -72,6 +72,19 @@
>>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>>  #define MMA8452_TRANSIENT_COUNT  0x20
>>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
>> +#define MMA8452_PULSE_CFG0x21
>> +#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2)
>> +#define

Re: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[harinath Nampally]

> > This patch adds following related changes:
> > - defines pulse event related registers
> > - enables and handles single pulse interrupt for fxls8471
> > - handles IIO_EV_DIR_EITHER in read/write callbacks (because
> >   event direction for pulse is either rising or falling)
> > - configures read/write event value for pulse latency register
> >   using IIO_EV_INFO_HYSTERESIS
> > - adds multiple events like pulse and tranient event spec
> >   as elements of event_spec array named 'mma8452_accel_events'
> >
> > Except mma8653 chip all other chips like mma845x and
> > fxls8471 have single tap detection feature.
> > Tested thoroughly using iio_event_monitor application on
> > imx6ul-evk board which has fxls8471.
> >
> > Signed-off-by: Harinath Nampally 
> > ---
> What tree is this written against? It doesn't apply to the current -next
> anyways.
Thanks for the review.
It is actually against 'testing' branch, I think two of my earlier
patches are not yet applied to
any branch, that might be reason this patch is not good against
current -next or 'togreg'.

> I think the defintions would deserve to be in a separate patch, but
> that's debatable.
Yes, I would argue that definitions are not a logical change.

> >   .type = IIO_EV_TYPE_MAG,
> >   .dir = IIO_EV_DIR_RISING,
> >   .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > @@ -1139,6 +1274,15 @@ static const struct iio_event_spec 
> > mma8452_transient_event[] = {
> >   BIT(IIO_EV_INFO_PERIOD) |
> >   BIT(IIO_EV_INFO_HIGH_PASS_FILTER_3DB)
> >   },
> > + {
> > + //pulse event
> > + .type = IIO_EV_TYPE_MAG,
> > + .dir = IIO_EV_DIR_EITHER,
> > + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
> > + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> > + BIT(IIO_EV_INFO_PERIOD) |
> > + BIT(IIO_EV_INFO_HYSTERESIS)
> > + },
> >  };
> >
> >  static const struct iio_event_spec mma8452_motion_event[] = {
> > @@ -1202,8 +1346,8 @@ static struct attribute_group 
> > mma8452_event_attribute_group = {
> >   .shift = 16 - (bits), \
> >   .endianness = IIO_BE, \
> >   }, \
> > - .event_spec = mma8452_transient_event, \
> > - .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> > + .event_spec = mma8452_accel_events, \
> > + .num_event_specs = ARRAY_SIZE(mma8452_accel_events), \
> that would go in the mentioned separate renaming-patch
OK so I will make a patch set; patch 1/2 to just rename
'mma8452_transient_event[]'
to 'mma8452_accel_events[]'(without adding pulse event).
and everything else would go in 2/2. Does that makes sense?

Thanks,
Harinath

On Fri, Nov 10, 2017 at 5:35 PM, Martin Kepplinger  wrote:
> On 2017-11-09 04:12, Harinath Nampally wrote:
>> This patch adds following related changes:
>> - defines pulse event related registers
>> - enables and handles single pulse interrupt for fxls8471
>> - handles IIO_EV_DIR_EITHER in read/write callbacks (because
>>   event direction for pulse is either rising or falling)
>> - configures read/write event value for pulse latency register
>>   using IIO_EV_INFO_HYSTERESIS
>> - adds multiple events like pulse and tranient event spec
>>   as elements of event_spec array named 'mma8452_accel_events'
>>
>> Except mma8653 chip all other chips like mma845x and
>> fxls8471 have single tap detection feature.
>> Tested thoroughly using iio_event_monitor application on
>> imx6ul-evk board which has fxls8471.
>>
>> Signed-off-by: Harinath Nampally 
>> ---
>
> What tree is this written against? It doesn't apply to the current -next
> anyways.
>
>>  drivers/iio/accel/mma8452.c | 156 
>> ++--
>>  1 file changed, 151 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
>> index 43c3a6b..36f1b56 100644
>> --- a/drivers/iio/accel/mma8452.c
>> +++ b/drivers/iio/accel/mma8452.c
>> @@ -72,6 +72,19 @@
>>  #define  MMA8452_TRANSIENT_THS_MASK  GENMASK(6, 0)
>>  #define MMA8452_TRANSIENT_COUNT  0x20
>>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
>> +#define MMA8452_PULSE_CFG0x21
>> +#define MMA8452_PULSE_CFG_CHAN(chan) BIT(chan * 2)
>> +#define MMA8452_PULSE_CFG_ELEBIT(6)
>> +#define MMA845

Re: [PATCH] iio: mma8452: add power_mode sysfs configuration

[harinath Nampally]

Hi Martin,

Thanks for publishing the patch.
I will work on it, but unfortunately I can't promise anything before 11/27.

Thanks,
Harinath

On Mon, Nov 6, 2017 at 2:19 AM, Martin Kepplinger  wrote:
> This adds the power_mode sysfs interface to the device as documented in
> sysfs-bus-iio.
>
> ---
>
> Note that I explicitely don't sign off on this.
>
> This is a starting point for anybody who can test it and check for correct
> API usage, and ABI correctness, as documented in 
> Documentation/ABI/testing/sys-bus-iio
> (grep it for "power_mode"). The ABI doc probably would need an addition
> too, if the 4 power modes here seem generally useful (there are only
>  2 listed there)!
>
> So, if you can test this, feel free to set up a proper patch or
> two, and I'm happy to review.
>
> Please note that this patch is quite old. It really should be that simple
> as far as my understanding back then. We always list the available frequencies
> of the given power mode we are in, for example, already, and everything
> basically is in place except for the user interface.
>
> thanks
> martin
>
>
>
>  drivers/iio/accel/mma8452.c | 37 +
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index bfd4bc806fc2..640bbd9872ab 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1166,6 +1166,41 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> .attrs = mma8452_event_attributes,
>  };
>
> +static const char * const mma8452_power_modes[] = {"normal",
> +  "low_noise_low_power",
> +  "low_noise",
> +  "low_power"};
> +
> +static int mma8452_get_power_mode_iio_enum(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan)
> +{
> +   struct mma8452_data *data = iio_priv(indio_dev);
> +
> +   return mma8452_get_power_mode(data);
> +}
> +
> +static int mma8452_set_power_mode_iio_enum(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan,
> +  unsigned int mode)
> +{
> +   struct mma8452_data *data = iio_priv(indio_dev);
> +
> +   return mma8452_set_power_mode(data, mode);
> +}
> +
> +static const struct iio_enum mma8452_power_mode_enum = {
> +   .items = mma8452_power_modes,
> +   .num_items = ARRAY_SIZE(mma8452_power_modes),
> +   .get = mma8452_get_power_mode_iio_enum,
> +   .set = mma8452_set_power_mode_iio_enum,
> +};
> +
> +static const struct iio_chan_spec_ext_info mma8452_ext_info[] = {
> +   IIO_ENUM("power_mode", true, _power_mode_enum),
> +   IIO_ENUM_AVAILABLE("power_mode", _power_mode_enum),
> +   { },
> +};
> +
>  #define MMA8452_FREEFALL_CHANNEL(modifier) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -1204,6 +1239,7 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> }, \
> .event_spec = mma8452_transient_event, \
> .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> +   .ext_info = mma8452_ext_info, \
>  }
>
>  #define MMA8652_CHANNEL(axis, idx, bits) { \
> @@ -1225,6 +1261,7 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> }, \
> .event_spec = mma8452_motion_event, \
> .num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
> +   .ext_info = mma8452_ext_info, \
>  }
>
>  static const struct iio_chan_spec mma8451_channels[] = {
> --
> 2.11.0
>

Re: [PATCH] iio: mma8452: add power_mode sysfs configuration

[harinath Nampally]

Hi Martin,

Thanks for publishing the patch.
I will work on it, but unfortunately I can't promise anything before 11/27.

Thanks,
Harinath

On Mon, Nov 6, 2017 at 2:19 AM, Martin Kepplinger  wrote:
> This adds the power_mode sysfs interface to the device as documented in
> sysfs-bus-iio.
>
> ---
>
> Note that I explicitely don't sign off on this.
>
> This is a starting point for anybody who can test it and check for correct
> API usage, and ABI correctness, as documented in 
> Documentation/ABI/testing/sys-bus-iio
> (grep it for "power_mode"). The ABI doc probably would need an addition
> too, if the 4 power modes here seem generally useful (there are only
>  2 listed there)!
>
> So, if you can test this, feel free to set up a proper patch or
> two, and I'm happy to review.
>
> Please note that this patch is quite old. It really should be that simple
> as far as my understanding back then. We always list the available frequencies
> of the given power mode we are in, for example, already, and everything
> basically is in place except for the user interface.
>
> thanks
> martin
>
>
>
>  drivers/iio/accel/mma8452.c | 37 +
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index bfd4bc806fc2..640bbd9872ab 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -1166,6 +1166,41 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> .attrs = mma8452_event_attributes,
>  };
>
> +static const char * const mma8452_power_modes[] = {"normal",
> +  "low_noise_low_power",
> +  "low_noise",
> +  "low_power"};
> +
> +static int mma8452_get_power_mode_iio_enum(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan)
> +{
> +   struct mma8452_data *data = iio_priv(indio_dev);
> +
> +   return mma8452_get_power_mode(data);
> +}
> +
> +static int mma8452_set_power_mode_iio_enum(struct iio_dev *indio_dev,
> +  const struct iio_chan_spec *chan,
> +  unsigned int mode)
> +{
> +   struct mma8452_data *data = iio_priv(indio_dev);
> +
> +   return mma8452_set_power_mode(data, mode);
> +}
> +
> +static const struct iio_enum mma8452_power_mode_enum = {
> +   .items = mma8452_power_modes,
> +   .num_items = ARRAY_SIZE(mma8452_power_modes),
> +   .get = mma8452_get_power_mode_iio_enum,
> +   .set = mma8452_set_power_mode_iio_enum,
> +};
> +
> +static const struct iio_chan_spec_ext_info mma8452_ext_info[] = {
> +   IIO_ENUM("power_mode", true, _power_mode_enum),
> +   IIO_ENUM_AVAILABLE("power_mode", _power_mode_enum),
> +   { },
> +};
> +
>  #define MMA8452_FREEFALL_CHANNEL(modifier) { \
> .type = IIO_ACCEL, \
> .modified = 1, \
> @@ -1204,6 +1239,7 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> }, \
> .event_spec = mma8452_transient_event, \
> .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
> +   .ext_info = mma8452_ext_info, \
>  }
>
>  #define MMA8652_CHANNEL(axis, idx, bits) { \
> @@ -1225,6 +1261,7 @@ static struct attribute_group 
> mma8452_event_attribute_group = {
> }, \
> .event_spec = mma8452_motion_event, \
> .num_event_specs = ARRAY_SIZE(mma8452_motion_event), \
> +   .ext_info = mma8452_ext_info, \
>  }
>
>  static const struct iio_chan_spec mma8451_channels[] = {
> --
> 2.11.0
>

[PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[Harinath Nampally]

This patch adds following related changes:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks (because
  event direction for pulse is either rising or falling)
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS
- adds multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board which has fxls8471.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 156 ++--
 1 file changed, 151 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+ 

[PATCH] iio: accel: mma8452: Add single pulse/tap event detection

[Harinath Nampally]

This patch adds following related changes:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks (because
  event direction for pulse is either rising or falling)
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS
- adds multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board which has fxls8471.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events

Re: [PATCH v4 2/2] iio: accel: mma8452: Rename config structs for readability

[harinath Nampally]

Hi Martin,

> I think it'd be great to have the "power_mode" iio ABI interface
> for mma8452 too, and I just found an old patch pf mine for this.
>
> If you say you could test it and check for correct API usage and
> ABI provisioning in sysfs, I'd be happy to publish it for you to
> make any necessary corrections and submit it after testing.
>
> Do we have a deal?
Sure, sounds great!

Thanks,
Harinath

On Sun, Nov 5, 2017 at 3:36 PM, Martin Kepplinger <mart...@posteo.de> wrote:
> On 2017-11-05 19:00, Harinath Nampally wrote:
>> Rename structs holding event configuration registers
>> to more appropriate names. This naming is consistent
>> with the event config register names given in the
>> mma845x and fxls8471 datasheets.
>>
>> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>
> Makes sense to me.
>
> Acked-by: Martin Kepplinger <mart...@posteo.de>
>
>
> Hey Harinath,
>
> I think it'd be great to have the "power_mode" iio ABI interface
> for mma8452 too, and I just found an old patch pf mine for this.
>
> If you say you could test it and check for correct API usage and
> ABI provisioning in sysfs, I'd be happy to publish it for you to
> make any necessary corrections and submit it after testing.
>
> Do we have a deal?
>
> thanks
>
>  martin

Re: [PATCH v4 2/2] iio: accel: mma8452: Rename config structs for readability

[harinath Nampally]

Hi Martin,

> I think it'd be great to have the "power_mode" iio ABI interface
> for mma8452 too, and I just found an old patch pf mine for this.
>
> If you say you could test it and check for correct API usage and
> ABI provisioning in sysfs, I'd be happy to publish it for you to
> make any necessary corrections and submit it after testing.
>
> Do we have a deal?
Sure, sounds great!

Thanks,
Harinath

On Sun, Nov 5, 2017 at 3:36 PM, Martin Kepplinger  wrote:
> On 2017-11-05 19:00, Harinath Nampally wrote:
>> Rename structs holding event configuration registers
>> to more appropriate names. This naming is consistent
>> with the event config register names given in the
>> mma845x and fxls8471 datasheets.
>>
>> Signed-off-by: Harinath Nampally 
>
> Makes sense to me.
>
> Acked-by: Martin Kepplinger 
>
>
> Hey Harinath,
>
> I think it'd be great to have the "power_mode" iio ABI interface
> for mma8452 too, and I just found an old patch pf mine for this.
>
> If you say you could test it and check for correct API usage and
> ABI provisioning in sysfs, I'd be happy to publish it for you to
> make any necessary corrections and submit it after testing.
>
> Do we have a deal?
>
> thanks
>
>  martin

[PATCH v4 1/2] iio: accel: mma8452: Rename a struct for code readibility

[Harinath Nampally]

Rename time step look up struct to generic name
as the values in the look table are same for all
the other events like pulse, transient etc.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 v4:
 Shorter subject line and better description in commit message

 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index bfd4bc8..16adf47 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH v4 1/2] iio: accel: mma8452: Rename a struct for code readibility

[Harinath Nampally]

Rename time step look up struct to generic name
as the values in the look table are same for all
the other events like pulse, transient etc.

Signed-off-by: Harinath Nampally 
---
 v4:
 Shorter subject line and better description in commit message

 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index bfd4bc8..16adf47 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH v4 2/2] iio: accel: mma8452: Rename config structs for readability

[Harinath Nampally]

Rename structs holding event configuration registers
to more appropriate names. This naming is consistent
with the event config register names given in the
mma845x and fxls8471 datasheets.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 v4:
 Shorter subject line and better description in commit message

 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 16adf47..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH v4 0/2] Refactor event related code

[Harinath Nampally]

Rename some struct names improve code readability.

Harinath Nampally (2):
  iio: accel: mma8452: Rename a struct for code readibility
  iio: accel: mma8452: Rename config structs for readability

 drivers/iio/accel/mma8452.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
 v4:
 - Dropped a patch from v3 which was already merged
 - Subject line and commit message have better descriptions
2.7.4

[PATCH v4 2/2] iio: accel: mma8452: Rename config structs for readability

[Harinath Nampally]

Rename structs holding event configuration registers
to more appropriate names. This naming is consistent
with the event config register names given in the
mma845x and fxls8471 datasheets.

Signed-off-by: Harinath Nampally 
---
 v4:
 Shorter subject line and better description in commit message

 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 16adf47..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH v4 0/2] Refactor event related code

[Harinath Nampally]

Rename some struct names improve code readability.

Harinath Nampally (2):
  iio: accel: mma8452: Rename a struct for code readibility
  iio: accel: mma8452: Rename config structs for readability

 drivers/iio/accel/mma8452.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

-- 
 v4:
 - Dropped a patch from v3 which was already merged
 - Subject line and commit message have better descriptions
2.7.4

Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[harinath Nampally]

> > On 2017-09-30 19:59, Jonathan Cameron wrote:
> > > On Wed, 27 Sep 2017 08:52:54 +0200
> > > Martin Kepplinger <mart...@posteo.de> wrote:
> > >
> > >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > >>> Improves code readability, no impact on functionality.
> > >>>
> > >>> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
> > >>> ---
> > >>
> > >> I'd prefer a shorter subject line here too, see patch 2/3
> > >
> > > Agreed.  I'm unconvinced the change helps.  Perhaps that is
> > > because I don't fully understand why you are making the change?
> > >
> >
> > It's understandable for me. It simply uses "transient" and "ff_mt" in
> > order to describe sets of device-registers instead of "rising" and
> > "falling". That's more appropriate. I'd apply this if possible.
> >
> > Even though it should be clear after reading the data sheet, this
> > particular issue sometimes gets confusing for people. Maybe a short
> > descriptive comment, pointing to the data sheet, would make sense?
> >
>
> With the addition of some description, this should be fine.
Sure will do.

Thanks,
Harinath

On Mon, Oct 2, 2017 at 6:35 AM, Jonathan Cameron
<jonathan.came...@huawei.com> wrote:
> On Sun, 1 Oct 2017 20:10:49 +0200
> Martin Kepplinger <mart...@posteo.de> wrote:
>
>> On 2017-09-30 19:59, Jonathan Cameron wrote:
>> > On Wed, 27 Sep 2017 08:52:54 +0200
>> > Martin Kepplinger <mart...@posteo.de> wrote:
>> >
>> >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> >>> Improves code readability, no impact on functionality.
>> >>>
>> >>> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>> >>> ---
>> >>
>> >> I'd prefer a shorter subject line here too, see patch 2/3
>> >
>> > Agreed.  I'm unconvinced the change helps.  Perhaps that is
>> > because I don't fully understand why you are making the change?
>> >
>>
>> It's understandable for me. It simply uses "transient" and "ff_mt" in
>> order to describe sets of device-registers instead of "rising" and
>> "falling". That's more appropriate. I'd apply this if possible.
>>
>> Even though it should be clear after reading the data sheet, this
>> particular issue sometimes gets confusing for people. Maybe a short
>> descriptive comment, pointing to the data sheet, would make sense?
>>
>
> With the addition of some description, this should be fine.
>
> Thanks,
>
> Jonathan
>
>>   martin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[harinath Nampally]

> > On 2017-09-30 19:59, Jonathan Cameron wrote:
> > > On Wed, 27 Sep 2017 08:52:54 +0200
> > > Martin Kepplinger  wrote:
> > >
> > >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > >>> Improves code readability, no impact on functionality.
> > >>>
> > >>> Signed-off-by: Harinath Nampally 
> > >>> ---
> > >>
> > >> I'd prefer a shorter subject line here too, see patch 2/3
> > >
> > > Agreed.  I'm unconvinced the change helps.  Perhaps that is
> > > because I don't fully understand why you are making the change?
> > >
> >
> > It's understandable for me. It simply uses "transient" and "ff_mt" in
> > order to describe sets of device-registers instead of "rising" and
> > "falling". That's more appropriate. I'd apply this if possible.
> >
> > Even though it should be clear after reading the data sheet, this
> > particular issue sometimes gets confusing for people. Maybe a short
> > descriptive comment, pointing to the data sheet, would make sense?
> >
>
> With the addition of some description, this should be fine.
Sure will do.

Thanks,
Harinath

On Mon, Oct 2, 2017 at 6:35 AM, Jonathan Cameron
 wrote:
> On Sun, 1 Oct 2017 20:10:49 +0200
> Martin Kepplinger  wrote:
>
>> On 2017-09-30 19:59, Jonathan Cameron wrote:
>> > On Wed, 27 Sep 2017 08:52:54 +0200
>> > Martin Kepplinger  wrote:
>> >
>> >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> >>> Improves code readability, no impact on functionality.
>> >>>
>> >>> Signed-off-by: Harinath Nampally 
>> >>> ---
>> >>
>> >> I'd prefer a shorter subject line here too, see patch 2/3
>> >
>> > Agreed.  I'm unconvinced the change helps.  Perhaps that is
>> > because I don't fully understand why you are making the change?
>> >
>>
>> It's understandable for me. It simply uses "transient" and "ff_mt" in
>> order to describe sets of device-registers instead of "rising" and
>> "falling". That's more appropriate. I'd apply this if possible.
>>
>> Even though it should be clear after reading the data sheet, this
>> particular issue sometimes gets confusing for people. Maybe a short
>> descriptive comment, pointing to the data sheet, would make sense?
>>
>
> With the addition of some description, this should be fine.
>
> Thanks,
>
> Jonathan
>
>>   martin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v3 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[harinath Nampally]

> Obviously drop any patches that have already been taken.
> In this case it will be a v4 series containing patches 1 and 2 only.
Sure will do.

Thanks,
Harinath

On Sat, Sep 30, 2017 at 2:05 PM, Jonathan Cameron <ji...@kernel.org> wrote:
> On Wed, 27 Sep 2017 08:51:26 +0200
> Martin Kepplinger <mart...@posteo.de> wrote:
>
>> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> > Improves code readability, no impact on functionality.
>> >
>> > Signed-off-by: Harinath Nampally <harinath...@gmail.com>
>>
>> Please make the headline shorter and put some of it in the git commit
>> message.
>> (And please just resend it "--in-reply-to" this conversation, this patch
>> nr 2 of 3)
>
> From a patch management point of view I actually disagree with this.
> I would prefer to see a clean fresh series.  Otherwise it very rapidly
> gets hard to be sure that I am picking up the latest versions.
>
> Obviously drop any patches that have already been taken.
> In this case it will be a v4 series containing patches 1 and 2 only.
>
> Thanks
>
> Jonathan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v3 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[harinath Nampally]

> Obviously drop any patches that have already been taken.
> In this case it will be a v4 series containing patches 1 and 2 only.
Sure will do.

Thanks,
Harinath

On Sat, Sep 30, 2017 at 2:05 PM, Jonathan Cameron  wrote:
> On Wed, 27 Sep 2017 08:51:26 +0200
> Martin Kepplinger  wrote:
>
>> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> > Improves code readability, no impact on functionality.
>> >
>> > Signed-off-by: Harinath Nampally 
>>
>> Please make the headline shorter and put some of it in the git commit
>> message.
>> (And please just resend it "--in-reply-to" this conversation, this patch
>> nr 2 of 3)
>
> From a patch management point of view I actually disagree with this.
> I would prefer to see a clean fresh series.  Otherwise it very rapidly
> gets hard to be sure that I am picking up the latest versions.
>
> Obviously drop any patches that have already been taken.
> In this case it will be a v4 series containing patches 1 and 2 only.
>
> Thanks
>
> Jonathan
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH v3 0/3] Refactor event related code

[Harinath Nampally]

Rename some struct names and function names to
improve code readability.

Harinath Nampally (3):
  iio: accel: mma8452: Rename structs holding event configuration
registers to more appropriate names.
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.

 drivers/iio/accel/mma8452.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
changes v2->v3
-Remove one unrelated patch in the patchset
-Add version v3 in the subject

changes v1->v2
Add one more related patch in the patchset

2.7.4

[PATCH v3 0/3] Refactor event related code

[Harinath Nampally]

Rename some struct names and function names to
improve code readability.

Harinath Nampally (3):
  iio: accel: mma8452: Rename structs holding event configuration
registers to more appropriate names.
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.

 drivers/iio/accel/mma8452.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
changes v2->v3
-Remove one unrelated patch in the patchset
-Add version v3 in the subject

changes v1->v2
Add one more related patch in the patchset

2.7.4

[PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH v3 3/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH v3 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH v3 3/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH v3 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

Re: [PATCH 0/3] Refactor event related code

[harinath Nampally]

> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
ok I was not sure whether to make it v2 as the change was to only cover letter
and no code changes. But now I know, will send v3 of this series.

> It's sometimes not trivial to get email right. Why not test sending it 
> privately
> to yourself first?
Sure will do, sorry for the convenience.

Thanks,
Harinath

On Mon, Sep 25, 2017 at 6:11 AM, Martin Kepplinger <mart...@posteo.de> wrote:
> Am 25.09.2017 12:07 schrieb Harinath Nampally:
>>
>> Rename some struct names and function names to
>> improve code readability.
>>
>> Harinath Nampally (3):
>>   iio: accel: mma8452: Rename structs holding event configuration
>> registers to more appropriate names.
>>   iio: accel: mma8452: Rename time step look up struct to generic
>> name as the values are same for all the events.
>>   iio: accel: mma8452: Rename read/write event value callbacks to
>> generic function name.
>>
>
> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
>
> It's sometimes not trivial to get email right. Why not test sending it
> privately
> to yourself first?

Re: [PATCH 0/3] Refactor event related code

[harinath Nampally]

> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
ok I was not sure whether to make it v2 as the change was to only cover letter
and no code changes. But now I know, will send v3 of this series.

> It's sometimes not trivial to get email right. Why not test sending it 
> privately
> to yourself first?
Sure will do, sorry for the convenience.

Thanks,
Harinath

On Mon, Sep 25, 2017 at 6:11 AM, Martin Kepplinger  wrote:
> Am 25.09.2017 12:07 schrieb Harinath Nampally:
>>
>> Rename some struct names and function names to
>> improve code readability.
>>
>> Harinath Nampally (3):
>>   iio: accel: mma8452: Rename structs holding event configuration
>> registers to more appropriate names.
>>   iio: accel: mma8452: Rename time step look up struct to generic
>> name as the values are same for all the events.
>>   iio: accel: mma8452: Rename read/write event value callbacks to
>> generic function name.
>>
>
> First, this is at least v2 of this series. Second, it still seems to include
> a 4th unrelated patch.
>
> It's sometimes not trivial to get email right. Why not test sending it
> privately
> to yourself first?

Re: [PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[harinath Nampally]

Please disregard this patch set as this includes an unrelated patch
'[PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection
feature for fxls8471.'

On Mon, Sep 25, 2017 at 6:07 AM, Harinath Nampally
<harinath...@gmail.com> wrote:
> This patch adds following changes to support tap feature:
> - defines pulse event related registers
> - enables and handles single pulse interrupt for fxls8471
> - handles IIO_EV_DIR_EITHER in read/write callbacks
>   because event direction for pulse is either rising or
>   falling.
> - configures read/write event value for pulse latency register
>   using IIO_EV_INFO_HYSTERESIS.
> - add multiple events like pulse and tranient event spec
>   as elements of event_spec array named 'mma8452_accel_events'
>
> Except mma8653 chip all other chips like mma845x and
> fxls8471 have single tap detection feature.
> Tested thoroughly using iio_event_monitor application on
> imx6ul-evk board.
>
> Signed-off-by: Harinath Nampally <harinath...@gmail.com>
> ---
>  drivers/iio/accel/mma8452.c | 156 
> ++--
>  1 file changed, 151 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 43c3a6b..36f1b56 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -72,6 +72,19 @@
>  #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT0x20
>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
> +#define MMA8452_PULSE_CFG  0x21
> +#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
> +#define MMA8452_PULSE_CFG_ELE  BIT(6)
> +#define MMA8452_PULSE_SRC  0x22
> +#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
> +#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
> +#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
> +#define MMA8452_PULSE_THS  0x23
> +#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
> +#define MMA8452_PULSE_COUNT0x26
> +#define MMA8452_PULSE_CHAN_SHIFT   2
> +#define MMA8452_PULSE_LTCY 0x27
> +
>  #define MMA8452_CTRL_REG1  0x2a
>  #define  MMA8452_CTRL_ACTIVE   BIT(0)
>  #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
> @@ -91,6 +104,7 @@
>
>  #define  MMA8452_INT_DRDY  BIT(0)
>  #define  MMA8452_INT_FF_MT BIT(2)
> +#define  MMA8452_INT_PULSE BIT(3)
>  #define  MMA8452_INT_TRANS BIT(5)
>
>  #define MMA8451_DEVICE_ID  0x1a
> @@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
> .ev_count = MMA8452_TRANSIENT_COUNT,
>  };
>
> +static const struct mma8452_event_regs pulse_ev_regs = {
> +   .ev_cfg = MMA8452_PULSE_CFG,
> +   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
> +   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
> +   .ev_src = MMA8452_PULSE_SRC,
> +   .ev_ths = MMA8452_PULSE_THS,
> +   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
> +   .ev_count = MMA8452_PULSE_COUNT,
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id:   WHO_AM_I register's value
> @@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
> *data,
> case IIO_EV_DIR_FALLING:
> *ev_reg = _mt_ev_regs;
> return 0;
> +   case IIO_EV_DIR_EITHER:
> +   if (!(data->chip_info->all_events
> +   & MMA8452_INT_PULSE) ||
> +   !(data->chip_info->enabled_events
> +   & MMA8452_INT_PULSE))
> +   return -EINVAL;
> +   *ev_reg = _ev_regs;
> +   return 0;
> default:
> return -EINVAL;
> }
> @@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
> *indio_dev,
> return ret;
> }
>
> +   case IIO_EV_INFO_HYSTERESIS:
> +   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
> +   !(data->chip_info->enabled_events & 
> MMA8452_INT_PULSE))
> +   return -EINVAL;
> +
> +   ret = i2c_smbus_read_byte_data(data->client,
> +   MMA8452_PU

Re: [PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[harinath Nampally]

Please disregard this patch set as this includes an unrelated patch
'[PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection
feature for fxls8471.'

On Mon, Sep 25, 2017 at 6:07 AM, Harinath Nampally
 wrote:
> This patch adds following changes to support tap feature:
> - defines pulse event related registers
> - enables and handles single pulse interrupt for fxls8471
> - handles IIO_EV_DIR_EITHER in read/write callbacks
>   because event direction for pulse is either rising or
>   falling.
> - configures read/write event value for pulse latency register
>   using IIO_EV_INFO_HYSTERESIS.
> - add multiple events like pulse and tranient event spec
>   as elements of event_spec array named 'mma8452_accel_events'
>
> Except mma8653 chip all other chips like mma845x and
> fxls8471 have single tap detection feature.
> Tested thoroughly using iio_event_monitor application on
> imx6ul-evk board.
>
> Signed-off-by: Harinath Nampally 
> ---
>  drivers/iio/accel/mma8452.c | 156 
> ++--
>  1 file changed, 151 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
> index 43c3a6b..36f1b56 100644
> --- a/drivers/iio/accel/mma8452.c
> +++ b/drivers/iio/accel/mma8452.c
> @@ -72,6 +72,19 @@
>  #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
>  #define MMA8452_TRANSIENT_COUNT0x20
>  #define MMA8452_TRANSIENT_CHAN_SHIFT 1
> +#define MMA8452_PULSE_CFG  0x21
> +#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
> +#define MMA8452_PULSE_CFG_ELE  BIT(6)
> +#define MMA8452_PULSE_SRC  0x22
> +#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
> +#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
> +#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
> +#define MMA8452_PULSE_THS  0x23
> +#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
> +#define MMA8452_PULSE_COUNT0x26
> +#define MMA8452_PULSE_CHAN_SHIFT   2
> +#define MMA8452_PULSE_LTCY 0x27
> +
>  #define MMA8452_CTRL_REG1  0x2a
>  #define  MMA8452_CTRL_ACTIVE   BIT(0)
>  #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
> @@ -91,6 +104,7 @@
>
>  #define  MMA8452_INT_DRDY  BIT(0)
>  #define  MMA8452_INT_FF_MT BIT(2)
> +#define  MMA8452_INT_PULSE BIT(3)
>  #define  MMA8452_INT_TRANS BIT(5)
>
>  #define MMA8451_DEVICE_ID  0x1a
> @@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
> .ev_count = MMA8452_TRANSIENT_COUNT,
>  };
>
> +static const struct mma8452_event_regs pulse_ev_regs = {
> +   .ev_cfg = MMA8452_PULSE_CFG,
> +   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
> +   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
> +   .ev_src = MMA8452_PULSE_SRC,
> +   .ev_ths = MMA8452_PULSE_THS,
> +   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
> +   .ev_count = MMA8452_PULSE_COUNT,
> +};
> +
>  /**
>   * struct mma_chip_info - chip specific data
>   * @chip_id:   WHO_AM_I register's value
> @@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
> *data,
> case IIO_EV_DIR_FALLING:
> *ev_reg = _mt_ev_regs;
> return 0;
> +   case IIO_EV_DIR_EITHER:
> +   if (!(data->chip_info->all_events
> +   & MMA8452_INT_PULSE) ||
> +   !(data->chip_info->enabled_events
> +   & MMA8452_INT_PULSE))
> +   return -EINVAL;
> +   *ev_reg = _ev_regs;
> +   return 0;
> default:
> return -EINVAL;
> }
> @@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
> *indio_dev,
> return ret;
> }
>
> +   case IIO_EV_INFO_HYSTERESIS:
> +   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
> +   !(data->chip_info->enabled_events & 
> MMA8452_INT_PULSE))
> +   return -EINVAL;
> +
> +   ret = i2c_smbus_read_byte_data(data->client,
> +   MMA8452_PULSE_LTCY);
> +   if (ret < 0)
> +

[PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[Harinath Nampally]

This patch adds following changes to support tap feature:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks
  because event direction for pulse is either rising or
  falling.
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS.
- add multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 156 ++--
 1 file changed, 151 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+ 

[PATCH 4/4] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[Harinath Nampally]

This patch adds following changes to support tap feature:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks
  because event direction for pulse is either rising or
  falling.
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS.
- add multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8

[PATCH 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH 2/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH 3/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH 3/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH 0/3] Refactor event related code

[Harinath Nampally]

Rename some struct names and function names to
improve code readability.

Harinath Nampally (3):
  iio: accel: mma8452: Rename structs holding event configuration
registers to more appropriate names.
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.

 drivers/iio/accel/mma8452.c | 180 +++-
 1 file changed, 163 insertions(+), 17 deletions(-)

-- 
2.7.4

[PATCH 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH 0/3] Refactor event related code

[Harinath Nampally]

Rename some struct names and function names to
improve code readability.

Harinath Nampally (3):
  iio: accel: mma8452: Rename structs holding event configuration
registers to more appropriate names.
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.

 drivers/iio/accel/mma8452.c | 180 +++-
 1 file changed, 163 insertions(+), 17 deletions(-)

-- 
2.7.4

[PATCH 2/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH 1/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH 2/3] iio: accel: mma8452: Rename read/write event value callbacks to generic function name.

[Harinath Nampally]

'mma8452_read_thresh' and 'mma8452_write_thresh' functions
does more than just read/write threshold values.
They also handle  IIO_EV_INFO_HIGH_PASS_FILTER_3DB and
IIO_EV_INFO_PERIOD therefore renaming to generic names.

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 74b6221..43c3a6b 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -792,7 +792,7 @@ static int mma8452_get_event_regs(struct mma8452_data *data,
}
 }
 
-static int mma8452_read_thresh(struct iio_dev *indio_dev,
+static int mma8452_read_event_value(struct iio_dev *indio_dev,
   const struct iio_chan_spec *chan,
   enum iio_event_type type,
   enum iio_event_direction dir,
@@ -855,7 +855,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
}
 }
 
-static int mma8452_write_thresh(struct iio_dev *indio_dev,
+static int mma8452_write_event_value(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
enum iio_event_direction dir,
@@ -1391,8 +1391,8 @@ static const struct iio_info mma8452_info = {
.read_raw = _read_raw,
.write_raw = _write_raw,
.event_attrs = _event_attribute_group,
-   .read_event_value = _read_thresh,
-   .write_event_value = _write_thresh,
+   .read_event_value = _read_event_value,
+   .write_event_value = _write_event_value,
.read_event_config = _read_event_config,
.write_event_config = _write_event_config,
.debugfs_reg_access = _reg_access_dbg,
-- 
2.7.4

[PATCH 1/3] iio: accel: mma8452: Rename time step look up struct to generic name as the values are same for all the events.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 3472e7e..74b6221 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -284,7 +284,7 @@ static const int mma8452_samp_freq[8][2] = {
 };
 
 /* Datasheet table: step time "Relationship with the ODR" (sample frequency) */
-static const unsigned int mma8452_transient_time_step_us[4][8] = {
+static const unsigned int mma8452_time_step_us[4][8] = {
{ 1250, 2500, 5000, 1, 2, 2, 2, 2 },  /* normal */
{ 1250, 2500, 5000, 1, 2, 8, 8, 8 },  /* l p l n */
{ 1250, 2500, 2500, 2500, 2500, 2500, 2500, 2500 },   /* high res*/
@@ -826,7 +826,7 @@ static int mma8452_read_thresh(struct iio_dev *indio_dev,
if (power_mode < 0)
return power_mode;
 
-   us = ret * mma8452_transient_time_step_us[power_mode][
+   us = ret * mma8452_time_step_us[power_mode][
mma8452_get_odr_index(data)];
*val = us / USEC_PER_SEC;
*val2 = us % USEC_PER_SEC;
@@ -883,7 +883,7 @@ static int mma8452_write_thresh(struct iio_dev *indio_dev,
return ret;
 
steps = (val * USEC_PER_SEC + val2) /
-   mma8452_transient_time_step_us[ret][
+   mma8452_time_step_us[ret][
mma8452_get_odr_index(data)];
 
if (steps < 0 || steps > 0xff)
-- 
2.7.4

[PATCH 0/3] This patchset refactors event related functions

[Harinath Nampally]

Harinath Nampally (3):
Following 2 patches are for refactor:
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.
Following patch adds new feature:
  iio: accel: mma8452: Add single pulse/tap event detection feature
for fxls8471.

 drivers/iio/accel/mma8452.c | 170 
 1 file changed, 158 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH 3/3] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[Harinath Nampally]

This patch adds following changes to support tap feature:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks
  because event direction for pulse is either rising or
  falling.
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS.
- add multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 156 ++--
 1 file changed, 151 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+ 

[PATCH 0/3] This patchset refactors event related functions

[Harinath Nampally]

Harinath Nampally (3):
Following 2 patches are for refactor:
  iio: accel: mma8452: Rename time step look up struct to generic
name as the values are same for all the events.
  iio: accel: mma8452: Rename read/write event value callbacks to
generic function name.
Following patch adds new feature:
  iio: accel: mma8452: Add single pulse/tap event detection feature
for fxls8471.

 drivers/iio/accel/mma8452.c | 170 
 1 file changed, 158 insertions(+), 12 deletions(-)

-- 
2.7.4

[PATCH 3/3] iio: accel: mma8452: Add single pulse/tap event detection feature for fxls8471.

[Harinath Nampally]

This patch adds following changes to support tap feature:
- defines pulse event related registers
- enables and handles single pulse interrupt for fxls8471
- handles IIO_EV_DIR_EITHER in read/write callbacks
  because event direction for pulse is either rising or
  falling.
- configures read/write event value for pulse latency register
  using IIO_EV_INFO_HYSTERESIS.
- add multiple events like pulse and tranient event spec
  as elements of event_spec array named 'mma8452_accel_events'

Except mma8653 chip all other chips like mma845x and
fxls8471 have single tap detection feature.
Tested thoroughly using iio_event_monitor application on
imx6ul-evk board.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 43c3a6b..36f1b56 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -72,6 +72,19 @@
 #define  MMA8452_TRANSIENT_THS_MASKGENMASK(6, 0)
 #define MMA8452_TRANSIENT_COUNT0x20
 #define MMA8452_TRANSIENT_CHAN_SHIFT 1
+#define MMA8452_PULSE_CFG  0x21
+#define MMA8452_PULSE_CFG_CHAN(chan)   BIT(chan * 2)
+#define MMA8452_PULSE_CFG_ELE  BIT(6)
+#define MMA8452_PULSE_SRC  0x22
+#define MMA8452_PULSE_SRC_XPULSE   BIT(4)
+#define MMA8452_PULSE_SRC_YPULSE   BIT(5)
+#define MMA8452_PULSE_SRC_ZPULSE   BIT(6)
+#define MMA8452_PULSE_THS  0x23
+#define MMA8452_PULSE_THS_MASK GENMASK(6, 0)
+#define MMA8452_PULSE_COUNT0x26
+#define MMA8452_PULSE_CHAN_SHIFT   2
+#define MMA8452_PULSE_LTCY 0x27
+
 #define MMA8452_CTRL_REG1  0x2a
 #define  MMA8452_CTRL_ACTIVE   BIT(0)
 #define  MMA8452_CTRL_DR_MASK  GENMASK(5, 3)
@@ -91,6 +104,7 @@
 
 #define  MMA8452_INT_DRDY  BIT(0)
 #define  MMA8452_INT_FF_MT BIT(2)
+#define  MMA8452_INT_PULSE BIT(3)
 #define  MMA8452_INT_TRANS BIT(5)
 
 #define MMA8451_DEVICE_ID  0x1a
@@ -155,6 +169,16 @@ static const struct mma8452_event_regs trans_ev_regs = {
.ev_count = MMA8452_TRANSIENT_COUNT,
 };
 
+static const struct mma8452_event_regs pulse_ev_regs = {
+   .ev_cfg = MMA8452_PULSE_CFG,
+   .ev_cfg_ele = MMA8452_PULSE_CFG_ELE,
+   .ev_cfg_chan_shift = MMA8452_PULSE_CHAN_SHIFT,
+   .ev_src = MMA8452_PULSE_SRC,
+   .ev_ths = MMA8452_PULSE_THS,
+   .ev_ths_mask = MMA8452_PULSE_THS_MASK,
+   .ev_count = MMA8452_PULSE_COUNT,
+};
+
 /**
  * struct mma_chip_info - chip specific data
  * @chip_id:   WHO_AM_I register's value
@@ -784,6 +808,14 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
case IIO_EV_DIR_FALLING:
*ev_reg = _mt_ev_regs;
return 0;
+   case IIO_EV_DIR_EITHER:
+   if (!(data->chip_info->all_events
+   & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events
+   & MMA8452_INT_PULSE))
+   return -EINVAL;
+   *ev_reg = _ev_regs;
+   return 0;
default:
return -EINVAL;
}
@@ -848,6 +880,25 @@ static int mma8452_read_event_value(struct iio_dev 
*indio_dev,
return ret;
}
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8452_INT_PULSE) ||
+   !(data->chip_info->enabled_events & MMA8452_INT_PULSE))
+   return -EINVAL;
+
+   ret = i2c_smbus_read_byte_data(data->client,
+   MMA8452_PULSE_LTCY);
+   if (ret < 0)
+   return ret;
+
+   power_mode = mma8452_get_power_mode(data);
+   if (power_mode < 0)
+   return power_mode;
+
+   us = ret * mma8452_time_step_us[power_mode][
+   mma8452_get_odr_index(data)];
+   *val = us / USEC_PER_SEC;
+   *val2 = us % USEC_PER_SEC;
+
return IIO_VAL_INT_PLUS_MICRO;
 
default:
@@ -908,6 +959,24 @@ static int mma8452_write_event_value(struct iio_dev 
*indio_dev,
 
return mma8452_change_config(data, MMA8452_TRANSIENT_CFG, reg);
 
+   case IIO_EV_INFO_HYSTERESIS:
+   if (!(data->chip_info->all_events & MMA8

[PATCH] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.

[Harinath Nampally]

Improves code readability, no impact on functionality.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 6194169..3472e7e 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -135,7 +135,7 @@ struct mma8452_event_regs {
u8 ev_count;
 };
 
-static const struct mma8452_event_regs ev_regs_accel_falling = {
+static const struct mma8452_event_regs ff_mt_ev_regs = {
.ev_cfg = MMA8452_FF_MT_CFG,
.ev_cfg_ele = MMA8452_FF_MT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_FF_MT_CHAN_SHIFT,
@@ -145,7 +145,7 @@ static const struct mma8452_event_regs 
ev_regs_accel_falling = {
.ev_count = MMA8452_FF_MT_COUNT
 };
 
-static const struct mma8452_event_regs ev_regs_accel_rising = {
+static const struct mma8452_event_regs trans_ev_regs = {
.ev_cfg = MMA8452_TRANSIENT_CFG,
.ev_cfg_ele = MMA8452_TRANSIENT_CFG_ELE,
.ev_cfg_chan_shift = MMA8452_TRANSIENT_CHAN_SHIFT,
@@ -777,12 +777,12 @@ static int mma8452_get_event_regs(struct mma8452_data 
*data,
& MMA8452_INT_TRANS) &&
(data->chip_info->enabled_events
& MMA8452_INT_TRANS))
-   *ev_reg = _regs_accel_rising;
+   *ev_reg = _ev_regs;
else
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
-   *ev_reg = _regs_accel_falling;
+   *ev_reg = _mt_ev_regs;
return 0;
default:
return -EINVAL;
-- 
2.7.4

[PATCH 0/2] This patchset is for fixes reported by checkpatch.pl

[Harinath Nampally]

Please find the following patches:
  iio: accel: mma8452: Fix code style warning for symbolic permissions
  iio: accel: mma8452: Fix code style warning for unsigned int
declarations

 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 0/2] This patchset is for fixes reported by checkpatch.pl

[Harinath Nampally]

Please find the following patches:
  iio: accel: mma8452: Fix code style warning for symbolic permissions
  iio: accel: mma8452: Fix code style warning for unsigned int
declarations

 drivers/iio/accel/mma8452.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH 2/2] iio: accel: mma8452: Fix code style warning for unsigned int declarations

[Harinath Nampally]

Replace 'unsigned' with 'unsigned int'
to improve code readability.

Issue found by checkpatch.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 4a33a26..6194169 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1088,8 +1088,8 @@ static irqreturn_t mma8452_trigger_handler(int irq, void 
*p)
 }
 
 static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
- unsigned reg, unsigned writeval,
- unsigned *readval)
+ unsigned int reg, unsigned int writeval,
+ unsigned int *readval)
 {
int ret;
struct mma8452_data *data = iio_priv(indio_dev);
-- 
2.7.4

[PATCH 1/2] iio: accel: mma8452: Fix code style warning

[Harinath Nampally]

Replace symbolic permissions with octal permissions
to improve code readability.

Issue found by checkpatch.

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index c352555..4a33a26 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -418,11 +418,11 @@ static ssize_t mma8452_show_os_ratio_avail(struct device 
*dev,
 }
 
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
-static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
+static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
   mma8452_show_scale_avail, NULL, 0);
 static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
-  S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
-static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, S_IRUGO,
+  0444, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, 0444,
   mma8452_show_os_ratio_avail, NULL, 0);
 
 static int mma8452_get_samp_freq_index(struct mma8452_data *data,
-- 
2.7.4

[PATCH 2/2] iio: accel: mma8452: Fix code style warning for unsigned int declarations

[Harinath Nampally]

Replace 'unsigned' with 'unsigned int'
to improve code readability.

Issue found by checkpatch.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 4a33a26..6194169 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1088,8 +1088,8 @@ static irqreturn_t mma8452_trigger_handler(int irq, void 
*p)
 }
 
 static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
- unsigned reg, unsigned writeval,
- unsigned *readval)
+ unsigned int reg, unsigned int writeval,
+ unsigned int *readval)
 {
int ret;
struct mma8452_data *data = iio_priv(indio_dev);
-- 
2.7.4

[PATCH 1/2] iio: accel: mma8452: Fix code style warning

[Harinath Nampally]

Replace symbolic permissions with octal permissions
to improve code readability.

Issue found by checkpatch.

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index c352555..4a33a26 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -418,11 +418,11 @@ static ssize_t mma8452_show_os_ratio_avail(struct device 
*dev,
 }
 
 static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);
-static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
+static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
   mma8452_show_scale_avail, NULL, 0);
 static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
-  S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
-static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, S_IRUGO,
+  0444, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, 0444,
   mma8452_show_os_ratio_avail, NULL, 0);
 
 static int mma8452_get_samp_freq_index(struct mma8452_data *data,
-- 
2.7.4

Re: [PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> Alright, I didn't test it but I kind of like it now. The one minor
> naming issue I had pointed out before is mentioned below. But if that's
> no issue for Jon:
> Reviewed-by: Martin Kepplinger <mart...@posteo.de>
Martin, Thanks a lot for the review.

> btw, Harianath: Would you point me to the imx6ul eval board you are
> using? thanks
https://www.nxp.com/products/microcontrollers-and-processors/arm-based-processors-and-mcus/i.mx-applications-processors/developer-resources/i.mx6ultralite-evaluation-kit:MCIMX6UL-EVK

> > @@ -740,6 +762,36 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
> >   return ret;
> >  }
> >
> > +static int mma8452_get_event_regs(struct mma8452_data *data,
> > + const struct iio_chan_spec *chan, enum iio_event_direction 
> > dir,
> > + const struct mma8452_event_regs **ev_reg)
> > +{
> > + if (!chan)
> > + return -EINVAL;
> > +
> > + switch (chan->type) {
> > + case IIO_ACCEL:
> > + switch (dir) {
> > + case IIO_EV_DIR_RISING:
> > + if ((data->chip_info->all_events
> > + & MMA8452_INT_TRANS) &&
> > + (data->chip_info->enabled_events
> > + & MMA8452_INT_TRANS))
> > + *ev_reg = _regs_accel_rising;
> > + else
> > + *ev_reg = _regs_accel_falling;
> > + return 0;
> I know it's fine, only the naming seems odd here.
I agree, I will replace 'ev_regs_accel_rising' to 'ev_trans_regs'
and 'ev_regs_accel_falling' to 'ev_ff_mt_regs'.
As Jon just applied this patch, I will cover this in my next patch set
which fix the checkpatch.pl warnings in this file.

Thanks,
Hari


On Sun, Sep 10, 2017 at 2:36 AM, Martin Kepplinger <mart...@posteo.de> wrote:
>
> On 2017-09-09 21:56, 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 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.
> >
> > Changes since v5 -> v6
> > -Rename supported_events to all_events(short name)
> > -Use get_event_regs function in read/write event
> >  config callbacks to pick appropriate config registers
> > -Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
> >  which are needed in read/write event callbacks
> >
> > Changes since v4 -> v5
> > -Add supported_events and enabled_events
> >  in chip_info structure so that devices(mma865x)
> >  which has no support for transient event will
> >  fallback to freefall event. Hence this patch changes
> >  won't break for devices that can't support
> >  transient events
> >
> > Changes since v3 -> v4
> > -Add 'const struct ev_regs_accel_falling'
> > -Add 'const struct ev_regs_accel_rising'
> > -Refactor mma8452_get_event_regs function to
> >  remove the fill in the struct and return above structs
> > -Condense the commit's subject message
> >
> > 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 red

Re: [PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> Alright, I didn't test it but I kind of like it now. The one minor
> naming issue I had pointed out before is mentioned below. But if that's
> no issue for Jon:
> Reviewed-by: Martin Kepplinger 
Martin, Thanks a lot for the review.

> btw, Harianath: Would you point me to the imx6ul eval board you are
> using? thanks
https://www.nxp.com/products/microcontrollers-and-processors/arm-based-processors-and-mcus/i.mx-applications-processors/developer-resources/i.mx6ultralite-evaluation-kit:MCIMX6UL-EVK

> > @@ -740,6 +762,36 @@ static int mma8452_write_raw(struct iio_dev *indio_dev,
> >   return ret;
> >  }
> >
> > +static int mma8452_get_event_regs(struct mma8452_data *data,
> > + const struct iio_chan_spec *chan, enum iio_event_direction 
> > dir,
> > + const struct mma8452_event_regs **ev_reg)
> > +{
> > + if (!chan)
> > + return -EINVAL;
> > +
> > + switch (chan->type) {
> > + case IIO_ACCEL:
> > + switch (dir) {
> > + case IIO_EV_DIR_RISING:
> > + if ((data->chip_info->all_events
> > + & MMA8452_INT_TRANS) &&
> > + (data->chip_info->enabled_events
> > + & MMA8452_INT_TRANS))
> > + *ev_reg = _regs_accel_rising;
> > + else
> > + *ev_reg = _regs_accel_falling;
> > + return 0;
> I know it's fine, only the naming seems odd here.
I agree, I will replace 'ev_regs_accel_rising' to 'ev_trans_regs'
and 'ev_regs_accel_falling' to 'ev_ff_mt_regs'.
As Jon just applied this patch, I will cover this in my next patch set
which fix the checkpatch.pl warnings in this file.

Thanks,
Hari


On Sun, Sep 10, 2017 at 2:36 AM, Martin Kepplinger  wrote:
>
> On 2017-09-09 21:56, 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 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.
> >
> > Changes since v5 -> v6
> > -Rename supported_events to all_events(short name)
> > -Use get_event_regs function in read/write event
> >  config callbacks to pick appropriate config registers
> > -Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
> >  which are needed in read/write event callbacks
> >
> > Changes since v4 -> v5
> > -Add supported_events and enabled_events
> >  in chip_info structure so that devices(mma865x)
> >  which has no support for transient event will
> >  fallback to freefall event. Hence this patch changes
> >  won't break for devices that can't support
> >  transient events
> >
> > Changes since v3 -> v4
> > -Add 'const struct ev_regs_accel_falling'
> > -Add 'const struct ev_regs_accel_rising'
> > -Refactor mma8452_get_event_regs function to
> >  remove the fill in the struct and return above structs
> > -Condense the commit's subject message
> >
> > 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 u

Re: [PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> I stripped the version change stuff from the commit message - they
> should have been below the ---  Useful during review, but generally
> not worth retaining once we have accepted it.

I didn't know that, thanks for letting me know.
Next time I will keep that in mind.

Thanks,
Hari

On Sun, Sep 10, 2017 at 11:51 AM, Jonathan Cameron <ji...@kernel.org> wrote:
> On Sun, 10 Sep 2017 08:36:43 +0200
> Martin Kepplinger <mart...@posteo.de> wrote:
>
>> On 2017-09-09 21:56, 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 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.
>> >
>> > Changes since v5 -> v6
>> > -Rename supported_events to all_events(short name)
>> > -Use get_event_regs function in read/write event
>> >  config callbacks to pick appropriate config registers
>> > -Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
>> >  which are needed in read/write event callbacks
>> >
>> > Changes since v4 -> v5
>> > -Add supported_events and enabled_events
>> >  in chip_info structure so that devices(mma865x)
>> >  which has no support for transient event will
>> >  fallback to freefall event. Hence this patch changes
>> >  won't break for devices that can't support
>> >  transient events
>> >
>> > Changes since v3 -> v4
>> > -Add 'const struct ev_regs_accel_falling'
>> > -Add 'const struct ev_regs_accel_rising'
>> > -Refactor mma8452_get_event_regs function to
>> >  remove the fill in the struct and return above structs
>> > -Condense the commit's subject message
>> >
>> > 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 <harinath...@gmail.com>
>> > ---
>>
>> Alright, I didn't test it but I kind of like it now. The one minor
>> naming issue I had pointed out before is mentioned below. But if that's
>> no issue for Jon:
>>
>> Reviewed-by: Martin Kepplinger <mart...@posteo.de>
>>
> Applied to the togreg branch of iio.git - pushed out as testing to
> let the autobuilders play with it.
>
> I stripped the version change stuff from the commit message - they
> should have been below the ---  Useful during review, but generally
> not worth retaining once we have accepted it.
>
> Thanks,
>
> Jonathan
>>
>> btw, Harianath: Would you point me to the imx6ul eval board you are
>> using? thanks
>>
>> > @@ -740,6 +762,36 @@ static int mma8452_write_raw(struct iio_dev 
>> > *indio_dev,
>> > return ret;
>> >  }
>> >
>> > +static int mma8452_get_event_regs(struct mma8452_data *data,
>> > +   const struct iio_chan_spec *chan, enum iio_event_direction dir,
>> > +   const struct mma8452_event_regs **ev_reg)
>>

Re: [PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> I stripped the version change stuff from the commit message - they
> should have been below the ---  Useful during review, but generally
> not worth retaining once we have accepted it.

I didn't know that, thanks for letting me know.
Next time I will keep that in mind.

Thanks,
Hari

On Sun, Sep 10, 2017 at 11:51 AM, Jonathan Cameron  wrote:
> On Sun, 10 Sep 2017 08:36:43 +0200
> Martin Kepplinger  wrote:
>
>> On 2017-09-09 21:56, 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 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.
>> >
>> > Changes since v5 -> v6
>> > -Rename supported_events to all_events(short name)
>> > -Use get_event_regs function in read/write event
>> >  config callbacks to pick appropriate config registers
>> > -Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
>> >  which are needed in read/write event callbacks
>> >
>> > Changes since v4 -> v5
>> > -Add supported_events and enabled_events
>> >  in chip_info structure so that devices(mma865x)
>> >  which has no support for transient event will
>> >  fallback to freefall event. Hence this patch changes
>> >  won't break for devices that can't support
>> >  transient events
>> >
>> > Changes since v3 -> v4
>> > -Add 'const struct ev_regs_accel_falling'
>> > -Add 'const struct ev_regs_accel_rising'
>> > -Refactor mma8452_get_event_regs function to
>> >  remove the fill in the struct and return above structs
>> > -Condense the commit's subject message
>> >
>> > 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 
>> > ---
>>
>> Alright, I didn't test it but I kind of like it now. The one minor
>> naming issue I had pointed out before is mentioned below. But if that's
>> no issue for Jon:
>>
>> Reviewed-by: Martin Kepplinger 
>>
> Applied to the togreg branch of iio.git - pushed out as testing to
> let the autobuilders play with it.
>
> I stripped the version change stuff from the commit message - they
> should have been below the ---  Useful during review, but generally
> not worth retaining once we have accepted it.
>
> Thanks,
>
> Jonathan
>>
>> btw, Harianath: Would you point me to the imx6ul eval board you are
>> using? thanks
>>
>> > @@ -740,6 +762,36 @@ static int mma8452_write_raw(struct iio_dev 
>> > *indio_dev,
>> > return ret;
>> >  }
>> >
>> > +static int mma8452_get_event_regs(struct mma8452_data *data,
>> > +   const struct iio_chan_spec *chan, enum iio_event_direction dir,
>> > +   const struct mma8452_event_regs **ev_reg)
>> > +{
>> > +   if (!chan)
>> > +   return -EINVAL;
>> > +
>> > +   switch (cha

[PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v5 -> v6
-Rename supported_events to all_events(short name)
-Use get_event_regs function in read/write event
 config callbacks to pick appropriate config registers
-Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
 which are needed in read/write event callbacks

Changes since v4 -> v5
-Add supported_events and enabled_events
 in chip_info structure so that devices(mma865x)
 which has no support for transient event will
 fallback to freefall event. Hence this patch changes
 won't break for devices that can't support
 transient events

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 363 +---
 1 file changed, 206 insertions(+), 157 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..678e2f7 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,51 @@ 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_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_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_cfg_ele;
+   u8 ev_cfg_chan_shift;
+   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_cfg_ele = 

[PATCH v6] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v5 -> v6
-Rename supported_events to all_events(short name)
-Use get_event_regs function in read/write event
 config callbacks to pick appropriate config registers
-Add ev_cfg_ele and ev_cfg_chan_shift in event_regs struct
 which are needed in read/write event callbacks

Changes since v4 -> v5
-Add supported_events and enabled_events
 in chip_info structure so that devices(mma865x)
 which has no support for transient event will
 fallback to freefall event. Hence this patch changes
 won't break for devices that can't support
 transient events

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 | 363 +---
 1 file changed, 206 insertions(+), 157 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..678e2f7 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,51 @@ 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_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_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_cfg_ele;
+   u8 ev_cfg_chan_shift;
+   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_cfg_ele = MMA8452_FF_MT_CFG_ELE,
+   .ev

Re: [PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> I agree with your understanding.  It's a rising threshold, just that the input
> will only reflect high frequency changes in the signal.
Thank you for the clarification. I am hoping this gets merged in the
next window if no other issues.

Thanks,
Hari

On Sun, Sep 3, 2017 at 12:24 PM, Jonathan Cameron <ji...@kernel.org> wrote:
> On Tue, 29 Aug 2017 23:01:16 -0400
> harinath Nampally <harinath...@gmail.com> wrote:
>
>> > We should never say "transient is for rising
>> > direction" or "ff_mt is for falling direction". any combination is fine.
>>
>> Ok I agree that there is no hard and fast rule that "transient is for rising
>> direction" or "ff_mt is for falling direction".
>> But in our case, datasheet for these chips define these events based on
>> acceleration magnitude rising or falling below a set threshold value.
>>
>> For quick reference, below excerpts are from fxls8471 datasheet:
>> Motion Event: "When the acceleration exceeds a set threshold for a set
>> amount of time,
>> the motion interrupt is asserted."
>>
>> Freefall event: "The detection of “Freefall” involves the monitoring
>> of the X, Y, and Z axes
>> for the condition where the acceleration magnitude is below a
>> user-specified threshold
>> for a user-definable amount of time"
>>
>> Transient event: "When the high-pass filter is bypassed, the
>> functionality becomes
>> similar to the motion-detection function; in this mode, acceleration
>> greater than
>> a programmable threshold is detected (along an axis)."
>>
>> Therefore I think in this driver freefall event is defined as
>> 'falling' event type and
>> motion event is defined as 'rising' event type and Transient is also defined 
>> as
>> 'rising' event type.
>>  As you might already know that mma8562 and mma8563 doesn't have
>> transient event support
>> but they do have freefall and motion event support which are defined
>> as 'fall' and 'rise'
>> event types respectively. Please note in this driver, motion event is
>> enabled/configured only
>> for mma8652 and mma8653.
>> Therefore if I read/write sysfs node for 'rise' it should use the
>> FF_MT registers for mma8652 and mma853, but for all others like
>> mma8451, mma8452 and
>> mma8453 which has transient event support it picks the Transient
>> registers if enabled. Also please
>> note transient event is enabled(but not motion event) for mma8451,
>> mma8452 and mma8453.
>> The problem seems like we have two different events(motion and
>> transient) that are defined
>> as same event type 'rising' but in fact both motion and transient are
>> pretty much similar as they
>> both raise interrupt flag when the acceleration magnitude rises above
>> the threshold.
>> Only difference is transient event has its own event config registers
>> with High pass filter.
>> If HPF bypassed using config register transient event acts like motion
>> detection event.
>
>>
>> That was my understanding but please correct me if I am wrong.
>
> I agree with your understanding.  It's a rising threshold, just that the input
> will only reflect high frequency changes in the signal.
>
>>
>> > Only freefall mode needs one fix: remembering to which set of registers to 
>> > fall back when
>> > disabling it.
>>
>> I don't quite understand what you mean by 'to fall back when disabling
>> it'. Please elaborate. I would
>> appreciate if you could suggest your logic in the form of pseudo-code.
>> Thanks for your time
>>
> ...

Re: [PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> I agree with your understanding.  It's a rising threshold, just that the input
> will only reflect high frequency changes in the signal.
Thank you for the clarification. I am hoping this gets merged in the
next window if no other issues.

Thanks,
Hari

On Sun, Sep 3, 2017 at 12:24 PM, Jonathan Cameron  wrote:
> On Tue, 29 Aug 2017 23:01:16 -0400
> harinath Nampally  wrote:
>
>> > We should never say "transient is for rising
>> > direction" or "ff_mt is for falling direction". any combination is fine.
>>
>> Ok I agree that there is no hard and fast rule that "transient is for rising
>> direction" or "ff_mt is for falling direction".
>> But in our case, datasheet for these chips define these events based on
>> acceleration magnitude rising or falling below a set threshold value.
>>
>> For quick reference, below excerpts are from fxls8471 datasheet:
>> Motion Event: "When the acceleration exceeds a set threshold for a set
>> amount of time,
>> the motion interrupt is asserted."
>>
>> Freefall event: "The detection of “Freefall” involves the monitoring
>> of the X, Y, and Z axes
>> for the condition where the acceleration magnitude is below a
>> user-specified threshold
>> for a user-definable amount of time"
>>
>> Transient event: "When the high-pass filter is bypassed, the
>> functionality becomes
>> similar to the motion-detection function; in this mode, acceleration
>> greater than
>> a programmable threshold is detected (along an axis)."
>>
>> Therefore I think in this driver freefall event is defined as
>> 'falling' event type and
>> motion event is defined as 'rising' event type and Transient is also defined 
>> as
>> 'rising' event type.
>>  As you might already know that mma8562 and mma8563 doesn't have
>> transient event support
>> but they do have freefall and motion event support which are defined
>> as 'fall' and 'rise'
>> event types respectively. Please note in this driver, motion event is
>> enabled/configured only
>> for mma8652 and mma8653.
>> Therefore if I read/write sysfs node for 'rise' it should use the
>> FF_MT registers for mma8652 and mma853, but for all others like
>> mma8451, mma8452 and
>> mma8453 which has transient event support it picks the Transient
>> registers if enabled. Also please
>> note transient event is enabled(but not motion event) for mma8451,
>> mma8452 and mma8453.
>> The problem seems like we have two different events(motion and
>> transient) that are defined
>> as same event type 'rising' but in fact both motion and transient are
>> pretty much similar as they
>> both raise interrupt flag when the acceleration magnitude rises above
>> the threshold.
>> Only difference is transient event has its own event config registers
>> with High pass filter.
>> If HPF bypassed using config register transient event acts like motion
>> detection event.
>
>>
>> That was my understanding but please correct me if I am wrong.
>
> I agree with your understanding.  It's a rising threshold, just that the input
> will only reflect high frequency changes in the signal.
>
>>
>> > Only freefall mode needs one fix: remembering to which set of registers to 
>> > fall back when
>> > disabling it.
>>
>> I don't quite understand what you mean by 'to fall back when disabling
>> it'. Please elaborate. I would
>> appreciate if you could suggest your logic in the form of pseudo-code.
>> Thanks for your time
>>
> ...

Re: [PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> We should never say "transient is for rising
> direction" or "ff_mt is for falling direction". any combination is fine.

Ok I agree that there is no hard and fast rule that "transient is for rising
direction" or "ff_mt is for falling direction".
But in our case, datasheet for these chips define these events based on
acceleration magnitude rising or falling below a set threshold value.

For quick reference, below excerpts are from fxls8471 datasheet:
Motion Event: "When the acceleration exceeds a set threshold for a set
amount of time,
the motion interrupt is asserted."

Freefall event: "The detection of “Freefall” involves the monitoring
of the X, Y, and Z axes
for the condition where the acceleration magnitude is below a
user-specified threshold
for a user-definable amount of time"

Transient event: "When the high-pass filter is bypassed, the
functionality becomes
similar to the motion-detection function; in this mode, acceleration
greater than
a programmable threshold is detected (along an axis)."

Therefore I think in this driver freefall event is defined as
'falling' event type and
motion event is defined as 'rising' event type and Transient is also defined as
'rising' event type.
 As you might already know that mma8562 and mma8563 doesn't have
transient event support
but they do have freefall and motion event support which are defined
as 'fall' and 'rise'
event types respectively. Please note in this driver, motion event is
enabled/configured only
for mma8652 and mma8653.
Therefore if I read/write sysfs node for 'rise' it should use the
FF_MT registers for mma8652 and mma853, but for all others like
mma8451, mma8452 and
mma8453 which has transient event support it picks the Transient
registers if enabled. Also please
note transient event is enabled(but not motion event) for mma8451,
mma8452 and mma8453.
The problem seems like we have two different events(motion and
transient) that are defined
as same event type 'rising' but in fact both motion and transient are
pretty much similar as they
both raise interrupt flag when the acceleration magnitude rises above
the threshold.
Only difference is transient event has its own event config registers
with High pass filter.
If HPF bypassed using config register transient event acts like motion
detection event.

That was my understanding but please correct me if I am wrong.

> Only freefall mode needs one fix: remembering to which set of registers to 
> fall back when
> disabling it.

I don't quite understand what you mean by 'to fall back when disabling
it'. Please elaborate. I would
appreciate if you could suggest your logic in the form of pseudo-code.
Thanks for your time

On Tue, Aug 29, 2017 at 10:55 PM, harinath Nampally
<harinath...@gmail.com> wrote:
>> We should never say "transient is for rising
>> direction" or "ff_mt is for falling direction". any combination is fine.
>
> Ok I agree that there is no hard and fast rule that "transient is for rising
> direction" or "ff_mt is for falling direction".
> But in our case, datasheet for these chips define these events based on
> acceleration magnitude rising or falling below a set threshold value.
>
> For quick reference, below excerpts are from fxls8471 datasheet:
> Motion Event: "When the acceleration exceeds a set threshold for a set
> amount of time,
> the motion interrupt is asserted."
>
> Freefall event: "The detection of “Freefall” involves the monitoring of the
> X, Y, and Z axes
> for the condition where the acceleration magnitude is below a user-specified
> threshold
> for a user-definable amount of time"
>
> Transient event: "When the high-pass filter is bypassed, the functionality
> becomes
> similar to the motion-detection function; in this mode, acceleration greater
> than
> a programmable threshold is detected (along an axis)."
>
> Therefore I think in this driver freefall event is defined as 'falling'
> event type and
> motion event is defined as 'rising' event type and Transient is also defined
> as
> 'rising' event type.
>  As you might already know that mma8562 and mma8563 doesn't have transient
> event support
> but they do have freefall and motion event support which are defined as
> 'fall' and 'rise'
> event types respectively. Please note in this driver, motion event is
> enabled/configured only
> for mma8652 and mma8653.
> Therefore if I read/write sysfs node for 'rise' it should use the
> FF_MT registers for mma8652 and mma853, but for all others like mma8451,
> mma8452 and
> mma8453 which has transient event support it picks the Transient registers
> if enabled. Also please
> note transient event is enabled(but not motion event) for mma8451, mma8452
> and mma8453.
> The problem seems 

Re: [PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> We should never say "transient is for rising
> direction" or "ff_mt is for falling direction". any combination is fine.

Ok I agree that there is no hard and fast rule that "transient is for rising
direction" or "ff_mt is for falling direction".
But in our case, datasheet for these chips define these events based on
acceleration magnitude rising or falling below a set threshold value.

For quick reference, below excerpts are from fxls8471 datasheet:
Motion Event: "When the acceleration exceeds a set threshold for a set
amount of time,
the motion interrupt is asserted."

Freefall event: "The detection of “Freefall” involves the monitoring
of the X, Y, and Z axes
for the condition where the acceleration magnitude is below a
user-specified threshold
for a user-definable amount of time"

Transient event: "When the high-pass filter is bypassed, the
functionality becomes
similar to the motion-detection function; in this mode, acceleration
greater than
a programmable threshold is detected (along an axis)."

Therefore I think in this driver freefall event is defined as
'falling' event type and
motion event is defined as 'rising' event type and Transient is also defined as
'rising' event type.
 As you might already know that mma8562 and mma8563 doesn't have
transient event support
but they do have freefall and motion event support which are defined
as 'fall' and 'rise'
event types respectively. Please note in this driver, motion event is
enabled/configured only
for mma8652 and mma8653.
Therefore if I read/write sysfs node for 'rise' it should use the
FF_MT registers for mma8652 and mma853, but for all others like
mma8451, mma8452 and
mma8453 which has transient event support it picks the Transient
registers if enabled. Also please
note transient event is enabled(but not motion event) for mma8451,
mma8452 and mma8453.
The problem seems like we have two different events(motion and
transient) that are defined
as same event type 'rising' but in fact both motion and transient are
pretty much similar as they
both raise interrupt flag when the acceleration magnitude rises above
the threshold.
Only difference is transient event has its own event config registers
with High pass filter.
If HPF bypassed using config register transient event acts like motion
detection event.

That was my understanding but please correct me if I am wrong.

> Only freefall mode needs one fix: remembering to which set of registers to 
> fall back when
> disabling it.

I don't quite understand what you mean by 'to fall back when disabling
it'. Please elaborate. I would
appreciate if you could suggest your logic in the form of pseudo-code.
Thanks for your time

On Tue, Aug 29, 2017 at 10:55 PM, harinath Nampally
 wrote:
>> We should never say "transient is for rising
>> direction" or "ff_mt is for falling direction". any combination is fine.
>
> Ok I agree that there is no hard and fast rule that "transient is for rising
> direction" or "ff_mt is for falling direction".
> But in our case, datasheet for these chips define these events based on
> acceleration magnitude rising or falling below a set threshold value.
>
> For quick reference, below excerpts are from fxls8471 datasheet:
> Motion Event: "When the acceleration exceeds a set threshold for a set
> amount of time,
> the motion interrupt is asserted."
>
> Freefall event: "The detection of “Freefall” involves the monitoring of the
> X, Y, and Z axes
> for the condition where the acceleration magnitude is below a user-specified
> threshold
> for a user-definable amount of time"
>
> Transient event: "When the high-pass filter is bypassed, the functionality
> becomes
> similar to the motion-detection function; in this mode, acceleration greater
> than
> a programmable threshold is detected (along an axis)."
>
> Therefore I think in this driver freefall event is defined as 'falling'
> event type and
> motion event is defined as 'rising' event type and Transient is also defined
> as
> 'rising' event type.
>  As you might already know that mma8562 and mma8563 doesn't have transient
> event support
> but they do have freefall and motion event support which are defined as
> 'fall' and 'rise'
> event types respectively. Please note in this driver, motion event is
> enabled/configured only
> for mma8652 and mma8653.
> Therefore if I read/write sysfs node for 'rise' it should use the
> FF_MT registers for mma8652 and mma853, but for all others like mma8451,
> mma8452 and
> mma8453 which has transient event support it picks the Transient registers
> if enabled. Also please
> note transient event is enabled(but not motion event) for mma8451, mma8452
> and mma8453.
> The problem seems like we have two differe

[PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v4 -> v5
-Add supported_events and enabled_events
 in chip_info structure so that devices(mma865x)
 which has no support for transient event will
 fallback to freefall event. Hence this patch changes
 won't break for devices that can't support
 transient events

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 349 +++-
 1 file changed, 183 insertions(+), 166 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..0a97e61b 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

[PATCH v5] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v4 -> v5
-Add supported_events and enabled_events
 in chip_info structure so that devices(mma865x)
 which has no support for transient event will
 fallback to freefall event. Hence this patch changes
 won't break for devices that can't support
 transient events

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 | 349 +++-
 1 file changed, 183 insertions(+), 166 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..0a97e61b 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,16 @@ struct mma8452_data {
  *

Re: [PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> Am 23.08.2017 02:29 schrieb Harinath Nampally:
> If rising: use transient OR ff_mt device-dependent like before. But now save 
> it in a simple flag,
> whether transient registers are available.
> Ok, is it good idea to add the flag to struct mma_chip_info like below?
>   * @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
> + * @transient_supported:   flag indicating whether chip support transient
> + * event, as not all chips support transient 
> event
>   */
>  struct mma_chip_info {
> u8 chip_id;
> const struct iio_chan_spec *channels;
> int num_channels;
> const int mma_scales[3][2];
> +   bool transient_supported;
>  };
>
> I'd avoid boolean and use int and define EVENT_TYPE_TRANSIENT BIT(1) and
> EVENT_TYPE_FF_MT BIT(0) for example. So something like "supported_event_types"
> can have all types supported.

ok sure, I am thinking to adding 'int supported_event_types'(chip
supported events) and 'int enabled_event_types'(events enabled by this
driver for this chip). So in the probe method based on chip specific
'supported_event_types' and 'enabled_event_types' I can configure the
interrupt register accordingly.

> But this has quite some implications on your implementation, so your complete
> solution would be more interesting to see. Keep it simple and focus on only 
> this one
> issue of enabling freefall (FF_MT registers) for the devices that currently 
> use
> transient registers.

The main motivation of this patch was to add new events like tap and
orientation for fxls8471, So I would like to
make code changes such a way that it fixes the issue of enabling
freefall(FF_MT registers) for the devices
that currently use transient registers and also make the driver
flexible enough to add multiple new events.

Thanks,
Hari

On Wed, Aug 23, 2017 at 12:52 AM, Martin Kepplinger <mart...@posteo.de> wrote:
> Am 23.08.2017 02:29 schrieb Harinath Nampally:
>>>
>>>
>>> If rising: use transient OR ff_mt device-dependent like before. But now
>>> save it in a simple flag,
>>> whether transient registers are available.
>>
>> Ok, is it good idea to add the flag to struct mma_chip_info like below?
>>
>>   * @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
>> + * @transient_supported:   flag indicating whether chip support
>> transient
>> + * event, as not all chips support transient
>> event
>>   */
>>  struct mma_chip_info {
>> u8 chip_id;
>> const struct iio_chan_spec *channels;
>> int num_channels;
>> const int mma_scales[3][2];
>> +   bool transient_supported;
>>  };
>>
>
> I'd avoid boolean and use int and define EVENT_TYPE_TRANSIENT BIT(1) and
> EVENT_TYPE_FF_MT BIT(0) for example. So something like
> "supported_event_types"
> can have all types supported.
>
> But this has quite some implications on your implementation, so your
> complete
> solution would be more interesting to see. Keep it simple and focus on only
> this one
> issue of enabling freefall (FF_MT registers) for the devices that currently
> use
> transient registers.
>
> thanks
>
>
>>>
>>> If falling: switch to ff_mt in any case. (fixing freefall for the
>>> transient-devices)
>>
>> ok sure.
>>
>> Thanks,
>>
>> Hari
>>
>> On 08/21/2017 04:47 AM, Martin Kepplinger wrote:
>>>
>>>
>>> If rising: use transient OR ff_mt device-dependent like before. But now
>>> save it in a simple flag,
>>> whether transient registers are available.
>>>
>>> If falling: switch to ff_mt in any case. (fixing freefall for the
>>> transient-devices)
>
>

Re: [PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[harinath Nampally]

> Am 23.08.2017 02:29 schrieb Harinath Nampally:
> If rising: use transient OR ff_mt device-dependent like before. But now save 
> it in a simple flag,
> whether transient registers are available.
> Ok, is it good idea to add the flag to struct mma_chip_info like below?
>   * @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
> + * @transient_supported:   flag indicating whether chip support transient
> + * event, as not all chips support transient 
> event
>   */
>  struct mma_chip_info {
> u8 chip_id;
> const struct iio_chan_spec *channels;
> int num_channels;
> const int mma_scales[3][2];
> +   bool transient_supported;
>  };
>
> I'd avoid boolean and use int and define EVENT_TYPE_TRANSIENT BIT(1) and
> EVENT_TYPE_FF_MT BIT(0) for example. So something like "supported_event_types"
> can have all types supported.

ok sure, I am thinking to adding 'int supported_event_types'(chip
supported events) and 'int enabled_event_types'(events enabled by this
driver for this chip). So in the probe method based on chip specific
'supported_event_types' and 'enabled_event_types' I can configure the
interrupt register accordingly.

> But this has quite some implications on your implementation, so your complete
> solution would be more interesting to see. Keep it simple and focus on only 
> this one
> issue of enabling freefall (FF_MT registers) for the devices that currently 
> use
> transient registers.

The main motivation of this patch was to add new events like tap and
orientation for fxls8471, So I would like to
make code changes such a way that it fixes the issue of enabling
freefall(FF_MT registers) for the devices
that currently use transient registers and also make the driver
flexible enough to add multiple new events.

Thanks,
Hari

On Wed, Aug 23, 2017 at 12:52 AM, Martin Kepplinger  wrote:
> Am 23.08.2017 02:29 schrieb Harinath Nampally:
>>>
>>>
>>> If rising: use transient OR ff_mt device-dependent like before. But now
>>> save it in a simple flag,
>>> whether transient registers are available.
>>
>> Ok, is it good idea to add the flag to struct mma_chip_info like below?
>>
>>   * @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
>> + * @transient_supported:   flag indicating whether chip support
>> transient
>> + * event, as not all chips support transient
>> event
>>   */
>>  struct mma_chip_info {
>> u8 chip_id;
>> const struct iio_chan_spec *channels;
>> int num_channels;
>> const int mma_scales[3][2];
>> +   bool transient_supported;
>>  };
>>
>
> I'd avoid boolean and use int and define EVENT_TYPE_TRANSIENT BIT(1) and
> EVENT_TYPE_FF_MT BIT(0) for example. So something like
> "supported_event_types"
> can have all types supported.
>
> But this has quite some implications on your implementation, so your
> complete
> solution would be more interesting to see. Keep it simple and focus on only
> this one
> issue of enabling freefall (FF_MT registers) for the devices that currently
> use
> transient registers.
>
> thanks
>
>
>>>
>>> If falling: switch to ff_mt in any case. (fixing freefall for the
>>> transient-devices)
>>
>> ok sure.
>>
>> Thanks,
>>
>> Hari
>>
>> On 08/21/2017 04:47 AM, Martin Kepplinger wrote:
>>>
>>>
>>> If rising: use transient OR ff_mt device-dependent like before. But now
>>> save it in a simple flag,
>>> whether transient registers are available.
>>>
>>> If falling: switch to ff_mt in any case. (fixing freefall for the
>>> transient-devices)
>
>

Re: [PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[Harinath Nampally]

If rising: use transient OR ff_mt device-dependent like before. But 
now save it in a simple flag,

whether transient registers are available.

Ok, is it good idea to add the flag to struct mma_chip_info like below?

  * @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
+ * @transient_supported:   flag indicating whether chip support 
transient
+ * event, as not all chips support 
transient event

  */
 struct mma_chip_info {
u8 chip_id;
const struct iio_chan_spec *channels;
int num_channels;
const int mma_scales[3][2];
+   bool transient_supported;
 };



If falling: switch to ff_mt in any case. (fixing freefall for the 
transient-devices) 

ok sure.

Thanks,

Hari

On 08/21/2017 04:47 AM, Martin Kepplinger wrote:


If rising: use transient OR ff_mt device-dependent like before. But 
now save it in a simple flag,

whether transient registers are available.

If falling: switch to ff_mt in any case. (fixing freefall for the 
transient-devices) 

Re: [PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[Harinath Nampally]

If rising: use transient OR ff_mt device-dependent like before. But 
now save it in a simple flag,

whether transient registers are available.

Ok, is it good idea to add the flag to struct mma_chip_info like below?

  * @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
+ * @transient_supported:   flag indicating whether chip support 
transient
+ * event, as not all chips support 
transient event

  */
 struct mma_chip_info {
u8 chip_id;
const struct iio_chan_spec *channels;
int num_channels;
const int mma_scales[3][2];
+   bool transient_supported;
 };



If falling: switch to ff_mt in any case. (fixing freefall for the 
transient-devices) 

ok sure.

Thanks,

Hari

On 08/21/2017 04:47 AM, Martin Kepplinger wrote:


If rising: use transient OR ff_mt device-dependent like before. But 
now save it in a simple flag,

whether transient registers are available.

If falling: switch to ff_mt in any case. (fixing freefall for the 
transient-devices) 

[PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 <harinath...@gmail.com>
---
 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 r

[PATCH v4] iio: accel: mma8452: improvements to handle multiple events

[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.

Changes since v3 -> v4
-Add 'const struct ev_regs_accel_falling'
-Add 'const struct ev_regs_accel_rising'
-Refactor mma8452_get_event_regs function to
 remove the fill in the struct and return above structs
-Condense the commit's subject message

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 | 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
- * @e

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

[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

[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.

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

[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 <harinath...@gmail.com>
---
 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

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

[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

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

[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 <harinath...@gmail.com>
---
 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 dif

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

[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 reg

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in
One small point. Don't put the word bugfix in the title (and fix
spelling of enable!).  I know this is obviously a false restriction
on the driver, but it doesn't not work, it is just limited in features
without this.

Sure, thanks for letting me know.


On 08/17/2017 10:40 AM, Jonathan Cameron wrote:

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

One small point. Don't put the word bugfix in the title (and fix
spelling of enable!).  I know this is obviously a false restriction
on the driver, but it doesn't not work, it is just limited in features
without this.

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in
One small point. Don't put the word bugfix in the title (and fix
spelling of enable!).  I know this is obviously a false restriction
on the driver, but it doesn't not work, it is just limited in features
without this.

Sure, thanks for letting me know.


On 08/17/2017 10:40 AM, Jonathan Cameron wrote:

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

One small point. Don't put the word bugfix in the title (and fix
spelling of enable!).  I know this is obviously a false restriction
on the driver, but it doesn't not work, it is just limited in features
without this.

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this
'supported events' field for each chip.
But I wonder should I add those changes as part of this patch?
is it necessary or can it be documentation?
I think its not necessary as we only have Freefall and Transient events 
for now.

Ok I will just update the documentation.


And this patch should have been called "v2". please include a persistent 
version history to v3 of this patch.
Sure I will send v3 patch, should I use '--in-reply-to' option of git 
send-email to send v3 patch as reply to

original thread?

On 08/17/2017 07:24 AM, Martin Kepplinger wrote:

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this
'supported events' field for each chip.
But I wonder should I add those changes as part of this patch?

is it necessary or can it be documentation?

And this patch should have been called "v2". please include a persistent 
version history to v3 of this patch.

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this
'supported events' field for each chip.
But I wonder should I add those changes as part of this patch?
is it necessary or can it be documentation?
I think its not necessary as we only have Freefall and Transient events 
for now.

Ok I will just update the documentation.


And this patch should have been called "v2". please include a persistent 
version history to v3 of this patch.
Sure I will send v3 patch, should I use '--in-reply-to' option of git 
send-email to send v3 patch as reply to

original thread?

On 08/17/2017 07:24 AM, Martin Kepplinger wrote:

This patch fixes 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 multiple events can be handled in

read/write callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this
'supported events' field for each chip.
But I wonder should I add those changes as part of this patch?

is it necessary or can it be documentation?

And this patch should have been called "v2". please include a persistent 
version history to v3 of this patch.

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

Hello,

fixes are always welcome, some comments regarding the patch

Thanks for the review.


in subject: typo "enable"

Will fix it.

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 fixes 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 multiple events can be handled in read/write 
callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this 'supported 
events' field for each chip.
But I wonder should I add those changes as part of this patch?

  

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.

thanks, p.
  

Signed-off-by: Harinath Nampally <harinath...@gmail.com>
---
  drivers/iio/accel/mma8452.c | 309 
  1 file changed, 141 insertions(+), 168 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..1b83e52 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_MASK   0x7f
  #define MMA8452_FF_MT_COUNT   0x18
+#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_BYPBIT(0)
  #define  MMA8452_TRANSIENT_CFG_ELEBIT(4)
  #define MMA8452_TRANSIENT_SRC 0x1e
@@ -69,6 +71,7 @@
  #define MMA8452_TRANSIENT_THS 0x1f
  #define  MMA8452_TRANSIENT_THS_MASK   GENMASK(6, 0)
  #define MMA8452_TRANSIENT_COUNT   0x20
+#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

Re: [PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

Hello,

fixes are always welcome, some comments regarding the patch

Thanks for the review.


in subject: typo "enable"

Will fix it.

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 fixes 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 multiple events can be handled in read/write 
callbacks.

which chip can have which event(s)?

I am planning to add 'supported events' field in

struct mma_chip_info which indicates which chip can have which events.
During initialization in 'mma_chip_info_table' would set this 'supported 
events' field for each chip.
But I wonder should I add those changes as part of this patch?

  

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.

thanks, p.
  

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

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..1b83e52 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_MASK   0x7f
  #define MMA8452_FF_MT_COUNT   0x18
+#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_BYPBIT(0)
  #define  MMA8452_TRANSIENT_CFG_ELEBIT(4)
  #define MMA8452_TRANSIENT_SRC 0x1e
@@ -69,6 +71,7 @@
  #define MMA8452_TRANSIENT_THS 0x1f
  #define  MMA8452_TRANSIENT_THS_MASK   GENMASK(6, 0)
  #define MMA8452_TRANSIENT_COUNT   0x20
+#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

[PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[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 fixes 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 multiple 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 <harinath...@gmail.com>
---
 drivers/iio/accel/mma8452.c | 309 
 1 file changed, 141 insertions(+), 168 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..1b83e52 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 included here.
  */
 struct mma_chip_info {
u8 chip_id;
const struct iio_chan_spec *channels;
int num_channels;
const int mma_scales[3][2];
-   u8 ev_cfg;
-   u8 ev_cfg_ele;
-   u8 ev_cfg_chan_shift;
-   u8 ev_src;
-   u8 ev_src_xe;
-   u8 ev_src_ye;
-   u8 ev_src_ze;
-   u8 ev_ths;
-   u8 ev_ths_mask;
-   u8 ev_count;
 };
 
 enum {
@@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma845

[PATCH] iio: accel: mma8452: Bugfix to enbale and allow different events to work parallely.

[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 fixes 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 multiple 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 | 309 
 1 file changed, 141 insertions(+), 168 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index eb6e3dc..1b83e52 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 included here.
  */
 struct mma_chip_info {
u8 chip_id;
const struct iio_chan_spec *channels;
int num_channels;
const int mma_scales[3][2];
-   u8 ev_cfg;
-   u8 ev_cfg_ele;
-   u8 ev_cfg_chan_shift;
-   u8 ev_src;
-   u8 ev_src_xe;
-   u8 ev_src_ye;
-   u8 ev_src_ze;
-   u8 ev_ths;
-   u8 ev_ths_mask;
-   u8 ev_count;
 };
 
 enum {
@@ -602,9 +597,8 @@ static int mma8452_set_power_mode(struct mma8452_data 
*data, u8 mode

Re: [PATCH] iio: accel: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

My only suggestion for adding all these chips' orientation features, is
to start the discussion independently from this driver. Are there other
device series that provide such an orientation interrupt? Is it worth
finding a representation in iio?
Given the number of accelerometers these days have built in orientation 
event support,


I think its worth to have a representation in IIO,


Additionally to portait up/down, landscape left/right there is
back/front facing, so you'd have 8 new channel modifiers.
Yes that's correct but I wonder if its good idea to add 8(too many!) new 
channel modifiers.

If IIO_ROT is a current userspace "standard" to read for rotating the
screen, it may be worth discussing how to fit this in without new
modifiers. Would you have to make up fake angle values? Anything else
userspace already uses for getting the orientation?

Yes I agree, I don't think I need to make up fake angle values, not sure
how userspace gets orientation currently. Need to do some research on that.

But again, instead of replying here and going off topic, write up a
proposal and post it independently.

Sure will do that. Thanks for your response.

On 08/01/2017 11:50 AM, Martin Kepplinger wrote:

On 2017-08-01 05:08, Harinath Nampally wrote:

Thanks for doing that work. I have had it on my list for a long time
and you seem to fix it. Although I'd happily review and possibly test
it, unfortunately I can't do so before the week of August 21st.

If this might go in quick, nothing will stop me from reviewing either,
so, whatever. Thanks again!

   Sure no problem, looking forward to your review comments.
   Actually I am planning to add Orientation events for FXLS8471Q, for
that is it good idea to overload existing
   IIO_ROT channel type? Also thinking of adding 4 channel modifiers i.e
portrait up/down, landscape left/right.
   Any suggestions are welcome. Thank you.


My only suggestion for adding all these chips' orientation features, is
to start the discussion independently from this driver. Are there other
device series that provide such an orientation interrupt? Is it worth
finding a representation in iio?

Additionally to portait up/down, landscape left/right there is
back/front facing, so you'd have 8 new channel modifiers.

If IIO_ROT is a current userspace "standard" to read for rotating the
screen, it may be worth discussing how to fit this in without new
modifiers. Would you have to make up fake angle values? Anything else
userspace already uses for getting the orientation?

But again, instead of replying here and going off topic, write up a
proposal and post it independently.

Re: [PATCH] iio: accel: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

My only suggestion for adding all these chips' orientation features, is
to start the discussion independently from this driver. Are there other
device series that provide such an orientation interrupt? Is it worth
finding a representation in iio?
Given the number of accelerometers these days have built in orientation 
event support,


I think its worth to have a representation in IIO,


Additionally to portait up/down, landscape left/right there is
back/front facing, so you'd have 8 new channel modifiers.
Yes that's correct but I wonder if its good idea to add 8(too many!) new 
channel modifiers.

If IIO_ROT is a current userspace "standard" to read for rotating the
screen, it may be worth discussing how to fit this in without new
modifiers. Would you have to make up fake angle values? Anything else
userspace already uses for getting the orientation?

Yes I agree, I don't think I need to make up fake angle values, not sure
how userspace gets orientation currently. Need to do some research on that.

But again, instead of replying here and going off topic, write up a
proposal and post it independently.

Sure will do that. Thanks for your response.

On 08/01/2017 11:50 AM, Martin Kepplinger wrote:

On 2017-08-01 05:08, Harinath Nampally wrote:

Thanks for doing that work. I have had it on my list for a long time
and you seem to fix it. Although I'd happily review and possibly test
it, unfortunately I can't do so before the week of August 21st.

If this might go in quick, nothing will stop me from reviewing either,
so, whatever. Thanks again!

   Sure no problem, looking forward to your review comments.
   Actually I am planning to add Orientation events for FXLS8471Q, for
that is it good idea to overload existing
   IIO_ROT channel type? Also thinking of adding 4 channel modifiers i.e
portrait up/down, landscape left/right.
   Any suggestions are welcome. Thank you.


My only suggestion for adding all these chips' orientation features, is
to start the discussion independently from this driver. Are there other
device series that provide such an orientation interrupt? Is it worth
finding a representation in iio?

Additionally to portait up/down, landscape left/right there is
back/front facing, so you'd have 8 new channel modifiers.

If IIO_ROT is a current userspace "standard" to read for rotating the
screen, it may be worth discussing how to fit this in without new
modifiers. Would you have to make up fake angle values? Anything else
userspace already uses for getting the orientation?

But again, instead of replying here and going off topic, write up a
proposal and post it independently.

Re: [PATCH] iio: accel: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

On Mon, 31 Jul 2017 07:17:38 -0400
Harinath Nampally <harinath...@gmail.com> 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 fixes 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 multiple 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 <harinath...@gmail.com>

Hi,

A few minor bits and bobs inline.

Jonathan

Thank you for the review.


+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @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 included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_cfg_ele;
+   u8 ev_cfg_chan_shift;
As far as I can tell the above isn't used...
Please sanity check the others

Yes they are not used and not really necessary I think, probably I 
should remove them!
It makes sense to only have ev_cfg, ev_src, ev_ths, ev_ths_mask and 
ev_count.
as they are common to other events as well like orientation, 
single/double tap etc.

So in future this same struct can be reused across different events.

  enum {
@@ -394,11 +403,11 @@ static ssize_t mma8452_show_os_ratio_avail(struct device 
*dev,
  }
  
  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);

-static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
+static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
   mma8452_show_scale_avail, NULL, 0);
  static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
-  S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
-static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, S_IRUGO,
+  0444, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, 0444,
   mma8452_show_os_ratio_avail, NULL, 0);
Separate change.  Please do it in a precursor patch rather than
adding noise to this one..

Sure I will.

case IIO_EV_INFO_PERIOD:
ret = i2c_smbus_read_byte_data(data->client,
-  data->chip_info->ev_count);
+   
ev_regs.ev_count);
This indenting looks somewhat odd..

Yes I agree, will fix it.

+   switch (chan->type) {
+   case IIO_ACCEL:
+   switch (dir) {
+   case IIO_EV_DIR_FALLING:
+   return mma8452_freefall_mode_enabled(data);
+   case IIO_EV_DIR_RISING:
+   ret = i2c_smbus_read_byte_data(data->client,
+   
MMA8452_TRANSIENT_CFG);
Again, some crazy stuff going on with indenting..

+   if (ret < 0)
+   return ret;
  
-		ret = i2c_smbus_read_byte_data(data->client,

-  data->chip_info->ev_cfg);
-   if (ret < 0)
-   return ret;
+   return ret & 
MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index) ? 1 : 0;

It's a nasty trick in a way, but commonly used in the kernel.
return !!(

Re: [PATCH] iio: accel: Bugfix to enbale and allow different events to work parallely.

[Harinath Nampally]

On Mon, 31 Jul 2017 07:17:38 -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 fixes 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 multiple 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 

Hi,

A few minor bits and bobs inline.

Jonathan

Thank you for the review.


+ /**
+  * struct mma8452_event_regs - chip specific data related to events
+  * @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 included here.
+  */
+struct mma8452_event_regs {
+   u8 ev_cfg;
+   u8 ev_cfg_ele;
+   u8 ev_cfg_chan_shift;
As far as I can tell the above isn't used...
Please sanity check the others

Yes they are not used and not really necessary I think, probably I 
should remove them!
It makes sense to only have ev_cfg, ev_src, ev_ths, ev_ths_mask and 
ev_count.
as they are common to other events as well like orientation, 
single/double tap etc.

So in future this same struct can be reused across different events.

  enum {
@@ -394,11 +403,11 @@ static ssize_t mma8452_show_os_ratio_avail(struct device 
*dev,
  }
  
  static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail);

-static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO,
+static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
   mma8452_show_scale_avail, NULL, 0);
  static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available,
-  S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0);
-static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, S_IRUGO,
+  0444, mma8452_show_hp_cutoff_avail, NULL, 0);
+static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, 0444,
   mma8452_show_os_ratio_avail, NULL, 0);
Separate change.  Please do it in a precursor patch rather than
adding noise to this one..

Sure I will.

case IIO_EV_INFO_PERIOD:
ret = i2c_smbus_read_byte_data(data->client,
-  data->chip_info->ev_count);
+   
ev_regs.ev_count);
This indenting looks somewhat odd..

Yes I agree, will fix it.

+   switch (chan->type) {
+   case IIO_ACCEL:
+   switch (dir) {
+   case IIO_EV_DIR_FALLING:
+   return mma8452_freefall_mode_enabled(data);
+   case IIO_EV_DIR_RISING:
+   ret = i2c_smbus_read_byte_data(data->client,
+   
MMA8452_TRANSIENT_CFG);
Again, some crazy stuff going on with indenting..

+   if (ret < 0)
+   return ret;
  
-		ret = i2c_smbus_read_byte_data(data->client,

-  data->chip_info->ev_cfg);
-   if (ret < 0)
-   return ret;
+   return ret & 
MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index) ? 1 : 0;

It's a nasty trick in a way, but commonly used in the kernel.
return !!(ret & MMA8452_TRANSIENT_CFG_CHAN(chan->scan_index));