Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path ......................................................................
lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path
Change-Id: Ib9a9c88d6cd4842df447f046bc0abaa7ef5032c7 Signed-off-by: Patrick Georgi pgeorgi@google.com --- M src/lib/imd_cbmem.c 1 file changed, 1 insertion(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37361/1
diff --git a/src/lib/imd_cbmem.c b/src/lib/imd_cbmem.c index d7f7d20..4172fa4 100644 --- a/src/lib/imd_cbmem.c +++ b/src/lib/imd_cbmem.c @@ -69,16 +69,7 @@ */ static struct imd *imd_init_backing(struct imd *backing) { - struct imd *imd; - - imd = &imd_cbmem; - - if (imd != NULL) - return imd; - - imd = backing; - - return imd; + return &imd_cbmem; }
static struct imd *imd_init_backing_with_recover(struct imd *backing)
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37361/1//COMMIT_MSG@7 PS1, Line 7: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path lib/imd_cbmem: eliminate unnecessary NULL check
&imd_cbmem is never NULL, so remove that path.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/1/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/1/src/lib/imd_cbmem.c@58 PS1, Line 58: /* These are the different situations to handle: How would this be handled with this change?
Hello Kyösti Mälkki, Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37361
to look at the new patch set (#2).
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
lib/imd_cbmem: Eliminate unnecessary NULL check
&imd_cbmem is never NULL, so remove that path
Change-Id: Ib9a9c88d6cd4842df447f046bc0abaa7ef5032c7 Signed-off-by: Patrick Georgi pgeorgi@google.com --- M src/lib/imd_cbmem.c 1 file changed, 1 insertion(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/37361/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37361/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37361/1//COMMIT_MSG@7 PS1, Line 7: lib/imd_cbmem: &imd_cbmem is never NULL, so remove that path
lib/imd_cbmem: eliminate unnecessary NULL check […]
Done
https://review.coreboot.org/c/coreboot/+/37361/1/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/1/src/lib/imd_cbmem.c@58 PS1, Line 58: /* These are the different situations to handle:
How would this be handled with this change?
It's all done through cbmem_initialize_id_size setting up IMD near cbmem_top, but to be honest, the code flow isn't that clear to me either. I just know that this doesn't really change behavior (except for reducing stack size) because it always fell back to the code paths I left behind.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c@67 PS2, Line 67: * cbmem_top(); The global, imd_cbmem on line 46, needs to be initialized which it seems to be be in the cbmem_initialize() path. I believe this whole compilation unit can be dropped to rely on the global imd_cbmem.
Every sequence that calls imd_init_backing_with_recover() can be dropped and imd_init_backing() can be dropped as well.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c@67 PS2, Line 67: * cbmem_top();
The global, imd_cbmem on line 46, needs to be initialized which it seems to be be in the cbmem_initi […]
CB:37362
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c@67 PS2, Line 67: * cbmem_top();
CB:37362
Why not just squash everything?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/37361/2/src/lib/imd_cbmem.c@67 PS2, Line 67: * cbmem_top();
Why not just squash everything?
This is a refactoring, and I prefer those to be granular enough to be easily followed.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37361 )
Change subject: lib/imd_cbmem: Eliminate unnecessary NULL check ......................................................................
lib/imd_cbmem: Eliminate unnecessary NULL check
&imd_cbmem is never NULL, so remove that path
Change-Id: Ib9a9c88d6cd4842df447f046bc0abaa7ef5032c7 Signed-off-by: Patrick Georgi pgeorgi@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37361 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/lib/imd_cbmem.c 1 file changed, 1 insertion(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved
diff --git a/src/lib/imd_cbmem.c b/src/lib/imd_cbmem.c index d7f7d20..4172fa4 100644 --- a/src/lib/imd_cbmem.c +++ b/src/lib/imd_cbmem.c @@ -69,16 +69,7 @@ */ static struct imd *imd_init_backing(struct imd *backing) { - struct imd *imd; - - imd = &imd_cbmem; - - if (imd != NULL) - return imd; - - imd = backing; - - return imd; + return &imd_cbmem; }
static struct imd *imd_init_backing_with_recover(struct imd *backing)