[PATCH v4 00/23] Fix various bugs

[PATCH v4 00/23] Fix various bugs

[Simon Glass]

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(-)

-- 
2.34.1

[PATCH v4 01/23] nvmxip: Drop the message on probe

[Simon Glass]

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;
 }
-- 
2.34.1

[PATCH v4 02/23] nvmxip: Avoid probing on boot

[Simon Glass]

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,
 };
-- 
2.34.1

[PATCH v4 03/23] test/py: Fix some pylint warnings in test_ut.py

[Simon Glass]

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')
-- 
2.34.1

[PATCH v4 04/23] scripts: Update pylint.base

[Simon Glass]

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
-- 
2.34.1

[PATCH v4 05/23] bootstd: Create a function to reset USB

[Simon Glass]

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
-- 
2.34.1

[PATCH v4 06/23] usb: Drop old non-DM code

[Simon Glass]

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
  *
-- 
2.34.1

[PATCH v4 07/23] log: Add a new log category for the console

[Simon Glass]

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 */
-- 
2.34.1

[PATCH v4 08/23] usb: Add DEV_FLAGS_DM to stdio for USB keyboard

[Simon Glass]

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;
-- 
2.34.1

[PATCH v4 09/23] dm: usb: Deal with USB keyboard persisting across tests

[Simon Glass]

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));
 
-- 
2.34.1

[PATCH v4 10/23] test: mbr: Adjust test to use lower-case hex

[Simon Glass]

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__);
-- 
2.34.1

[PATCH v4 11/23] test: mbr: Adjust test to drop 0x

[Simon Glass]

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]);
 	}
 
-- 
2.34.1

[PATCH v4 12/23] sandbox: Change the range used for memory-mapping tags

[Simon Glass]

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
+========  ========================   ===============================
-- 
2.34.1

[PATCH v4 13/23] sandbox: Update cpu to use logging

[Simon Glass]

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);
 	}
 
 	/*
-- 
2.34.1

[PATCH v4 14/23] sandbox: Unmap old tags

[Simon Glass]

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)
-- 
2.34.1

[PATCH v4 15/23] sandbox: Add some debugging to pci_io

[Simon Glass]

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;
 }
 
-- 
2.34.1

[PATCH v4 16/23] sandbox: Implement reference counting for address mapping

[Simon Glass]

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;
 };
-- 
2.34.1

[PATCH v4 17/23] mmc: Use map_sysmem() with buffers in the mmc command

[Simon Glass]

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;
 }
-- 
2.34.1

[PATCH v4 18/23] read: Tidy up use of map_sysmem() in the read command

[Simon Glass]

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);
-- 
2.34.1

[PATCH v4 19/23] cmd: Fix memory-mapping in cmp command

[Simon Glass]

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)
-- 
2.34.1

[PATCH v4 20/23] test: mbr: Unmap the buffers after use

[Simon Glass]

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;
 }
-- 
2.34.1

[PATCH v4 21/23] test: mbr: Use a constant for the block size

[Simon Glass]

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!");
-- 
2.34.1

[PATCH v4 22/23] test: mbr: Use RAM for the buffers

[Simon Glass]

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],
-- 
2.34.1

[PATCH v4 23/23] test: mbr: Drop a duplicate test

[Simon Glass]

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);
-- 
2.34.1

Re: [PATCH v4 02/23] nvmxip: Avoid probing on boot

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 19/23] cmd: Fix memory-mapping in cmp command

[Quentin Schulz]

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.

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

Re: [PATCH v4 02/23] nvmxip: Avoid probing on boot

[Simon Glass]

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

Re: [PATCH v4 02/23] nvmxip: Avoid probing on boot

[Abdellatif El Khlifi]

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

Re: [PATCH v4 00/23] Fix various bugs

[Tom Rini]

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!

-- 
Tom

Re: [PATCH v4 19/23] cmd: Fix memory-mapping in cmp command

[Simon Glass]

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