[PATCH] drivers: ipcbx21a: custom driver for the SIMATIC IPC BX-21A

50 views
Skip to first unread message

Cedric Hombourger

unread,
Sep 23, 2024, 5:46:06 AM9/23/24
to efibootg...@googlegroups.com, Cedric Hombourger
The BX-21A was unfortunately not designed to either use iTCO or WDAT
as we (Linux folks) are now recommending for SIMATIC IPCs. Their
custom Linux driver was consequently ported over to support this
somewhat recent SIMATIC IPC.

Signed-off-by: Cedric Hombourger <cedric.h...@siemens.com>
---
Makefile.am | 2 +
drivers/watchdog/ipcbx21a.c | 80 +++++++++++++++++++++++++++++++++++++
include/simatic.h | 1 +
3 files changed, 83 insertions(+)
create mode 100644 drivers/watchdog/ipcbx21a.c

diff --git a/Makefile.am b/Makefile.am
index 2ea47ae..638efbb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -163,6 +163,7 @@ if BOOTLOADER

if ARCH_IS_X86
# NOTE: wdat.c is placed first so it is tried before any other drivers
+# NOTE: ipcbx21a.c must be *before* itco.c
# NOTE: ipc4x7e_wdt.c must be *before* itco.c
# NOTE: ipmi_wdt.c must be *before* itco.c
efi_sources_watchdogs = \
@@ -170,6 +171,7 @@ efi_sources_watchdogs = \
drivers/watchdog/amdfch_wdt.c \
drivers/watchdog/i6300esb.c \
drivers/watchdog/atom-quark.c \
+ drivers/watchdog/ipcbx21a.c \
drivers/watchdog/ipc4x7e_wdt.c \
drivers/watchdog/w83627hf_wdt.c \
drivers/watchdog/ipmi_wdt.c \
diff --git a/drivers/watchdog/ipcbx21a.c b/drivers/watchdog/ipcbx21a.c
new file mode 100644
index 0000000..7078f40
--- /dev/null
+++ b/drivers/watchdog/ipcbx21a.c
@@ -0,0 +1,80 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2024
+ *
+ * Ported by Cedric Hombourger <cedric.h...@siemens.com>
+ * From unpublished code created by XingTong Wu <xingt...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <efi.h>
+#include <efilib.h>
+#include <pci/header.h>
+#include <sys/io.h>
+#include "simatic.h"
+#include "utils.h"
+
+#define WDT_CTRL_REG_BX_21A 0x1854
+#define TIMEOUT_MIN_BX_21A (1)
+#define TIMEOUT_DEF_BX_21A (60)
+#define TIMEOUT_MAX_BX_21A (1024)
+#define WDT_CTRL_REG_TOV_BIT_BX_21A (0)
+#define WDT_CTRL_REG_TOV_MASK_BX_21A (0x3FF << WDT_CTRL_REG_TOV_BIT_BX_21A)
+#define WDT_CTRL_REG_ICCSURV_BIT_BX_21A (13)
+#define WDT_CTRL_REG_ICCSURV_MASK_BX_21A \
+ (0x01 << WDT_CTRL_REG_ICCSURV_BIT_BX_21A)
+#define WDT_CTRL_REG_EN_BIT_BX_21A (14)
+#define WDT_CTRL_REG_EN_MASK_BX_21A (0x01 << WDT_CTRL_REG_EN_BIT_BX_21A)
+#define WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A (15)
+#define WDT_CTRL_REG_FORCE_ALL_MASK_BX_21A \
+ (0x01 << WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A)
+#define WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A (24)
+#define WDT_CTRL_REG_NO_ICCSURV_STS_MASK_BX_21A \
+ (0x01 << WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A)
+#define WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A (25)
+#define WDT_CTRL_REG_ICCSURV_STS_MASK_BX_21A \
+ (0x01 << WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A)
+
+static EFI_STATUS init(EFI_PCI_IO __attribute__((unused)) * pci_io,
+ UINT16 __attribute__((unused)) pci_vendor_id,
+ UINT16 __attribute__((unused)) pci_device_id,
+ UINTN timeout)
+{
+ UINT32 regval;
+
+ if (simatic_station_id() != SIMATIC_IPCBX_21A)
+ return EFI_UNSUPPORTED;
+
+ INFO(L"Detected SIMATIC BX-21A watchdog\n");
+
+ if (timeout < TIMEOUT_MIN_BX_21A || timeout > TIMEOUT_MAX_BX_21A) {
+ WARNING(L"Invalid timeout value (%d), default (%ds) is used.\n",
+ timeout, TIMEOUT_DEF_BX_21A);
+ timeout = TIMEOUT_DEF_BX_21A;
+ }
+
+ regval = inl(WDT_CTRL_REG_BX_21A);
+ /* setup timeout value */
+ regval &= (~WDT_CTRL_REG_TOV_MASK_BX_21A);
+ regval |= (timeout - 1) << WDT_CTRL_REG_TOV_BIT_BX_21A;
+ /* get and clear status */
+ regval |= WDT_CTRL_REG_NO_ICCSURV_STS_MASK_BX_21A;
+ regval |= WDT_CTRL_REG_ICCSURV_STS_MASK_BX_21A;
+ outl(regval, WDT_CTRL_REG_BX_21A);
+
+ /* start watchdog */
+ regval = inl(WDT_CTRL_REG_BX_21A);
+ regval |= (WDT_CTRL_REG_EN_MASK_BX_21A |
+ WDT_CTRL_REG_ICCSURV_MASK_BX_21A |
+ WDT_CTRL_REG_FORCE_ALL_MASK_BX_21A);
+ outl(regval, WDT_CTRL_REG_BX_21A);
+
+ return EFI_SUCCESS;
+}
+
+WATCHDOG_REGISTER(init);
diff --git a/include/simatic.h b/include/simatic.h
index 43c5515..192299b 100644
--- a/include/simatic.h
+++ b/include/simatic.h
@@ -25,6 +25,7 @@

#define SIMATIC_IPC427E 0x0a01
#define SIMATIC_IPC477E 0x0a02
+#define SIMATIC_IPCBX_21A 0x1101
#define SIMATIC_IPCBX_56A 0x1201
#define SIMATIC_IPCBX_59A 0x1202

--
2.34.1

Jan Kiszka

unread,
Sep 25, 2024, 4:59:47 AM9/25/24
to Cedric Hombourger, efibootg...@googlegroups.com
On 23.09.24 11:45, 'Cedric Hombourger' via EFI Boot Guard wrote:
> The BX-21A was unfortunately not designed to either use iTCO or WDAT
> as we (Linux folks) are now recommending for SIMATIC IPCs. Their
> custom Linux driver was consequently ported over to support this
> somewhat recent SIMATIC IPC.

This has to be seen also in the context of
https://lore.kernel.org/linux-watchdog/df8d53db-0056-434d...@roeck-us.net/:
WDAT cannot be used under Linux because the register bank is not
exclusive for the watchdog.
This looks weird. WDT_CTRL_REG_TOV_BIT_BX_21A should rather be a shift,
not a bit. But I would simply drop WDT_CTRL_REG_TOV_BIT_BX_21A.

> +#define WDT_CTRL_REG_ICCSURV_BIT_BX_21A (13)
> +#define WDT_CTRL_REG_ICCSURV_MASK_BX_21A \
> + (0x01 << WDT_CTRL_REG_ICCSURV_BIT_BX_21A)

Let's make the bit directly usable so that we don't have to or-in masks
- which is completely odd.

Do we have semantics for this bit as well? The name does not tell us
anything.

> +#define WDT_CTRL_REG_EN_BIT_BX_21A (14)
> +#define WDT_CTRL_REG_EN_MASK_BX_21A (0x01 << WDT_CTRL_REG_EN_BIT_BX_21A)
> +#define WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A (15)
> +#define WDT_CTRL_REG_FORCE_ALL_MASK_BX_21A \
> + (0x01 << WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A)
> +#define WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A (24)
> +#define WDT_CTRL_REG_NO_ICCSURV_STS_MASK_BX_21A \
> + (0x01 << WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A)
> +#define WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A (25)
> +#define WDT_CTRL_REG_ICCSURV_STS_MASK_BX_21A \
> + (0x01 << WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A)

Same question for the last 3 above (EN = enable is clear).
Thanks,
Jan

--
Siemens AG, Technology
Linux Expert Center

Cedric Hombourger

unread,
Nov 27, 2024, 1:58:24 PM11/27/24
to efibootg...@googlegroups.com, Cedric Hombourger
The BX-21A was unfortunately not designed to either use iTCO or WDAT
as we (Linux folks) are now recommending for SIMATIC IPCs. Their
custom Linux driver was consequently ported over to support this
somewhat recent SIMATIC IPC.

Signed-off-by: Cedric Hombourger <cedric.h...@siemens.com>
---
Makefile.am | 2 +
drivers/watchdog/ipcbx21a.c | 76 +++++++++++++++++++++++++++++++++++++
include/simatic.h | 1 +
3 files changed, 79 insertions(+)
create mode 100644 drivers/watchdog/ipcbx21a.c

diff --git a/Makefile.am b/Makefile.am
index 21e2d0d..0ceb090 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -163,6 +163,7 @@ if BOOTLOADER

if ARCH_IS_X86
# NOTE: wdat.c is placed first so it is tried before any other drivers
+# NOTE: ipcbx21a.c must be *before* itco.c
# NOTE: ipc4x7e_wdt.c must be *before* itco.c
# NOTE: ipmi_wdt.c must be *before* itco.c
efi_sources_watchdogs = \
@@ -170,6 +171,7 @@ efi_sources_watchdogs = \
drivers/watchdog/amdfch_wdt.c \
drivers/watchdog/i6300esb.c \
drivers/watchdog/atom-quark.c \
+ drivers/watchdog/ipcbx21a.c \
drivers/watchdog/ipc4x7e_wdt.c \
drivers/watchdog/w83627hf_wdt.c \
drivers/watchdog/ipmi_wdt.c \
diff --git a/drivers/watchdog/ipcbx21a.c b/drivers/watchdog/ipcbx21a.c
new file mode 100644
index 0000000..a1b8a9a
--- /dev/null
+++ b/drivers/watchdog/ipcbx21a.c
@@ -0,0 +1,76 @@
+/* Over-Clocking WDT Timeout Value */
+#define WDT_CTRL_REG_TOV_MASK_BX_21A (0x3FF)
+/* Over-Clocking WDT ICC Survivability Impact */
+#define WDT_CTRL_REG_ICCSURV_BIT_BX_21A BIT(13)
+/* Over-Clocking WDT Enable */
+#define WDT_CTRL_REG_EN_BIT_BX_21A BIT(14)
+/* Over-Clocking WDT Force All */
+#define WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A BIT(15)
+/* Over-Clocking WDT Non-ICC Survivability Mode Timeout Status */
+#define WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A BIT(24)
+/* Over-Clocking WDT ICC Survivability Mode Timeout Status */
+#define WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A BIT(25)
+
+static EFI_STATUS init(EFI_PCI_IO __attribute__((unused)) * pci_io,
+ UINT16 __attribute__((unused)) pci_vendor_id,
+ UINT16 __attribute__((unused)) pci_device_id,
+ UINTN timeout)
+{
+ UINT32 regval;
+
+ if (simatic_station_id() != SIMATIC_IPCBX_21A)
+ return EFI_UNSUPPORTED;
+
+ INFO(L"Detected SIMATIC BX-21A watchdog\n");
+
+ if (timeout < TIMEOUT_MIN_BX_21A || timeout > TIMEOUT_MAX_BX_21A) {
+ WARNING(L"Invalid timeout value (%d), default (%ds) is used.\n",
+ timeout, TIMEOUT_DEF_BX_21A);
+ timeout = TIMEOUT_DEF_BX_21A;
+ }
+
+ regval = inl(WDT_CTRL_REG_BX_21A);
+ /* setup timeout value */
+ regval &= (~WDT_CTRL_REG_TOV_MASK_BX_21A);
+ regval |= (timeout - 1);
+ /* get and clear status */
+ regval |= WDT_CTRL_REG_NO_ICCSURV_STS_BIT_BX_21A;
+ regval |= WDT_CTRL_REG_ICCSURV_STS_BIT_BX_21A;
+ outl(regval, WDT_CTRL_REG_BX_21A);
+
+ /* start watchdog */
+ regval = inl(WDT_CTRL_REG_BX_21A);
+ regval |= (WDT_CTRL_REG_EN_BIT_BX_21A |
+ WDT_CTRL_REG_ICCSURV_BIT_BX_21A |
+ WDT_CTRL_REG_FORCE_ALL_BIT_BX_21A);
+ outl(regval, WDT_CTRL_REG_BX_21A);
+
+ return EFI_SUCCESS;
+}
+
+WATCHDOG_REGISTER(init);
diff --git a/include/simatic.h b/include/simatic.h
index 6685045..eed82da 100644
--- a/include/simatic.h
+++ b/include/simatic.h
@@ -25,6 +25,7 @@

#define SIMATIC_IPC427E 0x0a01
#define SIMATIC_IPC477E 0x0a02
+#define SIMATIC_IPCBX_21A 0x1101
#define SIMATIC_IPCBX_56A 0x1201
#define SIMATIC_IPCBX_59A 0x1202

--
2.34.1

Jan Kiszka

unread,
Dec 8, 2024, 9:31:58 AM12/8/24
to Cedric Hombourger, efibootg...@googlegroups.com
Thanks, applied.
Reply all
Reply to author
Forward
0 new messages