2023-12-17 13:20:29

by Karel Balej

[permalink] [raw]
Subject: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs

From: Karel Balej <[email protected]>

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 <[email protected]>
---
.../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
+
+maintainers:
+ - Karel Balej <[email protected]>
+
+description: |
+ 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
+
+ reg:
+ description: I2C device address
+ 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 {
+ compatible = "marvell,88pm886-a1";
+ reg = <0x30>;
+ interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gic>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ };
+ };
+...
--
2.43.0



2023-12-17 14:24:54

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs


On Sun, 17 Dec 2023 14:16:59 +0100, Karel Balej wrote:
> From: Karel Balej <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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/[email protected]

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.


2023-12-18 15:26:42

by Rob Herring

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs

On Sun, Dec 17, 2023 at 02:16:59PM +0100, Karel Balej wrote:
> From: Karel Balej <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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
>

2023-12-22 17:25:40

by Karel Balej

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs

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.