* [PATCH v2 0/7] OHCI changes
@ 2023-02-20 18:15 BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 1/7] usb/ohci: Code style fix comments BALATON Zoltan
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
Trying to debug some MacOS versions on mac99 with OHCI I've added some
debug logging that might be useful to get traces from this device and
attempted to implement missing feature. To avoid checkpatch errors
start with updating code style for th ewhole file.
v2: Address review commnts, add R-b tags and a patch from Philippe to
fix a typo
Regards,
BALATON Zoltan
BALATON Zoltan (7):
usb/ohci: Code style fix comments
usb/ohci: Code style fix white space errors
usb/ohci: Code style fix missing braces and extra parenthesis
usb/ohci: Move a function next to where it is used
usb/ohci: Add trace points for register access
usb/ohci: Implement resume on connection status change
hw/usb/hcd-ohci: Fix typo
hw/usb/hcd-ohci.c | 461 ++++++++++++++++++++++++--------------------
hw/usb/trace-events | 4 +
2 files changed, 259 insertions(+), 206 deletions(-)
--
2.30.7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/7] usb/ohci: Code style fix comments
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 2/7] usb/ohci: Code style fix white space errors BALATON Zoltan
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 99 +++++++++++++++++++++++------------------------
1 file changed, 49 insertions(+), 50 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 9d68036d23..9f138e7f25 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -23,7 +23,7 @@
* o Disable timers when nothing needs to be done, or remove timer usage
* all together.
* o BIOS work to boot from USB storage
-*/
+ */
#include "qemu/osdep.h"
#include "hw/irq.h"
@@ -39,7 +39,7 @@
#include "hcd-ohci.h"
/* This causes frames to occur 1000x slower */
-//#define OHCI_TIME_WARP 1
+/*#define OHCI_TIME_WARP 1*/
#define ED_LINK_LIMIT 32
@@ -58,7 +58,7 @@ struct ohci_hcca {
#define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
#define ED_WBACK_SIZE 4
-/* Bitfields for the first word of an Endpoint Desciptor. */
+/* Bitfields for the first word of an Endpoint Desciptor. */
#define OHCI_ED_FA_SHIFT 0
#define OHCI_ED_FA_MASK (0x7f<<OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
@@ -71,11 +71,11 @@ struct ohci_hcca {
#define OHCI_ED_MPS_SHIFT 16
#define OHCI_ED_MPS_MASK (0x7ff<<OHCI_ED_MPS_SHIFT)
-/* Flags in the head field of an Endpoint Desciptor. */
+/* Flags in the head field of an Endpoint Desciptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
-/* Bitfields for the first word of a Transfer Desciptor. */
+/* Bitfields for the first word of a Transfer Desciptor. */
#define OHCI_TD_R (1<<18)
#define OHCI_TD_DP_SHIFT 19
#define OHCI_TD_DP_MASK (3<<OHCI_TD_DP_SHIFT)
@@ -88,7 +88,7 @@ struct ohci_hcca {
#define OHCI_TD_CC_SHIFT 28
#define OHCI_TD_CC_MASK (0xf<<OHCI_TD_CC_SHIFT)
-/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
+/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
/* CC & DI - same as in the General Transfer Desciptor */
#define OHCI_TD_SF_SHIFT 0
#define OHCI_TD_SF_MASK (0xffff<<OHCI_TD_SF_SHIFT)
@@ -335,8 +335,8 @@ static void ohci_soft_reset(OHCIState *ohci)
ohci->per_cur = 0;
ohci->done = 0;
ohci->done_count = 7;
-
- /* FSMPS is marked TBD in OCHI 1.0, what gives ffs?
+ /*
+ * FSMPS is marked TBD in OCHI 1.0, what gives ffs?
* I took the value linux sets ...
*/
ohci->fsmps = 0x2778;
@@ -460,10 +460,10 @@ static inline int ohci_read_hcca(OHCIState *ohci,
static inline int ohci_put_ed(OHCIState *ohci,
dma_addr_t addr, struct ohci_ed *ed)
{
- /* ed->tail is under control of the HCD.
+ /*
+ * ed->tail is under control of the HCD.
* Since just ed->head is changed by HC, just write back this
*/
-
return put_dwords(ohci, addr + ED_WBACK_OFFSET,
(uint32_t *)((char *)ed + ED_WBACK_OFFSET),
ED_WBACK_SIZE >> 2);
@@ -601,8 +601,10 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
trace_usb_ohci_iso_td_relative_frame_number_neg(relative_frame_number);
return 1;
} else if (relative_frame_number > frame_count) {
- /* ISO TD expired - retire the TD to the Done Queue and continue with
- the next ISO TD of the same ED */
+ /*
+ * ISO TD expired - retire the TD to the Done Queue and continue with
+ * the next ISO TD of the same ED
+ */
trace_usb_ohci_iso_td_relative_frame_number_big(relative_frame_number,
frame_count);
if (OHCI_CC_DATAOVERRUN == OHCI_BM(iso_td.flags, TD_CC)) {
@@ -845,9 +847,10 @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
}
}
-/* Service a transport descriptor.
- Returns nonzero to terminate processing of this endpoint. */
-
+/*
+ * Service a transport descriptor.
+ * Returns nonzero to terminate processing of this endpoint.
+ */
static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
{
int dir;
@@ -869,7 +872,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
return 1;
}
- /* See if this TD has already been submitted to the device. */
+ /* See if this TD has already been submitted to the device. */
completion = (addr == ohci->async_td);
if (completion && !ohci->async_complete) {
trace_usb_ohci_td_skip_async();
@@ -885,7 +888,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
switch (dir) {
case OHCI_TD_DIR_OUT:
case OHCI_TD_DIR_IN:
- /* Same value. */
+ /* Same value. */
break;
default:
dir = OHCI_BM(td.flags, TD_DP);
@@ -956,11 +959,12 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
}
ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
if (ohci->async_td) {
- /* ??? The hardware should allow one active packet per
- endpoint. We only allow one active packet per controller.
- This should be sufficient as long as devices respond in a
- timely manner.
- */
+ /*
+ * ??? The hardware should allow one active packet per
+ * endpoint. We only allow one active packet per controller.
+ * This should be sufficient as long as devices respond in a
+ * timely manner.
+ */
trace_usb_ohci_td_too_many_pending(ep->nr);
return 1;
}
@@ -996,7 +1000,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
/* Writeback */
if (ret == pktlen || (dir == OHCI_TD_DIR_IN && ret >= 0 && flag_r)) {
- /* Transmission succeeded. */
+ /* Transmission succeeded. */
if (ret == len) {
td.cbp = 0;
} else {
@@ -1048,8 +1052,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
OHCI_SET_BM(td.flags, TD_EC, 3);
break;
}
- /* An error occurred so we have to clear the interrupt counter. See
- * spec at 6.4.4 on page 104 */
+ /*
+ * An error occurred so we have to clear the interrupt counter.
+ * See spec at 6.4.4 on page 104
+ */
ohci->done_count = 0;
}
ed->head |= OHCI_ED_H;
@@ -1071,7 +1077,7 @@ exit_no_retire:
return OHCI_BM(td.flags, TD_CC) != OHCI_CC_NOERROR;
}
-/* Service an endpoint list. Returns nonzero if active TD were found. */
+/* Service an endpoint list. Returns nonzero if active TD were found. */
static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
{
struct ohci_ed ed;
@@ -1095,7 +1101,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) {
uint32_t addr;
- /* Cancel pending packets for ED that have been paused. */
+ /* Cancel pending packets for ED that have been paused. */
addr = ed.head & OHCI_DPTR_MASK;
if (ohci->async_td && addr == ohci->async_td) {
usb_cancel_packet(&ohci->usb_packet);
@@ -1151,7 +1157,7 @@ static void ohci_sof(OHCIState *ohci)
ohci_set_interrupt(ohci, OHCI_INTR_SF);
}
-/* Process Control and Bulk lists. */
+/* Process Control and Bulk lists. */
static void ohci_process_lists(OHCIState *ohci)
{
if ((ohci->ctl & OHCI_CTL_CLE) && (ohci->status & OHCI_STATUS_CLF)) {
@@ -1192,7 +1198,7 @@ static void ohci_frame_boundary(void *opaque)
ohci_service_ed_list(ohci, le32_to_cpu(hcca.intr[n]));
}
- /* Cancel all pending packets if either of the lists has been disabled. */
+ /* Cancel all pending packets if either of the lists has been disabled. */
if (ohci->old_ctl & (~ohci->ctl) & (OHCI_CTL_BLE | OHCI_CTL_CLE)) {
ohci_stop_endpoints(ohci);
}
@@ -1234,18 +1240,17 @@ static void ohci_frame_boundary(void *opaque)
}
}
-/* Start sending SOF tokens across the USB bus, lists are processed in
+/*
+ * Start sending SOF tokens across the USB bus, lists are processed in
* next frame
*/
static int ohci_bus_start(OHCIState *ohci)
{
trace_usb_ohci_start(ohci->name);
-
- /* Delay the first SOF event by one frame time as
- * linux driver is not ready to receive it and
- * can meet some race conditions
+ /*
+ * Delay the first SOF event by one frame time as linux driver is
+ * not ready to receive it and can meet some race conditions
*/
-
ohci->sof_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
ohci_eof_timer(ohci);
@@ -1259,9 +1264,9 @@ void ohci_bus_stop(OHCIState *ohci)
timer_del(ohci->eof_timer);
}
-/* Sets a flag in a port status register but only set it if the port is
- * connected, if not set ConnectStatusChange flag. If flag is enabled
- * return 1.
+/*
+ * 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.
*/
static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
{
@@ -1271,9 +1276,7 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
if (val == 0)
return 0;
- /* If CurrentConnectStatus is cleared we set
- * ConnectStatusChange
- */
+ /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
if (ohci->rhstatus & OHCI_RHS_DRWE) {
@@ -1291,7 +1294,7 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
return ret;
}
-/* Set the frame interval - frame interval toggle is manipulated by the hcd only */
+/* Frame interval toggle is manipulated by the hcd only */
static void ohci_set_frame_interval(OHCIState *ohci, uint16_t val)
{
val &= OHCI_FMI_FI;
@@ -1357,9 +1360,7 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL)
return (ohci->frt << 31);
- /* Being in USB operational state guarnatees sof_time was
- * set already.
- */
+ /* Being in USB operational state guarnatees sof_time was set already. */
tks = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ohci->sof_time;
if (tks < 0) {
tks = 0;
@@ -1439,13 +1440,11 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
trace_usb_ohci_port_reset(portnum);
usb_device_reset(port->port.dev);
port->ctrl &= ~OHCI_PORT_PRS;
- /* ??? Should this also set OHCI_PORT_PESC. */
+ /* ??? Should this also set OHCI_PORT_PESC. */
port->ctrl |= OHCI_PORT_PES | OHCI_PORT_PRSC;
}
- /* Invert order here to ensure in ambiguous case, device is
- * powered up...
- */
+ /* Invert order here to ensure in ambiguous case, device is powered up. */
if (val & OHCI_PORT_LSDA)
ohci_port_power(ohci, portnum, 0);
if (val & OHCI_PORT_PPS)
@@ -1892,7 +1891,7 @@ void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
ohci_frame_boundary, ohci);
}
-/**
+/*
* A typical OHCI will stop operating and set itself into error state
* (which can be queried by MMIO) to signal that it got an error.
*/
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/7] usb/ohci: Code style fix white space errors
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 1/7] usb/ohci: Code style fix comments BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis BALATON Zoltan
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 160 +++++++++++++++++++++++-----------------------
1 file changed, 79 insertions(+), 81 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 9f138e7f25..b8f8fd048d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -60,46 +60,46 @@ struct ohci_hcca {
/* Bitfields for the first word of an Endpoint Desciptor. */
#define OHCI_ED_FA_SHIFT 0
-#define OHCI_ED_FA_MASK (0x7f<<OHCI_ED_FA_SHIFT)
+#define OHCI_ED_FA_MASK (0x7f << OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
-#define OHCI_ED_EN_MASK (0xf<<OHCI_ED_EN_SHIFT)
+#define OHCI_ED_EN_MASK (0xf << OHCI_ED_EN_SHIFT)
#define OHCI_ED_D_SHIFT 11
-#define OHCI_ED_D_MASK (3<<OHCI_ED_D_SHIFT)
-#define OHCI_ED_S (1<<13)
-#define OHCI_ED_K (1<<14)
-#define OHCI_ED_F (1<<15)
+#define OHCI_ED_D_MASK (3 << OHCI_ED_D_SHIFT)
+#define OHCI_ED_S (1 << 13)
+#define OHCI_ED_K (1 << 14)
+#define OHCI_ED_F (1 << 15)
#define OHCI_ED_MPS_SHIFT 16
-#define OHCI_ED_MPS_MASK (0x7ff<<OHCI_ED_MPS_SHIFT)
+#define OHCI_ED_MPS_MASK (0x7ff << OHCI_ED_MPS_SHIFT)
/* Flags in the head field of an Endpoint Desciptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
/* Bitfields for the first word of a Transfer Desciptor. */
-#define OHCI_TD_R (1<<18)
+#define OHCI_TD_R (1 << 18)
#define OHCI_TD_DP_SHIFT 19
-#define OHCI_TD_DP_MASK (3<<OHCI_TD_DP_SHIFT)
+#define OHCI_TD_DP_MASK (3 << OHCI_TD_DP_SHIFT)
#define OHCI_TD_DI_SHIFT 21
-#define OHCI_TD_DI_MASK (7<<OHCI_TD_DI_SHIFT)
-#define OHCI_TD_T0 (1<<24)
-#define OHCI_TD_T1 (1<<25)
+#define OHCI_TD_DI_MASK (7 << OHCI_TD_DI_SHIFT)
+#define OHCI_TD_T0 (1 << 24)
+#define OHCI_TD_T1 (1 << 25)
#define OHCI_TD_EC_SHIFT 26
-#define OHCI_TD_EC_MASK (3<<OHCI_TD_EC_SHIFT)
+#define OHCI_TD_EC_MASK (3 << OHCI_TD_EC_SHIFT)
#define OHCI_TD_CC_SHIFT 28
-#define OHCI_TD_CC_MASK (0xf<<OHCI_TD_CC_SHIFT)
+#define OHCI_TD_CC_MASK (0xf << OHCI_TD_CC_SHIFT)
/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
/* CC & DI - same as in the General Transfer Desciptor */
#define OHCI_TD_SF_SHIFT 0
-#define OHCI_TD_SF_MASK (0xffff<<OHCI_TD_SF_SHIFT)
+#define OHCI_TD_SF_MASK (0xffff << OHCI_TD_SF_SHIFT)
#define OHCI_TD_FC_SHIFT 24
-#define OHCI_TD_FC_MASK (7<<OHCI_TD_FC_SHIFT)
+#define OHCI_TD_FC_MASK (7 << OHCI_TD_FC_SHIFT)
/* Isochronous Transfer Desciptor - Offset / PacketStatusWord */
#define OHCI_TD_PSW_CC_SHIFT 12
-#define OHCI_TD_PSW_CC_MASK (0xf<<OHCI_TD_PSW_CC_SHIFT)
+#define OHCI_TD_PSW_CC_MASK (0xf << OHCI_TD_PSW_CC_SHIFT)
#define OHCI_TD_PSW_SIZE_SHIFT 0
-#define OHCI_TD_PSW_SIZE_MASK (0xfff<<OHCI_TD_PSW_SIZE_SHIFT)
+#define OHCI_TD_PSW_SIZE_MASK (0xfff << OHCI_TD_PSW_SIZE_SHIFT)
#define OHCI_PAGE_MASK 0xfffff000
#define OHCI_OFFSET_MASK 0xfff
@@ -112,7 +112,7 @@ struct ohci_hcca {
#define OHCI_SET_BM(val, field, newval) do { \
val &= ~OHCI_##field##_MASK; \
val |= ((newval) << OHCI_##field##_SHIFT) & OHCI_##field##_MASK; \
- } while(0)
+ } while (0)
/* endpoint descriptor */
struct ohci_ed {
@@ -142,35 +142,35 @@ struct ohci_iso_td {
#define USB_HZ 12000000
/* OHCI Local stuff */
-#define OHCI_CTL_CBSR ((1<<0)|(1<<1))
-#define OHCI_CTL_PLE (1<<2)
-#define OHCI_CTL_IE (1<<3)
-#define OHCI_CTL_CLE (1<<4)
-#define OHCI_CTL_BLE (1<<5)
-#define OHCI_CTL_HCFS ((1<<6)|(1<<7))
+#define OHCI_CTL_CBSR ((1 << 0) | (1 << 1))
+#define OHCI_CTL_PLE (1 << 2)
+#define OHCI_CTL_IE (1 << 3)
+#define OHCI_CTL_CLE (1 << 4)
+#define OHCI_CTL_BLE (1 << 5)
+#define OHCI_CTL_HCFS ((1 << 6) | (1 << 7))
#define OHCI_USB_RESET 0x00
#define OHCI_USB_RESUME 0x40
#define OHCI_USB_OPERATIONAL 0x80
#define OHCI_USB_SUSPEND 0xc0
-#define OHCI_CTL_IR (1<<8)
-#define OHCI_CTL_RWC (1<<9)
-#define OHCI_CTL_RWE (1<<10)
-
-#define OHCI_STATUS_HCR (1<<0)
-#define OHCI_STATUS_CLF (1<<1)
-#define OHCI_STATUS_BLF (1<<2)
-#define OHCI_STATUS_OCR (1<<3)
-#define OHCI_STATUS_SOC ((1<<6)|(1<<7))
-
-#define OHCI_INTR_SO (1U<<0) /* Scheduling overrun */
-#define OHCI_INTR_WD (1U<<1) /* HcDoneHead writeback */
-#define OHCI_INTR_SF (1U<<2) /* Start of frame */
-#define OHCI_INTR_RD (1U<<3) /* Resume detect */
-#define OHCI_INTR_UE (1U<<4) /* Unrecoverable error */
-#define OHCI_INTR_FNO (1U<<5) /* Frame number overflow */
-#define OHCI_INTR_RHSC (1U<<6) /* Root hub status change */
-#define OHCI_INTR_OC (1U<<30) /* Ownership change */
-#define OHCI_INTR_MIE (1U<<31) /* Master Interrupt Enable */
+#define OHCI_CTL_IR (1 << 8)
+#define OHCI_CTL_RWC (1 << 9)
+#define OHCI_CTL_RWE (1 << 10)
+
+#define OHCI_STATUS_HCR (1 << 0)
+#define OHCI_STATUS_CLF (1 << 1)
+#define OHCI_STATUS_BLF (1 << 2)
+#define OHCI_STATUS_OCR (1 << 3)
+#define OHCI_STATUS_SOC ((1 << 6) | (1 << 7))
+
+#define OHCI_INTR_SO (1U << 0) /* Scheduling overrun */
+#define OHCI_INTR_WD (1U << 1) /* HcDoneHead writeback */
+#define OHCI_INTR_SF (1U << 2) /* Start of frame */
+#define OHCI_INTR_RD (1U << 3) /* Resume detect */
+#define OHCI_INTR_UE (1U << 4) /* Unrecoverable error */
+#define OHCI_INTR_FNO (1U << 5) /* Frame number overflow */
+#define OHCI_INTR_RHSC (1U << 6) /* Root hub status change */
+#define OHCI_INTR_OC (1U << 30) /* Ownership change */
+#define OHCI_INTR_MIE (1U << 31) /* Master Interrupt Enable */
#define OHCI_HCCA_SIZE 0x100
#define OHCI_HCCA_MASK 0xffffff00
@@ -181,40 +181,40 @@ struct ohci_iso_td {
#define OHCI_FMI_FSMPS 0xffff0000
#define OHCI_FMI_FIT 0x80000000
-#define OHCI_FR_RT (1U<<31)
+#define OHCI_FR_RT (1U << 31)
#define OHCI_LS_THRESH 0x628
#define OHCI_RHA_RW_MASK 0x00000000 /* Mask of supported features. */
-#define OHCI_RHA_PSM (1<<8)
-#define OHCI_RHA_NPS (1<<9)
-#define OHCI_RHA_DT (1<<10)
-#define OHCI_RHA_OCPM (1<<11)
-#define OHCI_RHA_NOCP (1<<12)
+#define OHCI_RHA_PSM (1 << 8)
+#define OHCI_RHA_NPS (1 << 9)
+#define OHCI_RHA_DT (1 << 10)
+#define OHCI_RHA_OCPM (1 << 11)
+#define OHCI_RHA_NOCP (1 << 12)
#define OHCI_RHA_POTPGT_MASK 0xff000000
-#define OHCI_RHS_LPS (1U<<0)
-#define OHCI_RHS_OCI (1U<<1)
-#define OHCI_RHS_DRWE (1U<<15)
-#define OHCI_RHS_LPSC (1U<<16)
-#define OHCI_RHS_OCIC (1U<<17)
-#define OHCI_RHS_CRWE (1U<<31)
-
-#define OHCI_PORT_CCS (1<<0)
-#define OHCI_PORT_PES (1<<1)
-#define OHCI_PORT_PSS (1<<2)
-#define OHCI_PORT_POCI (1<<3)
-#define OHCI_PORT_PRS (1<<4)
-#define OHCI_PORT_PPS (1<<8)
-#define OHCI_PORT_LSDA (1<<9)
-#define OHCI_PORT_CSC (1<<16)
-#define OHCI_PORT_PESC (1<<17)
-#define OHCI_PORT_PSSC (1<<18)
-#define OHCI_PORT_OCIC (1<<19)
-#define OHCI_PORT_PRSC (1<<20)
-#define OHCI_PORT_WTC (OHCI_PORT_CSC|OHCI_PORT_PESC|OHCI_PORT_PSSC \
- |OHCI_PORT_OCIC|OHCI_PORT_PRSC)
-
+#define OHCI_RHS_LPS (1U << 0)
+#define OHCI_RHS_OCI (1U << 1)
+#define OHCI_RHS_DRWE (1U << 15)
+#define OHCI_RHS_LPSC (1U << 16)
+#define OHCI_RHS_OCIC (1U << 17)
+#define OHCI_RHS_CRWE (1U << 31)
+
+#define OHCI_PORT_CCS (1 << 0)
+#define OHCI_PORT_PES (1 << 1)
+#define OHCI_PORT_PSS (1 << 2)
+#define OHCI_PORT_POCI (1 << 3)
+#define OHCI_PORT_PRS (1 << 4)
+#define OHCI_PORT_PPS (1 << 8)
+#define OHCI_PORT_LSDA (1 << 9)
+#define OHCI_PORT_CSC (1 << 16)
+#define OHCI_PORT_PESC (1 << 17)
+#define OHCI_PORT_PSSC (1 << 18)
+#define OHCI_PORT_OCIC (1 << 19)
+#define OHCI_PORT_PRSC (1 << 20)
+#define OHCI_PORT_WTC (OHCI_PORT_CSC | OHCI_PORT_PESC | \
+ OHCI_PORT_PSSC | OHCI_PORT_OCIC | \
+ OHCI_PORT_PRSC)
#define OHCI_TD_DIR_SETUP 0x0
#define OHCI_TD_DIR_OUT 0x1
#define OHCI_TD_DIR_IN 0x2
@@ -584,7 +584,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
starting_frame = OHCI_BM(iso_td.flags, TD_SF);
frame_count = OHCI_BM(iso_td.flags, TD_FC);
- relative_frame_number = USUB(ohci->frame_number, starting_frame);
+ relative_frame_number = USUB(ohci->frame_number, starting_frame);
trace_usb_ohci_iso_td_head(
ed->head & OHCI_DPTR_MASK, ed->tail & OHCI_DPTR_MASK,
@@ -657,8 +657,8 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
next_offset = iso_td.be;
}
- if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) ||
- ((relative_frame_number < frame_count) &&
+ if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) ||
+ ((relative_frame_number < frame_count) &&
!(OHCI_BM(next_offset, TD_PSW_CC) & 0xe))) {
trace_usb_ohci_iso_td_bad_cc_not_accessed(start_offset, next_offset);
return 1;
@@ -1118,7 +1118,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
ed.tail & OHCI_DPTR_MASK, ed.next & OHCI_DPTR_MASK);
trace_usb_ohci_ed_pkt_flags(
OHCI_BM(ed.flags, ED_FA), OHCI_BM(ed.flags, ED_EN),
- OHCI_BM(ed.flags, ED_D), (ed.flags & OHCI_ED_S)!= 0,
+ OHCI_BM(ed.flags, ED_D), (ed.flags & OHCI_ED_S) != 0,
(ed.flags & OHCI_ED_K) != 0, (ed.flags & OHCI_ED_F) != 0,
OHCI_BM(ed.flags, ED_MPS));
@@ -1311,10 +1311,8 @@ static void ohci_port_power(OHCIState *ohci, int i, int p)
if (p) {
ohci->rhport[i].ctrl |= OHCI_PORT_PPS;
} else {
- ohci->rhport[i].ctrl &= ~(OHCI_PORT_PPS|
- OHCI_PORT_CCS|
- OHCI_PORT_PSS|
- OHCI_PORT_PRS);
+ ohci->rhport[i].ctrl &= ~(OHCI_PORT_PPS | OHCI_PORT_CCS |
+ OHCI_PORT_PSS | OHCI_PORT_PRS);
}
}
@@ -1858,7 +1856,7 @@ void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
ohci->num_ports = num_ports;
if (masterbus) {
USBPort *ports[OHCI_MAX_PORTS];
- for(i = 0; i < num_ports; i++) {
+ for (i = 0; i < num_ports; i++) {
ports[i] = &ohci->rhport[i].port;
}
usb_register_companion(masterbus, ports, num_ports,
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 1/7] usb/ohci: Code style fix comments BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 2/7] usb/ohci: Code style fix white space errors BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 4/7] usb/ohci: Move a function next to where it is used BALATON Zoltan
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 106 ++++++++++++++++++++++++++--------------------
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index b8f8fd048d..a2cdba4058 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -499,9 +499,9 @@ static int ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
ptr = td->cbp;
n = 0x1000 - (ptr & 0xfff);
- if (n > len)
+ if (n > len) {
n = len;
-
+ }
if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
n, dir, MEMTXATTRS_UNSPECIFIED)) {
return -1;
@@ -527,9 +527,9 @@ static int ohci_copy_iso_td(OHCIState *ohci,
ptr = start_addr;
n = 0x1000 - (ptr & 0xfff);
- if (n > len)
+ if (n > len) {
n = len;
-
+ }
if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
n, dir, MEMTXATTRS_UNSPECIFIED)) {
return -1;
@@ -617,8 +617,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
iso_td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(iso_td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
if (ohci_put_iso_td(ohci, addr, &iso_td)) {
ohci_die(ohci);
return 1;
@@ -803,8 +804,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
iso_td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(iso_td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
}
if (ohci_put_iso_td(ohci, addr, &iso_td)) {
ohci_die(ohci);
@@ -1022,8 +1024,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
/* Setting ED_C is part of the TD retirement process */
ed->head &= ~OHCI_ED_C;
- if (td.flags & OHCI_TD_T0)
+ if (td.flags & OHCI_TD_T0) {
ed->head |= OHCI_ED_C;
+ }
} else {
if (ret >= 0) {
trace_usb_ohci_td_underrun();
@@ -1067,8 +1070,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
exit_no_retire:
if (ohci_put_td(ohci, addr, &td)) {
ohci_die(ohci);
@@ -1087,9 +1091,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
uint32_t link_cnt = 0;
active = 0;
- if (head == 0)
+ if (head == 0) {
return 0;
-
+ }
for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) {
if (ohci_read_ed(ohci, cur, &ed)) {
trace_usb_ohci_ed_read_error(cur);
@@ -1125,8 +1129,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
active = 1;
if ((ed.flags & OHCI_ED_F) == 0) {
- if (ohci_service_td(ohci, &ed))
+ if (ohci_service_td(ohci, &ed)) {
break;
+ }
} else {
/* Handle isochronous endpoints */
if (ohci_service_iso_td(ohci, &ed)) {
@@ -1218,19 +1223,21 @@ static void ohci_frame_boundary(void *opaque)
hcca.frame = cpu_to_le16(ohci->frame_number);
if (ohci->done_count == 0 && !(ohci->intr_status & OHCI_INTR_WD)) {
- if (!ohci->done)
+ if (!ohci->done) {
abort();
- if (ohci->intr & ohci->intr_status)
+ }
+ if (ohci->intr & ohci->intr_status) {
ohci->done |= 1;
+ }
hcca.done = cpu_to_le32(ohci->done);
ohci->done = 0;
ohci->done_count = 7;
ohci_set_interrupt(ohci, OHCI_INTR_WD);
}
- if (ohci->done_count != 7 && ohci->done_count != 0)
+ if (ohci->done_count != 7 && ohci->done_count != 0) {
ohci->done_count--;
-
+ }
/* Do SOF stuff here */
ohci_sof(ohci);
@@ -1273,9 +1280,9 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
int ret = 1;
/* writing a 0 has no effect */
- if (val == 0)
+ if (val == 0) {
return 0;
-
+ }
/* If CurrentConnectStatus is cleared we set ConnectStatusChange */
if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
@@ -1285,9 +1292,9 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
return 0;
}
- if (ohci->rhport[i].ctrl & val)
+ if (ohci->rhport[i].ctrl & val) {
ret = 0;
-
+ }
/* set the bit */
ohci->rhport[i].ctrl |= val;
@@ -1327,9 +1334,9 @@ static void ohci_set_ctl(OHCIState *ohci, uint32_t val)
new_state = ohci->ctl & OHCI_CTL_HCFS;
/* no state change */
- if (old_state == new_state)
+ if (old_state == new_state) {
return;
-
+ }
trace_usb_ohci_set_ctl(ohci->name, new_state);
switch (new_state) {
case OHCI_USB_OPERATIONAL:
@@ -1355,9 +1362,9 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
uint16_t fr;
int64_t tks;
- if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL)
- return (ohci->frt << 31);
-
+ if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL) {
+ return ohci->frt << 31;
+ }
/* Being in USB operational state guarnatees sof_time was set already. */
tks = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ohci->sof_time;
if (tks < 0) {
@@ -1365,9 +1372,9 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
}
/* avoid muldiv if possible */
- if (tks >= usb_frame_time)
- return (ohci->frt << 31);
-
+ if (tks >= usb_frame_time) {
+ return ohci->frt << 31;
+ }
tks = tks / usb_bit_time;
fr = (uint16_t)(ohci->fi - tks);
@@ -1383,33 +1390,36 @@ static void ohci_set_hub_status(OHCIState *ohci, uint32_t val)
old_state = ohci->rhstatus;
/* write 1 to clear OCIC */
- if (val & OHCI_RHS_OCIC)
+ if (val & OHCI_RHS_OCIC) {
ohci->rhstatus &= ~OHCI_RHS_OCIC;
-
+ }
if (val & OHCI_RHS_LPS) {
int i;
- for (i = 0; i < ohci->num_ports; i++)
+ for (i = 0; i < ohci->num_ports; i++) {
ohci_port_power(ohci, i, 0);
+ }
trace_usb_ohci_hub_power_down();
}
if (val & OHCI_RHS_LPSC) {
int i;
- for (i = 0; i < ohci->num_ports; i++)
+ for (i = 0; i < ohci->num_ports; i++) {
ohci_port_power(ohci, i, 1);
+ }
trace_usb_ohci_hub_power_up();
}
- if (val & OHCI_RHS_DRWE)
+ if (val & OHCI_RHS_DRWE) {
ohci->rhstatus |= OHCI_RHS_DRWE;
-
- if (val & OHCI_RHS_CRWE)
+ }
+ if (val & OHCI_RHS_CRWE) {
ohci->rhstatus &= ~OHCI_RHS_DRWE;
-
- if (old_state != ohci->rhstatus)
+ }
+ if (old_state != ohci->rhstatus) {
ohci_set_interrupt(ohci, OHCI_INTR_RHSC);
+ }
}
/* Set root hub port status */
@@ -1422,12 +1432,12 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
old_state = port->ctrl;
/* Write to clear CSC, PESC, PSSC, OCIC, PRSC */
- if (val & OHCI_PORT_WTC)
+ if (val & OHCI_PORT_WTC) {
port->ctrl &= ~(val & OHCI_PORT_WTC);
-
- if (val & OHCI_PORT_CCS)
+ }
+ if (val & OHCI_PORT_CCS) {
port->ctrl &= ~OHCI_PORT_PES;
-
+ }
ohci_port_set_if_connected(ohci, portnum, val & OHCI_PORT_PES);
if (ohci_port_set_if_connected(ohci, portnum, val & OHCI_PORT_PSS)) {
@@ -1443,13 +1453,15 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
}
/* Invert order here to ensure in ambiguous case, device is powered up. */
- if (val & OHCI_PORT_LSDA)
+ if (val & OHCI_PORT_LSDA) {
ohci_port_power(ohci, portnum, 0);
- if (val & OHCI_PORT_PPS)
+ }
+ if (val & OHCI_PORT_PPS) {
ohci_port_power(ohci, portnum, 1);
-
- if (old_state != port->ctrl)
+ }
+ if (old_state != port->ctrl) {
ohci_set_interrupt(ohci, OHCI_INTR_RHSC);
+ }
}
static uint64_t ohci_mem_read(void *opaque,
@@ -1606,8 +1618,9 @@ static void ohci_mem_write(void *opaque,
/* Bits written as '0' remain unchanged in the register */
ohci->status |= val;
- if (ohci->status & OHCI_STATUS_HCR)
+ if (ohci->status & OHCI_STATUS_HCR) {
ohci_soft_reset(ohci);
+ }
break;
case 3: /* HcInterruptStatus */
@@ -1685,8 +1698,9 @@ static void ohci_mem_write(void *opaque,
case 25: /* HcHReset */
ohci->hreset = val & ~OHCI_HRESET_FSBIR;
- if (val & OHCI_HRESET_FSBIR)
+ if (val & OHCI_HRESET_FSBIR) {
ohci_hard_reset(ohci);
+ }
break;
case 26: /* HcHInterruptEnable */
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/7] usb/ohci: Move a function next to where it is used
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
` (2 preceding siblings ...)
2023-02-20 18:15 ` [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 5/7] usb/ohci: Add trace points for register access BALATON Zoltan
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
The ohci_port_set_if_connected() function is only used by
ohci_port_set_status(), move next to it to have them at the same place.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 60 +++++++++++++++++++++++------------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index a2cdba4058..52fcfcd4ab 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1271,36 +1271,6 @@ void ohci_bus_stop(OHCIState *ohci)
timer_del(ohci->eof_timer);
}
-/*
- * 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.
- */
-static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
-{
- int ret = 1;
-
- /* writing a 0 has no effect */
- if (val == 0) {
- return 0;
- }
- /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
- if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
- ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
- if (ohci->rhstatus & OHCI_RHS_DRWE) {
- /* TODO: CSC is a wakeup event */
- }
- return 0;
- }
-
- if (ohci->rhport[i].ctrl & val) {
- ret = 0;
- }
- /* set the bit */
- ohci->rhport[i].ctrl |= val;
-
- return ret;
-}
-
/* Frame interval toggle is manipulated by the hcd only */
static void ohci_set_frame_interval(OHCIState *ohci, uint16_t val)
{
@@ -1422,6 +1392,36 @@ static void ohci_set_hub_status(OHCIState *ohci, uint32_t val)
}
}
+/*
+ * 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.
+ */
+static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
+{
+ int ret = 1;
+
+ /* writing a 0 has no effect */
+ if (val == 0) {
+ return 0;
+ }
+ /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
+ if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
+ ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
+ if (ohci->rhstatus & OHCI_RHS_DRWE) {
+ /* TODO: CSC is a wakeup event */
+ }
+ return 0;
+ }
+
+ if (ohci->rhport[i].ctrl & val) {
+ ret = 0;
+ }
+ /* set the bit */
+ ohci->rhport[i].ctrl |= val;
+
+ return ret;
+}
+
/* Set root hub port status */
static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
{
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] usb/ohci: Add trace points for register access
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
` (3 preceding siblings ...)
2023-02-20 18:15 ` [PATCH v2 4/7] usb/ohci: Move a function next to where it is used BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 6/7] usb/ohci: Implement resume on connection status change BALATON Zoltan
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
To help debugging add trace points that print values read from or
written to the device's registers.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/usb/hcd-ohci.c | 27 +++++++++++++++++++++++++++
hw/usb/trace-events | 4 ++++
2 files changed, 31 insertions(+)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 52fcfcd4ab..bad8db7b1d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -235,6 +235,24 @@ struct ohci_iso_td {
#define OHCI_HRESET_FSBIR (1 << 0)
+static const char *ohci_reg_names[] = {
+ "HcRevision", "HcControl", "HcCommandStatus", "HcInterruptStatus",
+ "HcInterruptEnable", "HcInterruptDisable", "HcHCCA", "HcPeriodCurrentED",
+ "HcControlHeadED", "HcControlCurrentED", "HcBulkHeadED", "HcBulkCurrentED",
+ "HcDoneHead", "HcFmInterval", "HcFmRemaining", "HcFmNumber",
+ "HcPeriodicStart", "HcLSThreshold", "HcRhDescriptorA", "HcRhDescriptorB",
+ "HcRhStatus"
+};
+
+static const char *ohci_reg_name(hwaddr addr)
+{
+ if (addr >> 2 < ARRAY_SIZE(ohci_reg_names)) {
+ return ohci_reg_names[addr >> 2];
+ } else {
+ return "<unknown>";
+ }
+}
+
static void ohci_die(OHCIState *ohci)
{
ohci->ohci_die(ohci);
@@ -1478,6 +1496,8 @@ static uint64_t ohci_mem_read(void *opaque,
} else if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
/* HcRhPortStatus */
retval = ohci->rhport[(addr - 0x54) >> 2].ctrl | OHCI_PORT_PPS;
+ trace_usb_ohci_mem_port_read(size, "HcRhPortStatus", (addr - 0x50) >> 2,
+ addr, addr >> 2, retval);
} else {
switch (addr >> 2) {
case 0: /* HcRevision */
@@ -1582,6 +1602,10 @@ static uint64_t ohci_mem_read(void *opaque,
trace_usb_ohci_mem_read_bad_offset(addr);
retval = 0xffffffff;
}
+ if (addr != 0xc || retval) {
+ trace_usb_ohci_mem_read(size, ohci_reg_name(addr), addr, addr >> 2,
+ retval);
+ }
}
return retval;
@@ -1602,10 +1626,13 @@ static void ohci_mem_write(void *opaque,
if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
/* HcRhPortStatus */
+ trace_usb_ohci_mem_port_write(size, "HcRhPortStatus", (addr - 0x50) >> 2,
+ addr, addr >> 2, val);
ohci_port_set_status(ohci, (addr - 0x54) >> 2, val);
return;
}
+ trace_usb_ohci_mem_write(size, ohci_reg_name(addr), addr, addr >> 2, val);
switch (addr >> 2) {
case 1: /* HcControl */
ohci_set_ctl(ohci, val);
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index b65269892c..6bb9655c8d 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -57,8 +57,12 @@ usb_ohci_ed_read_error(uint32_t addr) "ED read error at 0x%x"
usb_ohci_ed_pkt(uint32_t cur, int h, int c, uint32_t head, uint32_t tail, uint32_t next) "ED @ 0x%.8x h=%u c=%u\n head=0x%.8x tailp=0x%.8x next=0x%.8x"
usb_ohci_ed_pkt_flags(uint32_t fa, uint32_t en, uint32_t d, int s, int k, int f, uint32_t mps) "fa=%u en=%u d=%u s=%u k=%u f=%u mps=%u"
usb_ohci_hcca_read_error(uint32_t addr) "HCCA read error at 0x%x"
+usb_ohci_mem_read(uint32_t size, const char *name, uint32_t addr, uint32_t offs, uint32_t val) "%d %s 0x%x %d -> 0x%x"
+usb_ohci_mem_port_read(uint32_t size, const char *name, uint32_t port, uint32_t addr, uint32_t offs, uint32_t val) "%d %s[%d] 0x%x %d -> 0x%x"
usb_ohci_mem_read_unaligned(uint32_t addr) "at 0x%x"
usb_ohci_mem_read_bad_offset(uint32_t addr) "0x%x"
+usb_ohci_mem_write(uint32_t size, const char *name, uint32_t addr, uint32_t offs, uint32_t val) "%d %s 0x%x %d <- 0x%x"
+usb_ohci_mem_port_write(uint32_t size, const char *name, uint32_t port, uint32_t addr, uint32_t offs, uint32_t val) "%d %s[%d] 0x%x %d <- 0x%x"
usb_ohci_mem_write_unaligned(uint32_t addr) "at 0x%x"
usb_ohci_mem_write_bad_offset(uint32_t addr) "0x%x"
usb_ohci_process_lists(uint32_t head, uint32_t cur) "head 0x%x, cur 0x%x"
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
` (4 preceding siblings ...)
2023-02-20 18:15 ` [PATCH v2 5/7] usb/ohci: Add trace points for register access BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-28 13:54 ` Philippe Mathieu-Daudé
2023-02-20 18:15 ` [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo BALATON Zoltan
2023-02-20 18:19 ` [PATCH v2 RESEND 0/7] OHCI changes BALATON Zoltan
7 siblings, 2 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
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)
+{
+ 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;
if (ohci->rhstatus & OHCI_RHS_DRWE) {
- /* TODO: CSC is a wakeup event */
+ /* CSC is a wakeup event */
+ if (ohci_resume(ohci)) {
+ ohci_set_interrupt(ohci, OHCI_INTR_RD);
+ }
}
return 0;
}
@@ -1828,11 +1843,7 @@ static void ohci_wakeup(USBPort *port1)
intr = OHCI_INTR_RHSC;
}
/* Note that the controller can be suspended even if this port is not */
- if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
- trace_usb_ohci_remote_wakeup(s->name);
- /* This is the one state transition the controller can do by itself */
- s->ctl &= ~OHCI_CTL_HCFS;
- s->ctl |= OHCI_USB_RESUME;
+ if (ohci_resume(s)) {
/*
* In suspend mode only ResumeDetected is possible, not RHSC:
* see the OHCI spec 5.1.2.3.
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
` (5 preceding siblings ...)
2023-02-20 18:15 ` [PATCH v2 6/7] usb/ohci: Implement resume on connection status change BALATON Zoltan
@ 2023-02-20 18:15 ` BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:19 ` [PATCH v2 RESEND 0/7] OHCI changes BALATON Zoltan
7 siblings, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[balaton: rebased on clean up series]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/usb/hcd-ohci.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 88bd42b14a..98315b2301 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -58,7 +58,7 @@ struct ohci_hcca {
#define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
#define ED_WBACK_SIZE 4
-/* Bitfields for the first word of an Endpoint Desciptor. */
+/* Bitfields for the first word of an Endpoint Descriptor. */
#define OHCI_ED_FA_SHIFT 0
#define OHCI_ED_FA_MASK (0x7f << OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
@@ -71,11 +71,11 @@ struct ohci_hcca {
#define OHCI_ED_MPS_SHIFT 16
#define OHCI_ED_MPS_MASK (0x7ff << OHCI_ED_MPS_SHIFT)
-/* Flags in the head field of an Endpoint Desciptor. */
+/* Flags in the head field of an Endpoint Descriptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
-/* Bitfields for the first word of a Transfer Desciptor. */
+/* Bitfields for the first word of a Transfer Descriptor. */
#define OHCI_TD_R (1 << 18)
#define OHCI_TD_DP_SHIFT 19
#define OHCI_TD_DP_MASK (3 << OHCI_TD_DP_SHIFT)
@@ -88,14 +88,14 @@ struct ohci_hcca {
#define OHCI_TD_CC_SHIFT 28
#define OHCI_TD_CC_MASK (0xf << OHCI_TD_CC_SHIFT)
-/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
-/* CC & DI - same as in the General Transfer Desciptor */
+/* Bitfields for the first word of an Isochronous Transfer Descriptor. */
+/* CC & DI - same as in the General Transfer Descriptor */
#define OHCI_TD_SF_SHIFT 0
#define OHCI_TD_SF_MASK (0xffff << OHCI_TD_SF_SHIFT)
#define OHCI_TD_FC_SHIFT 24
#define OHCI_TD_FC_MASK (7 << OHCI_TD_FC_SHIFT)
-/* Isochronous Transfer Desciptor - Offset / PacketStatusWord */
+/* Isochronous Transfer Descriptor - Offset / PacketStatusWord */
#define OHCI_TD_PSW_CC_SHIFT 12
#define OHCI_TD_PSW_CC_MASK (0xf << OHCI_TD_PSW_CC_SHIFT)
#define OHCI_TD_PSW_SIZE_SHIFT 0
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 RESEND 0/7] OHCI changes
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
` (6 preceding siblings ...)
2023-02-20 18:15 ` [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
7 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
Resend after fixing email address in cc.
Trying to debug some MacOS versions on mac99 with OHCI I've added some
debug logging that might be useful to get traces from this device and
attempted to implement missing feature. To avoid checkpatch errors
start with updating code style for th ewhole file.
v2: Address review commnts, add R-b tags and a patch from Philippe to
fix a typo
Regards,
BALATON Zoltan
BALATON Zoltan (7):
usb/ohci: Code style fix comments
usb/ohci: Code style fix white space errors
usb/ohci: Code style fix missing braces and extra parenthesis
usb/ohci: Move a function next to where it is used
usb/ohci: Add trace points for register access
usb/ohci: Implement resume on connection status change
hw/usb/hcd-ohci: Fix typo
hw/usb/hcd-ohci.c | 461 ++++++++++++++++++++++++--------------------
hw/usb/trace-events | 4 +
2 files changed, 259 insertions(+), 206 deletions(-)
--
2.30.7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/7] usb/ohci: Code style fix comments
2023-02-20 18:15 ` [PATCH v2 1/7] usb/ohci: Code style fix comments BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 99 +++++++++++++++++++++++------------------------
1 file changed, 49 insertions(+), 50 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 9d68036d23..9f138e7f25 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -23,7 +23,7 @@
* o Disable timers when nothing needs to be done, or remove timer usage
* all together.
* o BIOS work to boot from USB storage
-*/
+ */
#include "qemu/osdep.h"
#include "hw/irq.h"
@@ -39,7 +39,7 @@
#include "hcd-ohci.h"
/* This causes frames to occur 1000x slower */
-//#define OHCI_TIME_WARP 1
+/*#define OHCI_TIME_WARP 1*/
#define ED_LINK_LIMIT 32
@@ -58,7 +58,7 @@ struct ohci_hcca {
#define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
#define ED_WBACK_SIZE 4
-/* Bitfields for the first word of an Endpoint Desciptor. */
+/* Bitfields for the first word of an Endpoint Desciptor. */
#define OHCI_ED_FA_SHIFT 0
#define OHCI_ED_FA_MASK (0x7f<<OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
@@ -71,11 +71,11 @@ struct ohci_hcca {
#define OHCI_ED_MPS_SHIFT 16
#define OHCI_ED_MPS_MASK (0x7ff<<OHCI_ED_MPS_SHIFT)
-/* Flags in the head field of an Endpoint Desciptor. */
+/* Flags in the head field of an Endpoint Desciptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
-/* Bitfields for the first word of a Transfer Desciptor. */
+/* Bitfields for the first word of a Transfer Desciptor. */
#define OHCI_TD_R (1<<18)
#define OHCI_TD_DP_SHIFT 19
#define OHCI_TD_DP_MASK (3<<OHCI_TD_DP_SHIFT)
@@ -88,7 +88,7 @@ struct ohci_hcca {
#define OHCI_TD_CC_SHIFT 28
#define OHCI_TD_CC_MASK (0xf<<OHCI_TD_CC_SHIFT)
-/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
+/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
/* CC & DI - same as in the General Transfer Desciptor */
#define OHCI_TD_SF_SHIFT 0
#define OHCI_TD_SF_MASK (0xffff<<OHCI_TD_SF_SHIFT)
@@ -335,8 +335,8 @@ static void ohci_soft_reset(OHCIState *ohci)
ohci->per_cur = 0;
ohci->done = 0;
ohci->done_count = 7;
-
- /* FSMPS is marked TBD in OCHI 1.0, what gives ffs?
+ /*
+ * FSMPS is marked TBD in OCHI 1.0, what gives ffs?
* I took the value linux sets ...
*/
ohci->fsmps = 0x2778;
@@ -460,10 +460,10 @@ static inline int ohci_read_hcca(OHCIState *ohci,
static inline int ohci_put_ed(OHCIState *ohci,
dma_addr_t addr, struct ohci_ed *ed)
{
- /* ed->tail is under control of the HCD.
+ /*
+ * ed->tail is under control of the HCD.
* Since just ed->head is changed by HC, just write back this
*/
-
return put_dwords(ohci, addr + ED_WBACK_OFFSET,
(uint32_t *)((char *)ed + ED_WBACK_OFFSET),
ED_WBACK_SIZE >> 2);
@@ -601,8 +601,10 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
trace_usb_ohci_iso_td_relative_frame_number_neg(relative_frame_number);
return 1;
} else if (relative_frame_number > frame_count) {
- /* ISO TD expired - retire the TD to the Done Queue and continue with
- the next ISO TD of the same ED */
+ /*
+ * ISO TD expired - retire the TD to the Done Queue and continue with
+ * the next ISO TD of the same ED
+ */
trace_usb_ohci_iso_td_relative_frame_number_big(relative_frame_number,
frame_count);
if (OHCI_CC_DATAOVERRUN == OHCI_BM(iso_td.flags, TD_CC)) {
@@ -845,9 +847,10 @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
}
}
-/* Service a transport descriptor.
- Returns nonzero to terminate processing of this endpoint. */
-
+/*
+ * Service a transport descriptor.
+ * Returns nonzero to terminate processing of this endpoint.
+ */
static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
{
int dir;
@@ -869,7 +872,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
return 1;
}
- /* See if this TD has already been submitted to the device. */
+ /* See if this TD has already been submitted to the device. */
completion = (addr == ohci->async_td);
if (completion && !ohci->async_complete) {
trace_usb_ohci_td_skip_async();
@@ -885,7 +888,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
switch (dir) {
case OHCI_TD_DIR_OUT:
case OHCI_TD_DIR_IN:
- /* Same value. */
+ /* Same value. */
break;
default:
dir = OHCI_BM(td.flags, TD_DP);
@@ -956,11 +959,12 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
}
ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
if (ohci->async_td) {
- /* ??? The hardware should allow one active packet per
- endpoint. We only allow one active packet per controller.
- This should be sufficient as long as devices respond in a
- timely manner.
- */
+ /*
+ * ??? The hardware should allow one active packet per
+ * endpoint. We only allow one active packet per controller.
+ * This should be sufficient as long as devices respond in a
+ * timely manner.
+ */
trace_usb_ohci_td_too_many_pending(ep->nr);
return 1;
}
@@ -996,7 +1000,7 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
/* Writeback */
if (ret == pktlen || (dir == OHCI_TD_DIR_IN && ret >= 0 && flag_r)) {
- /* Transmission succeeded. */
+ /* Transmission succeeded. */
if (ret == len) {
td.cbp = 0;
} else {
@@ -1048,8 +1052,10 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
OHCI_SET_BM(td.flags, TD_EC, 3);
break;
}
- /* An error occurred so we have to clear the interrupt counter. See
- * spec at 6.4.4 on page 104 */
+ /*
+ * An error occurred so we have to clear the interrupt counter.
+ * See spec at 6.4.4 on page 104
+ */
ohci->done_count = 0;
}
ed->head |= OHCI_ED_H;
@@ -1071,7 +1077,7 @@ exit_no_retire:
return OHCI_BM(td.flags, TD_CC) != OHCI_CC_NOERROR;
}
-/* Service an endpoint list. Returns nonzero if active TD were found. */
+/* Service an endpoint list. Returns nonzero if active TD were found. */
static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
{
struct ohci_ed ed;
@@ -1095,7 +1101,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
if ((ed.head & OHCI_ED_H) || (ed.flags & OHCI_ED_K)) {
uint32_t addr;
- /* Cancel pending packets for ED that have been paused. */
+ /* Cancel pending packets for ED that have been paused. */
addr = ed.head & OHCI_DPTR_MASK;
if (ohci->async_td && addr == ohci->async_td) {
usb_cancel_packet(&ohci->usb_packet);
@@ -1151,7 +1157,7 @@ static void ohci_sof(OHCIState *ohci)
ohci_set_interrupt(ohci, OHCI_INTR_SF);
}
-/* Process Control and Bulk lists. */
+/* Process Control and Bulk lists. */
static void ohci_process_lists(OHCIState *ohci)
{
if ((ohci->ctl & OHCI_CTL_CLE) && (ohci->status & OHCI_STATUS_CLF)) {
@@ -1192,7 +1198,7 @@ static void ohci_frame_boundary(void *opaque)
ohci_service_ed_list(ohci, le32_to_cpu(hcca.intr[n]));
}
- /* Cancel all pending packets if either of the lists has been disabled. */
+ /* Cancel all pending packets if either of the lists has been disabled. */
if (ohci->old_ctl & (~ohci->ctl) & (OHCI_CTL_BLE | OHCI_CTL_CLE)) {
ohci_stop_endpoints(ohci);
}
@@ -1234,18 +1240,17 @@ static void ohci_frame_boundary(void *opaque)
}
}
-/* Start sending SOF tokens across the USB bus, lists are processed in
+/*
+ * Start sending SOF tokens across the USB bus, lists are processed in
* next frame
*/
static int ohci_bus_start(OHCIState *ohci)
{
trace_usb_ohci_start(ohci->name);
-
- /* Delay the first SOF event by one frame time as
- * linux driver is not ready to receive it and
- * can meet some race conditions
+ /*
+ * Delay the first SOF event by one frame time as linux driver is
+ * not ready to receive it and can meet some race conditions
*/
-
ohci->sof_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
ohci_eof_timer(ohci);
@@ -1259,9 +1264,9 @@ void ohci_bus_stop(OHCIState *ohci)
timer_del(ohci->eof_timer);
}
-/* Sets a flag in a port status register but only set it if the port is
- * connected, if not set ConnectStatusChange flag. If flag is enabled
- * return 1.
+/*
+ * 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.
*/
static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
{
@@ -1271,9 +1276,7 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
if (val == 0)
return 0;
- /* If CurrentConnectStatus is cleared we set
- * ConnectStatusChange
- */
+ /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
if (ohci->rhstatus & OHCI_RHS_DRWE) {
@@ -1291,7 +1294,7 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
return ret;
}
-/* Set the frame interval - frame interval toggle is manipulated by the hcd only */
+/* Frame interval toggle is manipulated by the hcd only */
static void ohci_set_frame_interval(OHCIState *ohci, uint16_t val)
{
val &= OHCI_FMI_FI;
@@ -1357,9 +1360,7 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL)
return (ohci->frt << 31);
- /* Being in USB operational state guarnatees sof_time was
- * set already.
- */
+ /* Being in USB operational state guarnatees sof_time was set already. */
tks = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ohci->sof_time;
if (tks < 0) {
tks = 0;
@@ -1439,13 +1440,11 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
trace_usb_ohci_port_reset(portnum);
usb_device_reset(port->port.dev);
port->ctrl &= ~OHCI_PORT_PRS;
- /* ??? Should this also set OHCI_PORT_PESC. */
+ /* ??? Should this also set OHCI_PORT_PESC. */
port->ctrl |= OHCI_PORT_PES | OHCI_PORT_PRSC;
}
- /* Invert order here to ensure in ambiguous case, device is
- * powered up...
- */
+ /* Invert order here to ensure in ambiguous case, device is powered up. */
if (val & OHCI_PORT_LSDA)
ohci_port_power(ohci, portnum, 0);
if (val & OHCI_PORT_PPS)
@@ -1892,7 +1891,7 @@ void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
ohci_frame_boundary, ohci);
}
-/**
+/*
* A typical OHCI will stop operating and set itself into error state
* (which can be queried by MMIO) to signal that it got an error.
*/
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/7] usb/ohci: Code style fix white space errors
2023-02-20 18:15 ` [PATCH v2 2/7] usb/ohci: Code style fix white space errors BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 160 +++++++++++++++++++++++-----------------------
1 file changed, 79 insertions(+), 81 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 9f138e7f25..b8f8fd048d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -60,46 +60,46 @@ struct ohci_hcca {
/* Bitfields for the first word of an Endpoint Desciptor. */
#define OHCI_ED_FA_SHIFT 0
-#define OHCI_ED_FA_MASK (0x7f<<OHCI_ED_FA_SHIFT)
+#define OHCI_ED_FA_MASK (0x7f << OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
-#define OHCI_ED_EN_MASK (0xf<<OHCI_ED_EN_SHIFT)
+#define OHCI_ED_EN_MASK (0xf << OHCI_ED_EN_SHIFT)
#define OHCI_ED_D_SHIFT 11
-#define OHCI_ED_D_MASK (3<<OHCI_ED_D_SHIFT)
-#define OHCI_ED_S (1<<13)
-#define OHCI_ED_K (1<<14)
-#define OHCI_ED_F (1<<15)
+#define OHCI_ED_D_MASK (3 << OHCI_ED_D_SHIFT)
+#define OHCI_ED_S (1 << 13)
+#define OHCI_ED_K (1 << 14)
+#define OHCI_ED_F (1 << 15)
#define OHCI_ED_MPS_SHIFT 16
-#define OHCI_ED_MPS_MASK (0x7ff<<OHCI_ED_MPS_SHIFT)
+#define OHCI_ED_MPS_MASK (0x7ff << OHCI_ED_MPS_SHIFT)
/* Flags in the head field of an Endpoint Desciptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
/* Bitfields for the first word of a Transfer Desciptor. */
-#define OHCI_TD_R (1<<18)
+#define OHCI_TD_R (1 << 18)
#define OHCI_TD_DP_SHIFT 19
-#define OHCI_TD_DP_MASK (3<<OHCI_TD_DP_SHIFT)
+#define OHCI_TD_DP_MASK (3 << OHCI_TD_DP_SHIFT)
#define OHCI_TD_DI_SHIFT 21
-#define OHCI_TD_DI_MASK (7<<OHCI_TD_DI_SHIFT)
-#define OHCI_TD_T0 (1<<24)
-#define OHCI_TD_T1 (1<<25)
+#define OHCI_TD_DI_MASK (7 << OHCI_TD_DI_SHIFT)
+#define OHCI_TD_T0 (1 << 24)
+#define OHCI_TD_T1 (1 << 25)
#define OHCI_TD_EC_SHIFT 26
-#define OHCI_TD_EC_MASK (3<<OHCI_TD_EC_SHIFT)
+#define OHCI_TD_EC_MASK (3 << OHCI_TD_EC_SHIFT)
#define OHCI_TD_CC_SHIFT 28
-#define OHCI_TD_CC_MASK (0xf<<OHCI_TD_CC_SHIFT)
+#define OHCI_TD_CC_MASK (0xf << OHCI_TD_CC_SHIFT)
/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
/* CC & DI - same as in the General Transfer Desciptor */
#define OHCI_TD_SF_SHIFT 0
-#define OHCI_TD_SF_MASK (0xffff<<OHCI_TD_SF_SHIFT)
+#define OHCI_TD_SF_MASK (0xffff << OHCI_TD_SF_SHIFT)
#define OHCI_TD_FC_SHIFT 24
-#define OHCI_TD_FC_MASK (7<<OHCI_TD_FC_SHIFT)
+#define OHCI_TD_FC_MASK (7 << OHCI_TD_FC_SHIFT)
/* Isochronous Transfer Desciptor - Offset / PacketStatusWord */
#define OHCI_TD_PSW_CC_SHIFT 12
-#define OHCI_TD_PSW_CC_MASK (0xf<<OHCI_TD_PSW_CC_SHIFT)
+#define OHCI_TD_PSW_CC_MASK (0xf << OHCI_TD_PSW_CC_SHIFT)
#define OHCI_TD_PSW_SIZE_SHIFT 0
-#define OHCI_TD_PSW_SIZE_MASK (0xfff<<OHCI_TD_PSW_SIZE_SHIFT)
+#define OHCI_TD_PSW_SIZE_MASK (0xfff << OHCI_TD_PSW_SIZE_SHIFT)
#define OHCI_PAGE_MASK 0xfffff000
#define OHCI_OFFSET_MASK 0xfff
@@ -112,7 +112,7 @@ struct ohci_hcca {
#define OHCI_SET_BM(val, field, newval) do { \
val &= ~OHCI_##field##_MASK; \
val |= ((newval) << OHCI_##field##_SHIFT) & OHCI_##field##_MASK; \
- } while(0)
+ } while (0)
/* endpoint descriptor */
struct ohci_ed {
@@ -142,35 +142,35 @@ struct ohci_iso_td {
#define USB_HZ 12000000
/* OHCI Local stuff */
-#define OHCI_CTL_CBSR ((1<<0)|(1<<1))
-#define OHCI_CTL_PLE (1<<2)
-#define OHCI_CTL_IE (1<<3)
-#define OHCI_CTL_CLE (1<<4)
-#define OHCI_CTL_BLE (1<<5)
-#define OHCI_CTL_HCFS ((1<<6)|(1<<7))
+#define OHCI_CTL_CBSR ((1 << 0) | (1 << 1))
+#define OHCI_CTL_PLE (1 << 2)
+#define OHCI_CTL_IE (1 << 3)
+#define OHCI_CTL_CLE (1 << 4)
+#define OHCI_CTL_BLE (1 << 5)
+#define OHCI_CTL_HCFS ((1 << 6) | (1 << 7))
#define OHCI_USB_RESET 0x00
#define OHCI_USB_RESUME 0x40
#define OHCI_USB_OPERATIONAL 0x80
#define OHCI_USB_SUSPEND 0xc0
-#define OHCI_CTL_IR (1<<8)
-#define OHCI_CTL_RWC (1<<9)
-#define OHCI_CTL_RWE (1<<10)
-
-#define OHCI_STATUS_HCR (1<<0)
-#define OHCI_STATUS_CLF (1<<1)
-#define OHCI_STATUS_BLF (1<<2)
-#define OHCI_STATUS_OCR (1<<3)
-#define OHCI_STATUS_SOC ((1<<6)|(1<<7))
-
-#define OHCI_INTR_SO (1U<<0) /* Scheduling overrun */
-#define OHCI_INTR_WD (1U<<1) /* HcDoneHead writeback */
-#define OHCI_INTR_SF (1U<<2) /* Start of frame */
-#define OHCI_INTR_RD (1U<<3) /* Resume detect */
-#define OHCI_INTR_UE (1U<<4) /* Unrecoverable error */
-#define OHCI_INTR_FNO (1U<<5) /* Frame number overflow */
-#define OHCI_INTR_RHSC (1U<<6) /* Root hub status change */
-#define OHCI_INTR_OC (1U<<30) /* Ownership change */
-#define OHCI_INTR_MIE (1U<<31) /* Master Interrupt Enable */
+#define OHCI_CTL_IR (1 << 8)
+#define OHCI_CTL_RWC (1 << 9)
+#define OHCI_CTL_RWE (1 << 10)
+
+#define OHCI_STATUS_HCR (1 << 0)
+#define OHCI_STATUS_CLF (1 << 1)
+#define OHCI_STATUS_BLF (1 << 2)
+#define OHCI_STATUS_OCR (1 << 3)
+#define OHCI_STATUS_SOC ((1 << 6) | (1 << 7))
+
+#define OHCI_INTR_SO (1U << 0) /* Scheduling overrun */
+#define OHCI_INTR_WD (1U << 1) /* HcDoneHead writeback */
+#define OHCI_INTR_SF (1U << 2) /* Start of frame */
+#define OHCI_INTR_RD (1U << 3) /* Resume detect */
+#define OHCI_INTR_UE (1U << 4) /* Unrecoverable error */
+#define OHCI_INTR_FNO (1U << 5) /* Frame number overflow */
+#define OHCI_INTR_RHSC (1U << 6) /* Root hub status change */
+#define OHCI_INTR_OC (1U << 30) /* Ownership change */
+#define OHCI_INTR_MIE (1U << 31) /* Master Interrupt Enable */
#define OHCI_HCCA_SIZE 0x100
#define OHCI_HCCA_MASK 0xffffff00
@@ -181,40 +181,40 @@ struct ohci_iso_td {
#define OHCI_FMI_FSMPS 0xffff0000
#define OHCI_FMI_FIT 0x80000000
-#define OHCI_FR_RT (1U<<31)
+#define OHCI_FR_RT (1U << 31)
#define OHCI_LS_THRESH 0x628
#define OHCI_RHA_RW_MASK 0x00000000 /* Mask of supported features. */
-#define OHCI_RHA_PSM (1<<8)
-#define OHCI_RHA_NPS (1<<9)
-#define OHCI_RHA_DT (1<<10)
-#define OHCI_RHA_OCPM (1<<11)
-#define OHCI_RHA_NOCP (1<<12)
+#define OHCI_RHA_PSM (1 << 8)
+#define OHCI_RHA_NPS (1 << 9)
+#define OHCI_RHA_DT (1 << 10)
+#define OHCI_RHA_OCPM (1 << 11)
+#define OHCI_RHA_NOCP (1 << 12)
#define OHCI_RHA_POTPGT_MASK 0xff000000
-#define OHCI_RHS_LPS (1U<<0)
-#define OHCI_RHS_OCI (1U<<1)
-#define OHCI_RHS_DRWE (1U<<15)
-#define OHCI_RHS_LPSC (1U<<16)
-#define OHCI_RHS_OCIC (1U<<17)
-#define OHCI_RHS_CRWE (1U<<31)
-
-#define OHCI_PORT_CCS (1<<0)
-#define OHCI_PORT_PES (1<<1)
-#define OHCI_PORT_PSS (1<<2)
-#define OHCI_PORT_POCI (1<<3)
-#define OHCI_PORT_PRS (1<<4)
-#define OHCI_PORT_PPS (1<<8)
-#define OHCI_PORT_LSDA (1<<9)
-#define OHCI_PORT_CSC (1<<16)
-#define OHCI_PORT_PESC (1<<17)
-#define OHCI_PORT_PSSC (1<<18)
-#define OHCI_PORT_OCIC (1<<19)
-#define OHCI_PORT_PRSC (1<<20)
-#define OHCI_PORT_WTC (OHCI_PORT_CSC|OHCI_PORT_PESC|OHCI_PORT_PSSC \
- |OHCI_PORT_OCIC|OHCI_PORT_PRSC)
-
+#define OHCI_RHS_LPS (1U << 0)
+#define OHCI_RHS_OCI (1U << 1)
+#define OHCI_RHS_DRWE (1U << 15)
+#define OHCI_RHS_LPSC (1U << 16)
+#define OHCI_RHS_OCIC (1U << 17)
+#define OHCI_RHS_CRWE (1U << 31)
+
+#define OHCI_PORT_CCS (1 << 0)
+#define OHCI_PORT_PES (1 << 1)
+#define OHCI_PORT_PSS (1 << 2)
+#define OHCI_PORT_POCI (1 << 3)
+#define OHCI_PORT_PRS (1 << 4)
+#define OHCI_PORT_PPS (1 << 8)
+#define OHCI_PORT_LSDA (1 << 9)
+#define OHCI_PORT_CSC (1 << 16)
+#define OHCI_PORT_PESC (1 << 17)
+#define OHCI_PORT_PSSC (1 << 18)
+#define OHCI_PORT_OCIC (1 << 19)
+#define OHCI_PORT_PRSC (1 << 20)
+#define OHCI_PORT_WTC (OHCI_PORT_CSC | OHCI_PORT_PESC | \
+ OHCI_PORT_PSSC | OHCI_PORT_OCIC | \
+ OHCI_PORT_PRSC)
#define OHCI_TD_DIR_SETUP 0x0
#define OHCI_TD_DIR_OUT 0x1
#define OHCI_TD_DIR_IN 0x2
@@ -584,7 +584,7 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
starting_frame = OHCI_BM(iso_td.flags, TD_SF);
frame_count = OHCI_BM(iso_td.flags, TD_FC);
- relative_frame_number = USUB(ohci->frame_number, starting_frame);
+ relative_frame_number = USUB(ohci->frame_number, starting_frame);
trace_usb_ohci_iso_td_head(
ed->head & OHCI_DPTR_MASK, ed->tail & OHCI_DPTR_MASK,
@@ -657,8 +657,8 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
next_offset = iso_td.be;
}
- if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) ||
- ((relative_frame_number < frame_count) &&
+ if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) ||
+ ((relative_frame_number < frame_count) &&
!(OHCI_BM(next_offset, TD_PSW_CC) & 0xe))) {
trace_usb_ohci_iso_td_bad_cc_not_accessed(start_offset, next_offset);
return 1;
@@ -1118,7 +1118,7 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
ed.tail & OHCI_DPTR_MASK, ed.next & OHCI_DPTR_MASK);
trace_usb_ohci_ed_pkt_flags(
OHCI_BM(ed.flags, ED_FA), OHCI_BM(ed.flags, ED_EN),
- OHCI_BM(ed.flags, ED_D), (ed.flags & OHCI_ED_S)!= 0,
+ OHCI_BM(ed.flags, ED_D), (ed.flags & OHCI_ED_S) != 0,
(ed.flags & OHCI_ED_K) != 0, (ed.flags & OHCI_ED_F) != 0,
OHCI_BM(ed.flags, ED_MPS));
@@ -1311,10 +1311,8 @@ static void ohci_port_power(OHCIState *ohci, int i, int p)
if (p) {
ohci->rhport[i].ctrl |= OHCI_PORT_PPS;
} else {
- ohci->rhport[i].ctrl &= ~(OHCI_PORT_PPS|
- OHCI_PORT_CCS|
- OHCI_PORT_PSS|
- OHCI_PORT_PRS);
+ ohci->rhport[i].ctrl &= ~(OHCI_PORT_PPS | OHCI_PORT_CCS |
+ OHCI_PORT_PSS | OHCI_PORT_PRS);
}
}
@@ -1858,7 +1856,7 @@ void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
ohci->num_ports = num_ports;
if (masterbus) {
USBPort *ports[OHCI_MAX_PORTS];
- for(i = 0; i < num_ports; i++) {
+ for (i = 0; i < num_ports; i++) {
ports[i] = &ohci->rhport[i].port;
}
usb_register_companion(masterbus, ports, num_ports,
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis
2023-02-20 18:15 ` [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 106 ++++++++++++++++++++++++++--------------------
1 file changed, 60 insertions(+), 46 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index b8f8fd048d..a2cdba4058 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -499,9 +499,9 @@ static int ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
ptr = td->cbp;
n = 0x1000 - (ptr & 0xfff);
- if (n > len)
+ if (n > len) {
n = len;
-
+ }
if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
n, dir, MEMTXATTRS_UNSPECIFIED)) {
return -1;
@@ -527,9 +527,9 @@ static int ohci_copy_iso_td(OHCIState *ohci,
ptr = start_addr;
n = 0x1000 - (ptr & 0xfff);
- if (n > len)
+ if (n > len) {
n = len;
-
+ }
if (dma_memory_rw(ohci->as, ptr + ohci->localmem_base, buf,
n, dir, MEMTXATTRS_UNSPECIFIED)) {
return -1;
@@ -617,8 +617,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
iso_td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(iso_td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
if (ohci_put_iso_td(ohci, addr, &iso_td)) {
ohci_die(ohci);
return 1;
@@ -803,8 +804,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
iso_td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(iso_td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
}
if (ohci_put_iso_td(ohci, addr, &iso_td)) {
ohci_die(ohci);
@@ -1022,8 +1024,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
/* Setting ED_C is part of the TD retirement process */
ed->head &= ~OHCI_ED_C;
- if (td.flags & OHCI_TD_T0)
+ if (td.flags & OHCI_TD_T0) {
ed->head |= OHCI_ED_C;
+ }
} else {
if (ret >= 0) {
trace_usb_ohci_td_underrun();
@@ -1067,8 +1070,9 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
td.next = ohci->done;
ohci->done = addr;
i = OHCI_BM(td.flags, TD_DI);
- if (i < ohci->done_count)
+ if (i < ohci->done_count) {
ohci->done_count = i;
+ }
exit_no_retire:
if (ohci_put_td(ohci, addr, &td)) {
ohci_die(ohci);
@@ -1087,9 +1091,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
uint32_t link_cnt = 0;
active = 0;
- if (head == 0)
+ if (head == 0) {
return 0;
-
+ }
for (cur = head; cur && link_cnt++ < ED_LINK_LIMIT; cur = next_ed) {
if (ohci_read_ed(ohci, cur, &ed)) {
trace_usb_ohci_ed_read_error(cur);
@@ -1125,8 +1129,9 @@ static int ohci_service_ed_list(OHCIState *ohci, uint32_t head)
active = 1;
if ((ed.flags & OHCI_ED_F) == 0) {
- if (ohci_service_td(ohci, &ed))
+ if (ohci_service_td(ohci, &ed)) {
break;
+ }
} else {
/* Handle isochronous endpoints */
if (ohci_service_iso_td(ohci, &ed)) {
@@ -1218,19 +1223,21 @@ static void ohci_frame_boundary(void *opaque)
hcca.frame = cpu_to_le16(ohci->frame_number);
if (ohci->done_count == 0 && !(ohci->intr_status & OHCI_INTR_WD)) {
- if (!ohci->done)
+ if (!ohci->done) {
abort();
- if (ohci->intr & ohci->intr_status)
+ }
+ if (ohci->intr & ohci->intr_status) {
ohci->done |= 1;
+ }
hcca.done = cpu_to_le32(ohci->done);
ohci->done = 0;
ohci->done_count = 7;
ohci_set_interrupt(ohci, OHCI_INTR_WD);
}
- if (ohci->done_count != 7 && ohci->done_count != 0)
+ if (ohci->done_count != 7 && ohci->done_count != 0) {
ohci->done_count--;
-
+ }
/* Do SOF stuff here */
ohci_sof(ohci);
@@ -1273,9 +1280,9 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
int ret = 1;
/* writing a 0 has no effect */
- if (val == 0)
+ if (val == 0) {
return 0;
-
+ }
/* If CurrentConnectStatus is cleared we set ConnectStatusChange */
if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
@@ -1285,9 +1292,9 @@ static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
return 0;
}
- if (ohci->rhport[i].ctrl & val)
+ if (ohci->rhport[i].ctrl & val) {
ret = 0;
-
+ }
/* set the bit */
ohci->rhport[i].ctrl |= val;
@@ -1327,9 +1334,9 @@ static void ohci_set_ctl(OHCIState *ohci, uint32_t val)
new_state = ohci->ctl & OHCI_CTL_HCFS;
/* no state change */
- if (old_state == new_state)
+ if (old_state == new_state) {
return;
-
+ }
trace_usb_ohci_set_ctl(ohci->name, new_state);
switch (new_state) {
case OHCI_USB_OPERATIONAL:
@@ -1355,9 +1362,9 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
uint16_t fr;
int64_t tks;
- if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL)
- return (ohci->frt << 31);
-
+ if ((ohci->ctl & OHCI_CTL_HCFS) != OHCI_USB_OPERATIONAL) {
+ return ohci->frt << 31;
+ }
/* Being in USB operational state guarnatees sof_time was set already. */
tks = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ohci->sof_time;
if (tks < 0) {
@@ -1365,9 +1372,9 @@ static uint32_t ohci_get_frame_remaining(OHCIState *ohci)
}
/* avoid muldiv if possible */
- if (tks >= usb_frame_time)
- return (ohci->frt << 31);
-
+ if (tks >= usb_frame_time) {
+ return ohci->frt << 31;
+ }
tks = tks / usb_bit_time;
fr = (uint16_t)(ohci->fi - tks);
@@ -1383,33 +1390,36 @@ static void ohci_set_hub_status(OHCIState *ohci, uint32_t val)
old_state = ohci->rhstatus;
/* write 1 to clear OCIC */
- if (val & OHCI_RHS_OCIC)
+ if (val & OHCI_RHS_OCIC) {
ohci->rhstatus &= ~OHCI_RHS_OCIC;
-
+ }
if (val & OHCI_RHS_LPS) {
int i;
- for (i = 0; i < ohci->num_ports; i++)
+ for (i = 0; i < ohci->num_ports; i++) {
ohci_port_power(ohci, i, 0);
+ }
trace_usb_ohci_hub_power_down();
}
if (val & OHCI_RHS_LPSC) {
int i;
- for (i = 0; i < ohci->num_ports; i++)
+ for (i = 0; i < ohci->num_ports; i++) {
ohci_port_power(ohci, i, 1);
+ }
trace_usb_ohci_hub_power_up();
}
- if (val & OHCI_RHS_DRWE)
+ if (val & OHCI_RHS_DRWE) {
ohci->rhstatus |= OHCI_RHS_DRWE;
-
- if (val & OHCI_RHS_CRWE)
+ }
+ if (val & OHCI_RHS_CRWE) {
ohci->rhstatus &= ~OHCI_RHS_DRWE;
-
- if (old_state != ohci->rhstatus)
+ }
+ if (old_state != ohci->rhstatus) {
ohci_set_interrupt(ohci, OHCI_INTR_RHSC);
+ }
}
/* Set root hub port status */
@@ -1422,12 +1432,12 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
old_state = port->ctrl;
/* Write to clear CSC, PESC, PSSC, OCIC, PRSC */
- if (val & OHCI_PORT_WTC)
+ if (val & OHCI_PORT_WTC) {
port->ctrl &= ~(val & OHCI_PORT_WTC);
-
- if (val & OHCI_PORT_CCS)
+ }
+ if (val & OHCI_PORT_CCS) {
port->ctrl &= ~OHCI_PORT_PES;
-
+ }
ohci_port_set_if_connected(ohci, portnum, val & OHCI_PORT_PES);
if (ohci_port_set_if_connected(ohci, portnum, val & OHCI_PORT_PSS)) {
@@ -1443,13 +1453,15 @@ static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
}
/* Invert order here to ensure in ambiguous case, device is powered up. */
- if (val & OHCI_PORT_LSDA)
+ if (val & OHCI_PORT_LSDA) {
ohci_port_power(ohci, portnum, 0);
- if (val & OHCI_PORT_PPS)
+ }
+ if (val & OHCI_PORT_PPS) {
ohci_port_power(ohci, portnum, 1);
-
- if (old_state != port->ctrl)
+ }
+ if (old_state != port->ctrl) {
ohci_set_interrupt(ohci, OHCI_INTR_RHSC);
+ }
}
static uint64_t ohci_mem_read(void *opaque,
@@ -1606,8 +1618,9 @@ static void ohci_mem_write(void *opaque,
/* Bits written as '0' remain unchanged in the register */
ohci->status |= val;
- if (ohci->status & OHCI_STATUS_HCR)
+ if (ohci->status & OHCI_STATUS_HCR) {
ohci_soft_reset(ohci);
+ }
break;
case 3: /* HcInterruptStatus */
@@ -1685,8 +1698,9 @@ static void ohci_mem_write(void *opaque,
case 25: /* HcHReset */
ohci->hreset = val & ~OHCI_HRESET_FSBIR;
- if (val & OHCI_HRESET_FSBIR)
+ if (val & OHCI_HRESET_FSBIR) {
ohci_hard_reset(ohci);
+ }
break;
case 26: /* HcHInterruptEnable */
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/7] usb/ohci: Move a function next to where it is used
2023-02-20 18:15 ` [PATCH v2 4/7] usb/ohci: Move a function next to where it is used BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
The ohci_port_set_if_connected() function is only used by
ohci_port_set_status(), move next to it to have them at the same place.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/usb/hcd-ohci.c | 60 +++++++++++++++++++++++------------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index a2cdba4058..52fcfcd4ab 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1271,36 +1271,6 @@ void ohci_bus_stop(OHCIState *ohci)
timer_del(ohci->eof_timer);
}
-/*
- * 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.
- */
-static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
-{
- int ret = 1;
-
- /* writing a 0 has no effect */
- if (val == 0) {
- return 0;
- }
- /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
- if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
- ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
- if (ohci->rhstatus & OHCI_RHS_DRWE) {
- /* TODO: CSC is a wakeup event */
- }
- return 0;
- }
-
- if (ohci->rhport[i].ctrl & val) {
- ret = 0;
- }
- /* set the bit */
- ohci->rhport[i].ctrl |= val;
-
- return ret;
-}
-
/* Frame interval toggle is manipulated by the hcd only */
static void ohci_set_frame_interval(OHCIState *ohci, uint16_t val)
{
@@ -1422,6 +1392,36 @@ static void ohci_set_hub_status(OHCIState *ohci, uint32_t val)
}
}
+/*
+ * 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.
+ */
+static int ohci_port_set_if_connected(OHCIState *ohci, int i, uint32_t val)
+{
+ int ret = 1;
+
+ /* writing a 0 has no effect */
+ if (val == 0) {
+ return 0;
+ }
+ /* If CurrentConnectStatus is cleared we set ConnectStatusChange */
+ if (!(ohci->rhport[i].ctrl & OHCI_PORT_CCS)) {
+ ohci->rhport[i].ctrl |= OHCI_PORT_CSC;
+ if (ohci->rhstatus & OHCI_RHS_DRWE) {
+ /* TODO: CSC is a wakeup event */
+ }
+ return 0;
+ }
+
+ if (ohci->rhport[i].ctrl & val) {
+ ret = 0;
+ }
+ /* set the bit */
+ ohci->rhport[i].ctrl |= val;
+
+ return ret;
+}
+
/* Set root hub port status */
static void ohci_port_set_status(OHCIState *ohci, int portnum, uint32_t val)
{
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] usb/ohci: Add trace points for register access
2023-02-20 18:15 ` [PATCH v2 5/7] usb/ohci: Add trace points for register access BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
To help debugging add trace points that print values read from or
written to the device's registers.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/usb/hcd-ohci.c | 27 +++++++++++++++++++++++++++
hw/usb/trace-events | 4 ++++
2 files changed, 31 insertions(+)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 52fcfcd4ab..bad8db7b1d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -235,6 +235,24 @@ struct ohci_iso_td {
#define OHCI_HRESET_FSBIR (1 << 0)
+static const char *ohci_reg_names[] = {
+ "HcRevision", "HcControl", "HcCommandStatus", "HcInterruptStatus",
+ "HcInterruptEnable", "HcInterruptDisable", "HcHCCA", "HcPeriodCurrentED",
+ "HcControlHeadED", "HcControlCurrentED", "HcBulkHeadED", "HcBulkCurrentED",
+ "HcDoneHead", "HcFmInterval", "HcFmRemaining", "HcFmNumber",
+ "HcPeriodicStart", "HcLSThreshold", "HcRhDescriptorA", "HcRhDescriptorB",
+ "HcRhStatus"
+};
+
+static const char *ohci_reg_name(hwaddr addr)
+{
+ if (addr >> 2 < ARRAY_SIZE(ohci_reg_names)) {
+ return ohci_reg_names[addr >> 2];
+ } else {
+ return "<unknown>";
+ }
+}
+
static void ohci_die(OHCIState *ohci)
{
ohci->ohci_die(ohci);
@@ -1478,6 +1496,8 @@ static uint64_t ohci_mem_read(void *opaque,
} else if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
/* HcRhPortStatus */
retval = ohci->rhport[(addr - 0x54) >> 2].ctrl | OHCI_PORT_PPS;
+ trace_usb_ohci_mem_port_read(size, "HcRhPortStatus", (addr - 0x50) >> 2,
+ addr, addr >> 2, retval);
} else {
switch (addr >> 2) {
case 0: /* HcRevision */
@@ -1582,6 +1602,10 @@ static uint64_t ohci_mem_read(void *opaque,
trace_usb_ohci_mem_read_bad_offset(addr);
retval = 0xffffffff;
}
+ if (addr != 0xc || retval) {
+ trace_usb_ohci_mem_read(size, ohci_reg_name(addr), addr, addr >> 2,
+ retval);
+ }
}
return retval;
@@ -1602,10 +1626,13 @@ static void ohci_mem_write(void *opaque,
if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
/* HcRhPortStatus */
+ trace_usb_ohci_mem_port_write(size, "HcRhPortStatus", (addr - 0x50) >> 2,
+ addr, addr >> 2, val);
ohci_port_set_status(ohci, (addr - 0x54) >> 2, val);
return;
}
+ trace_usb_ohci_mem_write(size, ohci_reg_name(addr), addr, addr >> 2, val);
switch (addr >> 2) {
case 1: /* HcControl */
ohci_set_ctl(ohci, val);
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index b65269892c..6bb9655c8d 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -57,8 +57,12 @@ usb_ohci_ed_read_error(uint32_t addr) "ED read error at 0x%x"
usb_ohci_ed_pkt(uint32_t cur, int h, int c, uint32_t head, uint32_t tail, uint32_t next) "ED @ 0x%.8x h=%u c=%u\n head=0x%.8x tailp=0x%.8x next=0x%.8x"
usb_ohci_ed_pkt_flags(uint32_t fa, uint32_t en, uint32_t d, int s, int k, int f, uint32_t mps) "fa=%u en=%u d=%u s=%u k=%u f=%u mps=%u"
usb_ohci_hcca_read_error(uint32_t addr) "HCCA read error at 0x%x"
+usb_ohci_mem_read(uint32_t size, const char *name, uint32_t addr, uint32_t offs, uint32_t val) "%d %s 0x%x %d -> 0x%x"
+usb_ohci_mem_port_read(uint32_t size, const char *name, uint32_t port, uint32_t addr, uint32_t offs, uint32_t val) "%d %s[%d] 0x%x %d -> 0x%x"
usb_ohci_mem_read_unaligned(uint32_t addr) "at 0x%x"
usb_ohci_mem_read_bad_offset(uint32_t addr) "0x%x"
+usb_ohci_mem_write(uint32_t size, const char *name, uint32_t addr, uint32_t offs, uint32_t val) "%d %s 0x%x %d <- 0x%x"
+usb_ohci_mem_port_write(uint32_t size, const char *name, uint32_t port, uint32_t addr, uint32_t offs, uint32_t val) "%d %s[%d] 0x%x %d <- 0x%x"
usb_ohci_mem_write_unaligned(uint32_t addr) "at 0x%x"
usb_ohci_mem_write_bad_offset(uint32_t addr) "0x%x"
usb_ohci_process_lists(uint32_t head, uint32_t cur) "head 0x%x, cur 0x%x"
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-20 18:15 ` [PATCH v2 6/7] usb/ohci: Implement resume on connection status change BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
2023-02-28 13:54 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
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)
+{
+ 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;
if (ohci->rhstatus & OHCI_RHS_DRWE) {
- /* TODO: CSC is a wakeup event */
+ /* CSC is a wakeup event */
+ if (ohci_resume(ohci)) {
+ ohci_set_interrupt(ohci, OHCI_INTR_RD);
+ }
}
return 0;
}
@@ -1828,11 +1843,7 @@ static void ohci_wakeup(USBPort *port1)
intr = OHCI_INTR_RHSC;
}
/* Note that the controller can be suspended even if this port is not */
- if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
- trace_usb_ohci_remote_wakeup(s->name);
- /* This is the one state transition the controller can do by itself */
- s->ctl &= ~OHCI_CTL_HCFS;
- s->ctl |= OHCI_USB_RESUME;
+ if (ohci_resume(s)) {
/*
* In suspend mode only ResumeDetected is possible, not RHSC:
* see the OHCI spec 5.1.2.3.
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo
2023-02-20 18:15 ` [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo BALATON Zoltan
@ 2023-02-20 18:19 ` BALATON Zoltan
0 siblings, 0 replies; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-20 18:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell, philmd
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[balaton: rebased on clean up series]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/usb/hcd-ohci.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 88bd42b14a..98315b2301 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -58,7 +58,7 @@ struct ohci_hcca {
#define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
#define ED_WBACK_SIZE 4
-/* Bitfields for the first word of an Endpoint Desciptor. */
+/* Bitfields for the first word of an Endpoint Descriptor. */
#define OHCI_ED_FA_SHIFT 0
#define OHCI_ED_FA_MASK (0x7f << OHCI_ED_FA_SHIFT)
#define OHCI_ED_EN_SHIFT 7
@@ -71,11 +71,11 @@ struct ohci_hcca {
#define OHCI_ED_MPS_SHIFT 16
#define OHCI_ED_MPS_MASK (0x7ff << OHCI_ED_MPS_SHIFT)
-/* Flags in the head field of an Endpoint Desciptor. */
+/* Flags in the head field of an Endpoint Descriptor. */
#define OHCI_ED_H 1
#define OHCI_ED_C 2
-/* Bitfields for the first word of a Transfer Desciptor. */
+/* Bitfields for the first word of a Transfer Descriptor. */
#define OHCI_TD_R (1 << 18)
#define OHCI_TD_DP_SHIFT 19
#define OHCI_TD_DP_MASK (3 << OHCI_TD_DP_SHIFT)
@@ -88,14 +88,14 @@ struct ohci_hcca {
#define OHCI_TD_CC_SHIFT 28
#define OHCI_TD_CC_MASK (0xf << OHCI_TD_CC_SHIFT)
-/* Bitfields for the first word of an Isochronous Transfer Desciptor. */
-/* CC & DI - same as in the General Transfer Desciptor */
+/* Bitfields for the first word of an Isochronous Transfer Descriptor. */
+/* CC & DI - same as in the General Transfer Descriptor */
#define OHCI_TD_SF_SHIFT 0
#define OHCI_TD_SF_MASK (0xffff << OHCI_TD_SF_SHIFT)
#define OHCI_TD_FC_SHIFT 24
#define OHCI_TD_FC_MASK (7 << OHCI_TD_FC_SHIFT)
-/* Isochronous Transfer Desciptor - Offset / PacketStatusWord */
+/* Isochronous Transfer Descriptor - Offset / PacketStatusWord */
#define OHCI_TD_PSW_CC_SHIFT 12
#define OHCI_TD_PSW_CC_MASK (0xf << OHCI_TD_PSW_CC_SHIFT)
#define OHCI_TD_PSW_SIZE_SHIFT 0
--
2.30.7
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-20 18:15 ` [PATCH v2 6/7] usb/ohci: Implement resume on connection status change BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
@ 2023-02-28 13:54 ` Philippe Mathieu-Daudé
2023-02-28 14:55 ` Gerd Hoffmann
2023-02-28 14:55 ` BALATON Zoltan
1 sibling, 2 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-28 13:54 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel; +Cc: Gerd Hoffmann, Peter Maydell
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.
> +{
> + 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.
> - /* 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>
Gerd, if you Ack I can queue this.
> + }
> }
> return 0;
> }
> @@ -1828,11 +1843,7 @@ static void ohci_wakeup(USBPort *port1)
> intr = OHCI_INTR_RHSC;
> }
> /* Note that the controller can be suspended even if this port is not */
> - if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
> - trace_usb_ohci_remote_wakeup(s->name);
> - /* This is the one state transition the controller can do by itself */
> - s->ctl &= ~OHCI_CTL_HCFS;
> - s->ctl |= OHCI_USB_RESUME;
> + if (ohci_resume(s)) {
> /*
> * In suspend mode only ResumeDetected is possible, not RHSC:
> * see the OHCI spec 5.1.2.3.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-28 13:54 ` Philippe Mathieu-Daudé
@ 2023-02-28 14:55 ` Gerd Hoffmann
2023-02-28 14:55 ` BALATON Zoltan
1 sibling, 0 replies; 20+ messages in thread
From: Gerd Hoffmann @ 2023-02-28 14:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: BALATON Zoltan, qemu-devel, Peter Maydell
> > - /* 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>
>
> Gerd, if you Ack I can queue this.
Looks good to me (whole series).
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
thanks,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-28 13:54 ` Philippe Mathieu-Daudé
2023-02-28 14:55 ` Gerd Hoffmann
@ 2023-02-28 14:55 ` BALATON Zoltan
2023-02-28 15:50 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 20+ messages in thread
From: BALATON Zoltan @ 2023-02-28 14:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, Gerd Hoffmann, Peter Maydell
[-- Attachment #1: Type: text/plain, Size: 3130 bytes --]
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).
>> +{
>> + 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.
>> - /* 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.
Regards,
BALATON Zoltan
> Gerd, if you Ack I can queue this.
>
>> + }
>> }
>> return 0;
>> }
>> @@ -1828,11 +1843,7 @@ static void ohci_wakeup(USBPort *port1)
>> intr = OHCI_INTR_RHSC;
>> }
>> /* Note that the controller can be suspended even if this port is not
>> */
>> - if ((s->ctl & OHCI_CTL_HCFS) == OHCI_USB_SUSPEND) {
>> - trace_usb_ohci_remote_wakeup(s->name);
>> - /* This is the one state transition the controller can do by
>> itself */
>> - s->ctl &= ~OHCI_CTL_HCFS;
>> - s->ctl |= OHCI_USB_RESUME;
>> + if (ohci_resume(s)) {
>> /*
>> * In suspend mode only ResumeDetected is possible, not RHSC:
>> * see the OHCI spec 5.1.2.3.
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 6/7] usb/ohci: Implement resume on connection status change
2023-02-28 14:55 ` BALATON Zoltan
@ 2023-02-28 15:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-28 15:50 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, Gerd Hoffmann, Peter Maydell
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;
>>
>> // 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é <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).
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-02-28 15:51 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-20 18:15 [PATCH v2 0/7] OHCI changes BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 1/7] usb/ohci: Code style fix comments BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 2/7] usb/ohci: Code style fix white space errors BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 3/7] usb/ohci: Code style fix missing braces and extra parenthesis BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 4/7] usb/ohci: Move a function next to where it is used BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 5/7] usb/ohci: Add trace points for register access BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:15 ` [PATCH v2 6/7] usb/ohci: Implement resume on connection status change BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-28 13:54 ` Philippe Mathieu-Daudé
2023-02-28 14:55 ` Gerd Hoffmann
2023-02-28 14:55 ` BALATON Zoltan
2023-02-28 15:50 ` Philippe Mathieu-Daudé
2023-02-20 18:15 ` [PATCH v2 7/7] hw/usb/hcd-ohci: Fix typo BALATON Zoltan
2023-02-20 18:19 ` BALATON Zoltan
2023-02-20 18:19 ` [PATCH v2 RESEND 0/7] OHCI changes BALATON Zoltan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).