From patchwork Mon Jul 2 22:38:37 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Greer X-Patchwork-Id: 1148481 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 07959DFFAD for ; Mon, 2 Jul 2012 22:50:15 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1SlpN8-0004go-5b; Mon, 02 Jul 2012 22:45:34 +0000 Received: from mail20.dotsterhost.com ([66.11.232.73]) by merlin.infradead.org with smtps (Exim 4.76 #1 (Red Hat Linux)) id 1SlpMy-0004ga-I2 for linux-arm-kernel@lists.infradead.org; Mon, 02 Jul 2012 22:45:28 +0000 Received: (qmail 7785 invoked from network); 2 Jul 2012 22:38:38 -0000 Received: from unknown (HELO blue.animalcreek.com) (mgreer@animalcreek.com@[68.2.83.159]) by 66.11.232.73 with SMTP; 2 Jul 2012 22:38:37 -0000 Received: by blue.animalcreek.com (Postfix, from userid 1001) id 5C8BD65AEC; Mon, 2 Jul 2012 15:38:37 -0700 (MST) Date: Mon, 2 Jul 2012 15:38:37 -0700 From: "Mark A. Greer" To: linux-omap , linux-arm-kernel Subject: [FYI/RFC] usb: musb: DMA doesn't work with "nop" PHYs Message-ID: <20120702223837.GA31796@animalcreek.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-Spam-Note: CRM114 invocation failed X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [66.11.232.73 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Felipe Balbi X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org [The reason I'm sending this is that there is an issue but I probably won't be able to spend much time on it for a while. So, I'm sending it in case a) it helps someone else who bumps into it and b) someone else picks up and completes the fix.] Hello. In testing USB OTG Gadget on the am37x EVM with the mainline kernel I discovered that DMA doesn't work (but PIO does). When I tested the same kernel on the BeagleBoard-xM (BBxM), which has essentially the same SoC, it worked fine. They do have different OTG PHYs, though. The am37x EVM uses an ISP 1507 and the BBxM uses the PHY on the TPS65950 (twl4030). The different PHYs means that they use different drivers in drivers/usb/otg. The am37x EVM uses nop-usb-xceiv.c and the BBxM uses twl4030-usb.c. After a little digging, the issue appears to be that the generic musb code, which uses pm_runtime calls to shut things off when its not busy, assumes that PHY code will turn things back on when there is activity. For example, on the BBxM (twl4030-usb.c) there is a PHY interrupt that is handled by twl4030_usb_irq() which notifies the generic code and eventually calls the appropriate pm_runtime call to turn things on again. There isn't interrupt support like that in nop-usb-xceiv.c so the IP stays off when DMA should be happening. There are at least a couple options for fixing this (that I can think of): a) Enhance ulip.c to support interrupt generation from the PHY. IIUC, this will cause a non-dma interrupt so there would be some hacking in the generic musb code required to handle this. b) Add pm_runtime hooks to musbhsdma.c since its the DMA driver being used and it probably makes sense for it to be pm_runtime-ized anyway. I went down the path of b) and added pm_runtime calls to the channel_alloc() and channel_release() DMA driver hooks. The patch is here: I'm not very familiar with the musb code so I could be way off in the weeds. If you have comments or better ideas, please let me know. If you have the time to fix it, even better! :) Thanks, Mark diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index 57a6085..8eb13c7 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c @@ -76,10 +76,17 @@ static struct dma_channel *dma_channel_allocate(struct dma_controller *c, { struct musb_dma_controller *controller = container_of(c, struct musb_dma_controller, controller); + struct musb *musb = controller->private_data; struct musb_dma_channel *musb_channel = NULL; struct dma_channel *channel = NULL; u8 bit; + if (!controller->used_channels) { + spin_unlock(&musb->lock); + pm_runtime_get_sync(musb->controller); + spin_lock(&musb->lock); + } + for (bit = 0; bit < MUSB_HSDMA_CHANNELS; bit++) { if (!(controller->used_channels & (1 << bit))) { controller->used_channels |= (1 << bit); @@ -105,15 +112,25 @@ static struct dma_channel *dma_channel_allocate(struct dma_controller *c, static void dma_channel_release(struct dma_channel *channel) { struct musb_dma_channel *musb_channel = channel->private_data; + struct musb_dma_controller *controller = musb_channel->controller; + struct musb *musb = controller->private_data; + u8 prev_used_channels; channel->actual_len = 0; musb_channel->start_addr = 0; musb_channel->len = 0; - musb_channel->controller->used_channels &= - ~(1 << musb_channel->idx); + prev_used_channels = controller->used_channels; + controller->used_channels &= ~(1 << musb_channel->idx); channel->status = MUSB_DMA_STATUS_UNKNOWN; + + /* Only _put() when going from 1 channel to 0 channels */ + if (prev_used_channels && !controller->used_channels) { + spin_unlock(&musb->lock); + pm_runtime_put(musb->controller); + spin_lock(&musb->lock); + } } static void configure_channel(struct dma_channel *channel, I also have a simpler version that calls pm_runtime every time those routines are called. Either way, the gadget code resets the device and it doesn't work. I don't understand the code well enough to know why and I don't have time to pursue it right now. In the meantime, if pm_runtime calls are added to the start() and stop() hooks of the DMA driver, it will work fine. The problem with putting the pm_runtime calls there is that it will stay powered on even when DMA isn't being used. This patch works for me on the am37x EVM: diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c index 57a6085..b69e1e1 100644 --- a/drivers/usb/musb/musbhsdma.c +++ b/drivers/usb/musb/musbhsdma.c @@ -39,7 +39,12 @@ static int dma_controller_start(struct dma_controller *c) { - /* nothing to do */ + struct musb_dma_controller *controller = container_of(c, + struct musb_dma_controller, controller); + struct musb *musb = controller->private_data; + + pm_runtime_get_sync(musb->controller); + return 0; } @@ -68,6 +73,8 @@ static int dma_controller_stop(struct dma_controller *c) } } + pm_runtime_put(musb->controller); + return 0; }