lkml.org 
[lkml]   [2017]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] iio: accel: mma8452: Add single pulse/tap event detection
Am 14.11.2017 05:36 schrieb 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 <harinath922@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.
>

I would argue definitions don't break the build and maybe slightly
better
support features like bisect or revert :)

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

It does to me.

\
 
 \ /
  Last update: 2017-11-14 08:23    [W:0.048 / U:1.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site