* [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems
@ 2023-08-13 21:46 Marek Vasut
2023-08-13 21:46 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Marek Vasut
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-13 21:46 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Abdellatif El Khlifi, Simon Glass
Get rid of nvmxip_mmio_rawread() and just implement the readl()/readq()
reader loop within nvmxip_blk_read(). Cast the destination buffer as
needed and increment the read by either 4 or 8 bytes depending on if
this is systemd with 32bit or 64bit physical address.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Simon Glass <sjg@chromium.org>
---
drivers/mtd/nvmxip/nvmxip.c | 38 ++++++++++++-------------------------
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
index a359e3b4822..0bd98d64275 100644
--- a/drivers/mtd/nvmxip/nvmxip.c
+++ b/drivers/mtd/nvmxip/nvmxip.c
@@ -15,23 +15,6 @@
#include <linux/errno.h>
#include "nvmxip.h"
-/**
- * nvmxip_mmio_rawread() - read from the XIP flash
- * @address: address of the data
- * @value: pointer to where storing the value read
- *
- * Read raw data from the XIP flash.
- *
- * Return:
- *
- * Always return 0.
- */
-static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
-{
- *value = readq(address);
- return 0;
-}
-
/**
* nvmxip_blk_read() - block device read operation
* @dev: the block device
@@ -49,15 +32,14 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
{
struct nvmxip_plat *plat = dev_get_plat(dev->parent);
struct blk_desc *desc = dev_get_uclass_plat(dev);
- /* number of the u64 words to read */
- u32 qwords = (blkcnt * desc->blksz) / sizeof(u64);
+ /* number of bytes to read */
+ u32 size = blkcnt * desc->blksz;
/* physical address of the first block to read */
phys_addr_t blkaddr = plat->phys_base + blknr * desc->blksz;
- u64 *virt_blkaddr;
- u64 *pdst = buffer;
+ void *virt_blkaddr;
uint qdata_idx;
- if (!pdst)
+ if (!buffer)
return -EINVAL;
log_debug("[%s]: reading from blknr: %lu , blkcnt: %lu\n", dev->name, blknr, blkcnt);
@@ -66,12 +48,16 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
/* assumption: the data is virtually contiguous */
- for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
- nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
-
+#if IS_ENABLED(CONFIG_PHYS_64BIT)
+ for (qdata_idx = 0 ; qdata_idx < size; qdata_idx += sizeof(u64))
+ *(u64 *)(buffer + qdata_idx) = readq(virt_blkaddr + qdata_idx);
+#else
+ for (qdata_idx = 0 ; qdata_idx < size; qdata_idx += sizeof(u32))
+ *(u32 *)(buffer + qdata_idx) = readl(virt_blkaddr + qdata_idx);
+#endif
log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
dev->name,
- *virt_blkaddr,
+ *(u64 *)virt_blkaddr,
*(u64 *)buffer,
*(u64 *)((u8 *)virt_blkaddr + desc->blksz * blkcnt - sizeof(u64)),
*(u64 *)((u8 *)buffer + desc->blksz * blkcnt - sizeof(u64)));
--
2.40.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test
2023-08-13 21:46 [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
@ 2023-08-13 21:46 ` Marek Vasut
2023-08-14 22:42 ` Simon Glass
2023-08-13 23:55 ` [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
2023-08-19 0:23 ` Marek Vasut
2 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2023-08-13 21:46 UTC (permalink / raw)
To: u-boot; +Cc: Marek Vasut, Abdellatif El Khlifi, Simon Glass
Enable NVMXIP QSPI driver on sandbox, since it is already enabled
on sandbox64. Update blk tests to match.
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Simon Glass <sjg@chromium.org>
---
configs/sandbox_defconfig | 1 +
test/dm/blk.c | 63 +++++++++++++++++++++++++++++++--------
2 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1cd1c2ed7cd..f451a94362e 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -227,6 +227,7 @@ CONFIG_SPI_FLASH_SPANSION=y
CONFIG_SPI_FLASH_STMICRO=y
CONFIG_SPI_FLASH_SST=y
CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_NVMXIP_QSPI=y
CONFIG_MULTIPLEXER=y
CONFIG_MUX_MMIO=y
CONFIG_NVME_PCI=y
diff --git a/test/dm/blk.c b/test/dm/blk.c
index 446c4423e6f..d268a441c99 100644
--- a/test/dm/blk.c
+++ b/test/dm/blk.c
@@ -82,12 +82,12 @@ static int dm_test_blk_usb(struct unit_test_state *uts)
ut_asserteq_ptr(usb_dev, dev_get_parent(dev));
/* Check we have one block device for each mass storage device */
- ut_asserteq(6, count_blk_devices());
+ ut_asserteq(8, count_blk_devices());
/* Now go around again, making sure the old devices were unbound */
ut_assertok(usb_stop());
ut_assertok(usb_init());
- ut_asserteq(6, count_blk_devices());
+ ut_asserteq(8, count_blk_devices());
ut_assertok(usb_stop());
return 0;
@@ -184,6 +184,10 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
*/
ut_assertok(blk_first_device_err(BLKF_FIXED, &dev));
ut_asserteq_str("mmc2.blk", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_FIXED, &dev));
+ ut_asserteq_str("nvmxip-qspi1@08000000.blk#1", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_FIXED, &dev));
+ ut_asserteq_str("nvmxip-qspi2@08200000.blk#2", dev->name);
ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev));
ut_assertok(blk_first_device_err(BLKF_REMOVABLE, &dev));
@@ -198,16 +202,23 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
ut_asserteq_str("mmc1.blk", dev->name);
ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
ut_asserteq_str("mmc0.blk", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
+ ut_asserteq_str("nvmxip-qspi1@08000000.blk#1", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
+ ut_asserteq_str("nvmxip-qspi2@08200000.blk#2", dev->name);
ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev));
- ut_asserteq(1, blk_count_devices(BLKF_FIXED));
+ ut_asserteq(3, blk_count_devices(BLKF_FIXED));
ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE));
- ut_asserteq(3, blk_count_devices(BLKF_BOTH));
+ ut_asserteq(5, blk_count_devices(BLKF_BOTH));
i = 0;
blk_foreach_probe(BLKF_FIXED, dev)
- ut_asserteq_str((i++, "mmc2.blk"), dev->name);
- ut_asserteq(1, i);
+ ut_asserteq_str((++i == 1 ? "mmc2.blk" :
+ i == 2 ? "nvmxip-qspi1@08000000.blk#1" :
+ "nvmxip-qspi2@08200000.blk#2"),
+ dev->name);
+ ut_asserteq(3, i);
i = 0;
blk_foreach_probe(BLKF_REMOVABLE, dev)
@@ -216,9 +227,13 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
i = 0;
blk_foreach_probe(BLKF_BOTH, dev)
- ut_asserteq_str((++i == 1 ? "mmc2.blk" : i == 2 ?
- "mmc1.blk" : "mmc0.blk"), dev->name);
- ut_asserteq(3, i);
+ ut_asserteq_str((++i == 1 ? "mmc2.blk" :
+ i == 2 ? "mmc1.blk" :
+ i == 3 ? "mmc0.blk" :
+ i == 4 ? "nvmxip-qspi1@08000000.blk#1" :
+ "nvmxip-qspi2@08200000.blk#2"),
+ dev->name);
+ ut_asserteq(5, i);
return 0;
}
@@ -242,6 +257,14 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
ut_assertnonnull(dev);
ut_asserteq_str("mmc0.blk", dev->name);
+ ut_assertok(blk_find_next(BLKF_BOTH, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi1@08000000.blk#1", dev->name);
+
+ ut_assertok(blk_find_next(BLKF_BOTH, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi2@08200000.blk#2", dev->name);
+
ut_asserteq(-ENODEV, blk_find_next(BLKF_BOTH, &dev));
ut_assertnull(dev);
@@ -265,6 +288,14 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
ut_assertnonnull(dev);
ut_asserteq_str("mmc0.blk", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi1@08000000.blk#1", dev->name);
+
+ ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi2@08200000.blk#2", dev->name);
+
ut_asserteq(-ENODEV, blk_next_device_err(BLKF_BOTH, &dev));
/* Look only for fixed devices */
@@ -272,6 +303,14 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
ut_assertnonnull(dev);
ut_asserteq_str("mmc2.blk", dev->name);
+ ut_assertok(blk_next_device_err(BLKF_FIXED, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi1@08000000.blk#1", dev->name);
+
+ ut_assertok(blk_next_device_err(BLKF_FIXED, &dev));
+ ut_assertnonnull(dev);
+ ut_asserteq_str("nvmxip-qspi2@08200000.blk#2", dev->name);
+
ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev));
/* Look only for removable devices */
@@ -317,13 +356,13 @@ static int dm_test_blk_foreach(struct unit_test_state *uts)
blk_foreach_probe(BLKF_BOTH, dev)
found |= 1 << dectoul(&dev->name[3], NULL);
ut_asserteq(7, found);
- ut_asserteq(3, blk_count_devices(BLKF_BOTH));
+ ut_asserteq(5, blk_count_devices(BLKF_BOTH));
found = 0;
blk_foreach_probe(BLKF_FIXED, dev)
found |= 1 << dectoul(&dev->name[3], NULL);
- ut_asserteq(4, found);
- ut_asserteq(1, blk_count_devices(BLKF_FIXED));
+ ut_asserteq(5, found);
+ ut_asserteq(3, blk_count_devices(BLKF_FIXED));
found = 0;
blk_foreach_probe(BLKF_REMOVABLE, dev)
--
2.40.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems
2023-08-13 21:46 [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
2023-08-13 21:46 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Marek Vasut
@ 2023-08-13 23:55 ` Marek Vasut
2023-08-19 0:23 ` Marek Vasut
2 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-13 23:55 UTC (permalink / raw)
To: Marek Vasut, u-boot; +Cc: Abdellatif El Khlifi, Simon Glass
On 8/13/23 23:46, Marek Vasut wrote:
> Get rid of nvmxip_mmio_rawread() and just implement the readl()/readq()
> reader loop within nvmxip_blk_read(). Cast the destination buffer as
> needed and increment the read by either 4 or 8 bytes depending on if
> this is systemd with 32bit or 64bit physical address.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
These two patches need a bit more work, skip for now.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test
2023-08-13 21:46 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Marek Vasut
@ 2023-08-14 22:42 ` Simon Glass
2023-08-14 22:45 ` Marek Vasut
0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-08-14 22:42 UTC (permalink / raw)
To: Marek Vasut; +Cc: u-boot, Abdellatif El Khlifi
On Sun, 13 Aug 2023 at 15:47, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
>
> Enable NVMXIP QSPI driver on sandbox, since it is already enabled
> on sandbox64. Update blk tests to match.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> configs/sandbox_defconfig | 1 +
> test/dm/blk.c | 63 +++++++++++++++++++++++++++++++--------
> 2 files changed, 52 insertions(+), 12 deletions(-)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test
2023-08-14 22:42 ` Simon Glass
@ 2023-08-14 22:45 ` Marek Vasut
2023-08-15 16:43 ` [PATCH] nvmxip: add sandbox support Abdellatif El Khlifi
2023-08-15 17:08 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Abdellatif El Khlifi
0 siblings, 2 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-14 22:45 UTC (permalink / raw)
To: Simon Glass, Marek Vasut; +Cc: u-boot, Abdellatif El Khlifi
On 8/15/23 00:42, Simon Glass wrote:
> On Sun, 13 Aug 2023 at 15:47, Marek Vasut
> <marek.vasut+renesas@mailbox.org> wrote:
>>
>> Enable NVMXIP QSPI driver on sandbox, since it is already enabled
>> on sandbox64. Update blk tests to match.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> ---
>> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>> configs/sandbox_defconfig | 1 +
>> test/dm/blk.c | 63 +++++++++++++++++++++++++++++++--------
>> 2 files changed, 52 insertions(+), 12 deletions(-)
>>
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
NAK, this one has to wait. All of this NVMXIP stuff needs closer look, I
think it is really weirdly broken and doesn't even pass sandbox and
sandbox64 tests. Can Abdellatif try and check whether ALL sandbox and
sandbox64 tests pass with this enabled ?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] nvmxip: add sandbox support
2023-08-14 22:45 ` Marek Vasut
@ 2023-08-15 16:43 ` Abdellatif El Khlifi
2023-08-15 18:39 ` Simon Glass
2023-08-15 20:44 ` [PATCH] nvmxip: add sandbox support Marek Vasut
2023-08-15 17:08 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Abdellatif El Khlifi
1 sibling, 2 replies; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-15 16:43 UTC (permalink / raw)
To: marek.vasut
Cc: abdellatif.elkhlifi, marek.vasut+renesas, sjg, u-boot, nd,
Tom Rini
enable the 32-bit version of sandbox
Initially NVMXIP came with sandbox64 support.
Let's enable sandbox support as well.
Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
---
arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
configs/sandbox_defconfig | 4 ++--
drivers/mtd/nvmxip/nvmxip-uclass.c | 6 +++---
drivers/mtd/nvmxip/nvmxip.c | 13 +++++++++++--
drivers/mtd/nvmxip/nvmxip_qspi.c | 10 +++++++++-
test/dm/Makefile | 2 +-
6 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 12d3eff5fa..1d38a939c4 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -97,6 +97,20 @@
compatible = "sandbox,spi";
cs-gpios = <0>, <&gpio_a 0>;
};
+
+ nvmxip-qspi1@08000000 {
+ compatible = "nvmxip,qspi";
+ reg = <0x08000000 0x00200000>;
+ lba_shift = <9>;
+ lba = <4096>;
+ };
+
+ nvmxip-qspi2@08200000 {
+ compatible = "nvmxip,qspi";
+ reg = <0x08200000 0x00100000>;
+ lba_shift = <9>;
+ lba = <2048>;
+ };
};
#include "sandbox.dtsi"
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1cd1c2ed7c..296f9d7326 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -145,7 +145,6 @@ CONFIG_IP_DEFRAG=y
CONFIG_BOOTP_SERVERIP=y
CONFIG_IPV6=y
CONFIG_DM_DMA=y
-CONFIG_DEVRES=y
CONFIG_DEBUG_DEVRES=y
CONFIG_SIMPLE_PM_BUS=y
CONFIG_ADC=y
@@ -184,6 +183,7 @@ CONFIG_DMA_CHANNELS=y
CONFIG_SANDBOX_DMA=y
CONFIG_FASTBOOT_FLASH=y
CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_ARM_FFA_TRANSPORT=y
CONFIG_GPIO_HOG=y
CONFIG_DM_GPIO_LOOKUP_LABEL=y
CONFIG_QCOM_PMIC_GPIO=y
@@ -227,6 +227,7 @@ CONFIG_SPI_FLASH_SPANSION=y
CONFIG_SPI_FLASH_STMICRO=y
CONFIG_SPI_FLASH_SST=y
CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_NVMXIP_QSPI=y
CONFIG_MULTIPLEXER=y
CONFIG_MUX_MMIO=y
CONFIG_NVME_PCI=y
@@ -345,4 +346,3 @@ CONFIG_TEST_FDTDEC=y
CONFIG_UNIT_TEST=y
CONFIG_UT_TIME=y
CONFIG_UT_DM=y
-CONFIG_ARM_FFA_TRANSPORT=y
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c
index 6d8eb177b5..7177f8f314 100644
--- a/drivers/mtd/nvmxip/nvmxip-uclass.c
+++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
@@ -9,7 +9,7 @@
#include <common.h>
#include <dm.h>
#include <log.h>
-#if CONFIG_IS_ENABLED(SANDBOX64)
+#if CONFIG_IS_ENABLED(SANDBOX)
#include <asm/test.h>
#endif
#include <linux/bitops.h>
@@ -39,7 +39,7 @@ static int nvmxip_post_bind(struct udevice *udev)
char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
int devnum;
-#if CONFIG_IS_ENABLED(SANDBOX64)
+#if CONFIG_IS_ENABLED(SANDBOX)
sandbox_set_enable_memio(true);
#endif
@@ -62,7 +62,7 @@ static int nvmxip_post_bind(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;
}
diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
index a359e3b482..9191e69a40 100644
--- a/drivers/mtd/nvmxip/nvmxip.c
+++ b/drivers/mtd/nvmxip/nvmxip.c
@@ -11,6 +11,7 @@
#include <log.h>
#include <mapmem.h>
#include <asm/io.h>
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/errno.h>
#include "nvmxip.h"
@@ -26,9 +27,17 @@
*
* Always return 0.
*/
-static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
+static int nvmxip_mmio_rawread(const u64 *address, u64 *value)
{
+#if CONFIG_IS_ENABLED(PHYS_64BIT)
*value = readq(address);
+#else
+ u32 h_word, l_word;
+
+ l_word = readl(address);
+ h_word = readl((u8 *)address + sizeof(u32));
+ *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
+#endif
return 0;
}
@@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
/* assumption: the data is virtually contiguous */
for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
- nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
+ nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
dev->name,
diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
index 7221fd1cb4..b6b94ca57f 100644
--- a/drivers/mtd/nvmxip/nvmxip_qspi.c
+++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
@@ -17,6 +17,14 @@ DECLARE_GLOBAL_DATA_PTR;
#define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
+/* Select the right physical address formatting according to the platform */
+#ifdef CONFIG_PHYS_64BIT
+#define PhysAddrLength "ll"
+#else
+#define PhysAddrLength ""
+#endif
+#define PHYS_ADDR_LN "%" PhysAddrLength "x"
+
/**
* nvmxip_qspi_of_to_plat() -read from DT
* @dev: the NVMXIP device
@@ -50,7 +58,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
return -EINVAL;
}
- log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
+ log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
dev->name, plat->phys_base, plat->lba_shift, plat->lba);
return 0;
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 7ed00733c1..77172d9012 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -18,7 +18,7 @@ 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)
+ifeq ($(CONFIG_NVMXIP_QSPI),y)
obj-y += nvmxip.o
endif
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test
2023-08-14 22:45 ` Marek Vasut
2023-08-15 16:43 ` [PATCH] nvmxip: add sandbox support Abdellatif El Khlifi
@ 2023-08-15 17:08 ` Abdellatif El Khlifi
1 sibling, 0 replies; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-15 17:08 UTC (permalink / raw)
To: Marek Vasut; +Cc: nd, u-boot, sjg
Hi,
On Tue, Aug 15, 2023 at 12:45:57AM +0200, Marek Vasut wrote:
> On 8/15/23 00:42, Simon Glass wrote:
> > On Sun, 13 Aug 2023 at 15:47, Marek Vasut
> > <marek.vasut+renesas@mailbox.org> wrote:
> > >
> > > Enable NVMXIP QSPI driver on sandbox, since it is already enabled
> > > on sandbox64. Update blk tests to match.
> > >
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > ---
> > > Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > > Cc: Simon Glass <sjg@chromium.org>
> > > ---
> > > configs/sandbox_defconfig | 1 +
> > > test/dm/blk.c | 63 +++++++++++++++++++++++++++++++--------
> > > 2 files changed, 52 insertions(+), 12 deletions(-)
> > >
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> NAK, this one has to wait. All of this NVMXIP stuff needs closer look, I
> think it is really weirdly broken and doesn't even pass sandbox and
> sandbox64 tests. Can Abdellatif try and check whether ALL sandbox and
> sandbox64 tests pass with this enabled ?
Initially the NVMXIP feature has been introduced with sandbox64 support.
The test case runs fine on sandbox64.
I've just sent a commit enabling NVMXIP for sandbox [1].
Tested on both sandbox and sandbox64.
I hope this helps.
[1]: https://lore.kernel.org/all/20230815164300.444701-1-abdellatif.elkhlifi@arm.com/
Cheers
Abdellatif
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] nvmxip: add sandbox support
2023-08-15 16:43 ` [PATCH] nvmxip: add sandbox support Abdellatif El Khlifi
@ 2023-08-15 18:39 ` Simon Glass
2023-08-16 11:05 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Abdellatif El Khlifi
2023-08-15 20:44 ` [PATCH] nvmxip: add sandbox support Marek Vasut
1 sibling, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-08-15 18:39 UTC (permalink / raw)
To: Abdellatif El Khlifi
Cc: marek.vasut, marek.vasut+renesas, u-boot, nd, Tom Rini
On Tue, 15 Aug 2023 at 10:43, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> enable the 32-bit version of sandbox
>
> Initially NVMXIP came with sandbox64 support.
> Let's enable sandbox support as well.
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
> configs/sandbox_defconfig | 4 ++--
> drivers/mtd/nvmxip/nvmxip-uclass.c | 6 +++---
> drivers/mtd/nvmxip/nvmxip.c | 13 +++++++++++--
> drivers/mtd/nvmxip/nvmxip_qspi.c | 10 +++++++++-
> test/dm/Makefile | 2 +-
> 6 files changed, 40 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
>
> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
> index 12d3eff5fa..1d38a939c4 100644
> --- a/arch/sandbox/dts/sandbox.dts
> +++ b/arch/sandbox/dts/sandbox.dts
> @@ -97,6 +97,20 @@
> compatible = "sandbox,spi";
> cs-gpios = <0>, <&gpio_a 0>;
> };
> +
> + nvmxip-qspi1@08000000 {
> + compatible = "nvmxip,qspi";
> + reg = <0x08000000 0x00200000>;
> + lba_shift = <9>;
> + lba = <4096>;
> + };
> +
> + nvmxip-qspi2@08200000 {
> + compatible = "nvmxip,qspi";
> + reg = <0x08200000 0x00100000>;
> + lba_shift = <9>;
> + lba = <2048>;
> + };
> };
>
> #include "sandbox.dtsi"
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 1cd1c2ed7c..296f9d7326 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -145,7 +145,6 @@ CONFIG_IP_DEFRAG=y
> CONFIG_BOOTP_SERVERIP=y
> CONFIG_IPV6=y
> CONFIG_DM_DMA=y
> -CONFIG_DEVRES=y
> CONFIG_DEBUG_DEVRES=y
> CONFIG_SIMPLE_PM_BUS=y
> CONFIG_ADC=y
> @@ -184,6 +183,7 @@ CONFIG_DMA_CHANNELS=y
> CONFIG_SANDBOX_DMA=y
> CONFIG_FASTBOOT_FLASH=y
> CONFIG_FASTBOOT_FLASH_MMC_DEV=0
> +CONFIG_ARM_FFA_TRANSPORT=y
> CONFIG_GPIO_HOG=y
> CONFIG_DM_GPIO_LOOKUP_LABEL=y
> CONFIG_QCOM_PMIC_GPIO=y
> @@ -227,6 +227,7 @@ CONFIG_SPI_FLASH_SPANSION=y
> CONFIG_SPI_FLASH_STMICRO=y
> CONFIG_SPI_FLASH_SST=y
> CONFIG_SPI_FLASH_WINBOND=y
> +CONFIG_NVMXIP_QSPI=y
> CONFIG_MULTIPLEXER=y
> CONFIG_MUX_MMIO=y
> CONFIG_NVME_PCI=y
> @@ -345,4 +346,3 @@ CONFIG_TEST_FDTDEC=y
> CONFIG_UNIT_TEST=y
> CONFIG_UT_TIME=y
> CONFIG_UT_DM=y
> -CONFIG_ARM_FFA_TRANSPORT=y
> diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c
> index 6d8eb177b5..7177f8f314 100644
> --- a/drivers/mtd/nvmxip/nvmxip-uclass.c
> +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
> @@ -9,7 +9,7 @@
> #include <common.h>
> #include <dm.h>
> #include <log.h>
> -#if CONFIG_IS_ENABLED(SANDBOX64)
> +#if CONFIG_IS_ENABLED(SANDBOX)
> #include <asm/test.h>
> #endif
> #include <linux/bitops.h>
> @@ -39,7 +39,7 @@ static int nvmxip_post_bind(struct udevice *udev)
> char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
> int devnum;
>
> -#if CONFIG_IS_ENABLED(SANDBOX64)
> +#if CONFIG_IS_ENABLED(SANDBOX)
> sandbox_set_enable_memio(true);
> #endif
>
> @@ -62,7 +62,7 @@ static int nvmxip_post_bind(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;
> }
> diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
> index a359e3b482..9191e69a40 100644
> --- a/drivers/mtd/nvmxip/nvmxip.c
> +++ b/drivers/mtd/nvmxip/nvmxip.c
> @@ -11,6 +11,7 @@
> #include <log.h>
> #include <mapmem.h>
> #include <asm/io.h>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/errno.h>
> #include "nvmxip.h"
> @@ -26,9 +27,17 @@
> *
> * Always return 0.
> */
> -static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
> +static int nvmxip_mmio_rawread(const u64 *address, u64 *value)
> {
> +#if CONFIG_IS_ENABLED(PHYS_64BIT)
> *value = readq(address);
> +#else
> + u32 h_word, l_word;
> +
> + l_word = readl(address);
> + h_word = readl((u8 *)address + sizeof(u32));
> + *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
> +#endif
> return 0;
> }
>
> @@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
> /* assumption: the data is virtually contiguous */
>
> for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
> - nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
> + nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
>
> log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
> dev->name,
> diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
> index 7221fd1cb4..b6b94ca57f 100644
> --- a/drivers/mtd/nvmxip/nvmxip_qspi.c
> +++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
> @@ -17,6 +17,14 @@ DECLARE_GLOBAL_DATA_PTR;
>
> #define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
>
> +/* Select the right physical address formatting according to the platform */
> +#ifdef CONFIG_PHYS_64BIT
> +#define PhysAddrLength "ll"
> +#else
> +#define PhysAddrLength ""
> +#endif
> +#define PHYS_ADDR_LN "%" PhysAddrLength "x"
Could this go in a standard header?
> +
> /**
> * nvmxip_qspi_of_to_plat() -read from DT
> * @dev: the NVMXIP device
> @@ -50,7 +58,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
> return -EINVAL;
> }
>
> - log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
> + log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
> dev->name, plat->phys_base, plat->lba_shift, plat->lba);
>
> return 0;
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 7ed00733c1..77172d9012 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -18,7 +18,7 @@ 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)
> +ifeq ($(CONFIG_NVMXIP_QSPI),y)
> obj-y += nvmxip.o
> endif
>
> --
> 2.25.1
>
Regars,
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] nvmxip: add sandbox support
2023-08-15 16:43 ` [PATCH] nvmxip: add sandbox support Abdellatif El Khlifi
2023-08-15 18:39 ` Simon Glass
@ 2023-08-15 20:44 ` Marek Vasut
1 sibling, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-15 20:44 UTC (permalink / raw)
To: Abdellatif El Khlifi; +Cc: marek.vasut+renesas, sjg, u-boot, nd, Tom Rini
On 8/15/23 18:43, Abdellatif El Khlifi wrote:
> enable the 32-bit version of sandbox
>
> Initially NVMXIP came with sandbox64 support.
> Let's enable sandbox support as well.
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
Can you please split this into multiple patches and add proper commit
message description to each one ? Thanks.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/2] log: select physical address formatting in a generic way
2023-08-15 18:39 ` Simon Glass
@ 2023-08-16 11:05 ` Abdellatif El Khlifi
2023-08-16 11:05 ` [PATCH v2 2/2] nvmxip: add sandbox support Abdellatif El Khlifi
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-16 11:05 UTC (permalink / raw)
To: sjg
Cc: abdellatif.elkhlifi, marek.vasut+renesas, marek.vasut, nd, trini,
u-boot
sets the log formatting according to the platform (64-bit vs 32-bit)
Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Simon Glass <sjg@chromium.org>
---
cmd/armffa.c | 8 --------
include/log.h | 9 +++++++++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/cmd/armffa.c b/cmd/armffa.c
index 7e6eafc03a..ab0fd54d97 100644
--- a/cmd/armffa.c
+++ b/cmd/armffa.c
@@ -13,14 +13,6 @@
#include <stdlib.h>
#include <asm/io.h>
-/* Select the right physical address formatting according to the platform */
-#ifdef CONFIG_PHYS_64BIT
-#define PhysAddrLength "ll"
-#else
-#define PhysAddrLength ""
-#endif
-#define PHYS_ADDR_LN "%" PhysAddrLength "x"
-
/**
* ffa_get_dev() - Return the FF-A device
* @devp: pointer to the FF-A device
diff --git a/include/log.h b/include/log.h
index 6e84f080ef..3f631d6816 100644
--- a/include/log.h
+++ b/include/log.h
@@ -370,6 +370,15 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
#define log_msg_retz(_msg, _ret) ((void)(_msg), _ret)
#endif
+/* Select the right physical address formatting according to the platform */
+#ifdef CONFIG_PHYS_64BIT
+#define PhysAddrLength "ll"
+#else
+#define PhysAddrLength ""
+#endif
+#define PHYS_ADDR_LN "%" PhysAddrLength "x"
+#define PHYS_ADDR_LNU "%" PhysAddrLength "u"
+
/** * enum log_rec_flags - Flags for a log record */
enum log_rec_flags {
/** @LOGRECF_FORCE_DEBUG: Force output of debug record */
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] nvmxip: add sandbox support
2023-08-16 11:05 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Abdellatif El Khlifi
@ 2023-08-16 11:05 ` Abdellatif El Khlifi
2023-08-16 11:18 ` Marek Vasut
2023-08-16 11:13 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Marek Vasut
2023-10-20 12:12 ` Abdellatif El Khlifi
2 siblings, 1 reply; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-16 11:05 UTC (permalink / raw)
To: sjg
Cc: abdellatif.elkhlifi, marek.vasut+renesas, marek.vasut, nd, trini,
u-boot
enable the 32-bit version of sandbox
Initially NVMXIP came with sandbox64 support.
Let's enable sandbox support as well.
Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Marek Vasut <marek.vasut@mailbox.org>
---
Changelog:
===============
v2:
* split into 2 commits: one for sandbox 32-bit support,
and one for moving PHYS_ADDR_LN* to log.h
v1: introduce the sandbox 32-bit support
arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
configs/sandbox_defconfig | 4 ++--
drivers/mtd/nvmxip/nvmxip-uclass.c | 6 +++---
drivers/mtd/nvmxip/nvmxip.c | 13 +++++++++++--
drivers/mtd/nvmxip/nvmxip_qspi.c | 2 +-
test/dm/Makefile | 2 +-
6 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 12d3eff5fa..1d38a939c4 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -97,6 +97,20 @@
compatible = "sandbox,spi";
cs-gpios = <0>, <&gpio_a 0>;
};
+
+ nvmxip-qspi1@08000000 {
+ compatible = "nvmxip,qspi";
+ reg = <0x08000000 0x00200000>;
+ lba_shift = <9>;
+ lba = <4096>;
+ };
+
+ nvmxip-qspi2@08200000 {
+ compatible = "nvmxip,qspi";
+ reg = <0x08200000 0x00100000>;
+ lba_shift = <9>;
+ lba = <2048>;
+ };
};
#include "sandbox.dtsi"
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1cd1c2ed7c..296f9d7326 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -145,7 +145,6 @@ CONFIG_IP_DEFRAG=y
CONFIG_BOOTP_SERVERIP=y
CONFIG_IPV6=y
CONFIG_DM_DMA=y
-CONFIG_DEVRES=y
CONFIG_DEBUG_DEVRES=y
CONFIG_SIMPLE_PM_BUS=y
CONFIG_ADC=y
@@ -184,6 +183,7 @@ CONFIG_DMA_CHANNELS=y
CONFIG_SANDBOX_DMA=y
CONFIG_FASTBOOT_FLASH=y
CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_ARM_FFA_TRANSPORT=y
CONFIG_GPIO_HOG=y
CONFIG_DM_GPIO_LOOKUP_LABEL=y
CONFIG_QCOM_PMIC_GPIO=y
@@ -227,6 +227,7 @@ CONFIG_SPI_FLASH_SPANSION=y
CONFIG_SPI_FLASH_STMICRO=y
CONFIG_SPI_FLASH_SST=y
CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_NVMXIP_QSPI=y
CONFIG_MULTIPLEXER=y
CONFIG_MUX_MMIO=y
CONFIG_NVME_PCI=y
@@ -345,4 +346,3 @@ CONFIG_TEST_FDTDEC=y
CONFIG_UNIT_TEST=y
CONFIG_UT_TIME=y
CONFIG_UT_DM=y
-CONFIG_ARM_FFA_TRANSPORT=y
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c
index 6d8eb177b5..7177f8f314 100644
--- a/drivers/mtd/nvmxip/nvmxip-uclass.c
+++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
@@ -9,7 +9,7 @@
#include <common.h>
#include <dm.h>
#include <log.h>
-#if CONFIG_IS_ENABLED(SANDBOX64)
+#if CONFIG_IS_ENABLED(SANDBOX)
#include <asm/test.h>
#endif
#include <linux/bitops.h>
@@ -39,7 +39,7 @@ static int nvmxip_post_bind(struct udevice *udev)
char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
int devnum;
-#if CONFIG_IS_ENABLED(SANDBOX64)
+#if CONFIG_IS_ENABLED(SANDBOX)
sandbox_set_enable_memio(true);
#endif
@@ -62,7 +62,7 @@ static int nvmxip_post_bind(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;
}
diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
index a359e3b482..9191e69a40 100644
--- a/drivers/mtd/nvmxip/nvmxip.c
+++ b/drivers/mtd/nvmxip/nvmxip.c
@@ -11,6 +11,7 @@
#include <log.h>
#include <mapmem.h>
#include <asm/io.h>
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/errno.h>
#include "nvmxip.h"
@@ -26,9 +27,17 @@
*
* Always return 0.
*/
-static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
+static int nvmxip_mmio_rawread(const u64 *address, u64 *value)
{
+#if CONFIG_IS_ENABLED(PHYS_64BIT)
*value = readq(address);
+#else
+ u32 h_word, l_word;
+
+ l_word = readl(address);
+ h_word = readl((u8 *)address + sizeof(u32));
+ *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
+#endif
return 0;
}
@@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
/* assumption: the data is virtually contiguous */
for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
- nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
+ nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
dev->name,
diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
index 7221fd1cb4..faeb99b4ad 100644
--- a/drivers/mtd/nvmxip/nvmxip_qspi.c
+++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
@@ -50,7 +50,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
return -EINVAL;
}
- log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
+ log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
dev->name, plat->phys_base, plat->lba_shift, plat->lba);
return 0;
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 7ed00733c1..77172d9012 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -18,7 +18,7 @@ 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)
+ifeq ($(CONFIG_NVMXIP_QSPI),y)
obj-y += nvmxip.o
endif
--
2.25.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] log: select physical address formatting in a generic way
2023-08-16 11:05 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Abdellatif El Khlifi
2023-08-16 11:05 ` [PATCH v2 2/2] nvmxip: add sandbox support Abdellatif El Khlifi
@ 2023-08-16 11:13 ` Marek Vasut
2023-08-16 14:39 ` Simon Glass
2023-10-20 12:12 ` Abdellatif El Khlifi
2 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2023-08-16 11:13 UTC (permalink / raw)
To: Abdellatif El Khlifi, sjg; +Cc: marek.vasut+renesas, nd, trini, u-boot
On 8/16/23 13:05, Abdellatif El Khlifi wrote:
> sets the log formatting according to the platform (64-bit vs 32-bit)
Use imperative mood please, see
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> cmd/armffa.c | 8 --------
> include/log.h | 9 +++++++++
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/armffa.c b/cmd/armffa.c
> index 7e6eafc03a..ab0fd54d97 100644
> --- a/cmd/armffa.c
> +++ b/cmd/armffa.c
> @@ -13,14 +13,6 @@
> #include <stdlib.h>
> #include <asm/io.h>
>
> -/* Select the right physical address formatting according to the platform */
> -#ifdef CONFIG_PHYS_64BIT
> -#define PhysAddrLength "ll"
> -#else
> -#define PhysAddrLength ""
> -#endif
> -#define PHYS_ADDR_LN "%" PhysAddrLength "x"
Would it make sense to implement printf '%pa' formatting string instead ?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] nvmxip: add sandbox support
2023-08-16 11:05 ` [PATCH v2 2/2] nvmxip: add sandbox support Abdellatif El Khlifi
@ 2023-08-16 11:18 ` Marek Vasut
2023-08-16 16:39 ` Abdellatif El Khlifi
0 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2023-08-16 11:18 UTC (permalink / raw)
To: Abdellatif El Khlifi, sjg; +Cc: marek.vasut+renesas, nd, trini, u-boot
On 8/16/23 13:05, Abdellatif El Khlifi wrote:
> enable the 32-bit version of sandbox
>
> Initially NVMXIP came with sandbox64 support.
> Let's enable sandbox support as well.
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Marek Vasut <marek.vasut@mailbox.org>
>
> ---
>
> Changelog:
> ===============
>
> v2:
>
> * split into 2 commits: one for sandbox 32-bit support,
> and one for moving PHYS_ADDR_LN* to log.h
>
> v1: introduce the sandbox 32-bit support
>
> arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
Please separate DT change
> configs/sandbox_defconfig | 4 ++--
Config change too, separate patch that goes last.
> drivers/mtd/nvmxip/nvmxip-uclass.c | 6 +++---
> drivers/mtd/nvmxip/nvmxip.c | 13 +++++++++++--
> drivers/mtd/nvmxip/nvmxip_qspi.c | 2 +-
> test/dm/Makefile | 2 +-
> 6 files changed, 32 insertions(+), 9 deletions(-)
[...]
> --- a/drivers/mtd/nvmxip/nvmxip-uclass.c
> +++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
> @@ -9,7 +9,7 @@
> #include <common.h>
> #include <dm.h>
> #include <log.h>
> -#if CONFIG_IS_ENABLED(SANDBOX64)
> +#if CONFIG_IS_ENABLED(SANDBOX)
> #include <asm/test.h>
> #endif
> #include <linux/bitops.h>
> @@ -39,7 +39,7 @@ static int nvmxip_post_bind(struct udevice *udev)
> char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
> int devnum;
>
> -#if CONFIG_IS_ENABLED(SANDBOX64)
> +#if CONFIG_IS_ENABLED(SANDBOX)
> sandbox_set_enable_memio(true);
This should not be in drivers at all, this should be in tests/ , see
https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/b07772226b405d6212d4faa9329d64a09708b188
> #endif
>
> @@ -62,7 +62,7 @@ static int nvmxip_post_bind(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);
Unrelated change -> separate patch please.
> return 0;
> }
> diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
> index a359e3b482..9191e69a40 100644
> --- a/drivers/mtd/nvmxip/nvmxip.c
> +++ b/drivers/mtd/nvmxip/nvmxip.c
> @@ -11,6 +11,7 @@
> #include <log.h>
> #include <mapmem.h>
> #include <asm/io.h>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/errno.h>
> #include "nvmxip.h"
> @@ -26,9 +27,17 @@
> *
> * Always return 0.
> */
> -static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
> +static int nvmxip_mmio_rawread(const u64 *address, u64 *value)
> {
> +#if CONFIG_IS_ENABLED(PHYS_64BIT)
> *value = readq(address);
> +#else
> + u32 h_word, l_word;
> +
> + l_word = readl(address);
> + h_word = readl((u8 *)address + sizeof(u32));
> + *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
> +#endif
> return 0;
> }
>
> @@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
> /* assumption: the data is virtually contiguous */
>
> for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
> - nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
> + nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
Separate patch please, or just use this commit as part of this series:
https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/85a662e98c44921a0f5652fa4b170625424ef9a9
> log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
> dev->name,
> diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
> index 7221fd1cb4..faeb99b4ad 100644
> --- a/drivers/mtd/nvmxip/nvmxip_qspi.c
> +++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
> @@ -50,7 +50,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
> return -EINVAL;
> }
>
> - log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
> + log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
> dev->name, plat->phys_base, plat->lba_shift, plat->lba);
Another separate patch.
> return 0;
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 7ed00733c1..77172d9012 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -18,7 +18,7 @@ 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)
> +ifeq ($(CONFIG_NVMXIP_QSPI),y)
Separate patch.
Look here for some ideas:
https://source.denx.de/u-boot/custodians/u-boot-sh/-/commits/ci%2Ftest-sandbox64?ref_type=heads
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] log: select physical address formatting in a generic way
2023-08-16 11:13 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Marek Vasut
@ 2023-08-16 14:39 ` Simon Glass
2023-08-16 15:23 ` Marek Vasut
0 siblings, 1 reply; 20+ messages in thread
From: Simon Glass @ 2023-08-16 14:39 UTC (permalink / raw)
To: Marek Vasut; +Cc: Abdellatif El Khlifi, marek.vasut+renesas, nd, trini, u-boot
Hi Marek,
On Wed, 16 Aug 2023 at 05:13, Marek Vasut <marek.vasut@mailbox.org> wrote:
>
> On 8/16/23 13:05, Abdellatif El Khlifi wrote:
> > sets the log formatting according to the platform (64-bit vs 32-bit)
>
> Use imperative mood please, see
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > ---
> > cmd/armffa.c | 8 --------
> > include/log.h | 9 +++++++++
> > 2 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/cmd/armffa.c b/cmd/armffa.c
> > index 7e6eafc03a..ab0fd54d97 100644
> > --- a/cmd/armffa.c
> > +++ b/cmd/armffa.c
> > @@ -13,14 +13,6 @@
> > #include <stdlib.h>
> > #include <asm/io.h>
> >
> > -/* Select the right physical address formatting according to the platform */
> > -#ifdef CONFIG_PHYS_64BIT
> > -#define PhysAddrLength "ll"
> > -#else
> > -#define PhysAddrLength ""
> > -#endif
> > -#define PHYS_ADDR_LN "%" PhysAddrLength "x"
>
> Would it make sense to implement printf '%pa' formatting string instead ?
The problem is that it is not a pointer. Also we want the compiler to
check the size.
Regards,
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] log: select physical address formatting in a generic way
2023-08-16 14:39 ` Simon Glass
@ 2023-08-16 15:23 ` Marek Vasut
0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-16 15:23 UTC (permalink / raw)
To: Simon Glass; +Cc: Abdellatif El Khlifi, marek.vasut+renesas, nd, trini, u-boot
On 8/16/23 16:39, Simon Glass wrote:
> Hi Marek,
>
> On Wed, 16 Aug 2023 at 05:13, Marek Vasut <marek.vasut@mailbox.org> wrote:
>>
>> On 8/16/23 13:05, Abdellatif El Khlifi wrote:
>>> sets the log formatting according to the platform (64-bit vs 32-bit)
>>
>> Use imperative mood please, see
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
>>
>>> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>> cmd/armffa.c | 8 --------
>>> include/log.h | 9 +++++++++
>>> 2 files changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/cmd/armffa.c b/cmd/armffa.c
>>> index 7e6eafc03a..ab0fd54d97 100644
>>> --- a/cmd/armffa.c
>>> +++ b/cmd/armffa.c
>>> @@ -13,14 +13,6 @@
>>> #include <stdlib.h>
>>> #include <asm/io.h>
>>>
>>> -/* Select the right physical address formatting according to the platform */
>>> -#ifdef CONFIG_PHYS_64BIT
>>> -#define PhysAddrLength "ll"
>>> -#else
>>> -#define PhysAddrLength ""
>>> -#endif
>>> -#define PHYS_ADDR_LN "%" PhysAddrLength "x"
>>
>> Would it make sense to implement printf '%pa' formatting string instead ?
>
> The problem is that it is not a pointer. Also we want the compiler to
> check the size.
What does Linux use ?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] nvmxip: add sandbox support
2023-08-16 11:18 ` Marek Vasut
@ 2023-08-16 16:39 ` Abdellatif El Khlifi
2023-08-16 21:47 ` Marek Vasut
0 siblings, 1 reply; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-16 16:39 UTC (permalink / raw)
To: Marek Vasut; +Cc: nd, u-boot, sjg
Hi Marek,
Please kindly find my comments below.
> > arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
>
> Please separate DT change
>
> > configs/sandbox_defconfig | 4 ++--
>
> Config change too, separate patch that goes last.
This commit is doing 1 thing: adding 32-bit sandbox support.
The DT change comes into the same context.
It makes sense to keep it in this same commit.
In previous contributions I made, it was accepted that DT
and defconfig are part of the same commit as the code.
Let's see what Simon thinks.
I'm happy to split if that has becone a new requirement.
> > int devnum;
> > -#if CONFIG_IS_ENABLED(SANDBOX64)
> > +#if CONFIG_IS_ENABLED(SANDBOX)
> > sandbox_set_enable_memio(true);
>
> This should not be in drivers at all, this should be in tests/ , see
>
> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/b07772226b405d6212d4faa9329d64a09708b188
Thanks, I'm happy to improve that one. I'll move it to tests in a seperate commit :)
>
> > #endif
> > @@ -62,7 +62,7 @@ static int nvmxip_post_bind(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);
>
> Unrelated change -> separate patch please.
Valid point, I'll do thanks.
> > + l_word = readl(address);
> > + h_word = readl((u8 *)address + sizeof(u32));
> > + *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
> > +#endif
> > return 0;
> > }
> > @@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
> > /* assumption: the data is virtually contiguous */
> > for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
> > - nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
> > + nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
>
> Separate patch please, or just use this commit as part of this series:
>
> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/85a662e98c44921a0f5652fa4b170625424ef9a9
This is part of the 32-bit support work.
Before this commit, it works fine on sandbox64.
> > log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
> > dev->name,
> > diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
> > index 7221fd1cb4..faeb99b4ad 100644
> > --- a/drivers/mtd/nvmxip/nvmxip_qspi.c
> > +++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
> > @@ -50,7 +50,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
> > return -EINVAL;
> > }
> > - log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
> > + log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
> > dev->name, plat->phys_base, plat->lba_shift, plat->lba);
>
> Another separate patch.
This is part of the 32-bit support work.
>
> > return 0;
> > diff --git a/test/dm/Makefile b/test/dm/Makefile
> > index 7ed00733c1..77172d9012 100644
> > --- a/test/dm/Makefile
> > +++ b/test/dm/Makefile
> > @@ -18,7 +18,7 @@ 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)
> > +ifeq ($(CONFIG_NVMXIP_QSPI),y)
>
> Separate patch.
>
> Look here for some ideas:
>
> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commits/ci%2Ftest-sandbox64?ref_type=heads
SANDBOX applies for both sandbox64 and sandbox.
This is part of enabling sandbox alongside sandbox64.
This has been tested and works.
Kind regards
Abdellatif
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/2] nvmxip: add sandbox support
2023-08-16 16:39 ` Abdellatif El Khlifi
@ 2023-08-16 21:47 ` Marek Vasut
0 siblings, 0 replies; 20+ messages in thread
From: Marek Vasut @ 2023-08-16 21:47 UTC (permalink / raw)
To: Abdellatif El Khlifi; +Cc: nd, u-boot, sjg
On 8/16/23 18:39, Abdellatif El Khlifi wrote:
> Hi Marek,
Hi,
> Please kindly find my comments below.
>
>>> arch/sandbox/dts/sandbox.dts | 14 ++++++++++++++
>>
>> Please separate DT change
>>
>>> configs/sandbox_defconfig | 4 ++--
>>
>> Config change too, separate patch that goes last.
>
> This commit is doing 1 thing: adding 32-bit sandbox support.
No, it does not do one thing, else I would not ask you to split this up
into a series. It does multiple things and squashes those in single commit.
> The DT change comes into the same context.
> It makes sense to keep it in this same commit.
>
> In previous contributions I made, it was accepted that DT
> and defconfig are part of the same commit as the code.
>
> Let's see what Simon thinks.
>
> I'm happy to split if that has becone a new requirement.
This is not a new requirement, that requirement has been around since
forever, see:
https://docs.kernel.org/process/submitting-patches.html#split-changes
U-Boot follows the same rule set.
>>> int devnum;
>>> -#if CONFIG_IS_ENABLED(SANDBOX64)
>>> +#if CONFIG_IS_ENABLED(SANDBOX)
>>> sandbox_set_enable_memio(true);
>>
>> This should not be in drivers at all, this should be in tests/ , see
>>
>> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/b07772226b405d6212d4faa9329d64a09708b188
>
> Thanks, I'm happy to improve that one. I'll move it to tests in a seperate commit :)
>
>>
>>> #endif
>>> @@ -62,7 +62,7 @@ static int nvmxip_post_bind(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);
>>
>> Unrelated change -> separate patch please.
>
> Valid point, I'll do thanks.
>
>>> + l_word = readl(address);
>>> + h_word = readl((u8 *)address + sizeof(u32));
>>> + *value = FIELD_PREP(GENMASK_ULL(63, 32), h_word) | l_word;
>>> +#endif
>>> return 0;
>>> }
>>> @@ -67,7 +76,7 @@ static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcn
>>> /* assumption: the data is virtually contiguous */
>>> for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
>>> - nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
>>> + nvmxip_mmio_rawread(virt_blkaddr + qdata_idx, pdst++);
>>
>> Separate patch please, or just use this commit as part of this series:
>>
>> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commit/85a662e98c44921a0f5652fa4b170625424ef9a9
>
> This is part of the 32-bit support work.
> Before this commit, it works fine on sandbox64.
Have you tested the entire test suite ? Like e.g. the DM/UT 'host' test
? That one fails on sandbox64 iirc .
>>> log_debug("[%s]: src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
>>> dev->name,
>>> diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
>>> index 7221fd1cb4..faeb99b4ad 100644
>>> --- a/drivers/mtd/nvmxip/nvmxip_qspi.c
>>> +++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
>>> @@ -50,7 +50,7 @@ static int nvmxip_qspi_of_to_plat(struct udevice *dev)
>>> return -EINVAL;
>>> }
>>> - log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
>>> + log_debug("[%s]: XIP device base addr: " PHYS_ADDR_LN " , lba_shift: %d , lbas: %lu\n",
>>> dev->name, plat->phys_base, plat->lba_shift, plat->lba);
>>
>> Another separate patch.
>
> This is part of the 32-bit support work.
This fixes a print of phys_addr_t and is a good example for others to
follow if they need to print such an address. If this is a separate
patch with proper commit message explaining the change, then others can
be pointed to that commit as an example to follow. If this is buried in
a mega-patch, this benefit is lost.
>>> return 0;
>>> diff --git a/test/dm/Makefile b/test/dm/Makefile
>>> index 7ed00733c1..77172d9012 100644
>>> --- a/test/dm/Makefile
>>> +++ b/test/dm/Makefile
>>> @@ -18,7 +18,7 @@ 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)
>>> +ifeq ($(CONFIG_NVMXIP_QSPI),y)
>>
>> Separate patch.
>>
>> Look here for some ideas:
>>
>> https://source.denx.de/u-boot/custodians/u-boot-sh/-/commits/ci%2Ftest-sandbox64?ref_type=heads
>
> SANDBOX applies for both sandbox64 and sandbox.
> This is part of enabling sandbox alongside sandbox64.
>
> This has been tested and works.
If this is split up into proper patches with proper commit messages, the
resulting change would be identical, so the "tested and works" argument
still applies, even if this is split into proper series.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems
2023-08-13 21:46 [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
2023-08-13 21:46 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Marek Vasut
2023-08-13 23:55 ` [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
@ 2023-08-19 0:23 ` Marek Vasut
2023-08-21 11:34 ` Abdellatif El Khlifi
2 siblings, 1 reply; 20+ messages in thread
From: Marek Vasut @ 2023-08-19 0:23 UTC (permalink / raw)
To: u-boot, Abdellatif El Khlifi; +Cc: Simon Glass
On 8/13/23 23:46, Marek Vasut wrote:
> Get rid of nvmxip_mmio_rawread() and just implement the readl()/readq()
> reader loop within nvmxip_blk_read(). Cast the destination buffer as
> needed and increment the read by either 4 or 8 bytes depending on if
> this is systemd with 32bit or 64bit physical address.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Simon Glass <sjg@chromium.org>
Sandbox CI test fails with this series:
https://source.denx.de/u-boot/custodians/u-boot-sh/-/pipelines/17427
https://u-boot.source-pages.denx.de/-/custodians/u-boot-sh/-/jobs/679256/artifacts/test-log.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems
2023-08-19 0:23 ` Marek Vasut
@ 2023-08-21 11:34 ` Abdellatif El Khlifi
0 siblings, 0 replies; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-08-21 11:34 UTC (permalink / raw)
To: Marek Vasut; +Cc: sjg, u-boot, nd, trini
Hi,
On Sat, Aug 19, 2023 at 02:23:02AM +0200, Marek Vasut wrote:
> On 8/13/23 23:46, Marek Vasut wrote:
> > Get rid of nvmxip_mmio_rawread() and just implement the readl()/readq()
> > reader loop within nvmxip_blk_read(). Cast the destination buffer as
> > needed and increment the read by either 4 or 8 bytes depending on if
> > this is systemd with 32bit or 64bit physical address.
> >
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > ---
> > Cc: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > Cc: Simon Glass <sjg@chromium.org>
>
> Sandbox CI test fails with this series:
>
> https://source.denx.de/u-boot/custodians/u-boot-sh/-/pipelines/17427
>
> https://u-boot.source-pages.denx.de/-/custodians/u-boot-sh/-/jobs/679256/artifacts/test-log.html
>
I run the {host , blk_usb, blk_iter, blk_foreach, blk_flags} tests using the commit [1] preceding the NVMXIP patchset.
The tests fail as shown below [3]. I think these issues are not related to NVMXIP.
Regarding your comments for [2], I'll address them as soon as I can.
[1]: b197f1f05dee730e173a0756cb1a5f2be5d3ba5b
[2]: https://lore.kernel.org/all/20230816110551.86930-2-abdellatif.elkhlifi@arm.com/
[3]: tests results:
host:
test/dm/host.c:44, dm_test_host(): 0 == host_attach_file(dev, filename): Expected 0x0 (0), got 0xfffffffe (-2)
Segmentation fault (core dumped)
blk_usb:
test/dm/blk.c:88, dm_test_blk_usb(): 6 == count_blk_devices(): Expected 0x6 (6), got 0x1 (1)
Bus usb@1: scanning bus usb@1 for devices... 2 USB Device(s) found
test/dm/blk.c:93, dm_test_blk_usb(): 6 == count_blk_devices(): Expected 0x6 (6), got 0x1 (1)
Test blk_usb failed 4 times
blk_iter:
test/dm/blk.c:188, dm_test_blk_iter(): 0 == blk_first_device_err(BLKF_FIXED, &dev): Expected 0x0 (0), got 0xffffffed (-19)
Segmentation fault (core dumped)
blk_foreach:
test/dm/blk.c:305, dm_test_blk_foreach(): 7 == found: Expected 0x7 (7), got 0x0 (0)
test/dm/blk.c:316, dm_test_blk_foreach(): 7 == found: Expected 0x7 (7), got 0x0 (0)
test/dm/blk.c:322, dm_test_blk_foreach(): 7 == found: Expected 0x7 (7), got 0x0 (0)
test/dm/blk.c:323, dm_test_blk_foreach(): 3 == blk_count_devices(BLKF_BOTH): Expected 0x3 (3), got 0x0 (0)
test/dm/blk.c:328, dm_test_blk_foreach(): 4 == found: Expected 0x4 (4), got 0x0 (0)
test/dm/blk.c:329, dm_test_blk_foreach(): 1 == blk_count_devices(BLKF_FIXED): Expected 0x1 (1), got 0x0 (0)
test/dm/blk.c:334, dm_test_blk_foreach(): 3 == found: Expected 0x3 (3), got 0x0 (0)
test/dm/blk.c:335, dm_test_blk_foreach(): 2 == blk_count_devices(BLKF_REMOVABLE): Expected 0x2 (2), got 0x0 (0)
...
blk_flags:
test/dm/blk.c:236, dm_test_blk_flags(): 0 == blk_find_first(BLKF_BOTH, &dev): Expected 0x0 (0), got 0xffffffed (-19)
test/dm/blk.c:237, dm_test_blk_flags(): dev = NULL: Expected non-null, got NULL
Segmentation fault (core dumped)
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] log: select physical address formatting in a generic way
2023-08-16 11:05 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Abdellatif El Khlifi
2023-08-16 11:05 ` [PATCH v2 2/2] nvmxip: add sandbox support Abdellatif El Khlifi
2023-08-16 11:13 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Marek Vasut
@ 2023-10-20 12:12 ` Abdellatif El Khlifi
2 siblings, 0 replies; 20+ messages in thread
From: Abdellatif El Khlifi @ 2023-10-20 12:12 UTC (permalink / raw)
To: sjg; +Cc: nd, trini, u-boot, xueliang.zhong
Hi Simon,
> sets the log formatting according to the platform (64-bit vs 32-bit)
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> cmd/armffa.c | 8 --------
> include/log.h | 9 +++++++++
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/cmd/armffa.c b/cmd/armffa.c
> index 7e6eafc03a..ab0fd54d97 100644
> --- a/cmd/armffa.c
> +++ b/cmd/armffa.c
> @@ -13,14 +13,6 @@
> #include <stdlib.h>
> #include <asm/io.h>
>
> -/* Select the right physical address formatting according to the platform */
> -#ifdef CONFIG_PHYS_64BIT
> -#define PhysAddrLength "ll"
> -#else
> -#define PhysAddrLength ""
> -#endif
> -#define PHYS_ADDR_LN "%" PhysAddrLength "x"
> -
> /**
> * ffa_get_dev() - Return the FF-A device
> * @devp: pointer to the FF-A device
> diff --git a/include/log.h b/include/log.h
> index 6e84f080ef..3f631d6816 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -370,6 +370,15 @@ void __assert_fail(const char *assertion, const char *file, unsigned int line,
> #define log_msg_retz(_msg, _ret) ((void)(_msg), _ret)
> #endif
>
> +/* Select the right physical address formatting according to the platform */
> +#ifdef CONFIG_PHYS_64BIT
> +#define PhysAddrLength "ll"
> +#else
> +#define PhysAddrLength ""
> +#endif
> +#define PHYS_ADDR_LN "%" PhysAddrLength "x"
> +#define PHYS_ADDR_LNU "%" PhysAddrLength "u"
> +
> /** * enum log_rec_flags - Flags for a log record */
> enum log_rec_flags {
> /** @LOGRECF_FORCE_DEBUG: Force output of debug record */
> --
> 2.25.1
>
Any feedback about this specific commit please ?
A gentle reminder about the context: While I was working on the FF-A support,
you suggested that I move PHYS_ADDR_LN macros to a common location so it can be used
in a generic way.
Cheers,
Abdellatif
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-10-20 12:12 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-13 21:46 [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
2023-08-13 21:46 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Marek Vasut
2023-08-14 22:42 ` Simon Glass
2023-08-14 22:45 ` Marek Vasut
2023-08-15 16:43 ` [PATCH] nvmxip: add sandbox support Abdellatif El Khlifi
2023-08-15 18:39 ` Simon Glass
2023-08-16 11:05 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Abdellatif El Khlifi
2023-08-16 11:05 ` [PATCH v2 2/2] nvmxip: add sandbox support Abdellatif El Khlifi
2023-08-16 11:18 ` Marek Vasut
2023-08-16 16:39 ` Abdellatif El Khlifi
2023-08-16 21:47 ` Marek Vasut
2023-08-16 11:13 ` [PATCH v2 1/2] log: select physical address formatting in a generic way Marek Vasut
2023-08-16 14:39 ` Simon Glass
2023-08-16 15:23 ` Marek Vasut
2023-10-20 12:12 ` Abdellatif El Khlifi
2023-08-15 20:44 ` [PATCH] nvmxip: add sandbox support Marek Vasut
2023-08-15 17:08 ` [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test Abdellatif El Khlifi
2023-08-13 23:55 ` [PATCH 1/2] drivers/mtd/nvmxip: Rework the read accessor to support 32bit systems Marek Vasut
2023-08-19 0:23 ` Marek Vasut
2023-08-21 11:34 ` Abdellatif El Khlifi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox