Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37591 )
Change subject: mb/google/hatch/variant/hatch: Config MEM_STRAP GPIOs ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37591/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37591/1//COMMIT_MSG@9 PS1, Line 9: Hatch always used the default MEM_STRAPs in baseboard.
I know however the assumptions already encoded in baseboard for all variants to be laptop/tablets has been superseeded.
Independent of the form factor, the baseboard GPIOs are just one way of configuring the GPIOs which applies to most variants. And only the variants that do not follow this model then can provide their own overrides. I understand that we are supporting more and more variants and so the baseboard GPIOs might not be right for them. In that case it is better to think whether the baseboard GPIOs even make sense for the variant at all and if not, then just switch to providing a complete base GPIO table in that variant.
https://review.coreboot.org/c/coreboot/+/37591/1//COMMIT_MSG@10 PS1, Line 10: in the event that MEM_STRAP is set : differently in the baseboard gpio file
This is not unlike commit 5a0edcbde153 (https://review.coreboot.org/c/coreboot/+/37106) and fixed in https://review.coreboot.org/c/coreboot/+/37589
I don't think there was anything broken with 5a0edcbde153. I had added a comment indicating that the early_gpio_table configuration was intentionally skipped. Doing the configuration in early_gpio_table isn't wrong.
The memstrap muxing is a primary case of assumptions made in baseboard that no longer generalise across all variants, in fact not all variants even have the same muxing.
Agreed. And the variants that do not use the same muxing override it in the variant specific GPIO tables. I know variant "hatch" has been neglected for a while, so there can be things broken there.