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>
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;
> + }
> +}
> +
