Re: [PATCH 3/5] usb/ohci: Move USBPortOps related functions together

[Philippe Mathieu-Daudé]

On 16/1/22 14:20, BALATON Zoltan wrote:
> This also allows removing two forward declarations
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/usb/hcd-ohci.c | 204 +++++++++++++++++++++++-----------------------
>   1 file changed, 100 insertions(+), 104 deletions(-)

> +static const MemoryRegionOps ohci_mem_ops = {
> +    .read = ohci_mem_read,
> +    .write = ohci_mem_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +/* USBPortOps */
> +static void ohci_attach(USBPort *port1)
> +{
> +    OHCIState *s = port1->opaque;
> +    OHCIPort *port = &s->rhport[port1->index];
> +    uint32_t old_state = port->ctrl;
> +
> +    /* set connect status */
> +    port->ctrl |= OHCI_PORT_CCS | OHCI_PORT_CSC;
> +
> +    /* update speed */
> +    if (port->port.dev->speed == USB_SPEED_LOW) {
> +        port->ctrl |= OHCI_PORT_LSDA;
> +    } else {
> +        port->ctrl &= ~OHCI_PORT_LSDA;
> +    }
> +
> +    /* notify of remote-wakeup */
> +    if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
> +        ohci_set_interrupt(s, OHCI_INTR_RD);
> +    }
> +
> +    trace_usb_ohci_port_attach(port1->index);
> +
> +    if (old_state != port->ctrl) {
> +        ohci_set_interrupt(s, OHCI_INTR_RHSC);
> +    }
> +}
> +
>   static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev)
>   {
>       if (ohci->async_td &&
> @@ -1799,11 +1735,71 @@ static void ohci_async_cancel_device(OHCIState *ohci, 
> USBDevice *dev)
>       }
>   }

We could have ohci_attach() just before ohci*_detach(),
anyhow:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> -static const MemoryRegionOps ohci_mem_ops = {
> -    .read = ohci_mem_read,
> -    .write = ohci_mem_write,
> -    .endianness = DEVICE_LITTLE_ENDIAN,
> -};
> +static void ohci_child_detach(USBPort *port1, USBDevice *child)
> +{
> +    OHCIState *s = port1->opaque;
> +
> +    ohci_async_cancel_device(s, child);
> +}
> +
> +static void ohci_detach(USBPort *port1)
> +{
> +    OHCIState *s = port1->opaque;
> +    OHCIPort *port = &s->rhport[port1->index];
> +    uint32_t old_state = port->ctrl;
> +
> +    ohci_async_cancel_device(s, port1->dev);
> +
> +    /* set connect status */
> +    if (port->ctrl & OHCI_PORT_CCS) {
> +        port->ctrl &= ~OHCI_PORT_CCS;
> +        port->ctrl |= OHCI_PORT_CSC;
> +    }
> +    /* disable port */
> +    if (port->ctrl & OHCI_PORT_PES) {
> +        port->ctrl &= ~OHCI_PORT_PES;
> +        port->ctrl |= OHCI_PORT_PESC;
> +    }
> +    trace_usb_ohci_port_detach(port1->index);
> +
> +    if (old_state != port->ctrl) {
> +        ohci_set_interrupt(s, OHCI_INTR_RHSC);
> +    }
> +}