[PATCH v2 11/13] usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled

[Gregory Herrero]

Use Streaming DMA mappings to handle cache coherency of frame list and
descriptor list. Cache are always flushed before controller access it
or before cpu access it.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/core.c |   3 ++
 drivers/usb/dwc2/core.h |   2 +
 drivers/usb/dwc2/hcd.c  |   4 +-
 drivers/usb/dwc2/hcd.h  |   4 ++
 drivers/usb/dwc2/hcd_ddma.c | 106 ++--
 5 files changed, 94 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 5568d9c..542c9e6 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1934,6 +1934,9 @@ void dwc2_hc_start_transfer_ddma(struct dwc2_hsotg *hsotg,
 
dwc2_writel(hctsiz, hsotg->regs + HCTSIZ(chan->hc_num));
 
+   dma_sync_single_for_device(hsotg->dev, chan->desc_list_addr,
+  chan->desc_list_sz, DMA_TO_DEVICE);
+
hc_dma = (u32)chan->desc_list_addr & HCDMA_DMA_ADDR_MASK;
 
/* Always start from first descriptor */
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index fd4c236..e7cc542 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -685,6 +685,7 @@ struct dwc2_hregs_backup {
  * @otg_port:   OTG port number
  * @frame_list: Frame list
  * @frame_list_dma: Frame list DMA address
+ * @frame_list_sz:  Frame list size
  *
  * These are for peripheral mode:
  *
@@ -804,6 +805,7 @@ struct dwc2_hsotg {
u8 otg_port;
u32 *frame_list;
dma_addr_t frame_list_dma;
+   u32 frame_list_sz;
 
 #ifdef DEBUG
u32 frrem_samples;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 45b1c6a..03cbca3 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -880,8 +880,10 @@ static int dwc2_assign_and_init_hc(struct dwc2_hsotg 
*hsotg, struct dwc2_qh *qh)
 */
chan->multi_count = dwc2_hb_mult(qh->maxp);
 
-   if (hsotg->core_params->dma_desc_enable > 0)
+   if (hsotg->core_params->dma_desc_enable > 0) {
chan->desc_list_addr = qh->desc_list_dma;
+   chan->desc_list_sz = qh->desc_list_sz;
+   }
 
dwc2_hc_init(hsotg, chan);
chan->qh = qh;
diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index 2e5e9d9..6e82266 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -107,6 +107,7 @@ struct dwc2_qh;
  * @qh: QH for the transfer being processed by this channel
  * @hc_list_entry:  For linking to list of host channels
  * @desc_list_addr: Current QH's descriptor list DMA address
+ * @desc_list_sz:   Current QH's descriptor list size
  *
  * This structure represents the state of a single host channel when acting in
  * host mode. It contains the data items needed to transfer packets to an
@@ -159,6 +160,7 @@ struct dwc2_host_chan {
struct dwc2_qh *qh;
struct list_head hc_list_entry;
dma_addr_t desc_list_addr;
+   u32 desc_list_sz;
 };
 
 struct dwc2_hcd_pipe_info {
@@ -251,6 +253,7 @@ enum dwc2_transaction_type {
  *  schedule
  * @desc_list:  List of transfer descriptors
  * @desc_list_dma:  Physical address of desc_list
+ * @desc_list_sz:   Size of descriptors list
  * @n_bytes:Xfer Bytes array. Each element corresponds to a 
transfer
  *  descriptor and indicates original XferSize value for 
the
  *  descriptor
@@ -284,6 +287,7 @@ struct dwc2_qh {
struct list_head qh_list_entry;
struct dwc2_hcd_dma_desc *desc_list;
dma_addr_t desc_list_dma;
+   u32 desc_list_sz;
u32 *n_bytes;
unsigned tt_buffer_dirty:1;
 };
diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index f98c7e91..85d7816 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -87,22 +87,23 @@ static u16 dwc2_frame_incr_val(struct dwc2_qh *qh)
 static int dwc2_desc_list_alloc(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
gfp_t flags)
 {
-   qh->desc_list = dma_alloc_coherent(hsotg->dev,
-   sizeof(struct dwc2_hcd_dma_desc) *
-   dwc2_max_desc_num(qh), >desc_list_dma,
-   flags);
+   qh->desc_list_sz = sizeof(struct dwc2_hcd_dma_desc) *
+   dwc2_max_desc_num(qh);
 
+   qh->desc_list = kzalloc(qh->desc_list_sz, flags | GFP_DMA);
if (!qh->desc_list)
return -ENOMEM;
 
-   memset(qh->desc_list, 0,
-  sizeof(struct dwc2_hcd_dma_desc) * dwc2_max_desc_num(qh));
+   qh->desc_list_dma = dma_map_single(hsotg->dev, qh->desc_list,
+  qh->desc_list_sz,
+  DMA_TO_DEVICE);
 
qh->n_bytes = 

[PATCH v2 06/13] usb: dwc2: host: add function to compare frame index

[Gregory Herrero]

This function allow comparing frame index used for
descriptor list which has 64 entries.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index f105bad..a19837f 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -535,6 +535,19 @@ static inline bool dbg_perio(void) { return false; }
 #define dwc2_max_packet(wmaxpacketsize) ((wmaxpacketsize) & 0x07ff)
 
 /*
+ * Returns true if frame1 index is greater than frame2 index. The comparison
+ * is done modulo FRLISTEN_64_SIZE. This accounts for the rollover of the
+ * frame number when the max index frame number is reached.
+ */
+static inline bool dwc2_frame_idx_num_gt(u16 fr_idx1, u16 fr_idx2)
+{
+   u16 diff = fr_idx1 - fr_idx2;
+   u16 sign = diff & (FRLISTEN_64_SIZE >> 1);
+
+   return diff && !sign;
+}
+
+/*
  * Returns true if frame1 is less than or equal to frame2. The comparison is
  * done modulo HFNUM_MAX_FRNUM. This accounts for the rollover of the
  * frame number when the max frame number is reached.
-- 
2.6.2

--
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

[PATCH v2 10/13] usb: dwc2: host: enable descriptor dma for fs devices

[Gregory Herrero]

From: Mian Yousaf Kaukab 

As descriptor dma mode does not support split transfers, it can't be
enabled for high speed devices. Add a core parameter to enable it for
full speed devices.

Ensure frame list and descriptor list are correctly freed during
disconnect.

Signed-off-by: Mian Yousaf Kaukab 
Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/core.c  | 24 
 drivers/usb/dwc2/core.h  | 20 
 drivers/usb/dwc2/hcd.c   | 22 ++
 drivers/usb/dwc2/hcd_intr.c  | 15 +--
 drivers/usb/dwc2/hcd_queue.c |  2 +-
 drivers/usb/dwc2/platform.c  |  4 
 6 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index ef73e49..5568d9c 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -2485,6 +2485,29 @@ void dwc2_set_param_dma_desc_enable(struct dwc2_hsotg 
*hsotg, int val)
hsotg->core_params->dma_desc_enable = val;
 }
 
+void dwc2_set_param_dma_desc_fs_enable(struct dwc2_hsotg *hsotg, int val)
+{
+   int valid = 1;
+
+   if (val > 0 && (hsotg->core_params->dma_enable <= 0 ||
+   !hsotg->hw_params.dma_desc_enable))
+   valid = 0;
+   if (val < 0)
+   valid = 0;
+
+   if (!valid) {
+   if (val >= 0)
+   dev_err(hsotg->dev,
+   "%d invalid for dma_desc_fs_enable parameter. 
Check HW configuration.\n",
+   val);
+   val = (hsotg->core_params->dma_enable > 0 &&
+   hsotg->hw_params.dma_desc_enable);
+   }
+
+   hsotg->core_params->dma_desc_fs_enable = val;
+   dev_dbg(hsotg->dev, "Setting dma_desc_fs_enable to %d\n", val);
+}
+
 void dwc2_set_param_host_support_fs_ls_low_power(struct dwc2_hsotg *hsotg,
 int val)
 {
@@ -3016,6 +3039,7 @@ void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
dwc2_set_param_otg_cap(hsotg, params->otg_cap);
dwc2_set_param_dma_enable(hsotg, params->dma_enable);
dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
+   dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
dwc2_set_param_host_support_fs_ls_low_power(hsotg,
params->host_support_fs_ls_low_power);
dwc2_set_param_enable_dynamic_fifo(hsotg,
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a66d3cb..fd4c236 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -246,6 +246,13 @@ enum dwc2_ep0_state {
  *  value for this if none is specified.
  *   0 - Address DMA
  *   1 - Descriptor DMA (default, if available)
+ * @dma_desc_fs_enable: When DMA mode is enabled, specifies whether to use
+ *  address DMA mode or descriptor DMA mode for accessing
+ *  the data FIFOs in Full Speed mode only. The driver
+ *  will automatically detect the value for this if none is
+ *  specified.
+ *   0 - Address DMA
+ *   1 - Descriptor DMA in FS (default, if available)
  * @speed:  Specifies the maximum speed of operation in host and
  *  device mode. The actual speed depends on the speed of
  *  the attached device and the value of phy_type.
@@ -375,6 +382,7 @@ struct dwc2_core_params {
int otg_ver;
int dma_enable;
int dma_desc_enable;
+   int dma_desc_fs_enable;
int speed;
int enable_dynamic_fifo;
int en_multiple_tx_fifo;
@@ -456,6 +464,7 @@ struct dwc2_hw_params {
unsigned op_mode:3;
unsigned arch:2;
unsigned dma_desc_enable:1;
+   unsigned dma_desc_fs_enable:1;
unsigned enable_dynamic_fifo:1;
unsigned en_multiple_tx_fifo:1;
unsigned host_rx_fifo_size:16;
@@ -770,6 +779,7 @@ struct dwc2_hsotg {
u16 frame_number;
u16 periodic_qh_count;
bool bus_suspended;
+   bool new_connection;
 
 #ifdef CONFIG_USB_DWC2_TRACK_MISSED_SOFS
 #define FRAME_NUM_ARRAY_SIZE 1000
@@ -942,6 +952,16 @@ extern void dwc2_set_param_dma_enable(struct dwc2_hsotg 
*hsotg, int val);
 extern void dwc2_set_param_dma_desc_enable(struct dwc2_hsotg *hsotg, int val);
 
 /*
+ * When DMA mode is enabled specifies whether to use
+ * address DMA or DMA Descritor mode with full speed devices
+ * for accessing the data FIFOs in host mode.
+ * 0 - address DMA
+ * 1 - FS DMA Descriptor(default, if available)
+ */
+extern void dwc2_set_param_dma_desc_fs_enable(struct dwc2_hsotg *hsotg,
+ int val);
+
+/*
  * Specifies the maximum speed of operation in host and device mode.
  * The actual speed 

[PATCH v2 04/13] usb: dwc2: host: fix use of qtd after free in desc dma mode

[Gregory Herrero]

When completing non isoc xfer, dwc2_complete_non_isoc_xfer_ddma()
is relying on qtd->n_desc to process the corresponding number of
descriptors.

During the processing of these descriptors, qtd could be unlinked
and freed if xfer is done and urb is no more in progress.

In this case, dwc2_complete_non_isoc_xfer_ddma() will read again
qtd->n_desc whereas qtd has been freed. This will lead to unpredictable
results since qtd->n_desc is no more valid value.

To avoid this error, return a result != 0 in dwc2_process_non_isoc_desc(),
so that dwc2_complete_non_isoc_xfer_ddma() stops desc processing.

This has been seen with Slub debug enabled.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 5f72656..4801e69 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -1033,7 +1033,10 @@ static int dwc2_process_non_isoc_desc(struct dwc2_hsotg 
*hsotg,
failed = dwc2_update_non_isoc_urb_state_ddma(hsotg, chan, qtd, dma_desc,
 halt_status, n_bytes,
 xfer_done);
-   if (failed || (*xfer_done && urb->status != -EINPROGRESS)) {
+   if (*xfer_done && urb->status != -EINPROGRESS)
+   failed = 1;
+
+   if (failed) {
dwc2_host_complete(hsotg, qtd, urb->status);
dwc2_hcd_qtd_unlink_and_free(hsotg, qtd, qh);
dev_vdbg(hsotg->dev, "failed=%1x xfer_done=%1x status=%08x\n",
-- 
2.6.2

--
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

[PATCH v2 05/13] usb: dwc2: host: spinlock release channel

[Gregory Herrero]

Prevent dwc2 driver from accessing channel while it frees it.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 4801e69..a76a58c 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -360,6 +360,8 @@ err0:
  */
 void dwc2_hcd_qh_free_ddma(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh)
 {
+   unsigned long flags;
+
dwc2_desc_list_free(hsotg, qh);
 
/*
@@ -369,8 +371,10 @@ void dwc2_hcd_qh_free_ddma(struct dwc2_hsotg *hsotg, 
struct dwc2_qh *qh)
 * when it comes here from endpoint disable routine
 * channel remains assigned.
 */
+   spin_lock_irqsave(>lock, flags);
if (qh->channel)
dwc2_release_channel_ddma(hsotg, qh);
+   spin_unlock_irqrestore(>lock, flags);
 
if ((qh->ep_type == USB_ENDPOINT_XFER_ISOC ||
 qh->ep_type == USB_ENDPOINT_XFER_INT) &&
-- 
2.6.2

--
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

[PATCH v2 09/13] usb: dwc2: host: process all completed urbs

[Gregory Herrero]

Process all completed urbs, if more urbs are complete by the time
driver processes completion interrupt.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index edccac6..f98c7e91 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -940,17 +940,51 @@ static void dwc2_complete_isoc_xfer_ddma(struct 
dwc2_hsotg *hsotg,
list_for_each_entry_safe(qtd, qtd_tmp, >qtd_list, qtd_list_entry) {
if (!qtd->in_process)
break;
+
+   /*
+* Ensure idx corresponds to descriptor where first urb of this
+* qtd was added. In fact, during isoc desc init, dwc2 may skip
+* an index if current frame number is already over this index.
+*/
+   if (idx != qtd->isoc_td_first) {
+   dev_vdbg(hsotg->dev,
+"try to complete %d instead of %d\n",
+idx, qtd->isoc_td_first);
+   idx = qtd->isoc_td_first;
+   }
+
do {
+   struct dwc2_qtd *qtd_next;
+   u16 cur_idx;
+
rc = dwc2_cmpl_host_isoc_dma_desc(hsotg, chan, qtd, qh,
  idx);
if (rc < 0)
return;
idx = dwc2_desclist_idx_inc(idx, qh->interval,
chan->speed);
-   if (rc == DWC2_CMPL_STOP)
-   goto stop_scan;
+   if (!rc)
+   continue;
+
if (rc == DWC2_CMPL_DONE)
break;
+
+   /* rc == DWC2_CMPL_STOP */
+
+   if (qh->interval >= 32)
+   goto stop_scan;
+
+   qh->td_first = idx;
+   cur_idx = dwc2_frame_list_idx(hsotg->frame_number);
+   qtd_next = list_first_entry(>qtd_list,
+   struct dwc2_qtd,
+   qtd_list_entry);
+   if (dwc2_frame_idx_num_gt(cur_idx,
+ qtd_next->isoc_td_last))
+   break;
+
+   goto stop_scan;
+
} while (idx != qh->td_first);
}
 
-- 
2.6.2

--
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

[PATCH v2 12/13] usb: dwc2: host: fix descriptor list address masking

[Gregory Herrero]

From: Mian Yousaf Kaukab 

Masks for HCDMA.CTD and HCDMA.DMAAddr are incorrect. As we always
start from first descriptor, no need to mask the address anyway.

Signed-off-by: Mian Yousaf Kaukab 
Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/core.c | 10 +++---
 drivers/usb/dwc2/hw.h   |  4 
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 542c9e6..97de855 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1905,7 +1905,6 @@ void dwc2_hc_start_transfer_ddma(struct dwc2_hsotg *hsotg,
 struct dwc2_host_chan *chan)
 {
u32 hcchar;
-   u32 hc_dma;
u32 hctsiz = 0;
 
if (chan->do_ping)
@@ -1937,14 +1936,11 @@ void dwc2_hc_start_transfer_ddma(struct dwc2_hsotg 
*hsotg,
dma_sync_single_for_device(hsotg->dev, chan->desc_list_addr,
   chan->desc_list_sz, DMA_TO_DEVICE);
 
-   hc_dma = (u32)chan->desc_list_addr & HCDMA_DMA_ADDR_MASK;
+   dwc2_writel(chan->desc_list_addr, hsotg->regs + HCDMA(chan->hc_num));
 
-   /* Always start from first descriptor */
-   hc_dma &= ~HCDMA_CTD_MASK;
-   dwc2_writel(hc_dma, hsotg->regs + HCDMA(chan->hc_num));
if (dbg_hc(chan))
-   dev_vdbg(hsotg->dev, "Wrote %08x to HCDMA(%d)\n",
-hc_dma, chan->hc_num);
+   dev_vdbg(hsotg->dev, "Wrote %pad to HCDMA(%d)\n",
+>desc_list_addr, chan->hc_num);
 
hcchar = dwc2_readl(hsotg->regs + HCCHAR(chan->hc_num));
hcchar &= ~HCCHAR_MULTICNT_MASK;
diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 553f246..281b57b 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -769,10 +769,6 @@
 #define TSIZ_XFERSIZE_SHIFT0
 
 #define HCDMA(_ch) HSOTG_REG(0x0514 + 0x20 * (_ch))
-#define HCDMA_DMA_ADDR_MASK(0x1f << 11)
-#define HCDMA_DMA_ADDR_SHIFT   11
-#define HCDMA_CTD_MASK (0xff << 3)
-#define HCDMA_CTD_SHIFT3
 
 #define HCDMAB(_ch)HSOTG_REG(0x051c + 0x20 * (_ch))
 
-- 
2.6.2

--
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

[PATCH v2 03/13] usb: dwc2: host: rework isochronous halt path

[Gregory Herrero]

When a channel is halted because of urb dequeue during transfer
completion, no other qtds must be scheduled until halt is done.
Moreover, all in progress qtds must be given back.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 98d8627..5f72656 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -1161,6 +1161,21 @@ void dwc2_hcd_complete_xfer_ddma(struct dwc2_hsotg 
*hsotg,
/* Release the channel if halted or session completed */
if (halt_status != DWC2_HC_XFER_COMPLETE ||
list_empty(>qtd_list)) {
+   struct dwc2_qtd *qtd, *qtd_tmp;
+
+   /*
+* Kill all remainings QTDs since channel has been
+* halted.
+*/
+   list_for_each_entry_safe(qtd, qtd_tmp,
+>qtd_list,
+qtd_list_entry) {
+   dwc2_host_complete(hsotg, qtd,
+  -ECONNRESET);
+   dwc2_hcd_qtd_unlink_and_free(hsotg,
+qtd, qh);
+   }
+
/* Halt the channel if session completed */
if (halt_status == DWC2_HC_XFER_COMPLETE)
dwc2_hc_halt(hsotg, chan, halt_status);
@@ -1170,7 +1185,12 @@ void dwc2_hcd_complete_xfer_ddma(struct dwc2_hsotg 
*hsotg,
/* Keep in assigned schedule to continue transfer */
list_move(>qh_list_entry,
  >periodic_sched_assigned);
-   continue_isoc_xfer = 1;
+   /*
+* If channel has been halted during giveback of urb
+* then prevent any new scheduling.
+*/
+   if (!chan->halt_status)
+   continue_isoc_xfer = 1;
}
/*
 * Todo: Consider the case when period exceeds FrameList size.
-- 
2.6.2

--
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

[PATCH v2 01/13] usb: dwc2: host: ensure filling of isoc desc is correctly done

[Gregory Herrero]

Increment qtd->isoc_frame_index_last before testing it, else below
check will never be true and IOC (Interrupt On Complete) bit for
last frame will never be set in descriptor status.

  /* Set IOC for each descriptor corresponding to last frame of URB */
  if (qtd->isoc_frame_index_last == qtd->urb->packet_count)
dma_desc->status |= HOST_DMA_IOC;

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 78993ab..4b0be93 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -524,14 +524,15 @@ static void dwc2_fill_host_isoc_dma_desc(struct 
dwc2_hsotg *hsotg,
dma_desc->status = qh->n_bytes[idx] << HOST_DMA_ISOC_NBYTES_SHIFT &
   HOST_DMA_ISOC_NBYTES_MASK;
 
+   qh->ntd++;
+   qtd->isoc_frame_index_last++;
+
 #ifdef ISOC_URB_GIVEBACK_ASAP
/* Set IOC for each descriptor corresponding to last frame of URB */
if (qtd->isoc_frame_index_last == qtd->urb->packet_count)
dma_desc->status |= HOST_DMA_IOC;
 #endif
 
-   qh->ntd++;
-   qtd->isoc_frame_index_last++;
 }
 
 static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg *hsotg,
-- 
2.6.2

--
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

[PATCH v2 07/13] usb: dwc2: host: program descriptor for next frame

[Gregory Herrero]

Isochronous descriptor is currently programmed for the frame
after the last descriptor was programmed.
If the last descriptor frame underrun, then current descriptor must
take this into account and must be programmed on the current frame + 1.
This overrun usually happens when system is loaded and dwc2 can't init
descriptor list in time.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd.h  |  2 ++
 drivers/usb/dwc2/hcd_ddma.c | 32 ++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
index a19837f..2e5e9d9 100644
--- a/drivers/usb/dwc2/hcd.h
+++ b/drivers/usb/dwc2/hcd.h
@@ -340,6 +340,8 @@ struct dwc2_qtd {
u8 isoc_split_pos;
u16 isoc_frame_index;
u16 isoc_split_offset;
+   u16 isoc_td_last;
+   u16 isoc_td_first;
u32 ssplit_out_xfer_count;
u8 error_count;
u8 n_desc;
diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index a76a58c..9635d8d 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -547,11 +547,32 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
 {
struct dwc2_qtd *qtd;
u32 max_xfer_size;
-   u16 idx, inc, n_desc, ntd_max = 0;
+   u16 idx, inc, n_desc = 0, ntd_max = 0;
+   u16 cur_idx;
+   u16 next_idx;
 
idx = qh->td_last;
inc = qh->interval;
-   n_desc = 0;
+   hsotg->frame_number = dwc2_hcd_get_frame_number(hsotg);
+   cur_idx = dwc2_frame_list_idx(hsotg->frame_number);
+   next_idx = dwc2_desclist_idx_inc(qh->td_last, inc, qh->dev_speed);
+
+   /*
+* Ensure current frame number didn't overstep last scheduled
+* descriptor. If it happens, the only way to recover is to move
+* qh->td_last to current frame number + 1.
+* So that next isoc descriptor will be scheduled on frame number + 1
+* and not on a past frame.
+*/
+   if (dwc2_frame_idx_num_gt(cur_idx, next_idx) || (cur_idx == next_idx)) {
+   if (inc < 32) {
+   dev_vdbg(hsotg->dev,
+"current frame number overstep last 
descriptor\n");
+   qh->td_last = dwc2_desclist_idx_inc(cur_idx, inc,
+   qh->dev_speed);
+   idx = qh->td_last;
+   }
+   }
 
if (qh->interval) {
ntd_max = (dwc2_max_desc_num(qh) + qh->interval - 1) /
@@ -564,6 +585,12 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
MAX_ISOC_XFER_SIZE_HS : MAX_ISOC_XFER_SIZE_FS;
 
list_for_each_entry(qtd, >qtd_list, qtd_list_entry) {
+   if (qtd->in_process &&
+   qtd->isoc_frame_index_last ==
+   qtd->urb->packet_count)
+   continue;
+
+   qtd->isoc_td_first = idx;
while (qh->ntd < ntd_max && qtd->isoc_frame_index_last <
qtd->urb->packet_count) {
dwc2_fill_host_isoc_dma_desc(hsotg, qtd, qh,
@@ -571,6 +598,7 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
idx = dwc2_desclist_idx_inc(idx, inc, qh->dev_speed);
n_desc++;
}
+   qtd->isoc_td_last = idx;
qtd->in_process = 1;
}
 
-- 
2.6.2

--
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

[PATCH v2 00/13] usb: dwc2: descriptor dma mode bug fixes

[Gregory Herrero]

Hi,

This patchset contains bug fixes for host descriptor dma mode.

Descriptor dma mode can't be used as the default mode since controller
does not support split transfers in this mode.
So we add a new configuration parameter which allows descriptor dma mode
to be enabled for full-speed devices only.

All patches are verified on dwc2 v3.0a with dedicated fifos and our
main test target was usb audio devices.

All patches are based on Felipe's testing/next branch.

Regards,
Gregory

History:
v2:
  - Use dma cache in "usb: dwc2: host: use kmem cache to allocate descriptors"
and replace usage of deprecated GFP_DMA32 flag.
  - Remove "usb: dwc2: host: free status_buf on hcd de-init".

v1:
  - Fix compilation error introduced by:
"usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled"
  - Fix warning introduced by:
"usb: dwc2: host: fix descriptor list address masking"

Gregory Herrero (11):
  usb: dwc2: host: ensure filling of isoc desc is correctly done
  usb: dwc2: host: set active bit in isochronous descriptors
  usb: dwc2: host: rework isochronous halt path
  usb: dwc2: host: fix use of qtd after free in desc dma mode
  usb: dwc2: host: spinlock release channel
  usb: dwc2: host: add function to compare frame index
  usb: dwc2: host: program descriptor for next frame
  usb: dwc2: host: always increment available host channel during
release
  usb: dwc2: host: process all completed urbs
  usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled
  usb: dwc2: host: use kmem cache to allocate descriptors

Mian Yousaf Kaukab (2):
  usb: dwc2: host: enable descriptor dma for fs devices
  usb: dwc2: host: fix descriptor list address masking

 drivers/usb/dwc2/core.c  |  37 +--
 drivers/usb/dwc2/core.h  |  26 +
 drivers/usb/dwc2/hcd.c   |  76 +-
 drivers/usb/dwc2/hcd.h   |  19 
 drivers/usb/dwc2/hcd_ddma.c  | 240 +++
 drivers/usb/dwc2/hcd_intr.c  |  15 ++-
 drivers/usb/dwc2/hcd_queue.c |   2 +-
 drivers/usb/dwc2/hw.h|   4 -
 drivers/usb/dwc2/platform.c  |   4 +
 9 files changed, 367 insertions(+), 56 deletions(-)

-- 
2.6.2

--
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

Re: [PATCH 1/1] usb: gadget: f_loopback: fix the warning during the enumeration

[Krzysztof Opasiak]

On 11/03/2015 08:57 AM, Peter Chen wrote:

The current code tries to allocate memory with GFP_KERNEL at
interrupt context, it would show below warning during the enumeration
when I test it with chipidea hardware, change GFP flag as GFP_ATOMIC
can fix this issue.

[   40.438237] zero gadget: high-speed config #2: loopback
[   40.444924] [ cut here ]
[   40.449609] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2755 
lockdep_trace_alloc+0x108/0x128()
[   40.461715] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
[   40.467130] Modules linked in:
[   40.470216]  usb_f_ss_lb g_zero libcomposite evbug
[   40.473822] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
4.3.0-rc5-00168-gb730aaf #604
[   40.481496] Hardware name: Freescale i.MX6 SoloX (Device Tree)
[   40.487345] Backtrace:
[   40.489857] [<80014e94>] (dump_backtrace) from [<80015088>] 
(show_stack+0x18/0x1c)
[   40.497445]  r6:80b67a80 r5: r4: r3:
[   40.503234] [<80015070>] (show_stack) from [<802e27b4>] 
(dump_stack+0x8c/0xa4)
[   40.510503] [<802e2728>] (dump_stack) from [<8002cfe8>] 
(warn_slowpath_common+0x80/0xbc)
[   40.518612]  r6:8007510c r5:0009 r4:80b49c88 r3:0001
[   40.524396] [<8002cf68>] (warn_slowpath_common) from [<8002d05c>] 
(warn_slowpath_fmt+0x38/0x40)
[   40.533109]  r8:bcfdef80 r7:bdb705cc r6:80d0 r5:be001e80 r4:809cc278
[   40.539965] [<8002d028>] (warn_slowpath_fmt) from [<8007510c>] 
(lockdep_trace_alloc+0x108/0x128)
[   40.548766]  r3:809d0128 r2:809cc278
[   40.552401]  r4:600b0193
[   40.554990] [<80075004>] (lockdep_trace_alloc) from [<801093d4>] 
(kmem_cache_alloc+0x28/0x15c)
[   40.563618]  r4:80d0 r3:80b4aa8c
[   40.567270] [<801093ac>] (kmem_cache_alloc) from [<804d95e4>] 
(ep_alloc_request+0x58/0x68)
[   40.575550]  r10:7f01f104 r9:0001 r8:bcfdef80 r7:bdb705cc r6:bc178700 
r5:
[   40.583512]  r4:bcfdef80 r3:813c0a38
[   40.587183] [<804d958c>] (ep_alloc_request) from [<7f01f7ec>] 
(loopback_set_alt+0x114/0x21c [usb_f_ss_lb])
[   40.596929] [<7f01f6d8>] (loopback_set_alt [usb_f_ss_lb]) from [<7f006910>] 
(composite_setup+0xbd0/0x17e8 [libcomposite])
[   40.607902]  r10:bd3a2c0c r9: r8:bcfdef80 r7:bc178700 r6:bdb702d0 
r5:bcfdefdc
[   40.615866]  r4:7f0199b4 r3:0002
[   40.619542] [<7f005d40>] (composite_setup [libcomposite]) from [<804dae88>] 
(udc_irq+0x784/0xd1c)
[   40.628431]  r10:80bb5619 r9:c0876140 r8:00012001 r7:bdb71010 r6:bdb70568 
r5:00010001
[   40.636392]  r4:bdb70014
[   40.638985] [<804da704>] (udc_irq) from [<804d64f8>] (ci_irq+0x5c/0x118)
[   40.645702]  r10:80bb5619 r9:be11e000 r8:0117 r7: r6:bdb71010 
r5:be11e060
[   40.653666]  r4:bdb70010
[   40.656261] [<804d649c>] (ci_irq) from [<8007f638>] 
(handle_irq_event_percpu+0x7c/0x13c)
[   40.664367]  r6: r5:be11e060 r4:bdb05cc0 r3:804d649c
[   40.670149] [<8007f5bc>] (handle_irq_event_percpu) from [<8007f740>] 
(handle_irq_event+0x48/0x6c)
[   40.679036]  r10: r9:be008000 r8:0001 r7: r6:bdb05cc0 
r5:be11e060
[   40.686998]  r4:be11e000
[   40.689581] [<8007f6f8>] (handle_irq_event) from [<80082850>] 
(handle_fasteoi_irq+0xd4/0x1b0)
[   40.698120]  r6:80b56a30 r5:be11e060 r4:be11e000 r3:
[   40.703898] [<8008277c>] (handle_fasteoi_irq) from [<8007ec04>] 
(generic_handle_irq+0x28/0x3c)
[   40.712524]  r7: r6:80b4aaf4 r5:0117 r4:80b445fc
[   40.718304] [<8007ebdc>] (generic_handle_irq) from [<8007ef20>] 
(__handle_domain_irq+0x6c/0xe8)
[   40.727033] [<8007eeb4>] (__handle_domain_irq) from [<800095d4>] 
(gic_handle_irq+0x48/0x94)
[   40.735402]  r9:c080f100 r8:80b4ac6c r7:c080e100 r6:80b67d40 r5:80b49f00 
r4:c080e10c
[   40.743290] [<8000958c>] (gic_handle_irq) from [<80015d38>] 
(__irq_svc+0x58/0x78)
[   40.750791] Exception stack(0x80b49f00 to 0x80b49f48)
[   40.755873] 9f00: 0001 0001  80024320 80b48000 80b4a9d0 
80b4a984 80b433e4
[   40.764078] 9f20: 0001 807f4680  80b49f5c 80b49f20 80b49f50 
80071ca4 800113fc
[   40.772272] 9f40: 200b0013 
[   40.775776]  r9:807f4680 r8:0001 r7:80b49f34 r6: r5:200b0013 
r4:800113fc
[   40.783677] [<800113d4>] (arch_cpu_idle) from [<8006c5bc>] 
(default_idle_call+0x28/0x38)
[   40.791798] [<8006c594>] (default_idle_call) from [<8006c6dc>] 
(cpu_startup_entry+0x110/0x1b0)
[   40.800445] [<8006c5cc>] (cpu_startup_entry) from [<807e95dc>] 
(rest_init+0x12c/0x168)
[   40.808376]  r7:80b4a8c0 r3:807f4b7c
[   40.812030] [<807e94b0>] (rest_init) from [<80ad7cc0>] 
(start_kernel+0x360/0x3d4)
[   40.819528]  r5:80bcb000 r4:80bcb050
[   40.823171] [<80ad7960>] (start_kernel) from [<8000807c>] (0x8000807c)

It fixes commit 91c42b0da8e3 ("usb: gadget: loopback: Fix looping back
logic implementation").

Signed-off-by: Peter Chen 
Cc: Krzysztof Opasiak 
Cc: 
---
  drivers/usb/gadget/function/f_loopback.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff 

[PATCH v2 02/13] usb: dwc2: host: set active bit in isochronous descriptors

[Gregory Herrero]

Active bit must be enabled in all scheduled descriptors. Else transfer
never start.
Remove previous code which was not correctly configuring descriptors.
Active bit was set before calling dwc2_fill_host_isoc_dma_desc() which
is erasing dma_desc->status.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 4b0be93..98d8627 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -524,6 +524,9 @@ static void dwc2_fill_host_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
dma_desc->status = qh->n_bytes[idx] << HOST_DMA_ISOC_NBYTES_SHIFT &
   HOST_DMA_ISOC_NBYTES_MASK;
 
+   /* Set active bit */
+   dma_desc->status |= HOST_DMA_A;
+
qh->ntd++;
qtd->isoc_frame_index_last++;
 
@@ -559,8 +562,6 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
list_for_each_entry(qtd, >qtd_list, qtd_list_entry) {
while (qh->ntd < ntd_max && qtd->isoc_frame_index_last <
qtd->urb->packet_count) {
-   if (n_desc > 1)
-   qh->desc_list[n_desc - 1].status |= HOST_DMA_A;
dwc2_fill_host_isoc_dma_desc(hsotg, qtd, qh,
 max_xfer_size, idx);
idx = dwc2_desclist_idx_inc(idx, inc, qh->dev_speed);
@@ -606,12 +607,6 @@ static void dwc2_init_isoc_dma_desc(struct dwc2_hsotg 
*hsotg,
 
qh->desc_list[idx].status |= HOST_DMA_IOC;
 #endif
-
-   if (n_desc) {
-   qh->desc_list[n_desc - 1].status |= HOST_DMA_A;
-   if (n_desc > 1)
-   qh->desc_list[0].status |= HOST_DMA_A;
-   }
 }
 
 static void dwc2_fill_host_dma_desc(struct dwc2_hsotg *hsotg,
-- 
2.6.2

--
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

Re: [RFC PATCH 2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

[Oliver Neukum]

On Wed, 2015-11-04 at 16:26 -0500, Alan Stern wrote:
> This doesn't need to be stored as one of the usb-storage flags.  And
> since we are close to running out of flag bits (on 32-bit
> architectures), it would be better not to use one of them for this.

Do we want the opposite flag? 25% for known good devices
is visible.

Regards
Oliver


--
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

Re: f_loopback's new problem

[Krzysztof Opasiak]

On 11/02/2015 06:49 AM, Peter Chen wrote:

On Fri, Oct 30, 2015 at 10:28:09AM +0100, Krzysztof Opasiak wrote:



On 10/30/2015 08:31 AM, Peter Chen wrote:

Hi Krzysztof and Felipe,

With commit (91c42b0d usb: gadget: loopback: Fix looping back logic
implementation), the gadget loopback supports real loopback function
that is read data from the host, and them send the same data back.

But it breaks the testing function between f_loopback and usbtest,
usbtest does not expect the data will be back from opposite endpoint,
and our documentation (Documentation/usb/gadget-testing.txt) also
says we can use usbtest to test f_loopback.

How about supports old function as well as loopback function at the
same time?


Personally I don't agree with this approach. Loopback function had
been working until commit:

e0857ce usb: gadget: loopback: don't queue requests to bogus endpoints
dated: Mon Oct 13 15:30:52 2014 -0500

After that it began to behave like /dev/null and /dev/zero so it was
more like SourceSink. Then commit:

ec91aff7 Documentation: usb: LOOPBACK function testing
dated: Tue Dec 16 14:56:31 2014 +0100

added entry in documentation about testing the Loopback function but
in that time this function was not working correctly. Then with my
commit:

91c42b0 usb: gadget: loopback: Fix looping back logic implementation
dated: Wed Oct 14 22:49:40 2015 +0200

I have fixed loopback function so now it working now as it was
working before e0857ce. This time line shows that the documentation
has been added when function was not really working. So in my humble
opinion the broken part is the documentation and that's the place
for fix (and update testusb.c if you would like to use it for
testing loopback function).


I don't want to break the some usb tests I added which
can test the maximum USB throughput for bulk.



Is there any reason why didn;t you use SourceSink function which can
work exactly like loopback did before the fix (/dev/zero /dev/null)?



The SourceSink has only one request in queue, the performance is poor



Hmm so maybe it would be a good idea to add qlen param to SourceSink 
function?;)  This should fix the performance problem

--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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

[PATCH v2 08/13] usb: dwc2: host: always increment available host channel during release

[Gregory Herrero]

When releasing a channel, increment hsotg->available_host_channels even
in case a periodic channel is released.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/hcd_ddma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 9635d8d..edccac6 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -278,6 +278,7 @@ static void dwc2_release_channel_ddma(struct dwc2_hsotg 
*hsotg,
hsotg->non_periodic_channels--;
} else {
dwc2_update_frame_list(hsotg, qh, 0);
+   hsotg->available_host_channels++;
}
 
/*
-- 
2.6.2

--
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

[PATCH v2 13/13] usb: dwc2: host: use kmem cache to allocate descriptors

[Gregory Herrero]

Kmem caches help to get correct boundary for descriptor buffers
which need to be 512 bytes aligned for dwc2 controller.
Two kmem caches are needed for generic descriptors and for
hs isochronous descriptors which doesn't have same size.

Signed-off-by: Gregory Herrero 
---
 drivers/usb/dwc2/core.h |  4 
 drivers/usb/dwc2/hcd.c  | 50 -
 drivers/usb/dwc2/hcd_ddma.c | 20 --
 3 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index e7cc542..baee2bc 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -686,6 +686,8 @@ struct dwc2_hregs_backup {
  * @frame_list: Frame list
  * @frame_list_dma: Frame list DMA address
  * @frame_list_sz:  Frame list size
+ * @desc_gen_cache: Kmem cache for generic descriptors
+ * @desc_hsisoc_cache:  Kmem cache for hs isochronous descriptors
  *
  * These are for peripheral mode:
  *
@@ -806,6 +808,8 @@ struct dwc2_hsotg {
u32 *frame_list;
dma_addr_t frame_list_dma;
u32 frame_list_sz;
+   struct kmem_cache *desc_gen_cache;
+   struct kmem_cache *desc_hsisoc_cache;
 
 #ifdef DEBUG
u32 frrem_samples;
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 03cbca3..e3cd5dd 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3145,6 +3145,47 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
if (!hsotg->status_buf)
goto error3;
 
+   /*
+* Create kmem caches to handle descriptor buffers in descriptor
+* DMA mode.
+* Alignment must be set to 512 bytes.
+*/
+   if (hsotg->core_params->dma_desc_enable ||
+   hsotg->core_params->dma_desc_fs_enable) {
+   hsotg->desc_gen_cache = kmem_cache_create("dwc2-gen-desc",
+   sizeof(struct dwc2_hcd_dma_desc) *
+   MAX_DMA_DESC_NUM_GENERIC, 512, SLAB_CACHE_DMA,
+   NULL);
+   if (!hsotg->desc_gen_cache) {
+   dev_err(hsotg->dev,
+   "unable to create dwc2 generic desc cache\n");
+
+   /*
+* Disable descriptor dma mode since it will not be
+* usable.
+*/
+   hsotg->core_params->dma_desc_enable = 0;
+   hsotg->core_params->dma_desc_fs_enable = 0;
+   }
+
+   hsotg->desc_hsisoc_cache = kmem_cache_create("dwc2-hsisoc-desc",
+   sizeof(struct dwc2_hcd_dma_desc) *
+   MAX_DMA_DESC_NUM_HS_ISOC, 512, 0, NULL);
+   if (!hsotg->desc_hsisoc_cache) {
+   dev_err(hsotg->dev,
+   "unable to create dwc2 hs isoc desc cache\n");
+
+   kmem_cache_destroy(hsotg->desc_gen_cache);
+
+   /*
+* Disable descriptor dma mode since it will not be
+* usable.
+*/
+   hsotg->core_params->dma_desc_enable = 0;
+   hsotg->core_params->dma_desc_fs_enable = 0;
+   }
+   }
+
hsotg->otg_port = 1;
hsotg->frame_list = NULL;
hsotg->frame_list_dma = 0;
@@ -3168,7 +3209,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 */
retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (retval < 0)
-   goto error3;
+   goto error4;
 
device_wakeup_enable(hcd->self.controller);
 
@@ -3178,6 +3219,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
 
return 0;
 
+error4:
+   kmem_cache_destroy(hsotg->desc_gen_cache);
+   kmem_cache_destroy(hsotg->desc_hsisoc_cache);
 error3:
dwc2_hcd_release(hsotg);
 error2:
@@ -3218,6 +3262,10 @@ void dwc2_hcd_remove(struct dwc2_hsotg *hsotg)
 
usb_remove_hcd(hcd);
hsotg->priv = NULL;
+
+   kmem_cache_destroy(hsotg->desc_gen_cache);
+   kmem_cache_destroy(hsotg->desc_hsisoc_cache);
+
dwc2_hcd_release(hsotg);
usb_put_hcd(hcd);
 
diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 85d7816..36606fc 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -87,10 +87,18 @@ static u16 dwc2_frame_incr_val(struct dwc2_qh *qh)
 static int dwc2_desc_list_alloc(struct dwc2_hsotg *hsotg, struct dwc2_qh *qh,
gfp_t flags)
 {
+   struct kmem_cache *desc_cache;
+
+   if (qh->ep_type == USB_ENDPOINT_XFER_ISOC
+   && qh->dev_speed == USB_SPEED_HIGH)
+   desc_cache = hsotg->desc_hsisoc_cache;
+   else
+   desc_cache = hsotg->desc_gen_cache;
+
qh->desc_list_sz = sizeof(struct 

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Thu, Nov 05, 2015 at 04:12:24PM +0800, Lu, Baolu wrote:
> 1) apply the attached patch on top the latest kernel.
> 2) build and install the kernel.
> 3) boot your machine with the new kernel.

Do you want this on top of Alan's LPM-disabling patch or on a clean
4.3.0 tree?

> 4) insert one  Blackmagic Design device into USB3 root port.
> 7) wait for a few seconds
> 6) check below sysfs nodes:

I guess there's no step 5. :-)

/* Steinar */
-- 
Homepage: http://www.sesse.net/
--
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

Re: Overly conservative xHCI bandwidth estimation

[Lu, Baolu]

Hi Steinar,

Do you mind trying attached patch? This is not a fix but to verify
whether LPM really works when only one device is used. You can
follow below steps:

1) apply the attached patch on top the latest kernel.
2) build and install the kernel.
3) boot your machine with the new kernel.
4) insert one  Blackmagic Design device into USB3 root port.
7) wait for a few seconds
6) check below sysfs nodes:

$ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u1
$ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u2

Can you please post values of above two sysfs nodes?
A copy of dmesg will also be helpful.

Thanks,
Baolu

On 11/05/2015 02:23 AM, Steinar H. Gunderson wrote:

On Mon, Nov 02, 2015 at 04:00:42PM -0500, Alan Stern wrote:

That commit was included in (approximately) the 4.1.5 or later stable
kernel, and it is included in 4.2.  You should be able to put one of
those on a bootable USB stick.

I tried going down this route. After some back and forth, I realize that...
the machine has only two USB ports, so I can't boot off USB stick and still
test how it behaves with two cards. (I could buy a hub, but that would surely
introduce new potential errors in the mix.)

So I'm afraid I can't help you at this point. My recommendation to include
the patch still stands, though.

/* Steinar */


>From c8cd71a0703cf26cfca9d89a3ad451f79c7232af Mon Sep 17 00:00:00 2001
From: Lu Baolu 
Date: Thu, 5 Nov 2015 14:08:48 +0800
Subject: [PATCH 1/2] usb: core: LPM: fix usb3_hardware_lpm sysfs node

Commit 655fe4effe0f ("usbcore: add sysfs support to xHCI usb3
hardware LPM") introduced usb3_hardware_lpm sysfs node. This
doesn't show the correct status of USB3 U1 and U2 LPM status.

This patch fixes this by replacing usb3_hardware_lpm with two
nodes, usb3_hardware_lpm_u1 (for U1) and usb3_hardware_lpm_u2
(for U2), and recording the U1/U2 LPM status in right places.

Signed-off-by: Lu Baolu 
---
 Documentation/ABI/testing/sysfs-bus-usb | 16 +---
 Documentation/usb/power-management.txt  | 11 ++-
 drivers/usb/core/hub.c  | 29 +
 drivers/usb/core/sysfs.c| 28 
 include/linux/usb.h |  2 ++
 5 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 3a4abfc..136ba17 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -134,19 +134,21 @@ Description:
 		enabled for the device. Developer can write y/Y/1 or n/N/0 to
 		the file to enable/disable the feature.
 
-What:		/sys/bus/usb/devices/.../power/usb3_hardware_lpm
-Date:		June 2015
+What:		/sys/bus/usb/devices/.../power/usb3_hardware_lpm_u1
+		/sys/bus/usb/devices/.../power/usb3_hardware_lpm_u2
+Date:		November 2015
 Contact:	Kevin Strasser 
+		Lu Baolu 
 Description:
 		If CONFIG_PM is set and a USB 3.0 lpm-capable device is plugged
 		in to a xHCI host which supports link PM, it will check if U1
 		and U2 exit latencies have been set in the BOS descriptor; if
-		the check is is passed and the host supports USB3 hardware LPM,
+		the check is passed and the host supports USB3 hardware LPM,
 		USB3 hardware LPM will be enabled for the device and the USB
-		device directory will contain a file named
-		power/usb3_hardware_lpm. The file holds a string value (enable
-		or disable) indicating whether or not USB3 hardware LPM is
-		enabled for the device.
+		device directory will contain two files named
+		power/usb3_hardware_lpm_u1 and power/usb3_hardware_lpm_u2. These
+		files hold a string value (enable or disable) indicating whether
+		or not USB3 hardware LPM U1 or U2 is enabled for the device.
 
 What:		/sys/bus/usb/devices/.../removable
 Date:		February 2012
diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt
index 4a15c90..0a94ffe 100644
--- a/Documentation/usb/power-management.txt
+++ b/Documentation/usb/power-management.txt
@@ -537,17 +537,18 @@ relevant attribute files are usb2_hardware_lpm and usb3_hardware_lpm.
 		can write y/Y/1 or n/N/0 to the file to	enable/disable
 		USB2 hardware LPM manually. This is for	test purpose mainly.
 
-	power/usb3_hardware_lpm
+	power/usb3_hardware_lpm_u1
+	power/usb3_hardware_lpm_u2
 
 		When a USB 3.0 lpm-capable device is plugged in to a
 		xHCI host which supports link PM, it will check if U1
 		and U2 exit latencies have been set in the BOS
 		descriptor; if the check is is passed and the host
 		supports USB3 hardware LPM, USB3 hardware LPM will be
-		enabled for the device and this file will be created.
-		The file holds a string value (enable or disable)
-		indicating whether or not USB3 hardware LPM is
-		enabled for the device.
+		enabled for the device and these files will be created.
+		The files hold a string value 

Re: [PATCH V4] usb: remove unnecessary CONFIG_PM dependency from USB_OTG

[Felipe Balbi]

Hi,

Peter Chen  writes:
> On Tue, Nov 03, 2015 at 09:51:11PM -0600, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Peter Chen  writes:
>> > On Tue, Nov 03, 2015 at 07:56:55AM -0600, Felipe Balbi wrote:
>> >> 
>> >> Hi,
>> >> 
>> >> Nathan Sullivan  writes:
>> >> > The USB OTG support currently depends on power management
>> >> > (CONFIG_PM) being enabled, but does not actually need it enabled.
>> >> > Remove this dependency.
>> >> >
>> >> > Tested on Bay Trail hardware with dwc3 USB.
>> >> >
>> >> > Signed-off-by: Nathan Sullivan 
>> >> > ---
>> >> >  drivers/usb/core/Kconfig |1 -
>> >> >  1 file changed, 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>> >> > index a99c89e..9c5cdf3 100644
>> >> > --- a/drivers/usb/core/Kconfig
>> >> > +++ b/drivers/usb/core/Kconfig
>> >> > @@ -43,7 +43,6 @@ config USB_DYNAMIC_MINORS
>> >> >  
>> >> >  config USB_OTG
>> >> > bool "OTG support"
>> >> > -   depends on PM
>> >> 
>> >> I don't think this is correct. OTG depends on USB bus suspend, which is
>> >> only available on PM builds. Care to further detail why you think PM is
>> >> not needed on OTG ?
>> >> 
>> >
>> > OTG depends on USB bus suspend is not a must, the hardware controlled OTG
>> > design do HNP when the bus goes to suspend; but if the software
>> > implements OTG FSM, it is the user option whether do HNP, and bus
>> > suspend is controlled by OTG FSM software (stop SOF), but not by host 
>> > stack (eg, ehci).
>> >
>> > I am sorry I did not consider the legacy OTG design, this patch should
>> > be dropped.
>> 
>> there is no "legacy" OTG design. OTG requires a bus suspend to enter
>> HNP, and that's achieved by stopping all transfers and avoid new URB
>> submission so usbcore can put the bus in suspend (by means of USB
>> autosuspend). If you're bypassing that in the OTG FSM thing, that needs
>> to be fixed ASAP as that makes it a lot harder for any generic changes
>> in usbcore to be validated. Specially when you consider not many will
>> have whatever special HW which, likely, doesn't even work with mainline
>> to validate a change.
>> 
>
> Felipe, thanks for your comments.
>
> But you may need to consider the user option ~a_bus_req (for A) and
> b_bus_req (for B) when do HNP, we can't do HNP without user option.
>
> - Eg, if the bus enters suspend, but the A does not want role switch,
> we can't try to do HNP, the same for B device.
> - The A device may want to support auto-suspend but without role switch.
>
> You are absolutely right, the SAFE HNP needs to do auto-suspend first, we
> need to add this in documentation. But it does not mean OTG FSM design is
> incorrect, there are two things, eg, if we want to do HNP safely, we
> need

sure it does. If you're bypassing anything in usbcore, it's wrong :-)
For that [ab]_bus_req polling, as soon as you notice that flag, you
should just giveback all URBs to the link partner so the link can
autosuspend and HNP will happen exactly the same.

> to do below things:
> /* B requests HNP */
> echo 1 > /sys/bus/../otg_fsm//inputs/b_bus_req 
>
> /* Put the A to suspend */
> echo auto > /sys/bus/usb/devices/1-1/power/control 
> /* A goes to try HNP */
> echo 0 > /sys/bus/../otg_fsm/inputs/a_bus_req

When you echo 0 to a_bus_req, A should stop all in-flight transfers to
the link partner and wait for the bus to autosuspend. There should be no
differences other than the need to set these flags.

This even gives us opportunity to tune "how fast" we wand HNP to happen
by fiddling with autosuspend delay ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[Petr Štetiar]

Bjørn Mork  [2015-11-05 12:03:36]:

> make this function return bool.  And the patch should go to netdev.

Thanks, done.

> With those changes it looks good to me.  But I haven't tested it, so I
> trust your testing :)

usb 2-1.2: new high-speed USB device number 3 using ci_hdrc
qcserial 2-1.2:1.0: Qualcomm USB modem converter detected
usb 2-1.2: Qualcomm USB modem converter now attached to ttyUSB0
qcserial 2-1.2:1.1: Qualcomm USB modem converter detected
usb 2-1.2: Qualcomm USB modem converter now attached to ttyUSB1
qcserial 2-1.2:1.2: Qualcomm USB modem converter detected
usb 2-1.2: Qualcomm USB modem converter now attached to ttyUSB2
qcserial 2-1.2:1.3: Qualcomm USB modem converter detected
usb 2-1.2: Qualcomm USB modem converter now attached to ttyUSB3
qmi_wwan 2-1.2:1.4: cdc-wdm0: USB WDM device
qmi_wwan 2-1.2:1.4 wwan0: register 'qmi_wwan' at usb-ci_hdrc.1-1.2, WWAN/QMI 
device, 56:3a:52:a3:e9:f7

root@OpenWrt:/# ifconfig wwan0
wwan0 Link encap:Ethernet  HWaddr 56:3A:52:A3:E9:F7  
  BROADCAST MULTICAST  MTU:1500  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000 
  RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

root@OpenWrt:/# uqmi -d /dev/cdc-wdm0 --get-capabilities
{
"max_tx_channel_rate": 5000,
"max_rx_channel_rate": 1,
"data_service": "non_simultaneous_cs_ps",
"sim": "supported",
"networks": [
"gsm",
"umts",
"lte"
]
}
root@OpenWrt:/# microcom -s115200 /dev/ttyUSB2
AT+QGPS=1
OK

root@OpenWrt:/# microcom -s115200 /dev/ttyUSB1
$GPRMC,,V,,N*53
$GPVTG,,T,,M,,N,,K,N*2C

-- ynezz
--
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

[PATCH 1/2] cdc_acm: Ignore Infineon Flash Loader utility

[Jonas Jonsson]

Some modems, such as the Telit UE910, are using an Infineon Flash Loader
utility. It has two interfaces, 2/2/0 (Abstract Modem) and 10/0/0 (CDC
Data). The latter can be used as a serial interface to upgrade the
firmware of the modem. However, that isn't possible when the cdc-acm
driver takes control of the device.

Signed-off-by: Jonas Jonsson 
---
 drivers/usb/class/cdc-acm.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b30e742..26ca4f9 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1838,6 +1838,11 @@ static const struct usb_device_id acm_ids[] = {
},
 #endif
 
+   /* Exclude Infineon Flash Loader utility */
+   { USB_DEVICE(0x058b, 0x0041),
+   .driver_info = IGNORE_DEVICE,
+   },
+
/* control interfaces without any protocol set */
{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM,
USB_CDC_PROTO_NONE) },
-- 
2.5.0

--
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

[PATCH 2/2] USB: serial: Another Infineon flash loader USB ID

[Jonas Jonsson]

This has been seen on a Telit UE910 modem.

Signed-off-by: Jonas Jonsson 
---
 drivers/usb/serial/usb-serial-simple.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial-simple.c 
b/drivers/usb/serial/usb-serial-simple.c
index 3658662..93ab784 100644
--- a/drivers/usb/serial/usb-serial-simple.c
+++ b/drivers/usb/serial/usb-serial-simple.c
@@ -53,7 +53,9 @@ DEVICE(funsoft, FUNSOFT_IDS);
 
 /* Infineon Flashloader driver */
 #define FLASHLOADER_IDS()  \
-   { USB_DEVICE(0x8087, 0x0716) }
+   { USB_DEVICE(0x8087, 0x0716) }, \
+   { USB_DEVICE_INTERFACE_CLASS(0x058b, 0x0041, 0x0a) }
+
 DEVICE(flashloader, FLASHLOADER_IDS);
 
 /* Google Serial USB SubClass */
-- 
2.5.0

--
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

[PATCH] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[Petr Štetiar]

This device has same vendor and product IDs as G2K devices, but it has
different number of interfaces(4 vs 5) and also different interface
layout where EC20 has QMI on interface 4 instead of 0.

lsusb output:

Bus 002 Device 003: ID 05c6:9215 Qualcomm, Inc. Acer Gobi 2000
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0
  bDeviceProtocol 0
  bMaxPacketSize064
  idVendor   0x05c6 Qualcomm, Inc.
  idProduct  0x9215 Acer Gobi 2000 Wireless Modem
  bcdDevice2.32
  iManufacturer   1 Quectel
  iProduct2 Quectel LTE Module
  iSerial 0
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  209
bNumInterfaces  5
bConfigurationValue 1
iConfiguration  0
bmAttributes 0xa0
  (Bus Powered)
  Remote Wakeup
MaxPower  500mA

Signed-off-by: Petr Štetiar 
---
 drivers/net/usb/qmi_wwan.c |   21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 2a7c1be..385b5e3 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -822,6 +822,7 @@ static const struct usb_device_id products[] = {
{QMI_GOBI_DEVICE(0x05c6, 0x9245)},  /* Samsung Gobi 2000 Modem 
device (VL176) */
{QMI_GOBI_DEVICE(0x03f0, 0x251d)},  /* HP Gobi 2000 Modem device 
(VP412) */
{QMI_GOBI_DEVICE(0x05c6, 0x9215)},  /* Acer Gobi 2000 Modem device 
(VP413) */
+   {QMI_FIXED_INTF(0x05c6, 0x9215, 4)},/* Quectel EC20 Mini PCIe */
{QMI_GOBI_DEVICE(0x05c6, 0x9265)},  /* Asus Gobi 2000 Modem device 
(VR305) */
{QMI_GOBI_DEVICE(0x05c6, 0x9235)},  /* Top Global Gobi 2000 Modem 
device (VR306) */
{QMI_GOBI_DEVICE(0x05c6, 0x9275)},  /* iRex Technologies Gobi 2000 
Modem device (VR307) */
@@ -853,10 +854,24 @@ static const struct usb_device_id products[] = {
 };
 MODULE_DEVICE_TABLE(usb, products);
 
+static int quectel_ec20_detected(struct usb_interface *intf)
+{
+   struct usb_device *dev = interface_to_usbdev(intf);
+
+   if (dev->actconfig &&
+   le16_to_cpu(dev->descriptor.idVendor) == 0x05c6 &&
+   le16_to_cpu(dev->descriptor.idProduct) == 0x9215 &&
+   dev->actconfig->desc.bNumInterfaces == 5)
+   return 1;
+
+   return 0;
+}
+
 static int qmi_wwan_probe(struct usb_interface *intf,
  const struct usb_device_id *prod)
 {
struct usb_device_id *id = (struct usb_device_id *)prod;
+   struct usb_interface_descriptor *desc = >cur_altsetting->desc;
 
/* Workaround to enable dynamic IDs.  This disables usbnet
 * blacklisting functionality.  Which, if required, can be
@@ -868,6 +883,12 @@ static int qmi_wwan_probe(struct usb_interface *intf,
id->driver_info = (unsigned long)_wwan_info;
}
 
+   /* Quectel EC20 quirk where we've QMI on interface 4 instead of 0 */
+   if (quectel_ec20_detected(intf) && desc->bInterfaceNumber == 0) {
+   dev_dbg(>dev, "Quectel EC20 quirk, skipping interface 
0\n");
+   return -ENODEV;
+   }
+
return usbnet_probe(intf, id);
 }
 
-- 
1.7.9.5

--
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

Re: [PATCH] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[Bjørn Mork]

Petr Štetiar  writes:

> This device has same vendor and product IDs as G2K devices, but it has
> different number of interfaces(4 vs 5) and also different interface
> layout where EC20 has QMI on interface 4 instead of 0.
>
> lsusb output:
>
>   Bus 002 Device 003: ID 05c6:9215 Qualcomm, Inc. Acer Gobi 2000
>   Device Descriptor:
> bLength18
> bDescriptorType 1
> bcdUSB   2.00
> bDeviceClass0 (Defined at Interface level)
> bDeviceSubClass 0
> bDeviceProtocol 0
> bMaxPacketSize064
> idVendor   0x05c6 Qualcomm, Inc.
> idProduct  0x9215 Acer Gobi 2000 Wireless Modem
> bcdDevice2.32
> iManufacturer   1 Quectel
> iProduct2 Quectel LTE Module
> iSerial 0
> bNumConfigurations  1
> Configuration Descriptor:
>   bLength 9
>   bDescriptorType 2
>   wTotalLength  209
>   bNumInterfaces  5
>   bConfigurationValue 1
>   iConfiguration  0
>   bmAttributes 0xa0
> (Bus Powered)
> Remote Wakeup
>   MaxPower  500mA
>
> Signed-off-by: Petr Štetiar 
> ---
>  drivers/net/usb/qmi_wwan.c |   21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 2a7c1be..385b5e3 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -822,6 +822,7 @@ static const struct usb_device_id products[] = {
>   {QMI_GOBI_DEVICE(0x05c6, 0x9245)},  /* Samsung Gobi 2000 Modem 
> device (VL176) */
>   {QMI_GOBI_DEVICE(0x03f0, 0x251d)},  /* HP Gobi 2000 Modem device 
> (VP412) */
>   {QMI_GOBI_DEVICE(0x05c6, 0x9215)},  /* Acer Gobi 2000 Modem device 
> (VP413) */
> + {QMI_FIXED_INTF(0x05c6, 0x9215, 4)},/* Quectel EC20 Mini PCIe */
>   {QMI_GOBI_DEVICE(0x05c6, 0x9265)},  /* Asus Gobi 2000 Modem device 
> (VR305) */
>   {QMI_GOBI_DEVICE(0x05c6, 0x9235)},  /* Top Global Gobi 2000 Modem 
> device (VR306) */
>   {QMI_GOBI_DEVICE(0x05c6, 0x9275)},  /* iRex Technologies Gobi 2000 
> Modem device (VR307) */
> @@ -853,10 +854,24 @@ static const struct usb_device_id products[] = {
>  };
>  MODULE_DEVICE_TABLE(usb, products);
>  
> +static int quectel_ec20_detected(struct usb_interface *intf)
> +{
> + struct usb_device *dev = interface_to_usbdev(intf);
> +
> + if (dev->actconfig &&
> + le16_to_cpu(dev->descriptor.idVendor) == 0x05c6 &&
> + le16_to_cpu(dev->descriptor.idProduct) == 0x9215 &&
> + dev->actconfig->desc.bNumInterfaces == 5)
> + return 1;
> +
> + return 0;
> +}

make this function return bool.  And the patch should go to netdev.
With those changes it looks good to me.  But I haven't tested it, so I
trust your testing :)


Bjørn
--
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

[PATCH v2] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[Petr Štetiar]

This device has same vendor and product IDs as G2K devices, but it has
different number of interfaces(4 vs 5) and also different interface
layout where EC20 has QMI on interface 4 instead of 0.

lsusb output:

Bus 002 Device 003: ID 05c6:9215 Qualcomm, Inc. Acer Gobi 2000
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass0 (Defined at Interface level)
  bDeviceSubClass 0
  bDeviceProtocol 0
  bMaxPacketSize064
  idVendor   0x05c6 Qualcomm, Inc.
  idProduct  0x9215 Acer Gobi 2000 Wireless Modem
  bcdDevice2.32
  iManufacturer   1 Quectel
  iProduct2 Quectel LTE Module
  iSerial 0
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  209
bNumInterfaces  5
bConfigurationValue 1
iConfiguration  0
bmAttributes 0xa0
  (Bus Powered)
  Remote Wakeup
MaxPower  500mA

Signed-off-by: Petr Štetiar 
---

Changes since v1:
 * make quectel_ec20_detected return bool (Bjørn)

 drivers/net/usb/qmi_wwan.c |   21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 2a7c1be..b81a32c 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -822,6 +822,7 @@ static const struct usb_device_id products[] = {
{QMI_GOBI_DEVICE(0x05c6, 0x9245)},  /* Samsung Gobi 2000 Modem 
device (VL176) */
{QMI_GOBI_DEVICE(0x03f0, 0x251d)},  /* HP Gobi 2000 Modem device 
(VP412) */
{QMI_GOBI_DEVICE(0x05c6, 0x9215)},  /* Acer Gobi 2000 Modem device 
(VP413) */
+   {QMI_FIXED_INTF(0x05c6, 0x9215, 4)},/* Quectel EC20 Mini PCIe */
{QMI_GOBI_DEVICE(0x05c6, 0x9265)},  /* Asus Gobi 2000 Modem device 
(VR305) */
{QMI_GOBI_DEVICE(0x05c6, 0x9235)},  /* Top Global Gobi 2000 Modem 
device (VR306) */
{QMI_GOBI_DEVICE(0x05c6, 0x9275)},  /* iRex Technologies Gobi 2000 
Modem device (VR307) */
@@ -853,10 +854,24 @@ static const struct usb_device_id products[] = {
 };
 MODULE_DEVICE_TABLE(usb, products);
 
+static bool quectel_ec20_detected(struct usb_interface *intf)
+{
+   struct usb_device *dev = interface_to_usbdev(intf);
+
+   if (dev->actconfig &&
+   le16_to_cpu(dev->descriptor.idVendor) == 0x05c6 &&
+   le16_to_cpu(dev->descriptor.idProduct) == 0x9215 &&
+   dev->actconfig->desc.bNumInterfaces == 5)
+   return true;
+
+   return false;
+}
+
 static int qmi_wwan_probe(struct usb_interface *intf,
  const struct usb_device_id *prod)
 {
struct usb_device_id *id = (struct usb_device_id *)prod;
+   struct usb_interface_descriptor *desc = >cur_altsetting->desc;
 
/* Workaround to enable dynamic IDs.  This disables usbnet
 * blacklisting functionality.  Which, if required, can be
@@ -868,6 +883,12 @@ static int qmi_wwan_probe(struct usb_interface *intf,
id->driver_info = (unsigned long)_wwan_info;
}
 
+   /* Quectel EC20 quirk where we've QMI on interface 4 instead of 0 */
+   if (quectel_ec20_detected(intf) && desc->bInterfaceNumber == 0) {
+   dev_dbg(>dev, "Quectel EC20 quirk, skipping interface 
0\n");
+   return -ENODEV;
+   }
+
return usbnet_probe(intf, id);
 }
 
-- 
1.7.9.5

--
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

Re: [RESEND PATCH] USB: usbmon: Use 64bit timestamp for mon_bin_hdr

[Greg Kroah-Hartman]

On Thu, Nov 05, 2015 at 05:18:41PM +0100, Arnd Bergmann wrote:
> On Thursday 29 October 2015 22:44:31 Tina Ruchandani wrote:
> > struct mon_bin_hdr allows for a 64-bit seconds timestamp. The code
> > currently uses 'struct timeval' to populate the timestamp in mon_bin_hdr,
> > which has a 32-bit seconds field and will overflow in year 2038 and beyond.
> > This patch replaces 'struct timeval' with 'struct timespec64' which is
> > y2038 safe. This patch is part of a larger attempt to remove instances
> > of struct timeval and other 32-bit timekeeping (time_t, struct timespec)
> > from the kernel.
> > 
> > Signed-off-by: Tina Ruchandani 
> 
> Reviewed-by: Arnd Bergmann 
> 
> As the patch is a week old and Greg hasn't picked it up yet, I'm guessing
> that he doesn't have it in his queue any more and you should send it once
> more with my 'Reviewed-by' tag.

It's the merge window, I can't pick anything new up, please be patient
and wait for 4.4-rc1 to come out first...

thanks,

greg k-h
--
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

Re: [PATCH v2] USB: usbmon: Remove timeval usage for timestamp

[Arnd Bergmann]

On Thursday 29 October 2015 22:58:28 Tina Ruchandani wrote:
> struct timeval' uses 32-bits for its seconds field and will overflow in
> the year 2038 and beyond. This patch replaces the usage of 'struct timeval'
> in mon_get_timestamp() with timespec64 which uses a 64-bit seconds field
> and is y2038-safe. mon_get_timestamp() truncates the timestamp at 4096 
> seconds,
> so the correctness of the code is not affected. This patch is part of a larger
> attempt to remove instances of struct timeval and other 32-bit timekeeping
> (time_t, struct timespec) from the kernel.
> 
> Signed-off-by: Tina Ruchandani 
> Suggested-by: Arnd Bergmann 

Reviewed-by: Arnd Bergmann 
--
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

Re: [RESEND PATCH] USB: usbmon: Use 64bit timestamp for mon_bin_hdr

[Arnd Bergmann]

On Thursday 29 October 2015 22:44:31 Tina Ruchandani wrote:
> struct mon_bin_hdr allows for a 64-bit seconds timestamp. The code
> currently uses 'struct timeval' to populate the timestamp in mon_bin_hdr,
> which has a 32-bit seconds field and will overflow in year 2038 and beyond.
> This patch replaces 'struct timeval' with 'struct timespec64' which is
> y2038 safe. This patch is part of a larger attempt to remove instances
> of struct timeval and other 32-bit timekeeping (time_t, struct timespec)
> from the kernel.
> 
> Signed-off-by: Tina Ruchandani 

Reviewed-by: Arnd Bergmann 

As the patch is a week old and Greg hasn't picked it up yet, I'm guessing
that he doesn't have it in his queue any more and you should send it once
more with my 'Reviewed-by' tag.

> @@ -494,7 +495,7 @@ static void mon_bin_event(struct mon_reader_bin *rp, 
> struct urb *urb,
> struct mon_bin_hdr *ep;
> char data_tag = 0;
>  
> -   do_gettimeofday();
> +   getnstimeofday64();
>  
> spin_lock_irqsave(>b_lock, flags);
>  

This is a very minor comment I still have, and the patch is fine as it, but
I tend to prefer ktime_get_real_ts64() over getnstimeofday64() these days.
The two functions do the exact same thing, and I hope to remove the latter
eventually.

Arnd
--
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

Broken usb stick after plug it on an usb port?

[Javier Barroso]

Hello,

I have a usb stick which was (apparently) broken after plug it on a
usb port on my laptop [1]

My sequence of operation is attached at kern.log-commented.gz

This laptop have a port which "Anytime USB Charge (see inside
http://www.shopfujitsu.com/www/content/products/notebooks/notebooks.php?products/notebooks/features_benefits/ah572_features_benefits)",
but it is disabled on BIOS (so there should not be a problem)

stick owner tell me about this stick was working before I plug it on my laptop.

Is it possible that my laptop kill/broke usb sticks ? I think this is
the second time that such thing happens.

I have tried to recover the stick without success:

* 
https://paulphilippov.com/articles/how-to-fix-device-not-accepting-address-error
* Try it on linux / windows / mac
* echo -1 >/sys/module/usbcore/parameters/autosuspend [2]
* echo Y > /sys/module/usbcore/parameters/use_both_schemes [3]
* echo  Y > /sys/module/usbcore/parameters/old_scheme_first

Do you have any recomendation? What could be happen to my usb stick?

Thank you very much !

[1] Fujitsu laptop, with next info obtain from dmidecode:

System Information
Manufacturer: FUJITSU
Product Name: LIFEBOOK U772
Version: 10601581541

BIOS Information
Vendor: FUJITSU // Phoenix Technologies Ltd.
Version: Version 2.03
Release Date: 10/15/2012
...
Characteristics:
...
USB legacy is supported
...

And with lsusb -v output attached (xzipped) to this mail

[2] After reading
https://www.kernel.org/doc/Documentation/usb/power-management.txt I
can read that my device can be damaged by autosuspend (this can apply
to a usb stick?):

"The kernel will not prevent you from enabling autosuspend on devices
that can't handle it.  It is even possible in theory to damage a
device by suspending it at the wrong time.  (Highly unlikely, but
possible.)  Take care."

[3] https://bugzilla.kernel.org/show_bug.cgi?id=4052


lsusb-fujitsu-LIFEBOOK-U772.txt.xz
Description: application/xz


kern.log-commented.gz
Description: GNU Zip compressed data

Re: [PATCH v2] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[David Miller]

From: Petr Štetiar 
Date: Thu,  5 Nov 2015 12:55:01 +0100

> This device has same vendor and product IDs as G2K devices, but it has
> different number of interfaces(4 vs 5) and also different interface
> layout where EC20 has QMI on interface 4 instead of 0.
> 
> lsusb output:
 ...
> Signed-off-by: Petr Štetiar 

Applied, thanks.
--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

On 11/05/2015 10:54 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set at the
same time? If so, I will drop this change in V3.


   Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for device
disconnect, but harmless, so the next two changes are important to give better
user experience.


   Hm, looks like some errata... and hiding this from users while there's no 
workaround doesn't seem a good policy.



Thanks,
-Bin.



So I don't think this change is valid.


- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 


[...]

MBR, Sergei

--
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

Re: [PATCH] usb: musb: fix tx fifo flush handling

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Here are a few changes in musb_h_tx_flush_fifo().
>
> - Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
>   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
>   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
>   MUSB_TXCSR_TXPKTRDY should be checked, not set.
>
> - It seems to be common that sometimes (if not always) musb is unable
>   to flush tx fifo during urb dequeue. But it seems to be harmless,
>   since the tx fifo flush is done again in musb_ep_program() when
>   re-use the hw_ep.
>
>   But the WARN() scares many end users, so change it to dev_dbg().
>
> - applications could queue up many tx urbs, then the 1ms delay could
>   causes minutes of delay in device disconnect. So remove it to get
>   better user experience. The 1ms delay does not help the flushing
>   anyway.
>
> - cleanup the debug code - related to lastcsr.
>
> Signed-off-by: Bin Liu 
> ---
>  drivers/usb/musb/musb_host.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 883a9ad..f9d4b4f 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
>   struct musb *musb = ep->musb;
>   void __iomem*epio = ep->regs;
>   u16 csr;
> - u16 lastcsr = 0;
>   int retries = 1000;
>  
>   csr = musb_readw(epio, MUSB_TXCSR);
> - while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
> - if (csr != lastcsr)
> - dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: 
> %02x\n", csr);
> - lastcsr = csr;
> - csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
> + while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {

this doesn't really fix the problem. This check will be true if either
or both bits are set. You want only when both are set, so it should be:

while ((csr & FIFONOTEMPTY) && (csr & TXPKTRDY))

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v2] usb: musb: fix tx fifo flush handling

[Bin Liu]

Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
  musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
  is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
  MUSB_TXCSR_TXPKTRDY should be checked, not set.

- It seems to be common that sometimes (if not always) musb is unable
  to flush tx fifo during urb dequeue. But it seems to be harmless,
  since the tx fifo flush is done again in musb_ep_program() when
  re-use the hw_ep.

  But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
  causes minutes of delay in device disconnect. So remove it to get
  better user experience. The 1ms delay does not help the flushing
  anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_host.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..5db24f2 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
struct musb *musb = ep->musb;
void __iomem*epio = ep->regs;
u16 csr;
-   u16 lastcsr = 0;
int retries = 1000;
 
csr = musb_readw(epio, MUSB_TXCSR);
-   while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-   if (csr != lastcsr)
-   dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: 
%02x\n", csr);
-   lastcsr = csr;
-   csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+   while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {
+   csr |= MUSB_TXCSR_FLUSHFIFO;
musb_writew(epio, MUSB_TXCSR, csr);
csr = musb_readw(epio, MUSB_TXCSR);
-   if (WARN(retries-- < 1,
-   "Could not flush host TX%d fifo: csr: %04x\n",
-   ep->epnum, csr))
+   if (retries-- < 1) {
+   dev_dbg(musb->controller, "Could not flush host TX%d 
fifo: csr: %04x\n",
+   ep->epnum, csr);
return;
-   mdelay(1);
+   }
}
 }
 
-- 
1.8.4

--
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

[PATCH] usb: musb: fix tx fifo flush handling

[Bin Liu]

Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
  musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
  is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
  MUSB_TXCSR_TXPKTRDY should be checked, not set.

- It seems to be common that sometimes (if not always) musb is unable
  to flush tx fifo during urb dequeue. But it seems to be harmless,
  since the tx fifo flush is done again in musb_ep_program() when
  re-use the hw_ep.

  But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
  causes minutes of delay in device disconnect. So remove it to get
  better user experience. The 1ms delay does not help the flushing
  anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_host.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..f9d4b4f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
struct musb *musb = ep->musb;
void __iomem*epio = ep->regs;
u16 csr;
-   u16 lastcsr = 0;
int retries = 1000;
 
csr = musb_readw(epio, MUSB_TXCSR);
-   while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-   if (csr != lastcsr)
-   dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: 
%02x\n", csr);
-   lastcsr = csr;
-   csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+   while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {
+   csr |= MUSB_TXCSR_FLUSHFIFO;
musb_writew(epio, MUSB_TXCSR, csr);
csr = musb_readw(epio, MUSB_TXCSR);
-   if (WARN(retries-- < 1,
-   "Could not flush host TX%d fifo: csr: %04x\n",
-   ep->epnum, csr))
+   if (retries-- < 1) {
+   dev_dbg(musb->controller, "Could not flush host TX%d 
fifo: csr: %04x\n",
+   ep->epnum, csr);
return;
-   mdelay(1);
+   }
}
 }
 
-- 
1.8.4

--
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

Re: [RESEND PATCH] USB: usbmon: Use 64bit timestamp for mon_bin_hdr

[Arnd Bergmann]

On Thursday 05 November 2015 08:37:39 Greg Kroah-Hartman wrote:
> On Thu, Nov 05, 2015 at 05:18:41PM +0100, Arnd Bergmann wrote:
> > On Thursday 29 October 2015 22:44:31 Tina Ruchandani wrote:
> > > struct mon_bin_hdr allows for a 64-bit seconds timestamp. The code
> > > currently uses 'struct timeval' to populate the timestamp in mon_bin_hdr,
> > > which has a 32-bit seconds field and will overflow in year 2038 and 
> > > beyond.
> > > This patch replaces 'struct timeval' with 'struct timespec64' which is
> > > y2038 safe. This patch is part of a larger attempt to remove instances
> > > of struct timeval and other 32-bit timekeeping (time_t, struct timespec)
> > > from the kernel.
> > > 
> > > Signed-off-by: Tina Ruchandani 
> > 
> > Reviewed-by: Arnd Bergmann 
> > 
> > As the patch is a week old and Greg hasn't picked it up yet, I'm guessing
> > that he doesn't have it in his queue any more and you should send it once
> > more with my 'Reviewed-by' tag.
> 
> It's the merge window, I can't pick anything new up, please be patient
> and wait for 4.4-rc1 to come out first...

Sorry, I wasn't thinking straight.

Arnd
--
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

Re: [PATCH 0/3] dwc2: Speed up the interrupt handler quite a bit

[Heiko Stuebner]

Am Mittwoch, 4. November 2015, 14:53:02 schrieb Douglas Anderson:
> The dwc2 interrupt handler is quite slow.  On rk3288 with a few things
> plugged into the ports and with cpufreq locked at 696MHz (to simulate
> real world idle system), I can easily observe dwc2_handle_hcd_intr()
> taking > 120 us, sometimes > 150 us.  Note that SOF interrupts come
> every 125 us with high speed USB, so taking > 120 us in the interrupt
> handler is a big deal.
> 
> The patches here will speed up the interrupt controller significantly.
> After this series, I have a hard time seeing the interrupt controller
> taking > 20 us and I don't ever see it taking > 30 us ever in my tests
> unless I bring the cpufreq back down.  With the cpufreq at 126 MHz I can
> still see the interrupt handler take > 50 us, so I'm sure we could
> improve this further.  ...but hey, it's a start.
> 
> In addition to the speedup, this series also has the advantage of
> simplifying dwc2 and making it more like everyone else (introducing the
> possibility of future simplifications).  Picking this series up will
> help your diffstat and likely win you friends.  ;)

I gave this a simple spin on my veyron-pinky with both a device attached 
directly to the port as well as with an usb-hub in between. Everything was 
still working smoothly, so

Tested-by: Heiko Stuebner 
--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

On 11/05/2015 10:46 PM, Sergei Shtylyov wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the endpoint Tx
FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is cleared and an
interrupt is generated. May be set simultaneously with TxPktRdy to abort the
packet that is currently being loaded into the FIFO. Note: FlushFIFO should
only be used when TxPktRdy is set. At other times, it may cause data to be
corrupted. Also note that, if the FIFO is double-buffered, FlushFIFO may need
to be set twice to completely clear the FIFO.
 >>

So I don't think this change is valid.


- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
  drivers/usb/musb/musb_host.c | 16 ++--
  1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..5db24f2 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
  struct musb*musb = ep->musb;
  void __iomem*epio = ep->regs;
  u16csr;
-u16lastcsr = 0;
  intretries = 1000;

  csr = musb_readw(epio, MUSB_TXCSR);
-while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-if (csr != lastcsr)
-dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n",
csr);
-lastcsr = csr;
-csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {


 while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {


   Oops, I misread your check, it had &&, not ||.




+csr |= MUSB_TXCSR_FLUSHFIFO;



 No, this clearly contradicts the programmer's guide.


   Still I disagree with this change.

MBR, Sergei

--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Bin Liu]

Hi,

On 11/05/2015 01:46 PM, Sergei Shtylyov wrote:

Hello.

On 11/05/2015 09:53 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set 
at the same time? If so, I will drop this change in V3.


In either case, musb is unable to flush the tx fifo in urb dequque for 
device disconnect, but harmless, so the next two changes are important 
to give better user experience.


Thanks,
-Bin.



So I don't think this change is valid.


- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
  drivers/usb/musb/musb_host.c | 16 ++--
  1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..5db24f2 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct
musb_hw_ep *ep)
  struct musb*musb = ep->musb;
  void __iomem*epio = ep->regs;
  u16csr;
-u16lastcsr = 0;
  intretries = 1000;

  csr = musb_readw(epio, MUSB_TXCSR);
-while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-if (csr != lastcsr)
-dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr:
%02x\n", csr);
-lastcsr = csr;
-csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr &
MUSB_TXCSR_TXPKTRDY)) {


 while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {


+csr |= MUSB_TXCSR_FLUSHFIFO;


 No, this clearly contradicts the programmer's guide.




--
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

Re: [PATCH] usb: musb: fix tx fifo flush handling

[Bin Liu]

On 11/05/2015 12:25 PM, Felipe Balbi wrote:


Hi,

Bin Liu  writes:

Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.

- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
  drivers/usb/musb/musb_host.c | 16 ++--
  1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..f9d4b4f 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
struct musb *musb = ep->musb;
void __iomem*epio = ep->regs;
u16 csr;
-   u16 lastcsr = 0;
int retries = 1000;

csr = musb_readw(epio, MUSB_TXCSR);
-   while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-   if (csr != lastcsr)
-   dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: 
%02x\n", csr);
-   lastcsr = csr;
-   csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+   while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {


this doesn't really fix the problem. This check will be true if either
or both bits are set. You want only when both are set, so it should be:

while ((csr & FIFONOTEMPTY) && (csr & TXPKTRDY))



Ahh, I should have used (csr & (MUSB_TXCSR_FIFONOTEMPTY | 
MUSB_TXCSR_TXPKTRDY) == MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY), 
but it is too long. V2 comes soon.


Thanks,
-Bin.
--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

Hello.

On 11/05/2015 09:53 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


   Hum, my copy of the MUSBHDRC  programmer's guide says about 
TXCSR.FlushFIFO in host mode:


<<
The CPU writes a 1 to this bit to flush the latest packet from the endpoint Tx 
FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is cleared and an 
interrupt is generated. May be set simultaneously with TxPktRdy to abort the 
packet that is currently being loaded into the FIFO. Note: FlushFIFO should 
only be used when TxPktRdy is set. At other times, it may cause data to be 
corrupted. Also note that, if the FIFO is double-buffered, FlushFIFO may need 
to be set twice to completely clear the FIFO.

>>

   So I don't think this change is valid.


- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 
---
  drivers/usb/musb/musb_host.c | 16 ++--
  1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..5db24f2 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
struct musb *musb = ep->musb;
void __iomem*epio = ep->regs;
u16 csr;
-   u16 lastcsr = 0;
int retries = 1000;

csr = musb_readw(epio, MUSB_TXCSR);
-   while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
-   if (csr != lastcsr)
-   dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: 
%02x\n", csr);
-   lastcsr = csr;
-   csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+   while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {


while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {


+   csr |= MUSB_TXCSR_FLUSHFIFO;


No, this clearly contradicts the programmer's guide.



--
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

Re: Broken usb stick after plug it on an usb port?

[Greg KH]

On Thu, Nov 05, 2015 at 06:03:15PM +0100, Javier Barroso wrote:
> Hello,
> 
> I have a usb stick which was (apparently) broken after plug it on a
> usb port on my laptop [1]

What do you mean exactly by "broken"?

> My sequence of operation is attached at kern.log-commented.gz
> 
> This laptop have a port which "Anytime USB Charge (see inside
> http://www.shopfujitsu.com/www/content/products/notebooks/notebooks.php?products/notebooks/features_benefits/ah572_features_benefits)",
> but it is disabled on BIOS (so there should not be a problem)
> 
> stick owner tell me about this stick was working before I plug it on my 
> laptop.
> 
> Is it possible that my laptop kill/broke usb sticks ? I think this is
> the second time that such thing happens.

Sounds like a really messed up USB port, but note, software can not
"break" any USB device, there is nothing for it to control that can harm
anything.

thanks,

greg k-h
--
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

Re: Broken usb stick after plug it on an usb port?

[Javi Barroso]

Hello,

El 6 de noviembre de 2015 0:28:40 CET, Greg KH  escribió:
>On Thu, Nov 05, 2015 at 06:03:15PM +0100, Javier Barroso wrote:
>> Hello,
>> 
>> I have a usb stick which was (apparently) broken after plug it on a
>> usb port on my laptop [1]
>
>What do you mean exactly by "broken"?
>
USB stick owner tell me about the stick was working about 5 years more or less 
without any problem with her windows laptop. She used this stick to order print 
 some picture last week in a paper shop (other computer different) and it 
worked fine.

The first time that I plugged the stick on my laptop, it was not recognized any 
more. No windows recognize, no linux, no apple.

So the stick is currently broken

>> My sequence of operation is attached at kern.log-commented.gz
>> 
>> This laptop have a port which "Anytime USB Charge (see inside
>>
>http://www.shopfujitsu.com/www/content/products/notebooks/notebooks.php?products/notebooks/features_benefits/ah572_features_benefits)",
>> but it is disabled on BIOS (so there should not be a problem)
>> 
>> stick owner tell me about this stick was working before I plug it on
>my laptop.
>> 
>> Is it possible that my laptop kill/broke usb sticks ? I think this is
>> the second time that such thing happens.
>
>Sounds like a really messed up USB port, but note, software can not
>"break" any USB device, there is nothing for it to control that can
>harm
>anything.

How can I check USB ports on my laptop?

I suspect that cheapest stick are not welcome to that port (both broken sticks 
were gifts)

Thank you very much

--
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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Fri, Nov 06, 2015 at 08:24:15AM +0800, Lu, Baolu wrote:
> Yeah, sorry about it.
> 
> 1) apply the attached patch on top the latest clean kernel.
> 2) build and install the kernel.
> 3) boot your machine with the new kernel.
> 4) insert one  Blackmagic Design device into USB3 root port.
> 5) wait for a few seconds
> 6) check below sysfs nodes:
> 
> $ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u1
> $ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u2

OK, what I sent you this earlier today should be exactly this; I think I
might not have sent the results to the list, though.

/* Steinar */
-- 
Homepage: http://www.sesse.net/
--
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

Re: [PATCH 2/3] usb: dwc2: host: Giveback URB in tasklet context

[Doug Anderson]

Alan,

On Thu, Nov 5, 2015 at 7:19 AM, Alan Stern  wrote:
> On Wed, 4 Nov 2015, Doug Anderson wrote:
>
>> In the ChromeOS gerrit
>>  Julius Werner
>> points out that for EHCI it was good to take the optimization from
>> commit 9118f9eb4f1e ("USB: EHCI: improve interrupt qh unlink") before
>> this one.  I'm still trying to learn USB / dwc2 so it's unclear to me
>> whether we also need a similar change before landing.
>>
>> I'll see if I can do some investigation about this and also some
>> benchmarking before and after.  Certainly profiling the interrupt
>> handler itself showed a huge improvement, but I'd hate to see a
>> regression elsewhere.
>>
>> If anyone else knows better than I, please speak up!  :)
>
> This is a matter of both efficiency and correctness.  Giving back URBs
> in a tasklet is not a simple change.
>
> Have you read the kerneldoc for usb_submit_urb() in
> drivers/usb/core/urb.c?  The portion about "Reserved Bandwidth
> Transfers" is highly relevant.  I don't know how dwc2 goes about
> reserving bandwidth for periodic transfers, but if it relies on the
> endpoint queue being non-empty to maintain a reservation then it will
> be affected by this change.

It does look as if you are right and the reservation will end up being
released.  It looks to me like dwc2_deschedule_periodic() is in charge
of releasing the reservation.  I'll work on trying to actually confirm
this.  I guess I need to find a USB test setup where there are enough
devices that I exceed the available time so I can see the brokenness
of my old solution...

I hadn't realized that this was a correctness problem and not just an
optimization problem, so thank you very much for the info!  :)  I ran
with a bunch of USB devices and it worked fine (and performance
improved!) so I figured I was good to go...  Now I've read the
kerneldoc you pointed at and it was very helpful.  As I understand it,
it's considered OK if I copy what EHCI did and release the reservation
if nothing has been scheduled for 5 ms.

Quoting a friend of mine: I'm now all done adding the delayed
reservation release code.  Now I just need to compile it and test it.
:-P  My plan is add some printouts to my current implementation to see
cases where the deferred "unreserve" actually saved us and (to me)
that will help indicate that it's working properly.  Presumably I
won't see this case hit (or not much) without HCD_BH and I will see
this case with HCD_BH.

Please consider this patch "on hold" until my next spin.

-Doug
--
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

Re: Overly conservative xHCI bandwidth estimation

[Lu, Baolu]

On 11/06/2015 02:45 AM, Steinar H. Gunderson wrote:

On Thu, Nov 05, 2015 at 04:12:24PM +0800, Lu, Baolu wrote:

1) apply the attached patch on top the latest kernel.

I applied on top of a clean 4.3.0.


$ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u1
$ cat /sys/bus/usb/devices//power/usb3_hardware_lpm_u2

klump:~> cat /sys/bus/usb/devices/2-2/product
Intensity Shuttle
klump:~> cat /sys/bus/usb/devices/2-2/power/usb3_hardware_lpm_u1
disabled
klump:~> cat /sys/bus/usb/devices/2-2/power/usb3_hardware_lpm_u2
disabled


Thank you.

Have you set CONFIG_PM? Can you reproduce the problem with this kernel?




Can you please post values of above two sysfs nodes?
A copy of dmesg will also be helpful.

Attached.

/* Steinar */


--
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

Re: [PATCH v2 00/13] usb: dwc2: descriptor dma mode bug fixes

[John Youn]

On 11/5/2015 12:41 AM, Gregory Herrero wrote:
> Hi,
> 
> This patchset contains bug fixes for host descriptor dma mode.
> 
> Descriptor dma mode can't be used as the default mode since controller
> does not support split transfers in this mode.
> So we add a new configuration parameter which allows descriptor dma mode
> to be enabled for full-speed devices only.
> 
> All patches are verified on dwc2 v3.0a with dedicated fifos and our
> main test target was usb audio devices.
> 
> All patches are based on Felipe's testing/next branch.
> 
> Regards,
> Gregory
> 
> History:
> v2:
>   - Use dma cache in "usb: dwc2: host: use kmem cache to allocate descriptors"
> and replace usage of deprecated GFP_DMA32 flag.
>   - Remove "usb: dwc2: host: free status_buf on hcd de-init".
> 
> v1:
>   - Fix compilation error introduced by:
> "usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled"
>   - Fix warning introduced by:
> "usb: dwc2: host: fix descriptor list address masking"
> 
> Gregory Herrero (11):
>   usb: dwc2: host: ensure filling of isoc desc is correctly done
>   usb: dwc2: host: set active bit in isochronous descriptors
>   usb: dwc2: host: rework isochronous halt path
>   usb: dwc2: host: fix use of qtd after free in desc dma mode
>   usb: dwc2: host: spinlock release channel
>   usb: dwc2: host: add function to compare frame index
>   usb: dwc2: host: program descriptor for next frame
>   usb: dwc2: host: always increment available host channel during
> release
>   usb: dwc2: host: process all completed urbs
>   usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled
>   usb: dwc2: host: use kmem cache to allocate descriptors
> 
> Mian Yousaf Kaukab (2):
>   usb: dwc2: host: enable descriptor dma for fs devices
>   usb: dwc2: host: fix descriptor list address masking
> 
>  drivers/usb/dwc2/core.c  |  37 +--
>  drivers/usb/dwc2/core.h  |  26 +
>  drivers/usb/dwc2/hcd.c   |  76 +-
>  drivers/usb/dwc2/hcd.h   |  19 
>  drivers/usb/dwc2/hcd_ddma.c  | 240 
> +++
>  drivers/usb/dwc2/hcd_intr.c  |  15 ++-
>  drivers/usb/dwc2/hcd_queue.c |   2 +-
>  drivers/usb/dwc2/hw.h|   4 -
>  drivers/usb/dwc2/platform.c  |   4 +
>  9 files changed, 367 insertions(+), 56 deletions(-)
> 

For this series:

Acked-by: John Youn 


I'm not able to test DDMA on host. But I checked that it didn't
break anything else.

Regards,
John


--
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

Re: [PATCH v2 00/13] usb: dwc2: descriptor dma mode bug fixes

[Doug Anderson]

Hi,

On Thu, Nov 5, 2015 at 12:41 AM, Gregory Herrero
 wrote:
> Hi,
>
> This patchset contains bug fixes for host descriptor dma mode.
>
> Descriptor dma mode can't be used as the default mode since controller
> does not support split transfers in this mode.
> So we add a new configuration parameter which allows descriptor dma mode
> to be enabled for full-speed devices only.
>
> All patches are verified on dwc2 v3.0a with dedicated fifos and our
> main test target was usb audio devices.
>
> All patches are based on Felipe's testing/next branch.
>
> Regards,
> Gregory
>
> History:
> v2:
>   - Use dma cache in "usb: dwc2: host: use kmem cache to allocate descriptors"
> and replace usage of deprecated GFP_DMA32 flag.
>   - Remove "usb: dwc2: host: free status_buf on hcd de-init".
>
> v1:
>   - Fix compilation error introduced by:
> "usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled"
>   - Fix warning introduced by:
> "usb: dwc2: host: fix descriptor list address masking"
>
> Gregory Herrero (11):
>   usb: dwc2: host: ensure filling of isoc desc is correctly done
>   usb: dwc2: host: set active bit in isochronous descriptors
>   usb: dwc2: host: rework isochronous halt path
>   usb: dwc2: host: fix use of qtd after free in desc dma mode
>   usb: dwc2: host: spinlock release channel
>   usb: dwc2: host: add function to compare frame index
>   usb: dwc2: host: program descriptor for next frame
>   usb: dwc2: host: always increment available host channel during
> release
>   usb: dwc2: host: process all completed urbs
>   usb: dwc2: host: avoid usage of dma_alloc_coherent with irqs disabled
>   usb: dwc2: host: use kmem cache to allocate descriptors
>
> Mian Yousaf Kaukab (2):
>   usb: dwc2: host: enable descriptor dma for fs devices
>   usb: dwc2: host: fix descriptor list address masking
>
>  drivers/usb/dwc2/core.c  |  37 +--
>  drivers/usb/dwc2/core.h  |  26 +
>  drivers/usb/dwc2/hcd.c   |  76 +-
>  drivers/usb/dwc2/hcd.h   |  19 
>  drivers/usb/dwc2/hcd_ddma.c  | 240 
> +++
>  drivers/usb/dwc2/hcd_intr.c  |  15 ++-
>  drivers/usb/dwc2/hcd_queue.c |   2 +-
>  drivers/usb/dwc2/hw.h|   4 -
>  drivers/usb/dwc2/platform.c  |   4 +
>  9 files changed, 367 insertions(+), 56 deletions(-)

With the previous version of this series, I could confirm that the
series didn't break things terribly (after fixing the DMA flag) and it
_sometimes_ made DMA Descriptor mode work for me (I believe it never
worked before).  It was unclear why it didn't work sometimes and I
haven't had a chance to debug it yet.

I'm not sure I can give a Tested-by for that, but I figured I'd at
least let you know the current status on my rk3288 board.

-Doug
--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Bin Liu]

Hi,

On 11/05/2015 02:04 PM, Sergei Shtylyov wrote:

On 11/05/2015 10:54 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the 
flushFIFO bit, and wait for the interrupt.


However, the driver does 1000-times retry loop to probe the FIFONOTEMPTY 
bit. This is there in the very first version of the driver back in 2008. 
So I would think musb just behaves like that, rather than an errata. 
Also the WARN() was not there in the early version of the driver.


Please guide what we will do with this patch? Should I send V3 with the 
first change removed, or just drop this patch completely?


Thanks,
-Bin.




Thanks,
-Bin.



So I don't think this change is valid.


- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. But it seems to be harmless,
   since the tx fifo flush is done again in musb_ep_program() when
   re-use the hw_ep.

   But the WARN() scares many end users, so change it to dev_dbg().

- applications could queue up many tx urbs, then the 1ms delay could
   causes minutes of delay in device disconnect. So remove it to get
   better user experience. The 1ms delay does not help the flushing
   anyway.

- cleanup the debug code - related to lastcsr.

Signed-off-by: Bin Liu 


[...]

MBR, Sergei


--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

On 11/05/2015 11:53 PM, Sergei Shtylyov wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the flushFIFO
bit, and wait for the interrupt.


That's oversimplified, consider the double-buffered FIFO. ;-)


However, the driver does 1000-times retry loop to probe the FIFONOTEMPTY bit.
This is there in the very first version of the driver back in 2008. So I would
think musb just behaves like that, rather than an errata. Also the WARN() was
not there in the early version of the driver.


Yes, but added shortly after, in 2008.


   Actually, the driver has always loudly complained in this case -- I've 
just rummaged in the commits.


MBR, Sergei

--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Bin Liu]

On 11/05/2015 02:53 PM, Sergei Shtylyov wrote:

On 11/05/2015 11:34 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit
(below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At
other
times, it may cause data to be corrupted. Also note that, if the
FIFO is
double-buffered, FlushFIFO may need to be set twice to completely
clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the
flushFIFO
bit, and wait for the interrupt.


That's oversimplified, consider the double-buffered FIFO. ;-)


Agreed, but my point was the driver doing the 1000 times retry must be 
for a reason.


But I guess the problem is nobody knows the reason...

Regards,
-Bin.




However, the driver does 1000-times retry loop to probe the
FIFONOTEMPTY bit.
This is there in the very first version of the driver back in 2008. So
I would
think musb just behaves like that, rather than an errata. Also the
WARN() was
not there in the early version of the driver.


Yes, but added shortly after, in 2008.


Please guide what we will do with this patch? Should I send V3 with
the first
change removed, or just drop this patch completely?


I'll let Felipe decide, it's his domain.


Thanks,
-Bin.


[...]

MBR, Sergei


--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

Hello.

On 11/06/2015 12:06 AM, Felipe Balbi wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
MUSB_TXCSR_TXPKTRDY should be checked, not set.


 Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
  >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


 Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


 Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the flushFIFO
bit, and wait for the interrupt.


 That's oversimplified, consider the double-buffered FIFO. ;-)


Seems like we have some misinterpretation here and IMO Bin's patch is
almost complete. The way I read MUSB's docs, it tells me three different
things:

1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set


   Don't see where they say that...


Bin's new while condition solves this part:

+ while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {

2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
twice.

This does not seem to be taken care of, but it would be
something below:


   Why? It should just take another loop iteration to clear the 
double-buffered FIFO, as far as my reading of the code goes...




csr |= MUSB_TXCSR_FLUSH_FIFO;
musb_writew(epio, MUSB_TXCSR, csr);

if (ep->double_buffering_enabled)
musb_write(epio, MUSB_TXCSR, csr);

3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
the packet which is being loaded in the fifo right now.

This seems to be a regression with current patch, however I
don't think current code is perfect either. It unconditionally
sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
of doing that. What happens if we set both of them but there's
no packet being currently loaded into the FIFO ?

To minimize changes, I think Bin just needs to keep the original csr
unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.


   If TxPktRdy is already set, what's the point of forcing it set on write?
If it gets cleared in-between (perhaps the packet was sent?), we'll get into 
the dubious situation with non data in FIFO (you've described), no?



I would also add a very verbose and descriptive comment on that
particular location so we never forget about these MUSB oddities next
time someone's looking at this.


   Agreed.

MBR, Sergei

--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Felipe Balbi]

Hi again,

Felipe Balbi  writes:
> Hi,
>
> Sergei Shtylyov  writes:
>> On 11/05/2015 11:34 PM, Bin Liu wrote:
>>
>>> Here are a few changes in musb_h_tx_flush_fifo().
>>>
>>> - Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
>>>musb_h_tx_flush_fifo()), the datasheet says that
>>> MUSB_TXCSR_FLUSHFIFO
>>>is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
>>>MUSB_TXCSR_TXPKTRDY should be checked, not set.
>>
>> Hum, my copy of the MUSBHDRC  programmer's guide says about
>> TXCSR.FlushFIFO in host mode:
>>
>> <<
>> The CPU writes a 1 to this bit to flush the latest packet from the
>> endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
>> cleared and an interrupt is generated. May be set simultaneously with
>> TxPktRdy to abort the packet that is currently being loaded into the
>> FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
>> times, it may cause data to be corrupted. Also note that, if the FIFO is
>> double-buffered, FlushFIFO may need to be set twice to completely clear
>> the FIFO.
>>  >>
>
> So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
> at the
> same time? If so, I will drop this change in V3.

 Yes, I think it's rather clear in this respect.

> In either case, musb is unable to flush the tx fifo in urb dequque for
> device
> disconnect, but harmless, so the next two changes are important to
> give better
> user experience.

 Hm, looks like some errata... and hiding this from users while
 there's no workaround doesn't seem a good policy.
>>>
>>> Well, based on the programmer's guide, the driver should set the flushFIFO
>>> bit, and wait for the interrupt.
>>
>> That's oversimplified, consider the double-buffered FIFO. ;-)
>
> Seems like we have some misinterpretation here and IMO Bin's patch is
> almost complete. The way I read MUSB's docs, it tells me three different
> things:
>
> 1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set
>
>   Bin's new while condition solves this part:
>
> + while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {
>
> 2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
> twice.
>
>   This does not seem to be taken care of, but it would be
>   something below:
>
> csr |= MUSB_TXCSR_FLUSH_FIFO;
> musb_writew(epio, MUSB_TXCSR, csr);
>
> if (ep->double_buffering_enabled)
>   musb_write(epio, MUSB_TXCSR, csr);
>
> 3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
> the packet which is being loaded in the fifo right now.
>
>   This seems to be a regression with current patch, however I
>   don't think current code is perfect either. It unconditionally
>   sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
>   of doing that. What happens if we set both of them but there's
>   no packet being currently loaded into the FIFO ?
>
> To minimize changes, I think Bin just needs to keep the original csr
> unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
> making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.
>
> I would also add a very verbose and descriptive comment on that
> particular location so we never forget about these MUSB oddities next
> time someone's looking at this.

Also, leave the WARN() alone. We don't want FIFOFLUSH to ever fail. If
it does, we're doing it wrong.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Sergei Shtylyov]

On 11/05/2015 11:34 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.


Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the flushFIFO
bit, and wait for the interrupt.


   That's oversimplified, consider the double-buffered FIFO. ;-)


However, the driver does 1000-times retry loop to probe the FIFONOTEMPTY bit.
This is there in the very first version of the driver back in 2008. So I would
think musb just behaves like that, rather than an errata. Also the WARN() was
not there in the early version of the driver.


   Yes, but added shortly after, in 2008.


Please guide what we will do with this patch? Should I send V3 with the first
change removed, or just drop this patch completely?


   I'll let Felipe decide, it's his domain.


Thanks,
-Bin.


[...]

MBR, Sergei

--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Felipe Balbi]

Hi,

Sergei Shtylyov  writes:
> On 11/05/2015 11:34 PM, Bin Liu wrote:
>
>> Here are a few changes in musb_h_tx_flush_fifo().
>>
>> - Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
>>musb_h_tx_flush_fifo()), the datasheet says that
>> MUSB_TXCSR_FLUSHFIFO
>>is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
>>MUSB_TXCSR_TXPKTRDY should be checked, not set.
>
> Hum, my copy of the MUSBHDRC  programmer's guide says about
> TXCSR.FlushFIFO in host mode:
>
> <<
> The CPU writes a 1 to this bit to flush the latest packet from the
> endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
> cleared and an interrupt is generated. May be set simultaneously with
> TxPktRdy to abort the packet that is currently being loaded into the
> FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
> times, it may cause data to be corrupted. Also note that, if the FIFO is
> double-buffered, FlushFIFO may need to be set twice to completely clear
> the FIFO.
>  >>

 So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
 at the
 same time? If so, I will drop this change in V3.
>>>
>>> Yes, I think it's rather clear in this respect.
>>>
 In either case, musb is unable to flush the tx fifo in urb dequque for
 device
 disconnect, but harmless, so the next two changes are important to
 give better
 user experience.
>>>
>>> Hm, looks like some errata... and hiding this from users while
>>> there's no workaround doesn't seem a good policy.
>>
>> Well, based on the programmer's guide, the driver should set the flushFIFO
>> bit, and wait for the interrupt.
>
> That's oversimplified, consider the double-buffered FIFO. ;-)

Seems like we have some misinterpretation here and IMO Bin's patch is
almost complete. The way I read MUSB's docs, it tells me three different
things:

1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set

Bin's new while condition solves this part:

+ while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {

2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
twice.

This does not seem to be taken care of, but it would be
something below:

csr |= MUSB_TXCSR_FLUSH_FIFO;
musb_writew(epio, MUSB_TXCSR, csr);

if (ep->double_buffering_enabled)
musb_write(epio, MUSB_TXCSR, csr);

3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
the packet which is being loaded in the fifo right now.

This seems to be a regression with current patch, however I
don't think current code is perfect either. It unconditionally
sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
of doing that. What happens if we set both of them but there's
no packet being currently loaded into the FIFO ?

To minimize changes, I think Bin just needs to keep the original csr
unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.

I would also add a very verbose and descriptive comment on that
particular location so we never forget about these MUSB oddities next
time someone's looking at this.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Bin Liu]

Hi,

On 11/05/2015 03:07 PM, Felipe Balbi wrote:


Hi again,

Felipe Balbi  writes:

Hi,

Sergei Shtylyov  writes:

On 11/05/2015 11:34 PM, Bin Liu wrote:


Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
MUSB_TXCSR_TXPKTRDY should be checked, not set.


 Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
  >>


So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.


 Yes, I think it's rather clear in this respect.


In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.


 Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.


Well, based on the programmer's guide, the driver should set the flushFIFO
bit, and wait for the interrupt.


 That's oversimplified, consider the double-buffered FIFO. ;-)


Seems like we have some misinterpretation here and IMO Bin's patch is
almost complete. The way I read MUSB's docs, it tells me three different
things:

1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set

Bin's new while condition solves this part:

+ while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {

2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
twice.

This does not seem to be taken care of, but it would be
something below:

csr |= MUSB_TXCSR_FLUSH_FIFO;
musb_writew(epio, MUSB_TXCSR, csr);

if (ep->double_buffering_enabled)
musb_write(epio, MUSB_TXCSR, csr);

3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
the packet which is being loaded in the fifo right now.

This seems to be a regression with current patch, however I
don't think current code is perfect either. It unconditionally
sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
of doing that. What happens if we set both of them but there's
no packet being currently loaded into the FIFO ?

To minimize changes, I think Bin just needs to keep the original csr
unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.



I am not sure this is any different, since TXPKTRDY is checked, setting 
it again along with FLUSHFIFO does not change TXPKTRDY bit, isn't it?



I would also add a very verbose and descriptive comment on that
particular location so we never forget about these MUSB oddities next
time someone's looking at this.


Also, leave the WARN() alone. We don't want FIFOFLUSH to ever fail. If
it does, we're doing it wrong.



Well, I have tried the changes you mentioned above, but the WARN() was 
still triggered for dequeue all the tx urbs while disconnect the device.


I have a few different test cases, as long as a tx urb is queued, the 
WARN() will be triggered when dequque called by device disconnect, that 
is why I concluded that currently musb is never able to flush tx fifo.


If we are doing it wrong, I am not what the problem is...

Regards,
-Bin.
--
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

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Felipe Balbi]

Hi,

Bin Liu  writes:
> Hi,
>
> On 11/05/2015 03:07 PM, Felipe Balbi wrote:
>>
>> Hi again,
>>
>> Felipe Balbi  writes:
>>> Hi,
>>>
>>> Sergei Shtylyov  writes:
 On 11/05/2015 11:34 PM, Bin Liu wrote:

> Here are a few changes in musb_h_tx_flush_fifo().
>
> - Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
> musb_h_tx_flush_fifo()), the datasheet says that
> MUSB_TXCSR_FLUSHFIFO
> is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
> MUSB_TXCSR_TXPKTRDY should be checked, not set.

  Hum, my copy of the MUSBHDRC  programmer's guide says about
 TXCSR.FlushFIFO in host mode:

 <<
 The CPU writes a 1 to this bit to flush the latest packet from the
 endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) 
 is
 cleared and an interrupt is generated. May be set simultaneously with
 TxPktRdy to abort the packet that is currently being loaded into the
 FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At 
 other
 times, it may cause data to be corrupted. Also note that, if the FIFO 
 is
 double-buffered, FlushFIFO may need to be set twice to completely clear
 the FIFO.
   >>
>>>
>>> So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
>>> at the
>>> same time? If so, I will drop this change in V3.
>>
>>  Yes, I think it's rather clear in this respect.
>>
>>> In either case, musb is unable to flush the tx fifo in urb dequque for
>>> device
>>> disconnect, but harmless, so the next two changes are important to
>>> give better
>>> user experience.
>>
>>  Hm, looks like some errata... and hiding this from users while
>> there's no workaround doesn't seem a good policy.
>
> Well, based on the programmer's guide, the driver should set the flushFIFO
> bit, and wait for the interrupt.

  That's oversimplified, consider the double-buffered FIFO. ;-)
>>>
>>> Seems like we have some misinterpretation here and IMO Bin's patch is
>>> almost complete. The way I read MUSB's docs, it tells me three different
>>> things:
>>>
>>> 1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set
>>>
>>> Bin's new while condition solves this part:
>>>
>>> + while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & 
>>> MUSB_TXCSR_TXPKTRDY)) {
>>>
>>> 2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
>>> twice.
>>>
>>> This does not seem to be taken care of, but it would be
>>> something below:
>>>
>>> csr |= MUSB_TXCSR_FLUSH_FIFO;
>>> musb_writew(epio, MUSB_TXCSR, csr);
>>>
>>> if (ep->double_buffering_enabled)
>>> musb_write(epio, MUSB_TXCSR, csr);
>>>
>>> 3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
>>> the packet which is being loaded in the fifo right now.
>>>
>>> This seems to be a regression with current patch, however I
>>> don't think current code is perfect either. It unconditionally
>>> sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
>>> of doing that. What happens if we set both of them but there's
>>> no packet being currently loaded into the FIFO ?
>>>
>>> To minimize changes, I think Bin just needs to keep the original csr
>>> unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
>>> making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.
>>>
>
> I am not sure this is any different, since TXPKTRDY is checked, setting 
> it again along with FLUSHFIFO does not change TXPKTRDY bit, isn't it?

you're missing one detail:

If you set only FLUSHFIFO, TXPKTRDY is cleared and the packet *already*
in the FIFO gets flushed.

If you set them both together, you're telling MUSB that you want to
flush a packet which is *still* being loaded into the FIFO. After that
TXPKTRDY will be cleared by MUSB.

>>> I would also add a very verbose and descriptive comment on that
>>> particular location so we never forget about these MUSB oddities next
>>> time someone's looking at this.
>>
>> Also, leave the WARN() alone. We don't want FIFOFLUSH to ever fail. If
>> it does, we're doing it wrong.
>>
>
> Well, I have tried the changes you mentioned above, but the WARN() was 
> still triggered for dequeue all the tx urbs while disconnect the
> device.

then we need to figure out why isn't the patcket being flushed. Are you
using double buffering ? Is a packet being loaded in the FIFO right now?
Does it change if you set *only* FLUSHFIFO but *not* TXKPKTRDY ?

> I have a few different test cases, as long as a tx urb is queued, the 
> WARN() will be triggered when dequque called by device disconnect, that 
> is why I concluded that currently musb is never able to flush tx fifo.
>
> If we are doing it wrong, 

Re: [PATCH v2] usb: musb: fix tx fifo flush handling

[Felipe Balbi]

Hi,

Sergei Shtylyov  writes:
> Hello.
>
> On 11/06/2015 12:06 AM, Felipe Balbi wrote:
>
 Here are a few changes in musb_h_tx_flush_fifo().

 - Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
 musb_h_tx_flush_fifo()), the datasheet says that
 MUSB_TXCSR_FLUSHFIFO
 is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
 MUSB_TXCSR_TXPKTRDY should be checked, not set.
>>>
>>>  Hum, my copy of the MUSBHDRC  programmer's guide says about
>>> TXCSR.FlushFIFO in host mode:
>>>
>>> <<
>>> The CPU writes a 1 to this bit to flush the latest packet from the
>>> endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
>>> cleared and an interrupt is generated. May be set simultaneously with
>>> TxPktRdy to abort the packet that is currently being loaded into the
>>> FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
>>> times, it may cause data to be corrupted. Also note that, if the FIFO is
>>> double-buffered, FlushFIFO may need to be set twice to completely clear
>>> the FIFO.
>>>   >>
>>
>> So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
>> at the
>> same time? If so, I will drop this change in V3.
>
>  Yes, I think it's rather clear in this respect.
>
>> In either case, musb is unable to flush the tx fifo in urb dequque for
>> device
>> disconnect, but harmless, so the next two changes are important to
>> give better
>> user experience.
>
>  Hm, looks like some errata... and hiding this from users while
> there's no workaround doesn't seem a good policy.

 Well, based on the programmer's guide, the driver should set the flushFIFO
 bit, and wait for the interrupt.
>>>
>>>  That's oversimplified, consider the double-buffered FIFO. ;-)
>>
>> Seems like we have some misinterpretation here and IMO Bin's patch is
>> almost complete. The way I read MUSB's docs, it tells me three different
>> things:
>>
>> 1. FIFONOTEMPTY is valid *only* when TXPKTRDY is also set
>
> Don't see where they say that...

it's subtle, but it's there:


" When the TxPktRdy bit is set, either manually or automatically, the
packet is deemed ready to be sent. The FIFONotEmpty bit in TxCSR is also
set.

When the packet has been successfully sent, both TxPktRdy and
FIFONotEmpty will be cleared and the appropriate Tx endpoint interrupt
generated (if enabled). The next packet can then be loaded into the
FIFO.  "

And this is also easy to conclude when you realise that FIFONotEmpty is
set when "at least 1 packet has been loaded into the FIFO".

>>  Bin's new while condition solves this part:
>>
>> + while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) 
>> {
>>
>> 2. If we have Double Buffering, we need to set MUSB_TXCSR_FLUSHFIFO
>> twice.
>>
>>  This does not seem to be taken care of, but it would be
>>  something below:
>
> Why? It should just take another loop iteration to clear the 
> double-buffered FIFO, as far as my reading of the code goes...

right. Or we avoid the loop altogether :-) There's no reason for that
loop; We only need it now because this code doesn't work :-p

>> csr |= MUSB_TXCSR_FLUSH_FIFO;
>> musb_writew(epio, MUSB_TXCSR, csr);
>>
>> if (ep->double_buffering_enabled)
>>  musb_write(epio, MUSB_TXCSR, csr);
>>
>> 3. We can set TXPKTRDY *together* with MUSB_TXCSR_FLUSH_FIFO to cancel
>> the packet which is being loaded in the fifo right now.
>>
>>  This seems to be a regression with current patch, however I
>>  don't think current code is perfect either. It unconditionally
>>  sets FLUSH_FIFO and TXPKTRDY without caring for the consequences
>>  of doing that. What happens if we set both of them but there's
>>  no packet being currently loaded into the FIFO ?
>>
>> To minimize changes, I think Bin just needs to keep the original csr
>> unconditionally setting both FLUSHFIFO and TXPKTRDY alone while also
>> making sure to check that TKPKTRDY is set when checking FIFONOTEMPTY.
>
> If TxPktRdy is already set, what's the point of forcing it set on
> write?

Coming back to what you pointed at originally:

" [...] May be set simultaneously with TxPktRdy to abort the packet that
is currently being loaded into the FIFO. [...]"

It's not setting it back, it's telling MUSB which packet to flush. If
you go back to double buffering. Consider you already have one packet in
the FIFO and you're loading the second. TXPKTRDY will tell MUSB which of
the two you want to flush. (it might be that you either flush the one
currently being loaded or both, but that's not clear in the
documentation, so it would be have to be tested)


And here's another important note:

" Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause 

[PATCH 0/2] Configure AHB to post data transfers

[Andy Gross]

This patch configures the ChipIdea USB 2.0 controller found on
Qualcomm platforms to post data transfers on the AHB bus.  This
yields approximately a 50% increase in performance.

Andy Gross (2):
  usb: chipidea: msm: Use posted data writes on AHB
  usb: host: ehci-msm: Use posted data writes on AHB

 drivers/usb/chipidea/ci_hdrc_msm.c |3 ++-
 drivers/usb/host/ehci-msm.c|4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
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

[PATCH 2/2] usb: host: ehci-msm: Use posted data writes on AHB

[Andy Gross]

This patch sets the AHBMODE to allow for posted data writes.  This
results in higher performance.

Signed-off-by: Andy Gross 
---
 drivers/usb/host/ehci-msm.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ehci-msm.c b/drivers/usb/host/ehci-msm.c
index c4f84c8..c23e285 100644
--- a/drivers/usb/host/ehci-msm.c
+++ b/drivers/usb/host/ehci-msm.c
@@ -57,8 +57,8 @@ static int ehci_msm_reset(struct usb_hcd *hcd)
 
/* bursts of unspecified length. */
writel(0, USB_AHBBURST);
-   /* Use the AHB transactor */
-   writel(0, USB_AHBMODE);
+   /* Use the AHB transactor, allow posted data writes */
+   writel(0x8, USB_AHBMODE);
/* Disable streaming mode and select host mode */
writel(0x13, USB_USBMODE);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
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

Re: Overly conservative xHCI bandwidth estimation

[Steinar H. Gunderson]

On Fri, Nov 06, 2015 at 08:39:28AM +0800, Lu, Baolu wrote:
> Have you set CONFIG_PM?

Yes. CONFIG_PM=y.

> Can you reproduce the problem with this kernel?

I reproduced the U1/U2 disconnect issues several times. I didn't try the
issue of not enough bandwidth for two devices.

/* Steinar */
-- 
Homepage: http://www.sesse.net/
--
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

Re: [PATCH V4] usb: remove unnecessary CONFIG_PM dependency from USB_OTG

[Peter Chen]

On Thu, Nov 05, 2015 at 08:36:41AM -0600, Felipe Balbi wrote:
> 
> Hi,
> 
> Peter Chen  writes:
> > On Tue, Nov 03, 2015 at 09:51:11PM -0600, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Peter Chen  writes:
> >> > On Tue, Nov 03, 2015 at 07:56:55AM -0600, Felipe Balbi wrote:
> >> >> 
> >> >> Hi,
> >> >> 
> >> >> Nathan Sullivan  writes:
> >> >> > The USB OTG support currently depends on power management
> >> >> > (CONFIG_PM) being enabled, but does not actually need it enabled.
> >> >> > Remove this dependency.
> >> >> >
> >> >> > Tested on Bay Trail hardware with dwc3 USB.
> >> >> >
> >> >> > Signed-off-by: Nathan Sullivan 
> >> >> > ---
> >> >> >  drivers/usb/core/Kconfig |1 -
> >> >> >  1 file changed, 1 deletion(-)
> >> >> >
> >> >> > diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> >> >> > index a99c89e..9c5cdf3 100644
> >> >> > --- a/drivers/usb/core/Kconfig
> >> >> > +++ b/drivers/usb/core/Kconfig
> >> >> > @@ -43,7 +43,6 @@ config USB_DYNAMIC_MINORS
> >> >> >  
> >> >> >  config USB_OTG
> >> >> >   bool "OTG support"
> >> >> > - depends on PM
> >> >> 
> >> >> I don't think this is correct. OTG depends on USB bus suspend, which is
> >> >> only available on PM builds. Care to further detail why you think PM is
> >> >> not needed on OTG ?
> >> >> 
> >> >
> >> > OTG depends on USB bus suspend is not a must, the hardware controlled OTG
> >> > design do HNP when the bus goes to suspend; but if the software
> >> > implements OTG FSM, it is the user option whether do HNP, and bus
> >> > suspend is controlled by OTG FSM software (stop SOF), but not by host 
> >> > stack (eg, ehci).
> >> >
> >> > I am sorry I did not consider the legacy OTG design, this patch should
> >> > be dropped.
> >> 
> >> there is no "legacy" OTG design. OTG requires a bus suspend to enter
> >> HNP, and that's achieved by stopping all transfers and avoid new URB
> >> submission so usbcore can put the bus in suspend (by means of USB
> >> autosuspend). If you're bypassing that in the OTG FSM thing, that needs
> >> to be fixed ASAP as that makes it a lot harder for any generic changes
> >> in usbcore to be validated. Specially when you consider not many will
> >> have whatever special HW which, likely, doesn't even work with mainline
> >> to validate a change.
> >> 
> >
> > Felipe, thanks for your comments.
> >
> > But you may need to consider the user option ~a_bus_req (for A) and
> > b_bus_req (for B) when do HNP, we can't do HNP without user option.
> >
> > - Eg, if the bus enters suspend, but the A does not want role switch,
> > we can't try to do HNP, the same for B device.
> > - The A device may want to support auto-suspend but without role switch.
> >
> > You are absolutely right, the SAFE HNP needs to do auto-suspend first, we
> > need to add this in documentation. But it does not mean OTG FSM design is
> > incorrect, there are two things, eg, if we want to do HNP safely, we
> > need
> 
> sure it does. If you're bypassing anything in usbcore, it's wrong :-)
> For that [ab]_bus_req polling, as soon as you notice that flag, you
> should just giveback all URBs to the link partner so the link can
> autosuspend and HNP will happen exactly the same.
> 
> > to do below things:
> > /* B requests HNP */
> > echo 1 > /sys/bus/../otg_fsm//inputs/b_bus_req 
> >
> > /* Put the A to suspend */
> > echo auto > /sys/bus/usb/devices/1-1/power/control 
> > /* A goes to try HNP */
> > echo 0 > /sys/bus/../otg_fsm/inputs/a_bus_req
> 
> When you echo 0 to a_bus_req, A should stop all in-flight transfers to
> the link partner and wait for the bus to autosuspend. There should be no
> differences other than the need to set these flags.
> 
> This even gives us opportunity to tune "how fast" we wand HNP to happen
> by fiddling with autosuspend delay ;-)
> 

That's reasonable, we will add this in OTG FSM, thanks.

-- 

Best Regards,
Peter Chen
--
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

[PATCH 1/2] usb: chipidea: msm: Use posted data writes on AHB

[Andy Gross]

This patch sets the AHBMODE to allow for posted data writes.  This
results in higher performance.

Signed-off-by: Andy Gross 
---
 drivers/usb/chipidea/ci_hdrc_msm.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c 
b/drivers/usb/chipidea/ci_hdrc_msm.c
index d79ecc0..3889809 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -25,7 +25,8 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, 
unsigned event)
case CI_HDRC_CONTROLLER_RESET_EVENT:
dev_dbg(dev, "CI_HDRC_CONTROLLER_RESET_EVENT received\n");
writel(0, USB_AHBBURST);
-   writel(0, USB_AHBMODE);
+   /* use AHB transactor, allow posted data writes */
+   writel(0x8, USB_AHBMODE);
usb_phy_init(ci->usb_phy);
break;
case CI_HDRC_CONTROLLER_STOPPED_EVENT:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

--
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

Re: [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable

[Peter Chen]

On Tue, Nov 03, 2015 at 01:53:40PM +0100, Robert Baldyga wrote:
> So far it was decided during the bind process whether is iso altsetting
> included to f_sourcesink function or not. This decision was based on
> availability of isochronous endpoints.
> 
> Since we can assemble gadget driver using composite framework and configfs
> from many different functions, availability of given type of endpoint
> can depend on selected components or even on their order in given
> configuration.
> 
> This can result with non-obvious behavior - even small, seemingly unrelated
> change in gadget configuration can decide if we have second altsetting with
> iso endpoints in given sourcesink function instance or not.
> 
> Because of this it's way better to have additional parameter allowing user
> to decide if he/she wants to have iso altsetting, and if iso altsetting is
> included, and there are no iso endpoints available, function bind will fail
> instead of silently allowing to have non-complete function bound.

Hi Robert,

Why another isoc_enabled parameter is needed instead of judging if it
is supported through gadget framework? Any use cases can't be supported
by current way?

Peter

> 
> Signed-off-by: Robert Baldyga 
> ---
>  drivers/usb/gadget/function/f_sourcesink.c | 99 
> --
>  drivers/usb/gadget/function/g_zero.h   |  3 +
>  drivers/usb/gadget/legacy/zero.c   |  6 ++
>  3 files changed, 77 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
> b/drivers/usb/gadget/function/f_sourcesink.c
> index d7646d3..1d6ec88 100644
> --- a/drivers/usb/gadget/function/f_sourcesink.c
> +++ b/drivers/usb/gadget/function/f_sourcesink.c
> @@ -56,6 +56,7 @@ struct f_sourcesink {
>   unsigned isoc_maxpacket;
>   unsigned isoc_mult;
>   unsigned isoc_maxburst;
> + unsigned isoc_enabled;
>   unsigned buflen;
>  };
>  
> @@ -347,17 +348,28 @@ sourcesink_bind(struct usb_configuration *c, struct 
> usb_function *f)
>  
>   /* allocate bulk endpoints */
>   ss->in_ep = usb_ep_autoconfig(cdev->gadget, _source_desc);
> - if (!ss->in_ep) {
> -autoconf_fail:
> - ERROR(cdev, "%s: can't autoconfigure on %s\n",
> - f->name, cdev->gadget->name);
> - return -ENODEV;
> - }
> + if (!ss->in_ep)
> + goto autoconf_fail;
>  
>   ss->out_ep = usb_ep_autoconfig(cdev->gadget, _sink_desc);
>   if (!ss->out_ep)
>   goto autoconf_fail;
>  
> + /* support high speed hardware */
> + hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
> + hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
> +
> + /* support super speed hardware */
> + ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
> + ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
> +
> + if (!ss->isoc_enabled) {
> + fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
> + hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
> + ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
> + goto no_iso;
> + }
> +
>   /* sanity check the isoc module parameters */
>   if (ss->isoc_interval < 1)
>   ss->isoc_interval = 1;
> @@ -379,30 +391,14 @@ autoconf_fail:
>   /* allocate iso endpoints */
>   ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
>   if (!ss->iso_in_ep)
> - goto no_iso;
> + goto autoconf_fail;
>  
>   ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, _iso_sink_desc);
> - if (!ss->iso_out_ep) {
> - usb_ep_autoconfig_release(ss->iso_in_ep);
> - ss->iso_in_ep = NULL;
> -no_iso:
> - /*
> -  * We still want to work even if the UDC doesn't have isoc
> -  * endpoints, so null out the alt interface that contains
> -  * them and continue.
> -  */
> - fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
> - hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
> - ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
> - }
> + if (!ss->iso_out_ep)
> + goto autoconf_fail;
>  
>   if (ss->isoc_maxpacket > 1024)
>   ss->isoc_maxpacket = 1024;
> -
> - /* support high speed hardware */
> - hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
> - hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
> -
>   /*
>* Fill in the HS isoc descriptors from the module parameters.
>* We assume that the user knows what they are doing and won't
> @@ -419,12 +415,6 @@ no_iso:
>   hs_iso_sink_desc.bInterval = ss->isoc_interval;
>   hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>  
> - /* support super speed hardware */
> - ss_source_desc.bEndpointAddress =
> -

Re: Overly conservative xHCI bandwidth estimation

[Lu, Baolu]

On 11/06/2015 08:45 AM, Steinar H. Gunderson wrote:

On Fri, Nov 06, 2015 at 08:39:28AM +0800, Lu, Baolu wrote:

Have you set CONFIG_PM?

Yes. CONFIG_PM=y.


Can you reproduce the problem with this kernel?

I reproduced the U1/U2 disconnect issues several times. I didn't try the
issue of not enough bandwidth for two devices.


Can you please try the not enough bandwidth issue?

Can you cat /sys/bus/usb/devices/2-2/power/usb3_hardware_lpm_u1
and /sys/bus/usb/devices/2-2/power/usb3_hardware_lpm_u2 for several
times? Don't use that device when cat these two files.

I am hoping that LPM works when only one device is used (hence
there is enough bus bandwidth).



/* Steinar */


Thanks,
Baolu
--
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

Re: [PATCH 02/23] usb: gadget: f_sourcesink: compute req size once

[Peter Chen]

On Tue, Nov 03, 2015 at 01:53:41PM +0100, Robert Baldyga wrote:
> Compute request size once before the loop instead of computing it in each
> loop iteration.
> 
> Signed-off-by: Robert Baldyga 
> ---
>  drivers/usb/gadget/function/f_sourcesink.c | 45 
> +++---
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
> b/drivers/usb/gadget/function/f_sourcesink.c
> index 1d6ec88..a8b68c6 100644
> --- a/drivers/usb/gadget/function/f_sourcesink.c
> +++ b/drivers/usb/gadget/function/f_sourcesink.c
> @@ -591,31 +591,30 @@ static int source_sink_start_ep(struct f_sourcesink 
> *ss, bool is_in,
>  {
>   struct usb_ep   *ep;
>   struct usb_request  *req;
> - int i, size, status;
> -
> - for (i = 0; i < 8; i++) {
> - if (is_iso) {
> - switch (speed) {
> - case USB_SPEED_SUPER:
> - size = ss->isoc_maxpacket *
> - (ss->isoc_mult + 1) *
> - (ss->isoc_maxburst + 1);
> - break;
> - case USB_SPEED_HIGH:
> - size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
> - break;
> - default:
> - size = ss->isoc_maxpacket > 1023 ?
> - 1023 : ss->isoc_maxpacket;
> - break;
> - }
> - ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
> - req = ss_alloc_ep_req(ep, size);
> - } else {
> - ep = is_in ? ss->in_ep : ss->out_ep;
> - req = ss_alloc_ep_req(ep, 0);
> + int i, size = 0, status;
> +
> + if (is_iso) {
> + switch (speed) {
> + case USB_SPEED_SUPER:
> + size = ss->isoc_maxpacket *
> + (ss->isoc_mult + 1) *
> + (ss->isoc_maxburst + 1);
> + break;
> + case USB_SPEED_HIGH:
> + size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
> + break;
> + default:
> + size = ss->isoc_maxpacket > 1023 ?
> + 1023 : ss->isoc_maxpacket;
> + break;
>   }
> + ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
> + } else {
> + ep = is_in ? ss->in_ep : ss->out_ep;
> + }
>  
> + for (i = 0; i < 8; i++) {
> + req = ss_alloc_ep_req(ep, size);
>   if (!req)
>   return -ENOMEM;
>  

Reviewed-by: Peter Chen 

-- 

Best Regards,
Peter Chen
--
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

Re: [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable

[Robert Baldyga]

On 11/06/2015 04:05 AM, Peter Chen wrote:
> On Tue, Nov 03, 2015 at 01:53:40PM +0100, Robert Baldyga wrote:
>> So far it was decided during the bind process whether is iso altsetting
>> included to f_sourcesink function or not. This decision was based on
>> availability of isochronous endpoints.
>>
>> Since we can assemble gadget driver using composite framework and configfs
>> from many different functions, availability of given type of endpoint
>> can depend on selected components or even on their order in given
>> configuration.
>>
>> This can result with non-obvious behavior - even small, seemingly unrelated
>> change in gadget configuration can decide if we have second altsetting with
>> iso endpoints in given sourcesink function instance or not.
>>
>> Because of this it's way better to have additional parameter allowing user
>> to decide if he/she wants to have iso altsetting, and if iso altsetting is
>> included, and there are no iso endpoints available, function bind will fail
>> instead of silently allowing to have non-complete function bound.
> 
> Hi Robert,
> 
> Why another isoc_enabled parameter is needed instead of judging if it
> is supported through gadget framework? Any use cases can't be supported
> by current way?
> 

It's because guessing during bind process leads to non-obvious behavior
- we can have iso altsetting included into function or not depending on
many seemingly unrelated factors.

Moreover SourceSink, which is in fact testing Function, is the only
Function which implements conditional altsetting enabling, and we
definitely don't want testing function to behave that much differently
from another USB Functions.

The third reason is that modifying descriptors set depending on
availability of given endpoint type during bind process complicates bind
process automatization, which I implement in this patch set. After all,
I don't know any real USB device doing such strange thing.

Thanks,
Robert

>>
>> Signed-off-by: Robert Baldyga 
>> ---
>>  drivers/usb/gadget/function/f_sourcesink.c | 99 
>> --
>>  drivers/usb/gadget/function/g_zero.h   |  3 +
>>  drivers/usb/gadget/legacy/zero.c   |  6 ++
>>  3 files changed, 77 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
>> b/drivers/usb/gadget/function/f_sourcesink.c
>> index d7646d3..1d6ec88 100644
>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>> @@ -56,6 +56,7 @@ struct f_sourcesink {
>>  unsigned isoc_maxpacket;
>>  unsigned isoc_mult;
>>  unsigned isoc_maxburst;
>> +unsigned isoc_enabled;
>>  unsigned buflen;
>>  };
>>  
>> @@ -347,17 +348,28 @@ sourcesink_bind(struct usb_configuration *c, struct 
>> usb_function *f)
>>  
>>  /* allocate bulk endpoints */
>>  ss->in_ep = usb_ep_autoconfig(cdev->gadget, _source_desc);
>> -if (!ss->in_ep) {
>> -autoconf_fail:
>> -ERROR(cdev, "%s: can't autoconfigure on %s\n",
>> -f->name, cdev->gadget->name);
>> -return -ENODEV;
>> -}
>> +if (!ss->in_ep)
>> +goto autoconf_fail;
>>  
>>  ss->out_ep = usb_ep_autoconfig(cdev->gadget, _sink_desc);
>>  if (!ss->out_ep)
>>  goto autoconf_fail;
>>  
>> +/* support high speed hardware */
>> +hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> +hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
>> +
>> +/* support super speed hardware */
>> +ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> +ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
>> +
>> +if (!ss->isoc_enabled) {
>> +fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
>> +hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
>> +ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>> +goto no_iso;
>> +}
>> +
>>  /* sanity check the isoc module parameters */
>>  if (ss->isoc_interval < 1)
>>  ss->isoc_interval = 1;
>> @@ -379,30 +391,14 @@ autoconf_fail:
>>  /* allocate iso endpoints */
>>  ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
>>  if (!ss->iso_in_ep)
>> -goto no_iso;
>> +goto autoconf_fail;
>>  
>>  ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, _iso_sink_desc);
>> -if (!ss->iso_out_ep) {
>> -usb_ep_autoconfig_release(ss->iso_in_ep);
>> -ss->iso_in_ep = NULL;
>> -no_iso:
>> -/*
>> - * We still want to work even if the UDC doesn't have isoc
>> - * endpoints, so null out the alt interface that contains
>> - * them and continue.
>> - */
>> -fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
>> -hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
>> -

Re: [RFC PATCH 1/2] usb: storage: scsiglue: further describe our 240 sector limit

[Felipe Balbi]

Hi,

Alan Stern  writes:
> On Wed, 4 Nov 2015, Felipe Balbi wrote:
>
>> >> +  * Tests show that other operating have similar limits with Microsoft
>> >> +  * Windows™ 7 limitting transfers to 128 sectors for both USB2 and USB3
>> >> +  * and Apple Mac OS X™ 10.11 limitting transfers to 256 sectors for USB2
>> >> +  * and 2048 for USB3 devices.
>> >
>> > Do we try to avoid non-ASCII characters in comments, or are they 
>> > accepted?  Documentation/CodingStyle doesn't say anything about it.
>> 
>> I can remove, no problems. Just remove it or add (TM) ?
>
> Up to you.  I wouldn't bother.  If there are any legal requirements, 
> I'm not aware of them.

all right, removed.

-- 
balbi


signature.asc
Description: PGP signature

Re: [RFC PATCH 2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

[Felipe Balbi]

Hi,

Oliver Neukum  writes:
> On Wed, 2015-11-04 at 16:26 -0500, Alan Stern wrote:
>> This doesn't need to be stored as one of the usb-storage flags.  And
>> since we are close to running out of flag bits (on 32-bit
>> architectures), it would be better not to use one of them for this.
>
> Do we want the opposite flag? 25% for known good devices
> is visible.

You mean for USB2 devices ? I was thinking about that last night and
wanted to "whitelist" at least our own mass storage gadget, provided
that it works just fine with anything you give it (so far).

Something like US_FL_NO_SECTOR_LIMIT ?

-- 
balbi


signature.asc
Description: PGP signature

Re: [RFC PATCH 2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

[Alan Stern]

On Thu, 5 Nov 2015, Felipe Balbi wrote:

> Hi,
> 
> Oliver Neukum  writes:
> > On Wed, 2015-11-04 at 16:26 -0500, Alan Stern wrote:
> >> This doesn't need to be stored as one of the usb-storage flags.  And
> >> since we are close to running out of flag bits (on 32-bit
> >> architectures), it would be better not to use one of them for this.
> >
> > Do we want the opposite flag? 25% for known good devices
> > is visible.
> 
> You mean for USB2 devices ? I was thinking about that last night and
> wanted to "whitelist" at least our own mass storage gadget, provided
> that it works just fine with anything you give it (so far).
> 
> Something like US_FL_NO_SECTOR_LIMIT ?

There already is a whitelist flag for the Mass Storage gadget:  
US_FL_CAPACITY_OK.  Right now its meaning is limited to the READ
CAPACITY bug, but it could easily be repurposed since no other 
whitelist entries use that flag.

Alan Stern

--
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

Re: [RFC PATCH 2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

[Felipe Balbi]

Hi,

Alan Stern  writes:
> On Thu, 5 Nov 2015, Felipe Balbi wrote:
>
>> Hi,
>> 
>> Oliver Neukum  writes:
>> > On Wed, 2015-11-04 at 16:26 -0500, Alan Stern wrote:
>> >> This doesn't need to be stored as one of the usb-storage flags.  And
>> >> since we are close to running out of flag bits (on 32-bit
>> >> architectures), it would be better not to use one of them for this.
>> >
>> > Do we want the opposite flag? 25% for known good devices
>> > is visible.
>> 
>> You mean for USB2 devices ? I was thinking about that last night and
>> wanted to "whitelist" at least our own mass storage gadget, provided
>> that it works just fine with anything you give it (so far).
>> 
>> Something like US_FL_NO_SECTOR_LIMIT ?
>
> There already is a whitelist flag for the Mass Storage gadget:  
> US_FL_CAPACITY_OK.  Right now its meaning is limited to the READ
> CAPACITY bug, but it could easily be repurposed since no other 
> whitelist entries use that flag.

cool, I'll have a look at try to make sure we increase sector limit to
2048 for that one as well :-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] uas: use the BIT() macro

[Hans de Goede]

Hi,

On 05-11-15 16:20, Oliver Neukum wrote:

Use this macro to make the driver more readable.

Signed-off-by: Oliver Neukum 


Looks good to me:

Reviewed-by: Hans de Goede 

Regards,

Hans



---
  drivers/usb/storage/uas.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index eb68162..45ed241 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -49,18 +49,18 @@ struct uas_dev_info {
  };

  enum {
-   SUBMIT_STATUS_URB   = (1 << 1),
-   ALLOC_DATA_IN_URB   = (1 << 2),
-   SUBMIT_DATA_IN_URB  = (1 << 3),
-   ALLOC_DATA_OUT_URB  = (1 << 4),
-   SUBMIT_DATA_OUT_URB = (1 << 5),
-   ALLOC_CMD_URB   = (1 << 6),
-   SUBMIT_CMD_URB  = (1 << 7),
-   COMMAND_INFLIGHT= (1 << 8),
-   DATA_IN_URB_INFLIGHT= (1 << 9),
-   DATA_OUT_URB_INFLIGHT   = (1 << 10),
-   COMMAND_ABORTED = (1 << 11),
-   IS_IN_WORK_LIST = (1 << 12),
+   SUBMIT_STATUS_URB   = BIT(1),
+   ALLOC_DATA_IN_URB   = BIT(2),
+   SUBMIT_DATA_IN_URB  = BIT(3),
+   ALLOC_DATA_OUT_URB  = BIT(4),
+   SUBMIT_DATA_OUT_URB = BIT(5),
+   ALLOC_CMD_URB   = BIT(6),
+   SUBMIT_CMD_URB  = BIT(7),
+   COMMAND_INFLIGHT= BIT(8),
+   DATA_IN_URB_INFLIGHT= BIT(9),
+   DATA_OUT_URB_INFLIGHT   = BIT(10),
+   COMMAND_ABORTED = BIT(11),
+   IS_IN_WORK_LIST = BIT(12),
  };

  /* Overrides scsi_pointer */


--
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

Re: [RFC PATCH 1/2] usb: storage: scsiglue: further describe our 240 sector limit

[Alan Stern]

On Wed, 4 Nov 2015, Felipe Balbi wrote:

> >> +   * Tests show that other operating have similar limits with Microsoft
> >> +   * Windows™ 7 limitting transfers to 128 sectors for both USB2 and USB3
> >> +   * and Apple Mac OS X™ 10.11 limitting transfers to 256 sectors for USB2
> >> +   * and 2048 for USB3 devices.
> >
> > Do we try to avoid non-ASCII characters in comments, or are they 
> > accepted?  Documentation/CodingStyle doesn't say anything about it.
> 
> I can remove, no problems. Just remove it or add (TM) ?

Up to you.  I wouldn't bother.  If there are any legal requirements, 
I'm not aware of them.

Alan Stern

--
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

Re: [RFC PATCH 2/2] usb: storage: scsiglue: limit USB3 devices to 2048 sectors

[Alan Stern]

On Thu, 5 Nov 2015, Oliver Neukum wrote:

> On Wed, 2015-11-04 at 16:26 -0500, Alan Stern wrote:
> > This doesn't need to be stored as one of the usb-storage flags.  And
> > since we are close to running out of flag bits (on 32-bit
> > architectures), it would be better not to use one of them for this.
> 
> Do we want the opposite flag? 25% for known good devices
> is visible.

We don't have a list of known good devices.  Such a list would
undoubtedly be very large, unmanageable in practice.

Since the max_sectors value is exposed in sysfs, we can rely on 
userspace scripts to increase it if the device is known to work well 
with large transfers.

Alan Stern

--
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

[PATCH] uas: use the BIT() macro

[Oliver Neukum]

Use this macro to make the driver more readable.

Signed-off-by: Oliver Neukum 
---
 drivers/usb/storage/uas.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index eb68162..45ed241 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -49,18 +49,18 @@ struct uas_dev_info {
 };
 
 enum {
-   SUBMIT_STATUS_URB   = (1 << 1),
-   ALLOC_DATA_IN_URB   = (1 << 2),
-   SUBMIT_DATA_IN_URB  = (1 << 3),
-   ALLOC_DATA_OUT_URB  = (1 << 4),
-   SUBMIT_DATA_OUT_URB = (1 << 5),
-   ALLOC_CMD_URB   = (1 << 6),
-   SUBMIT_CMD_URB  = (1 << 7),
-   COMMAND_INFLIGHT= (1 << 8),
-   DATA_IN_URB_INFLIGHT= (1 << 9),
-   DATA_OUT_URB_INFLIGHT   = (1 << 10),
-   COMMAND_ABORTED = (1 << 11),
-   IS_IN_WORK_LIST = (1 << 12),
+   SUBMIT_STATUS_URB   = BIT(1),
+   ALLOC_DATA_IN_URB   = BIT(2),
+   SUBMIT_DATA_IN_URB  = BIT(3),
+   ALLOC_DATA_OUT_URB  = BIT(4),
+   SUBMIT_DATA_OUT_URB = BIT(5),
+   ALLOC_CMD_URB   = BIT(6),
+   SUBMIT_CMD_URB  = BIT(7),
+   COMMAND_INFLIGHT= BIT(8),
+   DATA_IN_URB_INFLIGHT= BIT(9),
+   DATA_OUT_URB_INFLIGHT   = BIT(10),
+   COMMAND_ABORTED = BIT(11),
+   IS_IN_WORK_LIST = BIT(12),
 };
 
 /* Overrides scsi_pointer */
-- 
2.1.4

--
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

Re: [PATCH 2/3] usb: dwc2: host: Giveback URB in tasklet context

[Alan Stern]

On Wed, 4 Nov 2015, Doug Anderson wrote:

> In the ChromeOS gerrit
>  Julius Werner
> points out that for EHCI it was good to take the optimization from
> commit 9118f9eb4f1e ("USB: EHCI: improve interrupt qh unlink") before
> this one.  I'm still trying to learn USB / dwc2 so it's unclear to me
> whether we also need a similar change before landing.
> 
> I'll see if I can do some investigation about this and also some
> benchmarking before and after.  Certainly profiling the interrupt
> handler itself showed a huge improvement, but I'd hate to see a
> regression elsewhere.
> 
> If anyone else knows better than I, please speak up!  :)

This is a matter of both efficiency and correctness.  Giving back URBs 
in a tasklet is not a simple change.

Have you read the kerneldoc for usb_submit_urb() in 
drivers/usb/core/urb.c?  The portion about "Reserved Bandwidth 
Transfers" is highly relevant.  I don't know how dwc2 goes about 
reserving bandwidth for periodic transfers, but if it relies on the 
endpoint queue being non-empty to maintain a reservation then it will 
be affected by this change.

Alan Stern

--
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

Re: [PATCH v2] USB: qmi_wwan: Add quirk for Quectel EC20 Mini PCIe module

[Bjørn Mork]

Petr Štetiar  writes:

> This device has same vendor and product IDs as G2K devices, but it has
> different number of interfaces(4 vs 5) and also different interface
> layout where EC20 has QMI on interface 4 instead of 0.
>
> lsusb output:
>
>   Bus 002 Device 003: ID 05c6:9215 Qualcomm, Inc. Acer Gobi 2000
>   Device Descriptor:
> bLength18
> bDescriptorType 1
> bcdUSB   2.00
> bDeviceClass0 (Defined at Interface level)
> bDeviceSubClass 0
> bDeviceProtocol 0
> bMaxPacketSize064
> idVendor   0x05c6 Qualcomm, Inc.
> idProduct  0x9215 Acer Gobi 2000 Wireless Modem
> bcdDevice2.32
> iManufacturer   1 Quectel
> iProduct2 Quectel LTE Module
> iSerial 0
> bNumConfigurations  1
> Configuration Descriptor:
>   bLength 9
>   bDescriptorType 2
>   wTotalLength  209
>   bNumInterfaces  5
>   bConfigurationValue 1
>   iConfiguration  0
>   bmAttributes 0xa0
> (Bus Powered)
> Remote Wakeup
>   MaxPower  500mA
>
> Signed-off-by: Petr Štetiar 

This looks good to me.  

Acked-by: Bjørn Mork 
--
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