[1/2] dt-bindings: timer: add a binding for LiteX Timer

  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
  

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.