Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47369 )
Change subject: mb/google/zork: Add func fpmcu_needs_delay ......................................................................
mb/google/zork: Add func fpmcu_needs_delay
This adds a function to tell if boards need an extra delay for the FPMCU's reset logic.
BUG=b:171837716 TEST=Boot on early board, see extra delay BRANCH=Zork
Signed-off-by: Martin Roth martin@coreboot.org Change-Id: I60eec69aff0715b23fdf8ec695674f708fb37bcc --- M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/berknip/variant.c M src/mainboard/google/zork/variants/morphius/variant.c 4 files changed, 37 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/47369/1
diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index 624d1b2..2fc2bde 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -155,5 +155,9 @@ /* Default to no fingerprint sensor */ return 0; } + +__weak int fpmcu_needs_delay(void) +{ + /* Default to no delay */ return 0; } diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index 4ba7843..054094a 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -33,6 +33,9 @@ /* Return 1 if the board has a fingerprint sensor. Return 0 otherwise */ int variant_has_fingerprint(void);
+/* Return 1 if the board has an FPMCU that needs an extra delay. Return 0 otherwise. */ +int fpmcu_needs_delay(void); + /* Modify devictree settings during ramstage. */ void variant_devtree_update(void); /* Update audio configuration in devicetree during ramstage. */ diff --git a/src/mainboard/google/zork/variants/berknip/variant.c b/src/mainboard/google/zork/variants/berknip/variant.c index aba5527..714063b 100644 --- a/src/mainboard/google/zork/variants/berknip/variant.c +++ b/src/mainboard/google/zork/variants/berknip/variant.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
#include <baseboard/variants.h> +#include <ec/google/chromeec/ec.h>
void variant_devtree_update(void) { @@ -20,3 +21,17 @@ { return 1; } + +int fpmcu_needs_delay(void) +{ + uint32_t board_version; + + if (google_chromeec_cbi_get_board_version(&board_version)) + board_version = 1; + + /* The FPMCU on early boards needs an extra delay */ + if (board_version <= 3) + return 1; + + return 0; +} diff --git a/src/mainboard/google/zork/variants/morphius/variant.c b/src/mainboard/google/zork/variants/morphius/variant.c index 2e7ed8a..bf61396 100644 --- a/src/mainboard/google/zork/variants/morphius/variant.c +++ b/src/mainboard/google/zork/variants/morphius/variant.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
#include <baseboard/variants.h> +#include <ec/google/chromeec/ec.h>
void variant_devtree_update(void) { @@ -20,3 +21,17 @@ /* All Morphius SKUs have a fingerprint sensor */ return 1; } + +int fpmcu_needs_delay(void) +{ + uint32_t board_version; + + if (google_chromeec_cbi_get_board_version(&board_version)) + board_version = 1; + + /* The FPMCU on early boards needs an extra delay */ + if (board_version <= 4) + return 1; + + return 0; +}
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47369 )
Change subject: mb/google/zork: Add func fpmcu_needs_delay ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47369/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47369/1/src/mainboard/google/zork/v... PS1, Line 37: fpmcu_needs_delay A thought: Instead of having a function call back, does it make sense to have a Kconfig here like https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th....
* VARIANT_HAS_BROKEN_FPMCU_POWER * VARIANT_MAX_BOARD_ID_BROKEN_FMPCU_POWER
and these can be used directly in gpio_baseboard_trembyle.c. It is one less variant function to care about and newer boards using newer schematics don't need to worry about this at all. (I know weak function provides the same functionality, but having the Kconfig allows easier audit to decide when to drop the support for these quirks).
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47369 )
Change subject: mb/google/zork: Add func fpmcu_needs_delay ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47369/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47369/1/src/mainboard/google/zork/v... PS1, Line 37: fpmcu_needs_delay
A thought: Instead of having a function call back, does it make sense to have a Kconfig here like ht […]
Sure, that works. Let me update the patch. I can also add a kconfig option for whether or not the board has a fingerprint sensor at all. That'll simplify things even more.
Martin Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47369 )
Change subject: mb/google/zork: Add func fpmcu_needs_delay ......................................................................
Abandoned
No longer needed.