[PATCH v3 0/3] Refactor event related code

[PATCH v3 0/3] Refactor event related code

[Harinath Nampally]

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

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

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath922@gmail.com>
---
 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

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

[Harinath Nampally]

Improves code readability, no impact on functionality.

Signed-off-by: Harinath Nampally <harinath922@gmail.com>
---
 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

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

[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 <harinath922@gmail.com>
---
 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

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

[Martin Kepplinger]

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 <harinath922@gmail.com>

Acked-by: Martin Kepplinger <martink@posteo.de>

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.

[Martin Kepplinger]

Am 25.09.2017 12:40 schrieb Harinath Nampally:
> Improves code readability, no impact on functionality.
> 
> Signed-off-by: Harinath Nampally <harinath922@gmail.com>

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)

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

[Martin Kepplinger]

Am 25.09.2017 12:40 schrieb Harinath Nampally:
> Improves code readability, no impact on functionality.
> 
> Signed-off-by: Harinath Nampally <harinath922@gmail.com>
> ---

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

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

[Jonathan Cameron]

On Wed, 27 Sep 2017 08:52:54 +0200
Martin Kepplinger <martink@posteo.de> wrote:

> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > Improves code readability, no impact on functionality.
> > 
> > Signed-off-by: Harinath Nampally <harinath922@gmail.com>
> > ---  
> 
> 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

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

[Jonathan Cameron]

On Wed, 27 Sep 2017 08:46:27 +0200
Martin Kepplinger <martink@posteo.de> 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 <harinath922@gmail.com>  
> 
> Acked-by: Martin Kepplinger <martink@posteo.de>

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.

[Jonathan Cameron]

On Wed, 27 Sep 2017 08:51:26 +0200
Martin Kepplinger <martink@posteo.de> wrote:

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

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.

[harinath Nampally]

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

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

[Martin Kepplinger]

On 2017-09-30 19:59, Jonathan Cameron wrote:
> On Wed, 27 Sep 2017 08:52:54 +0200
> Martin Kepplinger <martink@posteo.de> wrote:
> 
>> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>>> Improves code readability, no impact on functionality.
>>>
>>> Signed-off-by: Harinath Nampally <harinath922@gmail.com>
>>> ---  
>>
>> 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?
> 

It's understandable for me. It simply uses "transient" and "ff_mt" in
order to describe sets of device-registers instead of "rising" and
"falling". That's more appropriate. I'd apply this if possible.

Even though it should be clear after reading the data sheet, this
particular issue sometimes gets confusing for people. Maybe a short
descriptive comment, pointing to the data sheet, would make sense?

                      martin

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

[Jonathan Cameron]

On Sun, 1 Oct 2017 20:10:49 +0200
Martin Kepplinger <martink@posteo.de> wrote:

> On 2017-09-30 19:59, Jonathan Cameron wrote:
> > On Wed, 27 Sep 2017 08:52:54 +0200
> > Martin Kepplinger <martink@posteo.de> wrote:
> >   
> >> Am 25.09.2017 12:40 schrieb Harinath Nampally:  
> >>> Improves code readability, no impact on functionality.
> >>>
> >>> Signed-off-by: Harinath Nampally <harinath922@gmail.com>
> >>> ---    
> >>
> >> 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?
> >   
> 
> It's understandable for me. It simply uses "transient" and "ff_mt" in
> order to describe sets of device-registers instead of "rising" and
> "falling". That's more appropriate. I'd apply this if possible.
> 
> Even though it should be clear after reading the data sheet, this
> particular issue sometimes gets confusing for people. Maybe a short
> descriptive comment, pointing to the data sheet, would make sense?
> 

With the addition of some description, this should be fine.

Thanks,

Jonathan

>                       martin
> --
> 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

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

[harinath Nampally]

> > On 2017-09-30 19:59, Jonathan Cameron wrote:
> > > On Wed, 27 Sep 2017 08:52:54 +0200
> > > Martin Kepplinger <martink@posteo.de> wrote:
> > >
> > >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
> > >>> Improves code readability, no impact on functionality.
> > >>>
> > >>> Signed-off-by: Harinath Nampally <harinath922@gmail.com>
> > >>> ---
> > >>
> > >> 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?
> > >
> >
> > It's understandable for me. It simply uses "transient" and "ff_mt" in
> > order to describe sets of device-registers instead of "rising" and
> > "falling". That's more appropriate. I'd apply this if possible.
> >
> > Even though it should be clear after reading the data sheet, this
> > particular issue sometimes gets confusing for people. Maybe a short
> > descriptive comment, pointing to the data sheet, would make sense?
> >
>
> With the addition of some description, this should be fine.
Sure will do.

Thanks,
Harinath

On Mon, Oct 2, 2017 at 6:35 AM, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Sun, 1 Oct 2017 20:10:49 +0200
> Martin Kepplinger <martink@posteo.de> wrote:
>
>> On 2017-09-30 19:59, Jonathan Cameron wrote:
>> > On Wed, 27 Sep 2017 08:52:54 +0200
>> > Martin Kepplinger <martink@posteo.de> wrote:
>> >
>> >> Am 25.09.2017 12:40 schrieb Harinath Nampally:
>> >>> Improves code readability, no impact on functionality.
>> >>>
>> >>> Signed-off-by: Harinath Nampally <harinath922@gmail.com>
>> >>> ---
>> >>
>> >> 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?
>> >
>>
>> It's understandable for me. It simply uses "transient" and "ff_mt" in
>> order to describe sets of device-registers instead of "rising" and
>> "falling". That's more appropriate. I'd apply this if possible.
>>
>> Even though it should be clear after reading the data sheet, this
>> particular issue sometimes gets confusing for people. Maybe a short
>> descriptive comment, pointing to the data sheet, would make sense?
>>
>
> With the addition of some description, this should be fine.
>
> Thanks,
>
> Jonathan
>
>>                       martin
>> --
>> 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
>