[PATCH-next,00/20] gpio: gpio-omap: set of fixes and big clean-up

Message ID 20190610171103.30903-1-grygorii.strashko@ti.com
Headers show
Series
  • gpio: gpio-omap: set of fixes and big clean-up
Related show

Message

Grygorii Strashko June 10, 2019, 5:10 p.m.
Hi Linus, Russell, Tony, All,

This series contains set of patches from Russell King which were circulated
internally for quite some time already and I fill it's reasonable to move
future discussion upstream (and also avoid rebasing).
Fisrt two patches are fixes and the rest are big, great clean up
from Russell King.

Personally, I like this clean up and refactoring very much and don't want
it to be lost.

Code can be found at:
 git@git.ti.com:~gragst/ti-linux-kernel/gragsts-ti-linux-kernel.git
branch:
 lkml-next-gpio-clean-up

Russell King (20):
  gpio: gpio-omap: ensure irq is enabled before wakeup
  gpio: gpio-omap: fix lack of irqstatus_raw0 for OMAP4
  gpio: gpio-omap: remove remainder of list management
  gpio: gpio-omap: clean up edge interrupt handling
  gpio: gpio-omap: remove irq_ack method
  gpio: gpio-omap: move omap_gpio_request() and omap_gpio_free()
  gpio: gpio-omap: simplify omap_gpio_get_direction()
  gpio: gpio-omap: simplify get() method
  gpio: gpio-omap: simplify get_multiple()
  gpio: gpio-omap: simplify set_multiple()
  gpio: gpio-omap: simplify bank->level_mask
  gpio: gpio-omap: simplify read-modify-write
  gpio: gpio-omap: simplify omap_toggle_gpio_edge_triggering()
  gpio: gpio-omap: simplify omap_set_gpio_irqenable()
  gpio: gpio-omap: remove dataout variation in context handling
  gpio: gpio-omap: clean up omap_gpio_restore_context()
  gpio: gpio-omap: constify register tables
  gpio: gpio-omap: clean up wakeup handling
  gpio: gpio-omap: irq_startup() must not return error codes
  gpio: gpio-omap: clean up register access in omap2_set_gpio_debounce()

 drivers/gpio/gpio-omap.c                | 497 ++++++++----------------
 include/linux/platform_data/gpio-omap.h |   2 +-
 2 files changed, 161 insertions(+), 338 deletions(-)

-- 
2.17.1

Comments

Tony Lindgren June 11, 2019, 7:37 a.m. | #1
Hi,

* Grygorii Strashko <grygorii.strashko@ti.com> [190610 10:11]:
> Hi Linus, Russell, Tony, All,

> 

> This series contains set of patches from Russell King which were circulated

> internally for quite some time already and I fill it's reasonable to move

> future discussion upstream (and also avoid rebasing).

> Fisrt two patches are fixes and the rest are big, great clean up

> from Russell King.

> 

> Personally, I like this clean up and refactoring very much and don't want

> it to be lost.


Adding Aaro to Cc too hopefully for more testing. Yes this is very nice
and behaves for my idle test cases. I've also boot tested omap1 osk and
it still works just fine for NFSroot.

FYI, after this series, the only issue I'm aware of still remaining
is the lost edge wake-up interrupts for L4 PER idle that I posted a WIP
fix as "[PATCH] gpio: gpio-omap: Fix lost edge wake-up interrupts".
But that one still needs a bit more work and is a separate fix from this
series.

So for this whole series, please feel free to add:

Tested-by: Tony Lindgren <tony@atomide.com>




> Code can be found at:

>  git@git.ti.com:~gragst/ti-linux-kernel/gragsts-ti-linux-kernel.git

> branch:

>  lkml-next-gpio-clean-up

> 

> Russell King (20):

>   gpio: gpio-omap: ensure irq is enabled before wakeup

>   gpio: gpio-omap: fix lack of irqstatus_raw0 for OMAP4

>   gpio: gpio-omap: remove remainder of list management

>   gpio: gpio-omap: clean up edge interrupt handling

>   gpio: gpio-omap: remove irq_ack method

>   gpio: gpio-omap: move omap_gpio_request() and omap_gpio_free()

>   gpio: gpio-omap: simplify omap_gpio_get_direction()

>   gpio: gpio-omap: simplify get() method

>   gpio: gpio-omap: simplify get_multiple()

>   gpio: gpio-omap: simplify set_multiple()

>   gpio: gpio-omap: simplify bank->level_mask

>   gpio: gpio-omap: simplify read-modify-write

>   gpio: gpio-omap: simplify omap_toggle_gpio_edge_triggering()

>   gpio: gpio-omap: simplify omap_set_gpio_irqenable()

>   gpio: gpio-omap: remove dataout variation in context handling

>   gpio: gpio-omap: clean up omap_gpio_restore_context()

>   gpio: gpio-omap: constify register tables

>   gpio: gpio-omap: clean up wakeup handling

>   gpio: gpio-omap: irq_startup() must not return error codes

>   gpio: gpio-omap: clean up register access in omap2_set_gpio_debounce()

> 

>  drivers/gpio/gpio-omap.c                | 497 ++++++++----------------

>  include/linux/platform_data/gpio-omap.h |   2 +-

>  2 files changed, 161 insertions(+), 338 deletions(-)

> 

> -- 

> 2.17.1

>
Linus Walleij June 12, 2019, 7:54 a.m. | #2
On Mon, Jun 10, 2019 at 7:11 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> Documentation states:

>

>   NOTE: There must be a correlation between the wake-up enable and

>   interrupt-enable registers. If a GPIO pin has a wake-up configured

>   on it, it must also have the corresponding interrupt enabled (on

>   one of the two interrupt lines).

>

> Ensure that this condition is always satisfied by enabling the detection

> events after enabling the interrupt, and disabling the detection before

> disabling the interrupt.  This ensures interrupt/wakeup events can not

> happen until both the wakeup and interrupt enables correlate.

>

> If we do any clearing, clear between the interrupt enable/disable and

> trigger setting.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:42 a.m. | #3
On Mon, Jun 10, 2019 at 7:11 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> The irq_ack method does not fit our hardware requirements. Edge

> interrupts must be cleared before we handle them, and level interrupts

> must be cleared after handling them.

>

> We handle the interrupt clearance in our interrupt handler for edge IRQs

> and in the unmask method for level IRQs.

>

> Replace the irq_ack method with the no-op method from the dummy irq

> chip.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:49 a.m. | #4
On Mon, Jun 10, 2019 at 7:12 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> omap_gpio_get() calls omap_get_gpio_datain() or omap_get_gpio_dataout()

> to read the GPIO state. These two functions are only called from this

> method, so they don't add much value.  Move their contents into

> omap_gpio_get() method and simplify.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:50 a.m. | #5
On Mon, Jun 10, 2019 at 7:12 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> There is no reason to have helper functions to read the datain and

> dataout registers when they are only used in one location.  Simplify

> this code to make it more readable.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:55 a.m. | #6
On Mon, Jun 10, 2019 at 7:13 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> When a GPIO block has the set/clear dataout registers implemented, it

> also has the normal dataout register implemented. Reading this register

> reads the current GPIO output state, and writing it sets the GPIOs to

> the explicit state. This is the behaviour that we want when saving and

> restoring the context, so use the dataout register exclusively.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:58 a.m. | #7
On Mon, Jun 10, 2019 at 7:13 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij
Linus Walleij June 12, 2019, 8:59 a.m. | #8
On Mon, Jun 10, 2019 at 7:13 PM Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Russell King <rmk+kernel@armlinux.org.uk>

>

> The irq_startup() method returns an unsigned int, but in __irq_startup()

> it is assigned to an int.  However, nothing checks for errors, so any

> error that is returned is ignored.

>

> Remove the check for GPIO-input mode and the error return.

>

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>


Patch applied.

Yours,
Linus Walleij