| Message ID | 1648209491-30165-2-git-send-email-quic_c_skakit@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Qualcomm Technologies, Inc. PM8008 regulator driver | expand |
Quoting Satya Priya (2022-03-25 04:58:05) > Modify the compatible string as per the pm8008 mfd driver. > Add reset-gpios property and make interrupts and interrupt-cells > as optional properties, they are not strictly required and may > cause yaml compilation errors when not added in the DT files. Does it have an interrupt controller inside? If so, the properties should be present even if the driver isn't using them. > > Also, change the node name in example to match with the > pm8008_infra DT node. > > Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> > --- > 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. > > Changes in V8: > - Change the compatible as per driver, remove interrupts from required > properties, add reset-gpios and move regulators to separate binding. > > Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > index ec3138c..12431ea 100644 > --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > @@ -16,7 +16,7 @@ description: | > > properties: > compatible: > - const: qcom,pm8008 > + const: qcom,pm8008-infra Why is the compatible being replaced with -infra postfix? > > reg: > description: > @@ -44,6 +44,10 @@ properties: > "#size-cells": > const: 0 > > + reset-gpios: > + description: | > + Specifies the GPIO to be toggled to bring pm8008 chip out of reset. Remove description. Add maxItems: 1 > + > patternProperties: > "^gpio@[0-9a-f]+$": > type: object > @@ -88,10 +92,8 @@ patternProperties: > required: > - compatible > - reg > - - interrupts > - "#address-cells" > - "#size-cells" > - - "#interrupt-cells" Should reset-gpios be required? I'd expect this patch to be adding reset-gpios and making it required and that's about it. Given that there isn't a DT using this compatible upstream so far it looks like we don't need to do anything like worry about backwards compatibility. > > additionalProperties: false > > @@ -99,11 +101,12 @@ examples: > - | > #include <dt-bindings/mfd/qcom-pm8008.h> > #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/gpio/gpio.h> > qupv3_se13_i2c { > #address-cells = <1>; > #size-cells = <0>; > - pm8008i@8 { > - compatible = "qcom,pm8008"; > + pm8008_infra: pmic@8 { Remove unused phandle. > + compatible = "qcom,pm8008-infra"; > reg = <0x8>; > #address-cells = <1>; > #size-cells = <0>; > @@ -113,6 +116,8 @@ examples: > interrupt-parent = <&tlmm>; > interrupts = <32 IRQ_TYPE_EDGE_RISING>; > > + reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>; > + > pm8008_gpios: gpio@c000 { > compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio"; > reg = <0xc000>; > -- > 2.7.4 >
On 3/25/2022 11:32 PM, Stephen Boyd wrote: > Quoting Satya Priya (2022-03-25 04:58:05) >> Modify the compatible string as per the pm8008 mfd driver. >> Add reset-gpios property and make interrupts and interrupt-cells >> as optional properties, they are not strictly required and may >> cause yaml compilation errors when not added in the DT files. > Does it have an interrupt controller inside? If so, the properties > should be present even if the driver isn't using them. Yes it has an interrupt controller but we are not configuring/using any interrupts currently in the DT node. So, if we add the properties under "required" list it throws a make dtbs error. Do you think we can somehow bypass this error? >> Also, change the node name in example to match with the >> pm8008_infra DT node. >> >> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> >> --- >> 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. >> >> Changes in V8: >> - Change the compatible as per driver, remove interrupts from required >> properties, add reset-gpios and move regulators to separate binding. >> >> Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >> index ec3138c..12431ea 100644 >> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >> @@ -16,7 +16,7 @@ description: | >> >> properties: >> compatible: >> - const: qcom,pm8008 >> + const: qcom,pm8008-infra > Why is the compatible being replaced with -infra postfix? I've changed the compatible after splitting the probe into two parts pm8008-infra-probe and pm8008-regulators-probe. Only reason for change is to make it more readable/understandable. >> reg: >> description: >> @@ -44,6 +44,10 @@ properties: >> "#size-cells": >> const: 0 >> >> + reset-gpios: >> + description: | >> + Specifies the GPIO to be toggled to bring pm8008 chip out of reset. > Remove description. Add maxItems: 1 Okay. >> + >> patternProperties: >> "^gpio@[0-9a-f]+$": >> type: object >> @@ -88,10 +92,8 @@ patternProperties: >> required: >> - compatible >> - reg >> - - interrupts >> - "#address-cells" >> - "#size-cells" >> - - "#interrupt-cells" > Should reset-gpios be required? I'd expect this patch to be adding > reset-gpios and making it required and that's about it. Given that > there isn't a DT using this compatible upstream so far it looks like we > don't need to do anything like worry about backwards compatibility. Okay. >> additionalProperties: false >> >> @@ -99,11 +101,12 @@ examples: >> - | >> #include <dt-bindings/mfd/qcom-pm8008.h> >> #include <dt-bindings/interrupt-controller/irq.h> >> + #include <dt-bindings/gpio/gpio.h> >> qupv3_se13_i2c { >> #address-cells = <1>; >> #size-cells = <0>; >> - pm8008i@8 { >> - compatible = "qcom,pm8008"; >> + pm8008_infra: pmic@8 { > Remove unused phandle. Okay. >> + compatible = "qcom,pm8008-infra"; >> reg = <0x8>; >> #address-cells = <1>; >> #size-cells = <0>; >> @@ -113,6 +116,8 @@ examples: >> interrupt-parent = <&tlmm>; >> interrupts = <32 IRQ_TYPE_EDGE_RISING>; >> >> + reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>; >> + >> pm8008_gpios: gpio@c000 { >> compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio"; >> reg = <0xc000>; >> -- >> 2.7.4 >>
Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:55:03) > > On 3/25/2022 11:32 PM, Stephen Boyd wrote: > > Quoting Satya Priya (2022-03-25 04:58:05) > >> Modify the compatible string as per the pm8008 mfd driver. > >> Add reset-gpios property and make interrupts and interrupt-cells > >> as optional properties, they are not strictly required and may > >> cause yaml compilation errors when not added in the DT files. > > Does it have an interrupt controller inside? If so, the properties > > should be present even if the driver isn't using them. > > > Yes it has an interrupt controller but we are not configuring/using any > interrupts currently in the DT node. So, if we add the properties under > "required" list it throws a make dtbs error. Do you think we can somehow > bypass this error? > Can you fix the existing node to have those properties? > > >> Also, change the node name in example to match with the > >> pm8008_infra DT node. > >> > >> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> > >> --- > >> 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. > >> > >> Changes in V8: > >> - Change the compatible as per driver, remove interrupts from required > >> properties, add reset-gpios and move regulators to separate binding. > >> > >> Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++----- > >> 1 file changed, 10 insertions(+), 5 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > >> index ec3138c..12431ea 100644 > >> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > >> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml > >> @@ -16,7 +16,7 @@ description: | > >> > >> properties: > >> compatible: > >> - const: qcom,pm8008 > >> + const: qcom,pm8008-infra > > Why is the compatible being replaced with -infra postfix? > > > I've changed the compatible after splitting the probe into two parts > pm8008-infra-probe and pm8008-regulators-probe. Only reason for change > is to make it more readable/understandable. > > Please leave the pm8008 compatible alone.
On 4/1/2022 2:45 AM, Stephen Boyd wrote: > Quoting Satya Priya Kakitapalli (Temp) (2022-03-30 03:55:03) >> On 3/25/2022 11:32 PM, Stephen Boyd wrote: >>> Quoting Satya Priya (2022-03-25 04:58:05) >>>> Modify the compatible string as per the pm8008 mfd driver. >>>> Add reset-gpios property and make interrupts and interrupt-cells >>>> as optional properties, they are not strictly required and may >>>> cause yaml compilation errors when not added in the DT files. >>> Does it have an interrupt controller inside? If so, the properties >>> should be present even if the driver isn't using them. >> >> Yes it has an interrupt controller but we are not configuring/using any >> interrupts currently in the DT node. So, if we add the properties under >> "required" list it throws a make dtbs error. Do you think we can somehow >> bypass this error? >> > Can you fix the existing node to have those properties? Okay. >>>> Also, change the node name in example to match with the >>>> pm8008_infra DT node. >>>> >>>> Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> >>>> --- >>>> 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. >>>> >>>> Changes in V8: >>>> - Change the compatible as per driver, remove interrupts from required >>>> properties, add reset-gpios and move regulators to separate binding. >>>> >>>> Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++----- >>>> 1 file changed, 10 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >>>> index ec3138c..12431ea 100644 >>>> --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >>>> +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml >>>> @@ -16,7 +16,7 @@ description: | >>>> >>>> properties: >>>> compatible: >>>> - const: qcom,pm8008 >>>> + const: qcom,pm8008-infra >>> Why is the compatible being replaced with -infra postfix? >> >> I've changed the compatible after splitting the probe into two parts >> pm8008-infra-probe and pm8008-regulators-probe. Only reason for change >> is to make it more readable/understandable. >> >> > Please leave the pm8008 compatible alone. Okay.
diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml index ec3138c..12431ea 100644 --- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml +++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml @@ -16,7 +16,7 @@ description: | properties: compatible: - const: qcom,pm8008 + const: qcom,pm8008-infra reg: description: @@ -44,6 +44,10 @@ properties: "#size-cells": const: 0 + reset-gpios: + description: | + Specifies the GPIO to be toggled to bring pm8008 chip out of reset. + patternProperties: "^gpio@[0-9a-f]+$": type: object @@ -88,10 +92,8 @@ patternProperties: required: - compatible - reg - - interrupts - "#address-cells" - "#size-cells" - - "#interrupt-cells" additionalProperties: false @@ -99,11 +101,12 @@ examples: - | #include <dt-bindings/mfd/qcom-pm8008.h> #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> qupv3_se13_i2c { #address-cells = <1>; #size-cells = <0>; - pm8008i@8 { - compatible = "qcom,pm8008"; + pm8008_infra: pmic@8 { + compatible = "qcom,pm8008-infra"; reg = <0x8>; #address-cells = <1>; #size-cells = <0>; @@ -113,6 +116,8 @@ examples: interrupt-parent = <&tlmm>; interrupts = <32 IRQ_TYPE_EDGE_RISING>; + reset-gpios = <&pm8350c_gpios 4 GPIO_ACTIVE_HIGH>; + pm8008_gpios: gpio@c000 { compatible = "qcom,pm8008-gpio", "qcom,spmi-gpio"; reg = <0xc000>;
Modify the compatible string as per the pm8008 mfd driver. Add reset-gpios property and make interrupts and interrupt-cells as optional properties, they are not strictly required and may cause yaml compilation errors when not added in the DT files. Also, change the node name in example to match with the pm8008_infra DT node. Signed-off-by: Satya Priya <quic_c_skakit@quicinc.com> --- 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. Changes in V8: - Change the compatible as per driver, remove interrupts from required properties, add reset-gpios and move regulators to separate binding. Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)