Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1738915 > unrolled thread

[PATCH v3 0/3] Refactor event related code

Started byHarinath Nampally <harinath922@gmail.com>
First post2017-09-25 12:50 +0200
Last post2017-10-02 12:40 +0200
Articles 10 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/3] Refactor event related code Harinath Nampally <harinath922@gmail.com> - 2017-09-25 12:50 +0200
    [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 <harinath922@gmail.com> - 2017-09-25 12:50 +0200
      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 <martink@posteo.de> - 2017-09-27 09:00 +0200
        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 <jic23@kernel.org> - 2017-09-30 20:10 +0200
          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 <harinath922@gmail.com> - 2017-09-30 20:30 +0200
    [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names. Harinath Nampally <harinath922@gmail.com> - 2017-09-25 12:50 +0200
      Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding  event configuration registers to more appropriate names. Martin Kepplinger <martink@posteo.de> - 2017-09-27 09:00 +0200
        Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding  event configuration registers to more appropriate names. Jonathan Cameron <jic23@kernel.org> - 2017-09-30 20:00 +0200
          Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event  configuration registers to more appropriate names. Martin Kepplinger <martink@posteo.de> - 2017-10-01 20:20 +0200
            Re: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding  event configuration registers to more appropriate names. Jonathan Cameron <Jonathan.Cameron@huawei.com> - 2017-10-02 12:40 +0200

#1738915 — [PATCH v3 0/3] Refactor event related code

FromHarinath Nampally <harinath922@gmail.com>
Date2017-09-25 12:50 +0200
Subject[PATCH v3 0/3] Refactor event related code
Message-ID<utz7z-5aK-3@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1738919 — [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.

FromHarinath Nampally <harinath922@gmail.com>
Date2017-09-25 12:50 +0200
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.
Message-ID<utz7A-5aK-15@gated-at.bofh.it>
In reply to#1738915
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

[toc] | [prev] | [next] | [standalone]


#1740438 — 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.

FromMartin Kepplinger <martink@posteo.de>
Date2017-09-27 09:00 +0200
SubjectRe: [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.
Message-ID<uueu6-7gi-15@gated-at.bofh.it>
In reply to#1738919
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)

[toc] | [prev] | [next] | [standalone]


#1742673 — 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.

FromJonathan Cameron <jic23@kernel.org>
Date2017-09-30 20:10 +0200
SubjectRe: [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.
Message-ID<uvun7-7CT-5@gated-at.bofh.it>
In reply to#1740438
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

[toc] | [prev] | [next] | [standalone]


#1742675 — 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.

Fromharinath Nampally <harinath922@gmail.com>
Date2017-09-30 20:30 +0200
SubjectRe: [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.
Message-ID<uvuGt-7KB-7@gated-at.bofh.it>
In reply to#1742673
> 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
>

[toc] | [prev] | [next] | [standalone]


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

FromHarinath Nampally <harinath922@gmail.com>
Date2017-09-25 12:50 +0200
Subject[PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.
Message-ID<utz7B-5aK-27@gated-at.bofh.it>
In reply to#1738915
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

[toc] | [prev] | [next] | [standalone]


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

FromMartin Kepplinger <martink@posteo.de>
Date2017-09-27 09:00 +0200
SubjectRe: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.
Message-ID<uueu6-7gi-1@gated-at.bofh.it>
In reply to#1738925
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.

[toc] | [prev] | [next] | [standalone]


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

FromJonathan Cameron <jic23@kernel.org>
Date2017-09-30 20:00 +0200
SubjectRe: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.
Message-ID<uvuds-7k9-17@gated-at.bofh.it>
In reply to#1740434
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

[toc] | [prev] | [next] | [standalone]


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

FromMartin Kepplinger <martink@posteo.de>
Date2017-10-01 20:20 +0200
SubjectRe: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.
Message-ID<uvR0m-5s3-9@gated-at.bofh.it>
In reply to#1742671
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

[toc] | [prev] | [next] | [standalone]


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

FromJonathan Cameron <Jonathan.Cameron@huawei.com>
Date2017-10-02 12:40 +0200
SubjectRe: [PATCH v3 1/3] iio: accel: mma8452: Rename structs holding event configuration registers to more appropriate names.
Message-ID<uw6iK-6a9-19@gated-at.bofh.it>
In reply to#1742856
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web