From: Nick Hawkins <[email protected]>
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 <[email protected]>
---
arch/arm/Kconfig | 2 ++
arch/arm/Makefile | 1 +
arch/arm/mach-hpe/Kconfig | 20 +++++++++++++
arch/arm/mach-hpe/Makefile | 1 +
arch/arm/mach-hpe/gxp.c | 61 ++++++++++++++++++++++++++++++++++++++
5 files changed, 85 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 4c97cb40eebb..6998b5b5f59e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -618,6 +618,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 77172d555c7e..1cc0523d0a0a 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -178,6 +178,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..9b27f97c6536
--- /dev/null
+++ b/arch/arm/mach-hpe/Kconfig
@@ -0,0 +1,20 @@
+menuconfig ARCH_HPE
+ bool "HPE SoC support"
+ help
+ This enables support for HPE ARM based SoC chips
+if ARCH_HPE
+
+config ARCH_HPE_GXP
+ bool "HPE GXP SoC"
+ select ARM_VIC
+ select PINCTRL
+ select IRQ_DOMAIN
+ select GENERIC_IRQ_CHIP
+ select MULTI_IRQ_HANDLER
+ select SPARSE_IRQ
+ select CLKSRC_MMIO
+ depends on ARCH_MULTI_V7
+ 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..44f6d719a346
--- /dev/null
+++ b/arch/arm/mach-hpe/gxp.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.*/
+
+
+#include <linux/init.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+
+#define IOP_REGS_PHYS_BASE 0xc0000000
+#define IOP_REGS_VIRT_BASE 0xf0000000
+#define IOP_REGS_SIZE (240*SZ_1M)
+#define RESET_CMD 0x00080002
+
+static struct map_desc gxp_io_desc[] __initdata = {
+ {
+ .virtual = (unsigned long)IOP_REGS_VIRT_BASE,
+ .pfn = __phys_to_pfn(IOP_REGS_PHYS_BASE),
+ .length = IOP_REGS_SIZE,
+ .type = MT_DEVICE,
+ },
+};
+
+void __init gxp_map_io(void)
+{
+ iotable_init(gxp_io_desc, ARRAY_SIZE(gxp_io_desc));
+}
+
+static void __init gxp_dt_init(void)
+{
+ void __iomem *gxp_init_regs;
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "hpe,gxp-cpu-init");
+ gxp_init_regs = of_iomap(np, 0);
+
+ /*it is necessary for our SOC to reset ECHI through this*/
+ /* register due to a hardware limitation*/
+ __raw_writel(RESET_CMD,
+ (gxp_init_regs));
+
+}
+
+static void gxp_restart(enum reboot_mode mode, const char *cmd)
+{
+ __raw_writel(1, (void __iomem *) IOP_REGS_VIRT_BASE);
+}
+
+static const char * const gxp_board_dt_compat[] = {
+ "hpe,gxp",
+ NULL,
+};
+
+DT_MACHINE_START(GXP_DT, "HPE GXP")
+ .init_machine = gxp_dt_init,
+ .map_io = gxp_map_io,
+ .restart = gxp_restart,
+ .dt_compat = gxp_board_dt_compat,
+MACHINE_END
--
2.17.1
From: Nick Hawkins <[email protected]>
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.
Signed-off-by: Nick Hawkins <[email protected]>
---
drivers/watchdog/Kconfig | 8 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/gxp-wdt.c | 191 +++++++++++++++++++++++++++++++++++++
3 files changed, 200 insertions(+)
create mode 100644 drivers/watchdog/gxp-wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c8fa79da23b3..cb210d2978d2 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1820,6 +1820,14 @@ 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.
+
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..d2b489cb4774
--- /dev/null
+++ b/drivers/watchdog/gxp-wdt.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/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)
+
+struct gxp_wdt {
+ void __iomem *counter;
+ void __iomem *control;
+ struct watchdog_device wdd;
+};
+
+static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata)
+{
+ uint8_t val;
+
+ val = readb(drvdata->control);
+ val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD);
+ writeb(val, drvdata->control);
+}
+
+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->counter);
+ gxp_wdt_enable_reload(drvdata);
+ return 0;
+}
+
+static int gxp_wdt_stop(struct watchdog_device *wdd)
+{
+ struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+ uint8_t val;
+
+ val = readb_relaxed(drvdata->control);
+ val &= ~MASK_WDGCS_ENABLE;
+ writeb(val, drvdata->control);
+ return 0;
+}
+
+static int gxp_wdt_set_timeout(struct watchdog_device *wdd,
+ unsigned int timeout)
+{
+ struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+ uint32_t actual;
+
+ wdd->timeout = timeout;
+ actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
+ writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter);
+
+ return 0;
+}
+
+static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
+ uint32_t val = readw(drvdata->counter);
+
+ 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->counter);
+ 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 resource *res;
+ struct device *dev = &pdev->dev;
+ struct gxp_wdt *drvdata;
+ int err;
+ uint8_t val;
+
+ drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, drvdata);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ drvdata->counter = devm_ioremap_resource(dev, res);
+ if (IS_ERR(drvdata->counter))
+ return PTR_ERR(drvdata->counter);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ drvdata->control = devm_ioremap_resource(dev, res);
+ if (IS_ERR(drvdata->control))
+ return PTR_ERR(drvdata->control);
+
+ 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 = &pdev->dev;
+
+ watchdog_set_drvdata(&drvdata->wdd, drvdata);
+ watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev);
+ watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT);
+
+ val = readb(drvdata->control);
+ 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 int gxp_wdt_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id gxp_wdt_of_match[] = {
+ { .compatible = "hpe,gxp-wdt", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gxp_wdt_of_match);
+
+static struct platform_driver gxp_wdt_driver = {
+ .probe = gxp_wdt_probe,
+ .remove = gxp_wdt_remove,
+ .driver = {
+ .name = "gxp-wdt",
+ .of_match_table = gxp_wdt_of_match,
+ },
+};
+module_platform_driver(gxp_wdt_driver);
+
+MODULE_AUTHOR("Nick Hawkins <[email protected]>");
+MODULE_AUTHOR("Jean-Marie Verdun <[email protected]>");
+MODULE_DESCRIPTION("Driver for GXP watchdog timer");
--
2.17.1
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/timers/core]
[also build test WARNING on soc/for-next robh/for-next linus/master v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 58dedf0a4782ce42b4d31f1f62e5ad80a1b73d72
config: arm-defconfig (https://download.01.org/0day-ci/archive/20220311/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/0day-ci/linux/commit/9fbfc32473a65e025764e0a1456c421b4706fe8e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
git checkout 9fbfc32473a65e025764e0a1456c421b4706fe8e
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> arch/arm/mach-hpe/gxp.c:26:13: warning: no previous prototype for function 'gxp_map_io' [-Wmissing-prototypes]
void __init gxp_map_io(void)
^
arch/arm/mach-hpe/gxp.c:26:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void __init gxp_map_io(void)
^
static
1 warning generated.
vim +/gxp_map_io +26 arch/arm/mach-hpe/gxp.c
25
> 26 void __init gxp_map_io(void)
27 {
28 iotable_init(gxp_io_desc, ARRAY_SIZE(gxp_io_desc));
29 }
30
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/[email protected]
From: Nick Hawkins <[email protected]>
Create a section in MAINTAINERS for the GXP HPE
architecture.
Signed-off-by: Nick Hawkins <[email protected]>
---
MAINTAINERS | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6db79f3b209e..f227d76ec43d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8791,6 +8791,20 @@ L: [email protected]
S: Orphan
F: drivers/platform/x86/tc1100-wmi.c
+HPE GXP ARCHITECTURE
+M: Jean-Marie Verdun <[email protected]>
+M: Nick Hawkins <[email protected]>
+S: MAINTAINED
+F: Documentation/devicetree/bindings/arm/cpu-enable-method/hpe,gxp-cpu-init.yaml
+F: Documentation/devicetree/bindings/arm/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/gxp-timer.c
+F: drivers/watchdog/gxp-wdt.c
+
HPET: High Precision Event Timers driver
M: Clemens Ladisch <[email protected]>
S: Maintained
--
2.17.1
From: Nick Hawkins <[email protected]>
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 <[email protected]>
---
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 8863fa969ede..b93d213b7e60 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -1006,6 +1006,8 @@ CONFIG_QCOM_GSBI=y
CONFIG_QCOM_SMD_RPM=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
@@ -1169,3 +1171,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
From: Nick Hawkins <[email protected]>
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 <[email protected]>
---
.../devicetree/bindings/arm/gxp.yaml | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/gxp.yaml
diff --git a/Documentation/devicetree/bindings/arm/gxp.yaml b/Documentation/devicetree/bindings/arm/gxp.yaml
new file mode 100644
index 000000000000..edfd331c493e
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gxp.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/gxp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HPE bmc GXP SoC driver
+
+maintainers:
+ - Nick Hawkins <[email protected]>
+ - Jean-Marie Verdun <[email protected]>
+
+properties:
+ compatible:
+ const: hpe,gxp
+
+ "#address-cells":
+ const: 1
+
+required:
+ - compatible
+
+additionalProperties: true
+
+examples:
+ - |
+ / {
+ model = "Hewlett Packard Enterprise GXP BMC";
+ compatible = "hpe,gxp";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
+ compatible = "arm,cortex-a9";
+ device_type = "cpu";
+ reg = <0>;
+ };
+ };
+
+ memory@40000000 {
+ device_type = "memory";
+ reg = <0x40000000 0x20000000>;
+ };
+
+ ahb {
+
+ };
+ };
+
+...
--
2.17.1
On Thu, Mar 10, 2022 at 8:52 PM <[email protected]> wrote:
>
> From: Nick Hawkins <[email protected]>
>
> 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 <[email protected]>
Please add me to Cc for the entire series when resending.
> +config ARCH_HPE_GXP
> + bool "HPE GXP SoC"
> + select ARM_VIC
> + select PINCTRL
> + select IRQ_DOMAIN
> + select GENERIC_IRQ_CHIP
> + select MULTI_IRQ_HANDLER
> + select SPARSE_IRQ
> + select CLKSRC_MMIO
> + depends on ARCH_MULTI_V7
By convention, the 'depends on' usually comes first here.
Please drop the 'select' statements for things that are already selected
by ARCH_MULTIPLATFORM or ARCH_MULTI_V7.
> +
> +#define IOP_REGS_PHYS_BASE 0xc0000000
> +#define IOP_REGS_VIRT_BASE 0xf0000000
> +#define IOP_REGS_SIZE (240*SZ_1M)
> +#define RESET_CMD 0x00080002
> +
> +static struct map_desc gxp_io_desc[] __initdata = {
> + {
> + .virtual = (unsigned long)IOP_REGS_VIRT_BASE,
> + .pfn = __phys_to_pfn(IOP_REGS_PHYS_BASE),
> + .length = IOP_REGS_SIZE,
> + .type = MT_DEVICE,
> + },
> +};
It looks like this is only used for the pxf_restart() function below.
In this case, you should get rid of the static mapping entirely and
use an ioremap() in the gxp_dt_init() function instead, ideally getting
the address from an appropriate device node rather than hardcoding
it here.
If there are other drivers using the static mapping, either explain
here why this is required, or try to change them to dynamic mappings as well.
> +static void __init gxp_dt_init(void)
> +{
> + void __iomem *gxp_init_regs;
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "hpe,gxp-cpu-init");
> + gxp_init_regs = of_iomap(np, 0);
> +
> + /*it is necessary for our SOC to reset ECHI through this*/
> + /* register due to a hardware limitation*/
> + __raw_writel(RESET_CMD,
> + (gxp_init_regs));
My feeling is still that this should be done in the platform specific EHCI
driver front-end. I think I commented on this before but don't remember
getting an explanation why you can't have it there.
> +static void gxp_restart(enum reboot_mode mode, const char *cmd)
> +{
> + __raw_writel(1, (void __iomem *) IOP_REGS_VIRT_BASE);
> +}
With both of these, you should use writel() instead of __raw_write().
Using the __raw accessors breaks big-endian kernels (which you
probably don't need, but shouldn't break for no reason anyway), and
lacks serialization and atomicity of the access.
A better place for the restart logic may be a separate driver in
drivers/power/reset/, at least if this otherwise ends up being the only
platform specific code you need.
Arnd
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tip/timers/core]
[also build test WARNING on soc/for-next robh/for-next linus/master v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 58dedf0a4782ce42b4d31f1f62e5ad80a1b73d72
config: arm-defconfig (https://download.01.org/0day-ci/archive/20220311/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/9fbfc32473a65e025764e0a1456c421b4706fe8e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
git checkout 9fbfc32473a65e025764e0a1456c421b4706fe8e
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> arch/arm/mach-hpe/gxp.c:26:13: warning: no previous prototype for 'gxp_map_io' [-Wmissing-prototypes]
26 | void __init gxp_map_io(void)
| ^~~~~~~~~~
vim +/gxp_map_io +26 arch/arm/mach-hpe/gxp.c
25
> 26 void __init gxp_map_io(void)
27 {
28 iotable_init(gxp_io_desc, ARRAY_SIZE(gxp_io_desc));
29 }
30
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/[email protected]
On Thu, 2022-03-10 at 13:52 -0600, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> Create a section in MAINTAINERS for the GXP HPE
> architecture.
[]
> diff --git a/MAINTAINERS b/MAINTAINERS
[]
> @@ -8791,6 +8791,20 @@ L: [email protected]
> S: Orphan
> F: drivers/platform/x86/tc1100-wmi.c
>
> +HPE GXP ARCHITECTURE
> +M: Jean-Marie Verdun <[email protected]>
> +M: Nick Hawkins <[email protected]>
> +S: MAINTAINED
Use the case sensitive version instead
S: Maintained
$ git grep -h '^S:' MAINTAINERS | sort | uniq -c | sort -rn
1682 S: Maintained
637 S: Supported
71 S: Odd Fixes
64 S: Orphan
19 S: Odd fixes
7 S: Orphan / Obsolete
2 S: Obsolete
1 S: Buried alive in reporters
From: Nick Hawkins <[email protected]>
Add the hpe gxp watchdog timer binding hpe,gxp-wdt.
This will enable support for the HPE GXP Watchdog
Signed-off-by: Nick Hawkins <[email protected]>
---
.../bindings/watchdog/hpe,gxp-wdt.yaml | 37 +++++++++++++++++++
1 file changed, 37 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..6044496b4968
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/hpe,gxp-wdt.yaml
@@ -0,0 +1,37 @@
+# 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 <[email protected]>
+ - Jean-Marie Verdun <[email protected]>
+
+properties:
+ compatible:
+ const: hpe,gxp-wdt
+
+ reg:
+ items:
+ - description: WDGRST register
+ - description: WDGCS register
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ watchdog0: watchdog@c0000090 {
+ compatible = "hpe,gxp-wdt";
+ reg = <0xc0000090 0x02>, <0xc0000096 0x01>;
+ };
+
--
2.17.1
Hi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on tip/timers/core]
[also build test ERROR on soc/for-next robh/for-next linus/master v5.17-rc7 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 58dedf0a4782ce42b4d31f1f62e5ad80a1b73d72
config: arm-randconfig-c002-20220312 (https://download.01.org/0day-ci/archive/20220312/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/9fbfc32473a65e025764e0a1456c421b4706fe8e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review nick-hawkins-hpe-com/arch-arm-mach-hpe-Introduce-the-HPE-GXP-architecture/20220311-035513
git checkout 9fbfc32473a65e025764e0a1456c421b4706fe8e
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
drivers/memstick/host/r592.c:47:13: warning: no previous prototype for 'memstick_debug_get_tpc_name' [-Wmissing-prototypes]
47 | const char *memstick_debug_get_tpc_name(int tpc)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
init/main.c:768:20: warning: no previous prototype for 'arch_post_acpi_subsys_init' [-Wmissing-prototypes]
768 | void __init __weak arch_post_acpi_subsys_init(void) { }
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
init/main.c:780:20: warning: no previous prototype for 'mem_encrypt_init' [-Wmissing-prototypes]
780 | void __init __weak mem_encrypt_init(void) { }
| ^~~~~~~~~~~~~~~~
init/main.c:782:20: warning: no previous prototype for 'poking_init' [-Wmissing-prototypes]
782 | void __init __weak poking_init(void) { }
| ^~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
init/calibrate.c:261:37: warning: no previous prototype for 'calibrate_delay_is_known' [-Wmissing-prototypes]
261 | unsigned long __attribute__((weak)) calibrate_delay_is_known(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/ptrace.c:854:16: warning: no previous prototype for 'syscall_trace_enter' [-Wmissing-prototypes]
854 | asmlinkage int syscall_trace_enter(struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~~~~
arch/arm/kernel/ptrace.c:882:17: warning: no previous prototype for 'syscall_trace_exit' [-Wmissing-prototypes]
882 | asmlinkage void syscall_trace_exit(struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/reboot.c:78:6: warning: no previous prototype for 'soft_restart' [-Wmissing-prototypes]
78 | void soft_restart(unsigned long addr)
| ^~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/signal.c:186:16: warning: no previous prototype for 'sys_sigreturn' [-Wmissing-prototypes]
186 | asmlinkage int sys_sigreturn(struct pt_regs *regs)
| ^~~~~~~~~~~~~
arch/arm/kernel/signal.c:216:16: warning: no previous prototype for 'sys_rt_sigreturn' [-Wmissing-prototypes]
216 | asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~
arch/arm/kernel/signal.c:601:1: warning: no previous prototype for 'do_work_pending' [-Wmissing-prototypes]
601 | do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall)
| ^~~~~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/sys_arm.c:32:17: warning: no previous prototype for 'sys_arm_fadvise64_64' [-Wmissing-prototypes]
32 | asmlinkage long sys_arm_fadvise64_64(int fd, int advice,
| ^~~~~~~~~~~~~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/time.c:88:13: warning: no previous prototype for 'time_init' [-Wmissing-prototypes]
88 | void __init time_init(void)
| ^~~~~~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/traps.c:84:6: warning: no previous prototype for 'dump_backtrace_stm' [-Wmissing-prototypes]
84 | void dump_backtrace_stm(u32 *stack, u32 instruction, const char *loglvl)
| ^~~~~~~~~~~~~~~~~~
arch/arm/kernel/traps.c:433:17: warning: no previous prototype for 'do_undefinstr' [-Wmissing-prototypes]
433 | asmlinkage void do_undefinstr(struct pt_regs *regs)
| ^~~~~~~~~~~~~
arch/arm/kernel/traps.c:498:39: warning: no previous prototype for 'handle_fiq_as_nmi' [-Wmissing-prototypes]
498 | asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~~
arch/arm/kernel/traps.c:517:17: warning: no previous prototype for 'bad_mode' [-Wmissing-prototypes]
517 | asmlinkage void bad_mode(struct pt_regs *regs, int reason)
| ^~~~~~~~
arch/arm/kernel/traps.c:590:16: warning: no previous prototype for 'arm_syscall' [-Wmissing-prototypes]
590 | asmlinkage int arm_syscall(int no, struct pt_regs *regs)
| ^~~~~~~~~~~
arch/arm/kernel/traps.c:716:1: warning: no previous prototype for 'baddataabort' [-Wmissing-prototypes]
716 | baddataabort(int code, unsigned long instr, struct pt_regs *regs)
| ^~~~~~~~~~~~
arch/arm/kernel/traps.c:756:17: warning: no previous prototype for '__div0' [-Wmissing-prototypes]
756 | asmlinkage void __div0(void)
| ^~~~~~
arch/arm/kernel/traps.c:763:6: warning: no previous prototype for 'abort' [-Wmissing-prototypes]
763 | void abort(void)
| ^~~~~
--
>> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
arch/arm/kernel/suspend.c:75:6: warning: no previous prototype for '__cpu_suspend_save' [-Wmissing-prototypes]
75 | void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
| ^~~~~~~~~~~~~~~~~~
..
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/[email protected]
On Sat, Mar 12, 2022 at 2:27 PM kernel test robot <[email protected]> wrote:
>
> All error/warnings (new ones prefixed by >>):
>
> >> cc1: warning: arch/arm/mach-hpe/include: No such file or directory [-Wmissing-include-dirs]
You need to make CONFIG_ARCH_HPE depend on CONFIG_ARCH_MULTI_V7, otherwise
it becomes possible to select this for non-multiplatform configs that expect
an include directory below the platform.
Arnd
On 4/4/22 09:25, Hawkins, Nick wrote:
>
>
> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]]
> Sent: Monday, April 4, 2022 9:29 AM
> To: Hawkins, Nick <[email protected]>
> Cc: Verdun, Jean-Marie <[email protected]>; Wim Van Sebroeck <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v3 03/10] drivers: wdt: Introduce HPE GXP SoC Watchdog
>
> On Thu, Mar 10, 2022 at 01:52:22PM -0600, [email protected] wrote:
>>> From: Nick Hawkins <[email protected]>
>>>
>>> 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.
>>>
>>> Signed-off-by: Nick Hawkins <[email protected]>
>>> ---
>>> drivers/watchdog/Kconfig | 8 ++
>>> drivers/watchdog/Makefile | 1 +
>>> drivers/watchdog/gxp-wdt.c | 191
>>> +++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 200 insertions(+)
>>> create mode 100644 drivers/watchdog/gxp-wdt.c
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
>>> c8fa79da23b3..cb210d2978d2 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -1820,6 +1820,14 @@ 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.
>>> +
>>> 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..d2b489cb4774
>>> --- /dev/null
>>> +++ b/drivers/watchdog/gxp-wdt.c
>>> @@ -0,0 +1,191 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
>>> + *
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> +modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/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)
>>> +
>>> +struct gxp_wdt {
>>> + void __iomem *counter;
>>> + void __iomem *control;
>>> + struct watchdog_device wdd;
>
>> Odd variable alignment. Might as well just use spaces before the variable names.
>
> Fixed
>
>>> +};
>>> +
>>> +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) {
>>> + uint8_t val;
>>> +
>>> + val = readb(drvdata->control);
>>> + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD);
>>> + writeb(val, drvdata->control);
>>> +}
>>> +
>>> +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->counter);
>
>> Unnecessary iand confusing () around SECS_TO_WDOG_TICKS().
>
> Fixed
>
>>> + gxp_wdt_enable_reload(drvdata);
>>> + return 0;
>>> +}
>>> +
>>> +static int gxp_wdt_stop(struct watchdog_device *wdd) {
>>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
>>> + uint8_t val;
>>> +
>>> + val = readb_relaxed(drvdata->control);
>>> + val &= ~MASK_WDGCS_ENABLE;
>>> + writeb(val, drvdata->control);
>>> + return 0;
>>> +}
>>> +
>>> +static int gxp_wdt_set_timeout(struct watchdog_device *wdd,
>>> + unsigned int timeout)
>>> +{
>>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
>>> + uint32_t actual;
>
>> Please use u32 as suggested by checkpatch. Same everywhere.
>
> Fixed, checkpatch did not flag this, is there an option I should be using with checkpatch.pl?
--strict
Guenter
-----Original Message-----
From: Guenter Roeck [mailto:[email protected]]
Sent: Monday, April 4, 2022 9:29 AM
To: Hawkins, Nick <[email protected]>
Cc: Verdun, Jean-Marie <[email protected]>; Wim Van Sebroeck <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH v3 03/10] drivers: wdt: Introduce HPE GXP SoC Watchdog
On Thu, Mar 10, 2022 at 01:52:22PM -0600, [email protected] wrote:
>> From: Nick Hawkins <[email protected]>
>>
>> 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.
>>
>> Signed-off-by: Nick Hawkins <[email protected]>
>> ---
>> drivers/watchdog/Kconfig | 8 ++
>> drivers/watchdog/Makefile | 1 +
>> drivers/watchdog/gxp-wdt.c | 191
>> +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 200 insertions(+)
>> create mode 100644 drivers/watchdog/gxp-wdt.c
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
>> c8fa79da23b3..cb210d2978d2 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1820,6 +1820,14 @@ 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.
>> +
>> 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..d2b489cb4774
>> --- /dev/null
>> +++ b/drivers/watchdog/gxp-wdt.c
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
>> + *
>> + *
>> + * This program is free software; you can redistribute it and/or
>> +modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/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)
>> +
>> +struct gxp_wdt {
>> + void __iomem *counter;
>> + void __iomem *control;
>> + struct watchdog_device wdd;
> Odd variable alignment. Might as well just use spaces before the variable names.
Fixed
>> +};
>> +
>> +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata) {
>> + uint8_t val;
>> +
>> + val = readb(drvdata->control);
>> + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD);
>> + writeb(val, drvdata->control);
>> +}
>> +
>> +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->counter);
> Unnecessary iand confusing () around SECS_TO_WDOG_TICKS().
Fixed
>> + gxp_wdt_enable_reload(drvdata);
>> + return 0;
>> +}
>> +
>> +static int gxp_wdt_stop(struct watchdog_device *wdd) {
>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
>> + uint8_t val;
>> +
>> + val = readb_relaxed(drvdata->control);
>> + val &= ~MASK_WDGCS_ENABLE;
>> + writeb(val, drvdata->control);
>> + return 0;
>> +}
>> +
>> +static int gxp_wdt_set_timeout(struct watchdog_device *wdd,
>> + unsigned int timeout)
>> +{
>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
>> + uint32_t actual;
> Please use u32 as suggested by checkpatch. Same everywhere.
Fixed, checkpatch did not flag this, is there an option I should be using with checkpatch.pl?
>> +
>> + wdd->timeout = timeout;
>> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
>> + writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter);
> Unnecessary ()
Fixed
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd)
>> +{
>> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
>> + uint32_t val = readw(drvdata->counter);
>> +
>> + 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->counter);
>> + 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,
> Please be consistent with alignments.
Fixed.
>> +};
>> +
>> +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 resource *res;
>> + struct device *dev = &pdev->dev;
>> + struct gxp_wdt *drvdata;
>> + int err;
>> + uint8_t val;
> Please use u8.
Fixed.
>> +
>> + drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL);
>> + if (!drvdata)
>> + return -ENOMEM;
>> + platform_set_drvdata(pdev, drvdata);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + drvdata->counter = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(drvdata->counter))
>> + return PTR_ERR(drvdata->counter);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + drvdata->control = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(drvdata->control))
>> + return PTR_ERR(drvdata->control);
> Please use a single resource and offsets, or explain in a comment why that does not work for this driver.
I actually was flagged for this on our separate device tree review and have trying to determine a better solution for it. The primary issue is that there are many controls in the register region. The current suggestion I am pursuing is having the timer be the parent and watchdog be the child all in the same driver. I need to get feedback from the clocksource owners on this. For reference here is the discussion: https://lore.kernel.org/all/CAK8P3a1Cc+2oY9djdp11PuOW+TBQ0zf+p8QaDY3aerk1QqaG-g@mail.gmail.com/ Any input on this is welcome.
>> +
>> + 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 = &pdev->dev;
>> +
>> + watchdog_set_drvdata(&drvdata->wdd, drvdata);
>> + watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev);
> That disables setting the watchdog timeout through devicetree, and is thus mostly pointless. It is better to set the default timeout via variable assignment above if you don't want to use devicetree to set the timeout, or pass a timeout value of 0 to get the timeout from devicetree (if configured).
Thank you.
>> + watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT);
>> +
>> + val = readb(drvdata->control);
>> + 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 int gxp_wdt_remove(struct platform_device *pdev) {
>> + return 0;
>> +}
> Pointless remove function.
Fixed.
>> +
>> +static const struct of_device_id gxp_wdt_of_match[] = {
>> + { .compatible = "hpe,gxp-wdt", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, gxp_wdt_of_match);
>> +
>> +static struct platform_driver gxp_wdt_driver = {
>> + .probe = gxp_wdt_probe,
>> + .remove = gxp_wdt_remove,
>> + .driver = {
>> + .name = "gxp-wdt",
>> + .of_match_table = gxp_wdt_of_match,
>> + },
>> +};
>> +module_platform_driver(gxp_wdt_driver);
>> +
>> +MODULE_AUTHOR("Nick Hawkins <[email protected]>");
>> +MODULE_AUTHOR("Jean-Marie Verdun <[email protected]>");
>> +MODULE_DESCRIPTION("Driver for GXP watchdog timer");
On Thu, Mar 10, 2022 at 01:52:22PM -0600, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> 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.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> drivers/watchdog/Kconfig | 8 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/gxp-wdt.c | 191 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 200 insertions(+)
> create mode 100644 drivers/watchdog/gxp-wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index c8fa79da23b3..cb210d2978d2 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1820,6 +1820,14 @@ 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.
> +
> 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..d2b489cb4774
> --- /dev/null
> +++ b/drivers/watchdog/gxp-wdt.c
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/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)
> +
> +struct gxp_wdt {
> + void __iomem *counter;
> + void __iomem *control;
> + struct watchdog_device wdd;
Odd variable alignment. Might as well just use spaces before the
variable names.
> +};
> +
> +static void gxp_wdt_enable_reload(struct gxp_wdt *drvdata)
> +{
> + uint8_t val;
> +
> + val = readb(drvdata->control);
> + val |= (MASK_WDGCS_ENABLE | MASK_WDGCS_RELOAD);
> + writeb(val, drvdata->control);
> +}
> +
> +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->counter);
Unnecessary iand confusing () around SECS_TO_WDOG_TICKS().
> + gxp_wdt_enable_reload(drvdata);
> + return 0;
> +}
> +
> +static int gxp_wdt_stop(struct watchdog_device *wdd)
> +{
> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
> + uint8_t val;
> +
> + val = readb_relaxed(drvdata->control);
> + val &= ~MASK_WDGCS_ENABLE;
> + writeb(val, drvdata->control);
> + return 0;
> +}
> +
> +static int gxp_wdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
> + uint32_t actual;
Please use u32 as suggested by checkpatch. Same everywhere.
> +
> + wdd->timeout = timeout;
> + actual = min(timeout, wdd->max_hw_heartbeat_ms / 1000);
> + writew((SECS_TO_WDOG_TICKS(actual)), drvdata->counter);
Unnecessary ()
> +
> + return 0;
> +}
> +
> +static unsigned int gxp_wdt_get_timeleft(struct watchdog_device *wdd)
> +{
> + struct gxp_wdt *drvdata = watchdog_get_drvdata(wdd);
> + uint32_t val = readw(drvdata->counter);
> +
> + 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->counter);
> + 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,
Please be consistent with alignments.
> +};
> +
> +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 resource *res;
> + struct device *dev = &pdev->dev;
> + struct gxp_wdt *drvdata;
> + int err;
> + uint8_t val;
Please use u8.
> +
> + drvdata = devm_kzalloc(dev, sizeof(struct gxp_wdt), GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> + platform_set_drvdata(pdev, drvdata);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + drvdata->counter = devm_ioremap_resource(dev, res);
> + if (IS_ERR(drvdata->counter))
> + return PTR_ERR(drvdata->counter);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + drvdata->control = devm_ioremap_resource(dev, res);
> + if (IS_ERR(drvdata->control))
> + return PTR_ERR(drvdata->control);
Please use a single resource and offsets, or explain in a comment
why that does not work for this driver.
> +
> + 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 = &pdev->dev;
> +
> + watchdog_set_drvdata(&drvdata->wdd, drvdata);
> + watchdog_init_timeout(&drvdata->wdd, WDT_DEFAULT_TIMEOUT, dev);
That disables setting the watchdog timeout through devicetree, and is thus
mostly pointless. It is better to set the default timeout via variable
assignment above if you don't want to use devicetree to set the timeout,
or pass a timeout value of 0 to get the timeout from devicetree (if
configured).
> + watchdog_set_nowayout(&drvdata->wdd, WATCHDOG_NOWAYOUT);
> +
> + val = readb(drvdata->control);
> + 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 int gxp_wdt_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
Pointless remove function.
> +
> +static const struct of_device_id gxp_wdt_of_match[] = {
> + { .compatible = "hpe,gxp-wdt", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, gxp_wdt_of_match);
> +
> +static struct platform_driver gxp_wdt_driver = {
> + .probe = gxp_wdt_probe,
> + .remove = gxp_wdt_remove,
> + .driver = {
> + .name = "gxp-wdt",
> + .of_match_table = gxp_wdt_of_match,
> + },
> +};
> +module_platform_driver(gxp_wdt_driver);
> +
> +MODULE_AUTHOR("Nick Hawkins <[email protected]>");
> +MODULE_AUTHOR("Jean-Marie Verdun <[email protected]>");
> +MODULE_DESCRIPTION("Driver for GXP watchdog timer");