Re: [PATCH v3] platform/x86: portwell-ec: Add GPIO and WDT driver for Portwell EC
From: Guenter Roeck
Date: Thu Apr 10 2025 - 08:25:13 EST
On 4/10/25 05:07, Ilpo Järvinen wrote:
On Thu, 10 Apr 2025, Yen-Chi Huang wrote:
Adds a driver for the ITE Embedded Controller (EC) on Portwell boards.
It integrates with the Linux GPIO and watchdog subsystems to provide:
- Control/monitoring of up to 8 EC GPIO pins.
- Hardware watchdog timer with 1-255 second timeouts.
The driver communicates with the EC via I/O port 0xe300 and identifies
the hardware by the "PWG" firmware signature. This enables enhanced
system management for Portwell embedded/industrial platforms.
Signed-off-by: Yen-Chi Huang <jesse.huang@xxxxxxxxxxxxxxx>
---
V2->V3:
- Reworked based on review from Bartosz Golaszewski
- Changed to use platform_driver and platform_device
- Updated GPIO to use .set_rv() instead of .set()
- Used devm_ helpers for request_region, GPIO and watchdog registration
V1->V2:
- Addressed review comments from Ilpo Jarvinen
- Add DMI system check to avoid running on unsupported platforms
- Add 'force' module parameter to override DMI matching
- Use request_region() to claim I/O port access
- Extend WDT timeout handling to use both minute and second registers
- Increase WDT max timeout from 255s to 15300s
- Use named defines for WDT enable/disable
- Remove dummy pr_info() messages
- Fix several coding style issues (comments, alignment, spacing)
---
MAINTAINERS | 6 +
drivers/platform/x86/Kconfig | 14 ++
drivers/platform/x86/Makefile | 3 +
drivers/platform/x86/portwell-ec.c | 292 +++++++++++++++++++++++++++++
4 files changed, 315 insertions(+)
create mode 100644 drivers/platform/x86/portwell-ec.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d5dfb9186962..c52f819786dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19132,6 +19132,12 @@ F: kernel/time/itimer.c
F: kernel/time/posix-*
F: kernel/time/namespace.c
+PORTWELL EC DRIVER
+M: Yen-Chi Huang <jesse.huang@xxxxxxxxxxxxxxx>
+L: platform-driver-x86@xxxxxxxxxxxxxxx
+S: Maintained
+F: drivers/platform/x86/portwell-ec.c
+
POWER MANAGEMENT CORE
M: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
L: linux-pm@xxxxxxxxxxxxxxx
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 43407e76476b..2f26d1bf0a75 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -779,6 +779,20 @@ config PCENGINES_APU2
To compile this driver as a module, choose M here: the module
will be called pcengines-apuv2.
+config PORTWELL_EC
+ tristate "Portwell Embedded Controller driver"
+ depends on X86 && HAS_IOPORT && WATCHDOG && GPIOLIB
+ help
+ This driver provides support for the GPIO pins and watchdog timer
+ embedded in Portwell's EC.
+
+ Theoretically, this driver should work on multiple Portwell platforms,
+ but it has only been tested on the Portwell NANO-6064 board.
+ If you encounter any issues on other boards, please report them.
+
+ To compile this driver as a module, choose M here: the module
+ will be called portwell-ec.
+
config BARCO_P50_GPIO
tristate "Barco P50 GPIO driver for identify LED/button"
depends on GPIOLIB
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 650dfbebb6c8..83dd82e04457 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -92,6 +92,9 @@ obj-$(CONFIG_XO1_RFKILL) += xo1-rfkill.o
# PC Engines
obj-$(CONFIG_PCENGINES_APU2) += pcengines-apuv2.o
+# Portwell
+obj-$(CONFIG_PORTWELL_EC) += portwell-ec.o
+
# Barco
obj-$(CONFIG_BARCO_P50_GPIO) += barco-p50-gpio.o
diff --git a/drivers/platform/x86/portwell-ec.c b/drivers/platform/x86/portwell-ec.c
new file mode 100644
index 000000000000..7a60ced0c984
--- /dev/null
+++ b/drivers/platform/x86/portwell-ec.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * portwell-ec.c: Portwell embedded controller driver.
+ *
+ * Tested on:
+ * - Portwell NANO-6064
+ *
+ * This driver provides support for GPIO and Watchdog Timer
+ * functionalities of the Portwell boards with ITE embedded controller (EC).
+ * The EC is accessed through I/O ports and provides:
+ * - 8 GPIO pins for control and monitoring
+ * - Hardware watchdog with 1-15300 second timeout range
+ *
+ * It integrates with the Linux GPIO and Watchdog subsystems, allowing
+ * userspace interaction with EC GPIO pins and watchdog control,
+ * ensuring system stability and configurability.
+ *
+ * (C) Copyright 2025 Portwell, Inc.
+ * Author: Yen-Chi Huang (jesse.huang@xxxxxxxxxxxxxxx)
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/acpi.h>
+#include <linux/bitfield.h>
+#include <linux/dmi.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+#include <linux/watchdog.h>
+
+#define PORTWELL_EC_IOSPACE 0xe300
+#define PORTWELL_EC_IOSPACE_LEN 0x100
SZ_256 + add #include for it.
+
+#define PORTWELL_GPIO_PINS 8
+#define PORTWELL_GPIO_DIR_REG 0x2b
+#define PORTWELL_GPIO_VAL_REG 0x2c
+
+#define PORTWELL_WDT_EC_CONFIG_ADDR 0x06
+#define PORTWELL_WDT_CONFIG_ENABLE 0x1
+#define PORTWELL_WDT_CONFIG_DISABLE 0x0
Align values.
+#define PORTWELL_WDT_EC_COUNT_MIN_ADDR 0x07
+#define PORTWELL_WDT_EC_COUNT_SEC_ADDR 0x08
+#define PORTWELL_WDT_EC_MAX_COUNT_SECOND 15300 //255*60secs
Move the formula from the comment to the define itself. While doing so,
you need to add () around it and add spaces around *.
+
+#define PORTWELL_EC_FW_VENDOR_ADDRESS 0x4d
+#define PORTWELL_EC_FW_VENDOR_LENGTH 3
+#define PORTWELL_EC_FW_VENDOR_NAME "PWG"
+
+static bool force;
+module_param(force, bool, 0444);
+MODULE_PARM_DESC(force, "Force loading ec driver without checking DMI boardname");
EC
+static const struct dmi_system_id pwec_dmi_table[] = {
+ {
+ .ident = "NANO-6064 series",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_NAME, "NANO-6064"),
+ },
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(dmi, pwec_dmi_table);
+
+/* Functions for access EC via IOSPACE*/
+
+static void pwec_write(u8 index, u8 data)
+{
+ outb(data, PORTWELL_EC_IOSPACE + index);
+}
+
+static u8 pwec_read(u8 address)
+{
+ return inb(PORTWELL_EC_IOSPACE + address);
+}
+
+/* GPIO functions*/
Missing space. Please check all your comments as the one above seems to
have the same lack of space at the end.
+
+static int pwec_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ return (pwec_read(PORTWELL_GPIO_VAL_REG) & (1 << offset)) ? 1 : 0;
+}
+
+static int pwec_gpio_set_rv(struct gpio_chip *chip, unsigned int offset, int val)
+{
+ u8 tmp = pwec_read(PORTWELL_GPIO_VAL_REG);
+
+ if (val)
+ tmp |= (1 << offset);
+ else
+ tmp &= ~(1 << offset);
+ pwec_write(PORTWELL_GPIO_VAL_REG, tmp);
Add empty line here.
+ return 0;
+}
+
+static int pwec_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+ u8 direction = pwec_read(PORTWELL_GPIO_DIR_REG) & (1 << offset);
+
+ if (direction)
+ return GPIO_LINE_DIRECTION_IN;
+ else
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+/*
+ * Changing direction causes issues on some boards,
+ * so direction_input and direction_output are disabled for now.
+ */
+
+static int pwec_gpio_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+ return -EOPNOTSUPP;
+}
+
+static int pwec_gpio_direction_output(struct gpio_chip *gc, unsigned int offset, int value)
+{
+ return -EOPNOTSUPP;
+}
+
+static struct gpio_chip pwec_gpio_chip = {
+ .label = "portwell-ec-gpio",
+ .get_direction = pwec_gpio_get_direction,
+ .direction_input = pwec_gpio_direction_input,
+ .direction_output = pwec_gpio_direction_output,
+ .get = pwec_gpio_get,
+ .set_rv = pwec_gpio_set_rv,
+ .base = -1,
+ .ngpio = PORTWELL_GPIO_PINS,
+};
+
+/* Watchdog functions*/
+
+static int pwec_wdt_trigger(struct watchdog_device *wdd)
+{
+ int retry = 10;
+ u8 min, sec;
+ unsigned int timeout;
+
+ do {
+ pwec_write(PORTWELL_WDT_EC_COUNT_MIN_ADDR, wdd->timeout / 60);
+ pwec_write(PORTWELL_WDT_EC_COUNT_SEC_ADDR, wdd->timeout % 60);
+ pwec_write(PORTWELL_WDT_EC_CONFIG_ADDR, PORTWELL_WDT_CONFIG_ENABLE);
+ min = pwec_read(PORTWELL_WDT_EC_COUNT_MIN_ADDR);
+ sec = pwec_read(PORTWELL_WDT_EC_COUNT_SEC_ADDR);
+ timeout = min * 60 + sec;
This readback could reuse pwec_wdt_get_timeleft().
+ retry--;
Decrementing could be done within the condition that checks it.
+ } while (timeout != wdd->timeout && retry >= 0);
Is this write until timeout matches the one written typical thing for
watchdog drivers, or is there something specific to this HW you should
comment + note in the changelog so it is recorded for future readers of
this code?
No, this is absolutely not typical, and it has nothing to do with watchdog
drivers in the first place. If the code was in drivers/watchdog/ I'd
request a detailed comment explaining why it is needed.
Guenter