---------- Forwarded message ---------- From: <linux-usb-devel-owner@xxxxxxxxxxxxxxxxxxxxx> Date: Thu, Jan 28, 2010 at 11:28 AM Subject: [musb] A few questions regarding musb host code when shared fifo is used To: freeman.wang@xxxxxxxxx This list is now closed. Please post to the linux-usb list as described at http://vger.kernel.org/vger-lists.html#linux-usb instead. ---------- Forwarded message ---------- From: Freeman Wang <freeman.wang@xxxxxxxxx> To: linux-usb-devel@xxxxxxxxxxxxxxxxxxxxx Date: Thu, 28 Jan 2010 11:27:47 -0800 Subject: [musb] A few questions regarding musb host code when shared fifo is used Hi I just started working on musb and hope to get help from the group on a few questions. The code shows this in function musb_advance_schedule() of kernel 2.6.31.6 c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 359) static void musb_advance_schedule(struct musb *musb, struct urb *urb, c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 360) struct musb_hw_ep *hw_ep, int is_in) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 361) { c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 362) struct musb_qh *qh = musb_ep_get_qh(hw_ep, is_in); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 363) struct musb_hw_ep *ep = qh->hw_ep; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 364) int ready = qh->is_ready; c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 365) int status; c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 366) c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 367) status = (urb->status == -EINPROGRESS) ? 0 : urb->status; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 368) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 369) /* save toggle eagerly, for paranoia */ 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 370) switch (qh->type) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 371) case USB_ENDPOINT_XFER_BULK: 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 372) case USB_ENDPOINT_XFER_INT: 846099a6 (Sergei Shtylyov 2009-03-27 12:54:21 -0700 373) musb_save_toggle(qh, is_in, urb); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 374) break; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 375) case USB_ENDPOINT_XFER_ISOC: 1fe975f9 (Sergei Shtylyov 2009-07-10 20:02:44 +0300 376) if (status == 0 && urb->error_count) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 377) status = -EXDEV; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 378) break; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 379) } 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 380) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 381) qh->is_ready = 0; c9cd06b3 (Sergei Shtylyov 2009-03-27 12:58:31 -0700 382) musb_giveback(musb, urb, status); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 383) qh->is_ready = ready; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 384) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 385) /* reclaim resources (and bandwidth) ASAP; deschedule it, and 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 386) * invalidate qh as soon as list_empty(&hep->urb_list) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 387) */ 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 388) if (list_empty(&qh->hep->urb_list)) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 389) struct list_head *head; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 390) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 391) if (is_in) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 392) ep->rx_reinit = 1; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 393) else 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 394) ep->tx_reinit = 1; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 395) My understanding of this piece of code is: after the last urb in a host_endpoint urb_list is finished, we need to reinit the endpoint when picking up the next possible qh. However, there is only a musb_rx_reinit() implemented in the code ever since Felipe initially checked in the first version of this code, and tx_reinit has never been there. What confuses me most is, if rx_reinit (cleanup) is necessary, why tx_reinit is not? Is it possible the hw_ep be switched from tx to rx or rx to tx? Maybe in the bulk out case, the endpoint never gets stuck and we do not need to save the states, but do we need some kind of mode switch when fifo is shared? The chip I work on is using shared fifo for every endpoint. And this is another code snippet from the same file. 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 558) musb_rx_reinit(struct musb *musb, struct musb_qh *qh, struct musb_hw_ep *ep) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 559) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 560) u16 csr; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 561) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 562) /* NOTE: we know the "rx" fifo reinit never triggers for ep0. 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 563) * That always uses tx_reinit since ep0 repurposes TX register 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 564) * offsets; the initial SETUP packet is also a kind of OUT. 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 565) */ 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 566) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 567) /* if programmed for Tx, put it in RX mode */ 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 568) if (ep->is_shared_fifo) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 569) csr = musb_readw(ep->regs, MUSB_TXCSR); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 570) if (csr & MUSB_TXCSR_MODE) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 571) musb_h_tx_flush_fifo(ep); b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 572) csr = musb_readw(ep->regs, MUSB_TXCSR); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 573) musb_writew(ep->regs, MUSB_TXCSR, b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 574) csr | MUSB_TXCSR_FRCDATATOG); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 575) } b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 576) b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 577) /* b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 578) * Clear the MODE bit (and everything else) to enable Rx. b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 579) * NOTE: we mustn't clear the DMAMODE bit before DMAENAB. b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 580) */ b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 581) if (csr & MUSB_TXCSR_DMAMODE) b6e434a5 (Sergei Shtylyov 2009-03-26 18:27:47 -0700 582) musb_writew(ep->regs, MUSB_TXCSR, MUSB_TXCSR_DMAMODE); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 583) musb_writew(ep->regs, MUSB_TXCSR, 0); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 584) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 585) /* scrub all previous state, clearing toggle */ 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 586) } else { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 587) csr = musb_readw(ep->regs, MUSB_RXCSR); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 588) if (csr & MUSB_RXCSR_RXPKTRDY) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 589) WARNING("rx%d, packet/%d ready?\n", ep->epnum, 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 590) musb_readw(ep->regs, MUSB_RXCOUNT)); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 591) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 592) musb_h_flush_rxfifo(ep, MUSB_RXCSR_CLRDATATOG); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 593) } 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 594) If it is not using shared fifo, we will clear the rxfifo, which makes a lot sense to me. But if the ep is configured to use shared fifo, shouldn’t we clear the rx fifo too? The last urb is very possible to be an IN transfer. Felipe told me that he thought rx actually uses tx fifo but he never actually tested it on a shared fifo chip, so he recommended me to ask the group for help. I checked the code and found there are only two cases is_shared_fifo is checked in the host mode, excepting selecting the qh. One is in musb_core.c when deciding the fifo size, the other is the above musb_rx_reinit() function. It does not seem to be the case that we use TXCSR and tx fifo for rx processing in the .31 code, maybe long time ago. My third question came up when I read the function musb_schedule(). 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1813) /* use bulk reserved ep1 if no other ep is free */ aa5cbbec (Felipe Balbi 2008-11-17 09:08:16 +0200 1814) if (best_end < 0 && qh->type == USB_ENDPOINT_XFER_BULK) { 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1815) hw_ep = musb->bulk_ep; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1816) if (is_in) 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1817) head = &musb->in_bulk; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1818) else 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1819) head = &musb->out_bulk; 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1820) 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1821) /* Enable bulk RX NAK timeout scheme when bulk requests are 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1822) * multiplexed. This scheme doen't work in high speed to full 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1823) * speed scenario as NAK interrupts are not coming from a 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1824) * full speed device connected to a high speed device. 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1825) * NAK timeout interval is 8 (128 uframe or 16ms) for HS and 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1826) * 4 (8 frame or 8ms) for FS device. 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1827) */ 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1828) if (is_in && qh->dev) 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1829) qh->intv_reg = 1e0320f0 (Ajay Kumar Gupta 2009-02-24 15:26:13 -0800 1830) (USB_SPEED_HIGH == qh->dev->speed) ? 8 : 4; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1831) goto success; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1832) } else if (best_end < 0) { 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1833) return -ENOSPC; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1834) } 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1835) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1836) idle = 1; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1837) qh->mux = 0; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1838) hw_ep = musb->endpoints + best_end; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1839) DBG(4, "qh %p periodic slot %d\n", qh, best_end); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1840) success: 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1841) if (head) { 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1842) idle = list_empty(head); 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1843) list_add_tail(&qh->ring, head); 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1844) qh->mux = 1; 23d15e07 (Ajay Kumar Gupta 2008-10-29 15:10:35 +0200 1845) } 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1846) qh->hw_ep = hw_ep; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1847) qh->hep->hcpriv = qh; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1848) if (idle) 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1849) musb_start_urb(musb, is_in, qh); 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1850) return 0; 550a7375 (Felipe Balbi 2008-07-24 12:27:36 +0300 1851) } Again, it seems very confusing when shared fifo is used. In the case of dedicated fifo, the bulk endpoint actually has two independant set of registers, it is fine. However, if the bulk endpoint is using shared fifo, I'm afraid it will mess the hw_ep up. Say, I have a few qhs backloged in the in_bulk list and out_bulk is empty. If a new BULK OUT urb comes in, it will set head to out_bulk at line 1819 and move to line 1842 where idle is set to 1 because out_bulk is empty. Then at line 1848, idle is evaluated to be true and musb_start_urb is called. That will cause TXCSR to be written and switch the endpoint to TX mode. I'm afraid it will mess the internal states up, at least allow the shared fifo to be filled with the outgoing data while the IN transfers are still alive. It could be a big change to fix if my analysis right. Given that I’ve only worked on musb code for about a week, I really doubt my capability and hope to get help/suggestions from those who are more familiar with this chip. Thanks a lot. Freeman -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
