Change in coreboot[master]: Revert "arch/x86/smbios.c: Split out weak functions"

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Tim Wawrzynczak,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: Revert "arch/x86/smbios.c: Split out weak functions"
......................................................................

Revert "arch/x86/smbios.c: Split out weak functions"

This reverts commit bea5ce7a4b5faf0a8c12565eb133456db1c099da.

Reason for revert: Breaks some people's workflow.

Change-Id: Ia09f25e64dfc46d230e7ec6faf335b1d04ee6f78
Signed-off-by: Angel Pons 
---
M src/arch/x86/Makefile.inc
M src/arch/x86/smbios.c
D src/arch/x86/smbios_defaults.c
3 files changed, 132 insertions(+), 140 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/47455/1

diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 17ab3dc..00690ba0 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -255,7 +255,6 @@
 ramstage-$(CONFIG_GENERATE_PIRQ_TABLE) += pirq_routing.c
 ramstage-y += rdrand.c
 ramstage-$(CONFIG_GENERATE_SMBIOS_TABLES) += smbios.c
-ramstage-$(CONFIG_GENERATE_SMBIOS_TABLES) += smbios_defaults.c
 ramstage-y += tables.c
 ramstage-$(CONFIG_COOP_MULTITASKING) += thread.c
 ramstage-$(CONFIG_COOP_MULTITASKING) += thread_switch.S
diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 8c7f92c..1b51d90 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -205,6 +205,18 @@
    	t->manufacturer = smbios_add_string(t->eos, string_buffer);
    }
 }
+/* this function will fill the corresponding locator */
+__weak void smbios_fill_dimm_locator(const struct dimm_info *dimm, struct smbios_type17 *t)
+{
+	char locator[40];
+
+	snprintf(locator, sizeof(locator), "Channel-%d-DIMM-%d",
+		dimm->channel_num, dimm->dimm_num);
+	t->device_locator = smbios_add_string(t->eos, locator);
+
+	snprintf(locator, sizeof(locator), "BANK %d", dimm->bank_locator);
+	t->bank_locator = smbios_add_string(t->eos, locator);
+}

static void trim_trailing_whitespace(char *buffer, size_t buffer_size)
 {
@@ -392,6 +404,11 @@
    return coreboot_version;
 }

+__weak const char *smbios_mainboard_bios_version(void)
+{
+	return NULL;
+}
+
 static int smbios_write_type0(unsigned long *current, int handle)
 {
    struct smbios_type0 *t = (struct smbios_type0 *)*current;
@@ -441,6 +458,121 @@
    return len;
 }

+__weak const char *smbios_mainboard_serial_number(void)
+{
+	return CONFIG_MAINBOARD_SERIAL_NUMBER;
+}
+
+__weak const char *smbios_mainboard_version(void)
+{
+	return CONFIG_MAINBOARD_VERSION;
+}
+
+__weak const char *smbios_mainboard_manufacturer(void)
+{
+	return CONFIG_MAINBOARD_SMBIOS_MANUFACTURER;
+}
+
+__weak const char *smbios_mainboard_product_name(void)
+{
+	return CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME;
+}
+
+__weak const char *smbios_mainboard_asset_tag(void)
+{
+	return "";
+}
+
+__weak u8 smbios_mainboard_feature_flags(void)
+{
+	return 0;
+}
+
+__weak const char *smbios_mainboard_location_in_chassis(void)
+{
+	return "";
+}
+
+__weak smbios_board_type smbios_mainboard_board_type(void)
+{
+	return SMBIOS_BOARD_TYPE_UNKNOWN;
+}
+
+/*
+ * System Enclosure or Chassis Types as defined in SMBIOS specification.
+ * The default value is SMBIOS_ENCLOSURE_DESKTOP (0x03) but laptop,
+ * convertible, or tablet enclosure will be used if the appropriate
+ * system type is selected.
+ */
+__weak smbios_enclosure_type smbios_mainboard_enclosure_type(void)
+{
+	if (CONFIG(SYSTEM_TYPE_LAPTOP))
+		return SMBIOS_ENCLOSURE_LAPTOP;
+	else if (CONFIG(SYSTEM_TYPE_TABLET))
+		return SMBIOS_ENCLOSURE_TABLET;
+	else if (CONFIG(SYSTEM_TYPE_CONVERTIBLE))
+		return SMBIOS_ENCLOSURE_CONVERTIBLE;
+	else if (CONFIG(SYSTEM_TYPE_DETACHABLE))
+		return SMBIOS_ENCLOSURE_DETACHABLE;
+	else
+		return SMBIOS_ENCLOSURE_DESKTOP;
+}
+
+__weak const char *smbios_system_serial_number(void)
+{
+	return smbios_mainboard_serial_number();
+}
+
+__weak const char *smbios_system_version(void)
+{
+	return smbios_mainboard_version();
+}
+
+__weak const char *smbios_system_manufacturer(void)
+{
+	return smbios_mainboard_manufacturer();
+}
+
+__weak const char *smbios_system_product_name(void)
+{
+	return smbios_mainboard_product_name();
+}
+
+__weak void smbios_system_set_uuid(u8 *uuid)
+{
+	/* leave all zero */
+}
+
+__weak unsigned int smbios_cpu_get_max_speed_mhz(void)
+{
+	return 0; /* Unknown */
+}
+
+__weak unsigned int smbios_cpu_get_current_speed_mhz(void)
+{
+	return 0; /* Unknown */
+}
+
+__weak const char *smbios_system_sku(void)
+{
+	return "";
+}
+
+__weak const char *smbios_chassis_version(void)
+{
+	return "";
+}
+
+__weak const char *smbios_chassis_serial_number(void)
+{
+	return "";
+}
+
+__weak const char *smbios_processor_serial_number(void)
+{
+	return "";
+}
+
 static int get_socket_type(void)
 {
    if (CONFIG(CPU_INTEL_SLOT_1))
diff --git a/src/arch/x86/smbios_defaults.c b/src/arch/x86/smbios_defaults.c
deleted file mode 100644
index 22ce7a5..0000000
--- a/src/arch/x86/smbios_defaults.c
+++ /dev/null
@@ -1,139 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#include <memory_info.h>
-#include <smbios.h>
-#include <stdint.h>
-#include <string.h>
-
-/* this function will fill the corresponding locator */
-__weak void smbios_fill_dimm_locator(const struct dimm_info *dimm, struct smbios_type17 *t)
-{
-	char locator[40];
-
-	snprintf(locator, sizeof(locator), "Channel-%d-DIMM-%d",
-		dimm->channel_num, dimm->dimm_num);
-	t->device_locator = smbios_add_string(t->eos, locator);
-
-	snprintf(locator, sizeof(locator), "BANK %d", dimm->bank_locator);
-	t->bank_locator = smbios_add_string(t->eos, locator);
-}
-
-__weak const char *smbios_mainboard_bios_version(void)
-{
-	return NULL;
-}
-
-__weak const char *smbios_mainboard_serial_number(void)
-{
-	return CONFIG_MAINBOARD_SERIAL_NUMBER;
-}
-
-__weak const char *smbios_mainboard_version(void)
-{
-	return CONFIG_MAINBOARD_VERSION;
-}
-
-__weak const char *smbios_mainboard_manufacturer(void)
-{
-	return CONFIG_MAINBOARD_SMBIOS_MANUFACTURER;
-}
-
-__weak const char *smbios_mainboard_product_name(void)
-{
-	return CONFIG_MAINBOARD_SMBIOS_PRODUCT_NAME;
-}
-
-__weak const char *smbios_mainboard_asset_tag(void)
-{
-	return "";
-}
-
-__weak u8 smbios_mainboard_feature_flags(void)
-{
-	return 0;
-}
-
-__weak const char *smbios_mainboard_location_in_chassis(void)
-{
-	return "";
-}
-
-__weak smbios_board_type smbios_mainboard_board_type(void)
-{
-	return SMBIOS_BOARD_TYPE_UNKNOWN;
-}
-
-/*
- * System Enclosure or Chassis Types as defined in SMBIOS specification.
- * The default value is SMBIOS_ENCLOSURE_DESKTOP (0x03) but laptop,
- * convertible, or tablet enclosure will be used if the appropriate
- * system type is selected.
- */
-__weak smbios_enclosure_type smbios_mainboard_enclosure_type(void)
-{
-	if (CONFIG(SYSTEM_TYPE_LAPTOP))
-		return SMBIOS_ENCLOSURE_LAPTOP;
-	else if (CONFIG(SYSTEM_TYPE_TABLET))
-		return SMBIOS_ENCLOSURE_TABLET;
-	else if (CONFIG(SYSTEM_TYPE_CONVERTIBLE))
-		return SMBIOS_ENCLOSURE_CONVERTIBLE;
-	else if (CONFIG(SYSTEM_TYPE_DETACHABLE))
-		return SMBIOS_ENCLOSURE_DETACHABLE;
-	else
-		return SMBIOS_ENCLOSURE_DESKTOP;
-}
-
-__weak const char *smbios_system_serial_number(void)
-{
-	return smbios_mainboard_serial_number();
-}
-
-__weak const char *smbios_system_version(void)
-{
-	return smbios_mainboard_version();
-}
-
-__weak const char *smbios_system_manufacturer(void)
-{
-	return smbios_mainboard_manufacturer();
-}
-
-__weak const char *smbios_system_product_name(void)
-{
-	return smbios_mainboard_product_name();
-}
-
-__weak void smbios_system_set_uuid(u8 *uuid)
-{
-	/* leave all zero */
-}
-
-__weak unsigned int smbios_cpu_get_max_speed_mhz(void)
-{
-	return 0; /* Unknown */
-}
-
-__weak unsigned int smbios_cpu_get_current_speed_mhz(void)
-{
-	return 0; /* Unknown */
-}
-
-__weak const char *smbios_system_sku(void)
-{
-	return "";
-}
-
-__weak const char *smbios_chassis_version(void)
-{
-	return "";
-}
-
-__weak const char *smbios_chassis_serial_number(void)
-{
-	return "";
-}
-
-__weak const char *smbios_processor_serial_number(void)
-{
-	return "";
-}

[Angel Pons (Code Review)]

Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47455 )

Change subject: Revert "arch/x86/smbios.c: Split out weak functions"
......................................................................

Abandoned

I was confused by what was later determined to be a mere expression of disconformity.