Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750985AbdIJQRr (ORCPT ); Sun, 10 Sep 2017 12:17:47 -0400 Received: from mail-wr0-f195.google.com ([209.85.128.195]:35154 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750709AbdIJQRq (ORCPT ); Sun, 10 Sep 2017 12:17:46 -0400 X-Google-Smtp-Source: ADKCNb7mNy0VLc3mFf3EulBf2S8Ba1Km5WyIwpXlJuGOaEbQSoKkyz2RVOJdjyna5u0XNSnskIt1eIRJVwc1SgMg4H8= MIME-Version: 1.0 In-Reply-To: <20170910165137.4568b000@archlinux> References: <1504987018-8934-1-git-send-email-harinath922@gmail.com> <20170910165137.4568b000@archlinux> From: harinath Nampally Date: Sun, 10 Sep 2017 12:17:03 -0400 Message-ID: Subject: Re: [PATCH v6] iio: accel: mma8452: improvements to handle multiple events To: Jonathan Cameron Cc: Martin Kepplinger , knaack.h@gmx.de, lars@metafoo.de, Peter Meerwald-Stadler , Greg KH , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Alison Schofield Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5376 Lines: 140 > 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 (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@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >