linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support
@ 2020-05-21  0:53 Serge Semin
  2020-05-21  0:53 ` [PATCH v4 1/7] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema Serge Semin
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Maxim Kaurkin,
	Pavel Parkhomenko, Ramil Zaripov, Ekaterina Skachko,
	Vadim Vlasov, Alexey Kolotnikov, Paul Burton, Ralf Baechle,
	Arnd Bergmann, Alessandro Zummo, Alexandre Belloni, Rob Herring,
	linux-mips, linux-rtc, devicetree, linux-kernel

As for all Baikal-T1 SoC related patchsets, which need this, we replaced
the DW APB Timer legacy plain text-based dt-binding file with DT schema.
Similarly the MIPS GIC bindings file is also converted to DT schema seeing
it also defines the MIPS GIC Timer binding.

Aside from MIPS-specific r4k timer Baikal-T1 chip also provides a
functionality of two another timers: embedded into the MIPS GIC timer and
three external DW timers available over APB bus. But we can't use them
before the corresponding drivers are properly fixed. First of all DW APB
Timer shouldn't be bound to a single CPU, since as being accessible over
APB they are external with respect to all possible CPUs. Secondly there
might be more than just two DW APB Timers in the system (Baikal-T1 has
three of them), so permit the driver to use one of them as a clocksource
and the rest - for clockevents. Thirdly it's possible to use MIPS GIC
timer as a clocksource so register it in the corresponding subsystem
(the patch has been found in the Paul Burton MIPS repo so I left the
original Signed-off-by attribute). Finally in the same way as r4k timer
the MIPS GIC timer should be used with care when CPUFREQ config is enabled
since in case of CM2 the timer counting depends on the CPU reference clock
frequency while the clocksource subsystem currently doesn't support the
timers with non-stable clock.

This patchset is rebased and tested on the mainline Linux kernel 5.7-rc4:
base-commit: 0e698dfa2822 ("Linux 5.7-rc4")
tag: v5.7-rc4

Changelog v2:
- Fix the SoB tags.
- Our corporate email server doesn't change Message-Id anymore, so the
  patchset is resubmitted being in the cover-letter-threaded format.
- Convert the "snps,dw-apb-timer" binding to DT schema in a dedicated
  patch.
- Convert the "mti,gic" binding to DT schema in a dedicated patch.

Link: https://lore.kernel.org/linux-rtc/20200324174325.14213-1-Sergey.Semin@baikalelectronics.ru
Changelog v3:
- Make the MIPS GIC timer sub-node name not having a unit-address number.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ] from MIPS
  GIC bindings.
- Add patch moving the "snps,dw-apb-timer" binding file to the directory
  with timers binding files.

Link: https://lore.kernel.org/linux-rtc/20200506214107.25956-1-Sergey.Semin@baikalelectronics.ru
Changelog v4:
- Mark clocksource as unstable instead of lowering its rating.
- Move conditional sched clocks registration to the Paul' patch.
- Add Thomas Gleixner to the patchset To-list to draw his attention to the
  patch "dt-bindings: interrupt-controller: Convert mti,gic to DT schema".

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Maxim Kaurkin <Maxim.Kaurkin@baikalelectronics.ru>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru>
Cc: Ramil Zaripov <Ramil.Zaripov@baikalelectronics.ru>
Cc: Ekaterina Skachko <Ekaterina.Skachko@baikalelectronics.ru>
Cc: Vadim Vlasov <V.Vlasov@baikalelectronics.ru>
Cc: Alexey Kolotnikov <Alexey.Kolotnikov@baikalelectronics.ru>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Paul Burton (1):
  clocksource: mips-gic-timer: Register as sched_clock

Serge Semin (6):
  dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
  dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
  dt-bindings: interrupt-controller: Convert mti,gic to DT schema
  clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask
  clocksource: dw_apb_timer_of: Fix missing clockevent timers
  clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock
    changes

 .../interrupt-controller/mips-gic.txt         |  67 --------
 .../interrupt-controller/mti,gic.yaml         | 148 ++++++++++++++++++
 .../devicetree/bindings/rtc/dw-apb.txt        |  32 ----
 .../bindings/timer/snps,dw-apb-timer.yaml     |  88 +++++++++++
 drivers/clocksource/Kconfig                   |   1 +
 drivers/clocksource/dw_apb_timer.c            |  18 +--
 drivers/clocksource/dw_apb_timer_of.c         |   9 +-
 drivers/clocksource/mips-gic-timer.c          |  50 +++++-
 include/linux/dw_apb_timer.h                  |   2 +-
 9 files changed, 293 insertions(+), 122 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
 delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
 create mode 100644 Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/7] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 2/7] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc Serge Semin
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
	Alessandro Zummo, Alexandre Belloni, Rob Herring
  Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
	Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
	devicetree, linux-kernel

Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces Synopsys DW Timer
legacy bare text binding with YAML file. As before the binding file
states that the corresponding dts node is supposed to be compatible
with generic DW APB Timer indicated by the "snps,dw-apb-timer"
compatible string and to provide a mandatory registers memory range,
one timer interrupt, either reference clock source or a fixed clock
rate value. It may also have an optional APB bus reference clock
phandle specified.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org

---

This binding file doesn't belong to the bindings/rtc seeing it's a pure
timer with no rtc facilities like days/months/years counting and alarms.
The binding file will be moved to the
"Documentation/devicetree/bindings/timer/" directory in the next patch.

I also don't know who is the corresponding driver maintainer, so I added
Daniel Lezcano to the maintainers schema. Any idea what email should be
specified there instead?

Changelog v3:
- Since it's a conversion patch use GPL-2.0-only SPDX header.
- Replace "additionalProperties: false" property with
  "unevaluatedProperties: false".
---
 .../devicetree/bindings/rtc/dw-apb.txt        | 32 -------
 .../bindings/rtc/snps,dw-apb-timer.yaml       | 88 +++++++++++++++++++
 2 files changed, 88 insertions(+), 32 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml

diff --git a/Documentation/devicetree/bindings/rtc/dw-apb.txt b/Documentation/devicetree/bindings/rtc/dw-apb.txt
deleted file mode 100644
index c703d51abb6c..000000000000
--- a/Documentation/devicetree/bindings/rtc/dw-apb.txt
+++ /dev/null
@@ -1,32 +0,0 @@
-* Designware APB timer
-
-Required properties:
-- compatible: One of:
- 	"snps,dw-apb-timer"
-	"snps,dw-apb-timer-sp" <DEPRECATED>
-	"snps,dw-apb-timer-osc" <DEPRECATED>
-- reg: physical base address of the controller and length of memory mapped
-  region.
-- interrupts: IRQ line for the timer.
-- either clocks+clock-names or clock-frequency properties
-
-Optional properties:
-- clocks	: list of clock specifiers, corresponding to entries in
-		  the clock-names property;
-- clock-names	: should contain "timer" and "pclk" entries, matching entries
-		  in the clocks property.
-- clock-frequency: The frequency in HZ of the timer.
-- clock-freq: For backwards compatibility with picoxcell
-
-If using the clock specifiers, the pclk clock is optional, as not all
-systems may use one.
-
-
-Example:
-	timer@ffe00000 {
-		compatible = "snps,dw-apb-timer";
-		interrupts = <0 170 4>;
-		reg = <0xffe00000 0x1000>;
-		clocks = <&timer_clk>, <&timer_pclk>;
-		clock-names = "timer", "pclk";
-	};
diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
new file mode 100644
index 000000000000..002fe1ee709b
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare APB Timer
+
+maintainers:
+  - Daniel Lezcano <daniel.lezcano@linaro.org>
+
+properties:
+  compatible:
+    oneOf:
+      - const: snps,dw-apb-timer
+      - enum:
+          - snps,dw-apb-timer-sp
+          - snps,dw-apb-timer-osc
+        deprecated: true
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+       - description: Timer ticks reference clock source
+       - description: APB interface clock source
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: timer
+      - const: pclk
+
+  clock-frequency: true
+
+  clock-freq:
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    description: |
+      Has the same meaning as the 'clock-frequency' property - timer clock
+      frequency in HZ, but is defined only for the backwards compatibility
+      with the picoxcell platform.
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+oneOf:
+  - required:
+      - clocks
+      - clock-names
+  - required:
+      - clock-frequency
+  - required:
+      - clock-freq
+
+examples:
+  - |
+    timer@ffe00000 {
+      compatible = "snps,dw-apb-timer";
+      interrupts = <0 170 4>;
+      reg = <0xffe00000 0x1000>;
+      clocks = <&timer_clk>, <&timer_pclk>;
+      clock-names = "timer", "pclk";
+    };
+  - |
+    timer@ffe00000 {
+      compatible = "snps,dw-apb-timer";
+      interrupts = <0 170 4>;
+      reg = <0xffe00000 0x1000>;
+      clocks = <&timer_clk>;
+      clock-names = "timer";
+    };
+  - |
+    timer@ffe00000 {
+      compatible = "snps,dw-apb-timer";
+      interrupts = <0 170 4>;
+      reg = <0xffe00000 0x1000>;
+      clock-frequency = <25000000>;
+    };
+...
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/7] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
  2020-05-21  0:53 ` [PATCH v4 1/7] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 3/7] dt-bindings: interrupt-controller: Convert mti,gic to DT schema Serge Semin
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
	Alessandro Zummo, Alexandre Belloni, Rob Herring
  Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
	Paul Burton, Ralf Baechle, Arnd Bergmann, linux-mips, linux-rtc,
	devicetree, linux-kernel

This binding file doesn't belong to the rtc seeing it's a pure timer
with no rtc facilities like days/months/years counting and alarms.
So move the YAML-file to the Documentation/devicetree/bindings/timer/
directory.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
---
 .../devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/{rtc => timer}/snps,dw-apb-timer.yaml (96%)

diff --git a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
similarity index 96%
rename from Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
rename to Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
index 002fe1ee709b..5d300efdf0ca 100644
--- a/Documentation/devicetree/bindings/rtc/snps,dw-apb-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/snps,dw-apb-timer.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/rtc/snps,dw-apb-timer.yaml#
+$id: http://devicetree.org/schemas/timer/snps,dw-apb-timer.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Synopsys DesignWare APB Timer
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/7] dt-bindings: interrupt-controller: Convert mti,gic to DT schema
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
  2020-05-21  0:53 ` [PATCH v4 1/7] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema Serge Semin
  2020-05-21  0:53 ` [PATCH v4 2/7] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask Serge Semin
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
	Jason Cooper, Marc Zyngier, Rob Herring
  Cc: Serge Semin, Serge Semin, Rob Herring, Alexey Malahov,
	Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
	Arnd Bergmann, linux-mips, linux-rtc, linux-kernel, devicetree

Modern device tree bindings are supposed to be created as YAML-files
in accordance with DT schema. This commit replaces MIPS GIC legacy bare
text binding with YAML file. As before the binding file states that the
corresponding dts node is supposed to be compatible with MIPS Global
Interrupt Controller indicated by the "mti,gic" compatible string and
to provide a mandatory interrupt-controller and '#interrupt-cells'
properties. There might be optional registers memory range,
"mti,reserved-cpu-vectors" and "mti,reserved-ipi-vectors" properties
specified.

MIPS GIC also includes a free-running global timer, per-CPU count/compare
timers, and a watchdog. Since currently the GIC Timer is only supported the
DT schema expects an IRQ and clock-phandler charged timer sub-node with
"mti,mips-gic-timer" compatible string.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Rob Herring <robh@kernel.org>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org

---

I don't really know who is the corresponding driver maintainer, so I
added Paul to the maintainers property since he used to be looking for the
MIPS arch and Thomas looking after it now. Any idea what email should be
specified there instead?

Changelog v3:
- Since timer sub-node has no unit-address, the node shouldn't be named
  with one. So alter the MIPS GIC bindings to have a pure "timer"
  sub-node.
- Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ].
- Since it's a conversion patch use GPL-2.0-only SPDX header.
---
 .../interrupt-controller/mips-gic.txt         |  67 --------
 .../interrupt-controller/mti,gic.yaml         | 148 ++++++++++++++++++
 2 files changed, 148 insertions(+), 67 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt b/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
deleted file mode 100644
index 173595305e26..000000000000
--- a/Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-MIPS Global Interrupt Controller (GIC)
-
-The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
-It also supports local (per-processor) interrupts and software-generated
-interrupts which can be used as IPIs.  The GIC also includes a free-running
-global timer, per-CPU count/compare timers, and a watchdog.
-
-Required properties:
-- compatible : Should be "mti,gic".
-- interrupt-controller : Identifies the node as an interrupt controller
-- #interrupt-cells : Specifies the number of cells needed to encode an
-  interrupt specifier.  Should be 3.
-  - The first cell is the type of interrupt, local or shared.
-    See <include/dt-bindings/interrupt-controller/mips-gic.h>.
-  - The second cell is the GIC interrupt number.
-  - The third cell encodes the interrupt flags.
-    See <include/dt-bindings/interrupt-controller/irq.h> for a list of valid
-    flags.
-
-Optional properties:
-- reg : Base address and length of the GIC registers.  If not present,
-  the base address reported by the hardware GCR_GIC_BASE will be used.
-- mti,reserved-cpu-vectors : Specifies the list of CPU interrupt vectors
-  to which the GIC may not route interrupts.  Valid values are 2 - 7.
-  This property is ignored if the CPU is started in EIC mode.
-- mti,reserved-ipi-vectors : Specifies the range of GIC interrupts that are
-  reserved for IPIs.
-  It accepts 2 values, the 1st is the starting interrupt and the 2nd is the size
-  of the reserved range.
-  If not specified, the driver will allocate the last 2 * number of VPEs in the
-  system.
-
-Required properties for timer sub-node:
-- compatible : Should be "mti,gic-timer".
-- interrupts : Interrupt for the GIC local timer.
-
-Optional properties for timer sub-node:
-- clocks : GIC timer operating clock.
-- clock-frequency : Clock frequency at which the GIC timers operate.
-
-Note that one of clocks or clock-frequency must be specified.
-
-Example:
-
-	gic: interrupt-controller@1bdc0000 {
-		compatible = "mti,gic";
-		reg = <0x1bdc0000 0x20000>;
-
-		interrupt-controller;
-		#interrupt-cells = <3>;
-
-		mti,reserved-cpu-vectors = <7>;
-		mti,reserved-ipi-vectors = <40 8>;
-
-		timer {
-			compatible = "mti,gic-timer";
-			interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
-			clock-frequency = <50000000>;
-		};
-	};
-
-	uart@18101400 {
-		...
-		interrupt-parent = <&gic>;
-		interrupts = <GIC_SHARED 24 IRQ_TYPE_LEVEL_HIGH>;
-		...
-	};
diff --git a/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
new file mode 100644
index 000000000000..9f0eb3addac4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/mti,gic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MIPS Global Interrupt Controller
+
+maintainers:
+  - Paul Burton <paulburton@kernel.org>
+  - Thomas Bogendoerfer <tsbogend@alpha.franken.de>
+
+description: |
+  The MIPS GIC routes external interrupts to individual VPEs and IRQ pins.
+  It also supports local (per-processor) interrupts and software-generated
+  interrupts which can be used as IPIs. The GIC also includes a free-running
+  global timer, per-CPU count/compare timers, and a watchdog.
+
+properties:
+  compatible:
+    const: mti,gic
+
+  "#interrupt-cells":
+    const: 3
+    description: |
+      The 1st cell is the type of interrupt: local or shared defined in the
+      file 'dt-bindings/interrupt-controller/mips-gic.h'. The 2nd cell is the
+      GIC interrupt number. The 3d cell encodes the interrupt flags setting up
+      the IRQ trigger modes, which are defined in the file
+      'dt-bindings/interrupt-controller/irq.h'.
+
+  reg:
+    description: |
+      Base address and length of the GIC registers space. If not present,
+      the base address reported by the hardware GCR_GIC_BASE will be used.
+    maxItems: 1
+
+  interrupt-controller: true
+
+  mti,reserved-cpu-vectors:
+    description: |
+      Specifies the list of CPU interrupt vectors to which the GIC may not
+      route interrupts. This property is ignored if the CPU is started in EIC
+      mode.
+    allOf:
+      - $ref: /schemas/types.yaml#definitions/uint32-array
+      - minItems: 1
+        maxItems: 6
+        uniqueItems: true
+        items:
+          minimum: 2
+          maximum: 7
+
+  mti,reserved-ipi-vectors:
+    description: |
+      Specifies the range of GIC interrupts that are reserved for IPIs.
+      It accepts two values: the 1st is the starting interrupt and the 2nd is
+      the size of the reserved range. If not specified, the driver will
+      allocate the last (2 * number of VPEs in the system).
+    allOf:
+      - $ref: /schemas/types.yaml#definitions/uint32-array
+      - items:
+          - minimum: 0
+            maximum: 254
+          - minimum: 2
+            maximum: 254
+
+  timer:
+    type: object
+    description: |
+      MIPS GIC includes a free-running global timer, per-CPU count/compare
+      timers, and a watchdog. Currently only the GIC Timer is supported.
+    properties:
+      compatible:
+        const: mti,gic-timer
+
+      interrupts:
+        description: |
+          Interrupt for the GIC local timer, so normally it's suppose to be of
+          <GIC_LOCAL X IRQ_TYPE_NONE> format.
+        maxItems: 1
+
+      clocks:
+        maxItems: 1
+
+      clock-frequency: true
+
+    required:
+      - compatible
+      - interrupts
+
+    oneOf:
+      - required:
+          - clocks
+      - required:
+          - clock-frequency
+
+    additionalProperties: false
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - "#interrupt-cells"
+  - interrupt-controller
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    interrupt-controller@1bdc0000 {
+      compatible = "mti,gic";
+      reg = <0x1bdc0000 0x20000>;
+      interrupt-controller;
+      #interrupt-cells = <3>;
+      mti,reserved-cpu-vectors = <7>;
+      mti,reserved-ipi-vectors = <40 8>;
+
+      timer {
+        compatible = "mti,gic-timer";
+        interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+        clock-frequency = <50000000>;
+      };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/mips-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    interrupt-controller@1bdc0000 {
+      compatible = "mti,gic";
+      reg = <0x1bdc0000 0x20000>;
+      interrupt-controller;
+      #interrupt-cells = <3>;
+
+      timer {
+        compatible = "mti,gic-timer";
+        interrupts = <GIC_LOCAL 1 IRQ_TYPE_NONE>;
+        clocks = <&cpu_pll>;
+      };
+    };
+  - |
+    interrupt-controller {
+      compatible = "mti,gic";
+      interrupt-controller;
+      #interrupt-cells = <3>;
+    };
+...
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
                   ` (2 preceding siblings ...)
  2020-05-21  0:53 ` [PATCH v4 3/7] dt-bindings: interrupt-controller: Convert mti,gic to DT schema Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21 15:04   ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 5/7] clocksource: dw_apb_timer_of: Fix missing clockevent timers Serge Semin
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
	Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
	Rob Herring, linux-mips, linux-rtc, devicetree, Allison Randal,
	afzal mohammed, Kate Stewart, Greg Kroah-Hartman, Enrico Weigelt,
	Alexios Zavras, linux-kernel

Currently the DW APB Timer driver binds all clockevent timers to
CPU #0. This isn't good for multiple reasons. First of all seeing
the device is placed on APB bus (which makes it accessible from any
CPU core), accessible over MMIO and having the DYNIRQ flag set we
can be sure that manually binding the timer to any CPU just isn't
correct. By doing so we just set an extra limitation on device usage.
This also doesn't reflect the device actual capability, since by
setting the IRQ affinity we can make it virtually local to any CPU.
Secondly imagine if you had a real CPU-local timer with the same
rating and the same CPU-affinity. In this case if DW APB timer was
registered first, then due to the clockevent framework tick-timer
selection procedure we'll end up with the real CPU-local timer being
left unselected for clock-events tracking. But on most of the platforms
(MIPS/ARM/etc) such timers are normally embedded into the CPU core and
are accessible with much better performance then devices placed on APB.
For instance in MIPS architectures there is r4k-timer, which is
CPU-local, assigned with the same rating, and normally its
clockevent device is registered after the platform-specific one.

So in order to fix all of these issues lets set the DW APB clockevent
timer cpumask to be 'cpu_possible_mask'. By doing so the clockevent
framework would prefer to select the real CPU-local timer instead
of DW APB one. Otherwise if there is no other than DW APB device for
clockevents tracking then it will be selected.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/clocksource/dw_apb_timer.c    | 18 +++++++-----------
 drivers/clocksource/dw_apb_timer_of.c |  3 +--
 include/linux/dw_apb_timer.h          |  2 +-
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
index b207a77b0831..8ebb43916423 100644
--- a/drivers/clocksource/dw_apb_timer.c
+++ b/drivers/clocksource/dw_apb_timer.c
@@ -106,6 +106,7 @@ static irqreturn_t dw_apb_clockevent_irq(int irq, void *data)
 		dw_ced->eoi(&dw_ced->timer);
 
 	evt->event_handler(evt);
+
 	return IRQ_HANDLED;
 }
 
@@ -123,8 +124,7 @@ static int apbt_shutdown(struct clock_event_device *evt)
 	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
 	u32 ctrl;
 
-	pr_debug("%s CPU %d state=shutdown\n", __func__,
-		 cpumask_first(evt->cpumask));
+	pr_debug("%s state=shutdown\n", __func__);
 
 	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
 	ctrl &= ~APBTMR_CONTROL_ENABLE;
@@ -137,8 +137,7 @@ static int apbt_set_oneshot(struct clock_event_device *evt)
 	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
 	u32 ctrl;
 
-	pr_debug("%s CPU %d state=oneshot\n", __func__,
-		 cpumask_first(evt->cpumask));
+	pr_debug("%s state=oneshot\n", __func__);
 
 	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
 	/*
@@ -170,8 +169,7 @@ static int apbt_set_periodic(struct clock_event_device *evt)
 	unsigned long period = DIV_ROUND_UP(dw_ced->timer.freq, HZ);
 	u32 ctrl;
 
-	pr_debug("%s CPU %d state=periodic\n", __func__,
-		 cpumask_first(evt->cpumask));
+	pr_debug("%s state=periodic\n", __func__);
 
 	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
 	ctrl |= APBTMR_CONTROL_MODE_PERIODIC;
@@ -194,8 +192,7 @@ static int apbt_resume(struct clock_event_device *evt)
 {
 	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
 
-	pr_debug("%s CPU %d state=resume\n", __func__,
-		 cpumask_first(evt->cpumask));
+	pr_debug("%s state=resume\n", __func__);
 
 	apbt_enable_int(&dw_ced->timer);
 	return 0;
@@ -222,7 +219,6 @@ static int apbt_next_event(unsigned long delta,
 /**
  * dw_apb_clockevent_init() - use an APB timer as a clock_event_device
  *
- * @cpu:	The CPU the events will be targeted at.
  * @name:	The name used for the timer and the IRQ for it.
  * @rating:	The rating to give the timer.
  * @base:	I/O base for the timer registers.
@@ -237,7 +233,7 @@ static int apbt_next_event(unsigned long delta,
  * releasing the IRQ.
  */
 struct dw_apb_clock_event_device *
-dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
+dw_apb_clockevent_init(const char *name, unsigned int rating,
 		       void __iomem *base, int irq, unsigned long freq)
 {
 	struct dw_apb_clock_event_device *dw_ced =
@@ -257,7 +253,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
 	dw_ced->ced.max_delta_ticks = 0x7fffffff;
 	dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced);
 	dw_ced->ced.min_delta_ticks = 5000;
-	dw_ced->ced.cpumask = cpumask_of(cpu);
+	dw_ced->ced.cpumask = cpu_possible_mask;
 	dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
 				CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
 	dw_ced->ced.set_state_shutdown = apbt_shutdown;
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 8c28b127759f..0a2505b323d7 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -73,8 +73,7 @@ static void __init add_clockevent(struct device_node *event_timer)
 
 	timer_get_base_and_rate(event_timer, &iobase, &rate);
 
-	ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
-				     rate);
+	ced = dw_apb_clockevent_init(event_timer->name, 300, iobase, irq, rate);
 	if (!ced)
 		panic("Unable to initialise clockevent device");
 
diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h
index 82ebf9223948..689022bc8d17 100644
--- a/include/linux/dw_apb_timer.h
+++ b/include/linux/dw_apb_timer.h
@@ -39,7 +39,7 @@ void dw_apb_clockevent_resume(struct dw_apb_clock_event_device *dw_ced);
 void dw_apb_clockevent_stop(struct dw_apb_clock_event_device *dw_ced);
 
 struct dw_apb_clock_event_device *
-dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
+dw_apb_clockevent_init(const char *name, unsigned int rating,
 		       void __iomem *base, int irq, unsigned long freq);
 struct dw_apb_clocksource *
 dw_apb_clocksource_init(unsigned rating, const char *name, void __iomem *base,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 5/7] clocksource: dw_apb_timer_of: Fix missing clockevent timers
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
                   ` (3 preceding siblings ...)
  2020-05-21  0:53 ` [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 6/7] clocksource: mips-gic-timer: Register as sched_clock Serge Semin
  2020-05-21  0:53 ` [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes Serge Semin
  6 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano, Jamie Iles,
	Rob Herring, Dinh Nguyen, Heiko Stuebner, Arnd Bergmann
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
	Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Rob Herring,
	linux-mips, linux-rtc, devicetree, Allison Randal,
	Greg Kroah-Hartman, Alexios Zavras, linux-kernel

Commit 100214889973 ("clocksource: dw_apb_timer_of: use
clocksource_of_init") replaced a publicly available driver
initialization method with one called by the timer_probe() method
available after CLKSRC_OF. In current implementation it traverses
all the timers available in the system and calls their initialization
methods if corresponding devices were either in dtb or in acpi. But
if before the commit any number of available timers would be installed
as clockevent and clocksource devices, after that there would be at most
two. The rest are just ignored since default case branch doesn't do
anything. I don't see a reason of such behaviour, neither the commit
message explains it. Moreover this might be wrong if on some platforms
these timers might be used for different purpose, as virtually CPU-local
clockevent timers and as an independent broadcast timer. So in order
to keep the compatibility with the platforms where the order of the
timers detection has some meaning, lets add the secondly discovered
timer to be of clocksource/sched_clock type, while the very first and
the others would provide the clockevents service.

Fixes: 100214889973 ("clocksource: dw_apb_timer_of: use clocksource_of_init")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 drivers/clocksource/dw_apb_timer_of.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 0a2505b323d7..0b61b90a525e 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -146,10 +146,6 @@ static int num_called;
 static int __init dw_apb_timer_init(struct device_node *timer)
 {
 	switch (num_called) {
-	case 0:
-		pr_debug("%s: found clockevent timer\n", __func__);
-		add_clockevent(timer);
-		break;
 	case 1:
 		pr_debug("%s: found clocksource timer\n", __func__);
 		add_clocksource(timer);
@@ -160,6 +156,8 @@ static int __init dw_apb_timer_init(struct device_node *timer)
 #endif
 		break;
 	default:
+		pr_debug("%s: found clockevent timer\n", __func__);
+		add_clockevent(timer);
 		break;
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 6/7] clocksource: mips-gic-timer: Register as sched_clock
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
                   ` (4 preceding siblings ...)
  2020-05-21  0:53 ` [PATCH v4 5/7] clocksource: dw_apb_timer_of: Fix missing clockevent timers Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  0:53 ` [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes Serge Semin
  6 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Serge Semin, Paul Burton, Alexey Malahov,
	Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
	Rob Herring, linux-mips, linux-rtc, devicetree,
	Vincenzo Frascino, linux-kernel

From: Paul Burton <paulburton@kernel.org>

The MIPS GIC timer is well suited for use as sched_clock, so register it
as such.

Whilst the existing gic_read_count() function matches the prototype
needed by sched_clock_register() already, we split it into 2 functions
in order to remove the need to evaluate the mips_cm_is64 condition
within each call since sched_clock should be as fast as possible.

Note the sched clock framework needs the clock source being stable in
order to rely on it. So we register the MIPS GIC timer as schedule clocks
only if it's, if either the system doesn't have CPU-frequency enabled or
the CPU frequency is changed by means of the CPC core clock divider
available on the platforms with CM3 or newer.

Signed-off-by: Paul Burton <paulburton@kernel.org>
Co-developed-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
[Sergey.Semin@baikalelectronics.ru: Register sched-clock if CM3 or !CPU-freq]
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Register sched clocks only if MIPS GIC belongs to CM3 or if CPU-freq
  isn't supported.
---
 drivers/clocksource/mips-gic-timer.c | 31 ++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 8b5f8ae723cb..ef12c12c2432 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -16,6 +16,7 @@
 #include <linux/notifier.h>
 #include <linux/of_irq.h>
 #include <linux/percpu.h>
+#include <linux/sched_clock.h>
 #include <linux/smp.h>
 #include <linux/time.h>
 #include <asm/mips-cps.h>
@@ -24,13 +25,10 @@ static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
 static int gic_timer_irq;
 static unsigned int gic_frequency;
 
-static u64 notrace gic_read_count(void)
+static u64 notrace gic_read_count_2x32(void)
 {
 	unsigned int hi, hi2, lo;
 
-	if (mips_cm_is64)
-		return read_gic_counter();
-
 	do {
 		hi = read_gic_counter_32h();
 		lo = read_gic_counter_32l();
@@ -40,6 +38,19 @@ static u64 notrace gic_read_count(void)
 	return (((u64) hi) << 32) + lo;
 }
 
+static u64 notrace gic_read_count_64(void)
+{
+	return read_gic_counter();
+}
+
+static u64 notrace gic_read_count(void)
+{
+	if (mips_cm_is64)
+		return gic_read_count_64();
+
+	return gic_read_count_2x32();
+}
+
 static int gic_next_event(unsigned long delta, struct clock_event_device *evt)
 {
 	int cpu = cpumask_first(evt->cpumask);
@@ -228,6 +239,18 @@ static int __init gic_clocksource_of_init(struct device_node *node)
 	/* And finally start the counter */
 	clear_gic_config(GIC_CONFIG_COUNTSTOP);
 
+	/*
+	 * It's safe to use the MIPS GIC timer as a sched clock source only if
+	 * its ticks are stable, which is true on either the platforms with
+	 * stable CPU frequency or on the platforms with CM3 and CPU frequency
+	 * change performed by the CPC core clocks divider.
+	 */
+	if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
+		sched_clock_register(mips_cm_is64 ?
+				     gic_read_count_64 : gic_read_count_2x32,
+				     64, gic_frequency);
+	}
+
 	return 0;
 }
 TIMER_OF_DECLARE(mips_gic_timer, "mti,gic-timer",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
  2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
                   ` (5 preceding siblings ...)
  2020-05-21  0:53 ` [PATCH v4 6/7] clocksource: mips-gic-timer: Register as sched_clock Serge Semin
@ 2020-05-21  0:53 ` Serge Semin
  2020-05-21  9:09   ` Geert Uytterhoeven
  6 siblings, 1 reply; 11+ messages in thread
From: Serge Semin @ 2020-05-21  0:53 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Serge Semin, Alexey Malahov, Paul Burton,
	Ralf Baechle, Alessandro Zummo, Alexandre Belloni, Arnd Bergmann,
	Rob Herring, linux-mips, linux-rtc, devicetree, Paul Cercueil,
	Krzysztof Kozlowski, Geert Uytterhoeven, Randy Dunlap,
	Claudiu Beznea, Maarten ter Huurne, Vincenzo Frascino,
	linux-kernel

Currently clocksource framework doesn't support the clocks with variable
frequency. Since MIPS GIC timer ticks rate might be unstable on some
platforms, we must make sure that it justifies the clocksource
requirements. MIPS GIC timer is incremented with the CPU cluster reference
clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
changes synchronously. Due to this the clocksource subsystem can't rely on
the timer to measure system clocks anymore. This commit marks the MIPS GIC
based clocksource as unstable if reference clock (normally it's a CPU
reference clocks) rate changes. The clocksource will execute a watchdog
thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
new stable one.

Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
GIC clocksource since normally the timer is stable. The only reason why
it gets unstable is due to the ref clock rate change, which event we
detect here in the driver by means of the clocks event notifier.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Paul Burton <paulburton@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-mips@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: devicetree@vger.kernel.org

---

Changelog v3:
- Mark clocksource as unstable instead of lowering its rating.
---
 drivers/clocksource/Kconfig          |  1 +
 drivers/clocksource/mips-gic-timer.c | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index f2142e6bbea3..37a745f3ca91 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -572,6 +572,7 @@ config CLKSRC_VERSATILE
 config CLKSRC_MIPS_GIC
 	bool
 	depends on MIPS_GIC
+	select CLOCKSOURCE_WATCHDOG
 	select TIMER_OF
 
 config CLKSRC_TANGO_XTAL
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index ef12c12c2432..d78ad7b60cdc 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -24,6 +24,9 @@
 static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
 static int gic_timer_irq;
 static unsigned int gic_frequency;
+static bool __read_mostly gic_clock_unstable;
+
+static void git_clocksource_unstable(char *reason);
 
 static u64 notrace gic_read_count_2x32(void)
 {
@@ -125,8 +128,10 @@ static int gic_clk_notifier(struct notifier_block *nb, unsigned long action,
 {
 	struct clk_notifier_data *cnd = data;
 
-	if (action == POST_RATE_CHANGE)
+	if (action == POST_RATE_CHANGE) {
+		git_clocksource_unstable("ref clock rate change");
 		on_each_cpu(gic_update_frequency, (void *)cnd->new_rate, 1);
+	}
 
 	return NOTIFY_OK;
 }
@@ -172,6 +177,18 @@ static struct clocksource gic_clocksource = {
 	.vdso_clock_mode	= VDSO_CLOCKMODE_GIC,
 };
 
+static void git_clocksource_unstable(char *reason)
+{
+	if (gic_clock_unstable)
+		return;
+
+	gic_clock_unstable = true;
+
+	pr_info("GIC timer is unstable due to %s\n", reason);
+
+	clocksource_mark_unstable(&gic_clocksource);
+}
+
 static int __init __gic_clocksource_init(void)
 {
 	unsigned int count_width;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
  2020-05-21  0:53 ` [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes Serge Semin
@ 2020-05-21  9:09   ` Geert Uytterhoeven
  2020-05-21 14:22     ` Serge Semin
  0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2020-05-21  9:09 UTC (permalink / raw)
  To: Serge Semin
  Cc: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano,
	Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
	Alessandro Zummo, Alexandre Belloni, Arnd Bergmann, Rob Herring,
	open list:BROADCOM NVRAM DRIVER, linux-rtc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Paul Cercueil, Krzysztof Kozlowski, Geert Uytterhoeven,
	Randy Dunlap, Claudiu Beznea, Maarten ter Huurne,
	Vincenzo Frascino, Linux Kernel Mailing List

Hi Serge,

On Thu, May 21, 2020 at 2:54 AM Serge Semin
<Sergey.Semin@baikalelectronics.ru> wrote:
> Currently clocksource framework doesn't support the clocks with variable
> frequency. Since MIPS GIC timer ticks rate might be unstable on some
> platforms, we must make sure that it justifies the clocksource
> requirements. MIPS GIC timer is incremented with the CPU cluster reference
> clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
> changes synchronously. Due to this the clocksource subsystem can't rely on
> the timer to measure system clocks anymore. This commit marks the MIPS GIC
> based clocksource as unstable if reference clock (normally it's a CPU
> reference clocks) rate changes. The clocksource will execute a watchdog
> thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
> new stable one.
>
> Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
> GIC clocksource since normally the timer is stable. The only reason why
> it gets unstable is due to the ref clock rate change, which event we
> detect here in the driver by means of the clocks event notifier.
>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>

Thanks for your patch!

> --- a/drivers/clocksource/mips-gic-timer.c
> +++ b/drivers/clocksource/mips-gic-timer.c
> @@ -24,6 +24,9 @@
>  static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
>  static int gic_timer_irq;
>  static unsigned int gic_frequency;
> +static bool __read_mostly gic_clock_unstable;
> +
> +static void git_clocksource_unstable(char *reason);

gic_clocksource_unstable? (everywhere)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes
  2020-05-21  9:09   ` Geert Uytterhoeven
@ 2020-05-21 14:22     ` Serge Semin
  0 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21 14:22 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Serge Semin, Thomas Gleixner, Thomas Bogendoerfer,
	Daniel Lezcano, Alexey Malahov, Paul Burton, Ralf Baechle,
	Alessandro Zummo, Alexandre Belloni, Arnd Bergmann, Rob Herring,
	open list:BROADCOM NVRAM DRIVER, linux-rtc,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Paul Cercueil, Krzysztof Kozlowski, Geert Uytterhoeven,
	Randy Dunlap, Claudiu Beznea, Maarten ter Huurne,
	Vincenzo Frascino, Linux Kernel Mailing List

On Thu, May 21, 2020 at 11:09:50AM +0200, Geert Uytterhoeven wrote:
> Hi Serge,
> 
> On Thu, May 21, 2020 at 2:54 AM Serge Semin
> <Sergey.Semin@baikalelectronics.ru> wrote:
> > Currently clocksource framework doesn't support the clocks with variable
> > frequency. Since MIPS GIC timer ticks rate might be unstable on some
> > platforms, we must make sure that it justifies the clocksource
> > requirements. MIPS GIC timer is incremented with the CPU cluster reference
> > clocks rate. So in case if CPU frequency changes, the MIPS GIC tick rate
> > changes synchronously. Due to this the clocksource subsystem can't rely on
> > the timer to measure system clocks anymore. This commit marks the MIPS GIC
> > based clocksource as unstable if reference clock (normally it's a CPU
> > reference clocks) rate changes. The clocksource will execute a watchdog
> > thread, which lowers the MIPS GIC timer rating to zero and fallbacks to a
> > new stable one.
> >
> > Note we don't need to set the CLOCK_SOURCE_MUST_VERIFY flag to the MIPS
> > GIC clocksource since normally the timer is stable. The only reason why
> > it gets unstable is due to the ref clock rate change, which event we
> > detect here in the driver by means of the clocks event notifier.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> 
> Thanks for your patch!
> 
> > --- a/drivers/clocksource/mips-gic-timer.c
> > +++ b/drivers/clocksource/mips-gic-timer.c
> > @@ -24,6 +24,9 @@
> >  static DEFINE_PER_CPU(struct clock_event_device, gic_clockevent_device);
> >  static int gic_timer_irq;
> >  static unsigned int gic_frequency;
> > +static bool __read_mostly gic_clock_unstable;
> > +
> > +static void git_clocksource_unstable(char *reason);
> 
> gic_clocksource_unstable? (everywhere)

This is the most used word lately. So my hands write git everywhere by itself.)
Thanks for noticing this.

-Sergey

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask
  2020-05-21  0:53 ` [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask Serge Semin
@ 2020-05-21 15:04   ` Serge Semin
  0 siblings, 0 replies; 11+ messages in thread
From: Serge Semin @ 2020-05-21 15:04 UTC (permalink / raw)
  To: Thomas Gleixner, Thomas Bogendoerfer, Daniel Lezcano
  Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
	Alessandro Zummo, Alexandre Belloni, Arnd Bergmann, Rob Herring,
	linux-mips, linux-rtc, devicetree, Allison Randal,
	afzal mohammed, Kate Stewart, Greg Kroah-Hartman, Enrico Weigelt,
	Alexios Zavras, linux-kernel

Daniel,

On Thu, May 21, 2020 at 03:53:17AM +0300, Serge Semin wrote:
> Currently the DW APB Timer driver binds all clockevent timers to
> CPU #0. This isn't good for multiple reasons. First of all seeing
> the device is placed on APB bus (which makes it accessible from any
> CPU core), accessible over MMIO and having the DYNIRQ flag set we
> can be sure that manually binding the timer to any CPU just isn't
> correct. By doing so we just set an extra limitation on device usage.
> This also doesn't reflect the device actual capability, since by
> setting the IRQ affinity we can make it virtually local to any CPU.
> Secondly imagine if you had a real CPU-local timer with the same
> rating and the same CPU-affinity. In this case if DW APB timer was
> registered first, then due to the clockevent framework tick-timer
> selection procedure we'll end up with the real CPU-local timer being
> left unselected for clock-events tracking. But on most of the platforms
> (MIPS/ARM/etc) such timers are normally embedded into the CPU core and
> are accessible with much better performance then devices placed on APB.
> For instance in MIPS architectures there is r4k-timer, which is
> CPU-local, assigned with the same rating, and normally its
> clockevent device is registered after the platform-specific one.
> 
> So in order to fix all of these issues lets set the DW APB clockevent
> timer cpumask to be 'cpu_possible_mask'. By doing so the clockevent
> framework would prefer to select the real CPU-local timer instead
> of DW APB one. Otherwise if there is no other than DW APB device for
> clockevents tracking then it will be selected.

Hm, seems to me it has been a mistake to remove the CPU affiliation for the
timers since x86 has got a specific use-case with assigning the timers to all
detected CPUs. Though for our needs we don't need this feature. I'll make the
affiliation optional then. Like this:

> dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
>                      void __iomem *base, int irq, unsigned long freq)
> {
> ...
>       dw_ced->ced.cpumask = cpu < 0 ? cpu_possible_mask : cpumask_of(cpu);
> ...
> }

DW APB Timer OF-based devices won't make the timer affiliated to the CPU #0, but
to any possible CPU by passing -1 to the dw_apb_clockevent_init():
--- drivers/clocksource/dw_apb_timer_of.c
+++ drivers/clocksource/dw_apb_timer_of.c
-       ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
                                     rate);
+       ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq,
                                     rate);

What do you think?

-Sergey

> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Paul Burton <paulburton@kernel.org>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: linux-rtc@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  drivers/clocksource/dw_apb_timer.c    | 18 +++++++-----------
>  drivers/clocksource/dw_apb_timer_of.c |  3 +--
>  include/linux/dw_apb_timer.h          |  2 +-
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
> index b207a77b0831..8ebb43916423 100644
> --- a/drivers/clocksource/dw_apb_timer.c
> +++ b/drivers/clocksource/dw_apb_timer.c
> @@ -106,6 +106,7 @@ static irqreturn_t dw_apb_clockevent_irq(int irq, void *data)
>  		dw_ced->eoi(&dw_ced->timer);
>  
>  	evt->event_handler(evt);
> +
>  	return IRQ_HANDLED;
>  }
>  
> @@ -123,8 +124,7 @@ static int apbt_shutdown(struct clock_event_device *evt)
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=shutdown\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=shutdown\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	ctrl &= ~APBTMR_CONTROL_ENABLE;
> @@ -137,8 +137,7 @@ static int apbt_set_oneshot(struct clock_event_device *evt)
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=oneshot\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=oneshot\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	/*
> @@ -170,8 +169,7 @@ static int apbt_set_periodic(struct clock_event_device *evt)
>  	unsigned long period = DIV_ROUND_UP(dw_ced->timer.freq, HZ);
>  	u32 ctrl;
>  
> -	pr_debug("%s CPU %d state=periodic\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=periodic\n", __func__);
>  
>  	ctrl = apbt_readl(&dw_ced->timer, APBTMR_N_CONTROL);
>  	ctrl |= APBTMR_CONTROL_MODE_PERIODIC;
> @@ -194,8 +192,7 @@ static int apbt_resume(struct clock_event_device *evt)
>  {
>  	struct dw_apb_clock_event_device *dw_ced = ced_to_dw_apb_ced(evt);
>  
> -	pr_debug("%s CPU %d state=resume\n", __func__,
> -		 cpumask_first(evt->cpumask));
> +	pr_debug("%s state=resume\n", __func__);
>  
>  	apbt_enable_int(&dw_ced->timer);
>  	return 0;
> @@ -222,7 +219,6 @@ static int apbt_next_event(unsigned long delta,
>  /**
>   * dw_apb_clockevent_init() - use an APB timer as a clock_event_device
>   *
> - * @cpu:	The CPU the events will be targeted at.
>   * @name:	The name used for the timer and the IRQ for it.
>   * @rating:	The rating to give the timer.
>   * @base:	I/O base for the timer registers.
> @@ -237,7 +233,7 @@ static int apbt_next_event(unsigned long delta,
>   * releasing the IRQ.
>   */
>  struct dw_apb_clock_event_device *
> -dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
> +dw_apb_clockevent_init(const char *name, unsigned int rating,
>  		       void __iomem *base, int irq, unsigned long freq)
>  {
>  	struct dw_apb_clock_event_device *dw_ced =
> @@ -257,7 +253,7 @@ dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
>  	dw_ced->ced.max_delta_ticks = 0x7fffffff;
>  	dw_ced->ced.min_delta_ns = clockevent_delta2ns(5000, &dw_ced->ced);
>  	dw_ced->ced.min_delta_ticks = 5000;
> -	dw_ced->ced.cpumask = cpumask_of(cpu);
> +	dw_ced->ced.cpumask = cpu_possible_mask;
>  	dw_ced->ced.features = CLOCK_EVT_FEAT_PERIODIC |
>  				CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_DYNIRQ;
>  	dw_ced->ced.set_state_shutdown = apbt_shutdown;
> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index 8c28b127759f..0a2505b323d7 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -73,8 +73,7 @@ static void __init add_clockevent(struct device_node *event_timer)
>  
>  	timer_get_base_and_rate(event_timer, &iobase, &rate);
>  
> -	ced = dw_apb_clockevent_init(0, event_timer->name, 300, iobase, irq,
> -				     rate);
> +	ced = dw_apb_clockevent_init(event_timer->name, 300, iobase, irq, rate);
>  	if (!ced)
>  		panic("Unable to initialise clockevent device");
>  
> diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h
> index 82ebf9223948..689022bc8d17 100644
> --- a/include/linux/dw_apb_timer.h
> +++ b/include/linux/dw_apb_timer.h
> @@ -39,7 +39,7 @@ void dw_apb_clockevent_resume(struct dw_apb_clock_event_device *dw_ced);
>  void dw_apb_clockevent_stop(struct dw_apb_clock_event_device *dw_ced);
>  
>  struct dw_apb_clock_event_device *
> -dw_apb_clockevent_init(int cpu, const char *name, unsigned rating,
> +dw_apb_clockevent_init(const char *name, unsigned int rating,
>  		       void __iomem *base, int irq, unsigned long freq);
>  struct dw_apb_clocksource *
>  dw_apb_clocksource_init(unsigned rating, const char *name, void __iomem *base,
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-05-21 15:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21  0:53 [PATCH v4 0/7] clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support Serge Semin
2020-05-21  0:53 ` [PATCH v4 1/7] dt-bindings: rtc: Convert snps,dw-apb-timer to DT schema Serge Semin
2020-05-21  0:53 ` [PATCH v4 2/7] dt-bindings: timer: Move snps,dw-apb-timer DT schema from rtc Serge Semin
2020-05-21  0:53 ` [PATCH v4 3/7] dt-bindings: interrupt-controller: Convert mti,gic to DT schema Serge Semin
2020-05-21  0:53 ` [PATCH v4 4/7] clocksource: dw_apb_timer: Set clockevent any-possible-CPU mask Serge Semin
2020-05-21 15:04   ` Serge Semin
2020-05-21  0:53 ` [PATCH v4 5/7] clocksource: dw_apb_timer_of: Fix missing clockevent timers Serge Semin
2020-05-21  0:53 ` [PATCH v4 6/7] clocksource: mips-gic-timer: Register as sched_clock Serge Semin
2020-05-21  0:53 ` [PATCH v4 7/7] clocksource: mips-gic-timer: Mark GIC timer as unstable if ref clock changes Serge Semin
2020-05-21  9:09   ` Geert Uytterhoeven
2020-05-21 14:22     ` Serge Semin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).