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
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
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 >
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
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 >
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
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 ;)
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
© 2016 - 2024 Red Hat, Inc.