[PATCH v2 0/3] Release allocated periodic bandwidth data from reset_bandwidth()

[PATCH v2 0/3] Release allocated periodic bandwidth data from reset_bandwidth()

[Ikjoon Jang]

xhci-mtk releases allocated TT bandwidth data only when whole
endpoints of a device are dropped as there're only {add|drop}_endpoint()
hooks are defined. This patchset adds more hooks and releases all
bandwidth data from reset_bandwidth() path, not drop_endpoint().


Changes in v2:
- fix a 0-day warning from unused variable
- split one big patch into three patches
- bugfix in hw flags

Ikjoon Jang (3):
  usb: xhci-mtk: code cleanups in getting bandwidth table
  usb: xhci-mtk: delay association of tt and ep
  usb: xhci-mtk: fix unreleased bandwidth data

 drivers/usb/host/xhci-mtk-sch.c | 180 ++++++++++++++++++++------------
 drivers/usb/host/xhci-mtk.h     |  13 +++
 drivers/usb/host/xhci.c         |   9 ++
 3 files changed, 133 insertions(+), 69 deletions(-)

-- 
2.29.2.576.ga3fc446d84-goog

[PATCH v2 1/3] usb: xhci-mtk: code cleanups in getting bandwidth table

[Ikjoon Jang]

Simplifies the codes for getting internal bandwidth data,
No functional changes.

Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
---

 drivers/usb/host/xhci-mtk-sch.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 45c54d56ecbd..c334b6d76479 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -49,9 +49,11 @@ static int is_fs_or_ls(enum usb_device_speed speed)
 * so the bandwidth domain array is organized as follow for simplification:
 * SSport0-OUT, SSport0-IN, ..., SSportX-OUT, SSportX-IN, HSport0, ..., HSportY
 */
-static int get_bw_index(struct xhci_hcd *xhci, struct usb_device *udev,
-	struct usb_host_endpoint *ep)
+static struct mu3h_sch_bw_info *get_bw_info(struct xhci_hcd_mtk *mtk,
+					     struct usb_device *udev,
+					     struct usb_host_endpoint *ep)
 {
+	struct xhci_hcd *xhci = hcd_to_xhci(mtk->hcd);
 	struct xhci_virt_device *virt_dev;
 	int bw_index;
 
@@ -67,7 +69,7 @@ static int get_bw_index(struct xhci_hcd *xhci, struct usb_device *udev,
 		bw_index = virt_dev->real_port + xhci->usb3_rhub.num_ports - 1;
 	}
 
-	return bw_index;
+	return &mtk->sch_array[bw_index];
 }
 
 static u32 get_esit(struct xhci_ep_ctx *ep_ctx)
@@ -603,9 +605,7 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_virt_device *virt_dev;
 	struct mu3h_sch_bw_info *sch_bw;
 	struct mu3h_sch_ep_info *sch_ep;
-	struct mu3h_sch_bw_info *sch_array;
 	unsigned int ep_index;
-	int bw_index;
 	int ret = 0;
 
 	xhci = hcd_to_xhci(hcd);
@@ -613,7 +613,6 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 	ep_index = xhci_get_endpoint_index(&ep->desc);
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
 	ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->in_ctx, ep_index);
-	sch_array = mtk->sch_array;
 
 	xhci_dbg(xhci, "%s() type:%d, speed:%d, mpkt:%d, dir:%d, ep:%p\n",
 		__func__, usb_endpoint_type(&ep->desc), udev->speed,
@@ -632,8 +631,7 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 		return 0;
 	}
 
-	bw_index = get_bw_index(xhci, udev, ep);
-	sch_bw = &sch_array[bw_index];
+	sch_bw = get_bw_info(mtk, udev, ep);
 
 	sch_ep = create_sch_ep(udev, ep, ep_ctx);
 	if (IS_ERR_OR_NULL(sch_ep))
@@ -673,15 +671,12 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 	struct xhci_hcd *xhci;
 	struct xhci_slot_ctx *slot_ctx;
 	struct xhci_virt_device *virt_dev;
-	struct mu3h_sch_bw_info *sch_array;
 	struct mu3h_sch_bw_info *sch_bw;
 	struct mu3h_sch_ep_info *sch_ep;
-	int bw_index;
 
 	xhci = hcd_to_xhci(hcd);
 	virt_dev = xhci->devs[udev->slot_id];
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
-	sch_array = mtk->sch_array;
 
 	xhci_dbg(xhci, "%s() type:%d, speed:%d, mpks:%d, dir:%d, ep:%p\n",
 		__func__, usb_endpoint_type(&ep->desc), udev->speed,
@@ -691,8 +686,7 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 	if (!need_bw_sch(ep, udev->speed, slot_ctx->tt_info & TT_SLOT))
 		return;
 
-	bw_index = get_bw_index(xhci, udev, ep);
-	sch_bw = &sch_array[bw_index];
+	sch_bw = get_bw_info(mtk, udev, ep);
 
 	list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
 		if (sch_ep->ep == ep) {
-- 
2.29.2.576.ga3fc446d84-goog

[PATCH v2 2/3] usb: xhci-mtk: delay association of tt and ep

[Ikjoon Jang]

xhci-mtk creates internal data structures for representing the
relationship between endpoint and TT.

This patch simply delays its association between endpoint and TT
when it's really loaded onto internal bandwidth table.

This is a preparation step for fixing unreleased periodic
TT bandwidth data, no functional changes.

Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
---

 drivers/usb/host/xhci-mtk-sch.c | 40 ++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index c334b6d76479..439391f1dc78 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -174,7 +174,6 @@ static struct mu3h_sch_ep_info *create_sch_ep(struct usb_device *udev,
 	struct usb_host_endpoint *ep, struct xhci_ep_ctx *ep_ctx)
 {
 	struct mu3h_sch_ep_info *sch_ep;
-	struct mu3h_sch_tt *tt = NULL;
 	u32 len_bw_budget_table;
 	size_t mem_size;
 
@@ -192,15 +191,6 @@ static struct mu3h_sch_ep_info *create_sch_ep(struct usb_device *udev,
 	if (!sch_ep)
 		return ERR_PTR(-ENOMEM);
 
-	if (is_fs_or_ls(udev->speed)) {
-		tt = find_tt(udev);
-		if (IS_ERR(tt)) {
-			kfree(sch_ep);
-			return ERR_PTR(-ENOMEM);
-		}
-	}
-
-	sch_ep->sch_tt = tt;
 	sch_ep->ep = ep;
 
 	return sch_ep;
@@ -377,10 +367,10 @@ static void update_bus_bw(struct mu3h_sch_bw_info *sch_bw,
 	}
 }
 
-static int check_sch_tt(struct usb_device *udev,
-	struct mu3h_sch_ep_info *sch_ep, u32 offset)
+static int check_sch_tt(struct mu3h_sch_tt *tt,
+			struct mu3h_sch_ep_info *sch_ep,
+			u32 offset)
 {
-	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
 	u32 extra_cs_count;
 	u32 fs_budget_start;
 	u32 start_ss, last_ss;
@@ -450,10 +440,9 @@ static int check_sch_tt(struct usb_device *udev,
 	return 0;
 }
 
-static void update_sch_tt(struct usb_device *udev,
-	struct mu3h_sch_ep_info *sch_ep)
+static void update_sch_tt(struct mu3h_sch_tt *tt,
+			  struct mu3h_sch_ep_info *sch_ep)
 {
-	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
 	u32 base, num_esit;
 	int i, j;
 
@@ -465,10 +454,12 @@ static void update_sch_tt(struct usb_device *udev,
 	}
 
 	list_add_tail(&sch_ep->tt_endpoint, &tt->ep_list);
+	sch_ep->sch_tt = tt;
 }
 
 static int check_sch_bw(struct usb_device *udev,
-	struct mu3h_sch_bw_info *sch_bw, struct mu3h_sch_ep_info *sch_ep)
+			struct mu3h_sch_bw_info *sch_bw,
+			struct mu3h_sch_ep_info *sch_ep)
 {
 	u32 offset;
 	u32 esit;
@@ -480,8 +471,14 @@ static int check_sch_bw(struct usb_device *udev,
 	u32 min_cs_count;
 	bool tt_offset_ok = false;
 	int ret;
+	struct mu3h_sch_tt *tt = NULL;
 
 	esit = sch_ep->esit;
+	if (is_fs_or_ls(udev->speed)) {
+		tt = find_tt(udev);
+		if (IS_ERR(tt))
+			return -ENOMEM;
+	}
 
 	/*
 	 * Search through all possible schedule microframes.
@@ -493,7 +490,7 @@ static int check_sch_bw(struct usb_device *udev,
 	min_num_budget = sch_ep->num_budget_microframes;
 	for (offset = 0; offset < esit; offset++) {
 		if (is_fs_or_ls(udev->speed)) {
-			ret = check_sch_tt(udev, sch_ep, offset);
+			ret = check_sch_tt(tt, sch_ep, offset);
 			if (ret)
 				continue;
 			else
@@ -531,10 +528,11 @@ static int check_sch_bw(struct usb_device *udev,
 
 	if (is_fs_or_ls(udev->speed)) {
 		/* all offset for tt is not ok*/
-		if (!tt_offset_ok)
+		if (!tt_offset_ok) {
+			drop_tt(udev);
 			return -ERANGE;
-
-		update_sch_tt(udev, sch_ep);
+		}
+		update_sch_tt(tt, sch_ep);
 	}
 
 	/* update bus bandwidth info */
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH v2 2/3] usb: xhci-mtk: delay association of tt and ep

[Greg Kroah-Hartman]

On Thu, Dec 10, 2020 at 06:47:46PM +0800, Ikjoon Jang wrote:
> xhci-mtk creates internal data structures for representing the
> relationship between endpoint and TT.
> 
> This patch simply delays its association between endpoint and TT
> when it's really loaded onto internal bandwidth table.
> 
> This is a preparation step for fixing unreleased periodic
> TT bandwidth data, no functional changes.

So the next fix requires this on?  You might want to say that in the
next patch as well :)

thanks,

greg k-h

[PATCH v2 3/3] usb: xhci-mtk: fix unreleased bandwidth data

[Ikjoon Jang]

xhci-mtk has hooks on add_endpoint() and drop_endpoint() from xhci
to handle its own sw bandwidth managements and stores bandwidth data
into internal table every time add_endpoint() is called,
so when bandwidth allocation fails at one endpoint, all earlier
allocation from the same interface could still remain at the table.

This patch adds two more hooks from check_bandwidth() and
reset_bandwidth(), and make mtk-xhci to releases all failed endpoints
from reset_bandwidth().

Fixes: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
Signed-off-by: Ikjoon Jang <ikjn@chromium.org>

---

Changes in v2:
- fix wrong offset in mediatek hw flags
- fix 0-day warnings

 drivers/usb/host/xhci-mtk-sch.c | 120 ++++++++++++++++++++++----------
 drivers/usb/host/xhci-mtk.h     |  13 ++++
 drivers/usb/host/xhci.c         |   9 +++
 3 files changed, 107 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 439391f1dc78..102d8e0a50f1 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -583,6 +583,8 @@ int xhci_mtk_sch_init(struct xhci_hcd_mtk *mtk)
 
 	mtk->sch_array = sch_array;
 
+	INIT_LIST_HEAD(&mtk->bw_ep_list_new);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xhci_mtk_sch_init);
@@ -597,16 +599,14 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 		struct usb_host_endpoint *ep)
 {
 	struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
-	struct xhci_hcd *xhci;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 	struct xhci_ep_ctx *ep_ctx;
 	struct xhci_slot_ctx *slot_ctx;
 	struct xhci_virt_device *virt_dev;
 	struct mu3h_sch_bw_info *sch_bw;
 	struct mu3h_sch_ep_info *sch_ep;
 	unsigned int ep_index;
-	int ret = 0;
 
-	xhci = hcd_to_xhci(hcd);
 	virt_dev = xhci->devs[udev->slot_id];
 	ep_index = xhci_get_endpoint_index(&ep->desc);
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
@@ -637,42 +637,37 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 
 	setup_sch_info(udev, ep_ctx, sch_ep);
 
-	ret = check_sch_bw(udev, sch_bw, sch_ep);
-	if (ret) {
-		xhci_err(xhci, "Not enough bandwidth!\n");
-		if (is_fs_or_ls(udev->speed))
-			drop_tt(udev);
-
-		kfree(sch_ep);
-		return -ENOSPC;
-	}
+	list_add_tail(&sch_ep->endpoint, &mtk->bw_ep_list_new);
 
-	list_add_tail(&sch_ep->endpoint, &sch_bw->bw_ep_list);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_mtk_add_ep_quirk);
 
-	ep_ctx->reserved[0] |= cpu_to_le32(EP_BPKTS(sch_ep->pkts)
-		| EP_BCSCOUNT(sch_ep->cs_count) | EP_BBM(sch_ep->burst_mode));
-	ep_ctx->reserved[1] |= cpu_to_le32(EP_BOFFSET(sch_ep->offset)
-		| EP_BREPEAT(sch_ep->repeat));
+static void xhci_mtk_drop_ep(struct xhci_hcd_mtk *mtk, struct usb_device *udev,
+			     struct mu3h_sch_ep_info *sch_ep)
+{
+	struct mu3h_sch_bw_info *sch_bw = get_bw_info(mtk, udev, sch_ep->ep);
 
-	xhci_dbg(xhci, " PKTS:%x, CSCOUNT:%x, BM:%x, OFFSET:%x, REPEAT:%x\n",
-			sch_ep->pkts, sch_ep->cs_count, sch_ep->burst_mode,
-			sch_ep->offset, sch_ep->repeat);
+	update_bus_bw(sch_bw, sch_ep, 0);
+	list_del(&sch_ep->endpoint);
 
-	return 0;
+	if (sch_ep->sch_tt) {
+		list_del(&sch_ep->tt_endpoint);
+		drop_tt(udev);
+	}
+	kfree(sch_ep);
 }
-EXPORT_SYMBOL_GPL(xhci_mtk_add_ep_quirk);
 
 void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
-		struct usb_host_endpoint *ep)
+			    struct usb_host_endpoint *ep)
 {
 	struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
-	struct xhci_hcd *xhci;
-	struct xhci_slot_ctx *slot_ctx;
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 	struct xhci_virt_device *virt_dev;
+	struct xhci_slot_ctx *slot_ctx;
 	struct mu3h_sch_bw_info *sch_bw;
-	struct mu3h_sch_ep_info *sch_ep;
+	struct mu3h_sch_ep_info *sch_ep, *tmp;
 
-	xhci = hcd_to_xhci(hcd);
 	virt_dev = xhci->devs[udev->slot_id];
 	slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx);
 
@@ -686,17 +681,72 @@ void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 
 	sch_bw = get_bw_info(mtk, udev, ep);
 
-	list_for_each_entry(sch_ep, &sch_bw->bw_ep_list, endpoint) {
+	list_for_each_entry_safe(sch_ep, tmp, &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);
-				drop_tt(udev);
-			}
-			kfree(sch_ep);
+			xhci_mtk_drop_ep(mtk, udev, sch_ep);
 			break;
 		}
 	}
 }
 EXPORT_SYMBOL_GPL(xhci_mtk_drop_ep_quirk);
+
+int xhci_mtk_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
+	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	struct xhci_virt_device *virt_dev = xhci->devs[udev->slot_id];
+	struct mu3h_sch_ep_info *sch_ep, *tmp;
+
+	dev_dbg(&udev->dev, "%s\n", __func__);
+
+	list_for_each_entry(sch_ep, &mtk->bw_ep_list_new, endpoint) {
+		int ret;
+		struct mu3h_sch_bw_info *sch_bw;
+
+		sch_bw = get_bw_info(mtk, udev, sch_ep->ep);
+
+		ret = check_sch_bw(udev, sch_bw, sch_ep);
+		if (ret) {
+			xhci_err(xhci, "Not enough bandwidth!\n");
+			return ret;
+		}
+	}
+
+	list_for_each_entry_safe(sch_ep, tmp, &mtk->bw_ep_list_new, endpoint) {
+		struct mu3h_sch_bw_info *sch_bw;
+		struct xhci_ep_ctx *ep_ctx;
+		struct usb_host_endpoint *ep = sch_ep->ep;
+		unsigned int ep_index = xhci_get_endpoint_index(&ep->desc);
+
+		sch_bw = get_bw_info(mtk, udev, ep);
+
+		list_move_tail(&sch_ep->endpoint, &sch_bw->bw_ep_list);
+
+		ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->in_ctx, ep_index);
+		ep_ctx->reserved[0] |= cpu_to_le32(EP_BPKTS(sch_ep->pkts)
+			| EP_BCSCOUNT(sch_ep->cs_count)
+			| EP_BBM(sch_ep->burst_mode));
+		ep_ctx->reserved[1] |= cpu_to_le32(EP_BOFFSET(sch_ep->offset)
+			| EP_BREPEAT(sch_ep->repeat));
+
+		xhci_dbg(xhci, " PKTS:%x, CSCOUNT:%x, BM:%x, OFFSET:%x, REPEAT:%x\n",
+			sch_ep->pkts, sch_ep->cs_count, sch_ep->burst_mode,
+			sch_ep->offset, sch_ep->repeat);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_mtk_check_bandwidth);
+
+void xhci_mtk_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
+{
+	struct xhci_hcd_mtk *mtk = hcd_to_mtk(hcd);
+	struct mu3h_sch_ep_info *sch_ep, *tmp;
+
+	dev_dbg(&udev->dev, "%s\n", __func__);
+
+	list_for_each_entry_safe(sch_ep, tmp, &mtk->bw_ep_list_new, endpoint) {
+		xhci_mtk_drop_ep(mtk, udev, sch_ep);
+	}
+}
+EXPORT_SYMBOL_GPL(xhci_mtk_reset_bandwidth);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 8be8c5f7ff62..05ca989985fc 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -130,6 +130,7 @@ struct mu3c_ippc_regs {
 struct xhci_hcd_mtk {
 	struct device *dev;
 	struct usb_hcd *hcd;
+	struct list_head bw_ep_list_new;
 	struct mu3h_sch_bw_info *sch_array;
 	struct mu3c_ippc_regs __iomem *ippc_regs;
 	bool has_ippc;
@@ -165,6 +166,8 @@ int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 		struct usb_host_endpoint *ep);
 void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd, struct usb_device *udev,
 		struct usb_host_endpoint *ep);
+int xhci_mtk_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev);
+void xhci_mtk_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev);
 
 #else
 static inline int xhci_mtk_add_ep_quirk(struct usb_hcd *hcd,
@@ -178,6 +181,16 @@ static inline void xhci_mtk_drop_ep_quirk(struct usb_hcd *hcd,
 {
 }
 
+static inline int xhci_mtk_check_bandwidth(struct usb_hcd *hcd,
+		struct usb_device *udev)
+{
+	return 0;
+}
+
+static inline void xhci_mtk_reset_bandwidth(struct usb_hcd *hcd,
+		struct usb_device *udev)
+{
+}
 #endif
 
 #endif		/* _XHCI_MTK_H_ */
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2bf6c526ac7a..5a9e01b33688 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2854,6 +2854,12 @@ static int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 	xhci_dbg(xhci, "%s called for udev %p\n", __func__, udev);
 	virt_dev = xhci->devs[udev->slot_id];
 
+	if (xhci->quirks & XHCI_MTK_HOST) {
+		ret = xhci_mtk_check_bandwidth(hcd, udev);
+		if (ret < 0)
+			return ret;
+	}
+
 	command = xhci_alloc_command(xhci, true, GFP_KERNEL);
 	if (!command)
 		return -ENOMEM;
@@ -2941,6 +2947,9 @@ static void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 		return;
 	xhci = hcd_to_xhci(hcd);
 
+	if (xhci->quirks & XHCI_MTK_HOST)
+		xhci_mtk_reset_bandwidth(hcd, udev);
+
 	xhci_dbg(xhci, "%s called for udev %p\n", __func__, udev);
 	virt_dev = xhci->devs[udev->slot_id];
 	/* Free any rings allocated for added endpoints */
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH v2 3/3] usb: xhci-mtk: fix unreleased bandwidth data

[Greg Kroah-Hartman]

On Thu, Dec 10, 2020 at 06:47:47PM +0800, Ikjoon Jang wrote:
> xhci-mtk has hooks on add_endpoint() and drop_endpoint() from xhci
> to handle its own sw bandwidth managements and stores bandwidth data
> into internal table every time add_endpoint() is called,
> so when bandwidth allocation fails at one endpoint, all earlier
> allocation from the same interface could still remain at the table.
> 
> This patch adds two more hooks from check_bandwidth() and
> reset_bandwidth(), and make mtk-xhci to releases all failed endpoints
> from reset_bandwidth().
> 
> Fixes: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> Signed-off-by: Ikjoon Jang <ikjn@chromium.org>

Shouldn't this be the first patch in the series?  You don't want a fix
to be dependent on code style changes, otherwise it is really really
hard to backport it to older kernels that might need this fix, right?

Can you re-order these patches please?

thanks,

greg k-h

Re: [PATCH v2 3/3] usb: xhci-mtk: fix unreleased bandwidth data

[Ikjoon Jang]

On Thu, Dec 10, 2020 at 6:57 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Dec 10, 2020 at 06:47:47PM +0800, Ikjoon Jang wrote:
> > xhci-mtk has hooks on add_endpoint() and drop_endpoint() from xhci
> > to handle its own sw bandwidth managements and stores bandwidth data
> > into internal table every time add_endpoint() is called,
> > so when bandwidth allocation fails at one endpoint, all earlier
> > allocation from the same interface could still remain at the table.
> >
> > This patch adds two more hooks from check_bandwidth() and
> > reset_bandwidth(), and make mtk-xhci to releases all failed endpoints
> > from reset_bandwidth().
> >
> > Fixes: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> > Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
>
> Shouldn't this be the first patch in the series?  You don't want a fix
> to be dependent on code style changes, otherwise it is really really
> hard to backport it to older kernels that might need this fix, right?

yes, you're right.

This patchset also requires
"4b0f7a77fb3c usb: xhci-mtk: supports bandwidth scheduling with multi-TT",
which doesn't have a Fixes tag.

I think I can change Fixes to point to that commit (4b0f7a77fb3c),
as this unreleased bandwidth data is much more impactful to
TT bandwidth.

Thanks!

>
> Can you re-order these patches please?
>
> thanks,
>
> greg k-h

Re: [PATCH v2 0/3] Release allocated periodic bandwidth data from reset_bandwidth()

[Chunfeng Yun]

On Thu, 2020-12-10 at 18:47 +0800, Ikjoon Jang wrote:
> xhci-mtk releases allocated TT bandwidth data only when whole
> endpoints of a device are dropped as there're only {add|drop}_endpoint()
> hooks are defined. This patchset adds more hooks and releases all
> bandwidth data from reset_bandwidth() path, not drop_endpoint().
> 
> 
> Changes in v2:
> - fix a 0-day warning from unused variable
> - split one big patch into three patches
> - bugfix in hw flags
> 
> Ikjoon Jang (3):
>   usb: xhci-mtk: code cleanups in getting bandwidth table
>   usb: xhci-mtk: delay association of tt and ep
>   usb: xhci-mtk: fix unreleased bandwidth data
> 
>  drivers/usb/host/xhci-mtk-sch.c | 180 ++++++++++++++++++++------------
>  drivers/usb/host/xhci-mtk.h     |  13 +++
>  drivers/usb/host/xhci.c         |   9 ++
>  3 files changed, 133 insertions(+), 69 deletions(-)
Thanks for your patch, I'll test it and check it with our DE

> 

Re: [PATCH v2 0/3] Release allocated periodic bandwidth data from reset_bandwidth()

[Ikjoon Jang]

On Fri, Dec 11, 2020 at 9:53 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Thu, 2020-12-10 at 18:47 +0800, Ikjoon Jang wrote:
> > xhci-mtk releases allocated TT bandwidth data only when whole
> > endpoints of a device are dropped as there're only {add|drop}_endpoint()
> > hooks are defined. This patchset adds more hooks and releases all
> > bandwidth data from reset_bandwidth() path, not drop_endpoint().
> >
> >
> > Changes in v2:
> > - fix a 0-day warning from unused variable
> > - split one big patch into three patches
> > - bugfix in hw flags
> >
> > Ikjoon Jang (3):
> >   usb: xhci-mtk: code cleanups in getting bandwidth table
> >   usb: xhci-mtk: delay association of tt and ep
> >   usb: xhci-mtk: fix unreleased bandwidth data
> >
> >  drivers/usb/host/xhci-mtk-sch.c | 180 ++++++++++++++++++++------------
> >  drivers/usb/host/xhci-mtk.h     |  13 +++
> >  drivers/usb/host/xhci.c         |   9 ++
> >  3 files changed, 133 insertions(+), 69 deletions(-)
> Thanks for your patch, I'll test it and check it with our DE

Thanks, I will upload v3.
But I don't expect any logic changes from v2.
Can you please give me feedback on v2 if you find anything?

>
> >
>

Re: [PATCH v2 0/3] Release allocated periodic bandwidth data from reset_bandwidth()

[Chunfeng Yun]

On Fri, 2020-12-11 at 14:36 +0800, Ikjoon Jang wrote:
> On Fri, Dec 11, 2020 at 9:53 AM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Thu, 2020-12-10 at 18:47 +0800, Ikjoon Jang wrote:
> > > xhci-mtk releases allocated TT bandwidth data only when whole
> > > endpoints of a device are dropped as there're only {add|drop}_endpoint()
> > > hooks are defined. This patchset adds more hooks and releases all
> > > bandwidth data from reset_bandwidth() path, not drop_endpoint().
> > >
> > >
> > > Changes in v2:
> > > - fix a 0-day warning from unused variable
> > > - split one big patch into three patches
> > > - bugfix in hw flags
> > >
> > > Ikjoon Jang (3):
> > >   usb: xhci-mtk: code cleanups in getting bandwidth table
> > >   usb: xhci-mtk: delay association of tt and ep
> > >   usb: xhci-mtk: fix unreleased bandwidth data
> > >
> > >  drivers/usb/host/xhci-mtk-sch.c | 180 ++++++++++++++++++++------------
> > >  drivers/usb/host/xhci-mtk.h     |  13 +++
> > >  drivers/usb/host/xhci.c         |   9 ++
> > >  3 files changed, 133 insertions(+), 69 deletions(-)
> > Thanks for your patch, I'll test it and check it with our DE
> 
> Thanks, I will upload v3.
> But I don't expect any logic changes from v2.
> Can you please give me feedback on v2 if you find anything?
Ok

> 
> >
> > >
> >