> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > Sent: Monday, July 25, 2022 10:34 PM > To: Sa, Nuno <Nuno.Sa@xxxxxxxxxx> > Cc: linux-input <linux-input@xxxxxxxxxxxxxxx>; open list:GPIO > SUBSYSTEM <linux-gpio@xxxxxxxxxxxxxxx>; devicetree > <devicetree@xxxxxxxxxxxxxxx>; Linus Walleij > <linus.walleij@xxxxxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Hennerich, Michael > <Michael.Hennerich@xxxxxxxxxx>; Bartosz Golaszewski > <brgl@xxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Dmitry > Torokhov <dmitry.torokhov@xxxxxxxxx> > Subject: Re: [PATCH v3 01/10] input: keyboard: adp5588-keys: support > gpi key events as 'gpio keys' > > [External] > > On Thu, Jul 21, 2022 at 10:03 AM Nuno Sá <nuno.sa@xxxxxxxxxx> > 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. > > ... > > > +static void adp5588_irq_mask(struct irq_data *d) > > +{ > > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > > + struct adp5588_kpad *kpad = gpiochip_get_data(gc); > > Can it be > > irq_hw_number_t hwirq = irqd_to_hwirq(d); > > ? > Yeah, I thought about doing that way but in the end just "inlined" the call. Anyways, no strong feeling so I can have it as you prefer... - Nuno Sá