mbox series

[v4,00/10] adp5588-keys refactor and fw properties support

Message ID 20220829131553.690063-1-nuno.sa@analog.com
Headers show
Series adp5588-keys refactor and fw properties support | expand

Message

Nuno Sa Aug. 29, 2022, 1:15 p.m. UTC
The main goal of this patchset is to remove platform data and replace it by
firmware properties. Original discussion in [1].

While in here, some refactor was done to the driver. The most noticeable one
is to replace the GPIs events handling by irqchip support so that this gpi
keys can be "consumed" by the gpio-keys driver (also as suggested in [1]).
With this, the gpio-adp5588 can be removed. This change comes first so that
we can already remove some platform data variables making it easier to
completly replace it by firmware properties further down in the series.

As there's no users of the platform data, I just replace it in a single
patch as there's no point in having support for both (even though it might
be harder to review the patch as-is).

Special note to the gpio-adp5588 driver removal. I'm aware of some changes
to the driver in [2]. These changes are in the gpio tree and this patchset
is naturally based on the input tree which means that patch 2 will
not apply. So, I'm not really sure how to handle this. I guess in this
case the conflict is easy to handle :) but just let me know on how to
proceed in here if there's anything for me to do.

[1]: https://lore.kernel.org/linux-input/20220504084617.36844-1-u.kleine-koenig@pengutronix.de/
[2]: https://lore.kernel.org/linux-gpio/20220628193906.36350-3-andriy.shevchenko@linux.intel.com/

v2 changes:

[1/10]
 * Turn hwirq signed so we can compare < 0;
 * Replace WARN_ON with dev_warn();
 * Do not set of_node on gpiochip;
 * Moved to use a const irqchip within the gpiochip;
 * Set default handler to 'handle_bad_irq()' and change it
in irq_set_type;

[4/10]
 * Dropped "-keys" from compatible and added vendor prefix;
 * Fix -Wformat complains;
 * Don't use abbrev in comments (fw -> Firmware).

[5/10]
 * Be consistent on $refs;
 * Drop "-keys" from compatible.

[7/10]
 * Include bits.h;
 * Use GENMASK();
 * Use BIT() in KP_SEL();
 * Reflect code changes in the commit message.

[9/10]
 * One line for regulator_disable action.

v3 changes:

[1/10]
 * Use 'irqd_to_hwirq()' helper;
 * Use INVALID_HWIRQ to signal hwirq not found;
 * Just compare irq against 0 in 'irq_find_mapping()';
 * Renamed irq_data *desc to *irqd to avoid confusion.

[5/10]
 * Dropped the -keys suffix on the filename;
 * Compatible enum in alphabetical order;
 * Improved 'adi,unlock-keys' description;
 * 4 spaces indentation for dts example;
 * Renamed device node to a generic name and fixed the
compatible property in the example.

v4 changes:

[1/10]
 * Use local 'INVALID_HWIRQ' macro;
 * Explicitly define a local variable to hold 'irqd_to_hwirq()'

Nuno Sá (10):
  input: keyboard: adp5588-keys: support gpi key events as 'gpio keys'
  gpio: gpio-adp5588: drop the driver
  input: keyboard: adp5588-keys: bail out on returned error
  input: keyboard: adp5588-keys: add support for fw properties
  dt-bindings: input: adp5588: add bindings
  input: keyboard: adp5588-keys: do not check for irq presence
  input: keyboard: adp5588-keys: fix coding style warnings
  input: keyboard: adp5588-keys: add optional reset gpio
  input: keyboard: adp5588-keys: add regulator support
  input: keyboard: adp5588-keys: Use new PM macros

 .../bindings/input/adi,adp5588.yaml           | 111 +++
 MAINTAINERS                                   |   2 +-
 drivers/gpio/Kconfig                          |  14 -
 drivers/gpio/Makefile                         |   1 -
 drivers/gpio/gpio-adp5588.c                   | 446 -----------
 drivers/input/keyboard/Kconfig                |   3 +
 drivers/input/keyboard/adp5588-keys.c         | 724 ++++++++++++------
 include/linux/platform_data/adp5588.h         | 171 -----
 8 files changed, 594 insertions(+), 878 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/adi,adp5588.yaml
 delete mode 100644 drivers/gpio/gpio-adp5588.c
 delete mode 100644 include/linux/platform_data/adp5588.h

Comments

Linus Walleij Aug. 31, 2022, 12:37 p.m. UTC | #1
On Mon, Aug 29, 2022 at 3:15 PM Nuno Sá <nuno.sa@analog.com> wrote:

> This change replaces the support for GPIs as key event generators.
> Instead of reporting the events directly, we add a gpio based irqchip
> so that these events can be consumed by keys defined in the gpio-keys
> driver (as it's goal is indeed for keys on GPIOs capable of generating
> interrupts). With this, the gpio-adp5588 driver can also be dropped.
>
> The basic idea is that all the pins that are not being used as part of
> the keymap matrix can be possibly requested as GPIOs by gpio-keys
> (it's also fine to use these pins as plain interrupts though that's not
> really the point).
>
> Since the gpiochip now also has irqchip capabilities, we should only
> remove it after we free the device interrupt (otherwise we could, in
> theory, be handling GPIs interrupts while the gpiochip is concurrently
> removed). Thus the call 'adp5588_gpio_add()' is moved and since the
> setup phase also needs to come before making the gpios visible, we also
> need to move 'adp5588_setup()'.
>
> While at it, always select GPIOLIB so that we don't need to use #ifdef
> guards.
>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Makes the world simpler so:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij