Re: [Qemu-devel] [PATCH 3/7] tests/qgraph: sdhci driver and interface nodes

[Emanuele]

Hi Philippe,

On 07/11/2018 10:13 PM, Philippe Mathieu-Daudé wrote:
> Hi Emanuele,
>
> On 07/09/2018 06:11 AM, Emanuele Giuseppe Esposito wrote:
> > Add qgraph nodes for sdhci-pci and generic-sdhci (memory mapped) drivers.
> > Both drivers implement (produce) the same interface sdhci, that provides the
> > readw - readq - writeq functions.
> >
> > Signed-off-by: Emanuele Giuseppe Esposito <address@hidden>
> > ---
> >   tests/Makefile.include |   1 +
> >   tests/libqos/sdhci.c   | 142 +++++++++++++++++++++++++++++++++++++++++
> >   tests/libqos/sdhci.h   |  68 ++++++++++++++++++++
> >   3 files changed, 211 insertions(+)
> >   create mode 100644 tests/libqos/sdhci.c
> >   create mode 100644 tests/libqos/sdhci.h
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index b16bbd55df..acbf704a8a 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -770,6 +770,7 @@ libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) 
> > tests/libqos/usb.o
> >   libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) 
> > tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o 
> > tests/libqos/malloc-generic.o
> >   
> >   libqgraph-obj-y = tests/libqos/qgraph.o
> > +libqgraph-pc-obj-y = $(libqos-pc-obj-y) tests/libqos/sdhci.o
> Shouldn't this be:
>
>     libqgraph-obj-y = tests/libqos/qgraph.o tests/libqos/sdhci.o
>
> (not PC related)
I need to add $(libqos-pc-obj-y) because it will include pc-pci.c and 
all the pci targerts, loading their libqos_init() functions and building 
the graph.

Doing as you suggested won't work for these reasons:
- pci-bus-pc and pci-bus nodes won't be created, so graph would be 
incomplete
- sdhci needs libqos to use global_qtest,  qpci_device_init, etc. that 
won't be provided by check-unit-y even by adding $(libqos-pc-obj-y).

So since test-qgraph do not need libqos, I (suggested by Paolo) added 
the test to the check-unit-y target, adding to it just libqgraph-obj.

For other devices like sdhci that need libqos, I created 
libqgraph-pc-obj-y (to be renamed to libqgraph-pci-obj-y, since it will 
also contain libqos-spapr ?) that will be added to target 
check-qtest-pci-y (that provides libqos).
> > +    QSDHCIProperties props;
> > +};
> > +
> > +/* Memory Mapped implementation of QSDHCI */
> > +struct QSDHCI_MemoryMapped {
> > +    QOSGraphObject obj;
> > +    QSDHCI sdhci;
> > +    uint64_t addr;
> > +};
> > +
> > +/* PCI implementation of QSDHCI */
> > +struct QSDHCI_PCI {
> > +    QOSGraphObject obj;
> > +    QPCIDevice dev;
> > +    QSDHCI sdhci;
> > +    QPCIBar mem_bar;
> > +};
> > +
> > +/**
> > + * qos_create_sdhci_mm(): external constructor used by all drivers/machines
> > + * that "contain" a #QSDHCI_MemoryMapped driver
> > + */
> > +void qos_create_sdhci_mm(QSDHCI_MemoryMapped *sdhci, uint32_t addr,
> > +                            QSDHCIProperties *common);
> Your IDE uses a weird indentation.
I actually did by hand, not sure how the indentation to split >80 lines 
should be.
Maybe I should remove a tab? Like this:

+void qos_create_sdhci_mm(QSDHCI_MemoryMapped *sdhci, uint32_t addr,
+                                            QSDHCIProperties *common);