| Message ID | 20220713013721.68879-5-warthog618@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: cdev: code cleanup following hte integration | expand |
On Wed, Jul 13, 2022 at 3:39 AM Kent Gibson <warthog618@gmail.com> wrote: > > Reorganise line event identification code to reduce code duplication, > and replace if-else initializers with the ternary equivalent to > improve readability. ... > + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : > + GPIO_V2_LINE_EVENT_FALLING_EDGE; It seems several times you are doing the same, perhaps a helper?
On Wed, Jul 13, 2022 at 11:59:10AM +0200, Andy Shevchenko wrote: > On Wed, Jul 13, 2022 at 3:39 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > Reorganise line event identification code to reduce code duplication, > > and replace if-else initializers with the ternary equivalent to > > improve readability. > > ... > > > + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : > > + GPIO_V2_LINE_EVENT_FALLING_EDGE; > > It seems several times you are doing the same, perhaps a helper? > If by several times you mean twice, then yeah. Not sure that reaches the threshold for a helper though. Cheers, Kent.
On Wed, Jul 13, 2022 at 12:27 PM Kent Gibson <warthog618@gmail.com> wrote: > On Wed, Jul 13, 2022 at 11:59:10AM +0200, Andy Shevchenko wrote: > > On Wed, Jul 13, 2022 at 3:39 AM Kent Gibson <warthog618@gmail.com> wrote: ... > > > + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : > > > + GPIO_V2_LINE_EVENT_FALLING_EDGE; > > > > It seems several times you are doing the same, perhaps a helper? > > If by several times you mean twice, then yeah. > Not sure that reaches the threshold for a helper though. Up to you, then!
On Wed, Jul 13, 2022 at 01:24:33PM +0200, Andy Shevchenko wrote: > On Wed, Jul 13, 2022 at 12:27 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Wed, Jul 13, 2022 at 11:59:10AM +0200, Andy Shevchenko wrote: > > > On Wed, Jul 13, 2022 at 3:39 AM Kent Gibson <warthog618@gmail.com> wrote: > > ... > > > > > + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : > > > > + GPIO_V2_LINE_EVENT_FALLING_EDGE; > > > > > > It seems several times you are doing the same, perhaps a helper? > > > > If by several times you mean twice, then yeah. > > Not sure that reaches the threshold for a helper though. > > Up to you, then! > Turns out there are three instances, so a helper it is. Cheers, Kent.
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index bc7c8822ede0..34d0bdfe5498 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -590,26 +590,20 @@ static enum hte_return process_hw_ts_thread(void *p) switch (eflags) { case GPIO_V2_LINE_FLAG_EDGE_BOTH: - if (line->raw_level >= 0) { - if (test_bit(FLAG_ACTIVE_LOW, &line->desc->flags)) - level = !line->raw_level; - else - level = line->raw_level; - } else { - level = gpiod_get_value_cansleep(line->desc); - } + level = (line->raw_level >= 0) ? + line->raw_level : + gpiod_get_raw_value_cansleep(line->desc); - if (level) - le.id = GPIO_V2_LINE_EVENT_RISING_EDGE; - else - le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE; + if (test_bit(FLAG_ACTIVE_LOW, &line->desc->flags)) + level = !level; + + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : + GPIO_V2_LINE_EVENT_FALLING_EDGE; break; case GPIO_V2_LINE_FLAG_EDGE_RISING: - /* Emit low-to-high event */ le.id = GPIO_V2_LINE_EVENT_RISING_EDGE; break; case GPIO_V2_LINE_FLAG_EDGE_FALLING: - /* Emit high-to-low event */ le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE; break; default: @@ -684,20 +678,14 @@ static irqreturn_t edge_irq_thread(int irq, void *p) switch (READ_ONCE(line->eflags)) { case GPIO_V2_LINE_FLAG_EDGE_BOTH: - if (gpiod_get_value_cansleep(line->desc)) - /* Emit low-to-high event */ - le.id = GPIO_V2_LINE_EVENT_RISING_EDGE; - else - /* Emit high-to-low event */ - le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE; - + le.id = gpiod_get_value_cansleep(line->desc) ? + GPIO_V2_LINE_EVENT_RISING_EDGE : + GPIO_V2_LINE_EVENT_FALLING_EDGE; break; case GPIO_V2_LINE_FLAG_EDGE_RISING: - /* Emit low-to-high event */ le.id = GPIO_V2_LINE_EVENT_RISING_EDGE; break; case GPIO_V2_LINE_FLAG_EDGE_FALLING: - /* Emit high-to-low event */ le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE; break; default: @@ -821,12 +809,8 @@ static void debounce_work_func(struct work_struct *work) le.line_seqno : atomic_inc_return(&lr->seqno); } - if (level) - /* Emit low-to-high event */ - le.id = GPIO_V2_LINE_EVENT_RISING_EDGE; - else - /* Emit high-to-low event */ - le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE; + le.id = level ? GPIO_V2_LINE_EVENT_RISING_EDGE : + GPIO_V2_LINE_EVENT_FALLING_EDGE; linereq_put_event(lr, &le); }
Reorganise line event identification code to reduce code duplication, and replace if-else initializers with the ternary equivalent to improve readability. Signed-off-by: Kent Gibson <warthog618@gmail.com> --- drivers/gpio/gpiolib-cdev.c | 42 ++++++++++++------------------------- 1 file changed, 13 insertions(+), 29 deletions(-)