mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] usb: ehci: refactor scan_isoc function
@ 2015-12-11  0:55 Geyslan G. Bem
  2015-12-11 20:27 ` Geyslan G. Bem
  0 siblings, 1 reply; 2+ messages in thread
From: Geyslan G. Bem @ 2015-12-11  0:55 UTC (permalink / raw)
  To: peter.senna
  Cc: Geyslan G. Bem, Alan Stern, Greg Kroah-Hartman, linux-usb, linux-kernel

This patch removes an infinite for loop and makes use of the already
existing 'restart' tag, reducing one leading tab.

The comments and code were corrected conforming coding style.

Tested by compilation only.
Caught by checkpatch:
 WARNING: Too many leading tabs - consider code refactoring

Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
---
v2: fixes coding style (removes spaces before parens and open brackets)
---
 drivers/usb/host/ehci-sched.c | 203 ++++++++++++++++++++++--------------------
 1 file changed, 104 insertions(+), 99 deletions(-)

diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index f9a3327..86b2484 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -2383,6 +2383,9 @@ static void scan_isoc(struct ehci_hcd *ehci)
 	unsigned	fmask = ehci->periodic_size - 1;
 	bool		modified, live;
 
+	union ehci_shadow q, *q_p;
+	__hc32 type, *hw_p;
+
 	/*
 	 * When running, scan from last scan point up to "now"
 	 * else clean up by scanning everything that's left.
@@ -2399,119 +2402,121 @@ static void scan_isoc(struct ehci_hcd *ehci)
 	ehci->now_frame = now_frame;
 
 	frame = ehci->last_iso_frame;
-	for (;;) {
-		union ehci_shadow	q, *q_p;
-		__hc32			type, *hw_p;
 
 restart:
-		/* scan each element in frame's queue for completions */
-		q_p = &ehci->pshadow [frame];
-		hw_p = &ehci->periodic [frame];
-		q.ptr = q_p->ptr;
-		type = Q_NEXT_TYPE(ehci, *hw_p);
-		modified = false;
-
-		while (q.ptr != NULL) {
-			switch (hc32_to_cpu(ehci, type)) {
-			case Q_TYPE_ITD:
-				/* If this ITD is still active, leave it for
-				 * later processing ... check the next entry.
-				 * No need to check for activity unless the
-				 * frame is current.
-				 */
-				if (frame == now_frame && live) {
-					rmb();
-					for (uf = 0; uf < 8; uf++) {
-						if (q.itd->hw_transaction[uf] &
-							    ITD_ACTIVE(ehci))
-							break;
-					}
-					if (uf < 8) {
-						q_p = &q.itd->itd_next;
-						hw_p = &q.itd->hw_next;
-						type = Q_NEXT_TYPE(ehci,
-							q.itd->hw_next);
-						q = *q_p;
+	/* Scan each element in frame's queue for completions */
+	q_p = &ehci->pshadow[frame];
+	hw_p = &ehci->periodic[frame];
+	q.ptr = q_p->ptr;
+	type = Q_NEXT_TYPE(ehci, *hw_p);
+	modified = false;
+
+	while (q.ptr != NULL) {
+		switch (hc32_to_cpu(ehci, type)) {
+		case Q_TYPE_ITD:
+			/*
+			 * If this ITD is still active, leave it for
+			 * later processing ... check the next entry.
+			 * No need to check for activity unless the
+			 * frame is current.
+			 */
+			if (frame == now_frame && live) {
+				rmb();
+				for (uf = 0; uf < 8; uf++) {
+					if (q.itd->hw_transaction[uf] &
+					    ITD_ACTIVE(ehci))
 						break;
-					}
 				}
-
-				/* Take finished ITDs out of the schedule
-				 * and process them:  recycle, maybe report
-				 * URB completion.  HC won't cache the
-				 * pointer for much longer, if at all.
-				 */
-				*q_p = q.itd->itd_next;
-				if (!ehci->use_dummy_qh ||
-				    q.itd->hw_next != EHCI_LIST_END(ehci))
-					*hw_p = q.itd->hw_next;
-				else
-					*hw_p = cpu_to_hc32(ehci,
-							ehci->dummy->qh_dma);
-				type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
-				wmb();
-				modified = itd_complete (ehci, q.itd);
-				q = *q_p;
-				break;
-			case Q_TYPE_SITD:
-				/* If this SITD is still active, leave it for
-				 * later processing ... check the next entry.
-				 * No need to check for activity unless the
-				 * frame is current.
-				 */
-				if (((frame == now_frame) ||
-				     (((frame + 1) & fmask) == now_frame))
-				    && live
-				    && (q.sitd->hw_results &
-					SITD_ACTIVE(ehci))) {
-
-					q_p = &q.sitd->sitd_next;
-					hw_p = &q.sitd->hw_next;
+				if (uf < 8) {
+					q_p = &q.itd->itd_next;
+					hw_p = &q.itd->hw_next;
 					type = Q_NEXT_TYPE(ehci,
-							q.sitd->hw_next);
+							   q.itd->hw_next);
 					q = *q_p;
 					break;
 				}
+			}
 
-				/* Take finished SITDs out of the schedule
-				 * and process them:  recycle, maybe report
-				 * URB completion.
-				 */
-				*q_p = q.sitd->sitd_next;
-				if (!ehci->use_dummy_qh ||
-				    q.sitd->hw_next != EHCI_LIST_END(ehci))
-					*hw_p = q.sitd->hw_next;
-				else
-					*hw_p = cpu_to_hc32(ehci,
-							ehci->dummy->qh_dma);
-				type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
-				wmb();
-				modified = sitd_complete (ehci, q.sitd);
+			/*
+			 * Take finished ITDs out of the schedule
+			 * and process them:  recycle, maybe report
+			 * URB completion.  HC won't cache the
+			 * pointer for much longer, if at all.
+			 */
+			*q_p = q.itd->itd_next;
+			if (!ehci->use_dummy_qh ||
+			    q.itd->hw_next != EHCI_LIST_END(ehci))
+				*hw_p = q.itd->hw_next;
+			else
+				*hw_p = cpu_to_hc32(ehci,
+						    ehci->dummy->qh_dma);
+			type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
+			wmb();
+			modified = itd_complete(ehci, q.itd);
+			q = *q_p;
+			break;
+		case Q_TYPE_SITD:
+			/*
+			 * If this SITD is still active, leave it for
+			 * later processing ... check the next entry.
+			 * No need to check for activity unless the
+			 * frame is current.
+			 */
+			if (((frame == now_frame) ||
+			     (((frame + 1) & fmask) == now_frame))
+			    && live
+			    && (q.sitd->hw_results &
+				SITD_ACTIVE(ehci))) {
+
+				q_p = &q.sitd->sitd_next;
+				hw_p = &q.sitd->hw_next;
+				type = Q_NEXT_TYPE(ehci,
+						   q.sitd->hw_next);
 				q = *q_p;
 				break;
-			default:
-				ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n",
-					type, frame, q.ptr);
-				// BUG ();
-				/* FALL THROUGH */
-			case Q_TYPE_QH:
-			case Q_TYPE_FSTN:
-				/* End of the iTDs and siTDs */
-				q.ptr = NULL;
-				break;
 			}
 
-			/* assume completion callbacks modify the queue */
-			if (unlikely(modified && ehci->isoc_count > 0))
-				goto restart;
-		}
-
-		/* Stop when we have reached the current frame */
-		if (frame == now_frame)
+			/*
+			 * Take finished SITDs out of the schedule
+			 * and process them:  recycle, maybe report
+			 * URB completion.
+			 */
+			*q_p = q.sitd->sitd_next;
+			if (!ehci->use_dummy_qh ||
+			    q.sitd->hw_next != EHCI_LIST_END(ehci))
+				*hw_p = q.sitd->hw_next;
+			else
+				*hw_p = cpu_to_hc32(ehci,
+						    ehci->dummy->qh_dma);
+			type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
+			wmb();
+			modified = sitd_complete(ehci, q.sitd);
+			q = *q_p;
 			break;
+		default:
+			ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n",
+				 type, frame, q.ptr);
+			/* BUG(); */
+			/* FALL THROUGH */
+		case Q_TYPE_QH:
+		case Q_TYPE_FSTN:
+			/* End of the iTDs and siTDs */
+			q.ptr = NULL;
+			break;
+		}
 
-		/* The last frame may still have active siTDs */
-		ehci->last_iso_frame = frame;
-		frame = (frame + 1) & fmask;
+		/* Assume completion callbacks modify the queue */
+		if (unlikely(modified && ehci->isoc_count > 0))
+			goto restart;
 	}
+
+	/* Stop when we have reached the current frame */
+	if (frame == now_frame)
+		return;
+
+	/* The last frame may still have active siTDs */
+	ehci->last_iso_frame = frame;
+	frame = (frame + 1) & fmask;
+
+	goto restart;
 }
-- 
2.6.3


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

* Re: [PATCH v2] usb: ehci: refactor scan_isoc function
  2015-12-11  0:55 [PATCH v2] usb: ehci: refactor scan_isoc function Geyslan G. Bem
@ 2015-12-11 20:27 ` Geyslan G. Bem
  0 siblings, 0 replies; 2+ messages in thread
From: Geyslan G. Bem @ 2015-12-11 20:27 UTC (permalink / raw)
  To: Peter Senna Tschudin
  Cc: Geyslan G. Bem, Alan Stern, Greg Kroah-Hartman, linux-usb, LKML

2015-12-10 21:55 GMT-03:00 Geyslan G. Bem <geyslan@gmail.com>:
> This patch removes an infinite for loop and makes use of the already
> existing 'restart' tag, reducing one leading tab.
>
> The comments and code were corrected conforming coding style.
>
> Tested by compilation only.
> Caught by checkpatch:
>  WARNING: Too many leading tabs - consider code refactoring
>
> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com>
> ---
> v2: fixes coding style (removes spaces before parens and open brackets)
> ---
>  drivers/usb/host/ehci-sched.c | 203 ++++++++++++++++++++++--------------------
>  1 file changed, 104 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index f9a3327..86b2484 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -2383,6 +2383,9 @@ static void scan_isoc(struct ehci_hcd *ehci)
>         unsigned        fmask = ehci->periodic_size - 1;
>         bool            modified, live;
>
> +       union ehci_shadow q, *q_p;
> +       __hc32 type, *hw_p;
> +
>         /*
>          * When running, scan from last scan point up to "now"
>          * else clean up by scanning everything that's left.
> @@ -2399,119 +2402,121 @@ static void scan_isoc(struct ehci_hcd *ehci)
>         ehci->now_frame = now_frame;
>
>         frame = ehci->last_iso_frame;
> -       for (;;) {
> -               union ehci_shadow       q, *q_p;
> -               __hc32                  type, *hw_p;
>
>  restart:
> -               /* scan each element in frame's queue for completions */
> -               q_p = &ehci->pshadow [frame];
> -               hw_p = &ehci->periodic [frame];
> -               q.ptr = q_p->ptr;
> -               type = Q_NEXT_TYPE(ehci, *hw_p);
> -               modified = false;
> -
> -               while (q.ptr != NULL) {
> -                       switch (hc32_to_cpu(ehci, type)) {
> -                       case Q_TYPE_ITD:
> -                               /* If this ITD is still active, leave it for
> -                                * later processing ... check the next entry.
> -                                * No need to check for activity unless the
> -                                * frame is current.
> -                                */
> -                               if (frame == now_frame && live) {
> -                                       rmb();
> -                                       for (uf = 0; uf < 8; uf++) {
> -                                               if (q.itd->hw_transaction[uf] &
> -                                                           ITD_ACTIVE(ehci))
> -                                                       break;
> -                                       }
> -                                       if (uf < 8) {
> -                                               q_p = &q.itd->itd_next;
> -                                               hw_p = &q.itd->hw_next;
> -                                               type = Q_NEXT_TYPE(ehci,
> -                                                       q.itd->hw_next);
> -                                               q = *q_p;
> +       /* Scan each element in frame's queue for completions */
> +       q_p = &ehci->pshadow[frame];
> +       hw_p = &ehci->periodic[frame];
> +       q.ptr = q_p->ptr;
> +       type = Q_NEXT_TYPE(ehci, *hw_p);
> +       modified = false;
> +
> +       while (q.ptr != NULL) {
> +               switch (hc32_to_cpu(ehci, type)) {
> +               case Q_TYPE_ITD:
> +                       /*
> +                        * If this ITD is still active, leave it for
> +                        * later processing ... check the next entry.
> +                        * No need to check for activity unless the
> +                        * frame is current.
> +                        */
> +                       if (frame == now_frame && live) {
> +                               rmb();
> +                               for (uf = 0; uf < 8; uf++) {
> +                                       if (q.itd->hw_transaction[uf] &
> +                                           ITD_ACTIVE(ehci))
>                                                 break;
> -                                       }
>                                 }
> -
> -                               /* Take finished ITDs out of the schedule
> -                                * and process them:  recycle, maybe report
> -                                * URB completion.  HC won't cache the
> -                                * pointer for much longer, if at all.
> -                                */
> -                               *q_p = q.itd->itd_next;
> -                               if (!ehci->use_dummy_qh ||
> -                                   q.itd->hw_next != EHCI_LIST_END(ehci))
> -                                       *hw_p = q.itd->hw_next;
> -                               else
> -                                       *hw_p = cpu_to_hc32(ehci,
> -                                                       ehci->dummy->qh_dma);
> -                               type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
> -                               wmb();
> -                               modified = itd_complete (ehci, q.itd);
> -                               q = *q_p;
> -                               break;
> -                       case Q_TYPE_SITD:
> -                               /* If this SITD is still active, leave it for
> -                                * later processing ... check the next entry.
> -                                * No need to check for activity unless the
> -                                * frame is current.
> -                                */
> -                               if (((frame == now_frame) ||
> -                                    (((frame + 1) & fmask) == now_frame))
> -                                   && live
> -                                   && (q.sitd->hw_results &
> -                                       SITD_ACTIVE(ehci))) {
> -
> -                                       q_p = &q.sitd->sitd_next;
> -                                       hw_p = &q.sitd->hw_next;
> +                               if (uf < 8) {
> +                                       q_p = &q.itd->itd_next;
> +                                       hw_p = &q.itd->hw_next;
>                                         type = Q_NEXT_TYPE(ehci,
> -                                                       q.sitd->hw_next);
> +                                                          q.itd->hw_next);
>                                         q = *q_p;
>                                         break;
>                                 }
> +                       }
>
> -                               /* Take finished SITDs out of the schedule
> -                                * and process them:  recycle, maybe report
> -                                * URB completion.
> -                                */
> -                               *q_p = q.sitd->sitd_next;
> -                               if (!ehci->use_dummy_qh ||
> -                                   q.sitd->hw_next != EHCI_LIST_END(ehci))
> -                                       *hw_p = q.sitd->hw_next;
> -                               else
> -                                       *hw_p = cpu_to_hc32(ehci,
> -                                                       ehci->dummy->qh_dma);
> -                               type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
> -                               wmb();
> -                               modified = sitd_complete (ehci, q.sitd);
> +                       /*
> +                        * Take finished ITDs out of the schedule
> +                        * and process them:  recycle, maybe report
> +                        * URB completion.  HC won't cache the
> +                        * pointer for much longer, if at all.
> +                        */
> +                       *q_p = q.itd->itd_next;
> +                       if (!ehci->use_dummy_qh ||
> +                           q.itd->hw_next != EHCI_LIST_END(ehci))
> +                               *hw_p = q.itd->hw_next;
> +                       else
> +                               *hw_p = cpu_to_hc32(ehci,
> +                                                   ehci->dummy->qh_dma);
> +                       type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
> +                       wmb();
> +                       modified = itd_complete(ehci, q.itd);
> +                       q = *q_p;
> +                       break;
> +               case Q_TYPE_SITD:
> +                       /*
> +                        * If this SITD is still active, leave it for
> +                        * later processing ... check the next entry.
> +                        * No need to check for activity unless the
> +                        * frame is current.
> +                        */
> +                       if (((frame == now_frame) ||
> +                            (((frame + 1) & fmask) == now_frame))
> +                           && live
> +                           && (q.sitd->hw_results &
> +                               SITD_ACTIVE(ehci))) {
> +
> +                               q_p = &q.sitd->sitd_next;
> +                               hw_p = &q.sitd->hw_next;
> +                               type = Q_NEXT_TYPE(ehci,
> +                                                  q.sitd->hw_next);
>                                 q = *q_p;
>                                 break;
> -                       default:
> -                               ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n",
> -                                       type, frame, q.ptr);
> -                               // BUG ();
> -                               /* FALL THROUGH */
> -                       case Q_TYPE_QH:
> -                       case Q_TYPE_FSTN:
> -                               /* End of the iTDs and siTDs */
> -                               q.ptr = NULL;
> -                               break;
>                         }
>
> -                       /* assume completion callbacks modify the queue */
> -                       if (unlikely(modified && ehci->isoc_count > 0))
> -                               goto restart;
> -               }
> -
> -               /* Stop when we have reached the current frame */
> -               if (frame == now_frame)
> +                       /*
> +                        * Take finished SITDs out of the schedule
> +                        * and process them:  recycle, maybe report
> +                        * URB completion.
> +                        */
> +                       *q_p = q.sitd->sitd_next;
> +                       if (!ehci->use_dummy_qh ||
> +                           q.sitd->hw_next != EHCI_LIST_END(ehci))
> +                               *hw_p = q.sitd->hw_next;
> +                       else
> +                               *hw_p = cpu_to_hc32(ehci,
> +                                                   ehci->dummy->qh_dma);
> +                       type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
> +                       wmb();
> +                       modified = sitd_complete(ehci, q.sitd);
> +                       q = *q_p;
>                         break;
> +               default:
> +                       ehci_dbg(ehci, "corrupt type %d frame %d shadow %p\n",
> +                                type, frame, q.ptr);
> +                       /* BUG(); */
> +                       /* FALL THROUGH */
> +               case Q_TYPE_QH:
> +               case Q_TYPE_FSTN:
> +                       /* End of the iTDs and siTDs */
> +                       q.ptr = NULL;
> +                       break;
> +               }
>
> -               /* The last frame may still have active siTDs */
> -               ehci->last_iso_frame = frame;
> -               frame = (frame + 1) & fmask;
> +               /* Assume completion callbacks modify the queue */
> +               if (unlikely(modified && ehci->isoc_count > 0))
> +                       goto restart;
>         }
> +
> +       /* Stop when we have reached the current frame */
> +       if (frame == now_frame)
> +               return;
> +
> +       /* The last frame may still have active siTDs */
> +       ehci->last_iso_frame = frame;
> +       frame = (frame + 1) & fmask;
> +
> +       goto restart;
>  }
> --
> 2.6.3
>

I'm doing a cleanup in this file. It's better to wait the whole pack of patches.

-- 
Regards,

Geyslan G. Bem
hackingbits.com

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

end of thread, other threads:[~2015-12-11 20:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-11  0:55 [PATCH v2] usb: ehci: refactor scan_isoc function Geyslan G. Bem
2015-12-11 20:27 ` Geyslan G. Bem

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®