| Message ID | 20221118092218.480147-2-mranostay@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | mfd: add tps6594x support for Jacinto platforms | expand |
On 18/11/2022 10:22, Matt Ranostay wrote: > Add documentation for the TPS6594x PMIC including its RTC and GPIO > functionalities. > This is a friendly reminder during the review process. It seems my previous comments were not fully addressed. Maybe my feedback got lost between the quotes, maybe you just forgot to apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them. Thank you. The comment was about proper subject prefix. Drop also redundant, second "DT bindings" from the subject. > Signed-off-by: Matt Ranostay <mranostay@ti.com> > --- > .../devicetree/bindings/mfd/ti,tps6594.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > new file mode 100644 > index 000000000000..81613bcef39d > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/ti,tps6594x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TPS6594x Power Management Integrated Circuit (PMIC) > + > +maintainers: > + - Keerthy <j-keerthy@ti.com> > + > +properties: > + compatible: > + enum: > + - ti,tps6594 tps6594 is the model name? So what was previous tps6594x used also in title? This does not look correct. > + > + reg: > + const: 0x48 > + > + ti,system-power-controller: > + type: boolean > + description: PMIC is controlling the system power. > + > + rtc: > + type: object > + $ref: /schemas/rtc/rtc.yaml# > + unevaluatedProperties: false > + properties: > + compatible: > + const: ti,tps6594-rtc Same problem. > + > + gpio: > + type: object > + unevaluatedProperties: false > + properties: > + compatible: > + const: ti,tps6594x-gpio Not fixed. > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + > +examples: > + - | > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pmic: pmic@48 { > + compatible = "ti,tps6594"; > + reg = <0x48>; > + > + rtc { > + compatible = "ti,tps6594-rtc"; > + }; > + > + gpio { > + compatible = "ti,tps6594-gpio"; Does not look like you tested the bindings. Please run `make dt_binding_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). Best regards, Krzysztof
On Fri, 18 Nov 2022 01:22:15 -0800, Matt Ranostay wrote: > Add documentation for the TPS6594x PMIC including its RTC and GPIO > functionalities. > > Signed-off-by: Matt Ranostay <mranostay@ti.com> > --- > .../devicetree/bindings/mfd/ti,tps6594.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/mfd/ti,tps6594.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/mfd/ti,tps6594.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/ti,tps6594.example.dtb: pmic@48: gpio: Unevaluated properties are not allowed ('compatible' was unexpected) From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/ti,tps6594.example.dtb: pmic@48: gpio:compatible:0: 'ti,tps6594x-gpio' was expected From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml Documentation/devicetree/bindings/mfd/ti,tps6594.example.dtb:0:0: /example-0/i2c0/pmic@48/gpio: failed to match any schema with compatible: ['ti,tps6594-gpio'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221118092218.480147-2-mranostay@ti.com This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command.
On Fri, Nov 18, 2022 at 11:21:57AM +0100, Krzysztof Kozlowski wrote: > On 18/11/2022 10:22, Matt Ranostay wrote: > > Add documentation for the TPS6594x PMIC including its RTC and GPIO > > functionalities. > > > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. > I missed serveral feedback suggestions in the last revision. Will try to be a little more careful in the future. > Thank you. > > The comment was about proper subject prefix. Drop also redundant, second > "DT bindings" from the subject. > > > > Signed-off-by: Matt Ranostay <mranostay@ti.com> > > --- > > .../devicetree/bindings/mfd/ti,tps6594.yaml | 65 +++++++++++++++++++ > > 1 file changed, 65 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > > > > diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > > new file mode 100644 > > index 000000000000..81613bcef39d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml > > @@ -0,0 +1,65 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/ti,tps6594x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: TPS6594x Power Management Integrated Circuit (PMIC) > > + > > +maintainers: > > + - Keerthy <j-keerthy@ti.com> > > + > > +properties: > > + compatible: > > + enum: > > + - ti,tps6594 > > tps6594 is the model name? So what was previous tps6594x used also in > title? This does not look correct. > So the actual part number is TPS6594-Q1. Would tps6594q1 be more concise for naming across the device tree, and drivers? - Matt > > + > > + reg: > > + const: 0x48 > > + > > + ti,system-power-controller: > > + type: boolean > > + description: PMIC is controlling the system power. > > + > > + rtc: > > + type: object > > + $ref: /schemas/rtc/rtc.yaml# > > + unevaluatedProperties: false > > + properties: > > + compatible: > > + const: ti,tps6594-rtc > > Same problem. > > > + > > + gpio: > > + type: object > > + unevaluatedProperties: false > > + properties: > > + compatible: > > + const: ti,tps6594x-gpio > > > Not fixed. > > > > > + > > +additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + > > +examples: > > + - | > > + i2c0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pmic: pmic@48 { > > + compatible = "ti,tps6594"; > > + reg = <0x48>; > > + > > + rtc { > > + compatible = "ti,tps6594-rtc"; > > + }; > > + > > + gpio { > > + compatible = "ti,tps6594-gpio"; > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml new file mode 100644 index 000000000000..81613bcef39d --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/ti,tps6594.yaml @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/ti,tps6594x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TPS6594x Power Management Integrated Circuit (PMIC) + +maintainers: + - Keerthy <j-keerthy@ti.com> + +properties: + compatible: + enum: + - ti,tps6594 + + reg: + const: 0x48 + + ti,system-power-controller: + type: boolean + description: PMIC is controlling the system power. + + rtc: + type: object + $ref: /schemas/rtc/rtc.yaml# + unevaluatedProperties: false + properties: + compatible: + const: ti,tps6594-rtc + + gpio: + type: object + unevaluatedProperties: false + properties: + compatible: + const: ti,tps6594x-gpio + +additionalProperties: false + +required: + - compatible + - reg + +examples: + - | + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + pmic: pmic@48 { + compatible = "ti,tps6594"; + reg = <0x48>; + + rtc { + compatible = "ti,tps6594-rtc"; + }; + + gpio { + compatible = "ti,tps6594-gpio"; + }; + }; + }; + +...
Add documentation for the TPS6594x PMIC including its RTC and GPIO functionalities. Signed-off-by: Matt Ranostay <mranostay@ti.com> --- .../devicetree/bindings/mfd/ti,tps6594.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/ti,tps6594.yaml