From e96777b71e1e810e51f383da84f22c72096344ea Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:40 -0700 Subject: [PATCH 01/21] smem: Don't use -EPROBE_DEFER This has no useful meaning in U-Boot. Use -ENOMEM since that appears to be what has gone wrong in this case. We want to reserve this flag for internal driver model use. Signed-off-by: Simon Glass --- drivers/smem/msm_smem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/smem/msm_smem.c b/drivers/smem/msm_smem.c index 597d425d11..26462151b3 100644 --- a/drivers/smem/msm_smem.c +++ b/drivers/smem/msm_smem.c @@ -437,7 +437,7 @@ static int qcom_smem_alloc(unsigned int host, unsigned int item, size_t size) int ret; if (!__smem) - return -EPROBE_DEFER; + return -ENOMEM; if (item < SMEM_ITEM_LAST_FIXED) { dev_err(__smem->dev, @@ -559,7 +559,7 @@ static void *qcom_smem_get(unsigned int host, unsigned int item, size_t *size) { struct smem_partition_header *phdr; size_t cacheln; - void *ptr = ERR_PTR(-EPROBE_DEFER); + void *ptr = ERR_PTR(-ENOMEM); if (!__smem) return ptr; @@ -597,7 +597,7 @@ static int qcom_smem_get_free_space(unsigned int host) unsigned int ret; if (!__smem) - return -EPROBE_DEFER; + return -ENOMEM; if (host < SMEM_HOST_COUNT && __smem->partitions[host]) { phdr = __smem->partitions[host]; From f724f0af2b5f9d2d17caa3b76cf58574788d8515 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:41 -0700 Subject: [PATCH 02/21] nand: brcmnand: Don't use -EPROBE_DEFER This has no useful meaning in U-Boot and will never be returned. We want to reserve this flag for internal driver model use. Drop the code. Signed-off-by: Simon Glass --- drivers/mtd/nand/raw/brcmnand/brcmnand.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c index 7349a9bc99..99a1c2e6e2 100644 --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c @@ -2526,10 +2526,7 @@ int brcmnand_probe(struct udevice *dev, struct brcmnand_soc *soc) if (ret) return ret; } else { - ret = PTR_ERR(ctrl->clk); - if (ret == -EPROBE_DEFER) - return ret; - + /* Ignore PTR_ERR(ctrl->clk) */ ctrl->clk = NULL; } From a547fcb82880f89d9fdac218308b66e28d3e1481 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:42 -0700 Subject: [PATCH 03/21] dm: Rename DM_FLAG_REMOVE_WITH_PD_ON This flag has the word 'REMOVE' in it which means it conflicts with the DM_REMOVE flags. Rename it to DM_FLAG_LEAVE_PD_ON which seems to indicate its purpose well enough. Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 2 +- drivers/video/meson/meson_vpu.c | 2 +- drivers/watchdog/rti_wdt.c | 2 +- include/dm/device.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 7e8f3afb2d..7874d53c84 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -200,7 +200,7 @@ int device_remove(struct udevice *dev, uint flags) if (!(flags & DM_REMOVE_NO_PD) && !(drv->flags & - (DM_FLAG_DEFAULT_PD_CTRL_OFF | DM_FLAG_REMOVE_WITH_PD_ON)) && + (DM_FLAG_DEFAULT_PD_CTRL_OFF | DM_FLAG_LEAVE_PD_ON)) && dev != gd->cur_serial_dev) dev_power_domain_off(dev); diff --git a/drivers/video/meson/meson_vpu.c b/drivers/video/meson/meson_vpu.c index ca6933a6c5..558f9ba766 100644 --- a/drivers/video/meson/meson_vpu.c +++ b/drivers/video/meson/meson_vpu.c @@ -212,5 +212,5 @@ U_BOOT_DRIVER(meson_vpu) = { .probe = meson_vpu_probe, .bind = meson_vpu_bind, .priv_auto = sizeof(struct meson_vpu_priv), - .flags = DM_FLAG_PRE_RELOC | DM_FLAG_REMOVE_WITH_PD_ON, + .flags = DM_FLAG_PRE_RELOC | DM_FLAG_LEAVE_PD_ON, }; diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index f64a39f657..8335b20ae8 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -119,5 +119,5 @@ U_BOOT_DRIVER(rti_wdt) = { .ops = &rti_wdt_ops, .probe = rti_wdt_probe, .priv_auto = sizeof(struct rti_wdt_priv), - .flags = DM_FLAG_REMOVE_WITH_PD_ON, + .flags = DM_FLAG_LEAVE_PD_ON, }; diff --git a/include/dm/device.h b/include/dm/device.h index e665558444..8f38cf1633 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -71,7 +71,7 @@ struct driver_info; * Device is removed without switching off its power domain. This might * be required, i. e. for serial console (debug) output when booting OS. */ -#define DM_FLAG_REMOVE_WITH_PD_ON (1 << 13) +#define DM_FLAG_LEAVE_PD_ON (1 << 13) /* * One or multiple of these flags are passed to device_remove() so that From 4d7bab1adb1a8e3d5b30c5d19f1873dacc95bb93 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:43 -0700 Subject: [PATCH 04/21] dm: pci: Correct use of wrong flag name Update a driver that uses the incorrect flag. Add a comment to hopefully prevent furture mistakes. Signed-off-by: Simon Glass --- drivers/pci/pcie_iproc.c | 2 +- include/dm/device.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pcie_iproc.c b/drivers/pci/pcie_iproc.c index 6725ff6437..12ce9d525c 100644 --- a/drivers/pci/pcie_iproc.c +++ b/drivers/pci/pcie_iproc.c @@ -1283,5 +1283,5 @@ U_BOOT_DRIVER(pci_iproc) = { .probe = iproc_pcie_probe, .remove = iproc_pcie_remove, .priv_auto = sizeof(struct iproc_pcie), - .flags = DM_REMOVE_OS_PREPARE, + .flags = DM_FLAG_OS_PREPARE, }; diff --git a/include/dm/device.h b/include/dm/device.h index 8f38cf1633..204441fd23 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -77,6 +77,9 @@ struct driver_info; * One or multiple of these flags are passed to device_remove() so that * a selective device removal as specified by the remove-stage and the * driver flags can be done. + * + * DO NOT use these flags in your driver's @flags value... + * use the above DM_FLAG_... values instead */ enum { /* Normal remove, remove all devices */ From b1f25fcfefc4c8e05f91d948f316c7bdbb4cd527 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:44 -0700 Subject: [PATCH 05/21] dm: core: Remove children before advising uclass At present the uclass pre-remove method is called before the children are removed. But the children may refused to be removed, in whch case the uclass is in a tricky situation. At present we handle this by calling the uclass' post_probe() method. But it seems better to avoid doing anything with the uclass in this case. Switch the ordering so that we make sure the children can be removed before advising the uclass. Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 7874d53c84..35b625e7b1 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -172,13 +172,13 @@ int device_remove(struct udevice *dev, uint flags) drv = dev->driver; assert(drv); - ret = uclass_pre_remove_device(dev); + ret = device_chld_remove(dev, NULL, flags); if (ret) return ret; - ret = device_chld_remove(dev, NULL, flags); + ret = uclass_pre_remove_device(dev); if (ret) - goto err; + return ret; /* * Remove the device if called with the "normal" remove flag set, @@ -216,12 +216,6 @@ err_remove: /* We can't put the children back */ dm_warn("%s: Device '%s' failed to remove, but children are gone\n", __func__, dev->name); -err: - ret = uclass_post_probe_device(dev); - if (ret) { - dm_warn("%s: Device '%s' failed to post_probe on error path\n", - __func__, dev->name); - } return ret; } From c51d2e704a1c89d504b379b133bd552c3387fa6c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:45 -0700 Subject: [PATCH 06/21] dm: core: Avoid partially removing devices At present if device_remove() decides that the device should not actually be removed, it still calls the uclass pre_remove() method and powers the device down. Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 61 ++++++++++++++++++++++++------------ include/dm/device-internal.h | 9 +++++- test/dm/virtio.c | 4 ++- 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 35b625e7b1..bc99ef0032 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -8,6 +8,8 @@ * Pavel Herrmann */ +#define LOG_CATEGORY LOGC_DM + #include #include #include @@ -54,7 +56,7 @@ int device_chld_remove(struct udevice *dev, struct driver *drv, continue; ret = device_remove(pos, flags); - if (ret) + if (ret && ret != -EKEYREJECTED) return ret; } @@ -149,13 +151,24 @@ void device_free(struct udevice *dev) devres_release_probe(dev); } -static bool flags_remove(uint flags, uint drv_flags) +/** + * flags_remove() - Figure out whether to remove a device + * + * @flags: Flags passed to device_remove() + * @drv_flags: Driver flags + * @return 0 if the device should be removed, + * -EKEYREJECTED if @flags includes a flag in DM_REMOVE_ACTIVE_ALL but + * @drv_flags does not (indicates that this device has nothing to do for + * DMA shutdown or OS prepare) + */ +static int flags_remove(uint flags, uint drv_flags) { - if ((flags & DM_REMOVE_NORMAL) || - (flags && (drv_flags & (DM_FLAG_ACTIVE_DMA | DM_FLAG_OS_PREPARE)))) - return true; + if (flags & DM_REMOVE_NORMAL) + return 0; + if (flags && (drv_flags & DM_REMOVE_ACTIVE_ALL)) + return 0; - return false; + return -EKEYREJECTED; } int device_remove(struct udevice *dev, uint flags) @@ -169,22 +182,32 @@ int device_remove(struct udevice *dev, uint flags) if (!(dev_get_flags(dev) & DM_FLAG_ACTIVATED)) return 0; - drv = dev->driver; - assert(drv); - + /* + * If the child returns EKEYREJECTED, continue. It just means that it + * didn't match the flags. + */ ret = device_chld_remove(dev, NULL, flags); - if (ret) - return ret; - - ret = uclass_pre_remove_device(dev); - if (ret) + if (ret && ret != -EKEYREJECTED) return ret; /* * Remove the device if called with the "normal" remove flag set, * or if the remove flag matches any of the drivers remove flags */ - if (drv->remove && flags_remove(flags, drv->flags)) { + drv = dev->driver; + assert(drv); + ret = flags_remove(flags, drv->flags); + if (ret) { + log_debug("%s: When removing: flags=%x, drv->flags=%x, err=%d\n", + dev->name, flags, drv->flags, ret); + return ret; + } + + ret = uclass_pre_remove_device(dev); + if (ret) + return ret; + + if (drv->remove) { ret = drv->remove(dev); if (ret) goto err_remove; @@ -204,13 +227,11 @@ int device_remove(struct udevice *dev, uint flags) dev != gd->cur_serial_dev) dev_power_domain_off(dev); - if (flags_remove(flags, drv->flags)) { - device_free(dev); + device_free(dev); - dev_bic_flags(dev, DM_FLAG_ACTIVATED); - } + dev_bic_flags(dev, DM_FLAG_ACTIVATED); - return ret; + return 0; err_remove: /* We can't put the children back */ diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index 639bbd293d..b513b6861a 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -123,7 +123,8 @@ int device_probe(struct udevice *dev); * * @dev: Pointer to device to remove * @flags: Flags for selective device removal (DM_REMOVE_...) - * @return 0 if OK, -ve on error (an error here is normally a very bad thing) + * @return 0 if OK, -EKEYREJECTED if not removed due to flags, other -ve on + * error (such an error here is normally a very bad thing) */ #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) int device_remove(struct udevice *dev, uint flags); @@ -173,6 +174,12 @@ static inline int device_chld_unbind(struct udevice *dev, struct driver *drv) /** * device_chld_remove() - Stop all device's children + * + * This continues through all children recursively stopping part-way through if + * an error occurs. Return values of -EKEYREJECTED are ignored and processing + * continues, since they just indicate that the child did not elect to be + * removed based on the value of @flags. + * * @dev: The device whose children are to be removed * @drv: The targeted driver * @flags: Flag, if this functions is called in the pre-OS stage diff --git a/test/dm/virtio.c b/test/dm/virtio.c index ad355981cf..9a7e658cce 100644 --- a/test/dm/virtio.c +++ b/test/dm/virtio.c @@ -123,7 +123,9 @@ static int dm_test_virtio_remove(struct unit_test_state *uts) /* check the device can be successfully removed */ dev_or_flags(dev, DM_FLAG_ACTIVATED); - ut_assertok(device_remove(bus, DM_REMOVE_ACTIVE_ALL)); + ut_asserteq(-EKEYREJECTED, device_remove(bus, DM_REMOVE_ACTIVE_ALL)); + + ut_asserteq(false, device_active(dev)); return 0; } From cc6f4c8f2574472e359cb915811f6f93efdfcad4 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Sun, 24 Jan 2021 14:32:46 -0700 Subject: [PATCH 07/21] dm: core: Add late driver remove option Add another flag to the DM core which could be assigned to drivers and which makes those drivers call their remove callbacks last, just before booting OS and after all the other drivers finished with their remove callbacks. This is necessary for things like clock drivers, where the other drivers might depend on the clock driver in their remove callbacks. Prime example is the mmc subsystem, which can reconfigure a card from HS mode to slower modes in the remove callback and for that it needs to reconfigure the controller clock. Signed-off-by: Marek Vasut Signed-off-by: Simon Glass --- drivers/core/device-remove.c | 39 ++++++++++++--- drivers/core/root.c | 2 + include/dm/device-internal.h | 10 ++-- include/dm/device.h | 10 +++- test/dm/core.c | 94 ++++++++++++++++++++++++++++++++++++ test/dm/test-driver.c | 22 +++++++++ 6 files changed, 165 insertions(+), 12 deletions(-) diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index bc99ef0032..616dcf0785 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -47,20 +47,24 @@ int device_chld_remove(struct udevice *dev, struct driver *drv, uint flags) { struct udevice *pos, *n; - int ret; + int result = 0; assert(dev); list_for_each_entry_safe(pos, n, &dev->child_head, sibling_node) { + int ret; + if (drv && (pos->driver != drv)) continue; ret = device_remove(pos, flags); - if (ret && ret != -EKEYREJECTED) + if (ret == -EPROBE_DEFER) + result = ret; + else if (ret && ret != -EKEYREJECTED) return ret; } - return 0; + return result; } int device_unbind(struct udevice *dev) @@ -154,21 +158,40 @@ void device_free(struct udevice *dev) /** * flags_remove() - Figure out whether to remove a device * + * If this is called with @flags == DM_REMOVE_NON_VITAL | DM_REMOVE_ACTIVE_DMA, + * then it returns 0 (=go head and remove) if the device is not matked vital + * but is marked DM_REMOVE_ACTIVE_DMA. + * + * If this is called with @flags == DM_REMOVE_ACTIVE_DMA, + * then it returns 0 (=go head and remove) if the device is marked + * DM_REMOVE_ACTIVE_DMA, regardless of whether it is marked vital. + * * @flags: Flags passed to device_remove() * @drv_flags: Driver flags * @return 0 if the device should be removed, * -EKEYREJECTED if @flags includes a flag in DM_REMOVE_ACTIVE_ALL but * @drv_flags does not (indicates that this device has nothing to do for * DMA shutdown or OS prepare) + * -EPROBE_DEFER if @flags is DM_REMOVE_NON_VITAL but @drv_flags contains + * DM_FLAG_VITAL (indicates the device is vital and should not be removed) */ static int flags_remove(uint flags, uint drv_flags) { - if (flags & DM_REMOVE_NORMAL) - return 0; - if (flags && (drv_flags & DM_REMOVE_ACTIVE_ALL)) - return 0; + if (!(flags & DM_REMOVE_NORMAL)) { + bool vital_match; + bool active_match; - return -EKEYREJECTED; + active_match = !(flags & DM_REMOVE_ACTIVE_ALL) || + (drv_flags & flags); + vital_match = !(flags & DM_REMOVE_NON_VITAL) || + !(drv_flags & DM_FLAG_VITAL); + if (!vital_match) + return -EPROBE_DEFER; + if (!active_match) + return -EKEYREJECTED; + } + + return 0; } int device_remove(struct udevice *dev, uint flags) diff --git a/drivers/core/root.c b/drivers/core/root.c index 2bfa75b472..7ef2ec2da2 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -162,6 +162,8 @@ int dm_init(bool of_live) int dm_uninit(void) { + /* Remove non-vital devices first */ + device_remove(dm_root(), DM_REMOVE_NON_VITAL); device_remove(dm_root(), DM_REMOVE_NORMAL); device_unbind(dm_root()); gd->dm_root = NULL; diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index b513b6861a..39406c3f35 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -123,7 +123,8 @@ int device_probe(struct udevice *dev); * * @dev: Pointer to device to remove * @flags: Flags for selective device removal (DM_REMOVE_...) - * @return 0 if OK, -EKEYREJECTED if not removed due to flags, other -ve on + * @return 0 if OK, -EKEYREJECTED if not removed due to flags, -EPROBE_DEFER if + * this is a vital device and flags is DM_REMOVE_NON_VITAL, other -ve on * error (such an error here is normally a very bad thing) */ #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) @@ -178,12 +179,15 @@ static inline int device_chld_unbind(struct udevice *dev, struct driver *drv) * This continues through all children recursively stopping part-way through if * an error occurs. Return values of -EKEYREJECTED are ignored and processing * continues, since they just indicate that the child did not elect to be - * removed based on the value of @flags. + * removed based on the value of @flags. Return values of -EPROBE_DEFER cause + * processing of other children to continue, but the function will return + * -EPROBE_DEFER. * * @dev: The device whose children are to be removed * @drv: The targeted driver * @flags: Flag, if this functions is called in the pre-OS stage - * @return 0 on success, -ve on error + * @return 0 on success, -EPROBE_DEFER if any child failed to remove, other + * -ve on error */ #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) int device_chld_remove(struct udevice *dev, struct driver *drv, diff --git a/include/dm/device.h b/include/dm/device.h index 204441fd23..28533ce0b6 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -73,6 +73,13 @@ struct driver_info; */ #define DM_FLAG_LEAVE_PD_ON (1 << 13) +/* + * Device is vital to the operation of other devices. It is possible to remove + * removed this device after all regular devices are removed. This is useful + * e.g. for clock, which need to be active during the device-removal phase. + */ +#define DM_FLAG_VITAL (1 << 14) + /* * One or multiple of these flags are passed to device_remove() so that * a selective device removal as specified by the remove-stage and the @@ -91,7 +98,8 @@ enum { /* Remove devices which need some final OS preparation steps */ DM_REMOVE_OS_PREPARE = DM_FLAG_OS_PREPARE, - /* Add more use cases here */ + /* Remove only devices that are not marked vital */ + DM_REMOVE_NON_VITAL = DM_FLAG_VITAL, /* Remove devices with any active flag */ DM_REMOVE_ACTIVE_ALL = DM_REMOVE_ACTIVE_DMA | DM_REMOVE_OS_PREPARE, diff --git a/test/dm/core.c b/test/dm/core.c index 1f5ca570dc..bfd6565d95 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -72,6 +72,14 @@ static struct driver_info driver_info_act_dma = { .name = "test_act_dma_drv", }; +static struct driver_info driver_info_vital_clk = { + .name = "test_vital_clk_drv", +}; + +static struct driver_info driver_info_act_dma_vital_clk = { + .name = "test_act_dma_vital_clk_drv", +}; + void dm_leak_check_start(struct unit_test_state *uts) { uts->start = mallinfo(); @@ -883,6 +891,92 @@ static int dm_test_remove_active_dma(struct unit_test_state *uts) } DM_TEST(dm_test_remove_active_dma, 0); +/* Test removal of 'vital' devices */ +static int dm_test_remove_vital(struct unit_test_state *uts) +{ + struct dm_test_state *dms = uts->priv; + struct udevice *normal, *dma, *vital, *dma_vital; + + /* Skip the behaviour in test_post_probe() */ + dms->skip_post_probe = 1; + + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_manual, + &normal)); + ut_assertnonnull(normal); + + ut_assertok(device_bind_by_name(dms->root, false, &driver_info_act_dma, + &dma)); + ut_assertnonnull(dma); + + ut_assertok(device_bind_by_name(dms->root, false, + &driver_info_vital_clk, &vital)); + ut_assertnonnull(vital); + + ut_assertok(device_bind_by_name(dms->root, false, + &driver_info_act_dma_vital_clk, + &dma_vital)); + ut_assertnonnull(dma_vital); + + /* Probe the devices */ + ut_assertok(device_probe(normal)); + ut_assertok(device_probe(dma)); + ut_assertok(device_probe(vital)); + ut_assertok(device_probe(dma_vital)); + + /* Check that devices are active right now */ + ut_asserteq(true, device_active(normal)); + ut_asserteq(true, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(true, device_active(dma_vital)); + + /* Remove active devices via selective remove flag */ + dm_remove_devices_flags(DM_REMOVE_NON_VITAL | DM_REMOVE_ACTIVE_ALL); + + /* + * Check that this only has an effect on the dma device, since two + * devices are vital and the third does not have active DMA + */ + ut_asserteq(true, device_active(normal)); + ut_asserteq(false, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(true, device_active(dma_vital)); + + /* Remove active devices via selective remove flag */ + ut_assertok(device_probe(dma)); + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); + + /* This should have affected both active-dma devices */ + ut_asserteq(true, device_active(normal)); + ut_asserteq(false, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(false, device_active(dma_vital)); + + /* Remove non-vital devices */ + ut_assertok(device_probe(dma)); + ut_assertok(device_probe(dma_vital)); + dm_remove_devices_flags(DM_REMOVE_NON_VITAL); + + /* This should have affected only non-vital devices */ + ut_asserteq(false, device_active(normal)); + ut_asserteq(false, device_active(dma)); + ut_asserteq(true, device_active(vital)); + ut_asserteq(true, device_active(dma_vital)); + + /* Remove vital devices via normal remove flag */ + ut_assertok(device_probe(normal)); + ut_assertok(device_probe(dma)); + dm_remove_devices_flags(DM_REMOVE_NORMAL); + + /* Check that all devices are inactive right now */ + ut_asserteq(false, device_active(normal)); + ut_asserteq(false, device_active(dma)); + ut_asserteq(false, device_active(vital)); + ut_asserteq(false, device_active(dma_vital)); + + return 0; +} +DM_TEST(dm_test_remove_vital, 0); + static int dm_test_uclass_before_ready(struct unit_test_state *uts) { struct uclass *uc; diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index a67f5d3f98..ca7626a066 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -170,3 +170,25 @@ U_BOOT_DRIVER(test_act_dma_drv) = { .unbind = test_manual_unbind, .flags = DM_FLAG_ACTIVE_DMA, }; + +U_BOOT_DRIVER(test_vital_clk_drv) = { + .name = "test_vital_clk_drv", + .id = UCLASS_TEST, + .ops = &test_manual_ops, + .bind = test_manual_bind, + .probe = test_manual_probe, + .remove = test_manual_remove, + .unbind = test_manual_unbind, + .flags = DM_FLAG_VITAL, +}; + +U_BOOT_DRIVER(test_act_dma_vital_clk_drv) = { + .name = "test_act_dma_vital_clk_drv", + .id = UCLASS_TEST, + .ops = &test_manual_ops, + .bind = test_manual_bind, + .probe = test_manual_probe, + .remove = test_manual_remove, + .unbind = test_manual_unbind, + .flags = DM_FLAG_VITAL | DM_FLAG_ACTIVE_DMA, +}; From 6224dc9ba428a7b7f7433d2bfd7bdf070b5bf06f Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:47 -0700 Subject: [PATCH 08/21] arm: Remove vital devices last Update announce_and_cleanup() to remove all devices, with the vital ones being removed last. This is an extra patch on top of the recent RFC: http://patchwork.ozlabs.org/project/uboot/list/?series=223280 Signed-off-by: Simon Glass --- arch/arm/lib/bootm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 1206e306db..f46d51d655 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -119,6 +119,9 @@ static void announce_and_cleanup(int fake) * This may be useful for last-stage operations, like cancelling * of DMA operation or releasing device internal buffers. */ + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL | DM_REMOVE_NON_VITAL); + + /* Remove all active vital devices next */ dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); cleanup_before_linux(); From 7a3c628c43f7200684edf81783e11b69fd0fd7df Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 24 Jan 2021 14:32:48 -0700 Subject: [PATCH 09/21] dm: core: Add documentation about device removal Make mention of this feature in the core documentation so people can discover it without looking at a header file. Signed-off-by: Simon Glass --- doc/driver-model/design.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/doc/driver-model/design.rst b/doc/driver-model/design.rst index ffed7d5f79..2417976ab7 100644 --- a/doc/driver-model/design.rst +++ b/doc/driver-model/design.rst @@ -880,6 +880,26 @@ If a parent has children these will be destroyed first. After this point the device does not exist and its memory has be deallocated. +Special cases for removal +------------------------- + +Some devices need to do clean-up before the OS is called. For example, a USB +driver may want to stop the bus. This can be done in the remove() method. +Some special flags are used to determine whether to remove the device: + + DM_FLAG_OS_PREPARE - indicates that the device needs to get ready for OS + boot. The device will be removed just before the OS is booted + DM_REMOVE_ACTIVE_DMA - indicates that the device uses DMA. This is + effectively the same as DM_FLAG_OS_PREPARE, so the device is removed + before the OS is booted + DM_FLAG_VITAL - indicates that the device is 'vital' to the operation of + other devices. It is possible to remove this device after all regular + devices are removed. This is useful e.g. for a clock, which need to + be active during the device-removal phase. + +The dm_remove_devices_flags() function can be used to remove devices based on +their driver flags. + Data Structures --------------- From 85c714d8dcd56f63c2c0ae1b0f6a7e4a96c918a4 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:35:57 +0800 Subject: [PATCH 10/21] riscv: Adjust board_get_usable_ram_top() for 32-bit When testing QEMU RISC-V 'virt' machine with a 2 GiB memory configuration, it was discovered gd->ram_top is assigned to value zero in setup_dest_addr(). While gd->ram_top should not be declared as type `unsigned long`, which will be updated in a future patch, the current logic in board_get_usable_ram_top() can be updated to cover both 64-bit and 32-bit RISC-V. Signed-off-by: Bin Meng --- arch/riscv/cpu/fu540/dram.c | 7 +++---- arch/riscv/cpu/generic/dram.c | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/arch/riscv/cpu/fu540/dram.c b/arch/riscv/cpu/fu540/dram.c index 1dc77efeca..259da65a54 100644 --- a/arch/riscv/cpu/fu540/dram.c +++ b/arch/riscv/cpu/fu540/dram.c @@ -22,7 +22,6 @@ int dram_init_banksize(void) ulong board_get_usable_ram_top(ulong total_size) { -#ifdef CONFIG_64BIT /* * Ensure that we run from first 4GB so that all * addresses used by U-Boot are 32bit addresses. @@ -31,8 +30,8 @@ ulong board_get_usable_ram_top(ulong total_size) * devices work fine because DMA mapping APIs will * provide 32bit DMA addresses only. */ - if (gd->ram_top > SZ_4G) - return SZ_4G; -#endif + if (gd->ram_top >= SZ_4G) + return SZ_4G - 1; + return gd->ram_top; } diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c index 1dc77efeca..259da65a54 100644 --- a/arch/riscv/cpu/generic/dram.c +++ b/arch/riscv/cpu/generic/dram.c @@ -22,7 +22,6 @@ int dram_init_banksize(void) ulong board_get_usable_ram_top(ulong total_size) { -#ifdef CONFIG_64BIT /* * Ensure that we run from first 4GB so that all * addresses used by U-Boot are 32bit addresses. @@ -31,8 +30,8 @@ ulong board_get_usable_ram_top(ulong total_size) * devices work fine because DMA mapping APIs will * provide 32bit DMA addresses only. */ - if (gd->ram_top > SZ_4G) - return SZ_4G; -#endif + if (gd->ram_top >= SZ_4G) + return SZ_4G - 1; + return gd->ram_top; } From 65f354cd8df614e1262fe05691543b1947e5091c Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:35:58 +0800 Subject: [PATCH 11/21] arm: rockchip: Explicitly cast gd->ram_top in dram_init_banksize() The min() macro used in dram_init_banksize() requires two elements to compare have the same type. Let's explicitly cast gd->ram_top. Signed-off-by: Bin Meng --- arch/arm/mach-rockchip/sdram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c index 4c637b7767..c3d5fed7db 100644 --- a/arch/arm/mach-rockchip/sdram.c +++ b/arch/arm/mach-rockchip/sdram.c @@ -37,7 +37,7 @@ struct tos_parameter_t { int dram_init_banksize(void) { size_t top = min((unsigned long)(gd->ram_size + CONFIG_SYS_SDRAM_BASE), - gd->ram_top); + (unsigned long)(gd->ram_top)); #ifdef CONFIG_ARM64 /* Reserve 0x200000 for ATF bl31 */ From 37dc958947ed305fcbd732172e22ff758f43208a Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:35:59 +0800 Subject: [PATCH 12/21] global_data.h: Change ram_top type to phys_addr_t It's possible to have ram_top above 4 GiB in a 32-bit system, hence we need to declare ram_top as `phys_addr_t`. Signed-off-by: Bin Meng Reviewed-by: Simon Glass --- include/asm-generic/global_data.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index b6f707e97e..998beb0176 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -147,7 +147,7 @@ struct global_data { /** * @ram_top: top address of RAM used by U-Boot */ - unsigned long ram_top; + phys_addr_t ram_top; /** * @relocaddr: start address of U-Boot in RAM * From 5c7c9e623c5ffdc333ef4876ee800384c60a4955 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:00 +0800 Subject: [PATCH 13/21] serial: sifive: Cast dev_read_addr() with uintptr_t dev_read_addr() returns fdt_addr_t which is now a 64-bit address. In a 32-bit build, this causes the following warning seen when building serial_sifive.c: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Cast the return value with uintptr_t. Signed-off-by: Bin Meng --- drivers/serial/serial_sifive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/serial/serial_sifive.c b/drivers/serial/serial_sifive.c index d26fe7e770..97bf20c967 100644 --- a/drivers/serial/serial_sifive.c +++ b/drivers/serial/serial_sifive.c @@ -178,7 +178,7 @@ static int sifive_serial_of_to_plat(struct udevice *dev) { struct sifive_uart_plat *plat = dev_get_plat(dev); - plat->regs = (struct uart_sifive *)dev_read_addr(dev); + plat->regs = (struct uart_sifive *)(uintptr_t)dev_read_addr(dev); if (IS_ERR(plat->regs)) return PTR_ERR(plat->regs); From b7324b5d5380cddbac7f17bc6229776763c08ee4 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:01 +0800 Subject: [PATCH 14/21] riscv: ax25-ae350: Cast addr with uintptr_t addr was delcared as fdt_addr_t which is now a 64-bit address. In a 32-bit build, this causes the following warning seen when building ax25-ae350.c: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Cast addr with uintptr_t. Signed-off-by: Bin Meng --- board/AndesTech/ax25-ae350/ax25-ae350.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c b/board/AndesTech/ax25-ae350/ax25-ae350.c index 59a43e4dcc..3125233488 100644 --- a/board/AndesTech/ax25-ae350/ax25-ae350.c +++ b/board/AndesTech/ax25-ae350/ax25-ae350.c @@ -77,7 +77,7 @@ int smc_init(void) if (addr == FDT_ADDR_T_NONE) return -EINVAL; - regs = (struct ftsmc020_bank *)addr; + regs = (struct ftsmc020_bank *)(uintptr_t)addr; regs->cr &= ~FTSMC020_BANK_WPROT; return 0; From 673625c45938dc9616e5cdb64bbd6459fd74fb00 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:02 +0800 Subject: [PATCH 15/21] net: ftmac100: Cast priv->iobase with uintptr_t priv->iobase was declared as phys_addr_t which is now a 64-bit address. In a 32-bit build, this causes the following warning seen when building ftmac100.c: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] Cast priv->iobase with uintptr_t. Signed-off-by: Bin Meng --- drivers/net/ftmac100.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/ftmac100.c b/drivers/net/ftmac100.c index 0d672374fd..fc578b0375 100644 --- a/drivers/net/ftmac100.c +++ b/drivers/net/ftmac100.c @@ -35,7 +35,7 @@ struct ftmac100_data { */ static void ftmac100_reset(struct ftmac100_data *priv) { - struct ftmac100 *ftmac100 = (struct ftmac100 *)priv->iobase; + struct ftmac100 *ftmac100 = (struct ftmac100 *)(uintptr_t)priv->iobase; debug ("%s()\n", __func__); @@ -56,7 +56,7 @@ static void ftmac100_reset(struct ftmac100_data *priv) static void ftmac100_set_mac(struct ftmac100_data *priv , const unsigned char *mac) { - struct ftmac100 *ftmac100 = (struct ftmac100 *)priv->iobase; + struct ftmac100 *ftmac100 = (struct ftmac100 *)(uintptr_t)priv->iobase; unsigned int maddr = mac[0] << 8 | mac[1]; unsigned int laddr = mac[2] << 24 | mac[3] << 16 | mac[4] << 8 | mac[5]; @@ -71,7 +71,7 @@ static void ftmac100_set_mac(struct ftmac100_data *priv , */ static void _ftmac100_halt(struct ftmac100_data *priv) { - struct ftmac100 *ftmac100 = (struct ftmac100 *)priv->iobase; + struct ftmac100 *ftmac100 = (struct ftmac100 *)(uintptr_t)priv->iobase; debug ("%s()\n", __func__); writel (0, &ftmac100->maccr); } @@ -81,7 +81,7 @@ static void _ftmac100_halt(struct ftmac100_data *priv) */ static int _ftmac100_init(struct ftmac100_data *priv, unsigned char enetaddr[6]) { - struct ftmac100 *ftmac100 = (struct ftmac100 *)priv->iobase; + struct ftmac100 *ftmac100 = (struct ftmac100 *)(uintptr_t)priv->iobase; struct ftmac100_txdes *txdes = priv->txdes; struct ftmac100_rxdes *rxdes = priv->rxdes; unsigned int maccr; @@ -186,7 +186,7 @@ static int __ftmac100_recv(struct ftmac100_data *priv) */ static int _ftmac100_send(struct ftmac100_data *priv, void *packet, int length) { - struct ftmac100 *ftmac100 = (struct ftmac100 *)priv->iobase; + struct ftmac100 *ftmac100 = (struct ftmac100 *)(uintptr_t)priv->iobase; struct ftmac100_txdes *curr_des = priv->txdes; ulong start; From d17e9d2558ecbc2d6486555e6717396356b1f4e1 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:03 +0800 Subject: [PATCH 16/21] fdtdec: Cast prior_stage_fdt_address with uintptr_t At present prior_stage_fdt_address is declared as phys_addr_t. On a 32-bit platform where phys_addr_t can be 64-bit, assigning its value to gd->fdt_blob which is a pointer, can cause warnings. Cast it to uintptr_t before the assignment. Signed-off-by: Bin Meng Reviewed-by: Simon Glass --- lib/fdtdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fdtdec.c b/lib/fdtdec.c index a2d2fb4e1f..e048f7777d 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1572,7 +1572,7 @@ int fdtdec_setup(void) return -1; } # elif defined(CONFIG_OF_PRIOR_STAGE) - gd->fdt_blob = (void *)prior_stage_fdt_address; + gd->fdt_blob = (void *)(uintptr_t)prior_stage_fdt_address; # endif # ifndef CONFIG_SPL_BUILD /* Allow the early environment to override the fdt address */ From 86c915628d582a36029ff1f6c4443b6e81e0d51f Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:04 +0800 Subject: [PATCH 17/21] riscv: Change phys_addr_t and phys_size_t to 64-bit phys_addr_t and phys_size_t are currently defined as `unsigned long`, but RV32 supports 34-bit physical address, hence both phys_addr_t and phys_size_t should be defined to 64-bit using `unsigned long long`. Signed-off-by: Bin Meng --- arch/riscv/include/asm/types.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/riscv/include/asm/types.h b/arch/riscv/include/asm/types.h index b800b2d221..49f7a5d6b3 100644 --- a/arch/riscv/include/asm/types.h +++ b/arch/riscv/include/asm/types.h @@ -35,8 +35,8 @@ typedef u64 dma_addr_t; typedef u32 dma_addr_t; #endif -typedef unsigned long phys_addr_t; -typedef unsigned long phys_size_t; +typedef unsigned long long phys_addr_t; +typedef unsigned long long phys_size_t; #endif /* __KERNEL__ */ From 98592c7509278e73b2d56c5e307015d6f33c0f34 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:05 +0800 Subject: [PATCH 18/21] bdinfo: Rename function names to be clearer At present we have bdinfo_print_num() to print unsigned long numbers. We also have print_phys_addr() which accept numbers that might be 64-bit on a 32-bit platform. Rename these 2 functions to be clearer: bdinfo_print_num() => bdinfo_print_num_l() print_phys_addr() => bdinfo_print_num_ll() While we are here, make bdinfo_print_num_ll() public so that it can be used outside cmd/bdinfo.c in the future. Signed-off-by: Bin Meng --- arch/arm/lib/bdinfo.c | 16 ++++++------ arch/m68k/lib/bdinfo.c | 2 +- arch/powerpc/lib/bdinfo.c | 4 +-- cmd/bdinfo.c | 52 +++++++++++++++++++-------------------- include/init.h | 3 ++- 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/arch/arm/lib/bdinfo.c b/arch/arm/lib/bdinfo.c index 25bc6e80f4..4a98cb7ef5 100644 --- a/arch/arm/lib/bdinfo.c +++ b/arch/arm/lib/bdinfo.c @@ -15,23 +15,23 @@ void arch_print_bdinfo(void) { struct bd_info *bd = gd->bd; - bdinfo_print_num("arch_number", bd->bi_arch_number); + bdinfo_print_num_l("arch_number", bd->bi_arch_number); #ifdef CONFIG_SYS_MEM_RESERVE_SECURE if (gd->arch.secure_ram & MEM_RESERVE_SECURE_SECURED) { - bdinfo_print_num("Secure ram", - gd->arch.secure_ram & - MEM_RESERVE_SECURE_ADDR_MASK); + bdinfo_print_num_l("Secure ram", + gd->arch.secure_ram & + MEM_RESERVE_SECURE_ADDR_MASK); } #endif #ifdef CONFIG_RESV_RAM if (gd->arch.resv_ram) - bdinfo_print_num("Reserved ram", gd->arch.resv_ram); + bdinfo_print_num_l("Reserved ram", gd->arch.resv_ram); #endif #if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) - bdinfo_print_num("TLB addr", gd->arch.tlb_addr); + bdinfo_print_num_l("TLB addr", gd->arch.tlb_addr); #endif - bdinfo_print_num("irq_sp", gd->irq_sp); /* irq stack pointer */ - bdinfo_print_num("sp start ", gd->start_addr_sp); + bdinfo_print_num_l("irq_sp", gd->irq_sp); /* irq stack pointer */ + bdinfo_print_num_l("sp start ", gd->start_addr_sp); /* * TODO: Currently only support for davinci SOC's is added. * Remove this check once all the board implement this. diff --git a/arch/m68k/lib/bdinfo.c b/arch/m68k/lib/bdinfo.c index 404e5f19ed..92ea175202 100644 --- a/arch/m68k/lib/bdinfo.c +++ b/arch/m68k/lib/bdinfo.c @@ -38,7 +38,7 @@ void arch_print_bdinfo(void) bdinfo_print_mhz("busfreq", bd->bi_busfreq); #if defined(CONFIG_SYS_MBAR) - bdinfo_print_num("mbar", bd->bi_mbar_base); + bdinfo_print_num_l("mbar", bd->bi_mbar_base); #endif bdinfo_print_mhz("cpufreq", bd->bi_intfreq); if (IS_ENABLED(CONFIG_PCI)) diff --git a/arch/powerpc/lib/bdinfo.c b/arch/powerpc/lib/bdinfo.c index 36c9c99ee6..b14e75b68a 100644 --- a/arch/powerpc/lib/bdinfo.c +++ b/arch/powerpc/lib/bdinfo.c @@ -47,9 +47,9 @@ void arch_print_bdinfo(void) bdinfo_print_mhz("busfreq", bd->bi_busfreq); #if defined(CONFIG_MPC8xx) || defined(CONFIG_E500) - bdinfo_print_num("immr_base", bd->bi_immr_base); + bdinfo_print_num_l("immr_base", bd->bi_immr_base); #endif - bdinfo_print_num("bootflags", bd->bi_bootflags); + bdinfo_print_num_l("bootflags", bd->bi_bootflags); bdinfo_print_mhz("intfreq", bd->bi_intfreq); #ifdef CONFIG_ENABLE_36BIT_PHYS if (IS_ENABLED(CONFIG_PHYS_64BIT)) diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index 8d8daa6336..996546faf3 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -18,11 +18,16 @@ DECLARE_GLOBAL_DATA_PTR; -void bdinfo_print_num(const char *name, ulong value) +void bdinfo_print_num_l(const char *name, ulong value) { printf("%-12s= 0x%0*lx\n", name, 2 * (int)sizeof(value), value); } +void bdinfo_print_num_ll(const char *name, unsigned long long value) +{ + printf("%-12s= 0x%.*llx\n", name, 2 * (int)sizeof(ulong), value); +} + static void print_eth(int idx) { char name[10], *val; @@ -36,12 +41,6 @@ static void print_eth(int idx) printf("%-12s= %s\n", name, val); } -static void print_phys_addr(const char *name, phys_addr_t value) -{ - printf("%-12s= 0x%.*llx\n", name, 2 * (int)sizeof(ulong), - (unsigned long long)value); -} - void bdinfo_print_mhz(const char *name, unsigned long hz) { char buf[32]; @@ -55,9 +54,9 @@ static void print_bi_dram(const struct bd_info *bd) for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) { if (bd->bi_dram[i].size) { - bdinfo_print_num("DRAM bank", i); - bdinfo_print_num("-> start", bd->bi_dram[i].start); - bdinfo_print_num("-> size", bd->bi_dram[i].size); + bdinfo_print_num_l("DRAM bank", i); + bdinfo_print_num_l("-> start", bd->bi_dram[i].start); + bdinfo_print_num_l("-> size", bd->bi_dram[i].size); } } } @@ -77,9 +76,10 @@ static void show_video_info(void) if (device_active(dev)) { struct video_priv *upriv = dev_get_uclass_priv(dev); - print_phys_addr("FB base", (ulong)upriv->fb); + bdinfo_print_num_ll("FB base", (ulong)upriv->fb); if (upriv->copy_fb) - print_phys_addr("FB copy", (ulong)upriv->copy_fb); + bdinfo_print_num_ll("FB copy", + (ulong)upriv->copy_fb); printf("%-12s= %dx%dx%d\n", "FB size", upriv->xsize, upriv->ysize, 1 << upriv->bpix); } @@ -91,36 +91,36 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct bd_info *bd = gd->bd; #ifdef DEBUG - bdinfo_print_num("bd address", (ulong)bd); + bdinfo_print_num_l("bd address", (ulong)bd); #endif - bdinfo_print_num("boot_params", (ulong)bd->bi_boot_params); + bdinfo_print_num_l("boot_params", (ulong)bd->bi_boot_params); print_bi_dram(bd); if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) { - bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart); - bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize); + bdinfo_print_num_l("sramstart", (ulong)bd->bi_sramstart); + bdinfo_print_num_l("sramsize", (ulong)bd->bi_sramsize); } - bdinfo_print_num("flashstart", (ulong)bd->bi_flashstart); - bdinfo_print_num("flashsize", (ulong)bd->bi_flashsize); - bdinfo_print_num("flashoffset", (ulong)bd->bi_flashoffset); + bdinfo_print_num_l("flashstart", (ulong)bd->bi_flashstart); + bdinfo_print_num_l("flashsize", (ulong)bd->bi_flashsize); + bdinfo_print_num_l("flashoffset", (ulong)bd->bi_flashoffset); printf("baudrate = %u bps\n", gd->baudrate); - bdinfo_print_num("relocaddr", gd->relocaddr); - bdinfo_print_num("reloc off", gd->reloc_off); + bdinfo_print_num_l("relocaddr", gd->relocaddr); + bdinfo_print_num_l("reloc off", gd->reloc_off); printf("%-12s= %u-bit\n", "Build", (uint)sizeof(void *) * 8); if (IS_ENABLED(CONFIG_CMD_NET)) { printf("current eth = %s\n", eth_get_name()); print_eth(0); printf("IP addr = %s\n", env_get("ipaddr")); } - bdinfo_print_num("fdt_blob", (ulong)gd->fdt_blob); - bdinfo_print_num("new_fdt", (ulong)gd->new_fdt); - bdinfo_print_num("fdt_size", (ulong)gd->fdt_size); + bdinfo_print_num_l("fdt_blob", (ulong)gd->fdt_blob); + bdinfo_print_num_l("new_fdt", (ulong)gd->new_fdt); + bdinfo_print_num_l("fdt_size", (ulong)gd->fdt_size); if (IS_ENABLED(CONFIG_DM_VIDEO)) show_video_info(); #if defined(CONFIG_LCD) || defined(CONFIG_VIDEO) - bdinfo_print_num("FB base ", gd->fb_base); + bdinfo_print_num_l("FB base ", gd->fb_base); #endif #if CONFIG_IS_ENABLED(MULTI_DTB_FIT) - bdinfo_print_num("multi_dtb_fit", (ulong)gd->multi_dtb_fit); + bdinfo_print_num_l("multi_dtb_fit", (ulong)gd->multi_dtb_fit); #endif if (gd->fdt_blob) { struct lmb lmb; diff --git a/include/init.h b/include/init.h index 980be27993..88f84599e9 100644 --- a/include/init.h +++ b/include/init.h @@ -326,7 +326,8 @@ void relocate_code(ulong start_addr_sp, struct global_data *new_gd, #endif /* Print a numeric value (for use in arch_print_bdinfo()) */ -void bdinfo_print_num(const char *name, ulong value); +void bdinfo_print_num_l(const char *name, ulong value); +void bdinfo_print_num_ll(const char *name, unsigned long long value); /* Print a clock speed in MHz */ void bdinfo_print_mhz(const char *name, unsigned long hz); From 6424fba1bc4e80c43eb5eb69b9e66c0d89d1b1cf Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Sun, 31 Jan 2021 20:36:06 +0800 Subject: [PATCH 19/21] bdinfo: Change to use bdinfo_print_num_ll() where the number could be 64-bit There are some calls to bdinfo_print_num_l() with parameters that could be a 64-bit value on a 32-bit system. Change those calls to use bdinfo_print_num_ll() instead. Signed-off-by: Bin Meng Reviewed-by: Simon Glass --- arch/arm/lib/bdinfo.c | 8 ++++---- cmd/bdinfo.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/lib/bdinfo.c b/arch/arm/lib/bdinfo.c index 4a98cb7ef5..c905783bdc 100644 --- a/arch/arm/lib/bdinfo.c +++ b/arch/arm/lib/bdinfo.c @@ -18,14 +18,14 @@ void arch_print_bdinfo(void) bdinfo_print_num_l("arch_number", bd->bi_arch_number); #ifdef CONFIG_SYS_MEM_RESERVE_SECURE if (gd->arch.secure_ram & MEM_RESERVE_SECURE_SECURED) { - bdinfo_print_num_l("Secure ram", - gd->arch.secure_ram & - MEM_RESERVE_SECURE_ADDR_MASK); + bdinfo_print_num_ll("Secure ram", + gd->arch.secure_ram & + MEM_RESERVE_SECURE_ADDR_MASK); } #endif #ifdef CONFIG_RESV_RAM if (gd->arch.resv_ram) - bdinfo_print_num_l("Reserved ram", gd->arch.resv_ram); + bdinfo_print_num_ll("Reserved ram", gd->arch.resv_ram); #endif #if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) bdinfo_print_num_l("TLB addr", gd->arch.tlb_addr); diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index 996546faf3..dfd50ae849 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -55,8 +55,8 @@ static void print_bi_dram(const struct bd_info *bd) for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) { if (bd->bi_dram[i].size) { bdinfo_print_num_l("DRAM bank", i); - bdinfo_print_num_l("-> start", bd->bi_dram[i].start); - bdinfo_print_num_l("-> size", bd->bi_dram[i].size); + bdinfo_print_num_ll("-> start", bd->bi_dram[i].start); + bdinfo_print_num_ll("-> size", bd->bi_dram[i].size); } } } From c30a7093e81be0dbec963b6b347f678630c2eb8d Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 1 Feb 2021 01:24:10 +0100 Subject: [PATCH 20/21] sandbox: mark os_abort() as noreturn gcc -fanalyzer needs the information that a function does not return to provide accurate information. os_abort() does not return. Mark it accordingly. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- include/os.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/os.h b/include/os.h index e192e32d59..65bcb232ca 100644 --- a/include/os.h +++ b/include/os.h @@ -341,7 +341,7 @@ void os_localtime(struct rtc_time *rt); /** * os_abort() - raise SIGABRT to exit sandbox (e.g. to debugger) */ -void os_abort(void); +void os_abort(void) __attribute__((noreturn)); /** * os_mprotect_allow() - Remove write-protection on a region of memory From 6f5edbaf938952e4a9860727c633dcaa36caf699 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 1 Feb 2021 03:01:54 +0100 Subject: [PATCH 21/21] firmware: smci: possible NULL dereference sandbox_scmi_devices_ctx() may return NULL. We should not dereference this value in sandbox_scmi_devices_remove(). The problem was indicated by 'gcc-11 -fanalyzer'. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- drivers/firmware/scmi/sandbox-scmi_devices.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c index 414da6f4df..1a6fafbf53 100644 --- a/drivers/firmware/scmi/sandbox-scmi_devices.c +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c @@ -50,6 +50,9 @@ static int sandbox_scmi_devices_remove(struct udevice *dev) int ret = 0; size_t n; + if (!devices) + return 0; + for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) { int ret2 = reset_free(devices->reset + n);