[PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP is the HPE BMC SoC that is used in the majority
of HPE Generation 10 servers. Traditionally the asic will
last multiple generations of server before being replaced.
In gxp.c we reset the EHCI controller early to boot the asic.

Info about SoC:

HPE GXP is the name of the HPE Soc. This SoC is used to implement
many BMC features at HPE. It supports ARMv7 architecture based on
the Cortex A9 core. It is capable of using an AXI bus to which
a memory controller is attached. It has multiple SPI interfaces
to connect boot flash and BIOS flash. It uses a 10/100/1000 MAC
for network connectivity. It has multiple i2c engines to drive
connectivity with a host infrastructure. The initial patches
enable the watchdog and timer enabling the host to be able to
boot.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed version log
v4:
* Removed unecessary code: restart, iomap, init_machine
* Reordered Kconfig depends
* Removed SPARSE_IRQ, MULTI_IRQ_HANDLER, IRQ_DOMAIN, PINCTL from
  Kconfig
v3:
* Put into proper patchset format
v2:
* No change
---
 arch/arm/Kconfig           |  2 ++
 arch/arm/Makefile          |  1 +
 arch/arm/mach-hpe/Kconfig  | 17 +++++++++++++++++
 arch/arm/mach-hpe/Makefile |  1 +
 arch/arm/mach-hpe/gxp.c    | 16 ++++++++++++++++
 5 files changed, 37 insertions(+)
 create mode 100644 arch/arm/mach-hpe/Kconfig
 create mode 100644 arch/arm/mach-hpe/Makefile
 create mode 100644 arch/arm/mach-hpe/gxp.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2e8091e2d8a8..13f77eec7c40 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -620,6 +620,8 @@ source "arch/arm/mach-highbank/Kconfig"
 
 source "arch/arm/mach-hisi/Kconfig"
 
+source "arch/arm/mach-hpe/Kconfig"
+
 source "arch/arm/mach-imx/Kconfig"
 
 source "arch/arm/mach-integrator/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index a2391b8de5a5..97a89023c10f 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -179,6 +179,7 @@ machine-$(CONFIG_ARCH_FOOTBRIDGE)	+= footbridge
 machine-$(CONFIG_ARCH_GEMINI)		+= gemini
 machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
 machine-$(CONFIG_ARCH_HISI)		+= hisi
+machine-$(CONFIG_ARCH_HPE)		+= hpe
 machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
 machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
 machine-$(CONFIG_ARCH_IXP4XX)		+= ixp4xx
diff --git a/arch/arm/mach-hpe/Kconfig b/arch/arm/mach-hpe/Kconfig
new file mode 100644
index 000000000000..c075248b259e
--- /dev/null
+++ b/arch/arm/mach-hpe/Kconfig
@@ -0,0 +1,17 @@
+menuconfig ARCH_HPE
+	bool "HPE SoC support"
+	depends on ARCH_MULTI_V7
+	help
+	  This enables support for HPE ARM based SoC chips
+if ARCH_HPE
+
+config ARCH_HPE_GXP
+	bool "HPE GXP SoC"
+	depends on ARCH_MULTI_V7
+	select ARM_VIC
+	select GENERIC_IRQ_CHIP
+	select CLKSRC_MMIO
+	help
+	  Support for GXP SoCs
+
+endif
diff --git a/arch/arm/mach-hpe/Makefile b/arch/arm/mach-hpe/Makefile
new file mode 100644
index 000000000000..8b0a91234df4
--- /dev/null
+++ b/arch/arm/mach-hpe/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ARCH_HPE_GXP) += gxp.o
diff --git a/arch/arm/mach-hpe/gxp.c b/arch/arm/mach-hpe/gxp.c
new file mode 100644
index 000000000000..e2f0c3ae6bd8
--- /dev/null
+++ b/arch/arm/mach-hpe/gxp.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/
+
+#include <linux/of_platform.h>
+#include <asm/mach/arch.h>
+
+static const char * const gxp_board_dt_compat[] = {
+	"hpe,gxp",
+	NULL,
+};
+
+DT_MACHINE_START(GXP_DT, "HPE GXP")
+	.dt_compat	= gxp_board_dt_compat,
+	.l2c_aux_val = 0,
+	.l2c_aux_mask = 0,
+MACHINE_END
-- 
2.17.1

[PATCH v5 01/11] archh: arm: mach-hpe: Introduce the HPE GXP architecture

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The GXP is the HPE BMC SoC that is used in the majority
of HPE Generation 10 servers. Traditionally the asic will
last multiple generations of server before being replaced.

Info about SoC:

HPE GXP is the name of the HPE Soc. This SoC is used to implement
many BMC features at HPE. It supports ARMv7 architecture based on
the Cortex A9 core. It is capable of using an AXI bus to which
a memory controller is attached. It has multiple SPI interfaces
to connect boot flash and BIOS flash. It uses a 10/100/1000 MAC
for network connectivity. It has multiple i2c engines to drive
connectivity with a host infrastructure. The initial patches
enable the watchdog and timer enabling the host to be able to
boot.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed version log
* Removed incorrect statement about reset.
v4:
* Removed unecessary code: restart, iomap, init_machine
* Reordered Kconfig depends
* Removed SPARSE_IRQ, MULTI_IRQ_HANDLER, IRQ_DOMAIN, PINCTL from
  Kconfig
v3:
* Put into proper patchset format
v2:
* No change
---
 arch/arm/Kconfig           |  2 ++
 arch/arm/Makefile          |  1 +
 arch/arm/mach-hpe/Kconfig  | 17 +++++++++++++++++
 arch/arm/mach-hpe/Makefile |  1 +
 arch/arm/mach-hpe/gxp.c    | 16 ++++++++++++++++
 5 files changed, 37 insertions(+)
 create mode 100644 arch/arm/mach-hpe/Kconfig
 create mode 100644 arch/arm/mach-hpe/Makefile
 create mode 100644 arch/arm/mach-hpe/gxp.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2e8091e2d8a8..13f77eec7c40 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -620,6 +620,8 @@ source "arch/arm/mach-highbank/Kconfig"
 
 source "arch/arm/mach-hisi/Kconfig"
 
+source "arch/arm/mach-hpe/Kconfig"
+
 source "arch/arm/mach-imx/Kconfig"
 
 source "arch/arm/mach-integrator/Kconfig"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index a2391b8de5a5..97a89023c10f 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -179,6 +179,7 @@ machine-$(CONFIG_ARCH_FOOTBRIDGE)	+= footbridge
 machine-$(CONFIG_ARCH_GEMINI)		+= gemini
 machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
 machine-$(CONFIG_ARCH_HISI)		+= hisi
+machine-$(CONFIG_ARCH_HPE)		+= hpe
 machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
 machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
 machine-$(CONFIG_ARCH_IXP4XX)		+= ixp4xx
diff --git a/arch/arm/mach-hpe/Kconfig b/arch/arm/mach-hpe/Kconfig
new file mode 100644
index 000000000000..c075248b259e
--- /dev/null
+++ b/arch/arm/mach-hpe/Kconfig
@@ -0,0 +1,17 @@
+menuconfig ARCH_HPE
+	bool "HPE SoC support"
+	depends on ARCH_MULTI_V7
+	help
+	  This enables support for HPE ARM based SoC chips
+if ARCH_HPE
+
+config ARCH_HPE_GXP
+	bool "HPE GXP SoC"
+	depends on ARCH_MULTI_V7
+	select ARM_VIC
+	select GENERIC_IRQ_CHIP
+	select CLKSRC_MMIO
+	help
+	  Support for GXP SoCs
+
+endif
diff --git a/arch/arm/mach-hpe/Makefile b/arch/arm/mach-hpe/Makefile
new file mode 100644
index 000000000000..8b0a91234df4
--- /dev/null
+++ b/arch/arm/mach-hpe/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_ARCH_HPE_GXP) += gxp.o
diff --git a/arch/arm/mach-hpe/gxp.c b/arch/arm/mach-hpe/gxp.c
new file mode 100644
index 000000000000..e2f0c3ae6bd8
--- /dev/null
+++ b/arch/arm/mach-hpe/gxp.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/
+
+#include <linux/of_platform.h>
+#include <asm/mach/arch.h>
+
+static const char * const gxp_board_dt_compat[] = {
+	"hpe,gxp",
+	NULL,
+};
+
+DT_MACHINE_START(GXP_DT, "HPE GXP")
+	.dt_compat	= gxp_board_dt_compat,
+	.l2c_aux_val = 0,
+	.l2c_aux_mask = 0,
+MACHINE_END
-- 
2.17.1

[PATCH v5 02/11] arch: arm: configs: multi_v7_defconfig

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add support for the HPE GXP Processor by modifing the
multi_v7_defconfig instead. This adds basic HPE and GXP
architecture as well as enables the watchdog which is part
of this patch set.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fix version log
v4:
* No change
v3:
* Put into proper patch format
* Modified multi_v7_defconfig instead of created gxp_defconfig
v2:
* Created gxp_defconfig
---
 arch/arm/configs/multi_v7_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 6e0c8c19b35c..d44988a708a1 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -1056,6 +1056,8 @@ CONFIG_QCOM_SOCINFO=m
 CONFIG_QCOM_STATS=m
 CONFIG_QCOM_WCNSS_CTRL=m
 CONFIG_ARCH_EMEV2=y
+CONFIG_ARCH_HPE=y
+CONFIG_ARCH_HPE_GXP=y
 CONFIG_ARCH_R8A7794=y
 CONFIG_ARCH_R8A7779=y
 CONFIG_ARCH_R8A7790=y
@@ -1228,3 +1230,4 @@ CONFIG_CMA_SIZE_MBYTES=64
 CONFIG_PRINTK_TIME=y
 CONFIG_MAGIC_SYSRQ=y
 CONFIG_DEBUG_FS=y
+CONFIG_GXP_WATCHDOG=y
-- 
2.17.1

[PATCH v5 03/11] drivers: wdt: Introduce HPE GXP SoC Watchdog

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Adding support for the HPE GXP Watchdog. It is new to the linux
community and this along with several other patches is the first
support for it. The GXP asic contains a full compliment of
timers one of which is the watchdog timer. The watchdog timer
is 16 bit and has 10ms resolution. The watchdog is
created as a child device of timer since the same register
range is used.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed version log
* Added details to Kconfig for module support.
* Adjusted commit messaged
v4:
* Made watchdog a child of timer as they share the same register
  region per change request on dtsi.
* Removed extra parenthesis
* Fixed u8 u32 u64 usage
* Fixed alignment issue
* Reconfigured conditional statement for interrupt setup
* Removed unused gxp_wdt_remove function
v3:
* Put into proper patchset format
v2:
* No change
---
 drivers/watchdog/Kconfig   |  11 +++
 drivers/watchdog/Makefile  |   1 +
 drivers/watchdog/gxp-wdt.c | 166 +++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 drivers/watchdog/gxp-wdt.c

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c4e82a8d863f..a591cc6aa152 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1820,6 +1820,17 @@ config RALINK_WDT
 	help
 	  Hardware driver for the Ralink SoC Watchdog Timer.
 
+config GXP_WATCHDOG
+	tristate "HPE GXP watchdog support"
+	depends on ARCH_HPE_GXP
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for the watchdog timer
+	  in HPE GXP SoCs.
+
+	  To compile this driver as a module, choose M here.
+	  The module will be called gxp-wdt.
+
 config MT7621_WDT
 	tristate "Mediatek SoC watchdog"
 	select WATCHDOG_CORE
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index f7da867e8782..e2acf3a0d0fc 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
 obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
 obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o
 obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o
+obj-$(CONFIG_GXP_WATCHDOG) += gxp-wdt.o
 obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o
 obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
 obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
diff --git a/drivers/watchdog/gxp-wdt.c b/drivers/watchdog/gxp-wdt.c
new file mode 100644
index 000000000000..f45ab9a826d6
--- /dev/null
+++ b/drivers/watchdog/gxp-wdt.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/types.h>
+#include <linux/watchdog.h>
+
+#define MASK_WDGCS_ENABLE	0x01
+#define MASK_WDGCS_RELOAD	0x04
+#define MASK_WDGCS_NMIEN	0x08
+#define MASK_WDGCS_WARN		0x80
+
+#define WDT_MAX_TIMEOUT_MS	655000
+#define WDT_DEFAULT_TIMEOUT	30
+#define SECS_TO_WDOG_TICKS(x) ((x) * 100)
+#define WDOG_TICKS_TO_SECS(x) ((x) / 100)
+
+#define GXP_WDT_CNT_OFS		0x10
+#define GXP_WDT_CTRL_OFS	0x16
+
+struct gxp_wdt {
+	void __iomem *base;
+	struct watchdog_device wdd;
+};
+
+static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata)
+{
+	u8 val;
+
+	val = readb(drvdata->base + GXP_WDT_CTRL_OFS);
+	val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD);
+	writeb(val, drvdata->base + GXP_WDT_CTRL_OFS);
+}
+
+static int gxp_wdt_start(struct watchdog_device *wdd)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+
+	writew(SECS_TO_WDOG_TICKS(wdd->timeout), drvdata->base + GXP_WDT_CNT_OFS);
+	gxp_wdt_enable_reload(drvdata);
+	return 0;
+}
+
+static int gxp_wdt_stop(struct watchdog_device *wdd)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+	u8 val;
+
+	val = readb_relaxed(drvdata->base + GXP_WDT_CTRL_OFS);
+	val &= ~MASK_WDGCS_ENABLE;
+	writeb(val, drvdata->base + GXP_WDT_CTRL_OFS);
+	return 0;
+}
+
+static int gxp_wdt_set_timeout(struct watchdog_device *wdd,
+			       unsigned int timeout)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+	u32 actual;
+
+	wdd->timeout = timeout;
+	actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
+	writew(SECS_TO_WDOG_TICKS(actual), drvdata->base + GXP_WDT_CNT_OFS);
+
+	return 0;
+}
+
+static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+	u32 val = readw(drvdata->base + GXP_WDT_CNT_OFS);
+
+	return WDOG_TICKS_TO_SECS(val);
+}
+
+static int gxp_wdt_ping(struct watchdog_device *wdd)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+
+	gxp_wdt_enable_reload(drvdata);
+	return 0;
+}
+
+static int gxp_restart(struct watchdog_device *wdd, unsigned long action,
+		       void *data)
+{
+	struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+
+	writew(10, drvdata->base + GXP_WDT_CNT_OFS);
+	gxp_wdt_enable_reload(drvdata);
+	mdelay(100);
+	return 0;
+}
+
+static const struct watchdog_ops gxp_wdt_ops = {
+	.owner =	THIS_MODULE,
+	.start =	gxp_wdt_start,
+	.stop =		gxp_wdt_stop,
+	.ping =		gxp_wdt_ping,
+	.set_timeout =	gxp_wdt_set_timeout,
+	.get_timeleft =	gxp_wdt_get_timeleft,
+	.restart =	gxp_restart,
+};
+
+static const struct watchdog_info gxp_wdt_info = {
+	.options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING,
+	.identity = "HPE GXP Watchdog timer",
+};
+
+static int gxp_wdt_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gxp_wdt *drvdata;
+	int err;
+	u8 val;
+
+	drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->base = (void __iomem *)dev->platform_data;
+
+	drvdata->wdd.info = &gxp_wdt_info;
+	drvdata->wdd.ops = &gxp_wdt_ops;
+	drvdata->wdd.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT_MS;
+	drvdata->wdd.parent = dev;
+	drvdata->wdd.timeout = WDT_DEFAULT_TIMEOUT;
+
+	watchdog_set_drvdata(&drvdata->wdd, drvdata);
+	watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT);
+
+	val = readb(drvdata->base + GXP_WDT_CTRL_OFS);
+
+	if (val & MASK_WDGCS_ENABLE)
+		set_bit(WDOG_HW_RUNNING, &drvdata->wdd.status);
+
+	watchdog_set_restart_priority(&drvdata->wdd, 128);
+
+	watchdog_stop_on_reboot(&drvdata->wdd);
+	err = devm_watchdog_register_device(dev, &drvdata->wdd);
+	if (err) {
+		dev_err(dev, "Failed to register watchdog device");
+		return err;
+	}
+
+	dev_info(dev, "HPE GXP watchdog timer");
+
+	return 0;
+}
+
+static struct platform_driver gxp_wdt_driver = {
+	.probe = gxp_wdt_probe,
+	.driver = {
+		.name =	"gxp-wdt",
+	},
+};
+module_platform_driver(gxp_wdt_driver);
+
+MODULE_AUTHOR("Nick Hawkins <nick.hawkins@hpe.com>");
+MODULE_AUTHOR("Jean-Marie Verdun <verdun@hpe.com>");
+MODULE_DESCRIPTION("Driver for GXP watchdog timer");
-- 
2.17.1

[PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add support for the HPE GXP SOC timer. The GXP supports several
different kinds of timers but for the purpose of this driver there
is only support for the General Timer. The timer has a 1us
resolution and is 32 bits. The timer also creates a child watchdog
device as the register region is the same based on previous review
feedback.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Corrected version log
* Removed uncessary include file
v4:
* Made watchdog a child of timer as they share the same register
  region
* Fixed watchdog init timeout call
* Fixed variable usage u32/u64
* Removed Read Once
* fixed error that should have been debug
v3:
* Put into proper patchset form
v2:
* No change
---
 drivers/clocksource/Kconfig     |   8 ++
 drivers/clocksource/Makefile    |   1 +
 drivers/clocksource/timer-gxp.c | 182 ++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)
 create mode 100644 drivers/clocksource/timer-gxp.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 1589ae7d5abb..110dd10b32f2 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -617,6 +617,14 @@ config CLKSRC_ST_LPC
 	  Enable this option to use the Low Power controller timer
 	  as clocksource.
 
+config GXP_TIMER
+	bool "GXP timer driver" if COMPILE_TEST
+	depends on ARCH_HPE
+	default y
+	help
+	  Provides a driver for the timer control found on HPE
+	  GXP SOCs. This is required for all GXP SOCs.
+
 config RISCV_TIMER
 	bool "Timer for the RISC-V platform" if COMPILE_TEST
 	depends on GENERIC_SCHED_CLOCK && RISCV && RISCV_SBI
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 9c85ee2bb373..98017abf6c03 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -88,3 +88,4 @@ obj-$(CONFIG_GX6605S_TIMER)		+= timer-gx6605s.o
 obj-$(CONFIG_HYPERV_TIMER)		+= hyperv_timer.o
 obj-$(CONFIG_MICROCHIP_PIT64B)		+= timer-microchip-pit64b.o
 obj-$(CONFIG_MSC313E_TIMER)		+= timer-msc313e.o
+obj-$(CONFIG_GXP_TIMER)			+= timer-gxp.o
diff --git a/drivers/clocksource/timer-gxp.c b/drivers/clocksource/timer-gxp.c
new file mode 100644
index 000000000000..c9290fb8131c
--- /dev/null
+++ b/drivers/clocksource/timer-gxp.c
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/sched_clock.h>
+
+#define TIMER0_FREQ	1000000
+#define GXP_TIMER_CNT_OFS 0x00
+#define GXP_TIMESTAMP_OFS 0x08
+#define GXP_TIMER_CTRL_OFS 0x14
+
+/*TCS Stands for Timer Control/Status: these are masks to be used in*/
+/* the Timer Count Registers */
+#define MASK_TCS_ENABLE	0x01
+#define MASK_TCS_PERIOD	0x02
+#define MASK_TCS_RELOAD	0x04
+#define MASK_TCS_TC	0x80
+
+struct gxp_timer {
+	void __iomem *counter;
+	void __iomem *control;
+	struct clock_event_device evt;
+};
+
+static struct gxp_timer *local_gxp_timer;
+
+static void __iomem *system_clock __read_mostly;
+
+static inline struct gxp_timer *to_gxp_timer(struct clock_event_device *evt_dev)
+{
+	return container_of(evt_dev, struct gxp_timer, evt);
+}
+
+static u64 notrace gxp_sched_read(void)
+{
+	return readl_relaxed(system_clock);
+}
+
+static int gxp_time_set_next_event(unsigned long event,	struct clock_event_device *evt_dev)
+{
+	struct gxp_timer *timer = to_gxp_timer(evt_dev);
+
+	/* Stop counting and disable interrupt before updating */
+	writeb_relaxed(MASK_TCS_TC, timer->control);
+	writel_relaxed(event, timer->counter);
+	writeb_relaxed(MASK_TCS_TC | MASK_TCS_ENABLE, timer->control);
+
+	return 0;
+}
+
+static irqreturn_t gxp_timer_interrupt(int irq, void *dev_id)
+{
+	struct gxp_timer *timer = (struct gxp_timer *)dev_id;
+
+	if (!(readb_relaxed(timer->control) & MASK_TCS_TC))
+		return IRQ_NONE;
+
+	writeb_relaxed(MASK_TCS_TC, timer->control);
+
+	timer->evt.event_handler(&timer->evt);
+
+	return IRQ_HANDLED;
+}
+
+static int __init gxp_timer_init(struct device_node *node)
+{
+	void __iomem *base;
+	struct clk *clk;
+	u32 freq;
+	int ret, irq;
+	struct gxp_timer *gxp_timer;
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("Can't remap timer base register");
+		ret = -ENXIO;
+		return ret;
+	}
+
+	/*Set the offset to the clock register*/
+	system_clock = base + GXP_TIMESTAMP_OFS;
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		pr_err("%pOFn clock not found: %d\n", node, (int)PTR_ERR(clk));
+		ret = -EIO;
+		goto err_iounmap;
+	}
+
+	ret = clk_prepare_enable(clk);
+
+	freq = clk_get_rate(clk);
+
+	sched_clock_register(gxp_sched_read, 32, freq);
+	clocksource_mmio_init(system_clock, node->name, freq,
+			      300, 32, clocksource_mmio_readl_up);
+
+	irq = irq_of_parse_and_map(node, 0);
+	if (irq <= 0) {
+		ret = -EINVAL;
+		pr_err("GXP Timer Can't parse IRQ %d", irq);
+		goto err_iounmap;
+	}
+
+	gxp_timer = kzalloc(sizeof(*gxp_timer), GFP_KERNEL);
+	if (!gxp_timer) {
+		ret = -ENOMEM;
+		goto err_iounmap;
+	}
+
+	gxp_timer->counter = base + GXP_TIMER_CNT_OFS;
+	gxp_timer->control = base + GXP_TIMER_CTRL_OFS;
+	gxp_timer->evt.name = node->name;
+	gxp_timer->evt.rating = 300;
+	gxp_timer->evt.features = CLOCK_EVT_FEAT_ONESHOT;
+	gxp_timer->evt.set_next_event = gxp_time_set_next_event;
+	gxp_timer->evt.cpumask = cpumask_of(0);
+
+	local_gxp_timer = gxp_timer;
+
+	ret = request_irq(irq, gxp_timer_interrupt, IRQF_TIMER | IRQF_SHARED,
+			  node->name, gxp_timer);
+	if (ret) {
+		pr_err("%s: request_irq() failed %pe\n", "GXP Timer Tick", ERR_PTR(ret));
+		goto err_iounmap;
+	}
+
+	clockevents_config_and_register(&gxp_timer->evt, TIMER0_FREQ,
+					0xf, 0xffffffff);
+
+	pr_debug("gxp: system timer (irq = %d)\n", irq);
+	return 0;
+
+err_iounmap:
+	iounmap(system_clock);
+	iounmap(base);
+	return ret;
+}
+
+static struct platform_device gxp_watchdog_device = {
+	.name = "gxp-wdt",
+	.id = -1,
+};
+
+/*
+ * This probe gets called after the timer is already up and running. This will create
+ * the watchdog device as a child since the registers are shared.
+ */
+
+static int gxp_timer_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+
+	/* Pass the base address (counter) as platform data and nothing else */
+	gxp_watchdog_device.dev.platform_data = local_gxp_timer->counter;
+	gxp_watchdog_device.dev.parent = dev;
+	return platform_device_register(&gxp_watchdog_device);
+}
+
+static const struct of_device_id gxp_timer_of_match[] = {
+	{ .compatible = "hpe,gxp-timer", },
+	{},
+};
+
+static struct platform_driver gxp_timer_driver = {
+	.probe  = gxp_timer_probe,
+	.driver = {
+		.name = "gxp-timer",
+		.of_match_table = gxp_timer_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+
+builtin_platform_driver(gxp_timer_driver);
+
+TIMER_OF_DECLARE(gxp, "hpe,gxp-timer", gxp_timer_init);
-- 
2.17.1

[PATCH v5 05/11] dt-bindings: timer: Add HPE GXP Timer Binding

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Creating binding for gxp timer in device tree hpe,gxp-timer
Although there are multiple timers on the SoC we are only
enabling one at this time.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fix versioning
* Fixed typo time -> timer
v4:
* Made watchdog a child of timer
* Added reference clock
v3:
* Removed maintainer change from patch
* Verified there was no compilation errors
* Added reference code in separate patch of patchset
v2:
* Converted from txt to yaml
---
 .../bindings/timer/hpe,gxp-timer.yaml         | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
new file mode 100644
index 000000000000..a4572be8d89a
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/hpe,gxp-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP TIMER
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    items:
+      - const: hpe,gxp-timer
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: iopclk
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: true
+
+examples:
+  - |
+    timer0: timer@c0000000 {
+        compatible = "hpe,gxp-timer","simple-mfd";
+        reg = <0x80 0x16>;
+        interrupts = <0>;
+        interrupt-parent = <&vic0>;
+        clocks = <&iopclk>;
+        clock-names = "iopclk";
+    };
-- 
2.17.1

[PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add the hpe gxp watchdog timer binding hpe,gxp-wdt.
This will enable support for the HPE GXP Watchdog.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed version log
v4:
* Made watchdog a child of timer because of same register
  area based on review feedback
* Simplified the watchdog yaml as it will get information
  from parent device
v3:
* Used proper patchset format.
v2:
* Converted from txt to yaml
---
 .../bindings/watchdog/hpe,gxp-wdt.yaml        | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
new file mode 100644
index 000000000000..c20da146352f
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/hpe,gxp-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE GXP Controlled Watchdog
+
+allOf:
+  - $ref: "watchdog.yaml#"
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    const: hpe,gxp-wdt
+
+required:
+  - compatible
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    watchdog0:  watchdog {
+      compatible = "hpe,gxp-wdt";
+    };
+
-- 
2.17.1

[PATCH v5 07/11] dt-bindings: arm: Add HPE GXP Binding

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

This adds support for the hpe,gxp binding. The GXP is based on
the cortex a9 processor and supports arm7.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fix version log
v4:
* Removed gxp.yaml
* Created hpe,gxp.yaml based on reviewer input
v3:
* Created gxp.yaml
v2:
* No change
---
 .../devicetree/bindings/arm/hpe,gxp.yaml      | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/hpe,gxp.yaml

diff --git a/Documentation/devicetree/bindings/arm/hpe,gxp.yaml b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
new file mode 100644
index 000000000000..cd86b67ea207
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/hpe,gxp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE BMC GXP SoC driver
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - hpe,gxp-dl360gen10
+      - const: hpe,gxp
+
+additionalProperties: true
+
+...
-- 
2.17.1

[PATCH v5 08/11] dt-bindings: usb: generic-ehci: Add HPE GXP ehci binding

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add hpe,gxp-ehci to the generic-ehci list. This is to
enable the device tree.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed previous change log
* Fixed typo with echi -> ehci
v4:
* Based on previous feedback the hpe,gxp-ehci has been
  added to the list of devices
v3:
* No change
v2:
* No change
---
 Documentation/devicetree/bindings/usb/generic-ehci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
index 8913497624de..0b4524b6409e 100644
--- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
@@ -55,6 +55,7 @@ properties:
               - brcm,bcm7420-ehci
               - brcm,bcm7425-ehci
               - brcm,bcm7435-ehci
+              - hpe,gxp-ehci
               - ibm,476gtr-ehci
               - nxp,lpc1850-ehci
               - qca,ar7100-ehci
-- 
2.17.1

[PATCH v5 09/11] dt-bindings: usb: generic-ohci: Add HPE GXP ohci binding

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Add hpe,gxp-ohci to the generic-ohci list. This is to
enable the device tree support.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed version log
* Fixed typo ochi -> ohci
v4:
* Based on previous feedback the hpe,gxp-ohci is added
  to the list
v3:
* No change
v2:
* No change
---
 Documentation/devicetree/bindings/usb/generic-ohci.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
index acbf94fa5f74..e2ac84665316 100644
--- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
@@ -42,6 +42,7 @@ properties:
               - brcm,bcm7420-ohci
               - brcm,bcm7425-ohci
               - brcm,bcm7435-ohci
+              - hpe,gxp-ohci
               - ibm,476gtr-ohci
               - ingenic,jz4740-ohci
               - snps,hsdk-v1.0-ohci
-- 
2.17.1

[PATCH v5 10/11] arch: arm: boot: dts: Introduce HPE GXP Device tree

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

The HPE SoC is new to linux. This patch
creates the basic device tree layout with minimum required
for linux to boot. This includes timer and watchdog
support.

The dts file is empty at this point but will be
updated in subsequent updates as board specific features
are enabled.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed commit message to show previous changes
* Fixed typo ehci -> echi
v4:
* Removed hpe,gxp-cpu-init as it was no longer necessary
* Removed bootargs as requested
* Removed empty ahb node
* Moved reg after compatible, everywhere
* Removed osc and memclk
* Removed syscon@c00000f8 as it was not necessary for boot
* Fixed Alphabetical issue in dts/Makefile
* Added specific board binding for dl360gen10
* Removed empty node
* Added Accurate Clock Architecture
* Fixed generic-echi and generic-ochi issues
* Removed i2cg
v3:
* Fixed issues with warnings
* Used proper patchset format
v2:
* Reduced size of dtsi to essential components
* Followed the proper format for having a dtsi and
  dts
---
 arch/arm/boot/dts/Makefile               |   2 +
 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts |  13 +++
 arch/arm/boot/dts/hpe-gxp.dtsi           | 128 +++++++++++++++++++++++
 3 files changed, 143 insertions(+)
 create mode 100644 arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
 create mode 100644 arch/arm/boot/dts/hpe-gxp.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7c16f8a2b738..293717719c70 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -255,6 +255,8 @@ dtb-$(CONFIG_ARCH_HISI) += \
 	hi3519-demb.dtb
 dtb-$(CONFIG_ARCH_HIX5HD2) += \
 	hisi-x5hd2-dkb.dtb
+dtb-$(CONFIG_ARCH_HPE_GXP) += \
+	hpe-bmc-dl360gen10.dtb
 dtb-$(CONFIG_ARCH_INTEGRATOR) += \
 	integratorap.dtb \
 	integratorap-im-pd1.dtb \
diff --git a/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
new file mode 100644
index 000000000000..69e9c6672ea8
--- /dev/null
+++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree file for HPE DL360Gen10
+ */
+
+/include/ "hpe-gxp.dtsi"
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	compatible = "hpe,gxp-dl360gen10","hpe,gxp";
+	model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
+};
diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
new file mode 100644
index 000000000000..a3a082d21101
--- /dev/null
+++ b/arch/arm/boot/dts/hpe-gxp.dtsi
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device Tree file for HPE GXP
+ */
+
+/dts-v1/;
+/ {
+	model = "Hewlett Packard Enterprise GXP BMC";
+	compatible = "hpe,gxp","hpe,gxp-dl360gen10";
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			device_type = "cpu";
+		};
+	};
+
+	clocks {
+
+		pll: pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <1600000000>;
+		};
+
+		iopclk: iopclk {
+			compatible = "fixed-factor-clock";
+			#clock-cells = <0>;
+			clock-div = <4>;
+			clock-mult = <1>;
+			clocks = <&pll>;
+		};
+	};
+
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x40000000 0x20000000>;
+	};
+
+	axi {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		dma-ranges;
+
+		ahb@c0000000 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0xc0000000 0x30000000>;
+
+			vic0: interrupt-controller@eff0000 {
+				compatible = "arm,pl192-vic";
+				reg = <0xeff0000 0x1000>;
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+
+			vic1: interrupt-controller@80f00000 {
+				compatible = "arm,pl192-vic";
+				reg = <0x80f00000 0x1000>;
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+
+			uarta: serial@e0 {
+				compatible = "ns16550a";
+				reg = <0xe0 0x8>;
+				interrupts = <17>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			uartb: serial@e8 {
+				compatible = "ns16550a";
+				reg = <0xe8 0x8>;
+				interrupts = <18>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			uartc: serial@f0 {
+				compatible = "ns16550a";
+				reg = <0xf0 0x8>;
+				interrupts = <19>;
+				interrupt-parent = <&vic0>;
+				clock-frequency = <1846153>;
+				reg-shift = <0>;
+			};
+
+			usb0: usb@efe0000 {
+				compatible = "hpe,gxp-ehci","generic-ehci";
+				reg = <0xefe0000 0x100>;
+				interrupts = <7>;
+				interrupt-parent = <&vic0>;
+			};
+
+			st: timer@80 {
+				compatible = "hpe,gxp-timer","simple-mfd";
+				reg = <0x80 0x16>;
+				interrupts = <0>;
+				interrupt-parent = <&vic0>;
+				clocks = <&iopclk>;
+				clock-names = "iopclk";
+				watchdog {
+					compatible = "hpe,gxp-wdt";
+				};
+			};
+
+			usb1: usb@efe0100 {
+				compatible = "hpe,gxp-ohci","generic-ohci";
+				reg = <0xefe0100 0x110>;
+				interrupts = <6>;
+				interrupt-parent = <&vic0>;
+			};
+		};
+	};
+
+};
-- 
2.17.1

[PATCH v5 11/11] maintainers: Introduce HPE GXP Architecture

[nick.hawkins]

From: Nick Hawkins <nick.hawkins@hpe.com>

Create a section in MAINTAINERS for the GXP HPE
architecture.

Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

---
v5:
* Fixed commit message to list all previous changes
v4:
* Added ARM/ before HPE Title
* Changed MAINTAINED to Maintained
* Renamed gxp-timer.c to timer-gxp.c
* Renamed gxp.yaml to hpe,gxp.yaml

v3:
* Removed uncessary files
* Used proper patch-set format

v2:
* Fixed email address
* Removed multiple entries in favor of one
---
 MAINTAINERS | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 40fa1955ca3f..b3fdedc07791 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2130,6 +2130,19 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kristoffer/linux-hpc.git
 F:	arch/arm/mach-sa1100/include/mach/jornada720.h
 F:	arch/arm/mach-sa1100/jornada720.c
 
+ARM/HPE GXP ARCHITECTURE
+M:	Jean-Marie Verdun <verdun@hpe.com>
+M:	Nick Hawkins <nick.hawkins@hpe.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/hpe,gxp.yaml
+F:	Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
+F:	Documentation/devicetree/bindings/wdt/hpe,gxp-wdt.yaml
+F:	arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
+F:	arch/arm/boot/dts/hpe-gxp.dtsi
+F:	arch/arm/mach-hpe/gxp.c
+F:	drivers/clocksource/timer-gxp.c
+F:	drivers/watchdog/gxp-wdt.c
+
 ARM/IGEP MACHINE SUPPORT
 M:	Enric Balletbo i Serra <eballetbo@gmail.com>
 M:	Javier Martinez Canillas <javier@dowhile0.org>
-- 
2.17.1

Re: [PATCH v5 10/11] arch: arm: boot: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:

> +       axi {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +               dma-ranges;
> +
> +               ahb@c0000000 {
> +                       compatible = "simple-bus";
> +                       #address-cells = <1>;
> +                       #size-cells = <1>;
> +                       ranges = <0x0 0xc0000000 0x30000000>;

I think you should add a "dma-ranges" property in the ahb node as well,
otherwise this would mean that none of the children are DMA capable,
when at the minimum the USB controllers require DMA.

       Arnd

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Arnd Bergmann]

On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:

> +
> +static struct platform_device gxp_watchdog_device = {
> +       .name = "gxp-wdt",
> +       .id = -1,
> +};
> +/*
> + * This probe gets called after the timer is already up and running. This will create
> + * the watchdog device as a child since the registers are shared.
> + */
> +
> +static int gxp_timer_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +
> +       /* Pass the base address (counter) as platform data and nothing else */
> +       gxp_watchdog_device.dev.platform_data = local_gxp_timer->counter;
> +       gxp_watchdog_device.dev.parent = dev;
> +       return platform_device_register(&gxp_watchdog_device);
> +}

I don't understand what this is about: the device should be created from
DT, not defined statically in the code. There are multiple ways of creating
a platform_device from a DT node, or you can allocate one here, but static
definitions are generally a mistake.

I see that you copied this from the ixp4xx driver, so I think we should fix this
there as well.

      Arnd

Re: [PATCH v5 01/11] archh: arm: mach-hpe: Introduce the HPE GXP architecture

[Arnd Bergmann]

On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:
> +
> +DT_MACHINE_START(GXP_DT, "HPE GXP")
> +       .dt_compat      = gxp_board_dt_compat,
> +       .l2c_aux_val = 0,
> +       .l2c_aux_mask = 0,
> +MACHINE_END

Ther l2c initialization looks wrong here, where  you are saying here is
that all the bits of the aux register must be set to zero.

I also see that you don't have a device node for the cache controller, so
this is probably not actually used, but in general there should be one with
the correct properties.

        Arnd

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Thomas Gleixner]

On Thu, Apr 21 2022 at 14:21, nick hawkins wrote:
> +
> +static struct gxp_timer *local_gxp_timer;

What is local about that time?

> +static void __iomem *system_clock __read_mostly;

__ro_after_init perhaps?

> +static inline struct gxp_timer *to_gxp_timer(struct clock_event_device *evt_dev)
> +{
> +	return container_of(evt_dev, struct gxp_timer, evt);
> +}
> +
> +static u64 notrace gxp_sched_read(void)
> +{
> +	return readl_relaxed(system_clock);
> +}
> +
> +static int gxp_time_set_next_event(unsigned long event,	struct clock_event_device *evt_dev)

Stray TAB in arguments

> +static int __init gxp_timer_init(struct device_node *node)
> +{
> +	void __iomem *base;
> +	struct clk *clk;
> +	u32 freq;
> +	int ret, irq;
> +	struct gxp_timer *gxp_timer;
> +
> +	base = of_iomap(node, 0);
> +	if (!base) {
> +		pr_err("Can't remap timer base register");
> +		ret = -ENXIO;
> +		return ret;
> +	}
> +
> +	/*Set the offset to the clock register*/
> +	system_clock = base + GXP_TIMESTAMP_OFS;
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("%pOFn clock not found: %d\n", node, (int)PTR_ERR(clk));
> +		ret = -EIO;
> +		goto err_iounmap;
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +
> +	freq = clk_get_rate(clk);
> +
> +	sched_clock_register(gxp_sched_read, 32, freq);
> +	clocksource_mmio_init(system_clock, node->name, freq,
> +			      300, 32, clocksource_mmio_readl_up);

So this registers sched clock and clocksource and if one of the
following fails....

> +	irq = irq_of_parse_and_map(node, 0);
> +	if (irq <= 0) {
> +		ret = -EINVAL;
> +		pr_err("GXP Timer Can't parse IRQ %d", irq);
> +		goto err_iounmap;

... the error path unmaps 'system_clock'

> +	}
> +
> +	gxp_timer = kzalloc(sizeof(*gxp_timer), GFP_KERNEL);
> +	if (!gxp_timer) {
> +		ret = -ENOMEM;
> +		goto err_iounmap;
> +	}
> +
> +	gxp_timer->counter = base + GXP_TIMER_CNT_OFS;
> +	gxp_timer->control = base + GXP_TIMER_CTRL_OFS;
> +	gxp_timer->evt.name = node->name;
> +	gxp_timer->evt.rating = 300;
> +	gxp_timer->evt.features = CLOCK_EVT_FEAT_ONESHOT;
> +	gxp_timer->evt.set_next_event = gxp_time_set_next_event;
> +	gxp_timer->evt.cpumask = cpumask_of(0);
> +
> +	local_gxp_timer = gxp_timer;
> +
> +	ret = request_irq(irq, gxp_timer_interrupt, IRQF_TIMER | IRQF_SHARED,
> +			  node->name, gxp_timer);
> +	if (ret) {
> +		pr_err("%s: request_irq() failed %pe\n", "GXP Timer Tick", ERR_PTR(ret));
> +		goto err_iounmap;

... and here as a bonus leaks gxp_timer.

> +	}
> +
> +	clockevents_config_and_register(&gxp_timer->evt, TIMER0_FREQ,
> +					0xf, 0xffffffff);
> +
> +	pr_debug("gxp: system timer (irq = %d)\n", irq);
> +	return 0;
> +
> +err_iounmap:
> +	iounmap(system_clock);
> +	iounmap(base);
> +	return ret;
> +}

Thanks,

        tglx

Re: [PATCH v5 05/11] dt-bindings: timer: Add HPE GXP Timer Binding

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> diff --git a/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
> new file mode 100644
> index 000000000000..a4572be8d89a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/hpe,gxp-timer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP TIMER

s/TIMER/Timer/

> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +  - Jean-Marie Verdun <verdun@hpe.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: hpe,gxp-timer
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: iopclk

s/iopclk/iop/

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: true

This has to be false and you need to define the children (since you use
simple-mfd).

> +
> +examples:
> +  - |
> +    timer0: timer@c0000000 {
> +        compatible = "hpe,gxp-timer","simple-mfd";

Missing space after ','.

> +        reg = <0x80 0x16>;
> +        interrupts = <0>;
> +        interrupt-parent = <&vic0>;
> +        clocks = <&iopclk>;
> +        clock-names = "iopclk";
> +    };


Best regards,
Krzysztof

Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> ---
>  .../bindings/watchdog/hpe,gxp-wdt.yaml        | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> new file mode 100644
> index 000000000000..c20da146352f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/hpe,gxp-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP Controlled Watchdog
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"

allOf goes after maintainers, before properties.

> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +  - Jean-Marie Verdun <verdun@hpe.com>
> +
> +properties:
> +  compatible:
> +    const: hpe,gxp-wdt
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    watchdog0:  watchdog {

Doubled space.

> +      compatible = "hpe,gxp-wdt";
> +    };
> +


Best regards,
Krzysztof

Re: [PATCH v5 08/11] dt-bindings: usb: generic-ehci: Add HPE GXP ehci binding

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add hpe,gxp-ehci to the generic-ehci list. This is to
> enable the device tree.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v5 09/11] dt-bindings: usb: generic-ohci: Add HPE GXP ohci binding

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add hpe,gxp-ohci to the generic-ohci list. This is to
> enable the device tree support.

The last sentence is not needed, it's kind of obvious.


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v5 07/11] dt-bindings: arm: Add HPE GXP Binding

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> This adds support for the hpe,gxp binding.

Just "Add support for HPE GXP".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

>  The GXP is based on
> the cortex a9 processor and supports arm7.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v5:
> * Fix version log
> v4:
> * Removed gxp.yaml
> * Created hpe,gxp.yaml based on reviewer input
> v3:
> * Created gxp.yaml
> v2:
> * No change
> ---
>  .../devicetree/bindings/arm/hpe,gxp.yaml      | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/hpe,gxp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/hpe,gxp.yaml b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
> new file mode 100644
> index 000000000000..cd86b67ea207
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/hpe,gxp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE BMC GXP SoC driver

This is not a SoC driver anymore, so instead maybe:
"HPE BMC GXP SoC platforms"
or
"HPE BMC GXP platforms"


> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +  - Jean-Marie Verdun <verdun@hpe.com>
> +
> +properties:
> +  compatible:
You do not allow any extension of this (no oneOf), which is fine if you
do not plan any other SoCs or SoMs. This is okay, but if there is a
chance list will grow, then you should have here oneOf, like other bindings.

> +    items:
> +      - enum:
> +          - hpe,gxp-dl360gen10
> +      - const: hpe,gxp
> +
> +additionalProperties: true
> +
> +...


Best regards,
Krzysztof

Re: [PATCH v5 10/11] arch: arm: boot: dts: Introduce HPE GXP Device tree

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 

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

> +/include/ "hpe-gxp.dtsi"
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	compatible = "hpe,gxp-dl360gen10","hpe,gxp";

Missing space after ','.

> +	model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
> +};
> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> new file mode 100644
> index 000000000000..a3a082d21101
> --- /dev/null
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree file for HPE GXP
> + */
> +
> +/dts-v1/;
> +/ {
> +	model = "Hewlett Packard Enterprise GXP BMC";
> +	compatible = "hpe,gxp","hpe,gxp-dl360gen10";

The same.

> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,cortex-a9";
> +			reg = <0>;
> +			device_type = "cpu";
> +		};
> +	};
> +
> +	clocks {
> +

No need for blank line.

> +		pll: pll {

Generic node names, so either "clock-0" or "pll-clock"

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <1600000000>;
> +		};
> +
> +		iopclk: iopclk {

"clock-1" or "iop-clock"

> +			compatible = "fixed-factor-clock";
> +			#clock-cells = <0>;
> +			clock-div = <4>;
> +			clock-mult = <1>;
> +			clocks = <&pll>;
> +		};
> +	};

(...)

> +
> +			usb0: usb@efe0000 {
> +				compatible = "hpe,gxp-ehci","generic-ehci";

Here and in other places - always missing a space.

> +				reg = <0xefe0000 0x100>;
> +				interrupts = <7>;
> +				interrupt-parent = <&vic0>;
> +			};
> +
> +			st: timer@80 {
> +				compatible = "hpe,gxp-timer","simple-mfd";
> +				reg = <0x80 0x16>;
> +				interrupts = <0>;
> +				interrupt-parent = <&vic0>;
> +				clocks = <&iopclk>;
> +				clock-names = "iopclk";
> +				watchdog {
> +					compatible = "hpe,gxp-wdt";
> +				};
> +			};
> +
> +			usb1: usb@efe0100 {
> +				compatible = "hpe,gxp-ohci","generic-ohci";
> +				reg = <0xefe0100 0x110>;
> +				interrupts = <6>;
> +				interrupt-parent = <&vic0>;
> +			};
> +		};
> +	};
> +

No need for blank line.

> +};


Best regards,
Krzysztof

Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The GXP is the HPE BMC SoC that is used in the majority
> of HPE Generation 10 servers. Traditionally the asic will
> last multiple generations of server before being replaced.
> In gxp.c we reset the EHCI controller early to boot the asic.
> 
> Info about SoC:

Half of your patches did not make it to the lists. For example
linux-arm-kernel has only 1, 2 and 10.

Where is the rest? All your patches must be sent to linux-arm-kernel and
linux-kernel. Unless the list of people To/Cc is too big (~ 10 people),
entire patchset should be send to same addresses.


Best regards,
Krzysztof

Re: [PATCH v5 02/11] arch: arm: configs: multi_v7_defconfig

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> @@ -1228,3 +1230,4 @@ CONFIG_CMA_SIZE_MBYTES=64
>  CONFIG_PRINTK_TIME=y
>  CONFIG_MAGIC_SYSRQ=y
>  CONFIG_DEBUG_FS=y
> +CONFIG_GXP_WATCHDOG=y

This does not look like in correct place. You need to add entries how
the savedefconfig would do it.


Best regards,
Krzysztof

Re: [PATCH v5 02/11] arch: arm: configs: multi_v7_defconfig

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the HPE GXP Processor by modifing the

s/modifing/modifying/ ?

> multi_v7_defconfig instead. This adds basic HPE and GXP
> architecture as well as enables the watchdog which is part
> of this patch set.

The commit lands in the Git and there is no relation to "patchset", so just:

"Enable HPE GXP Architecture and its watchdog for basic support for HPE
GXP SoCs."

Your subject also does not look correct at all. Please use "git log
--oneline -- "  to get the idea for correct title.

Best regards,
Krzysztof

RE: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Hawkins, Nick]

-----Original Message-----
From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] 
Sent: Saturday, April 23, 2022 6:05 AM
To: Hawkins, Nick <nick.hawkins@hpe.com>; Verdun, Jean-Marie <verdun@hpe.com>; joel@jms.id.au; arnd@arndb.de; openbmc@lists.ozlabs.org
Cc: Russell King <linux@armlinux.org.uk>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> > From: Nick Hawkins <nick.hawkins@hpe.com>
> > 
> > The GXP is the HPE BMC SoC that is used in the majority of HPE 
> > Generation 10 servers. Traditionally the asic will last multiple 
> > generations of server before being replaced.
> > In gxp.c we reset the EHCI controller early to boot the asic.
> >
> > Info about SoC:

> Half of your patches did not make it to the lists. For example linux-arm-kernel has only 1, 2 and 10.

> Where is the rest? All your patches must be sent to linux-arm-kernel and linux-kernel. Unless the list of people To/Cc is too big (~ 10 people), entire patchset should be send to same addresses.

Apologies, I was relying on the ".scripts/get_maintainer.pl" in a script to grab and send emails. I will ensure on the next patchset that linux-arm-kernel and linux-kernel are included in all of the emails.

Thanks for the feedback,

-Nick Hawkins

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Linus Walleij]

On Fri, Apr 22, 2022 at 3:16 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:
>
> > +
> > +static struct platform_device gxp_watchdog_device = {
> > +       .name = "gxp-wdt",
> > +       .id = -1,
> > +};
> > +/*
> > + * This probe gets called after the timer is already up and running. This will create
> > + * the watchdog device as a child since the registers are shared.
> > + */
> > +
> > +static int gxp_timer_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +
> > +       /* Pass the base address (counter) as platform data and nothing else */
> > +       gxp_watchdog_device.dev.platform_data = local_gxp_timer->counter;
> > +       gxp_watchdog_device.dev.parent = dev;
> > +       return platform_device_register(&gxp_watchdog_device);
> > +}
>
> I don't understand what this is about: the device should be created from
> DT, not defined statically in the code. There are multiple ways of creating
> a platform_device from a DT node, or you can allocate one here, but static
> definitions are generally a mistake.
>
> I see that you copied this from the ixp4xx driver, so I think we should fix this
> there as well.

The ixp4xx driver looks like that because the register range used for
the timer and the watchdog is combined, i.e. it is a single IP block:

                timer@c8005000 {
                        compatible = "intel,ixp4xx-timer";
                        reg = <0xc8005000 0x100>;
                        interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
                };

Device tree probing does not allow two devices to probe from the same
DT node, so this was solved by letting the (less important) watchdog
be spawn as a platform device from the timer.

I don't know if double-probing for the same register range can be fixed,
but I was assuming that the one-compatible-to-one-driver assumption
was pretty hard-coded into the abstractions. Maybe it isn't?

Another way is of course to introduce an MFD. That becomes
problematic in another way: MFD abstractions are supposed to
be inbetween the resource and the devices it spawns, and with
timers/clocksources this creates a horrible special-casing since the
MFD bus (the parent may be providing e.g. an MMIO regmap)
then need to be early-populated and searched by the timer core
from TIMER_OF_DECLARE() early in boot.

So this solution was the lesser evil that I could think about.

Yours,
Linus Walleij

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Jonathan Neuschäfer]

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

On Mon, Apr 25, 2022 at 10:38:08PM +0200, Linus Walleij wrote:
> On Fri, Apr 22, 2022 at 3:16 PM Arnd Bergmann <arnd@arndb.de> wrote:
[...]
> The ixp4xx driver looks like that because the register range used for
> the timer and the watchdog is combined, i.e. it is a single IP block:
> 
>                 timer@c8005000 {
>                         compatible = "intel,ixp4xx-timer";
>                         reg = <0xc8005000 0x100>;
>                         interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>                 };
> 
> Device tree probing does not allow two devices to probe from the same
> DT node, so this was solved by letting the (less important) watchdog
> be spawn as a platform device from the timer.
> 
> I don't know if double-probing for the same register range can be fixed,
> but I was assuming that the one-compatible-to-one-driver assumption
> was pretty hard-coded into the abstractions. Maybe it isn't?
> 
> Another way is of course to introduce an MFD. That becomes
> problematic in another way: MFD abstractions are supposed to
> be inbetween the resource and the devices it spawns, and with
> timers/clocksources this creates a horrible special-casing since the
> MFD bus (the parent may be providing e.g. an MMIO regmap)
> then need to be early-populated and searched by the timer core
> from TIMER_OF_DECLARE() early in boot.
> 
> So this solution was the lesser evil that I could think about.

Nuvoton NPCM platforms use yet another approach:

	timer0: timer@8000 {
		compatible = "nuvoton,npcm750-timer";
		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
		reg = <0x8000 0x1C>;
		clocks = <&clk NPCM7XX_CLK_TIMER>;
	};

	watchdog0: watchdog@801C {
		compatible = "nuvoton,npcm750-wdt";
		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
		reg = <0x801C 0x4>;
		status = "disabled";
		clocks = <&clk NPCM7XX_CLK_TIMER>;
	};


The watchdog control register is in the same register block, but
represented by a separate DT node.

(not necessarily a recommendation, but it is another existing approach.)


Jonathan

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

Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Rob Herring]

On Thu, Apr 21, 2022 at 02:21:27PM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add the hpe gxp watchdog timer binding hpe,gxp-wdt.
> This will enable support for the HPE GXP Watchdog.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v5:
> * Fixed version log
> v4:
> * Made watchdog a child of timer because of same register
>   area based on review feedback
> * Simplified the watchdog yaml as it will get information
>   from parent device
> v3:
> * Used proper patchset format.
> v2:
> * Converted from txt to yaml
> ---
>  .../bindings/watchdog/hpe,gxp-wdt.yaml        | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> new file mode 100644
> index 000000000000..c20da146352f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/hpe,gxp-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP Controlled Watchdog
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"
> +
> +maintainers:
> +  - Nick Hawkins <nick.hawkins@hpe.com>
> +  - Jean-Marie Verdun <verdun@hpe.com>
> +
> +properties:
> +  compatible:
> +    const: hpe,gxp-wdt
> +
> +required:
> +  - compatible
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    watchdog0:  watchdog {
> +      compatible = "hpe,gxp-wdt";

How is this h/w controlled? I'm guessing it's part of the timer? If so, 
you don't need this node. A single node can implement multiple 
functions.

Rob

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Arnd Bergmann]

On Mon, Apr 25, 2022 at 10:38 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Apr 22, 2022 at 3:16 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:
> >
> > > +
> > > +static struct platform_device gxp_watchdog_device = {
> > > +       .name = "gxp-wdt",
> > > +       .id = -1,
> > > +};
> > > +/*
> > > + * This probe gets called after the timer is already up and running. This will create
> > > + * the watchdog device as a child since the registers are shared.
> > > + */
> > > +
> > > +static int gxp_timer_probe(struct platform_device *pdev)
> > > +{
> > > +       struct device *dev = &pdev->dev;
> > > +
> > > +       /* Pass the base address (counter) as platform data and nothing else */
> > > +       gxp_watchdog_device.dev.platform_data = local_gxp_timer->counter;
> > > +       gxp_watchdog_device.dev.parent = dev;
> > > +       return platform_device_register(&gxp_watchdog_device);
> > > +}
> >
> > I don't understand what this is about: the device should be created from
> > DT, not defined statically in the code. There are multiple ways of creating
> > a platform_device from a DT node, or you can allocate one here, but static
> > definitions are generally a mistake.
> >
> > I see that you copied this from the ixp4xx driver, so I think we should fix this
> > there as well.
>
> The ixp4xx driver looks like that because the register range used for
> the timer and the watchdog is combined, i.e. it is a single IP block:
>
>                 timer@c8005000 {
>                         compatible = "intel,ixp4xx-timer";
>                         reg = <0xc8005000 0x100>;
>                         interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
> Device tree probing does not allow two devices to probe from the same
> DT node, so this was solved by letting the (less important) watchdog
> be spawn as a platform device from the timer.
>
> I don't know if double-probing for the same register range can be fixed,
> but I was assuming that the one-compatible-to-one-driver assumption
> was pretty hard-coded into the abstractions. Maybe it isn't?

Having a child device is fine, my objection was about the way
the device is created from a 'static platform_device ...' definition
rather than having the device structure allocated at probe time.

> Another way is of course to introduce an MFD. That becomes
> problematic in another way: MFD abstractions are supposed to
> be inbetween the resource and the devices it spawns, and with
> timers/clocksources this creates a horrible special-casing since the
> MFD bus (the parent may be providing e.g. an MMIO regmap)
> then need to be early-populated and searched by the timer core
> from TIMER_OF_DECLARE() early in boot.
>
> So this solution was the lesser evil that I could think about.

There are multiple ways of doing this that we already discussed
in the thread. The easiest is probably to have a child node without
custom registers in the DT and then use the DT helpers to
populate the linux devices with the correct data.

       Arnd

Re: [PATCH v5 10/11] arch: arm: boot: dts: Introduce HPE GXP Device tree

[Krzysztof Kozlowski]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The HPE SoC is new to linux. This patch
> creates the basic device tree layout with minimum required
> for linux to boot. This includes timer and watchdog
> support.
> 
> The dts file is empty at this point but will be
> updated in subsequent updates as board specific features
> are enabled.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v5:
> * Fixed commit message to show previous changes
> * Fixed typo ehci -> echi

One more thought, but I am pretty sure we talked about this already:
please fix your commit subject. git log --oneline.


Best regards,
Krzysztof

Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Paul Menzel]

Dear Nick,


Thank you for the patches.

Am 21.04.22 um 21:21 schrieb nick.hawkins@hpe.com:
> From: Nick Hawkins <nick.hawkins@hpe.com>

Type in the prefix: s/aach/arch/. Looking at `git log --oneline 
arch/arm`, *ARM* or *arm* seems to be commonly used though.

> The GXP is the HPE BMC SoC that is used in the majority
> of HPE Generation 10 servers. Traditionally the asic will
> last multiple generations of server before being replaced.

Please mention what kind of documentation (datasheets, …) are available.

> In gxp.c we reset the EHCI controller early to boot the asic.

Why does the EHCI controller need to be reset?

> Info about SoC:
> 
> HPE GXP is the name of the HPE Soc. This SoC is used to implement
> many BMC features at HPE. It supports ARMv7 architecture based on
> the Cortex A9 core. It is capable of using an AXI bus to which
> a memory controller is attached. It has multiple SPI interfaces
> to connect boot flash and BIOS flash. It uses a 10/100/1000 MAC
> for network connectivity. It has multiple i2c engines to drive
> connectivity with a host infrastructure. The initial patches
> enable the watchdog and timer enabling the host to be able to
> boot.

Maybe doe that in separate commits?

Please reflow the commit message for 75 characters per line.

> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v5:
> * Fixed version log
> v4:
> * Removed unecessary code: restart, iomap, init_machine

unnecessary

> * Reordered Kconfig depends
> * Removed SPARSE_IRQ, MULTI_IRQ_HANDLER, IRQ_DOMAIN, PINCTL from
>    Kconfig
> v3:
> * Put into proper patchset format
> v2:
> * No change
> ---
>   arch/arm/Kconfig           |  2 ++
>   arch/arm/Makefile          |  1 +
>   arch/arm/mach-hpe/Kconfig  | 17 +++++++++++++++++
>   arch/arm/mach-hpe/Makefile |  1 +
>   arch/arm/mach-hpe/gxp.c    | 16 ++++++++++++++++
>   5 files changed, 37 insertions(+)
>   create mode 100644 arch/arm/mach-hpe/Kconfig
>   create mode 100644 arch/arm/mach-hpe/Makefile
>   create mode 100644 arch/arm/mach-hpe/gxp.c
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2e8091e2d8a8..13f77eec7c40 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -620,6 +620,8 @@ source "arch/arm/mach-highbank/Kconfig"
>   
>   source "arch/arm/mach-hisi/Kconfig"
>   
> +source "arch/arm/mach-hpe/Kconfig"
> +
>   source "arch/arm/mach-imx/Kconfig"
>   
>   source "arch/arm/mach-integrator/Kconfig"
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index a2391b8de5a5..97a89023c10f 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -179,6 +179,7 @@ machine-$(CONFIG_ARCH_FOOTBRIDGE)	+= footbridge
>   machine-$(CONFIG_ARCH_GEMINI)		+= gemini
>   machine-$(CONFIG_ARCH_HIGHBANK)		+= highbank
>   machine-$(CONFIG_ARCH_HISI)		+= hisi
> +machine-$(CONFIG_ARCH_HPE)		+= hpe
>   machine-$(CONFIG_ARCH_INTEGRATOR)	+= integrator
>   machine-$(CONFIG_ARCH_IOP32X)		+= iop32x
>   machine-$(CONFIG_ARCH_IXP4XX)		+= ixp4xx
> diff --git a/arch/arm/mach-hpe/Kconfig b/arch/arm/mach-hpe/Kconfig
> new file mode 100644
> index 000000000000..c075248b259e
> --- /dev/null
> +++ b/arch/arm/mach-hpe/Kconfig
> @@ -0,0 +1,17 @@
> +menuconfig ARCH_HPE
> +	bool "HPE SoC support"
> +	depends on ARCH_MULTI_V7
> +	help
> +	  This enables support for HPE ARM based SoC chips

Add a dot/period at the end?

> +if ARCH_HPE
> +
> +config ARCH_HPE_GXP
> +	bool "HPE GXP SoC"
> +	depends on ARCH_MULTI_V7
> +	select ARM_VIC
> +	select GENERIC_IRQ_CHIP
> +	select CLKSRC_MMIO
> +	help
> +	  Support for GXP SoCs

Please elaborate here, maybe copying parts of the commit message, in 
what servers it is used.

> +
> +endif
> diff --git a/arch/arm/mach-hpe/Makefile b/arch/arm/mach-hpe/Makefile
> new file mode 100644
> index 000000000000..8b0a91234df4
> --- /dev/null
> +++ b/arch/arm/mach-hpe/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ARCH_HPE_GXP) += gxp.o
> diff --git a/arch/arm/mach-hpe/gxp.c b/arch/arm/mach-hpe/gxp.c
> new file mode 100644
> index 000000000000..e2f0c3ae6bd8
> --- /dev/null
> +++ b/arch/arm/mach-hpe/gxp.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/

Space before closing comment delimiter.

> +
> +#include <linux/of_platform.h>
> +#include <asm/mach/arch.h>
> +
> +static const char * const gxp_board_dt_compat[] = {
> +	"hpe,gxp",
> +	NULL,
> +};
> +
> +DT_MACHINE_START(GXP_DT, "HPE GXP")
> +	.dt_compat	= gxp_board_dt_compat,
> +	.l2c_aux_val = 0,
> +	.l2c_aux_mask = 0,
> +MACHINE_END

Where is the EHCI controller reset?


Kind regards,

Paul

RE: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Hawkins, Nick]

-----Original Message-----
From: Rob Herring [mailto:robh@kernel.org] 
Sent: Monday, April 25, 2022 5:05 PM
To: Hawkins, Nick <nick.hawkins@hpe.com>
Cc: Verdun, Jean-Marie <verdun@hpe.com>; joel@jms.id.au; arnd@arndb.de; openbmc@lists.ozlabs.org; Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck <linux@roeck-us.net>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

(...)

> How is this h/w controlled? I'm guessing it's part of the timer? If so, you don't need this node. A single node can implement multiple functions.

It is associated with the timer because of the shared register set. Based on feedback from Krzysztof I need to create a child node for gxp-timer. I therefore will remove this file and move gxp-wdt to the hpe,gxp-timer.yaml as a child node.

Thank you for the feedback.

-Nick Hawkins

Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Krzysztof Kozlowski]

On 26/04/2022 15:21, Hawkins, Nick wrote:
>> How is this h/w controlled? I'm guessing it's part of the timer? If so, you don't need this node. A single node can implement multiple functions.
> 
> It is associated with the timer because of the shared register set. Based on feedback from Krzysztof I need to create a child node for gxp-timer. I therefore will remove this file and move gxp-wdt to the hpe,gxp-timer.yaml as a child node.

I have impression my feedback was about mapping entire address space,
not few registers of watchdog:
https://lore.kernel.org/all/c6309ed8-6e74-67d3-304a-f5399d16cc37@canonical.com/

However later during talks it turned out that the address space is
heavily shared.

Nick,
BTW, do you see my comments in the email I linked above? This v5 makes
the same mistake. We talked about this already, so please make note of
comments you receive and implement them.

Best regards,
Krzysztof

RE: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Hawkins, Nick]

-----Original Message-----
From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] 
Sent: Tuesday, April 26, 2022 8:35 AM
To: Hawkins, Nick <nick.hawkins@hpe.com>; Rob Herring <robh@kernel.org>
Cc: Verdun, Jean-Marie <verdun@hpe.com>; joel@jms.id.au; arnd@arndb.de; openbmc@lists.ozlabs.org; Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck <linux@roeck-us.net>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; linux-watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

On 26/04/2022 15:21, Hawkins, Nick wrote:
>>> How is this h/w controlled? I'm guessing it's part of the timer? If so, you don't need this node. A single node can implement multiple functions.
>> 
>> It is associated with the timer because of the shared register set. Based on feedback from Krzysztof I need to create a child node for gxp-timer. I therefore will remove this file and move gxp-wdt to the hpe,gxp-timer.yaml as a child node.

>I have impression my feedback was about mapping entire address space, not few registers of watchdog:
>https://lore.kernel.org/all/c6309ed8-6e74-67d3-304a-f5399d16cc37@canonical.com/

>However later during talks it turned out that the address space is heavily shared.

>Nick,
>BTW, do you see my comments in the email I linked above? This v5 makes the same mistake. We talked about this already, so please make note of comments you receive and implement them.

Krzysztof,

Apologies, I did miss the comment about the double spacing around the label and the label not being necessary. I will not make this mistake again. I became focused about the comment of mapping an entire register space which indirectly lead me on to the path which I am now having the gxp-timer have the gxp-wdt as a child. To be specific the feedback I was speaking of above was about the gxp-timer which is here: https://lore.kernel.org/all/704ffa56-4bae-fc33-fddf-3e3dd8be0db9@linaro.org/ That is the children must be defined for a simple-mfd device. Hence the plan I have now is to remove the hpe,gxp-wdt.yaml entirely and include it in the hpe,gxp-timer.yaml. I assume that is the correct thing to do?

Thank you for your time and feedback,

-Nick

Re: [PATCH v5 06/11] dt-bindings: watchdog: Add HPE GXP Watchdog timer binding

[Krzysztof Kozlowski]

On 26/04/2022 15:52, Hawkins, Nick wrote:
> Apologies, I did miss the comment about the double spacing around the label and the label not being necessary. I will not make this mistake again. I became focused about the comment of mapping an entire register space which indirectly lead me on to the path which I am now having the gxp-timer have the gxp-wdt as a child. To be specific the feedback I was speaking of above was about the gxp-timer which is here: https://lore.kernel.org/all/704ffa56-4bae-fc33-fddf-3e3dd8be0db9@linaro.org/ That is the children must be defined for a simple-mfd device. 

This was comment for this v5, not for previous patches. In this v5, you
have a child of timer, so it has to be defined in timer schema.

This was not a comment whether a child should exist or should not. It
was made under the assumption that you want to have a child node.

> Hence the plan I have now is to remove the hpe,gxp-wdt.yaml entirely and include it in the hpe,gxp-timer.yaml. I assume that is the correct thing to do?

I would follow here the advice from Rob, so since the blocks are mixed
significantly (same address space), then let's assume it's actually one
device with two functions. In such case Rob pointed out that child node
is not necessary.

The implementation might differ, depending how the features are mixed-up
with each other. It might be one driver having timer and watchdog, or
several drivers (usually bound together with a MFD driver which serves
as parents and binds to the OF node).

Best regards,
Krzysztof

RE: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Hawkins, Nick]

-----Original Message-----
From: Paul Menzel [mailto:pmenzel@molgen.mpg.de] 
Sent: Tuesday, April 26, 2022 3:26 AM
To: Hawkins, Nick <nick.hawkins@hpe.com>
Cc: Verdun, Jean-Marie <verdun@hpe.com>; joel@jms.id.au; arnd@arndb.de; openbmc@lists.ozlabs.org; Russell King <linux@armlinux.org.uk>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture


> Type in the prefix: s/aach/arch/. Looking at `git log --oneline arch/arm`, *ARM* or *arm* seems to be commonly used though.
I will correct this, I will also look in all other patches to make sure appropriate titles are being used.

> > The GXP is the HPE BMC SoC that is used in the majority of HPE 
> > Generation 10 servers. Traditionally the asic will last multiple 
> > generations of server before being replaced.

> Please mention what kind of documentation (datasheets, …) are available.

Currently there are none available. The only reference I can provide will be arm documentation.

> > In gxp.c we reset the EHCI controller early to boot the asic.

> Why does the EHCI controller need to be reset?
This functionality was moved into the boot loader. This message is stale and needs to be removed. It was necessary for the chip to boot.

> > Info about SoC:
> > 
> > HPE GXP is the name of the HPE Soc. This SoC is used to implement many 
> > BMC features at HPE. It supports ARMv7 architecture based on the 
> > Cortex A9 core. It is capable of using an AXI bus to which a memory 
> > controller is attached. It has multiple SPI interfaces to connect boot 
> > flash and BIOS flash. It uses a 10/100/1000 MAC for network 
> > connectivity. It has multiple i2c engines to drive connectivity with a 
> > host infrastructure. The initial patches enable the watchdog and timer 
> > enabling the host to be able to boot.

> Maybe doe that in separate commits?
Are you asking for me to have this paragraph in the other commits? Or perhaps not mention the other patches in this paragraph? 

> Please reflow the commit message for 75 characters per line.
I will verify all the lines are under 75 characters.

Thanks for the feedback,

-Nick Hawkins

Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[Paul Menzel]

Dear Nick,


Am 26.04.22 um 19:28 schrieb Hawkins, Nick:
> 
> 
> -----Original Message-----
> From: Paul Menzel [mailto:pmenzel@molgen.mpg.de]
> Sent: Tuesday, April 26, 2022 3:26 AM
> To: Hawkins, Nick <nick.hawkins@hpe.com>
> Cc: Verdun, Jean-Marie <verdun@hpe.com>; joel@jms.id.au; arnd@arndb.de; openbmc@lists.ozlabs.org; Russell King <linux@armlinux.org.uk>; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v5 01/11] aach: arm: mach-hpe: Introduce the HPE GXP architecture

[OT: Maybe use an email program, that does not add an unnecessary header.]

[…]

>>> The GXP is the HPE BMC SoC that is used in the majority of HPE
>>> Generation 10 servers. Traditionally the asic will last multiple
>>> generations of server before being replaced.
> 
>> Please mention what kind of documentation (datasheets, …) are available.
> 
> Currently there are none available. The only reference I can provide
> will be arm documentation.

Too bad.

>>> In gxp.c we reset the EHCI controller early to boot the asic.
> 
>> Why does the EHCI controller need to be reset?
> This functionality was moved into the boot loader. This message is
> stale and needs to be removed. It was necessary for the chip to
> boot.

Understood. Please mention somewhere, what bootloader is used.

>>> Info about SoC:
>>>
>>> HPE GXP is the name of the HPE Soc. This SoC is used to implement many
>>> BMC features at HPE. It supports ARMv7 architecture based on the
>>> Cortex A9 core. It is capable of using an AXI bus to which a memory
>>> controller is attached. It has multiple SPI interfaces to connect boot
>>> flash and BIOS flash. It uses a 10/100/1000 MAC for network
>>> connectivity. It has multiple i2c engines to drive connectivity with a
>>> host infrastructure. The initial patches enable the watchdog and timer
>>> enabling the host to be able to boot.
> 
>> Maybe doe that in separate commits?
> Are you asking for me to have this paragraph in the other commits?
> Or perhaps not mention the other patches in this paragraph?

Yes, please move:

> The initial patches enable the watchdog and timer enabling the host
> to be able to boot.

in a cover letter for example.

>> Please reflow the commit message for 75 characters per line.
> I will verify all the lines are under 75 characters.

Please make sure the lines are as long as possible, while being at most 
75 characters long.


Kind regards,

Paul

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Rob Herring]

On Tue, Apr 26, 2022 at 08:00:20AM +0200, Arnd Bergmann wrote:
> On Mon, Apr 25, 2022 at 10:38 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Fri, Apr 22, 2022 at 3:16 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Thu, Apr 21, 2022 at 9:21 PM <nick.hawkins@hpe.com> wrote:
> > >
> > > > +
> > > > +static struct platform_device gxp_watchdog_device = {
> > > > +       .name = "gxp-wdt",
> > > > +       .id = -1,
> > > > +};
> > > > +/*
> > > > + * This probe gets called after the timer is already up and running. This will create
> > > > + * the watchdog device as a child since the registers are shared.
> > > > + */
> > > > +
> > > > +static int gxp_timer_probe(struct platform_device *pdev)
> > > > +{
> > > > +       struct device *dev = &pdev->dev;
> > > > +
> > > > +       /* Pass the base address (counter) as platform data and nothing else */
> > > > +       gxp_watchdog_device.dev.platform_data = local_gxp_timer->counter;
> > > > +       gxp_watchdog_device.dev.parent = dev;
> > > > +       return platform_device_register(&gxp_watchdog_device);
> > > > +}
> > >
> > > I don't understand what this is about: the device should be created from
> > > DT, not defined statically in the code. There are multiple ways of creating
> > > a platform_device from a DT node, or you can allocate one here, but static
> > > definitions are generally a mistake.
> > >
> > > I see that you copied this from the ixp4xx driver, so I think we should fix this
> > > there as well.
> >
> > The ixp4xx driver looks like that because the register range used for
> > the timer and the watchdog is combined, i.e. it is a single IP block:
> >
> >                 timer@c8005000 {
> >                         compatible = "intel,ixp4xx-timer";
> >                         reg = <0xc8005000 0x100>;
> >                         interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
> >                 };
> >
> > Device tree probing does not allow two devices to probe from the same
> > DT node, so this was solved by letting the (less important) watchdog
> > be spawn as a platform device from the timer.
> >
> > I don't know if double-probing for the same register range can be fixed,
> > but I was assuming that the one-compatible-to-one-driver assumption
> > was pretty hard-coded into the abstractions. Maybe it isn't?
> 
> Having a child device is fine, my objection was about the way
> the device is created from a 'static platform_device ...' definition
> rather than having the device structure allocated at probe time.
> 
> > Another way is of course to introduce an MFD. That becomes
> > problematic in another way: MFD abstractions are supposed to
> > be inbetween the resource and the devices it spawns, and with
> > timers/clocksources this creates a horrible special-casing since the
> > MFD bus (the parent may be providing e.g. an MMIO regmap)
> > then need to be early-populated and searched by the timer core
> > from TIMER_OF_DECLARE() early in boot.
> >
> > So this solution was the lesser evil that I could think about.
> 
> There are multiple ways of doing this that we already discussed
> in the thread. The easiest is probably to have a child node without
> custom registers in the DT and then use the DT helpers to
> populate the linux devices with the correct data.

I think that's what the wdt binding is doing, but I don't like that. 
Maybe it's not a child node, I can't tell.

Bindings should not be decided on the *current* driver split on one 
particular OS. This looks like 1 block, so 1 node. If that doesn't work 
well or easy for Linux, then we should fix Linux.

Rob

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Arnd Bergmann]

On Tue, Apr 26, 2022 at 11:38 PM Rob Herring <robh@kernel.org> wrote:
> On Tue, Apr 26, 2022 at 08:00:20AM +0200, Arnd Bergmann wrote:
> > On Mon, Apr 25, 2022 at 10:38 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> > There are multiple ways of doing this that we already discussed
> > in the thread. The easiest is probably to have a child node without
> > custom registers in the DT and then use the DT helpers to
> > populate the linux devices with the correct data.
>
> I think that's what the wdt binding is doing, but I don't like that.
> Maybe it's not a child node, I can't tell.
>
> Bindings should not be decided on the *current* driver split on one
> particular OS. This looks like 1 block, so 1 node.

Fair enough.

> If that doesn't work well or easy for Linux, then we should fix Linux.

Doing a simple platform_device_create_pdata() should work fine here,
the only problem that might exist is if the wdt driver needs access to
DT properties, as we can't have both devices refer to the same of_node
pointer, which would cause them to be picked up by the timer driver
again.

      Arnd

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Rob Herring]

On Tue, Apr 26, 2022 at 4:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Apr 26, 2022 at 11:38 PM Rob Herring <robh@kernel.org> wrote:
> > On Tue, Apr 26, 2022 at 08:00:20AM +0200, Arnd Bergmann wrote:
> > > On Mon, Apr 25, 2022 at 10:38 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > > There are multiple ways of doing this that we already discussed
> > > in the thread. The easiest is probably to have a child node without
> > > custom registers in the DT and then use the DT helpers to
> > > populate the linux devices with the correct data.
> >
> > I think that's what the wdt binding is doing, but I don't like that.
> > Maybe it's not a child node, I can't tell.
> >
> > Bindings should not be decided on the *current* driver split on one
> > particular OS. This looks like 1 block, so 1 node.
>
> Fair enough.
>
> > If that doesn't work well or easy for Linux, then we should fix Linux.
>
> Doing a simple platform_device_create_pdata() should work fine here,
> the only problem that might exist is if the wdt driver needs access to
> DT properties, as we can't have both devices refer to the same of_node
> pointer,

Why not? There's even a struct device flag for that.

> which would cause them to be picked up by the timer driver
> again.

Huh?

That's not to say there might be some gotchas. The musb driver didn't
like sharing. But those are issues we should fix rather than
work-around in the binding.

Rob

Re: [PATCH v5 04/11] clocksource/drivers: Add HPE GXP timer

[Arnd Bergmann]

On Wed, Apr 27, 2022 at 12:04 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Apr 26, 2022 at 4:55 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Apr 26, 2022 at 11:38 PM Rob Herring <robh@kernel.org> wrote:
> > > On Tue, Apr 26, 2022 at 08:00:20AM +0200, Arnd Bergmann wrote:
> > > > On Mon, Apr 25, 2022 at 10:38 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > > > There are multiple ways of doing this that we already discussed
> > > > in the thread. The easiest is probably to have a child node without
> > > > custom registers in the DT and then use the DT helpers to
> > > > populate the linux devices with the correct data.
> > >
> > > I think that's what the wdt binding is doing, but I don't like that.
> > > Maybe it's not a child node, I can't tell.
> > >
> > > Bindings should not be decided on the *current* driver split on one
> > > particular OS. This looks like 1 block, so 1 node.
> >
> > Fair enough.
> >
> > > If that doesn't work well or easy for Linux, then we should fix Linux.
> >
> > Doing a simple platform_device_create_pdata() should work fine here,
> > the only problem that might exist is if the wdt driver needs access to
> > DT properties, as we can't have both devices refer to the same of_node
> > pointer,
>
> Why not? There's even a struct device flag for that.

Ah, I forgot about ->of_node_reused, that should work then if
it needs access to properties.

        Arnd

RE: [PATCH v5 02/11] arch: arm: configs: multi_v7_defconfig

[Hawkins, Nick]

On 21/04/2022 21:21, nick.hawkins@hpe.com wrote:
> > @@ -1228,3 +1230,4 @@ CONFIG_CMA_SIZE_MBYTES=64  CONFIG_PRINTK_TIME=y  
> > CONFIG_MAGIC_SYSRQ=y  CONFIG_DEBUG_FS=y
> > +CONFIG_GXP_WATCHDOG=y

> This does not look like in correct place. You need to add entries how the savedefconfig would do it.

Hello Krzysztof,

I have moved CONFIG_GXP_WATCHDOG between CONFIG_PM8916_WATCHDOG=m and CONFIG_BCM47XX_WDT=y. How do I run savedefconfig to make sure it is in the right place for this file?

Thanks,

-Nick Hawkins 

Re: [PATCH v5 02/11] arch: arm: configs: multi_v7_defconfig

[Krzysztof Kozlowski]

On 29/04/2022 22:34, Hawkins, Nick wrote:
> I have moved CONFIG_GXP_WATCHDOG between CONFIG_PM8916_WATCHDOG=m and CONFIG_BCM47XX_WDT=y. How do I run savedefconfig to make sure it is in the right place for this file?

make savedefconfig

Best regards,
Krzysztof