[PATCH] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number
Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx
evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)
And add auto zero flash image size to resolve error below after executing
`./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`
Error message:
qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
requires 134217728 bytes, mtd0 block backend provides 67108864 bytes
Tested:
Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally
Signed-off-by: Tim Lee
---
hw/arm/npcm8xx_boards.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
index 3fb8478e72..11b792c613 100644
--- a/hw/arm/npcm8xx_boards.c
+++ b/hw/arm/npcm8xx_boards.c
@@ -27,6 +27,7 @@
#include "qemu/error-report.h"
#include "qemu/datadir.h"
#include "qemu/units.h"
+#include "system/block-backend.h"
#define NPCM845_EVB_POWER_ON_STRAPS 0x17ff
@@ -59,10 +60,21 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int
cs_no,
{
DeviceState *flash;
qemu_irq flash_cs;
+BlockBackend *blk;
+uint64_t blk_size, perm, shared_perm;
flash = qdev_new(flash_type);
if (dinfo) {
qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
+blk = blk_by_legacy_dinfo(dinfo);
+blk_size = blk_getlength(blk);
+if (blk_size < fiu->flash_size) {
+blk_get_perm(blk, &perm, &shared_perm);
+blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
+blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
+ BDRV_REQ_ZERO_WRITE, &error_abort);
+blk_set_perm(blk, perm, shared_perm, &error_abort);
+}
}
qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
@@ -194,7 +206,8 @@ static void npcm845_evb_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm8xx_load_bootrom(machine, soc);
-npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
+npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
+ drive_get(IF_MTD, 0, 0));
npcm845_evb_i2c_init(soc);
npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
npcm8xx_load_kernel(machine, soc);
--
2.34.1
[PATCH] tests/qtest: Add qtest for NPCM8XX PSPI module
- Created qtest to check initialization of registers in PSPI Module
- Implemented test into Build File
Tested:
./build/tests/qtest/npcm8xx-pspi_test
Signed-off-by: Tim Lee
---
MAINTAINERS | 1 +
tests/qtest/meson.build | 3 +
tests/qtest/npcm8xx_pspi-test.c | 104
3 files changed, 108 insertions(+)
create mode 100644 tests/qtest/npcm8xx_pspi-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d54b5578f8..0162f59bf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -892,6 +892,7 @@ F: hw/sensor/adm1266.c
F: include/hw/*/npcm*
F: tests/qtest/npcm*
F: tests/qtest/adm1266-test.c
+F: tests/qtest/npcm8xx_pspi-test.c
F: pc-bios/npcm7xx_bootrom.bin
F: pc-bios/npcm8xx_bootrom.bin
F: roms/vbootrom
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 3136d15e0f..88672a8b00 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -210,6 +210,8 @@ qtests_npcm7xx = \
'npcm7xx_watchdog_timer-test',
'npcm_gmac-test'] + \
(slirp.found() ? ['npcm7xx_emc-test'] : [])
+qtests_npcm8xx = \
+ ['npcm8xx_pspi-test']
qtests_aspeed = \
['aspeed_hace-test',
'aspeed_smc-test',
@@ -257,6 +259,7 @@ qtests_aarch64 = \
(config_all_accel.has_key('CONFIG_TCG') and
\
config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
(config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
+ (config_all_devices.has_key('CONFIG_NPCM8XX') ? qtests_npcm8xx : []) + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
diff --git a/tests/qtest/npcm8xx_pspi-test.c b/tests/qtest/npcm8xx_pspi-test.c
new file mode 100644
index 00..107bce681f
--- /dev/null
+++ b/tests/qtest/npcm8xx_pspi-test.c
@@ -0,0 +1,104 @@
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+
+#define DATA_OFFSET 0x00
+#define CTL_SPIEN 0x01
+#define CTL_OFFSET 0x02
+#define CTL_MOD 0x04
+
+typedef struct PSPI {
+uint64_t base_addr;
+} PSPI;
+
+PSPI pspi_defs = {
+.base_addr = 0xf0201000
+};
+
+static uint16_t pspi_read_data(QTestState *qts, const PSPI *pspi)
+{
+return qtest_readw(qts, pspi->base_addr + DATA_OFFSET);
+}
+
+static void pspi_write_data(QTestState *qts, const PSPI *pspi, uint16_t value)
+{
+qtest_writew(qts, pspi->base_addr + DATA_OFFSET, value);
+}
+
+static uint32_t pspi_read_ctl(QTestState *qts, const PSPI *pspi)
+{
+return qtest_readl(qts, pspi->base_addr + CTL_OFFSET);
+}
+
+static void pspi_write_ctl(QTestState *qts, const PSPI *pspi, uint32_t value)
+{
+qtest_writel(qts, pspi->base_addr + CTL_OFFSET, value);
+}
+
+/* Check PSPI can be reset to default value */
+static void test_init(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+pspi_write_ctl(qts, pspi, CTL_SPIEN);
+g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, CTL_SPIEN);
+
+qtest_quit(qts);
+}
+
+/* Check PSPI can be r/w data register */
+static void test_data(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+uint16_t test = 0x1234;
+uint16_t output;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+
+/* Write to data register */
+pspi_write_data(qts, pspi, test);
+printf("Wrote 0x%x to data register\n", test);
+
+/* Read from data register */
+output = pspi_read_data(qts, pspi);
+printf("Read 0x%x from data register\n", output);
+
+qtest_quit(qts);
+}
+
+/* Check PSPI can be r/w control register */
+static void test_ctl(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+uint8_t control = CTL_MOD;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+
+/* Write CTL_MOD value to control register for 16-bit interface mode */
+qtest_memwrite(qts, pspi->base_addr + CTL_OFFSET,
+ &control, sizeof(control));
+g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, control);
+printf("Wrote CTL_MOD to control register\n");
+
+qtest_quit(qts);
+}
+
+static void pspi_add_test(const char *name, const PSPI* wd,
+GTestDataFunc fn)
+{
+g_autofree char *full_name = g_strdup_printf("npcm8xx_pspi/%s", name);
+qtest_add_data_func(full_name, wd, fn);
+}
+
+#define add_test(name, td) pspi_add_test(#name, td, test_##name)
+
+int main(int argc, char **argv)
+{
+g_test_init(&argc, &argv, NULL);
+
+add_test(init, &pspi_defs);
+add_test(ctl, &pspi_defs);
+add_test(data, &pspi_defs);
+return g_test_run();
+}
--
2.34.1
[PATCH] hw/arm: Attach PSPI module to NPCM8XX SoC
Nuvoton's PSPI is a general purpose SPI module which enables
connections to SPI-based peripheral devices. Attach it to the NPCM8XX.
Tested:
NPCM8XX PSPI driver probed successfully from dmesg log.
Signed-off-by: Tim Lee
---
hw/arm/npcm8xx.c | 11 ++-
include/hw/arm/npcm8xx.h | 2 ++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm8xx.c b/hw/arm/npcm8xx.c
index f182accc47..a7888afb6e 100644
--- a/hw/arm/npcm8xx.c
+++ b/hw/arm/npcm8xx.c
@@ -67,6 +67,9 @@
/* SDHCI Modules */
#define NPCM8XX_MMC_BA 0xf0842000
+/* PSPI Modules */
+#define NPCM8XX_PSPI_BA 0xf0201000
+
/* Run PLL1 at 1600 MHz */
#define NPCM8XX_PLLCON1_FIXUP_VAL 0x00402101
/* Run the CPU from PLL1 and UART from PLL2 */
@@ -83,6 +86,7 @@ enum NPCM8xxInterrupt {
NPCM8XX_PECI_IRQ= 6,
NPCM8XX_KCS_HIB_IRQ = 9,
NPCM8XX_MMC_IRQ = 26,
+NPCM8XX_PSPI_IRQ= 28,
NPCM8XX_TIMER0_IRQ = 32, /* Timer Module 0 */
NPCM8XX_TIMER1_IRQ,
NPCM8XX_TIMER2_IRQ,
@@ -441,6 +445,7 @@ static void npcm8xx_init(Object *obj)
}
object_initialize_child(obj, "mmc", &s->mmc, TYPE_NPCM7XX_SDHCI);
+object_initialize_child(obj, "pspi", &s->pspi, TYPE_NPCM_PSPI);
}
static void npcm8xx_realize(DeviceState *dev, Error **errp)
@@ -705,6 +710,11 @@ static void npcm8xx_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->mmc), 0,
npcm8xx_irq(s, NPCM8XX_MMC_IRQ));
+/* PSPI */
+sysbus_realize(SYS_BUS_DEVICE(&s->pspi), &error_abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->pspi), 0, NPCM8XX_PSPI_BA);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->pspi), 0,
+npcm8xx_irq(s, NPCM8XX_PSPI_IRQ));
create_unimplemented_device("npcm8xx.shm", 0xc0001000, 4 * KiB);
create_unimplemented_device("npcm8xx.gicextra", 0xdfffa000, 24 * KiB);
@@ -720,7 +730,6 @@ static void npcm8xx_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("npcm8xx.siox[1]", 0xf0101000, 4 * KiB);
create_unimplemented_device("npcm8xx.siox[2]", 0xf0102000, 4 * KiB);
create_unimplemented_device("npcm8xx.tmps", 0xf0188000, 4 * KiB);
-create_unimplemented_device("npcm8xx.pspi", 0xf0201000, 4 * KiB);
create_unimplemented_device("npcm8xx.viru1",0xf0204000, 4 * KiB);
create_unimplemented_device("npcm8xx.viru2",0xf0205000, 4 * KiB);
create_unimplemented_device("npcm8xx.jtm1", 0xf0208000, 4 * KiB);
diff --git a/include/hw/arm/npcm8xx.h b/include/hw/arm/npcm8xx.h
index 9812e6fa7e..3436abff99 100644
--- a/include/hw/arm/npcm8xx.h
+++ b/include/hw/arm/npcm8xx.h
@@ -36,6 +36,7 @@
#include "hw/usb/hcd-ehci.h"
#include "hw/usb/hcd-ohci.h"
#include "target/arm/cpu.h"
+#include "hw/ssi/npcm_pspi.h"
#define NPCM8XX_MAX_NUM_CPUS(4)
@@ -99,6 +100,7 @@ struct NPCM8xxState {
OHCISysBusState ohci[2];
NPCM7xxFIUState fiu[3];
NPCM7xxSDHCIState mmc;
+NPCMPSPIState pspi;
};
struct NPCM8xxClass {
--
2.34.1
Re: [PATCH] tests/qtest: Add qtest for NPCM8XX PSPI module
> This fails on top of current master, please take a look:
>
> $ QTEST_LOG=1 QTEST_QEMU_BINARY=./qemu-system-aarch64
> ./tests/qtest/npcm8xx_pspi-test
> # random seed: R02S03f79fc48ba73b76c881f93f90b015e9
> 1..3
> # Start of aarch64 tests
> # Start of npcm8xx_pspi tests
> # starting QEMU: exec ./qemu-system-aarch64 -qtest
> unix:/tmp/qtest-32530.sock -qtest-log /dev/fd/2 -chardev
> socket,path=/tmp/qtest-32530.qmp,id=char0 -mon
> chardev=char0,mode=control -display none -audio none -machine
> npcm845-evb -accel qtest
> [I 0.00] OPENED
> [R +0.034918] endianness
> [S +0.034944] OK little
> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 10},
> "package": "v10.0.0-530-g88d6459dae"}, "capabilities": ["oob"]}}
> {"execute": "qmp_capabilities"}
> {"return": {}}
> [R +0.037373] writel 0xf0201002 0x1
> [S +0.037396] OK
> [R +0.037417] readl 0xf0201002
> [S +0.037426] OK 0x
> **
> ERROR:../tests/qtest/npcm8xx_pspi-test.c:45:test_init: assertion failed
> (pspi_read_ctl(qts, pspi) == CTL_SPIEN): (0x == 0x0001)
> Bail out!
> [I +0.037909] CLOSED
> Aborted (core dumped)
Thank you for testing it. I think the failure seems to be related to
the following commit which, has not been merged yet.
https://patchew.org/QEMU/20250414020629.1867106-1-timlee660...@gmail.com/
Here is our test result for reference. Thanks.
tim@openbmc:~/qemu$ QTEST_LOG=1 ./build/tests/qtest/npcm8xx_pspi_test
# random seed: R02Sede04390f31d107799cc627dd5992309
1..3
# Start of aarch64 tests
# Start of npcm8xx_pspi tests
# starting QEMU: exec /home/tim/git/qemu/build/qemu-system-aarch64
-qtest unix:/tmp/qtest-974125.sock -qtest-log /dev/fd/2 -chardev
socket,path=/tmp/qtest-974125.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine
npcm845-evb -accel qtest
[I 0.00] OPENED
[R +0.075439] endianness
[S +0.075466] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
"package": "v9.2.0-2138-g694a7d11fc"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}
{"return": {}}
[R +0.077905] writel 0xf0201002 0x1
[S +0.077915] OK
[R +0.077943] readl 0xf0201002
[S +0.077948] OK 0x0001
[I +0.078171] CLOSED
ok 1 /aarch64/npcm8xx_pspi/init
# starting QEMU: exec /home/tim/git/qemu/build/qemu-system-aarch64
-qtest unix:/tmp/qtest-974125.sock -qtest-log /dev/fd/2 -chardev
socket,path=/tmp/qtest-974125.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine
npcm845-evb -accel qtest
[I 0.00] OPENED
[R +0.075965] endianness
[S +0.075991] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
"package": "v9.2.0-2138-g694a7d11fc"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}
{"return": {}}
[R +0.078505] write 0xf0201002 0x1 0x05
[S +0.078515] OK
[R +0.078546] readl 0xf0201002
[S +0.078551] OK 0x0005
Wrote 0x05 to control register
[I +0.078806] CLOSED
ok 2 /aarch64/npcm8xx_pspi/ctl
# starting QEMU: exec /home/tim/git/qemu/build/qemu-system-aarch64
-qtest unix:/tmp/qtest-974125.sock -qtest-log /dev/fd/2 -chardev
socket,path=/tmp/qtest-974125.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine
npcm845-evb -accel qtest
[I 0.00] OPENED
[R +0.076480] endianness
[S +0.076506] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
"package": "v9.2.0-2138-g694a7d11fc"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}
{"return": {}}
[R +0.079280] writel 0xf0201002 0x5
[S +0.079288] OK
[R +0.079312] writew 0xf0201000 0x1234
[S +0.079316] OK
Wrote 0x1234 to data register
[R +0.079337] readw 0xf0201000
[S +0.079341] OK 0x1234
Read 0x1234 from data register
[I +0.079565] CLOSED
ok 3 /aarch64/npcm8xx_pspi/data
# End of npcm8xx_pspi tests
# End of aarch64 tests
--
Best regards,
Tim Lee
Re: [PATCH] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number
> This won't work on read-only storage.
>
> > +}
> > }
> > qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
> >
> > @@ -194,7 +206,8 @@ static void npcm845_evb_init(MachineState *machine)
> > qdev_realize(DEVICE(soc), NULL, &error_fatal);
> >
> > npcm8xx_load_bootrom(machine, soc);
> > -npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0,
> > 0));
> > +npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
> > + drive_get(IF_MTD, 0, 0));
> > npcm845_evb_i2c_init(soc);
> > npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
> > npcm8xx_load_kernel(machine, soc);
>
Indeed, we didn't consider the read-only storage case. Should we add
bdrv_is_read_only() to check as shown in the code below? Thanks.
if (bdrv_is_read_only(bs)) {
// Handle read-only storage
if (blk_size < fiu->flash_size) {
error_report("Read-only storage is too small for flash device");
return;
}
} else {
// Handle writable storage
if (blk_size < fiu->flash_size) {
blk_get_perm(blk, &perm, &shared_perm);
blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
BDRV_REQ_ZERO_WRITE, &error_abort);
blk_set_perm(blk, perm, shared_perm, &error_abort);
}
}
--
Best regards,
Tim Lee
Re: [PATCH] hw/arm/npcm8xx_boards: Correct valid_cpu_types setting of NPCM8XX SoC
> Is this worth backporting to stable branches? > > Applied to target-arm.next, thanks. > > -- PMM Hi Peter, Thank you for taking the time to review this. I believe it is worth backporting to stable branches. The string "cortex-a9" and "cortex-a35" will point to different ARMCPUInfo which could cause some features to not be applied correctly in specific initial functions: "cortex_a9_initfn" and "aarch64_a35_initfn". Sincerely, Tim Lee
[PATCH] hw/arm/npcm8xx_boards: Correct valid_cpu_types setting of NPCM8XX SoC
NPCM8XX SoC is the successor of the NPCM7XX. It features quad-core
Cortex-A35 (Armv8, 64-bit) CPUs and some additional peripherals.
Correct the `valid_cpu_types` setting to match the NPCM8XX SoC.
Signed-off-by: Tim Lee
---
hw/arm/npcm8xx_boards.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
index 3fb8478e72..6d3e59f6b9 100644
--- a/hw/arm/npcm8xx_boards.c
+++ b/hw/arm/npcm8xx_boards.c
@@ -213,7 +213,7 @@ static void npcm8xx_machine_class_init(ObjectClass *oc,
void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
static const char * const valid_cpu_types[] = {
-ARM_CPU_TYPE_NAME("cortex-a9"),
+ARM_CPU_TYPE_NAME("cortex-a35"),
NULL
};
--
2.34.1
[v2] tests/qtest: Add qtest for NPCM8XX PSPI module
- Created qtest to check initialization of registers in PSPI Module
- Implemented test into Build File
Tested:
./build/tests/qtest/npcm8xx-pspi_test
Signed-off-by: Tim Lee
---
Changes since v1:
- MAINTAINERS file not need to change
- Add comment for copyright/license information
- Correct CTL registers to use 16 bits
- Remove printf() in test cases
tests/qtest/meson.build | 3 +
tests/qtest/npcm8xx_pspi-test.c | 118
2 files changed, 121 insertions(+)
create mode 100644 tests/qtest/npcm8xx_pspi-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 3136d15e0f..88672a8b00 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -210,6 +210,8 @@ qtests_npcm7xx = \
'npcm7xx_watchdog_timer-test',
'npcm_gmac-test'] + \
(slirp.found() ? ['npcm7xx_emc-test'] : [])
+qtests_npcm8xx = \
+ ['npcm8xx_pspi-test']
qtests_aspeed = \
['aspeed_hace-test',
'aspeed_smc-test',
@@ -257,6 +259,7 @@ qtests_aarch64 = \
(config_all_accel.has_key('CONFIG_TCG') and
\
config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] :
[]) + \
(config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
+ (config_all_devices.has_key('CONFIG_NPCM8XX') ? qtests_npcm8xx : []) + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
diff --git a/tests/qtest/npcm8xx_pspi-test.c b/tests/qtest/npcm8xx_pspi-test.c
new file mode 100644
index 00..13b8a8229c
--- /dev/null
+++ b/tests/qtest/npcm8xx_pspi-test.c
@@ -0,0 +1,118 @@
+/*
+ * QTests for the Nuvoton NPCM8XX PSPI Controller
+ *
+ * Copyright (c) 2025 Nuvoton Technology Corporation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+
+/* Register offsets */
+#define DATA_OFFSET 0x00
+#define CTL_SPIEN 0x01
+#define CTL_OFFSET 0x02
+#define CTL_MOD 0x04
+
+typedef struct PSPI {
+uint64_t base_addr;
+} PSPI;
+
+PSPI pspi_defs = {
+.base_addr = 0xf0201000
+};
+
+static uint16_t pspi_read_data(QTestState *qts, const PSPI *pspi)
+{
+return qtest_readw(qts, pspi->base_addr + DATA_OFFSET);
+}
+
+static void pspi_write_data(QTestState *qts, const PSPI *pspi, uint16_t value)
+{
+qtest_writew(qts, pspi->base_addr + DATA_OFFSET, value);
+}
+
+static uint16_t pspi_read_ctl(QTestState *qts, const PSPI *pspi)
+{
+return qtest_readw(qts, pspi->base_addr + CTL_OFFSET);
+}
+
+static void pspi_write_ctl(QTestState *qts, const PSPI *pspi, uint16_t value)
+{
+qtest_writew(qts, pspi->base_addr + CTL_OFFSET, value);
+}
+
+/* Check PSPI can be reset to default value */
+static void test_init(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+
+/* Write CTL_SPIEN value to control register for enable PSPI module */
+pspi_write_ctl(qts, pspi, CTL_SPIEN);
+g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, CTL_SPIEN);
+
+qtest_quit(qts);
+}
+
+/* Check PSPI can be r/w data register */
+static void test_data(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+uint16_t test = 0x1234;
+uint16_t output;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+
+/* Enable 16-bit data interface mode */
+pspi_write_ctl(qts, pspi, CTL_MOD);
+g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, CTL_MOD);
+
+/* Write to data register */
+pspi_write_data(qts, pspi, test);
+
+/* Read from data register */
+output = pspi_read_data(qts, pspi);
+g_assert_cmphex(output, ==, test);
+
+qtest_quit(qts);
+}
+
+/* Check PSPI can be r/w control register */
+static void test_ctl(gconstpointer pspi_p)
+{
+const PSPI *pspi = pspi_p;
+uint8_t control = CTL_MOD;
+
+QTestState *qts = qtest_init("-machine npcm845-evb");
+
+/* Write CTL_MOD value to control register for 16-bit interface mode */
+qtest_memwrite(qts, pspi->base_addr + CTL_OFFSET,
+ &control, sizeof(control));
+g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, control);
+
+qtest_quit(qts);
+}
+
+static void pspi_add_test(const char *name, const PSPI* wd,
+GTestDataFunc fn)
+{
+g_autofree char *full_name = g_strdup_printf("npcm8xx_pspi/%s", name);
+qtest_add_data_func(full_name, wd, fn);
+}
+
+#define add_test(name, td) pspi_add_test(#name, td, test_##name)
+
+int main(int argc, char **argv)
+{
+g_test_init(&argc, &argv, NULL);
+
+add_test(init, &pspi_defs);
+add_test(ctl, &pspi_defs);
+add_test(data, &pspi_defs);
+return g_test_run();
+}
--
2.34.1
[v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number
Fix flash device part number to `mx66l1g45g` according image-bmc run on npcm8xx
evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)
And add auto zero flash image size to resolve error below after executing
`./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`
Error message:
qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
requires 134217728 bytes, mtd0 block backend provides 67108864 bytes
Tested:
Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally
Signed-off-by: Tim Lee
---
Changes since v1:
- Add a statement that checks whether the storage is writable
hw/arm/npcm8xx_boards.c | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
index 3fb8478e72..79295a586c 100644
--- a/hw/arm/npcm8xx_boards.c
+++ b/hw/arm/npcm8xx_boards.c
@@ -27,6 +27,7 @@
#include "qemu/error-report.h"
#include "qemu/datadir.h"
#include "qemu/units.h"
+#include "system/block-backend.h"
#define NPCM845_EVB_POWER_ON_STRAPS 0x17ff
@@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu, int
cs_no,
{
DeviceState *flash;
qemu_irq flash_cs;
+BlockBackend *blk;
+BlockDriverState *bs;
+uint64_t blk_size, perm, shared_perm;
flash = qdev_new(flash_type);
if (dinfo) {
qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
+blk = blk_by_legacy_dinfo(dinfo);
+bs = blk_bs(blk);
+blk_size = blk_getlength(blk);
+
+if (!bdrv_is_read_only(bs)) {
+if (blk_size < fiu->flash_size) {
+blk_get_perm(blk, &perm, &shared_perm);
+blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL, &error_abort);
+blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
+ BDRV_REQ_ZERO_WRITE, &error_abort);
+blk_set_perm(blk, perm, shared_perm, &error_abort);
+}
+}
}
qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
@@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm8xx_load_bootrom(machine, soc);
-npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
+npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
+ drive_get(IF_MTD, 0, 0));
npcm845_evb_i2c_init(soc);
npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
npcm8xx_load_kernel(machine, soc);
--
2.34.1
Re: [v2] tests/qtest: Add qtest for NPCM8XX PSPI module
Hi Peter,
Sorry about that. When I ran this qtest and I found an error then I
tried to modify npcm_pspi driver to make data register read/write test
pass.
[R +0.080118] writew 0xf0201002 0x4
[S +0.080126] OK
[R +0.080148] readw 0xf0201002
[S +0.080153] OK 0x0004
[R +0.080168] writew 0xf0201000 0x1234
[S +0.080171] OK
[R +0.080191] readw 0xf0201000
[S +0.080194] OK 0x1234
[I +0.080445] CLOSED
ok 3 /aarch64/npcm8xx_pspi/data
# End of npcm8xx_pspi tests
# End of aarch64 tests
Here is the change diff of what I modified in npcm_pspi_write_data()
Should I submit this patch for npcm_pspi driver? I'm not sure if I
modified it correctly. Thanks for your time.
static void npcm_pspi_write_data(NPCMPSPIState *s, uint16_t data)
{
-uint16_t value = 0;
+uint16_t data_l, data_h = 0;
if (FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, MOD)) {
-value = ssi_transfer(s->spi, extract16(data, 8, 8)) << 8;
+data_h = (extract16(data, 8, 8) << 8);
+ssi_transfer(s->spi, data_h);
}
-value |= ssi_transfer(s->spi, extract16(data, 0, 8));
-s->regs[R_PSPI_DATA] = value;
+data_l = extract16(data, 0, 8);
+ssi_transfer(s->spi, data_l);
+s->regs[R_PSPI_DATA] = (data_h | data_l);
Peter Maydell 於 2025年5月11日 週日 下午9:58寫道：
>
> On Sun, 11 May 2025 at 14:47, Peter Maydell wrote:
> >
> > On Wed, 7 May 2025 at 10:19, Tim Lee wrote:
> > >
> > > - Created qtest to check initialization of registers in PSPI Module
> > > - Implemented test into Build File
> > >
> > > Tested:
> > > ./build/tests/qtest/npcm8xx-pspi_test
> > >
> > > Signed-off-by: Tim Lee
> > > ---
> > > Changes since v1:
> > > - MAINTAINERS file not need to change
> > > - Add comment for copyright/license information
> > > - Correct CTL registers to use 16 bits
> > > - Remove printf() in test cases
> >
> >
> >
> > Applied to target-arm.next, thanks.
>
> ...but it fails "make check", so I've dropped it:
>
> not ok /aarch64/npcm8xx_pspi/data -
> ERROR:../../tests/qtest/npcm8xx_pspi-test.c:
> 80:test_data: assertion failed (output == test): (0x == 0x1234)
> Bail out!
> --- stderr -----------
> **
> ERROR:../../tests/qtest/npcm8xx_pspi-test.c:80:test_data: assertion failed
> (outp
> ut == test): (0x == 0x1234)
>
> (test program exited with status code -6)
>
> thanks
> -- PMM
--
Best regards,
Tim Lee
Re: [v2] hw/arm/npcm8xx_boards: Add auto zero flash image and device part number
Philippe Mathieu-Daudé 於 2025年5月8日 週四 下午2:19寫道：
>
> Hi Tim,
>
> On 8/5/25 04:15, Tim Lee wrote:
> > Fix flash device part number to `mx66l1g45g` according image-bmc run on
> > npcm8xx
> > evb board (SPIFlash...SF: Detected mx66l1g45g, total 128 MiB)
> >
> > And add auto zero flash image size to resolve error below after executing
> > `./qemu-system-aarch64 -machine npcm845-evb -drive file=image-bmc`
> >
> > Error message:
> > qemu-system-aarch64: mx66l1g45g device '/machine/unattached/device[73]'
> > requires 134217728 bytes, mtd0 block backend provides 67108864 bytes
> >
> > Tested:
> > Build passes and runs ./qemu-system-aarch64 -machine npcm845-evb normally
> >
> > Signed-off-by: Tim Lee
> > ---
> > Changes since v1:
> > - Add a statement that checks whether the storage is writable
> >
> > hw/arm/npcm8xx_boards.c | 20 +++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c
> > index 3fb8478e72..79295a586c 100644
> > --- a/hw/arm/npcm8xx_boards.c
> > +++ b/hw/arm/npcm8xx_boards.c
> > @@ -27,6 +27,7 @@
> > #include "qemu/error-report.h"
> > #include "qemu/datadir.h"
> > #include "qemu/units.h"
> > +#include "system/block-backend.h"
> >
> > #define NPCM845_EVB_POWER_ON_STRAPS 0x17ff
> >
> > @@ -59,10 +60,26 @@ static void npcm8xx_connect_flash(NPCM7xxFIUState *fiu,
> > int cs_no,
> > {
> > DeviceState *flash;
> > qemu_irq flash_cs;
> > +BlockBackend *blk;
> > +BlockDriverState *bs;
> > +uint64_t blk_size, perm, shared_perm;
> >
> > flash = qdev_new(flash_type);
> > if (dinfo) {
> > qdev_prop_set_drive(flash, "drive", blk_by_legacy_dinfo(dinfo));
> > +blk = blk_by_legacy_dinfo(dinfo);
> > +bs = blk_bs(blk);
> > +blk_size = blk_getlength(blk);
> > +
> > +if (!bdrv_is_read_only(bs)) {
>
> This isn't what I meant, we'll get the same issue with read-only storage.
>
> See:
> https://lore.kernel.org/qemu-devel/cafeaca9itedtrznx1krveza__dch95abppzbdtj0-tuxwih...@mail.gmail.com/
>
> > +if (blk_size < fiu->flash_size) {
> > +blk_get_perm(blk, &perm, &shared_perm);
> > +blk_set_perm(blk, BLK_PERM_ALL, BLK_PERM_ALL,
> > &error_abort);
> > +blk_truncate(blk, fiu->flash_size, true, PREALLOC_MODE_OFF,
> > + BDRV_REQ_ZERO_WRITE, &error_abort);
> > +blk_set_perm(blk, perm, shared_perm, &error_abort);
> > +}
> > +}
> > }
> > qdev_realize_and_unref(flash, BUS(fiu->spi), &error_fatal);
> >
> > @@ -194,7 +211,8 @@ static void npcm845_evb_init(MachineState *machine)
> > qdev_realize(DEVICE(soc), NULL, &error_fatal);
> >
> > npcm8xx_load_bootrom(machine, soc);
> > -npcm8xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0,
> > 0));
> > +npcm8xx_connect_flash(&soc->fiu[0], 0, "mx66l1g45g",
> > + drive_get(IF_MTD, 0, 0));
> > npcm845_evb_i2c_init(soc);
> > npcm845_evb_fan_init(NPCM8XX_MACHINE(machine), soc);
> > npcm8xx_load_kernel(machine, soc);
>
Hi Philippe,
Thanks for your sharing. Now I understand why you say it won't work on
read-only storage.
Currently, without this change we cannot run the npcm845-evb of QEMU
with a large flash size (128 MB), including the latest OpenBMC build
image.
That's why we sync the method into npcm8xx_board for our npcm845 evb
board. It's seems not suitable for upstream, right? Thanks.
--
Best regards,
Tim Lee
Re: [PATCH] tests/qtest: Add qtest for NPCM8XX PSPI module
Hi Peter,
Thanks for your suggestion. Those changes will be included in v2.
Peter Maydell 於 2025年5月6日 週二 下午8:52寫道：
>
> On Fri, 18 Apr 2025 at 10:12, Tim Lee wrote:
> >
> > - Created qtest to check initialization of registers in PSPI Module
> > - Implemented test into Build File
> >
> > Tested:
> > ./build/tests/qtest/npcm8xx-pspi_test
> >
> > Signed-off-by: Tim Lee
> > ---
> > MAINTAINERS | 1 +
> > tests/qtest/meson.build | 3 +
> > tests/qtest/npcm8xx_pspi-test.c | 104
> > 3 files changed, 108 insertions(+)
> > create mode 100644 tests/qtest/npcm8xx_pspi-test.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d54b5578f8..0162f59bf7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -892,6 +892,7 @@ F: hw/sensor/adm1266.c
> > F: include/hw/*/npcm*
> > F: tests/qtest/npcm*
> > F: tests/qtest/adm1266-test.c
> > +F: tests/qtest/npcm8xx_pspi-test.c
>
> This file is already matched as being in this section by the
> wildcard two lines earlier.
MAINTAINERS file keep no change.
>
> > F: pc-bios/npcm7xx_bootrom.bin
> > F: pc-bios/npcm8xx_bootrom.bin
> > F: roms/vbootrom
>
> > diff --git a/tests/qtest/npcm8xx_pspi-test.c
> > b/tests/qtest/npcm8xx_pspi-test.c
> > new file mode 100644
> > index 00..107bce681f
> > --- /dev/null
> > +++ b/tests/qtest/npcm8xx_pspi-test.c
> > @@ -0,0 +1,104 @@
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +#include "qemu/module.h"
>
> Every source file needs to start with the usual brief
> comment giving its copyright/license information (and we
> like that to include an SPDX-license-Identifier these days
> for new source files).
>
Add comment for copyright/license information.
> > +
> > +#define DATA_OFFSET 0x00
> > +#define CTL_SPIEN 0x01
> > +#define CTL_OFFSET 0x02
> > +#define CTL_MOD 0x04
> > +
> > +typedef struct PSPI {
> > +uint64_t base_addr;
> > +} PSPI;
> > +
> > +PSPI pspi_defs = {
> > +.base_addr = 0xf0201000
> > +};
> > +
> > +static uint16_t pspi_read_data(QTestState *qts, const PSPI *pspi)
> > +{
> > +return qtest_readw(qts, pspi->base_addr + DATA_OFFSET);
> > +}
> > +
> > +static void pspi_write_data(QTestState *qts, const PSPI *pspi, uint16_t
> > value)
> > +{
> > +qtest_writew(qts, pspi->base_addr + DATA_OFFSET, value);
> > +}
> > +
> > +static uint32_t pspi_read_ctl(QTestState *qts, const PSPI *pspi)
> > +{
> > +return qtest_readl(qts, pspi->base_addr + CTL_OFFSET);
> > +}
> > +
> > +static void pspi_write_ctl(QTestState *qts, const PSPI *pspi, uint32_t
> > value)
> > +{
> > +qtest_writel(qts, pspi->base_addr + CTL_OFFSET, value);
> > +}
>
> If I'm reading the implementation correctly, it makes both
> the DATA and CTL registers 16 bits, but this code has the
> data register 16 bits and the control register 32 bits.
> Which is correct ?
>
Yes, you are right! DATA and CLT registers both use 16 bits.
> > +/* Check PSPI can be reset to default value */
> > +static void test_init(gconstpointer pspi_p)
> > +{
> > +const PSPI *pspi = pspi_p;
> > +
> > +QTestState *qts = qtest_init("-machine npcm845-evb");
> > +pspi_write_ctl(qts, pspi, CTL_SPIEN);
> > +g_assert_cmphex(pspi_read_ctl(qts, pspi), ==, CTL_SPIEN);
> > +
> > +qtest_quit(qts);
> > +}
> > +
> > +/* Check PSPI can be r/w data register */
> > +static void test_data(gconstpointer pspi_p)
> > +{
> > +const PSPI *pspi = pspi_p;
> > +uint16_t test = 0x1234;
> > +uint16_t output;
> > +
> > +QTestState *qts = qtest_init("-machine npcm845-evb");
> > +
> > +/* Write to data register */
> > +pspi_write_data(qts, pspi, test);
> > +printf("Wrote 0x%x to data register\n", test);
>
> Don't put printf()s in test cases, please. The test
> output is supposed to be TAP test protocol format, and
> the printfs insert random junk into that.
>
> If you need to output some kind of message, you can use
> g_test_message(), but for simple stuff like this I don't think
> the printfs are really adding anything, because the test is
> so short.
>
Remove printf() in test cases.
> > +
> > +/* Read from data register */
> > +output = pspi_read_data(qts, pspi);
> > +printf("Read 0x%x from data register\n", output);
>
> Can we assert something useful here about what we read
> (e.g. that it's the same as what we wrote) ?
>
Currently, I just write test data to data register then read data from
it and verify it.
> > +
> > +qtest_quit(qts);
> > +}
>
> thanks
> -- PMM
--
Best regards,
Tim Lee
