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

[Ninad Palsule]

Hello Cedric,

On 10/19/23 03:16, Cédric Le Goater wrote:
> 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.
I have incorporated most of your comments but couldn't reduce the files 
as there is a file per bus or device.
> 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 :)
Removed your name from signed off by.
> 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.

I am hoping to get it in 8.2

Thanks for the review.

Regards,

Ninad

> 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