The pr_debug() functions do not response to setting the log level and in fact have their own separate log level. Use U-Boot logging instead.

Perhaps we should make these options redirect to log_debug(), etc.?

Signed-off-by: Simon Glass sjg@chromium.org ---

drivers/core/lists.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/core/lists.c b/drivers/core/lists.c index a1f828463ec..4681b3e5dd1 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -6,6 +6,8 @@ * Marek Vasut marex@denx.de */

+#define LOG_CATEGORY LOGC_DM + #include <common.h> #include <errno.h> #include <dm/device.h> @@ -139,13 +141,13 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, if (devp) *devp = NULL; name = ofnode_get_name(node); - pr_debug("bind node %s\n", name); + log_debug("bind node %s\n", name);

compat_list = ofnode_get_property(node, "compatible", &compat_length); if (!compat_list) { if (compat_length == -FDT_ERR_NOTFOUND) { - pr_debug("Device '%s' has no compatible string\n", - name); + log_debug("Device '%s' has no compatible string\n", + name); return 0; }

@@ -160,8 +162,8 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, */ for (i = 0; i < compat_length; i += strlen(compat) + 1) { compat = compat_list + i; - pr_debug(" - attempt to match compatible string '%s'\n", - compat); + log_debug(" - attempt to match compatible string '%s'\n", + compat);

for (entry = driver; entry != driver + n_ents; entry++) { ret = driver_check_compatible(entry->of_match, &id, @@ -178,11 +180,13 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, return 0; }

- pr_debug(" - found match at '%s'\n", entry->name); + log_debug(" - found match at '%s': '%s' matches '%s'\n", + entry->name, entry->of_match->compatible, + id->compatible); ret = device_bind_with_driver_data(parent, entry, name, id->data, node, &dev); if (ret == -ENODEV) { - pr_debug("Driver '%s' refuses to bind\n", entry->name); + log_debug("Driver '%s' refuses to bind\n", entry->name); continue; } if (ret) { @@ -198,7 +202,7 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, }

if (!found && !result && ret != -ENODEV) - pr_debug("No match for node '%s'\n", name); + log_debug("No match for node '%s'\n", name);

return result; }

Sometimes devices don't appear and it can be confusing. Add a few notes to help with this situation.

Signed-off-by: Simon Glass sjg@chromium.org ---

doc/driver-model/debugging.rst | 62 ++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 doc/driver-model/debugging.rst

diff --git a/doc/driver-model/debugging.rst b/doc/driver-model/debugging.rst new file mode 100644 index 00000000000..9711dd6d653 --- /dev/null +++ b/doc/driver-model/debugging.rst @@ -0,0 +1,62 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. sectionauthor:: Simon Glass sjg@chromium.org + +Debugging driver model +====================== + +This document aims to provide help when you cannot work out why driver model is +not doing what you expect. + + +Useful techniques in general +---------------------------- + +Here are some useful debugging features generally. + + - If you are writing a new feature, consider doing it in sandbox instead of + on your board. Sandbox has no limits, allows each debugging (e.g. gdb) and + you can writing emulators for most common devices. + - Put '#define DEBUG' at the top of a file, to activate all the debug() and + log_debug() statements in that file. + - Where logging is used, change the logging level, e.g. in SPL with + CONFIG_SPL_LOG_MAX_LEVEL=7 (which is LOGL_DEBUG) and + CONFIG_LOG_DEFAULT_LEVEL=7 + - Where logging of return values is implemented with log_msg_ret(), set + CONFIG_LOG_ERROR_RETURN=y to see exactly where the error is happening + - Make sure you have a debug UART enabled - see CONFIG_DEBUG_UART. With this + you can get serial output (printf(), etc.) before the serial driver is + running. + - Use a JTAG emulator to set breakpoints and single-step through code + +Not that most of these increase code/data size somewhat when enabled. + + +Failure to locate a device +-------------------------- + +Let's say you have uclass_first_device_err() and it is not finding anything. + +If it is returning an error, then that gives you a clue. Look up linux/errno.h +to see errors. Common ones are: + + - -ENOMEM which indicates that memory is short. If it happens in SPL or + before relocation in U-Boot, check CONFIG_SPL_SYS_MALLOC_F_LEN and + CONFIG_SYS_MALLOC_F_LEN as they may need to be larger. Add '#define DEBUG' + at the very top of malloc_simple.c to get an idea of where your memory is + going. + - -EINVAL which typically indicates that something was missing or wrong in + the device tree node. Check that everything is correct and look at the + ofdata_to_platdata() method in the driver. + +If there is no error, you should check if the device is actually bound. Call +dm_dump_all() just before you locate the device to make sure it exists. + +If it does not exist, check your device tree compatible strings match up with +what the driver expects (in the struct udevice_id array). + +If you are using of-platdata (e.g. CONFIG_SPL_OF_PLATDATA), check that the +driver name is the same as the first compatible string in the device tree (with +invalid-variable characters converted to underscore). + +If you are really stuck, #define DEBUG at the top of lists.c should show you +what is going on.

At present this code passes an fdt_addr_t pointer as a u64 pointer which is not save, since sizeof(fdt_addr_t) may be 4, e.g. with sandbox. Correct this to avoid a stack corruption problem.

Fixes: e679d03b08 (core: ofnode: Add ofnode_get_addr_size_index) Signed-off-by: Simon Glass sjg@chromium.org ---

drivers/core/ofnode.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 5d47eccf1d1..297f0a0c7cc 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -261,12 +261,15 @@ fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size)

if (ofnode_is_np(node)) { const __be32 *prop_val; + u64 size64; uint flags;

- prop_val = of_get_address(ofnode_to_np(node), index, - (u64 *)size, &flags); + prop_val = of_get_address(ofnode_to_np(node), index, &size64, + &flags); if (!prop_val) return FDT_ADDR_T_NONE; + if (size) + *size = size64;

ns = of_n_size_cells(ofnode_to_np(node));

At present tests that are marked as only for livetree fail when executed on sandbox_flattree. They cannot actually be executed, but we should not resport them as 'not found', since this causes errors. Instead, they should be silently skipped.

Signed-off-by: Simon Glass sjg@chromium.org ---

test/dm/test-main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 487d8b96275..74f77ccc849 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -130,7 +130,7 @@ static int dm_test_main(const char *test_name) const int n_ents = ll_entry_count(struct unit_test, dm_test); struct unit_test_state *uts = &global_dm_test_state; struct unit_test *test; - int run_count; + int found;

uts->priv = &_global_priv_dm_test_state; uts->fail_count = 0; @@ -148,7 +148,7 @@ static int dm_test_main(const char *test_name) if (!test_name) printf("Running %d driver model tests\n", n_ents);

- run_count = 0; + found = 0; #ifdef CONFIG_OF_LIVE uts->of_root = gd->of_root; #endif @@ -180,10 +180,10 @@ static int dm_test_main(const char *test_name) ut_assertok(dm_do_test(uts, test, false)); runs++; } - run_count += runs; + found++; }

- if (test_name && !run_count) + if (test_name && !found) printf("Test '%s' not found\n", test_name); else printf("Failures: %d\n", uts->fail_count);

This condition is not correct since it should not include an SPL_ prefix. Fix it and also add a note about the driver name in the same file.

Signed-off-by: Simon Glass sjg@chromium.org ---

doc/driver-model/of-plat.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst index a38e58e4d29..557957d2a16 100644 --- a/doc/driver-model/of-plat.rst +++ b/doc/driver-model/of-plat.rst @@ -269,7 +269,7 @@ For example: };

U_BOOT_DRIVER(mmc_drv) = { - .name = "mmc", + .name = "vendor_mmc", /* matches compatible string */ .id = UCLASS_MMC, .of_match = mmc_ids, .ofdata_to_platdata = mmc_ofdata_to_platdata,

We have a 'safe' version of this function but sometimes it is not needed. Add a normal version too and update a few places that can use it.

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/arm/mach-uniphier/pinctrl-glue.c | 4 ++-- drivers/block/blk-uclass.c | 4 ++-- include/dm/device.h | 9 +++++++++ 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-uniphier/pinctrl-glue.c b/arch/arm/mach-uniphier/pinctrl-glue.c index c4d3b17c39d..b45f72f59a7 100644 --- a/arch/arm/mach-uniphier/pinctrl-glue.c +++ b/arch/arm/mach-uniphier/pinctrl-glue.c @@ -13,14 +13,14 @@

int uniphier_pin_init(const char *pinconfig_name) { - struct udevice *pctldev, *config, *next; + struct udevice *pctldev, *config; int ret;

ret = uclass_first_device(UCLASS_PINCTRL, &pctldev); if (ret) return ret;

- device_foreach_child_safe(config, next, pctldev) { + device_foreach_child(config, pctldev) { if (strcmp(config->name, pinconfig_name)) continue;

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index baaf431e5e0..e8f58b3f5e4 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -142,9 +142,9 @@ struct blk_desc *blk_get_devnum_by_typename(const char *if_typename, int devnum) */ struct blk_desc *blk_get_by_device(struct udevice *dev) { - struct udevice *child_dev, *next; + struct udevice *child_dev;

- device_foreach_child_safe(child_dev, next, dev) { + device_foreach_child(child_dev, dev) { if (device_get_uclass_id(child_dev) != UCLASS_BLK) continue;

diff --git a/include/dm/device.h b/include/dm/device.h index 27a6d7b9fdb..d1210429e92 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -679,6 +679,15 @@ static inline bool device_is_on_pci_bus(struct udevice *dev) #define device_foreach_child_safe(pos, next, parent) \ list_for_each_entry_safe(pos, next, &parent->child_head, sibling_node)

+/** + * device_foreach_child() - iterate through child devices + * + * @pos: struct udevice * for the current device + * @parent: parent device to scan + */ +#define device_foreach_child(pos, parent) \ + list_for_each_entry(pos, &parent->child_head, sibling_node) + /** * dm_scan_fdt_dev() - Bind child device in a the device tree *

When device-tree compilation fails it is sometimes tricky to see which line is broken, since the input file to dtc is a pre-processed version of the device tree.

Add a line that points to the file that needs to be checked:

Output is something like this:

Error: arch/x86/dts/.chromebook_coral.dtb.pre.tmp:170.15-16 syntax error FATAL ERROR: Unable to parse input tree Check /tmp/b/chromebook_coral/arch/x86/dts/.chromebook_coral.dtb.pre.tmp for errors

Signed-off-by: Simon Glass sjg@chromium.org ---

scripts/Makefile.lib | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index ef116e0e0ae..c10cd83a0a3 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -300,7 +300,9 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \ - -d $(depfile).dtc.tmp $(dtc-tmp) ; \ + -d $(depfile).dtc.tmp $(dtc-tmp) || \ + (echo "Check $(shell pwd)/$(pre-tmp) for errors" && false) \ + ; \ cat $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) ; \ sed -i "s:$(pre-tmp):$(<):" $(depfile)

At present running sandbox with valgrind produces some warnings due to the MMC emulator not filling in all the expected fields. Fix it.

Signed-off-by: Simon Glass sjg@chromium.org ---

drivers/mmc/sandbox_mmc.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c index 2fa7d8c3dcd..7ca8da0946c 100644 --- a/drivers/mmc/sandbox_mmc.c +++ b/drivers/mmc/sandbox_mmc.c @@ -27,6 +27,7 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, { switch (cmd->cmdidx) { case MMC_CMD_ALL_SEND_CID: + memset(cmd->response, '\0', 16); break; case SD_CMD_SEND_RELATIVE_ADDR: cmd->response[0] = 0 << 16; /* mmc->rca */ @@ -43,11 +44,14 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, case MMC_CMD_SEND_CSD: cmd->response[0] = 0; cmd->response[1] = 10 << 16; /* 1 << block_len */ + cmd->response[2] = 0; + cmd->response[3] = 0; break; case SD_CMD_SWITCH_FUNC: { if (!data) break; u32 *resp = (u32 *)data->dest; + resp[3] = 0; resp[7] = cpu_to_be32(SD_HIGHSPEED_BUSY); if ((cmd->cmdarg & 0xF) == UHS_SDR12_BUS_SPEED) resp[4] = (cmd->cmdarg & 0xF) << 24;

These functions are available on x86 but not sandbox. They are useful shortcuts and clarify the code, so add them to sandbox.

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/sandbox/include/asm/io.h | 42 ++++++++++++++++++++++++++++++----- arch/sandbox/lib/pci_io.c | 12 +++++----- 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h index 2a350a826c4..481787b516b 100644 --- a/arch/sandbox/include/asm/io.h +++ b/arch/sandbox/include/asm/io.h @@ -110,13 +110,21 @@ phys_addr_t map_to_sysmem(const void *ptr); #define clrsetbits_8(addr, clear, set) clrsetbits(8, addr, clear, set)

/* I/O access functions */ -int inl(unsigned int addr); -int inw(unsigned int addr); -int inb(unsigned int addr); +int _inl(unsigned int addr); +int _inw(unsigned int addr); +int _inb(unsigned int addr);

-void outl(unsigned int value, unsigned int addr); -void outw(unsigned int value, unsigned int addr); -void outb(unsigned int value, unsigned int addr); +void _outl(unsigned int value, unsigned int addr); +void _outw(unsigned int value, unsigned int addr); +void _outb(unsigned int value, unsigned int addr); + +#define inb(port) _inb((uintptr_t)(port)) +#define inw(port) _inw((uintptr_t)(port)) +#define inl(port) _inl((uintptr_t)(port)) + +#define outb(val, port) _outb(val, (uintptr_t)(port)) +#define outw(val, port) _outw(val, (uintptr_t)(port)) +#define outl(val, port) _outl(val, (uintptr_t)(port))

#define out_arch(type,endian,a,v) write##type(cpu_to_##endian(v),a) #define in_arch(type,endian,a) endian##_to_cpu(read##type(a)) @@ -188,6 +196,28 @@ static inline void memcpy_toio(volatile void *dst, const void *src, int count) #define insw(port, buf, ns) _insw((u16 *)port, buf, ns) #define outsw(port, buf, ns) _outsw((u16 *)port, buf, ns)

+/* IO space accessors */ +#define clrio(type, addr, clear) \ + out##type(in##type(addr) & ~(clear), (addr)) + +#define setio(type, addr, set) \ + out##type(in##type(addr) | (set), (addr)) + +#define clrsetio(type, addr, clear, set) \ + out##type((in##type(addr) & ~(clear)) | (set), (addr)) + +#define clrio_32(addr, clear) clrio(l, addr, clear) +#define clrio_16(addr, clear) clrio(w, addr, clear) +#define clrio_8(addr, clear) clrio(b, addr, clear) + +#define setio_32(addr, set) setio(l, addr, set) +#define setio_16(addr, set) setio(w, addr, set) +#define setio_8(addr, set) setio(b, addr, set) + +#define clrsetio_32(addr, clear, set) clrsetio(l, addr, clear, set) +#define clrsetio_16(addr, clear, set) clrsetio(w, addr, clear, set) +#define clrsetio_8(addr, clear, set) clrsetio(b, addr, clear, set) + #include <iotrace.h> #include <asm/types.h>

diff --git a/arch/sandbox/lib/pci_io.c b/arch/sandbox/lib/pci_io.c index 01822c60695..f22e47c7f6d 100644 --- a/arch/sandbox/lib/pci_io.c +++ b/arch/sandbox/lib/pci_io.c @@ -91,7 +91,7 @@ static int pci_io_write(unsigned int addr, ulong value, pci_size_t size) return -ENOSYS; }

-int inl(unsigned int addr) +int _inl(unsigned int addr) { unsigned long value; int ret; @@ -101,7 +101,7 @@ int inl(unsigned int addr) return ret ? 0 : value; }

-int inw(unsigned int addr) +int _inw(unsigned int addr) { unsigned long value; int ret; @@ -111,7 +111,7 @@ int inw(unsigned int addr) return ret ? 0 : value; }

-int inb(unsigned int addr) +int _inb(unsigned int addr) { unsigned long value; int ret; @@ -121,17 +121,17 @@ int inb(unsigned int addr) return ret ? 0 : value; }

-void outl(unsigned int value, unsigned int addr) +void _outl(unsigned int value, unsigned int addr) { pci_io_write(addr, value, PCI_SIZE_32); }

-void outw(unsigned int value, unsigned int addr) +void _outw(unsigned int value, unsigned int addr) { pci_io_write(addr, value, PCI_SIZE_16); }

-void outb(unsigned int value, unsigned int addr) +void _outb(unsigned int value, unsigned int addr) { pci_io_write(addr, value, PCI_SIZE_8); }

This method is not used anymore since the bus/device/function of PCI devices can be obtained from their (parent's per-child) platform data. Drop it.

Signed-off-by: Simon Glass sjg@chromium.org ---

drivers/misc/swap_case.c | 8 -------- include/pci.h | 7 ------- 2 files changed, 15 deletions(-)

diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c index 8abf88a8fff..18d756e9cd5 100644 --- a/drivers/misc/swap_case.c +++ b/drivers/misc/swap_case.c @@ -54,13 +54,6 @@ struct swap_case_priv { char mem_text[MEM_TEXT_SIZE]; };

-static int sandbox_swap_case_get_devfn(struct udevice *dev) -{ - struct pci_child_platdata *plat = dev_get_parent_platdata(dev); - - return plat->devfn; -} - static int sandbox_swap_case_use_ea(struct udevice *dev) { return !!ofnode_get_property(dev->node, "use-ea", NULL); @@ -393,7 +386,6 @@ static int sandbox_swap_case_unmap_physmem(struct udevice *dev, }

static struct dm_pci_emul_ops sandbox_swap_case_emul_ops = { - .get_devfn = sandbox_swap_case_get_devfn, .read_config = sandbox_swap_case_read_config, .write_config = sandbox_swap_case_write_config, .read_io = sandbox_swap_case_read_io, diff --git a/include/pci.h b/include/pci.h index 298d0d43559..999a594cddf 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1490,13 +1490,6 @@ int dm_pci_find_class(uint find_class, int index, struct udevice **devp); * struct dm_pci_emul_ops - PCI device emulator operations */ struct dm_pci_emul_ops { - /** - * get_devfn(): Check which device and function this emulators - * - * @dev: device to check - * @return the device and function this emulates, or -ve on error - */ - int (*get_devfn)(struct udevice *dev); /** * read_config() - Read a PCI configuration value *

U-Boot already supports using -D to indicate that it should use the normal device tree. It is sometimes useful to run with the test device tree, e.g. when running a test. Add a -T option for this along with some documentation.

It can be used like this:

/tmp/b/sandbox/u-boot -T -c "ut dm pci_busdev"

(this will use /tmp/b/sandbox/arch/sandbox/dts/test.dtb as the DT)

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/sandbox/cpu/start.c | 25 +++++++++++++++++++++++++ doc/arch/sandbox.rst | 9 +++++++++ 2 files changed, 34 insertions(+)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 82828f0c1d4..cfc542c8066 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -147,6 +147,31 @@ static int sandbox_cmdline_cb_default_fdt(struct sandbox_state *state, SANDBOX_CMDLINE_OPT_SHORT(default_fdt, 'D', 0, "Use the default u-boot.dtb control FDT in U-Boot directory");

+static int sandbox_cmdline_cb_test_fdt(struct sandbox_state *state, + const char *arg) +{ + const char *fmt = "/arch/sandbox/dts/test.dtb"; + char *p; + char *fname; + int len; + + len = strlen(state->argv[0]) + strlen(fmt) + 1; + fname = os_malloc(len); + if (!fname) + return -ENOMEM; + strcpy(fname, state->argv[0]); + p = strrchr(fname, '/'); + if (!p) + p = fname + strlen(fname); + len -= p - fname; + snprintf(p, len, fmt, p); + state->fdt_fname = fname; + + return 0; +} +SANDBOX_CMDLINE_OPT_SHORT(test_fdt, 'T', 0, + "Use the test.dtb control FDT in U-Boot directory"); + static int sandbox_cmdline_cb_interactive(struct sandbox_state *state, const char *arg) { diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index 5c0caebcbf0..54933b56759 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -103,6 +103,8 @@ A device tree binary file can be provided with -d. If you edit the source (it is stored at arch/sandbox/dts/sandbox.dts) you must rebuild U-Boot to recreate the binary file.

+To use the default device tree, use -D. To use the test device tree, use -T. + To execute commands directly, use the -c option. You can specify a single command, or multiple commands separated by a semicolon, as is normal in U-Boot. Be careful with quoting as the shell will normally process and @@ -499,6 +501,13 @@ run natively on your board if desired (and enabled).

To run all tests use "make check".

+To run a single test in an existing sandbox build, you can use -T to use the +test device tree, and -c to select the test: + + /tmp/b/sandbox/u-boot -T -c "ut dm pci_busdev" + +This runs dm_test_pci_busdev() which is in test/dm/pci.c +

Memory Map ----------

At present these functions are stubbed out. For more comprehensive testing with PCI devices it is useful to be able to fully emulate I/O access. Add simple implementations for these.

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/sandbox/cpu/cpu.c | 51 ++++++++++++++++++++++++++++++++ arch/sandbox/include/asm/io.h | 27 +++++++++++------ arch/sandbox/include/asm/state.h | 1 + drivers/nvme/nvme.h | 4 +-- 4 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index fdfb209f77d..f3af88d79e9 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -225,6 +225,57 @@ phys_addr_t map_to_sysmem(const void *ptr) return mentry->tag; }

+unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size) +{ + struct sandbox_state *state = state_get_current(); + + if (!state->allow_memio) + return 0; + + switch (size) { + case SB_SIZE_8: + return *(u8 *)addr; + case SB_SIZE_16: + return *(u16 *)addr; + case SB_SIZE_32: + return *(u32 *)addr; + case SB_SIZE_64: + return *(u64 *)addr; + } + + return 0; +} + +void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size) +{ + struct sandbox_state *state = state_get_current(); + + if (!state->allow_memio) + return; + + switch (size) { + case SB_SIZE_8: + *(u8 *)addr = val; + break; + case SB_SIZE_16: + *(u16 *)addr = val; + break; + case SB_SIZE_32: + *(u32 *)addr = val; + break; + case SB_SIZE_64: + *(u64 *)addr = val; + break; + } +} + +void sandbox_set_enable_memio(bool enable) +{ + struct sandbox_state *state = state_get_current(); + + state->allow_memio = enable; +} + void sandbox_set_enable_pci_map(int enable) { enable_pci_map = enable; diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h index 481787b516b..2bbaff26a61 100644 --- a/arch/sandbox/include/asm/io.h +++ b/arch/sandbox/include/asm/io.h @@ -6,6 +6,13 @@ #ifndef __SANDBOX_ASM_IO_H #define __SANDBOX_ASM_IO_H

+enum sandboxio_size_t { + SB_SIZE_8, + SB_SIZE_16, + SB_SIZE_32, + SB_SIZE_64, +}; + void *phys_to_virt(phys_addr_t paddr); #define phys_to_virt phys_to_virt

@@ -38,18 +45,20 @@ static inline void unmap_sysmem(const void *vaddr) /* Map from a pointer to our RAM buffer */ phys_addr_t map_to_sysmem(const void *ptr);

-/* Define nops for sandbox I/O access */ -#define readb(addr) ((void)addr, 0) -#define readw(addr) ((void)addr, 0) -#define readl(addr) ((void)addr, 0) +unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size); +void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size); + +#define readb(addr) sandbox_read(addr, SB_SIZE_8) +#define readw(addr) sandbox_read(addr, SB_SIZE_16) +#define readl(addr) sandbox_read(addr, SB_SIZE_32) #ifdef CONFIG_SANDBOX64 -#define readq(addr) ((void)addr, 0) +#define readq(addr) sandbox_read(addr, SB_SIZE_64) #endif -#define writeb(v, addr) ((void)addr) -#define writew(v, addr) ((void)addr) -#define writel(v, addr) ((void)addr) +#define writeb(v, addr) sandbox_write(addr, v, SB_SIZE_8) +#define writew(v, addr) sandbox_write(addr, v, SB_SIZE_16) +#define writel(v, addr) sandbox_write(addr, v, SB_SIZE_32) #ifdef CONFIG_SANDBOX64 -#define writeq(v, addr) ((void)addr) +#define writeq(v, addr) sandbox_write(addr, v, SB_SIZE_64) #endif

/* diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 2d773d3fa6b..ad3e94beb9a 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -102,6 +102,7 @@ struct sandbox_state { ulong next_tag; /* Next address tag to allocate */ struct list_head mapmem_head; /* struct sandbox_mapmem_entry */ bool hwspinlock; /* Hardware Spinlock status */ + bool allow_memio; /* Allow readl() etc. to work */

/* * This struct is getting large. diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h index 922f7abfe85..c56ea7e58e6 100644 --- a/drivers/nvme/nvme.h +++ b/drivers/nvme/nvme.h @@ -536,7 +536,7 @@ struct nvme_completion { static inline u64 nvme_readq(__le64 volatile *regs) { #if BITS_PER_LONG == 64 - return readq(regs); + return readq((__le64 *)regs); #else __u32 *ptr = (__u32 *)regs; u64 val_lo = readl(ptr); @@ -549,7 +549,7 @@ static inline u64 nvme_readq(__le64 volatile *regs) static inline void nvme_writeq(const u64 val, __le64 volatile *regs) { #if BITS_PER_LONG == 64 - writeq(val, regs); + writeq(val, (__le64 *)regs); #else __u32 *ptr = (__u32 *)regs; u32 val_lo = lower_32_bits(val);

Sandbox i2c works using emulation drivers which are currently children of the i2c device:

pci-controller { pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; emul@1f,0 { compatible = "sandbox,swap-case"; }; }; };

In this case the emulation device is attached to pci device on address f800 (device 1f, function 0) and provides the swap-case functionality.

However this is not ideal, since every device on a PCI bus has a child device. This is only really the case for sandbox, but we want to avoid special-case code for sandbox.

Worse, child devices cannot be probed before their parents. This forces us to use 'find' rather than 'get' to obtain the emulator device. In fact the emulator devices are never probed. There is code in sandbox_pci_emul_post_probe() which tries to track when emulators are active, but at present this does not work.

A better approach seems to be to add a separate node elsewhere in the device tree, an 'emulation parent'. This could be given a bogus address (such as -1) to hide the emulators away from the 'pci' command, but it seems better to keep it at the root node to avoid such hacks.

Then we can use a phandle to point from the evice to the correct emulator, and only on sandbox. The code to find an emulator does not interfere with normal pci operation.

Add a new UCLASS_PCI_EMUL_PARENT uclass which allows finding an emulator given a bus, and finding a bus given an emulator. Update the existing device trees and the code for finding an emulator.

This brings PCI emulators more into line with I2C.

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/sandbox/dts/sandbox.dtsi | 11 +++++++--- arch/sandbox/dts/test.dts | 38 +++++++++++++++++++++++------------ doc/driver-model/pci-info.rst | 23 +++++++++++---------- drivers/pci/pci-emul-uclass.c | 37 ++++++++++++++++++++++++++++------ include/dm/uclass-id.h | 1 + 5 files changed, 77 insertions(+), 33 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index c6d5650c20b..f09bc70b0da 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -103,9 +103,14 @@ pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul>; + }; + }; + + emul { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul: emul@1f,0 { + compatible = "sandbox,swap-case"; }; };

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 5669ede7051..a2e75981f0b 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -452,24 +452,31 @@ pci@0,0 { compatible = "pci-generic"; reg = <0x0000 0 0 0 0>; - emul@0,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul0>; }; pci@1,0 { compatible = "pci-generic"; reg = <0x0800 0 0 0 0>; - emul@0,0 { - compatible = "sandbox,swap-case"; - use-ea; - }; + sandbox,emul = <&swap_case_emul1>; }; pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul1f>; + }; + }; + + pci-emul0 { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul0: emul@0,0 { + compatible = "sandbox,swap-case"; + }; + swap_case_emul1: emul@1,0 { + compatible = "sandbox,swap-case"; + use-ea; + }; + swap_case_emul1f: emul@1f,0 { + compatible = "sandbox,swap-case"; }; };

@@ -499,9 +506,14 @@ pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul2_1f>; + }; + }; + + pci-emul2 { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul2_1f: emul2@1f,0 { + compatible = "sandbox,swap-case"; }; };

diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst index d93ab8b61d5..f39ff990a67 100644 --- a/doc/driver-model/pci-info.rst +++ b/doc/driver-model/pci-info.rst @@ -113,14 +113,17 @@ Sandbox -------

With sandbox we need a device emulator for each device on the bus since there -is no real PCI bus. This works by looking in the device tree node for a -driver. For example:: - +is no real PCI bus. This works by looking in the device tree node for an +emulator driver. For example::

pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { + sandbox,emul = <&emul_1f>; + }; + pci-emul { + compatible = "sandbox,pci-emul-parent"; + emul_1f: emul@1f,0 { compatible = "sandbox,swap-case"; }; }; @@ -130,14 +133,16 @@ Note that the first cell in the 'reg' value is the bus/device/function. See PCI_BDF() for the encoding (it is also specified in the IEEE Std 1275-1994 PCI bus binding document, v2.1)

+The pci-emul node should go outside the pci bus node, since otherwise it will +be scanned as a PCI device, causing confusion. + When this bus is scanned we will end up with something like this::

`- * pci-controller @ 05c660c8, 0 `- pci@1f,0 @ 05c661c8, 63488 - `- emul@1f,0 @ 05c662c8 + `- emul@1f,0 @ 05c662c8

-When accesses go to the pci@1f,0 device they are forwarded to its child, the -emulator. +When accesses go to the pci@1f,0 device they are forwarded to its emulator.

The sandbox PCI drivers also support dynamic driver binding, allowing device driver to declare the driver binding information via U_BOOT_PCI_DEVICE(), @@ -164,7 +169,3 @@ When this bus is scanned we will end up with something like this:: pci [ + ] pci_sandbo |-- pci-controller1 pci_emul [ ] sandbox_sw | |-- sandbox_swap_case_emul pci_emul [ ] sandbox_sw | `-- sandbox_swap_case_emul - -Note the difference from the statically declared device nodes is that the -device is directly attached to the host controller, instead of via a container -device like pci@1f,0. diff --git a/drivers/pci/pci-emul-uclass.c b/drivers/pci/pci-emul-uclass.c index 38227583547..f918e9a52fd 100644 --- a/drivers/pci/pci-emul-uclass.c +++ b/drivers/pci/pci-emul-uclass.c @@ -10,6 +10,7 @@ #include <linux/libfdt.h> #include <pci.h> #include <dm/lists.h> +#include <dm/uclass-internal.h>

struct sandbox_pci_emul_priv { int dev_count; @@ -30,13 +31,15 @@ int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn, } *containerp = dev;

- if (device_get_uclass_id(dev) == UCLASS_PCI_GENERIC) { - ret = device_find_first_child(dev, emulp); - if (ret) - return ret; - } else { + /* + * TODO(sjg@chromium.org): This code needs a comment as I'm not sure + * why UCLASS_PCI_GENERIC devices end up being their own emulators. I + * left this code as is. + */ + ret = uclass_find_device_by_phandle(UCLASS_PCI_EMUL, dev, + "sandbox,emul", emulp); + if (ret && device_get_uclass_id(dev) != UCLASS_PCI_GENERIC) *emulp = dev; - }

return *emulp ? 0 : -ENODEV; } @@ -68,3 +71,25 @@ UCLASS_DRIVER(pci_emul) = { .pre_remove = sandbox_pci_emul_pre_remove, .priv_auto_alloc_size = sizeof(struct sandbox_pci_emul_priv), }; + +/* + * This uclass is a child of the pci bus. Its platdata is not defined here so + * is defined by its parent, UCLASS_PCI, which uses struct pci_child_platdata. + * See per_child_platdata_auto_alloc_size in UCLASS_DRIVER(pci). + */ +UCLASS_DRIVER(pci_emul_parent) = { + .id = UCLASS_PCI_EMUL_PARENT, + .name = "pci_emul_parent", + .post_bind = dm_scan_fdt_dev, +}; + +static const struct udevice_id pci_emul_parent_ids[] = { + { .compatible = "sandbox,pci-emul-parent" }, + { } +}; + +U_BOOT_DRIVER(pci_emul_parent_drv) = { + .name = "pci_emul_parent_drv", + .id = UCLASS_PCI_EMUL_PARENT, + .of_match = pci_emul_parent_ids, +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d4d96106b37..f431f3bf294 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -23,6 +23,7 @@ enum uclass_id { UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ UCLASS_I2C_EMUL_PARENT, /* parent for I2C device emulators */ UCLASS_PCI_EMUL, /* sandbox PCI device emulator */ + UCLASS_PCI_EMUL_PARENT, /* parent for PCI device emulators */ UCLASS_USB_EMUL, /* sandbox USB bus device emulator */ UCLASS_AXI_EMUL, /* sandbox AXI bus device emulator */

Update the debugging info a little to show the result of trying to bind a PCI device.

Signed-off-by: Simon Glass sjg@chromium.org ---

drivers/pci/pci-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index ab3e1310eb5..4c84c656856 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -790,7 +790,7 @@ int pci_bind_bus_devices(struct udevice *bus) if (!PCI_FUNC(bdf)) found_multi = header_type & 0x80;

- debug("%s: bus %d/%s: found device %x, function %d\n", __func__, + debug("%s: bus %d/%s: found device %x, function %d", __func__, bus->seq, bus->name, PCI_DEV(bdf), PCI_FUNC(bdf)); pci_bus_read_config(bus, bdf, PCI_DEVICE_ID, &device, PCI_SIZE_16); @@ -800,6 +800,7 @@ int pci_bind_bus_devices(struct udevice *bus)

/* Find this device in the device tree */ ret = pci_bus_find_devfn(bus, PCI_MASK_BUS(bdf), &dev); + debug(": find ret=%d\n", ret);

/* If nothing in the device tree, bind a device */ if (ret == -ENODEV) {

Fix these spelling errors the header file and documentation.

Fix a small typo in the PCI documentation.

Signed-off-by: Simon Glass sjg@chromium.org ---

doc/driver-model/pci-info.rst | 2 +- include/pci.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst index f39ff990a67..3c1b1adf077 100644 --- a/doc/driver-model/pci-info.rst +++ b/doc/driver-model/pci-info.rst @@ -103,7 +103,7 @@ in each of these nodes.

If PCI devices are not listed in the device tree, U_BOOT_PCI_DEVICE can be used to specify the driver to use for the device. The device tree takes precedence -over U_BOOT_PCI_DEVICE. Plese note with U_BOOT_PCI_DEVICE, only drivers with +over U_BOOT_PCI_DEVICE. Please note with U_BOOT_PCI_DEVICE, only drivers with DM_FLAG_PRE_RELOC will be bound before relocation. If neither device tree nor U_BOOT_PCI_DEVICE is provided, the built-in driver (either pci_bridge_drv or pci_generic_drv) will be used. diff --git a/include/pci.h b/include/pci.h index 2b82b2c5a3e..8aa6636cfbf 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1595,7 +1595,7 @@ int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn, /** * pci_get_devfn() - Extract the devfn from fdt_pci_addr of the device * - * Get devfn from fdt_pci_addr of the specifified device + * Get devfn from fdt_pci_addr of the specified device * * @dev: PCI device * @return devfn in bits 15...8 if found, -ENODEV if not found

At present PCI address transaction is not supported so drivers must manually read the correct BAR after reading the device tree info. The ns16550 has a suitable implementation, so move this code into the core DM support.

Note that there is no live-tree equivalent at present.

Signed-off-by: Simon Glass sjg@chromium.org ---

arch/sandbox/dts/test.dts | 7 ++++-- drivers/core/fdtaddr.c | 33 ++++++++++++++++++++++++++++ drivers/core/read.c | 11 ++++++++++ drivers/serial/ns16550.c | 31 +-------------------------- include/dm/fdtaddr.h | 8 +++++++ include/dm/read.h | 25 ++++++++++++++++++++++ test/dm/pci.c | 45 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 128 insertions(+), 32 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index a2e75981f0b..8733e0d7e19 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -456,12 +456,15 @@ }; pci@1,0 { compatible = "pci-generic"; - reg = <0x0800 0 0 0 0>; + /* BAR0 is at 0x14, using FDT_PCI_SPACE_MEM32 */ + reg = <0x02000814 0 0 0 0 + 0x0100f810 0 0 0 0>; sandbox,emul = <&swap_case_emul1>; }; pci@1f,0 { compatible = "pci-generic"; - reg = <0xf800 0 0 0 0>; + /* BAR1 is at 0x10, using FDT_PCI_SPACE_IO */ + reg = <0x0100f810 0 0 0 0>; sandbox,emul = <&swap_case_emul1f>; }; }; diff --git a/drivers/core/fdtaddr.c b/drivers/core/fdtaddr.c index 6850003a287..c9a941116a3 100644 --- a/drivers/core/fdtaddr.c +++ b/drivers/core/fdtaddr.c @@ -190,3 +190,36 @@ void *devfdt_map_physmem(struct udevice *dev, unsigned long size)

return map_physmem(addr, size, MAP_NOCACHE); } + +fdt_addr_t devfdt_get_addr_pci(struct udevice *dev) +{ + ulong addr; + + addr = devfdt_get_addr(dev); + if (CONFIG_IS_ENABLED(PCI) && IS_ENABLED(CONFIG_DM_PCI) && + addr == FDT_ADDR_T_NONE) { + struct fdt_pci_addr pci_addr; + u32 bar; + int ret; + + ret = fdtdec_get_pci_addr(gd->fdt_blob, + dev_of_offset(dev), + FDT_PCI_SPACE_MEM32, "reg", + &pci_addr); + if (ret) { + /* try if there is any i/o-mapped register */ + ret = fdtdec_get_pci_addr(gd->fdt_blob, + dev_of_offset(dev), + FDT_PCI_SPACE_IO, "reg", + &pci_addr); + if (ret) + return FDT_ADDR_T_NONE; + } + ret = fdtdec_get_pci_bar32(dev, &pci_addr, &bar); + if (ret) + return FDT_ADDR_T_NONE; + addr = bar; + } + + return addr; +} diff --git a/drivers/core/read.c b/drivers/core/read.c index fb3dcd9a790..9602e52d1b1 100644 --- a/drivers/core/read.c +++ b/drivers/core/read.c @@ -307,3 +307,14 @@ int dev_read_alias_highest_id(const char *stem)

return fdtdec_get_alias_highest_id(gd->fdt_blob, stem); } + +fdt_addr_t dev_read_addr_pci(struct udevice *dev) +{ + ulong addr; + + addr = dev_read_addr(dev); + if (addr == FDT_ADDR_T_NONE && !of_live_active()) + addr = devfdt_get_addr_pci(dev); + + return addr; +} diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 01f334938ea..754b6e99215 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -440,36 +440,7 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev) int err;

/* try Processor Local Bus device first */ - addr = dev_read_addr(dev); -#if CONFIG_IS_ENABLED(PCI) && defined(CONFIG_DM_PCI) - if (addr == FDT_ADDR_T_NONE) { - /* then try pci device */ - struct fdt_pci_addr pci_addr; - u32 bar; - int ret; - - /* we prefer to use a memory-mapped register */ - ret = fdtdec_get_pci_addr(gd->fdt_blob, dev_of_offset(dev), - FDT_PCI_SPACE_MEM32, "reg", - &pci_addr); - if (ret) { - /* try if there is any i/o-mapped register */ - ret = fdtdec_get_pci_addr(gd->fdt_blob, - dev_of_offset(dev), - FDT_PCI_SPACE_IO, - "reg", &pci_addr); - if (ret) - return ret; - } - - ret = fdtdec_get_pci_bar32(dev, &pci_addr, &bar); - if (ret) - return ret; - - addr = bar; - } -#endif - + addr = dev_read_addr_pci(dev); if (addr == FDT_ADDR_T_NONE) return -EINVAL;

diff --git a/include/dm/fdtaddr.h b/include/dm/fdtaddr.h index 57b326cb336..959d3bc2d69 100644 --- a/include/dm/fdtaddr.h +++ b/include/dm/fdtaddr.h @@ -138,4 +138,12 @@ fdt_addr_t devfdt_get_addr_name(struct udevice *dev, const char *name); fdt_addr_t devfdt_get_addr_size_name(struct udevice *dev, const char *name, fdt_size_t *size);

+/** + * devfdt_get_addr_pci() - Read an address and handle PCI address translation + * + * @dev: Device to read from + * @return address or FDT_ADDR_T_NONE if not found + */ +fdt_addr_t devfdt_get_addr_pci(struct udevice *dev); + #endif diff --git a/include/dm/read.h b/include/dm/read.h index 03189838ff7..4f02d07d001 100644 --- a/include/dm/read.h +++ b/include/dm/read.h @@ -247,6 +247,26 @@ fdt_addr_t dev_read_addr(struct udevice *dev); */ void *dev_read_addr_ptr(struct udevice *dev);

+/** + * dev_read_addr_pci() - Read an address and handle PCI address translation + * + * At present U-Boot does not have address translation logic for PCI in the + * livetree implementation (of_addr.c). This special function supports this for + * the flat tree implementation. + * + * This function should be removed (and code should use dev_read() instead) + * once: + * + * 1. PCI address translation is added; and either + * 2. everything uses livetree where PCI translation is used (which is feasible + * in SPL and U-Boot proper) or PCI address translation is added to + * fdtdec_get_addr() and friends. + * + * @dev: Device to read from + * @return address or FDT_ADDR_T_NONE if not found + */ +fdt_addr_t dev_read_addr_pci(struct udevice *dev); + /** * dev_remap_addr() - Get the reg property of a device as a * memory-mapped I/O pointer @@ -690,6 +710,11 @@ static inline void *dev_read_addr_ptr(struct udevice *dev) return devfdt_get_addr_ptr(dev); }

+static inline fdt_addr_t dev_read_addr_pci(struct udevice *dev) +{ + return devfdt_get_addr_pci(dev); +} + static inline void *dev_remap_addr(struct udevice *dev) { return devfdt_remap_addr(dev); diff --git a/test/dm/pci.c b/test/dm/pci.c index e70b65aea4a..fb93e4c78ae 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -294,3 +294,48 @@ static int dm_test_pci_ea(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_pci_ea, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +/* Test the dev_read_addr_pci() function */ +static int dm_test_pci_addr_flat(struct unit_test_state *uts) +{ + struct udevice *swap1f, *swap1; + ulong io_addr, mem_addr; + + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap1f)); + io_addr = dm_pci_read_bar32(swap1f, 0); + ut_asserteq(io_addr, dev_read_addr_pci(swap1f)); + + /* + * This device has both I/O and MEM spaces but the MEM space appears + * first + */ + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1, 0), &swap1)); + mem_addr = dm_pci_read_bar32(swap1, 1); + ut_asserteq(mem_addr, dev_read_addr_pci(swap1)); + + return 0; +} +DM_TEST(dm_test_pci_addr_flat, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT | + DM_TESTF_FLAT_TREE); + +/* + * Test the dev_read_addr_pci() function with livetree. That function is + * not currently fully implemented, in that it fails to return the BAR address. + * Once that is implemented this test can be removed and dm_test_pci_addr_flat() + * can be used for both flattree and livetree by removing the DM_TESTF_FLAT_TREE + * flag above. + */ +static int dm_test_pci_addr_live(struct unit_test_state *uts) +{ + struct udevice *swap1f, *swap1; + + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1f, 0), &swap1f)); + ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1f)); + + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(0, 0x1, 0), &swap1)); + ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr_pci(swap1)); + + return 0; +} +DM_TEST(dm_test_pci_addr_live, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT | + DM_TESTF_LIVE_TREE);

At present the verbose flag only works for the 'build' command. This is not intended, nor is it useful. Update the code to support the verbose flag and make use of a command exception handler.

Signed-off-by: Simon Glass sjg@chromium.org ---

tools/binman/control.py | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/tools/binman/control.py b/tools/binman/control.py index d1ca798cfb9..1d05bbb87ac 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -475,29 +475,23 @@ def Binman(args): from image import Image import state

- if args.cmd == 'ls': + if args.cmd in ['ls', 'extract', 'replace']: try: + tout.Init(args.verbosity) tools.PrepareOutputDir(None) - ListEntries(args.image, args.paths) - finally: - tools.FinaliseOutputDir() - return 0 - - if args.cmd == 'extract': - try: - tools.PrepareOutputDir(None) - ExtractEntries(args.image, args.filename, args.outdir, args.paths, - not args.uncompressed) - finally: - tools.FinaliseOutputDir() - return 0 - - if args.cmd == 'replace': - try: - tools.PrepareOutputDir(None) - ReplaceEntries(args.image, args.filename, args.indir, args.paths, - do_compress=not args.compressed, - allow_resize=not args.fix_size, write_map=args.map) + if args.cmd == 'ls': + ListEntries(args.image, args.paths) + + if args.cmd == 'extract': + ExtractEntries(args.image, args.filename, args.outdir, args.paths, + not args.uncompressed) + + if args.cmd == 'replace': + ReplaceEntries(args.image, args.filename, args.indir, args.paths, + do_compress=not args.compressed, + allow_resize=not args.fix_size, write_map=args.map) + except: + raise finally: tools.FinaliseOutputDir() return 0