Change in coreboot[master]: cbfs: remove prepare() callback from struct cbfs_locator

[Aaron Durbin (Code Review)]

Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

cbfs: remove prepare() callback from struct cbfs_locator

The prepare() callback is no longer utilized in the code. Remove
the callback and support for it.

Change-Id: Ic438e5a80850a3df619dbbfdecb522a9dc2c1949
Signed-off-by: Aaron Durbin 
---
M src/include/cbfs.h
M src/lib/cbfs.c
M src/lib/prog_loaders.c
3 files changed, 1 insertion(+), 22 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/36690/1

diff --git a/src/include/cbfs.h b/src/include/cbfs.h
index 85e25b3..540a481 100644
--- a/src/include/cbfs.h
+++ b/src/include/cbfs.h
@@ -76,16 +76,10 @@
 /* Return < 0 on error otherwise props are filled out accordingly. */
 int cbfs_boot_region_properties(struct cbfs_props *props);

-/* Allow external logic to take action prior to locating a program
- * (stage or payload). */
-void cbfs_prepare_program_locate(void);
-
 /* Object used to identify location of current cbfs to use for cbfs_boot_*
- * operations. It's used by cbfs_boot_region_properties() and
- * cbfs_prepare_program_locate(). */
+ * operations. It's used by cbfs_boot_region_properties(). */
 struct cbfs_locator {
    const char *name;
-	void (*prepare)(void);
    /* Returns 0 on successful fill of cbfs properties. */
    int (*locate)(struct cbfs_props *props);
 };
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 13b5afb..91a9d6a 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -385,16 +385,3 @@

return -1;
 }
-
-void cbfs_prepare_program_locate(void)
-{
-	int i;
-
-	boot_device_init();
-
-	for (i = 0; i < ARRAY_SIZE(locators); i++) {
-		if (locators[i]->prepare == NULL)
-			continue;
-		locators[i]->prepare();
-	}
-}
diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c
index 72c1de1..7f320d9 100644
--- a/src/lib/prog_loaders.c
+++ b/src/lib/prog_loaders.c
@@ -43,8 +43,6 @@
    if (prog_locate_hook(prog))
    	return -1;

-	cbfs_prepare_program_locate();
-
    if (cbfs_boot_locate(&file, prog_name(prog), NULL))
    	return -1;

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1: Code-Review+2

[Wim Vervoorn (Code Review)]

Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1: Code-Review-1

The prepare mechanism is used by the eltan verified boot mechanism. So removing this completely would break that.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1:
> The prepare mechanism is used by the eltan verified boot mechanism. So removing this completely would break that.

Didn't you guys already add prog_locate_hook() recently to do something similar? Could you maybe rewrite your vendorcode stuff to use that instead? It would be nice if we didn't need to maintain two hook mechanisms that are both only needed for a single platform when one of them is practically a superset of the other anyway.

(Also, since it looks like you're contributing directly to vboot and making it usable for your platform now, any chance we could drop your vendorcode solution and the associated hooks at some point?)

[Wim Vervoorn (Code Review)]

Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1:
> Patch Set 1:
> > The prepare mechanism is used by the eltan verified boot mechanism. So removing this completely would break that.
>
> Didn't you guys already add prog_locate_hook() recently to do something similar? Could you maybe rewrite your vendorcode stuff to use that instead? It would be nice if we didn't need to maintain two hook mechanisms that are both only needed for a single platform when one of them is practically a superset of the other anyway.
>
> (Also, since it looks like you're contributing directly to vboot and making it usable for your platform now, any chance we could drop your vendorcode solution and the associated hooks at some point?)

We will look at moving the prepare functionality to prog_locate_hook(). We will let you know if this works out in the next few days.

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1:

You might rebase this patchset on CB:3682.
The .preprare (and whole locator) has been replaced in vendorcode/eltan

[Wim Vervoorn (Code Review)]

Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 1: Code-Review+1

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36690 )

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

Patch Set 2: Code-Review+1

[Patrick Georgi (Code Review)]

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

Change subject: cbfs: remove prepare() callback from struct cbfs_locator
......................................................................

cbfs: remove prepare() callback from struct cbfs_locator

The prepare() callback is no longer utilized in the code. Remove
the callback and support for it.

Change-Id: Ic438e5a80850a3df619dbbfdecb522a9dc2c1949
Signed-off-by: Aaron Durbin 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36690
Tested-by: build bot (Jenkins) 
Reviewed-by: Frans Hendriks 
Reviewed-by: Julius Werner 
Reviewed-by: Wim Vervoorn 
---
M src/include/cbfs.h
M src/lib/cbfs.c
M src/lib/prog_loaders.c
3 files changed, 1 insertion(+), 22 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  Wim Vervoorn: Looks good to me, but someone else must approve
  Frans Hendriks: Looks good to me, but someone else must approve

diff --git a/src/include/cbfs.h b/src/include/cbfs.h
index d76fdd3..60129d3 100644
--- a/src/include/cbfs.h
+++ b/src/include/cbfs.h
@@ -79,16 +79,10 @@
 /* Return < 0 on error otherwise props are filled out accordingly. */
 int cbfs_boot_region_properties(struct cbfs_props *props);

-/* Allow external logic to take action prior to locating a program
- * (stage or payload). */
-void cbfs_prepare_program_locate(void);
-
 /* Object used to identify location of current cbfs to use for cbfs_boot_*
- * operations. It's used by cbfs_boot_region_properties() and
- * cbfs_prepare_program_locate(). */
+ * operations. It's used by cbfs_boot_region_properties(). */
 struct cbfs_locator {
    const char *name;
-	void (*prepare)(void);
    /* Returns 0 on successful fill of cbfs properties. */
    int (*locate)(struct cbfs_props *props);
 };
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 408e685..fbe6e43 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -359,16 +359,3 @@

return -1;
 }
-
-void cbfs_prepare_program_locate(void)
-{
-	int i;
-
-	boot_device_init();
-
-	for (i = 0; i < ARRAY_SIZE(locators); i++) {
-		if (locators[i]->prepare == NULL)
-			continue;
-		locators[i]->prepare();
-	}
-}
diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c
index 43f4689..5787496 100644
--- a/src/lib/prog_loaders.c
+++ b/src/lib/prog_loaders.c
@@ -43,8 +43,6 @@
    if (prog_locate_hook(prog))
    	return -1;

-	cbfs_prepare_program_locate();
-
    if (cbfs_boot_locate(&file, prog_name(prog), NULL))
    	return -1;