Change in flashrom[master]: layout: Increase max rom layout size

[Patrick Rudolph (Code Review)]

Hello Patrick Rudolph,

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

https://review.coreboot.org/c/flashrom/+/34025

to review the following change.

Change subject: layout: Increase max rom layout size
......................................................................

layout: Increase max rom layout size

When trying to flash a single FMAP region on VBOOT enabled boards the default
of 32 entries is to small to store all regions.

Increase the maximum rom layout size to 128 to support complex FMAPs.

Change-Id: I68084b08f7b35a162b5f2d3109d82a8b63c194ff
Signed-off-by: Patrick Rudolph 
---
M layout.h
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/34025/1

diff --git a/layout.h b/layout.h
index 5c07407..1912ce2 100644
--- a/layout.h
+++ b/layout.h
@@ -33,7 +33,7 @@
 #define PRIxCHIPOFF "06"PRIx32
 #define PRIuCHIPSIZE PRIu32

-#define MAX_ROMLAYOUT	32
+#define MAX_ROMLAYOUT	128

struct romentry {
    chipoff_t start;

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 2: Code-Review+1

Tested it with a sf600 with a apollolake platform -works fine ( did not worked with 32 )

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 2: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 2:

Philipp told me that this resulted in a brick, I wonder
how? Doesn't flashrom bail out properly if the FMAP is
too big?

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 2: -Code-Review

That's... odd. Philipp, is that right? Any idea what's wrong, or should we just hold off on merging this patch until someone can investigate further?

[Patrick Rudolph (Code Review)]

Hello Patrick Rudolph, David Hendricks, Christian Walter, build bot (Jenkins),

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

https://review.coreboot.org/c/flashrom/+/34025

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

Change subject: layout: Increase max rom layout size
......................................................................

layout: Increase max rom layout size

When trying to flash a single FMAP region on VBOOT enabled boards the default
of 32 entries is to small to store all regions. Flashrom will bail out with
"Cannot add fmap entries to layout - Too many entries."

Increase the maximum rom layout size to 128 to support complex FMAPs.

Tested on coreboot's UP/squared mainboard using SF600.
With this patch it's possible to update a single FMAP region.

Change-Id: I68084b08f7b35a162b5f2d3109d82a8b63c194ff
Signed-off-by: Patrick Rudolph 
---
M layout.h
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/34025/3

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 3:
> Patch Set 2:
>
> Philipp told me that this resulted in a brick, I wonder
> how? Doesn't flashrom bail out properly if the FMAP is
> too big?

It does and there's no brick. Clarified commit message.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

Wouldn't be necessary with topic:fcp ;)

https://review.coreboot.org/c/flashrom/+/34025/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/34025/3//COMMIT_MSG@11 
PS3, Line 11: "Cannot add fmap entries to layout - Too many entries."
Please adhere to the 72 char line limit in commit messages.

[Patrick Rudolph (Code Review)]

Hello Patrick Rudolph, David Hendricks, Christian Walter, build bot (Jenkins), Nico Huber,

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

https://review.coreboot.org/c/flashrom/+/34025

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

Change subject: layout: Increase max rom layout size
......................................................................

layout: Increase max rom layout size

When trying to flash a single FMAP region on VBOOT enabled boards the
default of 32 entries is to small to store all regions. Flashrom will
bail out with "Cannot add fmap entries to layout - Too many entries."

Increase the maximum rom layout size to 128 to support complex FMAPs.

Tested on coreboot's UP/squared mainboard using SF600.
With this patch it's possible to update a single FMAP region.

Change-Id: I68084b08f7b35a162b5f2d3109d82a8b63c194ff
Signed-off-by: Patrick Rudolph 
---
M layout.h
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/34025/4

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

Patch Set 4: Verified+1

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/34025 )

Change subject: layout: Increase max rom layout size
......................................................................

layout: Increase max rom layout size

When trying to flash a single FMAP region on VBOOT enabled boards the
default of 32 entries is to small to store all regions. Flashrom will
bail out with "Cannot add fmap entries to layout - Too many entries."

Increase the maximum rom layout size to 128 to support complex FMAPs.

Tested on coreboot's UP/squared mainboard using SF600.
With this patch it's possible to update a single FMAP region.

Change-Id: I68084b08f7b35a162b5f2d3109d82a8b63c194ff
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/34025
Tested-by: Nico Huber 
Reviewed-by: Nico Huber 
Reviewed-by: Christian Walter 
---
M layout.h
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Nico Huber: Verified; Looks good to me, approved
  Christian Walter: Looks good to me, but someone else must approve

diff --git a/layout.h b/layout.h
index 53a20d6..8e4eb13 100644
--- a/layout.h
+++ b/layout.h
@@ -33,7 +33,7 @@
 #define PRIxCHIPOFF "06"PRIx32
 #define PRIuCHIPSIZE PRIu32

-#define MAX_ROMLAYOUT	32
+#define MAX_ROMLAYOUT	128

struct romentry {
    chipoff_t start;