| Message ID | 20240506150830.23709-11-johan+linaro@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sc8280xp-x13s: enable pm8008 camera pmic | expand |
On 06/05/2024 17:08, Johan Hovold wrote: > Rework the pm8008 binding by dropping internal details like register > offsets and interrupts and by adding the missing regulator and > temperature alarm properties. > > Note that child nodes are still used for pinctrl and regulator > configuration. > > Also note that the pinctrl state definition will be extended later and > could eventually also be shared with other PMICs (e.g. by breaking out > bits of qcom,pmic-gpio.yaml). > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > .../devicetree/bindings/mfd/qcom,pm8008.yaml | 154 ++++++++++-------- > 1 file changed, 90 insertions(+), 64 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > index e1e05921afb4..ac1bab0261b6 100644 > --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > @@ -19,116 +19,142 @@ properties: > const: qcom,pm8008 > > reg: > - description: > - I2C slave address. Please split cleanups from actual functional/content rework. > - > maxItems: 1 > > interrupts: > maxItems: 1 > > - description: Parent interrupt. > - > reset-gpios: > maxItems: 1 > > - "#interrupt-cells": > + vdd_l1_l2-supply: true No underscores in property names. Best regards, Krzysztof
On Tue, May 07, 2024 at 08:43:08AM +0200, Krzysztof Kozlowski wrote: > On 06/05/2024 17:08, Johan Hovold wrote: > > Rework the pm8008 binding by dropping internal details like register > > offsets and interrupts and by adding the missing regulator and > > temperature alarm properties. > > > > Note that child nodes are still used for pinctrl and regulator > > configuration. > > > > Also note that the pinctrl state definition will be extended later and > > could eventually also be shared with other PMICs (e.g. by breaking out > > bits of qcom,pmic-gpio.yaml). > > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > reg: > > - description: > > - I2C slave address. > > Please split cleanups from actual functional/content rework. Sure. > > - > > maxItems: 1 > > > > interrupts: > > maxItems: 1 > > > > - description: Parent interrupt. > > - > > reset-gpios: > > maxItems: 1 > > > > - "#interrupt-cells": > > + vdd_l1_l2-supply: true > > No underscores in property names. Indeed. These names come from Qualcomm's v15, but I should have caught that. Thanks. Johan
Quoting Johan Hovold (2024-05-07 08:23:04) > On Tue, May 07, 2024 at 08:43:08AM +0200, Krzysztof Kozlowski wrote: > > > - > > > maxItems: 1 > > > > > > interrupts: > > > maxItems: 1 > > > > > > - description: Parent interrupt. > > > - > > > reset-gpios: > > > maxItems: 1 > > > > > > - "#interrupt-cells": > > > + vdd_l1_l2-supply: true > > > > No underscores in property names. > > Indeed. These names come from Qualcomm's v15, but I should have caught > that. Thanks. Drive by comment: we have underscores to match the label on the datasheet. Not sure that will sway your opinion. Only trying to provide some background rationale.
On 09/05/2024 00:09, Stephen Boyd wrote: > Quoting Johan Hovold (2024-05-07 08:23:04) >> On Tue, May 07, 2024 at 08:43:08AM +0200, Krzysztof Kozlowski wrote: >>>> - >>>> maxItems: 1 >>>> >>>> interrupts: >>>> maxItems: 1 >>>> >>>> - description: Parent interrupt. >>>> - >>>> reset-gpios: >>>> maxItems: 1 >>>> >>>> - "#interrupt-cells": >>>> + vdd_l1_l2-supply: true >>> >>> No underscores in property names. >> >> Indeed. These names come from Qualcomm's v15, but I should have caught >> that. Thanks. > > Drive by comment: we have underscores to match the label on the > datasheet. Not sure that will sway your opinion. Only trying to provide > some background rationale. I know, but if datasheet calls this "yellow_pony_!!!#1l33t-supply" we still won't use datasheet names directly, so s/_/-/. That's also W=2 warning. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml index e1e05921afb4..ac1bab0261b6 100644 --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml @@ -19,116 +19,142 @@ properties: const: qcom,pm8008 reg: - description: - I2C slave address. - maxItems: 1 interrupts: maxItems: 1 - description: Parent interrupt. - reset-gpios: maxItems: 1 - "#interrupt-cells": + vdd_l1_l2-supply: true + vdd_l3_l4-supply: true + vdd_l5-supply: true + vdd_l6-supply: true + vdd_l7-supply: true + + gpio-controller: true + + "#gpio-cells": const: 2 - description: | - The first cell is the IRQ number, the second cell is the IRQ trigger - flag. All interrupts are listed in include/dt-bindings/mfd/qcom-pm8008.h. + gpio-ranges: + maxItems: 1 interrupt-controller: true - "#address-cells": - const: 1 + "#interrupt-cells": + const: 2 - "#size-cells": + "#thermal-sensor-cells": const: 0 -patternProperties: - "^gpio@[0-9a-f]+$": + pinctrl: type: object + additionalProperties: false + patternProperties: + "-state$": + type: object + $ref: "#/$defs/qcom-pm8008-pinctrl-state" + unevaluatedProperties: false - description: | - The GPIO peripheral. This node may be specified twice, one for each GPIO. - - properties: - compatible: - items: - - const: qcom,pm8008-gpio - - const: qcom,spmi-gpio + regulators: + type: object + additionalProperties: false + patternProperties: + "^ldo[1-7]$": + type: object + $ref: /schemas/regulator/regulator.yaml# + unevaluatedProperties: false - reg: - description: Peripheral address of one of the two GPIO peripherals. - maxItems: 1 +required: + - compatible + - reg + - interrupts + - vdd_l1_l2-supply + - vdd_l3_l4-supply + - vdd_l5-supply + - vdd_l6-supply + - vdd_l7-supply + - gpio-controller + - "#gpio-cells" + - gpio-ranges + - interrupt-controller + - "#interrupt-cells" + - "#thermal-sensor-cells" - gpio-controller: true +additionalProperties: false - gpio-ranges: - maxItems: 1 +$defs: + qcom-pm8008-pinctrl-state: + type: object - interrupt-controller: true + allOf: + - $ref: /schemas/pinctrl/pinmux-node.yaml + - $ref: /schemas/pinctrl/pincfg-node.yaml - "#interrupt-cells": - const: 2 + properties: + pins: + items: + pattern: "^gpio[12]$" - "#gpio-cells": - const: 2 + function: + items: + - enum: + - normal required: - - compatible - - reg - - gpio-controller - - interrupt-controller - - "#gpio-cells" - - gpio-ranges - - "#interrupt-cells" + - pins + - function additionalProperties: false -required: - - compatible - - reg - - interrupts - - "#address-cells" - - "#size-cells" - - "#interrupt-cells" - -additionalProperties: false - examples: - | #include <dt-bindings/gpio/gpio.h> - #include <dt-bindings/mfd/qcom-pm8008.h> #include <dt-bindings/interrupt-controller/irq.h> i2c { #address-cells = <1>; #size-cells = <0>; - pmic@8 { + pm8008: pmic@8 { compatible = "qcom,pm8008"; reg = <0x8>; - #address-cells = <1>; - #size-cells = <0>; - interrupt-controller; - #interrupt-cells = <2>; interrupt-parent = <&tlmm>; interrupts = <32 IRQ_TYPE_EDGE_RISING>; reset-gpios = <&tlmm 42 GPIO_ACTIVE_LOW>; - pm8008_gpios: gpio@c000 { - compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio"; - reg = <0xc000>; - gpio-controller; - gpio-ranges = <&pm8008_gpios 0 0 2>; - #gpio-cells = <2>; - interrupt-controller; - #interrupt-cells = <2>; + vdd_l1_l2-supply = <&vreg_s8b_1p2>; + vdd_l3_l4-supply = <&vreg_s1b_1p8>; + vdd_l5-supply = <&vreg_bob>; + vdd_l6-supply = <&vreg_bob>; + vdd_l7-supply = <&vreg_bob>; + + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&pm8008 0 0 2>; + + interrupt-controller; + #interrupt-cells = <2>; + + #thermal-sensor-cells = <0>; + + pinctrl { + gpio-keys-state { + pins = "gpio1"; + function = "normal"; + }; + }; + + regulators { + ldo1 { + regulator-name = "vreg_l1"; + regulator-min-microvolt = <950000>; + regulator-max-microvolt = <1300000>; + }; }; }; };
Rework the pm8008 binding by dropping internal details like register offsets and interrupts and by adding the missing regulator and temperature alarm properties. Note that child nodes are still used for pinctrl and regulator configuration. Also note that the pinctrl state definition will be extended later and could eventually also be shared with other PMICs (e.g. by breaking out bits of qcom,pmic-gpio.yaml). Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- .../devicetree/bindings/mfd/qcom,pm8008.yaml | 154 ++++++++++-------- 1 file changed, 90 insertions(+), 64 deletions(-)