diff mbox series

[1/2] usb: xhci-mtk: fix fs isoc's transfer error

Message ID 20220512064931.31670-1-chunfeng.yun@mediatek.com (mailing list archive)
State New
Headers show
Series [1/2] usb: xhci-mtk: fix fs isoc's transfer error | expand

Commit Message

Chunfeng Yun May 12, 2022, 6:49 a.m. UTC
Due to the scheduler allocates the optimal bandwidth for FS ISOC endpoints,
this may be not enough actually and causes data transfer error, so come up
with an estimate that is no less than the worst case bandwidth used for
any one mframe, but may be an over-estimate.

Fixes: 451d3912586a ("usb: xhci-mtk: update fs bus bandwidth by bw_budget_table")
Cc: stable@vger.kernel.org
Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk-sch.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

AngeloGioacchino Del Regno May 18, 2022, 12:01 p.m. UTC | #1
Il 12/05/22 08:49, Chunfeng Yun ha scritto:
> Due to the scheduler allocates the optimal bandwidth for FS ISOC endpoints,
> this may be not enough actually and causes data transfer error, so come up
> with an estimate that is no less than the worst case bandwidth used for
> any one mframe, but may be an over-estimate.
> 
> Fixes: 451d3912586a ("usb: xhci-mtk: update fs bus bandwidth by bw_budget_table")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

Hello Chunfeng,
I agree this is "a fix"... but is it the best fix?

Shooting the bandwidth very high will have power consumption consequences, are
those measurable?
And if they are, what is the expected power consumption increase in percentage
(and/or microamperes)? Also, out of the expected increase, have you got any
measurement for that?

Assuming that the measurement is done for one SoC, it's possible to make some
assumption about a different part.

Regards,
Angelo

> ---
>   drivers/usb/host/xhci-mtk-sch.c | 16 +++++++---------
>   1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
> index f3139ce7b0a9..953d2cd1d4cc 100644
> --- a/drivers/usb/host/xhci-mtk-sch.c
> +++ b/drivers/usb/host/xhci-mtk-sch.c
Chunfeng Yun May 23, 2022, 8:55 a.m. UTC | #2
On Wed, 2022-05-18 at 14:01 +0200, AngeloGioacchino Del Regno wrote:
> Il 12/05/22 08:49, Chunfeng Yun ha scritto:
> > Due to the scheduler allocates the optimal bandwidth for FS ISOC
> > endpoints,
> > this may be not enough actually and causes data transfer error, so
> > come up
> > with an estimate that is no less than the worst case bandwidth used
> > for
> > any one mframe, but may be an over-estimate.
> > 
> > Fixes: 451d3912586a ("usb: xhci-mtk: update fs bus bandwidth by
> > bw_budget_table")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> 
> Hello Chunfeng,
> I agree this is "a fix"... but is it the best fix?
> 
> Shooting the bandwidth very high will have power consumption
> consequences, are
> those measurable?
This is usually limited into one interval; e.g. the last interval
transfers 8 bytes in fact, but I assume it may transfer 188 bytes, I
think the consumption increase can be ignored.

> And if they are, what is the expected power consumption increase in
> percentage
> (and/or microamperes)? Also, out of the expected increase, have you
> got any
> measurement for that?
> 
> Assuming that the measurement is done for one SoC, it's possible to
> make some
> assumption about a different part.
> 
> Regards,
> Angelo
> 
> > ---
> >   drivers/usb/host/xhci-mtk-sch.c | 16 +++++++---------
> >   1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci-mtk-sch.c
> > b/drivers/usb/host/xhci-mtk-sch.c
> > index f3139ce7b0a9..953d2cd1d4cc 100644
> > --- a/drivers/usb/host/xhci-mtk-sch.c
> > +++ b/drivers/usb/host/xhci-mtk-sch.c
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index f3139ce7b0a9..953d2cd1d4cc 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -464,7 +464,7 @@  static int check_fs_bus_bw(struct mu3h_sch_ep_info *sch_ep, int offset)
 		 */
 		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
 			k = XHCI_MTK_BW_INDEX(base + j);
-			tmp = tt->fs_bus_bw[k] + sch_ep->bw_budget_table[j];
+			tmp = tt->fs_bus_bw[k] + sch_ep->bw_cost_per_microframe;
 			if (tmp > FS_PAYLOAD_MAX)
 				return -ESCH_BW_OVERFLOW;
 		}
@@ -538,19 +538,17 @@  static int check_sch_tt(struct mu3h_sch_ep_info *sch_ep, u32 offset)
 static void update_sch_tt(struct mu3h_sch_ep_info *sch_ep, bool used)
 {
 	struct mu3h_sch_tt *tt = sch_ep->sch_tt;
+	int bw_updated;
 	u32 base;
-	int i, j, k;
+	int i, j;
+
+	bw_updated = sch_ep->bw_cost_per_microframe * (used ? 1 : -1);
 
 	for (i = 0; i < sch_ep->num_esit; i++) {
 		base = sch_ep->offset + i * sch_ep->esit;
 
-		for (j = 0; j < sch_ep->num_budget_microframes; j++) {
-			k = XHCI_MTK_BW_INDEX(base + j);
-			if (used)
-				tt->fs_bus_bw[k] += sch_ep->bw_budget_table[j];
-			else
-				tt->fs_bus_bw[k] -= sch_ep->bw_budget_table[j];
-		}
+		for (j = 0; j < sch_ep->num_budget_microframes; j++)
+			tt->fs_bus_bw[XHCI_MTK_BW_INDEX(base + j)] += bw_updated;
 	}
 
 	if (used)