diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 0d21827e1b..1103530941 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -226,7 +226,7 @@ int os_setup_signal_handlers(void) act.sa_sigaction = os_signal_handler; sigemptyset(&act.sa_mask); - act.sa_flags = SA_SIGINFO | SA_NODEFER; + act.sa_flags = SA_SIGINFO; if (sigaction(SIGILL, &act, NULL) || sigaction(SIGBUS, &act, NULL) || sigaction(SIGSEGV, &act, NULL)) @@ -783,12 +783,14 @@ int os_jump_to_image(const void *dest, int size) return os_jump_to_file(fname, true); } -int os_find_u_boot(char *fname, int maxlen, bool use_img) +int os_find_u_boot(char *fname, int maxlen, bool use_img, + const char *cur_prefix, const char *next_prefix) { struct sandbox_state *state = state_get_current(); const char *progname = state->argv[0]; int len = strlen(progname); - const char *suffix; + char subdir[10]; + char *suffix; char *p; int fd; @@ -798,45 +800,36 @@ int os_find_u_boot(char *fname, int maxlen, bool use_img) strcpy(fname, progname); suffix = fname + len - 4; - /* If we are TPL, boot to SPL */ - if (!strcmp(suffix, "-tpl")) { - fname[len - 3] = 's'; - fd = os_open(fname, O_RDONLY); - if (fd >= 0) { - close(fd); - return 0; - } + /* Change the existing suffix to the new one */ + if (*suffix != '-') + return -EINVAL; - /* Look for 'u-boot-spl' in the spl/ directory */ - p = strstr(fname, "/spl/"); - if (p) { - p[1] = 's'; - fd = os_open(fname, O_RDONLY); - if (fd >= 0) { - close(fd); - return 0; - } - } - return -ENOENT; + if (*next_prefix) + strcpy(suffix + 1, next_prefix); /* e.g. "-tpl" to "-spl" */ + else + *suffix = '\0'; /* e.g. "-spl" to "" */ + fd = os_open(fname, O_RDONLY); + if (fd >= 0) { + close(fd); + return 0; } - /* Look for 'u-boot' in the same directory as 'u-boot-spl' */ - if (!strcmp(suffix, "-spl")) { - fname[len - 4] = '\0'; - fd = os_open(fname, O_RDONLY); - if (fd >= 0) { - close(fd); - return 0; - } - } - - /* Look for 'u-boot' in the parent directory of spl/ */ - p = strstr(fname, "spl/"); + /* + * We didn't find it, so try looking for 'u-boot-xxx' in the xxx/ + * directory. Replace the old dirname with the new one. + */ + snprintf(subdir, sizeof(subdir), "/%s/", cur_prefix); + p = strstr(fname, subdir); if (p) { - /* Remove the "spl" characters */ - memmove(p, p + 4, strlen(p + 4) + 1); + if (*next_prefix) + /* e.g. ".../tpl/u-boot-spl" to "../spl/u-boot-spl" */ + memcpy(p + 1, next_prefix, strlen(next_prefix)); + else + /* e.g. ".../spl/u-boot" to ".../u-boot" */ + strcpy(p, p + 1 + strlen(cur_prefix)); if (use_img) strcat(p, ".img"); + fd = os_open(fname, O_RDONLY); if (fd >= 0) { close(fd); diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index 8102649be3..bef5abd039 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -123,6 +123,9 @@ int sandbox_sdl_init_display(int width, int height, int log2_bpp, sdl.vis_height = sdl.height; } + if (!SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "1")) + printf("Unable to init hinting: %s", SDL_GetError()); + sdl.depth = 1 << log2_bpp; sdl.pitch = sdl.width * sdl.depth / 8; SDL_Window *screen = SDL_CreateWindow("U-Boot", SDL_WINDOWPOS_UNDEFINED, @@ -164,8 +167,29 @@ int sandbox_sdl_init_display(int width, int height, int log2_bpp, int sandbox_sdl_sync(void *lcd_base) { + struct SDL_Rect rect; + int ret; + + if (!sdl.texture) + return 0; + SDL_RenderClear(sdl.renderer); SDL_UpdateTexture(sdl.texture, NULL, lcd_base, sdl.pitch); - SDL_RenderCopy(sdl.renderer, sdl.texture, NULL, NULL); + ret = SDL_RenderCopy(sdl.renderer, sdl.texture, NULL, NULL); + if (ret) { + printf("SDL copy %d: %s\n", ret, SDL_GetError()); + return -EIO; + } + + /* + * On some machines this does not appear. Draw an empty rectangle which + * seems to fix that. + */ + rect.x = 0; + rect.y = 0; + rect.w = 0; + rect.h = 0; + SDL_RenderDrawRect(sdl.renderer, &rect); + SDL_RenderPresent(sdl.renderer); sandbox_sdl_poll_events(); diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index f82b0d3de1..650bdb0a70 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -17,6 +17,20 @@ DECLARE_GLOBAL_DATA_PTR; +int sandbox_find_next_phase(char *fname, int maxlen, bool use_img) +{ + const char *cur_prefix, *next_prefix; + int ret; + + cur_prefix = spl_phase_prefix(spl_phase()); + next_prefix = spl_phase_prefix(spl_next_phase()); + ret = os_find_u_boot(fname, maxlen, use_img, cur_prefix, next_prefix); + if (ret) + return log_msg_ret("find", ret); + + return 0; +} + /* SPL / TPL init function */ void board_init_f(ulong flag) { @@ -37,7 +51,7 @@ static int spl_board_load_image(struct spl_image_info *spl_image, char fname[256]; int ret; - ret = os_find_u_boot(fname, sizeof(fname), false); + ret = sandbox_find_next_phase(fname, sizeof(fname), false); if (ret) { printf("(%s not found, error %d)\n", fname, ret); return ret; diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index d85bb46ceb..0cee15a0ea 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -819,6 +819,7 @@ mmc2 { compatible = "sandbox,mmc"; + non-removable; }; mmc1 { diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h index 51e9d95d55..d25dc7c82a 100644 --- a/arch/sandbox/include/asm/spl.h +++ b/arch/sandbox/include/asm/spl.h @@ -12,4 +12,17 @@ enum { BOOT_DEVICE_BOARD, }; +/** + * sandbox_find_next_phase() - Find the next phase of U-Boot + * + * This function is intended to be called from within sandbox SPL. It uses + * a few rules to find the filename of the next U-Boot phase. See also + * os_find_u_boot(). + * + * @fname: place to put full path to U-Boot + * @maxlen: maximum size of @fname + * @use_img: select the 'u-boot.img' file instead of the 'u-boot' ELF file + */ +int sandbox_find_next_phase(char *fname, int maxlen, bool use_img); + #endif diff --git a/common/Kconfig b/common/Kconfig index 26496f9a2e..2ab20a6c85 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -322,6 +322,14 @@ config LOGF_FUNC Show the function name in log messages by default. This value can be overridden using the 'log format' command. +config LOGF_FUNC_PAD + int "Number of characters to use for function" + default 20 + help + Sets the field width to use when showing the function. Set this to + a larger value if you have lots of long function names, and want + things to line up. + config LOG_SYSLOG bool "Log output to syslog server" depends on NET @@ -724,7 +732,7 @@ config BLOBLIST_SIZE config BLOBLIST_ADDR hex "Address of bloblist" depends on BLOBLIST - default 0xe000 if SANDBOX + default 0xc000 if SANDBOX help Sets the address of the bloblist, set up by the first part of U-Boot which runs. Subsequent U-Boot stages typically use the same address. diff --git a/common/bloblist.c b/common/bloblist.c index eab63e9ca5..1290fff850 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -57,13 +57,22 @@ static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr) return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size); } -static struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr, - struct bloblist_rec *rec) +static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr, + struct bloblist_rec *rec) { ulong offset; offset = (void *)rec - (void *)hdr; offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN); + + return offset; +} + +static struct bloblist_rec *bloblist_next_blob(struct bloblist_hdr *hdr, + struct bloblist_rec *rec) +{ + ulong offset = bloblist_blob_end_ofs(hdr, rec); + if (offset >= hdr->alloced) return NULL; return (struct bloblist_rec *)((void *)hdr + offset); @@ -109,7 +118,7 @@ static int bloblist_addrec(uint tag, int size, int align, /* Calculate the new allocated total */ new_alloced = data_start + ALIGN(size, align); - if (new_alloced >= hdr->size) { + if (new_alloced > hdr->size) { log(LOGC_BLOBLIST, LOGL_ERR, "Failed to allocate %x bytes size=%x, need size=%x\n", size, hdr->size, new_alloced); @@ -215,6 +224,64 @@ int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp) return 0; } +static int bloblist_resize_rec(struct bloblist_hdr *hdr, + struct bloblist_rec *rec, + int new_size) +{ + int expand_by; /* Number of bytes to expand by (-ve to contract) */ + int new_alloced; /* New value for @hdr->alloced */ + ulong next_ofs; /* Offset of the record after @rec */ + + expand_by = ALIGN(new_size - rec->size, BLOBLIST_ALIGN); + new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_ALIGN); + if (new_size < 0) { + log(LOGC_BLOBLIST, LOGL_DEBUG, + "Attempt to shrink blob size below 0 (%x)\n", new_size); + return log_msg_ret("size", -EINVAL); + } + if (new_alloced > hdr->size) { + log(LOGC_BLOBLIST, LOGL_ERR, + "Failed to allocate %x bytes size=%x, need size=%x\n", + new_size, hdr->size, new_alloced); + return log_msg_ret("alloc", -ENOSPC); + } + + /* Move the following blobs up or down, if this is not the last */ + next_ofs = bloblist_blob_end_ofs(hdr, rec); + if (next_ofs != hdr->alloced) { + memmove((void *)hdr + next_ofs + expand_by, + (void *)hdr + next_ofs, new_alloced - next_ofs); + } + hdr->alloced = new_alloced; + + /* Zero the new part of the blob */ + if (expand_by > 0) { + memset((void *)rec + rec->hdr_size + rec->size, '\0', + new_size - rec->size); + } + + /* Update the size of this blob */ + rec->size = new_size; + + return 0; +} + +int bloblist_resize(uint tag, int new_size) +{ + struct bloblist_hdr *hdr = gd->bloblist; + struct bloblist_rec *rec; + int ret; + + rec = bloblist_findrec(tag); + if (!rec) + return log_msg_ret("find", -ENOENT); + ret = bloblist_resize_rec(hdr, rec, new_size); + if (ret) + return log_msg_ret("resize", ret); + + return 0; +} + static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr) { struct bloblist_rec *rec; diff --git a/common/image-fit.c b/common/image-fit.c index 8e23d51cf2..28bd8e78c7 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1377,7 +1377,7 @@ int fit_image_verify(const void *fit, int image_noffset) size_t size; char *err_msg = ""; - if (strchr(name, '@')) { + if (IS_ENABLED(CONFIG_FIT_SIGNATURE) && strchr(name, '@')) { /* * We don't support this since libfdt considers names with the * name root but different @ suffix to be equal diff --git a/common/log_console.c b/common/log_console.c index 3f6177499e..f1dcc04b97 100644 --- a/common/log_console.c +++ b/common/log_console.c @@ -38,7 +38,7 @@ static int log_console_emit(struct log_device *ldev, struct log_rec *rec) if (fmt & BIT(LOGF_LINE)) printf("%d-", rec->line); if (fmt & BIT(LOGF_FUNC)) - printf("%s()", rec->func); + printf("%*s()", CONFIG_LOGF_FUNC_PAD, rec->func); } if (fmt & BIT(LOGF_MSG)) printf("%s%s", add_space ? " " : "", rec->msg); diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 2df3e5d869..c0183521d2 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -91,6 +91,16 @@ config SPL_SYS_REPORT_STACK_F_USAGE occurrence of non 0xaa bytes. This default implementation works for stacks growing down only. +config SPL_SHOW_ERRORS + bool "Show more information when something goes wrong" + help + This enabled more verbose error messages and checking when something + goes wrong in SPL. For example, it shows the error code when U-Boot + cannot be located. This can help to diagnose the problem and figure + out a fix, particularly during development. + + This adds a small amount to SPL code size, perhaps 100 bytes. + menu "PowerPC and LayerScape SPL Boot options" config SPL_NAND_BOOT diff --git a/common/spl/spl.c b/common/spl/spl.c index eba77cace6..3b96f2fc31 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -593,32 +593,42 @@ static int spl_load_image(struct spl_image_info *spl_image, * @spl_image: Place to put the image details if successful * @spl_boot_list: List of boot devices to try * @count: Number of elements in spl_boot_list - * @return 0 if OK, -ve on error + * @return 0 if OK, -ENODEV if there were no boot devices + * if CONFIG_SHOW_ERRORS is enabled, returns -ENXIO if there were + * devices but none worked */ static int boot_from_devices(struct spl_image_info *spl_image, u32 spl_boot_list[], int count) { + int ret = -ENODEV; int i; for (i = 0; i < count && spl_boot_list[i] != BOOT_DEVICE_NONE; i++) { struct spl_image_loader *loader; + int bootdev = spl_boot_list[i]; - loader = spl_ll_find_loader(spl_boot_list[i]); -#if defined(CONFIG_SPL_SERIAL_SUPPORT) \ - && defined(CONFIG_SPL_LIBCOMMON_SUPPORT) \ - && !defined(CONFIG_SILENT_CONSOLE) - if (loader) - printf("Trying to boot from %s\n", loader->name); - else - puts(SPL_TPL_PROMPT "Unsupported Boot Device!\n"); -#endif + if (CONFIG_IS_ENABLED(SHOW_ERRORS)) + ret = -ENXIO; + loader = spl_ll_find_loader(bootdev); + if (CONFIG_IS_ENABLED(SERIAL_SUPPORT) && + CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) && + !IS_ENABLED(CONFIG_SILENT_CONSOLE)) { + if (loader) + printf("Trying to boot from %s\n", + spl_loader_name(loader)); + else if (CONFIG_IS_ENABLED(SHOW_ERRORS)) + printf(SPL_TPL_PROMPT + "Unsupported Boot Device %d\n", bootdev); + else + puts(SPL_TPL_PROMPT "Unsupported Boot Device!\n"); + } if (loader && !spl_load_image(spl_image, loader)) { - spl_image->boot_device = spl_boot_list[i]; + spl_image->boot_device = bootdev; return 0; } } - return -ENODEV; + return ret; } #if defined(CONFIG_SPL_FRAMEWORK_BOARD_INIT_F) @@ -710,9 +720,15 @@ void board_init_r(gd_t *dummy1, ulong dummy2) spl_image.boot_device = BOOT_DEVICE_NONE; board_boot_order(spl_boot_list); - if (boot_from_devices(&spl_image, spl_boot_list, - ARRAY_SIZE(spl_boot_list))) { - puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n"); + ret = boot_from_devices(&spl_image, spl_boot_list, + ARRAY_SIZE(spl_boot_list)); + if (ret) { + if (CONFIG_IS_ENABLED(SHOW_ERRORS) && + CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)) + printf(SPL_TPL_PROMPT "failed to boot from all boot devices (err=%d)\n", + ret); + else + puts(SPL_TPL_PROMPT "failed to boot from all boot devices\n"); hang(); } diff --git a/doc/arch/sandbox.rst b/doc/arch/sandbox.rst index e052b6bdb0..9e23e1618c 100644 --- a/doc/arch/sandbox.rst +++ b/doc/arch/sandbox.rst @@ -510,7 +510,7 @@ that are mapped into that memory: Addr Config Usage ======= ======================== =============================== 0 CONFIG_SYS_FDT_LOAD_ADDR Device tree - e000 CONFIG_BLOBLIST_ADDR Blob list + c000 CONFIG_BLOBLIST_ADDR Blob list 10000 CONFIG_MALLOC_F_ADDR Early memory allocation f0000 CONFIG_PRE_CON_BUF_ADDR Pre-console buffer 100000 CONFIG_TRACE_EARLY_ADDR Early trace buffer (if enabled). Also used diff --git a/doc/develop/driver-model/of-plat.rst b/doc/develop/driver-model/of-plat.rst index 74f1932473..8a8eaed4c1 100644 --- a/doc/develop/driver-model/of-plat.rst +++ b/doc/develop/driver-model/of-plat.rst @@ -597,6 +597,59 @@ as a macro included in the driver definition:: +Problems +-------- + +In some cases you will you see something like this:: + + WARNING: the driver rockchip_rk3188_grf was not found in the driver list + +The driver list is a list of drivers, each with a name. The name is in the +U_BOOT_DRIVER() declaration, repeated twice, one in brackets and once as the +.name member. For example, in the following declaration the driver name is +`rockchip_rk3188_grf`:: + + U_BOOT_DRIVER(rockchip_rk3188_grf) = { + .name = "rockchip_rk3188_grf", + .id = UCLASS_SYSCON, + .of_match = rk3188_syscon_ids + 1, + .bind = rk3188_syscon_bind_of_plat, + }; + +The first name U_BOOT_DRIVER(xx) is used to create a linker symbol so that the +driver can be accessed at build-time without any overhead. The second one +(.name = "xx") is used at runtime when something wants to print out the driver +name. + +The dtoc tool expects to be able to find a driver for each compatible string in +the devicetree. For example, if the devicetree has:: + + grf: grf@20008000 { + compatible = "rockchip,rk3188-grf", "syscon"; + reg = <0x20008000 0x200>; + u-boot,dm-spl; + }; + +then dtoc looks at the first compatible string ("rockchip,rk3188-grf"), +converts that to a C identifier (rockchip_rk3188_grf) and then looks for that. + +Various things can cause dtoc to fail to find the driver and it tries to +warn about these. For example: + + rockchip_rk3188_uart: Missing .compatible in drivers/serial/serial_rockchip.c + : WARNING: the driver rockchip_rk3188_uart was not found in the driver list + +Without a compatible string a driver cannot be used by dtoc, even if the +compatible string is not actually needed at runtime. + +If the problem is simply that there are multiple compatible strings, the +DM_DRIVER_ALIAS() macro can be used to tell dtoc about this and avoid a problem. + +Checks are also made to confirm that the referenced driver has a .compatible +member and a .id member. The first provides the array of compatible strings and +the second provides the uclass ID. + + Caveats ------- diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index dfc0d46970..83682dcc18 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -540,6 +540,55 @@ int blk_next_free_devnum(enum if_type if_type) return ret + 1; } +static int blk_flags_check(struct udevice *dev, enum blk_flag_t req_flags) +{ + const struct blk_desc *desc = dev_get_uclass_plat(dev); + enum blk_flag_t flags; + + flags = desc->removable ? BLKF_REMOVABLE : BLKF_FIXED; + + return flags & req_flags ? 0 : 1; +} + +int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp) +{ + int ret; + + for (ret = uclass_first_device_err(UCLASS_BLK, devp); + !ret; + ret = uclass_next_device_err(devp)) { + if (!blk_flags_check(*devp, flags)) + return 0; + } + + return -ENODEV; +} + +int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp) +{ + int ret; + + for (ret = uclass_next_device_err(devp); + !ret; + ret = uclass_next_device_err(devp)) { + if (!blk_flags_check(*devp, flags)) + return 0; + } + + return -ENODEV; +} + +int blk_count_devices(enum blk_flag_t flag) +{ + struct udevice *dev; + int count = 0; + + blk_foreach_probe(flag, dev) + count++; + + return count; +} + static int blk_claim_devnum(enum if_type if_type, int devnum) { struct udevice *dev; diff --git a/drivers/core/device.c b/drivers/core/device.c index 9f1400768d..29668f6fb3 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -87,8 +87,10 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) { if (uc->uc_drv->name && ofnode_valid(node)) { - if (!dev_read_alias_seq(dev, &dev->seq_)) + if (!dev_read_alias_seq(dev, &dev->seq_)) { auto_seq = false; + log_debug(" - seq=%d\n", dev->seq_); + } } } } diff --git a/drivers/core/of_extra.c b/drivers/core/of_extra.c index 7702beff97..632a1c2210 100644 --- a/drivers/core/of_extra.c +++ b/drivers/core/of_extra.c @@ -31,6 +31,8 @@ int ofnode_read_fmap_entry(ofnode node, struct fmap_entry *entry) if (prop) { if (!strcmp(prop, "lz4")) entry->compress_algo = FMAP_COMPRESS_LZ4; + else if (!strcmp(prop, "lzma")) + entry->compress_algo = FMAP_COMPRESS_LZMA; else return log_msg_ret("compression algo", -EINVAL); } else { diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index eeeccfb446..dda6c76e83 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -329,7 +329,8 @@ static fdt_addr_t __ofnode_get_addr_size_index(ofnode node, int index, { int na, ns; - *size = FDT_SIZE_T_NONE; + if (size) + *size = FDT_SIZE_T_NONE; if (ofnode_is_np(node)) { const __be32 *prop_val; @@ -340,6 +341,7 @@ static fdt_addr_t __ofnode_get_addr_size_index(ofnode node, int index, &flags); if (!prop_val) return FDT_ADDR_T_NONE; + if (size) *size = size64; @@ -359,8 +361,6 @@ static fdt_addr_t __ofnode_get_addr_size_index(ofnode node, int index, index, na, ns, size, translate); } - - return FDT_ADDR_T_NONE; } fdt_addr_t ofnode_get_addr_size_index(ofnode node, int index, fdt_size_t *size) diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c index 7904d5cc72..2a15094d20 100644 --- a/drivers/misc/cros_ec.c +++ b/drivers/misc/cros_ec.c @@ -754,17 +754,6 @@ int cros_ec_flash_protect(struct udevice *dev, uint32_t set_mask, return 0; } -int cros_ec_entering_mode(struct udevice *dev, int mode) -{ - int rc; - - rc = ec_command(dev, EC_CMD_ENTERING_MODE, 0, &mode, sizeof(mode), - NULL, 0); - if (rc) - return -1; - return 0; -} - static int cros_ec_check_version(struct udevice *dev) { struct cros_ec_dev *cdev = dev_get_uclass_priv(dev); @@ -1661,6 +1650,23 @@ int cros_ec_get_switches(struct udevice *dev) return ret; } +int cros_ec_read_batt_charge(struct udevice *dev, uint *chargep) +{ + struct ec_params_charge_state req; + struct ec_response_charge_state resp; + int ret; + + req.cmd = CHARGE_STATE_CMD_GET_STATE; + ret = ec_command(dev, EC_CMD_CHARGE_STATE, 0, &req, sizeof(req), + &resp, sizeof(resp)); + if (ret) + return log_msg_ret("read", ret); + + *chargep = resp.get_state.batt_state_of_charge; + + return 0; +} + UCLASS_DRIVER(cros_ec) = { .id = UCLASS_CROS_EC, .name = "cros-ec", diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c index beea47caa3..2173517cff 100644 --- a/drivers/misc/cros_ec_sandbox.c +++ b/drivers/misc/cros_ec_sandbox.c @@ -343,15 +343,13 @@ static int process_cmd(struct ec_state *ec, switch (req->op) { case EC_VBNV_CONTEXT_OP_READ: - /* TODO(sjg@chromium.org): Support full-size context */ memcpy(resp->block, ec->vbnv_context, - EC_VBNV_BLOCK_SIZE); - len = 16; + EC_VBNV_BLOCK_SIZE_V2); + len = EC_VBNV_BLOCK_SIZE_V2; break; case EC_VBNV_CONTEXT_OP_WRITE: - /* TODO(sjg@chromium.org): Support full-size context */ memcpy(ec->vbnv_context, req->block, - EC_VBNV_BLOCK_SIZE); + EC_VBNV_BLOCK_SIZE_V2); len = 0; break; default: @@ -496,9 +494,6 @@ static int process_cmd(struct ec_state *ec, case EC_CMD_MKBP_STATE: len = cros_ec_keyscan(ec, resp_data); break; - case EC_CMD_ENTERING_MODE: - len = 0; - break; case EC_CMD_GET_NEXT_EVENT: { struct ec_response_get_next_event *resp = resp_data; @@ -629,15 +624,19 @@ void cros_ec_check_keyboard(struct udevice *dev) struct ec_state *ec = dev_get_priv(dev); ulong start; - printf("Press keys for EC to detect on reset (ESC=recovery)..."); + printf("\nPress keys for EC to detect on reset (ESC=recovery)..."); start = get_timer(0); - while (get_timer(start) < 1000) - ; - putc('\n'); - if (!sandbox_sdl_key_pressed(KEY_ESC)) { - ec->recovery_req = true; - printf(" - EC requests recovery\n"); + while (get_timer(start) < 2000) { + if (tstc()) { + int ch = getchar(); + + if (ch == 0x1b) { + ec->recovery_req = true; + printf("EC requests recovery"); + } + } } + putc('\n'); } /* Return the byte of EC switch states */ diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c index 18ba020aac..895fbffecf 100644 --- a/drivers/mmc/sandbox_mmc.c +++ b/drivers/mmc/sandbox_mmc.c @@ -136,14 +136,31 @@ static const struct dm_mmc_ops sandbox_mmc_ops = { .get_cd = sandbox_mmc_get_cd, }; -int sandbox_mmc_probe(struct udevice *dev) +static int sandbox_mmc_of_to_plat(struct udevice *dev) +{ + struct sandbox_mmc_plat *plat = dev_get_plat(dev); + struct mmc_config *cfg = &plat->cfg; + struct blk_desc *blk; + int ret; + + ret = mmc_of_parse(dev, cfg); + if (ret) + return ret; + blk = mmc_get_blk_desc(&plat->mmc); + if (blk) + blk->removable = !(cfg->host_caps & MMC_CAP_NONREMOVABLE); + + return 0; +} + +static int sandbox_mmc_probe(struct udevice *dev) { struct sandbox_mmc_plat *plat = dev_get_plat(dev); return mmc_init(&plat->mmc); } -int sandbox_mmc_bind(struct udevice *dev) +static int sandbox_mmc_bind(struct udevice *dev) { struct sandbox_mmc_plat *plat = dev_get_plat(dev); struct mmc_config *cfg = &plat->cfg; @@ -158,7 +175,7 @@ int sandbox_mmc_bind(struct udevice *dev) return mmc_bind(dev, &plat->mmc, cfg); } -int sandbox_mmc_unbind(struct udevice *dev) +static int sandbox_mmc_unbind(struct udevice *dev) { mmc_unbind(dev); @@ -177,6 +194,7 @@ U_BOOT_DRIVER(mmc_sandbox) = { .ops = &sandbox_mmc_ops, .bind = sandbox_mmc_bind, .unbind = sandbox_mmc_unbind, + .of_to_plat = sandbox_mmc_of_to_plat, .probe = sandbox_mmc_probe, .priv_auto = sizeof(struct sandbox_mmc_priv), .plat_auto = sizeof(struct sandbox_mmc_plat), diff --git a/include/blk.h b/include/blk.h index c4401b0025..19bab081c2 100644 --- a/include/blk.h +++ b/include/blk.h @@ -19,6 +19,8 @@ typedef ulong lbaint_t; #define LBAF "%" LBAFlength "x" #define LBAFU "%" LBAFlength "u" +struct udevice; + /* Interface types: */ enum if_type { IF_TYPE_UNKNOWN = 0, @@ -683,4 +685,58 @@ const char *blk_get_if_type_name(enum if_type if_type); int blk_common_cmd(int argc, char *const argv[], enum if_type if_type, int *cur_devnump); +enum blk_flag_t { + BLKF_FIXED = 1 << 0, + BLKF_REMOVABLE = 1 << 1, + BLKF_BOTH = BLKF_FIXED | BLKF_REMOVABLE, +}; + +/** + * blk_first_device_err() - Get the first block device + * + * The device returned is probed if necessary, and ready for use + * + * @flags: Indicates type of device to return + * @devp: Returns pointer to the first device in that uclass, or NULL if none + * @return 0 if found, -ENODEV if not found, other -ve on error + */ +int blk_first_device_err(enum blk_flag_t flags, struct udevice **devp); + +/** + * blk_next_device_err() - Get the next block device + * + * The device returned is probed if necessary, and ready for use + * + * @flags: Indicates type of device to return + * @devp: On entry, pointer to device to lookup. On exit, returns pointer + * to the next device in the uclass if no error occurred, or -ENODEV if + * there is no next device. + * @return 0 if found, -ENODEV if not found, other -ve on error + */ +int blk_next_device_err(enum blk_flag_t flags, struct udevice **devp); + +/** + * blk_foreach_probe() - Helper function to iteration through block devices + * + * This creates a for() loop which works through the available devices in + * a uclass in order from start to end. Devices are probed if necessary, + * and ready for use. + * + * @flags: Indicates type of device to return + * @dev: struct udevice * to hold the current device. Set to NULL when there + * are no more devices. + */ +#define blk_foreach_probe(flags, pos) \ + for (int _ret = blk_first_device_err(flags, &(pos)); \ + !_ret && pos; \ + _ret = blk_next_device_err(flags, &(pos))) + +/** + * blk_count_devices() - count the number of devices of a particular type + * + * @flags: Indicates type of device to find + * @return number of devices matching those flags + */ +int blk_count_devices(enum blk_flag_t flag); + #endif diff --git a/include/bloblist.h b/include/bloblist.h index 964b974fda..9f007c7a94 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -64,10 +64,10 @@ enum bloblist_tag_t { * first bloblist_rec starts at this offset from the start of the header * @flags: Space for BLOBLISTF_... flags (none yet) * @magic: BLOBLIST_MAGIC - * @size: Total size of all records (non-zero if valid) including this header. + * @size: Total size of the bloblist (non-zero if valid) including this header. * The bloblist extends for this many bytes from the start of this header. - * @alloced: Total size allocated for this bloblist. When adding new records, - * the bloblist can grow up to this size. This starts out as + * When adding new records, the bloblist can grow up to this size. + * @alloced: Total size allocated so far for this bloblist. This starts out as * sizeof(bloblist_hdr) since we need at least that much space to store a * valid bloblist * @spare: Spare space (for future use) @@ -179,6 +179,19 @@ void *bloblist_ensure(uint tag, int size); */ int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp); +/** + * bloblist_resize() - resize a blob + * + * Any blobs above this one are relocated up or down. The resized blob remains + * in the same place. + * + * @tag: Tag to add (enum bloblist_tag_t) + * @new_size: New size of the blob (>0 to expand, <0 to contract) + * @return 0 if OK, -ENOSPC if the bloblist does not have enough space, -ENOENT + * if the tag is not found + */ +int bloblist_resize(uint tag, int new_size); + /** * bloblist_new() - Create a new, empty bloblist of a given size * @@ -217,6 +230,10 @@ int bloblist_finish(void); * bloblist_get_stats() - Get information about the bloblist * * This returns useful information about the bloblist + * + * @basep: Returns base address of bloblist + * @sizep: Returns the number of bytes used in the bloblist + * @allocedp: Returns the total space allocated to the bloblist */ void bloblist_get_stats(ulong *basep, ulong *sizep, ulong *allocedp); diff --git a/include/cros_ec.h b/include/cros_ec.h index 9396b4d246..ef89deff76 100644 --- a/include/cros_ec.h +++ b/include/cros_ec.h @@ -198,15 +198,6 @@ int cros_ec_flash_protect(struct udevice *dev, uint32_t set_mask, uint32_t set_flags, struct ec_response_flash_protect *resp); -/** - * Notify EC of current boot mode - * - * @param dev CROS-EC device - * @param vboot_mode Verified boot mode - * @return 0 if ok, <0 on error - */ -int cros_ec_entering_mode(struct udevice *dev, int mode); - /** * Run internal tests on the cros_ec interface. * @@ -652,4 +643,12 @@ int cros_ec_vstore_read(struct udevice *dev, int slot, uint8_t *data); int cros_ec_vstore_write(struct udevice *dev, int slot, const uint8_t *data, size_t size); +/** + * cros_ec_read_batt_charge() - Read the battery-charge state + * + * @dev: CROS-EC device + * @chargep: Return battery-charge state as a percentage + */ +int cros_ec_read_batt_charge(struct udevice *dev, uint *chargep); + #endif diff --git a/include/os.h b/include/os.h index bd1096eb8b..7b20d606dd 100644 --- a/include/os.h +++ b/include/os.h @@ -327,9 +327,12 @@ int os_jump_to_image(const void *dest, int size); * @fname: place to put full path to U-Boot * @maxlen: maximum size of @fname * @use_img: select the 'u-boot.img' file instead of the 'u-boot' ELF file + * @cur_prefix: prefix of current executable, e.g. "spl" or "tpl" + * @next_prefix: prefix of executable to find, e.g. "spl" or "" * Return: 0 if OK, -NOSPC if the filename is too large, -ENOENT if not found */ -int os_find_u_boot(char *fname, int maxlen, bool use_img); +int os_find_u_boot(char *fname, int maxlen, bool use_img, + const char *cur_prefix, const char *next_prefix); /** * os_spl_to_uboot() - Run U-Boot proper diff --git a/include/spl.h b/include/spl.h index c643943482..74a1939452 100644 --- a/include/spl.h +++ b/include/spl.h @@ -176,6 +176,27 @@ static inline const char *spl_phase_name(enum u_boot_phase phase) } } +/** + * spl_phase_prefix() - Get the prefix of the current phase + * + * @phase: Phase to look up + * @return phase prefix ("spl", "tpl", etc.) + */ +static inline const char *spl_phase_prefix(enum u_boot_phase phase) +{ + switch (phase) { + case PHASE_TPL: + return "tpl"; + case PHASE_SPL: + return "spl"; + case PHASE_BOARD_F: + case PHASE_BOARD_R: + return ""; + default: + return "phase?"; + } +} + /* A string name for SPL or TPL */ #ifdef CONFIG_SPL_BUILD # ifdef CONFIG_TPL_BUILD @@ -484,6 +505,16 @@ struct spl_image_loader { struct spl_boot_device *bootdev); }; +/* Helper function for accessing the name */ +static inline const char *spl_loader_name(const struct spl_image_loader *loader) +{ +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT + return loader->name; +#else + return NULL; +#endif +} + /* Declare an SPL image loader */ #define SPL_LOAD_IMAGE(__name) \ ll_entry_declare(struct spl_image_loader, __name, spl_image_loader) diff --git a/test/Makefile b/test/Makefile index a26e915e05..117839e584 100644 --- a/test/Makefile +++ b/test/Makefile @@ -3,7 +3,9 @@ # (C) Copyright 2012 The Chromium Authors obj-y += test-main.o +ifdef CONFIG_SPL_LOAD_FIT obj-$(CONFIG_SANDBOX) += image/ +endif ifneq ($(CONFIG_$(SPL_)BLOBLIST),) obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o diff --git a/test/bloblist.c b/test/bloblist.c index d876b63918..4104e6a92f 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -33,6 +33,9 @@ enum { ERASE_BYTE = '\xff', }; +static const char test1_str[] = "the eyes are open"; +static const char test2_str[] = "the mouth moves"; + static struct bloblist_hdr *clear_bloblist(void) { struct bloblist_hdr *hdr; @@ -384,6 +387,218 @@ static int bloblist_test_reloc(struct unit_test_state *uts) } BLOBLIST_TEST(bloblist_test_reloc, 0); +/* Test expansion of a blob */ +static int bloblist_test_grow(struct unit_test_state *uts) +{ + const uint small_size = 0x20; + void *blob1, *blob2, *blob1_new; + struct bloblist_hdr *hdr; + void *ptr; + + ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); + hdr = ptr; + memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE); + + /* Create two blobs */ + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + blob1 = bloblist_add(TEST_TAG, small_size, 0); + ut_assertnonnull(blob1); + ut_assertok(check_zero(blob1, small_size)); + strcpy(blob1, test1_str); + + blob2 = bloblist_add(TEST_TAG2, small_size, 0); + ut_assertnonnull(blob2); + strcpy(blob2, test2_str); + + ut_asserteq(sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2, + hdr->alloced); + + /* Resize the first one */ + ut_assertok(bloblist_resize(TEST_TAG, small_size + 4)); + + /* The first one should not have moved, just got larger */ + blob1_new = bloblist_find(TEST_TAG, small_size + 4); + ut_asserteq_ptr(blob1, blob1_new); + + /* The new space should be zeroed */ + ut_assertok(check_zero(blob1 + small_size, 4)); + + /* The second one should have moved */ + blob2 = bloblist_find(TEST_TAG2, small_size); + ut_assertnonnull(blob2); + ut_asserteq_str(test2_str, blob2); + + /* The header should have more bytes in use */ + hdr = ptr; + ut_asserteq(sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2 + + BLOBLIST_ALIGN, + hdr->alloced); + + return 0; +} +BLOBLIST_TEST(bloblist_test_grow, 0); + +/* Test shrinking of a blob */ +static int bloblist_test_shrink(struct unit_test_state *uts) +{ + const uint small_size = 0x20; + void *blob1, *blob2, *blob1_new; + struct bloblist_hdr *hdr; + int new_size; + void *ptr; + + ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); + + /* Create two blobs */ + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + blob1 = bloblist_add(TEST_TAG, small_size, 0); + ut_assertnonnull(blob1); + strcpy(blob1, test1_str); + + blob2 = bloblist_add(TEST_TAG2, small_size, 0); + ut_assertnonnull(blob2); + strcpy(blob2, test2_str); + + hdr = ptr; + ut_asserteq(sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2, + hdr->alloced); + + /* Resize the first one */ + new_size = small_size - BLOBLIST_ALIGN - 4; + ut_assertok(bloblist_resize(TEST_TAG, new_size)); + + /* The first one should not have moved, just got smaller */ + blob1_new = bloblist_find(TEST_TAG, new_size); + ut_asserteq_ptr(blob1, blob1_new); + + /* The second one should have moved */ + blob2 = bloblist_find(TEST_TAG2, small_size); + ut_assertnonnull(blob2); + ut_asserteq_str(test2_str, blob2); + + /* The header should have fewer bytes in use */ + hdr = ptr; + ut_asserteq(sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2 - + BLOBLIST_ALIGN, + hdr->alloced); + + return 0; +} +BLOBLIST_TEST(bloblist_test_shrink, 0); + +/* Test failing to adjust a blob size */ +static int bloblist_test_resize_fail(struct unit_test_state *uts) +{ + const uint small_size = 0x20; + struct bloblist_hdr *hdr; + void *blob1, *blob2; + int new_size; + void *ptr; + + ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); + + /* Create two blobs */ + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + blob1 = bloblist_add(TEST_TAG, small_size, 0); + ut_assertnonnull(blob1); + + blob2 = bloblist_add(TEST_TAG2, small_size, 0); + ut_assertnonnull(blob2); + + hdr = ptr; + ut_asserteq(sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2, + hdr->alloced); + + /* Resize the first one, to check the boundary conditions */ + ut_asserteq(-EINVAL, bloblist_resize(TEST_TAG, -1)); + + new_size = small_size + (hdr->size - hdr->alloced); + ut_asserteq(-ENOSPC, bloblist_resize(TEST_TAG, new_size + 1)); + ut_assertok(bloblist_resize(TEST_TAG, new_size)); + + return 0; +} +BLOBLIST_TEST(bloblist_test_resize_fail, 0); + +/* Test expanding the last blob in a bloblist */ +static int bloblist_test_resize_last(struct unit_test_state *uts) +{ + const uint small_size = 0x20; + struct bloblist_hdr *hdr; + void *blob1, *blob2, *blob2_new; + int alloced_val; + void *ptr; + + ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE); + memset(ptr, ERASE_BYTE, TEST_BLOBLIST_SIZE); + hdr = ptr; + + /* Create two blobs */ + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + blob1 = bloblist_add(TEST_TAG, small_size, 0); + ut_assertnonnull(blob1); + + blob2 = bloblist_add(TEST_TAG2, small_size, 0); + ut_assertnonnull(blob2); + + /* Check the byte after the last blob */ + alloced_val = sizeof(struct bloblist_hdr) + + sizeof(struct bloblist_rec) * 2 + small_size * 2; + ut_asserteq(alloced_val, hdr->alloced); + ut_asserteq_ptr((void *)hdr + alloced_val, blob2 + small_size); + ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced)); + + /* Resize the second one, checking nothing changes */ + ut_asserteq(0, bloblist_resize(TEST_TAG2, small_size + 4)); + + blob2_new = bloblist_find(TEST_TAG2, small_size + 4); + ut_asserteq_ptr(blob2, blob2_new); + + /* + * the new blob should encompass the byte we checked now, so it should + * be zeroed. This zeroing should affect only the four new bytes added + * to the blob. + */ + ut_asserteq(0, *((u8 *)hdr + alloced_val)); + ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + alloced_val + 4)); + + /* Check that the new top of the allocated blobs has not been touched */ + alloced_val += BLOBLIST_ALIGN; + ut_asserteq(alloced_val, hdr->alloced); + ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced)); + + return 0; +} +BLOBLIST_TEST(bloblist_test_resize_last, 0); + +/* Check a completely full bloblist */ +static int bloblist_test_blob_maxsize(struct unit_test_state *uts) +{ + void *ptr; + int size; + + /* At the start there should be no records */ + clear_bloblist(); + ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0)); + + /* Add a blob that takes up all space */ + size = TEST_BLOBLIST_SIZE - sizeof(struct bloblist_hdr) - + sizeof(struct bloblist_rec); + ptr = bloblist_add(TEST_TAG, size, 0); + ut_assertnonnull(ptr); + + ptr = bloblist_add(TEST_TAG, size + 1, 0); + ut_assertnull(ptr); + + return 0; +} +BLOBLIST_TEST(bloblist_test_blob_maxsize, 0); + int do_ut_bloblist(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { diff --git a/test/dm/blk.c b/test/dm/blk.c index b7f4304e9e..deccf05289 100644 --- a/test/dm/blk.c +++ b/test/dm/blk.c @@ -162,3 +162,58 @@ static int dm_test_blk_get_from_parent(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_blk_get_from_parent, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +/* Test iteration through block devices */ +static int dm_test_blk_iter(struct unit_test_state *uts) +{ + struct udevice *dev; + int i; + + /* + * See sandbox test.dts - it has: + * + * mmc0 - removable + * mmc1 - removable + * mmc2 - fixed + */ + ut_assertok(blk_first_device_err(BLKF_FIXED, &dev)); + ut_asserteq_str("mmc2.blk", dev->name); + ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev)); + + ut_assertok(blk_first_device_err(BLKF_REMOVABLE, &dev)); + ut_asserteq_str("mmc1.blk", dev->name); + ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev)); + ut_asserteq_str("mmc0.blk", dev->name); + ut_asserteq(-ENODEV, blk_next_device_err(BLKF_REMOVABLE, &dev)); + + ut_assertok(blk_first_device_err(BLKF_BOTH, &dev)); + ut_asserteq_str("mmc2.blk", dev->name); + ut_assertok(blk_next_device_err(BLKF_BOTH, &dev)); + ut_asserteq_str("mmc1.blk", dev->name); + ut_assertok(blk_next_device_err(BLKF_BOTH, &dev)); + ut_asserteq_str("mmc0.blk", dev->name); + ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev)); + + ut_asserteq(1, blk_count_devices(BLKF_FIXED)); + ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE)); + ut_asserteq(3, blk_count_devices(BLKF_BOTH)); + + i = 0; + blk_foreach_probe(BLKF_FIXED, dev) + ut_asserteq_str((i++, "mmc2.blk"), dev->name); + ut_asserteq(1, i); + + i = 0; + blk_foreach_probe(BLKF_REMOVABLE, dev) + ut_asserteq_str(i++ ? "mmc0.blk" : "mmc1.blk", dev->name); + ut_asserteq(2, i); + + i = 0; + blk_foreach_probe(BLKF_BOTH, dev) + ut_asserteq_str((++i == 1 ? "mmc2.blk" : i == 2 ? + "mmc1.blk" : "mmc0.blk"), dev->name); + ut_asserteq(3, i); + + return 0; +} +DM_TEST(dm_test_blk_iter, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); diff --git a/test/dm/core.c b/test/dm/core.c index 2210345dd1..0492698997 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -1171,6 +1171,7 @@ static int dm_test_all_have_seq(struct unit_test_state *uts) } DM_TEST(dm_test_all_have_seq, UT_TESTF_SCAN_PDATA); +#if CONFIG_IS_ENABLED(DM_DMA) static int dm_test_dma_offset(struct unit_test_state *uts) { struct udevice *dev; @@ -1200,3 +1201,4 @@ static int dm_test_dma_offset(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_dma_offset, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); +#endif diff --git a/test/image/spl_load.c b/test/image/spl_load.c index 851603ddd7..e7cabf5680 100644 --- a/test/image/spl_load.c +++ b/test/image/spl_load.c @@ -56,6 +56,7 @@ struct image_header *spl_get_load_buffer(ssize_t offset, size_t size) static int spl_test_load(struct unit_test_state *uts) { + const char *cur_prefix, *next_prefix; struct spl_image_info image; struct image_header *header; struct text_ctx text_ctx; @@ -68,7 +69,10 @@ static int spl_test_load(struct unit_test_state *uts) load.bl_len = 512; load.read = read_fit_image; - ret = os_find_u_boot(fname, sizeof(fname), true); + cur_prefix = spl_phase_prefix(spl_phase()); + next_prefix = spl_phase_prefix(spl_next_phase()); + ret = os_find_u_boot(fname, sizeof(fname), true, cur_prefix, + next_prefix); if (ret) { printf("(%s not found, error %d)\n", fname, ret); return ret; diff --git a/test/log/log_test.c b/test/log/log_test.c index f1e67509c1..db7170f304 100644 --- a/test/log/log_test.c +++ b/test/log/log_test.c @@ -62,9 +62,9 @@ static int do_check_log_entries(struct unit_test_state *uts, int flags, int min, for (i = min; i <= max; i++) { if (flags & EXPECT_LOG) - ut_assert_nextline("do_log_run() log %d", i); + ut_assert_nextline(" do_log_run() log %d", i); if (flags & EXPECT_DIRECT) - ut_assert_nextline("func() _log %d", i); + ut_assert_nextline(" func() _log %d", i); if (flags & EXPECT_DEBUG) { ut_assert_nextline("log %d", i); ut_assert_nextline("_log %d", i); @@ -72,11 +72,12 @@ static int do_check_log_entries(struct unit_test_state *uts, int flags, int min, } if (flags & EXPECT_EXTRA) for (; i <= LOGL_MAX ; i++) - ut_assert_nextline("func() _log %d", i); + ut_assert_nextline(" func() _log %d", i); for (i = LOGL_FIRST; i < LOGL_COUNT; i++) { if (flags & EXPECT_FORCE) - ut_assert_nextline("func() _log force %d", i); + ut_assert_nextline(" func() _log force %d", + i); if (flags & EXPECT_DEBUG) ut_assert_nextline("_log force %d", i); } @@ -277,7 +278,8 @@ int do_log_test_helpers(struct unit_test_state *uts) log_io("level %d\n", LOGL_DEBUG_IO); for (i = LOGL_EMERG; i <= _LOG_MAX_LEVEL; i++) - ut_assert_nextline("%s() level %d", __func__, i); + ut_assert_nextline("%*s() level %d", CONFIG_LOGF_FUNC_PAD, + __func__, i); ut_assert_console_end(); return 0; } @@ -297,14 +299,14 @@ int do_log_test_disable(struct unit_test_state *uts) { ut_assertok(console_record_reset_enable()); log_err("default\n"); - ut_assert_nextline("%s() default", __func__); + ut_assert_nextline("%*s() default", CONFIG_LOGF_FUNC_PAD, __func__); ut_assertok(log_device_set_enable(LOG_GET_DRIVER(console), false)); log_err("disabled\n"); ut_assertok(log_device_set_enable(LOG_GET_DRIVER(console), true)); log_err("enabled\n"); - ut_assert_nextline("%s() enabled", __func__); + ut_assert_nextline("%*s() enabled", CONFIG_LOGF_FUNC_PAD, __func__); ut_assert_console_end(); return 0; } diff --git a/test/test-main.c b/test/test-main.c index 7afe8741cf..3cdf6849c5 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -45,7 +45,7 @@ static int dm_test_pre_run(struct unit_test_state *uts) uts->force_fail_alloc = false; uts->skip_post_probe = false; gd->dm_root = NULL; - if (IS_ENABLED(CONFIG_UT_DM) && !CONFIG_IS_ENABLED(OF_PLATDATA)) + if (CONFIG_IS_ENABLED(UT_DM) && !CONFIG_IS_ENABLED(OF_PLATDATA)) memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); arch_reset_for_test(); diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index bc635aa00a..09e7b57198 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -1142,6 +1142,22 @@ adds a -v option to the call to binman:: make BINMAN_VERBOSE=5 +Building sections in parallel +----------------------------- + +By default binman uses multiprocessing to speed up compilation of large images. +This works at a section level, with one thread for each entry in the section. +This can speed things up if the entries are large and use compression. + +This feature can be disabled with the '-T' flag, which defaults to a suitable +value for your machine. This depends on the Python version, e.g on v3.8 it uses +12 threads on an 8-core machine. See ConcurrentFutures_ for more details. + +The special value -T0 selects single-threaded mode, useful for debugging during +development, since dealing with exceptions and problems in threads is more +difficult. This avoids any use of ThreadPoolExecutor. + + History / Credits ----------------- @@ -1190,3 +1206,5 @@ Some ideas: -- Simon Glass 7/7/2016 + +.. _ConcurrentFutures: https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.ThreadPoolExecutor diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 95f9ba27fb..d6156df408 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -32,6 +32,10 @@ controlled by a description in the board device tree.''' default=False, help='Display the README file') parser.add_argument('--toolpath', type=str, action='append', help='Add a path to the directories containing tools') + parser.add_argument('-T', '--threads', type=int, + default=None, help='Number of threads to use (0=single-thread)') + parser.add_argument('--test-section-timeout', action='store_true', + help='Use a zero timeout for section multi-threading (for testing)') parser.add_argument('-v', '--verbosity', default=1, type=int, help='Control verbosity: 0=silent, 1=warnings, 2=notices, ' '3=info, 4=detail, 5=debug') diff --git a/tools/binman/control.py b/tools/binman/control.py index f57e34daaa..dcba02ff7f 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -628,9 +628,13 @@ def Binman(args): tools.PrepareOutputDir(args.outdir, args.preserve) tools.SetToolPaths(args.toolpath) state.SetEntryArgs(args.entry_arg) + state.SetThreads(args.threads) images = PrepareImagesAndDtbs(dtb_fname, args.image, args.update_fdt, use_expanded) + if args.test_section_timeout: + # Set the first image to timeout, used in testThreadTimeout() + images[list(images.keys())[0]].test_section_timeout = True missing = False for image in images.values(): missing |= ProcessImage(image, args.update_fdt, args.map, @@ -642,6 +646,9 @@ def Binman(args): if missing: tout.Warning("\nSome images are invalid") + + # Use this to debug the time take to pack the image + #state.TimingShow() finally: tools.FinaliseOutputDir() finally: diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 018f8c9a31..fae86ca3ec 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -6,6 +6,7 @@ # from binman.entry import Entry +from binman import state from dtoc import fdt_util from patman import tools from patman import tout @@ -59,8 +60,12 @@ class Entry_blob(Entry): the data in chunks and avoid reading it all at once. For now this seems like an unnecessary complication. """ + state.TimingStart('read') indata = tools.ReadFile(self._pathname) + state.TimingAccum('read') + state.TimingStart('compress') data = self.CompressData(indata) + state.TimingAccum('compress') self.SetContents(data) return True diff --git a/tools/binman/etype/collection.py b/tools/binman/etype/collection.py index 1625575fe9..442b40b48b 100644 --- a/tools/binman/etype/collection.py +++ b/tools/binman/etype/collection.py @@ -40,7 +40,7 @@ class Entry_collection(Entry): """ # Join up all the data self.Info('Getting contents, required=%s' % required) - data = b'' + data = bytearray() for entry_phandle in self.content: entry_data = self.section.GetContentsByPhandle(entry_phandle, self, required) diff --git a/tools/binman/etype/files.py b/tools/binman/etype/files.py index 5db36abef0..9b04a496a8 100644 --- a/tools/binman/etype/files.py +++ b/tools/binman/etype/files.py @@ -34,6 +34,9 @@ class Entry_files(Entry_section): from binman import state super().__init__(section, etype, node) + + def ReadNode(self): + super().ReadNode() self._pattern = fdt_util.GetString(self._node, 'pattern') if not self._pattern: self.Raise("Missing 'pattern' property") diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index c3bac026c1..e2949fc916 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -9,10 +9,12 @@ images to be created. """ from collections import OrderedDict +import concurrent.futures import re import sys from binman.entry import Entry +from binman import state from dtoc import fdt_util from patman import tools from patman import tout @@ -164,7 +166,7 @@ class Entry_section(Entry): pad_byte = (entry._pad_byte if isinstance(entry, Entry_section) else self._pad_byte) - data = b'' + data = bytearray() # Handle padding before the entry if entry.pad_before: data += tools.GetBytes(self._pad_byte, entry.pad_before) @@ -198,7 +200,7 @@ class Entry_section(Entry): Returns: Contents of the section (bytes) """ - section_data = b'' + section_data = bytearray() for entry in self._entries.values(): entry_data = entry.GetData(required) @@ -525,15 +527,43 @@ class Entry_section(Entry): def GetEntryContents(self): """Call ObtainContents() for each entry in the section """ + def _CheckDone(entry): + if not entry.ObtainContents(): + next_todo.append(entry) + return entry + todo = self._entries.values() for passnum in range(3): + threads = state.GetThreads() next_todo = [] - for entry in todo: - if not entry.ObtainContents(): - next_todo.append(entry) + + if threads == 0: + for entry in todo: + _CheckDone(entry) + else: + with concurrent.futures.ThreadPoolExecutor( + max_workers=threads) as executor: + future_to_data = { + entry: executor.submit(_CheckDone, entry) + for entry in todo} + timeout = 60 + if self.GetImage().test_section_timeout: + timeout = 0 + done, not_done = concurrent.futures.wait( + future_to_data.values(), timeout=timeout) + # Make sure we check the result, so any exceptions are + # generated. Check the results in entry order, since tests + # may expect earlier entries to fail first. + for entry in todo: + job = future_to_data[entry] + job.result() + if not_done: + self.Raise('Timed out obtaining contents') + todo = next_todo if not todo: break + if todo: self.Raise('Internal error: Could not complete processing of contents: remaining %s' % todo) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5383eec489..cea3ebf2b9 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -308,7 +308,8 @@ class TestFunctional(unittest.TestCase): def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, entry_args=None, images=None, use_real_dtb=False, use_expanded=False, verbosity=None, allow_missing=False, - extra_indirs=None): + extra_indirs=None, threads=None, + test_section_timeout=False): """Run binman with a given test file Args: @@ -331,6 +332,8 @@ class TestFunctional(unittest.TestCase): allow_missing: Set the '--allow-missing' flag so that missing external binaries just produce a warning instead of an error extra_indirs: Extra input directories to add using -I + threads: Number of threads to use (None for default, 0 for + single-threaded) """ args = [] if debug: @@ -342,6 +345,10 @@ class TestFunctional(unittest.TestCase): if self.toolpath: for path in self.toolpath: args += ['--toolpath', path] + if threads is not None: + args.append('-T%d' % threads) + if test_section_timeout: + args.append('--test-section-timeout') args += ['build', '-p', '-I', self._indir, '-d', self.TestFile(fname)] if map: args.append('-m') @@ -412,7 +419,7 @@ class TestFunctional(unittest.TestCase): def _DoReadFileDtb(self, fname, use_real_dtb=False, use_expanded=False, map=False, update_dtb=False, entry_args=None, - reset_dtbs=True, extra_indirs=None): + reset_dtbs=True, extra_indirs=None, threads=None): """Run binman and return the resulting image This runs binman with a given test file and then reads the resulting @@ -439,6 +446,8 @@ class TestFunctional(unittest.TestCase): function. If reset_dtbs is True, then the original test dtb is written back before this function finishes extra_indirs: Extra input directories to add using -I + threads: Number of threads to use (None for default, 0 for + single-threaded) Returns: Tuple: @@ -463,7 +472,8 @@ class TestFunctional(unittest.TestCase): try: retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, entry_args=entry_args, use_real_dtb=use_real_dtb, - use_expanded=use_expanded, extra_indirs=extra_indirs) + use_expanded=use_expanded, extra_indirs=extra_indirs, + threads=threads) self.assertEqual(0, retcode) out_dtb_fname = tools.GetOutputFilename('u-boot.dtb.out') @@ -4542,5 +4552,30 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('201_opensbi.dts') self.assertEqual(OPENSBI_DATA, data[:len(OPENSBI_DATA)]) + def testSectionsSingleThread(self): + """Test sections without multithreading""" + data = self._DoReadFileDtb('055_sections.dts', threads=0)[0] + expected = (U_BOOT_DATA + tools.GetBytes(ord('!'), 12) + + U_BOOT_DATA + tools.GetBytes(ord('a'), 12) + + U_BOOT_DATA + tools.GetBytes(ord('&'), 4)) + self.assertEqual(expected, data) + + def testThreadTimeout(self): + """Test handling a thread that takes too long""" + with self.assertRaises(ValueError) as e: + self._DoTestFile('202_section_timeout.dts', + test_section_timeout=True) + self.assertIn("Node '/binman/section@0': Timed out obtaining contents", + str(e.exception)) + + def testTiming(self): + """Test output of timing information""" + data = self._DoReadFile('055_sections.dts') + with test_util.capture_sys_output() as (stdout, stderr): + state.TimingShow() + self.assertIn('read:', stdout.getvalue()) + self.assertIn('compress:', stdout.getvalue()) + + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/image.py b/tools/binman/image.py index 10778f47fe..cdc58b39a4 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -36,6 +36,8 @@ class Image(section.Entry_section): fdtmap_data: Contents of the fdtmap when loading from a file allow_repack: True to add properties to allow the image to be safely repacked later + test_section_timeout: Use a zero timeout for section multi-threading + (for testing) Args: copy_to_orig: Copy offset/size to orig_offset/orig_size after reading @@ -74,6 +76,7 @@ class Image(section.Entry_section): self.allow_repack = False self._ignore_missing = ignore_missing self.use_expanded = use_expanded + self.test_section_timeout = False if not test: self.ReadNode() diff --git a/tools/binman/state.py b/tools/binman/state.py index dfb1760455..9e5b8a3931 100644 --- a/tools/binman/state.py +++ b/tools/binman/state.py @@ -5,8 +5,11 @@ # Holds and modifies the state information held by binman # +from collections import defaultdict import hashlib import re +import time +import threading from dtoc import fdt import os @@ -55,6 +58,30 @@ allow_entry_expansion = True # to the new ones, the compressed size increases, etc. allow_entry_contraction = False +# Number of threads to use for binman (None means machine-dependent) +num_threads = None + + +class Timing: + """Holds information about an operation that is being timed + + Properties: + name: Operation name (only one of each name is stored) + start: Start time of operation in seconds (None if not start) + accum:: Amount of time spent on this operation so far, in seconds + """ + def __init__(self, name): + self.name = name + self.start = None # cause an error if TimingStart() is not called + self.accum = 0.0 + + +# Holds timing info for each name: +# key: name of Timing info (Timing.name) +# value: Timing object +timing_info = {} + + def GetFdtForEtype(etype): """Get the Fdt object for a particular device-tree entry @@ -420,3 +447,71 @@ def AllowEntryContraction(): raised """ return allow_entry_contraction + +def SetThreads(threads): + """Set the number of threads to use when building sections + + Args: + threads: Number of threads to use (None for default, 0 for + single-threaded) + """ + global num_threads + + num_threads = threads + +def GetThreads(): + """Get the number of threads to use when building sections + + Returns: + Number of threads to use (None for default, 0 for single-threaded) + """ + return num_threads + +def GetTiming(name): + """Get the timing info for a particular operation + + The object is created if it does not already exist. + + Args: + name: Operation name to get + + Returns: + Timing object for the current thread + """ + threaded_name = '%s:%d' % (name, threading.get_ident()) + timing = timing_info.get(threaded_name) + if not timing: + timing = Timing(threaded_name) + timing_info[threaded_name] = timing + return timing + +def TimingStart(name): + """Start the timer for an operation + + Args: + name: Operation name to start + """ + timing = GetTiming(name) + timing.start = time.monotonic() + +def TimingAccum(name): + """Stop and accumlate the time for an operation + + This measures the time since the last TimingStart() and adds that to the + accumulated time. + + Args: + name: Operation name to start + """ + timing = GetTiming(name) + timing.accum += time.monotonic() - timing.start + +def TimingShow(): + """Show all timing information""" + duration = defaultdict(float) + for threaded_name, timing in timing_info.items(): + name = threaded_name.split(':')[0] + duration[name] += timing.accum + + for name, seconds in duration.items(): + print('%10s: %10.1fms' % (name, seconds * 1000)) diff --git a/tools/binman/test/202_section_timeout.dts b/tools/binman/test/202_section_timeout.dts new file mode 100644 index 0000000000..1481450367 --- /dev/null +++ b/tools/binman/test/202_section_timeout.dts @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + pad-byte = <0x26>; + size = <0x28>; + section@0 { + read-only; + size = <0x10>; + pad-byte = <0x21>; + + u-boot { + }; + }; + }; +}; diff --git a/tools/dtoc/main.py b/tools/dtoc/main.py index 93706de89b..6f9b526bd7 100755 --- a/tools/dtoc/main.py +++ b/tools/dtoc/main.py @@ -21,7 +21,7 @@ options. For more information about the use of this options and tool please see doc/driver-model/of-plat.rst """ -from optparse import OptionParser +from argparse import ArgumentParser import os import sys import unittest @@ -51,7 +51,7 @@ def run_tests(processes, args): result = unittest.TestResult() sys.argv = [sys.argv[0]] - test_name = args and args[0] or None + test_name = args.files and args.files[0] or None test_dtoc.setup() @@ -66,47 +66,50 @@ def RunTestCoverage(): """Run the tests and check that we get 100% coverage""" sys.argv = [sys.argv[0]] test_util.RunTestCoverage('tools/dtoc/dtoc', '/main.py', - ['tools/patman/*.py', '*/fdt*', '*test*'], options.build_dir) + ['tools/patman/*.py', '*/fdt*', '*test*'], args.build_dir) if __name__ != '__main__': sys.exit(1) -parser = OptionParser() -parser.add_option('-B', '--build-dir', type='string', default='b', +epilog = '''Generate C code from devicetree files. See of-plat.rst for details''' + +parser = ArgumentParser(epilog=epilog) +parser.add_argument('-B', '--build-dir', type=str, default='b', help='Directory containing the build output') -parser.add_option('-c', '--c-output-dir', action='store', +parser.add_argument('-c', '--c-output-dir', action='store', help='Select output directory for C files') -parser.add_option('-C', '--h-output-dir', action='store', +parser.add_argument('-C', '--h-output-dir', action='store', help='Select output directory for H files (defaults to --c-output-di)') -parser.add_option('-d', '--dtb-file', action='store', +parser.add_argument('-d', '--dtb-file', action='store', help='Specify the .dtb input file') -parser.add_option('-i', '--instantiate', action='store_true', default=False, +parser.add_argument('-i', '--instantiate', action='store_true', default=False, help='Instantiate devices to avoid needing device_bind()') -parser.add_option('--include-disabled', action='store_true', +parser.add_argument('--include-disabled', action='store_true', help='Include disabled nodes') -parser.add_option('-o', '--output', action='store', +parser.add_argument('-o', '--output', action='store', help='Select output filename') -parser.add_option('-p', '--phase', type=str, +parser.add_argument('-p', '--phase', type=str, help='set phase of U-Boot this invocation is for (spl/tpl)') -parser.add_option('-P', '--processes', type=int, +parser.add_argument('-P', '--processes', type=int, help='set number of processes to use for running tests') -parser.add_option('-t', '--test', action='store_true', dest='test', +parser.add_argument('-t', '--test', action='store_true', dest='test', default=False, help='run tests') -parser.add_option('-T', '--test-coverage', action='store_true', - default=False, help='run tests and check for 100% coverage') -(options, args) = parser.parse_args() +parser.add_argument('-T', '--test-coverage', action='store_true', + default=False, help='run tests and check for 100%% coverage') +parser.add_argument('files', nargs='*') +args = parser.parse_args() # Run our meagre tests -if options.test: - ret_code = run_tests(options.processes, args) +if args.test: + ret_code = run_tests(args.processes, args) sys.exit(ret_code) -elif options.test_coverage: +elif args.test_coverage: RunTestCoverage() else: - dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled, - options.output, - [options.c_output_dir, options.h_output_dir], - options.phase, instantiate=options.instantiate) + dtb_platdata.run_steps(args.files, args.dtb_file, args.include_disabled, + args.output, + [args.c_output_dir, args.h_output_dir], + args.phase, instantiate=args.instantiate) diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py index 2db96884c8..3bef59d616 100644 --- a/tools/dtoc/src_scan.py +++ b/tools/dtoc/src_scan.py @@ -13,6 +13,7 @@ U_BOOT_DRIVER(), UCLASS_DRIVER and all struct declarations in header files. See doc/driver-model/of-plat.rst for more informaiton """ +import collections import os import re import sys @@ -190,6 +191,9 @@ class Scanner: value: Driver name declared with U_BOOT_DRIVER(driver_name) _drivers_additional (list or str): List of additional drivers to use during scanning + _warnings: Dict of warnings found: + key: Driver name + value: Set of warnings _of_match: Dict holding information about compatible strings key: Name of struct udevice_id variable value: Dict of compatible info in that variable: @@ -217,6 +221,7 @@ class Scanner: self._driver_aliases = {} self._drivers_additional = drivers_additional or [] self._missing_drivers = set() + self._warnings = collections.defaultdict(set) self._of_match = {} self._compat_to_driver = {} self._uclass = {} @@ -267,7 +272,10 @@ class Scanner: aliases_c.remove(compat_c) return compat_c, aliases_c - self._missing_drivers.add(compat_list_c[0]) + name = compat_list_c[0] + self._missing_drivers.add(name) + self._warnings[name].add( + 'WARNING: the driver %s was not found in the driver list' % name) return compat_list_c[0], compat_list_c[1:] @@ -444,8 +452,8 @@ class Scanner: # Collect the compatible string, e.g. 'rockchip,rk3288-grf' compat = None - re_compat = re.compile(r'{\s*.compatible\s*=\s*"(.*)"\s*' - r'(,\s*.data\s*=\s*(\S*))?\s*},') + re_compat = re.compile(r'{\s*\.compatible\s*=\s*"(.*)"\s*' + r'(,\s*\.data\s*=\s*(\S*))?\s*},') # This is a dict of compatible strings that were found: # key: Compatible string, e.g. 'rockchip,rk3288-grf' @@ -460,7 +468,7 @@ class Scanner: # Matches the references to the udevice_id list re_of_match = re.compile( - r'\.of_match\s*=\s*(of_match_ptr\()?([a-z0-9_]+)(\))?,') + r'\.of_match\s*=\s*(of_match_ptr\()?([a-z0-9_]+)([^,]*),') re_phase = re.compile('^\s*DM_PHASE\((.*)\).*$') re_hdr = re.compile('^\s*DM_HEADER\((.*)\).*$') @@ -506,6 +514,11 @@ class Scanner: driver.uclass_id = m_id.group(1) elif m_of_match: compat = m_of_match.group(2) + suffix = m_of_match.group(3) + if suffix and suffix != ')': + self._warnings[driver.name].add( + "%s: Warning: unexpected suffix '%s' on .of_match line for compat '%s'" % + (fname, suffix, compat)) elif m_phase: driver.phase = m_phase.group(1) elif m_hdr: @@ -533,7 +546,12 @@ class Scanner: # The driver does not have a uclass or compat string. # The first is required but the second is not, so just # ignore this. - pass + if not driver.uclass_id: + warn = 'Missing .uclass' + else: + warn = 'Missing .compatible' + self._warnings[driver.name].add('%s in %s' % + (warn, fname)) driver = None ids_name = None compat = None @@ -555,7 +573,7 @@ class Scanner: if ids_m: ids_name = ids_m.group(1) elif m_alias: - self._driver_aliases[m_alias[2]] = m_alias[1] + self._driver_aliases[m_alias.group(2)] = m_alias.group(1) # Make the updates based on what we found for driver in drivers.values(): @@ -577,9 +595,18 @@ class Scanner: def show_warnings(self): """Show any warnings that have been collected""" - for name in sorted(list(self._missing_drivers)): - print('WARNING: the driver %s was not found in the driver list' - % name) + used_drivers = [drv.name for drv in self._drivers.values() if drv.used] + missing = self._missing_drivers.copy() + for name in sorted(self._warnings.keys()): + if name in missing or name in used_drivers: + warns = sorted(list(self._warnings[name])) + print('%s: %s' % (name, warns[0])) + indent = ' ' * len(name) + for warn in warns[1:]: + print('%-s: %s' % (indent, warn)) + if name in missing: + missing.remove(name) + print() def scan_driver(self, fname): """Scan a driver file to build a list of driver names and aliases diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py index d6da03849f..f03cf8ed7c 100644 --- a/tools/dtoc/test_src_scan.py +++ b/tools/dtoc/test_src_scan.py @@ -20,6 +20,9 @@ from patman import tools OUR_PATH = os.path.dirname(os.path.realpath(__file__)) +EXPECT_WARN = {'rockchip_rk3288_grf': + {'WARNING: the driver rockchip_rk3288_grf was not found in the driver list'}} + class FakeNode: """Fake Node object for testing""" def __init__(self): @@ -152,6 +155,7 @@ class TestSrcScan(unittest.TestCase): 'nvidia,tegra20-i2c-dvc': 'TYPE_DVC'}, drv.compat) self.assertEqual('i2c_bus', drv.priv) self.assertEqual(1, len(scan._drivers)) + self.assertEqual({}, scan._warnings) def test_normalized_name(self): """Test operation of get_normalized_compat_name()""" @@ -171,8 +175,8 @@ class TestSrcScan(unittest.TestCase): self.assertEqual([], aliases) self.assertEqual(1, len(scan._missing_drivers)) self.assertEqual({'rockchip_rk3288_grf'}, scan._missing_drivers) - #'WARNING: the driver rockchip_rk3288_grf was not found in the driver list', - #stdout.getvalue().strip()) + self.assertEqual('', stdout.getvalue().strip()) + self.assertEqual(EXPECT_WARN, scan._warnings) i2c = 'I2C_UCLASS' compat = {'rockchip,rk3288-grf': 'ROCKCHIP_SYSCON_GRF', @@ -189,6 +193,7 @@ class TestSrcScan(unittest.TestCase): self.assertEqual('', stdout.getvalue().strip()) self.assertEqual('rockchip_rk3288_grf', name) self.assertEqual([], aliases) + self.assertEqual(EXPECT_WARN, scan._warnings) prop.value = 'rockchip,rk3288-srf' with test_util.capture_sys_output() as (stdout, _): @@ -196,6 +201,7 @@ class TestSrcScan(unittest.TestCase): self.assertEqual('', stdout.getvalue().strip()) self.assertEqual('rockchip_rk3288_grf', name) self.assertEqual(['rockchip_rk3288_srf'], aliases) + self.assertEqual(EXPECT_WARN, scan._warnings) def test_scan_errors(self): """Test detection of scanning errors""" @@ -490,3 +496,126 @@ U_BOOT_DRIVER(%s) = { self.assertEqual(3, seq) self.assertEqual({'mypath': 3}, uc.alias_path_to_num) self.assertEqual({2: node, 3: node}, uc.alias_num_to_node) + + def test_scan_warnings(self): + """Test detection of scanning warnings""" + buff = ''' +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .id = UCLASS_I2C, + .of_match = tegra_i2c_ids + 1, +}; +''' + # The '+ 1' above should generate a warning + + prop = FakeProp() + prop.name = 'compatible' + prop.value = 'rockchip,rk3288-grf' + node = FakeNode() + node.props = {'compatible': prop} + + # get_normalized_compat_name() uses this to check for root node + node.parent = FakeNode() + + scan = src_scan.Scanner(None, None) + scan._parse_driver('file.c', buff) + self.assertEqual( + {'i2c_tegra': + {"file.c: Warning: unexpected suffix ' + 1' on .of_match line for compat 'tegra_i2c_ids'"}}, + scan._warnings) + + tprop = FakeProp() + tprop.name = 'compatible' + tprop.value = 'nvidia,tegra114-i2c' + tnode = FakeNode() + tnode.props = {'compatible': tprop} + + # get_normalized_compat_name() uses this to check for root node + tnode.parent = FakeNode() + + with test_util.capture_sys_output() as (stdout, _): + scan.get_normalized_compat_name(node) + scan.get_normalized_compat_name(tnode) + self.assertEqual('', stdout.getvalue().strip()) + + self.assertEqual(2, len(scan._missing_drivers)) + self.assertEqual({'rockchip_rk3288_grf', 'nvidia_tegra114_i2c'}, + scan._missing_drivers) + with test_util.capture_sys_output() as (stdout, _): + scan.show_warnings() + self.assertIn('rockchip_rk3288_grf', stdout.getvalue()) + + # This should show just the rockchip warning, since the tegra driver + # is not in self._missing_drivers + scan._missing_drivers.remove('nvidia_tegra114_i2c') + with test_util.capture_sys_output() as (stdout, _): + scan.show_warnings() + self.assertIn('rockchip_rk3288_grf', stdout.getvalue()) + self.assertNotIn('tegra_i2c_ids', stdout.getvalue()) + + # Do a similar thing with used drivers. By marking the tegra driver as + # used, the warning related to that driver will be shown + drv = scan._drivers['i2c_tegra'] + drv.used = True + with test_util.capture_sys_output() as (stdout, _): + scan.show_warnings() + self.assertIn('rockchip_rk3288_grf', stdout.getvalue()) + self.assertIn('tegra_i2c_ids', stdout.getvalue()) + + # Add a warning to make sure multiple warnings are shown + scan._warnings['i2c_tegra'].update( + scan._warnings['nvidia_tegra114_i2c']) + del scan._warnings['nvidia_tegra114_i2c'] + with test_util.capture_sys_output() as (stdout, _): + scan.show_warnings() + self.assertEqual('''i2c_tegra: WARNING: the driver nvidia_tegra114_i2c was not found in the driver list + : file.c: Warning: unexpected suffix ' + 1' on .of_match line for compat 'tegra_i2c_ids' + +rockchip_rk3288_grf: WARNING: the driver rockchip_rk3288_grf was not found in the driver list + +''', + stdout.getvalue()) + self.assertIn('tegra_i2c_ids', stdout.getvalue()) + + def scan_uclass_warning(self): + """Test a missing .uclass in the driver""" + buff = ''' +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .of_match = tegra_i2c_ids, +}; +''' + scan = src_scan.Scanner(None, None) + scan._parse_driver('file.c', buff) + self.assertEqual( + {'i2c_tegra': {'Missing .uclass in file.c'}}, + scan._warnings) + + def scan_compat_warning(self): + """Test a missing .compatible in the driver""" + buff = ''' +static const struct udevice_id tegra_i2c_ids[] = { + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 }, + { } +}; + +U_BOOT_DRIVER(i2c_tegra) = { + .name = "i2c_tegra", + .id = UCLASS_I2C, +}; +''' + scan = src_scan.Scanner(None, None) + scan._parse_driver('file.c', buff) + self.assertEqual( + {'i2c_tegra': {'Missing .compatible in file.c'}}, + scan._warnings) diff --git a/tools/patman/cros_subprocess.py b/tools/patman/cros_subprocess.py index efd0a5aaf7..fdd5138685 100644 --- a/tools/patman/cros_subprocess.py +++ b/tools/patman/cros_subprocess.py @@ -169,11 +169,11 @@ class Popen(subprocess.Popen): self.stdin.close() if self.stdout: read_set.append(self.stdout) - stdout = b'' + stdout = bytearray() if self.stderr and self.stderr != self.stdout: read_set.append(self.stderr) - stderr = b'' - combined = b'' + stderr = bytearray() + combined = bytearray() input_offset = 0 while read_set or write_set: diff --git a/tools/patman/tools.py b/tools/patman/tools.py index ec95a543bd..877e37cd8d 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -466,6 +466,9 @@ def Compress(indata, algo, with_header=True): This requires 'lz4' and 'lzma_alone' tools. It also requires an output directory to be previously set up, by calling PrepareOutputDir(). + Care is taken to use unique temporary files so that this function can be + called from multiple threads. + Args: indata: Input data to compress algo: Algorithm to use ('none', 'gzip', 'lz4' or 'lzma') @@ -475,14 +478,16 @@ def Compress(indata, algo, with_header=True): """ if algo == 'none': return indata - fname = GetOutputFilename('%s.comp.tmp' % algo) + fname = tempfile.NamedTemporaryFile(prefix='%s.comp.tmp' % algo, + dir=outdir).name WriteFile(fname, indata) if algo == 'lz4': data = Run('lz4', '--no-frame-crc', '-B4', '-5', '-c', fname, binary=True) # cbfstool uses a very old version of lzma elif algo == 'lzma': - outfname = GetOutputFilename('%s.comp.otmp' % algo) + outfname = tempfile.NamedTemporaryFile(prefix='%s.comp.otmp' % algo, + dir=outdir).name Run('lzma_alone', 'e', fname, outfname, '-lc1', '-lp0', '-pb0', '-d8') data = ReadFile(outfname) elif algo == 'gzip':