Change in ...coreboot[master]: util/superiotool: Add AST2400

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32984

Change subject: util/superiotool: Add AST2400
......................................................................

util/superiotool: Add AST2400

Add support for AST2400 Super I/O.

The device doesn't have an ID register, so probe for PCI devices and
known bits in scratch registers.

Tested on platform which has an AST2400.

Change-Id: I86af69c6b2ccefe2c88eef875bc858239df834f1
Signed-off-by: Patrick Rudolph 
---
M util/superiotool/Makefile
A util/superiotool/aspeed.c
M util/superiotool/superiotool.h
3 files changed, 161 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32984/1

diff --git a/util/superiotool/Makefile b/util/superiotool/Makefile
index 3a0bcf0..d1d7f243 100644
--- a/util/superiotool/Makefile
+++ b/util/superiotool/Makefile
@@ -53,7 +53,7 @@

ifeq ($(CONFIG_PCI), yes)
 CFLAGS += -DPCI_SUPPORT
-OBJS += pci.o via.o amd.o
+OBJS += pci.o via.o amd.o aspeed.o
 LIBS += -lpci
 ifeq ($(OS_ARCH),NetBSD)
 LIBS += -lpciutils
diff --git a/util/superiotool/aspeed.c b/util/superiotool/aspeed.c
new file mode 100644
index 0000000..0a7b624
--- /dev/null
+++ b/util/superiotool/aspeed.c
@@ -0,0 +1,151 @@
+/*
+ * This file is part of the superiotool project.
+ *
+ * Copyright (C) 2010 Google Inc.
+ * Written by David Hendricks  for Aspeed Technology Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "superiotool.h"
+
+#define DEVICE_SCRATCH_REG	0x21
+
+static const struct superio_registers reg_table[] = {
+	{0x00, "AST2400", {
+		{NOLDN, NULL,
+			{0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28,
+			 0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,EOT},
+			{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
+			 0x00,0x00,0x00,0x00,0x00,0x00,0x00,EOT}},
+		{0x02, "SUART1",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x03,0xf8,0x04,0x02,RSVD,EOT}},
+		{0x03, "SUART1",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x02,0xf8,0x03,0x02,0x00,EOT}},
+		{0x04, "SWC",
+			{0x30,0x60,0x61,0x62,0x63,0x64,0x65,0x66,0x67,
+			 0x70,0x71,EOT},
+			{0x00,0x08,0xe6,0x08,0xe0,0x08,0xe4,0x08,0xe8,
+			 0x09,0x01,EOT}},
+		{0x05, "Keyboard config (KBC)",
+			{0x30,0x60,0x61,0x62,0x63,0x70,0x71,0x72,0x73,
+			 0xf0,EOT},
+			{0x00,0x00,0x60,0x00,0x64,0x01,0x02,0x0c,0x02,
+			 0x83,EOT}},
+		{0x07, "GPIO",
+			{0x30,0x38,0x70,0x71,EOT},
+			{0x00,0x00,0x0b,0x01,EOT}},
+		{0x0b, "SUART3",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x03,0xe8,0x06,0x02,0x00,EOT}},
+		{0x0c, "SUART4",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x02,0xe8,0x05,0x02,0x00,EOT}},
+		{0x0d, "iLPC2AHB",
+			{0x30,0x70,0x71,0xf0,0xf1,0xf2,0xf3,0xf4,0xf5,
+			 0xf6,0xf7,0xf8,0xfe,EOT},
+			{0x00,0x09,0x01,NANA,NANA,NANA,NANA,NANA,NANA,
+			 NANA,NANA,0x00,0x00,EOT}},
+		{0x0e, "Mailbox",
+			{0x30,0x60,0x61,0x70,0x71,EOT},
+			{0x00,0x08,0xc0,0x09,0x01,EOT}},
+		{EOT}}},
+	{EOT}
+};
+
+static void enter_conf_mode_ast(uint16_t port)
+{
+	OUTB(0xa5, port);
+	OUTB(0xa5, port);
+}
+
+static void exit_conf_mode_ast(uint16_t port)
+{
+	OUTB(0xaa, port);
+}
+
+static int detect_ast_pci(void)
+{
+	int i;
+	static const uint16_t ids[] = {
+		0x2000, 0x1150,
+	};
+
+	for (i = 0; i < ARRAY_SIZE(ids); i++) {
+		if (pci_dev_find(0x1a03, ids[i]))
+			return 1;
+	}
+
+	return 0;
+}
+
+static int detect_ast_superio(uint16_t port)
+{
+	int i;
+	enter_conf_mode_ast(port);
+
+	/* Aspeed devices doesn't have a DEVICE_ID_REG.
+	 * Host cycles that aren't decoded read as 0xff.
+	 * Probe for the scratch register that are initialized to zero.
+	 * The firmware might overwrite that, but it's the best we have.
+	 * Together with PCI detection that should be sufficient.
+	 */
+
+	for (i = DEVICE_SCRATCH_REG; i < 0x30; i++) {
+		if (regval(port, i) == 0)
+			break;
+	}
+	if (i == 0x30) {
+		if (verbose)
+			printf(NOTFOUND
+			       "scratch registers all read as non zero\n");
+	}
+	exit_conf_mode_ast(port);
+
+	return i < 0x30;
+}
+
+void probe_idregs_aspeed(uint16_t port)
+{
+	uint16_t chip_id = 0;
+
+	probing_for("Aspeed", "", port);
+
+	if (!detect_ast_pci()) {
+		if (verbose)
+			printf(NOTFOUND "missing PCI device\n");
+		return;
+	}
+
+	if (!detect_ast_superio(port))
+		return;
+
+	if (superio_unknown(reg_table, chip_id)) {
+		if (verbose)
+			printf(NOTFOUND "id=0x%02x\n", chip_id);
+		return;
+	}
+
+	printf("Found Aspeed %s (id=0x%02x) at 0x%x\n",
+	       get_superio_name(reg_table, chip_id), chip_id, port);
+	chip_found = 1;
+
+	enter_conf_mode_ast(port);
+	dump_superio("Aspeed", reg_table, port, chip_id, LDN_SEL);
+	exit_conf_mode_ast(port);
+}
+
+void print_aspeed_chips(void)
+{
+	print_vendor_chips("Aspeed", reg_table);
+}
diff --git a/util/superiotool/superiotool.h b/util/superiotool/superiotool.h
index 1a10fb6..f2a4d5e 100644
--- a/util/superiotool/superiotool.h
+++ b/util/superiotool/superiotool.h
@@ -184,6 +184,12 @@
 void probe_idregs_ali(uint16_t port);
 void print_ali_chips(void);

+/* aspeed.c */
+#ifdef PCI_SUPPORT
+void probe_idregs_aspeed(uint16_t port);
+void print_aspeed_chips(void);
+#endif
+
 /* amd.c */
 void probe_idregs_amd(uint16_t port);
 void print_amd_chips(void);
@@ -252,6 +258,8 @@
    {probe_idregs_via,	{0x2e, 0x4e, 0x3f0, EOT}},
    /* in fact read the BASE from HW */
    {probe_idregs_amd,	{0xaa, EOT}},
+	/* Probe for aspeed PCI devices */
+	{probe_idregs_aspeed,	{0x2e, 0x4e, EOT}},
 #endif
    {probe_idregs_serverengines,	{0x2e, EOT}},
    {probe_idregs_infineon,	{0x2e, 0x4e, EOT}},
@@ -272,6 +280,7 @@
 #ifdef PCI_SUPPORT
    {print_via_chips},
    {print_amd_chips},
+	{print_aspeed_chips},
 #endif
    {print_serverengines_chips},
    {print_infineon_chips},

[Patrick Rudolph (Code Review)]

Hello Felix Held, Frans Hendriks, Matt DeVillier, Christian Walter, Felix Singer, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32984

to look at the new patch set (#2).

Change subject: util/superiotool: Add AST2400
......................................................................

util/superiotool: Add AST2400

Add support for AST2400 Super I/O.

The device doesn't have an ID register, so probe for PCI devices and
known bits in scratch registers.

Tested on platform which has an AST2400.

Change-Id: I86af69c6b2ccefe2c88eef875bc858239df834f1
Signed-off-by: Patrick Rudolph 
---
M util/superiotool/Makefile
A util/superiotool/aspeed.c
M util/superiotool/superiotool.h
3 files changed, 160 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32984/2

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2: Code-Review+1

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:

So the scratch register probing is to find out if the IO decoding works for the SIO part of the BMC, right? that's the only reason I see why only relying on detecting the PCI ID might not be sufficient

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c 
File util/superiotool/aspeed.c:

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c@81 
PS2, Line 81: 	};
So 0x1150 is a PCI-to-PCI bridge with 0x2000 "graphics family" device on the secondary?

Is it possible that 0x2000 is disabled or why test 0x1150 at all?

This will currently mislabel almost any AST BMC as "AST2400". You can further read the PCI revision register of 0x2000 device to have this correcly either only detect the AST2400 part or to translate it to a correct silicon name string to display. I don't have datasheets to tell if LPC parts are compatible.

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/ast/ast_main....

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c@100 
PS2, Line 100: 	 * Together with PCI detection that should be sufficient.
From what I remember, outside the config mode, decoders are passive for the configuration registers (all IO at 2e/4e). The entry sequence 0xa5, 0xa5 seems to be unique to AST (in our codebase at least). So, just reading back anything else than 0xff from any global register 0x0..0x2f should alone identify it was AST part that decoded the IO cycle.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:

not that it makes a big difference here, but is the SIO part of the ast2400 behind a PCI(e) device or is it connected to the LPC bus?

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:
> Patch Set 2:
>
> not that it makes a big difference here, but is the SIO part of the ast2400 behind a PCI(e) device or is it connected to the LPC bus?

It's connected to PCIe, LPC and USB.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c 
File util/superiotool/aspeed.c:

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c@81 
PS2, Line 81: 	};
> So 0x1150 is a PCI-to-PCI bridge with 0x2000 "graphics family" device on the secondary? […]

Yes, the VGA device might be disabled.
Also as all Aspeed BMC use the same vendor/device id for PCI devices, it's not a very good indicator.

https://review.coreboot.org/#/c/32984/2/util/superiotool/aspeed.c@100 
PS2, Line 100: 	 * Together with PCI detection that should be sufficient.
> From what I remember, outside the config mode, decoders are passive for the configuration registers  […]

I'll improve the check based on that.

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> >
> > not that it makes a big difference here, but is the SIO part of the ast2400 behind a PCI(e) device or is it connected to the LPC bus?
>
> It's connected to PCIe, LPC and USB.

Logically, I believe SIO must be on LPC; PCI bridges only forward IO transactions that fall within the (configured) IO window in PCI configuration register space.

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 2:
> Patch Set 2:
> > Patch Set 2:
> > > Patch Set 2:
> > >
> > > not that it makes a big difference here, but is the SIO part of the ast2400 behind a PCI(e) device or is it connected to the LPC bus?
> >
> > It's connected to PCIe, LPC and USB.
>
> Logically, I believe SIO must be on LPC; PCI bridges only forward IO transactions that fall within the (configured) IO window in PCI configuration register space.

IIRC at least in some platforms it depends on if the LPC or PCI bus is selected as target for the subtraction IO decode; haven't looked at the details though

Change in coreboot[master]: util/superiotool: Add AST2400

[Patrick Rudolph (Code Review)]

Hello Felix Held, Frans Hendriks, Matt DeVillier, Christian Walter, Felix Singer, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32984

to look at the new patch set (#3).

Change subject: util/superiotool: Add AST2400
......................................................................

util/superiotool: Add AST2400

Add support for AST2400 Super I/O.

The device doesn't have an ID register, so probe for scratch register
not to read as 0xff.

Tested on platform which has an AST2400.

Change-Id: I86af69c6b2ccefe2c88eef875bc858239df834f1
Signed-off-by: Patrick Rudolph 
---
M util/superiotool/Makefile
A util/superiotool/aspeed.c
M util/superiotool/superiotool.h
3 files changed, 135 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32984/3

Change in coreboot[master]: util/superiotool: Add AST2400

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: util/superiotool: Add AST2400

[Felix Held (Code Review)]

Felix Held has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

util/superiotool: Add AST2400

Add support for AST2400 Super I/O.

The device doesn't have an ID register, so probe for scratch register
not to read as 0xff.

Tested on platform which has an AST2400.

Change-Id: I86af69c6b2ccefe2c88eef875bc858239df834f1
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32984
Tested-by: build bot (Jenkins) 
Reviewed-by: Kyösti Mälkki 
---
M util/superiotool/Makefile
A util/superiotool/aspeed.c
M util/superiotool/superiotool.h
3 files changed, 135 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved

diff --git a/util/superiotool/Makefile b/util/superiotool/Makefile
index 3a0bcf0..2bc88ab 100644
--- a/util/superiotool/Makefile
+++ b/util/superiotool/Makefile
@@ -29,7 +29,7 @@
 LDFLAGS += -lz

OBJS = superiotool.o serverengines.o ali.o exar.o fintek.o ite.o nsc.o \
-       nuvoton.o smsc.o winbond.o infineon.o
+       nuvoton.o smsc.o winbond.o infineon.o aspeed.o

OS_ARCH = $(shell uname)
 ifeq ($(OS_ARCH), Darwin)
diff --git a/util/superiotool/aspeed.c b/util/superiotool/aspeed.c
new file mode 100644
index 0000000..6d49c16
--- /dev/null
+++ b/util/superiotool/aspeed.c
@@ -0,0 +1,128 @@
+/*
+ * This file is part of the superiotool project.
+ *
+ * Copyright (C) 9elements Agency GmbH 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "superiotool.h"
+
+#define DEVICE_SCRATCH_REG	0x21
+
+static const struct superio_registers reg_table[] = {
+	{0x00, "AST2400", {
+		{NOLDN, NULL,
+			{0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28,
+			 0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,EOT},
+			{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
+			 0x00,0x00,0x00,0x00,0x00,0x00,0x00,EOT}},
+		{0x02, "SUART1",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x03,0xf8,0x04,0x02,RSVD,EOT}},
+		{0x03, "SUART1",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x02,0xf8,0x03,0x02,0x00,EOT}},
+		{0x04, "SWC",
+			{0x30,0x60,0x61,0x62,0x63,0x64,0x65,0x66,0x67,
+			 0x70,0x71,EOT},
+			{0x00,0x08,0xe6,0x08,0xe0,0x08,0xe4,0x08,0xe8,
+			 0x09,0x01,EOT}},
+		{0x05, "Keyboard config (KBC)",
+			{0x30,0x60,0x61,0x62,0x63,0x70,0x71,0x72,0x73,
+			 0xf0,EOT},
+			{0x00,0x00,0x60,0x00,0x64,0x01,0x02,0x0c,0x02,
+			 0x83,EOT}},
+		{0x07, "GPIO",
+			{0x30,0x38,0x70,0x71,EOT},
+			{0x00,0x00,0x0b,0x01,EOT}},
+		{0x0b, "SUART3",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x03,0xe8,0x06,0x02,0x00,EOT}},
+		{0x0c, "SUART4",
+			{0x30,0x60,0x61,0x70,0x71,0xf0,EOT},
+			{0x00,0x02,0xe8,0x05,0x02,0x00,EOT}},
+		{0x0d, "iLPC2AHB",
+			{0x30,0x70,0x71,0xf0,0xf1,0xf2,0xf3,0xf4,0xf5,
+			 0xf6,0xf7,0xf8,0xfe,EOT},
+			{0x00,0x09,0x01,NANA,NANA,NANA,NANA,NANA,NANA,
+			 NANA,NANA,0x00,0x00,EOT}},
+		{0x0e, "Mailbox",
+			{0x30,0x60,0x61,0x70,0x71,EOT},
+			{0x00,0x08,0xc0,0x09,0x01,EOT}},
+		{EOT}}},
+	{EOT}
+};
+
+static void enter_conf_mode_ast(uint16_t port)
+{
+	OUTB(0xa5, port);
+	OUTB(0xa5, port);
+}
+
+static void exit_conf_mode_ast(uint16_t port)
+{
+	OUTB(0xaa, port);
+}
+
+static int detect_ast_superio(uint16_t port)
+{
+	int i;
+	enter_conf_mode_ast(port);
+
+	/* Aspeed devices doesn't have a DEVICE_ID_REG.
+	 * Host cycles that aren't decoded read as 0xff.
+	 * Probe for the scratch register that are initialized to zero.
+	 * The firmware might overwrite that, but it's the best we have.
+	 */
+
+	for (i = DEVICE_SCRATCH_REG; i < 0x30; i++) {
+		if (regval(port, i) != 0xff)
+			break;
+	}
+	if (i == 0x30) {
+		if (verbose)
+			printf(NOTFOUND
+			       "scratch registers all read as 0xff\n");
+	}
+	exit_conf_mode_ast(port);
+
+	return i < 0x30;
+}
+
+void probe_idregs_aspeed(uint16_t port)
+{
+	uint16_t chip_id = 0;
+
+	probing_for("Aspeed", "", port);
+
+	if (!detect_ast_superio(port))
+		return;
+
+	if (superio_unknown(reg_table, chip_id)) {
+		if (verbose)
+			printf(NOTFOUND "id=0x%02x\n", chip_id);
+		return;
+	}
+
+	printf("Found Aspeed %s (id=0x%02x) at 0x%x\n",
+	       get_superio_name(reg_table, chip_id), chip_id, port);
+	chip_found = 1;
+
+	enter_conf_mode_ast(port);
+	dump_superio("Aspeed", reg_table, port, chip_id, LDN_SEL);
+	exit_conf_mode_ast(port);
+}
+
+void print_aspeed_chips(void)
+{
+	print_vendor_chips("Aspeed", reg_table);
+}
diff --git a/util/superiotool/superiotool.h b/util/superiotool/superiotool.h
index 6e59933..d3b9fd0 100644
--- a/util/superiotool/superiotool.h
+++ b/util/superiotool/superiotool.h
@@ -190,6 +190,10 @@
 void probe_idregs_ali(uint16_t port);
 void print_ali_chips(void);

+/* aspeed.c */
+void probe_idregs_aspeed(uint16_t port);
+void print_aspeed_chips(void);
+
 /* amd.c */
 void probe_idregs_amd(uint16_t port);
 void print_amd_chips(void);
@@ -243,6 +247,7 @@
    int ports[MAXNUMPORTS]; /* Signed, as we need EOT. */
 } superio_ports_table[] = {
    {probe_idregs_ali,	{0x3f0, 0x370, EOT}},
+        {probe_idregs_aspeed,   {0x2e, 0x4e, EOT}},
    {probe_idregs_exar,	{0x2e, 0x4e, EOT}},
    {probe_idregs_fintek,	{0x2e, 0x4e, EOT}},
    {probe_idregs_fintek_alternative,	{0x2e, 0x4e, EOT}},
@@ -278,6 +283,7 @@
 #ifdef PCI_SUPPORT
    {print_via_chips},
    {print_amd_chips},
+	{print_aspeed_chips},
 #endif
    {print_serverengines_chips},
    {print_infineon_chips},

Change in coreboot[master]: util/superiotool: Add AST2400

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32984 )

Change subject: util/superiotool: Add AST2400
......................................................................

Patch Set 4:

detect_ast_superio() detects about every chip without a conf-mode
sequence as AST. And maybe more. Having a non-ff value between
0x20 and 0x30 is really nothing AST specific.

Can we move this behind a command-line switch? at least until
somebody figured a more distinctive probing method out.