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/[email protected]/: WDAT cannot be used under Linux because the register bank is not exclusive for the watchdog. > > Signed-off-by: Cedric Hombourger <[email protected]> > --- > 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 <[email protected]> > + * From unpublished code created by XingTong Wu <[email protected]> > + * > + * 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) 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). > + > +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 > Thanks, Jan -- Siemens AG, Technology Linux Expert Center -- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/b627f6dc-143e-4344-b348-71f060d4ca92%40siemens.com.
