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

[Emanuele <e.emanuelegiuseppe@gmail.com>]

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 <e.emanuelegiuseppe@gmail.com>
>> ---
>>   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);