diff mbox series

[2/3] hw/ssi: Add Nuvoton PSPI Module

Message ID 20230206233428.2772669-3-wuhaotsh@google.com (mailing list archive)
State New, archived
Headers show
Series Nuvoton Peripheral SPI (PSPI) Module | expand

Commit Message

Hao Wu Feb. 6, 2023, 11:34 p.m. UTC
Nuvoton's PSPI is a general purpose SPI module which enables
connections to SPI-based peripheral devices.

Signed-off-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Chris Rauer <crauer@google.com>
---
 MAINTAINERS                |   6 +-
 hw/ssi/meson.build         |   2 +-
 hw/ssi/npcm_pspi.c         | 216 +++++++++++++++++++++++++++++++++++++
 hw/ssi/trace-events        |   5 +
 include/hw/ssi/npcm_pspi.h |  53 +++++++++
 5 files changed, 278 insertions(+), 4 deletions(-)
 create mode 100644 hw/ssi/npcm_pspi.c
 create mode 100644 include/hw/ssi/npcm_pspi.h

Comments

Philippe Mathieu-Daudé Feb. 7, 2023, 7:13 a.m. UTC | #1
On 7/2/23 00:34, Hao Wu wrote:
> Nuvoton's PSPI is a general purpose SPI module which enables
> connections to SPI-based peripheral devices.
> 
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Chris Rauer <crauer@google.com>
> ---
>   MAINTAINERS                |   6 +-
>   hw/ssi/meson.build         |   2 +-
>   hw/ssi/npcm_pspi.c         | 216 +++++++++++++++++++++++++++++++++++++
>   hw/ssi/trace-events        |   5 +
>   include/hw/ssi/npcm_pspi.h |  53 +++++++++
>   5 files changed, 278 insertions(+), 4 deletions(-)
>   create mode 100644 hw/ssi/npcm_pspi.c
>   create mode 100644 include/hw/ssi/npcm_pspi.h


> +static const MemoryRegionOps npcm_pspi_ctrl_ops = {
> +    .read = npcm_pspi_ctrl_read,
> +    .write = npcm_pspi_ctrl_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 2,

I'm not sure about ".max_access_size = 2". The datasheet does
not seem public. Does that mean the CPU bus can not do a 32-bit
access to read two consecutive 16-bit registers? (these fields
restrict the guest accesses to the device).

> +        .unaligned = false,
> +    },

You might want instead (which is how you implemented the r/w
handlers):

     .impl.min_access_size = 2,
     .impl.max_access_size = 2,

> +};


> +static void npcm_pspi_realize(DeviceState *dev, Error **errp)
> +{
> +    NPCMPSPIState *s = NPCM_PSPI(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    Object *obj = OBJECT(dev);
> +
> +    s->spi = ssi_create_bus(dev, "pspi");

FYI there is an ongoing discussion about how to model QOM tree. If
this bus isn't shared with another controller, the "embed QOM child
in parent" style could be preferred. If so, the bus would be created
as:

       object_initialize_child(obj, "pspi", &s->spi, TYPE_SSI_BUS);

> +    memory_region_init_io(&s->mmio, obj, &npcm_pspi_ctrl_ops, s,
> +                          "mmio", 4 * KiB);
> +    sysbus_init_mmio(sbd, &s->mmio);
> +    sysbus_init_irq(sbd, &s->irq);
> +}


> diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events
> index c707d4aaba..16ea9954c4 100644
> --- a/hw/ssi/trace-events
> +++ b/hw/ssi/trace-events

> +# npcm_pspi.c
> +npcm_pspi_enter_reset(const char *id, int reset_type) "%s reset type: %d"
> +npcm_pspi_ctrl_read(const char *id, uint64_t addr, uint16_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
> +npcm_pspi_ctrl_write(const char *id, uint64_t addr, uint16_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx16

Since the region is 4KiB and the implementation is 16-bit, the formats
could be simplified as offset 0x%03 and value 0x%04. The traces will
then be more digestible to human eyes.

Modulo the impl.access_size change:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Hao Wu Feb. 7, 2023, 6:46 p.m. UTC | #2
Thanks for your review!

On Mon, Feb 6, 2023 at 11:13 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 7/2/23 00:34, Hao Wu wrote:
> > Nuvoton's PSPI is a general purpose SPI module which enables
> > connections to SPI-based peripheral devices.
> >
> > Signed-off-by: Hao Wu <wuhaotsh@google.com>
> > Reviewed-by: Chris Rauer <crauer@google.com>
> > ---
> >   MAINTAINERS                |   6 +-
> >   hw/ssi/meson.build         |   2 +-
> >   hw/ssi/npcm_pspi.c         | 216 +++++++++++++++++++++++++++++++++++++
> >   hw/ssi/trace-events        |   5 +
> >   include/hw/ssi/npcm_pspi.h |  53 +++++++++
> >   5 files changed, 278 insertions(+), 4 deletions(-)
> >   create mode 100644 hw/ssi/npcm_pspi.c
> >   create mode 100644 include/hw/ssi/npcm_pspi.h
>
>
> > +static const MemoryRegionOps npcm_pspi_ctrl_ops = {
> > +    .read = npcm_pspi_ctrl_read,
> > +    .write = npcm_pspi_ctrl_write,
> > +    .endianness = DEVICE_LITTLE_ENDIAN,
> > +    .valid = {
> > +        .min_access_size = 1,
> > +        .max_access_size = 2,
>
> I'm not sure about ".max_access_size = 2". The datasheet does
> not seem public. Does that mean the CPU bus can not do a 32-bit
> access to read two consecutive 16-bit registers? (these fields
> restrict the guest accesses to the device).
>
> > +        .unaligned = false,
> > +    },
>
> You might want instead (which is how you implemented the r/w
> handlers):
>
>      .impl.min_access_size = 2,
>      .impl.max_access_size = 2,
>
Thanks for the reminder. The datasheet suggests it's either 8-bit or
16-bit accesses. But I think using your suggestion makes sense
and will be more widely adapted.

>
> > +};
>
>
> > +static void npcm_pspi_realize(DeviceState *dev, Error **errp)
> > +{
> > +    NPCMPSPIState *s = NPCM_PSPI(dev);
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > +    Object *obj = OBJECT(dev);
> > +
> > +    s->spi = ssi_create_bus(dev, "pspi");
>
> FYI there is an ongoing discussion about how to model QOM tree. If
> this bus isn't shared with another controller, the "embed QOM child
> in parent" style could be preferred. If so, the bus would be created
> as:
>
>        object_initialize_child(obj, "pspi", &s->spi, TYPE_SSI_BUS);
>
I was just following some existing code here. I think I can use the new
style.

>
> > +    memory_region_init_io(&s->mmio, obj, &npcm_pspi_ctrl_ops, s,
> > +                          "mmio", 4 * KiB);
> > +    sysbus_init_mmio(sbd, &s->mmio);
> > +    sysbus_init_irq(sbd, &s->irq);
> > +}
>
>
> > diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events
> > index c707d4aaba..16ea9954c4 100644
> > --- a/hw/ssi/trace-events
> > +++ b/hw/ssi/trace-events
>
> > +# npcm_pspi.c
> > +npcm_pspi_enter_reset(const char *id, int reset_type) "%s reset type:
> %d"
> > +npcm_pspi_ctrl_read(const char *id, uint64_t addr, uint16_t data) "%s
> offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
> > +npcm_pspi_ctrl_write(const char *id, uint64_t addr, uint16_t data) "%s
> offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
>
> Since the region is 4KiB and the implementation is 16-bit, the formats
> could be simplified as offset 0x%03 and value 0x%04. The traces will
> then be more digestible to human eyes.
>
I'll do this.

>
> Modulo the impl.access_size change:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
Hao Wu Feb. 7, 2023, 7:21 p.m. UTC | #3
On Tue, Feb 7, 2023 at 10:46 AM Hao Wu <wuhaotsh@google.com> wrote:

> Thanks for your review!
>
> On Mon, Feb 6, 2023 at 11:13 PM Philippe Mathieu-Daudé <philmd@linaro.org>
> wrote:
>
>> On 7/2/23 00:34, Hao Wu wrote:
>> > Nuvoton's PSPI is a general purpose SPI module which enables
>> > connections to SPI-based peripheral devices.
>> >
>> > Signed-off-by: Hao Wu <wuhaotsh@google.com>
>> > Reviewed-by: Chris Rauer <crauer@google.com>
>> > ---
>> >   MAINTAINERS                |   6 +-
>> >   hw/ssi/meson.build         |   2 +-
>> >   hw/ssi/npcm_pspi.c         | 216 +++++++++++++++++++++++++++++++++++++
>> >   hw/ssi/trace-events        |   5 +
>> >   include/hw/ssi/npcm_pspi.h |  53 +++++++++
>> >   5 files changed, 278 insertions(+), 4 deletions(-)
>> >   create mode 100644 hw/ssi/npcm_pspi.c
>> >   create mode 100644 include/hw/ssi/npcm_pspi.h
>>
>>
>> > +static const MemoryRegionOps npcm_pspi_ctrl_ops = {
>> > +    .read = npcm_pspi_ctrl_read,
>> > +    .write = npcm_pspi_ctrl_write,
>> > +    .endianness = DEVICE_LITTLE_ENDIAN,
>> > +    .valid = {
>> > +        .min_access_size = 1,
>> > +        .max_access_size = 2,
>>
>> I'm not sure about ".max_access_size = 2". The datasheet does
>> not seem public. Does that mean the CPU bus can not do a 32-bit
>> access to read two consecutive 16-bit registers? (these fields
>> restrict the guest accesses to the device).
>>
>> > +        .unaligned = false,
>> > +    },
>>
>> You might want instead (which is how you implemented the r/w
>> handlers):
>>
>>      .impl.min_access_size = 2,
>>      .impl.max_access_size = 2,
>>
> Thanks for the reminder. The datasheet suggests it's either 8-bit or
> 16-bit accesses. But I think using your suggestion makes sense
> and will be more widely adapted.
>
>>
>> > +};
>>
>>
>> > +static void npcm_pspi_realize(DeviceState *dev, Error **errp)
>> > +{
>> > +    NPCMPSPIState *s = NPCM_PSPI(dev);
>> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> > +    Object *obj = OBJECT(dev);
>> > +
>> > +    s->spi = ssi_create_bus(dev, "pspi");
>>
>> FYI there is an ongoing discussion about how to model QOM tree. If
>> this bus isn't shared with another controller, the "embed QOM child
>> in parent" style could be preferred. If so, the bus would be created
>> as:
>>
>>        object_initialize_child(obj, "pspi", &s->spi, TYPE_SSI_BUS);
>>
> I was just following some existing code here. I think I can use the new
> style.
>
I've tried to use this and got the following error:
**
ERROR:../qom/object.c:511:object_initialize_with_type: assertion failed:
(size >= type->instance_size)
Bail out! ERROR:../qom/object.c:511:object_initialize_with_type: assertion
failed: (size >= type->instance_size)

I think the problem is that we define s->spi as SSIBus* instead of SSIBus.
But if we define it as SSIBus, we'll
get an incomplete type error. Fixing it will require refactoring
hw/ssi/ssi.c which I'm not sure if we want to do
it right now. This code is consistent with other code in hw/ssi so I guess
we can leave it here for now and wait
for a future refactor.

>
>> > +    memory_region_init_io(&s->mmio, obj, &npcm_pspi_ctrl_ops, s,
>> > +                          "mmio", 4 * KiB);
>> > +    sysbus_init_mmio(sbd, &s->mmio);
>> > +    sysbus_init_irq(sbd, &s->irq);
>> > +}
>>
>>
>> > diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events
>> > index c707d4aaba..16ea9954c4 100644
>> > --- a/hw/ssi/trace-events
>> > +++ b/hw/ssi/trace-events
>>
>> > +# npcm_pspi.c
>> > +npcm_pspi_enter_reset(const char *id, int reset_type) "%s reset type:
>> %d"
>> > +npcm_pspi_ctrl_read(const char *id, uint64_t addr, uint16_t data) "%s
>> offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
>> > +npcm_pspi_ctrl_write(const char *id, uint64_t addr, uint16_t data) "%s
>> offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
>>
>> Since the region is 4KiB and the implementation is 16-bit, the formats
>> could be simplified as offset 0x%03 and value 0x%04. The traces will
>> then be more digestible to human eyes.
>>
> I'll do this.
>
>>
>> Modulo the impl.access_size change:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>
Philippe Mathieu-Daudé Feb. 8, 2023, 7:45 a.m. UTC | #4
On 7/2/23 20:21, Hao Wu wrote:
> On Tue, Feb 7, 2023 at 10:46 AM Hao Wu <wuhaotsh@google.com 
> <mailto:wuhaotsh@google.com>> wrote:
> 
>     Thanks for your review!
> 
>     On Mon, Feb 6, 2023 at 11:13 PM Philippe Mathieu-Daudé
>     <philmd@linaro.org <mailto:philmd@linaro.org>> wrote:
> 
>         On 7/2/23 00:34, Hao Wu wrote:
>          > Nuvoton's PSPI is a general purpose SPI module which enables
>          > connections to SPI-based peripheral devices.
>          >
>          > Signed-off-by: Hao Wu <wuhaotsh@google.com
>         <mailto:wuhaotsh@google.com>>
>          > Reviewed-by: Chris Rauer <crauer@google.com
>         <mailto:crauer@google.com>>
>          > ---
>          >   MAINTAINERS                |   6 +-
>          >   hw/ssi/meson.build         |   2 +-
>          >   hw/ssi/npcm_pspi.c         | 216
>         +++++++++++++++++++++++++++++++++++++
>          >   hw/ssi/trace-events        |   5 +
>          >   include/hw/ssi/npcm_pspi.h |  53 +++++++++
>          >   5 files changed, 278 insertions(+), 4 deletions(-)
>          >   create mode 100644 hw/ssi/npcm_pspi.c
>          >   create mode 100644 include/hw/ssi/npcm_pspi.h


>          > +static void npcm_pspi_realize(DeviceState *dev, Error **errp)
>          > +{
>          > +    NPCMPSPIState *s = NPCM_PSPI(dev);
>          > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>          > +    Object *obj = OBJECT(dev);
>          > +
>          > +    s->spi = ssi_create_bus(dev, "pspi");
> 
>         FYI there is an ongoing discussion about how to model QOM tree. If
>         this bus isn't shared with another controller, the "embed QOM child
>         in parent" style could be preferred. If so, the bus would be created
>         as:
> 
>                 object_initialize_child(obj, "pspi", &s->spi, TYPE_SSI_BUS);
> 
>     I was just following some existing code here. I think I can use the
>     new style. 
> 
> I've tried to use this and got the following error:
> **
> ERROR:../qom/object.c:511:object_initialize_with_type: assertion failed: 
> (size >= type->instance_size)
> Bail out! ERROR:../qom/object.c:511:object_initialize_with_type: 
> assertion failed: (size >= type->instance_size)
> 
> I think the problem is that we define s->spi as SSIBus* instead of 
> SSIBus. But if we define it as SSIBus, we'll
> get an incomplete type error. Fixing it will require refactoring 
> hw/ssi/ssi.c which I'm not sure if we want to do
> it right now. This code is consistent with other code in hw/ssi so I 
> guess we can leave it here for now and wait
> for a future refactor.
Sorry, I was just mumbling alone thinking about what would need to be
done, not asking you to change it yet :/ I should have been
more explicit.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 347936e41c..1e2a711373 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -803,9 +803,9 @@  M: Tyrone Ting <kfting@nuvoton.com>
 M: Hao Wu <wuhaotsh@google.com>
 L: qemu-arm@nongnu.org
 S: Supported
-F: hw/*/npcm7xx*
-F: include/hw/*/npcm7xx*
-F: tests/qtest/npcm7xx*
+F: hw/*/npcm*
+F: include/hw/*/npcm*
+F: tests/qtest/npcm*
 F: pc-bios/npcm7xx_bootrom.bin
 F: roms/vbootrom
 F: docs/system/arm/nuvoton.rst
diff --git a/hw/ssi/meson.build b/hw/ssi/meson.build
index 702aa5e4df..904a47161a 100644
--- a/hw/ssi/meson.build
+++ b/hw/ssi/meson.build
@@ -1,6 +1,6 @@ 
 softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_smc.c'))
 softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('mss-spi.c'))
-softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_fiu.c'))
+softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_fiu.c', 'npcm_pspi.c'))
 softmmu_ss.add(when: 'CONFIG_PL022', if_true: files('pl022.c'))
 softmmu_ss.add(when: 'CONFIG_SIFIVE_SPI', if_true: files('sifive_spi.c'))
 softmmu_ss.add(when: 'CONFIG_SSI', if_true: files('ssi.c'))
diff --git a/hw/ssi/npcm_pspi.c b/hw/ssi/npcm_pspi.c
new file mode 100644
index 0000000000..565bba5282
--- /dev/null
+++ b/hw/ssi/npcm_pspi.c
@@ -0,0 +1,216 @@ 
+/*
+ * Nuvoton NPCM Peripheral SPI Module (PSPI)
+ *
+ * Copyright 2023 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+
+#include "hw/irq.h"
+#include "hw/registerfields.h"
+#include "hw/ssi/npcm_pspi.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+
+#include "trace.h"
+
+REG16(PSPI_DATA, 0x0)
+REG16(PSPI_CTL1, 0x2)
+    FIELD(PSPI_CTL1, SPIEN, 0,  1)
+    FIELD(PSPI_CTL1, MOD,   2,  1)
+    FIELD(PSPI_CTL1, EIR,   5,  1)
+    FIELD(PSPI_CTL1, EIW,   6,  1)
+    FIELD(PSPI_CTL1, SCM,   7,  1)
+    FIELD(PSPI_CTL1, SCIDL, 8,  1)
+    FIELD(PSPI_CTL1, SCDV,  9,  7)
+REG16(PSPI_STAT, 0x4)
+    FIELD(PSPI_STAT, BSY,  0,  1)
+    FIELD(PSPI_STAT, RBF,  1,  1)
+
+static void npcm_pspi_update_irq(NPCMPSPIState *s)
+{
+    int level = 0;
+
+    /* Only fire IRQ when the module is enabled. */
+    if (FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, SPIEN)) {
+        /* Update interrupt as BSY is cleared. */
+        if ((!FIELD_EX16(s->regs[R_PSPI_STAT], PSPI_STAT, BSY)) &&
+            FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, EIW)) {
+            level = 1;
+        }
+
+        /* Update interrupt as RBF is set. */
+        if (FIELD_EX16(s->regs[R_PSPI_STAT], PSPI_STAT, RBF) &&
+            FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, EIR)) {
+            level = 1;
+        }
+    }
+    qemu_set_irq(s->irq, level);
+}
+
+static uint16_t npcm_pspi_read_data(NPCMPSPIState *s)
+{
+    uint16_t value = s->regs[R_PSPI_DATA];
+
+    /* Clear stat bits as the value are read out. */
+    s->regs[R_PSPI_STAT] = 0;
+
+    return value;
+}
+
+static void npcm_pspi_write_data(NPCMPSPIState *s, uint16_t data)
+{
+    uint16_t value = 0;
+
+    if (FIELD_EX16(s->regs[R_PSPI_CTL1], PSPI_CTL1, MOD)) {
+        value = ssi_transfer(s->spi, extract16(data, 8, 8)) << 8;
+    }
+    value |= ssi_transfer(s->spi, extract16(data, 0, 8));
+    s->regs[R_PSPI_DATA] = value;
+
+    /* Mark data as available */
+    s->regs[R_PSPI_STAT] = R_PSPI_STAT_BSY_MASK | R_PSPI_STAT_RBF_MASK;
+}
+
+/* Control register read handler. */
+static uint64_t npcm_pspi_ctrl_read(void *opaque, hwaddr addr,
+                                    unsigned int size)
+{
+    NPCMPSPIState *s = opaque;
+    uint16_t value;
+
+    switch (addr) {
+    case A_PSPI_DATA:
+        value = npcm_pspi_read_data(s);
+        break;
+
+    case A_PSPI_CTL1:
+        value = s->regs[R_PSPI_CTL1];
+        break;
+
+    case A_PSPI_STAT:
+        value = s->regs[R_PSPI_STAT];
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: write to invalid offset 0x%" PRIx64 "\n",
+                      DEVICE(s)->canonical_path, addr);
+        return 0;
+    }
+    trace_npcm_pspi_ctrl_read(DEVICE(s)->canonical_path, addr, value);
+    npcm_pspi_update_irq(s);
+
+    return value;
+}
+
+/* Control register write handler. */
+static void npcm_pspi_ctrl_write(void *opaque, hwaddr addr, uint64_t v,
+                                 unsigned int size)
+{
+    NPCMPSPIState *s = opaque;
+    uint16_t value = v;
+
+    trace_npcm_pspi_ctrl_write(DEVICE(s)->canonical_path, addr, value);
+
+    switch (addr) {
+    case A_PSPI_DATA:
+        npcm_pspi_write_data(s, value);
+        break;
+
+    case A_PSPI_CTL1:
+        s->regs[R_PSPI_CTL1] = value;
+        break;
+
+    case A_PSPI_STAT:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: write to read-only register PSPI_STAT: 0x%08"
+                      PRIx64 "\n", DEVICE(s)->canonical_path, v);
+        break;
+
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: write to invalid offset 0x%" PRIx64 "\n",
+                      DEVICE(s)->canonical_path, addr);
+        return;
+    }
+    npcm_pspi_update_irq(s);
+}
+
+static const MemoryRegionOps npcm_pspi_ctrl_ops = {
+    .read = npcm_pspi_ctrl_read,
+    .write = npcm_pspi_ctrl_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 2,
+        .unaligned = false,
+    },
+};
+
+static void npcm_pspi_enter_reset(Object *obj, ResetType type)
+{
+    NPCMPSPIState *s = NPCM_PSPI(obj);
+
+    trace_npcm_pspi_enter_reset(DEVICE(obj)->canonical_path, type);
+    memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void npcm_pspi_realize(DeviceState *dev, Error **errp)
+{
+    NPCMPSPIState *s = NPCM_PSPI(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Object *obj = OBJECT(dev);
+
+    s->spi = ssi_create_bus(dev, "pspi");
+    memory_region_init_io(&s->mmio, obj, &npcm_pspi_ctrl_ops, s,
+                          "mmio", 4 * KiB);
+    sysbus_init_mmio(sbd, &s->mmio);
+    sysbus_init_irq(sbd, &s->irq);
+}
+
+static const VMStateDescription vmstate_npcm_pspi = {
+    .name = "npcm-pspi",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT16_ARRAY(regs, NPCMPSPIState, NPCM_PSPI_NR_REGS),
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+
+static void npcm_pspi_class_init(ObjectClass *klass, void *data)
+{
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "NPCM Peripheral SPI Module";
+    dc->realize = npcm_pspi_realize;
+    dc->vmsd = &vmstate_npcm_pspi;
+    rc->phases.enter = npcm_pspi_enter_reset;
+}
+
+static const TypeInfo npcm_pspi_types[] = {
+    {
+        .name = TYPE_NPCM_PSPI,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(NPCMPSPIState),
+        .class_init = npcm_pspi_class_init,
+    },
+};
+DEFINE_TYPES(npcm_pspi_types);
diff --git a/hw/ssi/trace-events b/hw/ssi/trace-events
index c707d4aaba..16ea9954c4 100644
--- a/hw/ssi/trace-events
+++ b/hw/ssi/trace-events
@@ -21,6 +21,11 @@  npcm7xx_fiu_ctrl_write(const char *id, uint64_t addr, uint32_t data) "%s offset:
 npcm7xx_fiu_flash_read(const char *id, int cs, uint64_t addr, unsigned int size, uint64_t value) "%s[%d] offset: 0x%08" PRIx64 " size: %u value: 0x%" PRIx64
 npcm7xx_fiu_flash_write(const char *id, unsigned cs, uint64_t addr, unsigned int size, uint64_t value) "%s[%d] offset: 0x%08" PRIx64 " size: %u value: 0x%" PRIx64
 
+# npcm_pspi.c
+npcm_pspi_enter_reset(const char *id, int reset_type) "%s reset type: %d"
+npcm_pspi_ctrl_read(const char *id, uint64_t addr, uint16_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
+npcm_pspi_ctrl_write(const char *id, uint64_t addr, uint16_t data) "%s offset: 0x%04" PRIx64 " value: 0x%08" PRIx16
+
 # ibex_spi_host.c
 
 ibex_spi_host_reset(const char *msg) "%s"
diff --git a/include/hw/ssi/npcm_pspi.h b/include/hw/ssi/npcm_pspi.h
new file mode 100644
index 0000000000..37cc784d96
--- /dev/null
+++ b/include/hw/ssi/npcm_pspi.h
@@ -0,0 +1,53 @@ 
+/*
+ * Nuvoton Peripheral SPI Module
+ *
+ * Copyright 2023 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+#ifndef NPCM_PSPI_H
+#define NPCM_PSPI_H
+
+#include "hw/ssi/ssi.h"
+#include "hw/sysbus.h"
+
+/*
+ * Number of registers in our device state structure. Don't change this without
+ * incrementing the version_id in the vmstate.
+ */
+#define NPCM_PSPI_NR_REGS 3
+
+/**
+ * NPCMPSPIState - Device state for one Flash Interface Unit.
+ * @parent: System bus device.
+ * @mmio: Memory region for register access.
+ * @spi: The SPI bus mastered by this controller.
+ * @regs: Register contents.
+ * @irq: The interrupt request queue for this module.
+ *
+ * Each PSPI has a shared bank of registers, and controls up to four chip
+ * selects. Each chip select has a dedicated memory region which may be used to
+ * read and write the flash connected to that chip select as if it were memory.
+ */
+typedef struct NPCMPSPIState {
+    SysBusDevice parent;
+
+    MemoryRegion mmio;
+
+    SSIBus *spi;
+    uint16_t regs[NPCM_PSPI_NR_REGS];
+    qemu_irq irq;
+} NPCMPSPIState;
+
+#define TYPE_NPCM_PSPI "npcm-pspi"
+OBJECT_DECLARE_SIMPLE_TYPE(NPCMPSPIState, NPCM_PSPI)
+
+#endif /* NPCM_PSPI_H */