From ecea08916418a94f99f89c543303877cb6e08a11 Mon Sep 17 00:00:00 2001 From: Alper Nebi Yasak Date: Wed, 8 Nov 2023 21:25:13 +0300 Subject: firmware: coreboot: framebuffer: Avoid invalid zero physical address On ARM64 systems coreboot defers framebuffer allocation to its payload, to be done by a libpayload function call. In this case, coreboot tables still include a framebuffer entry with display format details, but the physical address field is set to zero (as in [1], for example). Unfortunately, this field is not automatically updated when the framebuffer is initialized through libpayload, citing that doing so would invalidate checksums over the entire coreboot table [2]. This can be observed on ARM64 Chromebooks with stock firmware. On a Google Kevin (RK3399), trying to use coreboot framebuffer driver as built-in to the kernel results in a benign error. But on Google Hana (MT8173) and Google Cozmo (MT8183) it causes a hang. When the framebuffer physical address field in the coreboot table is zero, we have no idea where coreboot initialized a framebuffer, or even if it did. Instead of trying to set up a framebuffer located at zero, return ENODEV to indicate that there isn't one. [1] https://review.coreboot.org/c/coreboot/+/17109 [2] https://review.coreboot.org/c/coreboot/+/8797 Signed-off-by: Alper Nebi Yasak Reviewed-by: Julius Werner Link: https://lore.kernel.org/r/20231108182625.46563-1-alpernebiyasak@gmail.com Signed-off-by: Tzung-Bi Shih --- drivers/firmware/google/framebuffer-coreboot.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c index c323a818805c..5c84bbebfef8 100644 --- a/drivers/firmware/google/framebuffer-coreboot.c +++ b/drivers/firmware/google/framebuffer-coreboot.c @@ -36,6 +36,9 @@ static int framebuffer_probe(struct coreboot_device *dev) .format = NULL, }; + if (!fb->physical_address) + return -ENODEV; + for (i = 0; i < ARRAY_SIZE(formats); ++i) { if (fb->bits_per_pixel == formats[i].bits_per_pixel && fb->red_mask_pos == formats[i].red.offset && -- cgit v1.2.3-58-ga151 From 09aeaabebdafbcf4afd1c481beaff37ecbc6b023 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 27 Dec 2023 17:26:27 +0100 Subject: firmware: coreboot: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/d323e4f24bfab3ac1480933deb51e7c5cb025b09.1703693980.git.u.kleine-koenig@pengutronix.de Signed-off-by: Tzung-Bi Shih --- drivers/firmware/google/coreboot_table.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c index 33ae94745aef..2a4469bf1b81 100644 --- a/drivers/firmware/google/coreboot_table.c +++ b/drivers/firmware/google/coreboot_table.c @@ -176,10 +176,9 @@ static int __cb_dev_unregister(struct device *dev, void *dummy) return 0; } -static int coreboot_table_remove(struct platform_device *pdev) +static void coreboot_table_remove(struct platform_device *pdev) { bus_for_each_dev(&coreboot_bus_type, NULL, NULL, __cb_dev_unregister); - return 0; } #ifdef CONFIG_ACPI @@ -201,7 +200,7 @@ MODULE_DEVICE_TABLE(of, coreboot_of_match); static struct platform_driver coreboot_table_driver = { .probe = coreboot_table_probe, - .remove = coreboot_table_remove, + .remove_new = coreboot_table_remove, .driver = { .name = "coreboot_table", .acpi_match_table = ACPI_PTR(cros_coreboot_acpi_match), -- cgit v1.2.3-58-ga151