Re: [PATCH v2 10/17] mfd: adp5585: add support for key events

[Lee Jones <lee@kernel.org>]

On Thu, 24 Apr 2025, Laurent Pinchart wrote:

> Hi Lee,
> 
> On Thu, Apr 24, 2025 at 05:07:06PM +0100, Lee Jones wrote:
> > On Mon, 21 Apr 2025, Laurent Pinchart wrote:
> > > On Tue, Apr 15, 2025 at 03:49:26PM +0100, Nuno Sá via B4 Relay wrote:
> > > > From: Nuno Sá <nuno.sa@analog.com>
> > > > 
> > > > This adds support for key events. Basically, it adds support for all the
> > > > FW parsing and configuration of the keys (also making sure there's no
> > > > overlaps). They can either be part of a matrix keymap (in which case
> > > > events will be handled by an input device) or they can be gpi's (in which
> > > > case events will be handled by the gpiochip device via gpio_keys). There's
> > > > also support for unlock keys as for reset keys.
> > > 
> > > There's lots of code below specific to matrix keypad handling. Please
> > > move it to the keypad driver, and keep this driver as lean as possible.
> > 
> > Exactly this.  MFD shouldn't need to contain anything that should be
> > handed in the leaf drivers.  Keypad handling should be processed by
> > keypad driver, etc.  If the GPIO driver needs to have visibility into
> > keypad API, add some exported helpers in the Keypad driver.
> 
> To give some context, the ADP5585 is an exotic device. It has this
> feature where different functions (keypad, GPIO, ...) can generate
> events that are all pushed to a single hardware FIFO. The driver reads
> events from the FIFO, and processes them accordingly to the event type.
> Handling the FIFO and reading the events doesn't belong to any
> particular leaf driver, I think it makes sense to keep that in the main
> driver. Processing of the events after dispatching based on the type,
> however, should belong to leaf drivers. I'd like to see less
> keypad-specific code in the main driver, but some additions are likely
> still required.

Understood.  Let's shift out what we can though.

-- 
Lee Jones [李琼斯]