Re: [PATCH 00/13] Input: adp5589: refactor and platform_data removal

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

 



On Sat, 2024-10-19 at 20:18 +0300, Laurent Pinchart wrote:
> On Fri, Oct 18, 2024 at 02:30:20PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 16, 2024 at 03:36:03PM +0200, Nuno Sá wrote:
> > > On Tue, 2024-10-01 at 15:41 +0200, Nuno Sa wrote:
> > > > This series aims to remove platform data dependency from the adp5589
> > > > driver (as no platform is really using it) and instead add support for
> > > > FW properties. Note that rows and columns for the keypad are being given
> > > > as masks and that was briefly discussed with Dmitry. For context
> > > > on why this is being done as mask [1].
> > > > 
> > > > The first couple of patches are fixes that we may want to backport...
> > > > 
> > > > [1]:
> > > > https://lore.kernel.org/linux-input/9db96c99c805e615ba40ca7fd3632174d1e8d11f.camel@xxxxxxxxx/
> > > > 
> > > > ---
> > > > Nuno Sa (13):
> > > >       Input: adp5589-keys: fix NULL pointer dereference
> > > >       Input: adp5589-keys: fix adp5589_gpio_get_value()
> > > >       Input: adp5589-keys: add chip_info structure
> > > >       Input: adp5589-keys: support gpi key events as 'gpio keys'
> > > >       dt-bindings: input: Document adp5589 and similar devices
> > > >       Input: adp5589-keys: add support for fw properties
> > > >       Input: adp5589-keys: add guard() notation
> > > >       Input: adp5589-keys: bail out on returned error
> > > >       Input: adp5589-keys: refactor adp5589_read()
> > > >       Input: adp5589-keys: fix coding style
> > > >       Input: adp5589-keys: unify adp_constants in info struct
> > > >       Input: adp5589-keys: make use of dev_err_probe()
> > > >       Input: adp5589-keys: add regulator support
> > > > 
> > > >  .../devicetree/bindings/input/adi,adp5589.yaml     |  310 +++++
> > > >  .../devicetree/bindings/trivial-devices.yaml       |    6 -
> > > >  MAINTAINERS                                        |    8 +
> > > >  drivers/input/keyboard/Kconfig                     |    3 +
> > > >  drivers/input/keyboard/adp5589-keys.c              | 1397
> > > > +++++++++++++-------
> > > >  include/linux/input/adp5589.h                      |  180 ---
> > > >  6 files changed, 1254 insertions(+), 650 deletions(-)
> > > > ---
> > > > base-commit: c7bf046925dc5885d9c4d8fbcbb7e4e73665bfcf
> > > > change-id: 20240930-b4-dev-adp5589-fw-conversion-955b2f42da70
> > > > --
> > > > 
> > > > Thanks!
> > > > - Nuno Sá
> > > 
> > > Hi Dmitry,
> > > 
> > > Something really caught my attention now while checking 6.12 merge window.
> > > It seems
> > > we have a new MFD device for adp5585 [1] which adds duplicated
> > > functionality (that
> > > was already present in adp5589-keys.c). So, having this as MFD might makes
> > > sense
> > > (even though it makes it harder to validate the keys and to make use of
> > > gpio-keys)
> > > but we are now duplicating GPIO support. Bottom line, not sure what we
> > > should do next
> > > and should I proceed for v2?
> > > 
> > > Also ccing Lee and Bartosz...
> > 
> > Let's add Laurent and Krzysztof too please.
> > 
> > I am surprised we do not see warnings for various bots because
> > "adi,adp5585" compatible is present in trivial devices.
> > 
> > I think moving it all to MFD makes sense (I think original drivers were
> > added well before we had MFD infrastructure), but we need to make sure
> > the device tree binding is complete and allows describing keypad (and if
> > not maybe we can pull it from the release and work on it so that it
> > describes the hardware fully).
> 
> Keypad support is nice. I didn't include it in my adp5585 driver
> submission because I had no way to test it. Would it be more difficult
> to add it to the MFD driver, compared to what is done in this series ?

Well, I still wonder about the GPIO part of it... It's duplicating something
that existed before even if we all agree that MFD makes more sense.

Anyways, there's no point in over discussing this... I'll see how it works
sending  my v2 on top of the MFD implementations. This means that adp5589 also
needs to be added.

- 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