All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct
@ 2020-09-07  7:11 Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder Chunfeng Yun
                   ` (7 more replies)
  0 siblings, 8 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:11 UTC (permalink / raw)
  To: u-boot

Add a member to save xHCI version, it's used some times.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 drivers/usb/host/xhci.c      | 1 +
 include/usb/xhci.h           | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6e..79bfc34 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -682,7 +682,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 			field |= TRB_ISP;
 
 		/* Set the TRB length, TD size, and interrupter fields. */
-		if (HC_VERSION(xhci_readl(&ctrl->hccr->cr_capbase)) < 0x100)
+		if (ctrl->hci_version < 0x100)
 			remainder = xhci_td_remainder(length - running_total);
 		else
 			remainder = xhci_v1_0_td_remainder(running_total,
@@ -830,7 +830,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		field |= 0x1;
 
 	/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-	if (HC_VERSION(xhci_readl(&ctrl->hccr->cr_capbase)) >= 0x100) {
+	if (ctrl->hci_version >= 0x100) {
 		if (length > 0) {
 			if (req->requesttype & USB_DIR_IN)
 				field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 126dabc..4be1411 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1283,6 +1283,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 
 	reg = HC_VERSION(xhci_readl(&hccr->cr_capbase));
 	printf("USB XHCI %x.%02x\n", reg >> 8, reg & 0xff);
+	ctrl->hci_version = reg;
 
 	return 0;
 }
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 7d34103..a3e5914 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1227,6 +1227,7 @@ struct xhci_ctrl {
 	struct xhci_scratchpad *scratchpad;
 	struct xhci_virt_device *devs[MAX_HC_SLOTS];
 	int rootdev;
+	u16 hci_version;
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder.
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-08  5:41   ` Bin Meng
  2020-09-07  7:12 ` [PATCH v3 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96 Chunfeng Yun
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

xhci versions 1.0 and later report the untransferred data remaining in a
TD a bit differently than older hosts.

We used to have separate functions for these, and needed to check host
version before calling the right function.

Now Mediatek host has an additional quirk on how it uses the TD Size
field for remaining data. To prevent yet another function for calculating
remainder we instead want to make one quirk friendly unified function.

Porting from the Linux:
c840d6ce772d("xhci: create one unified function to calculate TRB TD remainder.")
124c39371114("xhci: use boolean to indicate last trb in td remainder calculation")

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v3: no changes
---
 drivers/usb/host/xhci-ring.c | 105 +++++++++++++++++++++----------------------
 include/usb/xhci.h           |   2 +
 2 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 79bfc34..0f86b01 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
 	xhci_writel(&ctrl->dba->doorbell[0], DB_VALUE_HOST);
 }
 
-/**
- * The TD size is the number of bytes remaining in the TD (including this TRB),
- * right shifted by 10.
- * It must fit in bits 21:17, so it can't be bigger than 31.
+/*
+ * For xHCI 1.0 host controllers, TD size is the number of max packet sized
+ * packets remaining in the TD (*not* including this TRB).
  *
- * @param remainder	remaining packets to be sent
- * @return remainder if remainder is less than max else max
- */
-static u32 xhci_td_remainder(unsigned int remainder)
-{
-	u32 max = (1 << (21 - 17 + 1)) - 1;
-
-	if ((remainder >> 10) >= max)
-		return max << 17;
-	else
-		return (remainder >> 10) << 17;
-}
-
-/**
- * Finds out the remanining packets to be sent
+ * Total TD packet count = total_packet_count =
+ *     DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
+ *
+ * Packets transferred up to and including this TRB = packets_transferred =
+ *     rounddown(total bytes transferred including this TRB / wMaxPacketSize)
+ *
+ * TD size = total_packet_count - packets_transferred
  *
- * @param running_total	total size sent so far
+ * For xHCI 0.96 and older, TD size field should be the remaining bytes
+ * including this TRB, right shifted by 10
+ *
+ * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
+ * This is taken care of in the TRB_TD_SIZE() macro
+ *
+ * The last TRB in a TD must have the TD size set to zero.
+ *
+ * @param ctrl	host controller data structure
+ * @param transferred	total size sent so far
  * @param trb_buff_len	length of the TRB Buffer
- * @param total_packet_count	total packet count
- * @param maxpacketsize		max packet size of current pipe
- * @param num_trbs_left		number of TRBs left to be processed
- * @return 0 if running_total or trb_buff_len is 0, else remainder
+ * @param td_total_len	total packet count
+ * @param maxp	max packet size of current pipe
+ * @param more_trbs_coming	indicate last trb in TD
+ * @return remainder
  */
-static u32 xhci_v1_0_td_remainder(int running_total,
-				int trb_buff_len,
-				unsigned int total_packet_count,
-				int maxpacketsize,
-				unsigned int num_trbs_left)
+static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
+			     int trb_buff_len, unsigned int td_total_len,
+			     int maxp, bool more_trbs_coming)
 {
-	int packets_transferred;
+	u32 total_packet_count;
+
+	if (ctrl->hci_version < 0x100)
+		return ((td_total_len - transferred) >> 10);
 
 	/* One TRB with a zero-length data packet. */
-	if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
+	if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
+	    trb_buff_len == td_total_len)
 		return 0;
 
-	/*
-	 * All the TRB queueing functions don't count the current TRB in
-	 * running_total.
-	 */
-	packets_transferred = (running_total + trb_buff_len) / maxpacketsize;
+	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
-	if ((total_packet_count - packets_transferred) > 31)
-		return 31 << 17;
-	return (total_packet_count - packets_transferred) << 17;
+	/* Queueing functions don't count the current TRB into transferred */
+	return (total_packet_count - ((transferred + trb_buff_len) / maxp));
 }
 
 /**
@@ -572,7 +569,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	union xhci_trb *event;
 
 	int running_total, trb_buff_len;
-	unsigned int total_packet_count;
+	bool more_trbs_coming = true;
 	int maxpacketsize;
 	u64 addr;
 	int ret;
@@ -636,8 +633,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 	running_total = 0;
 	maxpacketsize = usb_maxpacket(udev, pipe);
 
-	total_packet_count = DIV_ROUND_UP(length, maxpacketsize);
-
 	/* How much data is in the first TRB? */
 	/*
 	 * How much data is (potentially) left before the 64KB boundary?
@@ -672,27 +667,24 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 		 * Chain all the TRBs together; clear the chain bit in the last
 		 * TRB to indicate it's the last TRB in the chain.
 		 */
-		if (num_trbs > 1)
+		if (num_trbs > 1) {
 			field |= TRB_CHAIN;
-		else
+		} else {
 			field |= TRB_IOC;
+			more_trbs_coming = false;
+		}
 
 		/* Only set interrupt on short packet for IN endpoints */
 		if (usb_pipein(pipe))
 			field |= TRB_ISP;
 
 		/* Set the TRB length, TD size, and interrupter fields. */
-		if (ctrl->hci_version < 0x100)
-			remainder = xhci_td_remainder(length - running_total);
-		else
-			remainder = xhci_v1_0_td_remainder(running_total,
-							   trb_buff_len,
-							   total_packet_count,
-							   maxpacketsize,
-							   num_trbs - 1);
+		remainder = xhci_td_remainder(ctrl, running_total, trb_buff_len,
+					      length, maxpacketsize,
+					      more_trbs_coming);
 
 		length_field = ((trb_buff_len & TRB_LEN_MASK) |
-				remainder |
+				TRB_TD_SIZE(remainder) |
 				((0 & TRB_INTR_TARGET_MASK) <<
 				TRB_INTR_TARGET_SHIFT));
 
@@ -764,6 +756,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
 	struct xhci_ring *ep_ring;
 	union xhci_trb *event;
+	u32 remainder;
 
 	debug("req=%u (%#x), type=%u (%#x), value=%u (%#x), index=%u\n",
 		req->request, req->request,
@@ -866,12 +859,14 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	else
 		field = (TRB_DATA << TRB_TYPE_SHIFT);
 
-	length_field = (length & TRB_LEN_MASK) | xhci_td_remainder(length) |
+	remainder = xhci_td_remainder(ctrl, 0, length, length,
+				      usb_maxpacket(udev, pipe), 1);
+	length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
 			((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
 	debug("length_field = %d, length = %d,"
 		"xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
 		length_field, (length & TRB_LEN_MASK),
-		xhci_td_remainder(length), 0);
+		TRB_TD_SIZE(remainder), 0);
 
 	if (length > 0) {
 		if (req->requesttype & USB_DIR_IN)
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index a3e5914..15926eb 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -850,6 +850,8 @@ struct xhci_event_cmd {
 /* transfer_len bitmasks - bits 0:16 */
 #define	TRB_LEN(p)			((p) & 0x1ffff)
 #define	TRB_LEN_MASK			(0x1ffff)
+/* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
+#define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
 /* Interrupter Target - which MSI-X vector to target the completion event@*/
 #define	TRB_INTR_TARGET_SHIFT		(22)
 #define	TRB_INTR_TARGET_MASK		(0x3ff)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 4/9] usb: xhci: convert to HCS_MAX_PORTS() Chunfeng Yun
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

There some vendor quirks for MTK xHCI 0.96 host controller:
1. It defines some extra SW scheduling parameters for HW
   to minimize the scheduling effort for synchronous and
   interrupt endpoints. The parameters are put into reserved
   DWs of slot context and endpoint context.
2. Its TDS in  Normal TRB defines a number of packets that
   remains to be transferred for a TD after processing all
   Max packets in all previous TRBs.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
v3: fix typo, and add reviewed-by Bin

v2: add Tested-by Frank
---
 drivers/usb/host/xhci-mtk.c  | 1 +
 drivers/usb/host/xhci-ring.c | 9 +++++++--
 drivers/usb/host/xhci.c      | 2 +-
 include/usb/xhci.h           | 2 ++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8ff7185..f3f181d 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -258,6 +258,7 @@ static int xhci_mtk_probe(struct udevice *dev)
 	if (ret)
 		goto ssusb_init_err;
 
+	mtk->ctrl.quirks = XHCI_MTK_HOST;
 	hcor = (struct xhci_hcor *)((uintptr_t)mtk->hcd +
 			HC_LENGTH(xhci_readl(&mtk->hcd->cr_capbase)));
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 0f86b01..cf8b9d2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,8 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
 {
 	u32 total_packet_count;
 
-	if (ctrl->hci_version < 0x100)
+	/* MTK xHCI 0.96 contains some features from 1.0 */
+	if (ctrl->hci_version < 0x100 && !(ctrl->quirks & XHCI_MTK_HOST))
 		return ((td_total_len - transferred) >> 10);
 
 	/* One TRB with a zero-length data packet. */
@@ -340,6 +341,10 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
 	    trb_buff_len == td_total_len)
 		return 0;
 
+	/* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+	if ((ctrl->quirks & XHCI_MTK_HOST) && (ctrl->hci_version < 0x100))
+		trb_buff_len = 0;
+
 	total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
 	/* Queueing functions don't count the current TRB into transferred */
@@ -823,7 +828,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 		field |= 0x1;
 
 	/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-	if (ctrl->hci_version >= 0x100) {
+	if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
 		if (length > 0) {
 			if (req->requesttype & USB_DIR_IN)
 				field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4be1411..51edeb2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -650,7 +650,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 		 * are put into reserved DWs in Slot and Endpoint Contexts
 		 * for synchronous endpoints.
 		 */
-		if (IS_ENABLED(CONFIG_USB_XHCI_MTK)) {
+		if (ctrl->quirks & XHCI_MTK_HOST) {
 			ep_ctx[ep_index]->reserved[0] =
 				cpu_to_le32(EP_BPKTS(1) | EP_BBM(1));
 		}
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 15926eb..3de46cd 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1230,6 +1230,8 @@ struct xhci_ctrl {
 	struct xhci_virt_device *devs[MAX_HC_SLOTS];
 	int rootdev;
 	u16 hci_version;
+	u32 quirks;
+#define XHCI_MTK_HOST		BIT(0)
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 4/9] usb: xhci: convert to HCS_MAX_PORTS()
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96 Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 5/9] usb: xhci: convert to TRB_TYPE() Chunfeng Yun
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

Use HCS_MAX_PORTS(p) instead of
((p & HCS_MAX_PORTS_MASK) >> HCS_MAX_PORTS_SHIFT)

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci.c | 3 +--
 include/usb/xhci.h      | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 51edeb2..5f3a0fb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1257,8 +1257,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 		return -ENOMEM;
 
 	reg = xhci_readl(&hccr->cr_hcsparams1);
-	descriptor.hub.bNbrPorts = ((reg & HCS_MAX_PORTS_MASK) >>
-						HCS_MAX_PORTS_SHIFT);
+	descriptor.hub.bNbrPorts = HCS_MAX_PORTS(reg);
 	printf("Register %x NbrPorts %d\n", reg, descriptor.hub.bNbrPorts);
 
 	/* Port Indicators */
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 3de46cd..cf4c020 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -101,8 +101,6 @@ struct xhci_hccr {
 /* bits 8:18, Max Interrupters */
 #define HCS_MAX_INTRS(p)	(((p) >> 8) & 0x7ff)
 /* bits 24:31, Max Ports - max value is 0x7F = 127 ports */
-#define HCS_MAX_PORTS_SHIFT	24
-#define HCS_MAX_PORTS_MASK	(0xff << HCS_MAX_PORTS_SHIFT)
 #define HCS_MAX_PORTS(p)	(((p) >> 24) & 0xff)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 5/9] usb: xhci: convert to TRB_TYPE()
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
                   ` (2 preceding siblings ...)
  2020-09-07  7:12 ` [PATCH v3 4/9] usb: xhci: convert to HCS_MAX_PORTS() Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET() Chunfeng Yun
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

Use TRB_TYPE(p) instead of ((p) << TRB_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci-mem.c  |  3 +--
 drivers/usb/host/xhci-ring.c | 11 +++++------
 include/usb/xhci.h           |  1 -
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1da0524..d627aa5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -236,8 +236,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 		 */
 		val = le32_to_cpu(prev->trbs[TRBS_PER_SEGMENT-1].link.control);
 		val &= ~TRB_TYPE_BITMASK;
-		val |= (TRB_LINK << TRB_TYPE_SHIFT);
-
+		val |= TRB_TYPE(TRB_LINK);
 		prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
 	}
 }
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index cf8b9d2..87891fd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -696,7 +696,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 		trb_fields[0] = lower_32_bits(addr);
 		trb_fields[1] = upper_32_bits(addr);
 		trb_fields[2] = length_field;
-		trb_fields[3] = field | (TRB_NORMAL << TRB_TYPE_SHIFT);
+		trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
 
 		queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
 
@@ -823,7 +823,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	/* Queue setup TRB - see section 6.4.1.2.1 */
 	/* FIXME better way to translate setup_packet into two u32 fields? */
 	field = 0;
-	field |= TRB_IDT | (TRB_SETUP << TRB_TYPE_SHIFT);
+	field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
 	if (start_cycle == 0)
 		field |= 0x1;
 
@@ -860,9 +860,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	/* If there's data, queue data TRBs */
 	/* Only set interrupt on short packet for IN endpoints */
 	if (usb_pipein(pipe))
-		field = TRB_ISP | (TRB_DATA << TRB_TYPE_SHIFT);
+		field = TRB_ISP | TRB_TYPE(TRB_DATA);
 	else
-		field = (TRB_DATA << TRB_TYPE_SHIFT);
+		field = TRB_TYPE(TRB_DATA);
 
 	remainder = xhci_td_remainder(ctrl, 0, length, length,
 				      usb_maxpacket(udev, pipe), 1);
@@ -904,8 +904,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
 		/* Event on completion */
 	trb_fields[3] = field | TRB_IOC |
-			(TRB_STATUS << TRB_TYPE_SHIFT) |
-			ep_ring->cycle_state;
+			TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
 
 	queue_trb(ctrl, ep_ring, false, trb_fields);
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index cf4c020..bdba51d 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -903,7 +903,6 @@ union xhci_trb {
 /* TRB bit mask */
 #define	TRB_TYPE_BITMASK	(0xfc00)
 #define TRB_TYPE(p)		((p) << 10)
-#define TRB_TYPE_SHIFT		(10)
 #define TRB_FIELD_TO_TYPE(p)	(((p) & TRB_TYPE_BITMASK) >> 10)
 
 /* TRB type IDs */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
                   ` (3 preceding siblings ...)
  2020-09-07  7:12 ` [PATCH v3 5/9] usb: xhci: convert to TRB_TYPE() Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-08  1:30   ` Bin Meng
  2020-09-07  7:12 ` [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE() Chunfeng Yun
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

For normal TRB fields:
use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
and use TRB_INTR_TARGET(x) instead of
(((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: merge patch [v2 6/11] and [v2 7/11] into one, both for normal TRB fileds

v2: no changes
---
 drivers/usb/host/xhci-ring.c | 16 +++++++---------
 include/usb/xhci.h           |  3 ---
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 87891fd..99c84f9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
 					      length, maxpacketsize,
 					      more_trbs_coming);
 
-		length_field = ((trb_buff_len & TRB_LEN_MASK) |
+		length_field = (TRB_LEN(trb_buff_len) |
 				TRB_TD_SIZE(remainder) |
-				((0 & TRB_INTR_TARGET_MASK) <<
-				TRB_INTR_TARGET_SHIFT));
+				TRB_INTR_TARGET(0));
 
 		trb_fields[0] = lower_32_bits(addr);
 		trb_fields[1] = upper_32_bits(addr);
@@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	trb_fields[1] = le16_to_cpu(req->index) |
 			le16_to_cpu(req->length) << 16;
 	/* TRB_LEN | (TRB_INTR_TARGET) */
-	trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
-			TRB_INTR_TARGET_SHIFT));
+	trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
 	/* Immediate data in pointer */
 	trb_fields[3] = field;
 	queue_trb(ctrl, ep_ring, true, trb_fields);
@@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 
 	remainder = xhci_td_remainder(ctrl, 0, length, length,
 				      usb_maxpacket(udev, pipe), 1);
-	length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
-			((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+	length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
+			TRB_INTR_TARGET(0);
 	debug("length_field = %d, length = %d,"
 		"xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
-		length_field, (length & TRB_LEN_MASK),
+		length_field, TRB_LEN(length),
 		TRB_TD_SIZE(remainder), 0);
 
 	if (length > 0) {
@@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 
 	trb_fields[0] = 0;
 	trb_fields[1] = 0;
-	trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+	trb_fields[2] = TRB_INTR_TARGET(0);
 		/* Event on completion */
 	trb_fields[3] = field | TRB_IOC |
 			TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index bdba51d..35c6604 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -847,12 +847,9 @@ struct xhci_event_cmd {
 /* Normal TRB fields */
 /* transfer_len bitmasks - bits 0:16 */
 #define	TRB_LEN(p)			((p) & 0x1ffff)
-#define	TRB_LEN_MASK			(0x1ffff)
 /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
 #define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
 /* Interrupter Target - which MSI-X vector to target the completion event@*/
-#define	TRB_INTR_TARGET_SHIFT		(22)
-#define	TRB_INTR_TARGET_MASK		(0x3ff)
 #define TRB_INTR_TARGET(p)		(((p) & 0x3ff) << 22)
 #define GET_INTR_TARGET(p)		(((p) >> 22) & 0x3ff)
 #define TRB_TBC(p)			(((p) & 0x3) << 7)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE()
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
                   ` (4 preceding siblings ...)
  2020-09-07  7:12 ` [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET() Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-08  1:31   ` Bin Meng
  2020-09-07  7:12 ` [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2 Chunfeng Yun
  2020-09-07  7:12 ` [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout() Chunfeng Yun
  7 siblings, 1 reply; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

Use TRB_TX_TYPE() instead of (TRB_DATA_OUT/IN << TRB_TX_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2: no changes
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 include/usb/xhci.h           | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 99c84f9..ccf2a35 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -830,9 +830,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
 	if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
 		if (length > 0) {
 			if (req->requesttype & USB_DIR_IN)
-				field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
+				field |= TRB_TX_TYPE(TRB_DATA_IN);
 			else
-				field |= (TRB_DATA_OUT << TRB_TX_TYPE_SHIFT);
+				field |= TRB_TX_TYPE(TRB_DATA_OUT);
 		}
 	}
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 35c6604..07b1aeb 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -879,7 +879,6 @@ struct xhci_event_cmd {
 /* Control transfer TRB specific fields */
 #define TRB_DIR_IN		(1<<16)
 #define	TRB_TX_TYPE(p)		((p) << 16)
-#define	TRB_TX_TYPE_SHIFT	(16)
 #define	TRB_DATA_OUT		2
 #define	TRB_DATA_IN		3
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
                   ` (5 preceding siblings ...)
  2020-09-07  7:12 ` [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE() Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-08  1:35   ` Bin Meng
  2020-09-07  7:12 ` [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout() Chunfeng Yun
  7 siblings, 1 reply; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

Use macros with parameter to fill ep_info2, then some macros
for MASK and SHIFT can be removed

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: merge patch [v2 9/11] and [v2 10/11] into one, both for ep_info2

v2: no changes
---
 drivers/usb/host/xhci-mem.c | 15 +++++----------
 drivers/usb/host/xhci.c     |  6 ++----
 include/usb/xhci.h          |  6 ------
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d627aa5..0b49614 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -825,25 +825,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 
 	/* Step 4 - ring already allocated */
 	/* Step 5 */
-	ep0_ctx->ep_info2 = cpu_to_le32(CTRL_EP << EP_TYPE_SHIFT);
+	ep0_ctx->ep_info2 = cpu_to_le32(EP_TYPE(CTRL_EP));
 	debug("SPEED = %d\n", speed);
 
 	switch (speed) {
 	case USB_SPEED_SUPER:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((512 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(512));
 		debug("Setting Packet size = 512bytes\n");
 		break;
 	case USB_SPEED_HIGH:
 	/* USB core guesses at a 64-byte max packet first for FS devices */
 	case USB_SPEED_FULL:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((64 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(64));
 		debug("Setting Packet size = 64bytes\n");
 		break;
 	case USB_SPEED_LOW:
-		ep0_ctx->ep_info2 |= cpu_to_le32(((8 & MAX_PACKET_MASK) <<
-					MAX_PACKET_SHIFT));
+		ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(8));
 		debug("Setting Packet size = 8bytes\n");
 		break;
 	default:
@@ -852,9 +849,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
 	}
 
 	/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
-	ep0_ctx->ep_info2 |=
-			cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
-			((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
+	ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
 	trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
 	ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5f3a0fb..fe30101 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -618,8 +618,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 			cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
 			EP_INTERVAL(interval) | EP_MULT(mult));
 
-		ep_ctx[ep_index]->ep_info2 =
-			cpu_to_le32(ep_type << EP_TYPE_SHIFT);
+		ep_ctx[ep_index]->ep_info2 = cpu_to_le32(EP_TYPE(ep_type));
 		ep_ctx[ep_index]->ep_info2 |=
 			cpu_to_le32(MAX_PACKET
 			(get_unaligned(&endpt_desc->wMaxPacketSize)));
@@ -832,8 +831,7 @@ int xhci_check_maxpacket(struct usb_device *udev)
 				ctrl->devs[slot_id]->out_ctx, ep_index);
 		in_ctx = ctrl->devs[slot_id]->in_ctx;
 		ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
-		ep_ctx->ep_info2 &= cpu_to_le32(~((0xffff & MAX_PACKET_MASK)
-						<< MAX_PACKET_SHIFT));
+		ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET(MAX_PACKET_MASK));
 		ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
 		/*
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 07b1aeb..e1d3823 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -632,11 +632,8 @@ struct xhci_ep_ctx {
  */
 #define	FORCE_EVENT		(0x1)
 #define ERROR_COUNT(p)		(((p) & 0x3) << 1)
-#define ERROR_COUNT_SHIFT	(1)
-#define ERROR_COUNT_MASK	(0x3)
 #define CTX_TO_EP_TYPE(p)	(((p) >> 3) & 0x7)
 #define EP_TYPE(p)		((p) << 3)
-#define EP_TYPE_SHIFT		(3)
 #define ISOC_OUT_EP		1
 #define BULK_OUT_EP		2
 #define INT_OUT_EP		3
@@ -647,13 +644,10 @@ struct xhci_ep_ctx {
 /* bit 6 reserved */
 /* bit 7 is Host Initiate Disable - for disabling stream selection */
 #define MAX_BURST(p)		(((p)&0xff) << 8)
-#define MAX_BURST_MASK		(0xff)
-#define MAX_BURST_SHIFT		(8)
 #define CTX_TO_MAX_BURST(p)	(((p) >> 8) & 0xff)
 #define MAX_PACKET(p)		(((p)&0xffff) << 16)
 #define MAX_PACKET_MASK		(0xffff)
 #define MAX_PACKET_DECODED(p)	(((p) >> 16) & 0xffff)
-#define MAX_PACKET_SHIFT	(16)
 
 /* Get max packet size from ep desc. Bit 10..0 specify the max packet size.
  * USB2.0 spec 9.6.6.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
                   ` (6 preceding siblings ...)
  2020-09-07  7:12 ` [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2 Chunfeng Yun
@ 2020-09-07  7:12 ` Chunfeng Yun
  2020-09-08  1:44   ` Bin Meng
  7 siblings, 1 reply; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-07  7:12 UTC (permalink / raw)
  To: u-boot

Use readx_poll_sleep_timeout() to poll the register status

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: no changes

v2: fix typo of title suggested by Frank
---
 drivers/usb/host/xhci.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fe30101..3547a9b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -33,6 +33,7 @@
 #include <linux/bug.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/iopoll.h>
 #include <usb/xhci.h>
 
 #ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
@@ -143,23 +144,19 @@ struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev)
  * @param usec	time to wait till
  * @return 0 if handshake is success else < 0 on failure
  */
-static int handshake(uint32_t volatile *ptr, uint32_t mask,
-					uint32_t done, int usec)
+static int
+handshake(uint32_t volatile *ptr, uint32_t mask, uint32_t done, int usec)
 {
 	uint32_t result;
+	int ret;
+
+	ret = readx_poll_sleep_timeout(xhci_readl, ptr, result,
+				 (result & mask) == done || result == U32_MAX,
+				 1, usec);
+	if (result == U32_MAX)		/* card removed */
+		return -ENODEV;
 
-	do {
-		result = xhci_readl(ptr);
-		if (result == ~(uint32_t)0)
-			return -ENODEV;
-		result &= mask;
-		if (result == done)
-			return 0;
-		usec--;
-		udelay(1);
-	} while (usec > 0);
-
-	return -ETIMEDOUT;
+	return ret;
 }
 
 /**
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()
  2020-09-07  7:12 ` [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET() Chunfeng Yun
@ 2020-09-08  1:30   ` Bin Meng
  2020-09-08  7:39     ` Chunfeng Yun
  0 siblings, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-08  1:30 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> For normal TRB fields:
> use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
> and use TRB_INTR_TARGET(x) instead of
> (((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v3: merge patch [v2 6/11] and [v2 7/11] into one, both for normal TRB fileds
>
> v2: no changes
> ---
>  drivers/usb/host/xhci-ring.c | 16 +++++++---------
>  include/usb/xhci.h           |  3 ---
>  2 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 87891fd..99c84f9 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>                                               length, maxpacketsize,
>                                               more_trbs_coming);
>
> -               length_field = ((trb_buff_len & TRB_LEN_MASK) |
> +               length_field = (TRB_LEN(trb_buff_len) |
>                                 TRB_TD_SIZE(remainder) |
> -                               ((0 & TRB_INTR_TARGET_MASK) <<
> -                               TRB_INTR_TARGET_SHIFT));
> +                               TRB_INTR_TARGET(0));

nits: should be aligned to TRB_LEN(length)

>
>                 trb_fields[0] = lower_32_bits(addr);
>                 trb_fields[1] = upper_32_bits(addr);
> @@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>         trb_fields[1] = le16_to_cpu(req->index) |
>                         le16_to_cpu(req->length) << 16;
>         /* TRB_LEN | (TRB_INTR_TARGET) */
> -       trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
> -                       TRB_INTR_TARGET_SHIFT));
> +       trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
>         /* Immediate data in pointer */
>         trb_fields[3] = field;
>         queue_trb(ctrl, ep_ring, true, trb_fields);
> @@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>
>         remainder = xhci_td_remainder(ctrl, 0, length, length,
>                                       usb_maxpacket(udev, pipe), 1);
> -       length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
> -                       ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
> +       length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
> +                       TRB_INTR_TARGET(0);
>         debug("length_field = %d, length = %d,"
>                 "xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
> -               length_field, (length & TRB_LEN_MASK),
> +               length_field, TRB_LEN(length),
>                 TRB_TD_SIZE(remainder), 0);
>
>         if (length > 0) {
> @@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>
>         trb_fields[0] = 0;
>         trb_fields[1] = 0;
> -       trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
> +       trb_fields[2] = TRB_INTR_TARGET(0);
>                 /* Event on completion */
>         trb_fields[3] = field | TRB_IOC |
>                         TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> index bdba51d..35c6604 100644
> --- a/include/usb/xhci.h
> +++ b/include/usb/xhci.h
> @@ -847,12 +847,9 @@ struct xhci_event_cmd {
>  /* Normal TRB fields */
>  /* transfer_len bitmasks - bits 0:16 */
>  #define        TRB_LEN(p)                      ((p) & 0x1ffff)
> -#define        TRB_LEN_MASK                    (0x1ffff)
>  /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
>  #define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
>  /* Interrupter Target - which MSI-X vector to target the completion event at */
> -#define        TRB_INTR_TARGET_SHIFT           (22)
> -#define        TRB_INTR_TARGET_MASK            (0x3ff)
>  #define TRB_INTR_TARGET(p)             (((p) & 0x3ff) << 22)
>  #define GET_INTR_TARGET(p)             (((p) >> 22) & 0x3ff)
>  #define TRB_TBC(p)                     (((p) & 0x3) << 7)
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE()
  2020-09-07  7:12 ` [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE() Chunfeng Yun
@ 2020-09-08  1:31   ` Bin Meng
  0 siblings, 0 replies; 31+ messages in thread
From: Bin Meng @ 2020-09-08  1:31 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Use TRB_TX_TYPE() instead of (TRB_DATA_OUT/IN << TRB_TX_TYPE_SHIFT)
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2: no changes
> ---
>  drivers/usb/host/xhci-ring.c | 4 ++--
>  include/usb/xhci.h           | 1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2
  2020-09-07  7:12 ` [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2 Chunfeng Yun
@ 2020-09-08  1:35   ` Bin Meng
  0 siblings, 0 replies; 31+ messages in thread
From: Bin Meng @ 2020-09-08  1:35 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Use macros with parameter to fill ep_info2, then some macros
> for MASK and SHIFT can be removed
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v3: merge patch [v2 9/11] and [v2 10/11] into one, both for ep_info2
>
> v2: no changes
> ---
>  drivers/usb/host/xhci-mem.c | 15 +++++----------
>  drivers/usb/host/xhci.c     |  6 ++----
>  include/usb/xhci.h          |  6 ------
>  3 files changed, 7 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-07  7:12 ` [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout() Chunfeng Yun
@ 2020-09-08  1:44   ` Bin Meng
  2020-09-08 11:13     ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-08  1:44 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Use readx_poll_sleep_timeout() to poll the register status
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v3: no changes
>
> v2: fix typo of title suggested by Frank
> ---
>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder.
  2020-09-07  7:12 ` [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder Chunfeng Yun
@ 2020-09-08  5:41   ` Bin Meng
  2020-09-08  7:40     ` Chunfeng Yun
  0 siblings, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-08  5:41 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>

nits: please remove the ending period in the commit title

> xhci versions 1.0 and later report the untransferred data remaining in a
> TD a bit differently than older hosts.
>
> We used to have separate functions for these, and needed to check host
> version before calling the right function.
>
> Now Mediatek host has an additional quirk on how it uses the TD Size
> field for remaining data. To prevent yet another function for calculating
> remainder we instead want to make one quirk friendly unified function.
>
> Porting from the Linux:
> c840d6ce772d("xhci: create one unified function to calculate TRB TD remainder.")
> 124c39371114("xhci: use boolean to indicate last trb in td remainder calculation")
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  drivers/usb/host/xhci-ring.c | 105 +++++++++++++++++++++----------------------
>  include/usb/xhci.h           |   2 +
>  2 files changed, 52 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 79bfc34..0f86b01 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
>         xhci_writel(&ctrl->dba->doorbell[0], DB_VALUE_HOST);
>  }
>
> -/**
> - * The TD size is the number of bytes remaining in the TD (including this TRB),
> - * right shifted by 10.
> - * It must fit in bits 21:17, so it can't be bigger than 31.
> +/*
> + * For xHCI 1.0 host controllers, TD size is the number of max packet sized
> + * packets remaining in the TD (*not* including this TRB).
>   *
> - * @param remainder    remaining packets to be sent
> - * @return remainder if remainder is less than max else max
> - */
> -static u32 xhci_td_remainder(unsigned int remainder)
> -{
> -       u32 max = (1 << (21 - 17 + 1)) - 1;
> -
> -       if ((remainder >> 10) >= max)
> -               return max << 17;
> -       else
> -               return (remainder >> 10) << 17;
> -}
> -
> -/**
> - * Finds out the remanining packets to be sent
> + * Total TD packet count = total_packet_count =
> + *     DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
> + *
> + * Packets transferred up to and including this TRB = packets_transferred =
> + *     rounddown(total bytes transferred including this TRB / wMaxPacketSize)
> + *
> + * TD size = total_packet_count - packets_transferred
>   *
> - * @param running_total        total size sent so far
> + * For xHCI 0.96 and older, TD size field should be the remaining bytes
> + * including this TRB, right shifted by 10
> + *
> + * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
> + * This is taken care of in the TRB_TD_SIZE() macro
> + *
> + * The last TRB in a TD must have the TD size set to zero.
> + *
> + * @param ctrl host controller data structure
> + * @param transferred  total size sent so far
>   * @param trb_buff_len length of the TRB Buffer
> - * @param total_packet_count   total packet count
> - * @param maxpacketsize                max packet size of current pipe
> - * @param num_trbs_left                number of TRBs left to be processed
> - * @return 0 if running_total or trb_buff_len is 0, else remainder
> + * @param td_total_len total packet count
> + * @param maxp max packet size of current pipe
> + * @param more_trbs_coming     indicate last trb in TD
> + * @return remainder
>   */
> -static u32 xhci_v1_0_td_remainder(int running_total,
> -                               int trb_buff_len,
> -                               unsigned int total_packet_count,
> -                               int maxpacketsize,
> -                               unsigned int num_trbs_left)
> +static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
> +                            int trb_buff_len, unsigned int td_total_len,
> +                            int maxp, bool more_trbs_coming)
>  {
> -       int packets_transferred;
> +       u32 total_packet_count;
> +
> +       if (ctrl->hci_version < 0x100)
> +               return ((td_total_len - transferred) >> 10);
>
>         /* One TRB with a zero-length data packet. */
> -       if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> +       if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
> +           trb_buff_len == td_total_len)
>                 return 0;
>
> -       /*
> -        * All the TRB queueing functions don't count the current TRB in
> -        * running_total.
> -        */
> -       packets_transferred = (running_total + trb_buff_len) / maxpacketsize;
> +       total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
>
> -       if ((total_packet_count - packets_transferred) > 31)
> -               return 31 << 17;
> -       return (total_packet_count - packets_transferred) << 17;
> +       /* Queueing functions don't count the current TRB into transferred */
> +       return (total_packet_count - ((transferred + trb_buff_len) / maxp));
>  }
>
>  /**
> @@ -572,7 +569,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>         union xhci_trb *event;
>
>         int running_total, trb_buff_len;
> -       unsigned int total_packet_count;
> +       bool more_trbs_coming = true;
>         int maxpacketsize;
>         u64 addr;
>         int ret;
> @@ -636,8 +633,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>         running_total = 0;
>         maxpacketsize = usb_maxpacket(udev, pipe);
>
> -       total_packet_count = DIV_ROUND_UP(length, maxpacketsize);
> -
>         /* How much data is in the first TRB? */
>         /*
>          * How much data is (potentially) left before the 64KB boundary?
> @@ -672,27 +667,24 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
>                  * Chain all the TRBs together; clear the chain bit in the last
>                  * TRB to indicate it's the last TRB in the chain.
>                  */
> -               if (num_trbs > 1)
> +               if (num_trbs > 1) {
>                         field |= TRB_CHAIN;
> -               else
> +               } else {
>                         field |= TRB_IOC;
> +                       more_trbs_coming = false;
> +               }
>
>                 /* Only set interrupt on short packet for IN endpoints */
>                 if (usb_pipein(pipe))
>                         field |= TRB_ISP;
>
>                 /* Set the TRB length, TD size, and interrupter fields. */
> -               if (ctrl->hci_version < 0x100)
> -                       remainder = xhci_td_remainder(length - running_total);
> -               else
> -                       remainder = xhci_v1_0_td_remainder(running_total,
> -                                                          trb_buff_len,
> -                                                          total_packet_count,
> -                                                          maxpacketsize,
> -                                                          num_trbs - 1);
> +               remainder = xhci_td_remainder(ctrl, running_total, trb_buff_len,
> +                                             length, maxpacketsize,
> +                                             more_trbs_coming);
>
>                 length_field = ((trb_buff_len & TRB_LEN_MASK) |
> -                               remainder |
> +                               TRB_TD_SIZE(remainder) |
>                                 ((0 & TRB_INTR_TARGET_MASK) <<
>                                 TRB_INTR_TARGET_SHIFT));
>
> @@ -764,6 +756,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>         struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
>         struct xhci_ring *ep_ring;
>         union xhci_trb *event;
> +       u32 remainder;
>
>         debug("req=%u (%#x), type=%u (%#x), value=%u (%#x), index=%u\n",
>                 req->request, req->request,
> @@ -866,12 +859,14 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
>         else
>                 field = (TRB_DATA << TRB_TYPE_SHIFT);
>
> -       length_field = (length & TRB_LEN_MASK) | xhci_td_remainder(length) |
> +       remainder = xhci_td_remainder(ctrl, 0, length, length,
> +                                     usb_maxpacket(udev, pipe), 1);

1 should be true

> +       length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
>                         ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
>         debug("length_field = %d, length = %d,"
>                 "xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
>                 length_field, (length & TRB_LEN_MASK),
> -               xhci_td_remainder(length), 0);
> +               TRB_TD_SIZE(remainder), 0);
>
>         if (length > 0) {
>                 if (req->requesttype & USB_DIR_IN)
> diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> index a3e5914..15926eb 100644
> --- a/include/usb/xhci.h
> +++ b/include/usb/xhci.h
> @@ -850,6 +850,8 @@ struct xhci_event_cmd {
>  /* transfer_len bitmasks - bits 0:16 */
>  #define        TRB_LEN(p)                      ((p) & 0x1ffff)
>  #define        TRB_LEN_MASK                    (0x1ffff)
> +/* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
> +#define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
>  /* Interrupter Target - which MSI-X vector to target the completion event at */
>  #define        TRB_INTR_TARGET_SHIFT           (22)
>  #define        TRB_INTR_TARGET_MASK            (0x3ff)

Otherwise,
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()
  2020-09-08  1:30   ` Bin Meng
@ 2020-09-08  7:39     ` Chunfeng Yun
  0 siblings, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-08  7:39 UTC (permalink / raw)
  To: u-boot

On Tue, 2020-09-08 at 09:30 +0800, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > For normal TRB fields:
> > use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
> > and use TRB_INTR_TARGET(x) instead of
> > (((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v3: merge patch [v2 6/11] and [v2 7/11] into one, both for normal TRB fileds
> >
> > v2: no changes
> > ---
> >  drivers/usb/host/xhci-ring.c | 16 +++++++---------
> >  include/usb/xhci.h           |  3 ---
> >  2 files changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 87891fd..99c84f9 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
> >                                               length, maxpacketsize,
> >                                               more_trbs_coming);
> >
> > -               length_field = ((trb_buff_len & TRB_LEN_MASK) |
> > +               length_field = (TRB_LEN(trb_buff_len) |
> >                                 TRB_TD_SIZE(remainder) |
> > -                               ((0 & TRB_INTR_TARGET_MASK) <<
> > -                               TRB_INTR_TARGET_SHIFT));
> > +                               TRB_INTR_TARGET(0));
> 
> nits: should be aligned to TRB_LEN(length)
Ok, will check it again

> 
> >
> >                 trb_fields[0] = lower_32_bits(addr);
> >                 trb_fields[1] = upper_32_bits(addr);
> > @@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
> >         trb_fields[1] = le16_to_cpu(req->index) |
> >                         le16_to_cpu(req->length) << 16;
> >         /* TRB_LEN | (TRB_INTR_TARGET) */
> > -       trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
> > -                       TRB_INTR_TARGET_SHIFT));
> > +       trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
> >         /* Immediate data in pointer */
> >         trb_fields[3] = field;
> >         queue_trb(ctrl, ep_ring, true, trb_fields);
> > @@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
> >
> >         remainder = xhci_td_remainder(ctrl, 0, length, length,
> >                                       usb_maxpacket(udev, pipe), 1);
> > -       length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
> > -                       ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
> > +       length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
> > +                       TRB_INTR_TARGET(0);
> >         debug("length_field = %d, length = %d,"
> >                 "xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
> > -               length_field, (length & TRB_LEN_MASK),
> > +               length_field, TRB_LEN(length),
> >                 TRB_TD_SIZE(remainder), 0);
> >
> >         if (length > 0) {
> > @@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
> >
> >         trb_fields[0] = 0;
> >         trb_fields[1] = 0;
> > -       trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
> > +       trb_fields[2] = TRB_INTR_TARGET(0);
> >                 /* Event on completion */
> >         trb_fields[3] = field | TRB_IOC |
> >                         TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index bdba51d..35c6604 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -847,12 +847,9 @@ struct xhci_event_cmd {
> >  /* Normal TRB fields */
> >  /* transfer_len bitmasks - bits 0:16 */
> >  #define        TRB_LEN(p)                      ((p) & 0x1ffff)
> > -#define        TRB_LEN_MASK                    (0x1ffff)
> >  /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
> >  #define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
> >  /* Interrupter Target - which MSI-X vector to target the completion event at */
> > -#define        TRB_INTR_TARGET_SHIFT           (22)
> > -#define        TRB_INTR_TARGET_MASK            (0x3ff)
> >  #define TRB_INTR_TARGET(p)             (((p) & 0x3ff) << 22)
> >  #define GET_INTR_TARGET(p)             (((p) >> 22) & 0x3ff)
> >  #define TRB_TBC(p)                     (((p) & 0x3) << 7)
> > --
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Thanks

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder.
  2020-09-08  5:41   ` Bin Meng
@ 2020-09-08  7:40     ` Chunfeng Yun
  0 siblings, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-08  7:40 UTC (permalink / raw)
  To: u-boot

On Tue, 2020-09-08 at 13:41 +0800, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> 
> nits: please remove the ending period in the commit title
Ok, will fix it

> 
> > xhci versions 1.0 and later report the untransferred data remaining in a
> > TD a bit differently than older hosts.
> >
> > We used to have separate functions for these, and needed to check host
> > version before calling the right function.
> >
> > Now Mediatek host has an additional quirk on how it uses the TD Size
> > field for remaining data. To prevent yet another function for calculating
> > remainder we instead want to make one quirk friendly unified function.
> >
> > Porting from the Linux:
> > c840d6ce772d("xhci: create one unified function to calculate TRB TD remainder.")
> > 124c39371114("xhci: use boolean to indicate last trb in td remainder calculation")
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v2~v3: no changes
> > ---
> >  drivers/usb/host/xhci-ring.c | 105 +++++++++++++++++++++----------------------
> >  include/usb/xhci.h           |   2 +
> >  2 files changed, 52 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 79bfc34..0f86b01 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, u32 slot_id,
> >         xhci_writel(&ctrl->dba->doorbell[0], DB_VALUE_HOST);
> >  }
> >
> > -/**
> > - * The TD size is the number of bytes remaining in the TD (including this TRB),
> > - * right shifted by 10.
> > - * It must fit in bits 21:17, so it can't be bigger than 31.
> > +/*
> > + * For xHCI 1.0 host controllers, TD size is the number of max packet sized
> > + * packets remaining in the TD (*not* including this TRB).
> >   *
> > - * @param remainder    remaining packets to be sent
> > - * @return remainder if remainder is less than max else max
> > - */
> > -static u32 xhci_td_remainder(unsigned int remainder)
> > -{
> > -       u32 max = (1 << (21 - 17 + 1)) - 1;
> > -
> > -       if ((remainder >> 10) >= max)
> > -               return max << 17;
> > -       else
> > -               return (remainder >> 10) << 17;
> > -}
> > -
> > -/**
> > - * Finds out the remanining packets to be sent
> > + * Total TD packet count = total_packet_count =
> > + *     DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
> > + *
> > + * Packets transferred up to and including this TRB = packets_transferred =
> > + *     rounddown(total bytes transferred including this TRB / wMaxPacketSize)
> > + *
> > + * TD size = total_packet_count - packets_transferred
> >   *
> > - * @param running_total        total size sent so far
> > + * For xHCI 0.96 and older, TD size field should be the remaining bytes
> > + * including this TRB, right shifted by 10
> > + *
> > + * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
> > + * This is taken care of in the TRB_TD_SIZE() macro
> > + *
> > + * The last TRB in a TD must have the TD size set to zero.
> > + *
> > + * @param ctrl host controller data structure
> > + * @param transferred  total size sent so far
> >   * @param trb_buff_len length of the TRB Buffer
> > - * @param total_packet_count   total packet count
> > - * @param maxpacketsize                max packet size of current pipe
> > - * @param num_trbs_left                number of TRBs left to be processed
> > - * @return 0 if running_total or trb_buff_len is 0, else remainder
> > + * @param td_total_len total packet count
> > + * @param maxp max packet size of current pipe
> > + * @param more_trbs_coming     indicate last trb in TD
> > + * @return remainder
> >   */
> > -static u32 xhci_v1_0_td_remainder(int running_total,
> > -                               int trb_buff_len,
> > -                               unsigned int total_packet_count,
> > -                               int maxpacketsize,
> > -                               unsigned int num_trbs_left)
> > +static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
> > +                            int trb_buff_len, unsigned int td_total_len,
> > +                            int maxp, bool more_trbs_coming)
> >  {
> > -       int packets_transferred;
> > +       u32 total_packet_count;
> > +
> > +       if (ctrl->hci_version < 0x100)
> > +               return ((td_total_len - transferred) >> 10);
> >
> >         /* One TRB with a zero-length data packet. */
> > -       if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> > +       if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
> > +           trb_buff_len == td_total_len)
> >                 return 0;
> >
> > -       /*
> > -        * All the TRB queueing functions don't count the current TRB in
> > -        * running_total.
> > -        */
> > -       packets_transferred = (running_total + trb_buff_len) / maxpacketsize;
> > +       total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
> >
> > -       if ((total_packet_count - packets_transferred) > 31)
> > -               return 31 << 17;
> > -       return (total_packet_count - packets_transferred) << 17;
> > +       /* Queueing functions don't count the current TRB into transferred */
> > +       return (total_packet_count - ((transferred + trb_buff_len) / maxp));
> >  }
> >
> >  /**
> > @@ -572,7 +569,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
> >         union xhci_trb *event;
> >
> >         int running_total, trb_buff_len;
> > -       unsigned int total_packet_count;
> > +       bool more_trbs_coming = true;
> >         int maxpacketsize;
> >         u64 addr;
> >         int ret;
> > @@ -636,8 +633,6 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
> >         running_total = 0;
> >         maxpacketsize = usb_maxpacket(udev, pipe);
> >
> > -       total_packet_count = DIV_ROUND_UP(length, maxpacketsize);
> > -
> >         /* How much data is in the first TRB? */
> >         /*
> >          * How much data is (potentially) left before the 64KB boundary?
> > @@ -672,27 +667,24 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long pipe,
> >                  * Chain all the TRBs together; clear the chain bit in the last
> >                  * TRB to indicate it's the last TRB in the chain.
> >                  */
> > -               if (num_trbs > 1)
> > +               if (num_trbs > 1) {
> >                         field |= TRB_CHAIN;
> > -               else
> > +               } else {
> >                         field |= TRB_IOC;
> > +                       more_trbs_coming = false;
> > +               }
> >
> >                 /* Only set interrupt on short packet for IN endpoints */
> >                 if (usb_pipein(pipe))
> >                         field |= TRB_ISP;
> >
> >                 /* Set the TRB length, TD size, and interrupter fields. */
> > -               if (ctrl->hci_version < 0x100)
> > -                       remainder = xhci_td_remainder(length - running_total);
> > -               else
> > -                       remainder = xhci_v1_0_td_remainder(running_total,
> > -                                                          trb_buff_len,
> > -                                                          total_packet_count,
> > -                                                          maxpacketsize,
> > -                                                          num_trbs - 1);
> > +               remainder = xhci_td_remainder(ctrl, running_total, trb_buff_len,
> > +                                             length, maxpacketsize,
> > +                                             more_trbs_coming);
> >
> >                 length_field = ((trb_buff_len & TRB_LEN_MASK) |
> > -                               remainder |
> > +                               TRB_TD_SIZE(remainder) |
> >                                 ((0 & TRB_INTR_TARGET_MASK) <<
> >                                 TRB_INTR_TARGET_SHIFT));
> >
> > @@ -764,6 +756,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
> >         struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
> >         struct xhci_ring *ep_ring;
> >         union xhci_trb *event;
> > +       u32 remainder;
> >
> >         debug("req=%u (%#x), type=%u (%#x), value=%u (%#x), index=%u\n",
> >                 req->request, req->request,
> > @@ -866,12 +859,14 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long pipe,
> >         else
> >                 field = (TRB_DATA << TRB_TYPE_SHIFT);
> >
> > -       length_field = (length & TRB_LEN_MASK) | xhci_td_remainder(length) |
> > +       remainder = xhci_td_remainder(ctrl, 0, length, length,
> > +                                     usb_maxpacket(udev, pipe), 1);
> 
> 1 should be true
Ok, it's better, will modify it

> 
> > +       length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
> >                         ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
> >         debug("length_field = %d, length = %d,"
> >                 "xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
> >                 length_field, (length & TRB_LEN_MASK),
> > -               xhci_td_remainder(length), 0);
> > +               TRB_TD_SIZE(remainder), 0);
> >
> >         if (length > 0) {
> >                 if (req->requesttype & USB_DIR_IN)
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index a3e5914..15926eb 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -850,6 +850,8 @@ struct xhci_event_cmd {
> >  /* transfer_len bitmasks - bits 0:16 */
> >  #define        TRB_LEN(p)                      ((p) & 0x1ffff)
> >  #define        TRB_LEN_MASK                    (0x1ffff)
> > +/* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
> > +#define TRB_TD_SIZE(p)          (min((p), (u32)31) << 17)
> >  /* Interrupter Target - which MSI-X vector to target the completion event at */
> >  #define        TRB_INTR_TARGET_SHIFT           (22)
> >  #define        TRB_INTR_TARGET_MASK            (0x3ff)
> 
> Otherwise,
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Thanks a lot

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08  1:44   ` Bin Meng
@ 2020-09-08 11:13     ` Marek Vasut
  2020-09-08 11:24       ` Chunfeng Yun
  2020-09-08 15:45       ` Bin Meng
  0 siblings, 2 replies; 31+ messages in thread
From: Marek Vasut @ 2020-09-08 11:13 UTC (permalink / raw)
  To: u-boot

On 9/8/20 3:44 AM, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>>
>> Use readx_poll_sleep_timeout() to poll the register status
>>
>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> ---
>> v3: no changes
>>
>> v2: fix typo of title suggested by Frank
>> ---
>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>
> 
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Thanks. So now I have half of a RESEND patchset with RB tags, and
another half of just RB tags without patchset. Can someone of you please
collect the tags and resend the patchset once more with the collected
tags, so it's all in one place ?

Thank you

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 11:13     ` Marek Vasut
@ 2020-09-08 11:24       ` Chunfeng Yun
  2020-09-08 15:45       ` Bin Meng
  1 sibling, 0 replies; 31+ messages in thread
From: Chunfeng Yun @ 2020-09-08 11:24 UTC (permalink / raw)
  To: u-boot

Hi Marek,

 I've sent out v4;

Hi Frank,

  Please forward this email to Marek, thanks a lot


On Tue, 2020-09-08 at 13:13 +0200, Marek Vasut wrote:
> On 9/8/20 3:44 AM, Bin Meng wrote:
> > On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>
> >> Use readx_poll_sleep_timeout() to poll the register status
> >>
> >> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >> ---
> >> v3: no changes
> >>
> >> v2: fix typo of title suggested by Frank
> >> ---
> >>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> >>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>
> > 
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> 
> Thanks. So now I have half of a RESEND patchset with RB tags, and
> another half of just RB tags without patchset. Can someone of you please
> collect the tags and resend the patchset once more with the collected
> tags, so it's all in one place ?
> 
> Thank you

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 11:13     ` Marek Vasut
  2020-09-08 11:24       ` Chunfeng Yun
@ 2020-09-08 15:45       ` Bin Meng
  2020-09-08 16:15         ` Marek Vasut
  1 sibling, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-08 15:45 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
>
> On 9/8/20 3:44 AM, Bin Meng wrote:
> > On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>
> >> Use readx_poll_sleep_timeout() to poll the register status
> >>
> >> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >> ---
> >> v3: no changes
> >>
> >> v2: fix typo of title suggested by Frank
> >> ---
> >>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> >>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>
> >
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>
> Thanks. So now I have half of a RESEND patchset with RB tags, and
> another half of just RB tags without patchset. Can someone of you please
> collect the tags and resend the patchset once more with the collected
> tags, so it's all in one place ?
>

I believe the latest version has all the RB tags.

Regards,
Bin

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 15:45       ` Bin Meng
@ 2020-09-08 16:15         ` Marek Vasut
  2020-09-08 16:34           ` Frank Wunderlich
  2020-09-09  1:55           ` Bin Meng
  0 siblings, 2 replies; 31+ messages in thread
From: Marek Vasut @ 2020-09-08 16:15 UTC (permalink / raw)
  To: u-boot

On 9/8/20 5:45 PM, Bin Meng wrote:
> Hi Marek,

Hi,

> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 9/8/20 3:44 AM, Bin Meng wrote:
>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>>>>
>>>> Use readx_poll_sleep_timeout() to poll the register status
>>>>
>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>>> ---
>>>> v3: no changes
>>>>
>>>> v2: fix typo of title suggested by Frank
>>>> ---
>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>>>
>>>
>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> Thanks. So now I have half of a RESEND patchset with RB tags, and
>> another half of just RB tags without patchset. Can someone of you please
>> collect the tags and resend the patchset once more with the collected
>> tags, so it's all in one place ?
>>
> 
> I believe the latest version has all the RB tags.

Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
because the original patches are just not there.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 16:15         ` Marek Vasut
@ 2020-09-08 16:34           ` Frank Wunderlich
  2020-09-08 16:47             ` Marek Vasut
  2020-09-09  1:55           ` Bin Meng
  1 sibling, 1 reply; 31+ messages in thread
From: Frank Wunderlich @ 2020-09-08 16:34 UTC (permalink / raw)
  To: u-boot



Am 8. September 2020 18:15:05 MESZ schrieb Marek Vasut <marex@denx.de>:
>Thank you, I cannot apply a patchset which only has "Re:" in my
>mailbox,
>because the original patches are just not there.

Should i resend v4? Or could you apply from Patchwork (series link in upper right of Patch)? Thats the way i use :)

https://patchwork.ozlabs.org/project/uboot/list/?series=200140
regards Frank

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 16:34           ` Frank Wunderlich
@ 2020-09-08 16:47             ` Marek Vasut
  2020-09-08 17:14               ` Frank Wunderlich
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2020-09-08 16:47 UTC (permalink / raw)
  To: u-boot

On 9/8/20 6:34 PM, Frank Wunderlich wrote:
> 
> 
> Am 8. September 2020 18:15:05 MESZ schrieb Marek Vasut <marex@denx.de>:
>> Thank you, I cannot apply a patchset which only has "Re:" in my
>> mailbox,
>> because the original patches are just not there.
> 
> Should i resend v4? Or could you apply from Patchwork (series link in upper right of Patch)? Thats the way i use :)
> 
> https://patchwork.ozlabs.org/project/uboot/list/?series=200140

Resend please, if I wanted to comment on any of those patches, I won't
be able.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 16:47             ` Marek Vasut
@ 2020-09-08 17:14               ` Frank Wunderlich
  2020-09-08 17:30                 ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Frank Wunderlich @ 2020-09-08 17:14 UTC (permalink / raw)
  To: u-boot



Am 8. September 2020 18:47:47 MESZ schrieb Marek Vasut <marex@denx.de>:

>Resend please, if I wanted to comment on any of those patches, I won't
>be able.

Have resend with bit reduced recipients (afair ryder and weijie are in GSS mailinglist),because this floods others mailbox. It's only a workaround.

Have you found problem or any idea why you not able to receive from chunfeng directly?
regards Frank

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 17:14               ` Frank Wunderlich
@ 2020-09-08 17:30                 ` Marek Vasut
  0 siblings, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2020-09-08 17:30 UTC (permalink / raw)
  To: u-boot

On 9/8/20 7:14 PM, Frank Wunderlich wrote:
> 
> 
> Am 8. September 2020 18:47:47 MESZ schrieb Marek Vasut <marex@denx.de>:
> 
>> Resend please, if I wanted to comment on any of those patches, I won't
>> be able.
> 
> Have resend with bit reduced recipients (afair ryder and weijie are in GSS mailinglist),because this floods others mailbox. It's only a workaround.
> 
> Have you found problem or any idea why you not able to receive from chunfeng directly?

No, however I have no problems receiving emails from others.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-08 16:15         ` Marek Vasut
  2020-09-08 16:34           ` Frank Wunderlich
@ 2020-09-09  1:55           ` Bin Meng
  2020-09-09 10:00             ` Marek Vasut
  1 sibling, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-09  1:55 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
>
> On 9/8/20 5:45 PM, Bin Meng wrote:
> > Hi Marek,
>
> Hi,
>
> > On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 9/8/20 3:44 AM, Bin Meng wrote:
> >>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>>
> >>>> Use readx_poll_sleep_timeout() to poll the register status
> >>>>
> >>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>>> ---
> >>>> v3: no changes
> >>>>
> >>>> v2: fix typo of title suggested by Frank
> >>>> ---
> >>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> >>>>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>>>
> >>>
> >>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> >>
> >> Thanks. So now I have half of a RESEND patchset with RB tags, and
> >> another half of just RB tags without patchset. Can someone of you please
> >> collect the tags and resend the patchset once more with the collected
> >> tags, so it's all in one place ?
> >>
> >
> > I believe the latest version has all the RB tags.
>
> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
> because the original patches are just not there.

You can use patchwork to help the custodian work. I found it very
helpful to download the patches from patchwork and apply.

Regards,
Bin

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09  1:55           ` Bin Meng
@ 2020-09-09 10:00             ` Marek Vasut
  2020-09-09 13:46               ` Bin Meng
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2020-09-09 10:00 UTC (permalink / raw)
  To: u-boot

On 9/9/20 3:55 AM, Bin Meng wrote:
> Hi Marek,

Hi,

> On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 9/8/20 5:45 PM, Bin Meng wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 9/8/20 3:44 AM, Bin Meng wrote:
>>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>>>>>>
>>>>>> Use readx_poll_sleep_timeout() to poll the register status
>>>>>>
>>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>>>>> ---
>>>>>> v3: no changes
>>>>>>
>>>>>> v2: fix typo of title suggested by Frank
>>>>>> ---
>>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>>>>>
>>>>>
>>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>>>
>>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
>>>> another half of just RB tags without patchset. Can someone of you please
>>>> collect the tags and resend the patchset once more with the collected
>>>> tags, so it's all in one place ?
>>>>
>>>
>>> I believe the latest version has all the RB tags.
>>
>> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
>> because the original patches are just not there.
> 
> You can use patchwork to help the custodian work. I found it very
> helpful to download the patches from patchwork and apply.

Can you also comment on patches via patchwork ?

I mean, really, can we please just stick to the usual workflow ? That's
all I am asking for , nothing else, nothing new.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09 10:00             ` Marek Vasut
@ 2020-09-09 13:46               ` Bin Meng
  2020-09-09 14:02                 ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Bin Meng @ 2020-09-09 13:46 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 9, 2020 at 7:05 PM Marek Vasut <marex@denx.de> wrote:
>
> On 9/9/20 3:55 AM, Bin Meng wrote:
> > Hi Marek,
>
> Hi,
>
> > On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 9/8/20 5:45 PM, Bin Meng wrote:
> >>> Hi Marek,
> >>
> >> Hi,
> >>
> >>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 9/8/20 3:44 AM, Bin Meng wrote:
> >>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>>>>
> >>>>>> Use readx_poll_sleep_timeout() to poll the register status
> >>>>>>
> >>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>>>>> ---
> >>>>>> v3: no changes
> >>>>>>
> >>>>>> v2: fix typo of title suggested by Frank
> >>>>>> ---
> >>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> >>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>>>>>
> >>>>>
> >>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> >>>>
> >>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
> >>>> another half of just RB tags without patchset. Can someone of you please
> >>>> collect the tags and resend the patchset once more with the collected
> >>>> tags, so it's all in one place ?
> >>>>
> >>>
> >>> I believe the latest version has all the RB tags.
> >>
> >> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
> >> because the original patches are just not there.
> >
> > You can use patchwork to help the custodian work. I found it very
> > helpful to download the patches from patchwork and apply.
>
> Can you also comment on patches via patchwork ?
>
> I mean, really, can we please just stick to the usual workflow ? That's
> all I am asking for , nothing else, nothing new.

I don't understand the question about "usual workflow". You asked "I
cannot apply a patchset which only has "Re:" in my mailbox" and I
recommended patchwork which will satisfy the requirement of what you
asked.

Regards,
Bin

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09 13:46               ` Bin Meng
@ 2020-09-09 14:02                 ` Marek Vasut
  2020-09-09 14:05                   ` Bin Meng
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2020-09-09 14:02 UTC (permalink / raw)
  To: u-boot

On 9/9/20 3:46 PM, Bin Meng wrote:
> On Wed, Sep 9, 2020 at 7:05 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 9/9/20 3:55 AM, Bin Meng wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 9/8/20 5:45 PM, Bin Meng wrote:
>>>>> Hi Marek,
>>>>
>>>> Hi,
>>>>
>>>>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 9/8/20 3:44 AM, Bin Meng wrote:
>>>>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>>>>>>>>
>>>>>>>> Use readx_poll_sleep_timeout() to poll the register status
>>>>>>>>
>>>>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>>>>>>> ---
>>>>>>>> v3: no changes
>>>>>>>>
>>>>>>>> v2: fix typo of title suggested by Frank
>>>>>>>> ---
>>>>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>>>>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>
>>>>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
>>>>>> another half of just RB tags without patchset. Can someone of you please
>>>>>> collect the tags and resend the patchset once more with the collected
>>>>>> tags, so it's all in one place ?
>>>>>>
>>>>>
>>>>> I believe the latest version has all the RB tags.
>>>>
>>>> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
>>>> because the original patches are just not there.
>>>
>>> You can use patchwork to help the custodian work. I found it very
>>> helpful to download the patches from patchwork and apply.
>>
>> Can you also comment on patches via patchwork ?
>>
>> I mean, really, can we please just stick to the usual workflow ? That's
>> all I am asking for , nothing else, nothing new.
> 
> I don't understand the question about "usual workflow". You asked "I
> cannot apply a patchset which only has "Re:" in my mailbox" and I
> recommended patchwork which will satisfy the requirement of what you
> asked.

Right, I can, but if there is more I want to say about the patch, I
cannot do it with patchwork. Then I need the patch, which I don't have.
Hence my remark about the usual email-based workflow.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09 14:02                 ` Marek Vasut
@ 2020-09-09 14:05                   ` Bin Meng
  2020-09-09 14:16                     ` Tom Rini
  2020-09-09 14:16                     ` Marek Vasut
  0 siblings, 2 replies; 31+ messages in thread
From: Bin Meng @ 2020-09-09 14:05 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 9, 2020 at 10:02 PM Marek Vasut <marex@denx.de> wrote:
>
> On 9/9/20 3:46 PM, Bin Meng wrote:
> > On Wed, Sep 9, 2020 at 7:05 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 9/9/20 3:55 AM, Bin Meng wrote:
> >>> Hi Marek,
> >>
> >> Hi,
> >>
> >>> On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 9/8/20 5:45 PM, Bin Meng wrote:
> >>>>> Hi Marek,
> >>>>
> >>>> Hi,
> >>>>
> >>>>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
> >>>>>>
> >>>>>> On 9/8/20 3:44 AM, Bin Meng wrote:
> >>>>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >>>>>>>>
> >>>>>>>> Use readx_poll_sleep_timeout() to poll the register status
> >>>>>>>>
> >>>>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>>>>>>> ---
> >>>>>>>> v3: no changes
> >>>>>>>>
> >>>>>>>> v2: fix typo of title suggested by Frank
> >>>>>>>> ---
> >>>>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> >>>>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>>>>>>>
> >>>>>>>
> >>>>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> >>>>>>
> >>>>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
> >>>>>> another half of just RB tags without patchset. Can someone of you please
> >>>>>> collect the tags and resend the patchset once more with the collected
> >>>>>> tags, so it's all in one place ?
> >>>>>>
> >>>>>
> >>>>> I believe the latest version has all the RB tags.
> >>>>
> >>>> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
> >>>> because the original patches are just not there.
> >>>
> >>> You can use patchwork to help the custodian work. I found it very
> >>> helpful to download the patches from patchwork and apply.
> >>
> >> Can you also comment on patches via patchwork ?
> >>
> >> I mean, really, can we please just stick to the usual workflow ? That's
> >> all I am asking for , nothing else, nothing new.
> >
> > I don't understand the question about "usual workflow". You asked "I
> > cannot apply a patchset which only has "Re:" in my mailbox" and I
> > recommended patchwork which will satisfy the requirement of what you
> > asked.
>
> Right, I can, but if there is more I want to say about the patch, I
> cannot do it with patchwork. Then I need the patch, which I don't have.
> Hence my remark about the usual email-based workflow.

Yes, that's true. If we need to post comments about patches, we will
have to use email.

Regards,
Bin

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09 14:05                   ` Bin Meng
@ 2020-09-09 14:16                     ` Tom Rini
  2020-09-09 14:16                     ` Marek Vasut
  1 sibling, 0 replies; 31+ messages in thread
From: Tom Rini @ 2020-09-09 14:16 UTC (permalink / raw)
  To: u-boot

On Wed, Sep 09, 2020 at 10:05:42PM +0800, Bin Meng wrote:
> On Wed, Sep 9, 2020 at 10:02 PM Marek Vasut <marex@denx.de> wrote:
> >
> > On 9/9/20 3:46 PM, Bin Meng wrote:
> > > On Wed, Sep 9, 2020 at 7:05 PM Marek Vasut <marex@denx.de> wrote:
> > >>
> > >> On 9/9/20 3:55 AM, Bin Meng wrote:
> > >>> Hi Marek,
> > >>
> > >> Hi,
> > >>
> > >>> On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
> > >>>>
> > >>>> On 9/8/20 5:45 PM, Bin Meng wrote:
> > >>>>> Hi Marek,
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>>>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
> > >>>>>>
> > >>>>>> On 9/8/20 3:44 AM, Bin Meng wrote:
> > >>>>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >>>>>>>>
> > >>>>>>>> Use readx_poll_sleep_timeout() to poll the register status
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > >>>>>>>> ---
> > >>>>>>>> v3: no changes
> > >>>>>>>>
> > >>>>>>>> v2: fix typo of title suggested by Frank
> > >>>>>>>> ---
> > >>>>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
> > >>>>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > >>>>>>
> > >>>>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
> > >>>>>> another half of just RB tags without patchset. Can someone of you please
> > >>>>>> collect the tags and resend the patchset once more with the collected
> > >>>>>> tags, so it's all in one place ?
> > >>>>>>
> > >>>>>
> > >>>>> I believe the latest version has all the RB tags.
> > >>>>
> > >>>> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
> > >>>> because the original patches are just not there.
> > >>>
> > >>> You can use patchwork to help the custodian work. I found it very
> > >>> helpful to download the patches from patchwork and apply.
> > >>
> > >> Can you also comment on patches via patchwork ?
> > >>
> > >> I mean, really, can we please just stick to the usual workflow ? That's
> > >> all I am asking for , nothing else, nothing new.
> > >
> > > I don't understand the question about "usual workflow". You asked "I
> > > cannot apply a patchset which only has "Re:" in my mailbox" and I
> > > recommended patchwork which will satisfy the requirement of what you
> > > asked.
> >
> > Right, I can, but if there is more I want to say about the patch, I
> > cannot do it with patchwork. Then I need the patch, which I don't have.
> > Hence my remark about the usual email-based workflow.
> 
> Yes, that's true. If we need to post comments about patches, we will
> have to use email.

Which, to close the loop, is trivial with patchwork, if you're making a
new comment, and hard if you are trying to join an ongoing conversation.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200909/dc079d0c/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()
  2020-09-09 14:05                   ` Bin Meng
  2020-09-09 14:16                     ` Tom Rini
@ 2020-09-09 14:16                     ` Marek Vasut
  1 sibling, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2020-09-09 14:16 UTC (permalink / raw)
  To: u-boot

On 9/9/20 4:05 PM, Bin Meng wrote:
> On Wed, Sep 9, 2020 at 10:02 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 9/9/20 3:46 PM, Bin Meng wrote:
>>> On Wed, Sep 9, 2020 at 7:05 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 9/9/20 3:55 AM, Bin Meng wrote:
>>>>> Hi Marek,
>>>>
>>>> Hi,
>>>>
>>>>> On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 9/8/20 5:45 PM, Bin Meng wrote:
>>>>>>> Hi Marek,
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>>>
>>>>>>>> On 9/8/20 3:44 AM, Bin Meng wrote:
>>>>>>>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>>>>>>>>>>
>>>>>>>>>> Use readx_poll_sleep_timeout() to poll the register status
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>>>>>>>>> ---
>>>>>>>>>> v3: no changes
>>>>>>>>>>
>>>>>>>>>> v2: fix typo of title suggested by Frank
>>>>>>>>>> ---
>>>>>>>>>>  drivers/usb/host/xhci.c | 25 +++++++++++--------------
>>>>>>>>>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>>>>>>>
>>>>>>>> Thanks. So now I have half of a RESEND patchset with RB tags, and
>>>>>>>> another half of just RB tags without patchset. Can someone of you please
>>>>>>>> collect the tags and resend the patchset once more with the collected
>>>>>>>> tags, so it's all in one place ?
>>>>>>>>
>>>>>>>
>>>>>>> I believe the latest version has all the RB tags.
>>>>>>
>>>>>> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
>>>>>> because the original patches are just not there.
>>>>>
>>>>> You can use patchwork to help the custodian work. I found it very
>>>>> helpful to download the patches from patchwork and apply.
>>>>
>>>> Can you also comment on patches via patchwork ?
>>>>
>>>> I mean, really, can we please just stick to the usual workflow ? That's
>>>> all I am asking for , nothing else, nothing new.
>>>
>>> I don't understand the question about "usual workflow". You asked "I
>>> cannot apply a patchset which only has "Re:" in my mailbox" and I
>>> recommended patchwork which will satisfy the requirement of what you
>>> asked.
>>
>> Right, I can, but if there is more I want to say about the patch, I
>> cannot do it with patchwork. Then I need the patch, which I don't have.
>> Hence my remark about the usual email-based workflow.
> 
> Yes, that's true. If we need to post comments about patches, we will
> have to use email.

The problem here is that for whatever reason, I didn't receive the
original patches, Frank had to resend them, then I got RBs on some of
each and it was a mess. I'd just like to get back to the usual. I think
we agree, don't we ?

Anyway, thanks for the RBs, that really helped.

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-09-09 14:16 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07  7:11 [PATCH v3 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder Chunfeng Yun
2020-09-08  5:41   ` Bin Meng
2020-09-08  7:40     ` Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96 Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 4/9] usb: xhci: convert to HCS_MAX_PORTS() Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 5/9] usb: xhci: convert to TRB_TYPE() Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET() Chunfeng Yun
2020-09-08  1:30   ` Bin Meng
2020-09-08  7:39     ` Chunfeng Yun
2020-09-07  7:12 ` [PATCH v3 7/9] usb: xhci: convert to TRB_TX_TYPE() Chunfeng Yun
2020-09-08  1:31   ` Bin Meng
2020-09-07  7:12 ` [PATCH v3 8/9] usb: xhci: use macros with parameter to fill ep_info2 Chunfeng Yun
2020-09-08  1:35   ` Bin Meng
2020-09-07  7:12 ` [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout() Chunfeng Yun
2020-09-08  1:44   ` Bin Meng
2020-09-08 11:13     ` Marek Vasut
2020-09-08 11:24       ` Chunfeng Yun
2020-09-08 15:45       ` Bin Meng
2020-09-08 16:15         ` Marek Vasut
2020-09-08 16:34           ` Frank Wunderlich
2020-09-08 16:47             ` Marek Vasut
2020-09-08 17:14               ` Frank Wunderlich
2020-09-08 17:30                 ` Marek Vasut
2020-09-09  1:55           ` Bin Meng
2020-09-09 10:00             ` Marek Vasut
2020-09-09 13:46               ` Bin Meng
2020-09-09 14:02                 ` Marek Vasut
2020-09-09 14:05                   ` Bin Meng
2020-09-09 14:16                     ` Tom Rini
2020-09-09 14:16                     ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.