[PATCH 00/19] dm: core: Fully separate ofdata_to_platdata() from probe()
At present ofdata_to_platdata() is done as part of device_probe(). Drivers are supposed to read their platdata in the ofdata_to_platdata method() but this requirement is not always followed.
Having these methods separate helps deal with of-platdata, where the probe() method is common to both DT/of-platdata operation, but the ofdata_to_platdata() method is implemented differently.
Another case has come up where this separate is useful. Generation of ACPI tables uses the of-platdata but does not want to probe the device. Probing would cause U-Boot to violate one of its design principles, viz that it should only probe devices that are used. For ACPI we want to generate a table for each device, even if U-Boot does not use it. In fact it may not even be possible to probe the device - e.g. an SD card which is not present will cause an error on probe, yet we still must tell Linux about the SD card connector in case it is used while Linux is running.
This series splits out the ofdata_to_platdata() part of device_probe() so that it can be used separately from device_probe(). Thus devices now go through two distinct states when probing: reading platform data and actually touching the hardware to bring the device up.
This should not break existing boards since the operations still happen in mostly the same order. The main change is that parents and uclasses are probed after ofdata_to_platdata() is called.
HOWEVER it is quite possible that some boards break the rules and due to a series of unfortunate events, something will break. Two boards were found in this category already. SO this series may require some tidying up from board maintainers, if problems arise.
Note that there are cases where devices must be probed in the ofdata_to_platdata() method. An example is where a GPIO is selected - this obviously requires that the GPIO device is probed.
One board was found to burst its size limit with this series, despite the very small size increase. The patches to remove use of BUG_ON() are to ensure that this series passes tests.
Simon Glass (19): common: Add a noisy assert() dm: core: Use assert_noisy() in devres usb: Drop use of BUG_ON() and WARN_ON() x86: apl: Avoid accessing the PCI bus before it is probed pci: Print a warning if the bus is accessed before probing aspeed: ast2500: Read clock ofdata in the correct method dm: core: Don't clear active flag twice when probe() fails dm: core: Move ofdata_to_platdata() call earlier dm: core: Allocate parent data separate from probing parent dm: core: Add a comment for DM_FLAG_OF_PLATDATA dm: core: Export a new function to read platdata dm: core: Add a new flag to track platform data dm: devres: Create a new devres header file test: Add functions to find the amount of allocated memory dm: devres: Convert to use logging dm: test: Add a test driver for devres dm: devres: Add tests dm: devres: Use an enum for the allocation phase dm: devres: Add a new OFDATA phase
arch/sandbox/dts/test.dts | 4 + arch/x86/cpu/apollolake/p2sb.c | 20 +- arch/x86/cpu/apollolake/pmc.c | 20 +- drivers/clk/aspeed/clk_ast2500.c | 4 +- drivers/core/device-remove.c | 1 + drivers/core/device.c | 56 +++-- drivers/core/devres.c | 57 ++++- drivers/pci/pci-uclass.c | 13 ++ drivers/usb/gadget/composite.c | 4 + drivers/usb/gadget/f_mass_storage.c | 4 + drivers/usb/musb-new/musb_core.c | 4 + drivers/usb/musb-new/musb_gadget_ep0.c | 2 +- include/dm/device-internal.h | 16 ++ include/dm/device.h | 259 +--------------------- include/dm/devres.h | 289 +++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + include/log.h | 15 ++ include/test/test.h | 10 + include/test/ut.h | 16 ++ test/dm/Makefile | 1 + test/dm/devres.c | 186 ++++++++++++++++ test/dm/test-fdt.c | 58 +++++ test/ut.c | 14 ++ 23 files changed, 760 insertions(+), 294 deletions(-) create mode 100644 include/dm/devres.h create mode 100644 test/dm/devres.c
Some U-Boot code uses BUG_ON() and WARN_ON() macros. These use __FILE__ which can include quite a large path, depending on how U-Boot is built.
The existing assert() is only checked if DEBUG is enabled. Add a new one which is always checked, and prints a (smaller) error in that case.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/log.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/log.h b/include/log.h index d8f18a6afd..c6f2f023b1 100644 --- a/include/log.h +++ b/include/log.h @@ -218,6 +218,20 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line, ({ if (!(x) && _DEBUG) \ __assert_fail(#x, __FILE__, __LINE__, __func__); })
+/* + * This one actually gets compiled in even without DEBUG. It doesn't include the + * full pathname as it may be huge. Only use this when the user should be + * warning, similar to BUG_ON() in linux. + * + * @return true if assertion succeeded (condition is true), else false + */ +#define assert_noisy(x) \ + ({ bool _val = (x); \ + if (!_val) \ + __assert_fail(#x, "?", __LINE__, __func__); \ + _val; \ + }) + #if CONFIG_IS_ENABLED(LOG) && defined(CONFIG_LOG_ERROR_RETURN) /* * Log an error return value, possibly with a message. Usage:
Some U-Boot code uses BUG_ON() and WARN_ON() macros. These use __FILE__ which can include quite a large path, depending on how U-Boot is built.
The existing assert() is only checked if DEBUG is enabled. Add a new one which is always checked, and prints a (smaller) error in that case.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/log.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Applied to u-boot-dm, thanks!
Use this macros instead of the linux ones, as the output is smaller.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/core/devres.c b/drivers/core/devres.c index f2a19ec61b..a3f915dd73 100644 --- a/drivers/core/devres.c +++ b/drivers/core/devres.c @@ -80,7 +80,7 @@ void devres_free(void *res) if (res) { struct devres *dr = container_of(res, struct devres, data);
- BUG_ON(!list_empty(&dr->entry)); + assert_noisy(list_empty(&dr->entry)); kfree(dr); } } @@ -90,7 +90,7 @@ void devres_add(struct udevice *dev, void *res) struct devres *dr = container_of(res, struct devres, data);
devres_log(dev, dr, "ADD"); - BUG_ON(!list_empty(&dr->entry)); + assert_noisy(list_empty(&dr->entry)); dr->probe = dev->flags & DM_FLAG_BOUND ? true : false; list_add_tail(&dr->entry, &dev->devres_head); } @@ -254,5 +254,5 @@ void devm_kfree(struct udevice *dev, void *p) int rc;
rc = devres_destroy(dev, devm_kmalloc_release, devm_kmalloc_match, p); - WARN_ON(rc); + assert_noisy(!rc); }
Use this macros instead of the linux ones, as the output is smaller.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!
These macros use __FILE__ which inserts the full path of the object file into U-Boot, thus increasing file size. Drop these usages.
An older version of this patch was submitted here:
http://patchwork.ozlabs.org/patch/1205784/
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/usb/gadget/composite.c | 4 ++++ drivers/usb/gadget/f_mass_storage.c | 4 ++++ drivers/usb/musb-new/musb_core.c | 4 ++++ drivers/usb/musb-new/musb_gadget_ep0.c | 2 +- 4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c index c98a444245..4a6f4271d5 100644 --- a/drivers/usb/gadget/composite.c +++ b/drivers/usb/gadget/composite.c @@ -1003,7 +1003,11 @@ static void composite_unbind(struct usb_gadget *gadget) * so there's no i/o concurrency that could affect the * state protected by cdev->lock. */ +#ifdef __UBOOT__ + assert_noisy(!cdev->config); +#else BUG_ON(cdev->config); +#endif
while (!list_empty(&cdev->configs)) { c = list_first_entry(&cdev->configs, diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c index 45c7b58eed..c1e6506659 100644 --- a/drivers/usb/gadget/f_mass_storage.c +++ b/drivers/usb/gadget/f_mass_storage.c @@ -390,7 +390,11 @@ static inline int __fsg_is_set(struct fsg_common *common, if (common->fsg) return 1; ERROR(common, "common->fsg is NULL in %s at %u\n", func, line); +#ifdef __UBOOT__ + assert_noisy(false); +#else WARN_ON(1); +#endif return 0; }
diff --git a/drivers/usb/musb-new/musb_core.c b/drivers/usb/musb-new/musb_core.c index afea9fbcef..ab5e3aa9d1 100644 --- a/drivers/usb/musb-new/musb_core.c +++ b/drivers/usb/musb-new/musb_core.c @@ -1859,7 +1859,11 @@ allocate_instance(struct device *dev, musb->ctrl_base = mbase; musb->nIrq = -ENODEV; musb->config = config; +#ifdef __UBOOT__ + assert_noisy(musb->config->num_eps <= MUSB_C_NUM_EPS); +#else BUG_ON(musb->config->num_eps > MUSB_C_NUM_EPS); +#endif for (epnum = 0, ep = musb->endpoints; epnum < musb->config->num_eps; epnum++, ep++) { diff --git a/drivers/usb/musb-new/musb_gadget_ep0.c b/drivers/usb/musb-new/musb_gadget_ep0.c index 9835a2e2bf..3ef8fe1373 100644 --- a/drivers/usb/musb-new/musb_gadget_ep0.c +++ b/drivers/usb/musb-new/musb_gadget_ep0.c @@ -882,7 +882,7 @@ finish:
default: /* "can't happen" */ - WARN_ON(1); + assert_noisy(false); musb_writew(regs, MUSB_CSR0, MUSB_CSR0_P_SENDSTALL); musb->ep0_state = MUSB_EP0_STAGE_IDLE; break;
These macros use __FILE__ which inserts the full path of the object file into U-Boot, thus increasing file size. Drop these usages.
An older version of this patch was submitted here:
http://patchwork.ozlabs.org/patch/1205784/
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/usb/gadget/composite.c | 4 ++++ drivers/usb/gadget/f_mass_storage.c | 4 ++++ drivers/usb/musb-new/musb_core.c | 4 ++++ drivers/usb/musb-new/musb_gadget_ep0.c | 2 +- 4 files changed, 13 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
The PCI bus is not actually probed by the time the ofdata_to_platdata() method is called since that happens in the uclass's post_probe() method. Update the PMC and P2SB drivers to access the bus in its probe() method.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/p2sb.c | 20 ++++++++++++-------- arch/x86/cpu/apollolake/pmc.c | 20 +++++++++++++++++--- 2 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/arch/x86/cpu/apollolake/p2sb.c b/arch/x86/cpu/apollolake/p2sb.c index eb27861b7a..b72f50a627 100644 --- a/arch/x86/cpu/apollolake/p2sb.c +++ b/arch/x86/cpu/apollolake/p2sb.c @@ -106,11 +106,6 @@ int apl_p2sb_ofdata_to_platdata(struct udevice *dev) if (plat->bdf < 0) return log_msg_ret("Cannot get p2sb PCI address", plat->bdf); - } else { - plat->mmio_base = dev_read_addr_pci(dev); - /* Don't set BDF since it should not be used */ - if (!plat->mmio_base || plat->mmio_base == FDT_ADDR_T_NONE) - return -EINVAL; } #else plat->mmio_base = plat->dtplat.early_regs[0]; @@ -124,10 +119,19 @@ int apl_p2sb_ofdata_to_platdata(struct udevice *dev)
static int apl_p2sb_probe(struct udevice *dev) { - if (spl_phase() == PHASE_TPL) + if (spl_phase() == PHASE_TPL) { return apl_p2sb_early_init(dev); - else if (spl_phase() == PHASE_SPL) - return apl_p2sb_spl_init(dev); + } else { + struct p2sb_platdata *plat = dev_get_platdata(dev); + + plat->mmio_base = dev_read_addr_pci(dev); + /* Don't set BDF since it should not be used */ + if (!plat->mmio_base || plat->mmio_base == FDT_ADDR_T_NONE) + return -EINVAL; + + if (spl_phase() == PHASE_SPL) + return apl_p2sb_spl_init(dev); + }
return 0; } diff --git a/arch/x86/cpu/apollolake/pmc.c b/arch/x86/cpu/apollolake/pmc.c index 683c6082f2..aec0c8394c 100644 --- a/arch/x86/cpu/apollolake/pmc.c +++ b/arch/x86/cpu/apollolake/pmc.c @@ -119,8 +119,16 @@ int apl_pmc_ofdata_to_uc_platdata(struct udevice *dev) ret = dev_read_u32_array(dev, "early-regs", base, ARRAY_SIZE(base)); if (ret) return log_msg_ret("Missing/short early-regs", ret); - upriv->pmc_bar0 = (void *)base[0]; - upriv->pmc_bar2 = (void *)base[2]; + if (spl_phase() == PHASE_TPL) { + upriv->pmc_bar0 = (void *)base[0]; + upriv->pmc_bar2 = (void *)base[2]; + + /* Since PCI is not enabled, we must get the BDF manually */ + plat->bdf = pci_get_devfn(dev); + if (plat->bdf < 0) + return log_msg_ret("Cannot get PMC PCI address", + plat->bdf); + } upriv->acpi_base = base[4];
/* Since PCI is not enabled, we must get the BDF manually */ @@ -187,8 +195,14 @@ static int enable_pmcbar(struct udevice *dev)
static int apl_pmc_probe(struct udevice *dev) { - if (spl_phase() == PHASE_TPL) + if (spl_phase() == PHASE_TPL) { return enable_pmcbar(dev); + } else { + struct acpi_pmc_upriv *upriv = dev_get_uclass_priv(dev); + + upriv->pmc_bar0 = (void *)dm_pci_read_bar32(dev, 0); + upriv->pmc_bar2 = (void *)dm_pci_read_bar32(dev, 2); + }
return 0; }
The PCI bus is not actually probed by the time the ofdata_to_platdata() method is called since that happens in the uclass's post_probe() method. Update the PMC and P2SB drivers to access the bus in its probe() method.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/x86/cpu/apollolake/p2sb.c | 20 ++++++++++++-------- arch/x86/cpu/apollolake/pmc.c | 20 +++++++++++++++++--- 2 files changed, 29 insertions(+), 11 deletions(-)
Applied to u-boot-dm, thanks!
It is not possible to access a device on a PCI bus that has not yet been probed, since the bus number is not known. Add a warning to catch this error.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/pci/pci-uclass.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 7308f612b6..5be2dfd0bf 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -48,6 +48,19 @@ pci_dev_t dm_pci_get_bdf(struct udevice *dev) struct pci_child_platdata *pplat = dev_get_parent_platdata(dev); struct udevice *bus = dev->parent;
+ /* + * This error indicates that @dev is a device on an unprobed PCI bus. + * The bus likely has bus=seq == -1, so the PCI_ADD_BUS() macro below + * will produce a bad BDF> + * + * A common cause of this problem is that this function is called in the + * ofdata_to_platdata() method of @dev. Accessing the PCI bus in that + * method is not allowed, since it has not yet been probed. To fix this, + * move that access to the probe() method of @dev instead. + */ + if (!device_active(bus)) + log_err("PCI: Device '%s' on unprobed bus '%s'\n", dev->name, + bus->name); return PCI_ADD_BUS(bus->seq, pplat->devfn); }
It is not possible to access a device on a PCI bus that has not yet been probed, since the bus number is not known. Add a warning to catch this error.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/pci/pci-uclass.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
Applied to u-boot-dm, thanks!
At present the clock driver reads its ofdata in the probe() method. This is not correct although it is often harmless.
However in this case it causes a problem, something like this:
- ast_get_scu() is called (from somewhere) to get the SCI address - this probes the clock - first sets up ofdata (which does nothing at present) - DM marks clock device as active - DM calls pinctrl - pinctrl probes and calls ast_get_scu() in ast2500_pinctrl_probe() - ast_get_scu() probes the clock, but sees it already marked as probed - ast_get_scu() accesses the clock's private data, with scu as NULL - DM calls clock probe function ast2500_clk_probe() which reads scu
By putting the read of scu into the correct method, scu is read as part of ofdata setup, and everything is OK.
Note: This problem did not matter until now since DM always probed all parents before reading a child's ofdata. The fact that pinctrl is a child of clock seems to trigger this strange bug.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/clk/aspeed/clk_ast2500.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c index 9249cf9cdf..b3a3f3d4dd 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++ b/drivers/clk/aspeed/clk_ast2500.c @@ -490,7 +490,7 @@ struct clk_ops ast2500_clk_ops = { .enable = ast2500_clk_enable, };
-static int ast2500_clk_probe(struct udevice *dev) +static int ast2500_clk_ofdata_to_platdata(struct udevice *dev) { struct ast2500_clk_priv *priv = dev_get_priv(dev);
@@ -525,5 +525,5 @@ U_BOOT_DRIVER(aspeed_ast2500_scu) = { .priv_auto_alloc_size = sizeof(struct ast2500_clk_priv), .ops = &ast2500_clk_ops, .bind = ast2500_clk_bind, - .probe = ast2500_clk_probe, + .ofdata_to_platdata = ast2500_clk_ofdata_to_platdata, };
On 12/30/19 5:19 AM, Simon Glass wrote:
At present the clock driver reads its ofdata in the probe() method. This is not correct although it is often harmless.
However in this case it causes a problem, something like this:
- ast_get_scu() is called (from somewhere) to get the SCI address
- this probes the clock
- first sets up ofdata (which does nothing at present)
- DM marks clock device as active
- DM calls pinctrl
- pinctrl probes and calls ast_get_scu() in ast2500_pinctrl_probe()
- ast_get_scu() probes the clock, but sees it already marked as probed
- ast_get_scu() accesses the clock's private data, with scu as NULL
- DM calls clock probe function ast2500_clk_probe() which reads scu
By putting the read of scu into the correct method, scu is read as part of ofdata setup, and everything is OK.
Note: This problem did not matter until now since DM always probed all parents before reading a child's ofdata. The fact that pinctrl is a child of clock seems to trigger this strange bug.
Did you find it with QEMU ?
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Cédric Le Goater clg@kaod.org
C.
drivers/clk/aspeed/clk_ast2500.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c index 9249cf9cdf..b3a3f3d4dd 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++ b/drivers/clk/aspeed/clk_ast2500.c @@ -490,7 +490,7 @@ struct clk_ops ast2500_clk_ops = { .enable = ast2500_clk_enable, };
-static int ast2500_clk_probe(struct udevice *dev) +static int ast2500_clk_ofdata_to_platdata(struct udevice *dev) { struct ast2500_clk_priv *priv = dev_get_priv(dev);
@@ -525,5 +525,5 @@ U_BOOT_DRIVER(aspeed_ast2500_scu) = { .priv_auto_alloc_size = sizeof(struct ast2500_clk_priv), .ops = &ast2500_clk_ops, .bind = ast2500_clk_bind,
- .probe = ast2500_clk_probe,
- .ofdata_to_platdata = ast2500_clk_ofdata_to_platdata,
};
Hi Cédric,
On Tue, 7 Jan 2020 at 21:20, Cédric Le Goater clg@kaod.org wrote:
On 12/30/19 5:19 AM, Simon Glass wrote:
At present the clock driver reads its ofdata in the probe() method. This is not correct although it is often harmless.
However in this case it causes a problem, something like this:
- ast_get_scu() is called (from somewhere) to get the SCI address
- this probes the clock
- first sets up ofdata (which does nothing at present)
- DM marks clock device as active
- DM calls pinctrl
- pinctrl probes and calls ast_get_scu() in ast2500_pinctrl_probe()
- ast_get_scu() probes the clock, but sees it already marked as probed
- ast_get_scu() accesses the clock's private data, with scu as NULL
- DM calls clock probe function ast2500_clk_probe() which reads scu
By putting the read of scu into the correct method, scu is read as part of ofdata setup, and everything is OK.
Note: This problem did not matter until now since DM always probed all parents before reading a child's ofdata. The fact that pinctrl is a child of clock seems to trigger this strange bug.
Did you find it with QEMU ?
Yes that's right.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Cédric Le Goater clg@kaod.org
C.
drivers/clk/aspeed/clk_ast2500.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c index 9249cf9cdf..b3a3f3d4dd 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++ b/drivers/clk/aspeed/clk_ast2500.c @@ -490,7 +490,7 @@ struct clk_ops ast2500_clk_ops = { .enable = ast2500_clk_enable, };
-static int ast2500_clk_probe(struct udevice *dev) +static int ast2500_clk_ofdata_to_platdata(struct udevice *dev) { struct ast2500_clk_priv *priv = dev_get_priv(dev);
@@ -525,5 +525,5 @@ U_BOOT_DRIVER(aspeed_ast2500_scu) = { .priv_auto_alloc_size = sizeof(struct ast2500_clk_priv), .ops = &ast2500_clk_ops, .bind = ast2500_clk_bind,
.probe = ast2500_clk_probe,
.ofdata_to_platdata = ast2500_clk_ofdata_to_platdata,};
Regards, Simon
Hi Cédric,
On Tue, 7 Jan 2020 at 21:20, Cédric Le Goater clg@kaod.org wrote:
On 12/30/19 5:19 AM, Simon Glass wrote:
At present the clock driver reads its ofdata in the probe() method. This is not correct although it is often harmless.
However in this case it causes a problem, something like this:
- ast_get_scu() is called (from somewhere) to get the SCI address
- this probes the clock
- first sets up ofdata (which does nothing at present)
- DM marks clock device as active
- DM calls pinctrl
- pinctrl probes and calls ast_get_scu() in ast2500_pinctrl_probe()
- ast_get_scu() probes the clock, but sees it already marked as probed
- ast_get_scu() accesses the clock's private data, with scu as NULL
- DM calls clock probe function ast2500_clk_probe() which reads scu
By putting the read of scu into the correct method, scu is read as part of ofdata setup, and everything is OK.
Note: This problem did not matter until now since DM always probed all parents before reading a child's ofdata. The fact that pinctrl is a child of clock seems to trigger this strange bug.
Did you find it with QEMU ?
Yes that's right.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Cédric Le Goater clg@kaod.org
C.
drivers/clk/aspeed/clk_ast2500.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
Remove this duplicated code, since the 'fail' label does this immediately.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 4e037083a6..2442b5834d 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -431,10 +431,8 @@ int device_probe(struct udevice *dev)
if (drv->probe) { ret = drv->probe(dev); - if (ret) { - dev->flags &= ~DM_FLAG_ACTIVATED; + if (ret) goto fail; - } }
ret = uclass_post_probe_device(dev);
Remove this duplicated code, since the 'fail' label does this immediately.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Applied to u-boot-dm, thanks!
This method is supposed to extract platform data from the device tree. It should be done before the device itself is probed. Move it earlier in the device_probe() function.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 2442b5834d..45754ead8f 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -375,6 +375,13 @@ int device_probe(struct udevice *dev) return 0; }
+ if (drv->ofdata_to_platdata && + (CONFIG_IS_ENABLED(OF_PLATDATA) || dev_has_of_node(dev))) { + ret = drv->ofdata_to_platdata(dev); + if (ret) + goto fail; + } + seq = uclass_resolve_seq(dev); if (seq < 0) { ret = seq; @@ -411,13 +418,6 @@ int device_probe(struct udevice *dev) goto fail; }
- if (drv->ofdata_to_platdata && - (CONFIG_IS_ENABLED(OF_PLATDATA) || dev_has_of_node(dev))) { - ret = drv->ofdata_to_platdata(dev); - if (ret) - goto fail; - } - /* Only handle devices that have a valid ofnode */ if (dev_of_valid(dev)) { /*
This method is supposed to extract platform data from the device tree. It should be done before the device itself is probed. Move it earlier in the device_probe() function.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Applied to u-boot-dm, thanks!
At present the parent is probed before the child's ofdata_to_platdata() method is called. Adjust the logic slightly so that probing parents is not done until afterwards.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index 45754ead8f..f0c23053eb 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -346,7 +346,7 @@ int device_probe(struct udevice *dev) } }
- /* Ensure all parents are probed */ + /* Allocate parent data for this child */ if (dev->parent) { size = dev->parent->driver->per_child_auto_alloc_size; if (!size) { @@ -360,7 +360,17 @@ int device_probe(struct udevice *dev) goto fail; } } + } + + if (drv->ofdata_to_platdata && + (CONFIG_IS_ENABLED(OF_PLATDATA) || dev_has_of_node(dev))) { + ret = drv->ofdata_to_platdata(dev); + if (ret) + goto fail; + }
+ /* Ensure all parents are probed */ + if (dev->parent) { ret = device_probe(dev->parent); if (ret) goto fail; @@ -375,13 +385,6 @@ int device_probe(struct udevice *dev) return 0; }
- if (drv->ofdata_to_platdata && - (CONFIG_IS_ENABLED(OF_PLATDATA) || dev_has_of_node(dev))) { - ret = drv->ofdata_to_platdata(dev); - if (ret) - goto fail; - } - seq = uclass_resolve_seq(dev); if (seq < 0) { ret = seq;
At present the parent is probed before the child's ofdata_to_platdata() method is called. Adjust the logic slightly so that probing parents is not done until afterwards.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
Applied to u-boot-dm, thanks!
This flag is missing a comment. Add one.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/device.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/dm/device.h b/include/dm/device.h index d7ad9d6728..21774708e7 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -45,6 +45,7 @@ struct driver_info; /* Device name is allocated and should be freed on unbind() */ #define DM_FLAG_NAME_ALLOCED (1 << 7)
+/* Device has platform data provided by of-platdata */ #define DM_FLAG_OF_PLATDATA (1 << 8)
/*
This flag is missing a comment. Add one.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/device.h | 1 + 1 file changed, 1 insertion(+)
Applied to u-boot-dm, thanks!
Add a new internal function, device_ofdata_to_platdata() to handle allocating private space associated with each device and reading the platform data from the device tree.
Call this new function from device_probe().
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 29 +++++++++++++++++++++++++++-- include/dm/device-internal.h | 16 ++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c index f0c23053eb..9506c7df8d 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -311,12 +311,11 @@ static void *alloc_priv(int size, uint flags) return priv; }
-int device_probe(struct udevice *dev) +int device_ofdata_to_platdata(struct udevice *dev) { const struct driver *drv; int size = 0; int ret; - int seq;
if (!dev) return -EINVAL; @@ -369,6 +368,32 @@ int device_probe(struct udevice *dev) goto fail; }
+ return 0; +fail: + device_free(dev); + + return ret; +} + +int device_probe(struct udevice *dev) +{ + const struct driver *drv; + int ret; + int seq; + + if (!dev) + return -EINVAL; + + if (dev->flags & DM_FLAG_ACTIVATED) + return 0; + + drv = dev->driver; + assert(drv); + + ret = device_ofdata_to_platdata(dev); + if (ret) + goto fail; + /* Ensure all parents are probed */ if (dev->parent) { ret = device_probe(dev->parent); diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index ee2b24a62a..294d6c1810 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -83,6 +83,22 @@ int device_bind_with_driver_data(struct udevice *parent, int device_bind_by_name(struct udevice *parent, bool pre_reloc_only, const struct driver_info *info, struct udevice **devp);
+/** + * device_ofdata_to_platdata() - Read platform data for a device + * + * Read platform data for a device (typically from the device tree) so that + * the information needed to probe the device is present. + * + * This may cause some others devices to be probed if this one depends on them, + * e.g. a GPIO line will cause a GPIO device to be probed. + * + * All private data associated with the device is allocated. + * + * @dev: Pointer to device to process + * @return 0 if OK, -ve on error + */ +int device_ofdata_to_platdata(struct udevice *dev); + /** * device_probe() - Probe a device, activating it *
Add a new internal function, device_ofdata_to_platdata() to handle allocating private space associated with each device and reading the platform data from the device tree.
Call this new function from device_probe().
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device.c | 29 +++++++++++++++++++++++++++-- include/dm/device-internal.h | 16 ++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
We want to avoid allocating platform data twice. This could happen if device_probe() is called after device_ofdata_to_platdata() for the same device.
Add a flag to track whether device_ofdata_to_platdata() has been called on a device. Check the flag to make sure it doesn't happen twice, and clear the flag when the data is freed.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device-remove.c | 1 + drivers/core/device.c | 4 +++- include/dm/device.h | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/core/device-remove.c b/drivers/core/device-remove.c index 5c8dc4ad70..444e34b492 100644 --- a/drivers/core/device-remove.c +++ b/drivers/core/device-remove.c @@ -140,6 +140,7 @@ void device_free(struct udevice *dev) dev->parent_priv = NULL; } } + dev->flags &= ~DM_FLAG_PLATDATA_VALID;
devres_release_probe(dev); } diff --git a/drivers/core/device.c b/drivers/core/device.c index 9506c7df8d..9f39218423 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -320,7 +320,7 @@ int device_ofdata_to_platdata(struct udevice *dev) if (!dev) return -EINVAL;
- if (dev->flags & DM_FLAG_ACTIVATED) + if (dev->flags & DM_FLAG_PLATDATA_VALID) return 0;
drv = dev->driver; @@ -368,6 +368,8 @@ int device_ofdata_to_platdata(struct udevice *dev) goto fail; }
+ dev->flags |= DM_FLAG_PLATDATA_VALID; + return 0; fail: device_free(dev); diff --git a/include/dm/device.h b/include/dm/device.h index 21774708e7..9ebfd0a34e 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -65,6 +65,9 @@ struct driver_info; /* DM does not enable/disable the power domains corresponding to this device */ #define DM_FLAG_DEFAULT_PD_CTRL_OFF (1 << 11)
+/* Driver platdata has been read. Cleared when the device is removed */ +#define DM_FLAG_PLATDATA_VALID (1 << 12) + /* * 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
We want to avoid allocating platform data twice. This could happen if device_probe() is called after device_ofdata_to_platdata() for the same device.
Add a flag to track whether device_ofdata_to_platdata() has been called on a device. Check the flag to make sure it doesn't happen twice, and clear the flag when the data is freed.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/device-remove.c | 1 + drivers/core/device.c | 4 +++- include/dm/device.h | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!
At present these functions are lumped in with the core device functions. They have their own #ifdef to control their availability, so it seems better to split them out.
Move them into their own header file.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/device.h | 255 +---------------------------------------- include/dm/devres.h | 269 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 254 deletions(-) create mode 100644 include/dm/devres.h
diff --git a/include/dm/device.h b/include/dm/device.h index 9ebfd0a34e..1138a09149 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -720,260 +720,7 @@ static inline bool device_is_on_pci_bus(struct udevice *dev) */ int dm_scan_fdt_dev(struct udevice *dev);
-/* device resource management */ -typedef void (*dr_release_t)(struct udevice *dev, void *res); -typedef int (*dr_match_t)(struct udevice *dev, void *res, void *match_data); - -#ifdef CONFIG_DEVRES - -#ifdef CONFIG_DEBUG_DEVRES -void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp, - const char *name); -#define _devres_alloc(release, size, gfp) \ - __devres_alloc(release, size, gfp, #release) -#else -void *_devres_alloc(dr_release_t release, size_t size, gfp_t gfp); -#endif - -/** - * devres_alloc() - Allocate device resource data - * @release: Release function devres will be associated with - * @size: Allocation size - * @gfp: Allocation flags - * - * Allocate devres of @size bytes. The allocated area is associated - * with @release. The returned pointer can be passed to - * other devres_*() functions. - * - * RETURNS: - * Pointer to allocated devres on success, NULL on failure. - */ -#define devres_alloc(release, size, gfp) \ - _devres_alloc(release, size, gfp | __GFP_ZERO) - -/** - * devres_free() - Free device resource data - * @res: Pointer to devres data to free - * - * Free devres created with devres_alloc(). - */ -void devres_free(void *res); - -/** - * devres_add() - Register device resource - * @dev: Device to add resource to - * @res: Resource to register - * - * Register devres @res to @dev. @res should have been allocated - * using devres_alloc(). On driver detach, the associated release - * function will be invoked and devres will be freed automatically. - */ -void devres_add(struct udevice *dev, void *res); - -/** - * devres_find() - Find device resource - * @dev: Device to lookup resource from - * @release: Look for resources associated with this release function - * @match: Match function (optional) - * @match_data: Data for the match function - * - * Find the latest devres of @dev which is associated with @release - * and for which @match returns 1. If @match is NULL, it's considered - * to match all. - * - * @return pointer to found devres, NULL if not found. - */ -void *devres_find(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data); - -/** - * devres_get() - Find devres, if non-existent, add one atomically - * @dev: Device to lookup or add devres for - * @new_res: Pointer to new initialized devres to add if not found - * @match: Match function (optional) - * @match_data: Data for the match function - * - * Find the latest devres of @dev which has the same release function - * as @new_res and for which @match return 1. If found, @new_res is - * freed; otherwise, @new_res is added atomically. - * - * @return ointer to found or added devres. - */ -void *devres_get(struct udevice *dev, void *new_res, - dr_match_t match, void *match_data); - -/** - * devres_remove() - Find a device resource and remove it - * @dev: Device to find resource from - * @release: Look for resources associated with this release function - * @match: Match function (optional) - * @match_data: Data for the match function - * - * Find the latest devres of @dev associated with @release and for - * which @match returns 1. If @match is NULL, it's considered to - * match all. If found, the resource is removed atomically and - * returned. - * - * @return ointer to removed devres on success, NULL if not found. - */ -void *devres_remove(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data); - -/** - * devres_destroy() - Find a device resource and destroy it - * @dev: Device to find resource from - * @release: Look for resources associated with this release function - * @match: Match function (optional) - * @match_data: Data for the match function - * - * Find the latest devres of @dev associated with @release and for - * which @match returns 1. If @match is NULL, it's considered to - * match all. If found, the resource is removed atomically and freed. - * - * Note that the release function for the resource will not be called, - * only the devres-allocated data will be freed. The caller becomes - * responsible for freeing any other data. - * - * @return 0 if devres is found and freed, -ENOENT if not found. - */ -int devres_destroy(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data); - -/** - * devres_release() - Find a device resource and destroy it, calling release - * @dev: Device to find resource from - * @release: Look for resources associated with this release function - * @match: Match function (optional) - * @match_data: Data for the match function - * - * Find the latest devres of @dev associated with @release and for - * which @match returns 1. If @match is NULL, it's considered to - * match all. If found, the resource is removed atomically, the - * release function called and the resource freed. - * - * @return 0 if devres is found and freed, -ENOENT if not found. - */ -int devres_release(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data); - -/* managed devm_k.alloc/kfree for device drivers */ -/** - * devm_kmalloc() - Resource-managed kmalloc - * @dev: Device to allocate memory for - * @size: Allocation size - * @gfp: Allocation gfp flags - * - * Managed kmalloc. Memory allocated with this function is - * automatically freed on driver detach. Like all other devres - * resources, guaranteed alignment is unsigned long long. - * - * @return pointer to allocated memory on success, NULL on failure. - */ -void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp); -static inline void *devm_kzalloc(struct udevice *dev, size_t size, gfp_t gfp) -{ - return devm_kmalloc(dev, size, gfp | __GFP_ZERO); -} -static inline void *devm_kmalloc_array(struct udevice *dev, - size_t n, size_t size, gfp_t flags) -{ - if (size != 0 && n > SIZE_MAX / size) - return NULL; - return devm_kmalloc(dev, n * size, flags); -} -static inline void *devm_kcalloc(struct udevice *dev, - size_t n, size_t size, gfp_t flags) -{ - return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); -} - -/** - * devm_kfree() - Resource-managed kfree - * @dev: Device this memory belongs to - * @ptr: Memory to free - * - * Free memory allocated with devm_kmalloc(). - */ -void devm_kfree(struct udevice *dev, void *ptr); - -#else /* ! CONFIG_DEVRES */ - -static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp) -{ - return kzalloc(size, gfp); -} - -static inline void devres_free(void *res) -{ - kfree(res); -} - -static inline void devres_add(struct udevice *dev, void *res) -{ -} - -static inline void *devres_find(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data) -{ - return NULL; -} - -static inline void *devres_get(struct udevice *dev, void *new_res, - dr_match_t match, void *match_data) -{ - return NULL; -} - -static inline void *devres_remove(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data) -{ - return NULL; -} - -static inline int devres_destroy(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data) -{ - return 0; -} - -static inline int devres_release(struct udevice *dev, dr_release_t release, - dr_match_t match, void *match_data) -{ - return 0; -} - -static inline void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp) -{ - return kmalloc(size, gfp); -} - -static inline void *devm_kzalloc(struct udevice *dev, size_t size, gfp_t gfp) -{ - return kzalloc(size, gfp); -} - -static inline void *devm_kmalloc_array(struct udevice *dev, - size_t n, size_t size, gfp_t flags) -{ - /* TODO: add kmalloc_array() to linux/compat.h */ - if (size != 0 && n > SIZE_MAX / size) - return NULL; - return kmalloc(n * size, flags); -} - -static inline void *devm_kcalloc(struct udevice *dev, - size_t n, size_t size, gfp_t flags) -{ - /* TODO: add kcalloc() to linux/compat.h */ - return kmalloc(n * size, flags | __GFP_ZERO); -} - -static inline void devm_kfree(struct udevice *dev, void *ptr) -{ - kfree(ptr); -} - -#endif /* ! CONFIG_DEVRES */ +#include <dm/devres.h>
/* * REVISIT: diff --git a/include/dm/devres.h b/include/dm/devres.h new file mode 100644 index 0000000000..32fbf38054 --- /dev/null +++ b/include/dm/devres.h @@ -0,0 +1,269 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2015 Masahiro Yamada yamada.masahiro@socionext.com + * + * Based on the original work in Linux by + * Copyright (c) 2006 SUSE Linux Products GmbH + * Copyright (c) 2006 Tejun Heo teheo@suse.de + * Copyright 2019 Google LLC + */ + +#ifndef _DM_DEVRES_H +#define _DM_DEVRES_H + +/* device resource management */ +typedef void (*dr_release_t)(struct udevice *dev, void *res); +typedef int (*dr_match_t)(struct udevice *dev, void *res, void *match_data); + +#ifdef CONFIG_DEVRES + +#ifdef CONFIG_DEBUG_DEVRES +void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp, + const char *name); +#define _devres_alloc(release, size, gfp) \ + __devres_alloc(release, size, gfp, #release) +#else +void *_devres_alloc(dr_release_t release, size_t size, gfp_t gfp); +#endif + +/** + * devres_alloc() - Allocate device resource data + * @release: Release function devres will be associated with + * @size: Allocation size + * @gfp: Allocation flags + * + * Allocate devres of @size bytes. The allocated area is associated + * with @release. The returned pointer can be passed to + * other devres_*() functions. + * + * RETURNS: + * Pointer to allocated devres on success, NULL on failure. + */ +#define devres_alloc(release, size, gfp) \ + _devres_alloc(release, size, (gfp) | __GFP_ZERO) + +/** + * devres_free() - Free device resource data + * @res: Pointer to devres data to free + * + * Free devres created with devres_alloc(). + */ +void devres_free(void *res); + +/** + * devres_add() - Register device resource + * @dev: Device to add resource to + * @res: Resource to register + * + * Register devres @res to @dev. @res should have been allocated + * using devres_alloc(). On driver detach, the associated release + * function will be invoked and devres will be freed automatically. + */ +void devres_add(struct udevice *dev, void *res); + +/** + * devres_find() - Find device resource + * @dev: Device to lookup resource from + * @release: Look for resources associated with this release function + * @match: Match function (optional) + * @match_data: Data for the match function + * + * Find the latest devres of @dev which is associated with @release + * and for which @match returns 1. If @match is NULL, it's considered + * to match all. + * + * @return pointer to found devres, NULL if not found. + */ +void *devres_find(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data); + +/** + * devres_get() - Find devres, if non-existent, add one atomically + * @dev: Device to lookup or add devres for + * @new_res: Pointer to new initialized devres to add if not found + * @match: Match function (optional) + * @match_data: Data for the match function + * + * Find the latest devres of @dev which has the same release function + * as @new_res and for which @match return 1. If found, @new_res is + * freed; otherwise, @new_res is added atomically. + * + * @return ointer to found or added devres. + */ +void *devres_get(struct udevice *dev, void *new_res, + dr_match_t match, void *match_data); + +/** + * devres_remove() - Find a device resource and remove it + * @dev: Device to find resource from + * @release: Look for resources associated with this release function + * @match: Match function (optional) + * @match_data: Data for the match function + * + * Find the latest devres of @dev associated with @release and for + * which @match returns 1. If @match is NULL, it's considered to + * match all. If found, the resource is removed atomically and + * returned. + * + * @return ointer to removed devres on success, NULL if not found. + */ +void *devres_remove(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data); + +/** + * devres_destroy() - Find a device resource and destroy it + * @dev: Device to find resource from + * @release: Look for resources associated with this release function + * @match: Match function (optional) + * @match_data: Data for the match function + * + * Find the latest devres of @dev associated with @release and for + * which @match returns 1. If @match is NULL, it's considered to + * match all. If found, the resource is removed atomically and freed. + * + * Note that the release function for the resource will not be called, + * only the devres-allocated data will be freed. The caller becomes + * responsible for freeing any other data. + * + * @return 0 if devres is found and freed, -ENOENT if not found. + */ +int devres_destroy(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data); + +/** + * devres_release() - Find a device resource and destroy it, calling release + * @dev: Device to find resource from + * @release: Look for resources associated with this release function + * @match: Match function (optional) + * @match_data: Data for the match function + * + * Find the latest devres of @dev associated with @release and for + * which @match returns 1. If @match is NULL, it's considered to + * match all. If found, the resource is removed atomically, the + * release function called and the resource freed. + * + * @return 0 if devres is found and freed, -ENOENT if not found. + */ +int devres_release(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data); + +/* managed devm_k.alloc/kfree for device drivers */ +/** + * devm_kmalloc() - Resource-managed kmalloc + * @dev: Device to allocate memory for + * @size: Allocation size + * @gfp: Allocation gfp flags + * + * Managed kmalloc. Memory allocated with this function is + * automatically freed on driver detach. Like all other devres + * resources, guaranteed alignment is unsigned long long. + * + * @return pointer to allocated memory on success, NULL on failure. + */ +void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp); +static inline void *devm_kzalloc(struct udevice *dev, size_t size, gfp_t gfp) +{ + return devm_kmalloc(dev, size, gfp | __GFP_ZERO); +} + +static inline void *devm_kmalloc_array(struct udevice *dev, + size_t n, size_t size, gfp_t flags) +{ + if (size != 0 && n > SIZE_MAX / size) + return NULL; + return devm_kmalloc(dev, n * size, flags); +} + +static inline void *devm_kcalloc(struct udevice *dev, + size_t n, size_t size, gfp_t flags) +{ + return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); +} + +/** + * devm_kfree() - Resource-managed kfree + * @dev: Device this memory belongs to + * @ptr: Memory to free + * + * Free memory allocated with devm_kmalloc(). + */ +void devm_kfree(struct udevice *dev, void *ptr); + +#else /* ! CONFIG_DEVRES */ + +static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp) +{ + return kzalloc(size, gfp); +} + +static inline void devres_free(void *res) +{ + kfree(res); +} + +static inline void devres_add(struct udevice *dev, void *res) +{ +} + +static inline void *devres_find(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data) +{ + return NULL; +} + +static inline void *devres_get(struct udevice *dev, void *new_res, + dr_match_t match, void *match_data) +{ + return NULL; +} + +static inline void *devres_remove(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data) +{ + return NULL; +} + +static inline int devres_destroy(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data) +{ + return 0; +} + +static inline int devres_release(struct udevice *dev, dr_release_t release, + dr_match_t match, void *match_data) +{ + return 0; +} + +static inline void *devm_kmalloc(struct udevice *dev, size_t size, gfp_t gfp) +{ + return kmalloc(size, gfp); +} + +static inline void *devm_kzalloc(struct udevice *dev, size_t size, gfp_t gfp) +{ + return kzalloc(size, gfp); +} + +static inline void *devm_kmalloc_array(struct udevice *dev, + size_t n, size_t size, gfp_t flags) +{ + /* TODO: add kmalloc_array() to linux/compat.h */ + if (size != 0 && n > SIZE_MAX / size) + return NULL; + return kmalloc(n * size, flags); +} + +static inline void *devm_kcalloc(struct udevice *dev, + size_t n, size_t size, gfp_t flags) +{ + /* TODO: add kcalloc() to linux/compat.h */ + return kmalloc(n * size, flags | __GFP_ZERO); +} + +static inline void devm_kfree(struct udevice *dev, void *ptr) +{ + kfree(ptr); +} +#endif /* DEVRES */ +#endif /* _DM_DEVRES_H */
At present these functions are lumped in with the core device functions. They have their own #ifdef to control their availability, so it seems better to split them out.
Move them into their own header file.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/device.h | 255 +---------------------------------------- include/dm/devres.h | 269 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 254 deletions(-) create mode 100644 include/dm/devres.h
Applied to u-boot-dm, thanks!
The malloc() implementations provides a way of finding out the approximate amount of memory that is allocated. Add helper functions to make it easier to access this and see changes over time. This is useful for tests that want to check if memory has been allocated or freed.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/test/ut.h | 16 ++++++++++++++++ test/ut.c | 14 ++++++++++++++ 2 files changed, 30 insertions(+)
diff --git a/include/test/ut.h b/include/test/ut.h index fbfde10719..f616c202f3 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -149,4 +149,20 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line, /* Assert that an operation succeeds (returns 0) */ #define ut_assertok(cond) ut_asserteq(0, cond)
+/** + * ut_check_free() - Return the number of bytes free in the malloc() pool + * + * @return bytes free + */ +ulong ut_check_free(void); + +/** + * ut_check_delta() - Return the number of bytes allocated/freed + * + * @last: Last value from ut_check_free + * @return free memory delta from @last; positive means more memory has been + * allocated, negative means less has been allocated (i.e. some is freed) + */ +long ut_check_delta(ulong last); + #endif diff --git a/test/ut.c b/test/ut.c index 55798041ba..265da4a0d8 100644 --- a/test/ut.c +++ b/test/ut.c @@ -6,6 +6,7 @@ */
#include <common.h> +#include <malloc.h> #include <test/test.h> #include <test/ut.h>
@@ -32,3 +33,16 @@ void ut_failf(struct unit_test_state *uts, const char *fname, int line, putc('\n'); uts->fail_count++; } + +ulong ut_check_free(void) +{ + struct mallinfo info = mallinfo(); + + return info.uordblks; +} + +long ut_check_delta(ulong last) +{ + return ut_check_free() - last; +} +
The malloc() implementations provides a way of finding out the approximate amount of memory that is allocated. Add helper functions to make it easier to access this and see changes over time. This is useful for tests that want to check if memory has been allocated or freed.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/test/ut.h | 16 ++++++++++++++++ test/ut.c | 14 ++++++++++++++ 2 files changed, 30 insertions(+)
Applied to u-boot-dm, thanks!
At present when CONFIG_DEBUG_DEVRES is enabled, U-Boot prints log messages to the console with every devres allocation/free event. This causes most tests to fail since the console output is not as expected.
In particular this prevents us from adding a device to sandbox which uses devres in its bind method.
Move devres over to use U-Boot's logging feature instead, and add a new category for devres.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 6 ++++-- include/log.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/core/devres.c b/drivers/core/devres.c index a3f915dd73..9c04499c6d 100644 --- a/drivers/core/devres.c +++ b/drivers/core/devres.c @@ -7,6 +7,8 @@ * Copyright (c) 2006 Tejun Heo teheo@suse.de */
+#define LOG_CATEGORY LOGC_DEVRES + #include <common.h> #include <linux/compat.h> #include <linux/kernel.h> @@ -46,8 +48,8 @@ static void set_node_dbginfo(struct devres *dr, const char *name, size_t size) static void devres_log(struct udevice *dev, struct devres *dr, const char *op) { - printf("%s: DEVRES %3s %p %s (%lu bytes)\n", - dev->name, op, dr, dr->name, (unsigned long)dr->size); + log_debug("%s: DEVRES %3s %p %s (%lu bytes)\n", dev->name, op, dr, + dr->name, (unsigned long)dr->size); } #else /* CONFIG_DEBUG_DEVRES */ #define set_node_dbginfo(dr, n, s) do {} while (0) diff --git a/include/log.h b/include/log.h index c6f2f023b1..64b787d0ba 100644 --- a/include/log.h +++ b/include/log.h @@ -49,6 +49,7 @@ enum log_category_t { LOGC_ALLOC, /* Memory allocation */ LOGC_SANDBOX, /* Related to the sandbox board */ LOGC_BLOBLIST, /* Bloblist */ + LOGC_DEVRES, /* Device resources (devres_... functions) */
LOGC_COUNT, /* Number of log categories */ LOGC_END, /* Sentinel value for a list of log categories */
At present when CONFIG_DEBUG_DEVRES is enabled, U-Boot prints log messages to the console with every devres allocation/free event. This causes most tests to fail since the console output is not as expected.
In particular this prevents us from adding a device to sandbox which uses devres in its bind method.
Move devres over to use U-Boot's logging feature instead, and add a new category for devres.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 6 ++++-- include/log.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
Add a driver which does devres allocations so that we can write tests for devres.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/test.dts | 4 ++++ include/dm/uclass-id.h | 1 + include/test/test.h | 9 ++++++++ test/dm/test-fdt.c | 47 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 57513a449f..3aef40d5ca 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -201,6 +201,10 @@ compatible = "denx,u-boot-fdt-test1"; };
+ devres-test { + compatible = "denx,u-boot-devres-test"; + }; + clocks { clk_fixed: clk-fixed { compatible = "fixed-clock"; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index c1bab17ad1..c9f49df3f2 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -19,6 +19,7 @@ enum uclass_id { UCLASS_TEST_BUS, UCLASS_TEST_PROBE, UCLASS_TEST_DUMMY, + UCLASS_TEST_DEVRES, UCLASS_SPI_EMUL, /* sandbox SPI device emulator */ UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ UCLASS_I2C_EMUL_PARENT, /* parent for I2C device emulators */ diff --git a/include/test/test.h b/include/test/test.h index 98fbcd11f6..5977c59d3f 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -46,5 +46,14 @@ struct unit_test { .func = _name, \ }
+/* Sizes for devres tests */ +enum { + TEST_DEVRES_SIZE = 100, + TEST_DEVRES_COUNT = 10, + TEST_DEVRES_TOTAL = TEST_DEVRES_SIZE * TEST_DEVRES_COUNT, + + /* A different size */ + TEST_DEVRES_SIZE2 = 15, +};
#endif /* __TEST_TEST_H */ diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 1fb8b5c248..bbac37761d 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -153,6 +153,53 @@ UCLASS_DRIVER(testprobe) = { .flags = DM_UC_FLAG_SEQ_ALIAS, };
+struct dm_testdevres_pdata { + void *ptr; +}; + +struct dm_testdevres_priv { + void *ptr; +}; + +static int testdevres_drv_bind(struct udevice *dev) +{ + struct dm_testdevres_pdata *pdata = dev_get_platdata(dev); + + pdata->ptr = devm_kmalloc(dev, TEST_DEVRES_SIZE, 0); + + return 0; +} + +static int testdevres_drv_probe(struct udevice *dev) +{ + struct dm_testdevres_priv *priv = dev_get_priv(dev); + + priv->ptr = devm_kmalloc(dev, TEST_DEVRES_SIZE2, 0); + + return 0; +} + +static const struct udevice_id testdevres_ids[] = { + { .compatible = "denx,u-boot-devres-test" }, + { } +}; + +U_BOOT_DRIVER(testdevres_drv) = { + .name = "testdevres_drv", + .of_match = testdevres_ids, + .id = UCLASS_TEST_DEVRES, + .bind = testdevres_drv_bind, + .probe = testdevres_drv_probe, + .platdata_auto_alloc_size = sizeof(struct dm_testdevres_pdata), + .priv_auto_alloc_size = sizeof(struct dm_testdevres_priv), +}; + +UCLASS_DRIVER(testdevres) = { + .name = "testdevres", + .id = UCLASS_TEST_DEVRES, + .flags = DM_UC_FLAG_SEQ_ALIAS, +}; + int dm_check_devices(struct unit_test_state *uts, int num_devices) { struct udevice *dev;
Add a driver which does devres allocations so that we can write tests for devres.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/dts/test.dts | 4 ++++ include/dm/uclass-id.h | 1 + include/test/test.h | 9 ++++++++ test/dm/test-fdt.c | 47 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+)
Applied to u-boot-dm, thanks!
The devres functionality has very few users in U-Boot, but it still should have tests. Add a few basic tests of the main functions.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 13 +++ include/dm/devres.h | 20 +++++ test/dm/Makefile | 1 + test/dm/devres.c | 178 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 test/dm/devres.c
diff --git a/drivers/core/devres.c b/drivers/core/devres.c index 9c04499c6d..36a8b1eb5f 100644 --- a/drivers/core/devres.c +++ b/drivers/core/devres.c @@ -223,6 +223,19 @@ void dm_dump_devres(void) if (root) dump_resources(root, 0); } + +void devres_get_stats(const struct udevice *dev, struct devres_stats *stats) +{ + struct devres *dr; + + stats->allocs = 0; + stats->total_size = 0; + list_for_each_entry(dr, &dev->devres_head, entry) { + stats->allocs++; + stats->total_size += dr->size; + } +} + #endif
/* diff --git a/include/dm/devres.h b/include/dm/devres.h index 32fbf38054..9c69196054 100644 --- a/include/dm/devres.h +++ b/include/dm/devres.h @@ -15,6 +15,17 @@ typedef void (*dr_release_t)(struct udevice *dev, void *res); typedef int (*dr_match_t)(struct udevice *dev, void *res, void *match_data);
+/** + * struct devres_stats - Information about devres allocations for a device + * + * @allocs: Number of allocations + * @total_size: Total size of allocations in bytes + */ +struct devres_stats { + int allocs; + int total_size; +}; + #ifdef CONFIG_DEVRES
#ifdef CONFIG_DEBUG_DEVRES @@ -189,6 +200,9 @@ static inline void *devm_kcalloc(struct udevice *dev, */ void devm_kfree(struct udevice *dev, void *ptr);
+/* Get basic stats on allocations */ +void devres_get_stats(const struct udevice *dev, struct devres_stats *stats); + #else /* ! CONFIG_DEVRES */
static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp) @@ -265,5 +279,11 @@ static inline void devm_kfree(struct udevice *dev, void *ptr) { kfree(ptr); } + +static inline void devres_get_stats(const struct udevice *dev, + struct devres_stats *stats) +{ +} + #endif /* DEVRES */ #endif /* _DM_DEVRES_H */ diff --git a/test/dm/Makefile b/test/dm/Makefile index a268783169..85cc0f7fb8 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -18,6 +18,7 @@ obj-$(CONFIG_BLK) += blk.o obj-$(CONFIG_BOARD) += board.o obj-$(CONFIG_DM_BOOTCOUNT) += bootcount.o obj-$(CONFIG_CLK) += clk.o clk_ccf.o +obj-$(CONFIG_DEVRES) += devres.o obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o obj-$(CONFIG_DM_ETH) += eth.o obj-$(CONFIG_FIRMWARE) += firmware.o diff --git a/test/dm/devres.c b/test/dm/devres.c new file mode 100644 index 0000000000..c351844db9 --- /dev/null +++ b/test/dm/devres.c @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for the devres ( + * + * Copyright 2019 Google LLC + */ + +#include <common.h> +#include <errno.h> +#include <dm.h> +#include <malloc.h> +#include <dm/device-internal.h> +#include <dm/test.h> +#include <dm/uclass-internal.h> +#include <test/ut.h> + +/* Test that devm_kmalloc() allocates memory, free when device is removed */ +static int dm_test_devres_alloc(struct unit_test_state *uts) +{ + ulong mem_start, mem_dev, mem_kmalloc; + struct udevice *dev; + void *ptr; + + mem_start = ut_check_delta(0); + ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); + mem_dev = ut_check_delta(mem_start); + ut_assert(mem_dev > 0); + + /* This should increase allocated memory */ + ptr = devm_kmalloc(dev, TEST_DEVRES_SIZE, 0); + ut_assert(ptr != NULL); + mem_kmalloc = ut_check_delta(mem_dev); + ut_assert(mem_kmalloc > 0); + + /* Check that ptr is freed */ + device_remove(dev, DM_REMOVE_NORMAL); + ut_asserteq(0, ut_check_delta(mem_start)); + + return 0; +} +DM_TEST(dm_test_devres_alloc, DM_TESTF_SCAN_PDATA); + +/* Test devm_kfree() can be used to free memory too */ +static int dm_test_devres_free(struct unit_test_state *uts) +{ + ulong mem_start, mem_dev, mem_kmalloc; + struct udevice *dev; + void *ptr; + + mem_start = ut_check_delta(0); + ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); + mem_dev = ut_check_delta(mem_start); + ut_assert(mem_dev > 0); + + ptr = devm_kmalloc(dev, TEST_DEVRES_SIZE, 0); + ut_assert(ptr != NULL); + mem_kmalloc = ut_check_delta(mem_dev); + ut_assert(mem_kmalloc > 0); + + /* Free the ptr and check that memory usage goes down */ + devm_kfree(dev, ptr); + ut_assert(ut_check_delta(mem_kmalloc) < 0); + + device_remove(dev, DM_REMOVE_NORMAL); + ut_asserteq(0, ut_check_delta(mem_start)); + + return 0; +} +DM_TEST(dm_test_devres_free, DM_TESTF_SCAN_PDATA); + + +/* Test that kzalloc() returns memory that is zeroed */ +static int dm_test_devres_kzalloc(struct unit_test_state *uts) +{ + struct udevice *dev; + u8 *ptr, val; + int i; + + ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); + + ptr = devm_kzalloc(dev, TEST_DEVRES_SIZE, 0); + ut_assert(ptr != NULL); + for (val = 0, i = 0; i < TEST_DEVRES_SIZE; i++) + val |= *ptr; + ut_asserteq(0, val); + + return 0; +} +DM_TEST(dm_test_devres_kzalloc, DM_TESTF_SCAN_PDATA); + +/* Test that devm_kmalloc_array() allocates an array that can be set */ +static int dm_test_devres_kmalloc_array(struct unit_test_state *uts) +{ + ulong mem_start, mem_dev; + struct udevice *dev; + u8 *ptr; + + mem_start = ut_check_delta(0); + ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); + mem_dev = ut_check_delta(mem_start); + + ptr = devm_kmalloc_array(dev, TEST_DEVRES_COUNT, TEST_DEVRES_SIZE, 0); + ut_assert(ptr != NULL); + memset(ptr, '\xff', TEST_DEVRES_TOTAL); + ut_assert(ut_check_delta(mem_dev) > 0); + + device_remove(dev, DM_REMOVE_NORMAL); + ut_asserteq(0, ut_check_delta(mem_start)); + + return 0; +} +DM_TEST(dm_test_devres_kmalloc_array, DM_TESTF_SCAN_PDATA); + +/* Test that devm_kcalloc() allocates a zeroed array */ +static int dm_test_devres_kcalloc(struct unit_test_state *uts) +{ + ulong mem_start, mem_dev; + struct udevice *dev; + u8 *ptr, val; + int i; + + mem_start = ut_check_delta(0); + ut_assertok(uclass_first_device_err(UCLASS_TEST, &dev)); + mem_dev = ut_check_delta(mem_start); + ut_assert(mem_dev > 0); + + /* This should increase allocated memory */ + ptr = devm_kcalloc(dev, TEST_DEVRES_SIZE, TEST_DEVRES_COUNT, 0); + ut_assert(ptr != NULL); + ut_assert(ut_check_delta(mem_dev) > 0); + for (val = 0, i = 0; i < TEST_DEVRES_TOTAL; i++) + val |= *ptr; + ut_asserteq(0, val); + + /* Check that ptr is freed */ + device_remove(dev, DM_REMOVE_NORMAL); + ut_asserteq(0, ut_check_delta(mem_start)); + + return 0; +} +DM_TEST(dm_test_devres_kcalloc, DM_TESTF_SCAN_PDATA); + +static int dm_test_devres_phase(struct unit_test_state *uts) +{ + struct devres_stats stats; + struct udevice *dev; + + /* + * The device is bound already, so find it and check that it has the + * allocation created in the bind() method. + */ + ut_assertok(uclass_find_first_device(UCLASS_TEST_DEVRES, &dev)); + devres_get_stats(dev, &stats); + ut_asserteq(1, stats.allocs); + ut_asserteq(TEST_DEVRES_SIZE, stats.total_size); + + /* Probing the device should add one allocation */ + ut_assertok(uclass_first_device(UCLASS_TEST_DEVRES, &dev)); + ut_assert(dev != NULL); + devres_get_stats(dev, &stats); + ut_asserteq(2, stats.allocs); + ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2, stats.total_size); + + /* Removing the device should drop one allocation */ + device_remove(dev, DM_REMOVE_NORMAL); + devres_get_stats(dev, &stats); + ut_asserteq(1, stats.allocs); + ut_asserteq(TEST_DEVRES_SIZE, stats.total_size); + + /* Unbinding removes the other. Note this access a freed pointer */ + device_unbind(dev); + devres_get_stats(dev, &stats); + ut_asserteq(0, stats.allocs); + ut_asserteq(0, stats.total_size); + + return 0; +} +DM_TEST(dm_test_devres_phase, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
The devres functionality has very few users in U-Boot, but it still should have tests. Add a few basic tests of the main functions.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 13 +++ include/dm/devres.h | 20 +++++ test/dm/Makefile | 1 + test/dm/devres.c | 178 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 test/dm/devres.c
Applied to u-boot-dm, thanks!
At present we only support two phases where devres can be used: bind and probe. This is handled with a boolean. We want to add a new phase (platdata), so change this to an enum.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/core/devres.c b/drivers/core/devres.c index 36a8b1eb5f..5376118f12 100644 --- a/drivers/core/devres.c +++ b/drivers/core/devres.c @@ -17,12 +17,21 @@ #include <dm/root.h> #include <dm/util.h>
+/** enum devres_phase - Shows where resource was allocated + * + * DEVRES_PHASE_BIND: In the bind() method + * DEVRES_PHASE_PROBE: In the probe() method + */ +enum devres_phase { + DEVRES_PHASE_BIND, + DEVRES_PHASE_PROBE, +}; + /** * struct devres - Bookkeeping info for managed device resource * @entry: List to associate this structure with a device * @release: Callback invoked when this resource is released - * @probe: Flag to show when this resource was allocated - (true = probe, false = bind) + * @probe: Show where this resource was allocated * @name: Name of release function * @size: Size of resource data * @data: Resource data @@ -30,7 +39,7 @@ struct devres { struct list_head entry; dr_release_t release; - bool probe; + enum devres_phase phase; #ifdef CONFIG_DEBUG_DEVRES const char *name; size_t size; @@ -93,7 +102,8 @@ void devres_add(struct udevice *dev, void *res)
devres_log(dev, dr, "ADD"); assert_noisy(list_empty(&dr->entry)); - dr->probe = dev->flags & DM_FLAG_BOUND ? true : false; + dr->phase = dev->flags & DM_FLAG_BOUND ? DEVRES_PHASE_PROBE : + DEVRES_PHASE_BIND; list_add_tail(&dr->entry, &dev->devres_head); }
@@ -179,7 +189,7 @@ static void release_nodes(struct udevice *dev, struct list_head *head, struct devres *dr, *tmp;
list_for_each_entry_safe_reverse(dr, tmp, head, entry) { - if (probe_only && !dr->probe) + if (probe_only && dr->phase != DEVRES_PHASE_PROBE) break; devres_log(dev, dr, "REL"); dr->release(dev, dr->data); @@ -209,7 +219,7 @@ static void dump_resources(struct udevice *dev, int depth) list_for_each_entry(dr, &dev->devres_head, entry) printf(" %p (%lu byte) %s %s\n", dr, (unsigned long)dr->size, dr->name, - dr->probe ? "PROBE" : "BIND"); + dr->phase == DEVRES_PHASE_PROBE ? "PROBE" : "BIND");
list_for_each_entry(child, &dev->child_head, sibling_node) dump_resources(child, depth + 1);
At present we only support two phases where devres can be used: bind and probe. This is handled with a boolean. We want to add a new phase (platdata), so change this to an enum.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
Applied to u-boot-dm, thanks!
Since the ofdata_to_platdata() method can allocate resources, add it as a new devres phase.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 18 +++++++++++++----- include/test/test.h | 3 ++- test/dm/devres.c | 14 +++++++++++--- test/dm/test-fdt.c | 11 +++++++++++ 4 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/core/devres.c b/drivers/core/devres.c index 5376118f12..237b42653c 100644 --- a/drivers/core/devres.c +++ b/drivers/core/devres.c @@ -20,10 +20,12 @@ /** enum devres_phase - Shows where resource was allocated * * DEVRES_PHASE_BIND: In the bind() method + * DEVRES_PHASE_OFDATA: In the ofdata_to_platdata() method * DEVRES_PHASE_PROBE: In the probe() method */ enum devres_phase { DEVRES_PHASE_BIND, + DEVRES_PHASE_OFDATA, DEVRES_PHASE_PROBE, };
@@ -102,8 +104,12 @@ void devres_add(struct udevice *dev, void *res)
devres_log(dev, dr, "ADD"); assert_noisy(list_empty(&dr->entry)); - dr->phase = dev->flags & DM_FLAG_BOUND ? DEVRES_PHASE_PROBE : - DEVRES_PHASE_BIND; + if (dev->flags & DM_FLAG_PLATDATA_VALID) + dr->phase = DEVRES_PHASE_PROBE; + else if (dev->flags & DM_FLAG_BOUND) + dr->phase = DEVRES_PHASE_OFDATA; + else + dr->phase = DEVRES_PHASE_BIND; list_add_tail(&dr->entry, &dev->devres_head); }
@@ -184,12 +190,12 @@ int devres_release(struct udevice *dev, dr_release_t release, }
static void release_nodes(struct udevice *dev, struct list_head *head, - bool probe_only) + bool probe_and_ofdata_only) { struct devres *dr, *tmp;
list_for_each_entry_safe_reverse(dr, tmp, head, entry) { - if (probe_only && dr->phase != DEVRES_PHASE_PROBE) + if (probe_and_ofdata_only && dr->phase == DEVRES_PHASE_BIND) break; devres_log(dev, dr, "REL"); dr->release(dev, dr->data); @@ -209,6 +215,8 @@ void devres_release_all(struct udevice *dev) }
#ifdef CONFIG_DEBUG_DEVRES +static char *const devres_phase_name[] = {"BIND", "OFDATA", "PROBE"}; + static void dump_resources(struct udevice *dev, int depth) { struct devres *dr; @@ -219,7 +227,7 @@ static void dump_resources(struct udevice *dev, int depth) list_for_each_entry(dr, &dev->devres_head, entry) printf(" %p (%lu byte) %s %s\n", dr, (unsigned long)dr->size, dr->name, - dr->phase == DEVRES_PHASE_PROBE ? "PROBE" : "BIND"); + devres_phase_name[dr->phase]);
list_for_each_entry(child, &dev->child_head, sibling_node) dump_resources(child, depth + 1); diff --git a/include/test/test.h b/include/test/test.h index 5977c59d3f..e5bef4759a 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -52,8 +52,9 @@ enum { TEST_DEVRES_COUNT = 10, TEST_DEVRES_TOTAL = TEST_DEVRES_SIZE * TEST_DEVRES_COUNT,
- /* A different size */ + /* A few different sizes */ TEST_DEVRES_SIZE2 = 15, + TEST_DEVRES_SIZE3 = 37, };
#endif /* __TEST_TEST_H */ diff --git a/test/dm/devres.c b/test/dm/devres.c index c351844db9..e7331897de 100644 --- a/test/dm/devres.c +++ b/test/dm/devres.c @@ -140,6 +140,7 @@ static int dm_test_devres_kcalloc(struct unit_test_state *uts) } DM_TEST(dm_test_devres_kcalloc, DM_TESTF_SCAN_PDATA);
+/* Test devres releases resources automatically as expected */ static int dm_test_devres_phase(struct unit_test_state *uts) { struct devres_stats stats; @@ -154,14 +155,21 @@ static int dm_test_devres_phase(struct unit_test_state *uts) ut_asserteq(1, stats.allocs); ut_asserteq(TEST_DEVRES_SIZE, stats.total_size);
+ /* Getting platdata should add one allocation */ + ut_assertok(device_ofdata_to_platdata(dev)); + devres_get_stats(dev, &stats); + ut_asserteq(2, stats.allocs); + ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE3, stats.total_size); + /* Probing the device should add one allocation */ ut_assertok(uclass_first_device(UCLASS_TEST_DEVRES, &dev)); ut_assert(dev != NULL); devres_get_stats(dev, &stats); - ut_asserteq(2, stats.allocs); - ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2, stats.total_size); + ut_asserteq(3, stats.allocs); + ut_asserteq(TEST_DEVRES_SIZE + TEST_DEVRES_SIZE2 + TEST_DEVRES_SIZE3, + stats.total_size);
- /* Removing the device should drop one allocation */ + /* Removing the device should drop both those allocations */ device_remove(dev, DM_REMOVE_NORMAL); devres_get_stats(dev, &stats); ut_asserteq(1, stats.allocs); diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index bbac37761d..d59c449ce0 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -159,6 +159,7 @@ struct dm_testdevres_pdata {
struct dm_testdevres_priv { void *ptr; + void *ptr_ofdata; };
static int testdevres_drv_bind(struct udevice *dev) @@ -170,6 +171,15 @@ static int testdevres_drv_bind(struct udevice *dev) return 0; }
+static int testdevres_drv_ofdata_to_platdata(struct udevice *dev) +{ + struct dm_testdevres_priv *priv = dev_get_priv(dev); + + priv->ptr_ofdata = devm_kmalloc(dev, TEST_DEVRES_SIZE3, 0); + + return 0; +} + static int testdevres_drv_probe(struct udevice *dev) { struct dm_testdevres_priv *priv = dev_get_priv(dev); @@ -189,6 +199,7 @@ U_BOOT_DRIVER(testdevres_drv) = { .of_match = testdevres_ids, .id = UCLASS_TEST_DEVRES, .bind = testdevres_drv_bind, + .ofdata_to_platdata = testdevres_drv_ofdata_to_platdata, .probe = testdevres_drv_probe, .platdata_auto_alloc_size = sizeof(struct dm_testdevres_pdata), .priv_auto_alloc_size = sizeof(struct dm_testdevres_priv),
Since the ofdata_to_platdata() method can allocate resources, add it as a new devres phase.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/core/devres.c | 18 +++++++++++++----- include/test/test.h | 3 ++- test/dm/devres.c | 14 +++++++++++--- test/dm/test-fdt.c | 11 +++++++++++ 4 files changed, 37 insertions(+), 9 deletions(-)
Applied to u-boot-dm, thanks!
participants (3)
-
Cédric Le Goater -
Simon Glass -
sjg＠google.com