Hello Philip Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44949
to review the following change.
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
trogdor: Shuffle RAM and SKU ID pins (again)
We're moving a lot of pins around on Trogdor again. For firmware this only affects the RAM and SKU strapping ID pins. Since there are quite a few of the old devices in circulation this time and some people seem to care about mosys RAM information working, let's actually check the board revision and support both cases this time.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: If7728d8ea4b7f6e7ff6721ade90f975f6efd5ddd --- M src/mainboard/google/trogdor/boardid.c 1 file changed, 25 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/44949/1
diff --git a/src/mainboard/google/trogdor/boardid.c b/src/mainboard/google/trogdor/boardid.c index c1a4892..f60dddbe 100644 --- a/src/mainboard/google/trogdor/boardid.c +++ b/src/mainboard/google/trogdor/boardid.c @@ -2,6 +2,7 @@
#include <boardid.h> #include <gpio.h> +#include <types.h>
uint32_t board_id(void) { @@ -15,14 +16,27 @@ return id; }
+/* Whether a revision was built before or after the great pin migration of August 2020. */ +static bool use_old_pins(void) +{ + return ((CONFIG(BOARD_GOOGLE_TROGDOR) && board_id() < 2) || + (CONFIG(BOARD_GOOGLE_LAZOR) && board_id() < 3) || + (CONFIG(BOARD_GOOGLE_POMPOM) && board_id() < 1)); +} + uint32_t ram_code(void) { static uint32_t id = UNDEFINED_STRAPPING_ID;
- const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; + const gpio_t old_pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; + const gpio_t pins[] = {[2] = GPIO(5), [1] = GPIO(3), [0] = GPIO(1)};
- if (id == UNDEFINED_STRAPPING_ID) - id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + if (id == UNDEFINED_STRAPPING_ID) { + if (use_old_pins()) + id = gpio_base2_value(old_pins, ARRAY_SIZE(old_pins)); + else + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + }
return id; } @@ -31,10 +45,15 @@ { static uint32_t id = UNDEFINED_STRAPPING_ID;
- const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; + const gpio_t old_pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; + const gpio_t pins[] = {[2] = GPIO(2), [1] = GPIO(90), [0] = GPIO(58)};
- if (id == UNDEFINED_STRAPPING_ID) - id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + if (id == UNDEFINED_STRAPPING_ID) { + if (use_old_pins()) + id = gpio_base2_value(old_pins, ARRAY_SIZE(old_pins)); + else + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + }
return id; }
Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44949 )
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
Patch Set 1: Code-Review+1
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44949 )
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
Patch Set 1: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44949 )
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
Patch Set 1:
Philip, you're the only one here that can +2... :P
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44949 )
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44949 )
Change subject: trogdor: Shuffle RAM and SKU ID pins (again) ......................................................................
trogdor: Shuffle RAM and SKU ID pins (again)
We're moving a lot of pins around on Trogdor again. For firmware this only affects the RAM and SKU strapping ID pins. Since there are quite a few of the old devices in circulation this time and some people seem to care about mosys RAM information working, let's actually check the board revision and support both cases this time.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: If7728d8ea4b7f6e7ff6721ade90f975f6efd5ddd Reviewed-on: https://review.coreboot.org/c/coreboot/+/44949 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Douglas Anderson dianders@chromium.org Reviewed-by: Philip Chen philipchen@google.com Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/mainboard/google/trogdor/boardid.c 1 file changed, 25 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Philip Chen: Looks good to me, but someone else must approve Douglas Anderson: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/trogdor/boardid.c b/src/mainboard/google/trogdor/boardid.c index c1a4892..f60dddbe 100644 --- a/src/mainboard/google/trogdor/boardid.c +++ b/src/mainboard/google/trogdor/boardid.c @@ -2,6 +2,7 @@
#include <boardid.h> #include <gpio.h> +#include <types.h>
uint32_t board_id(void) { @@ -15,14 +16,27 @@ return id; }
+/* Whether a revision was built before or after the great pin migration of August 2020. */ +static bool use_old_pins(void) +{ + return ((CONFIG(BOARD_GOOGLE_TROGDOR) && board_id() < 2) || + (CONFIG(BOARD_GOOGLE_LAZOR) && board_id() < 3) || + (CONFIG(BOARD_GOOGLE_POMPOM) && board_id() < 1)); +} + uint32_t ram_code(void) { static uint32_t id = UNDEFINED_STRAPPING_ID;
- const gpio_t pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; + const gpio_t old_pins[] = {[2] = GPIO(13), [1] = GPIO(19), [0] = GPIO(29)}; + const gpio_t pins[] = {[2] = GPIO(5), [1] = GPIO(3), [0] = GPIO(1)};
- if (id == UNDEFINED_STRAPPING_ID) - id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + if (id == UNDEFINED_STRAPPING_ID) { + if (use_old_pins()) + id = gpio_base2_value(old_pins, ARRAY_SIZE(old_pins)); + else + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + }
return id; } @@ -31,10 +45,15 @@ { static uint32_t id = UNDEFINED_STRAPPING_ID;
- const gpio_t pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; + const gpio_t old_pins[] = {[2] = GPIO(20), [1] = GPIO(90), [0] = GPIO(105)}; + const gpio_t pins[] = {[2] = GPIO(2), [1] = GPIO(90), [0] = GPIO(58)};
- if (id == UNDEFINED_STRAPPING_ID) - id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + if (id == UNDEFINED_STRAPPING_ID) { + if (use_old_pins()) + id = gpio_base2_value(old_pins, ARRAY_SIZE(old_pins)); + else + id = gpio_base2_value(pins, ARRAY_SIZE(pins)); + }
return id; }
-
Aaron Durbin (Code Review) -
Douglas Anderson (Code Review)
-
Julius Werner (Code Review)
-
Philip Chen (Code Review)