Hi Krzysztof Kozlowski,
Thanks for the feedback.
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Thursday, December 7, 2023 8:44 AM
> Subject: Re: [PATCH v4 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062
> to json-schema
>
> On 06/12/2023 16:57, Biju Das wrote:
> > Convert the da9062 PMIC device tree binding documentation to json-
> schema.
> >
> > Update the example to match reality.
> >
> > While at it, update description with link to product information.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v3->v4:
> > * Split the thermal binding patch separate.
> > * Updated the description.
> > v2->v3:
> > * Fixed bot errors related to MAINTAINERS entry, invalid doc
> > references and thermal examples by merging patch#4.
> > v2:
> > * New patch
> > ---
> > .../bindings/input/dlg,da9062-onkey.yaml | 3 +-
> > .../devicetree/bindings/mfd/da9062.txt | 124 ------------
> > .../devicetree/bindings/mfd/dlg,da9063.yaml | 186 +++++++++++++++++-
> > .../bindings/thermal/dlg,da9062-thermal.yaml | 2 +-
> > 4 files changed, 183 insertions(+), 132 deletions(-) delete mode
> > 100644 Documentation/devicetree/bindings/mfd/da9062.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> > b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> > index 4cff91f4bd34..18b6a3f02c07 100644
> > --- a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> > +++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> > @@ -11,8 +11,7 @@ maintainers:
> >
> > description: |
> > This module is part of the DA9061/DA9062/DA9063. For more details
> > about entire
> > - DA9062 and DA9061 chips see
> > Documentation/devicetree/bindings/mfd/da9062.txt
> > - For DA9063 see
> > Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> > + DA906{1,2,3} chips see
> > + Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> >
> > This module provides the KEY_POWER event.
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt
> > b/Documentation/devicetree/bindings/mfd/da9062.txt
> > deleted file mode 100644
> > index c8a7f119ac84..000000000000
> > --- a/Documentation/devicetree/bindings/mfd/da9062.txt
> > +++ /dev/null
> > @@ -1,124 +0,0 @@
> > -* Dialog DA9062 Power Management Integrated Circuit (PMIC)
> > -
> > -Product information for the DA9062 and DA9061 devices can be found
> here:
> > --
&sdata=yWQlq55fIMgsfBxaNXCj4zwBiK14xZcd6l3NYKVnAWM%3D&re
> > served=0
> > -
> > -The DA9062 PMIC consists of:
> > -
> > -Device Supply Names Description
> > ------- ------------ -----------
> > -da9062-regulator : : LDOs & BUCKs
> > -da9062-rtc : : Real-Time Clock
> > -da9062-onkey : : On Key
> > -da9062-watchdog : : Watchdog Timer
> > -da9062-thermal : : Thermal
> > -da9062-gpio : : GPIOs
> > -
> > -The DA9061 PMIC consists of:
> > -
> > -Device Supply Names Description
> > ------- ------------ -----------
> > -da9062-regulator : : LDOs & BUCKs
> > -da9062-onkey : : On Key
> > -da9062-watchdog : : Watchdog Timer
> > -da9062-thermal : : Thermal
> > -
> > -======
> > -
> > -Required properties:
> > -
> > -- compatible : Should be
> > - "dlg,da9062" for DA9062
> > - "dlg,da9061" for DA9061
> > -- reg : Specifies the I2C slave address (this defaults to 0x58 but it
> > can be
> > - modified to match the chip's OTP settings).
> > -
> > -Optional properties:
> > -
> > -- gpio-controller : Marks the device as a gpio controller.
> > -- #gpio-cells : Should be two. The first cell is the pin number and
> the
> > - second cell is used to specify the gpio polarity.
> > -
> > -See Documentation/devicetree/bindings/gpio/gpio.txt for further
> > information on -GPIO bindings.
> > -
> > -- interrupts : IRQ line information.
> > -- interrupt-controller
> > -
> > -See
> > Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> for -further information on IRQ bindings.
> > -
> > -Sub-nodes:
> > -
> > -- regulators : This node defines the settings for the LDOs and BUCKs.
> > - The DA9062 regulators are bound using their names listed below:
> > -
> > - buck1 : BUCK_1
> > - buck2 : BUCK_2
> > - buck3 : BUCK_3
> > - buck4 : BUCK_4
> > - ldo1 : LDO_1
> > - ldo2 : LDO_2
> > - ldo3 : LDO_3
> > - ldo4 : LDO_4
> > -
> > - The DA9061 regulators are bound using their names listed below:
> > -
> > - buck1 : BUCK_1
> > - buck2 : BUCK_2
> > - buck3 : BUCK_3
> > - ldo1 : LDO_1
> > - ldo2 : LDO_2
> > - ldo3 : LDO_3
> > - ldo4 : LDO_4
> > -
> > - The component follows the standard regulator framework and the
> > bindings
> > - details of individual regulator device can be found in:
> > - Documentation/devicetree/bindings/regulator/regulator.txt
> > -
> > - regulator-initial-mode may be specified for buck regulators using
> > mode values
> > - from include/dt-bindings/regulator/dlg,da9063-regulator.h.
> > -
> > -- rtc : This node defines settings required for the Real-Time Clock
> > associated
> > - with the DA9062. There are currently no entries in this binding,
> > however
> > - compatible = "dlg,da9062-rtc" should be added if a node is created.
> > -
> > -- onkey : See ../input/dlg,da9062-onkey.yaml
> > -
> > -- watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
> > -
> > -- thermal : See ../thermal/dlg,da9062-thermal.yaml
> > -
> > -Example:
> > -
> > - pmic0: da9062@58 {
> > - compatible = "dlg,da9062";
> > - reg = <0x58>;
> > - interrupt-parent = <&gpio6>;
> > - interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> > - interrupt-controller;
> > -
> > - rtc {
> > - compatible = "dlg,da9062-rtc";
> > - };
> > -
> > - regulators {
> > - DA9062_BUCK1: buck1 {
> > - regulator-name = "BUCK1";
> > - regulator-min-microvolt = <300000>;
> > - regulator-max-microvolt = <1570000>;
> > - regulator-min-microamp = <500000>;
> > - regulator-max-microamp = <2000000>;
> > - regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
> > - regulator-boot-on;
> > - };
> > - DA9062_LDO1: ldo1 {
> > - regulator-name = "LDO_1";
> > - regulator-min-microvolt = <900000>;
> > - regulator-max-microvolt = <3600000>;
> > - regulator-boot-on;
> > - };
> > - };
> > - };
> > -
> > diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> > b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> > index 676b4f2566ae..54bb23dbc73f 100644
> > --- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> > @@ -4,7 +4,7 @@
> > $id:
> >
> > -title: Dialog DA9063/DA9063L Power Management Integrated Circuit
> > (PMIC)
> > +title: Dialog DA906{3L,3,2,1} Power Management Integrated Circuit
> > +(PMIC)
> >
> > maintainers:
> > - Steve Twiss <stwiss.opensource@xxxxxxxxxxx> @@ -17,10 +17,17 @@
> > description: |
> > moment where all voltage monitors are disabled. Next, as da9063 only
> supports
> > UV *and* OV monitoring, both must be set to the same severity and
> value
> > (0: disable, 1: enable).
> > + Product information for the DA906{3L,3,2,1} devices can be found
> here:
> > + -
> >
> > properties:
> > compatible:
> > enum:
> > + - dlg,da9061
> > + - dlg,da9062
> > - dlg,da9063
> > - dlg,da9063l
> >
> > @@ -35,6 +42,19 @@ properties:
> > "#interrupt-cells":
> > const: 2
> >
> > + gpio:
>
> Old binding did not have such node and nothing in commit msg explained
> changes from pure conversion.
OK will update the commit message. Check patch complained about undocumented compatible.
>
> > + type: object
> > + $ref: /schemas/gpio/gpio.yaml#
>
> ?!? Why? First: It's always selected. Second, so you have two gpio
> controllers? And original binding had 0? Why this is not explained at all?
> Open the binding and look at its properties.
I have referred[1] and [2] to add gpio controller property.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/nxp/imx/imx6qdl-phytec-phycore-som.dtsi?h=next-20231207#n95
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/mfd/da9062.txt?h=next-20231207#n38
>
>
> > + unevaluatedProperties: false
> > + properties:
> > + compatible:
> > + const: dlg,da9062-gpio
> > +
> > + gpio-controller: true
>
> And here is the second gpio-controller...
So you mean it is redundant as $ref: /schemas/gpio/gpio.yaml
has already defined gpio-controller for this object??
>
> > +
> > + "#gpio-cells":
> > + const: 2
> > +
> > onkey:
> > $ref: /schemas/input/dlg,da9062-onkey.yaml
> >
> > @@ -42,7 +62,7 @@ properties:
> > type: object
> > additionalProperties: false
> > patternProperties:
> > - "^(ldo([1-9]|1[01])|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged)$":
> > + "^(ldo([1-9]|1[01])|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck[1-4])$":
> > $ref: /schemas/regulator/regulator.yaml
> > unevaluatedProperties: false
> >
> > @@ -52,7 +72,12 @@ properties:
> > unevaluatedProperties: false
> > properties:
> > compatible:
> > - const: dlg,da9063-rtc
> > + enum:
> > + - dlg,da9063-rtc
> > + - dlg,da9062-rtc
> > +
> > + thermal:
> > + $ref: /schemas/thermal/dlg,da9062-thermal.yaml
> >
> > watchdog:
> > $ref: /schemas/watchdog/dlg,da9062-watchdog.yaml
> > @@ -60,8 +85,65 @@ properties:
> > required:
> > - compatible
> > - reg
> > - - interrupts
> > - - interrupt-controller
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - dlg,da9063
> > + - dlg,da9063l
> > + then:
> > + properties:
> > + thermal: false
> > + gpio: false
> > + gpio-controller: false
> > + "#gpio-cells": false
> > + regulators:
> > + patternProperties:
> > + "^buck[1-4]$": false
> > + required:
> > + - interrupts
> > + - interrupt-controller
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - dlg,da9062
> > + then:
> > + properties:
> > + regulators:
> > + patternProperties:
> > + "^(ldo([5-9]|10|11)|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
> > + required:
> > + - gpio
> > + - onkey
> > + - rtc
> > + - thermal
> > + - watchdog
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - dlg,da9061
> > + then:
> > + properties:
> > + gpio: false
> > + gpio-controller: false
> > + "#gpio-cells": false
> > + regulators:
> > + patternProperties:
> > + "^(ldo([5-9]|10|11)|bcore([1-2]|s-
> merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
> > + rtc: false
> > + required:
> > + - onkey
> > + - thermal
> > + - watchdog
> >
> > additionalProperties: false
> >
> > @@ -118,4 +200,98 @@ examples:
> > };
> > };
> > };
> > +
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/regulator/dlg,da9063-regulator.h>
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + pmic@58 {
> > + compatible = "dlg,da9062";
> > + reg = <0x58>;
> > + #interrupt-cells = <2>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> > + interrupt-controller;
> > +
> > + gpio {
> > + compatible = "dlg,da9062-gpio";
> > + status = "disabled";
>
> Why?
> Where are gpio-controller and cells? For this node and for parent? Why
> this example is incomplete?
I have used a real example [1] here.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/nxp/imx/imx6qdl-phytec-phycore-som.dtsi?h=next-20231207#n95
Currently I don't see any driver is using this compatible other than MFD.
Cheers,
Biju
>
> > + };
> > +
> > + onkey {
> > + compatible = "dlg,da9062-onkey";
> > + };
>
>
> Best regards,
> Krzysztof
