[v3,00/12] arm64: dts: qcom: sc8280xp-x13s: enable pm8008 camera pmic

Message

Johan Hovold June 8, 2024, 3:55 p.m. UTC

The Qualcomm PM8008 PMIC is a so called QPNP PMIC with seven LDO
regulators, a temperature alarm block and two GPIO pins (which are also
used for interrupt signalling and reset).

Unlike previous QPNP PMICs it uses an I2C rather than SPMI interface,
which has implications for how interrupts are handled.

A previous attempt by Qualcomm to upstream support for PM8008 stalled
two years ago at version 15 after a lot of back and forth discussion on
how best to describe this device in the devicetree. [1]

After reviewing the backstory on this and surveying the current SPMI
PMIC bindings and implementation, I opted for a new approach that does
not describe internal details like register offsets and interrupts in
the devicetree.

The original decision to include registers offsets and internal
interrupts for SPMI PMICs has led to a number of PMIC dtsi being created
to avoid copying lots of boiler plate declarations. This in turn causes
trouble when the PMIC USID address is configurable as the address is
included in every interrupt specifier.

The current SPMI bindings still do not describe the devices fully and
additional data is therefore already provided by drivers (e.g.
additional register blocks, supplies, additional interrupt specifiers).

The fact that PMICs which use two USIDs (addresses) are modelled as two
separate devices causes trouble, for example, when there are
dependencies between subfunctions. [2]

Subfunctions also do not necessarily map neatly onto the 256-register
block partitioning of the SPMI register space, something which has lead
to unresolved inconsistencies in how functions like PWM are described.
[3]

In short, it's a bit of a mess.

With the new style of bindings, by contrast, only essential information
that actually differs between machines would be included in the
devicetree. The bindings would also be mostly decoupled from the
implementation, which has started to leak out into the binding (e.g. how
the QPNP interrupts are handled). This also allows for extending the
implementation without having to update the binding, which is especially
important as Qualcomm does not publish any documentation (e.g. to later
enable regulator over-current protection).

Some PMICs support both I2C and SPMI interfaces (e.g. PM8010) and we
want to be able to reuse the same bindings regardless of the interface.

As a proof concept I have written a new pmc8280 driver for one of the
SPMI PMICs in the Lenovo ThinkPad X13s that uses the new style of
bindings and I've been using that one to control backlight and
peripheral regulators for a while now. Specifically, the gpio and
temperature-alarm blocks can be used with some minor updates to the
current drivers.

That work still needs a bit of polish before posting, but my working PoC
means that I'm confident enough that the new model will work and that we
can go ahead and merge regulator support for the PM8008.

This series is specifically needed for the camera sensors in the X13s,
for which camera subsystem (camss) support has now been merged for 6.10.

The first seven patches are preparatory and can possibly be merged
separately from the rest of the series. The next two patches drop the
broken GPIO support for PM8008 which had already been upstreamed
(dropped in v3). The last five patches rework the binding and MFD
driver, add support for the regulators and enable the camera PMIC on the
X13s.

Johan

[1] https://lore.kernel.org/all/[e-mail]
[2] https://lore.kernel.org/lkml/[e-mail]
[3] https://lore.kernel.org/r/[e-mail]


Changes in v3
 - capitalise MFD commit summaries
 - drop pinctrl patches which have been applied for 6.10
 - amend binding commit message to clarify that the binding is unused
 - move pinctrl subschema under pinctrl node in binding

Changes in v2
 - use IRQ_TYPE_SENSE_MASK in regmap_irq table
 - add post-reset delay
 - reorder pinctrl binding and driver update
 - split out binding cleanups
 - use platform_device_id matching
 - replace underscore in supply names with dash
 - use more fine-grained includes in regulator driver
 - rework regulator driver and update authorship


Johan Hovold (12):
  dt-bindings: mfd: pm8008: Add reset gpio
  mfd: pm8008: Fix regmap irq chip initialisation
  mfd: pm8008: Deassert reset on probe
  mfd: pm8008: Mark regmap structures as const
  mfd: pm8008: Use lower case hex notation
  mfd: pm8008: Rename irq chip
  mfd: pm8008: Drop unused driver data
  dt-bindings: mfd: pm8008: Drop redundant descriptions
  dt-bindings: mfd: pm8008: Rework binding
  mfd: pm8008: Rework to match new DT binding
  regulator: add pm8008 pmic regulator driver
  arm64: dts: qcom: sc8280xp-x13s: enable pm8008 camera pmic

 .../devicetree/bindings/mfd/qcom,pm8008.yaml  | 144 +++++++------
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 123 +++++++++++
 drivers/mfd/Kconfig                           |   1 +
 drivers/mfd/qcom-pm8008.c                     | 169 ++++++++++-----
 drivers/regulator/Kconfig                     |   7 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/qcom-pm8008-regulator.c     | 198 ++++++++++++++++++
 include/dt-bindings/mfd/qcom-pm8008.h         |  19 --
 8 files changed, 532 insertions(+), 130 deletions(-)
 create mode 100644 drivers/regulator/qcom-pm8008-regulator.c
 delete mode 100644 include/dt-bindings/mfd/qcom-pm8008.h

Comments

Lee Jones June 13, 2024, 5:44 p.m. UTC | #1

On Sat, 08 Jun 2024, Johan Hovold wrote:

> The Qualcomm PM8008 PMIC is a so called QPNP PMIC with seven LDO
> regulators, a temperature alarm block and two GPIO pins (which are also
> used for interrupt signalling and reset).

[...]

> Johan
> 
> [1] https://lore.kernel.org/all/[e-mail]
> [2] https://lore.kernel.org/lkml/[e-mail]
> [3] https://lore.kernel.org/r/[e-mail]
> 
> 
> Changes in v3
>  - capitalise MFD commit summaries
>  - drop pinctrl patches which have been applied for 6.10
>  - amend binding commit message to clarify that the binding is unused
>  - move pinctrl subschema under pinctrl node in binding
> 
> Changes in v2
>  - use IRQ_TYPE_SENSE_MASK in regmap_irq table
>  - add post-reset delay
>  - reorder pinctrl binding and driver update
>  - split out binding cleanups
>  - use platform_device_id matching
>  - replace underscore in supply names with dash
>  - use more fine-grained includes in regulator driver
>  - rework regulator driver and update authorship
> 
> 
> Johan Hovold (12):
>   dt-bindings: mfd: pm8008: Add reset gpio
>   mfd: pm8008: Fix regmap irq chip initialisation
>   mfd: pm8008: Deassert reset on probe
>   mfd: pm8008: Mark regmap structures as const
>   mfd: pm8008: Use lower case hex notation
>   mfd: pm8008: Rename irq chip
>   mfd: pm8008: Drop unused driver data
>   dt-bindings: mfd: pm8008: Drop redundant descriptions
>   dt-bindings: mfd: pm8008: Rework binding
>   mfd: pm8008: Rework to match new DT binding
>   regulator: add pm8008 pmic regulator driver
>   arm64: dts: qcom: sc8280xp-x13s: enable pm8008 camera pmic
> 
>  .../devicetree/bindings/mfd/qcom,pm8008.yaml  | 144 +++++++------
>  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 123 +++++++++++
>  drivers/mfd/Kconfig                           |   1 +
>  drivers/mfd/qcom-pm8008.c                     | 169 ++++++++++-----
>  drivers/regulator/Kconfig                     |   7 +
>  drivers/regulator/Makefile                    |   1 +
>  drivers/regulator/qcom-pm8008-regulator.c     | 198 ++++++++++++++++++
>  include/dt-bindings/mfd/qcom-pm8008.h         |  19 --
>  8 files changed, 532 insertions(+), 130 deletions(-)
>  create mode 100644 drivers/regulator/qcom-pm8008-regulator.c
>  delete mode 100644 include/dt-bindings/mfd/qcom-pm8008.h

Applied patches 1-11 and submitted for build testing.

Once complete, I'll get the pull-request out for Mark.

Lee Jones June 14, 2024, 9:03 a.m. UTC | #2

On Sat, 08 Jun 2024 17:55:14 +0200, Johan Hovold wrote:
> The Qualcomm PM8008 PMIC is a so called QPNP PMIC with seven LDO
> regulators, a temperature alarm block and two GPIO pins (which are also
> used for interrupt signalling and reset).
> 
> Unlike previous QPNP PMICs it uses an I2C rather than SPMI interface,
> which has implications for how interrupts are handled.
> 
> [...]

Applied, thanks!

[01/12] dt-bindings: mfd: pm8008: Add reset gpio
        commit: 0682cfa3325fefe8a3cb1c02854135ee73b8ae16
[02/12] mfd: pm8008: Fix regmap irq chip initialisation
        commit: 6ad7f80b53251dbbca81e18a17cf6f8bcd34cb20
[03/12] mfd: pm8008: Deassert reset on probe
        commit: c251befb097ef1ebb509d48bb3e1181b94fd4d2a
[04/12] mfd: pm8008: Mark regmap structures as const
        commit: 742bdd99aa9acace13385b66c6f3946f26b109fe
[05/12] mfd: pm8008: Use lower case hex notation
        commit: a4b3225f06e4f8fa7266236407ab7de34b66f044
[06/12] mfd: pm8008: Rename irq chip
        commit: 3162cd961eba14fbb377d4f13c853c586cd5d063
[07/12] mfd: pm8008: Drop unused driver data
        commit: 40ac32d19985836348313b7087c8f37232084c54
[08/12] dt-bindings: mfd: pm8008: Drop redundant descriptions
        commit: 8643ef1213eeaf99ee529f35a6c2976ea1e316bc
[09/12] dt-bindings: mfd: pm8008: Rework binding
        commit: 8c72db5884a3c821de6ba9c387c8fe52e13e5a34
[10/12] mfd: pm8008: Rework to match new DT binding
        commit: 288b550463cf5dd21ad34f736b8c5ccb7ff69ceb
[11/12] regulator: add pm8008 pmic regulator driver
        commit: 11d861d227ed1c4068597289267247aac5ac50fa

--
Lee Jones [李琼斯]

Bjorn Andersson June 21, 2024, 6:11 a.m. UTC | #3

On Sat, 08 Jun 2024 17:55:14 +0200, Johan Hovold wrote:
> The Qualcomm PM8008 PMIC is a so called QPNP PMIC with seven LDO
> regulators, a temperature alarm block and two GPIO pins (which are also
> used for interrupt signalling and reset).
> 
> Unlike previous QPNP PMICs it uses an I2C rather than SPMI interface,
> which has implications for how interrupts are handled.
> 
> [...]

Applied, thanks!

[12/12] arm64: dts: qcom: sc8280xp-x13s: enable pm8008 camera pmic
        commit: b5477d5f5272a079e2ddeffd00490528884f5aa5

Best regards,