Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
soc/amd/picasso: Enable eSPI capability for Picasso
This change selects SOC_AMD_COMMON_BLOCK_HAS_ESPI which enables the capability for using eSPI on Picasso.
BUG=b:153675913,b:154445472
Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/41075/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 3d69966..a304819 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -29,6 +29,7 @@ select UDELAY_TSC select SOC_AMD_COMMON select SOC_AMD_COMMON_BLOCK + select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_ACPIMMIO select SOC_AMD_COMMON_BLOCK_BANKED_GPIOS
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41075
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
soc/amd/picasso: Enable eSPI capability for Picasso
This change selects SOC_AMD_COMMON_BLOCK_HAS_ESPI which enables the capability for using eSPI on Picasso.
Additionally, it also calls espi_setup() and espi_configure_decodes() if mainboard enables use of eSPI and skips LPC decodes in that case.
BUG=b:153675913,b:154445472
Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/southbridge.c 2 files changed, 17 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/41075/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) { please, no space before tabs
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) { please, no spaces at the start of a line
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) {
please, no space before tabs
Done
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) {
please, no spaces at the start of a line
Done
https://review.coreboot.org/c/coreboot/+/41075/5/src/soc/amd/picasso/southbr... PS5, Line 292: if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) {
code indent should use tabs where possible
Done
Hello build bot (Jenkins), Raul Rangel, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41075
to look at the new patch set (#6).
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
soc/amd/picasso: Enable eSPI capability for Picasso
This change selects SOC_AMD_COMMON_BLOCK_HAS_ESPI which enables the capability for using eSPI on Picasso.
Additionally, it also calls espi_setup() and espi_configure_decodes() if mainboard enables use of eSPI and skips LPC decodes in that case.
BUG=b:153675913,b:154445472
Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/southbridge.c 2 files changed, 17 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/41075/6
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 6: Code-Review+2
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
soc/amd/picasso: Enable eSPI capability for Picasso
This change selects SOC_AMD_COMMON_BLOCK_HAS_ESPI which enables the capability for using eSPI on Picasso.
Additionally, it also calls espi_setup() and espi_configure_decodes() if mainboard enables use of eSPI and skips LPC decodes in that case.
BUG=b:153675913,b:154445472
Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41075 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/southbridge.c 2 files changed, 17 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index ba44146..b065e2f 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -28,6 +28,7 @@ select UDELAY_TSC select SOC_AMD_COMMON select SOC_AMD_COMMON_BLOCK + select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_ACPIMMIO select SOC_AMD_COMMON_BLOCK_BANKED_GPIOS diff --git a/src/soc/amd/picasso/southbridge.c b/src/soc/amd/picasso/southbridge.c index 2df193a..2261a30 100644 --- a/src/soc/amd/picasso/southbridge.c +++ b/src/soc/amd/picasso/southbridge.c @@ -12,6 +12,7 @@ #include <amdblocks/amd_pci_util.h> #include <amdblocks/reset.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/espi.h> #include <amdblocks/lpc.h> #include <amdblocks/acpi.h> #include <amdblocks/spi.h> @@ -202,13 +203,20 @@ asf_write8(SMBSLVSTAT, SMBSLV_STAT_CLEAR); }
+static void lpc_configure_decodes(void) +{ + if (CONFIG(POST_IO) && (CONFIG_POST_IO_PORT == 0x80)) + lpc_enable_port80(); +} + /* Before console init */ void fch_pre_init(void) { lpc_early_init(); - if (CONFIG(POST_IO) && (CONFIG_POST_IO_PORT == 0x80) - && CONFIG(PICASSO_LPC_IOMUX)) - lpc_enable_port80(); + + if (!CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) + lpc_configure_decodes(); + fch_spi_early_init(); enable_acpimmio_decode_pm04(); fch_smbus_init(); @@ -280,6 +288,11 @@
if (CONFIG(DISABLE_SPI_FLASH_ROM_SHARING)) lpc_disable_spi_rom_sharing(); + + if (CONFIG(SOC_AMD_COMMON_BLOCK_USE_ESPI)) { + espi_setup(); + espi_configure_decodes(); + } }
void sb_enable(struct device *dev)
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX removing this broke the LPC serial console on Mandolin
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX
removing this broke the LPC serial console on Mandolin
ok, it's probably not exactly this place, but I'm still rather sure that the breakage occurred somewhere near this patch. after this patch PICASSO_LPC_IOMUX is unused in the chipset code and the LPC UART stopped working
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX
ok, it's probably not exactly this place, but I'm still rather sure that the breakage occurred somew […]
I see Mandolin selecting SOC_AMD_COMMON_BLOCK_USE_ESPI. So, I don't think lpc_configure_decodes() would ever get called with that.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX
I see Mandolin selecting SOC_AMD_COMMON_BLOCK_USE_ESPI. […]
https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src... calls lpc_enable_sio_decode(). On Mnadolin the EC is on eSPI, but the optional debug UART card is on LPC which is why I was worried about breakage due to the eSPI patch train. Not sure if missing LPC pin mux setup is the issue here or if the decode ranges just end up on the wrong bus. The name of PICASSO_LPC_IOMUX suggests that it should do something with the LPC/eMMC muxing; haven't further looked into the issue though.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX
A Mandolin build without selected SOC_AMD_COMMON_BLOCK_USE_ESPI doesn't result in the serial port on LPC working. I do however get more post codes on the LPC-attached port 0x80 LED display thing (stopped at 0x00a2 before, which seems to be the last post code before the call to fch_pre_init), so there's probably something else wrong with the LPC serial console
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41075 )
Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... File src/soc/amd/picasso/southbridge.c:
https://review.coreboot.org/c/coreboot/+/41075/11/src/soc/amd/picasso/southb... PS11, Line 210: PICASSO_LPC_IOMUX
I do however get more post codes on the LPC-attached port 0x80 LED display thing
I think that is expected because lpc_enable_port80() gets called when SOC_AMD_COMMON_BLOCK_USE_ESPI is not selected.
Not sure if missing LPC pin mux setup is the issue here or if the decode ranges just end up on the wrong bus. The name of PICASSO_LPC_IOMUX suggests that it should do something with the LPC/eMMC muxing; haven't further looked into the issue though.
There was no common code in soc/amd that was using PICASSO_LPC_IOMUX to control eMMC initialization. I think all of it lives under mainboard/amd/mandolin.