Hello Mike Banon,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/30756
to review the following change.
Change subject: AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE ......................................................................
AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE
Only a few boards are using IMC for the onboard fan control, so regarding the availability of IMC selection it should be opt-in, not opt-out.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I3590b13c3b155405d61e373daf1bd82ca8e3bd16 --- M src/mainboard/asus/f2a85-m/Kconfig M src/mainboard/msi/ms7721/Kconfig M src/mainboard/pcengines/apu2/Kconfig M src/southbridge/amd/agesa/hudson/Kconfig M src/southbridge/amd/pi/hudson/Kconfig M src/vendorcode/amd/Kconfig 6 files changed, 11 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/30756/1
diff --git a/src/mainboard/asus/f2a85-m/Kconfig b/src/mainboard/asus/f2a85-m/Kconfig index fc43ed9..100b33c 100644 --- a/src/mainboard/asus/f2a85-m/Kconfig +++ b/src/mainboard/asus/f2a85-m/Kconfig @@ -30,7 +30,6 @@ select SUPERIO_NUVOTON_NCT6779D if BOARD_ASUS_F2A85_M_PRO select BOARD_ROMSIZE_KB_8192 select GFXUMA - select HUDSON_DISABLE_IMC
choice prompt "DDR3 memory voltage" diff --git a/src/mainboard/msi/ms7721/Kconfig b/src/mainboard/msi/ms7721/Kconfig index 29d4915..5fa4768 100644 --- a/src/mainboard/msi/ms7721/Kconfig +++ b/src/mainboard/msi/ms7721/Kconfig @@ -30,7 +30,6 @@ select SUPERIO_FINTEK_F71869AD select BOARD_ROMSIZE_KB_8192 select GFXUMA - select HUDSON_DISABLE_IMC
config MAINBOARD_DIR string diff --git a/src/mainboard/pcengines/apu2/Kconfig b/src/mainboard/pcengines/apu2/Kconfig index 78addb2..6e65a6e 100644 --- a/src/mainboard/pcengines/apu2/Kconfig +++ b/src/mainboard/pcengines/apu2/Kconfig @@ -28,7 +28,6 @@ select HAVE_MP_TABLE select HAVE_ACPI_TABLES select BOARD_ROMSIZE_KB_8192 - select HUDSON_DISABLE_IMC select USE_BLOBS select GENERIC_SPD_BIN select MAINBOARD_HAS_LPC_TPM diff --git a/src/southbridge/amd/agesa/hudson/Kconfig b/src/southbridge/amd/agesa/hudson/Kconfig index 0c6ac3e..14e616a 100644 --- a/src/southbridge/amd/agesa/hudson/Kconfig +++ b/src/southbridge/amd/agesa/hudson/Kconfig @@ -56,8 +56,13 @@ help Add Hudson 2/3/4 XHCI Firmware to support the onboard USB 3.0
+config HUDSON_IMC_ENABLE + bool + default n + config HUDSON_IMC_FWM - bool "Add imc firmware" + bool "Add IMC firmware" + depends on HUDSON_IMC_ENABLE default y if USE_BLOBS select SPI_FLASH_HAS_VOLATILE_GROUP if SPI_FLASH help diff --git a/src/southbridge/amd/pi/hudson/Kconfig b/src/southbridge/amd/pi/hudson/Kconfig index 3c70c2a..76ba480 100644 --- a/src/southbridge/amd/pi/hudson/Kconfig +++ b/src/southbridge/amd/pi/hudson/Kconfig @@ -22,9 +22,6 @@ config SOUTHBRIDGE_AMD_PI_KERN bool
-config HUDSON_DISABLE_IMC - bool - if SOUTHBRIDGE_AMD_PI_AVALON || SOUTHBRIDGE_AMD_PI_BOLTON || SOUTHBRIDGE_AMD_PI_KERN
config SOUTHBRIDGE_SPECIFIC_OPTIONS # dummy @@ -58,9 +55,13 @@ help Add Hudson 2/3/4 XHCI Firmware to support the onboard USB 3.0
+config HUDSON_IMC_ENABLE + bool + default n + config HUDSON_IMC_FWM bool "Add IMC firmware" - depends on !HUDSON_DISABLE_IMC + depends on HUDSON_IMC_ENABLE default y help Add Hudson 2/3/4 IMC Firmware to support the onboard fan control diff --git a/src/vendorcode/amd/Kconfig b/src/vendorcode/amd/Kconfig index 0c52b2f..3fe0c82 100644 --- a/src/vendorcode/amd/Kconfig +++ b/src/vendorcode/amd/Kconfig @@ -29,7 +29,6 @@
config CPU_AMD_AGESA_BINARY_PI bool "binary PI" - select HUDSON_DISABLE_IMC if CPU_AMD_PI_00730F01 || CPU_AMD_PI_00630F01 help Use a binary PI package. Generally, these will be stored in the "3rdparty/blobs" directory. For some processors, these must be obtained
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30756 )
Change subject: AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE ......................................................................
Patch Set 2: Code-Review+1
mikeb mikeb has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30756 )
Change subject: AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/30756/2/src/vendorcode/amd/Kconfig File src/vendorcode/amd/Kconfig:
https://review.coreboot.org/#/c/30756/2/src/vendorcode/amd/Kconfig@a32 PS2, Line 32:
This seems to be lost in this change.
I lost it on purpose because now it is opt-in regarding the IMC: unless you select HUDSON_IMC_ENABLE it will not be enabled for them.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30756 )
Change subject: AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30756 )
Change subject: AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE ......................................................................
AGESA/PI: replace HUDSON_DISABLE_IMC with HUDSON_IMC_ENABLE
Only a few boards are using IMC for the onboard fan control, so regarding the availability of IMC selection it should be opt-in, not opt-out. Also, select HUDSON_IMC_ENABLE for Gizmo 2 because Gizmo 2 could use IMC for the onboard fan control.
Signed-off-by: Mike Banon mikebdp2@gmail.com Change-Id: I3590b13c3b155405d61e373daf1bd82ca8e3bd16 Reviewed-on: https://review.coreboot.org/c/30756 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Michał Żygowski michal.zygowski@3mdeb.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/asus/f2a85-m/Kconfig M src/mainboard/gizmosphere/gizmo2/Kconfig M src/mainboard/msi/ms7721/Kconfig M src/mainboard/pcengines/apu2/Kconfig M src/southbridge/amd/agesa/hudson/Kconfig M src/southbridge/amd/pi/hudson/Kconfig M src/vendorcode/amd/Kconfig 7 files changed, 12 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Michał Żygowski: Looks good to me, but someone else must approve
diff --git a/src/mainboard/asus/f2a85-m/Kconfig b/src/mainboard/asus/f2a85-m/Kconfig index fc43ed9..100b33c 100644 --- a/src/mainboard/asus/f2a85-m/Kconfig +++ b/src/mainboard/asus/f2a85-m/Kconfig @@ -30,7 +30,6 @@ select SUPERIO_NUVOTON_NCT6779D if BOARD_ASUS_F2A85_M_PRO select BOARD_ROMSIZE_KB_8192 select GFXUMA - select HUDSON_DISABLE_IMC
choice prompt "DDR3 memory voltage" diff --git a/src/mainboard/gizmosphere/gizmo2/Kconfig b/src/mainboard/gizmosphere/gizmo2/Kconfig index f5c9481..94158ba 100644 --- a/src/mainboard/gizmosphere/gizmo2/Kconfig +++ b/src/mainboard/gizmosphere/gizmo2/Kconfig @@ -28,6 +28,7 @@ select HAVE_ACPI_TABLES select BOARD_ROMSIZE_KB_4096 select GFXUMA + select HUDSON_IMC_ENABLE
config MAINBOARD_DIR string diff --git a/src/mainboard/msi/ms7721/Kconfig b/src/mainboard/msi/ms7721/Kconfig index 29d4915..5fa4768 100644 --- a/src/mainboard/msi/ms7721/Kconfig +++ b/src/mainboard/msi/ms7721/Kconfig @@ -30,7 +30,6 @@ select SUPERIO_FINTEK_F71869AD select BOARD_ROMSIZE_KB_8192 select GFXUMA - select HUDSON_DISABLE_IMC
config MAINBOARD_DIR string diff --git a/src/mainboard/pcengines/apu2/Kconfig b/src/mainboard/pcengines/apu2/Kconfig index 78addb2..6e65a6e 100644 --- a/src/mainboard/pcengines/apu2/Kconfig +++ b/src/mainboard/pcengines/apu2/Kconfig @@ -28,7 +28,6 @@ select HAVE_MP_TABLE select HAVE_ACPI_TABLES select BOARD_ROMSIZE_KB_8192 - select HUDSON_DISABLE_IMC select USE_BLOBS select GENERIC_SPD_BIN select MAINBOARD_HAS_LPC_TPM diff --git a/src/southbridge/amd/agesa/hudson/Kconfig b/src/southbridge/amd/agesa/hudson/Kconfig index b80f734..22e0b6f 100644 --- a/src/southbridge/amd/agesa/hudson/Kconfig +++ b/src/southbridge/amd/agesa/hudson/Kconfig @@ -55,8 +55,13 @@ help Add Hudson 2/3/4 XHCI Firmware to support the onboard USB 3.0
+config HUDSON_IMC_ENABLE + bool + default n + config HUDSON_IMC_FWM - bool "Add imc firmware" + bool "Add IMC firmware" + depends on HUDSON_IMC_ENABLE default y if USE_BLOBS select SPI_FLASH_HAS_VOLATILE_GROUP if SPI_FLASH help diff --git a/src/southbridge/amd/pi/hudson/Kconfig b/src/southbridge/amd/pi/hudson/Kconfig index 3c70c2a..76ba480 100644 --- a/src/southbridge/amd/pi/hudson/Kconfig +++ b/src/southbridge/amd/pi/hudson/Kconfig @@ -22,9 +22,6 @@ config SOUTHBRIDGE_AMD_PI_KERN bool
-config HUDSON_DISABLE_IMC - bool - if SOUTHBRIDGE_AMD_PI_AVALON || SOUTHBRIDGE_AMD_PI_BOLTON || SOUTHBRIDGE_AMD_PI_KERN
config SOUTHBRIDGE_SPECIFIC_OPTIONS # dummy @@ -58,9 +55,13 @@ help Add Hudson 2/3/4 XHCI Firmware to support the onboard USB 3.0
+config HUDSON_IMC_ENABLE + bool + default n + config HUDSON_IMC_FWM bool "Add IMC firmware" - depends on !HUDSON_DISABLE_IMC + depends on HUDSON_IMC_ENABLE default y help Add Hudson 2/3/4 IMC Firmware to support the onboard fan control diff --git a/src/vendorcode/amd/Kconfig b/src/vendorcode/amd/Kconfig index 0c52b2f..3fe0c82 100644 --- a/src/vendorcode/amd/Kconfig +++ b/src/vendorcode/amd/Kconfig @@ -29,7 +29,6 @@
config CPU_AMD_AGESA_BINARY_PI bool "binary PI" - select HUDSON_DISABLE_IMC if CPU_AMD_PI_00730F01 || CPU_AMD_PI_00630F01 help Use a binary PI package. Generally, these will be stored in the "3rdparty/blobs" directory. For some processors, these must be obtained