Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
samsung/exynos5420: add resources during read_resources()
The chipset code was incorrectly adding memory resources to the domain device after resource allocation occurred. It's not possible to get the correct view of the address space, and it's generally incorrect to not add resources during read_resources(). This change fixes the order by adding resources in read_resources().
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: I49ce6ac88c4cb7cd05ff9d78133593ce97304596 --- M src/soc/samsung/exynos5420/cpu.c 1 file changed, 10 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/41374/1
diff --git a/src/soc/samsung/exynos5420/cpu.c b/src/soc/samsung/exynos5420/cpu.c index 8a07552..6d1418a 100644 --- a/src/soc/samsung/exynos5420/cpu.c +++ b/src/soc/samsung/exynos5420/cpu.c @@ -97,8 +97,6 @@
dcache_clean_invalidate_by_mva((void *)lower, upper - lower); mmu_config_range(lower / MiB, (upper - lower) / MiB, DCACHE_OFF); - - mmio_resource(dev, 1, lcdbase/KiB, DIV_ROUND_UP(fb_size, KiB)); }
static void tps65090_thru_ec_fet_disable(int index) @@ -117,9 +115,6 @@ unsigned long fb_size = FB_SIZE_KB * KiB; u32 lcdbase = get_fb_base_kb() * KiB;
- ram_resource(dev, 0, RAM_BASE_KB, RAM_SIZE_KB - FB_SIZE_KB); - mmio_resource(dev, 1, lcdbase / KiB, DIV_ROUND_UP(fb_size, KiB)); - /* * Disable LCD FETs before we do anything with the display. * FIXME(dhendrix): This is a gross hack and should be done @@ -133,6 +128,15 @@ set_cpu_id(); }
+static void cpu_read_resources(struct device *dev) +{ + unsigned long fb_size = FB_SIZE_KB * KiB; + u32 lcdbase = get_fb_base_kb() * KiB; + + ram_resource(dev, 0, RAM_BASE_KB, RAM_SIZE_KB - FB_SIZE_KB); + mmio_resource(dev, 1, lcdbase / KiB, DIV_ROUND_UP(fb_size, KiB)); +} + static void cpu_init(struct device *dev) { printk(BIOS_INFO, "CPU: S5P%X @ %ldMHz\n", @@ -140,7 +144,7 @@ }
static struct device_operations cpu_ops = { - .read_resources = noop_read_resources, + .read_resources = cpu_read_resources, .set_resources = noop_set_resources, .enable_resources = cpu_enable, .init = cpu_init,
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
Patch Set 3: Code-Review+1
Did anyone boot-test this?
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
samsung/exynos5420: add resources during read_resources()
The chipset code was incorrectly adding memory resources to the domain device after resource allocation occurred. It's not possible to get the correct view of the address space, and it's generally incorrect to not add resources during read_resources(). This change fixes the order by adding resources in read_resources().
Signed-off-by: Furquan Shaikh furquan@google.com Change-Id: I49ce6ac88c4cb7cd05ff9d78133593ce97304596 Reviewed-on: https://review.coreboot.org/c/coreboot/+/41374 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/samsung/exynos5420/cpu.c 1 file changed, 10 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/soc/samsung/exynos5420/cpu.c b/src/soc/samsung/exynos5420/cpu.c index 8a07552..6d1418a 100644 --- a/src/soc/samsung/exynos5420/cpu.c +++ b/src/soc/samsung/exynos5420/cpu.c @@ -97,8 +97,6 @@
dcache_clean_invalidate_by_mva((void *)lower, upper - lower); mmu_config_range(lower / MiB, (upper - lower) / MiB, DCACHE_OFF); - - mmio_resource(dev, 1, lcdbase/KiB, DIV_ROUND_UP(fb_size, KiB)); }
static void tps65090_thru_ec_fet_disable(int index) @@ -117,9 +115,6 @@ unsigned long fb_size = FB_SIZE_KB * KiB; u32 lcdbase = get_fb_base_kb() * KiB;
- ram_resource(dev, 0, RAM_BASE_KB, RAM_SIZE_KB - FB_SIZE_KB); - mmio_resource(dev, 1, lcdbase / KiB, DIV_ROUND_UP(fb_size, KiB)); - /* * Disable LCD FETs before we do anything with the display. * FIXME(dhendrix): This is a gross hack and should be done @@ -133,6 +128,15 @@ set_cpu_id(); }
+static void cpu_read_resources(struct device *dev) +{ + unsigned long fb_size = FB_SIZE_KB * KiB; + u32 lcdbase = get_fb_base_kb() * KiB; + + ram_resource(dev, 0, RAM_BASE_KB, RAM_SIZE_KB - FB_SIZE_KB); + mmio_resource(dev, 1, lcdbase / KiB, DIV_ROUND_UP(fb_size, KiB)); +} + static void cpu_init(struct device *dev) { printk(BIOS_INFO, "CPU: S5P%X @ %ldMHz\n", @@ -140,7 +144,7 @@ }
static struct device_operations cpu_ops = { - .read_resources = noop_read_resources, + .read_resources = cpu_read_resources, .set_resources = noop_set_resources, .enable_resources = cpu_enable, .init = cpu_init,
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 2/2/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : FAIL : https://lava.9esec.io/r/3423 "QEMU x86 q35/ich9" using payload SeaBIOS : FAIL : https://lava.9esec.io/r/3422 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3421 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3420
Please note: This test is under development and might not be accurate at all!
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 2/2/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : FAIL : https://lava.9esec.io/r/3435 "QEMU x86 q35/ich9" using payload SeaBIOS : FAIL : https://lava.9esec.io/r/3434 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3433 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3432
Please note: This test is under development and might not be accurate at all!
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41374 )
Change subject: samsung/exynos5420: add resources during read_resources() ......................................................................
Patch Set 4:
Automatic boot test returned (PASS/FAIL/TOTAL): 2/2/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : FAIL : https://lava.9esec.io/r/3439 "QEMU x86 q35/ich9" using payload SeaBIOS : FAIL : https://lava.9esec.io/r/3438 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3437 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3436
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Angel Pons (Code Review)
-
Furquan Shaikh (Code Review)