|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [PATCH v2 6/7] usb/ohci: Implement resume on connection status change |
Date: | Tue, 28 Feb 2023 16:50:50 +0100 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 |
On 28/2/23 15:55, BALATON Zoltan wrote:
On Tue, 28 Feb 2023, Philippe Mathieu-Daudé wrote:On 20/2/23 19:19, BALATON Zoltan wrote:If certain bit is set remote wake up should change state from suspended to resume and generate interrupt. There was a todo comment for this, implement that by moving existing resume logic to a function and call that. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/usb/hcd-ohci.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index bad8db7b1d..88bd42b14a 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c@@ -1410,6 +1410,18 @@ static void ohci_set_hub_status(OHCIState *ohci, uint32_t val)} }+/* This is the one state transition the controller can do by itself */+static int ohci_resume(OHCIState *s)Preferably returning bool.I can change that on rebase. I just followed other exising functions in this file for consistency which return int (although some use 1 and others use -1 besides 0).
I'll squash that myself.
+{ + if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) { + trace_usb_ohci_remote_wakeup(s->name); + s->ctl &= ~OHCI_CTL_HCFS; + s->ctl |= OHCI_USB_RESUME; + return 1; + } + return 0; +} + /** Sets a flag in a port status reg but only set it if the port is connected.* If not set ConnectStatusChange flag. If flag is enabled return 1.@@ -1426,7 +1438,10 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) { ohci->rhport[i].ctrl |= OHCI_PORT_CSC;// ConnectStatusChangeif (ohci->rhstatus & OHCI_RHS_DRWE) {// DeviceRemoteWakeupEnable: ConnectStatusChange is a remote wakeup event.Not clear if you want any change here or the comments are just explanation in this email.
I was just taking notes while reviewing ;) Our OHCI definitions aren't very verbose.
- /* TODO: CSC is a wakeup event */ + /* CSC is a wakeup event */ + if (ohci_resume(ohci)) { + ohci_set_interrupt(ohci, OHCI_INTR_RD);Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>Thanks for the review. You put a lot of work in QEMU and we appreciate very much that you're also doing the job of other maintainers.
No problem. I'm queuing this patch for my next PR (hopefully much less patches, and before the freeze).
[Prev in Thread] | Current Thread | [Next in Thread] |