Re: [PATCH v5 00/10] Introduce model for IBM's FSI

[Cédric Le Goater]

Hello Ninad,


On 10/11/23 17:13, Ninad Palsule wrote:
> Hello,
>
> Please review the patch-set version 5.
> I have incorporated review comments from Cedric.
>
> Ninad Palsule (10):
>    hw/fsi: Introduce IBM's Local bus
>    hw/fsi: Introduce IBM's scratchpad
>    hw/fsi: Introduce IBM's cfam,fsi-slave
>    hw/fsi: Introduce IBM's FSI
>    hw/fsi: IBM's On-chip Peripheral Bus
>    hw/fsi: Aspeed APB2OPB interface
>    hw/arm: Hook up FSI module in AST2600
>    hw/fsi: Added qtest
>    hw/fsi: Added FSI documentation
>    hw/fsi: Update MAINTAINER list

I made some comments, mostly on the bus models which need to be reworked.
This is code reshuffling and it should simplify the models. Please consider
reducing the amount of files.

Also, could please remove my Sob ?  I didn't write any of this, only did
quick fixes over the years because I kept the series in my aspeed tree.
And this would help me to add a Rb :)

I expect one or two respins before merge. 8.2 freeze window is in less
than 3 weeks. It could make it, else the next. No big issue since I think
we have been dragging these models for at least 5/6 years.

Thanks,

C.


>   MAINTAINERS                        |   8 +
>   docs/specs/fsi.rst                 | 141 ++++++++++++
>   meson.build                        |   1 +
>   hw/fsi/trace.h                     |   1 +
>   include/hw/arm/aspeed_soc.h        |   4 +
>   include/hw/fsi/aspeed-apb2opb.h    |  33 +++
>   include/hw/fsi/cfam.h              |  58 +++++
>   include/hw/fsi/engine-scratchpad.h |  33 +++
>   include/hw/fsi/fsi-master.h        |  30 +++
>   include/hw/fsi/fsi-slave.h         |  29 +++
>   include/hw/fsi/fsi.h               |  37 +++
>   include/hw/fsi/lbus.h              |  51 +++++
>   include/hw/fsi/opb.h               |  43 ++++
>   include/qemu/bitops.h              |   6 +
>   hw/arm/aspeed_ast2600.c            |  19 ++
>   hw/fsi/aspeed-apb2opb.c            | 350 +++++++++++++++++++++++++++++
>   hw/fsi/cfam.c                      | 220 ++++++++++++++++++
>   hw/fsi/engine-scratchpad.c         |  99 ++++++++
>   hw/fsi/fsi-master.c                | 198 ++++++++++++++++
>   hw/fsi/fsi-slave.c                 |  96 ++++++++
>   hw/fsi/fsi.c                       |  54 +++++
>   hw/fsi/lbus.c                      |  87 +++++++
>   hw/fsi/opb.c                       | 185 +++++++++++++++
>   tests/qtest/fsi-test.c             | 210 +++++++++++++++++
>   hw/Kconfig                         |   1 +
>   hw/arm/Kconfig                     |   1 +
>   hw/fsi/Kconfig                     |  23 ++
>   hw/fsi/meson.build                 |   6 +
>   hw/fsi/trace-events                |  15 ++
>   hw/meson.build                     |   1 +
>   tests/qtest/meson.build            |   2 +
>   31 files changed, 2042 insertions(+)
>   create mode 100644 docs/specs/fsi.rst
>   create mode 100644 hw/fsi/trace.h
>   create mode 100644 include/hw/fsi/aspeed-apb2opb.h
>   create mode 100644 include/hw/fsi/cfam.h
>   create mode 100644 include/hw/fsi/engine-scratchpad.h
>   create mode 100644 include/hw/fsi/fsi-master.h
>   create mode 100644 include/hw/fsi/fsi-slave.h
>   create mode 100644 include/hw/fsi/fsi.h
>   create mode 100644 include/hw/fsi/lbus.h
>   create mode 100644 include/hw/fsi/opb.h
>   create mode 100644 hw/fsi/aspeed-apb2opb.c
>   create mode 100644 hw/fsi/cfam.c
>   create mode 100644 hw/fsi/engine-scratchpad.c
>   create mode 100644 hw/fsi/fsi-master.c
>   create mode 100644 hw/fsi/fsi-slave.c
>   create mode 100644 hw/fsi/fsi.c
>   create mode 100644 hw/fsi/lbus.c
>   create mode 100644 hw/fsi/opb.c
>   create mode 100644 tests/qtest/fsi-test.c
>   create mode 100644 hw/fsi/Kconfig
>   create mode 100644 hw/fsi/meson.build
>   create mode 100644 hw/fsi/trace-events