[PATCH v4 00/23] Fix various bugs
This series includes the patches needed to make make the EFI 'boot' test work. That test has now been split off into a separate series along with the EFI patches.
This series fixes these problems: - sandbox memory-mapping conflict with PCI - the fix for that causes the mbr test to crash as it sets up pointers instead of addresses for its 'mmc' commands - the mmc and read commands which cast addresses to pointers - a tricky bug to do with USB keyboard and stdio - a few other minor things
Note that I would like to get this series applied before the change to running unprivileged, since it has been almost a month and it will be a trivial change once that is applied.
Changes in v4: - Rebase to -next (on top of testc series) - Drop patch to add UT_TESTF_CONSOLE_REC to bootflow tests
Changes in v3: - Include the usb.h header file in all cases - Correct the commit subject and message - Add a Fixes tag - use SZ_512 instead of 0x200
Changes in v2: - Add many new patches to resolve all the outstanding test issues
Simon Glass (23): nvmxip: Drop the message on probe nvmxip: Avoid probing on boot test/py: Fix some pylint warnings in test_ut.py scripts: Update pylint.base bootstd: Create a function to reset USB usb: Drop old non-DM code log: Add a new log category for the console usb: Add DEV_FLAGS_DM to stdio for USB keyboard dm: usb: Deal with USB keyboard persisting across tests test: mbr: Adjust test to use lower-case hex test: mbr: Adjust test to drop 0x sandbox: Change the range used for memory-mapping tags sandbox: Update cpu to use logging sandbox: Unmap old tags sandbox: Add some debugging to pci_io sandbox: Implement reference counting for address mapping mmc: Use map_sysmem() with buffers in the mmc command read: Tidy up use of map_sysmem() in the read command cmd: Fix memory-mapping in cmp command test: mbr: Unmap the buffers after use test: mbr: Use a constant for the block size test: mbr: Use RAM for the buffers test: mbr: Drop a duplicate test
arch/sandbox/cpu/cpu.c | 38 ++- arch/sandbox/cpu/state.c | 9 +- arch/sandbox/include/asm/state.h | 3 + arch/sandbox/lib/pci_io.c | 9 +- cmd/mem.c | 26 +- cmd/mmc.c | 15 +- cmd/read.c | 10 +- cmd/usb.c | 20 -- common/console.c | 36 +++ common/log.c | 1 + common/usb_kbd.c | 74 +---- doc/arch/sandbox/sandbox.rst | 21 +- drivers/mtd/nvmxip/nvmxip-uclass.c | 10 +- drivers/usb/Kconfig | 3 +- include/console.h | 8 + include/log.h | 2 + include/usb.h | 20 +- scripts/pylint.base | 462 +++++++++++++++++------------ test/boot/bootdev.c | 19 +- test/boot/bootstd_common.c | 6 + test/boot/bootstd_common.h | 8 + test/cmd/mbr.c | 172 +++++------ test/py/tests/test_ut.py | 94 +++--- test/test-main.c | 38 +++ 24 files changed, 620 insertions(+), 484 deletions(-)
We should not need to announce this device. Drop the message.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/mtd/nvmxip/nvmxip-uclass.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c index 254f04e0b99..58e8c3fb74b 100644 --- a/drivers/mtd/nvmxip/nvmxip-uclass.c +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c @@ -47,7 +47,8 @@ int nvmxip_probe(struct udevice *udev) return ret; }
- log_info("[%s]: the block device %s ready for use\n", udev->name, bdev_name); + log_debug("[%s]: the block device %s ready for use\n", udev->name, + bdev_name);
return 0; }
Devices should be probed when they are used, not before. Drop this boot-time probing.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/mtd/nvmxip/nvmxip-uclass.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c index 58e8c3fb74b..d18bd0e3d6b 100644 --- a/drivers/mtd/nvmxip/nvmxip-uclass.c +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c @@ -53,14 +53,7 @@ int nvmxip_probe(struct udevice *udev) return 0; }
-static int nvmxip_post_bind(struct udevice *udev) -{ - dev_or_flags(udev, DM_FLAG_PROBE_AFTER_BIND); - return 0; -} - UCLASS_DRIVER(nvmxip) = { .name = "nvmxip", .id = UCLASS_NVMXIP, - .post_bind = nvmxip_post_bind, };
On Sun, Sep 01, 2024 at 04:26:13PM -0600, Simon Glass wrote:
Devices should be probed when they are used, not before. Drop this boot-time probing.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/mtd/nvmxip/nvmxip-uclass.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c index 58e8c3fb74b..d18bd0e3d6b 100644 --- a/drivers/mtd/nvmxip/nvmxip-uclass.c +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c @@ -53,14 +53,7 @@ int nvmxip_probe(struct udevice *udev) return 0; }
-static int nvmxip_post_bind(struct udevice *udev) -{
- dev_or_flags(udev, DM_FLAG_PROBE_AFTER_BIND);
- return 0;
-}
UCLASS_DRIVER(nvmxip) = { .name = "nvmxip", .id = UCLASS_NVMXIP,
- .post_bind = nvmxip_post_bind,
};
Once again, adding in the corstone1000 maintainers as the platform that actually uses this.
Hi Tom,
On Mon, 2 Sept 2024 at 10:10, Tom Rini trini@konsulko.com wrote:
On Sun, Sep 01, 2024 at 04:26:13PM -0600, Simon Glass wrote:
Devices should be probed when they are used, not before. Drop this boot-time probing.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/mtd/nvmxip/nvmxip-uclass.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c index 58e8c3fb74b..d18bd0e3d6b 100644 --- a/drivers/mtd/nvmxip/nvmxip-uclass.c +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c @@ -53,14 +53,7 @@ int nvmxip_probe(struct udevice *udev) return 0; }
-static int nvmxip_post_bind(struct udevice *udev) -{
dev_or_flags(udev, DM_FLAG_PROBE_AFTER_BIND);return 0;-}
UCLASS_DRIVER(nvmxip) = { .name = "nvmxip", .id = UCLASS_NVMXIP,
.post_bind = nvmxip_post_bind,};
Once again, adding in the corstone1000 maintainers as the platform that actually uses this.
Just to mention that Abdellatif was added automatically by patman for the first version, but if there is no reply, after v2 I tend to drop it back, to avoid sending too many emails.
Regards, Simon
Hi Simon, Tom,
Devices should be probed when they are used, not before. Drop this boot-time probing.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
drivers/mtd/nvmxip/nvmxip-uclass.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c index 58e8c3fb74b..d18bd0e3d6b 100644 --- a/drivers/mtd/nvmxip/nvmxip-uclass.c +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c @@ -53,14 +53,7 @@ int nvmxip_probe(struct udevice *udev) return 0; }
-static int nvmxip_post_bind(struct udevice *udev) -{
dev_or_flags(udev, DM_FLAG_PROBE_AFTER_BIND);return 0;-}
UCLASS_DRIVER(nvmxip) = { .name = "nvmxip", .id = UCLASS_NVMXIP,
.post_bind = nvmxip_post_bind,};
Once again, adding in the corstone1000 maintainers as the platform that actually uses this.
Just to mention that Abdellatif was added automatically by patman for the first version, but if there is no reply, after v2 I tend to drop it back, to avoid sending too many emails.
I'm happy to stay in the loop.
Regards, Simon
Cheers Abdellatif
Tidy up most of these warnings. Remaining are four of these:
R0914: Too many local variables
which can only by fixed by splitting things into functions, so that is left for another time.
Part of this change was done by the flynt tool.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_ut.py | 94 ++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 46 deletions(-)
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index 05e15830590..39aa1035e34 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -1,6 +1,12 @@ # SPDX-License-Identifier: GPL-2.0 -# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. +""" +Unit-test runner + +Provides a test_ut() function which is used by conftest.py to run each unit +test one at a time, as well setting up some files needed by the tests.
+# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved. +""" import collections import getpass import gzip @@ -44,8 +50,8 @@ def setup_image(cons, mmc_dev, part_type, second_part=False): if second_part: spec += '\ntype=c'
- u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname) - u_boot_utils.run_and_log(cons, 'sudo sfdisk %s' % fname, + u_boot_utils.run_and_log(cons, f'qemu-img create {fname} 20M') + u_boot_utils.run_and_log(cons, f'sudo sfdisk {fname}', stdin=spec.encode('utf-8')) return fname, mnt
@@ -61,13 +67,13 @@ def mount_image(cons, fname, mnt, fstype): Returns: str: Name of loop device used """ - out = u_boot_utils.run_and_log(cons, 'sudo losetup --show -f -P %s' % fname) + out = u_boot_utils.run_and_log(cons, f'sudo losetup --show -f -P {fname}') loop = out.strip() part = f'{loop}p1' u_boot_utils.run_and_log(cons, f'sudo mkfs.{fstype} {part}') opts = '' if fstype == 'vfat': - opts += f' -o uid={os.getuid()},gid={os.getgid()}' + opts += f' -o uid={os.getuid()},gid={os.getgid()}' u_boot_utils.run_and_log(cons, f'sudo mount -o loop {part} {mnt}{opts}') u_boot_utils.run_and_log(cons, f'sudo chown {getpass.getuser()} {mnt}') return loop @@ -82,9 +88,7 @@ def copy_prepared_image(cons, mmc_dev, fname): """ infname = os.path.join(cons.config.source_dir, f'test/py/tests/bootstd/mmc{mmc_dev}.img.xz') - u_boot_utils.run_and_log( - cons, - ['sh', '-c', 'xz -dc %s >%s' % (infname, fname)]) + u_boot_utils.run_and_log(cons, ['sh', '-c', f'xz -dc {infname} >{fname}'])
def setup_bootmenu_image(cons): """Create a 20MB disk image with a single ext4 partition @@ -101,9 +105,6 @@ def setup_bootmenu_image(cons): loop = mount_image(cons, fname, mnt, 'ext4') mounted = True
- vmlinux = 'Image' - initrd = 'uInitrd' - dtbdir = 'dtb' script = '''# DO NOT EDIT THIS FILE # # Please edit /boot/armbianEnv.txt to set supported parameters @@ -177,12 +178,12 @@ booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r}
# Recompile with: # mkimage -C none -A arm -T script -d /boot/boot.cmd /boot/boot.scr -''' % (mmc_dev) +''' bootdir = os.path.join(mnt, 'boot') mkdir_cond(bootdir) cmd_fname = os.path.join(bootdir, 'boot.cmd') scr_fname = os.path.join(bootdir, 'boot.scr') - with open(cmd_fname, 'w') as outf: + with open(cmd_fname, 'w', encoding='ascii') as outf: print(script, file=outf)
infname = os.path.join(cons.config.source_dir, @@ -212,13 +213,12 @@ booti ${kernel_addr_r} ${ramdisk_addr_r} ${fdt_addr_r} complete = True
except ValueError as exc: - print('Falled to create image, failing back to prepared copy: %s', - str(exc)) + print(f'Falled to create image, failing back to prepared copy: {exc}') finally: if mounted: - u_boot_utils.run_and_log(cons, 'sudo umount --lazy %s' % mnt) + u_boot_utils.run_and_log(cons, f'sudo umount --lazy {mnt}') if loop: - u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop) + u_boot_utils.run_and_log(cons, f'sudo losetup -d {loop}')
if not complete: copy_prepared_image(cons, mmc_dev, fname) @@ -254,32 +254,32 @@ label Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl) ext = os.path.join(mnt, 'extlinux') mkdir_cond(ext)
- with open(os.path.join(ext, 'extlinux.conf'), 'w') as fd: + conf = os.path.join(ext, 'extlinux.conf') + with open(conf, 'w', encoding='ascii') as fd: print(script, file=fd)
inf = os.path.join(cons.config.persistent_data_dir, 'inf') with open(inf, 'wb') as fd: fd.write(gzip.compress(b'vmlinux')) - u_boot_utils.run_and_log(cons, 'mkimage -f auto -d %s %s' % - (inf, os.path.join(mnt, vmlinux))) + u_boot_utils.run_and_log( + cons, f'mkimage -f auto -d {inf} {os.path.join(mnt, vmlinux)}')
- with open(os.path.join(mnt, initrd), 'w') as fd: + with open(os.path.join(mnt, initrd), 'w', encoding='ascii') as fd: print('initrd', file=fd)
mkdir_cond(os.path.join(mnt, dtbdir))
- dtb_file = os.path.join(mnt, '%s/sandbox.dtb' % dtbdir) + dtb_file = os.path.join(mnt, f'{dtbdir}/sandbox.dtb') u_boot_utils.run_and_log( - cons, 'dtc -o %s' % dtb_file, stdin=b'/dts-v1/; / {};') + cons, f'dtc -o {dtb_file}', stdin=b'/dts-v1/; / {};') complete = True except ValueError as exc: - print('Falled to create image, failing back to prepared copy: %s', - str(exc)) + print(f'Falled to create image, failing back to prepared copy: {exc}') finally: if mounted: - u_boot_utils.run_and_log(cons, 'sudo umount --lazy %s' % mnt) + u_boot_utils.run_and_log(cons, f'sudo umount --lazy {mnt}') if loop: - u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop) + u_boot_utils.run_and_log(cons, f'sudo losetup -d {loop}')
if not complete: copy_prepared_image(cons, mmc_dev, fname) @@ -303,7 +303,8 @@ def setup_cros_image(cons): Return: bytes: Packed-kernel data """ - kern_part = os.path.join(cons.config.result_dir, 'kern-part-{arch}.bin') + kern_part = os.path.join(cons.config.result_dir, + f'kern-part-{arch}.bin') u_boot_utils.run_and_log( cons, f'futility vbutil_kernel --pack {kern_part} ' @@ -332,7 +333,7 @@ def setup_cros_image(cons):
mmc_dev = 5 fname = os.path.join(cons.config.source_dir, f'mmc{mmc_dev}.img') - u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname) + u_boot_utils.run_and_log(cons, f'qemu-img create {fname} 20M') #mnt = os.path.join(cons.config.persistent_data_dir, 'mnt') #mkdir_cond(mnt) u_boot_utils.run_and_log(cons, f'cgpt create {fname}') @@ -381,20 +382,20 @@ def setup_cros_image(cons):
u_boot_utils.run_and_log(cons, f'cgpt boot -p {fname}') out = u_boot_utils.run_and_log(cons, f'cgpt show -q {fname}') - '''We expect something like this: - 8239 2048 1 Basic data - 45 2048 2 ChromeOS kernel - 8238 1 3 ChromeOS rootfs - 2093 2048 4 ChromeOS kernel - 8237 1 5 ChromeOS rootfs - 41 1 6 ChromeOS kernel - 42 1 7 ChromeOS rootfs - 4141 2048 8 Basic data - 43 1 9 ChromeOS reserved - 44 1 10 ChromeOS reserved - 40 1 11 ChromeOS firmware - 6189 2048 12 EFI System Partition - ''' + + # We expect something like this: + # 8239 2048 1 Basic data + # 45 2048 2 ChromeOS kernel + # 8238 1 3 ChromeOS rootfs + # 2093 2048 4 ChromeOS kernel + # 8237 1 5 ChromeOS rootfs + # 41 1 6 ChromeOS kernel + # 42 1 7 ChromeOS rootfs + # 4141 2048 8 Basic data + # 43 1 9 ChromeOS reserved + # 44 1 10 ChromeOS reserved + # 40 1 11 ChromeOS firmware + # 6189 2048 12 EFI System Partition
# Create a dict (indexed by partition number) containing the above info for line in out.splitlines(): @@ -446,7 +447,7 @@ def setup_android_image(cons):
mmc_dev = 7 fname = os.path.join(cons.config.source_dir, f'mmc{mmc_dev}.img') - u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname) + u_boot_utils.run_and_log(cons, f'qemu-img create {fname} 20M') u_boot_utils.run_and_log(cons, f'cgpt create {fname}')
ptr = 40 @@ -498,11 +499,12 @@ def setup_android_image(cons): with open(fname, 'wb') as outf: outf.write(disk_data)
- print('wrote to {}'.format(fname)) + print(f'wrote to {fname}')
return fname
def setup_cedit_file(cons): + """Set up a .dtb file for use with testing expo and configuration editor""" infname = os.path.join(cons.config.source_dir, 'test/boot/files/expo_layout.dts') inhname = os.path.join(cons.config.source_dir, @@ -584,7 +586,7 @@ def test_ut(u_boot_console, ut_subtest): # ut hush hush_test_simple_dollar prints "Unknown command" on purpose. with u_boot_console.disable_check('unknown_command'): output = u_boot_console.run_command('ut ' + ut_subtest) - assert('Unknown command 'quux' - try 'help'' in output) + assert 'Unknown command 'quux' - try 'help'' in output else: output = u_boot_console.run_command('ut ' + ut_subtest) assert output.endswith('Failures: 0')
There have been quite a few changes in the Python scripts, so update the pylint baseline.
This was created using:
make pylint cp pylint.cur scripts/pylint.base
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
scripts/pylint.base | 462 ++++++++++++++++++++++++++------------------ 1 file changed, 272 insertions(+), 190 deletions(-)
diff --git a/scripts/pylint.base b/scripts/pylint.base index c7d141ed393..bc39e2385a3 100644 --- a/scripts/pylint.base +++ b/scripts/pylint.base @@ -1,229 +1,311 @@ -test_conftest.py 6.56 -test_multiplexed_log.py 7.49 -test_test.py 8.18 +test_conftest.py 6.78 +test_multiplexed_log.py 7.68 +test_test.py 9.00 +test_tests_fit_util.py 7.19 +test_tests_fs_helper.py 10.00 test_tests_test_000_version.py 7.50 -test_tests_test_android_test_ab.py 6.50 -test_tests_test_android_test_abootimg.py 6.09 -test_tests_test_android_test_avb.py 5.52 -test_tests_test_bind.py -2.99 +test_tests_test_android_test_ab.py 7.00 +test_tests_test_android_test_abootimg.py 6.52 +test_tests_test_android_test_avb.py 6.38 +test_tests_test_bind.py 8.89 test_tests_test_bootmenu.py 10.00 +test_tests_test_bootstage.py 7.14 test_tests_test_button.py 3.33 -test_tests_test_dfu.py 5.45 -test_tests_test_dm.py 9.52 -test_tests_test_efi_capsule_capsule_defs.py 6.67 -test_tests_test_efi_capsule_conftest.py 1.86 -test_tests_test_efi_capsule_test_capsule_firmware.py 4.52 -test_tests_test_efi_capsule_test_capsule_firmware_signed.py 4.85 -test_tests_test_efi_fit.py 8.16 -test_tests_test_efi_loader.py 7.38 -test_tests_test_efi_secboot_conftest.py -3.29 -test_tests_test_efi_secboot_defs.py 6.67 -test_tests_test_efi_secboot_test_authvar.py 8.93 -test_tests_test_efi_secboot_test_signed.py 8.41 -test_tests_test_efi_secboot_test_signed_intca.py 8.10 -test_tests_test_efi_secboot_test_unsigned.py 8.00 -test_tests_test_efi_selftest.py 6.36 -test_tests_test_env.py 7.15 -test_tests_test_extension.py 2.14 -test_tests_test_fit.py 6.83 -test_tests_test_fit_ecdsa.py 7.94 -test_tests_test_fit_hashes.py 7.70 -test_tests_test_fpga.py 1.81 -test_tests_test_fs_conftest.py 5.13 +test_tests_test_cat_conftest.py 10.00 +test_tests_test_cat_test_cat.py 10.00 +test_tests_test_cleanup_build.py 10.00 +test_tests_test_dfu.py 6.23 +test_tests_test_dm.py 8.98 +test_tests_test_efi_bootmgr_conftest.py 10.00 +test_tests_test_efi_bootmgr_test_efi_bootmgr.py 10.00 +test_tests_test_efi_capsule_capsule_common.py 10.00 +test_tests_test_efi_capsule_capsule_defs.py 10.00 +test_tests_test_efi_capsule_conftest.py 3.26 +test_tests_test_efi_capsule_test_capsule_firmware_fit.py 10.00 +test_tests_test_efi_capsule_test_capsule_firmware_raw.py 9.62 +test_tests_test_efi_capsule_test_capsule_firmware_signed_fit.py 9.74 +test_tests_test_efi_capsule_test_capsule_firmware_signed_raw.py 9.75 +test_tests_test_efi_fit.py 8.84 +test_tests_test_efi_loader.py 8.00 +test_tests_test_efi_secboot_conftest.py 0.00 +test_tests_test_efi_secboot_defs.py 10.00 +test_tests_test_efi_secboot_test_authvar.py 9.38 +test_tests_test_efi_secboot_test_signed.py 8.60 +test_tests_test_efi_secboot_test_signed_intca.py 8.81 +test_tests_test_efi_secboot_test_unsigned.py 8.75 +test_tests_test_efi_selftest.py 8.07 +test_tests_test_eficonfig_conftest.py 10.00 +test_tests_test_eficonfig_test_eficonfig.py 9.47 +test_tests_test_env.py 7.76 +test_tests_test_event_dump.py 2.22 +test_tests_test_extension.py 2.50 +test_tests_test_fit.py 7.54 +test_tests_test_fit_auto_signed.py 9.09 +test_tests_test_fit_ecdsa.py 8.29 +test_tests_test_fit_hashes.py 7.94 +test_tests_test_fpga.py 2.94 +test_tests_test_fs_conftest.py 5.21 test_tests_test_fs_fstest_defs.py 8.33 test_tests_test_fs_fstest_helpers.py 4.29 -test_tests_test_fs_test_basic.py 0.60 -test_tests_test_fs_test_ext.py 0.00 +test_tests_test_fs_test_basic.py 1.90 +test_tests_test_fs_test_erofs.py 8.97 +test_tests_test_fs_test_ext.py 1.48 test_tests_test_fs_test_fs_cmd.py 8.00 -test_tests_test_fs_test_mkdir.py 1.96 -test_tests_test_fs_test_squashfs_sqfs_common.py 8.41 -test_tests_test_fs_test_squashfs_test_sqfs_load.py 7.46 -test_tests_test_fs_test_squashfs_test_sqfs_ls.py 8.00 -test_tests_test_fs_test_symlink.py 1.22 -test_tests_test_fs_test_unlink.py 2.78 -test_tests_test_gpio.py 6.09 -test_tests_test_gpt.py 7.67 +test_tests_test_fs_test_fs_fat.py 2.50 +test_tests_test_fs_test_mkdir.py 3.04 +test_tests_test_fs_test_squashfs_sqfs_common.py 8.38 +test_tests_test_fs_test_squashfs_test_sqfs_load.py 7.63 +test_tests_test_fs_test_squashfs_test_sqfs_ls.py 8.04 +test_tests_test_fs_test_symlink.py 2.04 +test_tests_test_fs_test_unlink.py 4.07 +test_tests_test_gpio.py 7.60 +test_tests_test_gpt.py 8.55 test_tests_test_handoff.py 5.00 -test_tests_test_help.py 5.00 -test_tests_test_hush_if_test.py 9.27 -test_tests_test_log.py 8.64 +test_tests_test_help.py 8.64 +test_tests_test_i2c.py 7.42 +test_tests_test_kconfig.py 5.38 +test_tests_test_log.py 8.75 test_tests_test_lsblk.py 8.00 test_tests_test_md.py 3.64 +test_tests_test_mdio.py 6.82 +test_tests_test_memtest.py 8.39 +test_tests_test_mii.py 8.55 +test_tests_test_mmc.py 7.01 test_tests_test_mmc_rd.py 6.05 test_tests_test_mmc_wr.py 3.33 -test_tests_test_net.py 6.84 +test_tests_test_net.py 8.43 +test_tests_test_net_boot.py 8.23 +test_tests_test_of_migrate.py 7.86 test_tests_test_ofplatdata.py 5.71 +test_tests_test_optee_rpmb.py 0.00 test_tests_test_part.py 8.00 -test_tests_test_pinmux.py 3.27 +test_tests_test_pinmux.py 3.40 test_tests_test_pstore.py 2.31 test_tests_test_qfw.py 8.75 +test_tests_test_reset.py 9.55 test_tests_test_sandbox_exit.py 6.50 +test_tests_test_sandbox_opts.py 1.11 +test_tests_test_saveenv.py 7.87 test_tests_test_scp03.py 3.33 -test_tests_test_sf.py 7.13 +test_tests_test_scsi.py 8.47 +test_tests_test_semihosting_conftest.py 10.00 +test_tests_test_semihosting_test_hostfs.py 10.00 +test_tests_test_sf.py 7.45 test_tests_test_shell_basics.py 9.58 -test_tests_test_sleep.py 7.78 -test_tests_test_spl.py 2.22 +test_tests_test_sleep.py 8.28 +test_tests_test_smbios.py 9.47 +test_tests_test_source.py 7.20 +test_tests_test_spl.py 6.67 test_tests_test_stackprotector.py 5.71 -test_tests_test_tpm2.py 8.51 -test_tests_test_ums.py 6.32 +test_tests_test_tpm2.py 8.45 +test_tests_test_trace.py 8.70 +test_tests_test_ums.py 5.92 test_tests_test_unknown_cmd.py 5.00 -test_tests_test_ut.py 7.06 -test_tests_test_vboot.py 6.01 -test_tests_vboot_evil.py 8.95 +test_tests_test_upl.py 5.33 +test_tests_test_usb.py 7.08 +test_tests_test_ut.py 9.44 +test_tests_test_vbe.py 7.22 +test_tests_test_vbe_vpl.py 6.11 +test_tests_test_vboot.py 5.37 +test_tests_test_vpl.py 2.22 +test_tests_test_xxd_conftest.py 10.00 +test_tests_test_xxd_test_xxd.py 10.00 +test_tests_test_zynq_secure.py 7.60 +test_tests_test_zynqmp_rpu.py 7.54 +test_tests_test_zynqmp_secure.py 7.68 +test_tests_vboot_evil.py 9.45 test_tests_vboot_forge.py 9.22 -test_u_boot_console_base.py 7.08 -test_u_boot_console_exec_attach.py 9.23 -test_u_boot_console_sandbox.py 8.06 -test_u_boot_spawn.py 7.65 -test_u_boot_utils.py 6.94 -tools_binman_bintool 8.59 +test_u_boot_console_base.py 7.73 +test_u_boot_console_exec_attach.py 9.62 +test_u_boot_console_sandbox.py 8.64 +test_u_boot_spawn.py 8.57 +test_u_boot_utils.py 7.83 +tools_binman_bintool 9.16 tools_binman_bintool_test 9.87 -tools_binman_btool__testing 6.09 +tools_binman_btool__testing 6.52 +tools_binman_btool_bootgen 4.50 +tools_binman_btool_btool_gzip 0.00 +tools_binman_btool_bzip2 0.00 tools_binman_btool_cbfstool 7.83 -tools_binman_btool_fiptool 7.62 -tools_binman_btool_futility 7.39 +tools_binman_btool_cst 5.00 +tools_binman_btool_fdt_add_pubkey 7.00 +tools_binman_btool_fdtgrep 5.20 +tools_binman_btool_fiptool 7.22 +tools_binman_btool_futility 6.67 tools_binman_btool_ifwitool 3.81 -tools_binman_btool_lz4 6.30 +tools_binman_btool_lz4 4.76 tools_binman_btool_lzma_alone 6.97 -tools_binman_btool_mkimage 7.86 -tools_binman_cbfs_util 8.46 -tools_binman_cbfs_util_test 9.38 -tools_binman_cmdline 9.03 -tools_binman_comp_util 6.88 -tools_binman_control 5.01 -tools_binman_elf 6.98 -tools_binman_elf_test 5.62 -tools_binman_entry 3.55 -tools_binman_entry_test 5.34 -tools_binman_etype__testing 0.83 -tools_binman_etype_atf_bl31 -6.00 -tools_binman_etype_atf_fip 0.29 -tools_binman_etype_blob -1.58 -tools_binman_etype_blob_dtb -10.00 -tools_binman_etype_blob_ext -19.09 +tools_binman_btool_lzop 0.00 +tools_binman_btool_mkeficapsule 7.69 +tools_binman_btool_mkimage 6.36 +tools_binman_btool_openssl 4.63 +tools_binman_btool_xz 0.00 +tools_binman_btool_zstd 0.00 +tools_binman_cbfs_util 8.93 +tools_binman_cbfs_util_test 9.81 +tools_binman_cmdline 9.33 +tools_binman_control 6.92 +tools_binman_elf 7.52 +tools_binman_elf_test 8.40 +tools_binman_entry 6.40 +tools_binman_entry_test 6.99 +tools_binman_etype__testing 2.02 +tools_binman_etype_alternates_fdt 5.09 +tools_binman_etype_atf_bl31 0.00 +tools_binman_etype_atf_fip 0.44 +tools_binman_etype_blob 0.41 +tools_binman_etype_blob_dtb 0.21 +tools_binman_etype_blob_ext 0.00 tools_binman_etype_blob_ext_list 0.00 -tools_binman_etype_blob_named_by_arg -7.78 -tools_binman_etype_blob_phase -5.00 -tools_binman_etype_cbfs -1.44 +tools_binman_etype_blob_named_by_arg 0.00 +tools_binman_etype_blob_phase 0.50 +tools_binman_etype_cbfs 1.86 tools_binman_etype_collection 2.67 -tools_binman_etype_cros_ec_rw -6.00 -tools_binman_etype_fdtmap -3.28 -tools_binman_etype_files -7.43 -tools_binman_etype_fill -6.43 -tools_binman_etype_fit 6.31 -tools_binman_etype_fmap -0.29 -tools_binman_etype_gbb 0.83 -tools_binman_etype_image_header 5.77 -tools_binman_etype_intel_cmc -12.50 +tools_binman_etype_cros_ec_rw 0.00 +tools_binman_etype_efi_capsule 3.33 +tools_binman_etype_efi_empty_capsule 0.00 +tools_binman_etype_encrypted 1.43 +tools_binman_etype_fdtmap 0.16 +tools_binman_etype_files 0.00 +tools_binman_etype_fill 0.00 +tools_binman_etype_fit 7.52 +tools_binman_etype_fmap 0.54 +tools_binman_etype_gbb 1.35 +tools_binman_etype_image_header 4.81 +tools_binman_etype_intel_cmc 0.00 tools_binman_etype_intel_descriptor 4.62 tools_binman_etype_intel_fit 0.00 tools_binman_etype_intel_fit_ptr 2.35 -tools_binman_etype_intel_fsp -12.50 -tools_binman_etype_intel_fsp_m -12.50 -tools_binman_etype_intel_fsp_s -12.50 -tools_binman_etype_intel_fsp_t -12.50 -tools_binman_etype_intel_ifwi 2.88 -tools_binman_etype_intel_me -12.50 -tools_binman_etype_intel_mrc -10.00 -tools_binman_etype_intel_refcode -10.00 -tools_binman_etype_intel_vbt -12.50 -tools_binman_etype_intel_vga -12.50 -tools_binman_etype_mkimage 1.47 -tools_binman_etype_opensbi -6.00 -tools_binman_etype_powerpc_mpc85xx_bootpg_resetvec -10.00 -tools_binman_etype_scp -6.00 -tools_binman_etype_section 4.57 -tools_binman_etype_tee_os -6.00 -tools_binman_etype_text -0.48 -tools_binman_etype_u_boot -15.71 -tools_binman_etype_u_boot_dtb -12.22 -tools_binman_etype_u_boot_dtb_with_ucode 0.39 -tools_binman_etype_u_boot_elf -8.42 +tools_binman_etype_intel_fsp 0.00 +tools_binman_etype_intel_fsp_m 0.00 +tools_binman_etype_intel_fsp_s 0.00 +tools_binman_etype_intel_fsp_t 0.00 +tools_binman_etype_intel_ifwi 3.13 +tools_binman_etype_intel_me 0.00 +tools_binman_etype_intel_mrc 0.00 +tools_binman_etype_intel_refcode 0.00 +tools_binman_etype_intel_vbt 0.00 +tools_binman_etype_intel_vga 0.00 +tools_binman_etype_mkimage 4.88 +tools_binman_etype_null 0.00 +tools_binman_etype_nxp_imx8mcst 2.44 +tools_binman_etype_nxp_imx8mimage 0.00 +tools_binman_etype_opensbi 0.00 +tools_binman_etype_powerpc_mpc85xx_bootpg_resetvec 0.00 +tools_binman_etype_pre_load 3.68 +tools_binman_etype_rockchip_tpl 0.00 +tools_binman_etype_scp 0.00 +tools_binman_etype_section 6.04 +tools_binman_etype_tee_os 4.00 +tools_binman_etype_text 0.00 +tools_binman_etype_ti_board_config 5.40 +tools_binman_etype_ti_dm 0.00 +tools_binman_etype_ti_secure 4.22 +tools_binman_etype_ti_secure_rom 2.22 +tools_binman_etype_u_boot 0.00 +tools_binman_etype_u_boot_dtb 0.00 +tools_binman_etype_u_boot_dtb_with_ucode 1.73 +tools_binman_etype_u_boot_elf 0.00 tools_binman_etype_u_boot_env 0.74 -tools_binman_etype_u_boot_expanded -10.00 -tools_binman_etype_u_boot_img -15.71 -tools_binman_etype_u_boot_nodtb -15.71 -tools_binman_etype_u_boot_spl -10.91 -tools_binman_etype_u_boot_spl_bss_pad -9.29 -tools_binman_etype_u_boot_spl_dtb -12.22 -tools_binman_etype_u_boot_spl_elf -15.71 -tools_binman_etype_u_boot_spl_expanded -9.09 -tools_binman_etype_u_boot_spl_nodtb -10.91 -tools_binman_etype_u_boot_spl_with_ucode_ptr -5.00 -tools_binman_etype_u_boot_tpl -10.91 -tools_binman_etype_u_boot_tpl_bss_pad -9.29 -tools_binman_etype_u_boot_tpl_dtb -12.22 -tools_binman_etype_u_boot_tpl_dtb_with_ucode -7.50 -tools_binman_etype_u_boot_tpl_elf -15.71 -tools_binman_etype_u_boot_tpl_expanded -9.09 -tools_binman_etype_u_boot_tpl_nodtb -10.91 -tools_binman_etype_u_boot_tpl_with_ucode_ptr -20.83 +tools_binman_etype_u_boot_expanded 0.00 +tools_binman_etype_u_boot_img 0.00 +tools_binman_etype_u_boot_nodtb 0.00 +tools_binman_etype_u_boot_spl 0.00 +tools_binman_etype_u_boot_spl_bss_pad 0.00 +tools_binman_etype_u_boot_spl_dtb 0.00 +tools_binman_etype_u_boot_spl_elf 0.00 +tools_binman_etype_u_boot_spl_expanded 0.00 +tools_binman_etype_u_boot_spl_nodtb 0.00 +tools_binman_etype_u_boot_spl_pubkey_dtb 1.21 +tools_binman_etype_u_boot_spl_with_ucode_ptr 0.00 +tools_binman_etype_u_boot_tpl 0.00 +tools_binman_etype_u_boot_tpl_bss_pad 0.00 +tools_binman_etype_u_boot_tpl_dtb 0.00 +tools_binman_etype_u_boot_tpl_dtb_with_ucode 0.00 +tools_binman_etype_u_boot_tpl_elf 0.00 +tools_binman_etype_u_boot_tpl_expanded 0.00 +tools_binman_etype_u_boot_tpl_nodtb 0.00 +tools_binman_etype_u_boot_tpl_with_ucode_ptr 0.00 tools_binman_etype_u_boot_ucode 1.52 -tools_binman_etype_u_boot_with_ucode_ptr -0.71 -tools_binman_etype_vblock 0.27 -tools_binman_etype_x86_reset16 -15.71 -tools_binman_etype_x86_reset16_spl -15.71 -tools_binman_etype_x86_reset16_tpl -15.71 -tools_binman_etype_x86_start16 -15.71 -tools_binman_etype_x86_start16_spl -15.71 -tools_binman_etype_x86_start16_tpl -15.71 -tools_binman_fdt_test 10.00 +tools_binman_etype_u_boot_vpl 0.00 +tools_binman_etype_u_boot_vpl_bss_pad 0.00 +tools_binman_etype_u_boot_vpl_dtb 0.00 +tools_binman_etype_u_boot_vpl_elf 0.00 +tools_binman_etype_u_boot_vpl_expanded 0.00 +tools_binman_etype_u_boot_vpl_nodtb 0.00 +tools_binman_etype_u_boot_with_ucode_ptr 0.00 +tools_binman_etype_vblock 0.79 +tools_binman_etype_x509_cert 3.10 +tools_binman_etype_x86_reset16 0.00 +tools_binman_etype_x86_reset16_spl 0.00 +tools_binman_etype_x86_reset16_tpl 0.00 +tools_binman_etype_x86_start16 0.00 +tools_binman_etype_x86_start16_spl 0.00 +tools_binman_etype_x86_start16_tpl 0.00 +tools_binman_etype_xilinx_bootgen 6.06 +tools_binman_fdt_test 7.74 tools_binman_fip_util 9.85 tools_binman_fip_util_test 10.00 -tools_binman_fmap_util 6.88 -tools_binman_ftest 7.46 -tools_binman_image 7.12 -tools_binman_image_test 4.48 -tools_binman_main 4.86 -tools_binman_setup 5.00 -tools_binman_state 4.15 -tools_buildman_board 7.82 -tools_buildman_bsettings 1.71 -tools_buildman_builder 6.92 -tools_buildman_builderthread 7.48 -tools_buildman_cfgutil 7.83 -tools_buildman_cmdline 8.89 -tools_buildman_control 8.12 -tools_buildman_func_test 7.18 -tools_buildman_kconfiglib 7.49 -tools_buildman_main -1.11 -tools_buildman_test 6.56 -tools_buildman_toolchain 6.44 -tools_concurrencytest_concurrencytest 7.26 -tools_dtoc_dtb_platdata 7.90 -tools_dtoc_fdt 4.46 -tools_dtoc_fdt_util 6.80 -tools_dtoc_main 7.78 -tools_dtoc_setup 5.00 -tools_dtoc_src_scan 8.91 -tools_dtoc_test_dtoc 8.56 -tools_dtoc_test_fdt 6.88 -tools_dtoc_test_src_scan 9.43 -tools_efivar 6.71 +tools_binman_fmap_util 6.94 +tools_binman_ftest 8.04 +tools_binman_image 7.29 +tools_binman_image_test 5.52 +tools_binman_main 5.63 +tools_binman_setup 0.00 +tools_binman_state 4.88 +tools_buildman_board 6.36 +tools_buildman_boards 9.72 +tools_buildman_bsettings 5.00 +tools_buildman_builder 7.66 +tools_buildman_builderthread 9.63 +tools_buildman_cfgutil 10.00 +tools_buildman_cmdline 10.00 +tools_buildman_control 9.26 +tools_buildman_func_test 8.38 +tools_buildman_kconfiglib 8.33 +tools_buildman_main 8.10 +tools_buildman_test 7.16 +tools_buildman_toolchain 6.99 +tools_dtoc_dtb_platdata 8.10 +tools_dtoc_fdt 6.31 +tools_dtoc_fdt_util 7.62 +tools_dtoc_main 8.54 +tools_dtoc_setup 0.00 +tools_dtoc_src_scan 9.14 +tools_dtoc_test_dtoc 8.97 +tools_dtoc_test_fdt 9.93 +tools_dtoc_test_src_scan 9.46 +tools_efivar 7.39 tools_endian-swap 9.29 -tools_microcode-tool 7.25 -tools_moveconfig 8.34 +tools_expo 9.72 +tools_key2dtsi 7.14 +tools_microcode-tool 6.55 tools_patman___init__ 0.00 -tools_patman_checkpatch 8.48 -tools_patman_command 5.51 -tools_patman_commit 4.50 -tools_patman_control 8.14 -tools_patman_cros_subprocess 7.76 -tools_patman_func_test 8.51 -tools_patman_get_maintainer 7.06 -tools_patman_gitutil 6.65 -tools_patman_main 7.90 -tools_patman_patchstream 9.11 +tools_patman___main__ 9.44 +tools_patman_checkpatch 8.90 +tools_patman_cmdline 10.00 +tools_patman_commit 6.43 +tools_patman_control 8.29 +tools_patman_func_test 9.02 +tools_patman_get_maintainer 7.50 +tools_patman_gitutil 7.37 +tools_patman_patchstream 9.21 tools_patman_project 7.78 -tools_patman_series 6.16 -tools_patman_settings 5.89 +tools_patman_series 7.54 +tools_patman_settings 7.94 tools_patman_setup 5.00 -tools_patman_status 8.62 -tools_patman_terminal 8.00 -tools_patman_test_checkpatch 7.75 -tools_patman_test_util 7.64 -tools_patman_tools 5.68 -tools_patman_tout 5.31 -tools_rkmux 6.90 -tools_rmboard 7.76 +tools_patman_status 8.52 +tools_patman_test_checkpatch 8.51 +tools_patman_test_settings 8.78 +tools_qconfig 9.79 +tools_rkmux 7.10 +tools_rmboard 8.06 +tools_u_bootlib___init__.py 0.00 +tools_u_bootlib___main__.py 7.78 +tools_u_bootlib_command.py 6.48 +tools_u_bootlib_cros_subprocess.py 9.25 +tools_u_bootlib_terminal.py 8.50 +tools_u_bootlib_test_util.py 7.31 +tools_u_bootlib_tools.py 6.97 +tools_u_bootlib_tout.py 6.56 tools_zynqmp_pm_cfg_obj_convert 6.67
Set up a function for this, since it needs to be used from multiple test files.
This test file is only used on sandbox, where USB is enabled, so drop the local declaration of usb_started
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Include the usb.h header file in all cases
test/boot/bootdev.c | 19 ++++++------------- test/boot/bootstd_common.c | 6 ++++++ test/boot/bootstd_common.h | 8 ++++++++ 3 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c index 23ebc61ff52..c635d06ec25 100644 --- a/test/boot/bootdev.c +++ b/test/boot/bootdev.c @@ -16,13 +16,6 @@ #include <test/ut.h> #include "bootstd_common.h"
-/* Allow reseting the USB-started flag */ -#if defined(CONFIG_USB_HOST) || defined(CONFIG_USB_GADGET) -extern bool usb_started; -#else -#include <usb.h> -#endif - /* Check 'bootdev list' command */ static int bootdev_test_cmd_list(struct unit_test_state *uts) { @@ -197,7 +190,7 @@ static int bootdev_test_order(struct unit_test_state *uts) test_set_skip_delays(true);
/* Start up USB which gives us three additional bootdevs */ - usb_started = false; + bootstd_reset_usb(); ut_assertok(run_command("usb start", 0));
/* @@ -313,7 +306,7 @@ static int bootdev_test_prio(struct unit_test_state *uts) test_set_eth_enable(false);
/* Start up USB which gives us three additional bootdevs */ - usb_started = false; + bootstd_reset_usb(); ut_assertok(run_command("usb start", 0));
ut_assertok(bootstd_test_drop_bootdev_order(uts)); @@ -352,7 +345,7 @@ static int bootdev_test_hunter(struct unit_test_state *uts) { struct bootstd_priv *std;
- usb_started = false; + bootstd_reset_usb(); test_set_skip_delays(true);
/* get access to the used hunters */ @@ -392,7 +385,7 @@ static int bootdev_test_cmd_hunt(struct unit_test_state *uts) struct bootstd_priv *std;
test_set_skip_delays(true); - usb_started = false; + bootstd_reset_usb();
/* get access to the used hunters */ ut_assertok(bootstd_get_priv(&std)); @@ -520,7 +513,7 @@ BOOTSTD_TEST(bootdev_test_bootable, UTF_DM | UTF_SCAN_FDT); /* Check hunting for bootdev of a particular priority */ static int bootdev_test_hunt_prio(struct unit_test_state *uts) { - usb_started = false; + bootstd_reset_usb(); test_set_skip_delays(true);
ut_assertok(bootdev_hunt_prio(BOOTDEVP_4_SCAN_FAST, false)); @@ -548,7 +541,7 @@ static int bootdev_test_hunt_label(struct unit_test_state *uts) struct bootstd_priv *std; int mflags;
- usb_started = false; + bootstd_reset_usb();
/* get access to the used hunters */ ut_assertok(bootstd_get_priv(&std)); diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c index e50539500a0..ff8ed2303b3 100644 --- a/test/boot/bootstd_common.c +++ b/test/boot/bootstd_common.c @@ -11,6 +11,7 @@ #include <dm.h> #include <memalign.h> #include <mmc.h> +#include <usb.h> #include <linux/log2.h> #include <test/suites.h> #include <test/ut.h> @@ -88,6 +89,11 @@ int bootstd_test_check_mmc_hunter(struct unit_test_state *uts) return 0; }
+void bootstd_reset_usb(void) +{ + usb_started = false; +} + int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test); diff --git a/test/boot/bootstd_common.h b/test/boot/bootstd_common.h index 4a126e43ff4..e29036c897c 100644 --- a/test/boot/bootstd_common.h +++ b/test/boot/bootstd_common.h @@ -53,4 +53,12 @@ int bootstd_setup_for_tests(void); */ int bootstd_test_check_mmc_hunter(struct unit_test_state *uts);
+/** + * bootstd_reset_usb() - Reset the USB subsystem + * + * Resets USB so that it can be started (and scanning) again. This is useful in + * tests which need to use USB. + */ +void bootstd_reset_usb(void); + #endif
The driver model deadline for USB was in 2019, so drop the old USB keyboard code, to avoid needing to deal with the extra code path.
Drop the unnecessary #ifdef around USB_KBD_BOOT_REPORT_SIZE while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/usb.c | 20 -------------- common/usb_kbd.c | 67 --------------------------------------------- drivers/usb/Kconfig | 3 +- include/usb.h | 8 ------ 4 files changed, 2 insertions(+), 96 deletions(-)
diff --git a/cmd/usb.c b/cmd/usb.c index 16c081bf128..13a2996c1f0 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -560,17 +560,6 @@ static int do_usbboot(struct cmd_tbl *cmdtp, int flag, int argc, } #endif /* CONFIG_USB_STORAGE */
-static int do_usb_stop_keyboard(int force) -{ -#if !defined CONFIG_DM_USB && defined CONFIG_USB_KEYBOARD - if (usb_kbd_deregister(force) != 0) { - printf("USB not stopped: usbkbd still using USB\n"); - return 1; - } -#endif - return 0; -} - static void do_usb_start(void) { bootstage_mark_name(BOOTSTAGE_ID_USB_START, "usb_start"); @@ -583,11 +572,6 @@ static void do_usb_start(void) /* try to recognize storage devices immediately */ usb_stor_curr_dev = usb_stor_scan(1); # endif -#ifndef CONFIG_DM_USB -# ifdef CONFIG_USB_KEYBOARD - drv_usb_kbd_init(); -# endif -#endif /* !CONFIG_DM_USB */ }
#ifdef CONFIG_DM_USB @@ -633,8 +617,6 @@ static int do_usb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
if (strncmp(argv[1], "reset", 5) == 0) { printf("resetting USB...\n"); - if (do_usb_stop_keyboard(1) != 0) - return 1; usb_stop(); do_usb_start(); return 0; @@ -642,8 +624,6 @@ static int do_usb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (strncmp(argv[1], "stop", 4) == 0) { if (argc != 2) console_assign(stdin, "serial"); - if (do_usb_stop_keyboard(0) != 0) - return 1; printf("stopping USB..\n"); usb_stop(); return 0; diff --git a/common/usb_kbd.c b/common/usb_kbd.c index f3b4a3c94e6..b834b2f703d 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -643,71 +643,6 @@ static int probe_usb_keyboard(struct usb_device *dev) return 0; }
-#if !CONFIG_IS_ENABLED(DM_USB) -/* Search for keyboard and register it if found. */ -int drv_usb_kbd_init(void) -{ - int error, i; - - debug("%s: Probing for keyboard\n", __func__); - /* Scan all USB Devices */ - for (i = 0; i < USB_MAX_DEVICE; i++) { - struct usb_device *dev; - - /* Get USB device. */ - dev = usb_get_dev_index(i); - if (!dev) - break; - - if (dev->devnum == -1) - continue; - - error = probe_usb_keyboard(dev); - if (!error) - return 1; - if (error && error != -ENOENT) - return error; - } - - /* No USB Keyboard found */ - return -1; -} - -/* Deregister the keyboard. */ -int usb_kbd_deregister(int force) -{ -#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER) - struct stdio_dev *dev; - struct usb_device *usb_kbd_dev; - struct usb_kbd_pdata *data; - - dev = stdio_get_by_name(DEVNAME); - if (dev) { - usb_kbd_dev = (struct usb_device *)dev->priv; - data = usb_kbd_dev->privptr; -#if CONFIG_IS_ENABLED(CONSOLE_MUX) - if (iomux_replace_device(stdin, DEVNAME, force ? "nulldev" : "")) - return 1; -#endif - if (stdio_deregister_dev(dev, force) != 0) - return 1; -#ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE - destroy_int_queue(usb_kbd_dev, data->intq); -#endif - free(data->new); - free(data); - } - - return 0; -#else - return 1; -#endif -} - -#endif - -#if CONFIG_IS_ENABLED(DM_USB) - static int usb_kbd_probe(struct udevice *dev) { struct usb_device *udev = dev_get_parent_priv(dev); @@ -788,5 +723,3 @@ static const struct usb_device_id kbd_id_table[] = { };
U_BOOT_USB_DEVICE(usb_kbd, kbd_id_table); - -#endif diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index a972d87c7ad..311aaa7e67f 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -99,7 +99,8 @@ config USB_STORAGE
config USB_KEYBOARD bool "USB Keyboard support" - select DM_KEYBOARD if DM_USB + depends on DM_USB + select DM_KEYBOARD select SYS_STDIO_DEREGISTER ---help--- Say Y here if you want to use a USB keyboard for U-Boot command line diff --git a/include/usb.h b/include/usb.h index fcbe2146f7d..e37f853482c 100644 --- a/include/usb.h +++ b/include/usb.h @@ -250,20 +250,12 @@ int usb_host_eth_scan(int mode);
#endif
-#ifdef CONFIG_USB_KEYBOARD - /* * USB Keyboard reports are 8 bytes in boot protocol. * Appendix B of HID Device Class Definition 1.11 */ #define USB_KBD_BOOT_REPORT_SIZE 8
-int drv_usb_kbd_init(void); -int usb_kbd_deregister(int force); - -#endif -/* routines */ - /* * usb_init() - initialize the USB Controllers *
Add a new category which covers the console, including the stdio drivers.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/console.c | 2 ++ common/log.c | 1 + include/log.h | 2 ++ 3 files changed, 5 insertions(+)
diff --git a/common/console.c b/common/console.c index 52d6df8150f..0f00f3d8d5b 100644 --- a/common/console.c +++ b/common/console.c @@ -4,6 +4,8 @@ * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio@tin.it */
+#define LOG_CATEGORY LOGC_CONSOLE + #include <console.h> #include <debug_uart.h> #include <display_options.h> diff --git a/common/log.c b/common/log.c index dfee250b158..b83a6618900 100644 --- a/common/log.c +++ b/common/log.c @@ -31,6 +31,7 @@ static const char *const log_cat_name[] = { "event", "fs", "expo", + "console", };
_Static_assert(ARRAY_SIZE(log_cat_name) == LOGC_COUNT - LOGC_NONE, diff --git a/include/log.h b/include/log.h index 69dcb339543..7c25bf0b081 100644 --- a/include/log.h +++ b/include/log.h @@ -104,6 +104,8 @@ enum log_category_t { LOGC_FS, /** @LOGC_EXPO: Related to expo handling */ LOGC_EXPO, + /** @LOGC_CONSOLE: Related to the console and stdio */ + LOGC_CONSOLE, /** @LOGC_COUNT: Number of log categories */ LOGC_COUNT, /** @LOGC_END: Sentinel value for lists of log categories */
This device contains a pointer to struct udevice so set the flag indicating that, just to be tidy.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/usb_kbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index b834b2f703d..1d9845b01ed 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -612,7 +612,7 @@ static int probe_usb_keyboard(struct usb_device *dev) debug("USB KBD: register.\n"); memset(&usb_kbd_dev, 0, sizeof(struct stdio_dev)); strcpy(usb_kbd_dev.name, DEVNAME); - usb_kbd_dev.flags = DEV_FLAGS_INPUT; + usb_kbd_dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_DM; usb_kbd_dev.getc = usb_kbd_getc; usb_kbd_dev.tstc = usb_kbd_testc; usb_kbd_dev.priv = (void *)dev;
Clear any USB-keyboard devices before running a unit test, to avoid using a stale udevice pointer in stdio. Add a long comment to explain this situation and why this solution seems best, at least for now.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/console.c | 34 ++++++++++++++++++++++++++++++++++ common/usb_kbd.c | 5 +++++ include/console.h | 8 ++++++++ include/usb.h | 12 ++++++++++++ test/test-main.c | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+)
diff --git a/common/console.c b/common/console.c index 0f00f3d8d5b..c9e206aec41 100644 --- a/common/console.c +++ b/common/console.c @@ -1244,3 +1244,37 @@ int console_init_r(void) }
#endif /* CONFIG_IS_ENABLED(SYS_CONSOLE_IS_IN_ENV) */ + +int console_remove_by_name(const char *name) +{ + int err = 0; + +#if CONFIG_IS_ENABLED(CONSOLE_MUX) + int fnum; + + log_debug("removing console device %s\n", name); + for (fnum = 0; fnum < MAX_FILES; fnum++) { + struct stdio_dev **src, **dest; + int i; + + log_debug("file %d: %d devices: ", fnum, cd_count[fnum]); + src = console_devices[fnum]; + dest = src; + for (i = 0; i < cd_count[fnum]; i++, src++) { + struct stdio_dev *sdev = *src; + int ret = 0; + + if (!strcmp(sdev->name, name)) + ret = stdio_deregister_dev(sdev, true); + else + *dest++ = *src; + if (ret && !err) + err = ret; + } + cd_count[fnum] = dest - console_devices[fnum]; + log_debug("now %d\n", cd_count[fnum]); + } +#endif /* CONSOLE_MUX */ + + return err; +} diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 1d9845b01ed..bbfee23bc26 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -137,6 +137,11 @@ extern int __maybe_unused net_busy_flag; /* The period of time between two calls of usb_kbd_testc(). */ static unsigned long kbd_testc_tms;
+int usb_kbd_remove_for_test(void) +{ + return console_remove_by_name(DEVNAME); +} + /* Puts character in the queue and sets up the in and out pointer. */ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c) { diff --git a/include/console.h b/include/console.h index 6b6d0f9de73..57fdb0834c1 100644 --- a/include/console.h +++ b/include/console.h @@ -179,6 +179,14 @@ void console_puts_select_stderr(bool serial_only, const char *s); */ int console_clear(void);
+/** + * console_remove_by_name() - Remove a console by its stdio name + * + * This must only be used in tests. It removes any use of the named stdio device + * from the console tables. + */ +int console_remove_by_name(const char *name); + /* * CONSOLE multiplexing. */ diff --git a/include/usb.h b/include/usb.h index e37f853482c..be37ed272e1 100644 --- a/include/usb.h +++ b/include/usb.h @@ -1092,4 +1092,16 @@ struct usb_generic_descriptor **usb_emul_find_descriptor( */ void usb_show_tree(void);
+/** + * usb_kbd_remove_for_test() - Remove any USB keyboard + * + * This can only be called from test_pre_run(). It removes the USB keyboard from + * the console system so that the USB device can be dropped + */ +#if CONFIG_IS_ENABLED(USB_KEYBOARD) +int usb_kbd_remove_for_test(void); +#else +static inline int usb_kbd_remove_for_test(void) { return 0; } +#endif + #endif /*_USB_H_ */ diff --git a/test/test-main.c b/test/test-main.c index 63e8be0ccd1..b3d3e24cdce 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -12,6 +12,7 @@ #include <net.h> #include <of_live.h> #include <os.h> +#include <usb.h> #include <dm/ofnode.h> #include <dm/root.h> #include <dm/test.h> @@ -289,6 +290,43 @@ static int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { ut_assertok(event_init());
+ /* + * Remove any USB keyboard, so that we can add and remove USB devices + * in tests. + * + * For UT_TESTF_DM tests, the old driver model state is saved and + * restored across each test. Within in each test there is therefore a + * new driver model state, which means that any USB keyboard device in + * stdio points to the old state. + * + * This is fine in most cases. But if a non-UT_TESTF_DM test starts up + * USB (thus creating a stdio record pointing to the USB keyboard + * device) then when the test finishes, the new driver model state is + * freed, meaning that there is now a stale pointer in stdio. + * + * This means that any future UT_TESTF_DM test which uses stdin will + * cause the console system to call tstc() on the stale device pointer, + * causing a crash. + * + * We don't want to fix this by enabling UT_TESTF_DM for all tests as + * this causes other problems. For example, bootflow_efi relies on + * U-Boot going through a proper init - without that we don't have the + * TCG measurement working and get an error + * 'tcg2 measurement fails(0x8000000000000007)'. Once we tidy up how EFI + * runs tests (e.g. get rid of all the restarting of U-Boot) we could + * potentially make the bootstd tests set UT_TESTF_DM, but other tests + * might do the same thing. + * + * We could add a test flag to declare that USB is being used, but that + * seems unnecessary, at least for now. We could detect USB being used + * in a test, but there is no obvious drawback to clearing out stale + * pointers always. + * + * So just remove any USB keyboards from the console tables. This allows + * UT_TESTF_DM and non-UT_TESTF_DM tests to coexist happily. + */ + usb_kbd_remove_for_test(); + if (test->flags & UTF_DM) ut_assertok(dm_test_pre_run(uts));
Switch to lower-case hex which is more commonly used in U-Boot.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/cmd/mbr.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index e1a9cdffb04..98a745df6bc 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -50,7 +50,7 @@ static char * mbr_parts_tail = "'"; 000001e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ -static unsigned mbr_cmp_start = 0x1B8; +static unsigned int mbr_cmp_start = 0x1b8; static unsigned mbr_cmp_size = 0x48; static unsigned char mbr_parts_ref_p1[] = { 0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05, @@ -257,7 +257,7 @@ static int mbr_test_run(struct unit_test_state *uts) mbr_wa = map_to_sysmem(mbr_wbuf); ebr_wa = map_to_sysmem(ebr_wbuf); ra = map_to_sysmem(rbuf); - ebr_blk = (ulong)0xB00000 / 0x200; + ebr_blk = (ulong)0xb00000 / 0x200;
/* Make sure mmc6 exists */ ut_asserteq(6, blk_get_device_by_str("mmc", "6", &mmc_dev_desc)); @@ -267,7 +267,8 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_console_end();
/* Make sure mmc6 is 12+ MiB in size */ - ut_assertok(run_commandf("mmc read 0x%lx 0x%lx 1", ra, (ulong)0xBFFE00 / 0x200)); + ut_assertok(run_commandf("mmc read 0x%lx 0x%lx 1", ra, + (ulong)0xbffe00 / 0x200));
/* Test one MBR partition */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
U-Boot commands typically don't need 0x to specify hex, since they use hex by default. Adding 0x in this test is confusing since it suggests that it is necessary. Drop it from the file.
Also use the %#x construct to get the 0x when needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/cmd/mbr.c | 74 +++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index 98a745df6bc..c38199740a3 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -267,19 +267,19 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_console_end();
/* Make sure mmc6 is 12+ MiB in size */ - ut_assertok(run_commandf("mmc read 0x%lx 0x%lx 1", ra, + ut_assertok(run_commandf("mmc read %lx %lx 1", ra, (ulong)0xbffe00 / 0x200));
/* Test one MBR partition */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 1)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa)); + ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk)); + ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); @@ -288,7 +288,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assert_console_end(); /* @@ -299,23 +299,23 @@ static int mbr_test_run(struct unit_test_state *uts) 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p1[i], - "1P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "1P MBR+0x%04X: expected %#02X, actual: %#02X\n", mbr_cmp_start + i, mbr_parts_ref_p1[i], rbuf[mbr_cmp_start + i]); }
/* Test two MBR partitions */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 2)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa)); + ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk)); + ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); @@ -323,7 +323,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assert_console_end(); /* @@ -334,23 +334,23 @@ static int mbr_test_run(struct unit_test_state *uts) 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p2[i], - "2P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "2P MBR+0x%04X: expected %#02X, actual: %#02X\n", mbr_cmp_start + i, mbr_parts_ref_p2[i], rbuf[mbr_cmp_start + i]); }
/* Test three MBR partitions */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 3)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa)); + ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk)); + ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); @@ -358,7 +358,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assert_console_end(); /* @@ -369,23 +369,23 @@ static int mbr_test_run(struct unit_test_state *uts) 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p3[i], - "3P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "3P MBR+0x%04X: expected %#02X, actual: %#02X\n", mbr_cmp_start + i, mbr_parts_ref_p3[i], rbuf[mbr_cmp_start + i]); }
/* Test four MBR partitions */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 4)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa)); + ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk)); + ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); @@ -393,7 +393,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assert_console_end(); /* @@ -404,23 +404,23 @@ static int mbr_test_run(struct unit_test_state *uts) 000001f0 26 01 0e 87 06 01 00 58 00 00 00 08 00 00 55 aa |&......X......U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p4[i], - "4P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "4P MBR+0x%04X: expected %#02X, actual: %#02X\n", mbr_cmp_start + i, mbr_parts_ref_p4[i], rbuf[mbr_cmp_start + i]); }
/* Test five MBR partitions */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 5)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa)); + ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); - ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk)); + ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertf(0 == run_commandf("mbr write mmc 6"), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -436,10 +436,10 @@ static int mbr_test_run(struct unit_test_state *uts) 000001f0 26 01 05 a7 26 01 00 58 00 00 00 10 00 00 55 aa |&...&..X......U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra)); + ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p5[i], - "5P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "5P MBR+0x%04X: expected %#02X, actual: %#02X\n", mbr_cmp_start + i, mbr_parts_ref_p5[i], rbuf[mbr_cmp_start + i]); } /* @@ -450,10 +450,10 @@ static int mbr_test_run(struct unit_test_state *uts) 00b001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ memset(rbuf, 0, sizeof(rbuf)); - ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk)); + ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); for (unsigned i = 0; i < ebr_cmp_size; i++) { ut_assertf(rbuf[ebr_cmp_start + i] == ebr_parts_ref_p5[i], - "5P EBR+0x%04X: expected 0x%02X, actual: 0x%02X\n", + "5P EBR+0x%04X: expected %#02X, actual: %#02X\n", ebr_cmp_start + i, ebr_parts_ref_p5[i], rbuf[ebr_cmp_start + i]); }
Sandbox keeps a table of addresses which map to pointers which are outside its emulated DRAM. The current range from 10000000 conflicts with the PCI range, meaning that if PCI mapping is on, that particular address can be decoded by PCI instead of the table.
Fix this by moving the range up to the top of memory. Update the docs while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/state.c | 9 +++++---- doc/arch/sandbox/sandbox.rst | 21 ++++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c index a9ca79e76d2..49236db99c2 100644 --- a/arch/sandbox/cpu/state.c +++ b/arch/sandbox/cpu/state.c @@ -373,12 +373,13 @@ void state_reset_for_test(struct sandbox_state *state) memset(state->spi, '\0', sizeof(state->spi));
/* - * Set up the memory tag list. Use the top of emulated SDRAM for the - * first tag number, since that address offset is outside the legal - * range, and can be assumed to be a tag. + * Set up the memory tag list. We could use the top of emulated SDRAM + * for the first tag number, since that address offset is outside the + * legal SDRAM range, but PCI can have address there. So use a very + * large address instead */ INIT_LIST_HEAD(&state->mapmem_head); - state->next_tag = state->ram_size; + state->next_tag = 0xff000000; }
bool autoboot_keyed(void) diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst index 5f8db126657..1515f93c84b 100644 --- a/doc/arch/sandbox/sandbox.rst +++ b/doc/arch/sandbox/sandbox.rst @@ -655,14 +655,17 @@ Memory Map Sandbox has its own emulated memory starting at 0. Here are some of the things that are mapped into that memory:
-======= ======================== =============================== +======== ======================== =============================== Addr Config Usage -======= ======================== =============================== - 100 CONFIG_SYS_FDT_LOAD_ADDR Device tree - b000 CONFIG_BLOBLIST_ADDR Blob list - 10000 CFG_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 +======== ======================== =============================== + 100 CONFIG_SYS_FDT_LOAD_ADDR Device tree + b000 CONFIG_BLOBLIST_ADDR Blob list + 10000 CFG_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 as the SPL load buffer in spl_test_load(). - 200000 CONFIG_TEXT_BASE Load buffer for U-Boot (sandbox_spl only) -======= ======================== =============================== + 200000 CONFIG_TEXT_BASE Load buffer for U-Boot (sandbox_spl only) +10000000 PCI address space (see test.dts) + +ff000000 Memory-mapping tags start here +======== ======================== ===============================
Use log_debug() instead of including the function name in the string. Add one more debug for PCI.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/cpu.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 4f15a560902..8eb055d6837 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -109,8 +109,8 @@ void *phys_to_virt(phys_addr_t paddr) state = state_get_current(); list_for_each_entry(mentry, &state->mapmem_head, sibling_node) { if (mentry->tag == paddr) { - debug("%s: Used map from %lx to %p\n", __func__, - (ulong)paddr, mentry->ptr); + log_debug("Used map from %lx to %p\n", (ulong)paddr, + mentry->ptr); return mentry->ptr; } } @@ -130,11 +130,12 @@ struct sandbox_mapmem_entry *find_tag(const void *ptr)
list_for_each_entry(mentry, &state->mapmem_head, sibling_node) { if (mentry->ptr == ptr) { - debug("%s: Used map from %p to %lx\n", __func__, ptr, - mentry->tag); + log_debug("Used map from %p to %lx\n", ptr, + mentry->tag); return mentry; } } + return NULL; }
@@ -156,7 +157,7 @@ phys_addr_t virt_to_phys(void *ptr) __func__, ptr, (ulong)gd->ram_size); os_abort(); } - debug("%s: Used map from %p to %lx\n", __func__, ptr, mentry->tag); + log_debug("Used map from %p to %lx\n", ptr, mentry->tag);
return mentry->tag; } @@ -174,6 +175,7 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags) __func__, (uint)paddr, len, plen); } map_len = len; + log_debug("pci map %lx -> %p\n", (ulong)paddr, ptr); return ptr; } #endif @@ -218,8 +220,8 @@ phys_addr_t map_to_sysmem(const void *ptr) mentry->tag = state->next_tag++; mentry->ptr = (void *)ptr; list_add_tail(&mentry->sibling_node, &state->mapmem_head); - debug("%s: Added map from %p to %lx\n", __func__, ptr, - (ulong)mentry->tag); + log_debug("Added map from %p to %lx\n", ptr, + (ulong)mentry->tag); }
/*
So far unmapping has not been implemented. This means that if one test maps a pointer to an address with map_sysmem(), then a second test can use that same pointer, by mapping the address back to a pointer with map_to_sysmem(). This is not really desirable, even if it doesn't cause any problems at the moment.
Implement unmapping, to clean this up.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/cpu.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 8eb055d6837..3e1c0dd583e 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -185,12 +185,28 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
void unmap_physmem(const void *ptr, unsigned long flags) { + struct sandbox_mapmem_entry *mentry; + #ifdef CONFIG_PCI if (map_dev) { pci_unmap_physmem(ptr, map_len, map_dev); map_dev = NULL; } #endif + + /* If it is in emulated RAM, we didn't create a tag, so nothing to do */ + if (is_in_sandbox_mem(ptr)) + return; + + mentry = find_tag(ptr); + if (mentry) { + list_del(&mentry->sibling_node); + log_debug("Removed map from %p to %lx\n", ptr, + (ulong)mentry->tag); + free(mentry); + } else { + log_warning("Address not mapped: %p\n", ptr); + } }
phys_addr_t map_to_sysmem(const void *ptr)
Add a little debugging to this driver. Convert the existing debugging to use logging.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/lib/pci_io.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/sandbox/lib/pci_io.c b/arch/sandbox/lib/pci_io.c index 6040eacb594..5eff7c7d65d 100644 --- a/arch/sandbox/lib/pci_io.c +++ b/arch/sandbox/lib/pci_io.c @@ -8,6 +8,8 @@ * IO space access commands. */
+#define LOG_CATEGORY UCLASS_PCI + #include <command.h> #include <dm.h> #include <log.h> @@ -31,10 +33,11 @@ int pci_map_physmem(phys_addr_t paddr, unsigned long *lenp, if (ret) continue; *devp = dev; + log_debug("addr=%lx, dev=%s\n", (ulong)paddr, dev->name); return 0; }
- debug("%s: failed: addr=%pap\n", __func__, &paddr); + log_debug("%s: failed: addr=%pap\n", __func__, &paddr); return -ENOSYS; }
@@ -66,7 +69,7 @@ static int pci_io_read(unsigned int addr, ulong *valuep, pci_size_t size) } }
- debug("%s: failed: addr=%x\n", __func__, addr); + log_debug("%s: failed: addr=%x\n", __func__, addr); return -ENOSYS; }
@@ -87,7 +90,7 @@ static int pci_io_write(unsigned int addr, ulong value, pci_size_t size) } }
- debug("%s: failed: addr=%x, value=%lx\n", __func__, addr, value); + log_debug("%s: failed: addr=%x, value=%lx\n", __func__, addr, value); return -ENOSYS; }
An address may be mapped twice and unmapped twice. Delete the mapping only when the last user unmaps it.
Fix a missing comment while here.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/sandbox/cpu/cpu.c | 14 ++++++++++---- arch/sandbox/include/asm/state.h | 3 +++ 2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 3e1c0dd583e..51ce40e7f08 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -111,6 +111,7 @@ void *phys_to_virt(phys_addr_t paddr) if (mentry->tag == paddr) { log_debug("Used map from %lx to %p\n", (ulong)paddr, mentry->ptr); + mentry->refcnt++; return mentry->ptr; } } @@ -200,10 +201,12 @@ void unmap_physmem(const void *ptr, unsigned long flags)
mentry = find_tag(ptr); if (mentry) { - list_del(&mentry->sibling_node); - log_debug("Removed map from %p to %lx\n", ptr, - (ulong)mentry->tag); - free(mentry); + if (!--mentry->refcnt) { + list_del(&mentry->sibling_node); + log_debug("Removed map from %p to %lx\n", ptr, + (ulong)mentry->tag); + free(mentry); + } } else { log_warning("Address not mapped: %p\n", ptr); } @@ -235,11 +238,14 @@ phys_addr_t map_to_sysmem(const void *ptr) } mentry->tag = state->next_tag++; mentry->ptr = (void *)ptr; + mentry->refcnt = 0; list_add_tail(&mentry->sibling_node, &state->mapmem_head); log_debug("Added map from %p to %lx\n", ptr, (ulong)mentry->tag); }
+ mentry->refcnt++; + /* * Return the tag as the address to use. A later call to map_sysmem() * will return ptr diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 6b50473ed41..e7dc01759e8 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -53,10 +53,13 @@ struct sandbox_wdt_info { * be returned, just as it would for a normal sandbox address. * * @tag: Address tag (a value which U-Boot uses to refer to the address) + * @refcnt: Number of references to this tag * @ptr: Associated pointer for that tag + * @sibling_node: Next node */ struct sandbox_mapmem_entry { ulong tag; + uint refcnt; void *ptr; struct list_head sibling_node; };
The current implementation casts an address to a pointer. Make it more sandbox-friendly by using map_sysmem().
Rename the variable to 'ptr' since it is a pointer, not an address.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/mmc.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/cmd/mmc.c b/cmd/mmc.c index ff7b8e555ba..8aa7b7d906b 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -8,6 +8,7 @@ #include <command.h> #include <console.h> #include <display_options.h> +#include <mapmem.h> #include <memalign.h> #include <mmc.h> #include <part.h> @@ -349,12 +350,12 @@ static int do_mmc_read(struct cmd_tbl *cmdtp, int flag, { struct mmc *mmc; u32 blk, cnt, n; - void *addr; + void *ptr;
if (argc != 4) return CMD_RET_USAGE;
- addr = (void *)hextoul(argv[1], NULL); + ptr = map_sysmem(hextoul(argv[1], NULL), 0); blk = hextoul(argv[2], NULL); cnt = hextoul(argv[3], NULL);
@@ -365,8 +366,9 @@ static int do_mmc_read(struct cmd_tbl *cmdtp, int flag, printf("MMC read: dev # %d, block # %d, count %d ... ", curr_device, blk, cnt);
- n = blk_dread(mmc_get_blk_desc(mmc), blk, cnt, addr); + n = blk_dread(mmc_get_blk_desc(mmc), blk, cnt, ptr); printf("%d blocks read: %s\n", n, (n == cnt) ? "OK" : "ERROR"); + unmap_sysmem(ptr);
return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE; } @@ -442,12 +444,12 @@ static int do_mmc_write(struct cmd_tbl *cmdtp, int flag, { struct mmc *mmc; u32 blk, cnt, n; - void *addr; + void *ptr;
if (argc != 4) return CMD_RET_USAGE;
- addr = (void *)hextoul(argv[1], NULL); + ptr = map_sysmem(hextoul(argv[1], NULL), 0); blk = hextoul(argv[2], NULL); cnt = hextoul(argv[3], NULL);
@@ -462,8 +464,9 @@ static int do_mmc_write(struct cmd_tbl *cmdtp, int flag, printf("Error: card is write protected!\n"); return CMD_RET_FAILURE; } - n = blk_dwrite(mmc_get_blk_desc(mmc), blk, cnt, addr); + n = blk_dwrite(mmc_get_blk_desc(mmc), blk, cnt, ptr); printf("%d blocks written: %s\n", n, (n == cnt) ? "OK" : "ERROR"); + unmap_sysmem(ptr);
return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE; }
Rename the variable to 'ptr' since it is a pointer, not an address. Make sure to unmap the pointer.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - Correct the commit subject and message
cmd/read.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/cmd/read.c b/cmd/read.c index af54bd17654..8e21f004423 100644 --- a/cmd/read.c +++ b/cmd/read.c @@ -20,7 +20,7 @@ do_rw(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct disk_partition part_info; ulong offset, limit; uint blk, cnt, res; - void *addr; + void *ptr; int part;
if (argc != 6) { @@ -33,7 +33,7 @@ do_rw(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (part < 0) return 1;
- addr = map_sysmem(hextoul(argv[3], NULL), 0); + ptr = map_sysmem(hextoul(argv[3], NULL), 0); blk = hextoul(argv[4], NULL); cnt = hextoul(argv[5], NULL);
@@ -48,13 +48,15 @@ do_rw(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
if (cnt + blk > limit) { printf("%s out of range\n", cmdtp->name); + unmap_sysmem(ptr); return 1; }
if (IS_ENABLED(CONFIG_CMD_WRITE) && !strcmp(cmdtp->name, "write")) - res = blk_dwrite(dev_desc, offset + blk, cnt, addr); + res = blk_dwrite(dev_desc, offset + blk, cnt, ptr); else - res = blk_dread(dev_desc, offset + blk, cnt, addr); + res = blk_dread(dev_desc, offset + blk, cnt, ptr); + unmap_sysmem(ptr);
if (res != cnt) { printf("%s error\n", cmdtp->name);
This unmaps a different address from what was mapped. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/mem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c index 274348068c2..4d6fde28531 100644 --- a/cmd/mem.c +++ b/cmd/mem.c @@ -245,7 +245,7 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, int size; int rcode = 0; const char *type; - const void *buf1, *buf2, *base; + const void *buf1, *buf2, *base, *ptr1, *ptr2; ulong word1, word2; /* 64-bit if MEM_SUPPORT_64BIT_DATA */
if (argc != 4) @@ -270,22 +270,22 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, bytes = size * count; base = buf1 = map_sysmem(addr1, bytes); buf2 = map_sysmem(addr2, bytes); - for (ngood = 0; ngood < count; ++ngood) { + for (ngood = 0, ptr1 = buf1, ptr2 = buf2; ngood < count; ++ngood) { if (size == 4) { - word1 = *(u32 *)buf1; - word2 = *(u32 *)buf2; + word1 = *(u32 *)ptr1; + word2 = *(u32 *)ptr2; } else if (MEM_SUPPORT_64BIT_DATA && size == 8) { - word1 = *(ulong *)buf1; - word2 = *(ulong *)buf2; + word1 = *(ulong *)ptr1; + word2 = *(ulong *)ptr2; } else if (size == 2) { - word1 = *(u16 *)buf1; - word2 = *(u16 *)buf2; + word1 = *(u16 *)ptr1; + word2 = *(u16 *)ptr2; } else { - word1 = *(u8 *)buf1; - word2 = *(u8 *)buf2; + word1 = *(u8 *)ptr1; + word2 = *(u8 *)ptr2; } if (word1 != word2) { - ulong offset = buf1 - base; + ulong offset = ptr1 - base; printf("%s at 0x%08lx (%#0*lx) != %s at 0x%08lx (%#0*lx)\n", type, (ulong)(addr1 + offset), size, word1, type, (ulong)(addr2 + offset), size, word2); @@ -293,8 +293,8 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, break; }
- buf1 += size; - buf2 += size; + ptr1 += size; + ptr2 += size;
/* reset watchdog from time to time */ if ((ngood % (64 << 10)) == 0)
Hi Simon,
On 9/2/24 12:26 AM, Simon Glass wrote:
This unmaps a different address from what was mapped. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
cmd/mem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c index 274348068c2..4d6fde28531 100644 --- a/cmd/mem.c +++ b/cmd/mem.c @@ -245,7 +245,7 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, int size; int rcode = 0; const char *type;
- const void *buf1, *buf2, *base;
const void *buf1, *buf2, *base, *ptr1, *ptr2; ulong word1, word2; /* 64-bit if MEM_SUPPORT_64BIT_DATA */
if (argc != 4)
@@ -270,22 +270,22 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, bytes = size * count; base = buf1 = map_sysmem(addr1, bytes);
"base" isn't changed in the rest of the code, so we could just reuse it instead of declaring yet another variable.
buf2 = map_sysmem(addr2, bytes);
We could also set ptr2 here... Allowing to avoid the diff from here to....
- for (ngood = 0; ngood < count; ++ngood) {
- for (ngood = 0, ptr1 = buf1, ptr2 = buf2; ngood < count; ++ngood) { > if (size == 4) {
word1 = *(u32 *)buf1;word2 = *(u32 *)buf2;
word1 = *(u32 *)ptr1;word2 = *(u32 *)ptr2;
word1 = *(ulong *)buf1;word2 = *(ulong *)buf2;
word1 = *(ulong *)ptr1;word2 = *(ulong *)ptr2;
word1 = *(u16 *)buf1;word2 = *(u16 *)buf2;
word1 = *(u16 *)ptr1;word2 = *(u16 *)ptr2;
word1 = *(u8 *)buf1;word2 = *(u8 *)buf2;
word1 = *(u8 *)ptr1;word2 = *(u8 *)ptr2;
ulong offset = buf1 - base;
ulong offset = ptr1 - base; printf("%s at 0x%08lx (%#0*lx) != %s at 0x%08lx (%#0*lx)\n", type, (ulong)(addr1 + offset), size, word1, type, (ulong)(addr2 + offset), size, word2);@@ -293,8 +293,8 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, break; }
buf1 += size;buf2 += size;
ptr1 += size;ptr2 += size;
here - making the commit all the more explicit (for me this commit is basically only renaming a variable, since unmap_system doesn't appear in the git context) - by only changing:
unmap_system(buf1); unmap_system(buf2);
to
unmap_system(base); unmap_system(ptr2);
I believe?
Additionally, my linter tells me that:
buf1 += size; buf2 += size;
is undefined behavior:
arithOperationsOnVoidPointer: 'buf1' is of type 'const void *'. When using void pointers in calculations, the behaviour is undefined.
I suggest the following:
buf1 = ((u8 *)buf1) + size; buf2 = ((u8 *)buf2) + size;
since size seems to be size in bytes?
What do you think?
We already have test/cmd/mem.c is this something we can augment to test the unmapping is proper too?
Cheers, Quentin
Hi Quentin,
On Tue, 3 Sept 2024 at 11:42, Quentin Schulz quentin.schulz@cherry.de wrote:
Hi Simon,
On 9/2/24 12:26 AM, Simon Glass wrote:
This unmaps a different address from what was mapped. Fix it.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
cmd/mem.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/cmd/mem.c b/cmd/mem.c index 274348068c2..4d6fde28531 100644 --- a/cmd/mem.c +++ b/cmd/mem.c @@ -245,7 +245,7 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, int size; int rcode = 0; const char *type;
const void *buf1, *buf2, *base;
const void *buf1, *buf2, *base, *ptr1, *ptr2; ulong word1, word2; /* 64-bit if MEM_SUPPORT_64BIT_DATA */ if (argc != 4)@@ -270,22 +270,22 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, bytes = size * count; base = buf1 = map_sysmem(addr1, bytes);
"base" isn't changed in the rest of the code, so we could just reuse it instead of declaring yet another variable.
buf2 = map_sysmem(addr2, bytes);We could also set ptr2 here... Allowing to avoid the diff from here to....
for (ngood = 0; ngood < count; ++ngood) {
for (ngood = 0, ptr1 = buf1, ptr2 = buf2; ngood < count; ++ngood) { > if (size == 4) {
word1 = *(u32 *)buf1;word2 = *(u32 *)buf2;
word1 = *(u32 *)ptr1;word2 = *(u32 *)ptr2; } else if (MEM_SUPPORT_64BIT_DATA && size == 8) {
word1 = *(ulong *)buf1;word2 = *(ulong *)buf2;
word1 = *(ulong *)ptr1;word2 = *(ulong *)ptr2; } else if (size == 2) {
word1 = *(u16 *)buf1;word2 = *(u16 *)buf2;
word1 = *(u16 *)ptr1;word2 = *(u16 *)ptr2; } else {
word1 = *(u8 *)buf1;word2 = *(u8 *)buf2;
word1 = *(u8 *)ptr1;word2 = *(u8 *)ptr2; } if (word1 != word2) {
ulong offset = buf1 - base;
ulong offset = ptr1 - base; printf("%s at 0x%08lx (%#0*lx) != %s at 0x%08lx (%#0*lx)\n", type, (ulong)(addr1 + offset), size, word1, type, (ulong)(addr2 + offset), size, word2);@@ -293,8 +293,8 @@ static int do_mem_cmp(struct cmd_tbl *cmdtp, int flag, int argc, break; }
buf1 += size;buf2 += size;
ptr1 += size;ptr2 += size;here - making the commit all the more explicit (for me this commit is basically only renaming a variable, since unmap_system doesn't appear in the git context) - by only changing:
unmap_system(buf1); unmap_system(buf2);
to
unmap_system(base); unmap_system(ptr2);
I believe?
Additionally, my linter tells me that:
buf1 += size; buf2 += size;
is undefined behavior:
arithOperationsOnVoidPointer: 'buf1' is of type 'const void *'. When using void pointers in calculations, the behaviour is undefined.
Hmmm I thought that got changed...or at least I rely on void * being taken as char * when diffing pointers.
I suggest the following:
buf1 = ((u8 *)buf1) + size; buf2 = ((u8 *)buf2) + size;
since size seems to be size in bytes?
What do you think?
Well I think you should send a patch! It seems reasonable to me.
We already have test/cmd/mem.c is this something we can augment to test the unmapping is proper too?
Yes, it would be possible to check the list of mapped pointers in sandbox, to make sure it has gone back to its original state (i.e. any new mappings in the test were deleted before the test ended).
Regards, Simon
This tests maps some local variables into sandbox's address space. Make sure to unmap them afterwards.
Note that the normal approach with sandbox is to use a fixed memory address in the RAM, to avoid needing to create a map for transient local variables.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 04291ee0aba ("cmd: mbr: Allow 4 MBR partitions without need...") ---
(no changes since v3)
Changes in v3: - Add a Fixes tag
test/cmd/mbr.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index c38199740a3..e750875ea43 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -456,6 +456,9 @@ static int mbr_test_run(struct unit_test_state *uts) "5P EBR+0x%04X: expected %#02X, actual: %#02X\n", ebr_cmp_start + i, ebr_parts_ref_p5[i], rbuf[ebr_cmp_start + i]); } + unmap_sysmem(mbr_wbuf); + unmap_sysmem(ebr_wbuf); + unmap_sysmem(rbuf);
return 0; }
It isn't that important to factor out constants in tests, but in this case we have 0x200 and 512 used. The commands don't use the constant as they use a block count ('1'). It doesn't create more code to use a constant, so create one.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v3)
Changes in v3: - use SZ_512 instead of 0x200
test/cmd/mbr.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index e750875ea43..e67e777c995 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -14,10 +14,14 @@ #include <asm/global_data.h> #include <dm/device-internal.h> #include <dm/lists.h> +#include <linux/sizes.h> #include <test/suites.h> #include <test/ut.h>
DECLARE_GLOBAL_DATA_PTR; + +#define BLKSZ SZ_512 /* block size */ + /* * Requirements for running test manually: * mmc6.img - File size needs to be at least 12 MiB @@ -228,7 +232,7 @@ static unsigned build_mbr_parts(char *buf, size_t buf_size, unsigned num_parts) static int mbr_test_run(struct unit_test_state *uts) { struct blk_desc *mmc_dev_desc; - unsigned char mbr_wbuf[512], ebr_wbuf[512], rbuf[512]; + unsigned char mbr_wbuf[BLKSZ], ebr_wbuf[BLKSZ], rbuf[BLKSZ]; char mbr_parts_buf[256]; ulong mbr_wa, ebr_wa, ra, ebr_blk, mbr_parts_max; struct udevice *dev; @@ -257,7 +261,7 @@ static int mbr_test_run(struct unit_test_state *uts) mbr_wa = map_to_sysmem(mbr_wbuf); ebr_wa = map_to_sysmem(ebr_wbuf); ra = map_to_sysmem(rbuf); - ebr_blk = (ulong)0xb00000 / 0x200; + ebr_blk = (ulong)0xb00000 / BLKSZ;
/* Make sure mmc6 exists */ ut_asserteq(6, blk_get_device_by_str("mmc", "6", &mmc_dev_desc)); @@ -268,7 +272,7 @@ static int mbr_test_run(struct unit_test_state *uts)
/* Make sure mmc6 is 12+ MiB in size */ ut_assertok(run_commandf("mmc read %lx %lx 1", ra, - (ulong)0xbffe00 / 0x200)); + (ulong)0xbffe00 / BLKSZ));
/* Test one MBR partition */ init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); @@ -276,11 +280,11 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); - ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); + ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); ut_assert_nextlinen("MMC read: dev # 6"); @@ -289,7 +293,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); /* 000001b0 00 00 00 00 00 00 00 00 78 56 34 12 00 00 80 05 |........xV4.....| @@ -312,11 +316,11 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); - ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); + ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); ut_assert_nextline("MBR: write success!"); @@ -324,7 +328,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); /* 000001b0 00 00 00 00 00 00 00 00 78 56 34 12 00 00 80 05 |........xV4.....| @@ -347,11 +351,11 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); - ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); + ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); ut_assert_nextline("MBR: write success!"); @@ -359,7 +363,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); /* 000001b0 00 00 00 00 00 00 00 00 78 56 34 12 00 00 80 05 |........xV4.....| @@ -382,11 +386,11 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); - ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); + ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertok(run_commandf("mbr write mmc 6")); ut_assert_nextline("MBR: write success!"); @@ -394,7 +398,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: verify success!"); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); /* 000001b0 00 00 00 00 00 00 00 00 78 56 34 12 00 00 80 05 |........xV4.....| @@ -417,11 +421,11 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); - ut_assertok(memcmp(mbr_wbuf, rbuf, 512)); + ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); memset(rbuf, 0, sizeof(rbuf)); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); - ut_assertok(memcmp(ebr_wbuf, rbuf, 512)); + ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assertf(0 == run_commandf("mbr write mmc 6"), "Invalid partitions string: %s\n", mbr_parts_buf); ut_assert_nextline("MBR: write success!");
The normal approach with sandbox is to use a fixed memory address in the RAM, to avoid needing to create a map for transient local variables.
Update this test to use this approach.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/cmd/mbr.c | 52 ++++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index e67e777c995..72799612381 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -232,9 +232,11 @@ static unsigned build_mbr_parts(char *buf, size_t buf_size, unsigned num_parts) static int mbr_test_run(struct unit_test_state *uts) { struct blk_desc *mmc_dev_desc; - unsigned char mbr_wbuf[BLKSZ], ebr_wbuf[BLKSZ], rbuf[BLKSZ]; + unsigned char *mbr_wbuf, *ebr_wbuf, *rbuf; char mbr_parts_buf[256]; - ulong mbr_wa, ebr_wa, ra, ebr_blk, mbr_parts_max; + ulong addr = 0x1000; /* start address for buffers */ + ulong mbr_wa = addr, ebr_wa = addr + BLKSZ, ra = addr + BLKSZ * 2; + ulong ebr_blk, mbr_parts_max; struct udevice *dev; ofnode root, node;
@@ -258,9 +260,9 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assertf(sizeof(mbr_parts_buf) >= mbr_parts_max, "Buffer avail: %ld; buffer req: %ld\n", sizeof(mbr_parts_buf), mbr_parts_max);
- mbr_wa = map_to_sysmem(mbr_wbuf); - ebr_wa = map_to_sysmem(ebr_wbuf); - ra = map_to_sysmem(rbuf); + mbr_wbuf = map_sysmem(mbr_wa, BLKSZ); + ebr_wbuf = map_sysmem(ebr_wa, BLKSZ); + rbuf = map_sysmem(ra, BLKSZ); ebr_blk = (ulong)0xb00000 / BLKSZ;
/* Make sure mmc6 exists */ @@ -278,11 +280,11 @@ static int mbr_test_run(struct unit_test_state *uts) init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 1)); ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -291,7 +293,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: write success!"); ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); @@ -302,7 +304,7 @@ static int mbr_test_run(struct unit_test_state *uts) 000001e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p1[i], @@ -314,11 +316,11 @@ static int mbr_test_run(struct unit_test_state *uts) init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 2)); ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -326,7 +328,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: write success!"); ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); @@ -337,7 +339,7 @@ static int mbr_test_run(struct unit_test_state *uts) 000001e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p2[i], @@ -349,11 +351,11 @@ static int mbr_test_run(struct unit_test_state *uts) init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 3)); ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -361,7 +363,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: write success!"); ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); @@ -372,7 +374,7 @@ static int mbr_test_run(struct unit_test_state *uts) 000001e0 06 01 0e 66 25 01 00 50 00 00 00 08 00 00 00 00 |...f%..P........| 000001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p3[i], @@ -384,11 +386,11 @@ static int mbr_test_run(struct unit_test_state *uts) init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 4)); ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -396,7 +398,7 @@ static int mbr_test_run(struct unit_test_state *uts) ut_assert_nextline("MBR: write success!"); ut_assertok(run_commandf("mbr verify mmc 6")); ut_assert_nextline("MBR: verify success!"); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assert_console_end(); @@ -407,7 +409,7 @@ static int mbr_test_run(struct unit_test_state *uts) 000001e0 06 01 0e 66 25 01 00 50 00 00 00 08 00 00 00 66 |...f%..P.......f| 000001f0 26 01 0e 87 06 01 00 58 00 00 00 08 00 00 55 aa |&......X......U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p4[i], @@ -419,11 +421,11 @@ static int mbr_test_run(struct unit_test_state *uts) init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__); ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 5)); ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ)); ut_assertok(run_commandf("write mmc 6:0 %lx %lx 1", ebr_wa, ebr_blk)); - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); ut_assertok(memcmp(ebr_wbuf, rbuf, BLKSZ)); ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf); @@ -439,7 +441,7 @@ static int mbr_test_run(struct unit_test_state *uts) 000001e0 06 01 0e 66 25 01 00 50 00 00 00 08 00 00 00 66 |...f%..P.......f| 000001f0 26 01 05 a7 26 01 00 58 00 00 00 10 00 00 55 aa |&...&..X......U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra)); for (unsigned i = 0; i < mbr_cmp_size; i++) { ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p5[i], @@ -453,7 +455,7 @@ static int mbr_test_run(struct unit_test_state *uts) 00b001e0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| 00b001f0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 aa |..............U.| */ - memset(rbuf, 0, sizeof(rbuf)); + memset(rbuf, '\0', BLKSZ); ut_assertok(run_commandf("read mmc 6:0 %lx %lx 1", ra, ebr_blk)); for (unsigned i = 0; i < ebr_cmp_size; i++) { ut_assertf(rbuf[ebr_cmp_start + i] == ebr_parts_ref_p5[i],
The test currently runs twice as it is declared twice. Unwind this.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Rebase to -next (on top of testc series) - Drop patch to add UT_TESTF_CONSOLE_REC to bootflow tests
Changes in v2: - Add many new patches to resolve all the outstanding test issues
test/cmd/mbr.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c index 72799612381..b14137e589a 100644 --- a/test/cmd/mbr.c +++ b/test/cmd/mbr.c @@ -479,9 +479,3 @@ int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
return cmd_ut_category("mbr", "mbr_test_", tests, n_ents, argc, argv); } - -static int dm_test_cmd_mbr(struct unit_test_state *uts) -{ - return mbr_test_run(uts); -} -DM_TEST(dm_test_cmd_mbr, UTF_SCAN_FDT | UTF_CONSOLE);
On Sun, 01 Sep 2024 16:26:11 -0600, Simon Glass wrote:
This series includes the patches needed to make make the EFI 'boot' test work. That test has now been split off into a separate series along with the EFI patches.
This series fixes these problems:
- sandbox memory-mapping conflict with PCI
- the fix for that causes the mbr test to crash as it sets up pointers instead of addresses for its 'mmc' commands
- the mmc and read commands which cast addresses to pointers
- a tricky bug to do with USB keyboard and stdio
- a few other minor things
[...]
Applied to u-boot/next, thanks!
participants (4)
-
Abdellatif El Khlifi -
Quentin Schulz -
Simon Glass -
Tom Rini