2017-09-25 10:40:14

by Harinath Nampally

[permalink] [raw]
Subject: [PATCH v3 0/3] Refactor event related code

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


2017-09-25 10:40:17

by Harinath Nampally

[permalink] [raw]
Subject: [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.

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <[email protected]>
---
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, 10000, 20000, 20000, 20000, 20000 }, /* normal */
{ 1250, 2500, 5000, 10000, 20000, 80000, 80000, 80000 }, /* 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

2017-09-25 10:40:28

by Harinath Nampally

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

'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 <[email protected]>
---
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 = &mma8452_read_raw,
.write_raw = &mma8452_write_raw,
.event_attrs = &mma8452_event_attribute_group,
- .read_event_value = &mma8452_read_thresh,
- .write_event_value = &mma8452_write_thresh,
+ .read_event_value = &mma8452_read_event_value,
+ .write_event_value = &mma8452_write_event_value,
.read_event_config = &mma8452_read_event_config,
.write_event_config = &mma8452_write_event_config,
.debugfs_reg_access = &mma8452_reg_access_dbg,
--
2.7.4

2017-09-25 10:40:52

by Harinath Nampally

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

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <[email protected]>
---
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 = &ev_regs_accel_rising;
+ *ev_reg = &trans_ev_regs;
else
- *ev_reg = &ev_regs_accel_falling;
+ *ev_reg = &ff_mt_ev_regs;
return 0;
case IIO_EV_DIR_FALLING:
- *ev_reg = &ev_regs_accel_falling;
+ *ev_reg = &ff_mt_ev_regs;
return 0;
default:
return -EINVAL;
--
2.7.4

2017-09-27 06:46:32

by Martin Kepplinger

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

Am 25.09.2017 12:40 schrieb 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 <[email protected]>

Acked-by: Martin Kepplinger <[email protected]>

2017-09-27 06:51:30

by Martin Kepplinger

[permalink] [raw]
Subject: 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.

Am 25.09.2017 12:40 schrieb Harinath Nampally:
> Improves code readability, no impact on functionality.
>
> Signed-off-by: Harinath Nampally <[email protected]>

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)

2017-09-27 06:52:58

by Martin Kepplinger

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

Am 25.09.2017 12:40 schrieb Harinath Nampally:
> Improves code readability, no impact on functionality.
>
> Signed-off-by: Harinath Nampally <[email protected]>
> ---

I'd prefer a shorter subject line here too, see patch 2/3.

2017-09-30 17:59:20

by Jonathan Cameron

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

On Wed, 27 Sep 2017 08:52:54 +0200
Martin Kepplinger <[email protected]> wrote:

> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > Improves code readability, no impact on functionality.
> >
> > Signed-off-by: Harinath Nampally <[email protected]>
> > ---
>
> 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?

Thanks,

Jonathan

2017-09-30 18:04:53

by Jonathan Cameron

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

On Wed, 27 Sep 2017 08:46:27 +0200
Martin Kepplinger <[email protected]> wrote:

> Am 25.09.2017 12:40 schrieb 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 <[email protected]>
>
> Acked-by: Martin Kepplinger <[email protected]>

Just this patch 3 applied for now to the togreg branch of iio.git and
pushed out as testing for the autobuilders to play with.

Thanks,

Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-09-30 18:05:16

by Jonathan Cameron

[permalink] [raw]
Subject: 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.

On Wed, 27 Sep 2017 08:51:26 +0200
Martin Kepplinger <[email protected]> wrote:

> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > Improves code readability, no impact on functionality.
> >
> > Signed-off-by: Harinath Nampally <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-09-30 18:21:28

by Harinath Nampally

[permalink] [raw]
Subject: 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.

> 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 <[email protected]> wrote:
> On Wed, 27 Sep 2017 08:51:26 +0200
> Martin Kepplinger <[email protected]> wrote:
>
>> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> > Improves code readability, no impact on functionality.
>> >
>> > Signed-off-by: Harinath Nampally <[email protected]>
>>
>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>