[PATCH v2 1/3] w83627hf: Add station id of IPC BX-56A

[Tobias Schaffner]

The simatic IPC BX-56A shares the same nct6116 superio chip as the
BX-59A. Allow efibootguard to configure the w83627hf watchdog for BX-56A
by adding its station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---
drivers/watchdog/w83627hf_wdt.c | 3 ++-
include/simatic.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index df1da11..fd2950d 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -208,11 +208,12 @@ static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
}

switch (simatic_station_id()) {
+ case SIMATIC_IPCBX_56A:
case SIMATIC_IPCBX_59A:
chip = wdt_find(0x2e);
if (chip < 0)
return EFI_UNSUPPORTED;
- INFO(L"Detected SIMATIC BX59A watchdog\n");
+ INFO(L"Detected SIMATIC BX5xA watchdog\n");
ret = w83627hf_init(chip);
if (ret < 0)
return EFI_UNSUPPORTED;
diff --git a/include/simatic.h b/include/simatic.h
index 724e79d..43c5515 100644
--- a/include/simatic.h
+++ b/include/simatic.h
@@ -25,6 +25,7 @@

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

typedef struct {
--
2.40.1

[Tobias Schaffner]

This series adds the station id of the BX-56A and allows to run the
driver for all configurations of the BX-56A and BX-59A.

As the host bridge device id varies over the different configurations,
we only depend on the pci vendor and station id.

This has been tested on both the BX-56A and the BX-59A.

changes since v1:
- move caching of station id out of the driver into to the simatic interface
- address other review comments

Tobias Schaffner (3):
w83627hf: Add station id of IPC BX-56A
w83627hf: Solely depend on pci vendor and station id
Cache simatic station id

drivers/utils/simatic.c | 11 ++++++++++-
drivers/watchdog/w83627hf_wdt.c | 14 +++++---------
include/simatic.h | 1 +
3 files changed, 16 insertions(+), 10 deletions(-)

--
2.40.1

[Tobias Schaffner]

Now that we solely depend on the vendor pci id the station id will be
compared for a large number of pci device ids. Cache the station id to
avoid retreiving it from SMBIOSTable every time.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

drivers/utils/simatic.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/utils/simatic.c b/drivers/utils/simatic.c
index a1c63cb..376dc42 100644
--- a/drivers/utils/simatic.c
+++ b/drivers/utils/simatic.c
@@ -20,6 +20,9 @@
#include "smbios.h"
#include "utils.h"

+static UINT32 station_id;
+static BOOLEAN station_id_cached;
+
static UINT32 get_station_id(SMBIOS_STRUCTURE_POINTER oem_strct)
{
SIMATIC_OEM_ENTRY *entry;
@@ -45,6 +48,9 @@ static UINT32 get_station_id(SMBIOS_STRUCTURE_POINTER oem_strct)

UINT32 simatic_station_id(VOID)
{
+ if (station_id_cached)
+ return station_id;
+
SMBIOS_STRUCTURE_TABLE *smbios_table;
SMBIOS_STRUCTURE_POINTER smbios_struct;
EFI_STATUS status;
@@ -60,5 +66,8 @@ UINT32 simatic_station_id(VOID)
return 0;
}

- return get_station_id(smbios_struct);
+ station_id = get_station_id(smbios_struct);
+ station_id_cached = TRUE;
+
+ return station_id;
}
--
2.40.1

[Tobias Schaffner]

The host bridge device id varies over the different configurations of
the BX-56A and BX-59A. Only filter intel based devices and then depend
on the station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

drivers/watchdog/w83627hf_wdt.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fd2950d..68cb42a 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -44,11 +44,6 @@
#include "simatic.h"
#include "utils.h"

-/* Use the host bridge device found on the BX-59A to limit probing to Intel
- * based machines that may be a BX59A; technically we do not use/need PCI
- * for this driver but only port I/Os */
-#define PCI_DEVICE_ID_INTEL_HOST_BRIDGE 0xa700
-
#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
(same as EFER) */
@@ -198,12 +193,12 @@ static int wdt_set_time(unsigned int timeout)

}

static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,

- UINT16 pci_device_id, UINTN timeout)
+ UINT16 __attribute__((unused)) pci_device_id,
+ UINTN timeout)
{
int chip, ret;

- if (!pci_io || pci_vendor_id != PCI_VENDOR_ID_INTEL ||
- pci_device_id != PCI_DEVICE_ID_INTEL_HOST_BRIDGE) {
+ if (!pci_io || pci_vendor_id != PCI_VENDOR_ID_INTEL) {
return EFI_UNSUPPORTED;
}

--
2.40.1

[Jan Kiszka]

Welcome back (I mis-predicted it to be later)! :)

Looks generally good, but we still need a small README update to add
56A, and I would suggest to reorder patch 2 and 3 (which minor commit
message adjustment).

Jan

--
Siemens AG, Technology
Linux Expert Center

[Tobias Schaffner]

The simatic IPC BX-56A shares the same nct6116 superio chip as the
BX-59A. Allow efibootguard to configure the w83627hf watchdog for BX-56A

by adding its station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

README.md | 2 +-
drivers/watchdog/w83627hf_wdt.c | 3 ++-
include/simatic.h | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index ef8a2fe..5ac1527 100644
--- a/README.md
+++ b/README.md
@@ -33,7 +33,7 @@ The following watchdog drivers are implemented (and are probed in this order):
* Intel i6300esb
* Intel Quark
* Siemens SIMATIC IPC4x7E
-* Siemens SIMATIC BX-59A
+* Siemens SIMATIC BX-56A and BX-59A
* Intel TCO
* HPE ProLiant

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index df1da11..fd2950d 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c

[Tobias Schaffner]

This series adds the station id of the BX-56A and allows to run the
driver for all configurations of the BX-56A and BX-59A.

As the host bridge device id varies over the different configurations,
we only depend on the pci vendor and station id.

This has been tested on both the BX-56A and the BX-59A.

changes since v1:
- move caching of station id out of the driver into to the simatic interface
- address other review comments

changes since v2:
- Add information about BX-56A to README
- reorder the patch series

Note: Please wait for somebody to do the final testing before merging
this.

Tobias Schaffner (3):
w83627hf: Add station id of IPC BX-56A

Cache simatic station id

w83627hf: Solely depend on pci vendor and station id

README.md | 2 +-

drivers/utils/simatic.c | 11 ++++++++++-
drivers/watchdog/w83627hf_wdt.c | 14 +++++---------
include/simatic.h | 1 +

4 files changed, 17 insertions(+), 11 deletions(-)

--
2.40.1

[Tobias Schaffner]

The simatic station id may be retreived for a large number of pci device

ids. Cache the station id to avoid retreiving it from SMBIOSTable every
time.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

[Tobias Schaffner]

The host bridge device id varies over the different configurations of
the BX-56A and BX-59A. Only filter intel based devices and then depend
on the station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

drivers/watchdog/w83627hf_wdt.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fd2950d..68cb42a 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c

@@ -44,11 +44,6 @@
#include "simatic.h"
#include "utils.h"

-/* Use the host bridge device found on the BX-59A to limit probing to Intel
- * based machines that may be a BX59A; technically we do not use/need PCI
- * for this driver but only port I/Os */
-#define PCI_DEVICE_ID_INTEL_HOST_BRIDGE 0xa700
-
#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
(same as EFER) */
@@ -198,12 +193,12 @@ static int wdt_set_time(unsigned int timeout)
}

static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,

[Shivaschandra KL]

Signed-off-by: K L, Shivaschandra <shivasch...@siemens.com>
Tested-by: K L, Shivaschandra <shivasch...@siemens.com>

[Shivaschandra KL]

From: Tobias Schaffner <tobias.s...@siemens.com>

The simatic IPC BX-56A shares the same nct6116 superio chip as the
BX-59A. Allow efibootguard to configure the w83627hf watchdog for BX-56A

by adding its station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

README.md | 2 +-
drivers/watchdog/w83627hf_wdt.c | 3 ++-
include/simatic.h | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index ef8a2fe..5ac1527 100644
--- a/README.md
+++ b/README.md
@@ -33,7 +33,7 @@ The following watchdog drivers are implemented (and are probed in this order):
* Intel i6300esb
* Intel Quark
* Siemens SIMATIC IPC4x7E
-* Siemens SIMATIC BX-59A
+* Siemens SIMATIC BX-56A and BX-59A
* Intel TCO
* HPE ProLiant

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index df1da11..fd2950d 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c

[Shivaschandra KL]

From: Tobias Schaffner <tobias.s...@siemens.com>

The simatic station id may be retreived for a large number of pci device
ids. Cache the station id to avoid retreiving it from SMBIOSTable every
time.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

[Shivaschandra KL]

From: Tobias Schaffner <tobias.s...@siemens.com>

The host bridge device id varies over the different configurations of
the BX-56A and BX-59A. Only filter intel based devices and then depend
on the station id.

Signed-off-by: Tobias Schaffner <tobias.s...@siemens.com>
---

drivers/watchdog/w83627hf_wdt.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fd2950d..68cb42a 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c

@@ -44,11 +44,6 @@
#include "simatic.h"
#include "utils.h"

-/* Use the host bridge device found on the BX-59A to limit probing to Intel
- * based machines that may be a BX59A; technically we do not use/need PCI
- * for this driver but only port I/Os */
-#define PCI_DEVICE_ID_INTEL_HOST_BRIDGE 0xa700
-
#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
(same as EFER) */
@@ -198,12 +193,12 @@ static int wdt_set_time(unsigned int timeout)
}

static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,

[Jan Kiszka]

On 16.05.24 17:50, 'Shivaschandra KL' via EFI Boot Guard wrote:
> Signed-off-by: K L, Shivaschandra <shivasch...@siemens.com>
> Tested-by: K L, Shivaschandra <shivasch...@siemens.com>
>

Thanks for testing! This is at least how I interpret this reply. Did you
only test patch 1 or the whole series?

Also note that only the tested-by tag is appended in that case.

[Jan Kiszka]

Thanks, applied.

[Shivaschandra K L Jois]

Hi Jan Kiszka,

Yes, I have tested all three patches in this series.

Thanks