Change in coreboot[master]: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer
Hello CK HU,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/46401
to review the following change.
Change subject: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer ......................................................................
soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer
Reserved region is 0x00115000 ~ 0x0011ffff and reduce PRERAM_CBMEM_CONSOLE buffer from 63K to 19K.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Ic82a194736eecd7bdc8df80b493290090a2ccba5 --- M src/soc/mediatek/mt8192/include/soc/memlayout.ld 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46401/1
diff --git a/src/soc/mediatek/mt8192/include/soc/memlayout.ld b/src/soc/mediatek/mt8192/include/soc/memlayout.ld index 3bef5b1..20aa431 100644 --- a/src/soc/mediatek/mt8192/include/soc/memlayout.ld +++ b/src/soc/mediatek/mt8192/include/soc/memlayout.ld @@ -21,12 +21,12 @@ TPM_TCPA_LOG(0x00103000, 2K) FMAP_CACHE(0x00103800, 2K) WATCHDOG_TOMBSTONE(0x00104000, 4) - PRERAM_CBMEM_CONSOLE(0x00104004, 63K - 4) - TIMESTAMP(0x00113c00, 1K) - STACK(0x00114000, 16K) - TTB(0x00118000, 28K) - DMA_COHERENT(0x0011f000, 4K) - SRAM_END(0x00120000) + PRERAM_CBMEM_CONSOLE(0x00104004, 19K - 4) + TIMESTAMP(0x00108c00, 1K) + STACK(0x00109000, 16K) + TTB(0x0010d000, 28K) + DMA_COHERENT(0x00114000, 4K) + SRAM_END(0x00115000)
SRAM_L2C_START(0x00200000) BOOTBLOCK(0x00201000, 64K)
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer ......................................................................
Patch Set 1:
Please stop CCing me on all these patches by default... Hung-Te is the official coreboot maintainer for all MediaTek SoCs now.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer ......................................................................
Patch Set 1:
Please stop CCing me on all these patches by default... Hung-Te is the official coreboot maintainer for all MediaTek SoCs now.
Whoops, actually I think I just got this one because it touches a memlayout.ld. And all the others were still from August before I updated the MAINTAINERS file. So not your fault, sorry for the noise.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46401/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/46401/1/src/soc/mediatek/mt8192/inc... PS1, Line 29: SRAM_END(0x00115000) The SRAM_END should be indicating real end of SRAM. You should preserve a new section instead of leaving that undefined.
Also, there's no need to move TIMESTAMP, STACK, ... etc. We can just define a new region before or after PRERAM_CBMEM_CONSOLE.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserved 44K SRAM for mcupm working buffer ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46401/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46401/4//COMMIT_MSG@7 PS4, Line 7: Reserved Imperative mood: Reserve.
Hello Hung-Te Lin, build bot (Jenkins), Julius Werner, CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46401
to look at the new patch set (#15).
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer
Reduce PRERAM_CBMEM_CONSOLE buffer from 63K to 19K and reserve 0x00115000 ~ 0x0011ffff for MPUPM.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Ic82a194736eecd7bdc8df80b493290090a2ccba5 --- M src/soc/mediatek/mt8192/include/soc/memlayout.ld 1 file changed, 6 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46401/15
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
Patch Set 15:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46401/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46401/4//COMMIT_MSG@7 PS4, Line 7: Reserved
Imperative mood: Reserve.
Ack
https://review.coreboot.org/c/coreboot/+/46401/1/src/soc/mediatek/mt8192/inc... File src/soc/mediatek/mt8192/include/soc/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/46401/1/src/soc/mediatek/mt8192/inc... PS1, Line 29: SRAM_END(0x00115000)
The SRAM_END should be indicating real end of SRAM. […]
Ack. The address used by MPUPM is fixed (0x00115000 ~ 0x0011ffff). Add a reserved region at the end of SRAM.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
Patch Set 17: Code-Review+1
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
Patch Set 17:
Hi Yidi, can you clarify:
1. Does MPUPM use SRAM even after kernel is up, or we're simply loading it for temporary, like SPM? 2. If a permanent RAM is needed, can we use DRAM (we can still preserve that as CBMEM)?
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
Patch Set 17:
Hi Hung-Te
- Does MPUPM use SRAM even after kernel is up, or we're simply loading it for temporary, like SPM?
MPUPM image will be loaded and run in its own SRAM 0x0C540000. (CB:46392) MPUPM exchanges data with kernel driver using SRAM 0x00115000 ~ 0x0011ffff. The address is hardcoded in MPUPM image and is unlikely to change.
- If a permanent RAM is needed, can we use DRAM (we can still preserve that as CBMEM)?
MPUPM does not use DRAM.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MPUPM working buffer ......................................................................
Patch Set 17:
Patch Set 17: MPUPM exchanges data with kernel driver using SRAM 0x00115000 ~ 0x0011ffff. The address is hardcoded in MPUPM image and is unlikely to change.
Ok then please provide this information in memlayout as comment so we know it can't be moved.
Hello Hung-Te Lin, build bot (Jenkins), Julius Werner, CK HU, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46401
to look at the new patch set (#19).
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer
Reduce PRERAM_CBMEM_CONSOLE buffer from 63K to 19K and reserve 0x00115000 ~ 0x0011ffff for MCUPM.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Ic82a194736eecd7bdc8df80b493290090a2ccba5 --- M src/soc/mediatek/mt8192/include/soc/memlayout.ld 1 file changed, 10 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46401/19
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46401/19/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/46401/19/src/soc/mediatek/mt8192/in... PS19, Line 31: e Missing period
Hello Hung-Te Lin, build bot (Jenkins), Julius Werner, CK HU, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46401
to look at the new patch set (#20).
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer
Reduce PRERAM_CBMEM_CONSOLE buffer from 63K to 19K and reserve 0x00115000 ~ 0x0011ffff for MCUPM.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Ic82a194736eecd7bdc8df80b493290090a2ccba5 --- M src/soc/mediatek/mt8192/include/soc/memlayout.ld 1 file changed, 10 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/46401/20
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46401/19/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/46401/19/src/soc/mediatek/mt8192/in... PS19, Line 31: e
Missing period
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
Patch Set 20: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46401 )
Change subject: soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer ......................................................................
soc/mediatek/mt8192: Reserve 44K SRAM for MCUPM working buffer
Reduce PRERAM_CBMEM_CONSOLE buffer from 63K to 19K and reserve 0x00115000 ~ 0x0011ffff for MCUPM.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Ic82a194736eecd7bdc8df80b493290090a2ccba5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46401 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Hung-Te Lin hungte@chromium.org --- M src/soc/mediatek/mt8192/include/soc/memlayout.ld 1 file changed, 10 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/soc/mediatek/mt8192/include/soc/memlayout.ld b/src/soc/mediatek/mt8192/include/soc/memlayout.ld index df9d376..57b258c 100644 --- a/src/soc/mediatek/mt8192/include/soc/memlayout.ld +++ b/src/soc/mediatek/mt8192/include/soc/memlayout.ld @@ -21,11 +21,16 @@ TPM_TCPA_LOG(0x00103000, 2K) FMAP_CACHE(0x00103800, 2K) WATCHDOG_TOMBSTONE(0x00104000, 4) - PRERAM_CBMEM_CONSOLE(0x00104004, 63K - 4) - TIMESTAMP(0x00113c00, 1K) - STACK(0x00114000, 16K) - TTB(0x00118000, 28K) - DMA_COHERENT(0x0011f000, 4K) + PRERAM_CBMEM_CONSOLE(0x00104004, 19K - 4) + TIMESTAMP(0x00108c00, 1K) + STACK(0x00109000, 16K) + TTB(0x0010d000, 28K) + DMA_COHERENT(0x00114000, 4K) + /* + * MCUPM exchanges data with kernel driver using SRAM 0x00115000 ~ 0x0011ffff. + * The address is hardcoded in MCUPM image and is unlikely to change. + */ + REGION(mcufw_reserved, 0x00115000, 44K, 4K) SRAM_END(0x00120000)
SRAM_L2C_START(0x00200000)
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Yidi Lin (Code Review)
-
Yu-Ping Wu (Code Review)