Date: Fri, 20 Mar 2009 09:35:35 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 159502 for review Message-ID: <200903200935.n2K9ZZm6039821@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=159502 Change 159502 by hselasky@hselasky_laptop001 on 2009/03/20 09:34:56 USB controller: - get Device Side drivers in line with Host Side drivers regarding the recent short control transfer patches. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/controller/at91dci.c#5 edit .. //depot/projects/usb/src/sys/dev/usb/controller/atmegadci.c#9 edit .. //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#9 edit .. //depot/projects/usb/src/sys/dev/usb/controller/musb_otg.c#4 edit .. //depot/projects/usb/src/sys/dev/usb/controller/ohci.c#6 edit .. //depot/projects/usb/src/sys/dev/usb/controller/uhci.c#5 edit .. //depot/projects/usb/src/sys/dev/usb/controller/uss820dci.c#5 edit .. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#133 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/controller/at91dci.c#5 (text+ko) ==== @@ -887,7 +887,6 @@ temp.td = NULL; temp.td_next = xfer->td_start[0]; - temp.setup_alt_next = xfer->flags_int.short_frames_ok; temp.offset = 0; sc = AT9100_DCI_BUS2SC(xfer->xroot->bus); @@ -902,6 +901,7 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.short_pkt = temp.len ? 1 : 0; + temp.setup_alt_next = 0; at91dci_setup_standard_chain_sub(&temp); } @@ -910,6 +910,8 @@ x = 0; } + temp.setup_alt_next = xfer->flags_int.short_frames_ok; + if (x != xfer->nframes) { if (xfer->endpoint & UE_DIR_IN) { temp.func = &at91dci_data_tx; @@ -933,7 +935,13 @@ x++; if (x == xfer->nframes) { - temp.setup_alt_next = 0; + if (xfer->flags_int.control_xfr) { + if (xfer->flags_int.control_act) { + temp.setup_alt_next = 0; + } + } else { + temp.setup_alt_next = 0; + } } if (temp.len == 0) { @@ -958,47 +966,46 @@ } } - /* always setup a valid "pc" pointer for status and sync */ - temp.pc = xfer->frbuffers + 0; + /* check for control transfer */ + if (xfer->flags_int.control_xfer) { - /* check if we need to sync */ - if (need_sync && xfer->flags_int.control_xfr) { - - /* we need a SYNC point after TX */ - temp.func = &at91dci_data_tx_sync; + /* always setup a valid "pc" pointer for status and sync */ + temp.pc = xfer->frbuffers + 0; temp.len = 0; temp.short_pkt = 0; + temp.setup_alt_next = 0; - at91dci_setup_standard_chain_sub(&temp); - } - /* check if we should append a status stage */ - if (xfer->flags_int.control_xfr && - !xfer->flags_int.control_act) { - - /* - * Send a DATA1 message and invert the current - * endpoint direction. - */ - if (xfer->endpoint & UE_DIR_IN) { - temp.func = &at91dci_data_rx; - need_sync = 0; - } else { - temp.func = &at91dci_data_tx; - need_sync = 1; - } - temp.len = 0; - temp.short_pkt = 0; - - at91dci_setup_standard_chain_sub(&temp); + /* check if we need to sync */ if (need_sync) { /* we need a SYNC point after TX */ temp.func = &at91dci_data_tx_sync; - temp.len = 0; - temp.short_pkt = 0; + at91dci_setup_standard_chain_sub(&temp); + } + + /* check if we should append a status stage */ + if (!xfer->flags_int.control_act) { + + /* + * Send a DATA1 message and invert the current + * endpoint direction. + */ + if (xfer->endpoint & UE_DIR_IN) { + temp.func = &at91dci_data_rx; + need_sync = 0; + } else { + temp.func = &at91dci_data_tx; + need_sync = 1; + } at91dci_setup_standard_chain_sub(&temp); + if (need_sync) { + /* we need a SYNC point after TX */ + temp.func = &at91dci_data_tx_sync; + at91dci_setup_standard_chain_sub(&temp); + } } } + /* must have at least one frame! */ td = temp.td; xfer->td_transfer_last = td; ==== //depot/projects/usb/src/sys/dev/usb/controller/atmegadci.c#9 (text+ko) ==== @@ -791,7 +791,6 @@ temp.td = NULL; temp.td_next = xfer->td_start[0]; - temp.setup_alt_next = xfer->flags_int.short_frames_ok; temp.offset = 0; sc = ATMEGA_BUS2SC(xfer->xroot->bus); @@ -806,6 +805,7 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.short_pkt = temp.len ? 1 : 0; + temp.setup_alt_next = 0; atmegadci_setup_standard_chain_sub(&temp); } @@ -814,6 +814,8 @@ x = 0; } + temp.setup_alt_next = xfer->flags_int.short_frames_ok; + if (x != xfer->nframes) { if (xfer->endpoint & UE_DIR_IN) { temp.func = &atmegadci_data_tx; @@ -837,7 +839,13 @@ x++; if (x == xfer->nframes) { - temp.setup_alt_next = 0; + if (xfer->flags_int.control_xfr) { + if (xfer->flags_int.control_act) { + temp.setup_alt_next = 0; + } + } else { + temp.setup_alt_next = 0; + } } if (temp.len == 0) { @@ -862,45 +870,42 @@ } } - /* always setup a valid "pc" pointer for status and sync */ - temp.pc = xfer->frbuffers + 0; + if (xfer->flags_int.control_xfr) { - /* check if we need to sync */ - if (need_sync && xfer->flags_int.control_xfr) { - - /* we need a SYNC point after TX */ - temp.func = &atmegadci_data_tx_sync; + /* always setup a valid "pc" pointer for status and sync */ + temp.pc = xfer->frbuffers + 0; temp.len = 0; temp.short_pkt = 0; + temp.setup_alt_next = 0; - atmegadci_setup_standard_chain_sub(&temp); - } - /* check if we should append a status stage */ - if (xfer->flags_int.control_xfr && - !xfer->flags_int.control_act) { - - /* - * Send a DATA1 message and invert the current - * endpoint direction. - */ - if (xfer->endpoint & UE_DIR_IN) { - temp.func = &atmegadci_data_rx; - need_sync = 0; - } else { - temp.func = &atmegadci_data_tx; - need_sync = 1; - } - temp.len = 0; - temp.short_pkt = 0; - - atmegadci_setup_standard_chain_sub(&temp); + /* check if we need to sync */ if (need_sync) { /* we need a SYNC point after TX */ temp.func = &atmegadci_data_tx_sync; - temp.len = 0; - temp.short_pkt = 0; + atmegadci_setup_standard_chain_sub(&temp); + } + + /* check if we should append a status stage */ + if (!xfer->flags_int.control_act) { + + /* + * Send a DATA1 message and invert the current + * endpoint direction. + */ + if (xfer->endpoint & UE_DIR_IN) { + temp.func = &atmegadci_data_rx; + need_sync = 0; + } else { + temp.func = &atmegadci_data_tx; + need_sync = 1; + } atmegadci_setup_standard_chain_sub(&temp); + if (need_sync) { + /* we need a SYNC point after TX */ + temp.func = &atmegadci_data_tx_sync; + atmegadci_setup_standard_chain_sub(&temp); + } } } /* must have at least one frame! */ ==== //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#9 (text+ko) ==== @@ -1784,7 +1784,6 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.shortpkt = temp.len ? 1 : 0; - /* no "alt_next" for SETUP stage */ temp.setup_alt_next = 0; /* check for last frame */ if (xfer->nframes == 1) { ==== //depot/projects/usb/src/sys/dev/usb/controller/musb_otg.c#4 (text+ko) ==== @@ -1132,7 +1132,6 @@ temp.td = NULL; temp.td_next = xfer->td_start[0]; - temp.setup_alt_next = xfer->flags_int.short_frames_ok; temp.offset = 0; sc = MUSBOTG_BUS2SC(xfer->xroot->bus); @@ -1147,6 +1146,7 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.short_pkt = temp.len ? 1 : 0; + temp.setup_alt_next = 0; musbotg_setup_standard_chain_sub(&temp); } @@ -1155,6 +1155,8 @@ x = 0; } + temp.setup_alt_next = xfer->flags_int.short_frames_ok; + if (x != xfer->nframes) { if (xfer->endpoint & UE_DIR_IN) { if (xfer->flags_int.control_xfr) @@ -1180,7 +1182,13 @@ x++; if (x == xfer->nframes) { - temp.setup_alt_next = 0; + if (xfer->flags_int.control_xfr) { + if (xfer->flags_int.control_act) { + temp.setup_alt_next = 0; + } + } else { + temp.setup_alt_next = 0; + } } if (temp.len == 0) { @@ -1205,23 +1213,24 @@ } } - /* always setup a valid "pc" pointer for status and sync */ - temp.pc = xfer->frbuffers + 0; + /* check for control transfer */ + if (xfer->flags_int.control_xfr) { - /* check if we should append a status stage */ - - if (xfer->flags_int.control_xfr && - !xfer->flags_int.control_act) { - - /* - * Send a DATA1 message and invert the current - * endpoint direction. - */ - temp.func = &musbotg_setup_status; + /* always setup a valid "pc" pointer for status and sync */ + temp.pc = xfer->frbuffers + 0; temp.len = 0; temp.short_pkt = 0; + temp.setup_alt_next = 0; - musbotg_setup_standard_chain_sub(&temp); + /* check if we should append a status stage */ + if (!xfer->flags_int.control_act) { + /* + * Send a DATA1 message and invert the current + * endpoint direction. + */ + temp.func = &musbotg_setup_status; + musbotg_setup_standard_chain_sub(&temp); + } } /* must have at least one frame! */ td = temp.td; ==== //depot/projects/usb/src/sys/dev/usb/controller/ohci.c#6 (text+ko) ==== @@ -1439,7 +1439,6 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.shortpkt = temp.len ? 1 : 0; - /* no "alt_next" for SETUP stage */ temp.setup_alt_next = 0; /* check for last frame */ if (xfer->nframes == 1) { ==== //depot/projects/usb/src/sys/dev/usb/controller/uhci.c#5 (text+ko) ==== @@ -1253,32 +1253,29 @@ td_self = td->td_self; td_alt_next = td->alt_next; - if ((xfer->flags_int.control_xfr) && - (!xfer->flags_int.control_act) && - (((void *)td) == xfer->td_transfer_last)) - goto skip; /* don't touch DT value on STATUS stage */ + if (xfer->flags_int.control_xfr) + goto skip; /* don't touch the DT value! */ - if ((td->td_token ^ td_token) & htole32(UHCI_TD_SET_DT(1))) { - /* - * The data toggle is wrong and - * we need to switch it ! - */ + if (!((td->td_token ^ td_token) & htole32(UHCI_TD_SET_DT(1)))) + goto skip; /* data toggle has correct value */ - while (1) { + /* + * The data toggle is wrong and we need to toggle it ! + */ + while (1) { - td->td_token ^= htole32(UHCI_TD_SET_DT(1)); - usb2_pc_cpu_flush(td->page_cache); + td->td_token ^= htole32(UHCI_TD_SET_DT(1)); + usb2_pc_cpu_flush(td->page_cache); - if (td == xfer->td_transfer_last) { - /* last transfer */ - break; - } - td = td->obj_next; + if (td == xfer->td_transfer_last) { + /* last transfer */ + break; + } + td = td->obj_next; - if (td->alt_next != td_alt_next) { - /* next frame */ - break; - } + if (td->alt_next != td_alt_next) { + /* next frame */ + break; } } skip: @@ -1710,7 +1707,6 @@ temp.len = xfer->frlengths[0]; temp.ml.buf_pc = xfer->frbuffers + 0; temp.shortpkt = temp.len ? 1 : 0; - /* no "alt_next" for SETUP stage */ temp.setup_alt_next = 0; /* check for last frame */ if (xfer->nframes == 1) { ==== //depot/projects/usb/src/sys/dev/usb/controller/uss820dci.c#5 (text+ko) ==== @@ -836,7 +836,6 @@ temp.td = NULL; temp.td_next = xfer->td_start[0]; - temp.setup_alt_next = xfer->flags_int.short_frames_ok; temp.offset = 0; sc = USS820_DCI_BUS2SC(xfer->xroot->bus); @@ -851,6 +850,7 @@ temp.len = xfer->frlengths[0]; temp.pc = xfer->frbuffers + 0; temp.short_pkt = temp.len ? 1 : 0; + temp.setup_alt_next = 0; uss820dci_setup_standard_chain_sub(&temp); } @@ -859,6 +859,8 @@ x = 0; } + temp.setup_alt_next = xfer->flags_int.short_frames_ok; + if (x != xfer->nframes) { if (xfer->endpoint & UE_DIR_IN) { temp.func = &uss820dci_data_tx; @@ -878,7 +880,13 @@ x++; if (x == xfer->nframes) { - temp.setup_alt_next = 0; + if (xfer->flags_int.control_xfr) { + if (xfer->flags_int.control_act) { + temp.setup_alt_next = 0; + } + } else { + temp.setup_alt_next = 0; + } } if (temp.len == 0) { @@ -903,37 +911,39 @@ } } - /* always setup a valid "pc" pointer for status and sync */ - temp.pc = xfer->frbuffers + 0; - - /* check if we should append a status stage */ - - if (xfer->flags_int.control_xfr && - !xfer->flags_int.control_act) { + /* check for control transfer */ + if (xfer->flags_int.control_xfr) { uint8_t need_sync; - /* - * Send a DATA1 message and invert the current - * endpoint direction. - */ - if (xfer->endpoint & UE_DIR_IN) { - temp.func = &uss820dci_data_rx; - need_sync = 0; - } else { - temp.func = &uss820dci_data_tx; - need_sync = 1; - } + /* always setup a valid "pc" pointer for status and sync */ + temp.pc = xfer->frbuffers + 0; temp.len = 0; temp.short_pkt = 0; + temp.setup_alt_next = 0; - uss820dci_setup_standard_chain_sub(&temp); - if (need_sync) { - /* we need a SYNC point after TX */ - temp.func = &uss820dci_data_tx_sync; + /* check if we should append a status stage */ + if (!xfer->flags_int.control_act) { + + /* + * Send a DATA1 message and invert the current + * endpoint direction. + */ + if (xfer->endpoint & UE_DIR_IN) { + temp.func = &uss820dci_data_rx; + need_sync = 0; + } else { + temp.func = &uss820dci_data_tx; + need_sync = 1; + } temp.len = 0; temp.short_pkt = 0; uss820dci_setup_standard_chain_sub(&temp); + if (need_sync) { + /* we need a SYNC point after TX */ + temp.func = &uss820dci_data_tx_sync; + uss820dci_setup_standard_chain_sub(&temp); + } } } /* must have at least one frame! */ ==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#133 (text+ko) ==== @@ -1251,6 +1251,20 @@ /* no longer active */ xfer->flags_int.control_act = 0; } + + /* Check for invalid number of frames */ + if (xfer->nframes > 2) { + /* + * If you need to split a control transfer, you + * have to do one part at a time. Only with + * non-control transfers you can do multiple + * parts a time. + */ + DPRINTFN(0, "Too many frames: %u\n", + (unsigned int)xfer->nframes); + goto error; + } + /* * Check if there is a control * transfer in progress: @@ -1495,32 +1509,28 @@ */ if (USB_GET_DATA_ISREAD(xfer)) { - if (xfer->flags_int.control_xfr) { + if (xfer->flags.short_frames_ok) { + xfer->flags_int.short_xfer_ok = 1; + xfer->flags_int.short_frames_ok = 1; + } else if (xfer->flags.short_xfer_ok) { + xfer->flags_int.short_xfer_ok = 1; - /* - * Control transfers do not support reception - * of multiple short USB frames ! - */ - - if (xfer->flags.short_xfer_ok) { - xfer->flags_int.short_xfer_ok = 1; + /* check for control transfer */ + if (xfer->flags_int.control_xfr) { /* - * Due to sometimes buggy device side - * firmware we need to do a STATUS - * stage in case of short control - * transfers in USB host mode, via - * the "alt_next" feature! + * 1) Control transfers do not support + * reception of multiple short USB + * frames in host mode and device side + * mode, with exception of: + * + * 2) Due to sometimes buggy device + * side firmware we need to do a + * STATUS stage in case of short + * control transfers in USB host mode. + * The STATUS stage then becomes the + * "alt_next" to the DATA stage. */ - if (udev->flags.usb2_mode == USB_MODE_HOST) - xfer->flags_int.short_frames_ok = 1; - } - } else { - - if (xfer->flags.short_frames_ok) { - xfer->flags_int.short_xfer_ok = 1; xfer->flags_int.short_frames_ok = 1; - } else if (xfer->flags.short_xfer_ok) { - xfer->flags_int.short_xfer_ok = 1; } } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903200935.n2K9ZZm6039821>