On Sun, 10 Sep 2017 08:36:43 +0200
Martin Kepplinger <martink@xxxxxxxxx> 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 <harinath922@xxxxxxxxx>
> > ---
>
> 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 <martink@xxxxxxxxx>
>
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 (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 = &ev_regs_accel_rising;
> > + else
> > + *ev_reg = &ev_regs_accel_falling;
> > + return 0;
>
> I know it's fine, only the naming seems odd here.
>
> > + case IIO_EV_DIR_FALLING:
> > + *ev_reg = &ev_regs_accel_falling;
> > + return 0;
> > + default:
> > + return -EINVAL;
> > + }
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
