All of lore.kernel.org
 help / color / mirror / Atom feed
* usb: dwc2: gadget: ISOC's starting flow improvement
@ 2018-07-26 11:25 Minas Harutyunyan
  0 siblings, 0 replies; 5+ messages in thread
From: Minas Harutyunyan @ 2018-07-26 11:25 UTC (permalink / raw)
  To: Felipe Balbi, Minas Harutyunyan, Greg Kroah-Hartman, linux-usb; +Cc: John Youn

Hi Felipe,

This patch should be applied after follow patch from Artur Petrosyan:

"[PATCH] usb: dwc2: Change reading of current frame number flow."

Thanks,
Minas

On 7/26/2018 2:45 PM, Felipe Balbi wrote:
> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
> 
>> To start ISOC transfers in handlers dwc2_gadget_handle_nak() and
>> dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame
>> number, based on which, set target frame number to start first ISOC
>> transfer.
>>
>> In case if system's high IRQ latency and multiple EP's asserted
>> interrupt in same frame, there are high probability that when reading
>> current frame number in EP's handlers, actual frame number can be
>> increased. As result for bInterval > 1, starting target frame
>> will be set wrongly and all ISOC packets will be dropped.
>>
>> In patch "usb: dwc2: Change reading of current frame number flow"
>> reading of current frame number done ASAP in common interrupt handler.
>> This frame number stored in frame_number variable which used as
>> starting frame number for ISOC EP's in above mentioned handlers.
>>
>> Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
> 
> please rebase on top of testing/next after a couple hours:
> 
> checking file drivers/usb/dwc2/gadget.c
> Hunk #1 FAILED at 2749.
> Hunk #2 succeeded at 2772 (offset -1 lines).
> Hunk #3 FAILED at 2811.
> 2 out of 3 hunks FAILED
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
* usb: dwc2: gadget: ISOC's starting flow improvement
@ 2018-07-27 10:48 Minas Harutyunyan
  0 siblings, 0 replies; 5+ messages in thread
From: Minas Harutyunyan @ 2018-07-27 10:48 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Minas Harutyunyan, linux-usb; +Cc: John Youn

To start ISOC transfers in handlers dwc2_gadget_handle_nak() and
dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame
number, based on which, set target frame number to start first ISOC
transfer.

In case if system's high IRQ latency and multiple EP's asserted
interrupt in same frame, there are high probability that when reading
current frame number in EP's handlers, actual frame number can be
increased. As result for bInterval > 1, starting target frame
will be set wrongly and all ISOC packets will be dropped.

In patch "usb: dwc2: Change reading of current frame number flow"
reading of current frame number done ASAP in common interrupt handler.
This frame number stored in frame_number variable which used as
starting frame number for ISOC EP's in above mentioned handlers.

Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 26fdb4b60ff6..887bea99dce8 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2750,21 +2750,14 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
 	struct dwc2_hsotg *hsotg = ep->parent;
 	int dir_in = ep->dir_in;
 	u32 doepmsk;
-	u32 tmp;
 
 	if (dir_in || !ep->isochronous)
 		return;
 
-	/*
-	 * Store frame in which irq was asserted here, as
-	 * it can change while completing request below.
-	 */
-	tmp = dwc2_hsotg_read_frameno(hsotg);
-
 	if (using_desc_dma(hsotg)) {
 		if (ep->target_frame == TARGET_FRAME_INITIAL) {
 			/* Start first ISO Out */
-			ep->target_frame = tmp;
+			ep->target_frame = hsotg->frame_number;
 			dwc2_gadget_start_isoc_ddma(ep);
 		}
 		return;
@@ -2772,11 +2765,9 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
 
 	if (ep->interval > 1 &&
 	    ep->target_frame == TARGET_FRAME_INITIAL) {
-		u32 dsts;
 		u32 ctrl;
 
-		dsts = dwc2_readl(hsotg, DSTS);
-		ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+		ep->target_frame = hsotg->frame_number;
 		dwc2_gadget_incr_frame_num(ep);
 
 		ctrl = dwc2_readl(hsotg, DOEPCTL(ep->index));
@@ -2812,22 +2803,20 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep *hs_ep)
 {
 	struct dwc2_hsotg *hsotg = hs_ep->parent;
 	int dir_in = hs_ep->dir_in;
-	u32 tmp;
 
 	if (!dir_in || !hs_ep->isochronous)
 		return;
 
 	if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
 
-		tmp = dwc2_hsotg_read_frameno(hsotg);
 		if (using_desc_dma(hsotg)) {
-			hs_ep->target_frame = tmp;
+			hs_ep->target_frame = hsotg->frame_number;
 			dwc2_gadget_incr_frame_num(hs_ep);
 			dwc2_gadget_start_isoc_ddma(hs_ep);
 			return;
 		}
 
-		hs_ep->target_frame = tmp;
+		hs_ep->target_frame = hsotg->frame_number;
 		if (hs_ep->interval > 1) {
 			u32 ctrl = dwc2_readl(hsotg,
 					      DIEPCTL(hs_ep->index));

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* usb: dwc2: gadget: ISOC's starting flow improvement
@ 2018-07-26 11:26 Felipe Balbi
  0 siblings, 0 replies; 5+ messages in thread
From: Felipe Balbi @ 2018-07-26 11:26 UTC (permalink / raw)
  To: Minas Harutyunyan; +Cc: John Youn

No top posting ;)

Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:

> Hi Felipe,
>
> This patch should be applied after follow patch from Artur Petrosyan:
>
> "[PATCH] usb: dwc2: Change reading of current frame number flow."

Hasn't this been part of mainline for a while already?

commit c7c24e7a047652c558e7aa4b0f54aae3a61aacc4
Author: Artur Petrosyan <Arthur.Petrosyan@synopsys.com>
Date:   Sat May 5 09:46:26 2018 -0400

    usb: dwc2: Change reading of current frame number flow.
    
    The current frame_number is read from core for both
    device and host modes. Reading of the current frame
    number needs to be performed ASAP due to IRQ latency's.
    This is why, it is moved to common interrupt handler.
    
    Accordingly updated dwc2_gadget_target_frame_elapsed()
    function which uses stored frame_number instead of
    reading frame number.
    
    In cases when target frame value is incremented
    the frame_number is required to read again.
    
    Signed-off-by: Artur Petrosyan <arturp@synopsys.com>
    Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* usb: dwc2: gadget: ISOC's starting flow improvement
@ 2018-07-26 10:41 Felipe Balbi
  0 siblings, 0 replies; 5+ messages in thread
From: Felipe Balbi @ 2018-07-26 10:41 UTC (permalink / raw)
  To: Minas Harutyunyan, Greg Kroah-Hartman; +Cc: John Youn

Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:

> To start ISOC transfers in handlers dwc2_gadget_handle_nak() and
> dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame
> number, based on which, set target frame number to start first ISOC
> transfer.
>
> In case if system's high IRQ latency and multiple EP's asserted
> interrupt in same frame, there are high probability that when reading
> current frame number in EP's handlers, actual frame number can be
> increased. As result for bInterval > 1, starting target frame
> will be set wrongly and all ISOC packets will be dropped.
>
> In patch "usb: dwc2: Change reading of current frame number flow"
> reading of current frame number done ASAP in common interrupt handler.
> This frame number stored in frame_number variable which used as
> starting frame number for ISOC EP's in above mentioned handlers.
>
> Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>

please rebase on top of testing/next after a couple hours:

checking file drivers/usb/dwc2/gadget.c
Hunk #1 FAILED at 2749.
Hunk #2 succeeded at 2772 (offset -1 lines).
Hunk #3 FAILED at 2811.
2 out of 3 hunks FAILED

^ permalink raw reply	[flat|nested] 5+ messages in thread
* usb: dwc2: gadget: ISOC's starting flow improvement
@ 2018-05-24 12:05 Minas Harutyunyan
  0 siblings, 0 replies; 5+ messages in thread
From: Minas Harutyunyan @ 2018-05-24 12:05 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Minas Harutyunyan, linux-usb; +Cc: John Youn

To start ISOC transfers in handlers dwc2_gadget_handle_nak() and
dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame
number, based on which, set target frame number to start first ISOC
transfer.

In case if system's high IRQ latency and multiple EP's asserted
interrupt in same frame, there are high probability that when reading
current frame number in EP's handlers, actual frame number can be
increased. As result for bInterval > 1, starting target frame
will be set wrongly and all ISOC packets will be dropped.

In patch "usb: dwc2: Change reading of current frame number flow"
reading of current frame number done ASAP in common interrupt handler.
This frame number stored in frame_number variable which used as
starting frame number for ISOC EP's in above mentioned handlers.

Signed-off-by: Minas Harutyunyan <hminas@synopsys.com>
---
 drivers/usb/dwc2/gadget.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 676712159980..fedf4dd65cc5 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2749,23 +2749,16 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
 	struct dwc2_hsotg *hsotg = ep->parent;
 	int dir_in = ep->dir_in;
 	u32 doepmsk;
-	u32 tmp;
 
 	if (dir_in || !ep->isochronous)
 		return;
 
-	/*
-	 * Store frame in which irq was asserted here, as
-	 * it can change while completing request below.
-	 */
-	tmp = dwc2_hsotg_read_frameno(hsotg);
-
 	dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), 0);
 
 	if (using_desc_dma(hsotg)) {
 		if (ep->target_frame == TARGET_FRAME_INITIAL) {
 			/* Start first ISO Out */
-			ep->target_frame = tmp;
+			ep->target_frame = hsotg->frame_number;
 			dwc2_gadget_start_isoc_ddma(ep);
 		}
 		return;
@@ -2773,11 +2766,9 @@ static void dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
 
 	if (ep->interval > 1 &&
 	    ep->target_frame == TARGET_FRAME_INITIAL) {
-		u32 dsts;
 		u32 ctrl;
 
-		dsts = dwc2_readl(hsotg->regs + DSTS);
-		ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+		ep->target_frame = hsotg->frame_number;
 		dwc2_gadget_incr_frame_num(ep);
 
 		ctrl = dwc2_readl(hsotg->regs + DOEPCTL(ep->index));
@@ -2813,25 +2804,23 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep *hs_ep)
 {
 	struct dwc2_hsotg *hsotg = hs_ep->parent;
 	int dir_in = hs_ep->dir_in;
-	u32 tmp;
 
 	if (!dir_in || !hs_ep->isochronous)
 		return;
 
 	if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
 
-		tmp = dwc2_hsotg_read_frameno(hsotg);
 		if (using_desc_dma(hsotg)) {
 			dwc2_hsotg_complete_request(hsotg, hs_ep,
 						    get_ep_head(hs_ep), 0);
 
-			hs_ep->target_frame = tmp;
+			hs_ep->target_frame = hsotg->frame_number;
 			dwc2_gadget_incr_frame_num(hs_ep);
 			dwc2_gadget_start_isoc_ddma(hs_ep);
 			return;
 		}
 
-		hs_ep->target_frame = tmp;
+		hs_ep->target_frame = hsotg->frame_number;
 		if (hs_ep->interval > 1) {
 			u32 ctrl = dwc2_readl(hsotg->regs +
 					      DIEPCTL(hs_ep->index));

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-27 10:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-26 11:25 usb: dwc2: gadget: ISOC's starting flow improvement Minas Harutyunyan
  -- strict thread matches above, loose matches on Subject: below --
2018-07-27 10:48 Minas Harutyunyan
2018-07-26 11:26 Felipe Balbi
2018-07-26 10:41 Felipe Balbi
2018-05-24 12:05 Minas Harutyunyan

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.