Hello Mike Banon,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31216
to review the following change.
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS
Disable xHCI by default if USE_BLOBS option has not been selected.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I1c3f0ff49fbe3db3ef095d99055f75d65cd6f661 --- M src/southbridge/amd/agesa/hudson/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/31216/1
diff --git a/src/southbridge/amd/agesa/hudson/Kconfig b/src/southbridge/amd/agesa/hudson/Kconfig index 3bb9385..8b6a60e 100644 --- a/src/southbridge/amd/agesa/hudson/Kconfig +++ b/src/southbridge/amd/agesa/hudson/Kconfig @@ -38,7 +38,7 @@
config HUDSON_XHCI_ENABLE bool "Enable Hudson XHCI Controller" - default y + default y if USE_BLOBS help The XHCI controller must be enabled and the XHCI firmware must be added in order to have USB 3.0 support configured
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 1:
This is a fresh start of CB:30749
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
This is an improvement. Though, I still don't see why you insist on the indirect relation.
https://review.coreboot.org/#/c/31216/1/src/southbridge/amd/agesa/hudson/Kco... File src/southbridge/amd/agesa/hudson/Kconfig:
https://review.coreboot.org/#/c/31216/1/src/southbridge/amd/agesa/hudson/Kco... PS1, Line 41: default y if USE_BLOBS Why `USE_BLOBS`? why not make it explicit that this is related to `HUDSON_XHCI_FWM`? or, why should only one that selects `USE_BLOBS` get a sane default but somebody that selects `HUDSON_XHCI_FWM` not?
Hello Kyösti Mälkki, Felix Held, Paul Menzel, Mike Banon, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31216
to look at the new patch set (#2).
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS
Disable xHCI by default if USE_BLOBS option has not been selected.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I1c3f0ff49fbe3db3ef095d99055f75d65cd6f661 --- M src/southbridge/amd/agesa/hudson/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/31216/2
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31216/1/src/southbridge/amd/agesa/hudson/Kco... File src/southbridge/amd/agesa/hudson/Kconfig:
https://review.coreboot.org/#/c/31216/1/src/southbridge/amd/agesa/hudson/Kco... PS1, Line 41: default y if USE_BLOBS
Why `USE_BLOBS`? why not make it explicit that this […]
Thanks, now HUDSON_XHCI_ENABLE will be selected by default if someone selects HUDSON_XHCI_FWM
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31216/2/src/southbridge/amd/agesa/hudson/Kco... File src/southbridge/amd/agesa/hudson/Kconfig:
https://review.coreboot.org/#/c/31216/2/src/southbridge/amd/agesa/hudson/Kco... PS2, Line 41: USE_BLOBS || Now this explicitly adds the default y for the case of `USE_BLOBS && !HUDSON_XHCI_FWM` which is probably not intended.
Hello Kyösti Mälkki, Felix Held, Paul Menzel, Mike Banon, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31216
to look at the new patch set (#3).
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS
Disable xHCI by default if USE_BLOBS option has not been selected.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I1c3f0ff49fbe3db3ef095d99055f75d65cd6f661 --- M src/southbridge/amd/agesa/hudson/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/31216/3
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/31216/2/src/southbridge/amd/agesa/hudson/Kco... File src/southbridge/amd/agesa/hudson/Kconfig:
https://review.coreboot.org/#/c/31216/2/src/southbridge/amd/agesa/hudson/Kco... PS2, Line 41: USE_BLOBS ||
Now this explicitly adds the default y for the case […]
if I correctly checked the truth table, it should be "default y if HUDSON_XHCI_FWM" to avoid the case you have mentioned.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
Patch Set 3: Code-Review+2
Felix Held has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31216 )
Change subject: sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS ......................................................................
sb/amd/agesa/hudson/Kconfig: Disable xHCI by default if no USE_BLOBS
Disable xHCI by default if USE_BLOBS option has not been selected.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I1c3f0ff49fbe3db3ef095d99055f75d65cd6f661 Reviewed-on: https://review.coreboot.org/c/31216 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/southbridge/amd/agesa/hudson/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/southbridge/amd/agesa/hudson/Kconfig b/src/southbridge/amd/agesa/hudson/Kconfig index 3bb9385..394a196 100644 --- a/src/southbridge/amd/agesa/hudson/Kconfig +++ b/src/southbridge/amd/agesa/hudson/Kconfig @@ -38,7 +38,7 @@
config HUDSON_XHCI_ENABLE bool "Enable Hudson XHCI Controller" - default y + default y if HUDSON_XHCI_FWM help The XHCI controller must be enabled and the XHCI firmware must be added in order to have USB 3.0 support configured