Message ID | 20221121042545.421532-1-uwu@icenowy.me |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | [1/2] dt-bindings: timer: add a binding for LiteX Timer | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/patch-applied | success | |
robh/dt-meta-schema | fail | build log |
On 21/11/2022 05:25, Icenowy Zheng wrote: > The LiteX SoC generator has a timer core, which by default only > generates a simple down counter. Subject: drop second, redundant "bindings". > > Add a DT binding for it. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > .../bindings/timer/litex,timer.yaml | 52 +++++++++++++++++++ > 1 file changed, 52 insertions(+) > create mode 100644 Documentation/devicetree/bindings/timer/litex,timer.yaml > > diff --git a/Documentation/devicetree/bindings/timer/litex,timer.yaml b/Documentation/devicetree/bindings/timer/litex,timer.yaml > new file mode 100644 > index 000000000000..bece07586c6b > --- /dev/null > +++ b/Documentation/devicetree/bindings/timer/litex,timer.yaml > @@ -0,0 +1,52 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/timer/litex,timer.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LiteX Timer > + > +maintainers: > + - Icenowy Zheng <uwu@icenowy.me> > + > +description: | > + The LiteX Timer is a count-down timer that is defaultly embedded > + into all LiteX SoCs, unless explicitly disabled. It's fed directly > + by the system clock like other LiteX peripherals. > + > +properties: > + compatible: > + const: litex,timer No model name/number? If it is part of Soc, then a SoC specific number is expected usually. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + litex,width: > + description: > + The width of the timer's value, specified as the width argument > + when creating an instance of litex.soc.cores.Timer. This lacks type ($ref) and units in description, but more important - why this is not part of compatible? Is it a width of register(s)? And what is "instance of litex.soc.cores.Timer"? Is it configurable, soft-core? BTW, there is reg-io-width property. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - litex,width > + > +additionalProperties: false > + > +examples: > + - | > + timer@a0006000 { Use 4 spaces for example indentation. > + compatible = "litex,timer"; > + reg = <0xa0006000 0x20>; > + clocks = <&sys_clk>; > + interrupts = <17>; > + litex,width = <32>; > + }; Best regards, Krzysztof
在 2022-11-21星期一的 11:19 +0100,Krzysztof Kozlowski写道: > On 21/11/2022 05:25, Icenowy Zheng wrote: > > The LiteX SoC generator has a timer core, which by default only > > generates a simple down counter. > > Subject: drop second, redundant "bindings". > > > > > Add a DT binding for it. > > > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > > --- > > .../bindings/timer/litex,timer.yaml | 52 > > +++++++++++++++++++ > > 1 file changed, 52 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/timer/litex,timer.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/timer/litex,timer.yaml > > b/Documentation/devicetree/bindings/timer/litex,timer.yaml > > new file mode 100644 > > index 000000000000..bece07586c6b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/timer/litex,timer.yaml > > @@ -0,0 +1,52 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/timer/litex,timer.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: LiteX Timer > > + > > +maintainers: > > + - Icenowy Zheng <uwu@icenowy.me> > > + > > +description: | > > + The LiteX Timer is a count-down timer that is defaultly embedded > > + into all LiteX SoCs, unless explicitly disabled. It's fed > > directly > > + by the system clock like other LiteX peripherals. > > + > > +properties: > > + compatible: > > + const: litex,timer > > No model name/number? If it is part of Soc, then a SoC specific > number > is expected usually. Ah it's part of a SoC generator, as a default core. [1] If you like, I think the version of LiteX SoC generator itself could be added, like `litex,timer-22.08`. > > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + litex,width: > > + description: > > + The width of the timer's value, specified as the width > > argument > > + when creating an instance of litex.soc.cores.Timer. > > This lacks type ($ref) and units in description, but more important - > why this is not part of compatible? Is it a width of register(s)? > > And what is "instance of litex.soc.cores.Timer"? Is it configurable, > soft-core? Yes, it is a configurable soft core, although the configuration of a non-32-bit counter is only available when directly generating this core w/o using the full SoC generator (the full SoC generator defaults to 32-bit). > > BTW, there is reg-io-width property. This is not register I/O width, it's only the width of the counter. And because of the LiteX CSR bus nature, all registers after this will be automatically moved if this register's size goes beyond the alignment (current all LiteX support in Linux mainline assumes 32-bit alignment, which is also the default configuration). > > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + - litex,width > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + timer@a0006000 { > > Use 4 spaces for example indentation. Thanks, will change this in the next iteration. > > > + compatible = "litex,timer"; > > + reg = <0xa0006000 0x20>; > > + clocks = <&sys_clk>; > > + interrupts = <17>; > > + litex,width = <32>; > > + }; > > Best regards, > Krzysztof > [1] https://github.com/enjoy-digital/litex/blob/master/litex/soc/cores/timer.py
On 21/11/2022 15:39, Icenowy Zheng wrote: > 在 2022-11-21星期一的 11:19 +0100,Krzysztof Kozlowski写道: >> On 21/11/2022 05:25, Icenowy Zheng wrote: >>> The LiteX SoC generator has a timer core, which by default only >>> generates a simple down counter. >> >> Subject: drop second, redundant "bindings". >> >>> >>> Add a DT binding for it. >>> >>> Signed-off-by: Icenowy Zheng <uwu@icenowy.me> >>> --- >>> .../bindings/timer/litex,timer.yaml | 52 >>> +++++++++++++++++++ >>> 1 file changed, 52 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/timer/litex,timer.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/timer/litex,timer.yaml >>> b/Documentation/devicetree/bindings/timer/litex,timer.yaml >>> new file mode 100644 >>> index 000000000000..bece07586c6b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/timer/litex,timer.yaml >>> @@ -0,0 +1,52 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/timer/litex,timer.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: LiteX Timer >>> + >>> +maintainers: >>> + - Icenowy Zheng <uwu@icenowy.me> >>> + >>> +description: | >>> + The LiteX Timer is a count-down timer that is defaultly embedded >>> + into all LiteX SoCs, unless explicitly disabled. It's fed >>> directly >>> + by the system clock like other LiteX peripherals. >>> + >>> +properties: >>> + compatible: >>> + const: litex,timer >> >> No model name/number? If it is part of Soc, then a SoC specific >> number >> is expected usually. > > Ah it's part of a SoC generator, as a default core. [1] > > If you like, I think the version of LiteX SoC generator itself could be > added, like `litex,timer-22.08`. How can you be sure that all soft-cores from Litex will be exactly the same and use the same compatible? The naming is poor here, but I don't know whether some arbitrary version number is the solution. > >> >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 1 >>> + >>> + litex,width: >>> + description: >>> + The width of the timer's value, specified as the width >>> argument >>> + when creating an instance of litex.soc.cores.Timer. >> >> This lacks type ($ref) and units in description, but more important - >> why this is not part of compatible? Is it a width of register(s)? >> >> And what is "instance of litex.soc.cores.Timer"? Is it configurable, >> soft-core? > > Yes, it is a configurable soft core, although the configuration of a > non-32-bit counter is only available when directly generating this core > w/o using the full SoC generator (the full SoC generator defaults to > 32-bit). > >> >> BTW, there is reg-io-width property. > > This is not register I/O width, it's only the width of the counter. > > And because of the LiteX CSR bus nature, all registers after this will > be automatically moved if this register's size goes beyond the > alignment (current all LiteX support in Linux mainline assumes 32-bit > alignment, which is also the default configuration). OK Best regards, Krzysztof
On Mon, 21 Nov 2022 12:25:44 +0800, Icenowy Zheng wrote: > The LiteX SoC generator has a timer core, which by default only > generates a simple down counter. > > Add a DT binding for it. > > Signed-off-by: Icenowy Zheng <uwu@icenowy.me> > --- > .../bindings/timer/litex,timer.yaml | 52 +++++++++++++++++++ > 1 file changed, 52 insertions(+) > create mode 100644 Documentation/devicetree/bindings/timer/litex,timer.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/timer/litex,timer.yaml: properties:litex,width: 'oneOf' conditional failed, one must be fixed: 'type' is a required property hint: A vendor boolean property can use "type: boolean" /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/timer/litex,timer.yaml: properties:litex,width: 'oneOf' conditional failed, one must be fixed: 'enum' is a required property 'const' is a required property hint: A vendor string property with exact values has an implicit type from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/timer/litex,timer.yaml: properties:litex,width: 'oneOf' conditional failed, one must be fixed: '$ref' is a required property 'allOf' is a required property hint: A vendor property needs a $ref to types.yaml from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221121042545.421532-1-uwu@icenowy.me 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.
diff --git a/Documentation/devicetree/bindings/timer/litex,timer.yaml b/Documentation/devicetree/bindings/timer/litex,timer.yaml new file mode 100644 index 000000000000..bece07586c6b --- /dev/null +++ b/Documentation/devicetree/bindings/timer/litex,timer.yaml @@ -0,0 +1,52 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/timer/litex,timer.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LiteX Timer + +maintainers: + - Icenowy Zheng <uwu@icenowy.me> + +description: | + The LiteX Timer is a count-down timer that is defaultly embedded + into all LiteX SoCs, unless explicitly disabled. It's fed directly + by the system clock like other LiteX peripherals. + +properties: + compatible: + const: litex,timer + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + + litex,width: + description: + The width of the timer's value, specified as the width argument + when creating an instance of litex.soc.cores.Timer. + +required: + - compatible + - reg + - interrupts + - clocks + - litex,width + +additionalProperties: false + +examples: + - | + timer@a0006000 { + compatible = "litex,timer"; + reg = <0xa0006000 0x20>; + clocks = <&sys_clk>; + interrupts = <17>; + litex,width = <32>; + };
The LiteX SoC generator has a timer core, which by default only generates a simple down counter. Add a DT binding for it. Signed-off-by: Icenowy Zheng <uwu@icenowy.me> --- .../bindings/timer/litex,timer.yaml | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/timer/litex,timer.yaml