Hi,
On Mon, Mar 04, 2013 at 12:21:46PM -0800, Paul Zimmerman wrote:
> These files contain the HCD code, and implement the Linux
> hc_driver API. Support for both slave mode and buffer DMA mode
> of the controller is included.
>
> Signed-off-by: Paul Zimmerman <paulz@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc2/hcd.c | 2890 ++++++++++++++++++++++++++++++++++++++++++
> drivers/usb/dwc2/hcd.h | 749 +++++++++++
> drivers/usb/dwc2/hcd_intr.c | 2095 ++++++++++++++++++++++++++++++
> drivers/usb/dwc2/hcd_queue.c | 690 ++++++++++
> 4 files changed, 6424 insertions(+)
> create mode 100644 drivers/usb/dwc2/hcd.c
> create mode 100644 drivers/usb/dwc2/hcd.h
> create mode 100644 drivers/usb/dwc2/hcd_intr.c
> create mode 100644 drivers/usb/dwc2/hcd_queue.c
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> new file mode 100644
> index 0000000..efcb0e8
> --- /dev/null
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -0,0 +1,2890 @@
> +/*
> + * hcd.c - DesignWare HS OTG Controller host-mode routines
> + *
> + * Copyright (C) 2004-2013 Synopsys, Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions, and the following disclaimer,
> + * without modification.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The names of the above-listed copyright holders may not be used
> + * to endorse or promote products derived from this software without
> + * specific prior written permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation; either version 2 of the License, or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> + * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
> + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +/*
> + * This file contains the core HCD code, and implements the Linux hc_driver
> + * API
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/interrupt.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +
> +#include <linux/usb/hcd.h>
> +#include <linux/usb/ch11.h>
> +
> +#include "core.h"
> +#include "hcd.h"
> +
> +#ifdef VERBOSE_DEBUG
could move this inside the function body so you can call it
unconditionally.
Also, add a comment stating that this should be called with irqs
disabled and lock held.
> +static void dwc2_dump_channel_info(struct dwc2_hsotg *hsotg,
> + struct dwc2_host_chan *chan)
> +{
> + int num_channels = hsotg->core_params->host_channels;
> + struct dwc2_qh *qh;
> + u32 hcchar;
> + u32 hcsplt;
> + u32 hctsiz;
> + u32 hc_dma;
> + int i;
> +
> + if (chan == NULL)
> + return;
> +
> + hcchar = readl(hsotg->regs + HCCHAR(chan->hc_num));
> + hcsplt = readl(hsotg->regs + HCSPLT(chan->hc_num));
> + hctsiz = readl(hsotg->regs + HCTSIZ(chan->hc_num));
> + hc_dma = readl(hsotg->regs + HCDMA(chan->hc_num));
> +
> + dev_dbg(hsotg->dev, " Assigned to channel %p:\n", chan);
> + dev_dbg(hsotg->dev, " hcchar 0x%08x, hcsplt 0x%08x\n",
> + hcchar, hcsplt);
> + dev_dbg(hsotg->dev, " hctsiz 0x%08x, hc_dma 0x%08x\n",
> + hctsiz, hc_dma);
> + dev_dbg(hsotg->dev, " dev_addr: %d, ep_num: %d, ep_is_in: %d\n",
> + chan->dev_addr, chan->ep_num, chan->ep_is_in);
> + dev_dbg(hsotg->dev, " ep_type: %d\n", chan->ep_type);
> + dev_dbg(hsotg->dev, " max_packet: %d\n", chan->max_packet);
> + dev_dbg(hsotg->dev, " data_pid_start: %d\n", chan->data_pid_start);
> + dev_dbg(hsotg->dev, " xfer_started: %d\n", chan->xfer_started);
> + dev_dbg(hsotg->dev, " halt_status: %d\n", chan->halt_status);
> + dev_dbg(hsotg->dev, " xfer_buf: %p\n", chan->xfer_buf);
> + dev_dbg(hsotg->dev, " xfer_dma: %08lx\n",
> + (unsigned long)chan->xfer_dma);
> + dev_dbg(hsotg->dev, " xfer_len: %d\n", chan->xfer_len);
> + dev_dbg(hsotg->dev, " qh: %p\n", chan->qh);
> + dev_dbg(hsotg->dev, " NP inactive sched:\n");
> + list_for_each_entry(qh, &hsotg->non_periodic_sched_inactive,
> + qh_list_entry)
> + dev_dbg(hsotg->dev, " %p\n", qh);
> + dev_dbg(hsotg->dev, " NP active sched:\n");
> + list_for_each_entry(qh, &hsotg->non_periodic_sched_active,
> + qh_list_entry)
> + dev_dbg(hsotg->dev, " %p\n", qh);
> + dev_dbg(hsotg->dev, " Channels:\n");
> + for (i = 0; i < num_channels; i++) {
> + struct dwc2_host_chan *chan = hsotg->hc_ptr_array[i];
> +
> + dev_dbg(hsotg->dev, " %2d: %p\n", i, chan);
> + }
> +}
> +#endif /* VERBOSE_DEBUG */
> +
> +/*
> + * Processes all the URBs in a single list of QHs. Completes them with
> + * -ETIMEDOUT and frees the QTD.
> + */
likewise, make sure to tell people that this should be called with lock
held and IRQs disabled.
> +static void dwc2_kill_urbs_in_qh_list(struct dwc2_hsotg *hsotg,
> + struct list_head *qh_list)
> +{
> + struct dwc2_qh *qh, *qh_tmp;
> + struct dwc2_qtd *qtd, *qtd_tmp;
> +
> + list_for_each_entry_safe(qh, qh_tmp, qh_list, qh_list_entry) {
> + list_for_each_entry_safe(qtd, qtd_tmp, &qh->qtd_list,
> + qtd_list_entry) {
> + if (qtd->urb != NULL) {
no need for the comparisson against NULL, but no strong feelings either.
> + dwc2_host_complete(hsotg, qtd->urb->priv,
> + qtd->urb, -ETIMEDOUT);
> + dwc2_hcd_qtd_remove_and_free(hsotg, qtd, qh);
> + }
> + }
> + }
> +}
> +
> +static void dwc2_qh_list_free(struct dwc2_hsotg *hsotg,
> + struct list_head *qh_list)
> +{
> + struct dwc2_qh *qh, *qh_tmp;
> + unsigned long flags;
> +
> + if (!qh_list->next)
> + /* The list hasn't been initialized yet */
> + return;
> +
> + /*
> + * Hold spinlock here. Not needed in that case if below function is
> + * being called from ISR.
> + */
> + spin_lock_irqsave(&hsotg->lock, flags);
> +
> + /* Ensure there are no QTDs or URBs left */
> + dwc2_kill_urbs_in_qh_list(hsotg, qh_list);
> + spin_unlock_irqrestore(&hsotg->lock, flags);
releasing the lock here is wrong and will likely cause quite some
headache if a race is triggered here. Looks like you have some
re-factoring to do.
Also, almost every single function related to qHs and qTDs is iterating
over those lists again and again and again. Eventually, if you have too
many of them, you will start to feel the pain :-p
I suggest going over this part of the code carefully and making sure you
can make the right assumptions (for example, assume that by the time
qh_remove_and_free() is called, you can assume all qHs to be empty or
something similar).
> + list_for_each_entry_safe(qh, qh_tmp, qh_list, qh_list_entry)
> + dwc2_hcd_qh_remove_and_free(hsotg, qh);
> +}
> +
> +/*
> + * Responds with an error status of -ETIMEDOUT to all URBs in the non-periodic
> + * and periodic schedules. The QTD associated with each URB is removed from
> + * the schedule and freed. This function may be called when a disconnect is
> + * detected or when the HCD is being stopped.
> + */
> +static void dwc2_kill_all_urbs(struct dwc2_hsotg *hsotg)
> +{
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->non_periodic_sched_inactive);
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->non_periodic_sched_active);
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->periodic_sched_inactive);
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->periodic_sched_ready);
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->periodic_sched_assigned);
> + dwc2_kill_urbs_in_qh_list(hsotg, &hsotg->periodic_sched_queued);
> +}
> +
> +/**
> + * dwc2_hcd_start() - Starts the HCD when switching to Host mode
> + *
> + * @hsotg: Pointer to struct dwc2_hsotg
> + */
> +void dwc2_hcd_start(struct dwc2_hsotg *hsotg)
> +{
> + u32 hprt0;
> +
> + if (hsotg->op_state == OTG_STATE_B_HOST) {
> + /*
> + * Reset the port. During a HNP mode switch the reset
> + * needs to occur within 1ms and have a duration of at
> + * least 50ms.
> + */
> + hprt0 = dwc2_read_hprt0(hsotg);
> + hprt0 |= HPRT0_RST;
> + writel(hprt0, hsotg->regs + HPRT0);
> + }
> +
> + queue_delayed_work(hsotg->wq_otg, &hsotg->start_work,
> + msecs_to_jiffies(50));
> +}
> +
> +static void dwc2_hcd_cleanup_channels(struct dwc2_hsotg *hsotg)
> +{
> + int num_channels = hsotg->core_params->host_channels;
> + struct dwc2_host_chan *channel;
> + u32 hcchar;
> + int i;
> +
> + if (hsotg->core_params->dma_enable <= 0) {
> + /* Flush out any channel requests in slave mode */
> + for (i = 0; i < num_channels; i++) {
> + channel = hsotg->hc_ptr_array[i];
> + if (list_empty(&channel->hc_list_entry)) {
> + hcchar = readl(hsotg->regs + HCCHAR(i));
> + if (hcchar & HCCHAR_CHENA) {
> + hcchar &= ~(HCCHAR_CHENA |
> + HCCHAR_EPDIR);
> + hcchar |= HCCHAR_CHDIS;
> + writel(hcchar, hsotg->regs +
> + HCCHAR(i));
> + }
> + }
> + }
> + }
please decrease some indentation levels here:
if (hsotg->core_params->dma_enable <= 0) {
/* Flush out any channel requests in slave mode */
for (i = 0; i < num_channels; i++) {
channel = hsotg->hc_ptr_array[i];
if (!list_empty(&channel->hc_list_entry))
continue;
hcchar = readl(hsotg->regs + HCCHAR(i));
if (hcchar & HCCHAR_CHENA)
continue;
hcchar &= ~(HCCHAR_CHENA |
HCCHAR_EPDIR);
hcchar |= HCCHAR_CHDIS;
writel(hcchar, hsotg->regs +
HCCHAR(i));
}
}
> + for (i = 0; i < num_channels; i++) {
> + channel = hsotg->hc_ptr_array[i];
> + if (list_empty(&channel->hc_list_entry)) {
> + hcchar = readl(hsotg->regs + HCCHAR(i));
> + if (hcchar & HCCHAR_CHENA) {
> + /* Halt the channel */
> + hcchar |= HCCHAR_CHDIS;
> + writel(hcchar, hsotg->regs + HCCHAR(i));
> + }
> +
> + dwc2_hc_cleanup(hsotg, channel);
> + list_add_tail(&channel->hc_list_entry,
> + &hsotg->free_hc_list);
> + /*
> + * Added for Descriptor DMA to prevent channel
> + * double cleanup in release_channel_ddma(),
> + * which is called from ep_disable when device
> + * disconnects
> + */
> + channel->qh = NULL;
> + }
> + }
likewise
> +void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg)
> +{
> + u32 intr;
> +
> + /* Set status flags for the hub driver */
> + hsotg->flags.b.port_connect_status_change = 1;
> + hsotg->flags.b.port_connect_status = 0;
> +
> + /*
> + * Shutdown any transfers in process by clearing the Tx FIFO Empty
> + * interrupt mask and status bits and disabling subsequent host
> + * channel interrupts.
> + */
> + intr = readl(hsotg->regs + GINTMSK);
> + intr &= ~(GINTSTS_NPTXFEMP | GINTSTS_PTXFEMP | GINTSTS_HCHINT);
> + writel(intr, hsotg->regs + GINTMSK);
> + intr = GINTSTS_NPTXFEMP | GINTSTS_PTXFEMP | GINTSTS_HCHINT;
> + writel(intr, hsotg->regs + GINTSTS);
> +
> + /*
> + * Turn off the vbus power only if the core has transitioned to device
> + * mode. If still in host mode, need to keep power on to detect a
> + * reconnection.
> + */
> + if (dwc2_is_device_mode(hsotg)) {
> + if (hsotg->op_state != OTG_STATE_A_SUSPEND) {
just a sidenote here: eventually this should be moved to a PHY driver.
> +static int dwc2_hcd_urb_dequeue(struct dwc2_hsotg *hsotg,
> + struct dwc2_hcd_urb *urb)
> +{
> + struct dwc2_qh *qh;
> + struct dwc2_qtd *urb_qtd;
> +
> + urb_qtd = urb->qtd;
> + if (!urb_qtd) {
> + dev_dbg(hsotg->dev, "## Urb QTD is NULL ##\n");
> + return 1;
errors are usually negative numbers.
> +static int dwc2_hcd_endpoint_disable(struct dwc2_hsotg *hsotg,
> + struct usb_host_endpoint *ep, int retry)
> +{
> + struct dwc2_qh *qh;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&hsotg->lock, flags);
> +
> + qh = ep->hcpriv;
> + if (!qh) {
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + return -EINVAL;
> + }
> +
> + while (!list_empty(&qh->qtd_list) && retry--) {
> + if (retry == 0) {
> + ep->hcpriv = NULL;
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + dev_err(hsotg->dev,
> + "## timeout in dwc2_hcd_endpoint_disable() ##\n");
> + return -EBUSY;
> + }
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> + usleep_range(20000, 40000);
> + spin_lock_irqsave(&hsotg->lock, flags);
> + }
> +
> + dwc2_hcd_qh_remove(hsotg, qh);
> + ep->hcpriv = NULL;
> + spin_unlock_irqrestore(&hsotg->lock, flags);
locking here is getting convoluted already, you release the lock in
multiple places, it might be better to figure out you if you grab it in
one place and release it in one place. Also, releasing the lock for that
usleep_range() creates a race. What if the endpoint gets reenabled when
release that lock ?
Perhaps using udelay() would be better in that case ?
> +static void dwc2_hcd_reinit(struct dwc2_hsotg *hsotg)
> +{
> + struct dwc2_host_chan *chan, *chan_tmp;
> + int num_channels;
> + int i;
> +
> + hsotg->flags.d32 = 0;
> +
> + hsotg->non_periodic_qh_ptr = &hsotg->non_periodic_sched_active;
> + hsotg->non_periodic_channels = 0;
> + hsotg->periodic_channels = 0;
> +
> + /*
> + * Put all channels in the free channel list and clean up channel
> + * states
> + */
> + list_for_each_entry_safe(chan, chan_tmp, &hsotg->free_hc_list,
> + hc_list_entry)
> + list_del_init(&chan->hc_list_entry);
not sure you need to initialize the list on every delete.
These comments summarize what needs to be changes all over the patch.
cheers
--
balbi
Attachment:
signature.asc
Description: Digital signature
