Change in coreboot[master]: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/google/volteer/variants/baseboard/devicetree.cb
2 files changed, 20 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/1

diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c
index 5bece68..ee67555 100644
--- a/src/mainboard/google/volteer/mainboard.c
+++ b/src/mainboard/google/volteer/mainboard.c
@@ -5,6 +5,7 @@
 #include <baseboard/variants.h>
 #include <device/device.h>
 #include <ec/ec.h>
+#include <fw_config.h>
 #include <soc/gpio.h>
 #include <vendorcode/google/chromeos/chromeos.h>
 #include <variant/gpio.h>
@@ -14,10 +15,28 @@
    mainboard_ec_init();
 }

+static void add_fw_config(const struct fw_config *config, void *arg)
+{
+	char buffer[64];
+	struct smbios_type11 *t = arg;
+
+	if (!config || config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
+		return;
+
+	snprintf(buffer, sizeof(buffer), "%s-%s", config->field_name, config->option_name);
+	t->count = smbios_add_string(t->eos, buffer);
+}
+
+static void mainboard_smbios_strings(struct device *dev, struct smbios_type11 *t)
+{
+	fw_config_for_each_cached(add_fw_config, t);
+}
+
 static void mainboard_enable(struct device *dev)
 {
    dev->ops->init = mainboard_init;
    dev->ops->acpi_inject_dsdt = chromeos_dsdt_generator;
+	dev->ops->get_smbios_strings = mainboard_smbios_strings;
 }

static void mainboard_chip_init(void *chip_info)
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
index ffae2f0..faf1dfb 100644
--- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
@@ -47,7 +47,7 @@
    register "HeciEnabled" = "1"

# FSP configuration
-	register "SaGv" = "SaGv_Enabled"
+	register "SaGv" = "SaGv_Disabled"
    register "SmbusEnable" = "0"

register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)"	# Type-A Port A0

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/2

[Tim Wawrzynczak (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 19 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
PS3, Line 23: config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
Is there any reason why we wouldn't want to do this for all found configs? That will also allow this function to be added directly to fw_config library.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
PS3, Line 23: config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
> Is there any reason why we wouldn't want to do this for all found configs? That will also allow this […]

Do any of these other drivers use DMI quirks for binding?

[Tim Wawrzynczak (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/4

[Tim Wawrzynczak (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/6

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
PS3, Line 23: config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
> Do any of these other drivers use DMI quirks for binding?

Currently, they don't. But, I think it wouldn't hurt to expose those in case any drivers need it. Also, it would avoid code duplication in mainboards using fw_config to add these checks.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
PS3, Line 23: config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
> Currently, they don't. But, I think it wouldn't hurt to expose those in case any drivers need it. […]

Ok, I'll add them all in.
I didn't want to add a dependency on smbios to the fw_config library, which is why I put this in the mainboard.

[Tim Wawrzynczak (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/volteer: Add audio fw_config to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add audio fw_config to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.

BUG=b:161963281
TEST=dmidecode -t 11 and see AUDIO-NONE for my board. Rewrote CBI field
to indicate another audio fw_config, and verify using dmidecode.

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/8

Change in coreboot[master]: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

[Tim Wawrzynczak (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.
But add all successfully probed fw_config items as well, because this
makes it easier to view what is selected from userspace.

BUG=b:161963281
TEST=With CBI FW_CONFIG field set to 0x201:

localhost ~ # dmidecode -t 11
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.0 present.

Handle 0x0009, DMI type 11, 5 bytes
OEM Strings
        String 1: DB_USB-USB4_GEN2
        String 2: AUDIO-MAX98373_ALC5682I_I2S

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 18 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/44783/9

Change in coreboot[master]: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings
......................................................................

Patch Set 9: Code-Review+2

Change in coreboot[master]: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44783/3/src/mainboard/google/voltee... 
PS3, Line 23: config->mask != FW_CONFIG_FIELD_AUDIO_MASK)
> Ok, I'll add them all in. […]

Done

Change in coreboot[master]: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings
......................................................................

mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

Some Linux kernel drivers bind to "DMI quirks." In this case, the audio
fw_config is added as an OEM string, e.g., "AUDIO-MAX98357_ALC5682I_I2S"
so the audio topology can be correctly discovered.
But add all successfully probed fw_config items as well, because this
makes it easier to view what is selected from userspace.

BUG=b:161963281
TEST=With CBI FW_CONFIG field set to 0x201:

localhost ~ # dmidecode -t 11
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 3.0 present.

Handle 0x0009, DMI type 11, 5 bytes
OEM Strings
        String 1: DB_USB-USB4_GEN2
        String 2: AUDIO-MAX98373_ALC5682I_I2S

Signed-off-by: Tim Wawrzynczak 
Change-Id: I7b7586b0ebfe7b2fd888f448a50ae086364fa718
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44783
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/google/volteer/mainboard.c
1 file changed, 18 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c
index 5bece68..ead187e 100644
--- a/src/mainboard/google/volteer/mainboard.c
+++ b/src/mainboard/google/volteer/mainboard.c
@@ -5,6 +5,7 @@
 #include <baseboard/variants.h>
 #include <device/device.h>
 #include <ec/ec.h>
+#include <fw_config.h>
 #include <soc/gpio.h>
 #include <vendorcode/google/chromeos/chromeos.h>
 #include <variant/gpio.h>
@@ -14,10 +15,27 @@
    mainboard_ec_init();
 }

+static void add_fw_config_oem_string(const struct fw_config *config, void *arg)
+{
+	struct smbios_type11 *t;
+	char buffer[64];
+
+	t = (struct smbios_type11 *)arg;
+
+	snprintf(buffer, sizeof(buffer), "%s-%s", config->field_name, config->option_name);
+	t->count = smbios_add_string(t->eos, buffer);
+}
+
+static void mainboard_smbios_strings(struct device *dev, struct smbios_type11 *t)
+{
+	fw_config_for_each_found(add_fw_config_oem_string, t);
+}
+
 static void mainboard_enable(struct device *dev)
 {
    dev->ops->init = mainboard_init;
    dev->ops->acpi_inject_dsdt = chromeos_dsdt_generator;
+	dev->ops->get_smbios_strings = mainboard_smbios_strings;
 }

static void mainboard_chip_init(void *chip_info)

Change in coreboot[master]: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44783 )

Change subject: mb/google/volteer: Add probed fw_configs to SMBIOS OEM strings
......................................................................

Patch Set 10:

Automatic boot test returned (PASS/FAIL/TOTAL): 6/1/7
"QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17272
"QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17271
"QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/17270
"QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17269
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/17268
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17274
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17273

Please note: This test is under development and might not be accurate at all!