mbox series

[v2,00/17] mfd: adp5585: support keymap events and drop legacy Input driver

Message ID 20250415-dev-adp5589-fw-v2-0-3a799c3ed812@analog.com (mailing list archive)
Headers show
Series mfd: adp5585: support keymap events and drop legacy Input driver | expand

Message

Nuno Sá April 15, 2025, 2:49 p.m. UTC
The adp5585 MFD driver was introduced in 6.11 adding support for gpio
and PWM. However, the gpio part of it was already supported as part of
the keyboard driver:

https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/input/keyboard/adp5589-keys.c#L532

On top of that it also overlapped with my refactoring of the above driver [1]
to drop usage of platform data and use FW properties instead.

Now, it actually makes sense for this device to be supported under MFD
and since the "legacy" input device depends on platform data that is not
defined anywhere the plan in this series is to add support for the
keyboard and adp5589 devices as part of the MFD driver. Once the MFD
driver supports all that's supported in the Input one, we drop it...

For DT Maintainers:

The compatible for adp5589 is part of trivial devices. To me, it makes
sense to remove it in the patch where we drop the driver but doing so
would result in a warning when adding the same compatible for the MFD
bindings. Hence, I remove it in that patch. Is that ok?

Uwe:

In my eval board, I could see that reading the GPIO value (when
configured as input) does not work when OSC_EN is not set. Therefore,
commit ("pwm: adp5585: don't control OSC_EN in the pwm driver") could
very well have a Fixes tag. However I'm not 100% sure it's a real issue
or something special to my eval board.

It would be nice if Laurent or Liu could test the PWM bits or even
check that the above is also an issue for their platform.

[1]: https://lore.kernel.org/linux-input/d1395bd61ce58b3734121bca4e09605a3e997af3.camel@gmail.com/

BTW the series is based on linux-next/master

---
Changes in v2:
- Patch 5:
   * Do not nest if:then:else::if:then.
- Patch 6:
   * Make use of the adp5585 info variables and adp5589 volatile regs.
- Patch 9:
   * Use standard "poll-interval" property (and move it before vendor
     properties).
- Patch 10:
   * Make sure to include bitfield.h.

- Link to v1: https://lore.kernel.org/r/20250313-dev-adp5589-fw-v1-0-20e80d4bd4ea@analog.com

---
Nuno Sá (17):
      dt-bindings: mfd: adp5585: ease on the required properties
      mfd: adp5585: enable oscilator during probe
      pwm: adp5585: don't control OSC_EN in the pwm driver
      mfd: adp5585: make use of MFD_CELL_NAME()
      dt-bindings: mfd: adp5585: document adp5589 I/O expander
      mfd: adp5585: add support for adp5589
      gpio: adp5585: add support for the ad5589 expander
      pwm: adp5585: add support for adp5589
      dt-bindings: mfd: adp5585: add properties for input events
      mfd: adp5585: add support for key events
      gpio: adp5585: support gpi events
      Input: adp5585: Add Analog Devices ADP5585/89 support
      Input: adp5589: remove the driver
      mfd: adp5585: support getting vdd regulator
      dt-bindings: mfd: adp5585: document reset gpio
      mfd: adp5585: add support for a reset pin
      pwm: adp5585: make sure to include mod_devicetable.h

 .../devicetree/bindings/mfd/adi,adp5585.yaml       |  240 ++++-
 .../devicetree/bindings/trivial-devices.yaml       |    2 -
 MAINTAINERS                                        |    1 +
 drivers/gpio/Kconfig                               |    1 +
 drivers/gpio/gpio-adp5585.c                        |  299 +++++-
 drivers/input/keyboard/Kconfig                     |   21 +-
 drivers/input/keyboard/Makefile                    |    2 +-
 drivers/input/keyboard/adp5585-keys.c              |  221 ++++
 drivers/input/keyboard/adp5589-keys.c              | 1066 --------------------
 drivers/mfd/adp5585.c                              |  808 ++++++++++++++-
 drivers/pwm/pwm-adp5585.c                          |   57 +-
 include/linux/mfd/adp5585.h                        |  153 ++-
 12 files changed, 1709 insertions(+), 1162 deletions(-)
---
base-commit: 5b37f7bfff3b1582c34be8fb23968b226db71ebd
change-id: 20250311-dev-adp5589-fw-e04cfd945286
--

Thanks!
- Nuno Sá

Comments

Laurent Pinchart April 15, 2025, 3:56 p.m. UTC | #1
Hi Nuno,

On Tue, Apr 15, 2025 at 03:49:16PM +0100, Nuno Sá via B4 Relay wrote:
> The adp5585 MFD driver was introduced in 6.11 adding support for gpio
> and PWM. However, the gpio part of it was already supported as part of
> the keyboard driver:
> 
> https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/input/keyboard/adp5589-keys.c#L532
> 
> On top of that it also overlapped with my refactoring of the above driver [1]
> to drop usage of platform data and use FW properties instead.
> 
> Now, it actually makes sense for this device to be supported under MFD
> and since the "legacy" input device depends on platform data that is not
> defined anywhere the plan in this series is to add support for the
> keyboard and adp5589 devices as part of the MFD driver. Once the MFD
> driver supports all that's supported in the Input one, we drop it...
> 
> For DT Maintainers:
> 
> The compatible for adp5589 is part of trivial devices. To me, it makes
> sense to remove it in the patch where we drop the driver but doing so
> would result in a warning when adding the same compatible for the MFD
> bindings. Hence, I remove it in that patch. Is that ok?
> 
> Uwe:
> 
> In my eval board, I could see that reading the GPIO value (when
> configured as input) does not work when OSC_EN is not set. Therefore,
> commit ("pwm: adp5585: don't control OSC_EN in the pwm driver") could
> very well have a Fixes tag. However I'm not 100% sure it's a real issue
> or something special to my eval board.
> 
> It would be nice if Laurent or Liu could test the PWM bits or even
> check that the above is also an issue for their platform.

I'll give it a try, but it will need to wait until next week.

> [1]: https://lore.kernel.org/linux-input/d1395bd61ce58b3734121bca4e09605a3e997af3.camel@gmail.com/
> 
> BTW the series is based on linux-next/master
> 
> ---
> Changes in v2:
> - Patch 5:
>    * Do not nest if:then:else::if:then.
> - Patch 6:
>    * Make use of the adp5585 info variables and adp5589 volatile regs.
> - Patch 9:
>    * Use standard "poll-interval" property (and move it before vendor
>      properties).
> - Patch 10:
>    * Make sure to include bitfield.h.
> 
> - Link to v1: https://lore.kernel.org/r/20250313-dev-adp5589-fw-v1-0-20e80d4bd4ea@analog.com
> 
> ---
> Nuno Sá (17):
>       dt-bindings: mfd: adp5585: ease on the required properties
>       mfd: adp5585: enable oscilator during probe
>       pwm: adp5585: don't control OSC_EN in the pwm driver
>       mfd: adp5585: make use of MFD_CELL_NAME()
>       dt-bindings: mfd: adp5585: document adp5589 I/O expander
>       mfd: adp5585: add support for adp5589
>       gpio: adp5585: add support for the ad5589 expander
>       pwm: adp5585: add support for adp5589
>       dt-bindings: mfd: adp5585: add properties for input events
>       mfd: adp5585: add support for key events
>       gpio: adp5585: support gpi events
>       Input: adp5585: Add Analog Devices ADP5585/89 support
>       Input: adp5589: remove the driver
>       mfd: adp5585: support getting vdd regulator
>       dt-bindings: mfd: adp5585: document reset gpio
>       mfd: adp5585: add support for a reset pin
>       pwm: adp5585: make sure to include mod_devicetable.h
> 
>  .../devicetree/bindings/mfd/adi,adp5585.yaml       |  240 ++++-
>  .../devicetree/bindings/trivial-devices.yaml       |    2 -
>  MAINTAINERS                                        |    1 +
>  drivers/gpio/Kconfig                               |    1 +
>  drivers/gpio/gpio-adp5585.c                        |  299 +++++-
>  drivers/input/keyboard/Kconfig                     |   21 +-
>  drivers/input/keyboard/Makefile                    |    2 +-
>  drivers/input/keyboard/adp5585-keys.c              |  221 ++++
>  drivers/input/keyboard/adp5589-keys.c              | 1066 --------------------
>  drivers/mfd/adp5585.c                              |  808 ++++++++++++++-
>  drivers/pwm/pwm-adp5585.c                          |   57 +-
>  include/linux/mfd/adp5585.h                        |  153 ++-
>  12 files changed, 1709 insertions(+), 1162 deletions(-)
> ---
> base-commit: 5b37f7bfff3b1582c34be8fb23968b226db71ebd
> change-id: 20250311-dev-adp5589-fw-e04cfd945286
> --
Liu Ying April 16, 2025, 9:02 a.m. UTC | #2
On 04/15/2025, Nuno Sá via B4 Relay wrote:
> The adp5585 MFD driver was introduced in 6.11 adding support for gpio
> and PWM. However, the gpio part of it was already supported as part of
> the keyboard driver:
> 
> https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/input/keyboard/adp5589-keys.c#L532
> 
> On top of that it also overlapped with my refactoring of the above driver [1]
> to drop usage of platform data and use FW properties instead.
> 
> Now, it actually makes sense for this device to be supported under MFD
> and since the "legacy" input device depends on platform data that is not
> defined anywhere the plan in this series is to add support for the
> keyboard and adp5589 devices as part of the MFD driver. Once the MFD
> driver supports all that's supported in the Input one, we drop it...
> 
> For DT Maintainers:
> 
> The compatible for adp5589 is part of trivial devices. To me, it makes
> sense to remove it in the patch where we drop the driver but doing so
> would result in a warning when adding the same compatible for the MFD
> bindings. Hence, I remove it in that patch. Is that ok?
> 
> Uwe:
> 
> In my eval board, I could see that reading the GPIO value (when
> configured as input) does not work when OSC_EN is not set. Therefore,
> commit ("pwm: adp5585: don't control OSC_EN in the pwm driver") could
> very well have a Fixes tag. However I'm not 100% sure it's a real issue
> or something special to my eval board.
> 
> It would be nice if Laurent or Liu could test the PWM bits or even
> check that the above is also an issue for their platform.

With this v2 patch series, PWM backlight(controlled by ADP5585 pin R3) still
works for my i.MX93 11x11 EVK.  Without this v2 patch series, if I change PWM
backlight to GPIO backlight(keep using pin R3), the GPIO backlight brightness
can be set to 0 or 1, meaning I can see the backlight is off or on.  So, it
appears that GPIO output still works when OSC_EN is zero(I dumped GENERAL_CFG
register @0x3b as 0x00), though I didn't test GPIO input.

> 
> [1]: https://lore.kernel.org/linux-input/d1395bd61ce58b3734121bca4e09605a3e997af3.camel@gmail.com/
> 
> BTW the series is based on linux-next/master
> 
> ---
> Changes in v2:
> - Patch 5:
>    * Do not nest if:then:else::if:then.
> - Patch 6:
>    * Make use of the adp5585 info variables and adp5589 volatile regs.
> - Patch 9:
>    * Use standard "poll-interval" property (and move it before vendor
>      properties).
> - Patch 10:
>    * Make sure to include bitfield.h.
> 
> - Link to v1: https://lore.kernel.org/r/20250313-dev-adp5589-fw-v1-0-20e80d4bd4ea@analog.com
> 
> ---
> Nuno Sá (17):
>       dt-bindings: mfd: adp5585: ease on the required properties
>       mfd: adp5585: enable oscilator during probe
>       pwm: adp5585: don't control OSC_EN in the pwm driver
>       mfd: adp5585: make use of MFD_CELL_NAME()
>       dt-bindings: mfd: adp5585: document adp5589 I/O expander
>       mfd: adp5585: add support for adp5589
>       gpio: adp5585: add support for the ad5589 expander
>       pwm: adp5585: add support for adp5589
>       dt-bindings: mfd: adp5585: add properties for input events
>       mfd: adp5585: add support for key events
>       gpio: adp5585: support gpi events
>       Input: adp5585: Add Analog Devices ADP5585/89 support
>       Input: adp5589: remove the driver
>       mfd: adp5585: support getting vdd regulator
>       dt-bindings: mfd: adp5585: document reset gpio
>       mfd: adp5585: add support for a reset pin
>       pwm: adp5585: make sure to include mod_devicetable.h
> 
>  .../devicetree/bindings/mfd/adi,adp5585.yaml       |  240 ++++-
>  .../devicetree/bindings/trivial-devices.yaml       |    2 -
>  MAINTAINERS                                        |    1 +
>  drivers/gpio/Kconfig                               |    1 +
>  drivers/gpio/gpio-adp5585.c                        |  299 +++++-
>  drivers/input/keyboard/Kconfig                     |   21 +-
>  drivers/input/keyboard/Makefile                    |    2 +-
>  drivers/input/keyboard/adp5585-keys.c              |  221 ++++
>  drivers/input/keyboard/adp5589-keys.c              | 1066 --------------------
>  drivers/mfd/adp5585.c                              |  808 ++++++++++++++-
>  drivers/pwm/pwm-adp5585.c                          |   57 +-
>  include/linux/mfd/adp5585.h                        |  153 ++-
>  12 files changed, 1709 insertions(+), 1162 deletions(-)
> ---
> base-commit: 5b37f7bfff3b1582c34be8fb23968b226db71ebd
> change-id: 20250311-dev-adp5589-fw-e04cfd945286
> --
> 
> Thanks!
> - Nuno Sá
> 
>
Nuno Sá April 16, 2025, 10:03 a.m. UTC | #3
On Wed, 2025-04-16 at 17:02 +0800, Liu Ying wrote:
> On 04/15/2025, Nuno Sá via B4 Relay wrote:
> > The adp5585 MFD driver was introduced in 6.11 adding support for gpio
> > and PWM. However, the gpio part of it was already supported as part of
> > the keyboard driver:
> > 
> > https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/input/keyboard/adp5589-keys.c#L532
> > 
> > On top of that it also overlapped with my refactoring of the above driver
> > [1]
> > to drop usage of platform data and use FW properties instead.
> > 
> > Now, it actually makes sense for this device to be supported under MFD
> > and since the "legacy" input device depends on platform data that is not
> > defined anywhere the plan in this series is to add support for the
> > keyboard and adp5589 devices as part of the MFD driver. Once the MFD
> > driver supports all that's supported in the Input one, we drop it...
> > 
> > For DT Maintainers:
> > 
> > The compatible for adp5589 is part of trivial devices. To me, it makes
> > sense to remove it in the patch where we drop the driver but doing so
> > would result in a warning when adding the same compatible for the MFD
> > bindings. Hence, I remove it in that patch. Is that ok?
> > 
> > Uwe:
> > 
> > In my eval board, I could see that reading the GPIO value (when
> > configured as input) does not work when OSC_EN is not set. Therefore,
> > commit ("pwm: adp5585: don't control OSC_EN in the pwm driver") could
> > very well have a Fixes tag. However I'm not 100% sure it's a real issue
> > or something special to my eval board.
> > 
> > It would be nice if Laurent or Liu could test the PWM bits or even
> > check that the above is also an issue for their platform.
> 
> With this v2 patch series, PWM backlight(controlled by ADP5585 pin R3) still
> works for my i.MX93 11x11 EVK.  Without this v2 patch series, if I change PWM
> backlight to GPIO backlight(keep using pin R3), the GPIO backlight brightness
> can be set to 0 or 1, meaning I can see the backlight is off or on.  So, it
> appears that GPIO output still works when OSC_EN is zero(I dumped GENERAL_CFG
> register @0x3b as 0x00), though I didn't test GPIO input.
> 

Yeah, the input case seems to be the problematic one... Anyways, thanks for
testing and confirm that PWM is not broken by this series.

- Nuno Sá

> > 
> > [1]:
> > https://lore.kernel.org/linux-input/d1395bd61ce58b3734121bca4e09605a3e997af3.camel@gmail.com/
> > 
> > BTW the series is based on linux-next/master
> > 
> > ---
> > Changes in v2:
> > - Patch 5:
> >    * Do not nest if:then:else::if:then.
> > - Patch 6:
> >    * Make use of the adp5585 info variables and adp5589 volatile regs.
> > - Patch 9:
> >    * Use standard "poll-interval" property (and move it before vendor
> >      properties).
> > - Patch 10:
> >    * Make sure to include bitfield.h.
> > 
> > - Link to v1:
> > https://lore.kernel.org/r/20250313-dev-adp5589-fw-v1-0-20e80d4bd4ea@analog.com
> > 
> > ---
> > Nuno Sá (17):
> >       dt-bindings: mfd: adp5585: ease on the required properties
> >       mfd: adp5585: enable oscilator during probe
> >       pwm: adp5585: don't control OSC_EN in the pwm driver
> >       mfd: adp5585: make use of MFD_CELL_NAME()
> >       dt-bindings: mfd: adp5585: document adp5589 I/O expander
> >       mfd: adp5585: add support for adp5589
> >       gpio: adp5585: add support for the ad5589 expander
> >       pwm: adp5585: add support for adp5589
> >       dt-bindings: mfd: adp5585: add properties for input events
> >       mfd: adp5585: add support for key events
> >       gpio: adp5585: support gpi events
> >       Input: adp5585: Add Analog Devices ADP5585/89 support
> >       Input: adp5589: remove the driver
> >       mfd: adp5585: support getting vdd regulator
> >       dt-bindings: mfd: adp5585: document reset gpio
> >       mfd: adp5585: add support for a reset pin
> >       pwm: adp5585: make sure to include mod_devicetable.h
> > 
> >  .../devicetree/bindings/mfd/adi,adp5585.yaml       |  240 ++++-
> >  .../devicetree/bindings/trivial-devices.yaml       |    2 -
> >  MAINTAINERS                                        |    1 +
> >  drivers/gpio/Kconfig                               |    1 +
> >  drivers/gpio/gpio-adp5585.c                        |  299 +++++-
> >  drivers/input/keyboard/Kconfig                     |   21 +-
> >  drivers/input/keyboard/Makefile                    |    2 +-
> >  drivers/input/keyboard/adp5585-keys.c              |  221 ++++
> >  drivers/input/keyboard/adp5589-keys.c              | 1066 -----------------
> > ---
> >  drivers/mfd/adp5585.c                              |  808 ++++++++++++++-
> >  drivers/pwm/pwm-adp5585.c                          |   57 +-
> >  include/linux/mfd/adp5585.h                        |  153 ++-
> >  12 files changed, 1709 insertions(+), 1162 deletions(-)
> > ---
> > base-commit: 5b37f7bfff3b1582c34be8fb23968b226db71ebd
> > change-id: 20250311-dev-adp5589-fw-e04cfd945286
> > --
> > 
> > Thanks!
> > - Nuno Sá
> > 
> >