Change in coreboot[master]: soc/intel/tigerlake: Fix overlapping memory address used for early GS...
Bora Guvendik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars
BAR address used during early initilization of GPSI 2 is overlapping with UART bar.
//For GSPI2 this is the address calculated GSPI_BUS_BASE(0xFE030000,2)=0xFE032000 GSPI_BUS_BASE(bar, bus) ((bar) + (bus) * 4 * KiB)
//overlaps with CONSOLE_UART_BASE_ADDRESS -> 0xfe032000
Change-Id: Id9f2140a6dd21c2cb8d75823cc83cced0c660179 Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/soc/intel/tigerlake/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/47643/1
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index 507f871..f29fb0e 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -166,7 +166,7 @@
config CONSOLE_UART_BASE_ADDRESS hex - default 0xfe032000 + default 0xfe03e000 depends on INTEL_LPSS_UART_FOR_CONSOLE
# Clock divider parameters for 115200 baud rate
Anil Kumar K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... PS1, Line 169: 0xfe03e000 I don't understand why we have the BAR defined in two different places: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/tigerlake/i...
Probably because it gets used by the payload as well. We might want to evaluate if there is a way to pass the address to the payload instead of using coreboot configs. Anyways, that would be for a separate change.
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1:
Looks like alderlake need the same fix.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... PS1, Line 169: 0xfe03e000
I don't understand why we have the BAR defined in two different places: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/tigerlake/i...
Probably because it gets used by the payload as well.
Yeah looks like SeaBIOS wants it in a build config file.
We might want to evaluate if there is a way to pass the address to the payload instead of using coreboot configs.
:) I think this is related to whole passing more info to payload we've been chatting about
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... PS1, Line 169: 0xfe03e000
I don't understand why we have the BAR defined in two different places: https://review.coreboot. […]
Agree that we need to use single definition. How about using config value in iomap.h rather than defining again with constant value? I mean change like below. #define UART_BASE_0_ADDRESS CONFIG_CONSOLE_UART_BASE_ADDRESS Then we can just correct value for Kconfig value.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/47643/1/src/soc/intel/tigerlake/Kco... PS1, Line 169: 0xfe03e000
Agree that we need to use single definition. […]
That is a good idea. You can also drop UART_BASE_0_ADDRESS and use CONFIG_CONSOLE_UART_BASE_ADDRESS directly in UART_BASE_0_ADDR.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47643 )
Change subject: soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars ......................................................................
soc/intel/tigerlake: Fix overlapping memory address used for early GSPI2 and UART bars
BAR address used during early initilization of GPSI 2 is overlapping with UART bar.
//For GSPI2 this is the address calculated GSPI_BUS_BASE(0xFE030000,2)=0xFE032000 GSPI_BUS_BASE(bar, bus) ((bar) + (bus) * 4 * KiB)
//overlaps with CONSOLE_UART_BASE_ADDRESS -> 0xfe032000
Change-Id: Id9f2140a6dd21c2cb8d75823cc83cced0c660179 Signed-off-by: Bora Guvendik bora.guvendik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47643 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Anil Kumar K anil.kumar.k@intel.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/tigerlake/Kconfig 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved Anil Kumar K: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index e962e4c..b97b92e 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -168,7 +168,7 @@
config CONSOLE_UART_BASE_ADDRESS hex - default 0xfe032000 + default 0xfe03e000 depends on INTEL_LPSS_UART_FOR_CONSOLE
# Clock divider parameters for 115200 baud rate
-
Anil Kumar K (Code Review) -
Bora Guvendik (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)
-
Wonkyu Kim (Code Review)