Change in coreboot[master]: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set...

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35848 )

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option

Avoid searching for cmos_layout.bin each time get/set_option is called
on platforms with NO_CAR_GLOBAL_MIGRATION.

Change-Id: Ie541017d2f8c8e9d4b592b71f44a08fd9670dd09
Signed-off-by: Arthur Heymans 
---
M src/drivers/pc80/rtc/mc146818rtc.c
1 file changed, 7 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35848/1

diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c
index 9ea5414..9a97aa1 100644
--- a/src/drivers/pc80/rtc/mc146818rtc.c
+++ b/src/drivers/pc80/rtc/mc146818rtc.c
@@ -245,7 +245,7 @@
 static enum cb_err locate_cmos_layout(struct region_device *rdev)
 {
    uint32_t cbfs_type = CBFS_COMPONENT_CMOS_LAYOUT;
-	struct cbfsf fh;
+	MAYBE_STATIC struct cbfsf fh;

/*
     * In case VBOOT is enabled and this function is called from SMM,
@@ -254,11 +254,13 @@
     *
     * Support only one CMOS layout in the 'COREBOOT' region for now.
     */
-	if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin",
-				       &cbfs_type)) {
-		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
+	if (!region_device_size(&(fh->data))) {
+		if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin",
+					       &cbfs_type)) {
+			printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
    					"Options are disabled\n");
-		return CB_CMOS_LAYOUT_NOT_FOUND;
+			return CB_CMOS_LAYOUT_NOT_FOUND;
+		}
    }

cbfs_file_data(rdev, &fh);

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option

Avoid searching for cmos_layout.bin each time get/set_option is called
on platforms with NO_CAR_GLOBAL_MIGRATION.

Change-Id: Ie541017d2f8c8e9d4b592b71f44a08fd9670dd09
Signed-off-by: Arthur Heymans 
---
M src/drivers/pc80/rtc/mc146818rtc.c
1 file changed, 7 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35848/2

[Arthur Heymans (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option

Avoid searching for cmos_layout.bin each time get/set_option is called
on platforms with NO_CAR_GLOBAL_MIGRATION.

Change-Id: Ie541017d2f8c8e9d4b592b71f44a08fd9670dd09
Signed-off-by: Arthur Heymans 
---
M src/drivers/pc80/rtc/mc146818rtc.c
1 file changed, 7 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/35848/3

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

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

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

Patch Set 3: Code-Review+1

I think this should work, but I am not familiar about region_device in general.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35848 )

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option
......................................................................

drivers/pc80/rtc: Avoid searching for cmos_layout.bin on each get/set_option

Avoid searching for cmos_layout.bin each time get/set_option is called
on platforms with NO_CAR_GLOBAL_MIGRATION.

Change-Id: Ie541017d2f8c8e9d4b592b71f44a08fd9670dd09
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35848
Tested-by: build bot (Jenkins) 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Aaron Durbin 
---
M src/drivers/pc80/rtc/mc146818rtc.c
1 file changed, 7 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Aaron Durbin: Looks good to me, approved

diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c
index f6c748d..abf8b25 100644
--- a/src/drivers/pc80/rtc/mc146818rtc.c
+++ b/src/drivers/pc80/rtc/mc146818rtc.c
@@ -241,7 +241,7 @@
 static enum cb_err locate_cmos_layout(struct region_device *rdev)
 {
    uint32_t cbfs_type = CBFS_COMPONENT_CMOS_LAYOUT;
-	struct cbfsf fh;
+	MAYBE_STATIC_BSS struct cbfsf fh = {};

/*
     * In case VBOOT is enabled and this function is called from SMM,
@@ -250,11 +250,13 @@
     *
     * Support only one CMOS layout in the 'COREBOOT' region for now.
     */
-	if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin",
-				       &cbfs_type)) {
-		printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
+	if (!region_device_sz(&(fh.data))) {
+		if (cbfs_locate_file_in_region(&fh, "COREBOOT", "cmos_layout.bin",
+					       &cbfs_type)) {
+			printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. "
    					"Options are disabled\n");
-		return CB_CMOS_LAYOUT_NOT_FOUND;
+			return CB_CMOS_LAYOUT_NOT_FOUND;
+		}
    }

cbfs_file_data(rdev, &fh);