The TX-interrupt fires (sometimes) too early and therefore we have the
early_tx timer to check periodically if the transfer is done. Initially
I started I started with a 150us delay which seemed to work well.
This value was reduced further, because depending on the usecase a
smaller poll interval was desired.
Instead of tunning the number further I hereby try to get rid of the
timer and instead use the TX-FIFO-empty interrupt. I've been playing
with it for a while and tried various things. Here is a small summary:
- Enable TX-empty interrupt "late"
The TX-empty interrupt would be enabled after "DMA IRQ" if the FIFO is
still full. This seems to work in generall but after removing the
debug code the TX-empty interrupt isn't generated.
- Use one of the two interrups
In general, the TX-empty interrupt comes after the DMA-interrupt. But
I've also seen it the other way around. So it not an option.
- Use both interrupts.
This patch.
For every TX-transfer we receive two interrupts: one from the DMA side
another from USB (FIFO empty). Once we seen both interrupts we consider
the USB-transfer as completed. The "downside" is that we have two
interrupts per TX-transfer. On the other hand we don't have the timer
anymore which may lead may to multiple timer-interrupts especially on
slow USB-disk media (where the device sends plenty of NAKs).
On the testing side:
- mass storage seems to play fine. The write perfomance is unchanged.
- g_zero test 12 gives sometimes "did not went well"
Testing is very welcome.
Signed-off-by: Sebastian Andrzej Siewior <bige...@linutronix.de>
---
drivers/usb/musb/musb_cppi41.c | 147 +++++++++++++++++------------------------
drivers/usb/musb/musb_dsps.c | 90 ++++---------------------
drivers/usb/musb/musb_dsps.h | 83 +++++++++++++++++++++++
3 files changed, 158 insertions(+), 162 deletions(-)
create mode 100644 drivers/usb/musb/musb_dsps.h
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index f64fd964dc6d..08846d05e671 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -6,6 +6,7 @@
#include <linux/of.h>
#include "musb_core.h"
+#include "musb_dsps.h"
#define RNDIS_REG(x) (0x80 + ((x - 1) * 4))
@@ -32,6 +33,7 @@ struct cppi41_dma_channel {
u8 is_tx;
u8 is_allocated;
u8 usb_toggle;
+ unsigned done_irqs;
dma_addr_t buf_addr;
u32 total_len;
@@ -49,8 +51,6 @@ struct cppi41_dma_controller {
struct cppi41_dma_channel rx_channel[MUSB_DMA_NUM_CHANNELS];
struct cppi41_dma_channel tx_channel[MUSB_DMA_NUM_CHANNELS];
struct musb *musb;
- struct hrtimer early_tx;
- struct list_head early_tx_list;
u32 rx_mode;
u32 tx_mode;
u32 auto_req;
@@ -184,40 +184,40 @@ static void cppi41_trans_done(struct cppi41_dma_channel
*cppi41_channel)
}
}
-static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
+static void glue_tx_fifo_empty_irq(struct musb *musb, u8 epnum, bool enable)
+{
+ struct device *dev = musb->controller;
+ struct platform_device *pdev = to_platform_device(dev->parent);
+ struct dsps_glue *glue = platform_get_drvdata(pdev);
+ const struct dsps_musb_wrapper *wrp = glue->wrp;
+ u32 ep_mask;
+ u32 reg;
+
+ ep_mask = 1 << (16 + epnum);
+ if (enable)
+ reg = wrp->coreintr_set;
+ else
+ reg = wrp->coreintr_clear;
+ musb_writel(musb->ctrl_base, reg, ep_mask);
+}
+
+static void cppi41_txep_complete(struct musb *musb, u8 epnum)
{
struct cppi41_dma_controller *controller;
- struct cppi41_dma_channel *cppi41_channel, *n;
- struct musb *musb;
- unsigned long flags;
- enum hrtimer_restart ret = HRTIMER_NORESTART;
+ struct cppi41_dma_channel *cppi41_channel;
- controller = container_of(timer, struct cppi41_dma_controller,
- early_tx);
- musb = controller->musb;
+ controller = container_of(musb->dma_controller,
+ struct cppi41_dma_controller, controller);
+ cppi41_channel = &controller->tx_channel[epnum - 1];
- spin_lock_irqsave(&musb->lock, flags);
- list_for_each_entry_safe(cppi41_channel, n, &controller->early_tx_list,
- tx_check) {
- bool empty;
- struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
-
- empty = musb_is_tx_fifo_empty(hw_ep);
- if (empty) {
- list_del_init(&cppi41_channel->tx_check);
- cppi41_trans_done(cppi41_channel);
- }
- }
-
- if (!list_empty(&controller->early_tx_list) &&
- !hrtimer_is_queued(&controller->early_tx)) {
- ret = HRTIMER_RESTART;
- hrtimer_forward_now(&controller->early_tx,
- ktime_set(0, 20 * NSEC_PER_USEC));
- }
+ cppi41_channel->done_irqs--;
+ if (cppi41_channel->done_irqs)
+ return;
- spin_unlock_irqrestore(&musb->lock, flags);
- return ret;
+ if (cppi41_channel->channel.status == MUSB_DMA_STATUS_BUSY)
+ cppi41_trans_done(cppi41_channel);
+ else
+ pr_err("%s(%d) oh no\n", __func__, __LINE__);
}
static void cppi41_dma_callback(void *private_data)
@@ -248,60 +248,17 @@ static void cppi41_dma_callback(void *private_data)
transferred < cppi41_channel->packet_sz)
cppi41_channel->prog_len = 0;
+ if (cppi41_channel->is_tx) {
+ cppi41_channel->done_irqs--;
+ if (cppi41_channel->done_irqs)
+ goto out;
+ }
+
empty = musb_is_tx_fifo_empty(hw_ep);
if (empty) {
cppi41_trans_done(cppi41_channel);
} else {
- struct cppi41_dma_controller *controller;
- int is_hs = 0;
- /*
- * On AM335x it has been observed that the TX interrupt fires
- * too early that means the TXFIFO is not yet empty but the DMA
- * engine says that it is done with the transfer. We don't
- * receive a FIFO empty interrupt so the only thing we can do is
- * to poll for the bit. On HS it usually takes 2us, on FS around
- * 110us - 150us depending on the transfer size.
- * We spin on HS (no longer than than 25us and setup a timer on
- * FS to check for the bit and complete the transfer.
- */
- controller = cppi41_channel->controller;
-
- if (is_host_active(musb)) {
- if (musb->port1_status & USB_PORT_STAT_HIGH_SPEED)
- is_hs = 1;
- } else {
- if (musb->g.speed == USB_SPEED_HIGH)
- is_hs = 1;
- }
- if (is_hs) {
- unsigned wait = 25;
-
- do {
- empty = musb_is_tx_fifo_empty(hw_ep);
- if (empty)
- break;
- wait--;
- if (!wait)
- break;
- udelay(1);
- } while (1);
-
- empty = musb_is_tx_fifo_empty(hw_ep);
- if (empty) {
- cppi41_trans_done(cppi41_channel);
- goto out;
- }
- }
- list_add_tail(&cppi41_channel->tx_check,
- &controller->early_tx_list);
- if (!hrtimer_is_queued(&controller->early_tx)) {
- unsigned long usecs = cppi41_channel->total_len / 10;
-
- hrtimer_start_range_ns(&controller->early_tx,
- ktime_set(0, usecs * NSEC_PER_USEC),
- 20 * NSEC_PER_USEC,
- HRTIMER_MODE_REL);
- }
+ pr_err("did not went well\n");
}
out:
spin_unlock_irqrestore(&musb->lock, flags);
@@ -390,8 +347,10 @@ static bool cppi41_configure_channel(struct dma_channel
*channel,
* Due to AM335x' Advisory 1.0.13 we are not allowed to transfer more
* than max packet size at a time.
*/
- if (cppi41_channel->is_tx)
+ if (cppi41_channel->is_tx) {
use_gen_rndis = 1;
+ cppi41_channel->done_irqs = 2;
+ }
if (use_gen_rndis) {
/* RNDIS mode */
@@ -461,6 +420,9 @@ static struct dma_channel
*cppi41_dma_channel_allocate(struct dma_controller *c,
cppi41_channel->hw_ep = hw_ep;
cppi41_channel->is_allocated = 1;
+ if (cppi41_channel->is_tx)
+ glue_tx_fifo_empty_irq(controller->musb,
+ cppi41_channel->hw_ep->epnum, true);
return &cppi41_channel->channel;
}
@@ -472,6 +434,10 @@ static void cppi41_dma_channel_release(struct dma_channel
*channel)
cppi41_channel->is_allocated = 0;
channel->status = MUSB_DMA_STATUS_FREE;
channel->actual_len = 0;
+ if (cppi41_channel->is_tx)
+ glue_tx_fifo_empty_irq(cppi41_channel->controller->musb,
+ cppi41_channel->hw_ep->epnum,
+ false);
}
}
@@ -544,6 +510,7 @@ static int cppi41_dma_channel_abort(struct dma_channel
*channel)
return 0;
list_del_init(&cppi41_channel->tx_check);
+ glue_tx_fifo_empty_irq(musb, cppi41_channel->hw_ep->epnum, false);
if (is_tx) {
csr = musb_readw(epio, MUSB_TXCSR);
csr &= ~MUSB_TXCSR_DMAENAB;
@@ -581,6 +548,7 @@ static int cppi41_dma_channel_abort(struct dma_channel
*channel)
}
cppi41_channel->channel.status = MUSB_DMA_STATUS_FREE;
+ glue_tx_fifo_empty_irq(musb, cppi41_channel->hw_ep->epnum, true);
return 0;
}
@@ -677,7 +645,6 @@ void dma_controller_destroy(struct dma_controller *c)
struct cppi41_dma_controller *controller = container_of(c,
struct cppi41_dma_controller, controller);
- hrtimer_cancel(&controller->early_tx);
cppi41_dma_controller_stop(controller);
kfree(controller);
}
@@ -685,6 +652,9 @@ void dma_controller_destroy(struct dma_controller *c)
struct dma_controller *dma_controller_create(struct musb *musb,
void __iomem *base)
{
+ struct device *dev = musb->controller;
+ struct platform_device *pdev = to_platform_device(dev->parent);
+ struct dsps_glue *glue = platform_get_drvdata(pdev);
struct cppi41_dma_controller *controller;
int ret = 0;
@@ -693,13 +663,18 @@ struct dma_controller *dma_controller_create(struct musb
*musb,
return NULL;
}
+ if (glue->id_magic != DSPS_ID_MAGIC) {
+ dev_err(musb->controller,
+ "Due to the early-TX workaround I need DSPS "
+ "platform.\n");
+ return NULL;
+ }
+ glue->tx_ep_fifo_empty = cppi41_txep_complete;
+
controller = kzalloc(sizeof(*controller), GFP_KERNEL);
if (!controller)
goto kzalloc_fail;
- hrtimer_init(&controller->early_tx, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
- controller->early_tx.function = cppi41_recheck_tx_req;
- INIT_LIST_HEAD(&controller->early_tx_list);
controller->musb = musb;
controller->controller.channel_alloc = cppi41_dma_channel_allocate;
diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 53bd0e71d19f..d49402c034d2 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -48,6 +48,7 @@
#include <linux/debugfs.h>
#include "musb_core.h"
+#include "musb_dsps.h"
static const struct of_device_id musb_dsps_of_match[];
@@ -75,82 +76,6 @@ static inline void dsps_writel(void __iomem *addr, unsigned
offset, u32 data)
__raw_writel(data, addr + offset);
}
-/**
- * DSPS musb wrapper register offset.
- * FIXME: This should be expanded to have all the wrapper registers from TI
DSPS
- * musb ips.
- */
-struct dsps_musb_wrapper {
- u16 revision;
- u16 control;
- u16 status;
- u16 epintr_set;
- u16 epintr_clear;
- u16 epintr_status;
- u16 coreintr_set;
- u16 coreintr_clear;
- u16 coreintr_status;
- u16 phy_utmi;
- u16 mode;
- u16 tx_mode;
- u16 rx_mode;
-
- /* bit positions for control */
- unsigned reset:5;
-
- /* bit positions for interrupt */
- unsigned usb_shift:5;
- u32 usb_mask;
- u32 usb_bitmap;
- unsigned drvvbus:5;
-
- unsigned txep_shift:5;
- u32 txep_mask;
- u32 txep_bitmap;
-
- unsigned rxep_shift:5;
- u32 rxep_mask;
- u32 rxep_bitmap;
-
- /* bit positions for phy_utmi */
- unsigned otg_disable:5;
-
- /* bit positions for mode */
- unsigned iddig:5;
- unsigned iddig_mux:5;
- /* miscellaneous stuff */
- u8 poll_seconds;
-};
-
-/*
- * register shadow for suspend
- */
-struct dsps_context {
- u32 control;
- u32 epintr;
- u32 coreintr;
- u32 phy_utmi;
- u32 mode;
- u32 tx_mode;
- u32 rx_mode;
-};
-
-/**
- * DSPS glue structure.
- */
-struct dsps_glue {
- struct device *dev;
- struct platform_device *musb; /* child musb pdev */
- const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
- struct timer_list timer; /* otg_workaround timer */
- unsigned long last_timer; /* last timer data for each instance */
- bool sw_babble_enabled;
-
- struct dsps_context context;
- struct debugfs_regset32 regset;
- struct dentry *dbgfs_root;
-};
-
static const struct debugfs_reg32 dsps_musb_regs[] = {
{ "revision", 0x00 },
{ "control", 0x14 },
@@ -398,6 +323,18 @@ static irqreturn_t dsps_interrupt(int irq, void *hci)
ret = IRQ_HANDLED;
}
+ if (usbintr >> 16) {
+ u32 tx_ep_mask;
+ u8 ep;
+
+ tx_ep_mask = usbintr >> 16;
+ while (tx_ep_mask) {
+ ep = __fls(tx_ep_mask);
+ tx_ep_mask &= ~(1 << ep);
+ glue->tx_ep_fifo_empty(musb, ep);
+ }
+ }
+
if (musb->int_tx || musb->int_rx || musb->int_usb)
ret |= musb_interrupt(musb);
@@ -783,6 +720,7 @@ static int dsps_probe(struct platform_device *pdev)
glue->dev = &pdev->dev;
glue->wrp = wrp;
+ glue->id_magic = DSPS_ID_MAGIC;
platform_set_drvdata(pdev, glue);
pm_runtime_enable(&pdev->dev);
diff --git a/drivers/usb/musb/musb_dsps.h b/drivers/usb/musb/musb_dsps.h
new file mode 100644
index 000000000000..192c0ac6af39
--- /dev/null
+++ b/drivers/usb/musb/musb_dsps.h
@@ -0,0 +1,83 @@
+#ifndef _MUSB_DSPS_H__
+#define _MUSB_DSPS_H__
+#include <linux/debugfs.h>
+
+/**
+ * DSPS musb wrapper register offset.
+ * FIXME: This should be expanded to have all the wrapper registers from TI
DSPS
+ * musb ips.
+ */
+struct dsps_musb_wrapper {
+ u16 revision;
+ u16 control;
+ u16 status;
+ u16 epintr_set;
+ u16 epintr_clear;
+ u16 epintr_status;
+ u16 coreintr_set;
+ u16 coreintr_clear;
+ u16 coreintr_status;
+ u16 phy_utmi;
+ u16 mode;
+ u16 tx_mode;
+ u16 rx_mode;
+
+ /* bit positions for control */
+ unsigned reset:5;
+
+ /* bit positions for interrupt */
+ unsigned usb_shift:5;
+ u32 usb_mask;
+ u32 usb_bitmap;
+ unsigned drvvbus:5;
+
+ unsigned txep_shift:5;
+ u32 txep_mask;
+ u32 txep_bitmap;
+
+ unsigned rxep_shift:5;
+ u32 rxep_mask;
+ u32 rxep_bitmap;
+
+ /* bit positions for phy_utmi */
+ unsigned otg_disable:5;
+
+ /* bit positions for mode */
+ unsigned iddig:5;
+ unsigned iddig_mux:5;
+ /* miscellaneous stuff */
+ u8 poll_seconds;
+};
+/*
+ * register shadow for suspend
+ */
+struct dsps_context {
+ u32 control;
+ u32 epintr;
+ u32 coreintr;
+ u32 phy_utmi;
+ u32 mode;
+ u32 tx_mode;
+ u32 rx_mode;
+};
+
+#define DSPS_ID_MAGIC 0x29788445
+/**
+ * DSPS glue structure.
+ */
+struct dsps_glue {
+ unsigned int id_magic;
+ struct device *dev;
+ struct platform_device *musb; /* child musb pdev */
+ const struct dsps_musb_wrapper *wrp; /* wrapper register offsets */
+ struct timer_list timer; /* otg_workaround timer */
+ unsigned long last_timer; /* last timer data for each instance */
+ bool sw_babble_enabled;
+ void (*tx_ep_fifo_empty)(struct musb *musb, u8 epnum);
+
+ struct dsps_context context;
+ struct debugfs_regset32 regset;
+ struct dentry *dbgfs_root;
+};
+
+#endif
--
2.1.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
