1. Patchew
  2. QEMU
  3. View series

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

Thomas Huth posted 2 patches 6 years, 9 months ago
Test asan passed
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190419075625.24251-1-thuth@redhat.com
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>
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
[Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
Posted by Thomas Huth 6 years, 9 months ago 
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

 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

-- 
2.21.0
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
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é <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> 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
>
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
Posted by Thomas Huth 6 years, 9 months ago 
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é <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

 Thanks!
  Thomas
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
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é <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
>  Thanks!
>   Thomas
>
Re: [Qemu-devel] [PATCH v2 for-4.1 0/2] Fix ohci_die() and move PCI code to separate file
Posted by Thomas Huth 6 years, 9 months ago 
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
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago 
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
Posted by Gerd Hoffmann 6 years, 9 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.