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

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

[Icenowy Zheng]

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

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>;
+     };
-- 
2.37.1

[PATCH 2/2] clocksource/drivers: add LiteX timer

[Icenowy Zheng]

Add a driver for the timer as part of LiteX SoC generator. By default,
it's a 32-bit down counter with reload support.

It has an optional uptime counter, however because it's noe defaultly
enabled, it's not supported yet.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 MAINTAINERS                       |   1 +
 drivers/clocksource/Kconfig       |  10 ++
 drivers/clocksource/Makefile      |   1 +
 drivers/clocksource/timer-litex.c | 163 ++++++++++++++++++++++++++++++
 4 files changed, 175 insertions(+)
 create mode 100644 drivers/clocksource/timer-litex.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1df62c469bd9..5892a0083531 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11973,6 +11973,7 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/*/litex,*.yaml
 F:	arch/openrisc/boot/dts/or1klitex.dts
 F:	include/linux/litex.h
+F:	drivers/clocksource/timer-litex.c
 F:	drivers/tty/serial/liteuart.c
 F:	drivers/soc/litex/*
 F:	drivers/net/ethernet/litex/*
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 4469e7f555e9..6936e09d1898 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -657,6 +657,16 @@ config GX6605S_TIMER
 	help
 	  This option enables support for gx6605s SOC's timer.
 
+config LITEX_TIMER
+	bool "LiteX SoC timer"
+	default LITEX
+	depends on OF
+	select TIMER_OF
+	help
+	  Say yes here to enable LiteX SoC timer driver automatically
+	  generated in a LiteX SoC. This timer could be useful when the
+	  CPU core itself does not contain a supported timer.
+
 config MILBEAUT_TIMER
 	bool "Milbeaut timer driver" if COMPILE_TEST
 	depends on OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 64ab547de97b..c7d3eda617a7 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_MILBEAUT_TIMER)	+= timer-milbeaut.o
 obj-$(CONFIG_SPRD_TIMER)	+= timer-sprd.o
 obj-$(CONFIG_NPCM7XX_TIMER)	+= timer-npcm7xx.o
 obj-$(CONFIG_RDA_TIMER)		+= timer-rda.o
+obj-$(CONFIG_LITEX_TIMER)	+= timer-litex.o
 
 obj-$(CONFIG_ARC_TIMERS)		+= arc_timer.o
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
diff --git a/drivers/clocksource/timer-litex.c b/drivers/clocksource/timer-litex.c
new file mode 100644
index 000000000000..609023403602
--- /dev/null
+++ b/drivers/clocksource/timer-litex.c
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * LiteX SoC builder timer handling.
+ *
+ * Copyright (C) 2022 Icenowy Zheng <uwu@icenowy.me>
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqreturn.h>
+#include <linux/sched_clock.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#include "timer-of.h"
+
+/*
+ * CSRs definitions (base address offsets + width)
+ *
+ * The definitions below are true for LiteX SoC configured for 32-bit CSR Bus,
+ * 32-bit aligned, and the timer configured as 32-bit.
+ *
+ * Supporting other configurations might require new definitions or a more
+ * generic way of indexing the LiteX CSRs.
+ *
+ * For more details on how CSRs are defined and handled in LiteX, see comments
+ * in the LiteX SoC Driver: drivers/soc/litex/litex_soc_ctrl.c
+ */
+#define OFF_LOAD		0x00
+#define OFF_RELOAD		0x04
+#define OFF_EN			0x08
+#define OFF_UPDATE_VALUE	0x0c
+#define OFF_VALUE		0x10
+#define OFF_EV_STATUS		0x14
+#define OFF_EV_PENDING		0x18
+#define OFF_EV_ENABLE		0x1c
+
+/* events */
+#define EV_ZERO			BIT(0)
+
+static void litex_timer_enable(struct timer_of *to, bool enable)
+{
+	writel(enable ? EV_ZERO : 0, timer_of_base(to) + OFF_EV_ENABLE);
+	writel(enable ? 1 : 0, timer_of_base(to) + OFF_EN);
+}
+
+static int litex_timer_next_event(unsigned long delta,
+				  struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	litex_timer_enable(to, false);
+	writel((uint32_t) delta, timer_of_base(to) + OFF_LOAD);
+	litex_timer_enable(to, true);
+
+	return 0;
+}
+
+static int litex_timer_state_oneshot(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	litex_timer_enable(to, false);
+	writel(0, timer_of_base(to) + OFF_RELOAD);
+	litex_timer_enable(to, true);
+
+	return 0;
+}
+
+static int litex_timer_state_periodic(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	litex_timer_enable(to, false);
+	writel((uint32_t) timer_of_period(to), timer_of_base(to) + OFF_RELOAD);
+	writel((uint32_t) timer_of_period(to), timer_of_base(to) + OFF_LOAD);
+	litex_timer_enable(to, true);
+
+	return 0;
+}
+
+static int litex_timer_state_shutdown(struct clock_event_device *evt)
+{
+	struct timer_of *to = to_timer_of(evt);
+
+	litex_timer_enable(to, false);
+
+	return 0;
+}
+
+static irqreturn_t litex_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+	struct timer_of *to = to_timer_of(evt);
+	u32 val;
+
+	val = readl(timer_of_base(to) + OFF_EV_PENDING);
+	if (!(val & EV_ZERO))
+		return IRQ_NONE;
+
+	writel(EV_ZERO, timer_of_base(to) + OFF_EV_PENDING);
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct timer_of to_litex = {
+	.flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
+	.clkevt = {
+		.name			= "LiteX Timer",
+		.features		= CLOCK_EVT_FEAT_ONESHOT |
+					  CLOCK_EVT_FEAT_PERIODIC,
+		.set_state_oneshot	= litex_timer_state_oneshot,
+		.set_state_periodic	= litex_timer_state_periodic,
+		.set_next_event		= litex_timer_next_event,
+		.set_state_shutdown	= litex_timer_state_shutdown,
+		.rating			= 101,
+	},
+	.of_irq = {
+		.handler		= litex_timer_interrupt,
+		.flags			= IRQF_TIMER,
+	},
+};
+
+static void __init litex_clockevent_init(void)
+{
+	to_litex.clkevt.cpumask = cpu_possible_mask;
+
+	clockevents_config_and_register(&to_litex.clkevt,
+					timer_of_rate(&to_litex),
+					0x1, 0xffffffff);
+}
+
+static int __init litex_timer_init(struct device_node *np)
+{
+	int ret = 0;
+	u32 width;
+
+	ret = of_property_read_u32(np, "litex,width", &width);
+	if (ret) {
+		pr_err("Cannot retrieve width\n");
+		return ret;
+	}
+	if (width != 32) {
+		pr_err("Unsupported width\n");
+		return -ENOTSUPP;
+	}
+
+	ret = timer_of_init(np, &to_litex);
+	if (ret) {
+		pr_err("Cannot parse DT for LiteX timer\n");
+		return ret;
+	}
+
+	litex_clockevent_init();
+
+	return 0;
+}
+TIMER_OF_DECLARE(litex, "litex,timer", litex_timer_init);
-- 
2.37.1

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

[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.

> +
> +  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

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

[Icenowy Zheng]

在 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

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

[Krzysztof Kozlowski]

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

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

[Rob Herring]

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.