Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
mb/google/volteer: Source Memory strap GPIOs from variant
Allow memory strap GPIOs to be sourced from variant gpio.h. This is done to fix compilation issue of macro redefinition when one of the variant uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c 2 files changed, 1 insertion(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h index 4e2733e..26308cf1 100644 --- a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h +++ b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h @@ -12,12 +12,6 @@ /* BIOS Flash Write Protect */ #define GPIO_PCH_WP GPP_B11
-/* Memory configuration board straps */ -#define GPIO_MEM_CONFIG_0 GPP_C12 -#define GPIO_MEM_CONFIG_1 GPP_C15 -#define GPIO_MEM_CONFIG_2 GPP_C14 -#define GPIO_MEM_CONFIG_3 GPP_D15 - /* EC wake is LAN_WAKE# */ #define GPE_EC_WAKE GPE0_LAN_WAK
diff --git a/src/mainboard/google/volteer/variants/baseboard/memory.c b/src/mainboard/google/volteer/variants/baseboard/memory.c index dafeb3b..d5a99a9 100644 --- a/src/mainboard/google/volteer/variants/baseboard/memory.c +++ b/src/mainboard/google/volteer/variants/baseboard/memory.c @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
-#include <baseboard/gpio.h> +#include <variant/gpio.h> #include <baseboard/variants.h> #include <gpio.h>
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#2).
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
mb/google/volteer: Source Memory strap GPIOs from variant
Allow memory strap GPIOs to be sourced from variant gpio.h. This is done to fix compilation issue of macro redefinition when one of the variant uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/halvor/include/variant/gpio.h M src/mainboard/google/volteer/variants/malefor/include/variant/gpio.h M src/mainboard/google/volteer/variants/terrador/include/variant/gpio.h M src/mainboard/google/volteer/variants/todor/include/variant/gpio.h M src/mainboard/google/volteer/variants/trondo/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer/include/variant/gpio.h M src/mainboard/google/volteer/variants/voxel/include/variant/gpio.h 10 files changed, 41 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/2
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#3).
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
mb/google/volteer: Source Memory strap GPIOs from variant
Allow memory strap GPIOs to be sourced from variant gpio.h. This is done to fix compilation issue of macro redefinition when one of the variant uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/halvor/include/variant/gpio.h M src/mainboard/google/volteer/variants/malefor/include/variant/gpio.h M src/mainboard/google/volteer/variants/terrador/include/variant/gpio.h M src/mainboard/google/volteer/variants/todor/include/variant/gpio.h M src/mainboard/google/volteer/variants/trondo/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer/include/variant/gpio.h M src/mainboard/google/volteer/variants/voxel/include/variant/gpio.h 10 files changed, 41 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/46234/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46234/3//COMMIT_MSG@9 PS3, Line 9: Allow memory strap GPIOs to be sourced from variant gpio.h. : This is done to fix compilation issue of macro redefinition : when one of the variant uses a different set of memory strap : GPIOs. nit: reflow for 72 chars
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/halvor/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... PS3, Line 13: #define GPIO_MEM_CONFIG_0 GPP_C12 : #define GPIO_MEM_CONFIG_1 GPP_C15 : #define GPIO_MEM_CONFIG_2 GPP_C14 : #define GPIO_MEM_CONFIG_3 GPP_D15 I think instead of making a copy of GPIO_MEM_CONFIG_* in every variant/gpio.h, it would be better to make use of the weak/strong implementation of variant_memory_sku():
variants/baseboard/memory.c:
int __weak variant_memory_sku(void) { » gpio_t spd_gpios[] = { » » GPP_C12, » » GPP_C15, » » GPP_C14, » » GPP_D15, » };
» return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); }
i.e. replace the use of GPIO_MEM_CONFIG_* with actual GPIOs and the variant/ that uses a different GPIO combination can provide its own strong implementation.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Source Memory strap GPIOs from variant ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/halvor/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... PS3, Line 13: #define GPIO_MEM_CONFIG_0 GPP_C12 : #define GPIO_MEM_CONFIG_1 GPP_C15 : #define GPIO_MEM_CONFIG_2 GPP_C14 : #define GPIO_MEM_CONFIG_3 GPP_D15
I think instead of making a copy of GPIO_MEM_CONFIG_* in every variant/gpio. […]
+1 what Furquan said
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Sridhar Siricilla, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#4).
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
mb/google/volteer: Drop GPIO_MEM_CONFIG macros
Use the GPIO pad defintion directly and drop use of GPIO_MEM_CONFIG macros. This would simplify support for new variant that uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/boldar/include/variant/gpio.h M src/mainboard/google/volteer/variants/delbin/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/lindar/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer2/include/variant/gpio.h 8 files changed, 8 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
This would need update as well: https://review.coreboot.org/cgit/coreboot.git/tree/util/mainboard/google/vol...
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... PS4, Line 72: GPP_C12, It would be good to have a comment here which will make it easier to associate which GPIO corresponds to which config bit.
GPP_C12, /* GPIO_MEM_CONFIG_0 */ GPP_C15, /* GPIO_MEM_CONFIG_1 */ ...
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 4:
Ditto Furquan's comments. Can we add the change to the template file into this CL?
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Subrata Banik, Sridhar Siricilla, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#5).
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
mb/google/volteer: Drop GPIO_MEM_CONFIG macros
Use the GPIO pad defintion directly and drop use of GPIO_MEM_CONFIG macros. This would simplify support for new variant that uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/boldar/include/variant/gpio.h M src/mainboard/google/volteer/variants/delbin/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/lindar/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer2/include/variant/gpio.h M util/mainboard/google/volteer/template/include/variant/gpio.h 9 files changed, 8 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/5
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 5:
Patch Set 4: Code-Review+2
(1 comment)
This would need update as well: https://review.coreboot.org/cgit/coreboot.git/tree/util/mainboard/google/vol...
Thanks. Done.
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... PS4, Line 72: GPP_C12,
It would be good to have a comment here which will make it easier to associate which GPIO correspond […]
yeah..added comment
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/46234/4/src/mainboard/google/voltee... PS4, Line 72: GPP_C12,
yeah.. […]
Done
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Subrata Banik, Sridhar Siricilla, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#6).
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
mb/google/volteer: Drop GPIO_MEM_CONFIG macros
Use the GPIO pad definition directly and drop use of GPIO_MEM_CONFIG macros. This would simplify support for new variant that uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/boldar/include/variant/gpio.h M src/mainboard/google/volteer/variants/delbin/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/lindar/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer2/include/variant/gpio.h M util/mainboard/google/volteer/template/include/variant/gpio.h 9 files changed, 8 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/6
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46234/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46234/3//COMMIT_MSG@9 PS3, Line 9: Allow memory strap GPIOs to be sourced from variant gpio.h. : This is done to fix compilation issue of macro redefinition : when one of the variant uses a different set of memory strap : GPIOs.
nit: reflow for 72 chars
Done
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/halvor/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/46234/3/src/mainboard/google/voltee... PS3, Line 13: #define GPIO_MEM_CONFIG_0 GPP_C12 : #define GPIO_MEM_CONFIG_1 GPP_C15 : #define GPIO_MEM_CONFIG_2 GPP_C14 : #define GPIO_MEM_CONFIG_3 GPP_D15
+1 what Furquan said
Done
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 6: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 6: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 6: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Paul Fagerburg, Subrata Banik, Sridhar Siricilla, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46234
to look at the new patch set (#8).
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
mb/google/volteer: Drop GPIO_MEM_CONFIG macros
Use the GPIO pad definition directly and drop use of GPIO_MEM_CONFIG macros. This would simplify support for new variant that uses a different set of memory strap GPIOs.
Change-Id: I5c1ebfb39b2a56ca94a92941e8beed1acc28e655 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/volteer/variants/baseboard/include/baseboard/gpio.h M src/mainboard/google/volteer/variants/baseboard/memory.c M src/mainboard/google/volteer/variants/boldar/include/variant/gpio.h M src/mainboard/google/volteer/variants/delbin/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/include/variant/gpio.h M src/mainboard/google/volteer/variants/eldrid/memory.c M src/mainboard/google/volteer/variants/elemi/include/variant/gpio.h M src/mainboard/google/volteer/variants/elemi/memory.c M src/mainboard/google/volteer/variants/lindar/include/variant/gpio.h M src/mainboard/google/volteer/variants/volteer2/include/variant/gpio.h M util/mainboard/google/volteer/template/include/variant/gpio.h 11 files changed, 8 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46234/8
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Patch Set 8: Code-Review+1
Just need to fix the extra \n at the end of elemi/memory.c and it's all good.
Aamir Bohra has removed Paul Fagerburg from this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Removed reviewer Paul Fagerburg with the following votes:
* Code-Review+1 by Paul Fagerburg pfagerburg@chromium.org
Aamir Bohra has removed Tim Wawrzynczak from this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Removed reviewer Tim Wawrzynczak.
Aamir Bohra has removed Tim Wawrzynczak from this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Removed reviewer Tim Wawrzynczak.
Aamir Bohra has removed Furquan Shaikh from this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Removed reviewer Furquan Shaikh.
Aamir Bohra has removed Sowmya V from this change. ( https://review.coreboot.org/c/coreboot/+/46234 )
Change subject: mb/google/volteer: Drop GPIO_MEM_CONFIG macros ......................................................................
Removed reviewer Sowmya V.