Skip to content

Commit

Permalink
usb/isp1760: Use polling instead of SOF interrupts to fix Errata 2
Browse files Browse the repository at this point in the history 
Errata 2 for the isp1760 explains that the chip sometimes does not issue
interrupts when an ATL (bulk or control) transfer is completed. There are
several issues with the current work-around (SOF interrupts) for this:

1) It seems the chip sometimes does not even set the done bit for a
   completed transfer, in which case SOF interrupts does not solve
   the problem since we still check the done map to find out which
   transfer descriptors to handle.

2) The above point seems to happen only when ATL and SOF interrupts
   are enabled at the same time. However, disabling ATL interrupts
   increases the latency between transfer completion and handling.
   This is very noticeable in the testusb suite, which take several
   minutes more to run with ATL interrupts disabled.

This patch removes the code to switch on SOF interrupts, and instead
use a kernel timer to periodically check for "old" descriptors that
have their VALID and ACTIVE flags unset, indicating completion, thus
avoiding the dependency on the chip's done map (and SOF interrupts)
to find transfers affected by this HW bug.

[bigeasy@linutronix: 80 lines limit]

Signed-off-by: Arvid Brodin <arvid.brodin@enea.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Arvid Brodin authored and Greg Kroah-Hartman committed Aug 22, 2011
1 parent 0ba7905 commit 6d50c60
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 45 deletions.
138 changes: 94 additions & 44 deletions drivers/usb/host/isp1760-hcd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <linux/uaccess.h>
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/timer.h>
#include <asm/unaligned.h>
#include <asm/cacheflush.h>

Expand All @@ -39,7 +40,6 @@ struct isp1760_hcd {
int int_done_map;
struct memory_chunk memory_pool[BLOCKS];
struct list_head controlqhs, bulkqhs, interruptqhs;
int active_ptds;

/* periodic schedule support */
#define DEFAULT_I_TDPS 1024
Expand Down Expand Up @@ -489,10 +489,6 @@ static int isp1760_hc_setup(struct usb_hcd *hcd)
16 : 32, (priv->devflags & ISP1760_FLAG_ANALOG_OC) ?
"analog" : "digital");

/* This is weird: at the first plug-in of a device there seems to be
one packet queued that never gets returned? */
priv->active_ptds = -1;

/* ATL reset */
reg_write32(hcd->regs, HC_HW_MODE_CTRL, hwmode | ALL_ATX_RESET);
mdelay(10);
Expand Down Expand Up @@ -741,8 +737,8 @@ static void start_bus_transfer(struct usb_hcd *hcd, u32 ptd_offset, int slot,
qh->slot = slot;
qtd->status = QTD_XFER_STARTED; /* Set this before writing ptd, since
interrupt routine may preempt and expects this value. */
slots[slot].timestamp = jiffies;
ptd_write(hcd->regs, ptd_offset, slot, ptd);
priv->active_ptds++;

/* Make sure done map has not triggered from some unlinked transfer */
if (ptd_offset == ATL_PTD_OFFSET) {
Expand Down Expand Up @@ -1091,11 +1087,9 @@ static int check_atl_transfer(struct usb_hcd *hcd, struct ptd *ptd,
return PTD_STATE_QTD_DONE;
}

static irqreturn_t isp1760_irq(struct usb_hcd *hcd)
static void handle_done_ptds(struct usb_hcd *hcd)
{
struct isp1760_hcd *priv = hcd_to_priv(hcd);
u32 imask;
irqreturn_t irqret = IRQ_NONE;
struct ptd ptd;
struct isp1760_qh *qh;
int slot;
Expand All @@ -1104,27 +1098,14 @@ static irqreturn_t isp1760_irq(struct usb_hcd *hcd)
u32 ptd_offset;
struct isp1760_qtd *qtd;
int modified;
static int last_active_ptds;
int int_skip_map, atl_skip_map;

spin_lock(&priv->lock);

if (!(hcd->state & HC_STATE_RUNNING))
goto leave;

imask = reg_read32(hcd->regs, HC_INTERRUPT_REG);
if (unlikely(!imask))
goto leave;
reg_write32(hcd->regs, HC_INTERRUPT_REG, imask); /* Clear */
int skip_map;

int_skip_map = reg_read32(hcd->regs, HC_INT_PTD_SKIPMAP_REG);
atl_skip_map = reg_read32(hcd->regs, HC_ATL_PTD_SKIPMAP_REG);
priv->int_done_map |= reg_read32(hcd->regs, HC_INT_PTD_DONEMAP_REG);
priv->atl_done_map |= reg_read32(hcd->regs, HC_ATL_PTD_DONEMAP_REG);
priv->int_done_map &= ~int_skip_map;
priv->atl_done_map &= ~atl_skip_map;
skip_map = reg_read32(hcd->regs, HC_INT_PTD_SKIPMAP_REG);
priv->int_done_map &= ~skip_map;
skip_map = reg_read32(hcd->regs, HC_ATL_PTD_SKIPMAP_REG);
priv->atl_done_map &= ~skip_map;

modified = priv->int_done_map | priv->atl_done_map;
modified = priv->int_done_map || priv->atl_done_map;

while (priv->int_done_map || priv->atl_done_map) {
if (priv->int_done_map) {
Expand Down Expand Up @@ -1163,7 +1144,6 @@ static irqreturn_t isp1760_irq(struct usb_hcd *hcd)
slots[slot].qtd = NULL;
qh = slots[slot].qh;
slots[slot].qh = NULL;
priv->active_ptds--;
qh->slot = -1;

WARN_ON(qtd->status != QTD_XFER_STARTED);
Expand Down Expand Up @@ -1234,22 +1214,28 @@ static irqreturn_t isp1760_irq(struct usb_hcd *hcd)

if (modified)
schedule_ptds(hcd);
}

/* ISP1760 Errata 2 explains that interrupts may be missed (or not
happen?) if two USB devices are running simultaneously. Perhaps
this happens when a PTD is finished during interrupt handling;
enable SOF interrupts if PTDs are still scheduled when exiting this
interrupt handler, just to be safe. */
static irqreturn_t isp1760_irq(struct usb_hcd *hcd)
{
struct isp1760_hcd *priv = hcd_to_priv(hcd);
u32 imask;
irqreturn_t irqret = IRQ_NONE;

if (priv->active_ptds != last_active_ptds) {
if (priv->active_ptds > 0)
reg_write32(hcd->regs, HC_INTERRUPT_ENABLE,
INTERRUPT_ENABLE_SOT_MASK);
else
reg_write32(hcd->regs, HC_INTERRUPT_ENABLE,
INTERRUPT_ENABLE_MASK);
last_active_ptds = priv->active_ptds;
}
spin_lock(&priv->lock);

if (!(hcd->state & HC_STATE_RUNNING))
goto leave;

imask = reg_read32(hcd->regs, HC_INTERRUPT_REG);
if (unlikely(!imask))
goto leave;
reg_write32(hcd->regs, HC_INTERRUPT_REG, imask); /* Clear */

priv->int_done_map |= reg_read32(hcd->regs, HC_INT_PTD_DONEMAP_REG);
priv->atl_done_map |= reg_read32(hcd->regs, HC_ATL_PTD_DONEMAP_REG);

handle_done_ptds(hcd);

irqret = IRQ_HANDLED;
leave:
Expand All @@ -1258,6 +1244,63 @@ static irqreturn_t isp1760_irq(struct usb_hcd *hcd)
return irqret;
}

/*
* Workaround for problem described in chip errata 2:
*
* Sometimes interrupts are not generated when ATL (not INT?) completion occurs.
* One solution suggested in the errata is to use SOF interrupts _instead_of_
* ATL done interrupts (the "instead of" might be important since it seems
* enabling ATL interrupts also causes the chip to sometimes - rarely - "forget"
* to set the PTD's done bit in addition to not generating an interrupt!).
*
* So if we use SOF + ATL interrupts, we sometimes get stale PTDs since their
* done bit is not being set. This is bad - it blocks the endpoint until reboot.
*
* If we use SOF interrupts only, we get latency between ptd completion and the
* actual handling. This is very noticeable in testusb runs which takes several
* minutes longer without ATL interrupts.
*
* A better solution is to run the code below every SLOT_CHECK_PERIOD ms. If it
* finds active ATL slots which are older than SLOT_TIMEOUT ms, it checks the
* slot's ACTIVE and VALID bits. If these are not set, the ptd is considered
* completed and its done map bit is set.
*
* The values of SLOT_TIMEOUT and SLOT_CHECK_PERIOD have been arbitrarily chosen
* not to cause too much lag when this HW bug occurs, while still hopefully
* ensuring that the check does not falsely trigger.
*/
#define SLOT_TIMEOUT 180
#define SLOT_CHECK_PERIOD 200
static struct timer_list errata2_timer;

void errata2_function(unsigned long data)
{
struct usb_hcd *hcd = (struct usb_hcd *) data;
struct isp1760_hcd *priv = hcd_to_priv(hcd);
int slot;
struct ptd ptd;
unsigned long spinflags;

spin_lock_irqsave(&priv->lock, spinflags);

for (slot = 0; slot < 32; slot++)
if ((priv->atl_slots[slot].qh || priv->atl_slots[slot].qtd) &&
time_after(jiffies + SLOT_TIMEOUT * HZ / 1000,
priv->atl_slots[slot].timestamp)) {
ptd_read(hcd->regs, ATL_PTD_OFFSET, slot, &ptd);
if (!FROM_DW0_VALID(ptd.dw0) &&
!FROM_DW3_ACTIVE(ptd.dw3))
priv->atl_done_map |= 1 << slot;
}

handle_done_ptds(hcd);

spin_unlock_irqrestore(&priv->lock, spinflags);

errata2_timer.expires = jiffies + SLOT_CHECK_PERIOD * HZ / 1000;
add_timer(&errata2_timer);
}

static int isp1760_run(struct usb_hcd *hcd)
{
int retval;
Expand Down Expand Up @@ -1303,6 +1346,12 @@ static int isp1760_run(struct usb_hcd *hcd)
if (retval)
return retval;

init_timer(&errata2_timer);
errata2_timer.function = errata2_function;
errata2_timer.data = (unsigned long) hcd;
errata2_timer.expires = jiffies + SLOT_CHECK_PERIOD * HZ / 1000;
add_timer(&errata2_timer);

chipid = reg_read32(hcd->regs, HC_CHIP_ID_REG);
dev_info(hcd->self.controller, "USB ISP %04x HW rev. %d started\n",
chipid & 0xffff, chipid >> 16);
Expand Down Expand Up @@ -1561,7 +1610,6 @@ static void kill_transfer(struct usb_hcd *hcd, struct urb *urb,
}

qh->slot = -1;
priv->active_ptds--;
}

static int isp1760_urb_dequeue(struct usb_hcd *hcd, struct urb *urb,
Expand Down Expand Up @@ -2012,6 +2060,8 @@ static void isp1760_stop(struct usb_hcd *hcd)
struct isp1760_hcd *priv = hcd_to_priv(hcd);
u32 temp;

del_timer(&errata2_timer);

isp1760_hub_control(hcd, ClearPortFeature, USB_PORT_FEAT_POWER, 1,
NULL, 0);
mdelay(20);
Expand Down
3 changes: 2 additions & 1 deletion drivers/usb/host/isp1760-hcd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ void deinit_kmem_cache(void);
#define HC_EOT_INT (1 << 3)
#define HC_SOT_INT (1 << 1)
#define INTERRUPT_ENABLE_MASK (HC_INTL_INT | HC_ATL_INT)
#define INTERRUPT_ENABLE_SOT_MASK (HC_SOT_INT)

#define HC_ISO_IRQ_MASK_OR_REG 0x318
#define HC_INT_IRQ_MASK_OR_REG 0x31C
Expand Down Expand Up @@ -107,6 +106,7 @@ struct ptd {
struct slotinfo {
struct isp1760_qh *qh;
struct isp1760_qtd *qtd;
unsigned long timestamp;
};


Expand Down Expand Up @@ -188,6 +188,7 @@ struct memory_chunk {
#define DW3_BABBLE_BIT (1 << 29)
#define DW3_HALT_BIT (1 << 30)
#define DW3_ACTIVE_BIT (1 << 31)
#define FROM_DW3_ACTIVE(x) (((x) >> 31) & 0x01)

#define INT_UNDERRUN (1 << 2)
#define INT_BABBLE (1 << 1)
Expand Down

0 comments on commit 6d50c60

Please sign in to comment.