From owner-freebsd-usb@FreeBSD.ORG Thu Jul 18 08:07:34 2013 Return-Path: Delivered-To: usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 97D9737B; Thu, 18 Jul 2013 08:07:34 +0000 (UTC) (envelope-from hps@bitfrost.no) Received: from mta.bitpro.no (mta.bitpro.no [92.42.64.202]) by mx1.freebsd.org (Postfix) with ESMTP id 76ACADBF; Thu, 18 Jul 2013 08:07:33 +0000 (UTC) Received: from mail.lockless.no (mail.lockless.no [46.29.221.38]) by mta.bitpro.no (Postfix) with ESMTP id 45A517A2C6; Thu, 18 Jul 2013 10:07:26 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mail.lockless.no (Postfix) with ESMTP id D9A1E8EF082; Thu, 18 Jul 2013 10:07:28 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.4 (20090625) (Debian) at lockless.no Received: from mail.lockless.no ([127.0.0.1]) by localhost (mail.lockless.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RNCI7c0eEZ6V; Thu, 18 Jul 2013 10:07:27 +0200 (CEST) Received: from laptop015.hselasky.homeunix.org (31.89-11-148.nextgentel.com [89.11.148.31]) by mail.lockless.no (Postfix) with ESMTPSA id EB13C8EF081; Thu, 18 Jul 2013 10:07:26 +0200 (CEST) Message-ID: <51E7A29B.6040701@bitfrost.no> Date: Thu, 18 Jul 2013 10:08:59 +0200 From: Hans Petter Selasky Organization: Bitfrost A/S User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/20130522 Thunderbird/17.0.6 MIME-Version: 1.0 To: Oleksandr Tymoshenko Subject: Re: Beaglebone USB driver (Mentor Graphics OTG) References: <51608AA4.2020804@bluezbox.com> <51611A7B.2010105@bitfrost.no> <0927BB4C-6917-408D-B102-AB98F72314B6@bluezbox.com> <51CBDFEA.7050203@bitfrost.no> <49E5BE45-208C-4AAD-980D-590F32D9B600@bluezbox.com> In-Reply-To: <49E5BE45-208C-4AAD-980D-590F32D9B600@bluezbox.com> Content-Type: multipart/mixed; boundary="------------070101070502010609080900" Cc: arm@freebsd.org, usb@freebsd.org X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jul 2013 08:07:34 -0000 This is a multi-part message in MIME format. --------------070101070502010609080900 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi Oleksandr, Got some time to review the code you committed to HEAD: 1) You need to distinguish between STALL and other errors. This is a bit important, because STALL is a PID and other errors can mean anything. See attached patch. 2) You should not use the NAKTO feature, because USB devices are allowed to NAK forever if they wish to. You should restart a USB transfer seamlessly after NAKTO or should use infinite NAKing. The USB stack has a timeout on the transfers, and will cancel when timeout happens. By feeding these error codes into the USB stack you risk loosing data on mouse/keyboard/modem devices! Upon cancel you need to use that trick I told you (included in the attached patch), else the double reset of the FIFO will crash! Could you clean this up and test a bit. I don't have hardware at hand to test at the moment. Thank you! --HPS --------------070101070502010609080900 Content-Type: text/x-patch; name="musb_check_for_stall.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="musb_check_for_stall.diff" diff --git a/sys/dev/usb/controller/musb_otg.c b/sys/dev/usb/controller/musb_otg.c index 5113d7a..bac8dd9 100644 --- a/sys/dev/usb/controller/musb_otg.c +++ b/sys/dev/usb/controller/musb_otg.c @@ -169,6 +169,9 @@ musbotg_channel_alloc(struct musbotg_softc *sc, struct musbotg_td *td) if (sc->sc_channel_mask & (1 << 0)) return (-1); sc->sc_channel_mask |= (1 << 0); + /* Clear previous status bits */ + MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + /* Enable interrupts */ musbotg_ep_int_set(sc, ep, 1); return (0); } @@ -176,6 +179,9 @@ musbotg_channel_alloc(struct musbotg_softc *sc, struct musbotg_td *td) for (ch = 1; ch < MUSB2_EP_MAX; ch++) { if (!(sc->sc_channel_mask & (1 << ch))) { sc->sc_channel_mask |= (1 << ch); + /* Clear previous status bits */ + MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + /* Enable interrupts */ musbotg_ep_int_set(sc, ch, 1); return (ch); } @@ -203,6 +209,19 @@ musbotg_channel_free(struct musbotg_softc *sc, struct musbotg_td *td) musbotg_ep_int_set(sc, td->channel, 0); sc->sc_channel_mask &= ~(1 << td->channel); + /* select endpoint */ + MUSB2_WRITE_1(sc, MUSB2_REG_EPINDEX, td->channel); + + /* clear NAK limit */ + MUSB2_WRITE_1(sc, MUSB2_REG_TXNAKLIMIT, 0); + + /* + * Set non-existing device address so that any NAK'ing only + * transfers get cancelled! Should wait 250us before using + * channel again. + */ + MUSB2_WRITE_1(sc, MUSB2_REG_TXFADDR(td->channel), MUSB2_RST_ADDR); + td->channel = -1; } @@ -518,13 +537,15 @@ musbotg_host_ctrl_setup_tx(struct musbotg_td *td) return (1); /* Failed */ - if (csr & (MUSB2_MASK_CSR0L_RXSTALL | - MUSB2_MASK_CSR0L_ERROR)) - { - /* Clear status bit */ - MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); - td->error = 1; + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } if (csr & MUSB2_MASK_CSR0L_NAKTIMO) { @@ -546,10 +567,10 @@ musbotg_host_ctrl_setup_tx(struct musbotg_td *td) csr &= ~MUSB2_MASK_CSR0L_NAKTIMO; MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, csr); - td->error = 1; + td->error_any = 1; } - if (td->error) { + if (td->error_any) { musbotg_channel_free(sc, td); return (0); } @@ -636,7 +657,7 @@ musbotg_dev_ctrl_data_rx(struct musbotg_td *td) /* * USB Host Aborted the transfer. */ - td->error = 1; + td->error_any = 1; return (0); /* complete */ } if (!(csr & MUSB2_MASK_CSR0L_RXPKTRDY)) { @@ -653,14 +674,14 @@ musbotg_dev_ctrl_data_rx(struct musbotg_td *td) got_short = 1; } else { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; return (0); /* we are complete */ } } /* verify the packet byte count */ if (count > td->remainder) { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; return (0); /* we are complete */ } while (count > 0) { @@ -769,7 +790,7 @@ musbotg_dev_ctrl_data_tx(struct musbotg_td *td) * The current transfer was aborted * by the USB Host */ - td->error = 1; + td->error_any = 1; return (0); /* complete */ } if (csr & MUSB2_MASK_CSR0L_TXPKTRDY) { @@ -911,22 +932,19 @@ musbotg_host_ctrl_data_rx(struct musbotg_td *td) csr &= ~MUSB2_MASK_CSR0L_NAKTIMO; MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, csr); - td->error = 1; + td->error_any = 1; XXX; } /* Failed */ - if (csr & (MUSB2_MASK_CSR0L_RXSTALL | - MUSB2_MASK_CSR0L_ERROR)) - { - /* Clear status bit */ - MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); - td->error = 1; - } - - if (td->error) { - musbotg_channel_free(sc, td); - return (0); /* we are complete */ + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } if (!(csr & MUSB2_MASK_CSR0L_RXPKTRDY)) @@ -943,7 +961,7 @@ musbotg_host_ctrl_data_rx(struct musbotg_td *td) got_short = 1; } else { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -951,7 +969,7 @@ musbotg_host_ctrl_data_rx(struct musbotg_td *td) /* verify the packet byte count */ if (count > td->remainder) { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -1064,11 +1082,16 @@ musbotg_host_ctrl_data_tx(struct musbotg_td *td) csr = MUSB2_READ_1(sc, MUSB2_REG_TXCSRL); DPRINTFN(4, "csr=0x%02x\n", csr); - if (csr & (MUSB2_MASK_CSR0L_RXSTALL | - MUSB2_MASK_CSR0L_ERROR)) { - /* clear status bits */ - MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); - td->error = 1; + /* Failed */ + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } if (csr & MUSB2_MASK_CSR0L_NAKTIMO ) { @@ -1089,11 +1112,11 @@ musbotg_host_ctrl_data_tx(struct musbotg_td *td) csr &= ~MUSB2_MASK_CSR0L_NAKTIMO; MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, csr); - td->error = 1; + td->error_any = 1; } - if (td->error) { + if (td->error_any) { musbotg_channel_free(sc, td); return (0); /* complete */ } @@ -1315,24 +1338,20 @@ musbotg_host_ctrl_status_rx(struct musbotg_td *td) csr &= ~MUSB2_MASK_CSR0L_NAKTIMO; MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, csr); - td->error = 1; + td->error_any = 1; } /* Failed */ - if (csr & (MUSB2_MASK_CSR0L_RXSTALL | - MUSB2_MASK_CSR0L_ERROR)) - { - /* Clear status bit */ - MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); - td->error = 1; - } - - if (td->error) { - musbotg_channel_free(sc, td); - return (0); + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } - return (1); /* Not ready yet */ } @@ -1366,15 +1385,15 @@ musbotg_host_ctrl_status_tx(struct musbotg_td *td) return (1); /* Failed */ - if (csr & (MUSB2_MASK_CSR0L_RXSTALL | - MUSB2_MASK_CSR0L_ERROR)) - { - /* Clear status bit */ - MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); - td->error = 1; - musbotg_channel_free(sc, td); - return (0); /* complete */ + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } if (td->transaction_started) { @@ -1460,7 +1479,7 @@ repeat: got_short = 1; } else { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -1468,7 +1487,7 @@ repeat: /* verify the packet byte count */ if (count > td->remainder) { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -1766,22 +1785,19 @@ repeat: MUSB2_WRITE_1(sc, MUSB2_REG_RXCSRL, csr); } - td->error = 1; - } - - if (csr & MUSB2_MASK_CSRL_RXERROR) { - DPRINTFN(4, "RXERROR\n"); - td->error = 1; - } - - if (csr & MUSB2_MASK_CSRL_RXSTALL) { - DPRINTFN(4, "RXSTALL\n"); - td->error = 1; + td->error_any = 1; } - if (td->error) { - musbotg_channel_free(sc, td); - return (0); /* we are complete */ + /* Failed */ + if (csr & MUSB2_MASK_CSR0L_RXSTALL) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + td->error_stall = 1; + return (0); /* complete */ + } else if (csr & MUSB2_MASK_CSR0L_ERROR) { + DPRINTFN(1, "error bit set, csr=0x%02x\n", csr); + td->error_any = 1; + return (0); /* complete */ } if (!(csr & MUSB2_MASK_CSRL_RXPKTRDY)) { @@ -1804,7 +1820,7 @@ repeat: got_short = 1; } else { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -1813,7 +1829,7 @@ repeat: /* verify the packet byte count */ if (count > td->remainder) { /* invalid USB packet */ - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* we are complete */ } @@ -1933,7 +1949,7 @@ musbotg_host_data_tx(struct musbotg_td *td) MUSB2_MASK_CSRL_TXERROR)) { /* clear status bits */ MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, 0); - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* complete */ } @@ -1956,7 +1972,7 @@ musbotg_host_data_tx(struct musbotg_td *td) csr &= ~MUSB2_MASK_CSRL_TXNAKTO; MUSB2_WRITE_1(sc, MUSB2_REG_TXCSRL, csr); - td->error = 1; + td->error_any = 1; musbotg_channel_free(sc, td); return (0); /* complete */ } @@ -2122,7 +2138,7 @@ musbotg_xfer_do_fifo(struct usb_xfer *xfer) if (((void *)td) == xfer->td_transfer_last) { goto done; } - if (td->error) { + if (td->error_any) { goto done; } else if (td->remainder > 0) { /* @@ -2350,7 +2366,8 @@ musbotg_setup_standard_chain_sub(struct musbotg_std_temp *temp) td->pc = temp->pc; td->offset = temp->offset; td->remainder = temp->len; - td->error = 0; + td->error_any = 0; + td->error_stall = 0; td->transaction_started = 0; td->did_stall = temp->did_stall; td->short_pkt = temp->short_pkt; @@ -2674,7 +2691,7 @@ musbotg_standard_done_sub(struct usb_xfer *xfer) { struct musbotg_td *td; uint32_t len; - uint8_t error; + usb_error_t error; DPRINTFN(8, "\n"); @@ -2691,15 +2708,16 @@ musbotg_standard_done_sub(struct usb_xfer *xfer) * the remainder from "frlengths[]": */ if (len > xfer->frlengths[xfer->aframes]) { - td->error = 1; + td->error_any = 1; } else { xfer->frlengths[xfer->aframes] -= len; } } /* Check for transfer error */ - if (td->error) { + if (td->error_any) { /* the transfer is finished */ - error = 1; + error = (td->error_stall ? + USB_ERR_STALLED : USB_ERR_IOERROR); td = NULL; break; } @@ -2731,8 +2749,7 @@ musbotg_standard_done_sub(struct usb_xfer *xfer) xfer->td_transfer_cache = td; - return (error ? - USB_ERR_STALLED : USB_ERR_NORMAL_COMPLETION); + return (error); } static void diff --git a/sys/dev/usb/controller/musb_otg.h b/sys/dev/usb/controller/musb_otg.h index 649cab8..b68ea21 100644 --- a/sys/dev/usb/controller/musb_otg.h +++ b/sys/dev/usb/controller/musb_otg.h @@ -32,7 +32,8 @@ #ifndef _MUSB2_OTG_H_ #define _MUSB2_OTG_H_ -#define MUSB2_MAX_DEVICES USB_MAX_DEVICES +#define MUSB2_RST_ADDR 127 +#define MUSB2_MAX_DEVICES MIN(USB_MAX_DEVICES, MUSB2_RST_ADDR) /* Common registers */ @@ -319,7 +320,8 @@ struct musbotg_td { uint8_t ep_no; uint8_t transfer_type; uint8_t max_packet; - uint8_t error:1; + uint8_t error_any:1; + uint8_t error_stall:1; uint8_t alt_next:1; uint8_t short_pkt:1; uint8_t support_multi_buffer:1; --------------070101070502010609080900--