Re: [PATCH v1 0/5] hw/ppc: SPI model

[Cédric Le Goater]

Chalapathi,

On 3/1/24 17:17, Chalapathi V wrote:
> Hello,
>
> I would greatly appreciate the review comments/suggestions on PATCH V1.
>
> Thank You and Regards,

I didn't forget but I lacked the time in this release cycle. Sorry
about that.

I have one quick comment though. There are already a few models
implementing SPI controllers in QEMU. When I skimmed through the
patches, I was surprised to see no use of the available SSI
framework. Doesn't the current framework fit your needs ?

Please take a look at files in : include/hw/ssi/* hw/ssi/*

Same comment for the Serial EEPROM model. This device is generic
and not POWER specific. It should be possible to attach the device
model on other machines and different SPI bus provided by QEMU.
This doesn't mean rewriting everything, but the "HW" interface
probably needs to be reworked. It would make it easier to write
unit test (see ests/qtest/) and ease the review also.

Thanks,

C.



> Chalapathi
>
> On 07-02-2024 21:38, Chalapathi V wrote:
> > Hello,
> >
> > In this series of patchset, SPI controller and responder models
> > for Power10 processor are modelled.
> >
> > Serial peripheral interface provides full-duplex synchronous serial
> > communication between single controller and multiple responder devices.
> >
> > The current configuration supports a single SPI controller reside on the
> > SPI bus. In p10, SPI controller device model supports a connection to a
> > single SPI responder such as SPI seeproms, TPM, flash device and an ADC
> > controller.
> >
> > SPI controller model is divided into configuration unit, sequencer FSM
> > and shifter engine. All SPI function control is mapped into the SPI register
> > space to enable full control by firmware.
> >
> > SPI configuration component is modelled which contains all SPI configuration
> > and status registers as well as the hold registers for data to be sent or
> > having been received.
> > Shift engine performs serialization and de-serialization according to the
> > control by the sequencer and according to the setup defined in the
> > configuration registers.
> > Sequencer implements the main control logic and
> > FSM to handle data transmit and data receive control of the shift engine.
> >
> > Microchip's 25CSM04 SEEPROM device is modelled and connected to SPI bus
> > "spi_bus2" of SPI controller "PIB_SPIC[2]".
> >
> > Patches overview in V1.
> > PATCH1: Create a SPI responder model which includes responder methods
> >          and SPI bus implementation.
> > PATCH2: Create a SPI controller model and implement configuration unit
> >          to model SCOM registers.
> > PATCH3: SPI controller model: implement sequencer FSM and shift engine.
> > PATCH4: create SPI SEEPROM model.
> > PATCH5: Connect SPI controllers to p10 chip and create keystore seeprom
> >          device on spi_bus2 of PIB_SPIC[2].
> >
> > Thank You,
> > Chalapathi
> >
> > Chalapathi V (5):
> >    hw/ppc: SPI responder model
> >    hw/ppc: SPI controller model - registers implementation
> >    hw/ppc: SPI controller model - sequencer and shifter
> >    hw/ppc: SPI SEEPROM model
> >    hw/ppc: SPI controller wiring to P10 chip and create seeprom device
> >
> >   include/hw/ppc/pnv_chip.h           |    4 +
> >   include/hw/ppc/pnv_spi_controller.h |  101 ++
> >   include/hw/ppc/pnv_spi_responder.h  |  109 ++
> >   include/hw/ppc/pnv_spi_seeprom.h    |   70 ++
> >   include/hw/ppc/pnv_xscom.h          |    3 +
> >   hw/ppc/pnv.c                        |   32 +
> >   hw/ppc/pnv_spi_controller.c         | 1609 +++++++++++++++++++++++++++
> >   hw/ppc/pnv_spi_responder.c          |  166 +++
> >   hw/ppc/pnv_spi_seeprom.c            |  989 ++++++++++++++++
> >   hw/ppc/meson.build                  |    3 +
> >   10 files changed, 3086 insertions(+)
> >   create mode 100644 include/hw/ppc/pnv_spi_controller.h
> >   create mode 100644 include/hw/ppc/pnv_spi_responder.h
> >   create mode 100644 include/hw/ppc/pnv_spi_seeprom.h
> >   create mode 100644 hw/ppc/pnv_spi_controller.c
> >   create mode 100644 hw/ppc/pnv_spi_responder.c
> >   create mode 100644 hw/ppc/pnv_spi_seeprom.c