Re: [PATCH v1 6/6] sandbox64: add a test case for UCLASS_NVMXIP

Hi,

On Mon, 16 Jan 2023 at 10:29, abdellatif.elkhlifi@arm.com wrote:

From: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com

provide a test for NVM XIP devices

The test case allows to make sure of the following:

• The NVM XIP QSPI devices are probed
• The DT entries are read correctly
• the data read from the flash by the NVMXIP block driver is correct

Signed-off-by: Abdellatif El Khlifi abdellatif.elkhlifi@arm.com

MAINTAINERS | 1 + test/dm/Makefile | 5 ++ test/dm/nvmxip.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 test/dm/nvmxip.c

diff --git a/MAINTAINERS b/MAINTAINERS index 539d01f68c..e92898e462 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1199,6 +1199,7 @@ S: Maintained F: doc/develop/driver-model/nvmxip.rst F: doc/device-tree-bindings/nvmxip/nvmxip.txt F: drivers/nvmxip/ +F: test/dm/nvmxip.c

NVMEM M: Sean Anderson seanga2@gmail.com diff --git a/test/dm/Makefile b/test/dm/Makefile index 7a79b6e1a2..7f9d0b3c38 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ # # Copyright (c) 2013 Google, Inc +# Copyright 2023 Arm Limited and/or its affiliates open-source-office@arm.com

obj-$(CONFIG_UT_DM) += test-dm.o

@@ -17,6 +18,10 @@ obj-$(CONFIG_UT_DM) += test-uclass.o obj-$(CONFIG_UT_DM) += core.o obj-$(CONFIG_UT_DM) += read.o obj-$(CONFIG_UT_DM) += phys2bus.o +ifeq ($(CONFIG_NVMXIP_QSPI)$(CONFIG_SANDBOX64),yy) +obj-y += nvmxip.o +endif

ifneq ($(CONFIG_SANDBOX),) ifeq ($(CONFIG_ACPIGEN),y) obj-y += acpi.o diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c new file mode 100644 index 0000000000..b65154d125 --- /dev/null +++ b/test/dm/nvmxip.c @@ -0,0 +1,117 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Functional tests for UCLASS_FFA class
• • Copyright 2023 Arm Limited and/or its affiliates open-source-office@arm.com
• • Authors:
• • Abdellatif El Khlifi abdellatif.elkhlifi@arm.com
• */

+#include <common.h> +#include <console.h> +#include <blk.h> +#include <dm.h> +#include <dm/test.h> +#include "../../drivers/nvmxip/nvmxip.h"

move to end

+#include <test/test.h> +#include <test/ut.h>

+/* NVMXIP devices described in the device tree */ +#define SANDBOX_NVMXIP_DEVICES 2

+/* reference device tree data for the probed devices */ +static struct nvmxip_plat nvmqspi_refdata[SANDBOX_NVMXIP_DEVICES] = {

•   {0x08000000, 9, 4096}, {0x08200000, 9, 2048}
  

+};

+#define NVMXIP_BLK_START_PATTERN 0x1122334455667788ULL +#define NVMXIP_BLK_END_PATTERN 0xa1a2a3a4a5a6a7a8ULL

+static int dm_nvmxip_flash_sanity(u8 device_idx, void *buffer) +{

•   int i;
  

•   u64 *ptr = NULL;
  

•   u8 *base = NULL;
  

•   unsigned long blksz;
  

•   blksz = 1 << nvmqspi_refdata[device_idx].lba_shift;
  

BIT() or BITUL() ?

•   /* if buffer not NULL, init the flash with the pattern data*/
  

space before */ (please fix globally)

can you explain why, in the comment?

•   if (!buffer)
  

•           base = map_sysmem(nvmqspi_refdata[device_idx].phys_base, 0);
  

•   else
  

•           base = buffer;
  

•   for (i = 0; i < nvmqspi_refdata[device_idx].lba ; i++) {
  

•           ptr = (u64 *)(base + i * blksz);
  

•           /* write an 8 bytes pattern at the start of the current block*/
  

•           if (!buffer)
  

•                   *ptr = NVMXIP_BLK_START_PATTERN;
  

•           else if (*ptr != NVMXIP_BLK_START_PATTERN)
  

•                   return -EINVAL;
  

•           ptr = (u64 *)((u8 *)ptr + blksz - sizeof(u64));
  

•           /* write an 8 bytes pattern at the end of the current block*/
  

•           if (!buffer)
  

•                   *ptr = NVMXIP_BLK_END_PATTERN;
  

•           else if (*ptr != NVMXIP_BLK_END_PATTERN)
  

•                   return -EINVAL;
  

You can use ut_assert...() here so that people can see which line failed

•   }
  

•   if (!buffer)
  

•           unmap_sysmem(base);
  

•   return 0;
  

+}

+static int dm_test_nvmxip(struct unit_test_state *uts) +{

•   struct nvmxip_plat *plat_data = NULL;
  

•   struct udevice *dev = NULL, *bdev = NULL;
  

•   u8 device_idx;
  

•   void *buffer = NULL;
  

•   unsigned long flashsz;
  

•   /* set the flash content first for both devices */
  

•   dm_nvmxip_flash_sanity(0, NULL);
  

•   dm_nvmxip_flash_sanity(1, NULL);
  

•   /*  probing all NVM XIP QSPI devices */
  

•   for (device_idx = 0, uclass_first_device(UCLASS_NVMXIP, &dev);
  

•        dev;
  

•        uclass_next_device(&dev), device_idx++) {
  

•           plat_data = dev_get_plat(dev);
  

•           /* device tree entries checks */
  

•           ut_assertok(nvmqspi_refdata[device_idx].phys_base != plat_data->phys_base);
  

•           ut_assertok(nvmqspi_refdata[device_idx].lba_shift != plat_data->lba_shift);
  

•           ut_assertok(nvmqspi_refdata[device_idx].lba != plat_data->lba);
  

•           /* before reading all the flash blocks, let's calculate the flash size */
  

•           flashsz = plat_data->lba << plat_data->lba_shift;
  

•           /* allocate the user buffer where to copy the blocks data to */
  

•           buffer = calloc(flashsz, 1);
  

•           ut_assertok(!buffer);
  

•           /* the block device is the child of the parent device probed with DT*/
  

•           ut_assertok(device_find_first_child(dev, &bdev));
  

•           /* reading all the flash blocks*/
  

•           ut_asserteq(plat_data->lba, blk_read(bdev, 0, plat_data->lba, buffer));
  

•           /* compare the data read from flash with the expected data */
  

•           ut_assertok(dm_nvmxip_flash_sanity(device_idx, buffer));
  

•           free(buffer);
  

•   }
  

•   ut_assertok(device_idx != SANDBOX_NVMXIP_DEVICES);
  

•   return CMD_RET_SUCCESS;
  

+}

+DM_TEST(dm_test_nvmxip, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);

2.17.1

It seems like you are using the same driver for sandbox as for the real hardware. Is that right? There is nothing really wrong with that, it's just unusual.