Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
On Fri, Apr 19, 2019 at 09:56:23AM +0200, 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. > > Thomas Huth (2): > hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in > ohci_die() > hw/usb/hcd-ohci: Move PCI-related code into a separate file Added to usb queue. thanks, Gerd
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
On 4/26/19 2:20 PM, Thomas Huth wrote:
> On 26/04/2019 14.14, Philippe Mathieu-Daudé wrote:
>> 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);
>
> Could work, too, but I don't like typedeferities... so unless Gerd
> forces me to use that here, I'd prefer to keep the patch in its current
> shape.
Fine with me ;)
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
On 26/04/2019 14.14, Philippe Mathieu-Daudé wrote:
> 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);
Could work, too, but I don't like typedeferities... so unless Gerd
forces me to use that here, I'd prefer to keep the patch in its current
shape.
Thomas
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
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é
>> Tested-by: Philippe Mathieu-Daudé
>
> Thanks!
> Thomas
>
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
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.
> Anyway to this series:
> Reviewed-by: Philippe Mathieu-Daudé
> Tested-by: Philippe Mathieu-Daudé
Thanks!
Thomas
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
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. Anyway to this series: Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé > > Thomas Huth (2): > hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in > ohci_die() > hw/usb/hcd-ohci: Move PCI-related code into a separate file > > hw/sh4/Kconfig| 2 +- > hw/usb/Kconfig| 6 +- > hw/usb/Makefile.objs | 1 + > hw/usb/hcd-ohci-pci.c | 163 +++ > hw/usb/hcd-ohci.c | 219 -- > hw/usb/hcd-ohci.h | 104 > 6 files changed, 293 insertions(+), 202 deletions(-) > create mode 100644 hw/usb/hcd-ohci-pci.c > create mode 100644 hw/usb/hcd-ohci.h >
