Hello Ting Shen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31914
to review the following change.
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
rockchip/rk3399: Remove obsolete BL31 resource reservation
RK3399 SoC code still manually excludes the BL31 region from the memory map, even though that is now automatically done with the BL31() memlayout region. CB:31123 and CB:31538 just forgot to remove this line. The resulting memory map stays the same.
Change-Id: I87458fa09f437b038af10e0fd9d76ef6d9394bc5 Signed-off-by: Julius Werner jwerner@chromium.org --- M src/soc/rockchip/rk3399/soc.c 1 file changed, 0 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/31914/1
diff --git a/src/soc/rockchip/rk3399/soc.c b/src/soc/rockchip/rk3399/soc.c index 65b791d..ee8ac8a 100644 --- a/src/soc/rockchip/rk3399/soc.c +++ b/src/soc/rockchip/rk3399/soc.c @@ -26,7 +26,6 @@ #include <stdlib.h> #include <string.h> #include <symbols.h> -#include <arm-trusted-firmware/plat/rockchip/rk3399/include/shared/bl31_param.h>
void bootmem_platform_add_ranges(void) { @@ -43,12 +42,6 @@
static void soc_init(struct device *dev) { - /* - * Reserve the whole TZRAM area because it will be marked as secure-only - * by BL31 and can not be accessed by the non-secure kernel. - */ - mmio_resource(dev, 1, (TZRAM_BASE / KiB), (TZRAM_SIZE / KiB)); - if (CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT) && display_init_required()) rk_display_init(dev); else
Ting Shen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Patch Set 1: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has removed a vote on this change.
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Removed Code-Review+2 by Julius Werner jwerner@chromium.org
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Patch Set 1:
No self-CR+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
rockchip/rk3399: Remove obsolete BL31 resource reservation
RK3399 SoC code still manually excludes the BL31 region from the memory map, even though that is now automatically done with the BL31() memlayout region. CB:31123 and CB:31538 just forgot to remove this line. The resulting memory map stays the same.
Change-Id: I87458fa09f437b038af10e0fd9d76ef6d9394bc5 Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/31914 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Ting Shen phoenixshen@google.com Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/soc/rockchip/rk3399/soc.c 1 file changed, 0 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Ting Shen: Looks good to me, but someone else must approve
diff --git a/src/soc/rockchip/rk3399/soc.c b/src/soc/rockchip/rk3399/soc.c index 65b791d..ee8ac8a 100644 --- a/src/soc/rockchip/rk3399/soc.c +++ b/src/soc/rockchip/rk3399/soc.c @@ -26,7 +26,6 @@ #include <stdlib.h> #include <string.h> #include <symbols.h> -#include <arm-trusted-firmware/plat/rockchip/rk3399/include/shared/bl31_param.h>
void bootmem_platform_add_ranges(void) { @@ -43,12 +42,6 @@
static void soc_init(struct device *dev) { - /* - * Reserve the whole TZRAM area because it will be marked as secure-only - * by BL31 and can not be accessed by the non-secure kernel. - */ - mmio_resource(dev, 1, (TZRAM_BASE / KiB), (TZRAM_SIZE / KiB)); - if (CONFIG(MAINBOARD_DO_NATIVE_VGA_INIT) && display_init_required()) rk_display_init(dev); else
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31914 )
Change subject: rockchip/rk3399: Remove obsolete BL31 resource reservation ......................................................................
Patch Set 2:
No self-CR+2
I have done this before on a few patches specific to SoCs or mainboards that I am maintainer for, if none of the other developers familiar with the code have +2 rights and I have a +1 from them. If you object to that I'm happy to send all those kinds of reviews to you in the future. I was just trying to avoid bothering people with trivialities in code they're not interested in.