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

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

 



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 ?

> Hopefully next time folks try to add drivers to Analog devices they will
> remember that Analog is pretty active upstream and they will reach out
> to you guys so that work can be coordinated better.

I had no idea, and neither did get_maintainer.pl. I suppose I could have
explored git log deeper, and I'll try to remember for next time.

-- 
Regards,

Laurent Pinchart




[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