hw/arm/npcm7xx_boards.c | 49 +++++++++++++++++++++++++++++++++------- include/hw/arm/npcm7xx.h | 1 + 2 files changed, 42 insertions(+), 8 deletions(-)
In some situations it is desirable to be able to specify the flash type
connected to a board. For example, the target operating system may not
support the default flash type, its support may be broken, or the qemu
emulation is insufficient and the default flash is not detected.
On top of that, the ability to test various flash types improves
testability since a single emulated board can be used to test a variety
of flash chips with the controller supported by that board.
The aspeed emulation supports an option to specify the flash type. Use
the same mechanism to configure the flash type for Nuvoton 7xx boards.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
I don't know if there is interest in this, but I figured I should at least
submit it. Background is that Macronix flash support is broken when running
upstream Linux v6.16-rc1, thanks to upstream Linux commit 947c86e481a027e
("mtd: spi-nor: macronix: Drop the redundant flash info fields"). I needed
a workaround, and using a different flash model was the easiest solution.
hw/arm/npcm7xx_boards.c | 49 +++++++++++++++++++++++++++++++++-------
include/hw/arm/npcm7xx.h | 1 +
2 files changed, 42 insertions(+), 8 deletions(-)
diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c
index 465a0e5ace..88b9aeddd4 100644
--- a/hw/arm/npcm7xx_boards.c
+++ b/hw/arm/npcm7xx_boards.c
@@ -363,6 +363,7 @@ static void kudo_bmc_i2c_init(NPCM7xxState *soc)
static void npcm750_evb_init(MachineState *machine)
{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(machine);
NPCM7xxState *soc;
soc = npcm7xx_create_soc(machine, NPCM750_EVB_POWER_ON_STRAPS);
@@ -370,14 +371,16 @@ static void npcm750_evb_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm7xx_load_bootrom(machine, soc);
- npcm7xx_connect_flash(&soc->fiu[0], 0, "w25q256", drive_get(IF_MTD, 0, 0));
+ npcm7xx_connect_flash(&soc->fiu[0], 0, ms->spi_model ? : "w25q256",
+ drive_get(IF_MTD, 0, 0));
npcm750_evb_i2c_init(soc);
- npcm750_evb_fan_init(NPCM7XX_MACHINE(machine), soc);
+ npcm750_evb_fan_init(ms, soc);
npcm7xx_load_kernel(machine, soc);
}
static void quanta_gsj_init(MachineState *machine)
{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(machine);
NPCM7xxState *soc;
soc = npcm7xx_create_soc(machine, QUANTA_GSJ_POWER_ON_STRAPS);
@@ -385,15 +388,16 @@ static void quanta_gsj_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm7xx_load_bootrom(machine, soc);
- npcm7xx_connect_flash(&soc->fiu[0], 0, "mx25l25635e",
+ npcm7xx_connect_flash(&soc->fiu[0], 0, ms->spi_model ? : "mx25l25635e",
drive_get(IF_MTD, 0, 0));
quanta_gsj_i2c_init(soc);
- quanta_gsj_fan_init(NPCM7XX_MACHINE(machine), soc);
+ quanta_gsj_fan_init(ms, soc);
npcm7xx_load_kernel(machine, soc);
}
static void quanta_gbs_init(MachineState *machine)
{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(machine);
NPCM7xxState *soc;
soc = npcm7xx_create_soc(machine, QUANTA_GBS_POWER_ON_STRAPS);
@@ -402,7 +406,7 @@ static void quanta_gbs_init(MachineState *machine)
npcm7xx_load_bootrom(machine, soc);
- npcm7xx_connect_flash(&soc->fiu[0], 0, "mx66u51235f",
+ npcm7xx_connect_flash(&soc->fiu[0], 0, ms->spi_model ? : "mx66u51235f",
drive_get(IF_MTD, 0, 0));
quanta_gbs_i2c_init(soc);
@@ -412,6 +416,7 @@ static void quanta_gbs_init(MachineState *machine)
static void kudo_bmc_init(MachineState *machine)
{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(machine);
NPCM7xxState *soc;
soc = npcm7xx_create_soc(machine, KUDO_BMC_POWER_ON_STRAPS);
@@ -419,9 +424,9 @@ static void kudo_bmc_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm7xx_load_bootrom(machine, soc);
- npcm7xx_connect_flash(&soc->fiu[0], 0, "mx66u51235f",
+ npcm7xx_connect_flash(&soc->fiu[0], 0, ms->spi_model ? : "mx66u51235f",
drive_get(IF_MTD, 0, 0));
- npcm7xx_connect_flash(&soc->fiu[1], 0, "mx66u51235f",
+ npcm7xx_connect_flash(&soc->fiu[1], 0, ms->spi_model ? : "mx66u51235f",
drive_get(IF_MTD, 3, 0));
kudo_bmc_i2c_init(soc);
@@ -431,6 +436,7 @@ static void kudo_bmc_init(MachineState *machine)
static void mori_bmc_init(MachineState *machine)
{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(machine);
NPCM7xxState *soc;
soc = npcm7xx_create_soc(machine, MORI_BMC_POWER_ON_STRAPS);
@@ -438,7 +444,7 @@ static void mori_bmc_init(MachineState *machine)
qdev_realize(DEVICE(soc), NULL, &error_fatal);
npcm7xx_load_bootrom(machine, soc);
- npcm7xx_connect_flash(&soc->fiu[1], 0, "mx66u51235f",
+ npcm7xx_connect_flash(&soc->fiu[1], 0, ms->spi_model ? : "mx66u51235f",
drive_get(IF_MTD, 3, 0));
npcm7xx_load_kernel(machine, soc);
@@ -453,6 +459,27 @@ static void npcm7xx_set_soc_type(NPCM7xxMachineClass *nmc, const char *type)
mc->default_cpus = mc->min_cpus = mc->max_cpus = sc->num_cpus;
}
+static char *npcm7xx_get_spi_model(Object *obj, Error **errp)
+ {
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(obj);
+ return g_strdup(ms->spi_model);
+}
+
+static void npcm7xx_set_spi_model(Object *obj, const char *value, Error **errp)
+{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(obj);
+
+ g_free(ms->spi_model);
+ ms->spi_model = g_strdup(value);
+}
+
+static void npcm7xx_machine_finalize(Object *obj)
+{
+ NPCM7xxMachine *ms = NPCM7XX_MACHINE(obj);
+
+ g_free(ms->spi_model);
+}
+
static void npcm7xx_machine_class_init(ObjectClass *oc, const void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -466,6 +493,11 @@ static void npcm7xx_machine_class_init(ObjectClass *oc, const void *data)
mc->no_parallel = 1;
mc->default_ram_id = "ram";
mc->valid_cpu_types = valid_cpu_types;
+
+ object_class_property_add_str(oc, "spi-model", npcm7xx_get_spi_model,
+ npcm7xx_set_spi_model);
+ object_class_property_set_description(oc, "spi-model",
+ "Change the SPI Flash model");
}
/*
@@ -542,6 +574,7 @@ static const TypeInfo npcm7xx_machine_types[] = {
.name = TYPE_NPCM7XX_MACHINE,
.parent = TYPE_MACHINE,
.instance_size = sizeof(NPCM7xxMachine),
+ .instance_finalize = npcm7xx_machine_finalize,
.class_size = sizeof(NPCM7xxMachineClass),
.class_init = npcm7xx_machine_class_init,
.abstract = true,
diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
index 56536565b7..e8844956db 100644
--- a/include/hw/arm/npcm7xx.h
+++ b/include/hw/arm/npcm7xx.h
@@ -62,6 +62,7 @@ struct NPCM7xxMachine {
*/
SplitIRQ fan_splitter[NPCM7XX_NR_PWM_MODULES *
NPCM7XX_PWM_PER_MODULE];
+ char *spi_model;
};
#define TYPE_NPCM7XX_MACHINE MACHINE_TYPE_NAME("npcm7xx")
--
2.45.2On Tue, 10 Jun 2025 at 02:28, Guenter Roeck <linux@roeck-us.net> wrote:
>
> In some situations it is desirable to be able to specify the flash type
> connected to a board. For example, the target operating system may not
> support the default flash type, its support may be broken, or the qemu
> emulation is insufficient and the default flash is not detected.
> On top of that, the ability to test various flash types improves
> testability since a single emulated board can be used to test a variety
> of flash chips with the controller supported by that board.
>
> The aspeed emulation supports an option to specify the flash type. Use
> the same mechanism to configure the flash type for Nuvoton 7xx boards.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> I don't know if there is interest in this, but I figured I should at least
> submit it. Background is that Macronix flash support is broken when running
> upstream Linux v6.16-rc1, thanks to upstream Linux commit 947c86e481a027e
> ("mtd: spi-nor: macronix: Drop the redundant flash info fields"). I needed
> a workaround, and using a different flash model was the easiest solution.
I think the question I would have here is "is this the flash
device the hardware actually has?". If QEMU's using the wrong
flash type, we should fix that. If QEMU's modelling the right
flash type and the kernel doesn't implement it, then that's
a kernel bug and the right fix is to get the kernel to
handle that flash type.
thanks
-- PMM
On 6/10/25 01:42, Peter Maydell wrote:
> On Tue, 10 Jun 2025 at 02:28, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> In some situations it is desirable to be able to specify the flash type
>> connected to a board. For example, the target operating system may not
>> support the default flash type, its support may be broken, or the qemu
>> emulation is insufficient and the default flash is not detected.
>> On top of that, the ability to test various flash types improves
>> testability since a single emulated board can be used to test a variety
>> of flash chips with the controller supported by that board.
>>
>> The aspeed emulation supports an option to specify the flash type. Use
>> the same mechanism to configure the flash type for Nuvoton 7xx boards.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> I don't know if there is interest in this, but I figured I should at least
>> submit it. Background is that Macronix flash support is broken when running
>> upstream Linux v6.16-rc1, thanks to upstream Linux commit 947c86e481a027e
>> ("mtd: spi-nor: macronix: Drop the redundant flash info fields"). I needed
>> a workaround, and using a different flash model was the easiest solution.
>
> I think the question I would have here is "is this the flash
> device the hardware actually has?". If QEMU's using the wrong
> flash type, we should fix that. If QEMU's modelling the right
> flash type and the kernel doesn't implement it, then that's
> a kernel bug and the right fix is to get the kernel to
> handle that flash type.
>
That isn't the point. Yes, one is that qemu's emulation is broken, the other is
that I wanted to be able to test with other flash types anyway. I understand
that this 'violates' the idea of exactly emulating some hardware, but I want
to be able to use qemu beyond that.
Either case, I did some debugging: For flashes with sfdp data (mx25l25635e),
qemu returns bad sfdp data (at least with quanta-gsj), causing the kernel
(as of v6.16-rc1) to bail out. For other flashes (mx66u51235f) it returns
no sfdp data, but the upstream kernel now depends on it.
NP, I'll just carry the patch locally in my tree. As mentioned above, I wasn't
sure if there is interest in this patch, but I wanted to at least publish it.
Thanks,
Guenter
On Tue, 10 Jun 2025 at 14:31, Guenter Roeck <linux@roeck-us.net> wrote: > > On 6/10/25 01:42, Peter Maydell wrote: > > I think the question I would have here is "is this the flash > > device the hardware actually has?". If QEMU's using the wrong > > flash type, we should fix that. If QEMU's modelling the right > > flash type and the kernel doesn't implement it, then that's > > a kernel bug and the right fix is to get the kernel to > > handle that flash type. > > > > That isn't the point. It is the point from my perspective. This board type is supposed to be modelling real hardware, and therefore in determining how it should behave we start by saying "so what does the real hardware do?". Adding extra behaviour that the real hardware does not have is something that has a tendency to lead into unspecified, untested and hard to maintain complexity. > Yes, one is that qemu's emulation is broken, the other is > that I wanted to be able to test with other flash types anyway. I understand > that this 'violates' the idea of exactly emulating some hardware, but I want > to be able to use qemu beyond that. Why this machine type in particular, though? You say in the commit message that aspeed lets you specify the flash type. > Either case, I did some debugging: For flashes with sfdp data (mx25l25635e), > qemu returns bad sfdp data (at least with quanta-gsj), causing the kernel > (as of v6.16-rc1) to bail out. For other flashes (mx66u51235f) it returns > no sfdp data, but the upstream kernel now depends on it. That sounds like a bug we should fix. Would one of the Nuvoton maintainers like to take a look? thanks -- PMM
On 6/10/25 06:58, Peter Maydell wrote: > On Tue, 10 Jun 2025 at 14:31, Guenter Roeck <linux@roeck-us.net> wrote: >> >> On 6/10/25 01:42, Peter Maydell wrote: >>> I think the question I would have here is "is this the flash >>> device the hardware actually has?". If QEMU's using the wrong >>> flash type, we should fix that. If QEMU's modelling the right >>> flash type and the kernel doesn't implement it, then that's >>> a kernel bug and the right fix is to get the kernel to >>> handle that flash type. >>> >> >> That isn't the point. > > It is the point from my perspective. This board type is > supposed to be modelling real hardware, and therefore in > determining how it should behave we start by saying "so > what does the real hardware do?". Adding extra behaviour > that the real hardware does not have is something that > has a tendency to lead into unspecified, untested and hard > to maintain complexity. > Depends. I would argue that the ability to easily test all supported flash types makes the code more robust, not less robust. >> Yes, one is that qemu's emulation is broken, the other is >> that I wanted to be able to test with other flash types anyway. I understand >> that this 'violates' the idea of exactly emulating some hardware, but I want >> to be able to use qemu beyond that. > > Why this machine type in particular, though? You say in the commit > message that aspeed lets you specify the flash type. > Yes, the aspeed emulations let me provide spi-model= and fmc-model= board parameters which I use to work around the problem there. That is why I thought a similar solution might be acceptable for Nuvoton boards, but apparently I was wrong. >> Either case, I did some debugging: For flashes with sfdp data (mx25l25635e), >> qemu returns bad sfdp data (at least with quanta-gsj), causing the kernel >> (as of v6.16-rc1) to bail out. For other flashes (mx66u51235f) it returns >> no sfdp data, but the upstream kernel now depends on it. > > That sounds like a bug we should fix. Would one of the Nuvoton > maintainers like to take a look? > I don't think it is a Nuvoton specific problem. I see the same or similar problems with Aspeed boards. Besides, the lack of sfdp data for mx66u51235f is most definitely not Nuvoton specific. Thanks, Guenter
© 2016 - 2025 Red Hat, Inc.