Change in coreboot[master]: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common pch/reset code block.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc D src/soc/intel/tigerlake/reset.c 13 files changed, 2 insertions(+), 228 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/1
diff --git a/src/soc/intel/cannonlake/Makefile.inc b/src/soc/intel/cannonlake/Makefile.inc index 96f1f97..7d829e9 100644 --- a/src/soc/intel/cannonlake/Makefile.inc +++ b/src/soc/intel/cannonlake/Makefile.inc @@ -26,7 +26,6 @@ romstage-y += i2c.c romstage-y += lpc.c romstage-y += pmutil.c -romstage-y += reset.c romstage-y += spi.c romstage-y += uart.c
@@ -45,7 +44,6 @@ ramstage-y += p2sb.c ramstage-y += pmc.c ramstage-y += pmutil.c -ramstage-y += reset.c ramstage-y += smmrelocate.c ramstage-y += spi.c ramstage-y += systemagent.c diff --git a/src/soc/intel/cannonlake/reset.c b/src/soc/intel/cannonlake/reset.c deleted file mode 100644 index d37ff54..0000000 --- a/src/soc/intel/cannonlake/reset.c +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <intelblocks/cse.h> -#include <intelblocks/pmclib.h> -#include <fsp/util.h> -#include <soc/intel/common/reset.h> -#include <soc/pci_devs.h> - -void do_global_reset(void) -{ - /* Ask CSE to do the global reset */ - if (cse_request_global_reset(GLOBAL_RESET)) - return; - - /* global reset if CSE fail to reset */ - pmc_global_reset_enable(1); - do_full_reset(); -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/common/pch/Kconfig b/src/soc/intel/common/pch/Kconfig index cca65d6..a2f38cc 100644 --- a/src/soc/intel/common/pch/Kconfig +++ b/src/soc/intel/common/pch/Kconfig @@ -42,6 +42,7 @@ select SOC_INTEL_COMMON_BLOCK_XDCI select SOC_INTEL_COMMON_BLOCK_XHCI select SOC_INTEL_COMMON_PCH_LOCKDOWN + select SOC_INTEL_COMMON_PCH_RESET select SOUTHBRIDGE_INTEL_COMMON_SMBUS
endif diff --git a/src/soc/intel/elkhartlake/Makefile.inc b/src/soc/intel/elkhartlake/Makefile.inc index 5c46f86..f642598 100644 --- a/src/soc/intel/elkhartlake/Makefile.inc +++ b/src/soc/intel/elkhartlake/Makefile.inc @@ -26,7 +26,6 @@ romstage-y += espi.c romstage-y += gpio.c romstage-y += meminit.c -romstage-y += reset.c
ramstage-y += acpi.c ramstage-y += chip.c @@ -39,7 +38,6 @@ ramstage-y += lockdown.c ramstage-y += p2sb.c ramstage-y += pmc.c -ramstage-y += reset.c ramstage-y += smmrelocate.c ramstage-y += systemagent.c ramstage-y += sd.c diff --git a/src/soc/intel/elkhartlake/reset.c b/src/soc/intel/elkhartlake/reset.c deleted file mode 100644 index 107db5a..0000000 --- a/src/soc/intel/elkhartlake/reset.c +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <fsp/util.h> -#include <intelblocks/cse.h> -#include <intelblocks/pmclib.h> -#include <soc/intel/common/reset.h> -#include <soc/pci_devs.h> - -void do_global_reset(void) -{ - /* Ask CSE to do the global reset */ - if (cse_request_global_reset(GLOBAL_RESET)) - return; - - /* global reset if CSE fail to reset */ - pmc_global_reset_enable(1); - do_full_reset(); -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/icelake/Makefile.inc b/src/soc/intel/icelake/Makefile.inc index 05f4846..58944a1 100644 --- a/src/soc/intel/icelake/Makefile.inc +++ b/src/soc/intel/icelake/Makefile.inc @@ -25,7 +25,6 @@
romstage-y += espi.c romstage-y += gpio.c -romstage-y += reset.c
ramstage-y += acpi.c ramstage-y += chip.c @@ -38,7 +37,6 @@ ramstage-y += lockdown.c ramstage-y += p2sb.c ramstage-y += pmc.c -ramstage-y += reset.c ramstage-y += smmrelocate.c ramstage-y += systemagent.c ramstage-y += sd.c diff --git a/src/soc/intel/icelake/reset.c b/src/soc/intel/icelake/reset.c deleted file mode 100644 index d37ff54..0000000 --- a/src/soc/intel/icelake/reset.c +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <intelblocks/cse.h> -#include <intelblocks/pmclib.h> -#include <fsp/util.h> -#include <soc/intel/common/reset.h> -#include <soc/pci_devs.h> - -void do_global_reset(void) -{ - /* Ask CSE to do the global reset */ - if (cse_request_global_reset(GLOBAL_RESET)) - return; - - /* global reset if CSE fail to reset */ - pmc_global_reset_enable(1); - do_full_reset(); -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/jasperlake/Makefile.inc b/src/soc/intel/jasperlake/Makefile.inc index 1cba218..dd5b249 100644 --- a/src/soc/intel/jasperlake/Makefile.inc +++ b/src/soc/intel/jasperlake/Makefile.inc @@ -26,7 +26,7 @@ romstage-y += espi.c romstage-y += gpio.c romstage-y += meminit.c -romstage-y += reset.c +
ramstage-y += acpi.c ramstage-y += chip.c @@ -39,7 +39,6 @@ ramstage-y += lockdown.c ramstage-y += p2sb.c ramstage-y += pmc.c -ramstage-y += reset.c ramstage-y += smmrelocate.c ramstage-y += systemagent.c ramstage-y += sd.c diff --git a/src/soc/intel/jasperlake/reset.c b/src/soc/intel/jasperlake/reset.c deleted file mode 100644 index d37ff54..0000000 --- a/src/soc/intel/jasperlake/reset.c +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <intelblocks/cse.h> -#include <intelblocks/pmclib.h> -#include <fsp/util.h> -#include <soc/intel/common/reset.h> -#include <soc/pci_devs.h> - -void do_global_reset(void) -{ - /* Ask CSE to do the global reset */ - if (cse_request_global_reset(GLOBAL_RESET)) - return; - - /* global reset if CSE fail to reset */ - pmc_global_reset_enable(1); - do_full_reset(); -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index de516c1..e52ef85 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -35,7 +35,6 @@ romstage-y += me.c romstage-y += pmc.c romstage-y += pmutil.c -romstage-y += reset.c romstage-y += spi.c romstage-y += uart.c
@@ -56,7 +55,6 @@ ramstage-y += p2sb.c ramstage-y += pmc.c ramstage-y += pmutil.c -ramstage-y += reset.c ramstage-y += sd.c ramstage-y += smmrelocate.c ramstage-y += spi.c diff --git a/src/soc/intel/skylake/reset.c b/src/soc/intel/skylake/reset.c deleted file mode 100644 index 1076ad2..0000000 --- a/src/soc/intel/skylake/reset.c +++ /dev/null @@ -1,46 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <fsp/util.h> -#include <intelblocks/pmclib.h> -#include <soc/intel/common/reset.h> -#include <soc/me.h> -#include <soc/pm.h> - -static void do_force_global_reset(void) -{ - /* - * BIOS should ensure it does a global reset - * to reset both host and Intel ME by setting - * PCH PMC [B0:D31:F2 register offset 0xAC bit 20] - */ - pmc_global_reset_enable(true); - - /* Now BIOS can write 0x06 or 0x0E to 0xCF9 port - * to global reset platform */ - do_full_reset(); -} - -void do_global_reset(void) -{ - if (!send_global_reset()) { - /* If ME unable to reset platform then - * force global reset using PMC CF9GR register*/ - do_force_global_reset(); - } -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/tigerlake/Makefile.inc b/src/soc/intel/tigerlake/Makefile.inc index c4f71c7..28e09ad 100644 --- a/src/soc/intel/tigerlake/Makefile.inc +++ b/src/soc/intel/tigerlake/Makefile.inc @@ -26,7 +26,6 @@ romstage-y += espi.c romstage-y += meminit.c romstage-y += gpio.c -romstage-y += reset.c
ramstage-y += acpi.c ramstage-y += chip.c @@ -39,7 +38,6 @@ ramstage-y += lockdown.c ramstage-y += p2sb.c ramstage-y += pmc.c -ramstage-y += reset.c ramstage-y += smmrelocate.c ramstage-y += soundwire.c ramstage-y += systemagent.c diff --git a/src/soc/intel/tigerlake/reset.c b/src/soc/intel/tigerlake/reset.c deleted file mode 100644 index d37ff54..0000000 --- a/src/soc/intel/tigerlake/reset.c +++ /dev/null @@ -1,34 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <cf9_reset.h> -#include <console/console.h> -#include <intelblocks/cse.h> -#include <intelblocks/pmclib.h> -#include <fsp/util.h> -#include <soc/intel/common/reset.h> -#include <soc/pci_devs.h> - -void do_global_reset(void) -{ - /* Ask CSE to do the global reset */ - if (cse_request_global_reset(GLOBAL_RESET)) - return; - - /* global reset if CSE fail to reset */ - pmc_global_reset_enable(1); - do_full_reset(); -} - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -}
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... File src/soc/intel/skylake/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... PS1, Line 25: void do_global_reset(void) SKL doesn't have CSE, and the reset function looks different.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... File src/soc/intel/skylake/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... PS1, Line 25: void do_global_reset(void)
SKL doesn't have CSE, and the reset function looks different.
Its not very different. just that SKL reset.c does an extra step before issuing global reset 1. Check the cse is enable and ME operating mode is normal if not then error and return 2. if normal then only send CSE command for global reset. 3. On failure force global reset using PMC
i believe, we should have this check for all platform
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... File src/soc/intel/skylake/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... PS1, Line 25: void do_global_reset(void)
Its not very different. […]
I agree. If SKL can use common CSE code, then perfect.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#2).
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common pch/reset code block.
Remove unused send_global_reset() from SKL directory.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/include/soc/me.h M src/soc/intel/skylake/me.c D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc D src/soc/intel/tigerlake/reset.c 15 files changed, 2 insertions(+), 248 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... File src/soc/intel/skylake/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/1/src/soc/intel/skylake/reset... PS1, Line 25: void do_global_reset(void)
I agree. If SKL can use common CSE code, then perfect.
Done now, marking resolve 😊
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 2:
I believe even APL can make use of the common reset functionality? It is probably just the FSP_STATUS value for global reset which is different?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 2:
Patch Set 2:
I believe even APL can make use of the common reset functionality? It is probably just the FSP_STATUS value for global reset which is different?
Agree
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#3).
Change subject: soc/intel/{apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common block/reset code block.
Remove unused send_global_reset() from SKL directory.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/include/soc/me.h M src/soc/intel/skylake/me.c D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c 25 files changed, 12 insertions(+), 316 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/3
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#4).
Change subject: soc/intel/{apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common block/reset code block.
Remove unused send_global_reset() from SKL directory.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/include/soc/me.h M src/soc/intel/skylake/me.c D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c 25 files changed, 12 insertions(+), 317 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/4
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#5).
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common block/reset code block.
Remove unused send_global_reset() from SKL directory.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/alderlake/chip.h D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/include/soc/me.h M src/soc/intel/skylake/me.c D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c 28 files changed, 13 insertions(+), 352 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/5
Werner Zeh has removed Name of user not set #1003096 from this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Removed reviewer null with the following votes:
* Verified+1 by Name of user not set (1003096)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
Change looks mostly okay to me. I just want to make sure that the changes for APL/GLK are not resulting in any regression.
https://review.coreboot.org/c/coreboot/+/45337/5/src/soc/intel/apollolake/Kc... File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/5/src/soc/intel/apollolake/Kc... PS5, Line 102: HAVE_CF9_RESET_PREPARE For APL, I think there are two different things:
1. cf9_reset_prepare: This just ensures that CSE is ready for upcoming reset. Used for system_reset and full_reset. This does not itself really do the reset. But, I think we are still going to need this? 2. do_global_reset: Currently, for APL, this does not make use of the CSE to perform the reset. Did you verify that doing a global reset via CSE works for APL/GLK as well?
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5: Code-Review+1
(1 comment)
Change looks mostly okay to me. I just want to make sure that the changes for APL/GLK are not resulting in any regression.
Yes Furquan, Validation is key here hence originally i had excluded APL into the reset common code scope. I'm working to get the board available online, Also requested Werner to help.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Thanks Werner for help. if you could just call this global reset function directly and see APL platform is able to survive the global reset, then we are good
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
yes Werner, we have this check here https://review.coreboot.org/c/coreboot/+/45341/7/src/soc/intel/common/block/... inside cse_request_reset() to ensure HECI is enable but it doesn't check if BAR is been implemented
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > @Matt: If you could help me to get this validated quickly on SKL platform > @Werner: Possible to check on APL platform > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
yes Werner, we have this check here https://review.coreboot.org/c/coreboot/+/45341/7/src/soc/intel/common/block/... inside cse_request_reset() to ensure HECI is enable but it doesn't check if BAR is been implemented
i guess call prior to heci_init() is key here. which is getting caleld from romstage.c
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > > Patch Set 5: > > > > @Matt: If you could help me to get this validated quickly on SKL platform > > @Werner: Possible to check on APL platform > > > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this. > > Hey Subrata. > We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
yes Werner, we have this check here https://review.coreboot.org/c/coreboot/+/45341/7/src/soc/intel/common/block/... inside cse_request_reset() to ensure HECI is enable but it doesn't check if BAR is been implemented
i guess call prior to heci_init() is key here. which is getting caleld from romstage.c
Currently heci_init() is not called for APL into romstage. I have called the function for test, but it does not change the global reset behavior during mainboard_init().
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > > Patch Set 5: > > > > > Patch Set 5: > > > > > > @Matt: If you could help me to get this validated quickly on SKL platform > > > @Werner: Possible to check on APL platform > > > > > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this. > > > > Hey Subrata. > > We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special? > > Hi Subrata, > I have tested your patches on mc_apl5. > do_global_reset() does not work at all times. > During mainboard_init() the HECI device is not available. > > Log output: > HECI: Global Reset(Type:1) Command > BUG: me_read_config32 requests hidden 00:0f.0 > PCI: dev is NULL! > > In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
yes Werner, we have this check here https://review.coreboot.org/c/coreboot/+/45341/7/src/soc/intel/common/block/... inside cse_request_reset() to ensure HECI is enable but it doesn't check if BAR is been implemented
i guess call prior to heci_init() is key here. which is getting caleld from romstage.c
Currently heci_init() is not called for APL into romstage. I have called the function for test, but it does not change the global reset behavior during mainboard_init().
got it, when you said, global reset is working in romstage is that after FSP-M return you mean ?
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > > Patch Set 5: > > > > > Patch Set 5: > > > > > > > Patch Set 5: > > > > > > > > @Matt: If you could help me to get this validated quickly on SKL platform > > > > @Werner: Possible to check on APL platform > > > > > > > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this. > > > > > > Hey Subrata. > > > We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special? > > > > Hi Subrata, > > I have tested your patches on mc_apl5. > > do_global_reset() does not work at all times. > > During mainboard_init() the HECI device is not available. > > > > Log output: > > HECI: Global Reset(Type:1) Command > > BUG: me_read_config32 requests hidden 00:0f.0 > > PCI: dev is NULL! > > > > In romstage and in mainboard_final (ramstage) the global reset works well. > > Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
And it seems like the HECI interface is involved in do_global_reset(). Do you have in mind that there are places in coreboot where the HECI interface might be hidden? Is there a check needed to make sure HECI is reachable?
yes Werner, we have this check here https://review.coreboot.org/c/coreboot/+/45341/7/src/soc/intel/common/block/... inside cse_request_reset() to ensure HECI is enable but it doesn't check if BAR is been implemented
i guess call prior to heci_init() is key here. which is getting caleld from romstage.c
Currently heci_init() is not called for APL into romstage. I have called the function for test, but it does not change the global reset behavior during mainboard_init().
got it, when you said, global reset is working in romstage is that after FSP-M return you mean ?
It’s before. I called the reset function into mainboard_memory_init_params().
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
I think it should be okay to drop reset.c from verstage. I don't think we need global reset before romstage anyways. But, it would be good to add a comment to explain the reason in that CL.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
@Matt: If you could help me to get this validated quickly on SKL platform @Werner: Possible to check on APL platform
I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
I think it should be okay to drop reset.c from verstage. I don't think we need global reset before romstage anyways. But, it would be good to add a comment to explain the reason in that CL.
Yes Furquan, there are 2 issues 1. reset.c can be included in verstage but cse_request_global_reset() as part of cse.c is not included in verstage hence need to drop reset.c from verstage as well. 2. global reset via CSE is not working before FSP-M as expected as DID command is not been send hence PMC is one is getting use there. Do we need to document that as well?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 6:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > @Matt: If you could help me to get this validated quickly on SKL platform > @Werner: Possible to check on APL platform > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this.
Hey Subrata. We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
I think it should be okay to drop reset.c from verstage. I don't think we need global reset before romstage anyways. But, it would be good to add a comment to explain the reason in that CL.
Yes Furquan, there are 2 issues
- reset.c can be included in verstage but cse_request_global_reset() as part of cse.c is not included in verstage hence need to drop reset.c from verstage as well.
- global reset via CSE is not working before FSP-M as expected as DID command is not been send hence PMC is one is getting use there. Do we need to document that as well?
Hi Mario,
Can I request you to check latest patch tree to verify once on MC_APL5 platform as you have done previously. Potential fix for this issue is here https://review.coreboot.org/c/coreboot/+/45469/1 (the one you have reported, "PCI: dev is NULL!"). So now you have to pick 4 patches in this tree to see if any issue. Again thanks a lot for your help.
Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > > Patch Set 5: > > > > @Matt: If you could help me to get this validated quickly on SKL platform > > @Werner: Possible to check on APL platform > > > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this. > > Hey Subrata. > We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special?
Hi Subrata, I have tested your patches on mc_apl5. do_global_reset() does not work at all times. During mainboard_init() the HECI device is not available.
Log output: HECI: Global Reset(Type:1) Command BUG: me_read_config32 requests hidden 00:0f.0 PCI: dev is NULL!
In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
I think it should be okay to drop reset.c from verstage. I don't think we need global reset before romstage anyways. But, it would be good to add a comment to explain the reason in that CL.
Yes Furquan, there are 2 issues
- reset.c can be included in verstage but cse_request_global_reset() as part of cse.c is not included in verstage hence need to drop reset.c from verstage as well.
- global reset via CSE is not working before FSP-M as expected as DID command is not been send hence PMC is one is getting use there. Do we need to document that as well?
Hi Mario,
Can I request you to check latest patch tree to verify once on MC_APL5 platform as you have done previously. Potential fix for this issue is here https://review.coreboot.org/c/coreboot/+/45469/1 (the one you have reported, "PCI: dev is NULL!"). So now you have to pick 4 patches in this tree to see if any issue. Again thanks a lot for your help.
With https://review.coreboot.org/c/coreboot/+/45469/1 do_global_reset() now also works during mainboard_init(). A side effect of enabling CSE over devicetree is that the reset no longer works before memory init, but that was not intended anyway.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 6:
Patch Set 6:
Patch Set 6:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
> Patch Set 5: > > > Patch Set 5: > > > > > Patch Set 5: > > > > > > @Matt: If you could help me to get this validated quickly on SKL platform > > > @Werner: Possible to check on APL platform > > > > > > I don't have SKL and APL connected remotely in current situation hence i'm working to enable those board for remote test setup. Your help might unblock this. > > > > Hey Subrata. > > We will have a look at this patch and give it a try on one of our APL-mainboard. I will provide feedback in a few hours. Is a successful boot enough or should I take a closer look to something special? > > Hi Subrata, > I have tested your patches on mc_apl5. > do_global_reset() does not work at all times. > During mainboard_init() the HECI device is not available. > > Log output: > HECI: Global Reset(Type:1) Command > BUG: me_read_config32 requests hidden 00:0f.0 > PCI: dev is NULL! > > In romstage and in mainboard_final (ramstage) the global reset works well.
Thanks a lot for your help. global reset is expected to work post romstage (DRAM initialization been done) somehow we might need to guard the global reset to ensure its not called from verstage or bootblock.
@Furquan, i might need to remove "reset.c" from verstage in https://review.coreboot.org/c/coreboot/+/45336/5/src/soc/intel/common/block/... / do you agree based on Mario's observation that "In romstage and in mainboard_final (ramstage) the global reset works well."
I think it should be okay to drop reset.c from verstage. I don't think we need global reset before romstage anyways. But, it would be good to add a comment to explain the reason in that CL.
Yes Furquan, there are 2 issues
- reset.c can be included in verstage but cse_request_global_reset() as part of cse.c is not included in verstage hence need to drop reset.c from verstage as well.
- global reset via CSE is not working before FSP-M as expected as DID command is not been send hence PMC is one is getting use there. Do we need to document that as well?
Hi Mario,
Can I request you to check latest patch tree to verify once on MC_APL5 platform as you have done previously. Potential fix for this issue is here https://review.coreboot.org/c/coreboot/+/45469/1 (the one you have reported, "PCI: dev is NULL!"). So now you have to pick 4 patches in this tree to see if any issue. Again thanks a lot for your help.
With https://review.coreboot.org/c/coreboot/+/45469/1 do_global_reset() now also works during mainboard_init(). A side effect of enabling CSE over devicetree is that the reset no longer works before memory init, but that was not intended anyway.
Yes, that is expected.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/5/src/soc/intel/apollolake/Kc... File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/5/src/soc/intel/apollolake/Kc... PS5, Line 102: HAVE_CF9_RESET_PREPARE
For APL, I think there are two different things: […]
Ack
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 7: Code-Review+1
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 7:
If you could help to review further.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 7: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 7: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Werner Zeh, Andrey Petrov, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#8).
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block
This patch removes all redundant reset code block and make use of common basecode/reset code block.
Remove unused send_global_reset() from SKL directory and CONFIG_HAVE_CF9_RESET_PREPARE selection from APL SoC Kconfig.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/alderlake/chip.h D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/pch/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/include/soc/me.h M src/soc/intel/skylake/me.c D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c 28 files changed, 13 insertions(+), 352 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/8
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 8: Code-Review+2
LGTM, but I haven't tested it. I'd wait for at least another ACK before submitting.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 8:
Patch Set 8: Code-Review+2
LGTM, but I haven't tested it. I'd wait for at least another ACK before submitting.
yes Angel, except SKL all testing done
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 8:
Patch Set 8:
yes Angel, except SKL all testing done
how exactly does this need to be tested? I can test SKL/KBL here, on devices with and without a disabled/neutered ME. If it's just a simple boot test, then a Librem 13v2 (SKL) passes
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 8:
Patch Set 8:
Patch Set 8:
yes Angel, except SKL all testing done
how exactly does this need to be tested? I can test SKL/KBL here, on devices with and without a disabled/neutered ME. If it's just a simple boot test, then a Librem 13v2 (SKL) passes
Try to invoke the global reset function and see if the machine is able to reset. It would be interesting to see if a non-working ME causes any problems, too.
Please don't get stuck in a reboot loop, though.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel/{adl,apl,cnl,ehl,icl,jsl,skl,tgl}: Make use of common reset code block ......................................................................
Patch Set 8: Code-Review+2
LGTM but let's wait for Matt's test on SKL
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Werner Zeh, Andrey Petrov, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#9).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Add FSP_STATUS_GLOBAL_RESET in each SoC's chip.h. Supported value: These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code Unsupported value: 0xFFFFFFFF
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/alderlake/chip.h D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Makefile.inc M src/soc/intel/denverton_ns/chip.h D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc M src/soc/intel/quark/chip.h D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc M src/soc/intel/xeon_sp/cpx/chip.h D src/soc/intel/xeon_sp/reset.c M src/soc/intel/xeon_sp/skx/chip.h 36 files changed, 56 insertions(+), 221 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/9
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/9/src/soc/intel/common/fsp_re... File src/soc/intel/common/fsp_reset.c:
https://review.coreboot.org/c/coreboot/+/45337/9/src/soc/intel/common/fsp_re... PS9, Line 22: if (FSP_STATUS_GLOBAL_RESET == 0xFFFFFFFF) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#10).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Add FSP_STATUS_GLOBAL_RESET in each SoC's chip.h. Supported value: These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code Unsupported value: 0xFFFFFFFF
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/alderlake/chip.h D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/chip.h D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/chip.h D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Makefile.inc M src/soc/intel/denverton_ns/chip.h D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/elkhartlake/chip.h D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/chip.h D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/chip.h D src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc M src/soc/intel/quark/chip.h D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/chip.h D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/chip.h D src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc M src/soc/intel/xeon_sp/cpx/chip.h D src/soc/intel/xeon_sp/reset.c M src/soc/intel/xeon_sp/skx/chip.h 36 files changed, 55 insertions(+), 221 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/10
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 10: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... File src/soc/intel/common/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... PS10, Line 32: postcar fsp_reset.c is really required only in romstage and ramstage. postcar does not integrate any FSP component.
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... File src/soc/intel/common/fsp_reset.c:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 3: #include <cf9_reset.h> Not required?
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 11: */ Probably add a #error here if FSP_STATUS_GLOBAL_RESET is not defined so that the user knows what needs to be done.
#ifndef FSP_STATUS_GLOBAL_RESET #error "Define FSP_STATUS_GLOBAL_RESET in SoC to match the FSP OEM status code for global reset" #endif
The other option if we want to avoid 0xffffffff being defined in SoC when global reset code is not present would be to add a Kconfig FSP_STATUS_GLOBAL_RESET in drivers/intel/fsp2_0/Kconfig which defaults to 0xffffffff.
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 22: Reorganize as: ``` if (status == FSP_STATUS_GLOBAL_RESET) { printk(BIOS_DEBUG, "GLOBAL RESET!\n"); global_reset(); }
printk(BIOS_ERR, "unhandled reset type %x\n", status); die("unknown reset type"); ```
Then you don't need to check for FSP_STATUS_GLOBAL_RESET separately.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#11).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Add CONFIG_FSP_STATUS_GLOBAL_RESET Kconfig to get correct FSP global reset type from respective SoC Kconfig. Supported value: 0x40000003-0x40000008, These are defined in FSP EAS v2.0 section 11.2.2 - OEM Status Code Unsupported value: 0xFFFFFFFF
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/common/Makefile.inc A src/soc/intel/common/basecode/reset/Kconfig A src/soc/intel/common/basecode/reset/Makefile.inc A src/soc/intel/common/basecode/reset/reset.c A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 29 files changed, 121 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/11
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 11:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... File src/soc/intel/common/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/Makef... PS10, Line 32: postcar
fsp_reset.c is really required only in romstage and ramstage. […]
Ack
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... File src/soc/intel/common/fsp_reset.c:
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 3: #include <cf9_reset.h>
Not required?
Ack
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 11: */
Probably add a #error here if FSP_STATUS_GLOBAL_RESET is not defined so that the user knows what nee […]
Ack
https://review.coreboot.org/c/coreboot/+/45337/10/src/soc/intel/common/fsp_r... PS10, Line 22:
Reorganize as: […]
Ack
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#12).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/Makefile.inc M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/common/Makefile.inc A src/soc/intel/common/basecode/reset/Kconfig A src/soc/intel/common/basecode/reset/Makefile.inc A src/soc/intel/common/basecode/reset/reset.c A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 28 files changed, 77 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/12
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 12:
(3 comments)
I think there is something wrong with the latest patchset.
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... PS11, Line 219: use used?
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... PS11, Line 256: correct global correct status value for global reset type?
https://review.coreboot.org/c/coreboot/+/45337/11/src/soc/intel/common/basec... File src/soc/intel/common/basecode/reset/reset.c:
PS11: I don't think this file is needed anymore. Looks like a bad rebase? Also, all reset.c files from SoCs are not deleted anymore in the latest patchset?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 12:
(3 comments)
Patch Set 12:
(3 comments)
I think there is something wrong with the latest patchset.
yeah, something happen while doing git add looks like
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... PS11, Line 219: use
used?
Ack
https://review.coreboot.org/c/coreboot/+/45337/11/src/drivers/intel/fsp2_0/K... PS11, Line 256: correct global
correct status value for global reset type?
Ack
https://review.coreboot.org/c/coreboot/+/45337/11/src/soc/intel/common/basec... File src/soc/intel/common/basecode/reset/reset.c:
PS11:
I don't think this file is needed anymore. Looks like a bad rebase? Also, all reset. […]
Ack
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#13).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/Makefile.inc D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc D src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc D src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 33 files changed, 30 insertions(+), 213 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/13
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 13:
@Furquan, kindly review this, would be good for me to start with next sets of ADL CL from Monday
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 13: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/13/src/soc/intel/alderlake/Kc... File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/13/src/soc/intel/alderlake/Kc... PS13, Line 38: FSP_STATUS_GLOBAL_RESET_REQUIRED_3 nit: add in alphabetical order here and in other SoC Kconfig files?
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#14).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/Makefile.inc D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc D src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc D src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 33 files changed, 30 insertions(+), 213 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/14
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/13/src/soc/intel/alderlake/Kc... File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/45337/13/src/soc/intel/alderlake/Kc... PS13, Line 38: FSP_STATUS_GLOBAL_RESET_REQUIRED_3
nit: add in alphabetical order here and in other SoC Kconfig files?
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 14: Code-Review+2
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#15).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/Makefile.inc D src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc D src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc D src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/Makefile.inc D src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc D src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc D src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc D src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 33 files changed, 30 insertions(+), 213 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/15
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15:
@Furquan, Need your help on this again. Merge need manual rebase and +2 gone after rebasing. so no changes but still +2 gone 😞
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/14/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
PS14: Quark doesn't have a CSE...
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/14/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
PS14:
Quark doesn't have a CSE...
did we include CSE for Quark. we have just migrate this logic into FSP driver for FSP2.0 code.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/14/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
PS14:
did we include CSE for Quark. we have just migrate this logic into FSP driver for FSP2.0 code.
It is now handled by the common reset code which provides the same function in common/reset.c. And Kconfig switch SOC_INTEL_COMMON_RESET is already selected.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 15:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/14/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
PS14:
It is now handled by the common reset code which provides the same function in common/reset.c. […]
Thanks Werner, I was thinking to revive this CL.
May be you or Angel (if having time) please feel free to pick this up.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#16).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c).
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/reset.c M src/soc/intel/common/Makefile.inc A src/soc/intel/common/fsp_reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/reset.c M src/soc/intel/quark/Makefile.inc D src/soc/intel/quark/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/reset.c M src/soc/intel/xeon_sp/Makefile.inc D src/soc/intel/xeon_sp/reset.c 25 files changed, 30 insertions(+), 177 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/16
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/14/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
PS14:
Thanks Werner, I was thinking to revive this CL. […]
Ack
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 17:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/common/Makef... File src/soc/intel/common/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/common/Makef... PS17, Line 30: CONFIG_PLATFORM_USES_FSP2_0 I would add a `config SOC_INTEL_COMMON_FSP_RESET` option and select it on the platforms that can use the code. It could also depend on CSE support.
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/quark/reset.... PS17, Line 7: void chipset_handle_reset(uint32_t status) Using common code for Quark will result in it failing to reboot when Quark FSP requests a reset. I would keep this as-is for now.
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/xeon_sp/rese... File src/soc/intel/xeon_sp/reset.c:
PS17: I doubt Xeon-SP FSP reset requests will be compatible with the SoC common reset code. I would keep this as-is for now.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#18).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c) based on SOC_INTEL_COMMON_FSP_RESET.
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/reset.c M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/denverton_ns/Makefile.inc D src/soc/intel/denverton_ns/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/reset.c 19 files changed, 18 insertions(+), 152 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/18
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/common/Makef... File src/soc/intel/common/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/common/Makef... PS17, Line 30: CONFIG_PLATFORM_USES_FSP2_0
I would add a `config SOC_INTEL_COMMON_FSP_RESET` option and select it on the platforms that can use […]
Ack
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/quark/reset.... File src/soc/intel/quark/reset.c:
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/quark/reset.... PS17, Line 7: void chipset_handle_reset(uint32_t status)
Using common code for Quark will result in it failing to reboot when Quark FSP requests a reset. […]
Ack
https://review.coreboot.org/c/coreboot/+/45337/17/src/soc/intel/xeon_sp/rese... File src/soc/intel/xeon_sp/reset.c:
PS17:
I doubt Xeon-SP FSP reset requests will be compatible with the SoC common reset code. […]
Ack
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#19).
Change subject: soc/intel: Make use of common reset code block ......................................................................
soc/intel: Make use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c) based on SOC_INTEL_COMMON_FSP_RESET.
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/reset.c 16 files changed, 16 insertions(+), 132 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/19
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Make use of common reset code block ......................................................................
Patch Set 19: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45337/19//COMMIT_MSG@7 PS19, Line 7: Make use of just `Use`
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Matt DeVillier, Angel Pons, Michael Niewöhner, Andrey Petrov, Patrick Rudolph, David Guckian, Martin Roth, Tim Wawrzynczak, Vanessa Eusebio, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45337
to look at the new patch set (#20).
Change subject: soc/intel: Use of common reset code block ......................................................................
soc/intel: Use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c) based on SOC_INTEL_COMMON_FSP_RESET.
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/reset.c 16 files changed, 16 insertions(+), 132 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/45337/20
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Use of common reset code block ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45337/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45337/19//COMMIT_MSG@7 PS19, Line 7: Make use of
just `Use`
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45337 )
Change subject: soc/intel: Use of common reset code block ......................................................................
soc/intel: Use of common reset code block
This patch removes all redundant reset code block from each SoC and make use of common reset code block(fsp_reset.c) based on SOC_INTEL_COMMON_FSP_RESET.
Respective SoC Kconfig to choose correct FSP global reset type as per FSP integration guide.
Signed-off-by: Subrata Banik subrata.banik@intel.com Change-Id: I71531f4cf7a40efa9ec55c48c2cb4fb6ea90531f Reviewed-on: https://review.coreboot.org/c/coreboot/+/45337 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/alderlake/Kconfig M src/soc/intel/alderlake/reset.c M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/reset.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/reset.c M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/elkhartlake/reset.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/reset.c M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/reset.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/reset.c M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/reset.c 16 files changed, 16 insertions(+), 132 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index 1a3f072..ade2fc1 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -16,6 +16,7 @@ select CPU_SUPPORTS_PM_TIMER_EMULATION select FSP_COMPRESS_FSP_S_LZ4 select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE @@ -49,6 +50,7 @@ select SOC_INTEL_COMMON_BLOCK_SA select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/alderlake/reset.c b/src/soc/intel/alderlake/reset.c index 1f7ea3c..bc5815a 100644 --- a/src/soc/intel/alderlake/reset.c +++ b/src/soc/intel/alderlake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> -#include <fsp/util.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 0c8eae2..3917fea 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -38,6 +38,7 @@ # Misc options select CACHE_MRC_SETTINGS select FSP_PLATFORM_MEMORY_SETTINGS_VERSIONS + select FSP_STATUS_GLOBAL_RESET_REQUIRED_5 select GENERIC_GPIO_LIB select INTEL_DESCRIPTOR_MODE_CAPABLE select HAVE_SMI_HANDLER @@ -91,6 +92,7 @@ select SOC_INTEL_COMMON_BLOCK_SPI select SOC_INTEL_COMMON_BLOCK_CSE select SOC_INTEL_COMMON_BLOCK_SMBUS + select SOC_INTEL_COMMON_FSP_RESET select SOUTHBRIDGE_INTEL_COMMON_SMBUS select UDELAY_TSC select TSC_MONOTONIC_TIMER diff --git a/src/soc/intel/apollolake/reset.c b/src/soc/intel/apollolake/reset.c index 8641b63..186a546 100644 --- a/src/soc/intel/apollolake/reset.c +++ b/src/soc/intel/apollolake/reset.c @@ -3,7 +3,6 @@ #include <cf9_reset.h> #include <console/console.h> #include <delay.h> -#include <fsp/util.h> #include <intelblocks/pmclib.h> #include <soc/heci.h> #include <soc/intel/common/reset.h> @@ -47,16 +46,3 @@ } printk(BIOS_SPEW, "CSE took %lu ms\n", stopwatch_duration_msecs(&sw)); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_5: /* Global Reset */ - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 5149274..2b862e7 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -82,6 +82,7 @@ select CPU_SUPPORTS_PM_TIMER_EMULATION select FSP_COMPRESS_FSP_S_LZMA select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select HAVE_FSP_LOGO_SUPPORT @@ -120,6 +121,7 @@ select SOC_INTEL_COMMON_NHLT select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT + select SOC_INTEL_COMMON_FSP_RESET select SSE2 select SUPPORT_CPU_UCODE_IN_CBFS select TSC_MONOTONIC_TIMER diff --git a/src/soc/intel/cannonlake/reset.c b/src/soc/intel/cannonlake/reset.c index 1f7ea3c..bc5815a 100644 --- a/src/soc/intel/cannonlake/reset.c +++ b/src/soc/intel/cannonlake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> -#include <fsp/util.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/elkhartlake/Kconfig b/src/soc/intel/elkhartlake/Kconfig index 05077ad..f833178 100644 --- a/src/soc/intel/elkhartlake/Kconfig +++ b/src/soc/intel/elkhartlake/Kconfig @@ -16,6 +16,7 @@ select CPU_SUPPORTS_PM_TIMER_EMULATION select FSP_COMPRESS_FSP_S_LZ4 select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE @@ -50,6 +51,7 @@ select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/elkhartlake/reset.c b/src/soc/intel/elkhartlake/reset.c index fe3d769..bc5815a 100644 --- a/src/soc/intel/elkhartlake/reset.c +++ b/src/soc/intel/elkhartlake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> -#include <fsp/util.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 464a11b..23b9ba2 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -16,6 +16,7 @@ select CPU_INTEL_FIRMWARE_INTERFACE_TABLE select CPU_SUPPORTS_PM_TIMER_EMULATION select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select HAVE_INTEL_FSP_REPO @@ -51,6 +52,7 @@ select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP select SOC_INTEL_COMMON_BLOCK_THERMAL + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/icelake/reset.c b/src/soc/intel/icelake/reset.c index 1f7ea3c..bc5815a 100644 --- a/src/soc/intel/icelake/reset.c +++ b/src/soc/intel/icelake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> -#include <fsp/util.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/jasperlake/Kconfig b/src/soc/intel/jasperlake/Kconfig index 4ea0837..00153fa 100644 --- a/src/soc/intel/jasperlake/Kconfig +++ b/src/soc/intel/jasperlake/Kconfig @@ -17,6 +17,7 @@ select COS_MAPPED_TO_MSB select FSP_COMPRESS_FSP_S_LZ4 select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE @@ -50,6 +51,7 @@ select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/jasperlake/reset.c b/src/soc/intel/jasperlake/reset.c index 1f7ea3c..bc5815a 100644 --- a/src/soc/intel/jasperlake/reset.c +++ b/src/soc/intel/jasperlake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> -#include <fsp/util.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 2478113..20b302d 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -26,6 +26,7 @@ select CPU_INTEL_FIRMWARE_INTERFACE_TABLE select CPU_SUPPORTS_PM_TIMER_EMULATION select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select HAVE_FSP_LOGO_SUPPORT @@ -63,6 +64,7 @@ select SOC_INTEL_COMMON_BLOCK_THERMAL select SOC_INTEL_COMMON_BLOCK_UART select SOC_INTEL_COMMON_BLOCK_XHCI_ELOG + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_NHLT select SOC_INTEL_COMMON_RESET diff --git a/src/soc/intel/skylake/reset.c b/src/soc/intel/skylake/reset.c index 1076ad2..8bf9db5 100644 --- a/src/soc/intel/skylake/reset.c +++ b/src/soc/intel/skylake/reset.c @@ -1,8 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> -#include <fsp/util.h> #include <intelblocks/pmclib.h> #include <soc/intel/common/reset.h> #include <soc/me.h> @@ -30,17 +28,3 @@ do_force_global_reset(); } } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -} diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 7959526..492d6fd4 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -17,6 +17,7 @@ select DRIVERS_USB_ACPI select FSP_COMPRESS_FSP_S_LZ4 select FSP_M_XIP + select FSP_STATUS_GLOBAL_RESET_REQUIRED_3 select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE @@ -53,6 +54,7 @@ select SOC_INTEL_COMMON_BLOCK_USB4 select SOC_INTEL_COMMON_BLOCK_USB4_PCIE select SOC_INTEL_COMMON_BLOCK_USB4_XHCI + select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET select SOC_INTEL_COMMON_BLOCK_CAR diff --git a/src/soc/intel/tigerlake/reset.c b/src/soc/intel/tigerlake/reset.c index 1f7ea3c..bc5815a 100644 --- a/src/soc/intel/tigerlake/reset.c +++ b/src/soc/intel/tigerlake/reset.c @@ -1,12 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <cf9_reset.h> -#include <console/console.h> #include <intelblocks/cse.h> #include <intelblocks/pmclib.h> -#include <fsp/util.h> #include <soc/intel/common/reset.h> -#include <soc/pci_devs.h>
void do_global_reset(void) { @@ -18,17 +15,3 @@ pmc_global_reset_enable(1); do_full_reset(); } - -void chipset_handle_reset(uint32_t status) -{ - switch (status) { - case FSP_STATUS_RESET_REQUIRED_3: /* Global Reset */ - printk(BIOS_DEBUG, "GLOBAL RESET!!\n"); - global_reset(); - break; - default: - printk(BIOS_ERR, "unhandled reset type %x\n", status); - die("unknown reset type"); - break; - } -}
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
HAOUAS Elyes (Code Review)
-
Mario Scheithauer (Code Review)
-
Matt DeVillier (Code Review)
-
Michael Niewöhner (Code Review)
-
Patrick Georgi (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Werner Zeh (Code Review)