Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file

[Philippe Mathieu-Daudé]

On 4/26/19 7:42 AM, Thomas Huth wrote:
> On 26/04/2019 00.55, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 4/19/19 9:56 AM, Thomas Huth wrote:
>>> First patch fixes a problem with ohci_die(), second patch moves PCI code 
>>> into
>>> a separate file, so that the sysbus OHCI device can also be used without
>>> the dependency on the PCI code.
>>>
>>> v2: Split the patch into two patches, one for the ohci_die() fix and one
>>>     for the PCI code movement.
>>
>> Way cleaner. I wonder why you don't use a typedef for the void
>> (*ohci_die_fn)(struct OHCIState *) prototype.
> 
> It does not work in that case:
> 
> typedef struct OHCIState {            // <-- struct OHCIState definition
>     [...]
>     uint32_t async_td;
>     bool async_complete;
> 
>     void (*ohci_die)(struct OHCIState *ohci); // <-- ohci_die definition
> } OHCIState;                         // <-- typedef OHCIState definition
> 
> The typedef is defined after the ohci_die entry.

I was thinking of forward declaration:

typedef struct OHCIState OHCIState;

typedef void (ohci_die_fn)(OHCIState *ohci);

struct OHCIState {
    [...]
    uint32_t async_td;
    bool async_complete;

    ohci_die_fn *ohci_die;
} OHCIState;

static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                          uint32_t num_ports,
                          dma_addr_t localmem_base,
                          char *masterbus, uint32_t firstport,
                          AddressSpace *as,
                          ohci_die_fn *ohci_die, Error **errp)
{ ...

> 
>> Anyway to this series:
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
> 
>  Thanks!
>   Thomas
>