All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
@ 2020-12-22  9:34 ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Chunfeng Yun, Tianping Fang, Zhanyong Wang, linux-mediatek,
	linux-usb, linux-kernel, Zhanyong Wang, Yaqii Wu

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
usb 1-1.1: Manufacturer: Apple, Inc.
usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
xhci-mtk 11200000.xhci: Not enough bandwidth!
usb 1-1.1: can't set config #2, error -28

improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
muli-TT.

Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v3: no changes
---
 drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
 drivers/usb/host/xhci-mtk.h     |  8 ++-
 2 files changed, 84 insertions(+), 15 deletions(-)
 mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
old mode 100644
new mode 100755
index 45c54d56ecbd..94292b9bbc63
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
 	u32 fs_budget_start;
 	u32 start_ss, last_ss;
 	u32 start_cs, last_cs;
-	int i;
+	u32 num_esit, base;
+	int i, j;
+	u32 tmp;
 
 	start_ss = offset % 8;
 	fs_budget_start = (start_ss + 1) % 8;
@@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
 		if (!(start_ss == 7 || last_ss < 6))
 			return -ERANGE;
 
-		for (i = 0; i < sch_ep->cs_count; i++)
-			if (test_bit(offset + i, tt->split_bit_map))
+		for (i = 0; i < sch_ep->cs_count; i++) {
+			if (test_bit(offset + i, tt->ss_bit_map))
 				return -ERANGE;
 
+			if (test_bit(offset + i, tt->cs_bit_map))
+				return -ERANGE;
+		}
 	} else {
 		u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
 
@@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
 		if (cs_count > 7)
 			cs_count = 7; /* HW limit */
 
-		for (i = 0; i < cs_count + 2; i++) {
-			if (test_bit(offset + i, tt->split_bit_map))
+		if (test_bit(offset, tt->ss_bit_map))
+			return -ERANGE;
+		for (i = 0; i < cs_count; i++) {
+			if (test_bit(offset + 2 + i, tt->cs_bit_map))
 				return -ERANGE;
 		}
 
@@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
 			sch_ep->num_budget_microframes = sch_ep->esit;
 	}
 
+	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
+	for (i = 0; i < num_esit; i++) {
+		base = sch_ep->offset + i * sch_ep->esit;
+		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+			tmp = tt->fs_bus_bw[base + j]
+			      + sch_ep->bw_cost_per_microframe;
+			if (tmp > FS_PAYLOAD_MAX)
+				return -ERANGE;
+		}
+	}
+
 	return 0;
 }
 
 static void update_sch_tt(struct usb_device *udev,
-	struct mu3h_sch_ep_info *sch_ep)
+	struct mu3h_sch_ep_info *sch_ep, bool used)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
 	u32 base, num_esit;
@@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
 	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
 	for (i = 0; i < num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
-		for (j = 0; j < sch_ep->num_budget_microframes; j++)
-			set_bit(base + j, tt->split_bit_map);
+		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+			if (used)
+				set_bit(base + j, tt->split_bit_map);
+			else
+				clear_bit(base + j, tt->split_bit_map);
+		}
+
+		if (sch_ep->ep_type == ISOC_OUT_EP) {
+			for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+				if (used) {
+					set_bit(base + j, tt->ss_bit_map);
+					set_bit(base + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + j] +=
+						sch_ep->bw_cost_per_microframe;
+				} else {
+					clear_bit(base + j, tt->ss_bit_map);
+					clear_bit(base + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + j] -=
+						sch_ep->bw_cost_per_microframe;
+				}
+			}
+		} else {
+			if (used)
+				set_bit(base, tt->ss_bit_map);
+			else
+				clear_bit(base, tt->ss_bit_map);
+
+			for (j = 0; j < sch_ep->cs_count; j++) {
+				if (used) {
+					set_bit(base + 2 + j, tt->cs_bit_map);
+
+					tt->fs_bus_bw[base + 2 + j] +=
+						sch_ep->bw_cost_per_microframe;
+				} else {
+					clear_bit(base + 2 + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + 2 + j] -=
+						sch_ep->bw_cost_per_microframe;
+				}
+			}
+		}
 	}
 
-	list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
+	if (used)
+		list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
+	else
+		list_del(&sch_ep->tt_endpoint);
 }
 
 static int check_sch_bw(struct usb_device *udev,
@@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
 {
 	u32 offset;
 	u32 esit;
+	u32 boundary;
 	u32 min_bw;
 	u32 min_index;
 	u32 worst_bw;
@@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
 	 */
 	min_bw = ~0;
 	min_index = 0;
+	boundary = esit;
 	min_cs_count = sch_ep->cs_count;
 	min_num_budget = sch_ep->num_budget_microframes;
 	for (offset = 0; offset < esit; offset++) {
 		if (is_fs_or_ls(udev->speed)) {
+			if (sch_ep->ep_type != ISOC_OUT_EP)
+				boundary = esit + 1;
 			ret = check_sch_tt(udev, sch_ep, offset);
 			if (ret)
 				continue;
@@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
 				tt_offset_ok = true;
 		}
 
-		if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
+		if ((offset + sch_ep->num_budget_microframes) > boundary)
 			break;
 
 		worst_bw = get_max_bw(sch_bw, sch_ep, offset);
@@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
 		if (!tt_offset_ok)
 			return -ERANGE;
 
-		update_sch_tt(udev, sch_ep);
+		update_sch_tt(udev, sch_ep, 1);
 	}
 
 	/* update bus bandwidth info */
@@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 
 	list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
 		if (sch_ep->ep == ep) {
-			update_bus_bw(sch_bw, sch_ep, 0);
-			list_del(&sch_ep->endpoint);
 			if (is_fs_or_ls(udev->speed)) {
-				list_del(&sch_ep->tt_endpoint);
+				update_sch_tt(udev, sch_ep, 0);
 				drop_tt(udev);
 			}
+			update_bus_bw(sch_bw, sch_ep, 0);
+			list_del(&sch_ep->endpoint);
 			kfree(sch_ep);
 			break;
 		}
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe817904..323b281933b9 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -21,12 +21,18 @@
 
 /**
  * @split_bit_map: used to avoid split microframes overlay
+ * @ss_bit_map: used to avoid start split microframes overlay
+ * @cs_bit_map: used to avoid complete split microframes overlay
+ * @fs_bus_bw: array to keep track of bandwidth already used at full speed
  * @ep_list: Endpoints using this TT
  * @usb_tt: usb TT related
  * @tt_port: TT port number
  */
 struct mu3h_sch_tt {
 	DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
+	DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
+	DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
+	u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
 	struct list_head ep_list;
 	struct usb_tt *usb_tt;
 	int tt_port;
@@ -42,7 +48,7 @@ struct mu3h_sch_tt {
  * two bandwidth domains, one for IN eps and another for OUT eps.
  */
 struct mu3h_sch_bw_info {
-	u32 bus_bw[XHCI_MTK_MAX_ESIT];
+	u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
 	struct list_head bw_ep_list;
 };
 
-- 
2.18.0


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

* [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
@ 2020-12-22  9:34 ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Yaqii Wu, Zhanyong Wang, Zhanyong Wang, linux-usb, linux-kernel,
	Tianping Fang, linux-mediatek, Chunfeng Yun

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
usb 1-1.1: Manufacturer: Apple, Inc.
usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
xhci-mtk 11200000.xhci: Not enough bandwidth!
usb 1-1.1: can't set config #2, error -28

improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
muli-TT.

Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v3: no changes
---
 drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
 drivers/usb/host/xhci-mtk.h     |  8 ++-
 2 files changed, 84 insertions(+), 15 deletions(-)
 mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
old mode 100644
new mode 100755
index 45c54d56ecbd..94292b9bbc63
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
 	u32 fs_budget_start;
 	u32 start_ss, last_ss;
 	u32 start_cs, last_cs;
-	int i;
+	u32 num_esit, base;
+	int i, j;
+	u32 tmp;
 
 	start_ss = offset % 8;
 	fs_budget_start = (start_ss + 1) % 8;
@@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
 		if (!(start_ss == 7 || last_ss < 6))
 			return -ERANGE;
 
-		for (i = 0; i < sch_ep->cs_count; i++)
-			if (test_bit(offset + i, tt->split_bit_map))
+		for (i = 0; i < sch_ep->cs_count; i++) {
+			if (test_bit(offset + i, tt->ss_bit_map))
 				return -ERANGE;
 
+			if (test_bit(offset + i, tt->cs_bit_map))
+				return -ERANGE;
+		}
 	} else {
 		u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
 
@@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
 		if (cs_count > 7)
 			cs_count = 7; /* HW limit */
 
-		for (i = 0; i < cs_count + 2; i++) {
-			if (test_bit(offset + i, tt->split_bit_map))
+		if (test_bit(offset, tt->ss_bit_map))
+			return -ERANGE;
+		for (i = 0; i < cs_count; i++) {
+			if (test_bit(offset + 2 + i, tt->cs_bit_map))
 				return -ERANGE;
 		}
 
@@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
 			sch_ep->num_budget_microframes = sch_ep->esit;
 	}
 
+	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
+	for (i = 0; i < num_esit; i++) {
+		base = sch_ep->offset + i * sch_ep->esit;
+		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+			tmp = tt->fs_bus_bw[base + j]
+			      + sch_ep->bw_cost_per_microframe;
+			if (tmp > FS_PAYLOAD_MAX)
+				return -ERANGE;
+		}
+	}
+
 	return 0;
 }
 
 static void update_sch_tt(struct usb_device *udev,
-	struct mu3h_sch_ep_info *sch_ep)
+	struct mu3h_sch_ep_info *sch_ep, bool used)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
 	u32 base, num_esit;
@@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
 	num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
 	for (i = 0; i < num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
-		for (j = 0; j < sch_ep->num_budget_microframes; j++)
-			set_bit(base + j, tt->split_bit_map);
+		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+			if (used)
+				set_bit(base + j, tt->split_bit_map);
+			else
+				clear_bit(base + j, tt->split_bit_map);
+		}
+
+		if (sch_ep->ep_type == ISOC_OUT_EP) {
+			for (j = 0; j < sch_ep->num_budget_microframes; j++) {
+				if (used) {
+					set_bit(base + j, tt->ss_bit_map);
+					set_bit(base + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + j] +=
+						sch_ep->bw_cost_per_microframe;
+				} else {
+					clear_bit(base + j, tt->ss_bit_map);
+					clear_bit(base + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + j] -=
+						sch_ep->bw_cost_per_microframe;
+				}
+			}
+		} else {
+			if (used)
+				set_bit(base, tt->ss_bit_map);
+			else
+				clear_bit(base, tt->ss_bit_map);
+
+			for (j = 0; j < sch_ep->cs_count; j++) {
+				if (used) {
+					set_bit(base + 2 + j, tt->cs_bit_map);
+
+					tt->fs_bus_bw[base + 2 + j] +=
+						sch_ep->bw_cost_per_microframe;
+				} else {
+					clear_bit(base + 2 + j, tt->cs_bit_map);
+					tt->fs_bus_bw[base + 2 + j] -=
+						sch_ep->bw_cost_per_microframe;
+				}
+			}
+		}
 	}
 
-	list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
+	if (used)
+		list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
+	else
+		list_del(&sch_ep->tt_endpoint);
 }
 
 static int check_sch_bw(struct usb_device *udev,
@@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
 {
 	u32 offset;
 	u32 esit;
+	u32 boundary;
 	u32 min_bw;
 	u32 min_index;
 	u32 worst_bw;
@@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
 	 */
 	min_bw = ~0;
 	min_index = 0;
+	boundary = esit;
 	min_cs_count = sch_ep->cs_count;
 	min_num_budget = sch_ep->num_budget_microframes;
 	for (offset = 0; offset < esit; offset++) {
 		if (is_fs_or_ls(udev->speed)) {
+			if (sch_ep->ep_type != ISOC_OUT_EP)
+				boundary = esit + 1;
 			ret = check_sch_tt(udev, sch_ep, offset);
 			if (ret)
 				continue;
@@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
 				tt_offset_ok = true;
 		}
 
-		if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
+		if ((offset + sch_ep->num_budget_microframes) > boundary)
 			break;
 
 		worst_bw = get_max_bw(sch_bw, sch_ep, offset);
@@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
 		if (!tt_offset_ok)
 			return -ERANGE;
 
-		update_sch_tt(udev, sch_ep);
+		update_sch_tt(udev, sch_ep, 1);
 	}
 
 	/* update bus bandwidth info */
@@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 
 	list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
 		if (sch_ep->ep == ep) {
-			update_bus_bw(sch_bw, sch_ep, 0);
-			list_del(&sch_ep->endpoint);
 			if (is_fs_or_ls(udev->speed)) {
-				list_del(&sch_ep->tt_endpoint);
+				update_sch_tt(udev, sch_ep, 0);
 				drop_tt(udev);
 			}
+			update_bus_bw(sch_bw, sch_ep, 0);
+			list_del(&sch_ep->endpoint);
 			kfree(sch_ep);
 			break;
 		}
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe817904..323b281933b9 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -21,12 +21,18 @@
 
 /**
  * @split_bit_map: used to avoid split microframes overlay
+ * @ss_bit_map: used to avoid start split microframes overlay
+ * @cs_bit_map: used to avoid complete split microframes overlay
+ * @fs_bus_bw: array to keep track of bandwidth already used at full speed
  * @ep_list: Endpoints using this TT
  * @usb_tt: usb TT related
  * @tt_port: TT port number
  */
 struct mu3h_sch_tt {
 	DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
+	DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
+	DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
+	u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
 	struct list_head ep_list;
 	struct usb_tt *usb_tt;
 	int tt_port;
@@ -42,7 +48,7 @@ struct mu3h_sch_tt {
  * two bandwidth domains, one for IN eps and another for OUT eps.
  */
 struct mu3h_sch_bw_info {
-	u32 bus_bw[XHCI_MTK_MAX_ESIT];
+	u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
 	struct list_head bw_ep_list;
 };
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [RFC PATCH v3 2/5] usb: xhci-mtk: add mt8192 wakeup
  2020-12-22  9:34 ` Chunfeng Yun
@ 2020-12-22  9:34   ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Chunfeng Yun, Tianping Fang, Zhanyong Wang, linux-mediatek,
	linux-usb, linux-kernel, Zhanyong Wang

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

need to add wakeup solution as V3
since not support presently.

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v3: no changes
---
 drivers/usb/host/xhci-mtk.c | 9 +++++++++
 1 file changed, 9 insertions(+)
 mode change 100644 => 100755 drivers/usb/host/xhci-mtk.c

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
old mode 100644
new mode 100755
index 8f321f39ab96..565ce4c16206
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -68,9 +68,13 @@
 #define SSC_IP_SLEEP_EN	BIT(4)
 #define SSC_SPM_INT_EN		BIT(1)
 
+/* mt8192 etc */
+#define SSC_IP_SLEEP_EN_V3	BIT(6)
+
 enum ssusb_uwk_vers {
 	SSUSB_UWK_V1 = 1,
 	SSUSB_UWK_V2,
+	SSUSB_UWK_V3,
 };
 
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
@@ -305,6 +309,11 @@ static void usb_wakeup_ip_sleep_set(struct xhci_hcd_mtk *mtk, bool enable)
 		msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN;
 		val = enable ? msk : 0;
 		break;
+	case SSUSB_UWK_V3:
+		reg = mtk->uwk_reg_base + PERI_SSUSB_SPM_CTRL;
+		msk = SSC_IP_SLEEP_EN_V3 | SSC_SPM_INT_EN;
+		val = enable ? msk : 0;
+		break;
 	default:
 		return;
 	}
-- 
2.18.0


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

* [RFC PATCH v3 2/5] usb: xhci-mtk: add mt8192 wakeup
@ 2020-12-22  9:34   ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Zhanyong Wang, Zhanyong Wang, linux-usb, linux-kernel,
	Tianping Fang, linux-mediatek, Chunfeng Yun

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

need to add wakeup solution as V3
since not support presently.

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v2~v3: no changes
---
 drivers/usb/host/xhci-mtk.c | 9 +++++++++
 1 file changed, 9 insertions(+)
 mode change 100644 => 100755 drivers/usb/host/xhci-mtk.c

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
old mode 100644
new mode 100755
index 8f321f39ab96..565ce4c16206
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -68,9 +68,13 @@
 #define SSC_IP_SLEEP_EN	BIT(4)
 #define SSC_SPM_INT_EN		BIT(1)
 
+/* mt8192 etc */
+#define SSC_IP_SLEEP_EN_V3	BIT(6)
+
 enum ssusb_uwk_vers {
 	SSUSB_UWK_V1 = 1,
 	SSUSB_UWK_V2,
+	SSUSB_UWK_V3,
 };
 
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
@@ -305,6 +309,11 @@ static void usb_wakeup_ip_sleep_set(struct xhci_hcd_mtk *mtk, bool enable)
 		msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN;
 		val = enable ? msk : 0;
 		break;
+	case SSUSB_UWK_V3:
+		reg = mtk->uwk_reg_base + PERI_SSUSB_SPM_CTRL;
+		msk = SSC_IP_SLEEP_EN_V3 | SSC_SPM_INT_EN;
+		val = enable ? msk : 0;
+		break;
 	default:
 		return;
 	}
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [RFC PATCH v3 3/5] usb: xhci-mtk: fix random remote wakeup
  2020-12-22  9:34 ` Chunfeng Yun
@ 2020-12-22  9:34   ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Chunfeng Yun, Tianping Fang, Zhanyong Wang, linux-mediatek,
	linux-usb, linux-kernel

Wakeup_signal will toggle status according to ssusb_ip_sleep
signal after debounc time, add a delay time to avoid spurious
wakeup event.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: new patch
---
 drivers/usb/host/xhci-mtk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 565ce4c16206..34bd3de090b1 100755
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -172,7 +172,10 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
 	if (ret) {
 		dev_err(mtk->dev, "ip sleep failed!!!\n");
 		return ret;
+	} else {
+		usleep_range(200, 250);
 	}
+
 	return 0;
 }
 
-- 
2.18.0


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

* [RFC PATCH v3 3/5] usb: xhci-mtk: fix random remote wakeup
@ 2020-12-22  9:34   ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Zhanyong Wang, linux-usb, linux-kernel, Tianping Fang,
	linux-mediatek, Chunfeng Yun

Wakeup_signal will toggle status according to ssusb_ip_sleep
signal after debounc time, add a delay time to avoid spurious
wakeup event.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: new patch
---
 drivers/usb/host/xhci-mtk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 565ce4c16206..34bd3de090b1 100755
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -172,7 +172,10 @@ static int xhci_mtk_host_disable(struct xhci_hcd_mtk *mtk)
 	if (ret) {
 		dev_err(mtk->dev, "ip sleep failed!!!\n");
 		return ret;
+	} else {
+		usleep_range(200, 250);
 	}
+
 	return 0;
 }
 
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
  2020-12-22  9:34 ` Chunfeng Yun
@ 2020-12-22  9:34   ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Chunfeng Yun, Tianping Fang, Zhanyong Wang, linux-mediatek,
	linux-usb, linux-kernel, CK Hu, Zhanyong Wang

From: CK Hu <ck.hu@mediatek.com>

add support runtime pm feature

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3:
  1. fix some issues
  2. remove attribute files

v2: fix error caused by request irq suggested by CK
---
 drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci-mtk.h |  14 ++
 2 files changed, 294 insertions(+), 5 deletions(-)
 mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
old mode 100755
new mode 100644
index 34bd3de090b1..c07d54acbcb7
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -14,6 +14,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
 	SSUSB_UWK_V3,
 };
 
+int xhci_mtk_runtime_ready;
+
+static int xhci_mtk_runtime_resume(struct device *dev);
+
+static void xhci_mtk_seal_work(struct work_struct *work)
+{
+	struct xhci_hcd_mtk *mtk =
+			container_of(work, struct xhci_hcd_mtk, seal.work);
+	struct usb_hcd *hcd = mtk->hcd;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
+	if (mtk->seal_status == SEAL_SUSPENDED) {
+		mtk->seal_status = SEAL_RESUMING;
+		pm_runtime_mark_last_busy(mtk->dev);
+		pm_runtime_put_sync_autosuspend(mtk->dev);
+	} else {
+		/*
+		 * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
+		 * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
+		 * and to survey why not to invoke runtime_resume callback named
+		 * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
+		 * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
+		 * on as soon as unseal event received.
+		 */
+		if (mtk->seal_status == SEAL_RESUMING)
+			xhci_mtk_runtime_resume(mtk->dev);
+		else
+			xhci_warn(xhci,
+				"Ignore seal wakeup source disordered in xHCI controller\n");
+	}
+}
+
+static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
+{
+	struct xhci_hcd_mtk *mtk = data;
+	struct usb_hcd *hcd = mtk->hcd;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	xhci_dbg(xhci, "seal irq ISR invoked\n");
+
+	schedule_delayed_work(&mtk->seal, 0);
+
+	return IRQ_HANDLED;
+}
+
+static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
+{
+	struct irq_desc *desc;
+	struct device *dev = mtk->dev;
+
+	if (mtk && mtk->seal_irq) {
+		desc = irq_to_desc(mtk->seal_irq);
+		if (enable) {
+			desc->irq_data.chip->irq_ack(&desc->irq_data);
+			enable_irq(mtk->seal_irq);
+			dev_dbg(dev, "%s: enable_irq %i\n",
+				 __func__, mtk->seal_irq);
+		} else {
+			disable_irq(mtk->seal_irq);
+			dev_dbg(dev, "%s: disable_irq %i\n",
+				 __func__, mtk->seal_irq);
+		}
+	}
+}
+
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
 {
 	struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
@@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
 			mtk->uwk_reg_base, mtk->uwk_vers);
 
 	return PTR_ERR_OR_ZERO(mtk->uwk);
-
 }
 
 static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
@@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	pm_runtime_set_active(dev);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_autosuspend_delay(dev,
+				CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
 	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
-	device_enable_async_suspend(dev);
 
 	ret = xhci_mtk_ldos_enable(mtk);
 	if (ret)
@@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		ret = irq;
 		goto disable_clk;
 	}
+	dev_dbg(dev, "irq %i\n", irq);
+
+	mtk->seal_irq = platform_get_irq_optional(pdev, 1);
+	if (mtk->seal_irq < 0) {
+		ret = mtk->seal_irq;
+		goto disable_clk;
+	}
+	dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
 
 	hcd = usb_create_hcd(driver, dev, dev_name(dev));
 	if (!hcd) {
@@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	if (ret)
 		goto dealloc_usb2_hcd;
 
+	INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
+	snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
+		 hcd->driver->description, hcd->self.busnum);
+	ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
+			  IRQF_TRIGGER_FALLING,	mtk->seal_descr, mtk);
+	if (ret != 0) {
+		dev_err(dev, "seal request interrupt %d failed\n",
+			mtk->seal_irq);
+		goto dealloc_usb2_hcd;
+	}
+	xhci_mtk_seal_wakeup_enable(mtk, false);
+
+	device_enable_async_suspend(dev);
+	xhci_mtk_runtime_ready = 1;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
+		 __func__, xhci_mtk_runtime_ready);
+
 	return 0;
 
 dealloc_usb2_hcd:
@@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	xhci_mtk_ldos_disable(mtk);
 
 disable_pm:
-	pm_runtime_put_sync(dev);
+	pm_runtime_put_sync_autosuspend(dev);
 	pm_runtime_disable(dev);
 	return ret;
 }
@@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	pm_runtime_put_noidle(&dev->dev);
 	pm_runtime_disable(&dev->dev);
 
+	xhci_mtk_runtime_ready = 0;
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
 	xhci_mtk_host_disable(mtk);
 	xhci_mtk_clks_disable(mtk);
 	usb_wakeup_set(mtk, true);
+
 	return 0;
 }
 
@@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
+{
+	struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+	struct xhci_hub *usb2_rhub;
+	struct xhci_hub *usb3_rhub;
+	struct xhci_bus_state *bus_state;
+	struct xhci_port *port;
+	u32	usb2_suspended_ports = -1;
+	u32	usb3_suspended_ports = -1;
+	u16 status;
+	int num_ports;
+	int ret = 0;
+	int i;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	usb2_rhub = &xhci->usb2_rhub;
+	if (usb2_rhub) {
+		bus_state  = &usb2_rhub->bus_state;
+		num_ports  = usb2_rhub->num_ports;
+		usb2_suspended_ports  = bus_state->suspended_ports;
+		usb2_suspended_ports ^= (BIT(num_ports) - 1);
+		usb2_suspended_ports &= (BIT(num_ports) - 1);
+		for (i = 0; i < num_ports; i++) {
+			if (usb2_suspended_ports & (1UL << i)) {
+				port = usb2_rhub->ports[i];
+				status = readl(port->addr);
+
+				xhci_dbg(xhci,
+					  "USB20: portsc[%i]: 0x%04X\n",
+					  i, status);
+
+				if (!(status & PORT_CONNECT))
+					usb2_suspended_ports &= ~(1UL << i);
+			}
+		}
+
+		if (usb2_suspended_ports) {
+			ret = -EBUSY;
+			goto ebusy;
+		}
+	}
+
+	usb3_rhub = &xhci->usb3_rhub;
+	if (usb3_rhub) {
+		bus_state  = &usb3_rhub->bus_state;
+		num_ports  = usb3_rhub->num_ports;
+		usb3_suspended_ports  = bus_state->suspended_ports;
+		usb3_suspended_ports ^= (BIT(num_ports) - 1);
+		usb3_suspended_ports &= (BIT(num_ports) - 1);
+		for (i = 0; i < num_ports; i++) {
+			if (usb3_suspended_ports & BIT(i)) {
+				port = usb3_rhub->ports[i];
+				status = readl(port->addr);
+
+				xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
+					  i, status);
+
+				if (!(status & PORT_CONNECT))
+					usb3_suspended_ports &= ~BIT(i);
+			}
+		}
+
+		if (usb3_suspended_ports) {
+			ret = -EBUSY;
+			goto ebusy;
+		}
+	}
+
+ebusy:
+	xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
+		  __func__, usb2_suspended_ports,
+		  usb3_suspended_ports, ret);
+
+	return ret;
+}
+
+static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
+{
+	bool wakeup = device_may_wakeup(dev);
+	struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+	int ret = 0;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	mtk->seal_status = SEAL_BUSY;
+	ret = xhci_mtk_bus_status(dev);
+	if (wakeup && !ret) {
+		mtk->seal_status = SEAL_SUSPENDING;
+		xhci_mtk_suspend(dev);
+		xhci_mtk_seal_wakeup_enable(mtk, true);
+		mtk->seal_status = SEAL_SUSPENDED;
+		xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
+	}
+
+	xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
+		  __func__, wakeup, ret);
+
+	return ret;
+}
+
+static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
+{
+	bool wakeup = device_may_wakeup(dev);
+	struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	/*
+	 *  list cases by one extra interrupt named seal to process!!!
+	 *  Who to process these module reinitilization after SPM wakeup
+	 *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
+	 *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
+	 *  case 3: usb client driver can invoke it in runtime mechanism
+	 *  case 4: user active
+	 */
+	if (wakeup) {
+		xhci_mtk_seal_wakeup_enable(mtk, false);
+		xhci_mtk_resume(dev);
+		xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
+	}
+	mtk->seal_status = SEAL_RESUMED;
+
+	xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
+
+	return 0;
+}
+
+static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
+		 __func__, xhci_mtk_runtime_ready);
+
+	if (!xhci_mtk_runtime_ready)
+		ret = -EAGAIN;
+
+	return ret;
+}
+
 static const struct dev_pm_ops xhci_mtk_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
+	SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
+			   xhci_mtk_runtime_resume,
+			   xhci_mtk_runtime_idle)
 };
-#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
+
+#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
 
 #ifdef CONFIG_OF
 static const struct of_device_id mtk_xhci_of_match[] = {
@@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
 
 static int __init xhci_mtk_init(void)
 {
+	xhci_mtk_runtime_ready = 0;
 	xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
 	return platform_driver_register(&mtk_xhci_driver);
 }
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 323b281933b9..103d83ce6a3e 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
 	__le32 reserved3[33]; /* 0x80 ~ 0xff */
 };
 
+enum xhci_mtk_seal {
+	SEAL_BUSY = 0,
+	SEAL_SUSPENDING,
+	SEAL_SUSPENDED,
+	SEAL_RESUMING,
+	SEAL_RESUMED
+};
+
 struct xhci_hcd_mtk {
 	struct device *dev;
 	struct usb_hcd *hcd;
@@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
 	struct regmap *uwk;
 	u32 uwk_reg_base;
 	u32 uwk_vers;
+
+	/* usb eint wakeup source */
+	int seal_irq;
+	enum xhci_mtk_seal seal_status;
+	struct delayed_work  seal;
+	char   seal_descr[32];	/* "seal" + driver + bus # */
 };
 
 static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
-- 
2.18.0


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

* [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
@ 2020-12-22  9:34   ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Zhanyong Wang, Zhanyong Wang, linux-usb, linux-kernel,
	Tianping Fang, CK Hu, linux-mediatek, Chunfeng Yun

From: CK Hu <ck.hu@mediatek.com>

add support runtime pm feature

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3:
  1. fix some issues
  2. remove attribute files

v2: fix error caused by request irq suggested by CK
---
 drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci-mtk.h |  14 ++
 2 files changed, 294 insertions(+), 5 deletions(-)
 mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
old mode 100755
new mode 100644
index 34bd3de090b1..c07d54acbcb7
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -14,6 +14,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
 	SSUSB_UWK_V3,
 };
 
+int xhci_mtk_runtime_ready;
+
+static int xhci_mtk_runtime_resume(struct device *dev);
+
+static void xhci_mtk_seal_work(struct work_struct *work)
+{
+	struct xhci_hcd_mtk *mtk =
+			container_of(work, struct xhci_hcd_mtk, seal.work);
+	struct usb_hcd *hcd = mtk->hcd;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
+	if (mtk->seal_status == SEAL_SUSPENDED) {
+		mtk->seal_status = SEAL_RESUMING;
+		pm_runtime_mark_last_busy(mtk->dev);
+		pm_runtime_put_sync_autosuspend(mtk->dev);
+	} else {
+		/*
+		 * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
+		 * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
+		 * and to survey why not to invoke runtime_resume callback named
+		 * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
+		 * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
+		 * on as soon as unseal event received.
+		 */
+		if (mtk->seal_status == SEAL_RESUMING)
+			xhci_mtk_runtime_resume(mtk->dev);
+		else
+			xhci_warn(xhci,
+				"Ignore seal wakeup source disordered in xHCI controller\n");
+	}
+}
+
+static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
+{
+	struct xhci_hcd_mtk *mtk = data;
+	struct usb_hcd *hcd = mtk->hcd;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+	xhci_dbg(xhci, "seal irq ISR invoked\n");
+
+	schedule_delayed_work(&mtk->seal, 0);
+
+	return IRQ_HANDLED;
+}
+
+static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
+{
+	struct irq_desc *desc;
+	struct device *dev = mtk->dev;
+
+	if (mtk && mtk->seal_irq) {
+		desc = irq_to_desc(mtk->seal_irq);
+		if (enable) {
+			desc->irq_data.chip->irq_ack(&desc->irq_data);
+			enable_irq(mtk->seal_irq);
+			dev_dbg(dev, "%s: enable_irq %i\n",
+				 __func__, mtk->seal_irq);
+		} else {
+			disable_irq(mtk->seal_irq);
+			dev_dbg(dev, "%s: disable_irq %i\n",
+				 __func__, mtk->seal_irq);
+		}
+	}
+}
+
 static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
 {
 	struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
@@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
 			mtk->uwk_reg_base, mtk->uwk_vers);
 
 	return PTR_ERR_OR_ZERO(mtk->uwk);
-
 }
 
 static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
@@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	pm_runtime_set_active(dev);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_autosuspend_delay(dev,
+				CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
 	pm_runtime_enable(dev);
-	pm_runtime_get_sync(dev);
-	device_enable_async_suspend(dev);
 
 	ret = xhci_mtk_ldos_enable(mtk);
 	if (ret)
@@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		ret = irq;
 		goto disable_clk;
 	}
+	dev_dbg(dev, "irq %i\n", irq);
+
+	mtk->seal_irq = platform_get_irq_optional(pdev, 1);
+	if (mtk->seal_irq < 0) {
+		ret = mtk->seal_irq;
+		goto disable_clk;
+	}
+	dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
 
 	hcd = usb_create_hcd(driver, dev, dev_name(dev));
 	if (!hcd) {
@@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	if (ret)
 		goto dealloc_usb2_hcd;
 
+	INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
+	snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
+		 hcd->driver->description, hcd->self.busnum);
+	ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
+			  IRQF_TRIGGER_FALLING,	mtk->seal_descr, mtk);
+	if (ret != 0) {
+		dev_err(dev, "seal request interrupt %d failed\n",
+			mtk->seal_irq);
+		goto dealloc_usb2_hcd;
+	}
+	xhci_mtk_seal_wakeup_enable(mtk, false);
+
+	device_enable_async_suspend(dev);
+	xhci_mtk_runtime_ready = 1;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
+		 __func__, xhci_mtk_runtime_ready);
+
 	return 0;
 
 dealloc_usb2_hcd:
@@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	xhci_mtk_ldos_disable(mtk);
 
 disable_pm:
-	pm_runtime_put_sync(dev);
+	pm_runtime_put_sync_autosuspend(dev);
 	pm_runtime_disable(dev);
 	return ret;
 }
@@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
 	pm_runtime_put_noidle(&dev->dev);
 	pm_runtime_disable(&dev->dev);
 
+	xhci_mtk_runtime_ready = 0;
 	usb_remove_hcd(shared_hcd);
 	xhci->shared_hcd = NULL;
 	device_init_wakeup(&dev->dev, false);
@@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
 	xhci_mtk_host_disable(mtk);
 	xhci_mtk_clks_disable(mtk);
 	usb_wakeup_set(mtk, true);
+
 	return 0;
 }
 
@@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
+{
+	struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+	struct xhci_hub *usb2_rhub;
+	struct xhci_hub *usb3_rhub;
+	struct xhci_bus_state *bus_state;
+	struct xhci_port *port;
+	u32	usb2_suspended_ports = -1;
+	u32	usb3_suspended_ports = -1;
+	u16 status;
+	int num_ports;
+	int ret = 0;
+	int i;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	usb2_rhub = &xhci->usb2_rhub;
+	if (usb2_rhub) {
+		bus_state  = &usb2_rhub->bus_state;
+		num_ports  = usb2_rhub->num_ports;
+		usb2_suspended_ports  = bus_state->suspended_ports;
+		usb2_suspended_ports ^= (BIT(num_ports) - 1);
+		usb2_suspended_ports &= (BIT(num_ports) - 1);
+		for (i = 0; i < num_ports; i++) {
+			if (usb2_suspended_ports & (1UL << i)) {
+				port = usb2_rhub->ports[i];
+				status = readl(port->addr);
+
+				xhci_dbg(xhci,
+					  "USB20: portsc[%i]: 0x%04X\n",
+					  i, status);
+
+				if (!(status & PORT_CONNECT))
+					usb2_suspended_ports &= ~(1UL << i);
+			}
+		}
+
+		if (usb2_suspended_ports) {
+			ret = -EBUSY;
+			goto ebusy;
+		}
+	}
+
+	usb3_rhub = &xhci->usb3_rhub;
+	if (usb3_rhub) {
+		bus_state  = &usb3_rhub->bus_state;
+		num_ports  = usb3_rhub->num_ports;
+		usb3_suspended_ports  = bus_state->suspended_ports;
+		usb3_suspended_ports ^= (BIT(num_ports) - 1);
+		usb3_suspended_ports &= (BIT(num_ports) - 1);
+		for (i = 0; i < num_ports; i++) {
+			if (usb3_suspended_ports & BIT(i)) {
+				port = usb3_rhub->ports[i];
+				status = readl(port->addr);
+
+				xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
+					  i, status);
+
+				if (!(status & PORT_CONNECT))
+					usb3_suspended_ports &= ~BIT(i);
+			}
+		}
+
+		if (usb3_suspended_ports) {
+			ret = -EBUSY;
+			goto ebusy;
+		}
+	}
+
+ebusy:
+	xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
+		  __func__, usb2_suspended_ports,
+		  usb3_suspended_ports, ret);
+
+	return ret;
+}
+
+static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
+{
+	bool wakeup = device_may_wakeup(dev);
+	struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+	int ret = 0;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	mtk->seal_status = SEAL_BUSY;
+	ret = xhci_mtk_bus_status(dev);
+	if (wakeup && !ret) {
+		mtk->seal_status = SEAL_SUSPENDING;
+		xhci_mtk_suspend(dev);
+		xhci_mtk_seal_wakeup_enable(mtk, true);
+		mtk->seal_status = SEAL_SUSPENDED;
+		xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
+	}
+
+	xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
+		  __func__, wakeup, ret);
+
+	return ret;
+}
+
+static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
+{
+	bool wakeup = device_may_wakeup(dev);
+	struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
+	struct usb_hcd *hcd;
+	struct xhci_hcd *xhci;
+
+	if (!mtk->hcd)
+		return -ESHUTDOWN;
+
+	hcd = mtk->hcd;
+	xhci = hcd_to_xhci(hcd);
+	if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
+			(xhci->xhc_state & XHCI_STATE_HALTED)) {
+		return -ESHUTDOWN;
+	}
+
+	/*
+	 *  list cases by one extra interrupt named seal to process!!!
+	 *  Who to process these module reinitilization after SPM wakeup
+	 *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
+	 *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
+	 *  case 3: usb client driver can invoke it in runtime mechanism
+	 *  case 4: user active
+	 */
+	if (wakeup) {
+		xhci_mtk_seal_wakeup_enable(mtk, false);
+		xhci_mtk_resume(dev);
+		xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
+	}
+	mtk->seal_status = SEAL_RESUMED;
+
+	xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
+
+	return 0;
+}
+
+static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
+{
+	int ret = 0;
+
+	dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
+		 __func__, xhci_mtk_runtime_ready);
+
+	if (!xhci_mtk_runtime_ready)
+		ret = -EAGAIN;
+
+	return ret;
+}
+
 static const struct dev_pm_ops xhci_mtk_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
+	SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
+			   xhci_mtk_runtime_resume,
+			   xhci_mtk_runtime_idle)
 };
-#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
+
+#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
 
 #ifdef CONFIG_OF
 static const struct of_device_id mtk_xhci_of_match[] = {
@@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
 
 static int __init xhci_mtk_init(void)
 {
+	xhci_mtk_runtime_ready = 0;
 	xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
 	return platform_driver_register(&mtk_xhci_driver);
 }
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 323b281933b9..103d83ce6a3e 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
 	__le32 reserved3[33]; /* 0x80 ~ 0xff */
 };
 
+enum xhci_mtk_seal {
+	SEAL_BUSY = 0,
+	SEAL_SUSPENDING,
+	SEAL_SUSPENDED,
+	SEAL_RESUMING,
+	SEAL_RESUMED
+};
+
 struct xhci_hcd_mtk {
 	struct device *dev;
 	struct usb_hcd *hcd;
@@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
 	struct regmap *uwk;
 	u32 uwk_reg_base;
 	u32 uwk_vers;
+
+	/* usb eint wakeup source */
+	int seal_irq;
+	enum xhci_mtk_seal seal_status;
+	struct delayed_work  seal;
+	char   seal_descr[32];	/* "seal" + driver + bus # */
 };
 
 static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* [RFC PATCH v3 5/5] arm64: dts: mt8192: add SSUSB related nodes
  2020-12-22  9:34 ` Chunfeng Yun
@ 2020-12-22  9:34   ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Chunfeng Yun, Tianping Fang, Zhanyong Wang, linux-mediatek,
	linux-usb, linux-kernel, Zhanyong Wang

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

Add SSUSB related nodes for mt8192

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: rename node as 'usb' instead of xhci

v2: include phy.h file

Depends on:
https://patchwork.kernel.org/patch/11713559/
[v4,1/3] arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 49 ++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 69d45c7b31f1..82d9f6eee404 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/phy/phy.h>
 
 / {
 	compatible = "mediatek,mt8192";
@@ -416,6 +417,54 @@
 			status = "disabled";
 		};
 
+		xhci: usb@11200000 {
+			compatible = "mediatek,mt8192-xhci",
+				     "mediatek,mtk-xhci";
+			reg = <0 0x11200000 0 0x1000>,
+			      <0 0x11203e00 0 0x0100>;
+			reg-names = "mac", "ippc";
+			interrupts-extended = <&gic GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH 0>,
+					      <&pio 211 IRQ_TYPE_LEVEL_LOW>;
+			phys = <&u2port0 PHY_TYPE_USB2>,
+			       <&u3port0 PHY_TYPE_USB3>;
+			assigned-clocks = <&topckgen CLK_TOP_USB_TOP_SEL>,
+					  <&topckgen CLK_TOP_SSUSB_XHCI_SEL>;
+			assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D5_D4>,
+						 <&topckgen CLK_TOP_UNIVPLL_D5_D4>;
+			clocks = <&infracfg CLK_INFRA_SSUSB>,
+				 <&infracfg CLK_INFRA_SSUSB_XHCI>,
+				 <&apmixedsys CLK_APMIXED_USBPLL>;
+			clock-names = "sys_ck", "xhci_ck", "ref_ck";
+			mediatek,syscon-wakeup = <&pericfg 0x420 3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+		};
+
+		u3phy0: usb-phy@11e40000 {
+			compatible = "mediatek,mt8192-tphy",
+				     "mediatek,generic-tphy-v2";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			status = "okay";
+
+			u2port0: usb-phy@11e40000 {
+				reg = <0 0x11e40000 0 0x700>;
+				clocks = <&clk26m>;
+				clock-names = "ref";
+				#phy-cells = <1>;
+				status = "okay";
+			};
+
+			u3port0: usb-phy@11e40700 {
+				reg = <0 0x11e40700 0 0x900>;
+				clocks = <&clk26m>;
+				clock-names = "ref";
+				#phy-cells = <1>;
+				status = "okay";
+			};
+		};
+
 		audsys: syscon@11210000 {
 			compatible = "mediatek,mt8192-audsys", "syscon";
 			reg = <0 0x11210000 0 0x1000>;
-- 
2.18.0


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

* [RFC PATCH v3 5/5] arm64: dts: mt8192: add SSUSB related nodes
@ 2020-12-22  9:34   ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2020-12-22  9:34 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Zhanyong Wang, Zhanyong Wang, linux-usb, linux-kernel,
	Tianping Fang, linux-mediatek, Chunfeng Yun

From: Zhanyong Wang <zhanyong.wang@mediatek.com>

Add SSUSB related nodes for mt8192

Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
v3: rename node as 'usb' instead of xhci

v2: include phy.h file

Depends on:
https://patchwork.kernel.org/patch/11713559/
[v4,1/3] arm64: dts: Add Mediatek SoC MT8192 and evaluation board dts and Makefile
---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 49 ++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 69d45c7b31f1..82d9f6eee404 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -9,6 +9,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/phy/phy.h>
 
 / {
 	compatible = "mediatek,mt8192";
@@ -416,6 +417,54 @@
 			status = "disabled";
 		};
 
+		xhci: usb@11200000 {
+			compatible = "mediatek,mt8192-xhci",
+				     "mediatek,mtk-xhci";
+			reg = <0 0x11200000 0 0x1000>,
+			      <0 0x11203e00 0 0x0100>;
+			reg-names = "mac", "ippc";
+			interrupts-extended = <&gic GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH 0>,
+					      <&pio 211 IRQ_TYPE_LEVEL_LOW>;
+			phys = <&u2port0 PHY_TYPE_USB2>,
+			       <&u3port0 PHY_TYPE_USB3>;
+			assigned-clocks = <&topckgen CLK_TOP_USB_TOP_SEL>,
+					  <&topckgen CLK_TOP_SSUSB_XHCI_SEL>;
+			assigned-clock-parents = <&topckgen CLK_TOP_UNIVPLL_D5_D4>,
+						 <&topckgen CLK_TOP_UNIVPLL_D5_D4>;
+			clocks = <&infracfg CLK_INFRA_SSUSB>,
+				 <&infracfg CLK_INFRA_SSUSB_XHCI>,
+				 <&apmixedsys CLK_APMIXED_USBPLL>;
+			clock-names = "sys_ck", "xhci_ck", "ref_ck";
+			mediatek,syscon-wakeup = <&pericfg 0x420 3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+		};
+
+		u3phy0: usb-phy@11e40000 {
+			compatible = "mediatek,mt8192-tphy",
+				     "mediatek,generic-tphy-v2";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			status = "okay";
+
+			u2port0: usb-phy@11e40000 {
+				reg = <0 0x11e40000 0 0x700>;
+				clocks = <&clk26m>;
+				clock-names = "ref";
+				#phy-cells = <1>;
+				status = "okay";
+			};
+
+			u3port0: usb-phy@11e40700 {
+				reg = <0 0x11e40700 0 0x900>;
+				clocks = <&clk26m>;
+				clock-names = "ref";
+				#phy-cells = <1>;
+				status = "okay";
+			};
+		};
+
 		audsys: syscon@11210000 {
 			compatible = "mediatek,mt8192-audsys", "syscon";
 			reg = <0 0x11210000 0 0x1000>;
-- 
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
  2020-12-22  9:34   ` Chunfeng Yun
@ 2020-12-29  7:38     ` Ikjoon Jang
  -1 siblings, 0 replies; 19+ messages in thread
From: Ikjoon Jang @ 2020-12-29  7:38 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Tianping Fang, Zhanyong Wang,
	moderated list:ARM/Mediatek SoC support, linux-usb, open list,
	CK Hu, Zhanyong Wang

On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> From: CK Hu <ck.hu@mediatek.com>
>
> add support runtime pm feature
>
> Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v3:
>   1. fix some issues
>   2. remove attribute files
>
> v2: fix error caused by request irq suggested by CK
> ---
>  drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/xhci-mtk.h |  14 ++
>  2 files changed, 294 insertions(+), 5 deletions(-)
>  mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> old mode 100755
> new mode 100644
> index 34bd3de090b1..c07d54acbcb7
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -14,6 +14,7 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> @@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
>         SSUSB_UWK_V3,
>  };
>
> +int xhci_mtk_runtime_ready;
> +
> +static int xhci_mtk_runtime_resume(struct device *dev);
> +
> +static void xhci_mtk_seal_work(struct work_struct *work)
> +{
> +       struct xhci_hcd_mtk *mtk =
> +                       container_of(work, struct xhci_hcd_mtk, seal.work);
> +       struct usb_hcd *hcd = mtk->hcd;
> +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +
> +       xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
> +       if (mtk->seal_status == SEAL_SUSPENDED) {
> +               mtk->seal_status = SEAL_RESUMING;
> +               pm_runtime_mark_last_busy(mtk->dev);
> +               pm_runtime_put_sync_autosuspend(mtk->dev);

If I understand correctly, this function is for waking up the device
but this function calls put() only without get().

> +       } else {
> +               /*
> +                * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
> +                * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
> +                * and to survey why not to invoke runtime_resume callback named
> +                * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
> +                * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
> +                * on as soon as unseal event received.

I guess actual resuming should be happened only from the 1st interrupt
(when in SEAL_SUSPENDED)
and following spurious interrupts can be just ignored.

> +                */
> +               if (mtk->seal_status == SEAL_RESUMING)
> +                       xhci_mtk_runtime_resume(mtk->dev);

xhci_mtk_runtime_resume() is defined as a runtime pm callback,
pm core will call that callback when pm usage counter reaches to zero.

> +               else
> +                       xhci_warn(xhci,
> +                               "Ignore seal wakeup source disordered in xHCI controller\n");
> +       }
> +}
> +
> +static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
> +{
> +       struct xhci_hcd_mtk *mtk = data;
> +       struct usb_hcd *hcd = mtk->hcd;
> +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +
> +       xhci_dbg(xhci, "seal irq ISR invoked\n");
> +
> +       schedule_delayed_work(&mtk->seal, 0);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
> +{
> +       struct irq_desc *desc;
> +       struct device *dev = mtk->dev;
> +
> +       if (mtk && mtk->seal_irq) {
> +               desc = irq_to_desc(mtk->seal_irq);
> +               if (enable) {
> +                       desc->irq_data.chip->irq_ack(&desc->irq_data);
> +                       enable_irq(mtk->seal_irq);
> +                       dev_dbg(dev, "%s: enable_irq %i\n",
> +                                __func__, mtk->seal_irq);
> +               } else {
> +                       disable_irq(mtk->seal_irq);
> +                       dev_dbg(dev, "%s: disable_irq %i\n",
> +                                __func__, mtk->seal_irq);
> +               }
> +       }
> +}
> +

I think this is unnecessary if this driver can check the current state
and ignore the spurious irqs if spm sometimes triggers the wake-up irqs.

>  static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
>  {
>         struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
> @@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
>                         mtk->uwk_reg_base, mtk->uwk_vers);
>
>         return PTR_ERR_OR_ZERO(mtk->uwk);
> -
>  }
>
>  static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
> @@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       pm_runtime_set_active(dev);
> +       pm_runtime_use_autosuspend(dev);
> +       pm_runtime_set_autosuspend_delay(dev,
> +                               CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
>         pm_runtime_enable(dev);
> -       pm_runtime_get_sync(dev);

The only one left pm_runtime_get() is removed by here,
now this driver only calls pm_runtime_put().

> -       device_enable_async_suspend(dev);
>
>         ret = xhci_mtk_ldos_enable(mtk);
>         if (ret)
> @@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 ret = irq;
>                 goto disable_clk;
>         }
> +       dev_dbg(dev, "irq %i\n", irq);
> +
> +       mtk->seal_irq = platform_get_irq_optional(pdev, 1);
> +       if (mtk->seal_irq < 0) {
> +               ret = mtk->seal_irq;
> +               goto disable_clk;
> +       }
> +       dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
>
>         hcd = usb_create_hcd(driver, dev, dev_name(dev));
>         if (!hcd) {
> @@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto dealloc_usb2_hcd;
>
> +       INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
> +       snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
> +                hcd->driver->description, hcd->self.busnum);
> +       ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
> +                         IRQF_TRIGGER_FALLING, mtk->seal_descr, mtk);
> +       if (ret != 0) {
> +               dev_err(dev, "seal request interrupt %d failed\n",
> +                       mtk->seal_irq);
> +               goto dealloc_usb2_hcd;
> +       }
> +       xhci_mtk_seal_wakeup_enable(mtk, false);
> +
> +       device_enable_async_suspend(dev);
> +       xhci_mtk_runtime_ready = 1;
> +
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);

I expect the usage count will be -1 by here in probe.

> +
> +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> +                __func__, xhci_mtk_runtime_ready);
> +
>         return 0;
>
>  dealloc_usb2_hcd:
> @@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         xhci_mtk_ldos_disable(mtk);
>
>  disable_pm:
> -       pm_runtime_put_sync(dev);
> +       pm_runtime_put_sync_autosuspend(dev);
>         pm_runtime_disable(dev);
>         return ret;
>  }
> @@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
>         pm_runtime_put_noidle(&dev->dev);
>         pm_runtime_disable(&dev->dev);
>
> +       xhci_mtk_runtime_ready = 0;
>         usb_remove_hcd(shared_hcd);
>         xhci->shared_hcd = NULL;
>         device_init_wakeup(&dev->dev, false);
> @@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
>         xhci_mtk_host_disable(mtk);
>         xhci_mtk_clks_disable(mtk);
>         usb_wakeup_set(mtk, true);
> +
>         return 0;
>  }
>
> @@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
>         return 0;
>  }
>
> +static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
> +{
> +       struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +       struct xhci_hub *usb2_rhub;
> +       struct xhci_hub *usb3_rhub;
> +       struct xhci_bus_state *bus_state;
> +       struct xhci_port *port;
> +       u32     usb2_suspended_ports = -1;
> +       u32     usb3_suspended_ports = -1;
> +       u16 status;
> +       int num_ports;
> +       int ret = 0;
> +       int i;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }

This is duplicated from xhci_mtk_runtime_suspend()

> +
> +       usb2_rhub = &xhci->usb2_rhub;
> +       if (usb2_rhub) {
> +               bus_state  = &usb2_rhub->bus_state;
> +               num_ports  = usb2_rhub->num_ports;
> +               usb2_suspended_ports  = bus_state->suspended_ports;
> +               usb2_suspended_ports ^= (BIT(num_ports) - 1);
> +               usb2_suspended_ports &= (BIT(num_ports) - 1);
> +               for (i = 0; i < num_ports; i++) {
> +                       if (usb2_suspended_ports & (1UL << i)) {
> +                               port = usb2_rhub->ports[i];
> +                               status = readl(port->addr);
> +
> +                               xhci_dbg(xhci,
> +                                         "USB20: portsc[%i]: 0x%04X\n",
> +                                         i, status);
> +
> +                               if (!(status & PORT_CONNECT))
> +                                       usb2_suspended_ports &= ~(1UL << i);
> +                       }
> +               }
> +
> +               if (usb2_suspended_ports) {
> +                       ret = -EBUSY;
> +                       goto ebusy;
> +               }
> +       }
> +
> +       usb3_rhub = &xhci->usb3_rhub;
> +       if (usb3_rhub) {
> +               bus_state  = &usb3_rhub->bus_state;
> +               num_ports  = usb3_rhub->num_ports;
> +               usb3_suspended_ports  = bus_state->suspended_ports;
> +               usb3_suspended_ports ^= (BIT(num_ports) - 1);
> +               usb3_suspended_ports &= (BIT(num_ports) - 1);
> +               for (i = 0; i < num_ports; i++) {
> +                       if (usb3_suspended_ports & BIT(i)) {
> +                               port = usb3_rhub->ports[i];
> +                               status = readl(port->addr);
> +
> +                               xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
> +                                         i, status);
> +
> +                               if (!(status & PORT_CONNECT))
> +                                       usb3_suspended_ports &= ~BIT(i);
> +                       }
> +               }
> +
> +               if (usb3_suspended_ports) {
> +                       ret = -EBUSY;
> +                       goto ebusy;
> +               }
> +       }
> +
> +ebusy:
> +       xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
> +                 __func__, usb2_suspended_ports,
> +                 usb3_suspended_ports, ret);
> +
> +       return ret;
> +}
> +

This is basically counting active ports by directly reading portsc register?
I expect this function never return -EBUSY  if you balance pm usage
counter well.
Are there any specific reasons of doing this manually?

> +static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
> +{
> +       bool wakeup = device_may_wakeup(dev);
> +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +       int ret = 0;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }
> +
> +       mtk->seal_status = SEAL_BUSY;
> +       ret = xhci_mtk_bus_status(dev);
> +       if (wakeup && !ret) {
> +               mtk->seal_status = SEAL_SUSPENDING;
> +               xhci_mtk_suspend(dev);
> +               xhci_mtk_seal_wakeup_enable(mtk, true);
> +               mtk->seal_status = SEAL_SUSPENDED;
> +               xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
> +       }
> +
> +       xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
> +                 __func__, wakeup, ret);
> +
> +       return ret;
> +}
> +
> +static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
> +{
> +       bool wakeup = device_may_wakeup(dev);
> +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }
> +
> +       /*
> +        *  list cases by one extra interrupt named seal to process!!!
> +        *  Who to process these module reinitilization after SPM wakeup
> +        *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
> +        *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
> +        *  case 3: usb client driver can invoke it in runtime mechanism
> +        *  case 4: user active
> +        */
> +       if (wakeup) {
> +               xhci_mtk_seal_wakeup_enable(mtk, false);
> +               xhci_mtk_resume(dev);
> +               xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
> +       }
> +       mtk->seal_status = SEAL_RESUMED;
> +
> +       xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
> +{
> +       int ret = 0;
> +
> +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> +                __func__, xhci_mtk_runtime_ready);
> +
> +       if (!xhci_mtk_runtime_ready)
> +               ret = -EAGAIN;
> +
> +       return ret;
> +}
> +
>  static const struct dev_pm_ops xhci_mtk_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
> +       SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
> +                          xhci_mtk_runtime_resume,
> +                          xhci_mtk_runtime_idle)
>  };
> -#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
> +
> +#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
>
>  #ifdef CONFIG_OF
>  static const struct of_device_id mtk_xhci_of_match[] = {
> @@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
>
>  static int __init xhci_mtk_init(void)
>  {
> +       xhci_mtk_runtime_ready = 0;
>         xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
>         return platform_driver_register(&mtk_xhci_driver);
>  }
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index 323b281933b9..103d83ce6a3e 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
>         __le32 reserved3[33]; /* 0x80 ~ 0xff */
>  };
>
> +enum xhci_mtk_seal {
> +       SEAL_BUSY = 0,
> +       SEAL_SUSPENDING,
> +       SEAL_SUSPENDED,
> +       SEAL_RESUMING,
> +       SEAL_RESUMED
> +};
> +
>  struct xhci_hcd_mtk {
>         struct device *dev;
>         struct usb_hcd *hcd;
> @@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
>         struct regmap *uwk;
>         u32 uwk_reg_base;
>         u32 uwk_vers;
> +
> +       /* usb eint wakeup source */
> +       int seal_irq;
> +       enum xhci_mtk_seal seal_status;
> +       struct delayed_work  seal;
> +       char   seal_descr[32];  /* "seal" + driver + bus # */
>  };
>
>  static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
> --
> 2.18.0
>

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

* Re: [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
@ 2020-12-29  7:38     ` Ikjoon Jang
  0 siblings, 0 replies; 19+ messages in thread
From: Ikjoon Jang @ 2020-12-29  7:38 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support, CK Hu

On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> From: CK Hu <ck.hu@mediatek.com>
>
> add support runtime pm feature
>
> Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v3:
>   1. fix some issues
>   2. remove attribute files
>
> v2: fix error caused by request irq suggested by CK
> ---
>  drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/xhci-mtk.h |  14 ++
>  2 files changed, 294 insertions(+), 5 deletions(-)
>  mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> old mode 100755
> new mode 100644
> index 34bd3de090b1..c07d54acbcb7
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -14,6 +14,7 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> @@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
>         SSUSB_UWK_V3,
>  };
>
> +int xhci_mtk_runtime_ready;
> +
> +static int xhci_mtk_runtime_resume(struct device *dev);
> +
> +static void xhci_mtk_seal_work(struct work_struct *work)
> +{
> +       struct xhci_hcd_mtk *mtk =
> +                       container_of(work, struct xhci_hcd_mtk, seal.work);
> +       struct usb_hcd *hcd = mtk->hcd;
> +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +
> +       xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
> +       if (mtk->seal_status == SEAL_SUSPENDED) {
> +               mtk->seal_status = SEAL_RESUMING;
> +               pm_runtime_mark_last_busy(mtk->dev);
> +               pm_runtime_put_sync_autosuspend(mtk->dev);

If I understand correctly, this function is for waking up the device
but this function calls put() only without get().

> +       } else {
> +               /*
> +                * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
> +                * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
> +                * and to survey why not to invoke runtime_resume callback named
> +                * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
> +                * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
> +                * on as soon as unseal event received.

I guess actual resuming should be happened only from the 1st interrupt
(when in SEAL_SUSPENDED)
and following spurious interrupts can be just ignored.

> +                */
> +               if (mtk->seal_status == SEAL_RESUMING)
> +                       xhci_mtk_runtime_resume(mtk->dev);

xhci_mtk_runtime_resume() is defined as a runtime pm callback,
pm core will call that callback when pm usage counter reaches to zero.

> +               else
> +                       xhci_warn(xhci,
> +                               "Ignore seal wakeup source disordered in xHCI controller\n");
> +       }
> +}
> +
> +static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
> +{
> +       struct xhci_hcd_mtk *mtk = data;
> +       struct usb_hcd *hcd = mtk->hcd;
> +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +
> +       xhci_dbg(xhci, "seal irq ISR invoked\n");
> +
> +       schedule_delayed_work(&mtk->seal, 0);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
> +{
> +       struct irq_desc *desc;
> +       struct device *dev = mtk->dev;
> +
> +       if (mtk && mtk->seal_irq) {
> +               desc = irq_to_desc(mtk->seal_irq);
> +               if (enable) {
> +                       desc->irq_data.chip->irq_ack(&desc->irq_data);
> +                       enable_irq(mtk->seal_irq);
> +                       dev_dbg(dev, "%s: enable_irq %i\n",
> +                                __func__, mtk->seal_irq);
> +               } else {
> +                       disable_irq(mtk->seal_irq);
> +                       dev_dbg(dev, "%s: disable_irq %i\n",
> +                                __func__, mtk->seal_irq);
> +               }
> +       }
> +}
> +

I think this is unnecessary if this driver can check the current state
and ignore the spurious irqs if spm sometimes triggers the wake-up irqs.

>  static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
>  {
>         struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
> @@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
>                         mtk->uwk_reg_base, mtk->uwk_vers);
>
>         return PTR_ERR_OR_ZERO(mtk->uwk);
> -
>  }
>
>  static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
> @@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       pm_runtime_set_active(dev);
> +       pm_runtime_use_autosuspend(dev);
> +       pm_runtime_set_autosuspend_delay(dev,
> +                               CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
>         pm_runtime_enable(dev);
> -       pm_runtime_get_sync(dev);

The only one left pm_runtime_get() is removed by here,
now this driver only calls pm_runtime_put().

> -       device_enable_async_suspend(dev);
>
>         ret = xhci_mtk_ldos_enable(mtk);
>         if (ret)
> @@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 ret = irq;
>                 goto disable_clk;
>         }
> +       dev_dbg(dev, "irq %i\n", irq);
> +
> +       mtk->seal_irq = platform_get_irq_optional(pdev, 1);
> +       if (mtk->seal_irq < 0) {
> +               ret = mtk->seal_irq;
> +               goto disable_clk;
> +       }
> +       dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
>
>         hcd = usb_create_hcd(driver, dev, dev_name(dev));
>         if (!hcd) {
> @@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         if (ret)
>                 goto dealloc_usb2_hcd;
>
> +       INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
> +       snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
> +                hcd->driver->description, hcd->self.busnum);
> +       ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
> +                         IRQF_TRIGGER_FALLING, mtk->seal_descr, mtk);
> +       if (ret != 0) {
> +               dev_err(dev, "seal request interrupt %d failed\n",
> +                       mtk->seal_irq);
> +               goto dealloc_usb2_hcd;
> +       }
> +       xhci_mtk_seal_wakeup_enable(mtk, false);
> +
> +       device_enable_async_suspend(dev);
> +       xhci_mtk_runtime_ready = 1;
> +
> +       pm_runtime_mark_last_busy(dev);
> +       pm_runtime_put_autosuspend(dev);

I expect the usage count will be -1 by here in probe.

> +
> +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> +                __func__, xhci_mtk_runtime_ready);
> +
>         return 0;
>
>  dealloc_usb2_hcd:
> @@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>         xhci_mtk_ldos_disable(mtk);
>
>  disable_pm:
> -       pm_runtime_put_sync(dev);
> +       pm_runtime_put_sync_autosuspend(dev);
>         pm_runtime_disable(dev);
>         return ret;
>  }
> @@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
>         pm_runtime_put_noidle(&dev->dev);
>         pm_runtime_disable(&dev->dev);
>
> +       xhci_mtk_runtime_ready = 0;
>         usb_remove_hcd(shared_hcd);
>         xhci->shared_hcd = NULL;
>         device_init_wakeup(&dev->dev, false);
> @@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
>         xhci_mtk_host_disable(mtk);
>         xhci_mtk_clks_disable(mtk);
>         usb_wakeup_set(mtk, true);
> +
>         return 0;
>  }
>
> @@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
>         return 0;
>  }
>
> +static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
> +{
> +       struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +       struct xhci_hub *usb2_rhub;
> +       struct xhci_hub *usb3_rhub;
> +       struct xhci_bus_state *bus_state;
> +       struct xhci_port *port;
> +       u32     usb2_suspended_ports = -1;
> +       u32     usb3_suspended_ports = -1;
> +       u16 status;
> +       int num_ports;
> +       int ret = 0;
> +       int i;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }

This is duplicated from xhci_mtk_runtime_suspend()

> +
> +       usb2_rhub = &xhci->usb2_rhub;
> +       if (usb2_rhub) {
> +               bus_state  = &usb2_rhub->bus_state;
> +               num_ports  = usb2_rhub->num_ports;
> +               usb2_suspended_ports  = bus_state->suspended_ports;
> +               usb2_suspended_ports ^= (BIT(num_ports) - 1);
> +               usb2_suspended_ports &= (BIT(num_ports) - 1);
> +               for (i = 0; i < num_ports; i++) {
> +                       if (usb2_suspended_ports & (1UL << i)) {
> +                               port = usb2_rhub->ports[i];
> +                               status = readl(port->addr);
> +
> +                               xhci_dbg(xhci,
> +                                         "USB20: portsc[%i]: 0x%04X\n",
> +                                         i, status);
> +
> +                               if (!(status & PORT_CONNECT))
> +                                       usb2_suspended_ports &= ~(1UL << i);
> +                       }
> +               }
> +
> +               if (usb2_suspended_ports) {
> +                       ret = -EBUSY;
> +                       goto ebusy;
> +               }
> +       }
> +
> +       usb3_rhub = &xhci->usb3_rhub;
> +       if (usb3_rhub) {
> +               bus_state  = &usb3_rhub->bus_state;
> +               num_ports  = usb3_rhub->num_ports;
> +               usb3_suspended_ports  = bus_state->suspended_ports;
> +               usb3_suspended_ports ^= (BIT(num_ports) - 1);
> +               usb3_suspended_ports &= (BIT(num_ports) - 1);
> +               for (i = 0; i < num_ports; i++) {
> +                       if (usb3_suspended_ports & BIT(i)) {
> +                               port = usb3_rhub->ports[i];
> +                               status = readl(port->addr);
> +
> +                               xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
> +                                         i, status);
> +
> +                               if (!(status & PORT_CONNECT))
> +                                       usb3_suspended_ports &= ~BIT(i);
> +                       }
> +               }
> +
> +               if (usb3_suspended_ports) {
> +                       ret = -EBUSY;
> +                       goto ebusy;
> +               }
> +       }
> +
> +ebusy:
> +       xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
> +                 __func__, usb2_suspended_ports,
> +                 usb3_suspended_ports, ret);
> +
> +       return ret;
> +}
> +

This is basically counting active ports by directly reading portsc register?
I expect this function never return -EBUSY  if you balance pm usage
counter well.
Are there any specific reasons of doing this manually?

> +static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
> +{
> +       bool wakeup = device_may_wakeup(dev);
> +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +       int ret = 0;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }
> +
> +       mtk->seal_status = SEAL_BUSY;
> +       ret = xhci_mtk_bus_status(dev);
> +       if (wakeup && !ret) {
> +               mtk->seal_status = SEAL_SUSPENDING;
> +               xhci_mtk_suspend(dev);
> +               xhci_mtk_seal_wakeup_enable(mtk, true);
> +               mtk->seal_status = SEAL_SUSPENDED;
> +               xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
> +       }
> +
> +       xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
> +                 __func__, wakeup, ret);
> +
> +       return ret;
> +}
> +
> +static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
> +{
> +       bool wakeup = device_may_wakeup(dev);
> +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> +       struct usb_hcd *hcd;
> +       struct xhci_hcd *xhci;
> +
> +       if (!mtk->hcd)
> +               return -ESHUTDOWN;
> +
> +       hcd = mtk->hcd;
> +       xhci = hcd_to_xhci(hcd);
> +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> +               return -ESHUTDOWN;
> +       }
> +
> +       /*
> +        *  list cases by one extra interrupt named seal to process!!!
> +        *  Who to process these module reinitilization after SPM wakeup
> +        *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
> +        *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
> +        *  case 3: usb client driver can invoke it in runtime mechanism
> +        *  case 4: user active
> +        */
> +       if (wakeup) {
> +               xhci_mtk_seal_wakeup_enable(mtk, false);
> +               xhci_mtk_resume(dev);
> +               xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
> +       }
> +       mtk->seal_status = SEAL_RESUMED;
> +
> +       xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
> +{
> +       int ret = 0;
> +
> +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> +                __func__, xhci_mtk_runtime_ready);
> +
> +       if (!xhci_mtk_runtime_ready)
> +               ret = -EAGAIN;
> +
> +       return ret;
> +}
> +
>  static const struct dev_pm_ops xhci_mtk_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
> +       SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
> +                          xhci_mtk_runtime_resume,
> +                          xhci_mtk_runtime_idle)
>  };
> -#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
> +
> +#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
>
>  #ifdef CONFIG_OF
>  static const struct of_device_id mtk_xhci_of_match[] = {
> @@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
>
>  static int __init xhci_mtk_init(void)
>  {
> +       xhci_mtk_runtime_ready = 0;
>         xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
>         return platform_driver_register(&mtk_xhci_driver);
>  }
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index 323b281933b9..103d83ce6a3e 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
>         __le32 reserved3[33]; /* 0x80 ~ 0xff */
>  };
>
> +enum xhci_mtk_seal {
> +       SEAL_BUSY = 0,
> +       SEAL_SUSPENDING,
> +       SEAL_SUSPENDED,
> +       SEAL_RESUMING,
> +       SEAL_RESUMED
> +};
> +
>  struct xhci_hcd_mtk {
>         struct device *dev;
>         struct usb_hcd *hcd;
> @@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
>         struct regmap *uwk;
>         u32 uwk_reg_base;
>         u32 uwk_vers;
> +
> +       /* usb eint wakeup source */
> +       int seal_irq;
> +       enum xhci_mtk_seal seal_status;
> +       struct delayed_work  seal;
> +       char   seal_descr[32];  /* "seal" + driver + bus # */
>  };
>
>  static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
> --
> 2.18.0
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
  2020-12-29  7:38     ` Ikjoon Jang
@ 2021-01-05  7:13       ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2021-01-05  7:13 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Tianping Fang, Zhanyong Wang,
	moderated list:ARM/Mediatek SoC support, linux-usb, open list,
	CK Hu, Zhanyong Wang

Hi Ikjoon,

   I try to rebuild the code flow, and want to apply the framework from
wakeirq.c

On Tue, 2020-12-29 at 15:38 +0800, Ikjoon Jang wrote:
> On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > From: CK Hu <ck.hu@mediatek.com>
> >
> > add support runtime pm feature
> >
> > Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v3:
> >   1. fix some issues
> >   2. remove attribute files
> >
> > v2: fix error caused by request irq suggested by CK
> > ---
> >  drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
> >  drivers/usb/host/xhci-mtk.h |  14 ++
> >  2 files changed, 294 insertions(+), 5 deletions(-)
> >  mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > old mode 100755
> > new mode 100644
> > index 34bd3de090b1..c07d54acbcb7
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/of_irq.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/regmap.h>
> > @@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
> >         SSUSB_UWK_V3,
> >  };
> >
> > +int xhci_mtk_runtime_ready;
> > +
> > +static int xhci_mtk_runtime_resume(struct device *dev);
> > +
> > +static void xhci_mtk_seal_work(struct work_struct *work)
> > +{
> > +       struct xhci_hcd_mtk *mtk =
> > +                       container_of(work, struct xhci_hcd_mtk, seal.work);
> > +       struct usb_hcd *hcd = mtk->hcd;
> > +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > +
> > +       xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
> > +       if (mtk->seal_status == SEAL_SUSPENDED) {
> > +               mtk->seal_status = SEAL_RESUMING;
> > +               pm_runtime_mark_last_busy(mtk->dev);
> > +               pm_runtime_put_sync_autosuspend(mtk->dev);
> 
> If I understand correctly, this function is for waking up the device
> but this function calls put() only without get().
will use pm_request_resume();

> 
> > +       } else {
> > +               /*
> > +                * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
> > +                * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
> > +                * and to survey why not to invoke runtime_resume callback named
> > +                * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
> > +                * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
> > +                * on as soon as unseal event received.
> 
> I guess actual resuming should be happened only from the 1st interrupt
> (when in SEAL_SUSPENDED)
> and following spurious interrupts can be just ignored.
Yes
> 
> > +                */
> > +               if (mtk->seal_status == SEAL_RESUMING)
> > +                       xhci_mtk_runtime_resume(mtk->dev);
> 
> xhci_mtk_runtime_resume() is defined as a runtime pm callback,
> pm core will call that callback when pm usage counter reaches to zero.
Yes
> 
> > +               else
> > +                       xhci_warn(xhci,
> > +                               "Ignore seal wakeup source disordered in xHCI controller\n");
> > +       }
> > +}
> > +
> > +static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
> > +{
> > +       struct xhci_hcd_mtk *mtk = data;
> > +       struct usb_hcd *hcd = mtk->hcd;
> > +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > +
> > +       xhci_dbg(xhci, "seal irq ISR invoked\n");
> > +
> > +       schedule_delayed_work(&mtk->seal, 0);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
> > +{
> > +       struct irq_desc *desc;
> > +       struct device *dev = mtk->dev;
> > +
> > +       if (mtk && mtk->seal_irq) {
> > +               desc = irq_to_desc(mtk->seal_irq);
> > +               if (enable) {
> > +                       desc->irq_data.chip->irq_ack(&desc->irq_data);
> > +                       enable_irq(mtk->seal_irq);
> > +                       dev_dbg(dev, "%s: enable_irq %i\n",
> > +                                __func__, mtk->seal_irq);
> > +               } else {
> > +                       disable_irq(mtk->seal_irq);
> > +                       dev_dbg(dev, "%s: disable_irq %i\n",
> > +                                __func__, mtk->seal_irq);
> > +               }
> > +       }
> > +}
> > +
> 
> I think this is unnecessary if this driver can check the current state
> and ignore the spurious irqs if spm sometimes triggers the wake-up irqs.
It helps to reduce some irqs, there are many spurious interrupts, I
think there is something wrong from the point of HW, will try to find
the root cause.

> 
> >  static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
> >  {
> >         struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
> > @@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
> >                         mtk->uwk_reg_base, mtk->uwk_vers);
> >
> >         return PTR_ERR_OR_ZERO(mtk->uwk);
> > -
> >  }
> >
> >  static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
> > @@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >         }
> >
> > +       pm_runtime_set_active(dev);
> > +       pm_runtime_use_autosuspend(dev);
> > +       pm_runtime_set_autosuspend_delay(dev,
> > +                               CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
> >         pm_runtime_enable(dev);
> > -       pm_runtime_get_sync(dev);
> 
> The only one left pm_runtime_get() is removed by here,
> now this driver only calls pm_runtime_put().
will add it back, and forbid runtime by default
> 
> > -       device_enable_async_suspend(dev);
> >
> >         ret = xhci_mtk_ldos_enable(mtk);
> >         if (ret)
> > @@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 ret = irq;
> >                 goto disable_clk;
> >         }
> > +       dev_dbg(dev, "irq %i\n", irq);
> > +
> > +       mtk->seal_irq = platform_get_irq_optional(pdev, 1);
> > +       if (mtk->seal_irq < 0) {
> > +               ret = mtk->seal_irq;
> > +               goto disable_clk;
> > +       }
> > +       dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
> >
> >         hcd = usb_create_hcd(driver, dev, dev_name(dev));
> >         if (!hcd) {
> > @@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto dealloc_usb2_hcd;
> >
> > +       INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
> > +       snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
> > +                hcd->driver->description, hcd->self.busnum);
> > +       ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
> > +                         IRQF_TRIGGER_FALLING, mtk->seal_descr, mtk);
> > +       if (ret != 0) {
> > +               dev_err(dev, "seal request interrupt %d failed\n",
> > +                       mtk->seal_irq);
> > +               goto dealloc_usb2_hcd;
> > +       }
> > +       xhci_mtk_seal_wakeup_enable(mtk, false);
> > +
> > +       device_enable_async_suspend(dev);
> > +       xhci_mtk_runtime_ready = 1;
> > +
> > +       pm_runtime_mark_last_busy(dev);
> > +       pm_runtime_put_autosuspend(dev);
> 
> I expect the usage count will be -1 by here in probe.
will change it
> 
> > +
> > +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> > +                __func__, xhci_mtk_runtime_ready);
> > +
> >         return 0;
> >
> >  dealloc_usb2_hcd:
> > @@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         xhci_mtk_ldos_disable(mtk);
> >
> >  disable_pm:
> > -       pm_runtime_put_sync(dev);
> > +       pm_runtime_put_sync_autosuspend(dev);
> >         pm_runtime_disable(dev);
> >         return ret;
> >  }
> > @@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
> >         pm_runtime_put_noidle(&dev->dev);
> >         pm_runtime_disable(&dev->dev);
> >
> > +       xhci_mtk_runtime_ready = 0;
> >         usb_remove_hcd(shared_hcd);
> >         xhci->shared_hcd = NULL;
> >         device_init_wakeup(&dev->dev, false);
> > @@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
> >         xhci_mtk_host_disable(mtk);
> >         xhci_mtk_clks_disable(mtk);
> >         usb_wakeup_set(mtk, true);
> > +
> >         return 0;
> >  }
> >
> > @@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
> >         return 0;
> >  }
> >
> > +static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
> > +{
> > +       struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +       struct xhci_hub *usb2_rhub;
> > +       struct xhci_hub *usb3_rhub;
> > +       struct xhci_bus_state *bus_state;
> > +       struct xhci_port *port;
> > +       u32     usb2_suspended_ports = -1;
> > +       u32     usb3_suspended_ports = -1;
> > +       u16 status;
> > +       int num_ports;
> > +       int ret = 0;
> > +       int i;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> 
> This is duplicated from xhci_mtk_runtime_suspend()
Yes, will remove it and rebuild the code
> 
> > +
> > +       usb2_rhub = &xhci->usb2_rhub;
> > +       if (usb2_rhub) {
> > +               bus_state  = &usb2_rhub->bus_state;
> > +               num_ports  = usb2_rhub->num_ports;
> > +               usb2_suspended_ports  = bus_state->suspended_ports;
> > +               usb2_suspended_ports ^= (BIT(num_ports) - 1);
> > +               usb2_suspended_ports &= (BIT(num_ports) - 1);
> > +               for (i = 0; i < num_ports; i++) {
> > +                       if (usb2_suspended_ports & (1UL << i)) {
> > +                               port = usb2_rhub->ports[i];
> > +                               status = readl(port->addr);
> > +
> > +                               xhci_dbg(xhci,
> > +                                         "USB20: portsc[%i]: 0x%04X\n",
> > +                                         i, status);
> > +
> > +                               if (!(status & PORT_CONNECT))
> > +                                       usb2_suspended_ports &= ~(1UL << i);
> > +                       }
> > +               }
> > +
> > +               if (usb2_suspended_ports) {
> > +                       ret = -EBUSY;
> > +                       goto ebusy;
> > +               }
> > +       }
> > +
> > +       usb3_rhub = &xhci->usb3_rhub;
> > +       if (usb3_rhub) {
> > +               bus_state  = &usb3_rhub->bus_state;
> > +               num_ports  = usb3_rhub->num_ports;
> > +               usb3_suspended_ports  = bus_state->suspended_ports;
> > +               usb3_suspended_ports ^= (BIT(num_ports) - 1);
> > +               usb3_suspended_ports &= (BIT(num_ports) - 1);
> > +               for (i = 0; i < num_ports; i++) {
> > +                       if (usb3_suspended_ports & BIT(i)) {
> > +                               port = usb3_rhub->ports[i];
> > +                               status = readl(port->addr);
> > +
> > +                               xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
> > +                                         i, status);
> > +
> > +                               if (!(status & PORT_CONNECT))
> > +                                       usb3_suspended_ports &= ~BIT(i);
> > +                       }
> > +               }
> > +
> > +               if (usb3_suspended_ports) {
> > +                       ret = -EBUSY;
> > +                       goto ebusy;
> > +               }
> > +       }
> > +
> > +ebusy:
> > +       xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
> > +                 __func__, usb2_suspended_ports,
> > +                 usb3_suspended_ports, ret);
> > +
> > +       return ret;
> > +}
> > +
> 
> This is basically counting active ports by directly reading portsc register?
Yes, 
> I expect this function never return -EBUSY  if you balance pm usage
> counter well.
> Are there any specific reasons of doing this manually?
pm_runtime_set_autosuspend_delay() is used, may check the ports status
periodically. I will check it.

If no spurious wakeup irq, apply the framework from wakeirq.c is the
easiest way to support runtime-suspend. hope that find out the root
cause and improve the HW design. In fact the EINT way is a deprecated
feature and not used before.

Thanks a lot.

> 
> > +static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
> > +{
> > +       bool wakeup = device_may_wakeup(dev);
> > +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +       int ret = 0;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> > +
> > +       mtk->seal_status = SEAL_BUSY;
> > +       ret = xhci_mtk_bus_status(dev);
> > +       if (wakeup && !ret) {
> > +               mtk->seal_status = SEAL_SUSPENDING;
> > +               xhci_mtk_suspend(dev);
> > +               xhci_mtk_seal_wakeup_enable(mtk, true);
> > +               mtk->seal_status = SEAL_SUSPENDED;
> > +               xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
> > +       }
> > +
> > +       xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
> > +                 __func__, wakeup, ret);
> > +
> > +       return ret;
> > +}
> > +
> > +static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
> > +{
> > +       bool wakeup = device_may_wakeup(dev);
> > +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> > +
> > +       /*
> > +        *  list cases by one extra interrupt named seal to process!!!
> > +        *  Who to process these module reinitilization after SPM wakeup
> > +        *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
> > +        *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
> > +        *  case 3: usb client driver can invoke it in runtime mechanism
> > +        *  case 4: user active
> > +        */
> > +       if (wakeup) {
> > +               xhci_mtk_seal_wakeup_enable(mtk, false);
> > +               xhci_mtk_resume(dev);
> > +               xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
> > +       }
> > +       mtk->seal_status = SEAL_RESUMED;
> > +
> > +       xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
> > +
> > +       return 0;
> > +}
> > +
> > +static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
> > +{
> > +       int ret = 0;
> > +
> > +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> > +                __func__, xhci_mtk_runtime_ready);
> > +
> > +       if (!xhci_mtk_runtime_ready)
> > +               ret = -EAGAIN;
> > +
> > +       return ret;
> > +}
> > +
> >  static const struct dev_pm_ops xhci_mtk_pm_ops = {
> >         SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
> > +       SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
> > +                          xhci_mtk_runtime_resume,
> > +                          xhci_mtk_runtime_idle)
> >  };
> > -#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
> > +
> > +#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
> >
> >  #ifdef CONFIG_OF
> >  static const struct of_device_id mtk_xhci_of_match[] = {
> > @@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
> >
> >  static int __init xhci_mtk_init(void)
> >  {
> > +       xhci_mtk_runtime_ready = 0;
> >         xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
> >         return platform_driver_register(&mtk_xhci_driver);
> >  }
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index 323b281933b9..103d83ce6a3e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
> >         __le32 reserved3[33]; /* 0x80 ~ 0xff */
> >  };
> >
> > +enum xhci_mtk_seal {
> > +       SEAL_BUSY = 0,
> > +       SEAL_SUSPENDING,
> > +       SEAL_SUSPENDED,
> > +       SEAL_RESUMING,
> > +       SEAL_RESUMED
> > +};
> > +
> >  struct xhci_hcd_mtk {
> >         struct device *dev;
> >         struct usb_hcd *hcd;
> > @@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
> >         struct regmap *uwk;
> >         u32 uwk_reg_base;
> >         u32 uwk_vers;
> > +
> > +       /* usb eint wakeup source */
> > +       int seal_irq;
> > +       enum xhci_mtk_seal seal_status;
> > +       struct delayed_work  seal;
> > +       char   seal_descr[32];  /* "seal" + driver + bus # */
> >  };
> >
> >  static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
> > --
> > 2.18.0
> >


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

* Re: [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm
@ 2021-01-05  7:13       ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2021-01-05  7:13 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support, CK Hu

Hi Ikjoon,

   I try to rebuild the code flow, and want to apply the framework from
wakeirq.c

On Tue, 2020-12-29 at 15:38 +0800, Ikjoon Jang wrote:
> On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > From: CK Hu <ck.hu@mediatek.com>
> >
> > add support runtime pm feature
> >
> > Signed-off-by: Zhanyong Wang <zhanyong.wang@mediatek.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v3:
> >   1. fix some issues
> >   2. remove attribute files
> >
> > v2: fix error caused by request irq suggested by CK
> > ---
> >  drivers/usb/host/xhci-mtk.c | 285 +++++++++++++++++++++++++++++++++++-
> >  drivers/usb/host/xhci-mtk.h |  14 ++
> >  2 files changed, 294 insertions(+), 5 deletions(-)
> >  mode change 100755 => 100644 drivers/usb/host/xhci-mtk.c
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > old mode 100755
> > new mode 100644
> > index 34bd3de090b1..c07d54acbcb7
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> > +#include <linux/of_irq.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/regmap.h>
> > @@ -77,6 +78,72 @@ enum ssusb_uwk_vers {
> >         SSUSB_UWK_V3,
> >  };
> >
> > +int xhci_mtk_runtime_ready;
> > +
> > +static int xhci_mtk_runtime_resume(struct device *dev);
> > +
> > +static void xhci_mtk_seal_work(struct work_struct *work)
> > +{
> > +       struct xhci_hcd_mtk *mtk =
> > +                       container_of(work, struct xhci_hcd_mtk, seal.work);
> > +       struct usb_hcd *hcd = mtk->hcd;
> > +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > +
> > +       xhci_dbg(xhci, "spm unseals xHCI controller %i\n", mtk->seal_status);
> > +       if (mtk->seal_status == SEAL_SUSPENDED) {
> > +               mtk->seal_status = SEAL_RESUMING;
> > +               pm_runtime_mark_last_busy(mtk->dev);
> > +               pm_runtime_put_sync_autosuspend(mtk->dev);
> 
> If I understand correctly, this function is for waking up the device
> but this function calls put() only without get().
will use pm_request_resume();

> 
> > +       } else {
> > +               /*
> > +                * FIXME: Sometimes seal_status will keep it on SEAL_RESUMING staus not to expect
> > +                * since pm_runtime_put_sync_autosuspend doesn't invoke runtime_resume of callback.
> > +                * and to survey why not to invoke runtime_resume callback named
> > +                * xhci_mtk_runtime_resume in time in short feature, Herely provide one solution
> > +                * that make sure seal_status to match h/w state machine,and MTK xHCI clocks
> > +                * on as soon as unseal event received.
> 
> I guess actual resuming should be happened only from the 1st interrupt
> (when in SEAL_SUSPENDED)
> and following spurious interrupts can be just ignored.
Yes
> 
> > +                */
> > +               if (mtk->seal_status == SEAL_RESUMING)
> > +                       xhci_mtk_runtime_resume(mtk->dev);
> 
> xhci_mtk_runtime_resume() is defined as a runtime pm callback,
> pm core will call that callback when pm usage counter reaches to zero.
Yes
> 
> > +               else
> > +                       xhci_warn(xhci,
> > +                               "Ignore seal wakeup source disordered in xHCI controller\n");
> > +       }
> > +}
> > +
> > +static irqreturn_t xhci_mtk_seal_irq(int irq, void *data)
> > +{
> > +       struct xhci_hcd_mtk *mtk = data;
> > +       struct usb_hcd *hcd = mtk->hcd;
> > +       struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> > +
> > +       xhci_dbg(xhci, "seal irq ISR invoked\n");
> > +
> > +       schedule_delayed_work(&mtk->seal, 0);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +static void xhci_mtk_seal_wakeup_enable(struct xhci_hcd_mtk *mtk, bool enable)
> > +{
> > +       struct irq_desc *desc;
> > +       struct device *dev = mtk->dev;
> > +
> > +       if (mtk && mtk->seal_irq) {
> > +               desc = irq_to_desc(mtk->seal_irq);
> > +               if (enable) {
> > +                       desc->irq_data.chip->irq_ack(&desc->irq_data);
> > +                       enable_irq(mtk->seal_irq);
> > +                       dev_dbg(dev, "%s: enable_irq %i\n",
> > +                                __func__, mtk->seal_irq);
> > +               } else {
> > +                       disable_irq(mtk->seal_irq);
> > +                       dev_dbg(dev, "%s: disable_irq %i\n",
> > +                                __func__, mtk->seal_irq);
> > +               }
> > +       }
> > +}
> > +
> 
> I think this is unnecessary if this driver can check the current state
> and ignore the spurious irqs if spm sometimes triggers the wake-up irqs.
It helps to reduce some irqs, there are many spurious interrupts, I
think there is something wrong from the point of HW, will try to find
the root cause.

> 
> >  static int xhci_mtk_host_enable(struct xhci_hcd_mtk *mtk)
> >  {
> >         struct mu3c_ippc_regs __iomem *ippc = mtk->ippc_regs;
> > @@ -347,7 +414,6 @@ static int usb_wakeup_of_property_parse(struct xhci_hcd_mtk *mtk,
> >                         mtk->uwk_reg_base, mtk->uwk_vers);
> >
> >         return PTR_ERR_OR_ZERO(mtk->uwk);
> > -
> >  }
> >
> >  static void usb_wakeup_set(struct xhci_hcd_mtk *mtk, bool enable)
> > @@ -482,9 +548,11 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 return ret;
> >         }
> >
> > +       pm_runtime_set_active(dev);
> > +       pm_runtime_use_autosuspend(dev);
> > +       pm_runtime_set_autosuspend_delay(dev,
> > +                               CONFIG_USB_AUTOSUSPEND_DELAY * 1000);
> >         pm_runtime_enable(dev);
> > -       pm_runtime_get_sync(dev);
> 
> The only one left pm_runtime_get() is removed by here,
> now this driver only calls pm_runtime_put().
will add it back, and forbid runtime by default
> 
> > -       device_enable_async_suspend(dev);
> >
> >         ret = xhci_mtk_ldos_enable(mtk);
> >         if (ret)
> > @@ -499,6 +567,14 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 ret = irq;
> >                 goto disable_clk;
> >         }
> > +       dev_dbg(dev, "irq %i\n", irq);
> > +
> > +       mtk->seal_irq = platform_get_irq_optional(pdev, 1);
> > +       if (mtk->seal_irq < 0) {
> > +               ret = mtk->seal_irq;
> > +               goto disable_clk;
> > +       }
> > +       dev_dbg(dev, "seal_irq %i\n", mtk->seal_irq);
> >
> >         hcd = usb_create_hcd(driver, dev, dev_name(dev));
> >         if (!hcd) {
> > @@ -565,6 +641,27 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         if (ret)
> >                 goto dealloc_usb2_hcd;
> >
> > +       INIT_DELAYED_WORK(&mtk->seal, xhci_mtk_seal_work);
> > +       snprintf(mtk->seal_descr, sizeof(mtk->seal_descr), "seal%s:usb%d",
> > +                hcd->driver->description, hcd->self.busnum);
> > +       ret = devm_request_irq(dev, mtk->seal_irq, &xhci_mtk_seal_irq,
> > +                         IRQF_TRIGGER_FALLING, mtk->seal_descr, mtk);
> > +       if (ret != 0) {
> > +               dev_err(dev, "seal request interrupt %d failed\n",
> > +                       mtk->seal_irq);
> > +               goto dealloc_usb2_hcd;
> > +       }
> > +       xhci_mtk_seal_wakeup_enable(mtk, false);
> > +
> > +       device_enable_async_suspend(dev);
> > +       xhci_mtk_runtime_ready = 1;
> > +
> > +       pm_runtime_mark_last_busy(dev);
> > +       pm_runtime_put_autosuspend(dev);
> 
> I expect the usage count will be -1 by here in probe.
will change it
> 
> > +
> > +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> > +                __func__, xhci_mtk_runtime_ready);
> > +
> >         return 0;
> >
> >  dealloc_usb2_hcd:
> > @@ -587,7 +684,7 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >         xhci_mtk_ldos_disable(mtk);
> >
> >  disable_pm:
> > -       pm_runtime_put_sync(dev);
> > +       pm_runtime_put_sync_autosuspend(dev);
> >         pm_runtime_disable(dev);
> >         return ret;
> >  }
> > @@ -602,6 +699,7 @@ static int xhci_mtk_remove(struct platform_device *dev)
> >         pm_runtime_put_noidle(&dev->dev);
> >         pm_runtime_disable(&dev->dev);
> >
> > +       xhci_mtk_runtime_ready = 0;
> >         usb_remove_hcd(shared_hcd);
> >         xhci->shared_hcd = NULL;
> >         device_init_wakeup(&dev->dev, false);
> > @@ -638,6 +736,7 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
> >         xhci_mtk_host_disable(mtk);
> >         xhci_mtk_clks_disable(mtk);
> >         usb_wakeup_set(mtk, true);
> > +
> >         return 0;
> >  }
> >
> > @@ -659,10 +758,185 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
> >         return 0;
> >  }
> >
> > +static int __maybe_unused xhci_mtk_bus_status(struct device *dev)
> > +{
> > +       struct xhci_hcd_mtk *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +       struct xhci_hub *usb2_rhub;
> > +       struct xhci_hub *usb3_rhub;
> > +       struct xhci_bus_state *bus_state;
> > +       struct xhci_port *port;
> > +       u32     usb2_suspended_ports = -1;
> > +       u32     usb3_suspended_ports = -1;
> > +       u16 status;
> > +       int num_ports;
> > +       int ret = 0;
> > +       int i;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> 
> This is duplicated from xhci_mtk_runtime_suspend()
Yes, will remove it and rebuild the code
> 
> > +
> > +       usb2_rhub = &xhci->usb2_rhub;
> > +       if (usb2_rhub) {
> > +               bus_state  = &usb2_rhub->bus_state;
> > +               num_ports  = usb2_rhub->num_ports;
> > +               usb2_suspended_ports  = bus_state->suspended_ports;
> > +               usb2_suspended_ports ^= (BIT(num_ports) - 1);
> > +               usb2_suspended_ports &= (BIT(num_ports) - 1);
> > +               for (i = 0; i < num_ports; i++) {
> > +                       if (usb2_suspended_ports & (1UL << i)) {
> > +                               port = usb2_rhub->ports[i];
> > +                               status = readl(port->addr);
> > +
> > +                               xhci_dbg(xhci,
> > +                                         "USB20: portsc[%i]: 0x%04X\n",
> > +                                         i, status);
> > +
> > +                               if (!(status & PORT_CONNECT))
> > +                                       usb2_suspended_ports &= ~(1UL << i);
> > +                       }
> > +               }
> > +
> > +               if (usb2_suspended_ports) {
> > +                       ret = -EBUSY;
> > +                       goto ebusy;
> > +               }
> > +       }
> > +
> > +       usb3_rhub = &xhci->usb3_rhub;
> > +       if (usb3_rhub) {
> > +               bus_state  = &usb3_rhub->bus_state;
> > +               num_ports  = usb3_rhub->num_ports;
> > +               usb3_suspended_ports  = bus_state->suspended_ports;
> > +               usb3_suspended_ports ^= (BIT(num_ports) - 1);
> > +               usb3_suspended_ports &= (BIT(num_ports) - 1);
> > +               for (i = 0; i < num_ports; i++) {
> > +                       if (usb3_suspended_ports & BIT(i)) {
> > +                               port = usb3_rhub->ports[i];
> > +                               status = readl(port->addr);
> > +
> > +                               xhci_dbg(xhci, "USB3: portsc[%i]: 0x%04X\n",
> > +                                         i, status);
> > +
> > +                               if (!(status & PORT_CONNECT))
> > +                                       usb3_suspended_ports &= ~BIT(i);
> > +                       }
> > +               }
> > +
> > +               if (usb3_suspended_ports) {
> > +                       ret = -EBUSY;
> > +                       goto ebusy;
> > +               }
> > +       }
> > +
> > +ebusy:
> > +       xhci_dbg(xhci, "%s: USB2: 0x%08X, USB3: 0x%08X ret: %i\n",
> > +                 __func__, usb2_suspended_ports,
> > +                 usb3_suspended_ports, ret);
> > +
> > +       return ret;
> > +}
> > +
> 
> This is basically counting active ports by directly reading portsc register?
Yes, 
> I expect this function never return -EBUSY  if you balance pm usage
> counter well.
> Are there any specific reasons of doing this manually?
pm_runtime_set_autosuspend_delay() is used, may check the ports status
periodically. I will check it.

If no spurious wakeup irq, apply the framework from wakeirq.c is the
easiest way to support runtime-suspend. hope that find out the root
cause and improve the HW design. In fact the EINT way is a deprecated
feature and not used before.

Thanks a lot.

> 
> > +static int __maybe_unused xhci_mtk_runtime_suspend(struct device *dev)
> > +{
> > +       bool wakeup = device_may_wakeup(dev);
> > +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +       int ret = 0;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> > +
> > +       mtk->seal_status = SEAL_BUSY;
> > +       ret = xhci_mtk_bus_status(dev);
> > +       if (wakeup && !ret) {
> > +               mtk->seal_status = SEAL_SUSPENDING;
> > +               xhci_mtk_suspend(dev);
> > +               xhci_mtk_seal_wakeup_enable(mtk, true);
> > +               mtk->seal_status = SEAL_SUSPENDED;
> > +               xhci_dbg(xhci, "%s: seals xHCI controller\n", __func__);
> > +       }
> > +
> > +       xhci_dbg(xhci, "%s: seals wakeup = %i, ret = %i!\n",
> > +                 __func__, wakeup, ret);
> > +
> > +       return ret;
> > +}
> > +
> > +static int __maybe_unused xhci_mtk_runtime_resume(struct device *dev)
> > +{
> > +       bool wakeup = device_may_wakeup(dev);
> > +       struct xhci_hcd_mtk  *mtk = dev_get_drvdata(dev);
> > +       struct usb_hcd *hcd;
> > +       struct xhci_hcd *xhci;
> > +
> > +       if (!mtk->hcd)
> > +               return -ESHUTDOWN;
> > +
> > +       hcd = mtk->hcd;
> > +       xhci = hcd_to_xhci(hcd);
> > +       if ((xhci->xhc_state & XHCI_STATE_REMOVING) ||
> > +                       (xhci->xhc_state & XHCI_STATE_HALTED)) {
> > +               return -ESHUTDOWN;
> > +       }
> > +
> > +       /*
> > +        *  list cases by one extra interrupt named seal to process!!!
> > +        *  Who to process these module reinitilization after SPM wakeup
> > +        *  case 1: usb remote wakeup, therefore xHCI need reinitilizate also.
> > +        *  case 2: other-wakeup-source wakeup, therefore, xHCI need reinit
> > +        *  case 3: usb client driver can invoke it in runtime mechanism
> > +        *  case 4: user active
> > +        */
> > +       if (wakeup) {
> > +               xhci_mtk_seal_wakeup_enable(mtk, false);
> > +               xhci_mtk_resume(dev);
> > +               xhci_dbg(xhci, "%s: unseals xHCI controller\n", __func__);
> > +       }
> > +       mtk->seal_status = SEAL_RESUMED;
> > +
> > +       xhci_dbg(xhci, "%s: unseals wakeup = %i\n", __func__, wakeup);
> > +
> > +       return 0;
> > +}
> > +
> > +static int __maybe_unused xhci_mtk_runtime_idle(struct device *dev)
> > +{
> > +       int ret = 0;
> > +
> > +       dev_dbg(dev, "%s: xhci_mtk_runtime_ready %i",
> > +                __func__, xhci_mtk_runtime_ready);
> > +
> > +       if (!xhci_mtk_runtime_ready)
> > +               ret = -EAGAIN;
> > +
> > +       return ret;
> > +}
> > +
> >  static const struct dev_pm_ops xhci_mtk_pm_ops = {
> >         SET_SYSTEM_SLEEP_PM_OPS(xhci_mtk_suspend, xhci_mtk_resume)
> > +       SET_RUNTIME_PM_OPS(xhci_mtk_runtime_suspend,
> > +                          xhci_mtk_runtime_resume,
> > +                          xhci_mtk_runtime_idle)
> >  };
> > -#define DEV_PM_OPS IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL
> > +
> > +#define DEV_PM_OPS (IS_ENABLED(CONFIG_PM) ? &xhci_mtk_pm_ops : NULL)
> >
> >  #ifdef CONFIG_OF
> >  static const struct of_device_id mtk_xhci_of_match[] = {
> > @@ -686,6 +960,7 @@ MODULE_ALIAS("platform:xhci-mtk");
> >
> >  static int __init xhci_mtk_init(void)
> >  {
> > +       xhci_mtk_runtime_ready = 0;
> >         xhci_init_driver(&xhci_mtk_hc_driver, &xhci_mtk_overrides);
> >         return platform_driver_register(&mtk_xhci_driver);
> >  }
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index 323b281933b9..103d83ce6a3e 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -133,6 +133,14 @@ struct mu3c_ippc_regs {
> >         __le32 reserved3[33]; /* 0x80 ~ 0xff */
> >  };
> >
> > +enum xhci_mtk_seal {
> > +       SEAL_BUSY = 0,
> > +       SEAL_SUSPENDING,
> > +       SEAL_SUSPENDED,
> > +       SEAL_RESUMING,
> > +       SEAL_RESUMED
> > +};
> > +
> >  struct xhci_hcd_mtk {
> >         struct device *dev;
> >         struct usb_hcd *hcd;
> > @@ -158,6 +166,12 @@ struct xhci_hcd_mtk {
> >         struct regmap *uwk;
> >         u32 uwk_reg_base;
> >         u32 uwk_vers;
> > +
> > +       /* usb eint wakeup source */
> > +       int seal_irq;
> > +       enum xhci_mtk_seal seal_status;
> > +       struct delayed_work  seal;
> > +       char   seal_descr[32];  /* "seal" + driver + bus # */
> >  };
> >
> >  static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
> > --
> > 2.18.0
> >

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
  2020-12-22  9:34 ` Chunfeng Yun
@ 2021-01-13  9:43   ` Ikjoon Jang
  -1 siblings, 0 replies; 19+ messages in thread
From: Ikjoon Jang @ 2021-01-13  9:43 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Yaqii Wu, Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support

On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> From: Zhanyong Wang <zhanyong.wang@mediatek.com>
>
> After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
> usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
> usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
> usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
> usb 1-1.1: Manufacturer: Apple, Inc.
> usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
> xhci-mtk 11200000.xhci: Not enough bandwidth!
> usb 1-1.1: can't set config #2, error -28
>
> improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
> muli-TT.
>
> Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
>  drivers/usb/host/xhci-mtk.h     |  8 ++-
>  2 files changed, 84 insertions(+), 15 deletions(-)
>  mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c
>
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> old mode 100644
> new mode 100755
> index 45c54d56ecbd..94292b9bbc63
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
>         u32 fs_budget_start;
>         u32 start_ss, last_ss;
>         u32 start_cs, last_cs;
> -       int i;
> +       u32 num_esit, base;
> +       int i, j;
> +       u32 tmp;
>
>         start_ss = offset % 8;
>         fs_budget_start = (start_ss + 1) % 8;
> @@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
>                 if (!(start_ss == 7 || last_ss < 6))
>                         return -ERANGE;
>
> -               for (i = 0; i < sch_ep->cs_count; i++)
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               for (i = 0; i < sch_ep->cs_count; i++) {
> +                       if (test_bit(offset + i, tt->ss_bit_map))
>                                 return -ERANGE;
>
> +                       if (test_bit(offset + i, tt->cs_bit_map))
> +                               return -ERANGE;
> +               }

Are there any reasons for doing this?
Why can only one split packet be scheduled in a u-frame for isochronous out?
This looks like overkill.

>         } else {
>                 u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
>
> @@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
>                 if (cs_count > 7)
>                         cs_count = 7; /* HW limit */
>
> -               for (i = 0; i < cs_count + 2; i++) {
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               if (test_bit(offset, tt->ss_bit_map))
> +                       return -ERANGE;
> +               for (i = 0; i < cs_count; i++) {
> +                       if (test_bit(offset + 2 + i, tt->cs_bit_map))
>                                 return -ERANGE;
>                 }
>

same here. It would be much better to understand
if you can provide some counterexamples of schedule
that can happen when this bitmap checking logic is omitted.

> @@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
>                         sch_ep->num_budget_microframes = sch_ep->esit;
>         }
>
> +       num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> +       for (i = 0; i < num_esit; i++) {
> +               base = sch_ep->offset + i * sch_ep->esit;
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       tmp = tt->fs_bus_bw[base + j]
> +                             + sch_ep->bw_cost_per_microframe;
> +                       if (tmp > FS_PAYLOAD_MAX)
> +                               return -ERANGE;
> +               }
> +       }

I guess this is enough to check the bandwidth limit of the
lower speed bus without a bitmap.

> +
>         return 0;
>  }
>
>  static void update_sch_tt(struct usb_device *udev,
> -       struct mu3h_sch_ep_info *sch_ep)
> +       struct mu3h_sch_ep_info *sch_ep, bool used)
>  {
>         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
>         u32 base, num_esit;
> @@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
>         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
>         for (i = 0; i < num_esit; i++) {
>                 base = sch_ep->offset + i * sch_ep->esit;
> -               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> -                       set_bit(base + j, tt->split_bit_map);
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       if (used)
> +                               set_bit(base + j, tt->split_bit_map);
> +                       else
> +                               clear_bit(base + j, tt->split_bit_map);
> +               }
> +
> +               if (sch_ep->ep_type == ISOC_OUT_EP) {
> +                       for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                               if (used) {
> +                                       set_bit(base + j, tt->ss_bit_map);
> +                                       set_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + j, tt->ss_bit_map);
> +                                       clear_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               } else {
> +                       if (used)
> +                               set_bit(base, tt->ss_bit_map);
> +                       else
> +                               clear_bit(base, tt->ss_bit_map);
> +
> +                       for (j = 0; j < sch_ep->cs_count; j++) {
> +                               if (used) {
> +                                       set_bit(base + 2 + j, tt->cs_bit_map);
> +
> +                                       tt->fs_bus_bw[base + 2 + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + 2 + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + 2 + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               }
>         }
>
> -       list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       if (used)
> +               list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       else
> +               list_del(&sch_ep->tt_endpoint);
>  }
>
>  static int check_sch_bw(struct usb_device *udev,
> @@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
>  {
>         u32 offset;
>         u32 esit;
> +       u32 boundary;
>         u32 min_bw;
>         u32 min_index;
>         u32 worst_bw;
> @@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
>          */
>         min_bw = ~0;
>         min_index = 0;
> +       boundary = esit;
>         min_cs_count = sch_ep->cs_count;
>         min_num_budget = sch_ep->num_budget_microframes;
>         for (offset = 0; offset < esit; offset++) {
>                 if (is_fs_or_ls(udev->speed)) {
> +                       if (sch_ep->ep_type != ISOC_OUT_EP)
> +                               boundary = esit + 1;
>                         ret = check_sch_tt(udev, sch_ep, offset);
>                         if (ret)
>                                 continue;
> @@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
>                                 tt_offset_ok = true;
>                 }
>
> -               if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
> +               if ((offset + sch_ep->num_budget_microframes) > boundary)
>                         break;
>
>                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
> @@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
>                 if (!tt_offset_ok)
>                         return -ERANGE;
>
> -               update_sch_tt(udev, sch_ep);
> +               update_sch_tt(udev, sch_ep, 1);
>         }
>
>         /* update bus bandwidth info */
> @@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
>
>         list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
>                 if (sch_ep->ep == ep) {
> -                       update_bus_bw(sch_bw, sch_ep, 0);
> -                       list_del(&sch_ep->endpoint);
>                         if (is_fs_or_ls(udev->speed)) {
> -                               list_del(&sch_ep->tt_endpoint);
> +                               update_sch_tt(udev, sch_ep, 0);
>                                 drop_tt(udev);
>                         }
> +                       update_bus_bw(sch_bw, sch_ep, 0);
> +                       list_del(&sch_ep->endpoint);
>                         kfree(sch_ep);
>                         break;
>                 }
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index a93cfe817904..323b281933b9 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -21,12 +21,18 @@
>
>  /**
>   * @split_bit_map: used to avoid split microframes overlay
> + * @ss_bit_map: used to avoid start split microframes overlay
> + * @cs_bit_map: used to avoid complete split microframes overlay
> + * @fs_bus_bw: array to keep track of bandwidth already used at full speed
>   * @ep_list: Endpoints using this TT
>   * @usb_tt: usb TT related
>   * @tt_port: TT port number
>   */
>  struct mu3h_sch_tt {
>         DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
> +       u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
>         struct list_head ep_list;
>         struct usb_tt *usb_tt;
>         int tt_port;
> @@ -42,7 +48,7 @@ struct mu3h_sch_tt {
>   * two bandwidth domains, one for IN eps and another for OUT eps.
>   */
>  struct mu3h_sch_bw_info {
> -       u32 bus_bw[XHCI_MTK_MAX_ESIT];
> +       u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
>         struct list_head bw_ep_list;
>  };
>
> --
> 2.18.0
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
@ 2021-01-13  9:43   ` Ikjoon Jang
  0 siblings, 0 replies; 19+ messages in thread
From: Ikjoon Jang @ 2021-01-13  9:43 UTC (permalink / raw)
  To: Chunfeng Yun
  Cc: Yaqii Wu, Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support

On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> From: Zhanyong Wang <zhanyong.wang@mediatek.com>
>
> After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
> usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
> usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
> usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
> usb 1-1.1: Manufacturer: Apple, Inc.
> usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
> xhci-mtk 11200000.xhci: Not enough bandwidth!
> usb 1-1.1: can't set config #2, error -28
>
> improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
> muli-TT.
>
> Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
>  drivers/usb/host/xhci-mtk.h     |  8 ++-
>  2 files changed, 84 insertions(+), 15 deletions(-)
>  mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c
>
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> old mode 100644
> new mode 100755
> index 45c54d56ecbd..94292b9bbc63
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
>         u32 fs_budget_start;
>         u32 start_ss, last_ss;
>         u32 start_cs, last_cs;
> -       int i;
> +       u32 num_esit, base;
> +       int i, j;
> +       u32 tmp;
>
>         start_ss = offset % 8;
>         fs_budget_start = (start_ss + 1) % 8;
> @@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
>                 if (!(start_ss == 7 || last_ss < 6))
>                         return -ERANGE;
>
> -               for (i = 0; i < sch_ep->cs_count; i++)
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               for (i = 0; i < sch_ep->cs_count; i++) {
> +                       if (test_bit(offset + i, tt->ss_bit_map))
>                                 return -ERANGE;
>
> +                       if (test_bit(offset + i, tt->cs_bit_map))
> +                               return -ERANGE;
> +               }

Are there any reasons for doing this?
Why can only one split packet be scheduled in a u-frame for isochronous out?
This looks like overkill.

>         } else {
>                 u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
>
> @@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
>                 if (cs_count > 7)
>                         cs_count = 7; /* HW limit */
>
> -               for (i = 0; i < cs_count + 2; i++) {
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               if (test_bit(offset, tt->ss_bit_map))
> +                       return -ERANGE;
> +               for (i = 0; i < cs_count; i++) {
> +                       if (test_bit(offset + 2 + i, tt->cs_bit_map))
>                                 return -ERANGE;
>                 }
>

same here. It would be much better to understand
if you can provide some counterexamples of schedule
that can happen when this bitmap checking logic is omitted.

> @@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
>                         sch_ep->num_budget_microframes = sch_ep->esit;
>         }
>
> +       num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> +       for (i = 0; i < num_esit; i++) {
> +               base = sch_ep->offset + i * sch_ep->esit;
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       tmp = tt->fs_bus_bw[base + j]
> +                             + sch_ep->bw_cost_per_microframe;
> +                       if (tmp > FS_PAYLOAD_MAX)
> +                               return -ERANGE;
> +               }
> +       }

I guess this is enough to check the bandwidth limit of the
lower speed bus without a bitmap.

> +
>         return 0;
>  }
>
>  static void update_sch_tt(struct usb_device *udev,
> -       struct mu3h_sch_ep_info *sch_ep)
> +       struct mu3h_sch_ep_info *sch_ep, bool used)
>  {
>         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
>         u32 base, num_esit;
> @@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
>         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
>         for (i = 0; i < num_esit; i++) {
>                 base = sch_ep->offset + i * sch_ep->esit;
> -               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> -                       set_bit(base + j, tt->split_bit_map);
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       if (used)
> +                               set_bit(base + j, tt->split_bit_map);
> +                       else
> +                               clear_bit(base + j, tt->split_bit_map);
> +               }
> +
> +               if (sch_ep->ep_type == ISOC_OUT_EP) {
> +                       for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                               if (used) {
> +                                       set_bit(base + j, tt->ss_bit_map);
> +                                       set_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + j, tt->ss_bit_map);
> +                                       clear_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               } else {
> +                       if (used)
> +                               set_bit(base, tt->ss_bit_map);
> +                       else
> +                               clear_bit(base, tt->ss_bit_map);
> +
> +                       for (j = 0; j < sch_ep->cs_count; j++) {
> +                               if (used) {
> +                                       set_bit(base + 2 + j, tt->cs_bit_map);
> +
> +                                       tt->fs_bus_bw[base + 2 + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + 2 + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + 2 + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               }
>         }
>
> -       list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       if (used)
> +               list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       else
> +               list_del(&sch_ep->tt_endpoint);
>  }
>
>  static int check_sch_bw(struct usb_device *udev,
> @@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
>  {
>         u32 offset;
>         u32 esit;
> +       u32 boundary;
>         u32 min_bw;
>         u32 min_index;
>         u32 worst_bw;
> @@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
>          */
>         min_bw = ~0;
>         min_index = 0;
> +       boundary = esit;
>         min_cs_count = sch_ep->cs_count;
>         min_num_budget = sch_ep->num_budget_microframes;
>         for (offset = 0; offset < esit; offset++) {
>                 if (is_fs_or_ls(udev->speed)) {
> +                       if (sch_ep->ep_type != ISOC_OUT_EP)
> +                               boundary = esit + 1;
>                         ret = check_sch_tt(udev, sch_ep, offset);
>                         if (ret)
>                                 continue;
> @@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
>                                 tt_offset_ok = true;
>                 }
>
> -               if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
> +               if ((offset + sch_ep->num_budget_microframes) > boundary)
>                         break;
>
>                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
> @@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
>                 if (!tt_offset_ok)
>                         return -ERANGE;
>
> -               update_sch_tt(udev, sch_ep);
> +               update_sch_tt(udev, sch_ep, 1);
>         }
>
>         /* update bus bandwidth info */
> @@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
>
>         list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
>                 if (sch_ep->ep == ep) {
> -                       update_bus_bw(sch_bw, sch_ep, 0);
> -                       list_del(&sch_ep->endpoint);
>                         if (is_fs_or_ls(udev->speed)) {
> -                               list_del(&sch_ep->tt_endpoint);
> +                               update_sch_tt(udev, sch_ep, 0);
>                                 drop_tt(udev);
>                         }
> +                       update_bus_bw(sch_bw, sch_ep, 0);
> +                       list_del(&sch_ep->endpoint);
>                         kfree(sch_ep);
>                         break;
>                 }
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> index a93cfe817904..323b281933b9 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -21,12 +21,18 @@
>
>  /**
>   * @split_bit_map: used to avoid split microframes overlay
> + * @ss_bit_map: used to avoid start split microframes overlay
> + * @cs_bit_map: used to avoid complete split microframes overlay
> + * @fs_bus_bw: array to keep track of bandwidth already used at full speed
>   * @ep_list: Endpoints using this TT
>   * @usb_tt: usb TT related
>   * @tt_port: TT port number
>   */
>  struct mu3h_sch_tt {
>         DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
> +       u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
>         struct list_head ep_list;
>         struct usb_tt *usb_tt;
>         int tt_port;
> @@ -42,7 +48,7 @@ struct mu3h_sch_tt {
>   * two bandwidth domains, one for IN eps and another for OUT eps.
>   */
>  struct mu3h_sch_bw_info {
> -       u32 bus_bw[XHCI_MTK_MAX_ESIT];
> +       u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
>         struct list_head bw_ep_list;
>  };
>
> --
> 2.18.0
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* 答复: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
  2021-01-13  9:43   ` Ikjoon Jang
  (?)
@ 2021-01-19  6:18   ` Yaqii Wu (武亚奇)
  -1 siblings, 0 replies; 19+ messages in thread
From: Yaqii Wu (武亚奇) @ 2021-01-19  6:18 UTC (permalink / raw)
  To: Ikjoon Jang, Chunfeng Yun (云春峰)
  Cc: Zhanyong Wang (王战勇),
	linux-usb, open list, Tianping Fang (方天平),
	moderated list:ARM/Mediatek SoC support

HI Chunfeng
	请回复谷歌以下三个问题:
	Queston: Are there any reasons for doing this? can only one split packet be scheduled in a u-frame for isochronous out? This looks like overkill.
	Answer: We will calculate the number of uframes based on the isoc out ep maxp. Except for the last uframe, all other consumed bandwidth is 188. But to be safe, we also count the bandwidth 	consumed by the last SS as 188.

	Queston: same here. It would be much better to understand if you can provide some counterexamples of schedule that can happen when this bitmap checking logic is omitted.
	Answer: Maybe you are right, it will indeed double check. I will delete the bandwidth check, and then perform a regression test.

	Queston: I guess this is enough to check the bandwidth limit of the lower speed bus without a bitmap.
	Answer: I explained to you last time that ss cannot be overlapped. This is a hub compatibility issue. Therefore we need bitmap to prevent ss overlap.
BR
yaqii

-----邮件原件-----
发件人: Ikjoon Jang [mailto:ikjn@chromium.org] 
发送时间: 2021年1月13日 17:44
收件人: Chunfeng Yun (云春峰) <Chunfeng.Yun@mediatek.com>
抄送: Yaqii Wu (武亚奇) <Yaqii.Wu@mediatek.com>; Zhanyong Wang (王战勇) <zhanyong.wang@mediatek.com>; Zhanyong Wang <zhangyong.wang@mediatek.com>; linux-usb@vger.kernel.org; open list <linux-kernel@vger.kernel.org>; Tianping Fang (方天平) <Tianping.Fang@mediatek.com>; moderated list:ARM/Mediatek SoC support <linux-mediatek@lists.infradead.org>
主题: Re: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT

On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> From: Zhanyong Wang <zhanyong.wang@mediatek.com>
>
> After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
> usb 1-1.1: new full-speed USB device number 5 using xhci-mtk usb 
> 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, 
> bcdDevice=26.11 usb 1-1.1: New USB device strings: Mfr=1, Product=2, 
> SerialNumber=3 usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack 
> Adapter usb 1-1.1: Manufacturer: Apple, Inc.
> usb 1-1.1: SerialNumber: DWH915501TFJKLTAM xhci-mtk 11200000.xhci: Not 
> enough bandwidth!
> usb 1-1.1: can't set config #2, error -28
>
> improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB 
> muli-TT.
>
> Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
> v2~v3: no changes
> ---
>  drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
>  drivers/usb/host/xhci-mtk.h     |  8 ++-
>  2 files changed, 84 insertions(+), 15 deletions(-)  mode change 
> 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c
>
> diff --git a/drivers/usb/host/xhci-mtk-sch.c 
> b/drivers/usb/host/xhci-mtk-sch.c old mode 100644 new mode 100755 
> index 45c54d56ecbd..94292b9bbc63
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
> @@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
>         u32 fs_budget_start;
>         u32 start_ss, last_ss;
>         u32 start_cs, last_cs;
> -       int i;
> +       u32 num_esit, base;
> +       int i, j;
> +       u32 tmp;
>
>         start_ss = offset % 8;
>         fs_budget_start = (start_ss + 1) % 8; @@ -398,10 +400,13 @@ 
> static int check_sch_tt(struct usb_device *udev,
>                 if (!(start_ss == 7 || last_ss < 6))
>                         return -ERANGE;
>
> -               for (i = 0; i < sch_ep->cs_count; i++)
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               for (i = 0; i < sch_ep->cs_count; i++) {
> +                       if (test_bit(offset + i, tt->ss_bit_map))
>                                 return -ERANGE;
>
> +                       if (test_bit(offset + i, tt->cs_bit_map))
> +                               return -ERANGE;
> +               }

Are there any reasons for doing this?
Why can only one split packet be scheduled in a u-frame for isochronous out?
This looks like overkill.

>         } else {
>                 u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, 
> FS_PAYLOAD_MAX);
>
> @@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
>                 if (cs_count > 7)
>                         cs_count = 7; /* HW limit */
>
> -               for (i = 0; i < cs_count + 2; i++) {
> -                       if (test_bit(offset + i, tt->split_bit_map))
> +               if (test_bit(offset, tt->ss_bit_map))
> +                       return -ERANGE;
> +               for (i = 0; i < cs_count; i++) {
> +                       if (test_bit(offset + 2 + i, tt->cs_bit_map))
>                                 return -ERANGE;
>                 }
>

same here. It would be much better to understand if you can provide some counterexamples of schedule that can happen when this bitmap checking logic is omitted.

> @@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
>                         sch_ep->num_budget_microframes = sch_ep->esit;
>         }
>
> +       num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> +       for (i = 0; i < num_esit; i++) {
> +               base = sch_ep->offset + i * sch_ep->esit;
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       tmp = tt->fs_bus_bw[base + j]
> +                             + sch_ep->bw_cost_per_microframe;
> +                       if (tmp > FS_PAYLOAD_MAX)
> +                               return -ERANGE;
> +               }
> +       }

I guess this is enough to check the bandwidth limit of the lower speed bus without a bitmap.

> +
>         return 0;
>  }
>
>  static void update_sch_tt(struct usb_device *udev,
> -       struct mu3h_sch_ep_info *sch_ep)
> +       struct mu3h_sch_ep_info *sch_ep, bool used)
>  {
>         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
>         u32 base, num_esit;
> @@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
>         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
>         for (i = 0; i < num_esit; i++) {
>                 base = sch_ep->offset + i * sch_ep->esit;
> -               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> -                       set_bit(base + j, tt->split_bit_map);
> +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                       if (used)
> +                               set_bit(base + j, tt->split_bit_map);
> +                       else
> +                               clear_bit(base + j, tt->split_bit_map);
> +               }
> +
> +               if (sch_ep->ep_type == ISOC_OUT_EP) {
> +                       for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> +                               if (used) {
> +                                       set_bit(base + j, tt->ss_bit_map);
> +                                       set_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + j, tt->ss_bit_map);
> +                                       clear_bit(base + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               } else {
> +                       if (used)
> +                               set_bit(base, tt->ss_bit_map);
> +                       else
> +                               clear_bit(base, tt->ss_bit_map);
> +
> +                       for (j = 0; j < sch_ep->cs_count; j++) {
> +                               if (used) {
> +                                       set_bit(base + 2 + j, 
> + tt->cs_bit_map);
> +
> +                                       tt->fs_bus_bw[base + 2 + j] +=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               } else {
> +                                       clear_bit(base + 2 + j, tt->cs_bit_map);
> +                                       tt->fs_bus_bw[base + 2 + j] -=
> +                                               sch_ep->bw_cost_per_microframe;
> +                               }
> +                       }
> +               }
>         }
>
> -       list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       if (used)
> +               list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> +       else
> +               list_del(&sch_ep->tt_endpoint);
>  }
>
>  static int check_sch_bw(struct usb_device *udev, @@ -470,6 +529,7 @@ 
> static int check_sch_bw(struct usb_device *udev,  {
>         u32 offset;
>         u32 esit;
> +       u32 boundary;
>         u32 min_bw;
>         u32 min_index;
>         u32 worst_bw;
> @@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
>          */
>         min_bw = ~0;
>         min_index = 0;
> +       boundary = esit;
>         min_cs_count = sch_ep->cs_count;
>         min_num_budget = sch_ep->num_budget_microframes;
>         for (offset = 0; offset < esit; offset++) {
>                 if (is_fs_or_ls(udev->speed)) {
> +                       if (sch_ep->ep_type != ISOC_OUT_EP)
> +                               boundary = esit + 1;
>                         ret = check_sch_tt(udev, sch_ep, offset);
>                         if (ret)
>                                 continue; @@ -498,7 +561,7 @@ static 
> int check_sch_bw(struct usb_device *udev,
>                                 tt_offset_ok = true;
>                 }
>
> -               if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
> +               if ((offset + sch_ep->num_budget_microframes) > 
> + boundary)
>                         break;
>
>                 worst_bw = get_max_bw(sch_bw, sch_ep, offset); @@ 
> -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
>                 if (!tt_offset_ok)
>                         return -ERANGE;
>
> -               update_sch_tt(udev, sch_ep);
> +               update_sch_tt(udev, sch_ep, 1);
>         }
>
>         /* update bus bandwidth info */ @@ -696,12 +759,12 @@ void 
> xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
>
>         list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
>                 if (sch_ep->ep == ep) {
> -                       update_bus_bw(sch_bw, sch_ep, 0);
> -                       list_del(&sch_ep->endpoint);
>                         if (is_fs_or_ls(udev->speed)) {
> -                               list_del(&sch_ep->tt_endpoint);
> +                               update_sch_tt(udev, sch_ep, 0);
>                                 drop_tt(udev);
>                         }
> +                       update_bus_bw(sch_bw, sch_ep, 0);
> +                       list_del(&sch_ep->endpoint);
>                         kfree(sch_ep);
>                         break;
>                 }
> diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h 
> index a93cfe817904..323b281933b9 100644
> --- a/drivers/usb/host/xhci-mtk.h
> +++ b/drivers/usb/host/xhci-mtk.h
> @@ -21,12 +21,18 @@
>
>  /**
>   * @split_bit_map: used to avoid split microframes overlay
> + * @ss_bit_map: used to avoid start split microframes overlay
> + * @cs_bit_map: used to avoid complete split microframes overlay
> + * @fs_bus_bw: array to keep track of bandwidth already used at full 
> + speed
>   * @ep_list: Endpoints using this TT
>   * @usb_tt: usb TT related
>   * @tt_port: TT port number
>   */
>  struct mu3h_sch_tt {
>         DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
> +       DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
> +       u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
>         struct list_head ep_list;
>         struct usb_tt *usb_tt;
>         int tt_port;
> @@ -42,7 +48,7 @@ struct mu3h_sch_tt {
>   * two bandwidth domains, one for IN eps and another for OUT eps.
>   */
>  struct mu3h_sch_bw_info {
> -       u32 bus_bw[XHCI_MTK_MAX_ESIT];
> +       u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
>         struct list_head bw_ep_list;
>  };
>
> --
> 2.18.0
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
  2021-01-13  9:43   ` Ikjoon Jang
@ 2021-01-19  6:57     ` Chunfeng Yun
  -1 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2021-01-19  6:57 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Yaqii Wu, Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support

On Wed, 2021-01-13 at 17:43 +0800, Ikjoon Jang wrote:
> On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > From: Zhanyong Wang <zhanyong.wang@mediatek.com>
> >
> > After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
> > usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
> > usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
> > usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
> > usb 1-1.1: Manufacturer: Apple, Inc.
> > usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
> > xhci-mtk 11200000.xhci: Not enough bandwidth!
> > usb 1-1.1: can't set config #2, error -28
> >
> > improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
> > muli-TT.
> >
> > Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v2~v3: no changes
> > ---
> >  drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
> >  drivers/usb/host/xhci-mtk.h     |  8 ++-
> >  2 files changed, 84 insertions(+), 15 deletions(-)
> >  mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c
> >
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> > old mode 100644
> > new mode 100755
> > index 45c54d56ecbd..94292b9bbc63
> > --- a/drivers/usb/host/xhci-mtk-sch.c
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > @@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
> >         u32 fs_budget_start;
> >         u32 start_ss, last_ss;
> >         u32 start_cs, last_cs;
> > -       int i;
> > +       u32 num_esit, base;
> > +       int i, j;
> > +       u32 tmp;
> >
> >         start_ss = offset % 8;
> >         fs_budget_start = (start_ss + 1) % 8;
> > @@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
> >                 if (!(start_ss == 7 || last_ss < 6))
> >                         return -ERANGE;
> >
> > -               for (i = 0; i < sch_ep->cs_count; i++)
> > -                       if (test_bit(offset + i, tt->split_bit_map))
> > +               for (i = 0; i < sch_ep->cs_count; i++) {
> > +                       if (test_bit(offset + i, tt->ss_bit_map))
> >                                 return -ERANGE;
> >
> > +                       if (test_bit(offset + i, tt->cs_bit_map))
> > +                               return -ERANGE;
> > +               }
> 
> Are there any reasons for doing this?
> Why can only one split packet be scheduled in a u-frame for isochronous out?
> This looks like overkill.
[From DR]: We will calculate the number of uframes based on the isoc out
ep maxp. Except for the last uframe, all other consumed bandwidth is
188. But to be safe, we also count the bandwidth consumed by the last SS
as 188.

> 
> >         } else {
> >                 u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
> >
> > @@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
> >                 if (cs_count > 7)
> >                         cs_count = 7; /* HW limit */
> >
> > -               for (i = 0; i < cs_count + 2; i++) {
> > -                       if (test_bit(offset + i, tt->split_bit_map))
> > +               if (test_bit(offset, tt->ss_bit_map))
> > +                       return -ERANGE;
> > +               for (i = 0; i < cs_count; i++) {
> > +                       if (test_bit(offset + 2 + i, tt->cs_bit_map))
> >                                 return -ERANGE;
> >                 }
> >
> 
> same here. It would be much better to understand
> if you can provide some counterexamples of schedule
> that can happen when this bitmap checking logic is omitted.
[From DR] Maybe you are right, it will indeed double check. I will
delete the bandwidth check, and then perform a regression test.

> 
> > @@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
> >                         sch_ep->num_budget_microframes = sch_ep->esit;
> >         }
> >
> > +       num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> > +       for (i = 0; i < num_esit; i++) {
> > +               base = sch_ep->offset + i * sch_ep->esit;
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                       tmp = tt->fs_bus_bw[base + j]
> > +                             + sch_ep->bw_cost_per_microframe;
> > +                       if (tmp > FS_PAYLOAD_MAX)
> > +                               return -ERANGE;
> > +               }
> > +       }
> 
> I guess this is enough to check the bandwidth limit of the
> lower speed bus without a bitmap.
[From DR]As explained in issue tracker that ss cannot be overlapped.
This is a hub compatibility issue. Therefore we need bitmap to prevent
ss overlap.

Thanks
> 
> > +
> >         return 0;
> >  }
> >
> >  static void update_sch_tt(struct usb_device *udev,
> > -       struct mu3h_sch_ep_info *sch_ep)
> > +       struct mu3h_sch_ep_info *sch_ep, bool used)
> >  {
> >         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> >         u32 base, num_esit;
> > @@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
> >         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> >         for (i = 0; i < num_esit; i++) {
> >                 base = sch_ep->offset + i * sch_ep->esit;
> > -               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> > -                       set_bit(base + j, tt->split_bit_map);
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                       if (used)
> > +                               set_bit(base + j, tt->split_bit_map);
> > +                       else
> > +                               clear_bit(base + j, tt->split_bit_map);
> > +               }
> > +
> > +               if (sch_ep->ep_type == ISOC_OUT_EP) {
> > +                       for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                               if (used) {
> > +                                       set_bit(base + j, tt->ss_bit_map);
> > +                                       set_bit(base + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + j] +=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               } else {
> > +                                       clear_bit(base + j, tt->ss_bit_map);
> > +                                       clear_bit(base + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + j] -=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               }
> > +                       }
> > +               } else {
> > +                       if (used)
> > +                               set_bit(base, tt->ss_bit_map);
> > +                       else
> > +                               clear_bit(base, tt->ss_bit_map);
> > +
> > +                       for (j = 0; j < sch_ep->cs_count; j++) {
> > +                               if (used) {
> > +                                       set_bit(base + 2 + j, tt->cs_bit_map);
> > +
> > +                                       tt->fs_bus_bw[base + 2 + j] +=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               } else {
> > +                                       clear_bit(base + 2 + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + 2 + j] -=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               }
> > +                       }
> > +               }
> >         }
> >
> > -       list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> > +       if (used)
> > +               list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> > +       else
> > +               list_del(&sch_ep->tt_endpoint);
> >  }
> >
> >  static int check_sch_bw(struct usb_device *udev,
> > @@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
> >  {
> >         u32 offset;
> >         u32 esit;
> > +       u32 boundary;
> >         u32 min_bw;
> >         u32 min_index;
> >         u32 worst_bw;
> > @@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
> >          */
> >         min_bw = ~0;
> >         min_index = 0;
> > +       boundary = esit;
> >         min_cs_count = sch_ep->cs_count;
> >         min_num_budget = sch_ep->num_budget_microframes;
> >         for (offset = 0; offset < esit; offset++) {
> >                 if (is_fs_or_ls(udev->speed)) {
> > +                       if (sch_ep->ep_type != ISOC_OUT_EP)
> > +                               boundary = esit + 1;
> >                         ret = check_sch_tt(udev, sch_ep, offset);
> >                         if (ret)
> >                                 continue;
> > @@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
> >                                 tt_offset_ok = true;
> >                 }
> >
> > -               if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
> > +               if ((offset + sch_ep->num_budget_microframes) > boundary)
> >                         break;
> >
> >                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
> > @@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
> >                 if (!tt_offset_ok)
> >                         return -ERANGE;
> >
> > -               update_sch_tt(udev, sch_ep);
> > +               update_sch_tt(udev, sch_ep, 1);
> >         }
> >
> >         /* update bus bandwidth info */
> > @@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
> >
> >         list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
> >                 if (sch_ep->ep == ep) {
> > -                       update_bus_bw(sch_bw, sch_ep, 0);
> > -                       list_del(&sch_ep->endpoint);
> >                         if (is_fs_or_ls(udev->speed)) {
> > -                               list_del(&sch_ep->tt_endpoint);
> > +                               update_sch_tt(udev, sch_ep, 0);
> >                                 drop_tt(udev);
> >                         }
> > +                       update_bus_bw(sch_bw, sch_ep, 0);
> > +                       list_del(&sch_ep->endpoint);
> >                         kfree(sch_ep);
> >                         break;
> >                 }
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..323b281933b9 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -21,12 +21,18 @@
> >
> >  /**
> >   * @split_bit_map: used to avoid split microframes overlay
> > + * @ss_bit_map: used to avoid start split microframes overlay
> > + * @cs_bit_map: used to avoid complete split microframes overlay
> > + * @fs_bus_bw: array to keep track of bandwidth already used at full speed
> >   * @ep_list: Endpoints using this TT
> >   * @usb_tt: usb TT related
> >   * @tt_port: TT port number
> >   */
> >  struct mu3h_sch_tt {
> >         DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
> > +       DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
> > +       DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
> > +       u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
> >         struct list_head ep_list;
> >         struct usb_tt *usb_tt;
> >         int tt_port;
> > @@ -42,7 +48,7 @@ struct mu3h_sch_tt {
> >   * two bandwidth domains, one for IN eps and another for OUT eps.
> >   */
> >  struct mu3h_sch_bw_info {
> > -       u32 bus_bw[XHCI_MTK_MAX_ESIT];
> > +       u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
> >         struct list_head bw_ep_list;
> >  };
> >
> > --
> > 2.18.0
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek


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

* Re: [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT
@ 2021-01-19  6:57     ` Chunfeng Yun
  0 siblings, 0 replies; 19+ messages in thread
From: Chunfeng Yun @ 2021-01-19  6:57 UTC (permalink / raw)
  To: Ikjoon Jang
  Cc: Yaqii Wu, Zhanyong Wang, Zhanyong Wang, linux-usb, open list,
	Tianping Fang, moderated list:ARM/Mediatek SoC support

On Wed, 2021-01-13 at 17:43 +0800, Ikjoon Jang wrote:
> On Tue, Dec 22, 2020 at 5:35 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > From: Zhanyong Wang <zhanyong.wang@mediatek.com>
> >
> > After inserted the usb type-c 3.5mm dongle with headset, dmesg showed:
> > usb 1-1.1: new full-speed USB device number 5 using xhci-mtk
> > usb 1-1.1: New USB device found, idVendor=05ac, idProduct=110a, bcdDevice=26.11
> > usb 1-1.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > usb 1-1.1: Product: USB-C to 3.5mm Headphone Jack Adapter
> > usb 1-1.1: Manufacturer: Apple, Inc.
> > usb 1-1.1: SerialNumber: DWH915501TFJKLTAM
> > xhci-mtk 11200000.xhci: Not enough bandwidth!
> > usb 1-1.1: can't set config #2, error -28
> >
> > improve low-speed/full-speed INT/ISOC bandwidth scheduling with USB
> > muli-TT.
> >
> > Signed-off-by: Yaqii Wu <yaqii.wu@mediatek.com>
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> > v2~v3: no changes
> > ---
> >  drivers/usb/host/xhci-mtk-sch.c | 91 ++++++++++++++++++++++++++++-----
> >  drivers/usb/host/xhci-mtk.h     |  8 ++-
> >  2 files changed, 84 insertions(+), 15 deletions(-)
> >  mode change 100644 => 100755 drivers/usb/host/xhci-mtk-sch.c
> >
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> > old mode 100644
> > new mode 100755
> > index 45c54d56ecbd..94292b9bbc63
> > --- a/drivers/usb/host/xhci-mtk-sch.c
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
> > @@ -383,7 +383,9 @@ static int check_sch_tt(struct usb_device *udev,
> >         u32 fs_budget_start;
> >         u32 start_ss, last_ss;
> >         u32 start_cs, last_cs;
> > -       int i;
> > +       u32 num_esit, base;
> > +       int i, j;
> > +       u32 tmp;
> >
> >         start_ss = offset % 8;
> >         fs_budget_start = (start_ss + 1) % 8;
> > @@ -398,10 +400,13 @@ static int check_sch_tt(struct usb_device *udev,
> >                 if (!(start_ss == 7 || last_ss < 6))
> >                         return -ERANGE;
> >
> > -               for (i = 0; i < sch_ep->cs_count; i++)
> > -                       if (test_bit(offset + i, tt->split_bit_map))
> > +               for (i = 0; i < sch_ep->cs_count; i++) {
> > +                       if (test_bit(offset + i, tt->ss_bit_map))
> >                                 return -ERANGE;
> >
> > +                       if (test_bit(offset + i, tt->cs_bit_map))
> > +                               return -ERANGE;
> > +               }
> 
> Are there any reasons for doing this?
> Why can only one split packet be scheduled in a u-frame for isochronous out?
> This looks like overkill.
[From DR]: We will calculate the number of uframes based on the isoc out
ep maxp. Except for the last uframe, all other consumed bandwidth is
188. But to be safe, we also count the bandwidth consumed by the last SS
as 188.

> 
> >         } else {
> >                 u32 cs_count = DIV_ROUND_UP(sch_ep->maxpkt, FS_PAYLOAD_MAX);
> >
> > @@ -428,8 +433,10 @@ static int check_sch_tt(struct usb_device *udev,
> >                 if (cs_count > 7)
> >                         cs_count = 7; /* HW limit */
> >
> > -               for (i = 0; i < cs_count + 2; i++) {
> > -                       if (test_bit(offset + i, tt->split_bit_map))
> > +               if (test_bit(offset, tt->ss_bit_map))
> > +                       return -ERANGE;
> > +               for (i = 0; i < cs_count; i++) {
> > +                       if (test_bit(offset + 2 + i, tt->cs_bit_map))
> >                                 return -ERANGE;
> >                 }
> >
> 
> same here. It would be much better to understand
> if you can provide some counterexamples of schedule
> that can happen when this bitmap checking logic is omitted.
[From DR] Maybe you are right, it will indeed double check. I will
delete the bandwidth check, and then perform a regression test.

> 
> > @@ -445,11 +452,22 @@ static int check_sch_tt(struct usb_device *udev,
> >                         sch_ep->num_budget_microframes = sch_ep->esit;
> >         }
> >
> > +       num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> > +       for (i = 0; i < num_esit; i++) {
> > +               base = sch_ep->offset + i * sch_ep->esit;
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                       tmp = tt->fs_bus_bw[base + j]
> > +                             + sch_ep->bw_cost_per_microframe;
> > +                       if (tmp > FS_PAYLOAD_MAX)
> > +                               return -ERANGE;
> > +               }
> > +       }
> 
> I guess this is enough to check the bandwidth limit of the
> lower speed bus without a bitmap.
[From DR]As explained in issue tracker that ss cannot be overlapped.
This is a hub compatibility issue. Therefore we need bitmap to prevent
ss overlap.

Thanks
> 
> > +
> >         return 0;
> >  }
> >
> >  static void update_sch_tt(struct usb_device *udev,
> > -       struct mu3h_sch_ep_info *sch_ep)
> > +       struct mu3h_sch_ep_info *sch_ep, bool used)
> >  {
> >         struct mu3h_sch_tt *tt = sch_ep->sch_tt;
> >         u32 base, num_esit;
> > @@ -458,11 +476,52 @@ static void update_sch_tt(struct usb_device *udev,
> >         num_esit = XHCI_MTK_MAX_ESIT / sch_ep->esit;
> >         for (i = 0; i < num_esit; i++) {
> >                 base = sch_ep->offset + i * sch_ep->esit;
> > -               for (j = 0; j < sch_ep->num_budget_microframes; j++)
> > -                       set_bit(base + j, tt->split_bit_map);
> > +               for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                       if (used)
> > +                               set_bit(base + j, tt->split_bit_map);
> > +                       else
> > +                               clear_bit(base + j, tt->split_bit_map);
> > +               }
> > +
> > +               if (sch_ep->ep_type == ISOC_OUT_EP) {
> > +                       for (j = 0; j < sch_ep->num_budget_microframes; j++) {
> > +                               if (used) {
> > +                                       set_bit(base + j, tt->ss_bit_map);
> > +                                       set_bit(base + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + j] +=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               } else {
> > +                                       clear_bit(base + j, tt->ss_bit_map);
> > +                                       clear_bit(base + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + j] -=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               }
> > +                       }
> > +               } else {
> > +                       if (used)
> > +                               set_bit(base, tt->ss_bit_map);
> > +                       else
> > +                               clear_bit(base, tt->ss_bit_map);
> > +
> > +                       for (j = 0; j < sch_ep->cs_count; j++) {
> > +                               if (used) {
> > +                                       set_bit(base + 2 + j, tt->cs_bit_map);
> > +
> > +                                       tt->fs_bus_bw[base + 2 + j] +=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               } else {
> > +                                       clear_bit(base + 2 + j, tt->cs_bit_map);
> > +                                       tt->fs_bus_bw[base + 2 + j] -=
> > +                                               sch_ep->bw_cost_per_microframe;
> > +                               }
> > +                       }
> > +               }
> >         }
> >
> > -       list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> > +       if (used)
> > +               list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
> > +       else
> > +               list_del(&sch_ep->tt_endpoint);
> >  }
> >
> >  static int check_sch_bw(struct usb_device *udev,
> > @@ -470,6 +529,7 @@ static int check_sch_bw(struct usb_device *udev,
> >  {
> >         u32 offset;
> >         u32 esit;
> > +       u32 boundary;
> >         u32 min_bw;
> >         u32 min_index;
> >         u32 worst_bw;
> > @@ -487,10 +547,13 @@ static int check_sch_bw(struct usb_device *udev,
> >          */
> >         min_bw = ~0;
> >         min_index = 0;
> > +       boundary = esit;
> >         min_cs_count = sch_ep->cs_count;
> >         min_num_budget = sch_ep->num_budget_microframes;
> >         for (offset = 0; offset < esit; offset++) {
> >                 if (is_fs_or_ls(udev->speed)) {
> > +                       if (sch_ep->ep_type != ISOC_OUT_EP)
> > +                               boundary = esit + 1;
> >                         ret = check_sch_tt(udev, sch_ep, offset);
> >                         if (ret)
> >                                 continue;
> > @@ -498,7 +561,7 @@ static int check_sch_bw(struct usb_device *udev,
> >                                 tt_offset_ok = true;
> >                 }
> >
> > -               if ((offset + sch_ep->num_budget_microframes) > sch_ep->esit)
> > +               if ((offset + sch_ep->num_budget_microframes) > boundary)
> >                         break;
> >
> >                 worst_bw = get_max_bw(sch_bw, sch_ep, offset);
> > @@ -532,7 +595,7 @@ static int check_sch_bw(struct usb_device *udev,
> >                 if (!tt_offset_ok)
> >                         return -ERANGE;
> >
> > -               update_sch_tt(udev, sch_ep);
> > +               update_sch_tt(udev, sch_ep, 1);
> >         }
> >
> >         /* update bus bandwidth info */
> > @@ -696,12 +759,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
> >
> >         list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
> >                 if (sch_ep->ep == ep) {
> > -                       update_bus_bw(sch_bw, sch_ep, 0);
> > -                       list_del(&sch_ep->endpoint);
> >                         if (is_fs_or_ls(udev->speed)) {
> > -                               list_del(&sch_ep->tt_endpoint);
> > +                               update_sch_tt(udev, sch_ep, 0);
> >                                 drop_tt(udev);
> >                         }
> > +                       update_bus_bw(sch_bw, sch_ep, 0);
> > +                       list_del(&sch_ep->endpoint);
> >                         kfree(sch_ep);
> >                         break;
> >                 }
> > diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
> > index a93cfe817904..323b281933b9 100644
> > --- a/drivers/usb/host/xhci-mtk.h
> > +++ b/drivers/usb/host/xhci-mtk.h
> > @@ -21,12 +21,18 @@
> >
> >  /**
> >   * @split_bit_map: used to avoid split microframes overlay
> > + * @ss_bit_map: used to avoid start split microframes overlay
> > + * @cs_bit_map: used to avoid complete split microframes overlay
> > + * @fs_bus_bw: array to keep track of bandwidth already used at full speed
> >   * @ep_list: Endpoints using this TT
> >   * @usb_tt: usb TT related
> >   * @tt_port: TT port number
> >   */
> >  struct mu3h_sch_tt {
> >         DECLARE_BITMAP(split_bit_map, XHCI_MTK_MAX_ESIT);
> > +       DECLARE_BITMAP(ss_bit_map, XHCI_MTK_MAX_ESIT);
> > +       DECLARE_BITMAP(cs_bit_map, XHCI_MTK_MAX_ESIT + 1);
> > +       u32 fs_bus_bw[XHCI_MTK_MAX_ESIT];
> >         struct list_head ep_list;
> >         struct usb_tt *usb_tt;
> >         int tt_port;
> > @@ -42,7 +48,7 @@ struct mu3h_sch_tt {
> >   * two bandwidth domains, one for IN eps and another for OUT eps.
> >   */
> >  struct mu3h_sch_bw_info {
> > -       u32 bus_bw[XHCI_MTK_MAX_ESIT];
> > +       u32 bus_bw[XHCI_MTK_MAX_ESIT + 1];
> >         struct list_head bw_ep_list;
> >  };
> >
> > --
> > 2.18.0
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

end of thread, other threads:[~2021-01-19  7:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-22  9:34 [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT Chunfeng Yun
2020-12-22  9:34 ` Chunfeng Yun
2020-12-22  9:34 ` [RFC PATCH v3 2/5] usb: xhci-mtk: add mt8192 wakeup Chunfeng Yun
2020-12-22  9:34   ` Chunfeng Yun
2020-12-22  9:34 ` [RFC PATCH v3 3/5] usb: xhci-mtk: fix random remote wakeup Chunfeng Yun
2020-12-22  9:34   ` Chunfeng Yun
2020-12-22  9:34 ` [RFC PATCH v3 4/5] usb: xhci-mtk: add support runtime pm Chunfeng Yun
2020-12-22  9:34   ` Chunfeng Yun
2020-12-29  7:38   ` Ikjoon Jang
2020-12-29  7:38     ` Ikjoon Jang
2021-01-05  7:13     ` Chunfeng Yun
2021-01-05  7:13       ` Chunfeng Yun
2020-12-22  9:34 ` [RFC PATCH v3 5/5] arm64: dts: mt8192: add SSUSB related nodes Chunfeng Yun
2020-12-22  9:34   ` Chunfeng Yun
2021-01-13  9:43 ` [RFC PATCH v3 1/5] usb: xhci-mtk: improve bandwidth scheduling with multi-TT Ikjoon Jang
2021-01-13  9:43   ` Ikjoon Jang
2021-01-19  6:18   ` 答复: " Yaqii Wu (武亚奇)
2021-01-19  6:57   ` Chunfeng Yun
2021-01-19  6:57     ` Chunfeng Yun

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.