Re: [RFC PATCH] RFC: drivers: gpio: helper for generic pin IRQ handling
From: Grygorii Strashko
Date: Tue Dec 08 2020 - 11:20:36 EST
On 08/12/2020 16:38, Andy Shevchenko wrote:
On Tue, Dec 8, 2020 at 4:19 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
On Tue, Dec 8, 2020 at 4:14 PM Enrico Weigelt, metux IT consult
<info@xxxxxxxxx> wrote:
Many gpio drivers already use gpiolib's builtin irqchip handling
(CONFIG_GPIOLIB_IRQCHIP), but still has some boilerplate for retrieving
the actual Linux IRQ number and calling into the generic handler.
That boilerplate can be reduced by moving that into a helper function.
This is an RFC patch to outline how that could be done. Note: it's
completely untested yet.
Several drivers still have their completely IRQ own implementation and
thus can't be converted yet. Some of them perhaps could be changed to
store their irq domain in the struct gpio, so the new helper could
also be used for those.
Having all GPIO drivers doing their IRQ management entirely through the
GPIO subsystem (eg. never calling generic_handle_irq() and using the builtin
IRQ handling) would also allow a more direct (eg. callback-based) pin change
notification for GPIO consumers, that doesn't involve registering them as
generic IRQ handlers.
Above part makes me worry - why?
Further reduction of boilerplate could be achieved by additional helpers
for common patterns like for_each_set_bit() loops on irq masks.
Have you able to test them all?
As the PCA953x case showed us this is not so simple, besides the name
which sucks — we don't *raise* and IRQ we *handle* it.
NAK.
To be on constructive side what I think can help here:
- split patch on per driver basis (and first patch is a simple
introduction of new API)
- rename function
- in each new per-driver patch explain what is the difference in behaviour
- test as many as you can and explain in a cover letter what has been
done and what are the expectations on the ones that you weren't able
to test.
agree.
--
Best regards,
grygorii