[PATCH v6 1/8] ARM: hpe: Introduce the HPE GXP architecture

[PATCH v6 1/8] ARM: 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 current generation HPE 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 whicha 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. There
currently are no public specifications but this process is being worked.

Previously there was a requirement to reset the EHCI controller for the
asic to boot. This functionality has been moved to the u-boot
bootloader.

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

---
v6:
* Adjusted title to match log entries
* Reconfigured commit message to be closer to 75 char.
* Add space before comment delimiter.
* Added a more elaborate Kconfig help section and fixed punctuation.
* Fixed l2c_aux_map initialization.
v5:
* Fixed version log
* Removed incorrect statement about reset.
v4:
* Removed unnecessary 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  | 23 +++++++++++++++++++++++
 arch/arm/mach-hpe/Makefile |  1 +
 arch/arm/mach-hpe/gxp.c    | 16 ++++++++++++++++
 5 files changed, 43 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..3372bbf38d38
--- /dev/null
+++ b/arch/arm/mach-hpe/Kconfig
@@ -0,0 +1,23 @@
+menuconfig ARCH_HPE
+	bool "HPE SoC support"
+	depends on ARCH_MULTI_V7
+	help
+	  This enables support for HPE ARM based BMC 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
+	  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.
+
+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..ef3341373006
--- /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 v6 2/8] ARM: configs: multi_v7_defconfig: Add HPE GXP ARCH

[nick.hawkins]

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

Enable HPE GXP Architecture and its watchdog for base support for HPE
GXP SoCs.

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

---
v6:
* Changed the title to match others in log
* Changed the patch description
* Ran savedefconfig to place GXP configs in file correctly
v5:
* Fix version log
v4:
* No change
v3:
* Put into proper patch format
* Modified the multi_v7_defconfig instead of creating a 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..bdbb1c90e65d 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -42,6 +42,8 @@ CONFIG_ARCH_HI3xxx=y
 CONFIG_ARCH_HIP01=y
 CONFIG_ARCH_HIP04=y
 CONFIG_ARCH_HIX5HD2=y
+CONFIG_ARCH_HPE=y
+CONFIG_ARCH_HPE_GXP=y
 CONFIG_ARCH_MXC=y
 CONFIG_SOC_IMX50=y
 CONFIG_SOC_IMX51=y
@@ -562,6 +564,7 @@ CONFIG_BCM47XX_WDT=y
 CONFIG_BCM2835_WDT=y
 CONFIG_BCM_KONA_WDT=y
 CONFIG_BCM7038_WDT=m
+CONFIG_GXP_WATCHDOG=y
 CONFIG_BCMA_HOST_SOC=y
 CONFIG_BCMA_DRIVER_GMAC_CMN=y
 CONFIG_BCMA_DRIVER_GPIO=y
-- 
2.17.1

[PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[nick.hawkins]

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

Adding support for the HPE GXP Watchdog. 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>

---
v6:
* No code change.
* Fixed commit subject line to match the ones in log.
* Adjusted commit message to be closer to 75 chars per line.
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 v6 4/8] clocksource/drivers/timer-gxp: 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.

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

---
v6:
* Changed global variable name from local_gxp_timer to gxp_timer while
  removing the requirement for a local variable.
* Changed static void __iomem *system_clock to __ro_after_init from
  __read_mostly
* Changed subject to match the format from the logs
* Removed stray tab in the argument for gxp_time_set_next_event
* Made the commit description fit to 75 characters per line
* Created watchdog child dynamically with platform_device_alloc
* Fixed spacing in comments
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 | 209 ++++++++++++++++++++++++++++++++
 3 files changed, 218 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..8b38b3212388
--- /dev/null
+++ b/drivers/clocksource/timer-gxp.c
@@ -0,0 +1,209 @@
+// 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 *gxp_timer;
+
+static void __iomem *system_clock __ro_after_init;
+
+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;
+
+	gxp_timer = kzalloc(sizeof(*gxp_timer), GFP_KERNEL);
+	if (!gxp_timer) {
+		ret = -ENOMEM;
+		pr_err("Can't allocate gxp_timer");
+		return ret;
+	}
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk)) {
+		ret = (int)PTR_ERR(clk);
+		pr_err("%pOFn clock not found: %d\n", node, ret);
+		goto err_free;
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		pr_err("%pOFn clock enable failed: %d\n", node, ret);
+		goto err_clk_enable;
+	}
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		ret = -ENXIO;
+		pr_err("Can't map timer base registers");
+		goto err_iomap;
+	}
+
+	/* Set the offsets to the clock register and timer registers */
+	gxp_timer->counter = base + GXP_TIMER_CNT_OFS;
+	gxp_timer->control = base + GXP_TIMER_CTRL_OFS;
+	system_clock = base + GXP_TIMESTAMP_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);
+
+	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_exit;
+	}
+
+	freq = clk_get_rate(clk);
+
+	ret = clocksource_mmio_init(system_clock, node->name, freq,
+				    300, 32, clocksource_mmio_readl_up);
+	if (ret) {
+		pr_err("%pOFn init clocksource failed: %d", node, ret);
+		goto err_exit;
+	}
+
+	sched_clock_register(gxp_sched_read, 32, freq);
+
+	irq = irq_of_parse_and_map(node, 0);
+	if (irq <= 0) {
+		ret = -EINVAL;
+		pr_err("%pOFn Can't parse IRQ %d", node, irq);
+		goto err_exit;
+	}
+
+	clockevents_config_and_register(&gxp_timer->evt, TIMER0_FREQ,
+					0xf, 0xffffffff);
+
+	ret = request_irq(irq, gxp_timer_interrupt, IRQF_TIMER | IRQF_SHARED,
+			  node->name, gxp_timer);
+	if (ret) {
+		pr_err("%pOFn request_irq() failed: %d", node, ret);
+		goto err_exit;
+	}
+
+	pr_debug("gxp: system timer (irq = %d)\n", irq);
+	return 0;
+
+err_exit:
+	iounmap(base);
+err_iomap:
+	clk_disable_unprepare(clk);
+err_clk_enable:
+	clk_put(clk);
+err_free:
+	kfree(gxp_timer);
+	return ret;
+}
+
+/*
+ * 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 platform_device *gxp_watchdog_device;
+	struct device *dev = &pdev->dev;
+
+	if (!gxp_timer) {
+		pr_err("Gxp Timer not initialized, cannot create watchdog");
+		return -ENOMEM;
+	}
+
+	gxp_watchdog_device = platform_device_alloc("gxp-wdt", -1);
+	if (!gxp_watchdog_device) {
+		pr_err("Timer failed to allocate gxp-wdt");
+		return -ENOMEM;
+	}
+
+	/* Pass the base address (counter) as platform data and nothing else */
+	gxp_watchdog_device->dev.platform_data = gxp_timer->counter;
+	gxp_watchdog_device->dev.parent = dev;
+
+	return platform_device_add(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 v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[nick.hawkins]

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

Add support for the HPE GXP Timer and Watchdog. There are multiple
timers on the SoC but only one is enabled at this time.

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

---
v6:
* Removed simple-mfd compatible, timer will create watchdog without
  watchdog node.
* Removed timer0 label
* Changed title from HPE GXP TIMER to HPE GXP Timer
* Changed clock name iopclk to iop
* Set additionalProperties to false
* Added space after ',' in the compatible list
* Changed subject to match the log better
* Altered description to take up 75 characters per line
* Changed description to better describe the patch
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         | 47 +++++++++++++++++++
 1 file changed, 47 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..d33d90f44d28
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
@@ -0,0 +1,47 @@
+# 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:
+    const: hpe,gxp-timer
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: iop
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    timer@c0000000 {
+        compatible = "hpe,gxp-timer";
+        reg = <0x80 0x16>;
+        interrupts = <0>;
+        interrupt-parent = <&vic0>;
+        clocks = <&iopclk>;
+        clock-names = "iop";
+    };
-- 
2.17.1

[PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[nick.hawkins]

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

Add support for HPE GXP. The GXP is based on the cortex a9 processor and
supports arm7.

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

---
v6:
* Changed subject to match others in log.
* Changed the commit description.
* Changed the title in the .yaml from HPE BMC GXP SoC Driver to HPE BMC
  GXP platforms.
* Added the oneOf attribute
* Fixed commit description to be close to 75 characters per line
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      | 27 +++++++++++++++++++
 1 file changed, 27 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..224bbcb93f95
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/hpe,gxp.yaml
@@ -0,0 +1,27 @@
+# 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 platforms
+
+maintainers:
+  - Nick Hawkins <nick.hawkins@hpe.com>
+  - Jean-Marie Verdun <verdun@hpe.com>
+
+properties:
+  compatible:
+    oneOf:
+      - description: GXP Based Boards
+        items:
+          - enum:
+              - hpe,gxp-dl360gen10
+          - const: hpe,gxp
+
+required:
+  - compatible
+
+additionalProperties: true
+
+...
-- 
2.17.1

[PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[nick.hawkins]

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

The HPE SoC is new to linux. A basic device tree layout with minimum
required for linux to boot including a timer and watchdog support has
been created.

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>

---
v6:
* Added cache-controller to CPU
* Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
* Added space after ',' in compatible lists containing more than one
  item
* Switched clock name iopclk to iop based on feedback from
  hpe,gxp-timer.yaml
* Added clock labels clock-0 and clock-1
* Added dma-ranges to ahb
* Changed subject to better match ones in the log
* Changed description to fit 75 characters per line
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           | 132 +++++++++++++++++++++++
 3 files changed, 147 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..7a99e174e4b3
--- /dev/null
+++ b/arch/arm/boot/dts/hpe-gxp.dtsi
@@ -0,0 +1,132 @@
+// 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";
+			next-level-cache = <&L2>;
+		};
+	};
+
+	clocks {
+		pll: clock-0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <1600000000>;
+		};
+
+		iopclk: clock-1 {
+			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;
+
+		L2: cache-controller@b0040000 {
+			compatible = "arm,pl310-cache";
+			reg = <0xb0040000 0xFFC>;
+			cache-unified;
+			cache-level = <2>;
+		};
+
+		ahb@c0000000 {
+			compatible = "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ranges = <0x0 0xc0000000 0x30000000>;
+			dma-ranges;
+
+			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";
+				reg = <0x80 0x16>;
+				interrupts = <0>;
+				interrupt-parent = <&vic0>;
+				clocks = <&iopclk>;
+				clock-names = "iop";
+			};
+
+			usb1: usb@efe0100 {
+				compatible = "hpe,gxp-ohci", "generic-ohci";
+				reg = <0xefe0100 0x110>;
+				interrupts = <6>;
+				interrupt-parent = <&vic0>;
+			};
+		};
+	};
+};
-- 
2.17.1

[PATCH v6 8/8] 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>

---
v6:
* Fixed subject to match other commits
* Removed hpe,gxp-wdt.yaml from file list
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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 40fa1955ca3f..b2a14e8b6396 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2130,6 +2130,18 @@ 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:	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 v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Guenter Roeck]

On 5/2/22 13:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Adding support for the HPE GXP Watchdog. The GXP asic contains a full

Add

> compliment of timers one of which is the watchdog timer. The watchdog

complement ?

> 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>
> 
> ---
> v6:
> * No code change.
> * Fixed commit subject line to match the ones in log.
> * Adjusted commit message to be closer to 75 chars per line.
> 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>

Where are those of_ includes used ?

> +#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

Shouldn't that be 655350 ?

> +#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);

First, the accuracy of actual is reduced to 1 second, then SECS_TO_WDOG_TICKS()
multiplies the result with 100, meaning the actual accuracy is 10ms. Why not
just use 10 ms ?

	actual = min(timeout * 100, wdd->max_hw_heartbeat_ms / 10);
	writew(actual, drvdata->base + GXP_WDT_CNT_OFS);

I guess it doesn't matter much since max_hw_heartbeat_ms is really a constant
rounded down to seconds, it just looks odd.

> +
> +	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);

Doesn't that translate to 100 ms timeout ? Why such a large reboot delay
instead of writing 1 ?

> +	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;

I'd personaly prefer if the address was passed as resource.

> +
> +	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");

Re: [PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the HPE GXP Timer and Watchdog. There are multiple
> timers on the SoC but only one is enabled at this time.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Removed simple-mfd compatible, timer will create watchdog without
>   watchdog node.
> * Removed timer0 label
> * Changed title from HPE GXP TIMER to HPE GXP Timer
> * Changed clock name iopclk to iop
> * Set additionalProperties to false
> * Added space after ',' in the compatible list
> * Changed subject to match the log better

The subject after prefixes does not match it better. What is "creation"?
"Add HPE GXP Timer and Watchdog"

Rest is okay, so this could be fixed while applying (if you are going to
resend, don't loose that comment and don't loose the tag):

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

Best regards,
Krzysztof

Re: [PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for HPE GXP. The GXP is based on the cortex a9 processor and
> supports arm7.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>

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


Best regards,
Krzysztof

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> The HPE SoC is new to linux. A basic device tree layout with minimum
> required for linux to boot including a timer and watchdog support has
> been created.
> 
> 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>
> 
> ---
> v6:
> * Added cache-controller to CPU
> * Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
> * Added space after ',' in compatible lists containing more than one
>   item

Hmmm....

>  	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";

Missing space after ','

With this fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v6 8/8] MAINTAINERS: Introduce HPE GXP Architecture

[Krzysztof Kozlowski]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
>  
> +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:	arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
> +F:	arch/arm/boot/dts/hpe-gxp.dtsi

Instead:
F:	arch/arm/boot/dts/hpe-bmc*
F:	arch/arm/boot/dts/hpe-gxp*

> +F:	arch/arm/mach-hpe/gxp.c

Instead:
F:	arch/arm/mach-hpe/

> +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>


Best regards,
Krzysztof

Re: [PATCH v6 4/8] clocksource/drivers/timer-gxp: Add HPE GXP Timer

[Arnd Bergmann]

On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:
>
> +config GXP_TIMER
> +       bool "GXP timer driver" if COMPILE_TEST
> +       depends on ARCH_HPE
> +       default y

I don't think this does what you intended: with the COMPILE_TEST option,
you make it possible to disable the driver when ARCH_HPE is set,
but you don't allow enabling it on other platforms, which is actually the
point of compile testing.

Maybe instead use

config GXP_TIMER
       bool "GXP timer driver" if COMPILE_TEST && !ARCH_HPE
       default ARCH_HPE

Also change the prompt to be more specific and mention HPE,
as the 'GXP timer' is not a particularly obvious name for random
users.

You probably also need

        select TIMER_OF if OF


> +/*
> + * 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 platform_device *gxp_watchdog_device;
> +       struct device *dev = &pdev->dev;
> +
> +       if (!gxp_timer) {
> +               pr_err("Gxp Timer not initialized, cannot create watchdog");
> +               return -ENOMEM;
> +       }
> +
> +       gxp_watchdog_device = platform_device_alloc("gxp-wdt", -1);
> +       if (!gxp_watchdog_device) {
> +               pr_err("Timer failed to allocate gxp-wdt");
> +               return -ENOMEM;
> +       }
> +
> +       /* Pass the base address (counter) as platform data and nothing else */
> +       gxp_watchdog_device->dev.platform_data = gxp_timer->counter;
> +       gxp_watchdog_device->dev.parent = dev;
> +
> +       return platform_device_add(gxp_watchdog_device);
> +}

This looks good to me now.

        Arnd

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:

> --- /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";
> +};

In the board specific file, you normally have "aliases", "chosen" and "memory"
nodes that define e.g. which uart is used for the console.

> diff --git a/arch/arm/boot/dts/hpe-gxp.dtsi b/arch/arm/boot/dts/hpe-gxp.dtsi
> new file mode 100644
> index 000000000000..7a99e174e4b3
> --- /dev/null
> +++ b/arch/arm/boot/dts/hpe-gxp.dtsi
> @@ -0,0 +1,132 @@
> +// 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";

I think "hpe,gxp-dl360gen10" is misplaced here, if that is the identifier
of a particular machine rather than the SoC.

> +       memory@40000000 {
> +               device_type = "memory";
> +               reg = <0x40000000 0x20000000>;
> +       };

If the memory is outside of the SoC, it should not be here.

> +       axi {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +               dma-ranges;
> +
> +               L2: cache-controller@b0040000 {
> +                       compatible = "arm,pl310-cache";
> +                       reg = <0xb0040000 0xFFC>;
> +                       cache-unified;
> +                       cache-level = <2>;
> +               };

The length of the register area looks wrong, this is usually the full
4KB area, rather than leaving out the last four bytes.

        Arnd

RE: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Hawkins, Nick]

On 02/05/2022 22:40, nick.hawkins@hpe.com wrote:
> > From: Nick Hawkins <nick.hawkins@hpe.com>
> > 
> > The HPE SoC is new to linux. A basic device tree layout with minimum 
> > required for linux to boot including a timer and watchdog support has 
> > been created.
> > 
> > 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>
> > 
> > ---
> > v6:
> > * Added cache-controller to CPU
> > * Removed hpe,gxp-wdt and removed simple-mfd from hpe,gxp-timer
> > * Added space after ',' in compatible lists containing more than one
> >   item

> Hmmm....

> >  	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";

> Missing space after ','

> With this fixed:
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I forgot to check this file while fixing the spacing. Thank you for your review and apologies about missing this.

Thanks,

-Nick Hawkins

RE: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Hawkins, Nick]

On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:


> > +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts

> In the board specific file, you normally have "aliases", "chosen" and "memory"
> nodes that define e.g. which uart is used for the console.

This is what I have come up with.

/ {
        #address-cells = <1>;
        #size-cells = <1>;
        compatible = "hpe,gxp-dl360gen10", "hpe,gxp";
        model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";

        aliases {
                serial2 = &uartc;
        };

        chosen {
                bootargs = "console=ttyS2,115200";
        };

        memory@40000000 {
                device_type = "memory";
                reg = <0x40000000 0x20000000>;
        };
};

> > +/dts-v1/;
> > +/ {
> > +       model = "Hewlett Packard Enterprise GXP BMC";
> > +       compatible = "hpe,gxp", "hpe,gxp-dl360gen10";

> I think "hpe,gxp-dl360gen10" is misplaced here, if that is the identifier of a particular machine rather than the SoC.

Yes that does not look correct there it will now be compatible = "hpe,gxp";

> > +       memory@40000000 {
> > +               device_type = "memory";
> > +               reg = <0x40000000 0x20000000>;
> > +       };

> If the memory is outside of the SoC, it should not be here.

It is part of the SoC. Should I put it in the axi bus instead? If so would I need to create an axi for the .dts as well?

Thanks for the review,

-Nick Hawkins

RE: [PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Hawkins, Nick]

On 5/2/22 13:40, nick.hawkins@hpe.com wrote:
> > +#include <linux/of_address.h>
> > +#include <linux/of_platform.h>

> Where are those of_ includes used ?

They were not used anymore with latest changes. Thank you for pointing this out. I will remember to check in the future for each new commit to double check this.

> > +#define WDT_MAX_TIMEOUT_MS	655000

> Shouldn't that be 655350 ?

Yes it should be. I will correct this.

> > +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);

> First, the accuracy of actual is reduced to 1 second, then SECS_TO_WDOG_TICKS() multiplies the result with 100, meaning the actual accuracy is 10ms. Why not just use 10 ms ?

>	actual = min(timeout * 100, wdd->max_hw_heartbeat_ms / 10);
>	writew(actual, drvdata->base + GXP_WDT_CNT_OFS);

I have replaced the mention code with what you recommended above.

> > +
> > +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);

> Doesn't that translate to 100 ms timeout ? Why such a large reboot delay instead of writing 1 ?

This has been changed to 1.

> > +	gxp_wdt_enable_reload(drvdata);
> > +	mdelay(100);
> > +	return 0;
> > +}
> > +
> > +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;

> I'd personaly prefer if the address was passed as resource.

Just to clarify for my understanding are you asking that in the device structure I use the "void *platform_data" to pass "struct *resource"? If I am incorrect here can you elaborate on what you would like to be done? Based on feedback in review for the device tree; the watchdog is being created as a child to the timer. Therefore the conclusion reached was there should not be a gxp-wdt listed in the device tree files. I took this implementation based on what I found in ixp4xx_wdt.c.

Thank you for your time and feedback Guenter,

-Nick Hawkins

Re: [PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Guenter Roeck]

On 5/3/22 09:22, Hawkins, Nick wrote:
> On 5/2/22 13:40, nick.hawkins@hpe.com wrote:
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_platform.h>
> 
>> Where are those of_ includes used ?
> 
> They were not used anymore with latest changes. Thank you for pointing this out. I will remember to check in the future for each new commit to double check this.
> 
>>> +#define WDT_MAX_TIMEOUT_MS	655000
> 
>> Shouldn't that be 655350 ?
> 
> Yes it should be. I will correct this.
> 
>>> +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);
> 
>> First, the accuracy of actual is reduced to 1 second, then SECS_TO_WDOG_TICKS() multiplies the result with 100, meaning the actual accuracy is 10ms. Why not just use 10 ms ?
> 
>> 	actual = min(timeout * 100, wdd->max_hw_heartbeat_ms / 10);
>> 	writew(actual, drvdata->base + GXP_WDT_CNT_OFS);
> 
> I have replaced the mention code with what you recommended above.
> 
>>> +
>>> +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);
> 
>> Doesn't that translate to 100 ms timeout ? Why such a large reboot delay instead of writing 1 ?
> 
> This has been changed to 1.
> 
>>> +	gxp_wdt_enable_reload(drvdata);
>>> +	mdelay(100);
>>> +	return 0;
>>> +}
>>> +
>>> +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;
> 
>> I'd personaly prefer if the address was passed as resource.
> 
> Just to clarify for my understanding are you asking that in the device structure I use the "void *platform_data" to pass "struct *resource"? If I am incorrect here can you elaborate on what you would like to be done? Based on feedback in review for the device tree; the watchdog is being created as a child to the timer. Therefore the conclusion reached was there should not be a gxp-wdt listed in the device tree files. I took this implementation based on what I found in ixp4xx_wdt.c.
> 

One bad deed tends to multiply.

No, I didn't ask to pass a struct resource as platform data.
That would be no different to the current code. Resources
can be added to a platform device using
platform_device_add_resources(), and the platform driver
can then use platform_get_resource() to use it. This
would make it independent of a "private" mechanism.

Thanks,
Guenter

Re: [PATCH v6 5/8] dt-bindings: timer: hpe,gxp-timer: Creation

[Rob Herring]

On Mon, 02 May 2022 15:40:47 -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for the HPE GXP Timer and Watchdog. There are multiple
> timers on the SoC but only one is enabled at this time.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Removed simple-mfd compatible, timer will create watchdog without
>   watchdog node.
> * Removed timer0 label
> * Changed title from HPE GXP TIMER to HPE GXP Timer
> * Changed clock name iopclk to iop
> * Set additionalProperties to false
> * Added space after ',' in the compatible list
> * Changed subject to match the log better
> * Altered description to take up 75 characters per line
> * Changed description to better describe the patch
> 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         | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/hpe,gxp-timer.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v6 6/8] dt-bindings: arm: hpe: add GXP Support

[Rob Herring]

On Mon, 02 May 2022 15:40:48 -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
> 
> Add support for HPE GXP. The GXP is based on the cortex a9 processor and
> supports arm7.
> 
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> 
> ---
> v6:
> * Changed subject to match others in log.
> * Changed the commit description.
> * Changed the title in the .yaml from HPE BMC GXP SoC Driver to HPE BMC
>   GXP platforms.
> * Added the oneOf attribute
> * Fixed commit description to be close to 75 characters per line
> 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      | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/hpe,gxp.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v6 7/8] ARM: dts: Introduce HPE GXP Device tree

[Arnd Bergmann]

On Tue, May 3, 2022 at 5:07 PM Hawkins, Nick <nick.hawkins@hpe.com> wrote:
> On Mon, May 2, 2022 at 10:40 PM <nick.hawkins@hpe.com> wrote:
> > > +++ b/arch/arm/boot/dts/hpe-bmc-dl360gen10.dts
>
> > In the board specific file, you normally have "aliases", "chosen" and "memory"
> > nodes that define e.g. which uart is used for the console.
>
> This is what I have come up with.
>
> / {
>         #address-cells = <1>;
>         #size-cells = <1>;
>         compatible = "hpe,gxp-dl360gen10", "hpe,gxp";
>         model = "Hewlett Packard Enterprise ProLiant dl360 Gen10";
>
>         aliases {
>                 serial2 = &uartc;
>         };

It's a bit unusual to have a serial2 uart when there is no serial0 or serial1.
Most boards that only have a single uart label it either '0' or '1'.
>
>         chosen {
>                 bootargs = "console=ttyS2,115200";
>         };

Use 'stdout-path' to pick the console device here, not the argument
string.

>
>         memory@40000000 {
>                 device_type = "memory";
>                 reg = <0x40000000 0x20000000>;
>         };
> };


> > > +       memory@40000000 {
> > > +               device_type = "memory";
> > > +               reg = <0x40000000 0x20000000>;
> > > +       };
>
> > If the memory is outside of the SoC, it should not be here.
>
> It is part of the SoC. Should I put it in the axi bus instead? If so would
> I need to create an axi for the .dts as well?

That's fine then. The memory node has to be at the root of the DT, not in a
child node. It's just that most SoCs have the memory in a separate chip
on the board, rather than inside of the SoC itself, so it would be part
of the board file. If the gxp package includes the memory and it's not
an option for the board design, you can leave it in the .dtsi file.

         Arnd

Re: [PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Arnd Bergmann]

On Tue, May 3, 2022 at 6:53 PM Guenter Roeck <linux@roeck-us.net> wrote:

> > Just to clarify for my understanding are you asking that in the device structure I use the "void *platform_data" to pass "struct *resource"? If I am incorrect here can you elaborate on what you would like to be done? Based on feedback in review for the device tree; the watchdog is being created as a child to the timer. Therefore the conclusion reached was there should not be a gxp-wdt listed in the device tree files. I took this implementation based on what I found in ixp4xx_wdt.c.
> >
>
> One bad deed tends to multiply.
>
> No, I didn't ask to pass a struct resource as platform data.
> That would be no different to the current code. Resources
> can be added to a platform device using
> platform_device_add_resources(), and the platform driver
> can then use platform_get_resource() to use it. This
> would make it independent of a "private" mechanism.

Unfortunately there is no resource type for __iomem tokens,
only for physical addresses, so you'd end up having to do
ioremap() of the same address twice to map it into both the
timer and the watchdog driver . Not the end of the world
of course, but that doesn't seem much better than abusing the
device private data.

       Arnd

RE: [PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Hawkins, Nick]

On Tue, May 3, 2022 at 6:53 PM Guenter Roeck <linux@roeck-us.net> wrote:

> > One bad deed tends to multiply.
> >
> > No, I didn't ask to pass a struct resource as platform data.
> > That would be no different to the current code. Resources can be added 
> > to a platform device using platform_device_add_resources(), and the 
> > platform driver can then use platform_get_resource() to use it. This 
> > would make it independent of a "private" mechanism.

> Unfortunately there is no resource type for __iomem tokens, only for physical addresses, so you'd end up having to do
ioremap() of the same address twice to map it into both the timer and the watchdog driver . Not the end of the world of course, but that doesn't seem much better than abusing the device private data.

Hello Guenter,

Given Arnd's feedback would you like me to proceed with this change still or do you have another recommendation?

Thanks,

-Nick Hawkins

Re: [PATCH v6 3/8] watchdog: hpe-wdt: Introduce HPE GXP Watchdog

[Guenter Roeck]

On Wed, May 04, 2022 at 04:25:59PM +0000, Hawkins, Nick wrote:
> 
> On Tue, May 3, 2022 at 6:53 PM Guenter Roeck <linux@roeck-us.net> wrote:
> 
> > > One bad deed tends to multiply.
> > >
> > > No, I didn't ask to pass a struct resource as platform data.
> > > That would be no different to the current code. Resources can be added 
> > > to a platform device using platform_device_add_resources(), and the 
> > > platform driver can then use platform_get_resource() to use it. This 
> > > would make it independent of a "private" mechanism.
> 
> > Unfortunately there is no resource type for __iomem tokens, only for physical addresses, so you'd end up having to do
> ioremap() of the same address twice to map it into both the timer and the watchdog driver . Not the end of the world of course, but that doesn't seem much better than abusing the device private data.
> 
> Hello Guenter,
> 
> Given Arnd's feedback would you like me to proceed with this change still or do you have another recommendation?
> 

Just leave it as is and add a note explaining why it is done
that way.

Guenter