RE: [PATCH v3 01/10] input: keyboard: adp5588-keys: support gpi key events as 'gpio keys'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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á




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux