On 2/19/2022 7:09 AM, Stephen Boyd wrote:
Quoting Satya Priya (2022-02-18 03:00:59)
Add regulators and their supply nodes. Add separate compatible
"qcom,pm8008-regulators" to differentiate between pm8008 infra
and pm8008 regulators mfd devices.
Signed-off-by: Satya Priya <quic_c_skakit@xxxxxxxxxxx>
---
Is the register layout compatible with SPMI regulators? The gpio node
seems to be fully compatible and the same driver probes there for SPMI
and i2c, so I wonder why we can't extend the existing SPMI gpio and
regulator bindings to have the new compatible strings for pm8008. Is
anything really different, or do we have the same device talking i2c
instead of SPMI now? Possibly it's exposing the different hardware
blocks inside the PMIC at different i2c addresses. It looks like the i2c
address is 0x8 and then there's 16-bits of address space inside the i2c
device to do things. 0x9 is the i2c address for the regulators and then
each ldo is at some offset in there?
The register layout is not compatible with spmi regulators, I see some
differences w.r.t VOLTAGE_CTL, EN_CTL, MODE_CTL registers. Also, there
is no headroom related stuff in the spmi driver.
Changes in V2:
- As per Rob's comments changed "pm8008[a-z]?-regulator" to
"^pm8008[a-z]?-regulators".
Changes in V3:
- Fixed bot errors.
- As per stephen's comments, changed "^pm8008[a-z]?-regulators$" to
"regulators".
Changes in V4:
- Changed compatible string to "qcom,pm8008-regulators"
Changes in V5:
- Remove compatible for regulators node.
- Move supply nodes of the regulators to chip level.
Changes in V6:
- No changes.
Changes in V7:
- Removed the intermediate regulators node and added ldos
directly under mfd node.
.../devicetree/bindings/mfd/qcom,pm8008.yaml | 50 +++++++++++++++++++---
1 file changed, 43 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index ec3138c..6b3b53e 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -16,7 +16,9 @@ description: |
properties:
compatible:
- const: qcom,pm8008
+ enum:
+ - qcom,pm8008
+ - qcom,pm8008-regulators
reg:
description:
@@ -44,6 +46,21 @@ properties:
"#size-cells":
const: 0
+ vdd_l1_l2-supply:
+ description: Input supply phandle of ldo1 and ldo2 regulators.
+
+ vdd_l3_l4-supply:
+ description: Input supply phandle of ldo3 and ldo4 regulators.
+
+ vdd_l5-supply:
+ description: Input supply phandle of ldo5 regulator.
+
+ vdd_l6-supply:
+ description: Input supply phandle of ldo6 regulator.
+
+ vdd_l7-supply:
+ description: Input supply phandle of ldo7 regulator.
+
patternProperties:
"^gpio@[0-9a-f]+$":
type: object
@@ -85,13 +102,16 @@ patternProperties:
additionalProperties: false
+ "^ldo[1-7]$":
+ type: object
+ $ref: "../regulator/regulator.yaml#"
+ description: PM8008 regulator peripherals of PM8008 regulator device
+
required:
- compatible
- reg
- - interrupts
- "#address-cells"
- "#size-cells"
- - "#interrupt-cells"
additionalProperties: false
@@ -102,13 +122,11 @@ examples:
qupv3_se13_i2c {
#address-cells = <1>;
#size-cells = <0>;
- pm8008i@8 {
+ pm8008_infra: pm8008@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>;
I still fail to see what this part of the diff has to do with
regulators. Can it be split off to a different patch with a clear
description of why interrupt-controller and #interrupt-cells is no
longer required for qcom,pm8008?
This diff has nothing to do with regulators, I removed it to avoid yaml
errors during dtbs check.
I'll move this to a separate patch.
It really looks like we're combining the binding for qcom,pm8008 and
qcom,pm8008-regulators at the same level, which looks wrong. We don't
want to describe the least common denominator between the two bindings.
Why not make two different bindings and files? One for the interrupty
gpio/interrupt controller device (at 0x8) and one for the regulator one
(at 0x9)?
Okay, I'll add a different binding for regulators
(mfd/qcom,pm8008-regulators.yaml), leave this binding as it is.. and
also add separate DT files for pm8008-infra and pm8008-regulators.
@@ -123,6 +141,24 @@ examples:
#interrupt-cells = <2>;
};
};
- };
+ pm8008_regulators: pm8008@9 {
pmic@9, or regulators@9? The node name should be generic.
+ compatible = "qcom,pm8008-regulators";
+ reg = <0x9>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ 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>;
+
+ pm8008_l1: ldo1 {
+ regulator-name = "pm8008_l1";
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <1300000>;
+ };
+ };
For some i2c devices that appear on multiple i2c addresses we make an
i2c client for each address in the driver that attaches to the node we
put in DT. I suppose that won't work easily here. Either way, it would
make it much clearer if this existing binding was left alone. Is there
other functionality inside the i2c address 0x9 register space that isn't
regulators?
As mentioned above, I'll make a separate binding for regulators. There
is no other functionality apart from regulators in the i2c 0x9 register
space.
