[PATCH 0/7] Nuvoton WPCM450 clock and reset driver

[PATCH 0/7] Nuvoton WPCM450 clock and reset driver

[Jonathan Neuschäfer]

This series adds support for the clock and reset controller in the Nuvoton
WPCM450 SoC. This means that the clock rates for peripherals will be calculated
automatically based on the clock tree as it was preconfigured by the bootloader.
The 24 MHz dummy clock, that is currently in the devicetree, is no longer needed.
Somewhat unfortunately, this also means that there is a breaking change once
the devicetree starts relying on the clock driver, but I find it acceptable in
this case, because WPCM450 is still at a somewhat early stage.


Upstreaming plan (although other suggestions are welcome):

Once reviewed,

- The ARM/dts changes should go through Joel Stanley's bmc tree
- The clocksource/timer changes should probably go via Daniel Lezcano and TIP
- The watchdog patch should go via the watchdog tree
- The clock controller bindings and driver should go through the clk tree
- It might make sense to delay the final ARM/dts patch ("ARM: dts: wpcm450:
  Switch clocks to clock controller") until next cycle to make sure it is
  merged after the clock driver.

Jonathan Neuschäfer (7):
  dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks
  clocksource: timer-npcm7xx: Enable timer 1 clock before use
  watchdog: npcm: Enable clock if provided
  dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller
  ARM: dts: wpcm450: Add clock controller node
  clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver
  ARM: dts: wpcm450: Switch clocks to clock controller

 .../bindings/clock/nuvoton,wpcm450-clk.yaml   |  74 ++++
 .../bindings/timer/nuvoton,npcm7xx-timer.yaml |   8 +-
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi        |  29 +-
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-wpcm450.c                     | 378 ++++++++++++++++++
 drivers/clocksource/timer-npcm7xx.c           |  14 +-
 drivers/reset/Kconfig                         |   2 +-
 drivers/watchdog/npcm_wdt.c                   |   9 +
 .../dt-bindings/clock/nuvoton,wpcm450-clk.h   |  67 ++++
 9 files changed, 572 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
 create mode 100644 drivers/clk/clk-wpcm450.c
 create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h

--
2.35.1

[PATCH 0/7] Nuvoton WPCM450 clock and reset driver

[Jonathan Neuschäfer]

This series adds support for the clock and reset controller in the Nuvoton
WPCM450 SoC. This means that the clock rates for peripherals will be calculated
automatically based on the clock tree as it was preconfigured by the bootloader.
The 24 MHz dummy clock, that is currently in the devicetree, is no longer needed.
Somewhat unfortunately, this also means that there is a breaking change once
the devicetree starts relying on the clock driver, but I find it acceptable in
this case, because WPCM450 is still at a somewhat early stage.


Upstreaming plan (although other suggestions are welcome):

Once reviewed,

- The ARM/dts changes should go through Joel Stanley's bmc tree
- The clocksource/timer changes should probably go via Daniel Lezcano and TIP
- The watchdog patch should go via the watchdog tree
- The clock controller bindings and driver should go through the clk tree
- It might make sense to delay the final ARM/dts patch ("ARM: dts: wpcm450:
  Switch clocks to clock controller") until next cycle to make sure it is
  merged after the clock driver.

Jonathan Neuschäfer (7):
  dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks
  clocksource: timer-npcm7xx: Enable timer 1 clock before use
  watchdog: npcm: Enable clock if provided
  dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller
  ARM: dts: wpcm450: Add clock controller node
  clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver
  ARM: dts: wpcm450: Switch clocks to clock controller

 .../bindings/clock/nuvoton,wpcm450-clk.yaml   |  74 ++++
 .../bindings/timer/nuvoton,npcm7xx-timer.yaml |   8 +-
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi        |  29 +-
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-wpcm450.c                     | 378 ++++++++++++++++++
 drivers/clocksource/timer-npcm7xx.c           |  14 +-
 drivers/reset/Kconfig                         |   2 +-
 drivers/watchdog/npcm_wdt.c                   |   9 +
 .../dt-bindings/clock/nuvoton,wpcm450-clk.h   |  67 ++++
 9 files changed, 572 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
 create mode 100644 drivers/clk/clk-wpcm450.c
 create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h

--
2.35.1

[PATCH 1/7] dt-bindings: timer: nuvoton, npcm7xx-timer: Allow specifying all clocks

[Jonathan Neuschäfer]

The timer module contains multiple timers. In the WPCM450 SoC, each timer
runs off a clock can be gated individually. To model this correctly, the
timer node in the devicetree needs to take multiple clock inputs.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 .../devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml  | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
index 0cbc26a721514..023c999113c38 100644
--- a/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
@@ -23,7 +23,13 @@ properties:
       - description: The timer interrupt of timer 0

   clocks:
-    maxItems: 1
+    items:
+      - description: The reference clock for timer 0
+      - description: The reference clock for timer 1
+      - description: The reference clock for timer 2
+      - description: The reference clock for timer 3
+      - description: The reference clock for timer 4
+    minItems: 1

 required:
   - compatible
--
2.35.1

[PATCH 1/7] dt-bindings: timer: nuvoton,npcm7xx-timer: Allow specifying all clocks

[Jonathan Neuschäfer]

The timer module contains multiple timers. In the WPCM450 SoC, each timer
runs off a clock can be gated individually. To model this correctly, the
timer node in the devicetree needs to take multiple clock inputs.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 .../devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml  | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
index 0cbc26a721514..023c999113c38 100644
--- a/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
+++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.yaml
@@ -23,7 +23,13 @@ properties:
       - description: The timer interrupt of timer 0

   clocks:
-    maxItems: 1
+    items:
+      - description: The reference clock for timer 0
+      - description: The reference clock for timer 1
+      - description: The reference clock for timer 2
+      - description: The reference clock for timer 3
+      - description: The reference clock for timer 4
+    minItems: 1

 required:
   - compatible
--
2.35.1

[PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Jonathan Neuschäfer]

In the WPCM450 SoC, the clocks for each timer can be gated individually.
To prevent the timer 1 clock from being gated, enable it explicitly.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
index a00520cbb660a..974269b6b0c36 100644
--- a/drivers/clocksource/timer-npcm7xx.c
+++ b/drivers/clocksource/timer-npcm7xx.c
@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)

 static int __init npcm7xx_timer_init(struct device_node *np)
 {
+	struct clk *clk;
 	int ret;

 	ret = timer_of_init(np, &npcm7xx_to);
-	if (ret)
+	if (ret) {
+		pr_warn("timer_of_init failed: %d\n", ret);
 		return ret;
+	}

 	/* Clock input is divided by PRESCALE + 1 before it is fed */
 	/* to the counter */
 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
 		(NPCM7XX_Tx_MIN_PRESCALE + 1);

+	/* Enable the clock for timer1, if it exists */
+	clk = of_clk_get(np, 1);
+	if (clk) {
+		if (!IS_ERR(clk))
+			clk_prepare_enable(clk);
+		else
+			pr_warn("Failed to get clock for timer1: %pe", clk);
+	}
+
 	npcm7xx_clocksource_init();
 	npcm7xx_clockevents_init();

--
2.35.1

[PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Jonathan Neuschäfer]

In the WPCM450 SoC, the clocks for each timer can be gated individually.
To prevent the timer 1 clock from being gated, enable it explicitly.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
index a00520cbb660a..974269b6b0c36 100644
--- a/drivers/clocksource/timer-npcm7xx.c
+++ b/drivers/clocksource/timer-npcm7xx.c
@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)

 static int __init npcm7xx_timer_init(struct device_node *np)
 {
+	struct clk *clk;
 	int ret;

 	ret = timer_of_init(np, &npcm7xx_to);
-	if (ret)
+	if (ret) {
+		pr_warn("timer_of_init failed: %d\n", ret);
 		return ret;
+	}

 	/* Clock input is divided by PRESCALE + 1 before it is fed */
 	/* to the counter */
 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
 		(NPCM7XX_Tx_MIN_PRESCALE + 1);

+	/* Enable the clock for timer1, if it exists */
+	clk = of_clk_get(np, 1);
+	if (clk) {
+		if (!IS_ERR(clk))
+			clk_prepare_enable(clk);
+		else
+			pr_warn("Failed to get clock for timer1: %pe", clk);
+	}
+
 	npcm7xx_clocksource_init();
 	npcm7xx_clockevents_init();

--
2.35.1

[PATCH 3/7] watchdog: npcm: Enable clock if provided

[Jonathan Neuschäfer]

On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
clocks are individually gated and ungated. Therefore, the watchdog
driver must be able to ungate the watchdog clock.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/watchdog/npcm_wdt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 28a24caa2627c..6d27f0e16188e 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -3,6 +3,7 @@
 // Copyright (c) 2018 IBM Corp.

 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct npcm_wdt *wdt;
+	struct clk *clk;
 	int irq;
 	int ret;

@@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(wdt->reg))
 		return PTR_ERR(wdt->reg);

+	clk = devm_clk_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	if (clk)
+		clk_prepare_enable(clk);
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
--
2.35.1

[PATCH 3/7] watchdog: npcm: Enable clock if provided

[Jonathan Neuschäfer]

On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
clocks are individually gated and ungated. Therefore, the watchdog
driver must be able to ungate the watchdog clock.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/watchdog/npcm_wdt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 28a24caa2627c..6d27f0e16188e 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -3,6 +3,7 @@
 // Copyright (c) 2018 IBM Corp.

 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
@@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct npcm_wdt *wdt;
+	struct clk *clk;
 	int irq;
 	int ret;

@@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(wdt->reg))
 		return PTR_ERR(wdt->reg);

+	clk = devm_clk_get_optional(&pdev->dev, NULL);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	if (clk)
+		clk_prepare_enable(clk);
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
--
2.35.1

[PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

The Nuvoton WPCM450 SoC has a combined clock and reset controller.
Add a devicetree binding for it, as well as definitions for the bit
numbers used by it.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
 .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
 2 files changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
 create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h

diff --git a/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
new file mode 100644
index 0000000000000..0fffa8a68dee4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/nuvoton,wpcm450-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton WPCM450 clock controller binding
+
+maintainers:
+  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+
+description:
+  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
+  supplies clocks and resets to the rest of the chip.
+
+properties:
+  compatible:
+    const: nuvoton,wpcm450-clk
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Reference clock oscillator (should be 48 MHz)
+
+  clock-names:
+    items:
+      - const: refclk
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    refclk: clock-48mhz {
+      /* 48 MHz reference oscillator */
+      compatible = "fixed-clock";
+      clock-output-names = "refclk";
+      clock-frequency = <48000000>;
+      #clock-cells = <0>;
+    };
+
+    clk: clock-controller@b0000200 {
+      reg = <0xb0000200 0x100>;
+      compatible = "nuvoton,wpcm450-clk";
+      clocks = <&refclk>;
+      clock-names = "refclk";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
+
+    serial@b8000000 {
+      compatible = "nuvoton,wpcm450-uart";
+      reg = <0xb8000000 0x20>;
+      reg-shift = <2>;
+      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&clk WPCM450_CLK_UART0>;
+    };
diff --git a/include/dt-bindings/clock/nuvoton,wpcm450-clk.h b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
new file mode 100644
index 0000000000000..86e1c895921b7
--- /dev/null
+++ b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
+#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
+
+/* Clocks based on CLKEN bits */
+#define WPCM450_CLK_FIU            0
+#define WPCM450_CLK_XBUS           1
+#define WPCM450_CLK_KCS            2
+#define WPCM450_CLK_SHM            4
+#define WPCM450_CLK_USB1           5
+#define WPCM450_CLK_EMC0           6
+#define WPCM450_CLK_EMC1           7
+#define WPCM450_CLK_USB0           8
+#define WPCM450_CLK_PECI           9
+#define WPCM450_CLK_AES           10
+#define WPCM450_CLK_UART0         11
+#define WPCM450_CLK_UART1         12
+#define WPCM450_CLK_SMB2          13
+#define WPCM450_CLK_SMB3          14
+#define WPCM450_CLK_SMB4          15
+#define WPCM450_CLK_SMB5          16
+#define WPCM450_CLK_HUART         17
+#define WPCM450_CLK_PWM           18
+#define WPCM450_CLK_TIMER0        19
+#define WPCM450_CLK_TIMER1        20
+#define WPCM450_CLK_TIMER2        21
+#define WPCM450_CLK_TIMER3        22
+#define WPCM450_CLK_TIMER4        23
+#define WPCM450_CLK_MFT0          24
+#define WPCM450_CLK_MFT1          25
+#define WPCM450_CLK_WDT           26
+#define WPCM450_CLK_ADC           27
+#define WPCM450_CLK_SDIO          28
+#define WPCM450_CLK_SSPI          29
+#define WPCM450_CLK_SMB0          30
+#define WPCM450_CLK_SMB1          31
+
+/* Other clocks */
+#define WPCM450_CLK_USBPHY        32
+
+#define WPCM450_NUM_CLKS          33
+
+/* Resets based on IPSRST bits */
+#define WPCM450_RESET_FIU          0
+#define WPCM450_RESET_EMC0         6
+#define WPCM450_RESET_EMC1         7
+#define WPCM450_RESET_USB0         8
+#define WPCM450_RESET_USB1         9
+#define WPCM450_RESET_AES_PECI    10
+#define WPCM450_RESET_UART        11
+#define WPCM450_RESET_MC          12
+#define WPCM450_RESET_SMB2        13
+#define WPCM450_RESET_SMB3        14
+#define WPCM450_RESET_SMB4        15
+#define WPCM450_RESET_SMB5        16
+#define WPCM450_RESET_PWM         18
+#define WPCM450_RESET_TIMER       19
+#define WPCM450_RESET_ADC         27
+#define WPCM450_RESET_SDIO        28
+#define WPCM450_RESET_SSPI        29
+#define WPCM450_RESET_SMB0        30
+#define WPCM450_RESET_SMB1        31
+
+#define WPCM450_NUM_RESETS        32
+
+#endif /* _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H */
--
2.35.1

[PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

The Nuvoton WPCM450 SoC has a combined clock and reset controller.
Add a devicetree binding for it, as well as definitions for the bit
numbers used by it.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
 .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
 2 files changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
 create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h

diff --git a/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
new file mode 100644
index 0000000000000..0fffa8a68dee4
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/nuvoton,wpcm450-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton WPCM450 clock controller binding
+
+maintainers:
+  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+
+description:
+  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
+  supplies clocks and resets to the rest of the chip.
+
+properties:
+  compatible:
+    const: nuvoton,wpcm450-clk
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Reference clock oscillator (should be 48 MHz)
+
+  clock-names:
+    items:
+      - const: refclk
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - '#clock-cells'
+
+examples:
+  - |
+    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    refclk: clock-48mhz {
+      /* 48 MHz reference oscillator */
+      compatible = "fixed-clock";
+      clock-output-names = "refclk";
+      clock-frequency = <48000000>;
+      #clock-cells = <0>;
+    };
+
+    clk: clock-controller@b0000200 {
+      reg = <0xb0000200 0x100>;
+      compatible = "nuvoton,wpcm450-clk";
+      clocks = <&refclk>;
+      clock-names = "refclk";
+      #clock-cells = <1>;
+      #reset-cells = <1>;
+    };
+
+    serial@b8000000 {
+      compatible = "nuvoton,wpcm450-uart";
+      reg = <0xb8000000 0x20>;
+      reg-shift = <2>;
+      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
+      clocks = <&clk WPCM450_CLK_UART0>;
+    };
diff --git a/include/dt-bindings/clock/nuvoton,wpcm450-clk.h b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
new file mode 100644
index 0000000000000..86e1c895921b7
--- /dev/null
+++ b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
+#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
+
+/* Clocks based on CLKEN bits */
+#define WPCM450_CLK_FIU            0
+#define WPCM450_CLK_XBUS           1
+#define WPCM450_CLK_KCS            2
+#define WPCM450_CLK_SHM            4
+#define WPCM450_CLK_USB1           5
+#define WPCM450_CLK_EMC0           6
+#define WPCM450_CLK_EMC1           7
+#define WPCM450_CLK_USB0           8
+#define WPCM450_CLK_PECI           9
+#define WPCM450_CLK_AES           10
+#define WPCM450_CLK_UART0         11
+#define WPCM450_CLK_UART1         12
+#define WPCM450_CLK_SMB2          13
+#define WPCM450_CLK_SMB3          14
+#define WPCM450_CLK_SMB4          15
+#define WPCM450_CLK_SMB5          16
+#define WPCM450_CLK_HUART         17
+#define WPCM450_CLK_PWM           18
+#define WPCM450_CLK_TIMER0        19
+#define WPCM450_CLK_TIMER1        20
+#define WPCM450_CLK_TIMER2        21
+#define WPCM450_CLK_TIMER3        22
+#define WPCM450_CLK_TIMER4        23
+#define WPCM450_CLK_MFT0          24
+#define WPCM450_CLK_MFT1          25
+#define WPCM450_CLK_WDT           26
+#define WPCM450_CLK_ADC           27
+#define WPCM450_CLK_SDIO          28
+#define WPCM450_CLK_SSPI          29
+#define WPCM450_CLK_SMB0          30
+#define WPCM450_CLK_SMB1          31
+
+/* Other clocks */
+#define WPCM450_CLK_USBPHY        32
+
+#define WPCM450_NUM_CLKS          33
+
+/* Resets based on IPSRST bits */
+#define WPCM450_RESET_FIU          0
+#define WPCM450_RESET_EMC0         6
+#define WPCM450_RESET_EMC1         7
+#define WPCM450_RESET_USB0         8
+#define WPCM450_RESET_USB1         9
+#define WPCM450_RESET_AES_PECI    10
+#define WPCM450_RESET_UART        11
+#define WPCM450_RESET_MC          12
+#define WPCM450_RESET_SMB2        13
+#define WPCM450_RESET_SMB3        14
+#define WPCM450_RESET_SMB4        15
+#define WPCM450_RESET_SMB5        16
+#define WPCM450_RESET_PWM         18
+#define WPCM450_RESET_TIMER       19
+#define WPCM450_RESET_ADC         27
+#define WPCM450_RESET_SDIO        28
+#define WPCM450_RESET_SSPI        29
+#define WPCM450_RESET_SMB0        30
+#define WPCM450_RESET_SMB1        31
+
+#define WPCM450_NUM_RESETS        32
+
+#endif /* _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H */
--
2.35.1

[PATCH 5/7] ARM: dts: wpcm450: Add clock controller node

[Jonathan Neuschäfer]

This declares the clock controller and the necessary 48 Mhz reference
clock in the WPCM450 device. Switching devices over to the clock
controller is intentionally done in a separate patch to give time for
the clock controller driver to land.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
index 1c63ab14c4383..62d70fda7b520 100644
--- a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
+++ b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
@@ -39,6 +39,14 @@ clk24m: clock-24mhz {
 		#clock-cells = <0>;
 	};

+	refclk: clock-48mhz {
+		/* 48 MHz reference oscillator */
+		compatible = "fixed-clock";
+		clock-output-names = "refclk";
+		clock-frequency = <48000000>;
+		#clock-cells = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -51,6 +59,15 @@ gcr: syscon@b0000000 {
 			reg = <0xb0000000 0x200>;
 		};

+		clk: clock-controller@b0000200 {
+			compatible = "nuvoton,wpcm450-clk";
+			reg = <0xb0000200 0x100>;
+			clocks = <&refclk>;
+			clock-names = "refclk";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
 		serial0: serial@b8000000 {
 			compatible = "nuvoton,wpcm450-uart";
 			reg = <0xb8000000 0x20>;
--
2.35.1

[PATCH 5/7] ARM: dts: wpcm450: Add clock controller node

[Jonathan Neuschäfer]

This declares the clock controller and the necessary 48 Mhz reference
clock in the WPCM450 device. Switching devices over to the clock
controller is intentionally done in a separate patch to give time for
the clock controller driver to land.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
index 1c63ab14c4383..62d70fda7b520 100644
--- a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
+++ b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
@@ -39,6 +39,14 @@ clk24m: clock-24mhz {
 		#clock-cells = <0>;
 	};

+	refclk: clock-48mhz {
+		/* 48 MHz reference oscillator */
+		compatible = "fixed-clock";
+		clock-output-names = "refclk";
+		clock-frequency = <48000000>;
+		#clock-cells = <0>;
+	};
+
 	soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;
@@ -51,6 +59,15 @@ gcr: syscon@b0000000 {
 			reg = <0xb0000000 0x200>;
 		};

+		clk: clock-controller@b0000200 {
+			compatible = "nuvoton,wpcm450-clk";
+			reg = <0xb0000200 0x100>;
+			clocks = <&refclk>;
+			clock-names = "refclk";
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+		};
+
 		serial0: serial@b8000000 {
 			compatible = "nuvoton,wpcm450-uart";
 			reg = <0xb8000000 0x20>;
--
2.35.1

[PATCH 6/7] clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver

[Jonathan Neuschäfer]

This driver implements the following features w.r.t. the clock and reset
controller in the WPCM450 SoC:

- It calculates the rates for all clocks managed by the clock controller
- It leaves the clock tree mostly unchanged, except that it enables/
  disables clock gates based on usage.
- It exposes the reset lines managed by the controller using the
  Generic Reset Controller subsystem

NOTE: If the driver and the corresponding devicetree node are present,
      the driver will disable "unused" clocks. This is problem until
      the clock relations are properly declared in the devicetree (in a
      later patch). Until then, the clk_ignore_unused kernel parameter
      can be used as a workaround.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-wpcm450.c | 378 ++++++++++++++++++++++++++++++++++++++
 drivers/reset/Kconfig     |   2 +-
 3 files changed, 380 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/clk-wpcm450.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 2bd5ffd595bf3..07edb0f4ba8ba 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
 obj-$(CONFIG_COMMON_CLK_RS9_PCIE)	+= clk-renesas-pcie.o
 obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
+obj-$(CONFIG_ARCH_WPCM450)		+= clk-wpcm450.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o

 # please keep this section sorted lexicographically by directory path name
diff --git a/drivers/clk/clk-wpcm450.c b/drivers/clk/clk-wpcm450.c
new file mode 100644
index 0000000000000..3b62b5961d5f0
--- /dev/null
+++ b/drivers/clk/clk-wpcm450.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton WPCM450 clock and reset controller driver.
+ *
+ * Copyright (C) 2022 Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/reset-controller.h>
+#include <linux/reset/reset-simple.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
+
+struct wpcm450_clk_pll {
+	struct clk_hw hw;
+	void __iomem *pllcon;
+	u8 flags;
+};
+
+#define to_wpcm450_clk_pll(_hw) container_of(_hw, struct wpcm450_clk_pll, hw)
+
+#define PLLCON_FBDV	GENMASK(24, 16)
+#define PLLCON_PRST	BIT(13)
+#define PLLCON_PWDEN	BIT(12)
+#define PLLCON_OTDV	GENMASK(10, 8)
+#define PLLCON_INDV	GENMASK(5, 0)
+
+static unsigned long wpcm450_clk_pll_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	unsigned long fbdv, indv, otdv;
+	u64 rate;
+	u32 pllcon;
+
+	if (parent_rate == 0) {
+		pr_err("%s: parent rate is zero", __func__);
+		return 0;
+	}
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	indv = FIELD_GET(PLLCON_INDV, pllcon) + 1;
+	fbdv = FIELD_GET(PLLCON_FBDV, pllcon) + 1;
+	otdv = FIELD_GET(PLLCON_OTDV, pllcon) + 1;
+
+	rate = (u64)parent_rate * fbdv;
+	do_div(rate, indv * otdv);
+
+	return rate;
+}
+
+static int wpcm450_clk_pll_is_enabled(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	return !(pllcon & PLLCON_PRST);
+}
+
+static void wpcm450_clk_pll_disable(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+	pllcon |= PLLCON_PRST | PLLCON_PWDEN;
+	writel(pllcon, pll->pllcon);
+}
+
+static const struct clk_ops wpcm450_clk_pll_ops = {
+	.recalc_rate = wpcm450_clk_pll_recalc_rate,
+	.is_enabled = wpcm450_clk_pll_is_enabled,
+	.disable = wpcm450_clk_pll_disable
+};
+
+static struct clk_hw *
+wpcm450_clk_register_pll(void __iomem *pllcon, const char *name,
+			 const char *parent_name, unsigned long flags)
+{
+	struct wpcm450_clk_pll *pll;
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	int ret;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &wpcm450_clk_pll_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll->pllcon = pllcon;
+	pll->hw.init = &init;
+
+	ret = clk_hw_register(NULL, &pll->hw);
+	if (ret) {
+		kfree(pll);
+		hw = ERR_PTR(ret);
+	}
+
+	return &pll->hw;
+}
+
+#define REG_CLKEN	0x00
+#define REG_CLKSEL	0x04
+#define REG_CLKDIV	0x08
+#define REG_PLLCON0	0x0c
+#define REG_PLLCON1	0x10
+#define REG_PMCON	0x14
+#define REG_IRQWAKECON	0x18
+#define REG_IRQWAKEFLAG	0x1c
+#define REG_IPSRST	0x20
+
+struct wpcm450_pll_data {
+	const char *name;
+	const char *parent_name;
+	unsigned int reg;
+	unsigned long flags;
+};
+
+static const struct wpcm450_pll_data pll_data[] = {
+	{ "pll0", "refclk", REG_PLLCON0, 0 },
+	{ "pll1", "refclk", REG_PLLCON1, 0 },
+};
+
+struct wpcm450_clksel_data {
+	const char *name;
+	const char *const *parent_names;
+	unsigned int num_parents;
+	const u32 *table;
+	int shift;
+	int width;
+	int index;
+	unsigned long flags;
+};
+
+static const u32 parent_table[] = { 0, 1, 2 };
+static const char *const default_parents[] = { "pll0", "pll1", "refclk" };
+static const char *const huart_parents[] = { "refclk", "refdiv2" };
+
+static const struct wpcm450_clksel_data clksel_data[] = {
+	{ "cpusel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 0, 2, -1, CLK_IS_CRITICAL },
+	{ "clkout", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 2, 2, -1, 0 },
+	{ "usbphy", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 6, 2, -1, 0 },
+	{ "uartsel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 8, 2, WPCM450_CLK_USBPHY, 0 },
+	{ "huartsel", huart_parents, ARRAY_SIZE(huart_parents),
+		parent_table, 10, 1, -1, 0 },
+};
+
+static const struct clk_div_table div_default[] = {
+	{ .val = 0, .div = 1, },
+	{ .val = 1, .div = 2, },
+	{ .val = 2, .div = 4, },
+	{ .val = 3, .div = 8, },
+	{ }
+};
+
+static const struct clk_div_table div_ahb[] = {
+	{ .val = 0, .div = 1, },
+	{ .val = 1, .div = 2, },
+	{ }
+};
+
+static const struct clk_div_table div_fixed2[] = {
+	{ .val = 0, .div = 2 },
+	{ }
+};
+
+struct wpcm450_clkdiv_data {
+	const char *name;
+	const char *parent_name;
+	int div_flags;
+	const struct clk_div_table *table;
+	int shift;
+	int width;
+	unsigned long flags;
+};
+
+static struct wpcm450_clkdiv_data clkdiv_data_early[] = {
+	{ "refdiv2", "refclk", 0, div_fixed2, 0, 0 },
+};
+
+static const struct wpcm450_clkdiv_data clkdiv_data[] = {
+	{ "cpu", "cpusel", 0, div_fixed2, 0, 0, CLK_IS_CRITICAL },
+	{ "adcdiv", "refclk", CLK_DIVIDER_POWER_OF_TWO, NULL, 28, 2, 0 },
+	{ "apb", "ahb", CLK_DIVIDER_POWER_OF_TWO, NULL, 26, 2, 0 },
+	{ "ahb", "cpu", CLK_DIVIDER_POWER_OF_TWO, NULL, 24, 2, 0 },
+	{ "uart", "uartsel", 0, NULL, 16, 4, 0 },
+	{ "ahb3", "ahb", CLK_DIVIDER_POWER_OF_TWO, NULL, 8, 2, 0 },
+};
+
+struct wpcm450_clken_data {
+	const char *name;
+	const char *parent_name;
+	int bitnum;
+	unsigned long flags;
+};
+
+static const struct wpcm450_clken_data clken_data[] = {
+	{ "fiu", "ahb3", WPCM450_CLK_FIU, 0 },
+	{ "xbus", "ahb3", WPCM450_CLK_XBUS, 0 },
+	{ "kcs", "apb", WPCM450_CLK_KCS, 0 },
+	{ "shm", "ahb3", WPCM450_CLK_SHM, 0 },
+	{ "usb1", "ahb", WPCM450_CLK_USB1, 0 },
+	{ "emc0", "ahb", WPCM450_CLK_EMC0, 0 },
+	{ "emc1", "ahb", WPCM450_CLK_EMC1, 0 },
+	{ "usb0", "ahb", WPCM450_CLK_USB0, 0 },
+	{ "peci", "apb", WPCM450_CLK_PECI, 0 },
+	{ "aes", "apb", WPCM450_CLK_AES, 0 },
+	{ "uart0", "uart", WPCM450_CLK_UART0, 0 },
+	{ "uart1", "uart", WPCM450_CLK_UART1, 0 },
+	{ "smb2", "apb", WPCM450_CLK_SMB2, 0 },
+	{ "smb3", "apb", WPCM450_CLK_SMB3, 0 },
+	{ "smb4", "apb", WPCM450_CLK_SMB4, 0 },
+	{ "smb5", "apb", WPCM450_CLK_SMB5, 0 },
+	{ "huart", "huartsel", WPCM450_CLK_HUART, 0 },
+	{ "pwm", "apb", WPCM450_CLK_PWM, 0 },
+	{ "timer0", "refdiv2", WPCM450_CLK_TIMER0, 0 },
+	{ "timer1", "refdiv2", WPCM450_CLK_TIMER1, 0 },
+	{ "timer2", "refdiv2", WPCM450_CLK_TIMER2, 0 },
+	{ "timer3", "refdiv2", WPCM450_CLK_TIMER3, 0 },
+	{ "timer4", "refdiv2", WPCM450_CLK_TIMER4, 0 },
+	{ "mft0", "apb", WPCM450_CLK_MFT0, 0 },
+	{ "mft1", "apb", WPCM450_CLK_MFT1, 0 },
+	{ "wdt", "refdiv2", WPCM450_CLK_WDT, 0 },
+	{ "adc", "adcdiv", WPCM450_CLK_ADC, 0 },
+	{ "sdio", "ahb", WPCM450_CLK_SDIO, 0 },
+	{ "sspi", "apb", WPCM450_CLK_SSPI, 0 },
+	{ "smb0", "apb", WPCM450_CLK_SMB0, 0 },
+	{ "smb1", "apb", WPCM450_CLK_SMB1, 0 },
+};
+
+static DEFINE_SPINLOCK(wpcm450_clk_lock);
+
+static void __init wpcm450_clk_init(struct device_node *clk_np)
+{
+	struct clk_hw_onecell_data *clk_data;
+	static struct clk_hw **hws;
+	static struct clk_hw *hw;
+	void __iomem *clk_base;
+	int i, ret;
+	struct reset_controller_dev *rcdev;
+
+	clk_base = of_iomap(clk_np, 0);
+	if (!clk_base) {
+		pr_err("%pOFP: failed to map registers\n", clk_np);
+		of_node_put(clk_np);
+		return;
+	}
+	of_node_put(clk_np);
+
+	clk_data = kzalloc(struct_size(clk_data, hws, WPCM450_NUM_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clk_data->num = WPCM450_NUM_CLKS;
+	hws = clk_data->hws;
+
+	for (i = 0; i < WPCM450_NUM_CLKS; i++)
+		hws[i] = ERR_PTR(-ENOENT);
+
+	// PLLs
+	for (i = 0; i < ARRAY_SIZE(pll_data); i++) {
+		const struct wpcm450_pll_data *data = &pll_data[i];
+
+		hw = wpcm450_clk_register_pll(clk_base + data->reg, data->name,
+					      data->parent_name, data->flags);
+		if (IS_ERR(hw)) {
+			pr_info("Failed to register PLL: %pe", hw);
+			goto err_free;
+		}
+	}
+
+	// Early divisors (REF/2)
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data_early); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data_early[i];
+
+		hw = clk_hw_register_divider_table(NULL, data->name, data->parent_name,
+						   data->flags, clk_base + REG_CLKDIV,
+						   data->shift, data->width, data->div_flags,
+						   data->table, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register div table: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Selects/muxes
+	for (i = 0; i < ARRAY_SIZE(clksel_data); i++) {
+		const struct wpcm450_clksel_data *data = &clksel_data[i];
+
+		hw = clk_hw_register_mux_table(NULL, data->name, data->parent_names,
+					       data->num_parents, data->flags,
+					       clk_base + REG_CLKSEL, data->shift,
+					       BIT(data->width) - 1, 0, data->table,
+					       &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register mux: %pe\n", hw);
+			goto err_free;
+		}
+		if (data->index >= 0)
+			clk_data->hws[data->index] = hw;
+	}
+
+	// Divisors
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data[i];
+
+		hw = clk_hw_register_divider_table(NULL, data->name, data->parent_name,
+						   data->flags, clk_base + REG_CLKDIV,
+						   data->shift, data->width, data->div_flags,
+						   data->table, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register divider: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Enables/gates
+	for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
+		const struct wpcm450_clken_data *data = &clken_data[i];
+
+		hw = clk_hw_register_gate(NULL, data->name, data->parent_name, data->flags,
+					  clk_base + REG_CLKEN, data->bitnum,
+					  data->flags, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register gate: %pe\n", hw);
+			goto err_free;
+		}
+		clk_data->hws[data->bitnum] = hw;
+	}
+
+	ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		pr_err("Failed to add DT provider: %d\n", ret);
+
+	// Reset controller
+	rcdev = kzalloc(sizeof(*rcdev), GFP_KERNEL);
+	if (!rcdev)
+		goto err_free;
+	rcdev->owner = THIS_MODULE;
+	rcdev->nr_resets = WPCM450_NUM_RESETS;
+	rcdev->ops = &reset_simple_ops;
+	rcdev->of_node = clk_np;
+	ret = reset_controller_register(rcdev);
+	if (ret)
+		pr_err("Failed to register reset controller: %d\n", ret);
+
+	of_node_put(clk_np);
+	return;
+
+err_free:
+	kfree(clk_data->hws);
+err_unmap:
+	iounmap(clk_base);
+	of_node_put(clk_np);
+}
+
+CLK_OF_DECLARE(wpcm450_clk_init, "nuvoton,wpcm450-clk", wpcm450_clk_init);
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index b496028b6bfaf..50a3c1403c335 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -201,7 +201,7 @@ config RESET_SCMI

 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_ASPEED || ARCH_BCM4908 || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC
+	default ARCH_ASPEED || ARCH_BCM4908 || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC || ARCH_NPCM
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
--
2.35.1

[PATCH 6/7] clk: wpcm450: Add Nuvoton WPCM450 clock/reset controller driver

[Jonathan Neuschäfer]

This driver implements the following features w.r.t. the clock and reset
controller in the WPCM450 SoC:

- It calculates the rates for all clocks managed by the clock controller
- It leaves the clock tree mostly unchanged, except that it enables/
  disables clock gates based on usage.
- It exposes the reset lines managed by the controller using the
  Generic Reset Controller subsystem

NOTE: If the driver and the corresponding devicetree node are present,
      the driver will disable "unused" clocks. This is problem until
      the clock relations are properly declared in the devicetree (in a
      later patch). Until then, the clk_ignore_unused kernel parameter
      can be used as a workaround.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-wpcm450.c | 378 ++++++++++++++++++++++++++++++++++++++
 drivers/reset/Kconfig     |   2 +-
 3 files changed, 380 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/clk-wpcm450.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 2bd5ffd595bf3..07edb0f4ba8ba 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
 obj-$(CONFIG_COMMON_CLK_RS9_PCIE)	+= clk-renesas-pcie.o
 obj-$(CONFIG_COMMON_CLK_VC5)		+= clk-versaclock5.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
+obj-$(CONFIG_ARCH_WPCM450)		+= clk-wpcm450.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o

 # please keep this section sorted lexicographically by directory path name
diff --git a/drivers/clk/clk-wpcm450.c b/drivers/clk/clk-wpcm450.c
new file mode 100644
index 0000000000000..3b62b5961d5f0
--- /dev/null
+++ b/drivers/clk/clk-wpcm450.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton WPCM450 clock and reset controller driver.
+ *
+ * Copyright (C) 2022 Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/reset-controller.h>
+#include <linux/reset/reset-simple.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
+
+struct wpcm450_clk_pll {
+	struct clk_hw hw;
+	void __iomem *pllcon;
+	u8 flags;
+};
+
+#define to_wpcm450_clk_pll(_hw) container_of(_hw, struct wpcm450_clk_pll, hw)
+
+#define PLLCON_FBDV	GENMASK(24, 16)
+#define PLLCON_PRST	BIT(13)
+#define PLLCON_PWDEN	BIT(12)
+#define PLLCON_OTDV	GENMASK(10, 8)
+#define PLLCON_INDV	GENMASK(5, 0)
+
+static unsigned long wpcm450_clk_pll_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	unsigned long fbdv, indv, otdv;
+	u64 rate;
+	u32 pllcon;
+
+	if (parent_rate == 0) {
+		pr_err("%s: parent rate is zero", __func__);
+		return 0;
+	}
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	indv = FIELD_GET(PLLCON_INDV, pllcon) + 1;
+	fbdv = FIELD_GET(PLLCON_FBDV, pllcon) + 1;
+	otdv = FIELD_GET(PLLCON_OTDV, pllcon) + 1;
+
+	rate = (u64)parent_rate * fbdv;
+	do_div(rate, indv * otdv);
+
+	return rate;
+}
+
+static int wpcm450_clk_pll_is_enabled(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+
+	return !(pllcon & PLLCON_PRST);
+}
+
+static void wpcm450_clk_pll_disable(struct clk_hw *hw)
+{
+	struct wpcm450_clk_pll *pll = to_wpcm450_clk_pll(hw);
+	u32 pllcon;
+
+	pllcon = readl_relaxed(pll->pllcon);
+	pllcon |= PLLCON_PRST | PLLCON_PWDEN;
+	writel(pllcon, pll->pllcon);
+}
+
+static const struct clk_ops wpcm450_clk_pll_ops = {
+	.recalc_rate = wpcm450_clk_pll_recalc_rate,
+	.is_enabled = wpcm450_clk_pll_is_enabled,
+	.disable = wpcm450_clk_pll_disable
+};
+
+static struct clk_hw *
+wpcm450_clk_register_pll(void __iomem *pllcon, const char *name,
+			 const char *parent_name, unsigned long flags)
+{
+	struct wpcm450_clk_pll *pll;
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	int ret;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &wpcm450_clk_pll_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll->pllcon = pllcon;
+	pll->hw.init = &init;
+
+	ret = clk_hw_register(NULL, &pll->hw);
+	if (ret) {
+		kfree(pll);
+		hw = ERR_PTR(ret);
+	}
+
+	return &pll->hw;
+}
+
+#define REG_CLKEN	0x00
+#define REG_CLKSEL	0x04
+#define REG_CLKDIV	0x08
+#define REG_PLLCON0	0x0c
+#define REG_PLLCON1	0x10
+#define REG_PMCON	0x14
+#define REG_IRQWAKECON	0x18
+#define REG_IRQWAKEFLAG	0x1c
+#define REG_IPSRST	0x20
+
+struct wpcm450_pll_data {
+	const char *name;
+	const char *parent_name;
+	unsigned int reg;
+	unsigned long flags;
+};
+
+static const struct wpcm450_pll_data pll_data[] = {
+	{ "pll0", "refclk", REG_PLLCON0, 0 },
+	{ "pll1", "refclk", REG_PLLCON1, 0 },
+};
+
+struct wpcm450_clksel_data {
+	const char *name;
+	const char *const *parent_names;
+	unsigned int num_parents;
+	const u32 *table;
+	int shift;
+	int width;
+	int index;
+	unsigned long flags;
+};
+
+static const u32 parent_table[] = { 0, 1, 2 };
+static const char *const default_parents[] = { "pll0", "pll1", "refclk" };
+static const char *const huart_parents[] = { "refclk", "refdiv2" };
+
+static const struct wpcm450_clksel_data clksel_data[] = {
+	{ "cpusel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 0, 2, -1, CLK_IS_CRITICAL },
+	{ "clkout", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 2, 2, -1, 0 },
+	{ "usbphy", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 6, 2, -1, 0 },
+	{ "uartsel", default_parents, ARRAY_SIZE(default_parents),
+		parent_table, 8, 2, WPCM450_CLK_USBPHY, 0 },
+	{ "huartsel", huart_parents, ARRAY_SIZE(huart_parents),
+		parent_table, 10, 1, -1, 0 },
+};
+
+static const struct clk_div_table div_default[] = {
+	{ .val = 0, .div = 1, },
+	{ .val = 1, .div = 2, },
+	{ .val = 2, .div = 4, },
+	{ .val = 3, .div = 8, },
+	{ }
+};
+
+static const struct clk_div_table div_ahb[] = {
+	{ .val = 0, .div = 1, },
+	{ .val = 1, .div = 2, },
+	{ }
+};
+
+static const struct clk_div_table div_fixed2[] = {
+	{ .val = 0, .div = 2 },
+	{ }
+};
+
+struct wpcm450_clkdiv_data {
+	const char *name;
+	const char *parent_name;
+	int div_flags;
+	const struct clk_div_table *table;
+	int shift;
+	int width;
+	unsigned long flags;
+};
+
+static struct wpcm450_clkdiv_data clkdiv_data_early[] = {
+	{ "refdiv2", "refclk", 0, div_fixed2, 0, 0 },
+};
+
+static const struct wpcm450_clkdiv_data clkdiv_data[] = {
+	{ "cpu", "cpusel", 0, div_fixed2, 0, 0, CLK_IS_CRITICAL },
+	{ "adcdiv", "refclk", CLK_DIVIDER_POWER_OF_TWO, NULL, 28, 2, 0 },
+	{ "apb", "ahb", CLK_DIVIDER_POWER_OF_TWO, NULL, 26, 2, 0 },
+	{ "ahb", "cpu", CLK_DIVIDER_POWER_OF_TWO, NULL, 24, 2, 0 },
+	{ "uart", "uartsel", 0, NULL, 16, 4, 0 },
+	{ "ahb3", "ahb", CLK_DIVIDER_POWER_OF_TWO, NULL, 8, 2, 0 },
+};
+
+struct wpcm450_clken_data {
+	const char *name;
+	const char *parent_name;
+	int bitnum;
+	unsigned long flags;
+};
+
+static const struct wpcm450_clken_data clken_data[] = {
+	{ "fiu", "ahb3", WPCM450_CLK_FIU, 0 },
+	{ "xbus", "ahb3", WPCM450_CLK_XBUS, 0 },
+	{ "kcs", "apb", WPCM450_CLK_KCS, 0 },
+	{ "shm", "ahb3", WPCM450_CLK_SHM, 0 },
+	{ "usb1", "ahb", WPCM450_CLK_USB1, 0 },
+	{ "emc0", "ahb", WPCM450_CLK_EMC0, 0 },
+	{ "emc1", "ahb", WPCM450_CLK_EMC1, 0 },
+	{ "usb0", "ahb", WPCM450_CLK_USB0, 0 },
+	{ "peci", "apb", WPCM450_CLK_PECI, 0 },
+	{ "aes", "apb", WPCM450_CLK_AES, 0 },
+	{ "uart0", "uart", WPCM450_CLK_UART0, 0 },
+	{ "uart1", "uart", WPCM450_CLK_UART1, 0 },
+	{ "smb2", "apb", WPCM450_CLK_SMB2, 0 },
+	{ "smb3", "apb", WPCM450_CLK_SMB3, 0 },
+	{ "smb4", "apb", WPCM450_CLK_SMB4, 0 },
+	{ "smb5", "apb", WPCM450_CLK_SMB5, 0 },
+	{ "huart", "huartsel", WPCM450_CLK_HUART, 0 },
+	{ "pwm", "apb", WPCM450_CLK_PWM, 0 },
+	{ "timer0", "refdiv2", WPCM450_CLK_TIMER0, 0 },
+	{ "timer1", "refdiv2", WPCM450_CLK_TIMER1, 0 },
+	{ "timer2", "refdiv2", WPCM450_CLK_TIMER2, 0 },
+	{ "timer3", "refdiv2", WPCM450_CLK_TIMER3, 0 },
+	{ "timer4", "refdiv2", WPCM450_CLK_TIMER4, 0 },
+	{ "mft0", "apb", WPCM450_CLK_MFT0, 0 },
+	{ "mft1", "apb", WPCM450_CLK_MFT1, 0 },
+	{ "wdt", "refdiv2", WPCM450_CLK_WDT, 0 },
+	{ "adc", "adcdiv", WPCM450_CLK_ADC, 0 },
+	{ "sdio", "ahb", WPCM450_CLK_SDIO, 0 },
+	{ "sspi", "apb", WPCM450_CLK_SSPI, 0 },
+	{ "smb0", "apb", WPCM450_CLK_SMB0, 0 },
+	{ "smb1", "apb", WPCM450_CLK_SMB1, 0 },
+};
+
+static DEFINE_SPINLOCK(wpcm450_clk_lock);
+
+static void __init wpcm450_clk_init(struct device_node *clk_np)
+{
+	struct clk_hw_onecell_data *clk_data;
+	static struct clk_hw **hws;
+	static struct clk_hw *hw;
+	void __iomem *clk_base;
+	int i, ret;
+	struct reset_controller_dev *rcdev;
+
+	clk_base = of_iomap(clk_np, 0);
+	if (!clk_base) {
+		pr_err("%pOFP: failed to map registers\n", clk_np);
+		of_node_put(clk_np);
+		return;
+	}
+	of_node_put(clk_np);
+
+	clk_data = kzalloc(struct_size(clk_data, hws, WPCM450_NUM_CLKS), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clk_data->num = WPCM450_NUM_CLKS;
+	hws = clk_data->hws;
+
+	for (i = 0; i < WPCM450_NUM_CLKS; i++)
+		hws[i] = ERR_PTR(-ENOENT);
+
+	// PLLs
+	for (i = 0; i < ARRAY_SIZE(pll_data); i++) {
+		const struct wpcm450_pll_data *data = &pll_data[i];
+
+		hw = wpcm450_clk_register_pll(clk_base + data->reg, data->name,
+					      data->parent_name, data->flags);
+		if (IS_ERR(hw)) {
+			pr_info("Failed to register PLL: %pe", hw);
+			goto err_free;
+		}
+	}
+
+	// Early divisors (REF/2)
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data_early); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data_early[i];
+
+		hw = clk_hw_register_divider_table(NULL, data->name, data->parent_name,
+						   data->flags, clk_base + REG_CLKDIV,
+						   data->shift, data->width, data->div_flags,
+						   data->table, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register div table: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Selects/muxes
+	for (i = 0; i < ARRAY_SIZE(clksel_data); i++) {
+		const struct wpcm450_clksel_data *data = &clksel_data[i];
+
+		hw = clk_hw_register_mux_table(NULL, data->name, data->parent_names,
+					       data->num_parents, data->flags,
+					       clk_base + REG_CLKSEL, data->shift,
+					       BIT(data->width) - 1, 0, data->table,
+					       &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register mux: %pe\n", hw);
+			goto err_free;
+		}
+		if (data->index >= 0)
+			clk_data->hws[data->index] = hw;
+	}
+
+	// Divisors
+	for (i = 0; i < ARRAY_SIZE(clkdiv_data); i++) {
+		const struct wpcm450_clkdiv_data *data = &clkdiv_data[i];
+
+		hw = clk_hw_register_divider_table(NULL, data->name, data->parent_name,
+						   data->flags, clk_base + REG_CLKDIV,
+						   data->shift, data->width, data->div_flags,
+						   data->table, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register divider: %pe\n", hw);
+			goto err_free;
+		}
+	}
+
+	// Enables/gates
+	for (i = 0; i < ARRAY_SIZE(clken_data); i++) {
+		const struct wpcm450_clken_data *data = &clken_data[i];
+
+		hw = clk_hw_register_gate(NULL, data->name, data->parent_name, data->flags,
+					  clk_base + REG_CLKEN, data->bitnum,
+					  data->flags, &wpcm450_clk_lock);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register gate: %pe\n", hw);
+			goto err_free;
+		}
+		clk_data->hws[data->bitnum] = hw;
+	}
+
+	ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, clk_data);
+	if (ret)
+		pr_err("Failed to add DT provider: %d\n", ret);
+
+	// Reset controller
+	rcdev = kzalloc(sizeof(*rcdev), GFP_KERNEL);
+	if (!rcdev)
+		goto err_free;
+	rcdev->owner = THIS_MODULE;
+	rcdev->nr_resets = WPCM450_NUM_RESETS;
+	rcdev->ops = &reset_simple_ops;
+	rcdev->of_node = clk_np;
+	ret = reset_controller_register(rcdev);
+	if (ret)
+		pr_err("Failed to register reset controller: %d\n", ret);
+
+	of_node_put(clk_np);
+	return;
+
+err_free:
+	kfree(clk_data->hws);
+err_unmap:
+	iounmap(clk_base);
+	of_node_put(clk_np);
+}
+
+CLK_OF_DECLARE(wpcm450_clk_init, "nuvoton,wpcm450-clk", wpcm450_clk_init);
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index b496028b6bfaf..50a3c1403c335 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -201,7 +201,7 @@ config RESET_SCMI

 config RESET_SIMPLE
 	bool "Simple Reset Controller Driver" if COMPILE_TEST
-	default ARCH_ASPEED || ARCH_BCM4908 || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC
+	default ARCH_ASPEED || ARCH_BCM4908 || ARCH_BITMAIN || ARCH_REALTEK || ARCH_STM32 || (ARCH_INTEL_SOCFPGA && ARM64) || ARCH_SUNXI || ARC || ARCH_NPCM
 	help
 	  This enables a simple reset controller driver for reset lines that
 	  that can be asserted and deasserted by toggling bits in a contiguous,
--
2.35.1

[PATCH 7/7] ARM: dts: wpcm450: Switch clocks to clock controller

[Jonathan Neuschäfer]

This change is incompatible with older kernels because it requires the
clock controller driver, but I think that's acceptable because WPCM450
support is generally still in an early phase.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
index 62d70fda7b520..f868bd7db009a 100644
--- a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
+++ b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
@@ -2,6 +2,7 @@
 // Copyright 2021 Jonathan Neuschäfer

 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>

 / {
 	compatible = "nuvoton,wpcm450";
@@ -32,13 +33,6 @@ cpu@0 {
 		};
 	};

-	clk24m: clock-24mhz {
-		/* 24 MHz dummy clock */
-		compatible = "fixed-clock";
-		clock-frequency = <24000000>;
-		#clock-cells = <0>;
-	};
-
 	refclk: clock-48mhz {
 		/* 48 MHz reference oscillator */
 		compatible = "fixed-clock";
@@ -73,7 +67,7 @@ serial0: serial@b8000000 {
 			reg = <0xb8000000 0x20>;
 			reg-shift = <2>;
 			interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_UART0>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&bsp_pins>;
 			status = "disabled";
@@ -84,7 +78,7 @@ serial1: serial@b8000100 {
 			reg = <0xb8000100 0x20>;
 			reg-shift = <2>;
 			interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_UART1>;
 			status = "disabled";
 		};

@@ -92,14 +86,18 @@ timer0: timer@b8001000 {
 			compatible = "nuvoton,wpcm450-timer";
 			interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xb8001000 0x1c>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_TIMER0>,
+				 <&clk WPCM450_CLK_TIMER1>,
+				 <&clk WPCM450_CLK_TIMER2>,
+				 <&clk WPCM450_CLK_TIMER3>,
+				 <&clk WPCM450_CLK_TIMER4>;
 		};

 		watchdog0: watchdog@b800101c {
 			compatible = "nuvoton,wpcm450-wdt";
 			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xb800101c 0x4>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_WDT>;
 		};

 		aic: interrupt-controller@b8002000 {
--
2.35.1

[PATCH 7/7] ARM: dts: wpcm450: Switch clocks to clock controller

[Jonathan Neuschäfer]

This change is incompatible with older kernels because it requires the
clock controller driver, but I think that's acceptable because WPCM450
support is generally still in an early phase.

Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
 arch/arm/boot/dts/nuvoton-wpcm450.dtsi | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
index 62d70fda7b520..f868bd7db009a 100644
--- a/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
+++ b/arch/arm/boot/dts/nuvoton-wpcm450.dtsi
@@ -2,6 +2,7 @@
 // Copyright 2021 Jonathan Neuschäfer

 #include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/clock/nuvoton,wpcm450-clk.h>

 / {
 	compatible = "nuvoton,wpcm450";
@@ -32,13 +33,6 @@ cpu@0 {
 		};
 	};

-	clk24m: clock-24mhz {
-		/* 24 MHz dummy clock */
-		compatible = "fixed-clock";
-		clock-frequency = <24000000>;
-		#clock-cells = <0>;
-	};
-
 	refclk: clock-48mhz {
 		/* 48 MHz reference oscillator */
 		compatible = "fixed-clock";
@@ -73,7 +67,7 @@ serial0: serial@b8000000 {
 			reg = <0xb8000000 0x20>;
 			reg-shift = <2>;
 			interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_UART0>;
 			pinctrl-names = "default";
 			pinctrl-0 = <&bsp_pins>;
 			status = "disabled";
@@ -84,7 +78,7 @@ serial1: serial@b8000100 {
 			reg = <0xb8000100 0x20>;
 			reg-shift = <2>;
 			interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_UART1>;
 			status = "disabled";
 		};

@@ -92,14 +86,18 @@ timer0: timer@b8001000 {
 			compatible = "nuvoton,wpcm450-timer";
 			interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xb8001000 0x1c>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_TIMER0>,
+				 <&clk WPCM450_CLK_TIMER1>,
+				 <&clk WPCM450_CLK_TIMER2>,
+				 <&clk WPCM450_CLK_TIMER3>,
+				 <&clk WPCM450_CLK_TIMER4>;
 		};

 		watchdog0: watchdog@b800101c {
 			compatible = "nuvoton,wpcm450-wdt";
 			interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xb800101c 0x4>;
-			clocks = <&clk24m>;
+			clocks = <&clk WPCM450_CLK_WDT>;
 		};

 		aic: interrupt-controller@b8002000 {
--
2.35.1

Re: [PATCH 3/7] watchdog: npcm: Enable clock if provided

[Guenter Roeck]

On Fri, Apr 22, 2022 at 08:30:08PM +0200, Jonathan Neuschäfer wrote:
> On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
> clocks are individually gated and ungated. Therefore, the watchdog
> driver must be able to ungate the watchdog clock.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
>  drivers/watchdog/npcm_wdt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 28a24caa2627c..6d27f0e16188e 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -3,6 +3,7 @@
>  // Copyright (c) 2018 IBM Corp.
> 
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct npcm_wdt *wdt;
> +	struct clk *clk;
>  	int irq;
>  	int ret;
> 
> @@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>  	if (IS_ERR(wdt->reg))
>  		return PTR_ERR(wdt->reg);
> 
> +	clk = devm_clk_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	if (clk)
> +		clk_prepare_enable(clk);
> +

This needs a matching clk_disable_unprepare().

Guenter

Re: [PATCH 3/7] watchdog: npcm: Enable clock if provided

[Guenter Roeck]

On Fri, Apr 22, 2022 at 08:30:08PM +0200, Jonathan Neuschäfer wrote:
> On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
> clocks are individually gated and ungated. Therefore, the watchdog
> driver must be able to ungate the watchdog clock.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---
>  drivers/watchdog/npcm_wdt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> index 28a24caa2627c..6d27f0e16188e 100644
> --- a/drivers/watchdog/npcm_wdt.c
> +++ b/drivers/watchdog/npcm_wdt.c
> @@ -3,6 +3,7 @@
>  // Copyright (c) 2018 IBM Corp.
> 
>  #include <linux/bitops.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
> @@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct npcm_wdt *wdt;
> +	struct clk *clk;
>  	int irq;
>  	int ret;
> 
> @@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
>  	if (IS_ERR(wdt->reg))
>  		return PTR_ERR(wdt->reg);
> 
> +	clk = devm_clk_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	if (clk)
> +		clk_prepare_enable(clk);
> +

This needs a matching clk_disable_unprepare().

Guenter

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Krzysztof Kozlowski]

On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> Add a devicetree binding for it, as well as definitions for the bit
> numbers used by it.
> 
> Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> ---

Thank you for your patch. There is something to discuss/improve.

>  .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
>  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
>  2 files changed, 141 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
>  create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> new file mode 100644
> index 0000000000000..0fffa8a68dee4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/nuvoton,wpcm450-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton WPCM450 clock controller binding

s/binding//

> +
> +maintainers:
> +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> +
> +description:
> +  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
> +  supplies clocks and resets to the rest of the chip.

s/This binding describes//

Just describe the hardware.

> +
> +properties:
> +  compatible:
> +    const: nuvoton,wpcm450-clk
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Reference clock oscillator (should be 48 MHz)
> +
> +  clock-names:
> +    items:
> +      - const: refclk
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  '#reset-cells':
> +    const: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    refclk: clock-48mhz {
> +      /* 48 MHz reference oscillator */
> +      compatible = "fixed-clock";
> +      clock-output-names = "refclk";
> +      clock-frequency = <48000000>;
> +      #clock-cells = <0>;
> +    };
> +
> +    clk: clock-controller@b0000200 {
> +      reg = <0xb0000200 0x100>;
> +      compatible = "nuvoton,wpcm450-clk";
> +      clocks = <&refclk>;
> +      clock-names = "refclk";
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +    };
> +
> +    serial@b8000000 {
> +      compatible = "nuvoton,wpcm450-uart";
> +      reg = <0xb8000000 0x20>;
> +      reg-shift = <2>;
> +      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
> +      clocks = <&clk WPCM450_CLK_UART0>;
> +    };

Skip the consumer example, it's obvious/trivial/duplicating.

> diff --git a/include/dt-bindings/clock/nuvoton,wpcm450-clk.h b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> new file mode 100644
> index 0000000000000..86e1c895921b7
> --- /dev/null
> +++ b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> +#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> +
> +/* Clocks based on CLKEN bits */
> +#define WPCM450_CLK_FIU            0
> +#define WPCM450_CLK_XBUS           1
> +#define WPCM450_CLK_KCS            2
> +#define WPCM450_CLK_SHM            4
> +#define WPCM450_CLK_USB1           5
> +#define WPCM450_CLK_EMC0           6
> +#define WPCM450_CLK_EMC1           7
> +#define WPCM450_CLK_USB0           8
> +#define WPCM450_CLK_PECI           9
> +#define WPCM450_CLK_AES           10
> +#define WPCM450_CLK_UART0         11
> +#define WPCM450_CLK_UART1         12
> +#define WPCM450_CLK_SMB2          13
> +#define WPCM450_CLK_SMB3          14
> +#define WPCM450_CLK_SMB4          15
> +#define WPCM450_CLK_SMB5          16
> +#define WPCM450_CLK_HUART         17
> +#define WPCM450_CLK_PWM           18
> +#define WPCM450_CLK_TIMER0        19
> +#define WPCM450_CLK_TIMER1        20
> +#define WPCM450_CLK_TIMER2        21
> +#define WPCM450_CLK_TIMER3        22
> +#define WPCM450_CLK_TIMER4        23
> +#define WPCM450_CLK_MFT0          24
> +#define WPCM450_CLK_MFT1          25
> +#define WPCM450_CLK_WDT           26
> +#define WPCM450_CLK_ADC           27
> +#define WPCM450_CLK_SDIO          28
> +#define WPCM450_CLK_SSPI          29
> +#define WPCM450_CLK_SMB0          30
> +#define WPCM450_CLK_SMB1          31
> +
> +/* Other clocks */
> +#define WPCM450_CLK_USBPHY        32
> +
> +#define WPCM450_NUM_CLKS          33
> +
> +/* Resets based on IPSRST bits */

All these defines should be in second header in dt-bindings/reset/...

> +#define WPCM450_RESET_FIU          0


Best regards,
Krzysztof

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Joel Stanley]

On Mon, 25 Apr 2022 at 07:59, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > Add a devicetree binding for it, as well as definitions for the bit
> > numbers used by it.
> >
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
>
> Thank you for your patch. There is something to discuss/improve.
>
> >  .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
> >  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
> >  2 files changed, 141 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> >  create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> > new file mode 100644
> > index 0000000000000..0fffa8a68dee4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/nuvoton,wpcm450-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Nuvoton WPCM450 clock controller binding
>
> s/binding//
>
> > +
> > +maintainers:
> > +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > +
> > +description:
> > +  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
> > +  supplies clocks and resets to the rest of the chip.
>
> s/This binding describes//
>
> Just describe the hardware.
>
> > +
> > +properties:
> > +  compatible:
> > +    const: nuvoton,wpcm450-clk
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: Reference clock oscillator (should be 48 MHz)
> > +
> > +  clock-names:
> > +    items:
> > +      - const: refclk
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  '#reset-cells':
> > +    const: 1
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - '#clock-cells'
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    refclk: clock-48mhz {
> > +      /* 48 MHz reference oscillator */
> > +      compatible = "fixed-clock";
> > +      clock-output-names = "refclk";
> > +      clock-frequency = <48000000>;
> > +      #clock-cells = <0>;
> > +    };
> > +
> > +    clk: clock-controller@b0000200 {
> > +      reg = <0xb0000200 0x100>;
> > +      compatible = "nuvoton,wpcm450-clk";
> > +      clocks = <&refclk>;
> > +      clock-names = "refclk";
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };
> > +
> > +    serial@b8000000 {
> > +      compatible = "nuvoton,wpcm450-uart";
> > +      reg = <0xb8000000 0x20>;
> > +      reg-shift = <2>;
> > +      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
> > +      clocks = <&clk WPCM450_CLK_UART0>;
> > +    };
>
> Skip the consumer example, it's obvious/trivial/duplicating.
>
> > diff --git a/include/dt-bindings/clock/nuvoton,wpcm450-clk.h b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> > new file mode 100644
> > index 0000000000000..86e1c895921b7
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> > @@ -0,0 +1,67 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +
> > +/* Clocks based on CLKEN bits */
> > +#define WPCM450_CLK_FIU            0
> > +#define WPCM450_CLK_XBUS           1
> > +#define WPCM450_CLK_KCS            2
> > +#define WPCM450_CLK_SHM            4
> > +#define WPCM450_CLK_USB1           5
> > +#define WPCM450_CLK_EMC0           6
> > +#define WPCM450_CLK_EMC1           7
> > +#define WPCM450_CLK_USB0           8
> > +#define WPCM450_CLK_PECI           9
> > +#define WPCM450_CLK_AES           10
> > +#define WPCM450_CLK_UART0         11
> > +#define WPCM450_CLK_UART1         12
> > +#define WPCM450_CLK_SMB2          13
> > +#define WPCM450_CLK_SMB3          14
> > +#define WPCM450_CLK_SMB4          15
> > +#define WPCM450_CLK_SMB5          16
> > +#define WPCM450_CLK_HUART         17
> > +#define WPCM450_CLK_PWM           18
> > +#define WPCM450_CLK_TIMER0        19
> > +#define WPCM450_CLK_TIMER1        20
> > +#define WPCM450_CLK_TIMER2        21
> > +#define WPCM450_CLK_TIMER3        22
> > +#define WPCM450_CLK_TIMER4        23
> > +#define WPCM450_CLK_MFT0          24
> > +#define WPCM450_CLK_MFT1          25
> > +#define WPCM450_CLK_WDT           26
> > +#define WPCM450_CLK_ADC           27
> > +#define WPCM450_CLK_SDIO          28
> > +#define WPCM450_CLK_SSPI          29
> > +#define WPCM450_CLK_SMB0          30
> > +#define WPCM450_CLK_SMB1          31
> > +
> > +/* Other clocks */
> > +#define WPCM450_CLK_USBPHY        32
> > +
> > +#define WPCM450_NUM_CLKS          33
> > +
> > +/* Resets based on IPSRST bits */
>
> All these defines should be in second header in dt-bindings/reset/...

I disagree. It makes more sense to keep the definitions together, and
it's all for the same hardware and driver.

>
> > +#define WPCM450_RESET_FIU          0
>
>
> Best regards,
> Krzysztof

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Joel Stanley]

On Mon, 25 Apr 2022 at 07:59, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > Add a devicetree binding for it, as well as definitions for the bit
> > numbers used by it.
> >
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
>
> Thank you for your patch. There is something to discuss/improve.
>
> >  .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
> >  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
> >  2 files changed, 141 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> >  create mode 100644 include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> > new file mode 100644
> > index 0000000000000..0fffa8a68dee4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/nuvoton,wpcm450-clk.yaml
> > @@ -0,0 +1,74 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/nuvoton,wpcm450-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Nuvoton WPCM450 clock controller binding
>
> s/binding//
>
> > +
> > +maintainers:
> > +  - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > +
> > +description:
> > +  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
> > +  supplies clocks and resets to the rest of the chip.
>
> s/This binding describes//
>
> Just describe the hardware.
>
> > +
> > +properties:
> > +  compatible:
> > +    const: nuvoton,wpcm450-clk
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: Reference clock oscillator (should be 48 MHz)
> > +
> > +  clock-names:
> > +    items:
> > +      - const: refclk
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  '#reset-cells':
> > +    const: 1
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - '#clock-cells'
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/nuvoton,wpcm450-clk.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    refclk: clock-48mhz {
> > +      /* 48 MHz reference oscillator */
> > +      compatible = "fixed-clock";
> > +      clock-output-names = "refclk";
> > +      clock-frequency = <48000000>;
> > +      #clock-cells = <0>;
> > +    };
> > +
> > +    clk: clock-controller@b0000200 {
> > +      reg = <0xb0000200 0x100>;
> > +      compatible = "nuvoton,wpcm450-clk";
> > +      clocks = <&refclk>;
> > +      clock-names = "refclk";
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };
> > +
> > +    serial@b8000000 {
> > +      compatible = "nuvoton,wpcm450-uart";
> > +      reg = <0xb8000000 0x20>;
> > +      reg-shift = <2>;
> > +      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
> > +      clocks = <&clk WPCM450_CLK_UART0>;
> > +    };
>
> Skip the consumer example, it's obvious/trivial/duplicating.
>
> > diff --git a/include/dt-bindings/clock/nuvoton,wpcm450-clk.h b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> > new file mode 100644
> > index 0000000000000..86e1c895921b7
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/nuvoton,wpcm450-clk.h
> > @@ -0,0 +1,67 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +
> > +/* Clocks based on CLKEN bits */
> > +#define WPCM450_CLK_FIU            0
> > +#define WPCM450_CLK_XBUS           1
> > +#define WPCM450_CLK_KCS            2
> > +#define WPCM450_CLK_SHM            4
> > +#define WPCM450_CLK_USB1           5
> > +#define WPCM450_CLK_EMC0           6
> > +#define WPCM450_CLK_EMC1           7
> > +#define WPCM450_CLK_USB0           8
> > +#define WPCM450_CLK_PECI           9
> > +#define WPCM450_CLK_AES           10
> > +#define WPCM450_CLK_UART0         11
> > +#define WPCM450_CLK_UART1         12
> > +#define WPCM450_CLK_SMB2          13
> > +#define WPCM450_CLK_SMB3          14
> > +#define WPCM450_CLK_SMB4          15
> > +#define WPCM450_CLK_SMB5          16
> > +#define WPCM450_CLK_HUART         17
> > +#define WPCM450_CLK_PWM           18
> > +#define WPCM450_CLK_TIMER0        19
> > +#define WPCM450_CLK_TIMER1        20
> > +#define WPCM450_CLK_TIMER2        21
> > +#define WPCM450_CLK_TIMER3        22
> > +#define WPCM450_CLK_TIMER4        23
> > +#define WPCM450_CLK_MFT0          24
> > +#define WPCM450_CLK_MFT1          25
> > +#define WPCM450_CLK_WDT           26
> > +#define WPCM450_CLK_ADC           27
> > +#define WPCM450_CLK_SDIO          28
> > +#define WPCM450_CLK_SSPI          29
> > +#define WPCM450_CLK_SMB0          30
> > +#define WPCM450_CLK_SMB1          31
> > +
> > +/* Other clocks */
> > +#define WPCM450_CLK_USBPHY        32
> > +
> > +#define WPCM450_NUM_CLKS          33
> > +
> > +/* Resets based on IPSRST bits */
>
> All these defines should be in second header in dt-bindings/reset/...

I disagree. It makes more sense to keep the definitions together, and
it's all for the same hardware and driver.

>
> > +#define WPCM450_RESET_FIU          0
>
>
> Best regards,
> Krzysztof

Re: [PATCH 3/7] watchdog: npcm: Enable clock if provided

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Fri, Apr 22, 2022 at 11:34:17AM -0700, Guenter Roeck wrote:
> On Fri, Apr 22, 2022 at 08:30:08PM +0200, Jonathan Neuschäfer wrote:
> > On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
> > clocks are individually gated and ungated. Therefore, the watchdog
> > driver must be able to ungate the watchdog clock.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
> >  drivers/watchdog/npcm_wdt.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> > index 28a24caa2627c..6d27f0e16188e 100644
> > --- a/drivers/watchdog/npcm_wdt.c
> > +++ b/drivers/watchdog/npcm_wdt.c
> > @@ -3,6 +3,7 @@
> >  // Copyright (c) 2018 IBM Corp.
> > 
> >  #include <linux/bitops.h>
> > +#include <linux/clk.h>
> >  #include <linux/delay.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/kernel.h>
> > @@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct npcm_wdt *wdt;
> > +	struct clk *clk;
> >  	int irq;
> >  	int ret;
> > 
> > @@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> >  	if (IS_ERR(wdt->reg))
> >  		return PTR_ERR(wdt->reg);
> > 
> > +	clk = devm_clk_get_optional(&pdev->dev, NULL);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	if (clk)
> > +		clk_prepare_enable(clk);
> > +
> 
> This needs a matching clk_disable_unprepare().

Good point. It's probably easiest if I move the clk calls to the
watchdog start/stop callbacks, then.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/7] watchdog: npcm: Enable clock if provided

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Fri, Apr 22, 2022 at 11:34:17AM -0700, Guenter Roeck wrote:
> On Fri, Apr 22, 2022 at 08:30:08PM +0200, Jonathan Neuschäfer wrote:
> > On the Nuvoton WPCM450 SoC, with its upcoming clock driver, peripheral
> > clocks are individually gated and ungated. Therefore, the watchdog
> > driver must be able to ungate the watchdog clock.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
> >  drivers/watchdog/npcm_wdt.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
> > index 28a24caa2627c..6d27f0e16188e 100644
> > --- a/drivers/watchdog/npcm_wdt.c
> > +++ b/drivers/watchdog/npcm_wdt.c
> > @@ -3,6 +3,7 @@
> >  // Copyright (c) 2018 IBM Corp.
> > 
> >  #include <linux/bitops.h>
> > +#include <linux/clk.h>
> >  #include <linux/delay.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/kernel.h>
> > @@ -180,6 +181,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct npcm_wdt *wdt;
> > +	struct clk *clk;
> >  	int irq;
> >  	int ret;
> > 
> > @@ -191,6 +193,13 @@ static int npcm_wdt_probe(struct platform_device *pdev)
> >  	if (IS_ERR(wdt->reg))
> >  		return PTR_ERR(wdt->reg);
> > 
> > +	clk = devm_clk_get_optional(&pdev->dev, NULL);
> > +	if (IS_ERR(clk))
> > +		return PTR_ERR(clk);
> > +
> > +	if (clk)
> > +		clk_prepare_enable(clk);
> > +
> 
> This needs a matching clk_disable_unprepare().

Good point. It's probably easiest if I move the clk calls to the
watchdog start/stop callbacks, then.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hello,

On Sat, Apr 23, 2022 at 11:56:42AM +0200, Krzysztof Kozlowski wrote:
> On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > Add a devicetree binding for it, as well as definitions for the bit
> > numbers used by it.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> >  .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
> >  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
> >  2 files changed, 141 insertions(+)
[...]
> > +title: Nuvoton WPCM450 clock controller binding
> 
> s/binding//

Will change.


> > +description:
> > +  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
> > +  supplies clocks and resets to the rest of the chip.
> 
> s/This binding describes//
> 
> Just describe the hardware.

Ok.


> > +    clk: clock-controller@b0000200 {
> > +      reg = <0xb0000200 0x100>;
> > +      compatible = "nuvoton,wpcm450-clk";
> > +      clocks = <&refclk>;
> > +      clock-names = "refclk";
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };
> > +
> > +    serial@b8000000 {
> > +      compatible = "nuvoton,wpcm450-uart";
> > +      reg = <0xb8000000 0x20>;
> > +      reg-shift = <2>;
> > +      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
> > +      clocks = <&clk WPCM450_CLK_UART0>;
> > +    };
> 
> Skip the consumer example, it's obvious/trivial/duplicating.

Ok.


> > +#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +
> > +/* Clocks based on CLKEN bits */
> > +#define WPCM450_CLK_FIU            0
[...]
> > +/* Other clocks */
> > +#define WPCM450_CLK_USBPHY        32
> > +
> > +#define WPCM450_NUM_CLKS          33
> > +
> > +/* Resets based on IPSRST bits */
> 
> All these defines should be in second header in dt-bindings/reset/...

(my reply is further down in the thread)
> 
> > +#define WPCM450_RESET_FIU          0



Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hello,

On Sat, Apr 23, 2022 at 11:56:42AM +0200, Krzysztof Kozlowski wrote:
> On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > Add a devicetree binding for it, as well as definitions for the bit
> > numbers used by it.
> > 
> > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > ---
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> >  .../bindings/clock/nuvoton,wpcm450-clk.yaml   | 74 +++++++++++++++++++
> >  .../dt-bindings/clock/nuvoton,wpcm450-clk.h   | 67 +++++++++++++++++
> >  2 files changed, 141 insertions(+)
[...]
> > +title: Nuvoton WPCM450 clock controller binding
> 
> s/binding//

Will change.


> > +description:
> > +  This binding describes the clock controller of the Nuvoton WPCM450 SoC, which
> > +  supplies clocks and resets to the rest of the chip.
> 
> s/This binding describes//
> 
> Just describe the hardware.

Ok.


> > +    clk: clock-controller@b0000200 {
> > +      reg = <0xb0000200 0x100>;
> > +      compatible = "nuvoton,wpcm450-clk";
> > +      clocks = <&refclk>;
> > +      clock-names = "refclk";
> > +      #clock-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };
> > +
> > +    serial@b8000000 {
> > +      compatible = "nuvoton,wpcm450-uart";
> > +      reg = <0xb8000000 0x20>;
> > +      reg-shift = <2>;
> > +      interrupts = <7 IRQ_TYPE_LEVEL_HIGH>;
> > +      clocks = <&clk WPCM450_CLK_UART0>;
> > +    };
> 
> Skip the consumer example, it's obvious/trivial/duplicating.

Ok.


> > +#ifndef _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +#define _DT_BINDINGS_CLOCK_NUVOTON_WPCM450_CLK_H
> > +
> > +/* Clocks based on CLKEN bits */
> > +#define WPCM450_CLK_FIU            0
[...]
> > +/* Other clocks */
> > +#define WPCM450_CLK_USBPHY        32
> > +
> > +#define WPCM450_NUM_CLKS          33
> > +
> > +/* Resets based on IPSRST bits */
> 
> All these defines should be in second header in dt-bindings/reset/...

(my reply is further down in the thread)
> 
> > +#define WPCM450_RESET_FIU          0



Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

On Tue, Apr 26, 2022 at 08:35:43AM +0000, Joel Stanley wrote:
> On Mon, 25 Apr 2022 at 07:59, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > > Add a devicetree binding for it, as well as definitions for the bit
> > > numbers used by it.
> > >
> > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > > ---
[...]
> > > +/* Other clocks */
> > > +#define WPCM450_CLK_USBPHY        32
> > > +
> > > +#define WPCM450_NUM_CLKS          33
> > > +
> > > +/* Resets based on IPSRST bits */
> >
> > All these defines should be in second header in dt-bindings/reset/...
> 
> I disagree. It makes more sense to keep the definitions together, and
> it's all for the same hardware and driver.

It's for the same hardware, DT node, and driver.

I could imagine splitting it into

	include/dt-bindings/clock/nuvoton,wpcm450-clk.h  and
	include/dt-bindings/reset/nuvoton,wpcm450-clk.h

if someone insists on it.

For convenience (being able to see all relevant definitions for
nuvoton,wpcm450-clk at once), I'd prefer to keep the definitions together.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]

On Tue, Apr 26, 2022 at 08:35:43AM +0000, Joel Stanley wrote:
> On Mon, 25 Apr 2022 at 07:59, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 22/04/2022 20:30, Jonathan Neuschäfer wrote:
> > > The Nuvoton WPCM450 SoC has a combined clock and reset controller.
> > > Add a devicetree binding for it, as well as definitions for the bit
> > > numbers used by it.
> > >
> > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> > > ---
[...]
> > > +/* Other clocks */
> > > +#define WPCM450_CLK_USBPHY        32
> > > +
> > > +#define WPCM450_NUM_CLKS          33
> > > +
> > > +/* Resets based on IPSRST bits */
> >
> > All these defines should be in second header in dt-bindings/reset/...
> 
> I disagree. It makes more sense to keep the definitions together, and
> it's all for the same hardware and driver.

It's for the same hardware, DT node, and driver.

I could imagine splitting it into

	include/dt-bindings/clock/nuvoton,wpcm450-clk.h  and
	include/dt-bindings/reset/nuvoton,wpcm450-clk.h

if someone insists on it.

For convenience (being able to see all relevant definitions for
nuvoton,wpcm450-clk at once), I'd prefer to keep the definitions together.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Zev Weiss]

On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote:
>In the WPCM450 SoC, the clocks for each timer can be gated individually.
>To prevent the timer 1 clock from being gated, enable it explicitly.
>
>Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
>---
> drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
>index a00520cbb660a..974269b6b0c36 100644
>--- a/drivers/clocksource/timer-npcm7xx.c
>+++ b/drivers/clocksource/timer-npcm7xx.c
>@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
>
> static int __init npcm7xx_timer_init(struct device_node *np)
> {
>+	struct clk *clk;
> 	int ret;
>
> 	ret = timer_of_init(np, &npcm7xx_to);
>-	if (ret)
>+	if (ret) {
>+		pr_warn("timer_of_init failed: %d\n", ret);

This seems like a somewhat opaque message to emit, especially given this
file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
slightly easier to trace back to the device it stems from?

> 		return ret;
>+	}
>
> 	/* Clock input is divided by PRESCALE + 1 before it is fed */
> 	/* to the counter */
> 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> 		(NPCM7XX_Tx_MIN_PRESCALE + 1);
>
>+	/* Enable the clock for timer1, if it exists */
>+	clk = of_clk_get(np, 1);
>+	if (clk) {
>+		if (!IS_ERR(clk))
>+			clk_prepare_enable(clk);
>+		else
>+			pr_warn("Failed to get clock for timer1: %pe", clk);

Likewise here (though to a slightly lesser extent).

>+	}
>+
> 	npcm7xx_clocksource_init();
> 	npcm7xx_clockevents_init();
>
>--
>2.35.1
>

Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Zev Weiss]

On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote:
>In the WPCM450 SoC, the clocks for each timer can be gated individually.
>To prevent the timer 1 clock from being gated, enable it explicitly.
>
>Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
>---
> drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
>index a00520cbb660a..974269b6b0c36 100644
>--- a/drivers/clocksource/timer-npcm7xx.c
>+++ b/drivers/clocksource/timer-npcm7xx.c
>@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
>
> static int __init npcm7xx_timer_init(struct device_node *np)
> {
>+	struct clk *clk;
> 	int ret;
>
> 	ret = timer_of_init(np, &npcm7xx_to);
>-	if (ret)
>+	if (ret) {
>+		pr_warn("timer_of_init failed: %d\n", ret);

This seems like a somewhat opaque message to emit, especially given this
file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
slightly easier to trace back to the device it stems from?

> 		return ret;
>+	}
>
> 	/* Clock input is divided by PRESCALE + 1 before it is fed */
> 	/* to the counter */
> 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> 		(NPCM7XX_Tx_MIN_PRESCALE + 1);
>
>+	/* Enable the clock for timer1, if it exists */
>+	clk = of_clk_get(np, 1);
>+	if (clk) {
>+		if (!IS_ERR(clk))
>+			clk_prepare_enable(clk);
>+		else
>+			pr_warn("Failed to get clock for timer1: %pe", clk);

Likewise here (though to a slightly lesser extent).

>+	}
>+
> 	npcm7xx_clocksource_init();
> 	npcm7xx_clockevents_init();
>
>--
>2.35.1
>

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Krzysztof Kozlowski]

On 28/04/2022 10:55, Jonathan Neuschäfer wrote:
>>>
>>> All these defines should be in second header in dt-bindings/reset/...
>>
>> I disagree. It makes more sense to keep the definitions together, and
>> it's all for the same hardware and driver.

These are bindings so the usage by same driver (Linux implementation)
matters less or even does not matter.

Driver can be split from one to several and you would need to include
clocks in your just-split-reset driver. Such driver split should not
affect bindings, therefore having the binding headers separate is
actually the most flexible.

> 
> It's for the same hardware, DT node, and driver.
> 
> I could imagine splitting it into
> 
> 	include/dt-bindings/clock/nuvoton,wpcm450-clk.h  and
> 	include/dt-bindings/reset/nuvoton,wpcm450-clk.h
> 
> if someone insists on it.
> 
> For convenience (being able to see all relevant definitions for
> nuvoton,wpcm450-clk at once), I'd prefer to keep the definitions together.

I don't insist. For some of the devices we split it, for some not.

Best regards,
Krzysztof

Re: [PATCH 4/7] dt-bindings: clock: Add Nuvoton WPCM450 clock/reset controller

[Krzysztof Kozlowski]

On 28/04/2022 10:55, Jonathan Neuschäfer wrote:
>>>
>>> All these defines should be in second header in dt-bindings/reset/...
>>
>> I disagree. It makes more sense to keep the definitions together, and
>> it's all for the same hardware and driver.

These are bindings so the usage by same driver (Linux implementation)
matters less or even does not matter.

Driver can be split from one to several and you would need to include
clocks in your just-split-reset driver. Such driver split should not
affect bindings, therefore having the binding headers separate is
actually the most flexible.

> 
> It's for the same hardware, DT node, and driver.
> 
> I could imagine splitting it into
> 
> 	include/dt-bindings/clock/nuvoton,wpcm450-clk.h  and
> 	include/dt-bindings/reset/nuvoton,wpcm450-clk.h
> 
> if someone insists on it.
> 
> For convenience (being able to see all relevant definitions for
> nuvoton,wpcm450-clk at once), I'd prefer to keep the definitions together.

I don't insist. For some of the devices we split it, for some not.

Best regards,
Krzysztof

Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1984 bytes --]

On Thu, Apr 28, 2022 at 09:11:58AM +0000, Zev Weiss wrote:
> On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote:
> >In the WPCM450 SoC, the clocks for each timer can be gated individually.
> >To prevent the timer 1 clock from being gated, enable it explicitly.
> >
> >Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> >---
> > drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
> >index a00520cbb660a..974269b6b0c36 100644
> >--- a/drivers/clocksource/timer-npcm7xx.c
> >+++ b/drivers/clocksource/timer-npcm7xx.c
> >@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
> >
> > static int __init npcm7xx_timer_init(struct device_node *np)
> > {
> >+	struct clk *clk;
> > 	int ret;
> >
> > 	ret = timer_of_init(np, &npcm7xx_to);
> >-	if (ret)
> >+	if (ret) {
> >+		pr_warn("timer_of_init failed: %d\n", ret);
> 
> This seems like a somewhat opaque message to emit, especially given this
> file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
> slightly easier to trace back to the device it stems from?

Now that I look at this code again, I think I should just drop the
pr_warn entirely, since I didn't mention it in the description, and it's
unrelated to enabling the clock.

> > 		return ret;
> >+	}
> >
> > 	/* Clock input is divided by PRESCALE + 1 before it is fed */
> > 	/* to the counter */
> > 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> > 		(NPCM7XX_Tx_MIN_PRESCALE + 1);
> >
> >+	/* Enable the clock for timer1, if it exists */
> >+	clk = of_clk_get(np, 1);
> >+	if (clk) {
> >+		if (!IS_ERR(clk))
> >+			clk_prepare_enable(clk);
> >+		else
> >+			pr_warn("Failed to get clock for timer1: %pe", clk);
> 
> Likewise here (though to a slightly lesser extent).

I'll add %pOF here.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/7] clocksource: timer-npcm7xx: Enable timer 1 clock before use

[Jonathan Neuschäfer]

[-- Attachment #1: Type: text/plain, Size: 1984 bytes --]

On Thu, Apr 28, 2022 at 09:11:58AM +0000, Zev Weiss wrote:
> On Fri, Apr 22, 2022 at 11:30:07AM PDT, Jonathan Neuschäfer wrote:
> >In the WPCM450 SoC, the clocks for each timer can be gated individually.
> >To prevent the timer 1 clock from being gated, enable it explicitly.
> >
> >Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
> >---
> > drivers/clocksource/timer-npcm7xx.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
> >index a00520cbb660a..974269b6b0c36 100644
> >--- a/drivers/clocksource/timer-npcm7xx.c
> >+++ b/drivers/clocksource/timer-npcm7xx.c
> >@@ -188,17 +188,29 @@ static void __init npcm7xx_clocksource_init(void)
> >
> > static int __init npcm7xx_timer_init(struct device_node *np)
> > {
> >+	struct clk *clk;
> > 	int ret;
> >
> > 	ret = timer_of_init(np, &npcm7xx_to);
> >-	if (ret)
> >+	if (ret) {
> >+		pr_warn("timer_of_init failed: %d\n", ret);
> 
> This seems like a somewhat opaque message to emit, especially given this
> file's lack of a pr_fmt() definition -- maybe add a %pOF so it's
> slightly easier to trace back to the device it stems from?

Now that I look at this code again, I think I should just drop the
pr_warn entirely, since I didn't mention it in the description, and it's
unrelated to enabling the clock.

> > 		return ret;
> >+	}
> >
> > 	/* Clock input is divided by PRESCALE + 1 before it is fed */
> > 	/* to the counter */
> > 	npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
> > 		(NPCM7XX_Tx_MIN_PRESCALE + 1);
> >
> >+	/* Enable the clock for timer1, if it exists */
> >+	clk = of_clk_get(np, 1);
> >+	if (clk) {
> >+		if (!IS_ERR(clk))
> >+			clk_prepare_enable(clk);
> >+		else
> >+			pr_warn("Failed to get clock for timer1: %pe", clk);
> 
> Likewise here (though to a slightly lesser extent).

I'll add %pOF here.


Thanks,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]