| Message ID | 20231226100431.331616-2-qiujingbao.dlmu@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | riscv: rtc: sophgo: add mfd and rtc support for CV1800 | expand |
On Tue, 26 Dec 2023 18:04:28 +0800, Jingbao Qiu wrote: > Add devicetree binding for Sophgo CV1800 SoC MFD subsys. > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > --- > .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.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: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml: Error in referenced schema matching $id: http://devicetree.org/schemas/rtc/sophgo,cv1800-rtc.yaml Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.example.dts:25:18: fatal error: dt-bindings/clock/sophgo,cv1800.h: No such file or directory 25 | #include <dt-bindings/clock/sophgo,cv1800.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2 make: *** [Makefile:234: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231226100431.331616-2-qiujingbao.dlmu@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. 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 yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 26/12/2023 11:04, Jingbao Qiu wrote: > Add devicetree binding for Sophgo CV1800 SoC MFD subsys. Subject and commit msg: there is no such hardware as "MFD subsys". Is this a PMIC? Does not look like. You must describe here hardware, not Linux subsystem. > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > --- Please mention the dependency here. > .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ > 1 file changed, 51 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > new file mode 100644 > index 000000000000..c2a071c8a2de > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > @@ -0,0 +1,51 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-subsys.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sophgo CV1800 SoC subsys controller > + > +maintainers: > + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> > + > +description: > + The Sophgo CV1800 SoC subsys controller contains many functions What is "subsys"? Why is it in MFD directory? SoC components like system-controllers do not go to MFD. > + for example, RTC and restart. In addition, CV1800 has an 8051 > + subsystem, which is configured through registers at this controller. > + > +properties: > + compatible: > + items: > + - const: sophgo,cv1800b-subsys > + - const: syscon > + - const: simple-mfd > + > + reg: > + maxItems: 1 > + > + rtc: > + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml# Your patchset is not bisectable. What's more, you have dependency between patches, so bindings cannot go via separate trees: mfd and rtc. You need to make this *EXPLICIT* in the cover letter or patch changelog. I do not see any resources in MFD block, so why having it as separate node? What other devices you did not describe here? You mentioned restart and 8051, so where are they? Which driver implements them? Best regards, Krzysztof
On 26/12/2023 11:04, Jingbao Qiu wrote: > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + #include <dt-bindings/clock/sophgo,cv1800.h> > + > + syscon@5025000 { This example and DTS suggest this is system-controller, so use that name. Assuming this is system-controller, because I am still not sure. Best regards, Krzysztof
On Tue, Dec 26, 2023 at 7:39 PM Rob Herring <robh@kernel.org> wrote: > > > On Tue, 26 Dec 2023 18:04:28 +0800, Jingbao Qiu wrote: > > Add devicetree binding for Sophgo CV1800 SoC MFD subsys. > > > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > > --- > > .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.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): > I'm sorry for that. My current patch depends on a CLK patch that has not been merged yet. So should I wait for the clk patch to merge before submitting? the depends patch link: https://lore.kernel.org/all/IA1PR20MB49539CDAD9A268CBF6CA184BBB9FA@IA1PR20MB4953.namprd20.prod.outlook.com/ > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml: > Error in referenced schema matching $id: http://devicetree.org/schemas/rtc/sophgo,cv1800-rtc.yaml > Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.example.dts:25:18: fatal error: dt-bindings/clock/sophgo,cv1800.h: No such file or directory > 25 | #include <dt-bindings/clock/sophgo,cv1800.h> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.example.dtb] Error 1 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1424: dt_binding_check] Error 2 > make: *** [Makefile:234: __sub-make] Error 2 > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231226100431.331616-2-qiujingbao.dlmu@gmail.com > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > 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 yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > Best regards, Jingbao Qiu
On Tue, Dec 26, 2023 at 8:18 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 26/12/2023 11:04, Jingbao Qiu wrote: > > Add devicetree binding for Sophgo CV1800 SoC MFD subsys. > > Subject and commit msg: there is no such hardware as "MFD subsys". Is > this a PMIC? Does not look like. You must describe here hardware, not > Linux subsystem. > I don't think this is a PMIC device. the RTC restart and 8051 configure register share one common range address, and the address have other function that power management. the datasheet link: Link: https://github.com/milkv-duo/duo-files/blob/main/duo/datasheet/CV1800B-CV1801B-Preliminary-Datasheet-full-en.pdf chapter: 3.9 RTC 3.12 8051 subsystem > > > > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> > > --- > > Please mention the dependency here. Thanks,I will fix it. > > > .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > > > diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > new file mode 100644 > > index 000000000000..c2a071c8a2de > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml > > @@ -0,0 +1,51 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-subsys.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Sophgo CV1800 SoC subsys controller > > + > > +maintainers: > > + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> > > + > > +description: > > + The Sophgo CV1800 SoC subsys controller contains many functions > > What is "subsys"? Why is it in MFD directory? SoC components like > system-controllers do not go to MFD. This device has multiple different functions, because they have 8051 subsystem, so I named him "subsys". I will carefully consider and rename it. > > > + for example, RTC and restart. In addition, CV1800 has an 8051 > > + subsystem, which is configured through registers at this controller. > > + > > +properties: > > + compatible: > > + items: > > + - const: sophgo,cv1800b-subsys > > + - const: syscon > > + - const: simple-mfd > > + > > + reg: > > + maxItems: 1 > > + > > + rtc: > > + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml# > > Your patchset is not bisectable. What's more, you have dependency > between patches, so bindings cannot go via separate trees: mfd and rtc. > You need to make this *EXPLICIT* in the cover letter or patch changelog. ok,I will fix it. > > I do not see any resources in MFD block, so why having it as separate > node? What other devices you did not describe here? You mentioned > restart and 8051, so where are they? Which driver implements them? > I'am sorry for that other drivers have not been implemented yet. I will implement it after rtc. They have the same address range, so I use mfd to describe them. > > Best regards, > Krzysztof > Best regards, Jingbao Qiu
On Tue, Dec 26, 2023 at 8:21 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 26/12/2023 11:04, Jingbao Qiu wrote: > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/irq.h> > > + #include <dt-bindings/clock/sophgo,cv1800.h> > > + > > + syscon@5025000 { > > This example and DTS suggest this is system-controller, so use that > name. Assuming this is system-controller, because I am still not sure. > Thanks,I will carefully consider how to name him. > Best regards, > Krzysztof >
On 27/12/2023 08:35, Jingbao Qiu wrote: >> >> I do not see any resources in MFD block, so why having it as separate >> node? What other devices you did not describe here? You mentioned >> restart and 8051, so where are they? Which driver implements them? >> > > I'am sorry for that other drivers have not been implemented yet. I > will implement it > after rtc. They have the same address range, so I use mfd to describe them. Bindings should be complete even if your driver is not ready. After looking at such device node, I say you do not need that rtc child. If you sent complete bindings, then of course discussion would be different, but... Best regards, Krzysztof
On Wed, Dec 27, 2023 at 7:37 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 27/12/2023 08:35, Jingbao Qiu wrote: > >> > >> I do not see any resources in MFD block, so why having it as separate > >> node? What other devices you did not describe here? You mentioned > >> restart and 8051, so where are they? Which driver implements them? > >> > > > > I'am sorry for that other drivers have not been implemented yet. I > > will implement it > > after rtc. They have the same address range, so I use mfd to describe them. > > Bindings should be complete even if your driver is not ready. After > looking at such device node, I say you do not need that rtc child. If > you sent complete bindings, then of course discussion would be > different, but... Thank you for your patient explanation. I will supplement it completely in the next version Best regards, Jingbao Qiu > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml new file mode 100644 index 000000000000..c2a071c8a2de --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/sophgo,cv1800-subsys.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sophgo CV1800 SoC subsys controller + +maintainers: + - Jingbao Qiu <qiujingbao.dlmu@gmail.com> + +description: + The Sophgo CV1800 SoC subsys controller contains many functions + for example, RTC and restart. In addition, CV1800 has an 8051 + subsystem, which is configured through registers at this controller. + +properties: + compatible: + items: + - const: sophgo,cv1800b-subsys + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + + rtc: + $ref: /schemas/rtc/sophgo,cv1800-rtc.yaml# + type: object + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/clock/sophgo,cv1800.h> + + syscon@5025000 { + compatible = "sophgo,cv1800b-subsys", "syscon", "simple-mfd"; + reg = <0x05025000 0x2000>; + + rtc { + compatible = "sophgo,cv1800b-rtc"; + interrupts = <17 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk CLK_RTC_25M>; + }; + };
Add devicetree binding for Sophgo CV1800 SoC MFD subsys. Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@gmail.com> --- .../bindings/mfd/sophgo,cv1800-subsys.yaml | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/sophgo,cv1800-subsys.yaml