| Message ID | 20240323085852.116756-2-knaerzche@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add RK816 PMIC support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 23/03/2024 09:58, Alex Bee wrote: > Add DT binding document for Rockchip's RK816 PMIC > > Signed-off-by: Alex Bee <knaerzche@gmail.com> > --- > changes since v1: > - lowercase/hyphens for regulator node names > - rename "-reg" to "-regulator" to make node names generic I don't understand why did you do it. I did not ask for it. If you want to rename, drop redundant regulator or reg suffix from node names. > - dropped superfluous description for clock-output-names and > wakeup-source > - dropped "|" for text blocks that don't require to preserve formatting > - use full path for `$ref`s > - added pins description to the binding > - added charger function to description > > .../bindings/mfd/rockchip,rk816.yaml | 269 ++++++++++++++++++ > 1 file changed, 269 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml > new file mode 100644 > index 000000000000..9664162f4f75 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml > @@ -0,0 +1,269 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/rockchip,rk816.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: RK816 Power Management Integrated Circuit > + > +maintainers: > + - Chris Zhong <zyw@rock-chips.com> > + - Zhang Qing <zhangqing@rock-chips.com> > + > +description: > + Rockchip RK816 series PMIC. This device consists of an i2c controlled MFD > + that includes regulators, a RTC, a GPIO controller, a power button, and a > + battery charger manager with fuel gauge. > + > +properties: > + compatible: > + enum: > + - rockchip,rk816 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + '#clock-cells': > + description: > + See <dt-bindings/clock/rockchip,rk808.h> for clock IDs. > + const: 1 > + > + clock-output-names: > + maxItems: 2 > + > + gpio-controller: true > + > + '#gpio-cells': > + const: 2 > + > + system-power-controller: > + type: boolean > + description: > + Telling whether or not this PMIC is controlling the system power. > + > + wakeup-source: > + type: boolean > + > + vcc1-supply: > + description: > + The input supply for dcdc1-regulator. > + > + vcc2-supply: > + description: > + The input supply for dcdc2-regulator. > + > + vcc3-supply: > + description: > + The input supply for dcdc3-regulator. > + > + vcc4-supply: > + description: > + The input supply for dcdc4-regulator. > + > + vcc5-supply: > + description: > + The input supply for ldo1-regulator, ldo2-regulator, and ldo3-regulator. > + > + vcc6-supply: > + description: > + The input supply for ldo4-regulator, ldo5-regulator, and ldo6-regulator. > + > + vcc7-supply: > + description: > + The input supply for boost. > + > + vcc8-supply: > + description: > + The input supply for otg-switch. > + > + regulators: > + type: object > + patternProperties: > + "^(boost|dcdc[1-4]-regulator|ldo[1-6]-regulator|otg-switch)$": > + type: object > + $ref: /schemas/regulator/regulator.yaml# > + unevaluatedProperties: false > + unevaluatedProperties: false > + > +patternProperties: > + '-pins$': Keep consistent quotes, so either ' or ". > + type: object > + additionalProperties: false > + $ref: /schemas/pinctrl/pinmux-node.yaml > + > + properties: > + function: > + enum: [pin_fun_gpio, pin_fun_thermistor] gpio, thermistor (pin_fun is redundant) > + > + pins: > + $ref: /schemas/types.yaml#/definitions/string > + const: gpio0 > + > +required: > + - compatible > + - reg > + - interrupts > + - "#clock-cells" > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/pinctrl/rockchip.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/gpio/gpio.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + rk816: pmic@1a { > + compatible = "rockchip,rk816"; > + reg = <0x1a>; > + interrupt-parent = <&gpio0>; > + interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>; > + clock-output-names = "xin32k", "rk816-clkout2"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pmic_int_l>; > + gpio-controller; > + system-power-controller; > + wakeup-source; > + #clock-cells = <1>; > + #gpio-cells = <2>; > + > + vcc1-supply = <&vcc_sys>; > + vcc2-supply = <&vcc_sys>; > + vcc3-supply = <&vcc_sys>; > + vcc4-supply = <&vcc_sys>; > + vcc5-supply = <&vcc33_io>; > + vcc6-supply = <&vcc_sys>; Add pins node to the example. Best regards, Krzysztof
Hi Krzysztof, Am 23.03.24 um 11:32 schrieb Krzysztof Kozlowski: > On 23/03/2024 09:58, Alex Bee wrote: >> Add DT binding document for Rockchip's RK816 PMIC >> >> Signed-off-by: Alex Bee <knaerzche@gmail.com> >> --- >> changes since v1: >> - lowercase/hyphens for regulator node names >> - rename "-reg" to "-regulator" to make node names generic > I don't understand why did you do it. I did not ask for it. If you want > to rename, drop redundant regulator or reg suffix from node names. You didn't ask for that, thats true. I did it regardless, since node names should be generic and the "-reg" suffix is not. They are all subnodes of "regulators", indeed, but I don't think dropping the suffix completely makes the binding any better, since there is a boost(-switch) and an otg-switch which are also subnodes of "regulators" and they are just switches. Alex > >> - dropped superfluous description for clock-output-names and >> wakeup-source >> - dropped "|" for text blocks that don't require to preserve formatting >> - use full path for `$ref`s >> - added pins description to the binding >> - added charger function to description >> >> .../bindings/mfd/rockchip,rk816.yaml | 269 ++++++++++++++++++ >> 1 file changed, 269 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml >> >> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml >> new file mode 100644 >> index 000000000000..9664162f4f75 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml >> @@ -0,0 +1,269 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mfd/rockchip,rk816.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: RK816 Power Management Integrated Circuit >> + >> +maintainers: >> + - Chris Zhong <zyw@rock-chips.com> >> + - Zhang Qing <zhangqing@rock-chips.com> >> + >> +description: >> + Rockchip RK816 series PMIC. This device consists of an i2c controlled MFD >> + that includes regulators, a RTC, a GPIO controller, a power button, and a >> + battery charger manager with fuel gauge. >> + >> +properties: >> + compatible: >> + enum: >> + - rockchip,rk816 >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + '#clock-cells': >> + description: >> + See <dt-bindings/clock/rockchip,rk808.h> for clock IDs. >> + const: 1 >> + >> + clock-output-names: >> + maxItems: 2 >> + >> + gpio-controller: true >> + >> + '#gpio-cells': >> + const: 2 >> + >> + system-power-controller: >> + type: boolean >> + description: >> + Telling whether or not this PMIC is controlling the system power. >> + >> + wakeup-source: >> + type: boolean >> + >> + vcc1-supply: >> + description: >> + The input supply for dcdc1-regulator. >> + >> + vcc2-supply: >> + description: >> + The input supply for dcdc2-regulator. >> + >> + vcc3-supply: >> + description: >> + The input supply for dcdc3-regulator. >> + >> + vcc4-supply: >> + description: >> + The input supply for dcdc4-regulator. >> + >> + vcc5-supply: >> + description: >> + The input supply for ldo1-regulator, ldo2-regulator, and ldo3-regulator. >> + >> + vcc6-supply: >> + description: >> + The input supply for ldo4-regulator, ldo5-regulator, and ldo6-regulator. >> + >> + vcc7-supply: >> + description: >> + The input supply for boost. >> + >> + vcc8-supply: >> + description: >> + The input supply for otg-switch. >> + >> + regulators: >> + type: object >> + patternProperties: >> + "^(boost|dcdc[1-4]-regulator|ldo[1-6]-regulator|otg-switch)$": >> + type: object >> + $ref: /schemas/regulator/regulator.yaml# >> + unevaluatedProperties: false >> + unevaluatedProperties: false >> + >> +patternProperties: >> + '-pins$': > Keep consistent quotes, so either ' or ". > >> + type: object >> + additionalProperties: false >> + $ref: /schemas/pinctrl/pinmux-node.yaml >> + >> + properties: >> + function: >> + enum: [pin_fun_gpio, pin_fun_thermistor] > gpio, thermistor > (pin_fun is redundant) > >> + >> + pins: >> + $ref: /schemas/types.yaml#/definitions/string >> + const: gpio0 >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - "#clock-cells" >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/pinctrl/rockchip.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + #include <dt-bindings/gpio/gpio.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + rk816: pmic@1a { >> + compatible = "rockchip,rk816"; >> + reg = <0x1a>; >> + interrupt-parent = <&gpio0>; >> + interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>; >> + clock-output-names = "xin32k", "rk816-clkout2"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pmic_int_l>; >> + gpio-controller; >> + system-power-controller; >> + wakeup-source; >> + #clock-cells = <1>; >> + #gpio-cells = <2>; >> + >> + vcc1-supply = <&vcc_sys>; >> + vcc2-supply = <&vcc_sys>; >> + vcc3-supply = <&vcc_sys>; >> + vcc4-supply = <&vcc_sys>; >> + vcc5-supply = <&vcc33_io>; >> + vcc6-supply = <&vcc_sys>; > Add pins node to the example. > > Best regards, > Krzysztof >
On 23/03/2024 11:56, Alex Bee wrote: > Hi Krzysztof, > > Am 23.03.24 um 11:32 schrieb Krzysztof Kozlowski: >> On 23/03/2024 09:58, Alex Bee wrote: >>> Add DT binding document for Rockchip's RK816 PMIC >>> >>> Signed-off-by: Alex Bee <knaerzche@gmail.com> >>> --- >>> changes since v1: >>> - lowercase/hyphens for regulator node names >>> - rename "-reg" to "-regulator" to make node names generic >> I don't understand why did you do it. I did not ask for it. If you want >> to rename, drop redundant regulator or reg suffix from node names. > You didn't ask for that, thats true. > > I did it regardless, since node names should be generic and the "-reg" device node names, here you do not have devices. > suffix is not. They are all subnodes of "regulators", indeed, but I don't > think dropping the suffix completely makes the binding any better, since > there is a boost(-switch) and an otg-switch which are also subnodes of > "regulators" and they are just switches. Adding "regulator" suffix does not make them more generic... anyway, that's not expected pattern. First, do you see such code anywhere? Second, the regulator node names should be usually named as the name of physical component. That's the most common pattern. Please do not bring some exceptions from coding style just for your device. Your device is not special. You did not respond to rest of my comments, so I assume you agree 100% with them. Also, please trim the replies from unneeded context. Best regards, Krzysztof
Am 23.03.24 um 12:02 schrieb Krzysztof Kozlowski: > On 23/03/2024 11:56, Alex Bee wrote: >> Hi Krzysztof, >> >> Am 23.03.24 um 11:32 schrieb Krzysztof Kozlowski: >>> On 23/03/2024 09:58, Alex Bee wrote: >>>> Add DT binding document for Rockchip's RK816 PMIC >>>> >>>> Signed-off-by: Alex Bee <knaerzche@gmail.com> >>>> --- >>>> changes since v1: >>>> - lowercase/hyphens for regulator node names >>>> - rename "-reg" to "-regulator" to make node names generic >>> I don't understand why did you do it. I did not ask for it. If you want >>> to rename, drop redundant regulator or reg suffix from node names. >> You didn't ask for that, thats true. >> >> I did it regardless, since node names should be generic and the "-reg" > device node names, here you do not have devices. OK. I wasn't aware there's a difference between device nodes and devices. I'll drop the suffix from the regulators. > You did not respond to rest of my comments, so I assume you agree 100% > with them. I do. Alex. >
diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml new file mode 100644 index 000000000000..9664162f4f75 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml @@ -0,0 +1,269 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/rockchip,rk816.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: RK816 Power Management Integrated Circuit + +maintainers: + - Chris Zhong <zyw@rock-chips.com> + - Zhang Qing <zhangqing@rock-chips.com> + +description: + Rockchip RK816 series PMIC. This device consists of an i2c controlled MFD + that includes regulators, a RTC, a GPIO controller, a power button, and a + battery charger manager with fuel gauge. + +properties: + compatible: + enum: + - rockchip,rk816 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + '#clock-cells': + description: + See <dt-bindings/clock/rockchip,rk808.h> for clock IDs. + const: 1 + + clock-output-names: + maxItems: 2 + + gpio-controller: true + + '#gpio-cells': + const: 2 + + system-power-controller: + type: boolean + description: + Telling whether or not this PMIC is controlling the system power. + + wakeup-source: + type: boolean + + vcc1-supply: + description: + The input supply for dcdc1-regulator. + + vcc2-supply: + description: + The input supply for dcdc2-regulator. + + vcc3-supply: + description: + The input supply for dcdc3-regulator. + + vcc4-supply: + description: + The input supply for dcdc4-regulator. + + vcc5-supply: + description: + The input supply for ldo1-regulator, ldo2-regulator, and ldo3-regulator. + + vcc6-supply: + description: + The input supply for ldo4-regulator, ldo5-regulator, and ldo6-regulator. + + vcc7-supply: + description: + The input supply for boost. + + vcc8-supply: + description: + The input supply for otg-switch. + + regulators: + type: object + patternProperties: + "^(boost|dcdc[1-4]-regulator|ldo[1-6]-regulator|otg-switch)$": + type: object + $ref: /schemas/regulator/regulator.yaml# + unevaluatedProperties: false + unevaluatedProperties: false + +patternProperties: + '-pins$': + type: object + additionalProperties: false + $ref: /schemas/pinctrl/pinmux-node.yaml + + properties: + function: + enum: [pin_fun_gpio, pin_fun_thermistor] + + pins: + $ref: /schemas/types.yaml#/definitions/string + const: gpio0 + +required: + - compatible + - reg + - interrupts + - "#clock-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/pinctrl/rockchip.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + rk816: pmic@1a { + compatible = "rockchip,rk816"; + reg = <0x1a>; + interrupt-parent = <&gpio0>; + interrupts = <RK_PA2 IRQ_TYPE_LEVEL_LOW>; + clock-output-names = "xin32k", "rk816-clkout2"; + pinctrl-names = "default"; + pinctrl-0 = <&pmic_int_l>; + gpio-controller; + system-power-controller; + wakeup-source; + #clock-cells = <1>; + #gpio-cells = <2>; + + vcc1-supply = <&vcc_sys>; + vcc2-supply = <&vcc_sys>; + vcc3-supply = <&vcc_sys>; + vcc4-supply = <&vcc_sys>; + vcc5-supply = <&vcc33_io>; + vcc6-supply = <&vcc_sys>; + + regulators { + vdd_cpu: dcdc1-regulator { + regulator-name = "vdd_cpu"; + regulator-min-microvolt = <750000>; + regulator-max-microvolt = <1450000>; + regulator-ramp-delay = <6001>; + regulator-initial-mode = <1>; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + vdd_logic: dcdc2-regulator { + regulator-name = "vdd_logic"; + regulator-min-microvolt = <800000>; + regulator-max-microvolt = <1250000>; + regulator-ramp-delay = <6001>; + regulator-initial-mode = <1>; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <1000000>; + }; + }; + + vcc_ddr: dcdc3-regulator { + regulator-name = "vcc_ddr"; + regulator-initial-mode = <1>; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-on-in-suspend; + }; + }; + + vcc33_io: dcdc4-regulator { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc33_io"; + regulator-initial-mode = <1>; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <3300000>; + }; + }; + + vccio_pmu: ldo1-regulator { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vccio_pmu"; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <3300000>; + }; + }; + + vcc_tp: ldo2-regulator { + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vcc_tp"; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + + vdd_10: ldo3-regulator { + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-name = "vdd_10"; + regulator-always-on; + regulator-boot-on; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <1000000>; + }; + }; + + vcc18_lcd: ldo4-regulator { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + regulator-name = "vcc18_lcd"; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <1800000>; + }; + }; + + vccio_sd: ldo5-regulator { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + regulator-name = "vccio_sd"; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <3300000>; + }; + }; + + vdd10_lcd: ldo6-regulator { + regulator-min-microvolt = <1000000>; + regulator-max-microvolt = <1000000>; + regulator-name = "vdd10_lcd"; + + regulator-state-mem { + regulator-on-in-suspend; + regulator-suspend-microvolt = <1000000>; + }; + }; + }; + }; + };
Add DT binding document for Rockchip's RK816 PMIC Signed-off-by: Alex Bee <knaerzche@gmail.com> --- changes since v1: - lowercase/hyphens for regulator node names - rename "-reg" to "-regulator" to make node names generic - dropped superfluous description for clock-output-names and wakeup-source - dropped "|" for text blocks that don't require to preserve formatting - use full path for `$ref`s - added pins description to the binding - added charger function to description .../bindings/mfd/rockchip,rk816.yaml | 269 ++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/rockchip,rk816.yaml