linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/3] arm: npcm: add basic support for Nuvoton BMCs
@ 2017-10-19 22:50 Brendan Higgins
  2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Brendan Higgins @ 2017-10-19 22:50 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, avifishman70, tmaimon77, raltherr,
	f.fainelli
  Cc: devicetree, linux-kernel, linux-arm-kernel, openbmc

Addressed comments from:
  - Rob: http://www.spinics.net/lists/devicetree/msg196376.html

Changes since previous update:
  - Moved all memory mapped nodes to busses.
  - Map ranges starting at 0
  - Moved external clocks to root
  - No changes to mach-npcm since v5
  - No changes to MAINTAINERS since v5

Changes have been tested on the NPCM750 evaluation board.

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

* [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-19 22:50 [PATCH v7 0/3] arm: npcm: add basic support for Nuvoton BMCs Brendan Higgins
@ 2017-10-19 22:50 ` Brendan Higgins
  2017-10-20 10:40   ` Julien Thierry
  2017-10-20 10:48   ` Russell King - ARM Linux
  2017-10-19 22:50 ` [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree Brendan Higgins
  2017-10-19 22:50 ` [PATCH v7 3/3] MAINTAINERS: Add entry for the Nuvoton NPCM architecture Brendan Higgins
  2 siblings, 2 replies; 13+ messages in thread
From: Brendan Higgins @ 2017-10-19 22:50 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, avifishman70, tmaimon77, raltherr,
	f.fainelli
  Cc: devicetree, linux-kernel, linux-arm-kernel, openbmc, Brendan Higgins

Adds basic support for the Nuvoton NPCM750 BMC.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
Reviewed-by: Avi Fishman <avifishman70@gmail.com>
Tested-by: Tomer Maimon <tmaimon77@gmail.com>
Tested-by: Avi Fishman <avifishman70@gmail.com>
---
 arch/arm/Kconfig             |  2 +
 arch/arm/Makefile            |  1 +
 arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
 arch/arm/mach-npcm/Makefile  |  3 ++
 arch/arm/mach-npcm/headsmp.S | 17 +++++++++
 arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
 arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 193 insertions(+)
 create mode 100644 arch/arm/mach-npcm/Kconfig
 create mode 100644 arch/arm/mach-npcm/Makefile
 create mode 100644 arch/arm/mach-npcm/headsmp.S
 create mode 100644 arch/arm/mach-npcm/npcm7xx.c
 create mode 100644 arch/arm/mach-npcm/platsmp.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 61a0cb15067e..05543f1cfbde 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
 
 source "arch/arm/mach-nomadik/Kconfig"
 
+source "arch/arm/mach-npcm/Kconfig"
+
 source "arch/arm/mach-nspire/Kconfig"
 
 source "arch/arm/plat-omap/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 47d3a1ab08d2..60ca50c7d762 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)		+= mediatek
 machine-$(CONFIG_ARCH_MXS)		+= mxs
 machine-$(CONFIG_ARCH_NETX)		+= netx
 machine-$(CONFIG_ARCH_NOMADIK)		+= nomadik
+machine-$(CONFIG_ARCH_NPCM)		+= npcm
 machine-$(CONFIG_ARCH_NSPIRE)		+= nspire
 machine-$(CONFIG_ARCH_OXNAS)		+= oxnas
 machine-$(CONFIG_ARCH_OMAP1)		+= omap1
diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
new file mode 100644
index 000000000000..21dfa8ce828f
--- /dev/null
+++ b/arch/arm/mach-npcm/Kconfig
@@ -0,0 +1,48 @@
+menuconfig ARCH_NPCM
+	bool "Nuvoton NPCM Architecture"
+	select ARCH_REQUIRE_GPIOLIB
+	select USE_OF
+	select PINCTRL
+	select PINCTRL_NPCM7XX
+
+if ARCH_NPCM
+
+comment "NPCMX50 CPU type"
+
+config CPU_NPCM750
+	depends on ARCH_NPCM && ARCH_MULTI_V7
+	bool "Support for NPCM750 BMC CPU (Poleg)"
+	select CACHE_L2X0
+	select CPU_V7
+	select ARM_GIC
+	select HAVE_SMP
+	select SMP
+	select SMP_ON_UP
+	select HAVE_ARM_SCU
+	select HAVE_ARM_TWD if SMP
+	select ARM_ERRATA_720789
+	select ARM_ERRATA_754322
+	select ARM_ERRATA_764369
+	select ARM_ERRATA_794072
+	select PL310_ERRATA_588369
+	select PL310_ERRATA_727915
+	select USB_EHCI_ROOT_HUB_TT
+	select USB_ARCH_HAS_HCD
+	select USB_ARCH_HAS_EHCI
+	select USB_EHCI_HCD
+	select USB_ARCH_HAS_OHCI
+	select USB_OHCI_HCD
+	select USB
+	select FIQ
+	select CPU_USE_DOMAINS
+	select GENERIC_CLOCKEVENTS
+	select CLKDEV_LOOKUP
+	select COMMON_CLK if OF
+	select NPCM750_TIMER
+	select MFD_SYSCON
+	help
+	  Support for NPCM750 BMC CPU (Poleg).
+
+	  Nuvoton NPCM750 BMC based on the Cortex A9.
+
+endif
diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
new file mode 100644
index 000000000000..78416055b854
--- /dev/null
+++ b/arch/arm/mach-npcm/Makefile
@@ -0,0 +1,3 @@
+AFLAGS_headsmp.o		+= -march=armv7-a
+
+obj-$(CONFIG_CPU_NPCM750)	+= npcm7xx.o platsmp.o headsmp.o
diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
new file mode 100644
index 000000000000..9fccbbd49ed4
--- /dev/null
+++ b/arch/arm/mach-npcm/headsmp.S
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+#include <asm/assembler.h>
+
+ENTRY(npcm7xx_secondary_startup)
+	safe_svcmode_maskall r0
+
+	b	secondary_startup
+ENDPROC(npcm7xx_secondary_startup)
diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
new file mode 100644
index 000000000000..132e9d587857
--- /dev/null
+++ b/arch/arm/mach-npcm/npcm7xx.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2017 Nuvoton Technology corporation.
+ * Copyright 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <asm/mach/arch.h>
+#include <asm/mach-types.h>
+#include <asm/mach/map.h>
+#include <asm/hardware/cache-l2x0.h>
+
+#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |			       \
+			 L310_AUX_CTRL_DATA_PREFETCH |			       \
+			 L310_AUX_CTRL_NS_LOCKDOWN |			       \
+			 L310_AUX_CTRL_CACHE_REPLACE_RR |		       \
+			 L2C_AUX_CTRL_SHARED_OVERRIDE |			       \
+			 L310_AUX_CTRL_FULL_LINE_ZERO)
+
+static const char *const npcm7xx_dt_match[] = {
+	"nuvoton,npcm750",
+	NULL
+};
+
+DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
+	.atag_offset	= 0x100,
+	.dt_compat	= npcm7xx_dt_match,
+	.l2c_aux_val	= NPCM7XX_AUX_VAL,
+	.l2c_aux_mask	= ~NPCM7XX_AUX_VAL,
+MACHINE_END
diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
new file mode 100644
index 000000000000..450e83c3c531
--- /dev/null
+++ b/arch/arm/mach-npcm/platsmp.c
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "PLATSMP: " fmt
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/smp.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <asm/cacheflush.h>
+#include <asm/smp.h>
+#include <asm/smp_plat.h>
+#include <asm/smp_scu.h>
+
+#define NPCM7XX_SCRPAD_REG 0x13c
+
+extern void npcm7xx_secondary_startup(void);
+
+static int npcm7xx_smp_boot_secondary(unsigned int cpu,
+				      struct task_struct *idle)
+{
+	struct device_node *gcr_np;
+	void __iomem *gcr_base;
+	int ret = 0;
+
+	gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
+	if (!gcr_np) {
+		pr_err("no gcr device node\n");
+		ret = -EFAULT;
+		goto out;
+	}
+	gcr_base = of_iomap(gcr_np, 0);
+	if (!gcr_base) {
+		pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
+		ret = -EFAULT;
+		goto out;
+	}
+
+	/* give boot ROM kernel start address. */
+	iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
+		  NPCM7XX_SCRPAD_REG);
+	/* make sure npcm7xx_secondary_startup is seen by all observers. */
+	smp_wmb();
+	dsb_sev();
+	/* make sure write buffer is drained */
+	mb();
+
+	iounmap(gcr_base);
+out:
+	return ret;
+}
+
+static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
+{
+	struct device_node *scu_np;
+	void __iomem *scu_base;
+
+	scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
+	if (!scu_np) {
+		pr_err("no scu device node\n");
+		return;
+	}
+	scu_base = of_iomap(scu_np, 0);
+	if (!scu_base) {
+		pr_err("could not iomap scu at: 0x%p\n", scu_base);
+		return;
+	}
+
+	scu_enable(scu_base);
+
+	iounmap(scu_base);
+}
+
+static struct smp_operations npcm7xx_smp_ops __initdata = {
+	.smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
+	.smp_boot_secondary = npcm7xx_smp_boot_secondary,
+};
+
+CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
-- 
2.15.0.rc0.271.g36b669edcc-goog

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

* [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree
  2017-10-19 22:50 [PATCH v7 0/3] arm: npcm: add basic support for Nuvoton BMCs Brendan Higgins
  2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
@ 2017-10-19 22:50 ` Brendan Higgins
  2017-10-20 19:45   ` Rob Herring
  2017-10-19 22:50 ` [PATCH v7 3/3] MAINTAINERS: Add entry for the Nuvoton NPCM architecture Brendan Higgins
  2 siblings, 1 reply; 13+ messages in thread
From: Brendan Higgins @ 2017-10-19 22:50 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, avifishman70, tmaimon77, raltherr,
	f.fainelli
  Cc: devicetree, linux-kernel, linux-arm-kernel, openbmc, Brendan Higgins

Add a common device tree for all Nuvoton NPCM750 BMCs and a board
specific device tree for the NPCM750 (Poleg) evaluation board.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
Reviewed-by: Avi Fishman <avifishman70@gmail.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Tomer Maimon <tmaimon77@gmail.com>
Tested-by: Avi Fishman <avifishman70@gmail.com>
---
 .../arm/cpu-enable-method/nuvoton,npcm7xx-smp      |  42 +++++
 .../devicetree/bindings/arm/npcm/npcm.txt          |   6 +
 arch/arm/boot/dts/nuvoton-npcm750-evb.dts          |  48 +++++
 arch/arm/boot/dts/nuvoton-npcm750.dtsi             | 198 +++++++++++++++++++++
 include/dt-bindings/clock/nuvoton,npcm7xx-clks.h   |  39 ++++
 5 files changed, 333 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/cpu-enable-method/nuvoton,npcm7xx-smp
 create mode 100644 Documentation/devicetree/bindings/arm/npcm/npcm.txt
 create mode 100644 arch/arm/boot/dts/nuvoton-npcm750-evb.dts
 create mode 100644 arch/arm/boot/dts/nuvoton-npcm750.dtsi
 create mode 100644 include/dt-bindings/clock/nuvoton,npcm7xx-clks.h

diff --git a/Documentation/devicetree/bindings/arm/cpu-enable-method/nuvoton,npcm7xx-smp b/Documentation/devicetree/bindings/arm/cpu-enable-method/nuvoton,npcm7xx-smp
new file mode 100644
index 000000000000..e81f85b400cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/cpu-enable-method/nuvoton,npcm7xx-smp
@@ -0,0 +1,42 @@
+=========================================================
+Secondary CPU enable-method "nuvoton,npcm7xx-smp" binding
+=========================================================
+
+To apply to all CPUs, a single "nuvoton,npcm7xx-smp" enable method should be
+defined in the "cpus" node.
+
+Enable method name:	"nuvoton,npcm7xx-smp"
+Compatible machines:	"nuvoton,npcm750"
+Compatible CPUs:	"arm,cortex-a9"
+Related properties:	(none)
+
+Note:
+This enable method needs valid nodes compatible with "arm,cortex-a9-scu" and
+"nuvoton,npcm750-gcr".
+
+Example:
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "nuvoton,npcm7xx-smp";
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			clocks = <&clk NPCM7XX_CLK_CPU>;
+			clock-names = "clk_cpu";
+			reg = <0>;
+			next-level-cache = <&L2>;
+		};
+
+		cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			clocks = <&clk NPCM7XX_CLK_CPU>;
+			clock-names = "clk_cpu";
+			reg = <1>;
+			next-level-cache = <&L2>;
+		};
+	};
+
diff --git a/Documentation/devicetree/bindings/arm/npcm/npcm.txt b/Documentation/devicetree/bindings/arm/npcm/npcm.txt
new file mode 100644
index 000000000000..2d87d9ecea85
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/npcm/npcm.txt
@@ -0,0 +1,6 @@
+NPCM Platforms Device Tree Bindings
+-----------------------------------
+NPCM750 SoC
+Required root node properties:
+	- compatible = "nuvoton,npcm750";
+
diff --git a/arch/arm/boot/dts/nuvoton-npcm750-evb.dts b/arch/arm/boot/dts/nuvoton-npcm750-evb.dts
new file mode 100644
index 000000000000..a0675e584125
--- /dev/null
+++ b/arch/arm/boot/dts/nuvoton-npcm750-evb.dts
@@ -0,0 +1,48 @@
+/*
+ * DTS file for all NPCM750 SoCs
+ *
+ * Copyright 2012 Tomer Maimon <tomer.maimon@nuvoton.com>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+/dts-v1/;
+#include "nuvoton-npcm750.dtsi"
+
+/ {
+	model = "Nuvoton npcm750 Development Board (Device Tree)";
+	compatible = "nuvoton,npcm750";
+
+	chosen {
+		stdout-path = &serial3;
+	};
+
+	memory {
+		reg = <0 0x40000000>;
+	};
+};
+
+&watchdog1 {
+	status = "okay";
+};
+
+&serial0 {
+	status = "okay";
+};
+
+&serial1 {
+	status = "okay";
+};
+
+&serial2 {
+	status = "okay";
+};
+
+&serial3 {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/nuvoton-npcm750.dtsi b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
new file mode 100644
index 000000000000..85506b3cdd39
--- /dev/null
+++ b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
@@ -0,0 +1,198 @@
+/*
+ * DTSi file for the NPCM750 SoC
+ *
+ * Copyright 2012 Tomer Maimon <tomer.maimon@nuvoton.com>
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/nuvoton,npcm7xx-clks.h>
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&gic>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		enable-method = "nuvoton,npcm7xx-smp";
+
+		cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			clocks = <&clk NPCM7XX_CLK_CPU>;
+			clock-names = "clk_cpu";
+			reg = <0>;
+			next-level-cache = <&l2>;
+		};
+
+		cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a9";
+			clocks = <&clk NPCM7XX_CLK_CPU>;
+			clock-names = "clk_cpu";
+			reg = <1>;
+			next-level-cache = <&l2>;
+		};
+	};
+
+/* external clock signal rg1refck, supplied by the phy */
+clk-rg1refck {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <125000000>;
+};
+
+/* external clock signal rg2refck, supplied by the phy */
+clk-rg2refck {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <125000000>;
+};
+
+clk-xin {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <50000000>;
+};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		ranges = <0x0 0xf0000000 0x00900000>;
+
+		gcr: gcr@f0800000 {
+			compatible = "nuvoton,npcm750-gcr", "syscon",
+				"simple-mfd";
+			reg = <0x800000 0x1000>;
+		};
+
+		scu: scu@f03fe000 {
+			compatible = "arm,cortex-a9-scu";
+			reg = <0x3fe000 0x1000>;
+		};
+
+		l2: cache-controller@f03fc000 {
+			compatible = "arm,pl310-cache";
+			reg = <0x3fc000 0x1000>;
+			interrupts = <0 21 4>;
+			cache-unified;
+			cache-level = <2>;
+			clocks = <&clk NPCM7XX_CLK_AXI>;
+		};
+
+		gic: interrupt-controller@f03ff000 {
+			compatible = "arm,cortex-a9-gic";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			reg = <0x3ff000 0x1000>,
+			    <0x3fe100 0x100>;
+		};
+
+		timer@f03fe600 {
+			compatible = "arm,cortex-a9-twd-timer";
+			reg = <0x3fe600 0x20>;
+			interrupts = <1 13 0x304>;
+			clocks = <&clk NPCM7XX_CLK_TIMER>;
+		};
+	};
+
+	ahb {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&gic>;
+		ranges = <0x80000000 0x80000000 0x40010000
+			  0xe0800000 0xe0800000 0x0f800000
+			  0x00000000 0xf0000000 0x00900000
+			  0xf8000000 0xf8000000 0x02000000
+			  0xfb000000 0xfb000000 0x00002000>;
+
+		clk: clock-controller@f0801000 {
+			compatible = "nuvoton,npcm750-clk";
+			#clock-cells = <1>;
+			reg = <0x801000 0x1000>;
+			status = "okay";
+		};
+
+		apb {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-bus";
+			interrupt-parent = <&gic>;
+			ranges = <0x0 0x0 0x00300000>;
+
+			timer0: timer@f0000000 {
+				compatible = "nuvoton,npcm750-timer";
+				interrupts = <0 32 4>;
+				reg = <0x0 0x1000>;
+				clocks = <&clk NPCM7XX_CLK_TIMER>;
+			};
+
+			watchdog0: watchdog@f0008000 {
+				compatible = "nuvoton,npcm750-wdt";
+				interrupts = <0 47 4>;
+				reg = <0x8000 0x1000>;
+				status = "disabled";
+				clocks = <&clk NPCM7XX_CLK_TIMER>;
+			};
+
+			watchdog1: watchdog@f0009000 {
+				compatible = "nuvoton,npcm750-wdt";
+				interrupts = <0 48 4>;
+				reg = <0x9000 0x1000>;
+				status = "disabled";
+				clocks = <&clk NPCM7XX_CLK_TIMER>;
+			};
+
+			watchdog2: watchdog@f000a000 {
+				compatible = "nuvoton,npcm750-wdt";
+				interrupts = <0 49 4>;
+				reg = <0xa000 0x1000>;
+				status = "disabled";
+				clocks = <&clk NPCM7XX_CLK_TIMER>;
+			};
+
+			serial0: serial@f0001000 {
+				compatible = "nuvoton,npcm750-uart";
+				reg = <0x1000 0x1000>;
+				clocks = <&clk NPCM7XX_CLK_UART_CORE>;
+				interrupts = <0 2 4>;
+				status = "disabled";
+			};
+
+			serial1: serial@f0002000 {
+				compatible = "nuvoton,npcm750-uart";
+				reg = <0x2000 0x1000>;
+				clocks = <&clk NPCM7XX_CLK_UART_CORE>;
+				interrupts = <0 3 4>;
+				status = "disabled";
+			};
+
+			serial2: serial@f0003000 {
+				compatible = "nuvoton,npcm750-uart";
+				reg = <0x3000 0x1000>;
+				clocks = <&clk NPCM7XX_CLK_UART_CORE>;
+				interrupts = <0 4 4>;
+				status = "disabled";
+			};
+
+			serial3: serial@f0004000 {
+				compatible = "nuvoton,npcm750-uart";
+				reg = <0x4000 0x1000>;
+				clocks = <&clk NPCM7XX_CLK_UART_CORE>;
+				interrupts = <0 5 4>;
+				status = "disabled";
+			};
+		};
+	};
+};
diff --git a/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
new file mode 100644
index 000000000000..c69d3bbf7e42
--- /dev/null
+++ b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2016 Nuvoton Technologies,  tali.perry@nuvoton.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ */
+
+#ifndef _DT_BINDINGS_CLK_NPCM7XX_H
+#define _DT_BINDINGS_CLK_NPCM7XX_H
+
+#define NPCM7XX_CLK_PLL0	0
+#define NPCM7XX_CLK_PLL1	1
+#define NPCM7XX_CLK_PLL2	2
+#define NPCM7XX_CLK_GFX		3
+#define NPCM7XX_CLK_APB1	4
+#define NPCM7XX_CLK_APB2	5
+#define NPCM7XX_CLK_APB3	6
+#define NPCM7XX_CLK_APB4	7
+#define NPCM7XX_CLK_APB5	8
+#define NPCM7XX_CLK_MC		9
+#define NPCM7XX_CLK_CPU		10
+#define NPCM7XX_CLK_SPI0	11
+#define NPCM7XX_CLK_SPI3	12
+#define NPCM7XX_CLK_SPIX	13
+#define NPCM7XX_CLK_UART_CORE	14
+#define NPCM7XX_CLK_TIMER	15
+#define NPCM7XX_CLK_HOST_UART	16
+#define NPCM7XX_CLK_MMC		17
+#define NPCM7XX_CLK_SDHC	18
+#define NPCM7XX_CLK_ADC		19
+#define NPCM7XX_CLK_GFX_MEM	20
+#define NPCM7XX_CLK_USB_BRIDGE	21
+#define NPCM7XX_CLK_AXI		22
+#define NPCM7XX_CLK_AHB		23
+#define NPCM7XX_CLK_EMC		24
+#define NPCM7XX_CLK_GMAC	25
+
+#endif
-- 
2.15.0.rc0.271.g36b669edcc-goog

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

* [PATCH v7 3/3] MAINTAINERS: Add entry for the Nuvoton NPCM architecture
  2017-10-19 22:50 [PATCH v7 0/3] arm: npcm: add basic support for Nuvoton BMCs Brendan Higgins
  2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
  2017-10-19 22:50 ` [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree Brendan Higgins
@ 2017-10-19 22:50 ` Brendan Higgins
  2 siblings, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2017-10-19 22:50 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, avifishman70, tmaimon77, raltherr,
	f.fainelli
  Cc: devicetree, linux-kernel, linux-arm-kernel, openbmc, Brendan Higgins

Add maintainers and reviewers for the Nuvoton NPCM architecture.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
Reviewed-by: Avi Fishman <avifishman70@gmail.com>
---
 MAINTAINERS | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 44cb004c765d..ec0d35384a16 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1598,6 +1598,19 @@ F:	drivers/pinctrl/nomadik/
 F:	drivers/i2c/busses/i2c-nomadik.c
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git
 
+ARM/NUVOTON NPCM ARCHITECTURE
+M:	Avi Fishman <avifishman70@gmail.com>
+M:	Tomer Maimon <tmaimon77@gmail.com>
+R:	Brendan Higgins <brendanhiggins@google.com>
+R:	Rick Altherr <raltherr@google.com>
+L:	openbmc@lists.ozlabs.org (moderated for non-subscribers)
+S:	Supported
+F:	arch/arm/mach-npcm/
+F:	arch/arm/boot/dts/nuvoton-npcm*
+F:	include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
+F:	drivers/*/*npcm*
+F:	Documentation/*/*npcm*
+
 ARM/NUVOTON W90X900 ARM ARCHITECTURE
 M:	Wan ZongShun <mcuos.com@gmail.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
-- 
2.15.0.rc0.271.g36b669edcc-goog

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
@ 2017-10-20 10:40   ` Julien Thierry
  2017-10-20 10:48   ` Russell King - ARM Linux
  1 sibling, 0 replies; 13+ messages in thread
From: Julien Thierry @ 2017-10-20 10:40 UTC (permalink / raw)
  To: Brendan Higgins, robh+dt, mark.rutland, linux, avifishman70,
	tmaimon77, raltherr, f.fainelli
  Cc: devicetree, linux-kernel, linux-arm-kernel, openbmc

Hi Brendan,

On 19/10/17 23:50, Brendan Higgins wrote:
> Adds basic support for the Nuvoton NPCM750 BMC.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
> Tested-by: Avi Fishman <avifishman70@gmail.com>
> ---
>   arch/arm/Kconfig             |  2 +
>   arch/arm/Makefile            |  1 +
>   arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>   arch/arm/mach-npcm/Makefile  |  3 ++
>   arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>   arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>   arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 193 insertions(+)
>   create mode 100644 arch/arm/mach-npcm/Kconfig
>   create mode 100644 arch/arm/mach-npcm/Makefile
>   create mode 100644 arch/arm/mach-npcm/headsmp.S
>   create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>   create mode 100644 arch/arm/mach-npcm/platsmp.c
> 

[...]

> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
> new file mode 100644
> index 000000000000..450e83c3c531
> --- /dev/null
> +++ b/arch/arm/mach-npcm/platsmp.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright 2017 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "PLATSMP: " fmt
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/smp.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <asm/cacheflush.h>
> +#include <asm/smp.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>
> +
> +#define NPCM7XX_SCRPAD_REG 0x13c
> +
> +extern void npcm7xx_secondary_startup(void);
> +
> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
> +				      struct task_struct *idle)
> +{
> +	struct device_node *gcr_np;
> +	void __iomem *gcr_base;
> +	int ret = 0;
> +
> +	gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
> +	if (!gcr_np) {
> +		pr_err("no gcr device node\n");
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +	gcr_base = of_iomap(gcr_np, 0);
> +	if (!gcr_base) {
> +		pr_err("could not iomap gcr at: 0x%p\n", gcr_base);

nit:
 From the condition we know gcr_base is NULL here so we don't get much 
info from it. Did you intend to print the value of gcr_np instead?

> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	/* give boot ROM kernel start address. */
> +	iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
> +		  NPCM7XX_SCRPAD_REG);
> +	/* make sure npcm7xx_secondary_startup is seen by all observers. */
> +	smp_wmb();
> +	dsb_sev();
> +	/* make sure write buffer is drained */
> +	mb();
> +
> +	iounmap(gcr_base);
> +out:
> +	return ret;
> +}
> +
> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *scu_np;
> +	void __iomem *scu_base;
> +
> +	scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> +	if (!scu_np) {
> +		pr_err("no scu device node\n");
> +		return;
> +	}
> +	scu_base = of_iomap(scu_np, 0);
> +	if (!scu_base) {
> +		pr_err("could not iomap scu at: 0x%p\n", scu_base);

Same comment about scu_base.

Cheers,

-- 
Julien Thierry

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
  2017-10-20 10:40   ` Julien Thierry
@ 2017-10-20 10:48   ` Russell King - ARM Linux
  2017-10-20 20:57     ` Brendan Higgins
  1 sibling, 1 reply; 13+ messages in thread
From: Russell King - ARM Linux @ 2017-10-20 10:48 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: robh+dt, mark.rutland, avifishman70, tmaimon77, raltherr,
	f.fainelli, devicetree, linux-kernel, linux-arm-kernel, openbmc

On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
> Adds basic support for the Nuvoton NPCM750 BMC.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
> Tested-by: Avi Fishman <avifishman70@gmail.com>
> ---
>  arch/arm/Kconfig             |  2 +
>  arch/arm/Makefile            |  1 +
>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>  arch/arm/mach-npcm/Makefile  |  3 ++
>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 193 insertions(+)
>  create mode 100644 arch/arm/mach-npcm/Kconfig
>  create mode 100644 arch/arm/mach-npcm/Makefile
>  create mode 100644 arch/arm/mach-npcm/headsmp.S
>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>  create mode 100644 arch/arm/mach-npcm/platsmp.c
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 61a0cb15067e..05543f1cfbde 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
>  
>  source "arch/arm/mach-nomadik/Kconfig"
>  
> +source "arch/arm/mach-npcm/Kconfig"
> +
>  source "arch/arm/mach-nspire/Kconfig"
>  
>  source "arch/arm/plat-omap/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 47d3a1ab08d2..60ca50c7d762 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)		+= mediatek
>  machine-$(CONFIG_ARCH_MXS)		+= mxs
>  machine-$(CONFIG_ARCH_NETX)		+= netx
>  machine-$(CONFIG_ARCH_NOMADIK)		+= nomadik
> +machine-$(CONFIG_ARCH_NPCM)		+= npcm
>  machine-$(CONFIG_ARCH_NSPIRE)		+= nspire
>  machine-$(CONFIG_ARCH_OXNAS)		+= oxnas
>  machine-$(CONFIG_ARCH_OMAP1)		+= omap1
> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
> new file mode 100644
> index 000000000000..21dfa8ce828f
> --- /dev/null
> +++ b/arch/arm/mach-npcm/Kconfig
> @@ -0,0 +1,48 @@
> +menuconfig ARCH_NPCM
> +	bool "Nuvoton NPCM Architecture"
> +	select ARCH_REQUIRE_GPIOLIB
> +	select USE_OF
> +	select PINCTRL
> +	select PINCTRL_NPCM7XX
> +
> +if ARCH_NPCM
> +
> +comment "NPCMX50 CPU type"
> +
> +config CPU_NPCM750
> +	depends on ARCH_NPCM && ARCH_MULTI_V7
> +	bool "Support for NPCM750 BMC CPU (Poleg)"
> +	select CACHE_L2X0
> +	select CPU_V7
> +	select ARM_GIC
> +	select HAVE_SMP
> +	select SMP
> +	select SMP_ON_UP
> +	select HAVE_ARM_SCU
> +	select HAVE_ARM_TWD if SMP
> +	select ARM_ERRATA_720789
> +	select ARM_ERRATA_754322
> +	select ARM_ERRATA_764369
> +	select ARM_ERRATA_794072
> +	select PL310_ERRATA_588369
> +	select PL310_ERRATA_727915
> +	select USB_EHCI_ROOT_HUB_TT
> +	select USB_ARCH_HAS_HCD
> +	select USB_ARCH_HAS_EHCI
> +	select USB_EHCI_HCD
> +	select USB_ARCH_HAS_OHCI
> +	select USB_OHCI_HCD
> +	select USB
> +	select FIQ
> +	select CPU_USE_DOMAINS
> +	select GENERIC_CLOCKEVENTS
> +	select CLKDEV_LOOKUP
> +	select COMMON_CLK if OF
> +	select NPCM750_TIMER
> +	select MFD_SYSCON
> +	help
> +	  Support for NPCM750 BMC CPU (Poleg).
> +
> +	  Nuvoton NPCM750 BMC based on the Cortex A9.
> +
> +endif
> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
> new file mode 100644
> index 000000000000..78416055b854
> --- /dev/null
> +++ b/arch/arm/mach-npcm/Makefile
> @@ -0,0 +1,3 @@
> +AFLAGS_headsmp.o		+= -march=armv7-a
> +
> +obj-$(CONFIG_CPU_NPCM750)	+= npcm7xx.o platsmp.o headsmp.o
> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
> new file mode 100644
> index 000000000000..9fccbbd49ed4
> --- /dev/null
> +++ b/arch/arm/mach-npcm/headsmp.S
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright 2017 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/linkage.h>
> +#include <linux/init.h>
> +#include <asm/assembler.h>
> +
> +ENTRY(npcm7xx_secondary_startup)
> +	safe_svcmode_maskall r0
> +
> +	b	secondary_startup
> +ENDPROC(npcm7xx_secondary_startup)

Why do you need this?  The switch to svc mode is already handled by
secondary_startup.

> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
> new file mode 100644
> index 000000000000..132e9d587857
> --- /dev/null
> +++ b/arch/arm/mach-npcm/npcm7xx.c
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (c) 2017 Nuvoton Technology corporation.
> + * Copyright 2017 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach-types.h>
> +#include <asm/mach/map.h>
> +#include <asm/hardware/cache-l2x0.h>
> +
> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |			       \
> +			 L310_AUX_CTRL_DATA_PREFETCH |			       \
> +			 L310_AUX_CTRL_NS_LOCKDOWN |			       \
> +			 L310_AUX_CTRL_CACHE_REPLACE_RR |		       \
> +			 L2C_AUX_CTRL_SHARED_OVERRIDE |			       \
> +			 L310_AUX_CTRL_FULL_LINE_ZERO)

Do you really need all these flags?

> +
> +static const char *const npcm7xx_dt_match[] = {
> +	"nuvoton,npcm750",
> +	NULL
> +};
> +
> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
> +	.atag_offset	= 0x100,
> +	.dt_compat	= npcm7xx_dt_match,
> +	.l2c_aux_val	= NPCM7XX_AUX_VAL,
> +	.l2c_aux_mask	= ~NPCM7XX_AUX_VAL,
> +MACHINE_END
> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
> new file mode 100644
> index 000000000000..450e83c3c531
> --- /dev/null
> +++ b/arch/arm/mach-npcm/platsmp.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright 2017 Google, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "PLATSMP: " fmt

Maybe something more descriptive?

> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/smp.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <asm/cacheflush.h>
> +#include <asm/smp.h>
> +#include <asm/smp_plat.h>
> +#include <asm/smp_scu.h>
> +
> +#define NPCM7XX_SCRPAD_REG 0x13c
> +
> +extern void npcm7xx_secondary_startup(void);
> +
> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
> +				      struct task_struct *idle)
> +{
> +	struct device_node *gcr_np;
> +	void __iomem *gcr_base;
> +	int ret = 0;
> +
> +	gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
> +	if (!gcr_np) {
> +		pr_err("no gcr device node\n");
> +		ret = -EFAULT;

EFAULT is used for bad userspace addresses, it should not be used for
other stuff - and a missing node is certainly not a "Bad address".

> +		goto out;
> +	}
> +	gcr_base = of_iomap(gcr_np, 0);
> +	if (!gcr_base) {
> +		pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
> +		ret = -EFAULT;

This is more -ENOMEM.

> +		goto out;
> +	}
> +
> +	/* give boot ROM kernel start address. */
> +	iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
> +		  NPCM7XX_SCRPAD_REG);
> +	/* make sure npcm7xx_secondary_startup is seen by all observers. */
> +	smp_wmb();

I think you mean "make sure the previous write is seen by all observers".
However, doesn't "dsb_sev()" already do that?

> +	dsb_sev();
> +	/* make sure write buffer is drained */
> +	mb();

This looks over the top. 

> +
> +	iounmap(gcr_base);
> +out:
> +	return ret;
> +}
> +
> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
> +{
> +	struct device_node *scu_np;
> +	void __iomem *scu_base;
> +
> +	scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> +	if (!scu_np) {
> +		pr_err("no scu device node\n");
> +		return;
> +	}
> +	scu_base = of_iomap(scu_np, 0);
> +	if (!scu_base) {
> +		pr_err("could not iomap scu at: 0x%p\n", scu_base);
> +		return;
> +	}
> +
> +	scu_enable(scu_base);
> +
> +	iounmap(scu_base);
> +}
> +
> +static struct smp_operations npcm7xx_smp_ops __initdata = {
> +	.smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
> +	.smp_boot_secondary = npcm7xx_smp_boot_secondary,
> +};
> +
> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
> -- 
> 2.15.0.rc0.271.g36b669edcc-goog
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

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

* Re: [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree
  2017-10-19 22:50 ` [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree Brendan Higgins
@ 2017-10-20 19:45   ` Rob Herring
  2017-11-17  2:50     ` Brendan Higgins
  0 siblings, 1 reply; 13+ messages in thread
From: Rob Herring @ 2017-10-20 19:45 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Mark Rutland, Russell King, avifishman70, Tomer Maimon,
	Rick Altherr, Florian Fainelli, devicetree, linux-kernel,
	linux-arm-kernel, openbmc

On Thu, Oct 19, 2017 at 5:50 PM, Brendan Higgins
<brendanhiggins@google.com> wrote:
> Add a common device tree for all Nuvoton NPCM750 BMCs and a board
> specific device tree for the NPCM750 (Poleg) evaluation board.
>
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
> Tested-by: Avi Fishman <avifishman70@gmail.com>
> ---

[...]

> diff --git a/arch/arm/boot/dts/nuvoton-npcm750.dtsi b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
> new file mode 100644
> index 000000000000..85506b3cdd39
> --- /dev/null
> +++ b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
> @@ -0,0 +1,198 @@
> +/*
> + * DTSi file for the NPCM750 SoC
> + *
> + * Copyright 2012 Tomer Maimon <tomer.maimon@nuvoton.com>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/nuvoton,npcm7xx-clks.h>
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +       interrupt-parent = <&gic>;
> +
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               enable-method = "nuvoton,npcm7xx-smp";
> +
> +               cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a9";
> +                       clocks = <&clk NPCM7XX_CLK_CPU>;
> +                       clock-names = "clk_cpu";
> +                       reg = <0>;
> +                       next-level-cache = <&l2>;
> +               };
> +
> +               cpu@1 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a9";
> +                       clocks = <&clk NPCM7XX_CLK_CPU>;
> +                       clock-names = "clk_cpu";
> +                       reg = <1>;
> +                       next-level-cache = <&l2>;
> +               };
> +       };
> +
> +/* external clock signal rg1refck, supplied by the phy */
> +clk-rg1refck {
> +       compatible = "fixed-clock";
> +       #clock-cells = <0>;
> +       clock-frequency = <125000000>;
> +};
> +
> +/* external clock signal rg2refck, supplied by the phy */
> +clk-rg2refck {
> +       compatible = "fixed-clock";
> +       #clock-cells = <0>;
> +       clock-frequency = <125000000>;
> +};
> +
> +clk-xin {
> +       compatible = "fixed-clock";
> +       #clock-cells = <0>;
> +       clock-frequency = <50000000>;
> +};
> +
> +       soc {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "simple-bus";
> +               interrupt-parent = <&gic>;
> +               ranges = <0x0 0xf0000000 0x00900000>;
> +
> +               gcr: gcr@f0800000 {

Should be gcr@800000. Build the dtb with W=1 and it will give you
warnings for this.

> +                       compatible = "nuvoton,npcm750-gcr", "syscon",
> +                               "simple-mfd";
> +                       reg = <0x800000 0x1000>;
> +               };
> +
> +               scu: scu@f03fe000 {
> +                       compatible = "arm,cortex-a9-scu";
> +                       reg = <0x3fe000 0x1000>;
> +               };
> +
> +               l2: cache-controller@f03fc000 {
> +                       compatible = "arm,pl310-cache";
> +                       reg = <0x3fc000 0x1000>;
> +                       interrupts = <0 21 4>;
> +                       cache-unified;
> +                       cache-level = <2>;
> +                       clocks = <&clk NPCM7XX_CLK_AXI>;
> +               };
> +
> +               gic: interrupt-controller@f03ff000 {
> +                       compatible = "arm,cortex-a9-gic";
> +                       interrupt-controller;
> +                       #interrupt-cells = <3>;
> +                       reg = <0x3ff000 0x1000>,
> +                           <0x3fe100 0x100>;
> +               };
> +
> +               timer@f03fe600 {
> +                       compatible = "arm,cortex-a9-twd-timer";
> +                       reg = <0x3fe600 0x20>;
> +                       interrupts = <1 13 0x304>;
> +                       clocks = <&clk NPCM7XX_CLK_TIMER>;
> +               };
> +       };
> +
> +       ahb {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "simple-bus";
> +               interrupt-parent = <&gic>;
> +               ranges = <0x80000000 0x80000000 0x40010000
> +                         0xe0800000 0xe0800000 0x0f800000
> +                         0x00000000 0xf0000000 0x00900000
> +                         0xf8000000 0xf8000000 0x02000000
> +                         0xfb000000 0xfb000000 0x00002000>;

Just do "ranges;" here if all the translation is at the next level down.

Sorry, it's hard to say exactly what to do without knowing the whole memory map.

> +
> +               clk: clock-controller@f0801000 {

Seems like this belongs under /soc based on the address.

> +                       compatible = "nuvoton,npcm750-clk";
> +                       #clock-cells = <1>;
> +                       reg = <0x801000 0x1000>;
> +                       status = "okay";
> +               };
> +
> +               apb {
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       compatible = "simple-bus";
> +                       interrupt-parent = <&gic>;
> +                       ranges = <0x0 0x0 0x00300000>;

And then this would be "<0x0 0xf0000000 0x300000>".

> +
> +                       timer0: timer@f0000000 {
> +                               compatible = "nuvoton,npcm750-timer";
> +                               interrupts = <0 32 4>;
> +                               reg = <0x0 0x1000>;
> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
> +                       };
> +
> +                       watchdog0: watchdog@f0008000 {
> +                               compatible = "nuvoton,npcm750-wdt";
> +                               interrupts = <0 47 4>;
> +                               reg = <0x8000 0x1000>;
> +                               status = "disabled";
> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
> +                       };
> +
> +                       watchdog1: watchdog@f0009000 {
> +                               compatible = "nuvoton,npcm750-wdt";
> +                               interrupts = <0 48 4>;
> +                               reg = <0x9000 0x1000>;
> +                               status = "disabled";
> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
> +                       };
> +
> +                       watchdog2: watchdog@f000a000 {
> +                               compatible = "nuvoton,npcm750-wdt";
> +                               interrupts = <0 49 4>;
> +                               reg = <0xa000 0x1000>;
> +                               status = "disabled";
> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
> +                       };
> +
> +                       serial0: serial@f0001000 {
> +                               compatible = "nuvoton,npcm750-uart";
> +                               reg = <0x1000 0x1000>;
> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
> +                               interrupts = <0 2 4>;
> +                               status = "disabled";
> +                       };
> +
> +                       serial1: serial@f0002000 {
> +                               compatible = "nuvoton,npcm750-uart";
> +                               reg = <0x2000 0x1000>;
> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
> +                               interrupts = <0 3 4>;
> +                               status = "disabled";
> +                       };
> +
> +                       serial2: serial@f0003000 {
> +                               compatible = "nuvoton,npcm750-uart";
> +                               reg = <0x3000 0x1000>;
> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
> +                               interrupts = <0 4 4>;
> +                               status = "disabled";
> +                       };
> +
> +                       serial3: serial@f0004000 {
> +                               compatible = "nuvoton,npcm750-uart";
> +                               reg = <0x4000 0x1000>;
> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
> +                               interrupts = <0 5 4>;
> +                               status = "disabled";
> +                       };
> +               };
> +       };
> +};
> diff --git a/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
> new file mode 100644
> index 000000000000..c69d3bbf7e42
> --- /dev/null
> +++ b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright (C) 2016 Nuvoton Technologies,  tali.perry@nuvoton.com
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_NPCM7XX_H
> +#define _DT_BINDINGS_CLK_NPCM7XX_H
> +
> +#define NPCM7XX_CLK_PLL0       0
> +#define NPCM7XX_CLK_PLL1       1
> +#define NPCM7XX_CLK_PLL2       2
> +#define NPCM7XX_CLK_GFX                3
> +#define NPCM7XX_CLK_APB1       4
> +#define NPCM7XX_CLK_APB2       5
> +#define NPCM7XX_CLK_APB3       6
> +#define NPCM7XX_CLK_APB4       7
> +#define NPCM7XX_CLK_APB5       8
> +#define NPCM7XX_CLK_MC         9
> +#define NPCM7XX_CLK_CPU                10
> +#define NPCM7XX_CLK_SPI0       11
> +#define NPCM7XX_CLK_SPI3       12
> +#define NPCM7XX_CLK_SPIX       13
> +#define NPCM7XX_CLK_UART_CORE  14
> +#define NPCM7XX_CLK_TIMER      15
> +#define NPCM7XX_CLK_HOST_UART  16
> +#define NPCM7XX_CLK_MMC                17
> +#define NPCM7XX_CLK_SDHC       18
> +#define NPCM7XX_CLK_ADC                19
> +#define NPCM7XX_CLK_GFX_MEM    20
> +#define NPCM7XX_CLK_USB_BRIDGE 21
> +#define NPCM7XX_CLK_AXI                22
> +#define NPCM7XX_CLK_AHB                23
> +#define NPCM7XX_CLK_EMC                24
> +#define NPCM7XX_CLK_GMAC       25
> +
> +#endif
> --
> 2.15.0.rc0.271.g36b669edcc-goog
>

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-20 10:48   ` Russell King - ARM Linux
@ 2017-10-20 20:57     ` Brendan Higgins
  2017-10-20 21:08       ` Russell King - ARM Linux
  0 siblings, 1 reply; 13+ messages in thread
From: Brendan Higgins @ 2017-10-20 20:57 UTC (permalink / raw)
  To: Russell King - ARM Linux, Avi Fishman, tmaimon77
  Cc: Rob Herring, Mark Rutland, Rick Altherr, Florian Fainelli,
	devicetree, Linux Kernel Mailing List, linux-arm-kernel,
	OpenBMC Maillist

On Fri, Oct 20, 2017 at 3:48 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
>> Adds basic support for the Nuvoton NPCM750 BMC.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
>> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
>> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
>> Tested-by: Avi Fishman <avifishman70@gmail.com>
>> ---
>>  arch/arm/Kconfig             |  2 +
>>  arch/arm/Makefile            |  1 +
>>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>>  arch/arm/mach-npcm/Makefile  |  3 ++
>>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>>  7 files changed, 193 insertions(+)
>>  create mode 100644 arch/arm/mach-npcm/Kconfig
>>  create mode 100644 arch/arm/mach-npcm/Makefile
>>  create mode 100644 arch/arm/mach-npcm/headsmp.S
>>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>>  create mode 100644 arch/arm/mach-npcm/platsmp.c
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 61a0cb15067e..05543f1cfbde 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
>>
>>  source "arch/arm/mach-nomadik/Kconfig"
>>
>> +source "arch/arm/mach-npcm/Kconfig"
>> +
>>  source "arch/arm/mach-nspire/Kconfig"
>>
>>  source "arch/arm/plat-omap/Kconfig"
>> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> index 47d3a1ab08d2..60ca50c7d762 100644
>> --- a/arch/arm/Makefile
>> +++ b/arch/arm/Makefile
>> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)           += mediatek
>>  machine-$(CONFIG_ARCH_MXS)           += mxs
>>  machine-$(CONFIG_ARCH_NETX)          += netx
>>  machine-$(CONFIG_ARCH_NOMADIK)               += nomadik
>> +machine-$(CONFIG_ARCH_NPCM)          += npcm
>>  machine-$(CONFIG_ARCH_NSPIRE)                += nspire
>>  machine-$(CONFIG_ARCH_OXNAS)         += oxnas
>>  machine-$(CONFIG_ARCH_OMAP1)         += omap1
>> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
>> new file mode 100644
>> index 000000000000..21dfa8ce828f
>> --- /dev/null
>> +++ b/arch/arm/mach-npcm/Kconfig
>> @@ -0,0 +1,48 @@
>> +menuconfig ARCH_NPCM
>> +     bool "Nuvoton NPCM Architecture"
>> +     select ARCH_REQUIRE_GPIOLIB
>> +     select USE_OF
>> +     select PINCTRL
>> +     select PINCTRL_NPCM7XX
>> +
>> +if ARCH_NPCM
>> +
>> +comment "NPCMX50 CPU type"
>> +
>> +config CPU_NPCM750
>> +     depends on ARCH_NPCM && ARCH_MULTI_V7
>> +     bool "Support for NPCM750 BMC CPU (Poleg)"
>> +     select CACHE_L2X0
>> +     select CPU_V7
>> +     select ARM_GIC
>> +     select HAVE_SMP
>> +     select SMP
>> +     select SMP_ON_UP
>> +     select HAVE_ARM_SCU
>> +     select HAVE_ARM_TWD if SMP
>> +     select ARM_ERRATA_720789
>> +     select ARM_ERRATA_754322
>> +     select ARM_ERRATA_764369
>> +     select ARM_ERRATA_794072
>> +     select PL310_ERRATA_588369
>> +     select PL310_ERRATA_727915
>> +     select USB_EHCI_ROOT_HUB_TT
>> +     select USB_ARCH_HAS_HCD
>> +     select USB_ARCH_HAS_EHCI
>> +     select USB_EHCI_HCD
>> +     select USB_ARCH_HAS_OHCI
>> +     select USB_OHCI_HCD
>> +     select USB
>> +     select FIQ
>> +     select CPU_USE_DOMAINS
>> +     select GENERIC_CLOCKEVENTS
>> +     select CLKDEV_LOOKUP
>> +     select COMMON_CLK if OF
>> +     select NPCM750_TIMER
>> +     select MFD_SYSCON
>> +     help
>> +       Support for NPCM750 BMC CPU (Poleg).
>> +
>> +       Nuvoton NPCM750 BMC based on the Cortex A9.
>> +
>> +endif
>> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
>> new file mode 100644
>> index 000000000000..78416055b854
>> --- /dev/null
>> +++ b/arch/arm/mach-npcm/Makefile
>> @@ -0,0 +1,3 @@
>> +AFLAGS_headsmp.o             += -march=armv7-a
>> +
>> +obj-$(CONFIG_CPU_NPCM750)    += npcm7xx.o platsmp.o headsmp.o
>> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
>> new file mode 100644
>> index 000000000000..9fccbbd49ed4
>> --- /dev/null
>> +++ b/arch/arm/mach-npcm/headsmp.S
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright 2017 Google, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <linux/init.h>
>> +#include <asm/assembler.h>
>> +
>> +ENTRY(npcm7xx_secondary_startup)
>> +     safe_svcmode_maskall r0
>> +
>> +     b       secondary_startup
>> +ENDPROC(npcm7xx_secondary_startup)
>
> Why do you need this?  The switch to svc mode is already handled by
> secondary_startup.

Florian Fainelli already brought this up:
https://www.spinics.net/lists/arm-kernel/msg605126.html
Without this the kernel complains that the core is started in
"wrong/inconsistent mode":

SMP: Total of 2 processors activated (398.13 BogoMIPS).
CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU
mode 0x13)
CPU: This may indicate a broken bootloader or firmware.

>
>> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
>> new file mode 100644
>> index 000000000000..132e9d587857
>> --- /dev/null
>> +++ b/arch/arm/mach-npcm/npcm7xx.c
>> @@ -0,0 +1,34 @@
>> +/*
>> + * Copyright (c) 2017 Nuvoton Technology corporation.
>> + * Copyright 2017 Google, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/types.h>
>> +#include <asm/mach/arch.h>
>> +#include <asm/mach-types.h>
>> +#include <asm/mach/map.h>
>> +#include <asm/hardware/cache-l2x0.h>
>> +
>> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |                             \
>> +                      L310_AUX_CTRL_DATA_PREFETCH |                         \
>> +                      L310_AUX_CTRL_NS_LOCKDOWN |                           \
>> +                      L310_AUX_CTRL_CACHE_REPLACE_RR |                      \
>> +                      L2C_AUX_CTRL_SHARED_OVERRIDE |                        \
>> +                      L310_AUX_CTRL_FULL_LINE_ZERO)
>
> Do you really need all these flags?

Tomer, Avi, do we need these? I got this from the original version of
the driver.

>
>> +
>> +static const char *const npcm7xx_dt_match[] = {
>> +     "nuvoton,npcm750",
>> +     NULL
>> +};
>> +
>> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
>> +     .atag_offset    = 0x100,
>> +     .dt_compat      = npcm7xx_dt_match,
>> +     .l2c_aux_val    = NPCM7XX_AUX_VAL,
>> +     .l2c_aux_mask   = ~NPCM7XX_AUX_VAL,
>> +MACHINE_END
>> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
>> new file mode 100644
>> index 000000000000..450e83c3c531
>> --- /dev/null
>> +++ b/arch/arm/mach-npcm/platsmp.c
>> @@ -0,0 +1,88 @@
>> +/*
>> + * Copyright 2017 Google, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#define pr_fmt(fmt) "PLATSMP: " fmt
>
> Maybe something more descriptive?
>
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/smp.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <asm/cacheflush.h>
>> +#include <asm/smp.h>
>> +#include <asm/smp_plat.h>
>> +#include <asm/smp_scu.h>
>> +
>> +#define NPCM7XX_SCRPAD_REG 0x13c
>> +
>> +extern void npcm7xx_secondary_startup(void);
>> +
>> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
>> +                                   struct task_struct *idle)
>> +{
>> +     struct device_node *gcr_np;
>> +     void __iomem *gcr_base;
>> +     int ret = 0;
>> +
>> +     gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
>> +     if (!gcr_np) {
>> +             pr_err("no gcr device node\n");
>> +             ret = -EFAULT;
>
> EFAULT is used for bad userspace addresses, it should not be used for
> other stuff - and a missing node is certainly not a "Bad address".
>
>> +             goto out;
>> +     }
>> +     gcr_base = of_iomap(gcr_np, 0);
>> +     if (!gcr_base) {
>> +             pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
>> +             ret = -EFAULT;
>
> This is more -ENOMEM.
>
>> +             goto out;
>> +     }
>> +
>> +     /* give boot ROM kernel start address. */
>> +     iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
>> +               NPCM7XX_SCRPAD_REG);
>> +     /* make sure npcm7xx_secondary_startup is seen by all observers. */
>> +     smp_wmb();
>
> I think you mean "make sure the previous write is seen by all observers".
> However, doesn't "dsb_sev()" already do that?
>
>> +     dsb_sev();
>> +     /* make sure write buffer is drained */
>> +     mb();
>
> This looks over the top.
>
>> +
>> +     iounmap(gcr_base);
>> +out:
>> +     return ret;
>> +}
>> +
>> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
>> +{
>> +     struct device_node *scu_np;
>> +     void __iomem *scu_base;
>> +
>> +     scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>> +     if (!scu_np) {
>> +             pr_err("no scu device node\n");
>> +             return;
>> +     }
>> +     scu_base = of_iomap(scu_np, 0);
>> +     if (!scu_base) {
>> +             pr_err("could not iomap scu at: 0x%p\n", scu_base);
>> +             return;
>> +     }
>> +
>> +     scu_enable(scu_base);
>> +
>> +     iounmap(scu_base);
>> +}
>> +
>> +static struct smp_operations npcm7xx_smp_ops __initdata = {
>> +     .smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
>> +     .smp_boot_secondary = npcm7xx_smp_boot_secondary,
>> +};
>> +
>> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
>> --
>> 2.15.0.rc0.271.g36b669edcc-goog
>>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-20 20:57     ` Brendan Higgins
@ 2017-10-20 21:08       ` Russell King - ARM Linux
  2017-10-26 11:45         ` Tomer Maimon
  0 siblings, 1 reply; 13+ messages in thread
From: Russell King - ARM Linux @ 2017-10-20 21:08 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Avi Fishman, tmaimon77, Rob Herring, Mark Rutland, Rick Altherr,
	Florian Fainelli, devicetree, Linux Kernel Mailing List,
	linux-arm-kernel, OpenBMC Maillist

On Fri, Oct 20, 2017 at 01:57:47PM -0700, Brendan Higgins wrote:
> On Fri, Oct 20, 2017 at 3:48 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
> >> Adds basic support for the Nuvoton NPCM750 BMC.
> >>
> >> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
> >> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
> >> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
> >> Tested-by: Avi Fishman <avifishman70@gmail.com>
> >> ---
> >>  arch/arm/Kconfig             |  2 +
> >>  arch/arm/Makefile            |  1 +
> >>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
> >>  arch/arm/mach-npcm/Makefile  |  3 ++
> >>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
> >>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
> >>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
> >>  7 files changed, 193 insertions(+)
> >>  create mode 100644 arch/arm/mach-npcm/Kconfig
> >>  create mode 100644 arch/arm/mach-npcm/Makefile
> >>  create mode 100644 arch/arm/mach-npcm/headsmp.S
> >>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
> >>  create mode 100644 arch/arm/mach-npcm/platsmp.c
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 61a0cb15067e..05543f1cfbde 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
> >>
> >>  source "arch/arm/mach-nomadik/Kconfig"
> >>
> >> +source "arch/arm/mach-npcm/Kconfig"
> >> +
> >>  source "arch/arm/mach-nspire/Kconfig"
> >>
> >>  source "arch/arm/plat-omap/Kconfig"
> >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> >> index 47d3a1ab08d2..60ca50c7d762 100644
> >> --- a/arch/arm/Makefile
> >> +++ b/arch/arm/Makefile
> >> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)           += mediatek
> >>  machine-$(CONFIG_ARCH_MXS)           += mxs
> >>  machine-$(CONFIG_ARCH_NETX)          += netx
> >>  machine-$(CONFIG_ARCH_NOMADIK)               += nomadik
> >> +machine-$(CONFIG_ARCH_NPCM)          += npcm
> >>  machine-$(CONFIG_ARCH_NSPIRE)                += nspire
> >>  machine-$(CONFIG_ARCH_OXNAS)         += oxnas
> >>  machine-$(CONFIG_ARCH_OMAP1)         += omap1
> >> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
> >> new file mode 100644
> >> index 000000000000..21dfa8ce828f
> >> --- /dev/null
> >> +++ b/arch/arm/mach-npcm/Kconfig
> >> @@ -0,0 +1,48 @@
> >> +menuconfig ARCH_NPCM
> >> +     bool "Nuvoton NPCM Architecture"
> >> +     select ARCH_REQUIRE_GPIOLIB
> >> +     select USE_OF
> >> +     select PINCTRL
> >> +     select PINCTRL_NPCM7XX
> >> +
> >> +if ARCH_NPCM
> >> +
> >> +comment "NPCMX50 CPU type"
> >> +
> >> +config CPU_NPCM750
> >> +     depends on ARCH_NPCM && ARCH_MULTI_V7
> >> +     bool "Support for NPCM750 BMC CPU (Poleg)"
> >> +     select CACHE_L2X0
> >> +     select CPU_V7
> >> +     select ARM_GIC
> >> +     select HAVE_SMP
> >> +     select SMP
> >> +     select SMP_ON_UP
> >> +     select HAVE_ARM_SCU
> >> +     select HAVE_ARM_TWD if SMP
> >> +     select ARM_ERRATA_720789
> >> +     select ARM_ERRATA_754322
> >> +     select ARM_ERRATA_764369
> >> +     select ARM_ERRATA_794072
> >> +     select PL310_ERRATA_588369
> >> +     select PL310_ERRATA_727915
> >> +     select USB_EHCI_ROOT_HUB_TT
> >> +     select USB_ARCH_HAS_HCD
> >> +     select USB_ARCH_HAS_EHCI
> >> +     select USB_EHCI_HCD
> >> +     select USB_ARCH_HAS_OHCI
> >> +     select USB_OHCI_HCD
> >> +     select USB
> >> +     select FIQ
> >> +     select CPU_USE_DOMAINS
> >> +     select GENERIC_CLOCKEVENTS
> >> +     select CLKDEV_LOOKUP
> >> +     select COMMON_CLK if OF
> >> +     select NPCM750_TIMER
> >> +     select MFD_SYSCON
> >> +     help
> >> +       Support for NPCM750 BMC CPU (Poleg).
> >> +
> >> +       Nuvoton NPCM750 BMC based on the Cortex A9.
> >> +
> >> +endif
> >> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
> >> new file mode 100644
> >> index 000000000000..78416055b854
> >> --- /dev/null
> >> +++ b/arch/arm/mach-npcm/Makefile
> >> @@ -0,0 +1,3 @@
> >> +AFLAGS_headsmp.o             += -march=armv7-a
> >> +
> >> +obj-$(CONFIG_CPU_NPCM750)    += npcm7xx.o platsmp.o headsmp.o
> >> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
> >> new file mode 100644
> >> index 000000000000..9fccbbd49ed4
> >> --- /dev/null
> >> +++ b/arch/arm/mach-npcm/headsmp.S
> >> @@ -0,0 +1,17 @@
> >> +/*
> >> + * Copyright 2017 Google, Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#include <linux/linkage.h>
> >> +#include <linux/init.h>
> >> +#include <asm/assembler.h>
> >> +
> >> +ENTRY(npcm7xx_secondary_startup)
> >> +     safe_svcmode_maskall r0
> >> +
> >> +     b       secondary_startup
> >> +ENDPROC(npcm7xx_secondary_startup)
> >
> > Why do you need this?  The switch to svc mode is already handled by
> > secondary_startup.
> 
> Florian Fainelli already brought this up:
> https://www.spinics.net/lists/arm-kernel/msg605126.html
> Without this the kernel complains that the core is started in
> "wrong/inconsistent mode":

The solution would be a comment to indicate why it's necessary :)

> 
> SMP: Total of 2 processors activated (398.13 BogoMIPS).
> CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU
> mode 0x13)
> CPU: This may indicate a broken bootloader or firmware.
> 
> >
> >> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
> >> new file mode 100644
> >> index 000000000000..132e9d587857
> >> --- /dev/null
> >> +++ b/arch/arm/mach-npcm/npcm7xx.c
> >> @@ -0,0 +1,34 @@
> >> +/*
> >> + * Copyright (c) 2017 Nuvoton Technology corporation.
> >> + * Copyright 2017 Google, Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#include <linux/kernel.h>
> >> +#include <linux/types.h>
> >> +#include <asm/mach/arch.h>
> >> +#include <asm/mach-types.h>
> >> +#include <asm/mach/map.h>
> >> +#include <asm/hardware/cache-l2x0.h>
> >> +
> >> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |                             \
> >> +                      L310_AUX_CTRL_DATA_PREFETCH |                         \
> >> +                      L310_AUX_CTRL_NS_LOCKDOWN |                           \
> >> +                      L310_AUX_CTRL_CACHE_REPLACE_RR |                      \
> >> +                      L2C_AUX_CTRL_SHARED_OVERRIDE |                        \
> >> +                      L310_AUX_CTRL_FULL_LINE_ZERO)
> >
> > Do you really need all these flags?
> 
> Tomer, Avi, do we need these? I got this from the original version of
> the driver.

The l2c driver manages L310_AUX_CTRL_FULL_LINE_ZERO itself, so you
shouldn't specify that.

L2C_AUX_CTRL_SHARED_OVERRIDE is managed via the "arm,shared-override"
DT property, so you should use that in DT instead.

Prefetching needs the prefetch control register configured to tune the
prefetching - that's platform specific so the l2c driver doesn't touch
it (it expects firmware to have configured it.)  If firmware needs to
configure that, it might as well configure the flags too as they're
shared into that very same register.  If not, I think the l2c prefetch
register defaults to a very small prefetch, and is probably not worth
setting the enable bits without configuring the depth of prefetch.

L310_AUX_CTRL_NS_LOCKDOWN is always enabled by the l2c driver, so
shouldn't be necessary.

So that leaves only L310_AUX_CTRL_CACHE_REPLACE_RR.

> 
> >
> >> +
> >> +static const char *const npcm7xx_dt_match[] = {
> >> +     "nuvoton,npcm750",
> >> +     NULL
> >> +};
> >> +
> >> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
> >> +     .atag_offset    = 0x100,
> >> +     .dt_compat      = npcm7xx_dt_match,
> >> +     .l2c_aux_val    = NPCM7XX_AUX_VAL,
> >> +     .l2c_aux_mask   = ~NPCM7XX_AUX_VAL,
> >> +MACHINE_END
> >> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
> >> new file mode 100644
> >> index 000000000000..450e83c3c531
> >> --- /dev/null
> >> +++ b/arch/arm/mach-npcm/platsmp.c
> >> @@ -0,0 +1,88 @@
> >> +/*
> >> + * Copyright 2017 Google, Inc.
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#define pr_fmt(fmt) "PLATSMP: " fmt
> >
> > Maybe something more descriptive?
> >
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/device.h>
> >> +#include <linux/smp.h>
> >> +#include <linux/io.h>
> >> +#include <linux/of.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/of_platform.h>
> >> +#include <linux/of_address.h>
> >> +#include <asm/cacheflush.h>
> >> +#include <asm/smp.h>
> >> +#include <asm/smp_plat.h>
> >> +#include <asm/smp_scu.h>
> >> +
> >> +#define NPCM7XX_SCRPAD_REG 0x13c
> >> +
> >> +extern void npcm7xx_secondary_startup(void);
> >> +
> >> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
> >> +                                   struct task_struct *idle)
> >> +{
> >> +     struct device_node *gcr_np;
> >> +     void __iomem *gcr_base;
> >> +     int ret = 0;
> >> +
> >> +     gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
> >> +     if (!gcr_np) {
> >> +             pr_err("no gcr device node\n");
> >> +             ret = -EFAULT;
> >
> > EFAULT is used for bad userspace addresses, it should not be used for
> > other stuff - and a missing node is certainly not a "Bad address".
> >
> >> +             goto out;
> >> +     }
> >> +     gcr_base = of_iomap(gcr_np, 0);
> >> +     if (!gcr_base) {
> >> +             pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
> >> +             ret = -EFAULT;
> >
> > This is more -ENOMEM.
> >
> >> +             goto out;
> >> +     }
> >> +
> >> +     /* give boot ROM kernel start address. */
> >> +     iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
> >> +               NPCM7XX_SCRPAD_REG);
> >> +     /* make sure npcm7xx_secondary_startup is seen by all observers. */
> >> +     smp_wmb();
> >
> > I think you mean "make sure the previous write is seen by all observers".
> > However, doesn't "dsb_sev()" already do that?
> >
> >> +     dsb_sev();
> >> +     /* make sure write buffer is drained */
> >> +     mb();
> >
> > This looks over the top.
> >
> >> +
> >> +     iounmap(gcr_base);
> >> +out:
> >> +     return ret;
> >> +}
> >> +
> >> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
> >> +{
> >> +     struct device_node *scu_np;
> >> +     void __iomem *scu_base;
> >> +
> >> +     scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
> >> +     if (!scu_np) {
> >> +             pr_err("no scu device node\n");
> >> +             return;
> >> +     }
> >> +     scu_base = of_iomap(scu_np, 0);
> >> +     if (!scu_base) {
> >> +             pr_err("could not iomap scu at: 0x%p\n", scu_base);
> >> +             return;
> >> +     }
> >> +
> >> +     scu_enable(scu_base);
> >> +
> >> +     iounmap(scu_base);
> >> +}
> >> +
> >> +static struct smp_operations npcm7xx_smp_ops __initdata = {
> >> +     .smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
> >> +     .smp_boot_secondary = npcm7xx_smp_boot_secondary,
> >> +};
> >> +
> >> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
> >> --
> >> 2.15.0.rc0.271.g36b669edcc-goog
> >>
> >
> > --
> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> > According to speedtest.net: 8.21Mbps down 510kbps up

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-20 21:08       ` Russell King - ARM Linux
@ 2017-10-26 11:45         ` Tomer Maimon
  2017-11-04  0:49           ` Brendan Higgins
  0 siblings, 1 reply; 13+ messages in thread
From: Tomer Maimon @ 2017-10-26 11:45 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Brendan Higgins, Avi Fishman, Rob Herring, Mark Rutland,
	Rick Altherr, Florian Fainelli, devicetree,
	Linux Kernel Mailing List, linux-arm-kernel, OpenBMC Maillist

Hi Brendan,

Sorry for the delay,

On 21 October 2017 at 00:08, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Fri, Oct 20, 2017 at 01:57:47PM -0700, Brendan Higgins wrote:
>> On Fri, Oct 20, 2017 at 3:48 AM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
>> >> Adds basic support for the Nuvoton NPCM750 BMC.
>> >>
>> >> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> >> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
>> >> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
>> >> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
>> >> Tested-by: Avi Fishman <avifishman70@gmail.com>
>> >> ---
>> >>  arch/arm/Kconfig             |  2 +
>> >>  arch/arm/Makefile            |  1 +
>> >>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>> >>  arch/arm/mach-npcm/Makefile  |  3 ++
>> >>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>> >>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>> >>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>> >>  7 files changed, 193 insertions(+)
>> >>  create mode 100644 arch/arm/mach-npcm/Kconfig
>> >>  create mode 100644 arch/arm/mach-npcm/Makefile
>> >>  create mode 100644 arch/arm/mach-npcm/headsmp.S
>> >>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>> >>  create mode 100644 arch/arm/mach-npcm/platsmp.c
>> >>
>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> >> index 61a0cb15067e..05543f1cfbde 100644
>> >> --- a/arch/arm/Kconfig
>> >> +++ b/arch/arm/Kconfig
>> >> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
>> >>
>> >>  source "arch/arm/mach-nomadik/Kconfig"
>> >>
>> >> +source "arch/arm/mach-npcm/Kconfig"
>> >> +
>> >>  source "arch/arm/mach-nspire/Kconfig"
>> >>
>> >>  source "arch/arm/plat-omap/Kconfig"
>> >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>> >> index 47d3a1ab08d2..60ca50c7d762 100644
>> >> --- a/arch/arm/Makefile
>> >> +++ b/arch/arm/Makefile
>> >> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)           += mediatek
>> >>  machine-$(CONFIG_ARCH_MXS)           += mxs
>> >>  machine-$(CONFIG_ARCH_NETX)          += netx
>> >>  machine-$(CONFIG_ARCH_NOMADIK)               += nomadik
>> >> +machine-$(CONFIG_ARCH_NPCM)          += npcm
>> >>  machine-$(CONFIG_ARCH_NSPIRE)                += nspire
>> >>  machine-$(CONFIG_ARCH_OXNAS)         += oxnas
>> >>  machine-$(CONFIG_ARCH_OMAP1)         += omap1
>> >> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
>> >> new file mode 100644
>> >> index 000000000000..21dfa8ce828f
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-npcm/Kconfig
>> >> @@ -0,0 +1,48 @@
>> >> +menuconfig ARCH_NPCM
>> >> +     bool "Nuvoton NPCM Architecture"
>> >> +     select ARCH_REQUIRE_GPIOLIB
>> >> +     select USE_OF
>> >> +     select PINCTRL
>> >> +     select PINCTRL_NPCM7XX
>> >> +
>> >> +if ARCH_NPCM
>> >> +
>> >> +comment "NPCMX50 CPU type"
>> >> +
>> >> +config CPU_NPCM750

I started the upstream process of NPCM7xx clocksource driver, I got
the following comment
https://www.spinics.net/lists/devicetree/msg197916.html

Is it possible to modify the the architecture configuration name to
ARCH_NPCM7XX.

>> >> +     depends on ARCH_NPCM && ARCH_MULTI_V7
>> >> +     bool "Support for NPCM750 BMC CPU (Poleg)"
>> >> +     select CACHE_L2X0
>> >> +     select CPU_V7
>> >> +     select ARM_GIC
>> >> +     select HAVE_SMP
>> >> +     select SMP
>> >> +     select SMP_ON_UP
>> >> +     select HAVE_ARM_SCU
>> >> +     select HAVE_ARM_TWD if SMP
>> >> +     select ARM_ERRATA_720789
>> >> +     select ARM_ERRATA_754322
>> >> +     select ARM_ERRATA_764369
>> >> +     select ARM_ERRATA_794072
>> >> +     select PL310_ERRATA_588369
>> >> +     select PL310_ERRATA_727915
>> >> +     select USB_EHCI_ROOT_HUB_TT
>> >> +     select USB_ARCH_HAS_HCD
>> >> +     select USB_ARCH_HAS_EHCI
>> >> +     select USB_EHCI_HCD
>> >> +     select USB_ARCH_HAS_OHCI
>> >> +     select USB_OHCI_HCD
>> >> +     select USB
>> >> +     select FIQ
>> >> +     select CPU_USE_DOMAINS
>> >> +     select GENERIC_CLOCKEVENTS
>> >> +     select CLKDEV_LOOKUP
>> >> +     select COMMON_CLK if OF
>> >> +     select NPCM750_TIMER
>> >> +     select MFD_SYSCON
>> >> +     help
>> >> +       Support for NPCM750 BMC CPU (Poleg).
>> >> +
>> >> +       Nuvoton NPCM750 BMC based on the Cortex A9.
>> >> +
>> >> +endif
>> >> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
>> >> new file mode 100644
>> >> index 000000000000..78416055b854
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-npcm/Makefile
>> >> @@ -0,0 +1,3 @@
>> >> +AFLAGS_headsmp.o             += -march=armv7-a
>> >> +
>> >> +obj-$(CONFIG_CPU_NPCM750)    += npcm7xx.o platsmp.o headsmp.o
>> >> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
>> >> new file mode 100644
>> >> index 000000000000..9fccbbd49ed4
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-npcm/headsmp.S
>> >> @@ -0,0 +1,17 @@
>> >> +/*
>> >> + * Copyright 2017 Google, Inc.
>> >> + *
>> >> + * This program is free software; you can redistribute it and/or modify
>> >> + * it under the terms of the GNU General Public License version 2 as
>> >> + * published by the Free Software Foundation.
>> >> + */
>> >> +
>> >> +#include <linux/linkage.h>
>> >> +#include <linux/init.h>
>> >> +#include <asm/assembler.h>
>> >> +
>> >> +ENTRY(npcm7xx_secondary_startup)
>> >> +     safe_svcmode_maskall r0
>> >> +
>> >> +     b       secondary_startup
>> >> +ENDPROC(npcm7xx_secondary_startup)
>> >
>> > Why do you need this?  The switch to svc mode is already handled by
>> > secondary_startup.
>>
>> Florian Fainelli already brought this up:
>> https://www.spinics.net/lists/arm-kernel/msg605126.html
>> Without this the kernel complains that the core is started in
>> "wrong/inconsistent mode":
>
> The solution would be a comment to indicate why it's necessary :)
>
>>
>> SMP: Total of 2 processors activated (398.13 BogoMIPS).
>> CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU
>> mode 0x13)
>> CPU: This may indicate a broken bootloader or firmware.
>>
>> >
>> >> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
>> >> new file mode 100644
>> >> index 000000000000..132e9d587857
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-npcm/npcm7xx.c
>> >> @@ -0,0 +1,34 @@
>> >> +/*
>> >> + * Copyright (c) 2017 Nuvoton Technology corporation.
>> >> + * Copyright 2017 Google, Inc.
>> >> + *
>> >> + * This program is free software; you can redistribute it and/or modify
>> >> + * it under the terms of the GNU General Public License version 2 as
>> >> + * published by the Free Software Foundation.
>> >> + */
>> >> +
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/types.h>
>> >> +#include <asm/mach/arch.h>
>> >> +#include <asm/mach-types.h>
>> >> +#include <asm/mach/map.h>
>> >> +#include <asm/hardware/cache-l2x0.h>
>> >> +
>> >> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |                             \
>> >> +                      L310_AUX_CTRL_DATA_PREFETCH |                         \
>> >> +                      L310_AUX_CTRL_NS_LOCKDOWN |                           \
>> >> +                      L310_AUX_CTRL_CACHE_REPLACE_RR |                      \
>> >> +                      L2C_AUX_CTRL_SHARED_OVERRIDE |                        \
>> >> +                      L310_AUX_CTRL_FULL_LINE_ZERO)
>> >
>> > Do you really need all these flags?
>>
>> Tomer, Avi, do we need these? I got this from the original version of
>> the driver.
>
> The l2c driver manages L310_AUX_CTRL_FULL_LINE_ZERO itself, so you
> shouldn't specify that.
>
> L2C_AUX_CTRL_SHARED_OVERRIDE is managed via the "arm,shared-override"
> DT property, so you should use that in DT instead.
>
> Prefetching needs the prefetch control register configured to tune the
> prefetching - that's platform specific so the l2c driver doesn't touch
> it (it expects firmware to have configured it.)  If firmware needs to
> configure that, it might as well configure the flags too as they're
> shared into that very same register.  If not, I think the l2c prefetch
> register defaults to a very small prefetch, and is probably not worth
> setting the enable bits without configuring the depth of prefetch.

Yes, we do it in the firmware.

>
> L310_AUX_CTRL_NS_LOCKDOWN is always enabled by the l2c driver, so
> shouldn't be necessary.
>
> So that leaves only L310_AUX_CTRL_CACHE_REPLACE_RR.
>

You dont need to add L310_AUX_CTRL_CACHE_REPLACE_RR to the l2c aux value.

>>
>> >
>> >> +
>> >> +static const char *const npcm7xx_dt_match[] = {
>> >> +     "nuvoton,npcm750",
>> >> +     NULL
>> >> +};
>> >> +
>> >> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
>> >> +     .atag_offset    = 0x100,
>> >> +     .dt_compat      = npcm7xx_dt_match,
>> >> +     .l2c_aux_val    = NPCM7XX_AUX_VAL,
>> >> +     .l2c_aux_mask   = ~NPCM7XX_AUX_VAL,
>> >> +MACHINE_END
>> >> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
>> >> new file mode 100644
>> >> index 000000000000..450e83c3c531
>> >> --- /dev/null
>> >> +++ b/arch/arm/mach-npcm/platsmp.c
>> >> @@ -0,0 +1,88 @@
>> >> +/*
>> >> + * Copyright 2017 Google, Inc.
>> >> + *
>> >> + * This program is free software; you can redistribute it and/or modify
>> >> + * it under the terms of the GNU General Public License version 2 as
>> >> + * published by the Free Software Foundation.
>> >> + */
>> >> +
>> >> +#define pr_fmt(fmt) "PLATSMP: " fmt
>> >
>> > Maybe something more descriptive?
>> >
>> >> +
>> >> +#include <linux/delay.h>
>> >> +#include <linux/device.h>
>> >> +#include <linux/smp.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/of.h>
>> >> +#include <linux/of_device.h>
>> >> +#include <linux/of_platform.h>
>> >> +#include <linux/of_address.h>
>> >> +#include <asm/cacheflush.h>
>> >> +#include <asm/smp.h>
>> >> +#include <asm/smp_plat.h>
>> >> +#include <asm/smp_scu.h>
>> >> +
>> >> +#define NPCM7XX_SCRPAD_REG 0x13c
>> >> +
>> >> +extern void npcm7xx_secondary_startup(void);
>> >> +
>> >> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
>> >> +                                   struct task_struct *idle)
>> >> +{
>> >> +     struct device_node *gcr_np;
>> >> +     void __iomem *gcr_base;
>> >> +     int ret = 0;
>> >> +
>> >> +     gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
>> >> +     if (!gcr_np) {
>> >> +             pr_err("no gcr device node\n");
>> >> +             ret = -EFAULT;
>> >
>> > EFAULT is used for bad userspace addresses, it should not be used for
>> > other stuff - and a missing node is certainly not a "Bad address".
>> >
>> >> +             goto out;
>> >> +     }
>> >> +     gcr_base = of_iomap(gcr_np, 0);
>> >> +     if (!gcr_base) {
>> >> +             pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
>> >> +             ret = -EFAULT;
>> >
>> > This is more -ENOMEM.
>> >
>> >> +             goto out;
>> >> +     }
>> >> +
>> >> +     /* give boot ROM kernel start address. */
>> >> +     iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
>> >> +               NPCM7XX_SCRPAD_REG);
>> >> +     /* make sure npcm7xx_secondary_startup is seen by all observers. */
>> >> +     smp_wmb();
>> >
>> > I think you mean "make sure the previous write is seen by all observers".
>> > However, doesn't "dsb_sev()" already do that?
>> >
>> >> +     dsb_sev();
>> >> +     /* make sure write buffer is drained */
>> >> +     mb();
>> >
>> > This looks over the top.
>> >
>> >> +
>> >> +     iounmap(gcr_base);
>> >> +out:
>> >> +     return ret;
>> >> +}
>> >> +
>> >> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
>> >> +{
>> >> +     struct device_node *scu_np;
>> >> +     void __iomem *scu_base;
>> >> +
>> >> +     scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>> >> +     if (!scu_np) {
>> >> +             pr_err("no scu device node\n");
>> >> +             return;
>> >> +     }
>> >> +     scu_base = of_iomap(scu_np, 0);
>> >> +     if (!scu_base) {
>> >> +             pr_err("could not iomap scu at: 0x%p\n", scu_base);
>> >> +             return;
>> >> +     }
>> >> +
>> >> +     scu_enable(scu_base);
>> >> +
>> >> +     iounmap(scu_base);
>> >> +}
>> >> +
>> >> +static struct smp_operations npcm7xx_smp_ops __initdata = {
>> >> +     .smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
>> >> +     .smp_boot_secondary = npcm7xx_smp_boot_secondary,
>> >> +};
>> >> +
>> >> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
>> >> --
>> >> 2.15.0.rc0.271.g36b669edcc-goog
>> >>
>> >
>> > --
>> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
>> > According to speedtest.net: 8.21Mbps down 510kbps up
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
> According to speedtest.net: 8.21Mbps down 510kbps up

Thanks,

Tomer

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-10-26 11:45         ` Tomer Maimon
@ 2017-11-04  0:49           ` Brendan Higgins
  2017-11-06 10:35             ` Tomer Maimon
  0 siblings, 1 reply; 13+ messages in thread
From: Brendan Higgins @ 2017-11-04  0:49 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: Russell King - ARM Linux, Avi Fishman, Rob Herring, Mark Rutland,
	Rick Altherr, Florian Fainelli, devicetree,
	Linux Kernel Mailing List, linux-arm-kernel, OpenBMC Maillist

On Thu, Oct 26, 2017 at 4:45 AM, Tomer Maimon <tmaimon77@gmail.com> wrote:
> Hi Brendan,
>
> Sorry for the delay,
>
> On 21 October 2017 at 00:08, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Fri, Oct 20, 2017 at 01:57:47PM -0700, Brendan Higgins wrote:
>>> On Fri, Oct 20, 2017 at 3:48 AM, Russell King - ARM Linux
>>> <linux@armlinux.org.uk> wrote:
>>> > On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
>>> >> Adds basic support for the Nuvoton NPCM750 BMC.
>>> >>
>>> >> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>> >> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
>>> >> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
>>> >> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
>>> >> Tested-by: Avi Fishman <avifishman70@gmail.com>
>>> >> ---
>>> >>  arch/arm/Kconfig             |  2 +
>>> >>  arch/arm/Makefile            |  1 +
>>> >>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>>> >>  arch/arm/mach-npcm/Makefile  |  3 ++
>>> >>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>>> >>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>>> >>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>>> >>  7 files changed, 193 insertions(+)
>>> >>  create mode 100644 arch/arm/mach-npcm/Kconfig
>>> >>  create mode 100644 arch/arm/mach-npcm/Makefile
>>> >>  create mode 100644 arch/arm/mach-npcm/headsmp.S
>>> >>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>>> >>  create mode 100644 arch/arm/mach-npcm/platsmp.c
>>> >>
>>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> >> index 61a0cb15067e..05543f1cfbde 100644
>>> >> --- a/arch/arm/Kconfig
>>> >> +++ b/arch/arm/Kconfig
>>> >> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
>>> >>
>>> >>  source "arch/arm/mach-nomadik/Kconfig"
>>> >>
>>> >> +source "arch/arm/mach-npcm/Kconfig"
>>> >> +
>>> >>  source "arch/arm/mach-nspire/Kconfig"
>>> >>
>>> >>  source "arch/arm/plat-omap/Kconfig"
>>> >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>>> >> index 47d3a1ab08d2..60ca50c7d762 100644
>>> >> --- a/arch/arm/Makefile
>>> >> +++ b/arch/arm/Makefile
>>> >> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)           += mediatek
>>> >>  machine-$(CONFIG_ARCH_MXS)           += mxs
>>> >>  machine-$(CONFIG_ARCH_NETX)          += netx
>>> >>  machine-$(CONFIG_ARCH_NOMADIK)               += nomadik
>>> >> +machine-$(CONFIG_ARCH_NPCM)          += npcm
>>> >>  machine-$(CONFIG_ARCH_NSPIRE)                += nspire
>>> >>  machine-$(CONFIG_ARCH_OXNAS)         += oxnas
>>> >>  machine-$(CONFIG_ARCH_OMAP1)         += omap1
>>> >> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
>>> >> new file mode 100644
>>> >> index 000000000000..21dfa8ce828f
>>> >> --- /dev/null
>>> >> +++ b/arch/arm/mach-npcm/Kconfig
>>> >> @@ -0,0 +1,48 @@
>>> >> +menuconfig ARCH_NPCM
>>> >> +     bool "Nuvoton NPCM Architecture"
>>> >> +     select ARCH_REQUIRE_GPIOLIB
>>> >> +     select USE_OF
>>> >> +     select PINCTRL
>>> >> +     select PINCTRL_NPCM7XX
>>> >> +
>>> >> +if ARCH_NPCM
>>> >> +
>>> >> +comment "NPCMX50 CPU type"
>>> >> +
>>> >> +config CPU_NPCM750
>
> I started the upstream process of NPCM7xx clocksource driver, I got
> the following comment
> https://www.spinics.net/lists/devicetree/msg197916.html
>
> Is it possible to modify the the architecture configuration name to
> ARCH_NPCM7XX.

You mean the CPU config: CPU_NPCM7XX?

I think the ARCH_NPCM is right, since that corresponds to this directory
which is for all ARM Nuvoton BMCs.

>
>>> >> +     depends on ARCH_NPCM && ARCH_MULTI_V7
>>> >> +     bool "Support for NPCM750 BMC CPU (Poleg)"
>>> >> +     select CACHE_L2X0
>>> >> +     select CPU_V7
>>> >> +     select ARM_GIC
>>> >> +     select HAVE_SMP
>>> >> +     select SMP
>>> >> +     select SMP_ON_UP
>>> >> +     select HAVE_ARM_SCU
>>> >> +     select HAVE_ARM_TWD if SMP
>>> >> +     select ARM_ERRATA_720789
>>> >> +     select ARM_ERRATA_754322
>>> >> +     select ARM_ERRATA_764369
>>> >> +     select ARM_ERRATA_794072
>>> >> +     select PL310_ERRATA_588369
>>> >> +     select PL310_ERRATA_727915
>>> >> +     select USB_EHCI_ROOT_HUB_TT
>>> >> +     select USB_ARCH_HAS_HCD
>>> >> +     select USB_ARCH_HAS_EHCI
>>> >> +     select USB_EHCI_HCD
>>> >> +     select USB_ARCH_HAS_OHCI
>>> >> +     select USB_OHCI_HCD
>>> >> +     select USB
>>> >> +     select FIQ
>>> >> +     select CPU_USE_DOMAINS
>>> >> +     select GENERIC_CLOCKEVENTS
>>> >> +     select CLKDEV_LOOKUP
>>> >> +     select COMMON_CLK if OF
>>> >> +     select NPCM750_TIMER
>>> >> +     select MFD_SYSCON
>>> >> +     help
>>> >> +       Support for NPCM750 BMC CPU (Poleg).
>>> >> +
>>> >> +       Nuvoton NPCM750 BMC based on the Cortex A9.
>>> >> +
>>> >> +endif
>>> >> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
>>> >> new file mode 100644
>>> >> index 000000000000..78416055b854
>>> >> --- /dev/null
>>> >> +++ b/arch/arm/mach-npcm/Makefile
>>> >> @@ -0,0 +1,3 @@
>>> >> +AFLAGS_headsmp.o             += -march=armv7-a
>>> >> +
>>> >> +obj-$(CONFIG_CPU_NPCM750)    += npcm7xx.o platsmp.o headsmp.o
>>> >> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
>>> >> new file mode 100644
>>> >> index 000000000000..9fccbbd49ed4
>>> >> --- /dev/null
>>> >> +++ b/arch/arm/mach-npcm/headsmp.S
>>> >> @@ -0,0 +1,17 @@
>>> >> +/*
>>> >> + * Copyright 2017 Google, Inc.
>>> >> + *
>>> >> + * This program is free software; you can redistribute it and/or modify
>>> >> + * it under the terms of the GNU General Public License version 2 as
>>> >> + * published by the Free Software Foundation.
>>> >> + */
>>> >> +
>>> >> +#include <linux/linkage.h>
>>> >> +#include <linux/init.h>
>>> >> +#include <asm/assembler.h>
>>> >> +
>>> >> +ENTRY(npcm7xx_secondary_startup)
>>> >> +     safe_svcmode_maskall r0
>>> >> +
>>> >> +     b       secondary_startup
>>> >> +ENDPROC(npcm7xx_secondary_startup)
>>> >
>>> > Why do you need this?  The switch to svc mode is already handled by
>>> > secondary_startup.
>>>
>>> Florian Fainelli already brought this up:
>>> https://www.spinics.net/lists/arm-kernel/msg605126.html
>>> Without this the kernel complains that the core is started in
>>> "wrong/inconsistent mode":
>>
>> The solution would be a comment to indicate why it's necessary :)
>>
>>>
>>> SMP: Total of 2 processors activated (398.13 BogoMIPS).
>>> CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU
>>> mode 0x13)
>>> CPU: This may indicate a broken bootloader or firmware.
>>>
>>> >
>>> >> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
>>> >> new file mode 100644
>>> >> index 000000000000..132e9d587857
>>> >> --- /dev/null
>>> >> +++ b/arch/arm/mach-npcm/npcm7xx.c
>>> >> @@ -0,0 +1,34 @@
>>> >> +/*
>>> >> + * Copyright (c) 2017 Nuvoton Technology corporation.
>>> >> + * Copyright 2017 Google, Inc.
>>> >> + *
>>> >> + * This program is free software; you can redistribute it and/or modify
>>> >> + * it under the terms of the GNU General Public License version 2 as
>>> >> + * published by the Free Software Foundation.
>>> >> + */
>>> >> +
>>> >> +#include <linux/kernel.h>
>>> >> +#include <linux/types.h>
>>> >> +#include <asm/mach/arch.h>
>>> >> +#include <asm/mach-types.h>
>>> >> +#include <asm/mach/map.h>
>>> >> +#include <asm/hardware/cache-l2x0.h>
>>> >> +
>>> >> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |                             \
>>> >> +                      L310_AUX_CTRL_DATA_PREFETCH |                         \
>>> >> +                      L310_AUX_CTRL_NS_LOCKDOWN |                           \
>>> >> +                      L310_AUX_CTRL_CACHE_REPLACE_RR |                      \
>>> >> +                      L2C_AUX_CTRL_SHARED_OVERRIDE |                        \
>>> >> +                      L310_AUX_CTRL_FULL_LINE_ZERO)
>>> >
>>> > Do you really need all these flags?
>>>
>>> Tomer, Avi, do we need these? I got this from the original version of
>>> the driver.
>>
>> The l2c driver manages L310_AUX_CTRL_FULL_LINE_ZERO itself, so you
>> shouldn't specify that.
>>
>> L2C_AUX_CTRL_SHARED_OVERRIDE is managed via the "arm,shared-override"
>> DT property, so you should use that in DT instead.
>>
>> Prefetching needs the prefetch control register configured to tune the
>> prefetching - that's platform specific so the l2c driver doesn't touch
>> it (it expects firmware to have configured it.)  If firmware needs to
>> configure that, it might as well configure the flags too as they're
>> shared into that very same register.  If not, I think the l2c prefetch
>> register defaults to a very small prefetch, and is probably not worth
>> setting the enable bits without configuring the depth of prefetch.
>
> Yes, we do it in the firmware.
>
>>
>> L310_AUX_CTRL_NS_LOCKDOWN is always enabled by the l2c driver, so
>> shouldn't be necessary.
>>
>> So that leaves only L310_AUX_CTRL_CACHE_REPLACE_RR.
>>
>
> You dont need to add L310_AUX_CTRL_CACHE_REPLACE_RR to the l2c aux value.
>
>>>
>>> >
>>> >> +
>>> >> +static const char *const npcm7xx_dt_match[] = {
>>> >> +     "nuvoton,npcm750",
>>> >> +     NULL
>>> >> +};
>>> >> +
>>> >> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
>>> >> +     .atag_offset    = 0x100,
>>> >> +     .dt_compat      = npcm7xx_dt_match,
>>> >> +     .l2c_aux_val    = NPCM7XX_AUX_VAL,
>>> >> +     .l2c_aux_mask   = ~NPCM7XX_AUX_VAL,
>>> >> +MACHINE_END
>>> >> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
>>> >> new file mode 100644
>>> >> index 000000000000..450e83c3c531
>>> >> --- /dev/null
>>> >> +++ b/arch/arm/mach-npcm/platsmp.c
>>> >> @@ -0,0 +1,88 @@
>>> >> +/*
>>> >> + * Copyright 2017 Google, Inc.
>>> >> + *
>>> >> + * This program is free software; you can redistribute it and/or modify
>>> >> + * it under the terms of the GNU General Public License version 2 as
>>> >> + * published by the Free Software Foundation.
>>> >> + */
>>> >> +
>>> >> +#define pr_fmt(fmt) "PLATSMP: " fmt
>>> >
>>> > Maybe something more descriptive?
>>> >
>>> >> +
>>> >> +#include <linux/delay.h>
>>> >> +#include <linux/device.h>
>>> >> +#include <linux/smp.h>
>>> >> +#include <linux/io.h>
>>> >> +#include <linux/of.h>
>>> >> +#include <linux/of_device.h>
>>> >> +#include <linux/of_platform.h>
>>> >> +#include <linux/of_address.h>
>>> >> +#include <asm/cacheflush.h>
>>> >> +#include <asm/smp.h>
>>> >> +#include <asm/smp_plat.h>
>>> >> +#include <asm/smp_scu.h>
>>> >> +
>>> >> +#define NPCM7XX_SCRPAD_REG 0x13c
>>> >> +
>>> >> +extern void npcm7xx_secondary_startup(void);
>>> >> +
>>> >> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
>>> >> +                                   struct task_struct *idle)
>>> >> +{
>>> >> +     struct device_node *gcr_np;
>>> >> +     void __iomem *gcr_base;
>>> >> +     int ret = 0;
>>> >> +
>>> >> +     gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
>>> >> +     if (!gcr_np) {
>>> >> +             pr_err("no gcr device node\n");
>>> >> +             ret = -EFAULT;
>>> >
>>> > EFAULT is used for bad userspace addresses, it should not be used for
>>> > other stuff - and a missing node is certainly not a "Bad address".
>>> >
>>> >> +             goto out;
>>> >> +     }
>>> >> +     gcr_base = of_iomap(gcr_np, 0);
>>> >> +     if (!gcr_base) {
>>> >> +             pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
>>> >> +             ret = -EFAULT;
>>> >
>>> > This is more -ENOMEM.
>>> >
>>> >> +             goto out;
>>> >> +     }
>>> >> +
>>> >> +     /* give boot ROM kernel start address. */
>>> >> +     iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
>>> >> +               NPCM7XX_SCRPAD_REG);
>>> >> +     /* make sure npcm7xx_secondary_startup is seen by all observers. */
>>> >> +     smp_wmb();
>>> >
>>> > I think you mean "make sure the previous write is seen by all observers".
>>> > However, doesn't "dsb_sev()" already do that?
>>> >
>>> >> +     dsb_sev();
>>> >> +     /* make sure write buffer is drained */
>>> >> +     mb();
>>> >
>>> > This looks over the top.
>>> >
>>> >> +
>>> >> +     iounmap(gcr_base);
>>> >> +out:
>>> >> +     return ret;
>>> >> +}
>>> >> +
>>> >> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
>>> >> +{
>>> >> +     struct device_node *scu_np;
>>> >> +     void __iomem *scu_base;
>>> >> +
>>> >> +     scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>>> >> +     if (!scu_np) {
>>> >> +             pr_err("no scu device node\n");
>>> >> +             return;
>>> >> +     }
>>> >> +     scu_base = of_iomap(scu_np, 0);
>>> >> +     if (!scu_base) {
>>> >> +             pr_err("could not iomap scu at: 0x%p\n", scu_base);
>>> >> +             return;
>>> >> +     }
>>> >> +
>>> >> +     scu_enable(scu_base);
>>> >> +
>>> >> +     iounmap(scu_base);
>>> >> +}
>>> >> +
>>> >> +static struct smp_operations npcm7xx_smp_ops __initdata = {
>>> >> +     .smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
>>> >> +     .smp_boot_secondary = npcm7xx_smp_boot_secondary,
>>> >> +};
>>> >> +
>>> >> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
>>> >> --
>>> >> 2.15.0.rc0.271.g36b669edcc-goog
>>> >>
>>> >
>>> > --
>>> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>>> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
>>> > According to speedtest.net: 8.21Mbps down 510kbps up
>>
>> --
>> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
>> According to speedtest.net: 8.21Mbps down 510kbps up
>
> Thanks,
>
> Tomer

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

* Re: [PATCH v7 1/3] arm: npcm: add basic support for Nuvoton BMCs
  2017-11-04  0:49           ` Brendan Higgins
@ 2017-11-06 10:35             ` Tomer Maimon
  0 siblings, 0 replies; 13+ messages in thread
From: Tomer Maimon @ 2017-11-06 10:35 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Russell King - ARM Linux, Avi Fishman, Rob Herring, Mark Rutland,
	Rick Altherr, Florian Fainelli, devicetree,
	Linux Kernel Mailing List, linux-arm-kernel, OpenBMC Maillist

On 4 November 2017 at 02:49, Brendan Higgins <brendanhiggins@google.com> wrote:
> On Thu, Oct 26, 2017 at 4:45 AM, Tomer Maimon <tmaimon77@gmail.com> wrote:
>> Hi Brendan,
>>
>> Sorry for the delay,
>>
>> On 21 October 2017 at 00:08, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>>> On Fri, Oct 20, 2017 at 01:57:47PM -0700, Brendan Higgins wrote:
>>>> On Fri, Oct 20, 2017 at 3:48 AM, Russell King - ARM Linux
>>>> <linux@armlinux.org.uk> wrote:
>>>> > On Thu, Oct 19, 2017 at 03:50:48PM -0700, Brendan Higgins wrote:
>>>> >> Adds basic support for the Nuvoton NPCM750 BMC.
>>>> >>
>>>> >> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>> >> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
>>>> >> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
>>>> >> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
>>>> >> Tested-by: Avi Fishman <avifishman70@gmail.com>
>>>> >> ---
>>>> >>  arch/arm/Kconfig             |  2 +
>>>> >>  arch/arm/Makefile            |  1 +
>>>> >>  arch/arm/mach-npcm/Kconfig   | 48 ++++++++++++++++++++++++
>>>> >>  arch/arm/mach-npcm/Makefile  |  3 ++
>>>> >>  arch/arm/mach-npcm/headsmp.S | 17 +++++++++
>>>> >>  arch/arm/mach-npcm/npcm7xx.c | 34 +++++++++++++++++
>>>> >>  arch/arm/mach-npcm/platsmp.c | 88 ++++++++++++++++++++++++++++++++++++++++++++
>>>> >>  7 files changed, 193 insertions(+)
>>>> >>  create mode 100644 arch/arm/mach-npcm/Kconfig
>>>> >>  create mode 100644 arch/arm/mach-npcm/Makefile
>>>> >>  create mode 100644 arch/arm/mach-npcm/headsmp.S
>>>> >>  create mode 100644 arch/arm/mach-npcm/npcm7xx.c
>>>> >>  create mode 100644 arch/arm/mach-npcm/platsmp.c
>>>> >>
>>>> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>>> >> index 61a0cb15067e..05543f1cfbde 100644
>>>> >> --- a/arch/arm/Kconfig
>>>> >> +++ b/arch/arm/Kconfig
>>>> >> @@ -782,6 +782,8 @@ source "arch/arm/mach-netx/Kconfig"
>>>> >>
>>>> >>  source "arch/arm/mach-nomadik/Kconfig"
>>>> >>
>>>> >> +source "arch/arm/mach-npcm/Kconfig"
>>>> >> +
>>>> >>  source "arch/arm/mach-nspire/Kconfig"
>>>> >>
>>>> >>  source "arch/arm/plat-omap/Kconfig"
>>>> >> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
>>>> >> index 47d3a1ab08d2..60ca50c7d762 100644
>>>> >> --- a/arch/arm/Makefile
>>>> >> +++ b/arch/arm/Makefile
>>>> >> @@ -191,6 +191,7 @@ machine-$(CONFIG_ARCH_MEDIATEK)           += mediatek
>>>> >>  machine-$(CONFIG_ARCH_MXS)           += mxs
>>>> >>  machine-$(CONFIG_ARCH_NETX)          += netx
>>>> >>  machine-$(CONFIG_ARCH_NOMADIK)               += nomadik
>>>> >> +machine-$(CONFIG_ARCH_NPCM)          += npcm
>>>> >>  machine-$(CONFIG_ARCH_NSPIRE)                += nspire
>>>> >>  machine-$(CONFIG_ARCH_OXNAS)         += oxnas
>>>> >>  machine-$(CONFIG_ARCH_OMAP1)         += omap1
>>>> >> diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig
>>>> >> new file mode 100644
>>>> >> index 000000000000..21dfa8ce828f
>>>> >> --- /dev/null
>>>> >> +++ b/arch/arm/mach-npcm/Kconfig
>>>> >> @@ -0,0 +1,48 @@
>>>> >> +menuconfig ARCH_NPCM
>>>> >> +     bool "Nuvoton NPCM Architecture"
>>>> >> +     select ARCH_REQUIRE_GPIOLIB
>>>> >> +     select USE_OF
>>>> >> +     select PINCTRL
>>>> >> +     select PINCTRL_NPCM7XX
>>>> >> +
>>>> >> +if ARCH_NPCM
>>>> >> +
>>>> >> +comment "NPCMX50 CPU type"
>>>> >> +
>>>> >> +config CPU_NPCM750
>>
>> I started the upstream process of NPCM7xx clocksource driver, I got
>> the following comment
>> https://www.spinics.net/lists/devicetree/msg197916.html
>>
>> Is it possible to modify the the architecture configuration name to
>> ARCH_NPCM7XX.
>
> You mean the CPU config: CPU_NPCM7XX?
Sorry about the confusion, I mean to modify the CPU_NPCM750 to ARCH_NPCM750.
>
> I think the ARCH_NPCM is right, since that corresponds to this directory
> which is for all ARM Nuvoton BMCs.
>
>>
>>>> >> +     depends on ARCH_NPCM && ARCH_MULTI_V7
>>>> >> +     bool "Support for NPCM750 BMC CPU (Poleg)"
>>>> >> +     select CACHE_L2X0
>>>> >> +     select CPU_V7
>>>> >> +     select ARM_GIC
>>>> >> +     select HAVE_SMP
>>>> >> +     select SMP
>>>> >> +     select SMP_ON_UP
>>>> >> +     select HAVE_ARM_SCU
>>>> >> +     select HAVE_ARM_TWD if SMP
>>>> >> +     select ARM_ERRATA_720789
>>>> >> +     select ARM_ERRATA_754322
>>>> >> +     select ARM_ERRATA_764369
>>>> >> +     select ARM_ERRATA_794072
>>>> >> +     select PL310_ERRATA_588369
>>>> >> +     select PL310_ERRATA_727915
>>>> >> +     select USB_EHCI_ROOT_HUB_TT
>>>> >> +     select USB_ARCH_HAS_HCD
>>>> >> +     select USB_ARCH_HAS_EHCI
>>>> >> +     select USB_EHCI_HCD
>>>> >> +     select USB_ARCH_HAS_OHCI
>>>> >> +     select USB_OHCI_HCD
>>>> >> +     select USB
>>>> >> +     select FIQ
>>>> >> +     select CPU_USE_DOMAINS
>>>> >> +     select GENERIC_CLOCKEVENTS
>>>> >> +     select CLKDEV_LOOKUP
>>>> >> +     select COMMON_CLK if OF
>>>> >> +     select NPCM750_TIMER
>>>> >> +     select MFD_SYSCON
>>>> >> +     help
>>>> >> +       Support for NPCM750 BMC CPU (Poleg).
>>>> >> +
>>>> >> +       Nuvoton NPCM750 BMC based on the Cortex A9.
>>>> >> +
>>>> >> +endif
>>>> >> diff --git a/arch/arm/mach-npcm/Makefile b/arch/arm/mach-npcm/Makefile
>>>> >> new file mode 100644
>>>> >> index 000000000000..78416055b854
>>>> >> --- /dev/null
>>>> >> +++ b/arch/arm/mach-npcm/Makefile
>>>> >> @@ -0,0 +1,3 @@
>>>> >> +AFLAGS_headsmp.o             += -march=armv7-a
>>>> >> +
>>>> >> +obj-$(CONFIG_CPU_NPCM750)    += npcm7xx.o platsmp.o headsmp.o
>>>> >> diff --git a/arch/arm/mach-npcm/headsmp.S b/arch/arm/mach-npcm/headsmp.S
>>>> >> new file mode 100644
>>>> >> index 000000000000..9fccbbd49ed4
>>>> >> --- /dev/null
>>>> >> +++ b/arch/arm/mach-npcm/headsmp.S
>>>> >> @@ -0,0 +1,17 @@
>>>> >> +/*
>>>> >> + * Copyright 2017 Google, Inc.
>>>> >> + *
>>>> >> + * This program is free software; you can redistribute it and/or modify
>>>> >> + * it under the terms of the GNU General Public License version 2 as
>>>> >> + * published by the Free Software Foundation.
>>>> >> + */
>>>> >> +
>>>> >> +#include <linux/linkage.h>
>>>> >> +#include <linux/init.h>
>>>> >> +#include <asm/assembler.h>
>>>> >> +
>>>> >> +ENTRY(npcm7xx_secondary_startup)
>>>> >> +     safe_svcmode_maskall r0
>>>> >> +
>>>> >> +     b       secondary_startup
>>>> >> +ENDPROC(npcm7xx_secondary_startup)
>>>> >
>>>> > Why do you need this?  The switch to svc mode is already handled by
>>>> > secondary_startup.
>>>>
>>>> Florian Fainelli already brought this up:
>>>> https://www.spinics.net/lists/arm-kernel/msg605126.html
>>>> Without this the kernel complains that the core is started in
>>>> "wrong/inconsistent mode":
>>>
>>> The solution would be a comment to indicate why it's necessary :)
>>>
>>>>
>>>> SMP: Total of 2 processors activated (398.13 BogoMIPS).
>>>> CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU
>>>> mode 0x13)
>>>> CPU: This may indicate a broken bootloader or firmware.
>>>>
>>>> >
>>>> >> diff --git a/arch/arm/mach-npcm/npcm7xx.c b/arch/arm/mach-npcm/npcm7xx.c
>>>> >> new file mode 100644
>>>> >> index 000000000000..132e9d587857
>>>> >> --- /dev/null
>>>> >> +++ b/arch/arm/mach-npcm/npcm7xx.c
>>>> >> @@ -0,0 +1,34 @@
>>>> >> +/*
>>>> >> + * Copyright (c) 2017 Nuvoton Technology corporation.
>>>> >> + * Copyright 2017 Google, Inc.
>>>> >> + *
>>>> >> + * This program is free software; you can redistribute it and/or modify
>>>> >> + * it under the terms of the GNU General Public License version 2 as
>>>> >> + * published by the Free Software Foundation.
>>>> >> + */
>>>> >> +
>>>> >> +#include <linux/kernel.h>
>>>> >> +#include <linux/types.h>
>>>> >> +#include <asm/mach/arch.h>
>>>> >> +#include <asm/mach-types.h>
>>>> >> +#include <asm/mach/map.h>
>>>> >> +#include <asm/hardware/cache-l2x0.h>
>>>> >> +
>>>> >> +#define NPCM7XX_AUX_VAL (L310_AUX_CTRL_INSTR_PREFETCH |                             \
>>>> >> +                      L310_AUX_CTRL_DATA_PREFETCH |                         \
>>>> >> +                      L310_AUX_CTRL_NS_LOCKDOWN |                           \
>>>> >> +                      L310_AUX_CTRL_CACHE_REPLACE_RR |                      \
>>>> >> +                      L2C_AUX_CTRL_SHARED_OVERRIDE |                        \
>>>> >> +                      L310_AUX_CTRL_FULL_LINE_ZERO)
>>>> >
>>>> > Do you really need all these flags?
>>>>
>>>> Tomer, Avi, do we need these? I got this from the original version of
>>>> the driver.
>>>
>>> The l2c driver manages L310_AUX_CTRL_FULL_LINE_ZERO itself, so you
>>> shouldn't specify that.
>>>
>>> L2C_AUX_CTRL_SHARED_OVERRIDE is managed via the "arm,shared-override"
>>> DT property, so you should use that in DT instead.
>>>
>>> Prefetching needs the prefetch control register configured to tune the
>>> prefetching - that's platform specific so the l2c driver doesn't touch
>>> it (it expects firmware to have configured it.)  If firmware needs to
>>> configure that, it might as well configure the flags too as they're
>>> shared into that very same register.  If not, I think the l2c prefetch
>>> register defaults to a very small prefetch, and is probably not worth
>>> setting the enable bits without configuring the depth of prefetch.
>>
>> Yes, we do it in the firmware.
>>
>>>
>>> L310_AUX_CTRL_NS_LOCKDOWN is always enabled by the l2c driver, so
>>> shouldn't be necessary.
>>>
>>> So that leaves only L310_AUX_CTRL_CACHE_REPLACE_RR.
>>>
>>
>> You dont need to add L310_AUX_CTRL_CACHE_REPLACE_RR to the l2c aux value.
>>
>>>>
>>>> >
>>>> >> +
>>>> >> +static const char *const npcm7xx_dt_match[] = {
>>>> >> +     "nuvoton,npcm750",
>>>> >> +     NULL
>>>> >> +};
>>>> >> +
>>>> >> +DT_MACHINE_START(NPCM7XX_DT, "NPCMX50 Chip family")
>>>> >> +     .atag_offset    = 0x100,
>>>> >> +     .dt_compat      = npcm7xx_dt_match,
>>>> >> +     .l2c_aux_val    = NPCM7XX_AUX_VAL,
>>>> >> +     .l2c_aux_mask   = ~NPCM7XX_AUX_VAL,
>>>> >> +MACHINE_END
>>>> >> diff --git a/arch/arm/mach-npcm/platsmp.c b/arch/arm/mach-npcm/platsmp.c
>>>> >> new file mode 100644
>>>> >> index 000000000000..450e83c3c531
>>>> >> --- /dev/null
>>>> >> +++ b/arch/arm/mach-npcm/platsmp.c
>>>> >> @@ -0,0 +1,88 @@
>>>> >> +/*
>>>> >> + * Copyright 2017 Google, Inc.
>>>> >> + *
>>>> >> + * This program is free software; you can redistribute it and/or modify
>>>> >> + * it under the terms of the GNU General Public License version 2 as
>>>> >> + * published by the Free Software Foundation.
>>>> >> + */
>>>> >> +
>>>> >> +#define pr_fmt(fmt) "PLATSMP: " fmt
>>>> >
>>>> > Maybe something more descriptive?
>>>> >
>>>> >> +
>>>> >> +#include <linux/delay.h>
>>>> >> +#include <linux/device.h>
>>>> >> +#include <linux/smp.h>
>>>> >> +#include <linux/io.h>
>>>> >> +#include <linux/of.h>
>>>> >> +#include <linux/of_device.h>
>>>> >> +#include <linux/of_platform.h>
>>>> >> +#include <linux/of_address.h>
>>>> >> +#include <asm/cacheflush.h>
>>>> >> +#include <asm/smp.h>
>>>> >> +#include <asm/smp_plat.h>
>>>> >> +#include <asm/smp_scu.h>
>>>> >> +
>>>> >> +#define NPCM7XX_SCRPAD_REG 0x13c
>>>> >> +
>>>> >> +extern void npcm7xx_secondary_startup(void);
>>>> >> +
>>>> >> +static int npcm7xx_smp_boot_secondary(unsigned int cpu,
>>>> >> +                                   struct task_struct *idle)
>>>> >> +{
>>>> >> +     struct device_node *gcr_np;
>>>> >> +     void __iomem *gcr_base;
>>>> >> +     int ret = 0;
>>>> >> +
>>>> >> +     gcr_np = of_find_compatible_node(NULL, NULL, "nuvoton,npcm750-gcr");
>>>> >> +     if (!gcr_np) {
>>>> >> +             pr_err("no gcr device node\n");
>>>> >> +             ret = -EFAULT;
>>>> >
>>>> > EFAULT is used for bad userspace addresses, it should not be used for
>>>> > other stuff - and a missing node is certainly not a "Bad address".
>>>> >
>>>> >> +             goto out;
>>>> >> +     }
>>>> >> +     gcr_base = of_iomap(gcr_np, 0);
>>>> >> +     if (!gcr_base) {
>>>> >> +             pr_err("could not iomap gcr at: 0x%p\n", gcr_base);
>>>> >> +             ret = -EFAULT;
>>>> >
>>>> > This is more -ENOMEM.
>>>> >
>>>> >> +             goto out;
>>>> >> +     }
>>>> >> +
>>>> >> +     /* give boot ROM kernel start address. */
>>>> >> +     iowrite32(__pa_symbol(npcm7xx_secondary_startup), gcr_base +
>>>> >> +               NPCM7XX_SCRPAD_REG);
>>>> >> +     /* make sure npcm7xx_secondary_startup is seen by all observers. */
>>>> >> +     smp_wmb();
>>>> >
>>>> > I think you mean "make sure the previous write is seen by all observers".
>>>> > However, doesn't "dsb_sev()" already do that?
>>>> >
>>>> >> +     dsb_sev();
>>>> >> +     /* make sure write buffer is drained */
>>>> >> +     mb();
>>>> >
>>>> > This looks over the top.
>>>> >
>>>> >> +
>>>> >> +     iounmap(gcr_base);
>>>> >> +out:
>>>> >> +     return ret;
>>>> >> +}
>>>> >> +
>>>> >> +static void __init npcm7xx_smp_prepare_cpus(unsigned int max_cpus)
>>>> >> +{
>>>> >> +     struct device_node *scu_np;
>>>> >> +     void __iomem *scu_base;
>>>> >> +
>>>> >> +     scu_np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>>>> >> +     if (!scu_np) {
>>>> >> +             pr_err("no scu device node\n");
>>>> >> +             return;
>>>> >> +     }
>>>> >> +     scu_base = of_iomap(scu_np, 0);
>>>> >> +     if (!scu_base) {
>>>> >> +             pr_err("could not iomap scu at: 0x%p\n", scu_base);
>>>> >> +             return;
>>>> >> +     }
>>>> >> +
>>>> >> +     scu_enable(scu_base);
>>>> >> +
>>>> >> +     iounmap(scu_base);
>>>> >> +}
>>>> >> +
>>>> >> +static struct smp_operations npcm7xx_smp_ops __initdata = {
>>>> >> +     .smp_prepare_cpus = npcm7xx_smp_prepare_cpus,
>>>> >> +     .smp_boot_secondary = npcm7xx_smp_boot_secondary,
>>>> >> +};
>>>> >> +
>>>> >> +CPU_METHOD_OF_DECLARE(npcm7xx_smp, "nuvoton,npcm7xx-smp", &npcm7xx_smp_ops);
>>>> >> --
>>>> >> 2.15.0.rc0.271.g36b669edcc-goog
>>>> >>
>>>> >
>>>> > --
>>>> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>>>> > FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
>>>> > According to speedtest.net: 8.21Mbps down 510kbps up
>>>
>>> --
>>> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
>>> FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
>>> According to speedtest.net: 8.21Mbps down 510kbps up
>>
>> Thanks,
>>
>> Tomer

Thanks

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

* Re: [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree
  2017-10-20 19:45   ` Rob Herring
@ 2017-11-17  2:50     ` Brendan Higgins
  0 siblings, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2017-11-17  2:50 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, Russell King, Avi Fishman, Tomer Maimon,
	Rick Altherr, Florian Fainelli, devicetree, linux-kernel,
	linux-arm-kernel, OpenBMC Maillist

Sorry for the delay. I thought all of the comments were straightforward, but
there are a couple which need some further discussion.

On Fri, Oct 20, 2017 at 12:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Thu, Oct 19, 2017 at 5:50 PM, Brendan Higgins
> <brendanhiggins@google.com> wrote:
>> Add a common device tree for all Nuvoton NPCM750 BMCs and a board
>> specific device tree for the NPCM750 (Poleg) evaluation board.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> Reviewed-by: Tomer Maimon <tmaimon77@gmail.com>
>> Reviewed-by: Avi Fishman <avifishman70@gmail.com>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> Tested-by: Tomer Maimon <tmaimon77@gmail.com>
>> Tested-by: Avi Fishman <avifishman70@gmail.com>
>> ---
>
> [...]
>
>> diff --git a/arch/arm/boot/dts/nuvoton-npcm750.dtsi b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
>> new file mode 100644
>> index 000000000000..85506b3cdd39
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/nuvoton-npcm750.dtsi
>> @@ -0,0 +1,198 @@
>> +/*
>> + * DTSi file for the NPCM750 SoC
>> + *
>> + * Copyright 2012 Tomer Maimon <tomer.maimon@nuvoton.com>
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/nuvoton,npcm7xx-clks.h>
>> +
>> +/ {
>> +       #address-cells = <1>;
>> +       #size-cells = <1>;
>> +       interrupt-parent = <&gic>;
>> +
>> +       cpus {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               enable-method = "nuvoton,npcm7xx-smp";
>> +
>> +               cpu@0 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a9";
>> +                       clocks = <&clk NPCM7XX_CLK_CPU>;
>> +                       clock-names = "clk_cpu";
>> +                       reg = <0>;
>> +                       next-level-cache = <&l2>;
>> +               };
>> +
>> +               cpu@1 {
>> +                       device_type = "cpu";
>> +                       compatible = "arm,cortex-a9";
>> +                       clocks = <&clk NPCM7XX_CLK_CPU>;
>> +                       clock-names = "clk_cpu";
>> +                       reg = <1>;
>> +                       next-level-cache = <&l2>;
>> +               };
>> +       };
>> +
>> +/* external clock signal rg1refck, supplied by the phy */
>> +clk-rg1refck {
>> +       compatible = "fixed-clock";
>> +       #clock-cells = <0>;
>> +       clock-frequency = <125000000>;
>> +};
>> +
>> +/* external clock signal rg2refck, supplied by the phy */
>> +clk-rg2refck {
>> +       compatible = "fixed-clock";
>> +       #clock-cells = <0>;
>> +       clock-frequency = <125000000>;
>> +};
>> +
>> +clk-xin {
>> +       compatible = "fixed-clock";
>> +       #clock-cells = <0>;
>> +       clock-frequency = <50000000>;
>> +};
>> +
>> +       soc {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               compatible = "simple-bus";
>> +               interrupt-parent = <&gic>;
>> +               ranges = <0x0 0xf0000000 0x00900000>;
>> +
>> +               gcr: gcr@f0800000 {
>
> Should be gcr@800000. Build the dtb with W=1 and it will give you
> warnings for this.
>
>> +                       compatible = "nuvoton,npcm750-gcr", "syscon",
>> +                               "simple-mfd";
>> +                       reg = <0x800000 0x1000>;
>> +               };
>> +
>> +               scu: scu@f03fe000 {
>> +                       compatible = "arm,cortex-a9-scu";
>> +                       reg = <0x3fe000 0x1000>;
>> +               };
>> +
>> +               l2: cache-controller@f03fc000 {
>> +                       compatible = "arm,pl310-cache";
>> +                       reg = <0x3fc000 0x1000>;
>> +                       interrupts = <0 21 4>;
>> +                       cache-unified;
>> +                       cache-level = <2>;
>> +                       clocks = <&clk NPCM7XX_CLK_AXI>;
>> +               };
>> +
>> +               gic: interrupt-controller@f03ff000 {
>> +                       compatible = "arm,cortex-a9-gic";
>> +                       interrupt-controller;
>> +                       #interrupt-cells = <3>;
>> +                       reg = <0x3ff000 0x1000>,
>> +                           <0x3fe100 0x100>;
>> +               };
>> +
>> +               timer@f03fe600 {
>> +                       compatible = "arm,cortex-a9-twd-timer";
>> +                       reg = <0x3fe600 0x20>;
>> +                       interrupts = <1 13 0x304>;
>> +                       clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +               };
>> +       };
>> +
>> +       ahb {
>> +               #address-cells = <1>;
>> +               #size-cells = <1>;
>> +               compatible = "simple-bus";
>> +               interrupt-parent = <&gic>;
>> +               ranges = <0x80000000 0x80000000 0x40010000
>> +                         0xe0800000 0xe0800000 0x0f800000
>> +                         0x00000000 0xf0000000 0x00900000
>> +                         0xf8000000 0xf8000000 0x02000000
>> +                         0xfb000000 0xfb000000 0x00002000>;
>
> Just do "ranges;" here if all the translation is at the next level down.

Actually we do have some peripherals that sit directly on the AHB, they
just have not had their DTS nodes written yet. I counted at least 6
devices which probably won't get a sub-memory space.

>
> Sorry, it's hard to say exactly what to do without knowing the whole memory map.

No problem. Let me see if I can get it for you.

>
>> +
>> +               clk: clock-controller@f0801000 {
>
> Seems like this belongs under /soc based on the address.

No, the AHB memory space is discontiguous. The "Cortex A9 Included" space
(/soc here) sits in one of the discontinuities.

>
>> +                       compatible = "nuvoton,npcm750-clk";
>> +                       #clock-cells = <1>;
>> +                       reg = <0x801000 0x1000>;
>> +                       status = "okay";
>> +               };
>> +
>> +               apb {
>> +                       #address-cells = <1>;
>> +                       #size-cells = <1>;
>> +                       compatible = "simple-bus";
>> +                       interrupt-parent = <&gic>;
>> +                       ranges = <0x0 0x0 0x00300000>;
>
> And then this would be "<0x0 0xf0000000 0x300000>".
>
>> +
>> +                       timer0: timer@f0000000 {
>> +                               compatible = "nuvoton,npcm750-timer";
>> +                               interrupts = <0 32 4>;
>> +                               reg = <0x0 0x1000>;
>> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +                       };
>> +
>> +                       watchdog0: watchdog@f0008000 {
>> +                               compatible = "nuvoton,npcm750-wdt";
>> +                               interrupts = <0 47 4>;
>> +                               reg = <0x8000 0x1000>;
>> +                               status = "disabled";
>> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +                       };
>> +
>> +                       watchdog1: watchdog@f0009000 {
>> +                               compatible = "nuvoton,npcm750-wdt";
>> +                               interrupts = <0 48 4>;
>> +                               reg = <0x9000 0x1000>;
>> +                               status = "disabled";
>> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +                       };
>> +
>> +                       watchdog2: watchdog@f000a000 {
>> +                               compatible = "nuvoton,npcm750-wdt";
>> +                               interrupts = <0 49 4>;
>> +                               reg = <0xa000 0x1000>;
>> +                               status = "disabled";
>> +                               clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +                       };
>> +
>> +                       serial0: serial@f0001000 {
>> +                               compatible = "nuvoton,npcm750-uart";
>> +                               reg = <0x1000 0x1000>;
>> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
>> +                               interrupts = <0 2 4>;
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       serial1: serial@f0002000 {
>> +                               compatible = "nuvoton,npcm750-uart";
>> +                               reg = <0x2000 0x1000>;
>> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
>> +                               interrupts = <0 3 4>;
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       serial2: serial@f0003000 {
>> +                               compatible = "nuvoton,npcm750-uart";
>> +                               reg = <0x3000 0x1000>;
>> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
>> +                               interrupts = <0 4 4>;
>> +                               status = "disabled";
>> +                       };
>> +
>> +                       serial3: serial@f0004000 {
>> +                               compatible = "nuvoton,npcm750-uart";
>> +                               reg = <0x4000 0x1000>;
>> +                               clocks = <&clk NPCM7XX_CLK_UART_CORE>;
>> +                               interrupts = <0 5 4>;
>> +                               status = "disabled";
>> +                       };
>> +               };
>> +       };
>> +};
>> diff --git a/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
>> new file mode 100644
>> index 000000000000..c69d3bbf7e42
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/nuvoton,npcm7xx-clks.h
>> @@ -0,0 +1,39 @@
>> +/*
>> + * Copyright (C) 2016 Nuvoton Technologies,  tali.perry@nuvoton.com
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_NPCM7XX_H
>> +#define _DT_BINDINGS_CLK_NPCM7XX_H
>> +
>> +#define NPCM7XX_CLK_PLL0       0
>> +#define NPCM7XX_CLK_PLL1       1
>> +#define NPCM7XX_CLK_PLL2       2
>> +#define NPCM7XX_CLK_GFX                3
>> +#define NPCM7XX_CLK_APB1       4
>> +#define NPCM7XX_CLK_APB2       5
>> +#define NPCM7XX_CLK_APB3       6
>> +#define NPCM7XX_CLK_APB4       7
>> +#define NPCM7XX_CLK_APB5       8
>> +#define NPCM7XX_CLK_MC         9
>> +#define NPCM7XX_CLK_CPU                10
>> +#define NPCM7XX_CLK_SPI0       11
>> +#define NPCM7XX_CLK_SPI3       12
>> +#define NPCM7XX_CLK_SPIX       13
>> +#define NPCM7XX_CLK_UART_CORE  14
>> +#define NPCM7XX_CLK_TIMER      15
>> +#define NPCM7XX_CLK_HOST_UART  16
>> +#define NPCM7XX_CLK_MMC                17
>> +#define NPCM7XX_CLK_SDHC       18
>> +#define NPCM7XX_CLK_ADC                19
>> +#define NPCM7XX_CLK_GFX_MEM    20
>> +#define NPCM7XX_CLK_USB_BRIDGE 21
>> +#define NPCM7XX_CLK_AXI                22
>> +#define NPCM7XX_CLK_AHB                23
>> +#define NPCM7XX_CLK_EMC                24
>> +#define NPCM7XX_CLK_GMAC       25
>> +
>> +#endif
>> --
>> 2.15.0.rc0.271.g36b669edcc-goog
>>

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

end of thread, other threads:[~2017-11-17  2:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 22:50 [PATCH v7 0/3] arm: npcm: add basic support for Nuvoton BMCs Brendan Higgins
2017-10-19 22:50 ` [PATCH v7 1/3] " Brendan Higgins
2017-10-20 10:40   ` Julien Thierry
2017-10-20 10:48   ` Russell King - ARM Linux
2017-10-20 20:57     ` Brendan Higgins
2017-10-20 21:08       ` Russell King - ARM Linux
2017-10-26 11:45         ` Tomer Maimon
2017-11-04  0:49           ` Brendan Higgins
2017-11-06 10:35             ` Tomer Maimon
2017-10-19 22:50 ` [PATCH v7 2/3] arm: dts: add Nuvoton NPCM750 device tree Brendan Higgins
2017-10-20 19:45   ` Rob Herring
2017-11-17  2:50     ` Brendan Higgins
2017-10-19 22:50 ` [PATCH v7 3/3] MAINTAINERS: Add entry for the Nuvoton NPCM architecture Brendan Higgins

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