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 <bala...@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;
// ConnectStatusChange
if (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é <phi...@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).
