[RFC,0/5] support for Marvell 88PM886 PMIC

From: Karel Balej <balejk@matfyz.cz>

Hello,

the following implements basic support for Marvell's 88PM886 PMIC which
is found for instance as a component of the samsung,coreprimevelte
smartphone which inspired this and also serves as a testing platform.

The code for the MFD is based primarily on this old series [1] with the
addition of poweroff based on the smartphone's downstream kernel tree
[2]. The onkey driver is based on the latter. I am not in possesion of
the datasheet.

The vendor version of this driver includes support for a similar chip:
88PM880. While that is not included here it was written with it in mind
and it should be quite straighforward to add it.

[1] https://lore.kernel.org/all/1434098601-3498-1-git-send-email-yizhang@marvell.com/
[2] https://github.com/CoderCharmander/g361f-kernel

Thank you and kind regards,
K. B.

Karel Balej (5):
  dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs
  mfd: add 88pm88x driver
  dt-bindings: input: add entry for 88pm88x-onkey
  input: add onkey driver for Marvell 88PM88X PMICs
  MAINTAINERS: add myself for Marvell 88PM88X PMICs

 .../bindings/input/marvell,88pm88x-onkey.yaml |  30 +++
 .../bindings/mfd/marvell,88pm88x.yaml         |  59 ++++++
 MAINTAINERS                                   |   9 +
 drivers/input/misc/88pm88x-onkey.c            | 103 +++++++++
 drivers/input/misc/Kconfig                    |  10 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/mfd/88pm88x.c                         | 199 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  11 +
 drivers/mfd/Makefile                          |   1 +
 include/linux/mfd/88pm88x.h                   |  60 ++++++
 10 files changed, 483 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
 create mode 100644 drivers/input/misc/88pm88x-onkey.c
 create mode 100644 drivers/mfd/88pm88x.c
 create mode 100644 include/linux/mfd/88pm88x.h

On Sun, 17 Dec 2023 14:16:59 +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
> 
> Marvell 88PM880 and 88PM886 are two similar PMICs with mostly matching
> register mapping and subdevices such as onkey, regulators or battery and
> charger. Both seem to come in two revisions which seem to be handled
> slightly differently in some subdevice drivers.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>  .../bindings/mfd/marvell,88pm88x.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/mfd/marvell,88pm88x.example.dts:30.31-32 syntax error
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/mfd/marvell,88pm88x.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2
make: *** [Makefile:234: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231217131838.7569-2-karelb@gimli.ms.mff.cuni.cz

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

On Sun, Dec 17, 2023 at 02:16:59PM +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
> 
> Marvell 88PM880 and 88PM886 are two similar PMICs with mostly matching
> register mapping and subdevices such as onkey, regulators or battery and
> charger. Both seem to come in two revisions which seem to be handled
> slightly differently in some subdevice drivers.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>  .../bindings/mfd/marvell,88pm88x.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> new file mode 100644
> index 000000000000..e075729c360f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell 88PM88X PMIC core MFD

Drop 'MFD'.

> +
> +maintainers:
> +  - Karel Balej <balejk@matfyz.cz>
> +
> +description: |

Don't need '|' as there is no formatting to preserve.

> +  Marvell 88PM880 and 88PM886 are two similar PMICs providing
> +  several functions such as onkey, regulators or battery and
> +  charger. Both seem to come in two revisions -- A0 and A1.
> +
> +properties:
> +  compatible:
> +    const: marvell,88pm886-a1

The description talks about 4 different devices, but only 1 here. 

Do you expect to need A0 support? Devices with these PMICs should be 
known and few, right? 

> +
> +  reg:
> +    description: I2C device address

Drop.

> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic0: 88pm886@30 {

pmic@30

Drop the unused label.

> +        compatible = "marvell,88pm886-a1";
> +        reg = <0x30>;
> +        interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;

You need the header for this.

You'll find the input binding fails too. Please test your bindings 
before sending.

> +        interrupt-parent = <&gic>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +      };
> +    };
> +...
> -- 
> 2.43.0
>

Rob,

thank you very much for your feedback.

On Mon Dec 18, 2023 at 4:17 PM CET, Rob Herring wrote:
> > +  Marvell 88PM880 and 88PM886 are two similar PMICs providing
> > +  several functions such as onkey, regulators or battery and
> > +  charger. Both seem to come in two revisions -- A0 and A1.
> > +
> > +properties:
> > +  compatible:
> > +    const: marvell,88pm886-a1
>
> The description talks about 4 different devices, but only 1 here. 
>
> Do you expect to need A0 support? Devices with these PMICs should be 
> known and few, right? 

I know of three smartphones which have 88PM886 and all of them (at least
the revisions that have been tested) seem to use A1. So no, I don't know
of any device that would need A0, but I wanted have the driver ready in
case somebody needed to add it later. What change do you then suggest?

Thank you and kind regards,
K. B.