[PATCH v2 00/20] Introduce Nuvoton Arbel NPCM8XX BMC SoC

[PATCH v2 00/20] Introduce Nuvoton Arbel NPCM8XX BMC SoC

[Tomer Maimon]

This patchset  adds initial support for the Nuvoton 
Arbel NPCM8XX Board Management controller (BMC) SoC family. 

The Nuvoton Arbel NPCM8XX SoC is a fourth-generation BMC.
The NPCM8XX computing subsystem comprises a quadcore ARM 
Cortex A35 ARM-V8 architecture.

This patchset adds minimal architecture and drivers such as:
Clocksource, Clock, Reset, and WD.

Some of the Arbel NPCM8XX peripherals are based on Poleg NPCM7XX.

This patchset was tested on the Arbel NPCM8XX evaluation board.

Addressed comments from:
 - Arnd Bergmann : https://www.spinics.net/lists/arm-kernel/msg982728.html
		   https://www.spinics.net/lists/linux-serial/msg48179.html
 - Stephen Boyd: https://www.spinics.net/lists/arm-kernel/msg983594.html
 - Krzysztof Kozlowski: https://www.spinics.net/lists/kernel/msg4368955.html
			https://www.spinics.net/lists/arm-kernel/msg982427.html
			https://lore.kernel.org/all/973d75b8-0eb6-ff5b-6cd2-9b7d7c5cbcaa@linaro.org/
			https://www.spinics.net/lists/arm-kernel/msg982778.html
			https://www.spinics.net/lists/arm-kernel/msg982588.html
 - Ilpo Järvinen: https://www.spinics.net/lists/kernel/msg4368936.html
 - Stephen Boyd: https://www.spinics.net/lists/arm-kernel/msg983596.html
 - Geert Uytterhoeven : https://www.spinics.net/lists/kernel/msg4369514.html

Changes since version 1:
 - NPCM8XX clock driver
	- Modify dt-binding.
	- Remove unsed definition and include.
	- Include alphabetically.
	- Use clock devm.
 - NPCM reset driver
	- Modify dt-binding.
	- Modify syscon name.
	- Add syscon support to NPCM7XX dts reset node.
	- use data structure.
 - NPCM8XX device tree:
	- Modify evb compatible name.
	- Add NPCM7xx compatible.
	- Remove disable nodes from the EVB DTS.

Tomer Maimon (20):
  clocksource: timer-npcm7xx: Add NPCM845 timer
  dt-bindings: serial: 8250: Add npcm845 compatible string
  tty: serial: 8250: Add NPCM845 UART support
  dt-bindings: watchdog: npcm: Add npcm845 compatible string
  watchdog: npcm_wdt: Add NPCM845 watchdog support
  dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock
  clk: npcm8xx: add clock controller
  dt-bindings: reset: modify to general NPCM name
  dt-bindings: reset: npcm: add GCR syscon property
  ARM: dts: nuvoton: add reset syscon property
  reset: npcm: using syscon instead of device data
  dt-bindings: reset: npcm: Add support for NPCM8XX
  reset: npcm: Add NPCM8XX support
  dt-bindings: arm: npcm: Add maintainer
  dt-bindings: arm: npcm: Add nuvoton,npcm845 compatible string
  dt-bindings: arm: npcm: Add nuvoton,npcm845 GCR compatible string
  arm64: npcm: Add support for Nuvoton NPCM8XX BMC SoC
  arm64: dts: nuvoton: Add initial NPCM8XX device tree
  arm64: dts: nuvoton: Add initial NPCM845 EVB device tree
  arm64: defconfig: Add Nuvoton NPCM family support

 .../devicetree/bindings/arm/npcm/npcm.yaml    |   7 +
 .../bindings/arm/npcm/nuvoton,gcr.yaml        |   2 +
 .../bindings/clock/nuvoton,npcm845-clk.yaml   |  63 ++
 ...750-reset.yaml => nuvoton,npcm-reset.yaml} |  21 +-
 .../devicetree/bindings/serial/8250.yaml      |   1 +
 .../bindings/watchdog/nuvoton,npcm-wdt.txt    |   3 +-
 MAINTAINERS                                   |   3 +
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |   1 +
 arch/arm64/Kconfig.platforms                  |  11 +
 arch/arm64/boot/dts/Makefile                  |   1 +
 arch/arm64/boot/dts/nuvoton/Makefile          |   2 +
 .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi   | 197 +++++
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts  |  30 +
 .../boot/dts/nuvoton/nuvoton-npcm845.dtsi     |  76 ++
 arch/arm64/configs/defconfig                  |   3 +
 drivers/clk/Kconfig                           |   6 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-npcm8xx.c                     | 756 ++++++++++++++++++
 drivers/clocksource/timer-npcm7xx.c           |   1 +
 drivers/reset/reset-npcm.c                    | 202 ++++-
 drivers/tty/serial/8250/8250_of.c             |   1 +
 drivers/watchdog/npcm_wdt.c                   |   1 +
 .../dt-bindings/clock/nuvoton,npcm8xx-clock.h |  50 ++
 .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 +++
 24 files changed, 1530 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
 rename Documentation/devicetree/bindings/reset/{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml} (58%)
 create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi
 create mode 100644 drivers/clk/clk-npcm8xx.c
 create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
 create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h

-- 
2.33.0

[PATCH v2 01/20] clocksource: timer-npcm7xx: Add NPCM845 timer

[Tomer Maimon]

Add Nuvoton BMC NPCM845 timer support.
The NPCM845 uses the same timer controller as the NPCM750.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/clocksource/timer-npcm7xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
index a00520cbb660..1630daad4df5 100644
--- a/drivers/clocksource/timer-npcm7xx.c
+++ b/drivers/clocksource/timer-npcm7xx.c
@@ -210,4 +210,5 @@ static int __init npcm7xx_timer_init(struct device_node *np)
 
 TIMER_OF_DECLARE(wpcm450, "nuvoton,wpcm450-timer", npcm7xx_timer_init);
 TIMER_OF_DECLARE(npcm7xx, "nuvoton,npcm750-timer", npcm7xx_timer_init);
+TIMER_OF_DECLARE(npcm8xx, "nuvoton,npcm845-timer", npcm7xx_timer_init);
 
-- 
2.33.0

[PATCH v2 02/20] dt-bindings: serial: 8250: Add npcm845 compatible string

[Tomer Maimon]

Add a compatible string for Nuvoton BMC NPCM845 UART.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 5f6b113d378f..6258f5f59b19 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -62,6 +62,7 @@ properties:
       - const: mrvl,pxa-uart
       - const: nuvoton,wpcm450-uart
       - const: nuvoton,npcm750-uart
+      - const: nuvoton,npcm845-uart
       - const: nvidia,tegra20-uart
       - const: nxp,lpc3220-uart
       - items:
-- 
2.33.0

[PATCH v2 03/20] tty: serial: 8250: Add NPCM845 UART support

[Tomer Maimon]

Add Nuvoton BMC NPCM845 UART support.
The NPCM845 uses the same UART as the NPCM750.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/tty/serial/8250/8250_of.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index 5a699a1aa79c..edf83c90b4d5 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -335,6 +335,7 @@ static const struct of_device_id of_platform_serial_table[] = {
 	{ .compatible = "ti,da830-uart", .data = (void *)PORT_DA830, },
 	{ .compatible = "nuvoton,wpcm450-uart", .data = (void *)PORT_NPCM, },
 	{ .compatible = "nuvoton,npcm750-uart", .data = (void *)PORT_NPCM, },
+	{ .compatible = "nuvoton,npcm845-uart", .data = (void *)PORT_NPCM, },
 	{ /* end of list */ },
 };
 MODULE_DEVICE_TABLE(of, of_platform_serial_table);
-- 
2.33.0

[PATCH v2 04/20] dt-bindings: watchdog: npcm: Add npcm845 compatible string

[Tomer Maimon]

Add a compatible string for Nuvoton BMC NPCM845 watchdog.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt          | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
index 9059f54dc023..866a958b8a2b 100644
--- a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
@@ -6,7 +6,8 @@ expiry.
 
 Required properties:
 - compatible      : "nuvoton,npcm750-wdt" for NPCM750 (Poleg), or
-                    "nuvoton,wpcm450-wdt" for WPCM450 (Hermon).
+                    "nuvoton,wpcm450-wdt" for WPCM450 (Hermon), or
+                    "nuvoton,npcm845-wdt" for NPCM845 (Arbel).
 - reg             : Offset and length of the register set for the device.
 - interrupts      : Contain the timer interrupt with flags for
                     falling edge.
-- 
2.33.0

[PATCH v2 05/20] watchdog: npcm_wdt: Add NPCM845 watchdog support

[Tomer Maimon]

Add Nuvoton BMC NPCM845 watchdog support.
The NPCM845 uses the same watchdog as the NPCM750.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/watchdog/npcm_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/npcm_wdt.c b/drivers/watchdog/npcm_wdt.c
index 28a24caa2627..0b91a3fbec09 100644
--- a/drivers/watchdog/npcm_wdt.c
+++ b/drivers/watchdog/npcm_wdt.c
@@ -231,6 +231,7 @@ static int npcm_wdt_probe(struct platform_device *pdev)
 static const struct of_device_id npcm_wdt_match[] = {
 	{.compatible = "nuvoton,wpcm450-wdt"},
 	{.compatible = "nuvoton,npcm750-wdt"},
+	{.compatible = "nuvoton,npcm845-wdt"},
 	{},
 };
 MODULE_DEVICE_TABLE(of, npcm_wdt_match);
-- 
2.33.0

[PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Tomer Maimon]

Add binding for the Arbel BMC NPCM8XX Clock controller.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
 .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
 create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h

diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
new file mode 100644
index 000000000000..e1f375716bc5
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton NPCM8XX Clock Controller Binding
+
+maintainers:
+  - Tomer Maimon <tmaimon77@gmail.com>
+
+description: |
+  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
+  generates and supplies clocks to all modules within the BMC.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,npcm845-clk
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: 25M reference clock
+      - description: CPU reference clock
+      - description: MC reference clock
+
+  clock-names:
+    items:
+      - const: refclk
+      - const: sysbypck
+      - const: mcbypck
+
+  '#clock-cells':
+    const: 1
+    description:
+      See include/dt-bindings/clock/nuvoton,npcm8xx-clock.h for the full
+      list of NPCM8XX clock IDs.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    ahb {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        clock-controller@f0801000 {
+            compatible = "nuvoton,npcm845-clk";
+            reg = <0x0 0xf0801000 0x0 0x1000>;
+            clocks = <&clk_refclk>, <&clk_sysbypck>, <&clk_mcbypck>;
+            #clock-cells = <1>;
+        };
+    };
+...
diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
new file mode 100644
index 000000000000..229915a254a5
--- /dev/null
+++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Nuvoton NPCM8xx Clock Generator binding
+ * clock binding number for all clocks supportted by nuvoton,npcm8xx-clk
+ *
+ * Copyright (C) 2021 Nuvoton Technologies.
+ * Author: Tomer Maimon <tomer.maimon@nuvoton.com>
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_NPCM8XX_H
+#define __DT_BINDINGS_CLOCK_NPCM8XX_H
+
+#define NPCM8XX_CLK_CPU		0
+#define NPCM8XX_CLK_GFX_PIXEL	1
+#define NPCM8XX_CLK_MC		2
+#define NPCM8XX_CLK_ADC		3
+#define NPCM8XX_CLK_AHB		4
+#define NPCM8XX_CLK_TIMER	5
+#define NPCM8XX_CLK_UART	6
+#define NPCM8XX_CLK_UART2	7
+#define NPCM8XX_CLK_MMC		8
+#define NPCM8XX_CLK_SPI3	9
+#define NPCM8XX_CLK_PCI		10
+#define NPCM8XX_CLK_AXI		11
+#define NPCM8XX_CLK_APB4	12
+#define NPCM8XX_CLK_APB3	13
+#define NPCM8XX_CLK_APB2	14
+#define NPCM8XX_CLK_APB1	15
+#define NPCM8XX_CLK_APB5	16
+#define NPCM8XX_CLK_CLKOUT	17
+#define NPCM8XX_CLK_GFX		18
+#define NPCM8XX_CLK_SU		19
+#define NPCM8XX_CLK_SU48	20
+#define NPCM8XX_CLK_SDHC	21
+#define NPCM8XX_CLK_SPI0	22
+#define NPCM8XX_CLK_SPI1	23
+#define NPCM8XX_CLK_SPIX	24
+#define NPCM8XX_CLK_RG		25
+#define NPCM8XX_CLK_RCP		26
+#define NPCM8XX_CLK_PRE_ADC	27
+#define NPCM8XX_CLK_ATB		28
+#define NPCM8XX_CLK_PRE_CLK	29
+#define NPCM8XX_CLK_TH		30
+#define NPCM8XX_CLK_REFCLK	31
+#define NPCM8XX_CLK_SYSBYPCK	32
+#define NPCM8XX_CLK_MCBYPCK	33
+
+#define NPCM8XX_NUM_CLOCKS	(NPCM8XX_CLK_MCBYPCK + 1)
+
+#endif
-- 
2.33.0

[PATCH v2 07/20] clk: npcm8xx: add clock controller

[Tomer Maimon]

Nuvoton Arbel BMC NPCM8XX contains an integrated clock
controller which generates and supplies clocks to all
modules within the BMC.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/clk/Kconfig       |   6 +
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-npcm8xx.c | 756 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 763 insertions(+)
 create mode 100644 drivers/clk/clk-npcm8xx.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 48f8f4221e21..9aa915f6e233 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -428,6 +428,12 @@ config COMMON_CLK_K210
 	help
 	  Support for the Canaan Kendryte K210 RISC-V SoC clocks.
 
+config COMMON_CLK_NPCM8XX
+	tristate "Clock driver for the NPCM8XX SoC Family"
+	depends on ARCH_NPCM || COMPILE_TEST
+	help
+          This driver supports the clocks on the Nuvoton BMC NPCM8XX SoC Family.
+
 source "drivers/clk/actions/Kconfig"
 source "drivers/clk/analogbits/Kconfig"
 source "drivers/clk/baikal-t1/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d5db170d38d2..b0ec1184a744 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_ARCH_MILBEAUT_M10V)	+= clk-milbeaut.o
 obj-$(CONFIG_ARCH_MOXART)		+= clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
 obj-$(CONFIG_ARCH_NPCM7XX)	    	+= clk-npcm7xx.o
+obj-$(CONFIG_COMMON_CLK_NPCM8XX)	+= clk-npcm8xx.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= clk-nspire.o
 obj-$(CONFIG_COMMON_CLK_OXNAS)		+= clk-oxnas.o
 obj-$(CONFIG_COMMON_CLK_PALMAS)		+= clk-palmas.o
diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c
new file mode 100644
index 000000000000..40340c3611b5
--- /dev/null
+++ b/drivers/clk/clk-npcm8xx.c
@@ -0,0 +1,756 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nuvoton NPCM8xx Clock Generator
+ * All the clocks are initialized by the bootloader, so this driver allow only
+ * reading of current settings directly from the hardware.
+ *
+ * Copyright (C) 2020 Nuvoton Technologies
+ * Author: Tomer Maimon <tomer.maimon@nuvoton.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
+
+struct npcm8xx_clk_pll {
+	struct clk_hw	hw;
+	void __iomem	*pllcon;
+	u8		flags;
+};
+
+#define to_npcm8xx_clk_pll(_hw) container_of(_hw, struct npcm8xx_clk_pll, hw)
+
+#define PLLCON_LOKI	BIT(31)
+#define PLLCON_LOKS	BIT(30)
+#define PLLCON_FBDV	GENMASK(27, 16)
+#define PLLCON_OTDV2	GENMASK(15, 13)
+#define PLLCON_PWDEN	BIT(12)
+#define PLLCON_OTDV1	GENMASK(10, 8)
+#define PLLCON_INDV	GENMASK(5, 0)
+
+static unsigned long npcm8xx_clk_pll_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct npcm8xx_clk_pll *pll = to_npcm8xx_clk_pll(hw);
+	unsigned long fbdv, indv, otdv1, otdv2;
+	unsigned int val;
+	u64 ret;
+
+	if (parent_rate == 0) {
+		pr_debug("%s: parent rate is zero", __func__);
+		return 0;
+	}
+
+	val = readl_relaxed(pll->pllcon);
+
+	indv = FIELD_GET(PLLCON_INDV, val);
+	fbdv = FIELD_GET(PLLCON_FBDV, val);
+	otdv1 = FIELD_GET(PLLCON_OTDV1, val);
+	otdv2 = FIELD_GET(PLLCON_OTDV2, val);
+
+	ret = (u64)parent_rate * fbdv;
+	do_div(ret, indv * otdv1 * otdv2);
+
+	return ret;
+}
+
+static const struct clk_ops npcm8xx_clk_pll_ops = {
+	.recalc_rate = npcm8xx_clk_pll_recalc_rate,
+};
+
+static struct clk_hw *
+npcm8xx_clk_register_pll(struct device *dev, void __iomem *pllcon,
+			 const char *name, const char *parent_name,
+			 unsigned long flags)
+{
+	struct npcm8xx_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);
+
+	pr_debug("%s reg, name=%s, p=%s\n", __func__, name, parent_name);
+
+	init.name = name;
+	init.ops = &npcm8xx_clk_pll_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll->pllcon = pllcon;
+	pll->hw.init = &init;
+
+	hw = &pll->hw;
+
+	ret = devm_clk_hw_register(dev, hw);
+	if (ret) {
+		kfree(pll);
+		return ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
+#define NPCM8XX_CLKEN1          (0x00)
+#define NPCM8XX_CLKEN2          (0x28)
+#define NPCM8XX_CLKEN3          (0x30)
+#define NPCM8XX_CLKEN4          (0x70)
+#define NPCM8XX_CLKSEL          (0x04)
+#define NPCM8XX_CLKDIV1         (0x08)
+#define NPCM8XX_CLKDIV2         (0x2C)
+#define NPCM8XX_CLKDIV3         (0x58)
+#define NPCM8XX_CLKDIV4         (0x7C)
+#define NPCM8XX_PLLCON0         (0x0C)
+#define NPCM8XX_PLLCON1         (0x10)
+#define NPCM8XX_PLLCON2         (0x54)
+#define NPCM8XX_SWRSTR          (0x14)
+#define NPCM8XX_IRQWAKECON      (0x18)
+#define NPCM8XX_IRQWAKEFLAG     (0x1C)
+#define NPCM8XX_IPSRST1         (0x20)
+#define NPCM8XX_IPSRST2         (0x24)
+#define NPCM8XX_IPSRST3         (0x34)
+#define NPCM8XX_WD0RCR          (0x38)
+#define NPCM8XX_WD1RCR          (0x3C)
+#define NPCM8XX_WD2RCR          (0x40)
+#define NPCM8XX_SWRSTC1         (0x44)
+#define NPCM8XX_SWRSTC2         (0x48)
+#define NPCM8XX_SWRSTC3         (0x4C)
+#define NPCM8XX_SWRSTC4         (0x50)
+#define NPCM8XX_CORSTC          (0x5C)
+#define NPCM8XX_PLLCONG         (0x60)
+#define NPCM8XX_AHBCKFI         (0x64)
+#define NPCM8XX_SECCNT          (0x68)
+#define NPCM8XX_CNTR25M         (0x6C)
+#define NPCM8XX_THRTL_CNT       (0xC0)
+
+struct npcm8xx_clk_gate_data {
+	u32 reg;
+	u8 bit_idx;
+	const char *name;
+	const char *parent_name;
+	unsigned long flags;
+	/*
+	 * If this clock is exported via DT, set onecell_idx to constant
+	 * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for
+	 * this specific clock.  Otherwise, set to -1.
+	 */
+	int onecell_idx;
+};
+
+struct npcm8xx_clk_mux_data {
+	u8 shift;
+	u8 mask;
+	u32 *table;
+	const char *name;
+	const char * const *parent_names;
+	u8 num_parents;
+	unsigned long flags;
+	/*
+	 * If this clock is exported via DT, set onecell_idx to constant
+	 * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for
+	 * this specific clock.  Otherwise, set to -1.
+	 */
+	int onecell_idx;
+
+};
+
+struct npcm8xx_clk_div_fixed_data {
+	u8 mult;
+	u8 div;
+	const char *name;
+	const char *parent_name;
+	u8 clk_divider_flags;
+	/*
+	 * If this clock is exported via DT, set onecell_idx to constant
+	 * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for
+	 * this specific clock.  Otherwise, set to -1.
+	 */
+	int onecell_idx;
+};
+
+struct npcm8xx_clk_div_data {
+	u32 reg;
+	u8 shift;
+	u8 width;
+	const char *name;
+	const char *parent_name;
+	u8 clk_divider_flags;
+	unsigned long flags;
+	/*
+	 * If this clock is exported via DT, set onecell_idx to constant
+	 * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for
+	 * this specific clock.  Otherwise, set to -1.
+	 */
+	int onecell_idx;
+};
+
+struct npcm8xx_clk_pll_data {
+	u32 reg;
+	const char *name;
+	const char *parent_name;
+	unsigned long flags;
+	/*
+	 * If this clock is exported via DT, set onecell_idx to constant
+	 * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for
+	 * this specific clock.  Otherwise, set to -1.
+	 */
+	int onecell_idx;
+};
+
+/*
+ * Single copy of strings used to refer to clocks within this driver indexed by
+ * above enum.
+ */
+#define NPCM8XX_CLK_S_REFCLK      "refclk"
+#define NPCM8XX_CLK_S_SYSBYPCK    "sysbypck"
+#define NPCM8XX_CLK_S_MCBYPCK     "mcbypck"
+#define NPCM8XX_CLK_S_PLL0        "pll0"
+#define NPCM8XX_CLK_S_PLL1        "pll1"
+#define NPCM8XX_CLK_S_PLL1_DIV2   "pll1_div2"
+#define NPCM8XX_CLK_S_PLL2        "pll2"
+#define NPCM8XX_CLK_S_PLL_GFX     "pll_gfx"
+#define NPCM8XX_CLK_S_PLL2_DIV2   "pll2_div2"
+#define NPCM8XX_CLK_S_PIX_MUX     "gfx_pixel"
+#define NPCM8XX_CLK_S_MC_MUX      "mc_phy"
+#define NPCM8XX_CLK_S_CPU_MUX     "cpu"  /* AKA system clock */
+#define NPCM8XX_CLK_S_MC          "mc"
+#define NPCM8XX_CLK_S_AXI         "axi"  /* AKA CLK2 */
+#define NPCM8XX_CLK_S_AHB         "ahb"  /* AKA CLK4 */
+#define NPCM8XX_CLK_S_CLKOUT_MUX  "clkout_mux"
+#define NPCM8XX_CLK_S_UART_MUX    "uart_mux"
+#define NPCM8XX_CLK_S_SD_MUX      "sd_mux"
+#define NPCM8XX_CLK_S_GFXM_MUX    "gfxm_mux"
+#define NPCM8XX_CLK_S_SU_MUX      "serial_usb_mux"
+#define NPCM8XX_CLK_S_DVC_MUX     "dvc_mux"
+#define NPCM8XX_CLK_S_GFX_MUX     "gfx_mux"
+#define NPCM8XX_CLK_S_ADC_MUX     "adc_mux"
+#define NPCM8XX_CLK_S_SPI0        "spi0"
+#define NPCM8XX_CLK_S_SPI1        "spi1"
+#define NPCM8XX_CLK_S_SPI3        "spi3"
+#define NPCM8XX_CLK_S_SPIX        "spix"
+#define NPCM8XX_CLK_S_APB1        "apb1"
+#define NPCM8XX_CLK_S_APB2        "apb2"
+#define NPCM8XX_CLK_S_APB3        "apb3"
+#define NPCM8XX_CLK_S_APB4        "apb4"
+#define NPCM8XX_CLK_S_APB5        "apb5"
+#define NPCM8XX_CLK_S_APB19       "apb19"
+#define NPCM8XX_CLK_S_TOCK        "tock"
+#define NPCM8XX_CLK_S_CLKOUT      "clkout"
+#define NPCM8XX_CLK_S_PRE_ADC     "pre adc"
+#define NPCM8XX_CLK_S_UART        "uart"
+#define NPCM8XX_CLK_S_UART2       "uart2"
+#define NPCM8XX_CLK_S_TIMER       "timer"
+#define NPCM8XX_CLK_S_MMC         "mmc"
+#define NPCM8XX_CLK_S_SDHC        "sdhc"
+#define NPCM8XX_CLK_S_ADC         "adc"
+#define NPCM8XX_CLK_S_GFX         "gfx0_gfx1_mem"
+#define NPCM8XX_CLK_S_USBIF       "serial_usbif"
+#define NPCM8XX_CLK_S_USB_HOST    "usb_host"
+#define NPCM8XX_CLK_S_USB_BRIDGE  "usb_bridge"
+#define NPCM8XX_CLK_S_PCI         "pci"
+#define NPCM8XX_CLK_S_TH          "th"
+#define NPCM8XX_CLK_S_ATB         "atb"
+#define NPCM8XX_CLK_S_PRE_CLK     "pre_clk"
+
+#define NPCM8XX_CLK_S_RG_MUX	  "rg_mux"
+#define NPCM8XX_CLK_S_RCP_MUX	  "rcp_mux"
+#define NPCM8XX_CLK_S_RG	  "rg"
+#define NPCM8XX_CLK_S_RCP	  "rcp"
+
+static u32 pll_mux_table[] = {0, 1, 2, 3};
+static const char * const pll_mux_parents[] = {
+	NPCM8XX_CLK_S_PLL0,
+	NPCM8XX_CLK_S_PLL1,
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_PLL2_DIV2,
+};
+
+static u32 cpuck_mux_table[] = {0, 1, 2, 3, 7};
+static const char * const cpuck_mux_parents[] = {
+	NPCM8XX_CLK_S_PLL0,
+	NPCM8XX_CLK_S_PLL1,
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_SYSBYPCK,
+	NPCM8XX_CLK_S_PLL2,
+};
+
+static u32 pixcksel_mux_table[] = {0, 2};
+static const char * const pixcksel_mux_parents[] = {
+	NPCM8XX_CLK_S_PLL_GFX,
+	NPCM8XX_CLK_S_REFCLK,
+};
+
+static u32 sucksel_mux_table[] = {2, 3};
+static const char * const sucksel_mux_parents[] = {
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_PLL2_DIV2,
+};
+
+static u32 mccksel_mux_table[] = {0, 2, 3};
+static const char * const mccksel_mux_parents[] = {
+	NPCM8XX_CLK_S_PLL1_DIV2,
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_MCBYPCK,
+};
+
+static u32 clkoutsel_mux_table[] = {0, 1, 2, 3, 4};
+static const char * const clkoutsel_mux_parents[] = {
+	NPCM8XX_CLK_S_PLL0,
+	NPCM8XX_CLK_S_PLL1,
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_PLL_GFX, // divided by 2
+	NPCM8XX_CLK_S_PLL2_DIV2,
+};
+
+static u32 gfxmsel_mux_table[] = {2, 3};
+static const char * const gfxmsel_mux_parents[] = {
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_PLL2_DIV2,
+};
+
+static u32 dvcssel_mux_table[] = {2, 3};
+static const char * const dvcssel_mux_parents[] = {
+	NPCM8XX_CLK_S_REFCLK,
+	NPCM8XX_CLK_S_PLL2,
+};
+
+static const struct npcm8xx_clk_pll_data npcm8xx_plls[] = {
+	{NPCM8XX_PLLCON0, NPCM8XX_CLK_S_PLL0, NPCM8XX_CLK_S_REFCLK, 0, -1},
+
+	{NPCM8XX_PLLCON1, NPCM8XX_CLK_S_PLL1,
+	NPCM8XX_CLK_S_REFCLK, 0, -1},
+
+	{NPCM8XX_PLLCON2, NPCM8XX_CLK_S_PLL2,
+	NPCM8XX_CLK_S_REFCLK, 0, -1},
+
+	{NPCM8XX_PLLCONG, NPCM8XX_CLK_S_PLL_GFX,
+	NPCM8XX_CLK_S_REFCLK, 0, -1},
+};
+
+static const struct npcm8xx_clk_mux_data npcm8xx_muxes[] = {
+	{0, GENMASK(1, 0), cpuck_mux_table, NPCM8XX_CLK_S_CPU_MUX,
+	cpuck_mux_parents, ARRAY_SIZE(cpuck_mux_parents), CLK_IS_CRITICAL,
+	NPCM8XX_CLK_CPU},
+
+	{4, GENMASK(1, 0), pixcksel_mux_table, NPCM8XX_CLK_S_PIX_MUX,
+	pixcksel_mux_parents, ARRAY_SIZE(pixcksel_mux_parents), 0,
+	NPCM8XX_CLK_GFX_PIXEL},
+
+	{6, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_SD_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+
+	{8, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_UART_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+
+	{10, GENMASK(1, 0), sucksel_mux_table, NPCM8XX_CLK_S_SU_MUX,
+	sucksel_mux_parents, ARRAY_SIZE(sucksel_mux_parents), 0, -1},
+
+	{12, GENMASK(1, 0), mccksel_mux_table, NPCM8XX_CLK_S_MC_MUX,
+	mccksel_mux_parents, ARRAY_SIZE(mccksel_mux_parents), 0, -1},
+
+	{14, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_ADC_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+
+	{16, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_GFX_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+
+	{18, GENMASK(2, 0), clkoutsel_mux_table, NPCM8XX_CLK_S_CLKOUT_MUX,
+	clkoutsel_mux_parents, ARRAY_SIZE(clkoutsel_mux_parents), 0, -1},
+
+	{21, GENMASK(1, 0), gfxmsel_mux_table, NPCM8XX_CLK_S_GFXM_MUX,
+	gfxmsel_mux_parents, ARRAY_SIZE(gfxmsel_mux_parents), 0, -1},
+
+	{23, GENMASK(1, 0), dvcssel_mux_table, NPCM8XX_CLK_S_DVC_MUX,
+	dvcssel_mux_parents, ARRAY_SIZE(dvcssel_mux_parents), 0, -1},
+
+	{25, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_RG_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+
+	{27, GENMASK(1, 0), pll_mux_table, NPCM8XX_CLK_S_RCP_MUX,
+	pll_mux_parents, ARRAY_SIZE(pll_mux_parents), 0, -1},
+};
+
+/* fixed ratio dividers (no register): */
+static const struct npcm8xx_clk_div_fixed_data npcm8xx_divs_fx[] = {
+	{ 1, 2, NPCM8XX_CLK_S_MC, NPCM8XX_CLK_S_MC_MUX, 0, NPCM8XX_CLK_MC},
+	{ 1, 2, NPCM8XX_CLK_S_AXI, NPCM8XX_CLK_S_TH, 0, NPCM8XX_CLK_AXI},
+	{ 1, 2, NPCM8XX_CLK_S_ATB, NPCM8XX_CLK_S_AXI, 0, NPCM8XX_CLK_ATB},
+	{ 1, 2, NPCM8XX_CLK_S_PRE_CLK, NPCM8XX_CLK_S_CPU_MUX, 0, -1},
+	{ 1, 2, NPCM8XX_CLK_S_PLL1_DIV2, NPCM8XX_CLK_S_PLL1, 0, -1},
+	{ 1, 2, NPCM8XX_CLK_S_PLL2_DIV2, NPCM8XX_CLK_S_PLL2, 0, -1},
+};
+
+/* configurable dividers: */
+static const struct npcm8xx_clk_div_data npcm8xx_divs[] = {
+	{NPCM8XX_CLKDIV1, 28, 3, NPCM8XX_CLK_S_ADC, NPCM8XX_CLK_S_PRE_ADC,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_ADC},
+	/* bit 30-28 ADCCKDIV*/
+	{NPCM8XX_CLKDIV1, 26, 2, NPCM8XX_CLK_S_AHB, NPCM8XX_CLK_S_PRE_CLK,
+		CLK_DIVIDER_READ_ONLY, CLK_IS_CRITICAL, NPCM8XX_CLK_AHB},
+	/* bit 28-26 CLK4DIV*/
+	{NPCM8XX_CLKDIV1, 21, 5, NPCM8XX_CLK_S_PRE_ADC,
+	NPCM8XX_CLK_S_ADC_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_PRE_ADC},
+	/* bit 25-21 PRE-ADCCKDIV*/
+	{NPCM8XX_CLKDIV1, 16, 5, NPCM8XX_CLK_S_UART,
+	NPCM8XX_CLK_S_UART_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_UART},
+	/* bit 20-16 UARTDIV*/
+	{NPCM8XX_CLKDIV1, 11, 5, NPCM8XX_CLK_S_MMC,
+	NPCM8XX_CLK_S_SD_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_MMC},
+	/* bit 15-11 MMCCKDIV*/
+	{NPCM8XX_CLKDIV1, 6, 5, NPCM8XX_CLK_S_SPI3,
+	NPCM8XX_CLK_S_AHB, 0, 0, NPCM8XX_CLK_SPI3},
+	/* bit 10-6 AHB3CKDIV*/
+	{NPCM8XX_CLKDIV1, 2, 4, NPCM8XX_CLK_S_PCI,
+	NPCM8XX_CLK_S_GFX_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_PCI},
+	/* bit 5-2 PCICKDIV*/
+
+	{NPCM8XX_CLKDIV2, 30, 2, NPCM8XX_CLK_S_APB4, NPCM8XX_CLK_S_AHB,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_APB4},
+	/* bit 31-30 APB4CKDIV*/
+	{NPCM8XX_CLKDIV2, 28, 2, NPCM8XX_CLK_S_APB3, NPCM8XX_CLK_S_AHB,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_APB3},
+	/* bit 29-28 APB3CKDIV*/
+	{NPCM8XX_CLKDIV2, 26, 2, NPCM8XX_CLK_S_APB2, NPCM8XX_CLK_S_AHB,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_APB2},
+	/* bit 28-26 APB2CKDIV*/
+	{NPCM8XX_CLKDIV2, 24, 2, NPCM8XX_CLK_S_APB1, NPCM8XX_CLK_S_AHB,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_APB1},
+	/* bit 25-24 APB1CKDIV*/
+	{NPCM8XX_CLKDIV2, 22, 2, NPCM8XX_CLK_S_APB5, NPCM8XX_CLK_S_AHB,
+		CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0,
+		NPCM8XX_CLK_APB5},
+	/* bit 23-22 APB5CKDIV*/
+	{NPCM8XX_CLKDIV2, 16, 5, NPCM8XX_CLK_S_CLKOUT, NPCM8XX_CLK_S_CLKOUT_MUX,
+		 CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_CLKOUT},
+	/* bit 20-16 CLKOUTDIV*/
+	{NPCM8XX_CLKDIV2, 13, 3, NPCM8XX_CLK_S_GFX, NPCM8XX_CLK_S_GFX_MUX,
+		CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_GFX},
+	/* bit 15-13 GFXCKDIV*/
+	{NPCM8XX_CLKDIV2, 8, 5, NPCM8XX_CLK_S_USB_BRIDGE, NPCM8XX_CLK_S_SU_MUX,
+		CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SU},
+	/* bit 12-8 SUCKDIV*/
+	{NPCM8XX_CLKDIV2, 4, 4, NPCM8XX_CLK_S_USB_HOST, NPCM8XX_CLK_S_SU_MUX,
+		CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SU48},
+	/* bit 8-4 SU48CKDIV*/
+	{NPCM8XX_CLKDIV2, 0, 4, NPCM8XX_CLK_S_SDHC,
+	NPCM8XX_CLK_S_SD_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SDHC}
+	,/* bit 3-0 SD1CKDIV*/
+
+	{NPCM8XX_CLKDIV3, 16, 8, NPCM8XX_CLK_S_SPI1,
+	NPCM8XX_CLK_S_AHB, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SPI1},
+	/* bit 23-16 SPI1CKDV*/
+	{NPCM8XX_CLKDIV3, 11, 5, NPCM8XX_CLK_S_UART2,
+	NPCM8XX_CLK_S_UART_MUX, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_UART2},
+	/* bit 15-11 UARTDIV2*/
+	{NPCM8XX_CLKDIV3, 6, 5, NPCM8XX_CLK_S_SPI0,
+	NPCM8XX_CLK_S_AHB, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SPI0},
+	/* bit 10-6 SPI0CKDV*/
+	{NPCM8XX_CLKDIV3, 1, 5, NPCM8XX_CLK_S_SPIX,
+	NPCM8XX_CLK_S_AHB, CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_SPIX},
+
+	/* bit 5-1 SPIXCKDV*/
+	{NPCM8XX_CLKDIV4, 28, 4, NPCM8XX_CLK_S_RG, NPCM8XX_CLK_S_RG_MUX,
+	CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_RG},
+
+	/* bit 31-28 RGREFDIV*/
+	{NPCM8XX_CLKDIV4, 12, 4, NPCM8XX_CLK_S_RCP, NPCM8XX_CLK_S_RCP_MUX,
+	CLK_DIVIDER_READ_ONLY, 0, NPCM8XX_CLK_RCP},
+
+	/* bit 15-12 RCPREFDIV*/
+	{NPCM8XX_THRTL_CNT, 0, 2, NPCM8XX_CLK_S_TH, NPCM8XX_CLK_S_CPU_MUX,
+	CLK_DIVIDER_READ_ONLY | CLK_DIVIDER_POWER_OF_TWO, 0, NPCM8XX_CLK_TH},
+	/* bit 1-0 TH_DIV*/
+};
+
+static const struct npcm8xx_clk_gate_data npcm8xx_gates[] = {
+	{NPCM8XX_CLKEN1, 31, "smb1-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 30, "smb0-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 29, "smb7-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 28, "smb6-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 27, "adc-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 26, "wdt-gate", NPCM8XX_CLK_S_TIMER, 0},
+	{NPCM8XX_CLKEN1, 25, "usbdev3-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 24, "usbdev6-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 23, "usbdev5-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 22, "usbdev4-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 21, "gmac4-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 20, "timer5_9-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 19, "timer0_4-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 18, "pwmm0-gate", NPCM8XX_CLK_S_APB3, 0},
+	{NPCM8XX_CLKEN1, 17, "huart-gate", NPCM8XX_CLK_S_UART, 0},
+	{NPCM8XX_CLKEN1, 16, "smb5-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 15, "smb4-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 14, "smb3-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 13, "smb2-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN1, 12, "mc-gate", NPCM8XX_CLK_S_MC, 0},
+	{NPCM8XX_CLKEN1, 11, "uart01-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 10, "aes-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 9, "peci-gate", NPCM8XX_CLK_S_APB3, 0},
+	{NPCM8XX_CLKEN1, 8, "usbdev2-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 7, "uart23-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 6, "gmac3-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 5, "usbdev1-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 4, "shm-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 3, "gdma0-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 2, "kcs-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN1, 1, "spi3-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN1, 0, "spi0-gate", NPCM8XX_CLK_S_AHB, 0},
+
+	{NPCM8XX_CLKEN2, 31, "cp-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 30, "tock-gate", NPCM8XX_CLK_S_TOCK, 0},
+	/* bit 29 is reserved */
+	{NPCM8XX_CLKEN2, 28, "gmac1-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 27, "usbif-gate", NPCM8XX_CLK_S_USBIF, 0},
+	{NPCM8XX_CLKEN2, 26, "usbhost1-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 25, "gmac2-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 24, "spi1-gate", NPCM8XX_CLK_S_APB5, 0},
+	{NPCM8XX_CLKEN2, 23, "pspi2-gate", NPCM8XX_CLK_S_APB5, 0},
+	/* bit 22 is reserved */
+	{NPCM8XX_CLKEN2, 21, "3des-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 20, "bt-gate", NPCM8XX_CLK_S_APB3, 0},
+	{NPCM8XX_CLKEN2, 19, "siox2-gate", NPCM8XX_CLK_S_APB3, 0},
+	{NPCM8XX_CLKEN2, 18, "siox1-gate", NPCM8XX_CLK_S_APB3, 0},
+	{NPCM8XX_CLKEN2, 17, "viruart2-gate", NPCM8XX_CLK_S_APB5, 0},
+	{NPCM8XX_CLKEN2, 16, "viruart1-gate", NPCM8XX_CLK_S_APB5, 0},
+	/*  bit 15 is reserved */
+	{NPCM8XX_CLKEN2, 14, "vcd-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 13, "ece-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 12, "vdma-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 11, "ahbpcibrg-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 10, "gfxsys-gate", NPCM8XX_CLK_S_APB1, 0},
+	//{NPCM8XX_CLKEN2, 9, "sdhc-gate", NPCM8XX_CLK_S_AHB, 0},
+	/*  bit 9 is reserved */
+	{NPCM8XX_CLKEN2, 8, "mmc-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN2, 7, "mft7-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 6, "mft6-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 5, "mft5-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 4, "mft4-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 3, "mft3-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 2, "mft2-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 1, "mft1-gate", NPCM8XX_CLK_S_APB4, 0},
+	{NPCM8XX_CLKEN2, 0, "mft0-gate", NPCM8XX_CLK_S_APB4, 0},
+
+	{NPCM8XX_CLKEN3, 31, "gpiom7-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 30, "gpiom6-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 29, "gpiom5-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 28, "gpiom4-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 27, "gpiom3-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 26, "gpiom2-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 25, "gpiom1-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 24, "gpiom0-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 23, "espi-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 22, "smb11-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 21, "smb10-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 20, "smb9-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 19, "smb8-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 18, "smb15-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 17, "rng-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 16, "timer10_14-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN3, 15, "pcirc-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 14, "sececc-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 13, "sha-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 12, "smb14-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 11, "gdma2-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 10, "gdma1-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 9, "pcimbx-gate", NPCM8XX_CLK_S_AHB, 0},
+	/* bit 8 is reserved */
+	{NPCM8XX_CLKEN3, 7, "usbdev9-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 6, "usbdev8-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 5, "usbdev7-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 4, "usbdev0-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 3, "smb13-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 2, "spix-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN3, 1, "smb12-gate", NPCM8XX_CLK_S_APB2, 0},
+	{NPCM8XX_CLKEN3, 0, "pwmm1-gate", NPCM8XX_CLK_S_APB3, 0},
+
+	{NPCM8XX_CLKEN4, 31, "usbhost2-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN4, 30, "jtm2-gate", NPCM8XX_CLK_S_APB5, 0},
+	{NPCM8XX_CLKEN4, 29, "jtm1-gate", NPCM8XX_CLK_S_APB5, 0},
+	{NPCM8XX_CLKEN4, 28, "pwmm2-gate", NPCM8XX_CLK_S_APB3, 0},
+	/* bit 27 is reserved */
+	/* bit 26 is reserved */
+	/* bit 25 is reserved */
+	{NPCM8XX_CLKEN4, 24, "smb26-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 23, "smb25-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 22, "smb24-gate", NPCM8XX_CLK_S_APB19, 0},
+	/* bit 21 is reserved */
+	/* bit 20 is reserved */
+	{NPCM8XX_CLKEN4, 19, "pcimbx2-gate", NPCM8XX_CLK_S_AHB, 0},
+	{NPCM8XX_CLKEN4, 18, "uart6-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN4, 17, "uart5-gate", NPCM8XX_CLK_S_APB1, 0},
+	{NPCM8XX_CLKEN4, 16, "uart4-gate", NPCM8XX_CLK_S_APB1, 0},
+	/* bit 15 is reserved */
+	/* bit 14 is reserved */
+	{NPCM8XX_CLKEN4, 13, "i3c5-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 12, "i3c4-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 11, "i3c3-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 10, "i3c2-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 9, "i3c1-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 8, "i3c0-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 7, "smb23-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 6, "smb22-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 5, "smb21-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 4, "smb20-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 3, "smb19-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 2, "smb18-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 1, "smb17-gate", NPCM8XX_CLK_S_APB19, 0},
+	{NPCM8XX_CLKEN4, 0, "smb16-gate", NPCM8XX_CLK_S_APB19, 0},
+
+};
+
+static DEFINE_SPINLOCK(npcm8xx_clk_lock);
+
+static int npcm8xx_clk_probe(struct platform_device *pdev)
+{
+	struct clk_hw_onecell_data *npcm8xx_clk_data;
+	struct device *dev = &pdev->dev;
+	void __iomem *clk_base;
+	struct clk_hw *hw;
+	int i;
+
+	npcm8xx_clk_data = devm_kzalloc(dev, struct_size(npcm8xx_clk_data, hws,
+							 NPCM8XX_NUM_CLOCKS),
+					GFP_KERNEL);
+	if (!npcm8xx_clk_data)
+		return -ENOMEM;
+
+	clk_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(clk_base))
+		return PTR_ERR(clk_base);
+
+	npcm8xx_clk_data->num = NPCM8XX_NUM_CLOCKS;
+
+	for (i = 0; i < NPCM8XX_NUM_CLOCKS; i++)
+		npcm8xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
+
+	/* Register plls */
+	for (i = 0; i < ARRAY_SIZE(npcm8xx_plls); i++) {
+		const struct npcm8xx_clk_pll_data *pll_data = &npcm8xx_plls[i];
+
+		hw = npcm8xx_clk_register_pll(dev, clk_base + pll_data->reg,
+					      pll_data->name,
+					      pll_data->parent_name,
+					      pll_data->flags);
+		if (IS_ERR(hw)) {
+			dev_err(dev, "npcm8xx_clk: Can't register pll\n");
+			return PTR_ERR(hw);
+		}
+
+		if (pll_data->onecell_idx >= 0)
+			npcm8xx_clk_data->hws[pll_data->onecell_idx] = hw;
+	}
+
+	/* Register fixed dividers */
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PLL1_DIV2,
+					       NPCM8XX_CLK_S_PLL1, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register fixed div\n");
+		return PTR_ERR(hw);
+	}
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PLL2_DIV2,
+					       NPCM8XX_CLK_S_PLL2, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register pll div2\n");
+		return PTR_ERR(hw);
+	}
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PRE_CLK,
+					       NPCM8XX_CLK_S_CPU_MUX, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register ckclk div2\n");
+		return PTR_ERR(hw);
+	}
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_AXI,
+					       NPCM8XX_CLK_S_TH, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register axi div2\n");
+		return PTR_ERR(hw);
+	}
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_ATB,
+					       NPCM8XX_CLK_S_AXI, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register atb div2\n");
+		return PTR_ERR(hw);
+	}
+
+	/* Register muxes */
+	for (i = 0; i < ARRAY_SIZE(npcm8xx_muxes); i++) {
+		const struct npcm8xx_clk_mux_data *mux_data = &npcm8xx_muxes[i];
+
+		hw = clk_hw_register_mux_table(dev, mux_data->name,
+					       mux_data->parent_names,
+					       mux_data->num_parents,
+					       mux_data->flags,
+					       clk_base + NPCM8XX_CLKSEL,
+					       mux_data->shift,
+					       mux_data->mask, 0,
+					       mux_data->table,
+					       &npcm8xx_clk_lock);
+
+		if (IS_ERR(hw)) {
+			dev_err(dev, "npcm8xx_clk: Can't register mux\n");
+			return PTR_ERR(hw);
+		}
+
+		if (mux_data->onecell_idx >= 0)
+			npcm8xx_clk_data->hws[mux_data->onecell_idx] = hw;
+	}
+
+	/* Register clock dividers specified in npcm8xx_divs */
+	for (i = 0; i < ARRAY_SIZE(npcm8xx_divs); i++) {
+		const struct npcm8xx_clk_div_data *div_data = &npcm8xx_divs[i];
+
+		hw = clk_hw_register_divider(dev, div_data->name,
+					     div_data->parent_name,
+					     div_data->flags,
+					     clk_base + div_data->reg,
+					     div_data->shift, div_data->width,
+					     div_data->clk_divider_flags,
+					     &npcm8xx_clk_lock);
+		if (IS_ERR(hw)) {
+			dev_err(dev, "npcm8xx_clk: Can't register div table\n");
+			return PTR_ERR(hw);
+		}
+
+		if (div_data->onecell_idx >= 0)
+			npcm8xx_clk_data->hws[div_data->onecell_idx] = hw;
+	}
+
+	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					   npcm8xx_clk_data);
+}
+
+static const struct of_device_id npcm8xx_clk_dt_ids[] = {
+	{ .compatible = "nuvoton,npcm845-clk", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, npcm8xx_clk_dt_ids);
+
+static struct platform_driver npcm8xx_clk_driver = {
+	.probe  = npcm8xx_clk_probe,
+	.driver = {
+		.name = "npcm8xx_clk",
+		.of_match_table = npcm8xx_clk_dt_ids,
+	},
+};
+builtin_platform_driver(npcm8xx_clk_driver);
+
-- 
2.33.0

[PATCH v2 08/20] dt-bindings: reset: modify to general NPCM name

[Tomer Maimon]

Modify nuvoton,npcm750-reset specific name to
general NPCM file name, nuvoton,npcm-reset.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml}     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/reset/{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml} (93%)

diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
similarity index 93%
rename from Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
rename to Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
index fa5e4ea6400e..0998f481578d 100644
--- a/Documentation/devicetree/bindings/reset/nuvoton,npcm750-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/reset/nuvoton,npcm750-reset.yaml#
+$id: http://devicetree.org/schemas/reset/nuvoton,npcm-reset.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Nuvoton NPCM Reset controller
-- 
2.33.0

[PATCH v2 09/20] dt-bindings: reset: npcm: add GCR syscon property

[Tomer Maimon]

Describe syscon property that handles general
control registers(GCR) in Nuvoton BMC NPCM
reset driver.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../devicetree/bindings/reset/nuvoton,npcm-reset.yaml       | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
index 0998f481578d..c6bbc1589ab9 100644
--- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
@@ -19,6 +19,10 @@ properties:
   '#reset-cells':
     const: 2
 
+  nuvoton,sysgcr:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description: a phandle to access GCR registers.
+
   nuvoton,sw-reset-number:
     $ref: /schemas/types.yaml#/definitions/uint32
     minimum: 1
@@ -31,6 +35,7 @@ required:
   - compatible
   - reg
   - '#reset-cells'
+  - nuvoton,sysgcr
 
 additionalProperties: false
 
@@ -41,6 +46,7 @@ examples:
         compatible = "nuvoton,npcm750-reset";
         reg = <0xf0801000 0x70>;
         #reset-cells = <2>;
+        nuvoton,sysgcr = <&gcr>;
         nuvoton,sw-reset-number = <2>;
     };
 
-- 
2.33.0

[PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Tomer Maimon]

Add nuvoton,sysgcr syscon property to the reset
node to handle the general control registers.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
index 3696980a3da1..8a2f29016291 100644
--- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
@@ -110,6 +110,7 @@ rstc: rstc@f0801000 {
 			compatible = "nuvoton,npcm750-reset";
 			reg = <0xf0801000 0x70>;
 			#reset-cells = <2>;
+			nuvoton,sysgcr = <&gcr>;
 		};
 
 		clk: clock-controller@f0801000 {
-- 
2.33.0

[PATCH v2 11/20] reset: npcm: using syscon instead of device data

[Tomer Maimon]

Using syscon device tree property instead of
device data to handle the NPCM general control
registers.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/reset/reset-npcm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
index 2ea4d3136e15..312c3b594b8f 100644
--- a/drivers/reset/reset-npcm.c
+++ b/drivers/reset/reset-npcm.c
@@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev,
 }
 
 static const struct of_device_id npcm_rc_match[] = {
-	{ .compatible = "nuvoton,npcm750-reset",
-		.data = (void *)"nuvoton,npcm750-gcr" },
+	{ .compatible = "nuvoton,npcm750-reset"},
 	{ }
 };
 
@@ -155,14 +154,10 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
 	u32 ipsrst1_bits = 0;
 	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
 	u32 ipsrst3_bits = 0;
-	const char *gcr_dt;
 
-	gcr_dt = (const char *)
-	of_match_device(dev->driver->of_match_table, dev)->data;
-
-	gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt);
+	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr");
 	if (IS_ERR(gcr_regmap)) {
-		dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt);
+		dev_err(&pdev->dev, "Failed to find gcr syscon");
 		return PTR_ERR(gcr_regmap);
 	}
 
-- 
2.33.0

[PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Tomer Maimon]

Add binding document and device tree binding
constants for Nuvoton BMC NPCM8XX reset controller.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 .../bindings/reset/nuvoton,npcm-reset.yaml    |  13 +-
 .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 ++++++++++++++++++
 2 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h

diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
index c6bbc1589ab9..93ea81686f58 100644
--- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
@@ -9,9 +9,20 @@ title: Nuvoton NPCM Reset controller
 maintainers:
   - Tomer Maimon <tmaimon77@gmail.com>
 
+description: |
+  The NPCM reset controller used to reset various set of peripherals. 
+  Please refer to reset.txt in this directory for common reset
+  controller binding usage.
+
+  For list of all valid reset indices see
+    <dt-bindings/reset/nuvoton,npcm7xx-reset.h> for Poleg NPCM7XX SoC,
+    <dt-bindings/reset/nuvoton,npcm8xx-reset.h> for Arbel NPCM8XX SoC.
+
 properties:
   compatible:
-    const: nuvoton,npcm750-reset
+    enum: 
+      - nuvoton,npcm750-reset        # Poleg NPCM7XX SoC
+      - nuvoton,npcm845-reset        # Arbel NPCM8XX SoC
 
   reg:
     maxItems: 1
diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
new file mode 100644
index 000000000000..5b3b74534b50
--- /dev/null
+++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2022 Nuvoton Technology corporation.
+ * Author: Tomer Maimon <tmaimon77@gmail.com>
+ */
+
+#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
+#define _DT_BINDINGS_NPCM8XX_RESET_H
+
+/* represent reset register offset */
+#define NPCM8XX_RESET_IPSRST1		0x20
+#define NPCM8XX_RESET_IPSRST2		0x24
+#define NPCM8XX_RESET_IPSRST3		0x34
+#define NPCM8XX_RESET_IPSRST4		0x74
+
+/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
+#define NPCM8XX_RESET_GDMA0		3
+#define NPCM8XX_RESET_UDC1		5
+#define NPCM8XX_RESET_GMAC3		6
+#define NPCM8XX_RESET_UART_2_3		7
+#define NPCM8XX_RESET_UDC2		8
+#define NPCM8XX_RESET_PECI		9
+#define NPCM8XX_RESET_AES		10
+#define NPCM8XX_RESET_UART_0_1		11
+#define NPCM8XX_RESET_MC		12
+#define NPCM8XX_RESET_SMB2		13
+#define NPCM8XX_RESET_SMB3		14
+#define NPCM8XX_RESET_SMB4		15
+#define NPCM8XX_RESET_SMB5		16
+#define NPCM8XX_RESET_PWM_M0		18
+#define NPCM8XX_RESET_TIMER_0_4		19
+#define NPCM8XX_RESET_TIMER_5_9		20
+#define NPCM8XX_RESET_GMAC4		21
+#define NPCM8XX_RESET_UDC4		22
+#define NPCM8XX_RESET_UDC5		23
+#define NPCM8XX_RESET_UDC6		24
+#define NPCM8XX_RESET_UDC3		25
+#define NPCM8XX_RESET_ADC		27
+#define NPCM8XX_RESET_SMB6		28
+#define NPCM8XX_RESET_SMB7		29
+#define NPCM8XX_RESET_SMB0		30
+#define NPCM8XX_RESET_SMB1		31
+
+/* Reset lines on IP2 reset module (NPCM8XX_RESET_IPSRST2) */
+#define NPCM8XX_RESET_MFT0		0
+#define NPCM8XX_RESET_MFT1		1
+#define NPCM8XX_RESET_MFT2		2
+#define NPCM8XX_RESET_MFT3		3
+#define NPCM8XX_RESET_MFT4		4
+#define NPCM8XX_RESET_MFT5		5
+#define NPCM8XX_RESET_MFT6		6
+#define NPCM8XX_RESET_MFT7		7
+#define NPCM8XX_RESET_MMC		8
+#define NPCM8XX_RESET_GFX_SYS		10
+#define NPCM8XX_RESET_AHB_PCIBRG	11
+#define NPCM8XX_RESET_VDMA		12
+#define NPCM8XX_RESET_ECE		13
+#define NPCM8XX_RESET_VCD		14
+#define NPCM8XX_RESET_VIRUART1		16
+#define NPCM8XX_RESET_VIRUART2		17
+#define NPCM8XX_RESET_SIOX1		18
+#define NPCM8XX_RESET_SIOX2		19
+#define NPCM8XX_RESET_BT		20
+#define NPCM8XX_RESET_3DES		21
+#define NPCM8XX_RESET_PSPI2		23
+#define NPCM8XX_RESET_GMAC2		25
+#define NPCM8XX_RESET_USBH1		26
+#define NPCM8XX_RESET_GMAC1		28
+#define NPCM8XX_RESET_CP1		31
+
+/* Reset lines on IP3 reset module (NPCM8XX_RESET_IPSRST3) */
+#define NPCM8XX_RESET_PWM_M1		0
+#define NPCM8XX_RESET_SMB12		1
+#define NPCM8XX_RESET_SPIX		2
+#define NPCM8XX_RESET_SMB13		3
+#define NPCM8XX_RESET_UDC0		4
+#define NPCM8XX_RESET_UDC7		5
+#define NPCM8XX_RESET_UDC8		6
+#define NPCM8XX_RESET_UDC9		7
+#define NPCM8XX_RESET_USBHUB		8
+#define NPCM8XX_RESET_PCI_MAILBOX	9
+#define NPCM8XX_RESET_GDMA1		10
+#define NPCM8XX_RESET_GDMA2		11
+#define NPCM8XX_RESET_SMB14		12
+#define NPCM8XX_RESET_SHA		13
+#define NPCM8XX_RESET_SEC_ECC		14
+#define NPCM8XX_RESET_PCIE_RC		15
+#define NPCM8XX_RESET_TIMER_10_14	16
+#define NPCM8XX_RESET_RNG		17
+#define NPCM8XX_RESET_SMB15		18
+#define NPCM8XX_RESET_SMB8		19
+#define NPCM8XX_RESET_SMB9		20
+#define NPCM8XX_RESET_SMB10		21
+#define NPCM8XX_RESET_SMB11		22
+#define NPCM8XX_RESET_ESPI		23
+#define NPCM8XX_RESET_USB_PHY_1		24
+#define NPCM8XX_RESET_USB_PHY_2		25
+
+/* Reset lines on IP4 reset module (NPCM8XX_RESET_IPSRST4) */
+#define NPCM8XX_RESET_SMB16		0
+#define NPCM8XX_RESET_SMB17		1
+#define NPCM8XX_RESET_SMB18		2
+#define NPCM8XX_RESET_SMB19		3
+#define NPCM8XX_RESET_SMB20		4
+#define NPCM8XX_RESET_SMB21		5
+#define NPCM8XX_RESET_SMB22		6
+#define NPCM8XX_RESET_SMB23		7
+#define NPCM8XX_RESET_I3C0		8
+#define NPCM8XX_RESET_I3C1		9
+#define NPCM8XX_RESET_I3C2		10
+#define NPCM8XX_RESET_I3C3		11
+#define NPCM8XX_RESET_I3C4		12
+#define NPCM8XX_RESET_I3C5		13
+#define NPCM8XX_RESET_UART4		16
+#define NPCM8XX_RESET_UART5		17
+#define NPCM8XX_RESET_UART6		18
+#define NPCM8XX_RESET_PCIMBX2		19
+#define NPCM8XX_RESET_SMB24		22
+#define NPCM8XX_RESET_SMB25		23
+#define NPCM8XX_RESET_SMB26		24
+#define NPCM8XX_RESET_USBPHY3		25
+#define NPCM8XX_RESET_PCIRCPHY		27
+#define NPCM8XX_RESET_PWM_M2		28
+#define NPCM8XX_RESET_JTM1		29
+#define NPCM8XX_RESET_JTM2		30
+#define NPCM8XX_RESET_USBH2		31
+
+#endif
-- 
2.33.0

[PATCH v2 13/20] reset: npcm: Add NPCM8XX support

[Tomer Maimon]

Updated the NPCM reset driver to add
support for Nuvoton BMC NPCM8XX SoC.
As part of adding NPCM8XX support
- Add NPCM8XX specific compatible string.
- Add NPCM8XX USB reset.
- Add data to handle architecture-specific reset parameters.
- Some of the Reset Id and number of resets are
  different from NPCM7XX.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/reset/reset-npcm.c | 197 +++++++++++++++++++++++++++++++------
 1 file changed, 168 insertions(+), 29 deletions(-)

diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
index 312c3b594b8f..861be56593ee 100644
--- a/drivers/reset/reset-npcm.c
+++ b/drivers/reset/reset-npcm.c
@@ -17,13 +17,20 @@
 
 /* NPCM7xx GCR registers */
 #define NPCM_MDLR_OFFSET	0x7C
-#define NPCM_MDLR_USBD0		BIT(9)
-#define NPCM_MDLR_USBD1		BIT(8)
-#define NPCM_MDLR_USBD2_4	BIT(21)
-#define NPCM_MDLR_USBD5_9	BIT(22)
+#define NPCM7XX_MDLR_USBD0	BIT(9)
+#define NPCM7XX_MDLR_USBD1	BIT(8)
+#define NPCM7XX_MDLR_USBD2_4	BIT(21)
+#define NPCM7XX_MDLR_USBD5_9	BIT(22)
+
+/* NPCM8xx MDLR bits */
+#define NPCM8XX_MDLR_USBD0_3	BIT(9)
+#define NPCM8XX_MDLR_USBD4_7	BIT(22)
+#define NPCM8XX_MDLR_USBD8	BIT(24)
+#define NPCM8XX_MDLR_USBD9	BIT(21)
 
 #define NPCM_USB1PHYCTL_OFFSET	0x140
 #define NPCM_USB2PHYCTL_OFFSET	0x144
+#define NPCM_USB3PHYCTL_OFFSET	0x148
 #define NPCM_USBXPHYCTL_RS	BIT(28)
 
 /* NPCM7xx Reset registers */
@@ -49,12 +56,38 @@
 #define NPCM_IPSRST3_USBPHY1	BIT(24)
 #define NPCM_IPSRST3_USBPHY2	BIT(25)
 
+#define NPCM_IPSRST4		0x74
+#define NPCM_IPSRST4_USBPHY3	BIT(25)
+#define NPCM_IPSRST4_USB_HOST2	BIT(31)
+
 #define NPCM_RC_RESETS_PER_REG	32
 #define NPCM_MASK_RESETS	GENMASK(4, 0)
 
+enum {
+	BMC_NPCM7XX = 0,
+	BMC_NPCM8XX,
+};
+
+static const u32 npxm7xx_ipsrst[] = {NPCM_IPSRST1, NPCM_IPSRST2, NPCM_IPSRST3};
+static const u32 npxm8xx_ipsrst[] = {NPCM_IPSRST1, NPCM_IPSRST2, NPCM_IPSRST3,
+	NPCM_IPSRST4};
+
+struct npcm_reset_info {
+	u32 bmc_id;
+	u32 num_ipsrst;
+	const u32 *ipsrst;
+};
+
+static const struct npcm_reset_info npxm7xx_reset_info[] = {
+	{.bmc_id = BMC_NPCM7XX, .num_ipsrst = 3, .ipsrst = npxm7xx_ipsrst}};
+static const struct npcm_reset_info npxm8xx_reset_info[] = {
+	{.bmc_id = BMC_NPCM8XX, .num_ipsrst = 4, .ipsrst = npxm8xx_ipsrst}};
+
 struct npcm_rc_data {
 	struct reset_controller_dev rcdev;
 	struct notifier_block restart_nb;
+	const struct npcm_reset_info *info;
+	struct regmap *gcr_regmap;
 	u32 sw_reset_number;
 	void __iomem *base;
 	spinlock_t lock;
@@ -120,14 +153,24 @@ static int npcm_rc_status(struct reset_controller_dev *rcdev,
 static int npcm_reset_xlate(struct reset_controller_dev *rcdev,
 			    const struct of_phandle_args *reset_spec)
 {
+	struct npcm_rc_data *rc = to_rc_data(rcdev);
 	unsigned int offset, bit;
+	bool offset_found = false;
+	int off_num;
 
 	offset = reset_spec->args[0];
-	if (offset != NPCM_IPSRST1 && offset != NPCM_IPSRST2 &&
-	    offset != NPCM_IPSRST3) {
+	for (off_num = 0 ; off_num < rc->info->num_ipsrst ; off_num++) {
+		if (offset == rc->info->ipsrst[off_num]) {
+			offset_found = true;
+			break;
+		}
+	}
+
+	if (!offset_found) {
 		dev_err(rcdev->dev, "Error reset register (0x%x)\n", offset);
 		return -EINVAL;
 	}
+
 	bit = reset_spec->args[1];
 	if (bit >= NPCM_RC_RESETS_PER_REG) {
 		dev_err(rcdev->dev, "Error reset number (%d)\n", bit);
@@ -138,40 +181,29 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev,
 }
 
 static const struct of_device_id npcm_rc_match[] = {
-	{ .compatible = "nuvoton,npcm750-reset"},
+	{ .compatible = "nuvoton,npcm750-reset", .data = &npxm7xx_reset_info},
+	{ .compatible = "nuvoton,npcm845-reset", .data = &npxm8xx_reset_info},
 	{ }
 };
 
-/*
- *  The following procedure should be observed in USB PHY, USB device and
- *  USB host initialization at BMC boot
- */
-static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
+static void npcm_usb_reset_npcm7xx(struct npcm_rc_data *rc)
 {
 	u32 mdlr, iprst1, iprst2, iprst3;
-	struct device *dev = &pdev->dev;
-	struct regmap *gcr_regmap;
 	u32 ipsrst1_bits = 0;
 	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
 	u32 ipsrst3_bits = 0;
 
-	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr");
-	if (IS_ERR(gcr_regmap)) {
-		dev_err(&pdev->dev, "Failed to find gcr syscon");
-		return PTR_ERR(gcr_regmap);
-	}
-
 	/* checking which USB device is enabled */
-	regmap_read(gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
-	if (!(mdlr & NPCM_MDLR_USBD0))
+	regmap_read(rc->gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
+	if (!(mdlr & NPCM7XX_MDLR_USBD0))
 		ipsrst3_bits |= NPCM_IPSRST3_USBD0;
-	if (!(mdlr & NPCM_MDLR_USBD1))
+	if (!(mdlr & NPCM7XX_MDLR_USBD1))
 		ipsrst1_bits |= NPCM_IPSRST1_USBD1;
-	if (!(mdlr & NPCM_MDLR_USBD2_4))
+	if (!(mdlr & NPCM7XX_MDLR_USBD2_4))
 		ipsrst1_bits |= (NPCM_IPSRST1_USBD2 |
 				 NPCM_IPSRST1_USBD3 |
 				 NPCM_IPSRST1_USBD4);
-	if (!(mdlr & NPCM_MDLR_USBD0)) {
+	if (!(mdlr & NPCM7XX_MDLR_USBD0)) {
 		ipsrst1_bits |= (NPCM_IPSRST1_USBD5 |
 				 NPCM_IPSRST1_USBD6);
 		ipsrst3_bits |= (NPCM_IPSRST3_USBD7 |
@@ -194,9 +226,9 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
 	writel(iprst3, rc->base + NPCM_IPSRST3);
 
 	/* clear USB PHY RS bit */
-	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
 			   NPCM_USBXPHYCTL_RS, 0);
-	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
 			   NPCM_USBXPHYCTL_RS, 0);
 
 	/* deassert reset USB PHY */
@@ -206,19 +238,126 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
 	udelay(50);
 
 	/* set USB PHY RS bit */
-	regmap_update_bits(gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
+
+	/* deassert reset USB devices*/
+	iprst1 &= ~ipsrst1_bits;
+	iprst2 &= ~ipsrst2_bits;
+	iprst3 &= ~ipsrst3_bits;
+
+	writel(iprst1, rc->base + NPCM_IPSRST1);
+	writel(iprst2, rc->base + NPCM_IPSRST2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+}
+
+static void npcm_usb_reset_npcm8xx(struct npcm_rc_data *rc)
+{
+	u32 mdlr, iprst1, iprst2, iprst3, iprst4;
+	u32 ipsrst1_bits = 0;
+	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
+	u32 ipsrst3_bits = 0;
+	u32 ipsrst4_bits = NPCM_IPSRST4_USB_HOST2 | NPCM_IPSRST4_USBPHY3;
+
+	/* checking which USB device is enabled */
+	regmap_read(rc->gcr_regmap, NPCM_MDLR_OFFSET, &mdlr);
+	if (!(mdlr & NPCM8XX_MDLR_USBD0_3)) {
+		ipsrst3_bits |= NPCM_IPSRST3_USBD0;
+		ipsrst1_bits |= (NPCM_IPSRST1_USBD1 |
+				 NPCM_IPSRST1_USBD2 |
+				 NPCM_IPSRST1_USBD3);
+	}
+	if (!(mdlr & NPCM8XX_MDLR_USBD4_7)) {
+		ipsrst1_bits |= (NPCM_IPSRST1_USBD4 |
+				 NPCM_IPSRST1_USBD5 |
+				 NPCM_IPSRST1_USBD6);
+		ipsrst3_bits |= NPCM_IPSRST3_USBD7;
+	}
+
+	if (!(mdlr & NPCM8XX_MDLR_USBD8))
+		ipsrst3_bits |= NPCM_IPSRST3_USBD8;
+	if (!(mdlr & NPCM8XX_MDLR_USBD9))
+		ipsrst3_bits |= NPCM_IPSRST3_USBD9;
+
+	/* assert reset USB PHY and USB devices */
+	iprst1 = readl(rc->base + NPCM_IPSRST1);
+	iprst2 = readl(rc->base + NPCM_IPSRST2);
+	iprst3 = readl(rc->base + NPCM_IPSRST3);
+	iprst4 = readl(rc->base + NPCM_IPSRST4);
+
+	iprst1 |= ipsrst1_bits;
+	iprst2 |= ipsrst2_bits;
+	iprst3 |= (ipsrst3_bits | NPCM_IPSRST3_USBPHY1 |
+		   NPCM_IPSRST3_USBPHY2);
+	iprst2 |= ipsrst4_bits;
+
+	writel(iprst1, rc->base + NPCM_IPSRST1);
+	writel(iprst2, rc->base + NPCM_IPSRST2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+	writel(iprst4, rc->base + NPCM_IPSRST4);
+
+	/* clear USB PHY RS bit */
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, 0);
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, 0);
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB3PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, 0);
+
+	/* deassert reset USB PHY */
+	iprst3 &= ~(NPCM_IPSRST3_USBPHY1 | NPCM_IPSRST3_USBPHY2);
+	writel(iprst3, rc->base + NPCM_IPSRST3);
+	iprst4 &= ~NPCM_IPSRST4_USBPHY3;
+	writel(iprst4, rc->base + NPCM_IPSRST4);
+
+	/* set USB PHY RS bit */
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB1PHYCTL_OFFSET,
+			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
 			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
-	regmap_update_bits(gcr_regmap, NPCM_USB2PHYCTL_OFFSET,
+	regmap_update_bits(rc->gcr_regmap, NPCM_USB3PHYCTL_OFFSET,
 			   NPCM_USBXPHYCTL_RS, NPCM_USBXPHYCTL_RS);
 
 	/* deassert reset USB devices*/
 	iprst1 &= ~ipsrst1_bits;
 	iprst2 &= ~ipsrst2_bits;
 	iprst3 &= ~ipsrst3_bits;
+	iprst4 &= ~ipsrst4_bits;
 
 	writel(iprst1, rc->base + NPCM_IPSRST1);
 	writel(iprst2, rc->base + NPCM_IPSRST2);
 	writel(iprst3, rc->base + NPCM_IPSRST3);
+	writel(iprst4, rc->base + NPCM_IPSRST4);
+}
+
+/*
+ *  The following procedure should be observed in USB PHY, USB device and
+ *  USB host initialization at BMC boot
+ */
+static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
+{
+	struct device *dev = &pdev->dev;
+
+	rc->gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr");
+	if (IS_ERR(rc->gcr_regmap)) {
+		dev_err(&pdev->dev, "Failed to find nuvoton,sysgcr");
+		return PTR_ERR(rc->gcr_regmap);
+	}
+
+	rc->info = (const struct npcm_reset_info *)
+			of_match_device(dev->driver->of_match_table, dev)->data;
+	switch (rc->info->bmc_id) {
+	case BMC_NPCM7XX:
+		npcm_usb_reset_npcm7xx(rc);
+		break;
+	case BMC_NPCM8XX:
+		npcm_usb_reset_npcm8xx(rc);
+		break;
+	default:
+		return -ENODEV;
+	}
 
 	return 0;
 }
-- 
2.33.0

[PATCH v2 14/20] dt-bindings: arm: npcm: Add maintainer

[Tomer Maimon]

Add Tomer Maimon to the maintainers list.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/arm/npcm/npcm.yaml        | 1 +
 Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/npcm/npcm.yaml b/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
index 95e51378089c..ea9c3103761d 100644
--- a/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
+++ b/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
@@ -8,6 +8,7 @@ title: NPCM Platforms Device Tree Bindings
 
 maintainers:
   - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+  - Tomer Maimon <tmaimon77@gmail.com>
 
 properties:
   $nodename:
diff --git a/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
index fcb211add7d3..aad7c85e787f 100644
--- a/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
+++ b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
@@ -8,6 +8,7 @@ title: Global Control Registers block in Nuvoton SoCs
 
 maintainers:
   - Jonathan Neuschäfer <j.neuschaefer@gmx.net>
+  - Tomer Maimon <tmaimon77@gmail.com>
 
 description:
   The Global Control Registers (GCR) are a block of registers in Nuvoton SoCs
-- 
2.33.0

[PATCH v2 15/20] dt-bindings: arm: npcm: Add nuvoton,npcm845 compatible string

[Tomer Maimon]

Add a compatible string for Nuvoton BMC NPCM845 SoC and a board
specific device tree for the NPCM845 (Arbel) evaluation board.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/arm/npcm/npcm.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/npcm/npcm.yaml b/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
index ea9c3103761d..43409e5721d5 100644
--- a/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
+++ b/Documentation/devicetree/bindings/arm/npcm/npcm.yaml
@@ -27,4 +27,10 @@ properties:
               - nuvoton,npcm750-evb         # NPCM750 evaluation board
           - const: nuvoton,npcm750
 
+      - description: NPCM845 based boards
+        items:
+          - enum:
+              - nuvoton,npcm845-evb         # NPCM845 evaluation board
+          - const: nuvoton,npcm845
+
 additionalProperties: true
-- 
2.33.0

[PATCH v2 16/20] dt-bindings: arm: npcm: Add nuvoton,npcm845 GCR compatible string

[Tomer Maimon]

Add a compatible string for Nuvoton BMC NPCM845
global control registers (GCR).

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
index aad7c85e787f..94e72f25b331 100644
--- a/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
+++ b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gcr.yaml
@@ -21,6 +21,7 @@ properties:
       - enum:
           - nuvoton,wpcm450-gcr
           - nuvoton,npcm750-gcr
+          - nuvoton,npcm845-gcr
       - const: syscon
       - const: simple-mfd
 
-- 
2.33.0

[PATCH v2 17/20] arm64: npcm: Add support for Nuvoton NPCM8XX BMC SoC

[Tomer Maimon]

This adds support for the Nuvoton NPCM8XX Board Management
controller (BMC) SoC family.

The NPCM8XX based quad-core Cortex-A35 ARMv8 architecture.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 MAINTAINERS                  |  3 +++
 arch/arm64/Kconfig.platforms | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a6d3bd9d2a8d..6e90ce4b4023 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2448,9 +2448,12 @@ F:	Documentation/devicetree/bindings/*/*npcm*
 F:	Documentation/devicetree/bindings/arm/npcm/*
 F:	arch/arm/boot/dts/nuvoton-npcm*
 F:	arch/arm/mach-npcm/
+F:	arch/arm64/boot/dts/nuvoton/
 F:	drivers/*/*npcm*
 F:	drivers/*/*/*npcm*
 F:	include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
+F:	include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
+F:	include/dt-bindings/clock/nuvoton,npcm8xx-reset.h
 
 ARM/NUVOTON WPCM450 ARCHITECTURE
 M:	Jonathan Neuschäfer <j.neuschaefer@gmx.net>
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 4e6d635a1731..c68d1b4f8975 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -202,6 +202,17 @@ config ARCH_MXC
 	  This enables support for the ARMv8 based SoCs in the
 	  NXP i.MX family.
 
+config ARCH_NPCM
+	bool "Nuvoton NPCM Architecture"
+	select PINCTRL
+	select GPIOLIB
+	select NPCM7XX_TIMER
+	select RESET_CONTROLLER
+	select MFD_SYSCON
+	help
+	  General support for NPCM8xx BMC (Arbel).
+	  Nuvoton NPCM8xx BMC based on the Cortex A35.
+
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select GPIOLIB
-- 
2.33.0

[PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Tomer Maimon]

This adds initial device tree support for the
Nuvoton NPCM845 Board Management controller (BMC) SoC family.

The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and
have various peripheral IPs.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm64/boot/dts/Makefile                  |   1 +
 .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi   | 197 ++++++++++++++++++
 .../boot/dts/nuvoton/nuvoton-npcm845.dtsi     |  76 +++++++
 3 files changed, 274 insertions(+)
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi

diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index 1ba04e31a438..7b107fa7414b 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -19,6 +19,7 @@ subdir-y += lg
 subdir-y += marvell
 subdir-y += mediatek
 subdir-y += microchip
+subdir-y += nuvoton
 subdir-y += nvidia
 subdir-y += qcom
 subdir-y += realtek
diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
new file mode 100644
index 000000000000..97e108c50760
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
+
+#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	interrupt-parent = <&gic>;
+
+	/* external reference clock */
+	clk_refclk: clk-refclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <25000000>;
+		clock-output-names = "refclk";
+	};
+
+	/* external reference clock for cpu. float in normal operation */
+	clk_sysbypck: clk-sysbypck {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <1000000000>;
+		clock-output-names = "sysbypck";
+	};
+
+	/* external reference clock for MC. float in normal operation */
+	clk_mcbypck: clk-mcbypck {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <1050000000>;
+		clock-output-names = "mcbypck";
+	};
+
+	soc {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		ranges;
+
+		gcr: gcr@f0800000 {
+			compatible = "nuvoton,npcm845-gcr", "syscon",
+				"simple-mfd";
+			reg = <0x0 0xf0800000 0x0 0x1000>;
+		};
+
+		gic: interrupt-controller@dfff9000 {
+			compatible = "arm,gic-400";
+			reg = <0x0 0xdfff9000 0x0 0x1000>,
+			      <0x0 0xdfffa000 0x0 0x2000>,
+			      <0x0 0xdfffc000 0x0 0x2000>,
+			      <0x0 0xdfffe000 0x0 0x2000>;
+			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+			#interrupt-cells = <3>;
+			interrupt-controller;
+			#address-cells = <0>;
+			ppi-partitions {
+				ppi_cluster0: interrupt-partition-0 {
+					affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
+				};
+			};
+		};
+	};
+
+	ahb {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		ranges;
+
+		rstc: rstc@f0801000 {
+			compatible = "nuvoton,npcm845-reset";
+			reg = <0x0 0xf0801000 0x0 0x78>;
+			#reset-cells = <2>;
+			nuvoton,sysgcr = <&gcr>;
+		};
+
+		clk: clock-controller@f0801000 {
+			compatible = "nuvoton,npcm845-clk";
+			#clock-cells = <1>;
+			reg = <0x0 0xf0801000 0x0 0x1000>;
+			clock-names = "refclk", "sysbypck", "mcbypck";
+			clocks = <&clk_refclk>, <&clk_sysbypck>, <&clk_mcbypck>;
+		};
+
+		apb {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-bus";
+			interrupt-parent = <&gic>;
+			ranges = <0x0 0x0 0xf0000000 0x00300000>,
+				<0xfff00000 0x0 0xfff00000 0x00016000>;
+
+			timer0: timer@8000 {
+				compatible = "nuvoton,npcm845-timer";
+				interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0x8000 0x1C>;
+				clocks	= <&clk_refclk>;
+				clock-names = "refclk";
+			};
+
+			serial0: serial@0 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x0 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial1: serial@1000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x1000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial2: serial@2000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x2000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial3: serial@3000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x3000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial4: serial@4000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x4000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial5: serial@5000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x5000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			serial6: serial@6000 {
+				compatible = "nuvoton,npcm845-uart", "nuvoton,npcm750-uart";
+				reg = <0x6000 0x1000>;
+				clocks = <&clk NPCM8XX_CLK_UART>;
+				interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>;
+				reg-shift = <2>;
+				status = "disabled";
+			};
+
+			watchdog0: watchdog@801c {
+				compatible = "nuvoton,npcm845-wdt", "nuvoton,npcm750-wdt";
+				interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0x801c 0x4>;
+				status = "disabled";
+				clocks = <&clk_refclk>;
+				syscon = <&gcr>;
+			};
+
+			watchdog1: watchdog@901c {
+				compatible = "nuvoton,npcm845-wdt", "nuvoton,npcm750-wdt";
+				interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0x901c 0x4>;
+				status = "disabled";
+				clocks = <&clk_refclk>;
+				syscon = <&gcr>;
+			};
+
+			watchdog2: watchdog@a01c {
+				compatible = "nuvoton,npcm845-wdt", "nuvoton,npcm750-wdt";
+				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+				reg = <0xa01c 0x4>;
+				status = "disabled";
+				clocks = <&clk_refclk>;
+				syscon = <&gcr>;
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi
new file mode 100644
index 000000000000..12118b75c0e6
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
+
+#include "nuvoton-common-npcm8xx.dtsi"
+
+/ {
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			clocks = <&clk NPCM8XX_CLK_CPU>;
+			reg = <0x0 0x0>;
+			next-level-cache = <&l2>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			clocks = <&clk NPCM8XX_CLK_CPU>;
+			reg = <0x0 0x1>;
+			next-level-cache = <&l2>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			clocks = <&clk NPCM8XX_CLK_CPU>;
+			reg = <0x0 0x2>;
+			next-level-cache = <&l2>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a35";
+			clocks = <&clk NPCM8XX_CLK_CPU>;
+			reg = <0x0 0x3>;
+			next-level-cache = <&l2>;
+			enable-method = "psci";
+		};
+
+		l2: l2-cache {
+			compatible = "cache";
+		};
+	};
+
+	arm-pmu {
+		compatible = "arm,cortex-a35-pmu";
+		interrupts = <GIC_SPI 242 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 243 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-affinity = <&cpu0>, <&cpu1>, <&cpu2>, <&cpu3>;
+	};
+
+	psci {
+		compatible      = "arm,psci-1.0";
+		method          = "smc";
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+};
-- 
2.33.0

[PATCH v2 19/20] arm64: dts: nuvoton: Add initial NPCM845 EVB device tree

[Tomer Maimon]

Add initial Nuvoton NPCM845 evaluation board device tree.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm64/boot/dts/nuvoton/Makefile          |  2 ++
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts  | 30 +++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 arch/arm64/boot/dts/nuvoton/Makefile
 create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts

diff --git a/arch/arm64/boot/dts/nuvoton/Makefile b/arch/arm64/boot/dts/nuvoton/Makefile
new file mode 100644
index 000000000000..a99dab90472a
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_ARCH_NPCM) += nuvoton-npcm845-evb.dtb
diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
new file mode 100644
index 000000000000..a5ab2bc0f835
--- /dev/null
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
+
+/dts-v1/;
+#include "nuvoton-npcm845.dtsi"
+
+/ {
+	model = "Nuvoton npcm845 Development Board (Device Tree)";
+	compatible = "nuvoton,npcm845-evb", "nuvoton,npcm845";
+
+	aliases {
+		serial0 = &serial0;
+	};
+
+	chosen {
+		stdout-path = &serial0;
+	};
+
+	memory {
+		reg = <0x0 0x0 0x0 0x40000000>;
+	};
+};
+
+&serial0 {
+	status = "okay";
+};
+
+&watchdog1 {
+	status = "okay";
+};
-- 
2.33.0

[PATCH v2 20/20] arm64: defconfig: Add Nuvoton NPCM family support

[Tomer Maimon]

Enable basic drivers for NPCM8XX booting up support:
Architecture, Clock, and WD.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm64/configs/defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7d1105343bc2..c4a237a84efa 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -49,6 +49,7 @@ CONFIG_ARCH_MEDIATEK=y
 CONFIG_ARCH_MESON=y
 CONFIG_ARCH_MVEBU=y
 CONFIG_ARCH_MXC=y
+CONFIG_ARCH_NPCM=y
 CONFIG_ARCH_QCOM=y
 CONFIG_ARCH_RENESAS=y
 CONFIG_ARCH_ROCKCHIP=y
@@ -627,6 +628,7 @@ CONFIG_RENESAS_RZG2LWDT=y
 CONFIG_UNIPHIER_WATCHDOG=y
 CONFIG_PM8916_WATCHDOG=m
 CONFIG_BCM2835_WDT=y
+CONFIG_NPCM7XX_WATCHDOG=y
 CONFIG_MFD_ALTERA_SYSMGR=y
 CONFIG_MFD_BD9571MWV=y
 CONFIG_MFD_AXP20X_I2C=y
@@ -1021,6 +1023,7 @@ CONFIG_COMMON_CLK_FSL_SAI=y
 CONFIG_COMMON_CLK_S2MPS11=y
 CONFIG_COMMON_CLK_PWM=y
 CONFIG_COMMON_CLK_VC5=y
+CONFIG_COMMON_CLK_NPCM8XX=y
 CONFIG_COMMON_CLK_BD718XX=m
 CONFIG_CLK_RASPBERRYPI=m
 CONFIG_CLK_IMX8MM=y
-- 
2.33.0

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Add binding for the Arbel BMC NPCM8XX Clock controller.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
>  .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
>  2 files changed, 113 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>  create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> new file mode 100644
> index 000000000000..e1f375716bc5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton NPCM8XX Clock Controller Binding
> +
> +maintainers:
> +  - Tomer Maimon <tmaimon77@gmail.com>
> +
> +description: |
> +  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
> +  generates and supplies clocks to all modules within the BMC.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nuvoton,npcm845-clk
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: 25M reference clock
> +      - description: CPU reference clock
> +      - description: MC reference clock
> +
> +  clock-names:
> +    items:
> +      - const: refclk
> +      - const: sysbypck
> +      - const: mcbypck
> +

I asked what is the suffix about and you replied "ck"... ok, so let's
make clear. This should be:

    items:
      - const: ref
      - const: sysbyp
      - const: mcbyp

or something similar, without the same suffix all over.

> diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> new file mode 100644
> index 000000000000..229915a254a5
> --- /dev/null
> +++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h

Same comment as before. No changes here...



Best regards,
Krzysztof

Re: [PATCH v2 08/20] dt-bindings: reset: modify to general NPCM name

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Modify nuvoton,npcm750-reset specific name to
> general NPCM file name, nuvoton,npcm-reset.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml}     | 2 +-


No. Name from the first compatible should be used.

Best regards,
Krzysztof

Re: [PATCH v2 09/20] dt-bindings: reset: npcm: add GCR syscon property

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Describe syscon property that handles general
> control registers(GCR) in Nuvoton BMC NPCM
> reset driver.

I already asked about this.

Could you please implement the comments you receive? It's not just one,
it's several of them have to be repeated.

> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../devicetree/bindings/reset/nuvoton,npcm-reset.yaml       | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> index 0998f481578d..c6bbc1589ab9 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> @@ -19,6 +19,10 @@ properties:
>    '#reset-cells':
>      const: 2
>  
> +  nuvoton,sysgcr:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"

Skip quotes.

> +    description: a phandle to access GCR registers.
> +
>    nuvoton,sw-reset-number:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      minimum: 1
> @@ -31,6 +35,7 @@ required:
>    - compatible
>    - reg
>    - '#reset-cells'
> +  - nuvoton,sysgcr

Aren't you breaking existing DTBs?

>  
>  additionalProperties: false
>  
> @@ -41,6 +46,7 @@ examples:
>          compatible = "nuvoton,npcm750-reset";
>          reg = <0xf0801000 0x70>;
>          #reset-cells = <2>;
> +        nuvoton,sysgcr = <&gcr>;
>          nuvoton,sw-reset-number = <2>;
>      };
>  


Best regards,
Krzysztof

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Add nuvoton,sysgcr syscon property to the reset
> node to handle the general control registers.

Wrong wrapping.

Best regards,
Krzysztof

Re: [PATCH v2 11/20] reset: npcm: using syscon instead of device data

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Using syscon device tree property instead of
> device data to handle the NPCM general control
> registers.
> 

Again ignored the comment.

> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  drivers/reset/reset-npcm.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
> index 2ea4d3136e15..312c3b594b8f 100644
> --- a/drivers/reset/reset-npcm.c
> +++ b/drivers/reset/reset-npcm.c
> @@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev,
>  }
>  
>  static const struct of_device_id npcm_rc_match[] = {
> -	{ .compatible = "nuvoton,npcm750-reset",
> -		.data = (void *)"nuvoton,npcm750-gcr" },
> +	{ .compatible = "nuvoton,npcm750-reset"},
>  	{ }
>  };
>  
> @@ -155,14 +154,10 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
>  	u32 ipsrst1_bits = 0;
>  	u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
>  	u32 ipsrst3_bits = 0;
> -	const char *gcr_dt;
>  
> -	gcr_dt = (const char *)
> -	of_match_device(dev->driver->of_match_table, dev)->data;
> -
> -	gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt);
> +	gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr");
>  	if (IS_ERR(gcr_regmap)) {
> -		dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt);
> +		dev_err(&pdev->dev, "Failed to find gcr syscon");
>  		return PTR_ERR(gcr_regmap);

Comment still ignored.

There is no point in this review if you keep ignoring what we ask to fix.

If something is unclear, ask for clarification. Resending without
implementing the comment means that you ignore the review which is waste
of my time.

I am sorry, but this is not acceptable.

Best regards,
Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> Add binding document and device tree binding
> constants for Nuvoton BMC NPCM8XX reset controller.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/reset/nuvoton,npcm-reset.yaml    |  13 +-
>  .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 ++++++++++++++++++
>  2 files changed, 140 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> index c6bbc1589ab9..93ea81686f58 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> @@ -9,9 +9,20 @@ title: Nuvoton NPCM Reset controller
>  maintainers:
>    - Tomer Maimon <tmaimon77@gmail.com>
>  
> +description: |
> +  The NPCM reset controller used to reset various set of peripherals. 
> +  Please refer to reset.txt in this directory for common reset
> +  controller binding usage.
> +
> +  For list of all valid reset indices see
> +    <dt-bindings/reset/nuvoton,npcm7xx-reset.h> for Poleg NPCM7XX SoC,
> +    <dt-bindings/reset/nuvoton,npcm8xx-reset.h> for Arbel NPCM8XX SoC.
> +
>  properties:
>    compatible:
> -    const: nuvoton,npcm750-reset
> +    enum: 
> +      - nuvoton,npcm750-reset        # Poleg NPCM7XX SoC
> +      - nuvoton,npcm845-reset        # Arbel NPCM8XX SoC
>  
>    reg:
>      maxItems: 1
> diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> new file mode 100644
> index 000000000000..5b3b74534b50
> --- /dev/null
> +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Again - ignored comment from v1.

> +/*
> + * Copyright (c) 2022 Nuvoton Technology corporation.
> + * Author: Tomer Maimon <tmaimon77@gmail.com>
> + */
> +
> +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
> +#define _DT_BINDINGS_NPCM8XX_RESET_H
> +
> +/* represent reset register offset */
> +#define NPCM8XX_RESET_IPSRST1		0x20
> +#define NPCM8XX_RESET_IPSRST2		0x24
> +#define NPCM8XX_RESET_IPSRST3		0x34
> +#define NPCM8XX_RESET_IPSRST4		0x74
> +
> +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */

Again - ignored comment from v1. My last message was quite clear, wasn't it?

https://lore.kernel.org/all/4a69902f-a545-23a1-1430-e5ece16997e9@linaro.org/

You ignored several of previous comments, so:

NAK.

Best regards,
Krzysztof

Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Krzysztof Kozlowski]

On 08/06/2022 11:56, Tomer Maimon wrote:
> This adds initial device tree support for the
> Nuvoton NPCM845 Board Management controller (BMC) SoC family.
> 
> The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and
> have various peripheral IPs.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  arch/arm64/boot/dts/Makefile                  |   1 +
>  .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi   | 197 ++++++++++++++++++
>  .../boot/dts/nuvoton/nuvoton-npcm845.dtsi     |  76 +++++++
>  3 files changed, 274 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
>  create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi
> 
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 1ba04e31a438..7b107fa7414b 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -19,6 +19,7 @@ subdir-y += lg
>  subdir-y += marvell
>  subdir-y += mediatek
>  subdir-y += microchip
> +subdir-y += nuvoton
>  subdir-y += nvidia
>  subdir-y += qcom
>  subdir-y += realtek
> diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> new file mode 100644
> index 000000000000..97e108c50760
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
> +
> +#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +	interrupt-parent = <&gic>;
> +
> +	/* external reference clock */
> +	clk_refclk: clk-refclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <25000000>;

Ignored comment.

> +		clock-output-names = "refclk";
> +	};
> +
> +	/* external reference clock for cpu. float in normal operation */
> +	clk_sysbypck: clk-sysbypck {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <1000000000>;

Ignored comment.

> +		clock-output-names = "sysbypck";
> +	};
> +
> +	/* external reference clock for MC. float in normal operation */
> +	clk_mcbypck: clk-mcbypck {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <1050000000>;
> +		clock-output-names = "mcbypck";
> +	};
> +
> +	soc {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&gic>;
> +		ranges;
> +
> +		gcr: gcr@f0800000 {

Ignored comment.

> +			compatible = "nuvoton,npcm845-gcr", "syscon",
> +				"simple-mfd";

This is not a simple-mfd... I see original bindings defined it that way,
but why? I think they should be corrected - remove simple-mfd from the
bindings and DTS.


> +			reg = <0x0 0xf0800000 0x0 0x1000>;
> +		};
> +
> +		gic: interrupt-controller@dfff9000 {
> +			compatible = "arm,gic-400";
> +			reg = <0x0 0xdfff9000 0x0 0x1000>,
> +			      <0x0 0xdfffa000 0x0 0x2000>,
> +			      <0x0 0xdfffc000 0x0 0x2000>,
> +			      <0x0 0xdfffe000 0x0 0x2000>;
> +			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> +			#interrupt-cells = <3>;
> +			interrupt-controller;
> +			#address-cells = <0>;
> +			ppi-partitions {
> +				ppi_cluster0: interrupt-partition-0 {
> +					affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
> +				};
> +			};
> +		};
> +	};
> +
> +	ahb {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		compatible = "simple-bus";
> +		interrupt-parent = <&gic>;
> +		ranges;
> +
> +		rstc: rstc@f0801000 {

Ignored comment.

Four comments from v1 ignored in this patch alone.

I'll stop reviewing, it is a waste of my time.

NAK for this change.

Best regards,
Krzysztof

Re: [PATCH v2 01/20] clocksource: timer-npcm7xx: Add NPCM845 timer

[Arnd Bergmann]

On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Add Nuvoton BMC NPCM845 timer support.
> The NPCM845 uses the same timer controller as the NPCM750.
>
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  drivers/clocksource/timer-npcm7xx.c | 1 +

This one should no longer be needed if the timers are compatible with the
old ones and correctly described in the DT.

      Arnd

Re: [PATCH v2 03/20] tty: serial: 8250: Add NPCM845 UART support

[Arnd Bergmann]

On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Add Nuvoton BMC NPCM845 UART support.
> The NPCM845 uses the same UART as the NPCM750.
>
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>


This one should no longer be needed if the timers are compatible with the
old ones and correctly described in the DT.

      Arnd

Re: [PATCH v2 05/20] watchdog: npcm_wdt: Add NPCM845 watchdog support

[Arnd Bergmann]

On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Add Nuvoton BMC NPCM845 watchdog support.
> The NPCM845 uses the same watchdog as the NPCM750.
>
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>

This one should no longer be needed if the timers are compatible with the
old ones and correctly described in the DT.

      Arnd

Re: [PATCH v2 03/20] tty: serial: 8250: Add NPCM845 UART support

[Tomer Maimon]

Hi Arnd,

Sorry but Just to clarify, This patch should not be applied and the
NPCM8XX UART should use nuvoton,npcm750-uart compatible in the device
tree?

Because I thought that in your comment a few weeks ago
https://www.spinics.net/lists/linux-serial/msg48179.html

We need only to modify the compatible string in the device tree as we
did in V2 patchset
https://www.spinics.net/lists/arm-kernel/msg986480.html

On Wed, 8 Jun 2022 at 15:01, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> > Add Nuvoton BMC NPCM845 UART support.
> > The NPCM845 uses the same UART as the NPCM750.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>
>
> This one should no longer be needed if the timers are compatible with the
> old ones and correctly described in the DT.
>
>       Arnd

Thanks,

Tomer

Re: [PATCH v2 03/20] tty: serial: 8250: Add NPCM845 UART support

[Arnd Bergmann]

On Wed, Jun 8, 2022 at 3:40 PM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Sorry but Just to clarify, This patch should not be applied and the
> NPCM8XX UART should use nuvoton,npcm750-uart compatible in the device
> tree?
>
> Because I thought that in your comment a few weeks ago
> https://www.spinics.net/lists/linux-serial/msg48179.html
>
> We need only to modify the compatible string in the device tree as we
> did in V2 patchset
> https://www.spinics.net/lists/arm-kernel/msg986480.html

Yes, this is correct: with the DT file from v2, the driver no longer needs to be
changed, it will just match the fallback value.

       Arnd

Re: [PATCH v2 09/20] dt-bindings: reset: npcm: add GCR syscon property

[Rob Herring]

On Wed, 08 Jun 2022 12:56:12 +0300, Tomer Maimon wrote:
> Describe syscon property that handles general
> control registers(GCR) in Nuvoton BMC NPCM
> reset driver.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../devicetree/bindings/reset/nuvoton,npcm-reset.yaml       | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


rstc@f0801000: 'nuvoton,sysgcr' is a required property
	arch/arm/boot/dts/nuvoton-npcm730-gbs.dtb
	arch/arm/boot/dts/nuvoton-npcm730-gsj.dtb
	arch/arm/boot/dts/nuvoton-npcm730-kudo.dtb
	arch/arm/boot/dts/nuvoton-npcm750-evb.dtb
	arch/arm/boot/dts/nuvoton-npcm750-runbmc-olympus.dtb

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Tomer Maimon]

Hi Krzysztof,

Thanks for your comments.

On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Add binding for the Arbel BMC NPCM8XX Clock controller.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
> >  .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
> >  2 files changed, 113 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> >  create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> > new file mode 100644
> > index 000000000000..e1f375716bc5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Nuvoton NPCM8XX Clock Controller Binding
> > +
> > +maintainers:
> > +  - Tomer Maimon <tmaimon77@gmail.com>
> > +
> > +description: |
> > +  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
> > +  generates and supplies clocks to all modules within the BMC.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nuvoton,npcm845-clk
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: 25M reference clock
> > +      - description: CPU reference clock
> > +      - description: MC reference clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: refclk
> > +      - const: sysbypck
> > +      - const: mcbypck
> > +
>
> I asked what is the suffix about and you replied "ck"... ok, so let's
> make clear. This should be:
>
>     items:
>       - const: ref
>       - const: sysbyp
>       - const: mcbyp
>
> or something similar, without the same suffix all over.
The clock names are the same clock name in our spec, this why we
prefer to leave the clock names as is.
>
> > diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> > new file mode 100644
> > index 000000000000..229915a254a5
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
>
> Same comment as before. No changes here...
>
about the comments from V1::
- Krzysztof: Filename - same as bindings, so nuvoton,npcm845-clk.h
In NPCM7XX we use the same include file and clock source
dt-binding
https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt
dt-binding include
https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
we prefer to be align with our older BMC version

- Krzysztof: Dual license, same as bindings.
modified in the file * SPDX-License-Identifier: (GPL-2.0-only OR
BSD-2-Clause) */
the same license approved in en7523-clk include file and pushed to
Linux kernel 5.19 :
https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/en7523-clk.h

>
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Krzysztof Kozlowski]

On 09/06/2022 15:17, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your comments.
> 
> On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 08/06/2022 11:56, Tomer Maimon wrote:
>>> Add binding for the Arbel BMC NPCM8XX Clock controller.
>>>
>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>>> ---
>>>  .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
>>>  .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
>>>  2 files changed, 113 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>>  create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>> new file mode 100644
>>> index 000000000000..e1f375716bc5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>> @@ -0,0 +1,63 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Nuvoton NPCM8XX Clock Controller Binding
>>> +
>>> +maintainers:
>>> +  - Tomer Maimon <tmaimon77@gmail.com>
>>> +
>>> +description: |
>>> +  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
>>> +  generates and supplies clocks to all modules within the BMC.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - nuvoton,npcm845-clk
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: 25M reference clock
>>> +      - description: CPU reference clock
>>> +      - description: MC reference clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: refclk
>>> +      - const: sysbypck
>>> +      - const: mcbypck
>>> +
>>
>> I asked what is the suffix about and you replied "ck"... ok, so let's
>> make clear. This should be:
>>
>>     items:
>>       - const: ref
>>       - const: sysbyp
>>       - const: mcbyp
>>
>> or something similar, without the same suffix all over.
> The clock names are the same clock name in our spec, this why we
> prefer to leave the clock names as is.

The naming with useless suffixes does not help. If your spec had
"refclk_really_clock_this_is_a_clock" you also would insist on that? It
does not make sense.

>>
>>> diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
>>> new file mode 100644
>>> index 000000000000..229915a254a5
>>> --- /dev/null
>>> +++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
>>
>> Same comment as before. No changes here...
>>
> about the comments from V1::
> - Krzysztof: Filename - same as bindings, so nuvoton,npcm845-clk.h
> In NPCM7XX we use the same include file and clock source
> dt-binding
> https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt
> dt-binding include
> https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
> we prefer to be align with our older BMC version

Older has incorrect name, so do not align to incorrect one. What is the
logic behind having header not matching the bindings file? It makes it
only more difficult to connect these two.

> 
> - Krzysztof: Dual license, same as bindings.
> modified in the file * SPDX-License-Identifier: (GPL-2.0-only OR
> BSD-2-Clause) */
> the same license approved in en7523-clk include file and pushed to
> Linux kernel 5.19 :
> https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/en7523-clk.h

I don't understand this comment at all. I am not commenting about
en7523-clk.h. I am commenting about the header here - it should have
dual license. What en7523-clk.h has to do with it?

Best regards,
Krzysztof

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Jonathan Neuschäfer]

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

Hello Tomer and Krzysztof,

On Wed, Jun 08, 2022 at 12:03:00PM +0200, Krzysztof Kozlowski wrote:
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Add binding for the Arbel BMC NPCM8XX Clock controller.
> > 
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
[...]
> > +  clocks:
> > +    items:
> > +      - description: 25M reference clock
> > +      - description: CPU reference clock
> > +      - description: MC reference clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: refclk
> > +      - const: sysbypck
> > +      - const: mcbypck
> > +
> 
> I asked what is the suffix about and you replied "ck"... ok, so let's
> make clear. This should be:
> 
>     items:
>       - const: ref
>       - const: sysbyp
>       - const: mcbyp
> 
> or something similar, without the same suffix all over.

A bit of a side note on these names:

To make the binding as easy to understand as possible, I think it would
help to have every part of the clock-names reflected in corresponding
clock description:

- sysbypck:  presumably means system bypass clock
- mcbypck:   presumably means memory controller bypass clock


As it currently is in the patch, the "byp" part stays unexplained and
unmentioned in the descriptions.



Thanks,
Jonathan

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

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Tomer Maimon]

Hi Krzysztof,


On Thu, 9 Jun 2022 at 16:22, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/06/2022 15:17, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Thanks for your comments.
> >
> > On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 08/06/2022 11:56, Tomer Maimon wrote:
> >>> Add binding for the Arbel BMC NPCM8XX Clock controller.
> >>>
> >>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> >>> ---
> >>>  .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
> >>>  .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
> >>>  2 files changed, 113 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> >>>  create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> >>> new file mode 100644
> >>> index 000000000000..e1f375716bc5
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
> >>> @@ -0,0 +1,63 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Nuvoton NPCM8XX Clock Controller Binding
> >>> +
> >>> +maintainers:
> >>> +  - Tomer Maimon <tmaimon77@gmail.com>
> >>> +
> >>> +description: |
> >>> +  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
> >>> +  generates and supplies clocks to all modules within the BMC.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - nuvoton,npcm845-clk
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    items:
> >>> +      - description: 25M reference clock
> >>> +      - description: CPU reference clock
> >>> +      - description: MC reference clock
> >>> +
> >>> +  clock-names:
> >>> +    items:
> >>> +      - const: refclk
> >>> +      - const: sysbypck
> >>> +      - const: mcbypck
> >>> +
> >>
> >> I asked what is the suffix about and you replied "ck"... ok, so let's
> >> make clear. This should be:
> >>
> >>     items:
> >>       - const: ref
> >>       - const: sysbyp
> >>       - const: mcbyp
> >>
> >> or something similar, without the same suffix all over.
> > The clock names are the same clock name in our spec, this why we
> > prefer to leave the clock names as is.
>
> The naming with useless suffixes does not help. If your spec had
> "refclk_really_clock_this_is_a_clock" you also would insist on that? It
> does not make sense.
Sorry but I don't understand why the clock name cause an issue, we
prefer it will be the same as in our spec-clock diagram
BTW, the same naming found in NPCM7XX
https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt#L36
>
> >>
> >>> diff --git a/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> >>> new file mode 100644
> >>> index 000000000000..229915a254a5
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
> >>
> >> Same comment as before. No changes here...
> >>
> > about the comments from V1::
> > - Krzysztof: Filename - same as bindings, so nuvoton,npcm845-clk.h
> > In NPCM7XX we use the same include file and clock source
> > dt-binding
> > https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt
> > dt-binding include
> > https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
> > we prefer to be align with our older BMC version
>
> Older has incorrect name, so do not align to incorrect one. What is the
> logic behind having header not matching the bindings file? It makes it
> only more difficult to connect these two.
Will modify the file name in V3 to be the same as dt-binding
>
> >
> > - Krzysztof: Dual license, same as bindings.
> > modified in the file * SPDX-License-Identifier: (GPL-2.0-only OR
> > BSD-2-Clause) */
> > the same license approved in en7523-clk include file and pushed to
> > Linux kernel 5.19 :
> > https://elixir.bootlin.com/linux/v5.19-rc1/source/include/dt-bindings/clock/en7523-clk.h
>
> I don't understand this comment at all. I am not commenting about
> en7523-clk.h. I am commenting about the header here - it should have
> dual license. What en7523-clk.h has to do with it?
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 08/20] dt-bindings: reset: modify to general NPCM name

[Tomer Maimon]

Hi Krzysztof,

Will revert the change in V3

On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Modify nuvoton,npcm750-reset specific name to
> > general NPCM file name, nuvoton,npcm-reset.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  .../{nuvoton,npcm750-reset.yaml => nuvoton,npcm-reset.yaml}     | 2 +-
>
>
> No. Name from the first compatible should be used.
>
> Best regards,
> Krzysztof

Thanks,

Tomer

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Tomer Maimon]

Hi Krzysztof,

Thanks for your comments

On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Add nuvoton,sysgcr syscon property to the reset
> > node to handle the general control registers.
>
> Wrong wrapping.
it will be very helpful if you could point me what wrong wrapped in
the commit message, is it the explanation or the header? or something
else?
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 11/20] reset: npcm: using syscon instead of device data

[Tomer Maimon]

Hi Krzysztof

Sorry but I didn't ignore your comment.

For not breaking exciting boards I add the following patch in V2
https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/

On Wed, 8 Jun 2022 at 13:08, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Using syscon device tree property instead of
> > device data to handle the NPCM general control
> > registers.
> >
>
> Again ignored the comment.
>
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  drivers/reset/reset-npcm.c | 11 +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c
> > index 2ea4d3136e15..312c3b594b8f 100644
> > --- a/drivers/reset/reset-npcm.c
> > +++ b/drivers/reset/reset-npcm.c
> > @@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev,
> >  }
> >
> >  static const struct of_device_id npcm_rc_match[] = {
> > -     { .compatible = "nuvoton,npcm750-reset",
> > -             .data = (void *)"nuvoton,npcm750-gcr" },
> > +     { .compatible = "nuvoton,npcm750-reset"},
> >       { }
> >  };
> >
> > @@ -155,14 +154,10 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc)
> >       u32 ipsrst1_bits = 0;
> >       u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST;
> >       u32 ipsrst3_bits = 0;
> > -     const char *gcr_dt;
> >
> > -     gcr_dt = (const char *)
> > -     of_match_device(dev->driver->of_match_table, dev)->data;
> > -
> > -     gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt);
> > +     gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr");
> >       if (IS_ERR(gcr_regmap)) {
> > -             dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt);
> > +             dev_err(&pdev->dev, "Failed to find gcr syscon");
> >               return PTR_ERR(gcr_regmap);
>
> Comment still ignored.
>
> There is no point in this review if you keep ignoring what we ask to fix.
>
> If something is unclear, ask for clarification. Resending without
> implementing the comment means that you ignore the review which is waste
> of my time.
>
> I am sorry, but this is not acceptable.
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Tomer Maimon]

Hi Krzysztof,

Sorry, but I thought the fix is only to add an explanation to the
dt-binding file as was done in V2.

The NPCM8XX binding is done in the same way as the NPCM7XX and both
use the same reset driver and use the same reset method in upstreamed
NPCM reset driver.

Can you please explain again what you suggest to do?


On Wed, 8 Jun 2022 at 13:11, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > Add binding document and device tree binding
> > constants for Nuvoton BMC NPCM8XX reset controller.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  .../bindings/reset/nuvoton,npcm-reset.yaml    |  13 +-
> >  .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 128 ++++++++++++++++++
> >  2 files changed, 140 insertions(+), 1 deletion(-)
> >  create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> >
> > diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > index c6bbc1589ab9..93ea81686f58 100644
> > --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.yaml
> > @@ -9,9 +9,20 @@ title: Nuvoton NPCM Reset controller
> >  maintainers:
> >    - Tomer Maimon <tmaimon77@gmail.com>
> >
> > +description: |
> > +  The NPCM reset controller used to reset various set of peripherals.
> > +  Please refer to reset.txt in this directory for common reset
> > +  controller binding usage.
> > +
> > +  For list of all valid reset indices see
> > +    <dt-bindings/reset/nuvoton,npcm7xx-reset.h> for Poleg NPCM7XX SoC,
> > +    <dt-bindings/reset/nuvoton,npcm8xx-reset.h> for Arbel NPCM8XX SoC.
> > +
> >  properties:
> >    compatible:
> > -    const: nuvoton,npcm750-reset
> > +    enum:
> > +      - nuvoton,npcm750-reset        # Poleg NPCM7XX SoC
> > +      - nuvoton,npcm845-reset        # Arbel NPCM8XX SoC
> >
> >    reg:
> >      maxItems: 1
> > diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> > new file mode 100644
> > index 000000000000..5b3b74534b50
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> > @@ -0,0 +1,128 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
>
> Again - ignored comment from v1.
>
> > +/*
> > + * Copyright (c) 2022 Nuvoton Technology corporation.
> > + * Author: Tomer Maimon <tmaimon77@gmail.com>
> > + */
> > +
> > +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
> > +#define _DT_BINDINGS_NPCM8XX_RESET_H
> > +
> > +/* represent reset register offset */
> > +#define NPCM8XX_RESET_IPSRST1                0x20
> > +#define NPCM8XX_RESET_IPSRST2                0x24
> > +#define NPCM8XX_RESET_IPSRST3                0x34
> > +#define NPCM8XX_RESET_IPSRST4                0x74
> > +
> > +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
>
> Again - ignored comment from v1. My last message was quite clear, wasn't it?
>
> https://lore.kernel.org/all/4a69902f-a545-23a1-1430-e5ece16997e9@linaro.org/
>
> You ignored several of previous comments, so:
>
> NAK.
>
> Best regards,
> Krzysztof

Appreciate your time and comments.

Best regards,

Tomer

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Benjamin Fair]

Hi Tomer,

On Thu, 9 Jun 2022 at 14:30, Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> Hi Krzysztof,
>
> Thanks for your comments
>
> On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 08/06/2022 11:56, Tomer Maimon wrote:
> > > Add nuvoton,sysgcr syscon property to the reset
> > > node to handle the general control registers.
> >
> > Wrong wrapping.
> it will be very helpful if you could point me what wrong wrapped in
> the commit message, is it the explanation or the header? or something
> else?

The commit message body should be wrapped at 72 chars. You can fit
more on the first line if you reflow:

Add nuvoton,sysgcr syscon property to the reset node to handle the
general control registers.

> >
> > Best regards,
> > Krzysztof
>
> Best regards,
>
> Tomer

Re: [PATCH v2 07/20] clk: npcm8xx: add clock controller

[Stephen Boyd]

Quoting Tomer Maimon (2022-06-08 02:56:10)
> diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c
> new file mode 100644
> index 000000000000..40340c3611b5
> --- /dev/null
> +++ b/drivers/clk/clk-npcm8xx.c
> @@ -0,0 +1,756 @@
[...]
> +
> +#define PLLCON_LOKI    BIT(31)
> +#define PLLCON_LOKS    BIT(30)
> +#define PLLCON_FBDV    GENMASK(27, 16)
> +#define PLLCON_OTDV2   GENMASK(15, 13)
> +#define PLLCON_PWDEN   BIT(12)
> +#define PLLCON_OTDV1   GENMASK(10, 8)
> +#define PLLCON_INDV    GENMASK(5, 0)
> +
> +static unsigned long npcm8xx_clk_pll_recalc_rate(struct clk_hw *hw,
> +                                                unsigned long parent_rate)
> +{
> +       struct npcm8xx_clk_pll *pll = to_npcm8xx_clk_pll(hw);
> +       unsigned long fbdv, indv, otdv1, otdv2;
> +       unsigned int val;
> +       u64 ret;
> +
> +       if (parent_rate == 0) {
> +               pr_debug("%s: parent rate is zero", __func__);

Missing newline.

> +               return 0;
> +       }
> +
> +       val = readl_relaxed(pll->pllcon);
> +
> +       indv = FIELD_GET(PLLCON_INDV, val);

Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Tomer Maimon]

Hi Krzysztof,

Sorry, probably I missed your comments (too many patches to handle at
one time :-))...

On Wed, 8 Jun 2022 at 13:21, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 08/06/2022 11:56, Tomer Maimon wrote:
> > This adds initial device tree support for the
> > Nuvoton NPCM845 Board Management controller (BMC) SoC family.
> >
> > The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and
> > have various peripheral IPs.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  arch/arm64/boot/dts/Makefile                  |   1 +
> >  .../dts/nuvoton/nuvoton-common-npcm8xx.dtsi   | 197 ++++++++++++++++++
> >  .../boot/dts/nuvoton/nuvoton-npcm845.dtsi     |  76 +++++++
> >  3 files changed, 274 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> >  create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> > index 1ba04e31a438..7b107fa7414b 100644
> > --- a/arch/arm64/boot/dts/Makefile
> > +++ b/arch/arm64/boot/dts/Makefile
> > @@ -19,6 +19,7 @@ subdir-y += lg
> >  subdir-y += marvell
> >  subdir-y += mediatek
> >  subdir-y += microchip
> > +subdir-y += nuvoton
> >  subdir-y += nvidia
> >  subdir-y += qcom
> >  subdir-y += realtek
> > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > new file mode 100644
> > index 000000000000..97e108c50760
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > @@ -0,0 +1,197 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
> > +
> > +#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
> > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +/ {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
> > +     interrupt-parent = <&gic>;
> > +
> > +     /* external reference clock */
> > +     clk_refclk: clk-refclk {
> > +             compatible = "fixed-clock";
> > +             #clock-cells = <0>;
> > +             clock-frequency = <25000000>;
>
> Ignored comment.
Could we use it as a default clock-frequency?
>
> > +             clock-output-names = "refclk";
> > +     };
> > +
> > +     /* external reference clock for cpu. float in normal operation */
> > +     clk_sysbypck: clk-sysbypck {
> > +             compatible = "fixed-clock";
> > +             #clock-cells = <0>;
> > +             clock-frequency = <1000000000>;
>
> Ignored comment.
same as above
>
> > +             clock-output-names = "sysbypck";
> > +     };
> > +
> > +     /* external reference clock for MC. float in normal operation */
> > +     clk_mcbypck: clk-mcbypck {
> > +             compatible = "fixed-clock";
> > +             #clock-cells = <0>;
> > +             clock-frequency = <1050000000>;
same as above
> > +             clock-output-names = "mcbypck";
> > +     };
> > +
> > +     soc {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             compatible = "simple-bus";
> > +             interrupt-parent = <&gic>;
> > +             ranges;
> > +
> > +             gcr: gcr@f0800000 {
I understand it sounds generic but I try to be as much compatible with NPCM7XX
https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L91
>
> Ignored comment.
>
> > +                     compatible = "nuvoton,npcm845-gcr", "syscon",
> > +                             "simple-mfd";
>
> This is not a simple-mfd... I see original bindings defined it that way,
> but why? I think they should be corrected - remove simple-mfd from the
> bindings and DTS.
will remove in both places in V3
>
>
> > +                     reg = <0x0 0xf0800000 0x0 0x1000>;
> > +             };
> > +
> > +             gic: interrupt-controller@dfff9000 {
> > +                     compatible = "arm,gic-400";
> > +                     reg = <0x0 0xdfff9000 0x0 0x1000>,
> > +                           <0x0 0xdfffa000 0x0 0x2000>,
> > +                           <0x0 0xdfffc000 0x0 0x2000>,
> > +                           <0x0 0xdfffe000 0x0 0x2000>;
> > +                     interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
> > +                     #interrupt-cells = <3>;
> > +                     interrupt-controller;
> > +                     #address-cells = <0>;
> > +                     ppi-partitions {
> > +                             ppi_cluster0: interrupt-partition-0 {
> > +                                     affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +
> > +     ahb {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             compatible = "simple-bus";
> > +             interrupt-parent = <&gic>;

> > +             ranges;
> > +
> > +             rstc: rstc@f0801000 {
>
> Ignored comment.
>
I understand it sounds generic but I try to be as much compatible with NPCM7XX
https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L109
> Four comments from v1 ignored in this patch alone.
>
one more comment in V1
 "+             cpu0: cpu@0 {
 +                     device_type = "cpu";
 +                     compatible = "arm,cortex-a35";
 +                     clocks = <&clk NPCM8XX_CLK_CPU>;
 +                     reg = <0x0 0x0>;
Why do you have two address cells? A bit more complicated and not
necessary, I think."
the arm,cortex-a35 is 64 Bit this is why we use  #address-cells = <2>;
and therefore reg = <0x0 0x0>;

> I'll stop reviewing, it is a waste of my time.
>
> NAK for this change.
>
> Best regards,
> Krzysztof

Again sorry to miss these comments in V1.

Appreciate your time.

Best regards,

Tomer

Re: [PATCH v2 07/20] clk: npcm8xx: add clock controller

[Tomer Maimon]

Hi Stephen,

Thanks for your comment, it will be addressed next patch set

On Fri, 10 Jun 2022 at 01:14, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Tomer Maimon (2022-06-08 02:56:10)
> > diff --git a/drivers/clk/clk-npcm8xx.c b/drivers/clk/clk-npcm8xx.c
> > new file mode 100644
> > index 000000000000..40340c3611b5
> > --- /dev/null
> > +++ b/drivers/clk/clk-npcm8xx.c
> > @@ -0,0 +1,756 @@
> [...]
> > +
> > +#define PLLCON_LOKI    BIT(31)
> > +#define PLLCON_LOKS    BIT(30)
> > +#define PLLCON_FBDV    GENMASK(27, 16)
> > +#define PLLCON_OTDV2   GENMASK(15, 13)
> > +#define PLLCON_PWDEN   BIT(12)
> > +#define PLLCON_OTDV1   GENMASK(10, 8)
> > +#define PLLCON_INDV    GENMASK(5, 0)
> > +
> > +static unsigned long npcm8xx_clk_pll_recalc_rate(struct clk_hw *hw,
> > +                                                unsigned long parent_rate)
> > +{
> > +       struct npcm8xx_clk_pll *pll = to_npcm8xx_clk_pll(hw);
> > +       unsigned long fbdv, indv, otdv1, otdv2;
> > +       unsigned int val;
> > +       u64 ret;
> > +
> > +       if (parent_rate == 0) {
> > +               pr_debug("%s: parent rate is zero", __func__);
>
> Missing newline.
>
> > +               return 0;
> > +       }
> > +
> > +       val = readl_relaxed(pll->pllcon);
> > +
> > +       indv = FIELD_GET(PLLCON_INDV, val);

Best regards,

Tomer

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Tomer Maimon]

Hi Benjamin,

Thanks a lot for your explanation.

will be applied in next patch set

Best regards,

Tomer

On Fri, 10 Jun 2022 at 01:11, Benjamin Fair <benjaminfair@google.com> wrote:
>
> Hi Tomer,
>
> On Thu, 9 Jun 2022 at 14:30, Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> > Hi Krzysztof,
> >
> > Thanks for your comments
> >
> > On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 08/06/2022 11:56, Tomer Maimon wrote:
> > > > Add nuvoton,sysgcr syscon property to the reset
> > > > node to handle the general control registers.
> > >
> > > Wrong wrapping.
> > it will be very helpful if you could point me what wrong wrapped in
> > the commit message, is it the explanation or the header? or something
> > else?
>
> The commit message body should be wrapped at 72 chars. You can fit
> more on the first line if you reflow:
>
> Add nuvoton,sysgcr syscon property to the reset node to handle the
> general control registers.
>
> > >
> > > Best regards,
> > > Krzysztof
> >
> > Best regards,
> >
> > Tomer

Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Geert Uytterhoeven]

Hi Tomer,

On Fri, Jun 10, 2022 at 12:30 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
> On Wed, 8 Jun 2022 at 13:21, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> > On 08/06/2022 11:56, Tomer Maimon wrote:
> > > This adds initial device tree support for the
> > > Nuvoton NPCM845 Board Management controller (BMC) SoC family.
> > >
> > > The NPCM845 based quad-core Cortex-A35 ARMv8 architecture and
> > > have various peripheral IPs.
> > >
> > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>

> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > > @@ -0,0 +1,197 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2021 Nuvoton Technology tomer.maimon@nuvoton.com
> > > +
> > > +#include <dt-bindings/clock/nuvoton,npcm8xx-clock.h>
> > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +#include <dt-bindings/interrupt-controller/irq.h>
> > > +
> > > +/ {
> > > +     #address-cells = <2>;
> > > +     #size-cells = <2>;
> > > +     interrupt-parent = <&gic>;
> > > +
> > > +     /* external reference clock */
> > > +     clk_refclk: clk-refclk {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <25000000>;
> >
> > Ignored comment.
> Could we use it as a default clock-frequency?

If the oscillator is present on the board, and not an SoC builtin, its
clock frequency should be described in the board DTS.
Some clocks may be optional, and left unpopulated.
Others clocks may be fed with different frequencies than the default.

> >
> > > +             clock-output-names = "refclk";
> > > +     };
> > > +
> > > +     /* external reference clock for cpu. float in normal operation */
> > > +     clk_sysbypck: clk-sysbypck {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <1000000000>;
> >
> > Ignored comment.
> same as above
> >
> > > +             clock-output-names = "sysbypck";
> > > +     };
> > > +
> > > +     /* external reference clock for MC. float in normal operation */
> > > +     clk_mcbypck: clk-mcbypck {
> > > +             compatible = "fixed-clock";
> > > +             #clock-cells = <0>;
> > > +             clock-frequency = <1050000000>;
> same as above
> > > +             clock-output-names = "mcbypck";
> > > +     };

>  "+             cpu0: cpu@0 {
>  +                     device_type = "cpu";
>  +                     compatible = "arm,cortex-a35";
>  +                     clocks = <&clk NPCM8XX_CLK_CPU>;
>  +                     reg = <0x0 0x0>;
> Why do you have two address cells? A bit more complicated and not
> necessary, I think."
> the arm,cortex-a35 is 64 Bit this is why we use  #address-cells = <2>;
> and therefore reg = <0x0 0x0>;

These addresses are not addresses on the main memory bus (which
is indeed 64-bit), but on the logical CPU bus.
Now, Documentation/devicetree/bindings/arm/cpus.yaml says you can
have #address-cells = <2> if you have non-zero MPIDR_EL1 high bits.

Gr{oetje,eeting}s,

                        Geert

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

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

Re: [PATCH v2 06/20] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock

[Krzysztof Kozlowski]

On 09/06/2022 23:21, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> 
> On Thu, 9 Jun 2022 at 16:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 09/06/2022 15:17, Tomer Maimon wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for your comments.
>>>
>>> On Wed, 8 Jun 2022 at 13:03, Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 08/06/2022 11:56, Tomer Maimon wrote:
>>>>> Add binding for the Arbel BMC NPCM8XX Clock controller.
>>>>>
>>>>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>>>>> ---
>>>>>  .../bindings/clock/nuvoton,npcm845-clk.yaml   | 63 +++++++++++++++++++
>>>>>  .../dt-bindings/clock/nuvoton,npcm8xx-clock.h | 50 +++++++++++++++
>>>>>  2 files changed, 113 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>>>>  create mode 100644 include/dt-bindings/clock/nuvoton,npcm8xx-clock.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..e1f375716bc5
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm845-clk.yaml
>>>>> @@ -0,0 +1,63 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/clock/nuvoton,npcm845-clk.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Nuvoton NPCM8XX Clock Controller Binding
>>>>> +
>>>>> +maintainers:
>>>>> +  - Tomer Maimon <tmaimon77@gmail.com>
>>>>> +
>>>>> +description: |
>>>>> +  Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller, which
>>>>> +  generates and supplies clocks to all modules within the BMC.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - nuvoton,npcm845-clk
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clocks:
>>>>> +    items:
>>>>> +      - description: 25M reference clock
>>>>> +      - description: CPU reference clock
>>>>> +      - description: MC reference clock
>>>>> +
>>>>> +  clock-names:
>>>>> +    items:
>>>>> +      - const: refclk
>>>>> +      - const: sysbypck
>>>>> +      - const: mcbypck
>>>>> +
>>>>
>>>> I asked what is the suffix about and you replied "ck"... ok, so let's
>>>> make clear. This should be:
>>>>
>>>>     items:
>>>>       - const: ref
>>>>       - const: sysbyp
>>>>       - const: mcbyp
>>>>
>>>> or something similar, without the same suffix all over.
>>> The clock names are the same clock name in our spec, this why we
>>> prefer to leave the clock names as is.
>>
>> The naming with useless suffixes does not help. If your spec had
>> "refclk_really_clock_this_is_a_clock" you also would insist on that? It
>> does not make sense.
> Sorry but I don't understand why the clock name cause an issue, we
> prefer it will be the same as in our spec-clock diagram
> BTW, the same naming found in NPCM7XX
> https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/clock/nuvoton,npcm750-clk.txt#L36

Because the names should not have irrelevant information.
interrupt-names should have "txirq". dma-names should not have "txdma".
clock-names should not have "refclock" or "refclk" because it is
irrelevant duplication. These are bindings, not DTS, so whatever you
have in your spec matters less. DTS is the representation of hardware
and there you can name clocks closer to the spec so it is easier for
you, if that's your preference.

And if your spec has "refclk_really_clock_this_is_a_clock" you still
should not use it.

Anyway, you should discuss it last time when I pointed it out. Instead
my comments were ignored and you decided to send v2. That's not how
discussion works and it will not bring you closer to your point.


Best regards,
Krzysztof

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Krzysztof Kozlowski]

On 09/06/2022 23:30, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your comments
> 
> On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 08/06/2022 11:56, Tomer Maimon wrote:
>>> Add nuvoton,sysgcr syscon property to the reset
>>> node to handle the general control registers.
>>
>> Wrong wrapping.
> it will be very helpful if you could point me what wrong wrapped in
> the commit message, is it the explanation or the header? or something
> else?

I pointed you last time. I pointed the exact line, exact rule you need
to follow. I pointed it three times already and three times I said
wrapping is wrong:
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586

"The body of the explanation, line wrapped at 75 columns, which will be
copied to the permanent changelog to describe this patch."

Your wrapping is not at 75 columns and it causes the commit to be less
readable, without any reason. Please follow Linux kernel coding style/rules.


Best regards,
Krzysztof

Re: [PATCH v2 11/20] reset: npcm: using syscon instead of device data

[Krzysztof Kozlowski]

On 09/06/2022 23:37, Tomer Maimon wrote:
> Hi Krzysztof
> 
> Sorry but I didn't ignore your comment.
> 
> For not breaking exciting boards I add the following patch in V2
> https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/

No, it does not solve it.
1. Patchset goes via separate trees (DTS are always separate), so it is
not bisectable. One of the branches/trees will have broken DTS.

2. All out of tree DTSes are broken. This is expressed as ABI and - with
some reasonable exceptions - you should not break it.
https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/ABI.rst

You have to keep backwards compatibility, so parse/handle both versions
of DTS.


Best regards,
Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Krzysztof Kozlowski]

On 10/06/2022 00:05, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Sorry, but I thought the fix is only to add an explanation to the
> dt-binding file as was done in V2.
> 
> The NPCM8XX binding is done in the same way as the NPCM7XX and both
> use the same reset driver and use the same reset method in upstreamed
> NPCM reset driver.
> 
> Can you please explain again what you suggest to do?

If you want abstract IDs, they must be abstract, so not representing
hardware registers. Then they start at 1 and are incremented by 1.

Other option is to skip such IDs entirely and use register
offsets/addresses directly, like Arnd suggested in linked documents. I
think he expressed it clearly, so please read his answers which I linked
in previous discussion.

There is no single reason to store register addresses/values/offsets as
binding headers. These are not bindings.

Best regards,
Krzysztof

Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Krzysztof Kozlowski]

On 10/06/2022 00:29, Tomer Maimon wrote:
>>> +     clk_refclk: clk-refclk {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <25000000>;
>>
>> Ignored comment.
> Could we use it as a default clock-frequency?

In DTS? If my assumption, that this clock is not on SoC itself, is
correct, then the answer is no, you cannot. The clock physically sits on
the board, so it is defined by board DTS. Feel free to embed in SoC DTSI
most of the clock properties, but the core property - frequency - must
be outside.

>>
>>> +             clock-output-names = "refclk";
>>> +     };
>>> +
>>> +     /* external reference clock for cpu. float in normal operation */
>>> +     clk_sysbypck: clk-sysbypck {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <1000000000>;
>>
>> Ignored comment.
> same as above
>>
>>> +             clock-output-names = "sysbypck";
>>> +     };
>>> +
>>> +     /* external reference clock for MC. float in normal operation */
>>> +     clk_mcbypck: clk-mcbypck {
>>> +             compatible = "fixed-clock";
>>> +             #clock-cells = <0>;
>>> +             clock-frequency = <1050000000>;
> same as above
>>> +             clock-output-names = "mcbypck";
>>> +     };
>>> +
>>> +     soc {
>>> +             #address-cells = <2>;
>>> +             #size-cells = <2>;
>>> +             compatible = "simple-bus";
>>> +             interrupt-parent = <&gic>;
>>> +             ranges;
>>> +
>>> +             gcr: gcr@f0800000 {
> I understand it sounds generic but I try to be as much compatible with NPCM7XX
> https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L91

Then fix NPCM7XX. Please do not multiple bad choices because it looks
similar. Fix the other wrong one.

>>
>> Ignored comment.
>>
>>> +                     compatible = "nuvoton,npcm845-gcr", "syscon",
>>> +                             "simple-mfd";
>>
>> This is not a simple-mfd... I see original bindings defined it that way,
>> but why? I think they should be corrected - remove simple-mfd from the
>> bindings and DTS.
> will remove in both places in V3
>>
>>
>>> +                     reg = <0x0 0xf0800000 0x0 0x1000>;
>>> +             };
>>> +
>>> +             gic: interrupt-controller@dfff9000 {
>>> +                     compatible = "arm,gic-400";
>>> +                     reg = <0x0 0xdfff9000 0x0 0x1000>,
>>> +                           <0x0 0xdfffa000 0x0 0x2000>,
>>> +                           <0x0 0xdfffc000 0x0 0x2000>,
>>> +                           <0x0 0xdfffe000 0x0 0x2000>;
>>> +                     interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
>>> +                     #interrupt-cells = <3>;
>>> +                     interrupt-controller;
>>> +                     #address-cells = <0>;
>>> +                     ppi-partitions {
>>> +                             ppi_cluster0: interrupt-partition-0 {
>>> +                                     affinity = <&cpu0 &cpu1 &cpu2 &cpu3>;
>>> +                             };
>>> +                     };
>>> +             };
>>> +     };
>>> +
>>> +     ahb {
>>> +             #address-cells = <2>;
>>> +             #size-cells = <2>;
>>> +             compatible = "simple-bus";
>>> +             interrupt-parent = <&gic>;
> 
>>> +             ranges;
>>> +
>>> +             rstc: rstc@f0801000 {
>>
>> Ignored comment.
>>
> I understand it sounds generic but I try to be as much compatible with NPCM7XX
> https://elixir.bootlin.com/linux/v5.19-rc1/source/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi#L109

Fix 7xx.


Best regards,
Krzysztof

Re: [PATCH v2 18/20] arm64: dts: nuvoton: Add initial NPCM8XX device tree

[Krzysztof Kozlowski]

On 10/06/2022 09:57, Geert Uytterhoeven wrote:
>>  "+             cpu0: cpu@0 {
>>  +                     device_type = "cpu";
>>  +                     compatible = "arm,cortex-a35";
>>  +                     clocks = <&clk NPCM8XX_CLK_CPU>;
>>  +                     reg = <0x0 0x0>;
>> Why do you have two address cells? A bit more complicated and not
>> necessary, I think."
>> the arm,cortex-a35 is 64 Bit this is why we use  #address-cells = <2>;
>> and therefore reg = <0x0 0x0>;
> 
> These addresses are not addresses on the main memory bus (which
> is indeed 64-bit), but on the logical CPU bus.
> Now, Documentation/devicetree/bindings/arm/cpus.yaml says you can
> have #address-cells = <2> if you have non-zero MPIDR_EL1 high bits.
> 

Thanks Tomer and Geert for explanation. OK for me.


Best regards,
Krzysztof

Re: [PATCH v2 10/20] ARM: dts: nuvoton: add reset syscon property

[Tomer Maimon]

Hi Krzysztof,

Thanks for the clarifications, will update the next version.

Best regards,

Tomer

On Fri, 10 Jun 2022 at 12:51, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/06/2022 23:30, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Thanks for your comments
> >
> > On Wed, 8 Jun 2022 at 13:07, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 08/06/2022 11:56, Tomer Maimon wrote:
> >>> Add nuvoton,sysgcr syscon property to the reset
> >>> node to handle the general control registers.
> >>
> >> Wrong wrapping.
> > it will be very helpful if you could point me what wrong wrapped in
> > the commit message, is it the explanation or the header? or something
> > else?
>
> I pointed you last time. I pointed the exact line, exact rule you need
> to follow. I pointed it three times already and three times I said
> wrapping is wrong:
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
>
> "The body of the explanation, line wrapped at 75 columns, which will be
> copied to the permanent changelog to describe this patch."
>
> Your wrapping is not at 75 columns and it causes the commit to be less
> readable, without any reason. Please follow Linux kernel coding style/rules.
>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 11/20] reset: npcm: using syscon instead of device data

[Tomer Maimon]

Hi Krzysztof,

I will make sure to add backward compatibility in the reset driver in
the next version.

Thanks,

Tomer


On Fri, 10 Jun 2022 at 12:53, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/06/2022 23:37, Tomer Maimon wrote:
> > Hi Krzysztof
> >
> > Sorry but I didn't ignore your comment.
> >
> > For not breaking exciting boards I add the following patch in V2
> > https://lore.kernel.org/linux-arm-kernel/20220608095623.22327-11-tmaimon77@gmail.com/
>
> No, it does not solve it.
> 1. Patchset goes via separate trees (DTS are always separate), so it is
> not bisectable. One of the branches/trees will have broken DTS.
>
> 2. All out of tree DTSes are broken. This is expressed as ABI and - with
> some reasonable exceptions - you should not break it.
> https://elixir.bootlin.com/linux/v5.19-rc1/source/Documentation/devicetree/bindings/ABI.rst
>
> You have to keep backwards compatibility, so parse/handle both versions
> of DTS.
>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Tomer Maimon]

Hi Krzysztof,

Thanks for your clarification.

We can remove the dt-binding file and use numbers in the DTS,
appreciate if you can answer few additional questions:
1. Do you suggest adding all NPCM reset values to the NPCM reset
document or the reset values should describe in the module
documentation that uses it?
2. Some of the NPCM7XX document modules describe the reset value they
use from the dt-binding for example:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
If we remove the NPCM8XX dt-binding file should we describe the
NPCM8XX values in the NPCM-ADC document file?

Best regards,

Tomer

On Fri, 10 Jun 2022 at 12:55, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 10/06/2022 00:05, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Sorry, but I thought the fix is only to add an explanation to the
> > dt-binding file as was done in V2.
> >
> > The NPCM8XX binding is done in the same way as the NPCM7XX and both
> > use the same reset driver and use the same reset method in upstreamed
> > NPCM reset driver.
> >
> > Can you please explain again what you suggest to do?
>
> If you want abstract IDs, they must be abstract, so not representing
> hardware registers. Then they start at 1 and are incremented by 1.
>
> Other option is to skip such IDs entirely and use register
> offsets/addresses directly, like Arnd suggested in linked documents. I
> think he expressed it clearly, so please read his answers which I linked
> in previous discussion.
>
> There is no single reason to store register addresses/values/offsets as
> binding headers. These are not bindings.
>
> Best regards,
> Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Krzysztof Kozlowski]

On 13/06/2022 02:25, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> Thanks for your clarification.
> 
> We can remove the dt-binding file and use numbers in the DTS,
> appreciate if you can answer few additional questions:
> 1. Do you suggest adding all NPCM reset values to the NPCM reset
> document or the reset values should describe in the module
> documentation that uses it?

What is "NPCM reset document"? Are these reset values anyhow different
than interrupts or pins?

> 2. Some of the NPCM7XX document modules describe the reset value they
> use from the dt-binding for example:
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61

This is NPCM750

> If we remove the NPCM8XX dt-binding file should we describe the
> NPCM8XX values in the NPCM-ADC document file?

What is NPCM-ADC document file? What do you want to describe there?
Again - how is it different than interrupts?



Best regards,
Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Tomer Maimon]

Hi Krzysztof,

On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 13/06/2022 02:25, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > Thanks for your clarification.
> >
> > We can remove the dt-binding file and use numbers in the DTS,
> > appreciate if you can answer few additional questions:
> > 1. Do you suggest adding all NPCM reset values to the NPCM reset
> > document or the reset values should describe in the module
> > documentation that uses it?
>
> What is "NPCM reset document"? Are these reset values anyhow different
> than interrupts or pins?
No, they represent the same values.
>
> > 2. Some of the NPCM7XX document modules describe the reset value they
> > use from the dt-binding for example:
> > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
>
> This is NPCM750
>
> > If we remove the NPCM8XX dt-binding file should we describe the
> > NPCM8XX values in the NPCM-ADC document file?
>
> What is NPCM-ADC document file? What do you want to describe there?
> Again - how is it different than interrupts?
It is not different from the interrupts.
I will remove the dt-binding reset include file, the reset property
will use numbers and not macro's.
>
>
>
> Best regards,
> Krzysztof

Best regards,

Tomer

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Krzysztof Kozlowski]

On 16/06/2022 06:24, Tomer Maimon wrote:
> Hi Krzysztof,
> 
> On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 13/06/2022 02:25, Tomer Maimon wrote:
>>> Hi Krzysztof,
>>>
>>> Thanks for your clarification.
>>>
>>> We can remove the dt-binding file and use numbers in the DTS,
>>> appreciate if you can answer few additional questions:
>>> 1. Do you suggest adding all NPCM reset values to the NPCM reset
>>> document or the reset values should describe in the module
>>> documentation that uses it?
>>
>> What is "NPCM reset document"? Are these reset values anyhow different
>> than interrupts or pins?
> No, they represent the same values.


We do not document in the bindings actual pin or interrupt numbers...

>>
>>> 2. Some of the NPCM7XX document modules describe the reset value they
>>> use from the dt-binding for example:
>>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
>>
>> This is NPCM750
>>
>>> If we remove the NPCM8XX dt-binding file should we describe the
>>> NPCM8XX values in the NPCM-ADC document file?
>>
>> What is NPCM-ADC document file? What do you want to describe there?
>> Again - how is it different than interrupts?
> It is not different from the interrupts.
> I will remove the dt-binding reset include file, the reset property
> will use numbers and not macro's.

I have no clue what are you referring now... This is NPCM8xx and it has
no binding header with reset values. What to remove then?


Best regards,
Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Tomer Maimon]

Hi Krzysztof,

On Thu, 16 Jun 2022 at 16:39, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 16/06/2022 06:24, Tomer Maimon wrote:
> > Hi Krzysztof,
> >
> > On Wed, 15 Jun 2022 at 20:03, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 13/06/2022 02:25, Tomer Maimon wrote:
> >>> Hi Krzysztof,
> >>>
> >>> Thanks for your clarification.
> >>>
> >>> We can remove the dt-binding file and use numbers in the DTS,
> >>> appreciate if you can answer few additional questions:
> >>> 1. Do you suggest adding all NPCM reset values to the NPCM reset
> >>> document or the reset values should describe in the module
> >>> documentation that uses it?
> >>
> >> What is "NPCM reset document"? Are these reset values anyhow different
> >> than interrupts or pins?
> > No, they represent the same values.
>
>
> We do not document in the bindings actual pin or interrupt numbers...
>
> >>
> >>> 2. Some of the NPCM7XX document modules describe the reset value they
> >>> use from the dt-binding for example:
> >>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/iio/adc/nuvoton%2Cnpcm750-adc.yaml#L61
> >>
> >> This is NPCM750
> >>
> >>> If we remove the NPCM8XX dt-binding file should we describe the
> >>> NPCM8XX values in the NPCM-ADC document file?
> >>
> >> What is NPCM-ADC document file? What do you want to describe there?
> >> Again - how is it different than interrupts?
> > It is not different from the interrupts.
> > I will remove the dt-binding reset include file, the reset property
> > will use numbers and not macro's.
>
> I have no clue what are you referring now... This is NPCM8xx and it has
> no binding header with reset values. What to remove then?
I refer nuvoton,npcm8xx-reset.h file, we don't need it.
>
>
> Best regards,
> Krzysztof

Re: [PATCH v2 12/20] dt-bindings: reset: npcm: Add support for NPCM8XX

[Krzysztof Kozlowski]

On 16/06/2022 06:41, Tomer Maimon wrote:

>>>> What is NPCM-ADC document file? What do you want to describe there?
>>>> Again - how is it different than interrupts?
>>> It is not different from the interrupts.
>>> I will remove the dt-binding reset include file, the reset property
>>> will use numbers and not macro's.
>>
>> I have no clue what are you referring now... This is NPCM8xx and it has
>> no binding header with reset values. What to remove then?
> I refer nuvoton,npcm8xx-reset.h file, we don't need it.

There is no such file in kernel, I believe. If you refer to the patchset
here, then of course it should not be sent.


Best regards,
Krzysztof

Re: [PATCH v2 05/20] watchdog: npcm_wdt: Add NPCM845 watchdog support

[Tomer Maimon]

Hi Arnd,

On Wed, 8 Jun 2022 at 16:05, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> > Add Nuvoton BMC NPCM845 watchdog support.
> > The NPCM845 uses the same watchdog as the NPCM750.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>
> This one should no longer be needed if the timers are compatible with the
> old ones and correctly described in the DT.
by timers do you mean clocks?
>
>       Arnd

Best regards,

Tomer

Re: [PATCH v2 05/20] watchdog: npcm_wdt: Add NPCM845 watchdog support

[Arnd Bergmann]

On Thu, Jun 16, 2022 at 11:06 PM Tomer Maimon <tmaimon77@gmail.com> wrote:
> On Wed, 8 Jun 2022 at 16:05, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Wed, Jun 8, 2022 at 11:56 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
> > >
> > > Add Nuvoton BMC NPCM845 watchdog support.
> > > The NPCM845 uses the same watchdog as the NPCM750.
> > >
> > > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> >
> > This one should no longer be needed if the timers are compatible with the
> > old ones and correctly described in the DT.
> by timers do you mean clocks?

Sorry, I mis-copied my comment. I meant the watchdog being compatible
with the old
version.

        Arnd