| Message ID | 20231206155740.5278-9-biju.das.jz@bp.renesas.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Convert DA906{1,2} bindings to json-schema | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 247 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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@bp.renesas.com> > --- > 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: > -- https://www.dialog-semiconductor.com/products/da9062 > -- https://www.dialog-semiconductor.com/products/da9061 > - > -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: http://devicetree.org/schemas/mfd/dlg,da9063.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -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@diasemi.com> > @@ -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: > + - https://www.dialog-semiconductor.com/products/da9063l > + - https://www.dialog-semiconductor.com/products/da9063 > + - https://www.dialog-semiconductor.com/products/da9062 > + - https://www.dialog-semiconductor.com/products/da9061 > > 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. > + 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. > + unevaluatedProperties: false > + properties: > + compatible: > + const: dlg,da9062-gpio > + > + gpio-controller: true And here is the second gpio-controller... > + > + "#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? > + }; > + > + onkey { > + compatible = "dlg,da9062-onkey"; > + }; Best regards, Krzysztof
Hi Krzysztof Kozlowski, Thanks for the feedback. > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > 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@bp.renesas.com> > > --- > > 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@diasemi.com> @@ -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
On 07/12/2023 18:03, Biju Das wrote: Trim the quote from irrelevant context, especially if your email client malforms replies... (because it does) >>> @@ -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 But does it make sense? Don't just blindly copy things, but actually investigate whether this is correct DTS. > >> >> >>> + 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?? I meant this would mean you have two GPIO controllers. Why one device would have two GPIO controllers? Please answer to this in commit msg, so there will be no questions/concerns. You have entire commit msg to explain all weird and unexpected things with this binding. ... >>> + #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? Why disabling? Drop all statuses from all your binding examples. >> 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. Open the MFD so you will see it... Best regards, Krzysztof
Hi Krzysztof Kozlowski, Thanks for the feedback. > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Friday, December 8, 2023 7:56 AM > Subject: Re: [PATCH v4 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 > to json-schema > > On 07/12/2023 18:03, Biju Das wrote: > > Trim the quote from irrelevant context, especially if your email client > malforms replies... (because it does) > > >>> @@ -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. > > > > But does it make sense? Don't just blindly copy things, but actually > investigate whether this is correct DTS. It is indeed incorrect. I have tested GPIO on my board. The gpio controller must be defined in parent node. Otherwise gpio probe will fail. the dt example is as below da9062: pmic@58 { compatible = "dlg,da9062"; reg = <0x58>; gpio-controller; #gpio-cells = <2>; sd0-pwr-sel { gpio-hog; gpios = <1 0>; input; line-name = "SD0_PWR_SEL"; }; sd1-pwr-sel { gpio-hog; gpios = <2 0>; input; line-name = "SD1_PWR_SEL"; }; sw-et0-en { gpio-hog; gpios = <3 0>; input; line-name = "SW_ET0_EN#"; }; pmic-good { gpio-hog; gpios = <4 0>; output-high; line-name = "PMIC_PGOOD"; }; da9062_rtc: rtc { compatible = "dlg,da9062-rtc"; }; da9062_onkey: onkey { compatible = "dlg,da9062-onkey"; status = "disabled"; }; watchdog { compatible = "dlg,da9062-watchdog"; status = "disabled"; }; thermal { compatible = "dlg,da9062-thermal"; status = "disabled"; }; gpio { compatible = "dlg,da9062-gpio"; }; }; > > > > >> > >> > >>> + 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?? > > I meant this would mean you have two GPIO controllers. Why one device > would have two GPIO controllers? Please answer to this in commit msg, so > there will be no questions/concerns. You have entire commit msg to explain > all weird and unexpected things with this binding. This is correct. gpio-controller should be defined in the parent node. Otherwise gpio probe will fail. > > ... > > >>> + #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? > > Why disabling? Drop all statuses from all your binding examples. > > >> Where are gpio-controller and cells? For this node and for parent? > >> Why this example is incomplete? > > > > Currently I don't see any driver is using this compatible other than > MFD. > > Open the MFD so you will see it... Actually, found the driver and tested GPIOs, For input gpio, I can see the sd1_pwr_sel values are toggled during card insert/removal. For outout gpio, System is entering into reset mode, if I set output-low in DT. So set Init state as output-high to avoid reset. drivers/pinctrl/pinctrl-da9062.c Cheers, Biju
On 09/12/2023 16:41, Biju Das wrote: >> >> Why disabling? Drop all statuses from all your binding examples. >> >>>> Where are gpio-controller and cells? For this node and for parent? >>>> Why this example is incomplete? >>> >>> Currently I don't see any driver is using this compatible other than >> MFD. >> >> Open the MFD so you will see it... > > Actually, found the driver and tested GPIOs, > For input gpio, I can see the sd1_pwr_sel values are > toggled during card insert/removal. > For outout gpio, > System is entering into reset mode, if I set output-low in DT. So set > Init state as output-high to avoid reset. > > drivers/pinctrl/pinctrl-da9062.c Anyway there is a GPIO child node and driver which binds to it. What's its purpose in such case? Best regards, Krzysztof
Hi Krzysztof Kozlowski, > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Saturday, December 9, 2023 4:08 PM > Subject: Re: [PATCH v4 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 > to json-schema > > On 09/12/2023 16:41, Biju Das wrote: > >> > >> Why disabling? Drop all statuses from all your binding examples. > >> > >>>> Where are gpio-controller and cells? For this node and for parent? > >>>> Why this example is incomplete? > >>> > >>> Currently I don't see any driver is using this compatible other than > >> MFD. > >> > >> Open the MFD so you will see it... > > > > Actually, found the driver and tested GPIOs, For input gpio, I can see > > the sd1_pwr_sel values are toggled during card insert/removal. > > For outout gpio, > > System is entering into reset mode, if I set output-low in DT. So set > > Init state as output-high to avoid reset. > > > > drivers/pinctrl/pinctrl-da9062.c > > Anyway there is a GPIO child node and driver which binds to it. What's its > purpose in such case? The pinctrl-da9062driver is instantiated from the parent, since there is a GPIO child node with matching compatible. MFD_CELL_OF("da9062-gpio", da9062_gpio_resources, NULL, 0, 0, "dlg,da9062-gpio"), root@smarc-rzg2ul:~# cat /sys/kernel/debug/gpio | grep da9062 -A5 gpiochip1: GPIOs 664-668, parent: platform/da9062-gpio, da9062-gpio, can sleep: gpio-665 ( |SD0_PWR_SEL ) in lo gpio-666 ( |SD1_PWR_SEL ) in lo gpio-667 ( |SW_ET0_EN# ) in hi gpio-668 ( |PMIC_PGOOD ) out hi root@smarc-rzg2ul:~#
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: -- https://www.dialog-semiconductor.com/products/da9062 -- https://www.dialog-semiconductor.com/products/da9061 - -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: http://devicetree.org/schemas/mfd/dlg,da9063.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -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@diasemi.com> @@ -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: + - https://www.dialog-semiconductor.com/products/da9063l + - https://www.dialog-semiconductor.com/products/da9063 + - https://www.dialog-semiconductor.com/products/da9062 + - https://www.dialog-semiconductor.com/products/da9061 properties: compatible: enum: + - dlg,da9061 + - dlg,da9062 - dlg,da9063 - dlg,da9063l @@ -35,6 +42,19 @@ properties: "#interrupt-cells": const: 2 + gpio: + type: object + $ref: /schemas/gpio/gpio.yaml# + unevaluatedProperties: false + properties: + compatible: + const: dlg,da9062-gpio + + gpio-controller: true + + "#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"; + }; + + onkey { + compatible = "dlg,da9062-onkey"; + }; + + regulators { + buck1 { + regulator-name = "vdd_arm"; + regulator-min-microvolt = <925000>; + regulator-max-microvolt = <1380000>; + regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>; + regulator-always-on; + }; + buck2 { + regulator-name = "vdd_soc"; + regulator-min-microvolt = <1150000>; + regulator-max-microvolt = <1380000>; + regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>; + regulator-always-on; + }; + buck3 { + regulator-name = "vdd_ddr3"; + regulator-min-microvolt = <1500000>; + regulator-max-microvolt = <1500000>; + regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>; + regulator-always-on; + }; + buck4 { + regulator-name = "vdd_eth"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>; + regulator-always-on; + }; + ldo1 { + regulator-name = "vdd_snvs"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + }; + ldo2 { + regulator-name = "vdd_high"; + regulator-min-microvolt = <3000000>; + regulator-max-microvolt = <3000000>; + regulator-always-on; + }; + ldo3 { + regulator-name = "vdd_eth_io"; + regulator-min-microvolt = <2500000>; + regulator-max-microvolt = <2500000>; + }; + ldo4 { + regulator-name = "vdd_emmc"; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-always-on; + }; + }; + + rtc { + compatible = "dlg,da9062-rtc"; + status = "disabled"; + }; + + thermal { + compatible = "dlg,da9062-thermal"; + status = "disabled"; + }; + + watchdog { + compatible = "dlg,da9062-watchdog"; + dlg,use-sw-pm; + }; + }; + }; ... diff --git a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml index 206635f74850..e8b2cac41084 100644 --- a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml +++ b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml @@ -11,7 +11,7 @@ maintainers: description: | This module is part of the DA9061/DA9062. For more details about entire - DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt + DA906{1,2} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml Junction temperature thermal module uses an interrupt signal to identify high THERMAL_TRIP_HOT temperatures for the PMIC device.
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@bp.renesas.com> --- 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