On 10/09/04 15:47 -0400, Alan Stern wrote:
> On Wed, 1 Sep 2004 [EMAIL PROTECTED] wrote:
>
> > Make code more readable with list_for_each_entry.
> > Compile tested.
> >
> > Signed-off-by: Domen Puncer <[EMAIL PROTECTED]>
> > Signed-off-by: Maximilian Attems <[EMAIL PROTECTED]>
>
> There are lots of things in this patch that could be improved. Below are
> my comments in line with hunks of the patch. Please submit an updated
> version!
Thank you!
Updated patch against 2.6.9-rc1-bk7 (without 3x while(!list_empty)
as they make longer code) follows.
Make code more readable with list_for_each and friends.
Compile tested.
Signed-off-by: Domen Puncer <[EMAIL PROTECTED]>
--- c/drivers/usb/host/uhci-hcd.c 2004-09-04 18:19:59.000000000 +0200
+++ a/drivers/usb/host/uhci-hcd.c 2004-09-11 00:05:48.000000000 +0200
@@ -330,7 +330,7 @@ static void uhci_free_qh(struct uhci_hcd
static void uhci_insert_qh(struct uhci_hcd *uhci, struct uhci_qh *skelqh, struct urb
*urb)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
- struct list_head *tmp;
+ struct urb_priv *turbp;
struct uhci_qh *lqh;
/* Grab the last QH */
@@ -358,12 +358,8 @@ static void uhci_insert_qh(struct uhci_h
*/
lqh->link = cpu_to_le32(urbp->qh->dma_handle) | UHCI_PTR_QH;
if (lqh->urbp) {
- list_for_each (tmp, &lqh->urbp->queue_list) {
- struct urb_priv *turbp =
- list_entry(tmp, struct urb_priv, queue_list);
-
+ list_for_each_entry(turbp, &lqh->urbp->queue_list, queue_list)
turbp->qh->link = lqh->link;
- }
}
list_add_tail(&urbp->qh->list, &skelqh->list);
@@ -405,18 +401,11 @@ static void uhci_remove_qh(struct uhci_h
pqh = list_entry(qh->list.prev, struct uhci_qh, list);
pqh->link = newlink;
if (pqh->urbp) {
- struct list_head *head, *tmp;
-
- head = &pqh->urbp->queue_list;
- tmp = head->next;
- while (head != tmp) {
- struct urb_priv *turbp =
- list_entry(tmp, struct urb_priv, queue_list);
-
- tmp = tmp->next;
+ struct urb_priv *turbp;
+ list_for_each_entry(turbp, &pqh->urbp->queue_list,
+ queue_list)
turbp->qh->link = newlink;
- }
}
wmb();
@@ -447,21 +436,14 @@ static void uhci_remove_qh(struct uhci_h
static int uhci_fixup_toggle(struct urb *urb, unsigned int toggle)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
- struct list_head *head, *tmp;
-
- head = &urbp->td_list;
- tmp = head->next;
- while (head != tmp) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
-
- tmp = tmp->next;
+ struct uhci_td *td;
+ list_for_each_entry(td, &urbp->td_list, list) {
if (toggle)
td->token |= cpu_to_le32(TD_TOKEN_TOGGLE);
else
td->token &= ~cpu_to_le32(TD_TOKEN_TOGGLE);
-
toggle ^= 1;
}
@@ -473,30 +455,18 @@ static int uhci_fixup_toggle(struct urb
static void uhci_append_queued_urb(struct uhci_hcd *uhci, struct urb *eurb, struct
urb *urb)
{
struct urb_priv *eurbp, *urbp, *furbp, *lurbp;
- struct list_head *tmp;
struct uhci_td *lltd;
eurbp = eurb->hcpriv;
urbp = urb->hcpriv;
/* Find the first URB in the queue */
- if (eurbp->queued) {
- struct list_head *head = &eurbp->queue_list;
-
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *turbp =
- list_entry(tmp, struct urb_priv, queue_list);
-
- if (!turbp->queued)
+ furbp = eurbp;
+ if (eurbp->queued)
+ list_for_each_entry(furbp, &eurbp->queue_list, queue_list)
+ if (!furbp->queued)
break;
- tmp = tmp->next;
- }
- } else
- tmp = &eurbp->queue_list;
-
- furbp = list_entry(tmp, struct urb_priv, queue_list);
lurbp = list_entry(furbp->queue_list.prev, struct urb_priv, queue_list);
lltd = list_entry(lurbp->td_list.prev, struct uhci_td, list);
@@ -522,9 +492,7 @@ static void uhci_append_queued_urb(struc
static void uhci_delete_queued_urb(struct uhci_hcd *uhci, struct urb *urb)
{
- struct urb_priv *urbp, *nurbp;
- struct list_head *head, *tmp;
- struct urb_priv *purbp;
+ struct urb_priv *urbp, *nurbp, *purbp, *turbp;
struct uhci_td *pltd;
unsigned int toggle;
@@ -556,14 +524,7 @@ static void uhci_delete_queued_urb(struc
toggle = uhci_toggle(td_token(pltd)) ^ 1;
}
- head = &urbp->queue_list;
- tmp = head->next;
- while (head != tmp) {
- struct urb_priv *turbp;
-
- turbp = list_entry(tmp, struct urb_priv, queue_list);
- tmp = tmp->next;
-
+ list_for_each_entry(turbp, &urbp->queue_list, queue_list) {
if (!turbp->queued)
break;
toggle = uhci_fixup_toggle(turbp->urb, toggle);
@@ -637,7 +598,7 @@ static void uhci_remove_td_from_urb(stru
static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *head, *tmp;
+ struct uhci_td *td, *tmp;
struct urb_priv *urbp;
unsigned int age;
@@ -660,13 +621,7 @@ static void uhci_destroy_urb_priv(struct
if (list_empty(&uhci->td_remove_list))
uhci_set_next_interrupt(uhci);
- head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
-
- tmp = tmp->next;
-
+ list_for_each_entry_safe(td, tmp, &urbp->td_list, list) {
uhci_remove_td_from_urb(td);
uhci_remove_td(uhci, td);
list_add(&td->remove_list, &uhci->td_remove_list);
@@ -1083,7 +1038,6 @@ static int uhci_submit_common(struct uhc
*/
static int uhci_result_common(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *tmp, *head;
struct urb_priv *urbp = urb->hcpriv;
struct uhci_td *td;
unsigned int status = 0;
@@ -1091,13 +1045,7 @@ static int uhci_result_common(struct uhc
urb->actual_length = 0;
- head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
- td = list_entry(tmp, struct uhci_td, list);
-
- tmp = tmp->next;
-
+ list_for_each_entry(td, &urbp->td_list, list) {
status = uhci_status_bits(td_status(td));
if (status & TD_CTRL_ACTIVE)
return -EINPROGRESS;
@@ -1176,17 +1124,12 @@ static inline int uhci_submit_interrupt(
static int isochronous_find_limits(struct uhci_hcd *uhci, struct urb *urb, unsigned
int *start, unsigned int *end)
{
struct urb *last_urb = NULL;
- struct list_head *tmp, *head;
+ struct urb_priv *up;
int ret = 0;
- head = &uhci->urb_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *up = list_entry(tmp, struct urb_priv, urb_list);
+ list_for_each_entry(up, &uhci->urb_list, urb_list) {
struct urb *u = up->urb;
- tmp = tmp->next;
-
/* look for pending URB's with identical pipe handle */
if ((urb->pipe == u->pipe) && (urb->dev == u->dev) &&
(u->status == -EINPROGRESS) && (u != urb)) {
@@ -1272,7 +1215,7 @@ static int uhci_submit_isochronous(struc
static int uhci_result_isochronous(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *tmp, *head;
+ struct uhci_td *td;
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
int status;
int i, ret = 0;
@@ -1280,14 +1223,9 @@ static int uhci_result_isochronous(struc
urb->actual_length = 0;
i = 0;
- head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
+ list_for_each_entry(td, &urbp->td_list, list) {
int actlength;
- tmp = tmp->next;
-
if (td_status(td) & TD_CTRL_ACTIVE)
return -EINPROGRESS;
@@ -1311,20 +1249,15 @@ static int uhci_result_isochronous(struc
static struct urb *uhci_find_urb_ep(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *tmp, *head;
+ struct urb_priv *up;
/* We don't match Isoc transfers since they are special */
if (usb_pipeisoc(urb->pipe))
return NULL;
- head = &uhci->urb_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *up = list_entry(tmp, struct urb_priv, urb_list);
+ list_for_each_entry(up, &uhci->urb_list, urb_list) {
struct urb *u = up->urb;
- tmp = tmp->next;
-
if (u->dev == urb->dev && u->status == -EINPROGRESS) {
/* For control, ignore the direction */
if (usb_pipecontrol(urb->pipe) &&
@@ -1475,7 +1408,8 @@ out:
static void uhci_unlink_generic(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *head, *tmp;
+ struct list_head *head;
+ struct uhci_td *td;
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
int prevactive = 1;
@@ -1495,15 +1429,10 @@ static void uhci_unlink_generic(struct u
* for all types
*/
head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
-
- tmp = tmp->next;
-
+ list_for_each_entry(td, head, list) {
if (!(td_status(td) & TD_CTRL_ACTIVE) &&
(uhci_actual_length(td_status(td)) <
uhci_expected_length(td_token(td)) ||
- tmp == head))
+ td->list.next == head))
usb_settoggle(urb->dev, uhci_endpoint(td_token(td)),
uhci_packetout(td_token(td)),
uhci_toggle(td_token(td)) ^ 1);
@@ -1556,7 +1485,8 @@ done:
static int uhci_fsbr_timeout(struct uhci_hcd *uhci, struct urb *urb)
{
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
- struct list_head *head, *tmp;
+ struct list_head *head;
+ struct uhci_td *td;
int count = 0;
uhci_dec_fsbr(uhci, urb);
@@ -1570,18 +1500,14 @@ static int uhci_fsbr_timeout(struct uhci
*/
head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
-
- tmp = tmp->next;
-
+ list_for_each_entry(td, head, list) {
/*
* Make sure we don't do the last one (since it'll have the
* TERM bit set) as well as we skip every so many TD's to
* make sure it doesn't hog the bandwidth
*/
- if (tmp != head && (count % DEPTH_INTERVAL) == (DEPTH_INTERVAL - 1))
+ if (td->list.next != head && (count % DEPTH_INTERVAL) ==
+ (DEPTH_INTERVAL - 1))
td->link |= UHCI_PTR_DEPTH;
count++;
@@ -1606,12 +1532,10 @@ static void stall_callback(unsigned long
{
struct usb_hcd *hcd = (struct usb_hcd *)ptr;
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- struct list_head list, *tmp, *head;
+ struct urb_priv *up;
unsigned long flags;
int called_uhci_finish_completion = 0;
- INIT_LIST_HEAD(&list);
-
spin_lock_irqsave(&uhci->schedule_lock, flags);
if (!list_empty(&uhci->urb_remove_list) &&
uhci_get_current_frame_number(uhci) != uhci->urb_remove_age) {
@@ -1620,14 +1544,9 @@ static void stall_callback(unsigned long
called_uhci_finish_completion = 1;
}
- head = &uhci->urb_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *up = list_entry(tmp, struct urb_priv, urb_list);
+ list_for_each_entry(up, &uhci->urb_list, urb_list) {
struct urb *u = up->urb;
- tmp = tmp->next;
-
spin_lock(&u->lock);
/* Check if the FSBR timed out */
@@ -1642,17 +1561,6 @@ static void stall_callback(unsigned long
if (called_uhci_finish_completion)
wake_up_all(&uhci->waitqh);
- head = &list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *up = list_entry(tmp, struct urb_priv, urb_list);
- struct urb *u = up->urb;
-
- tmp = tmp->next;
-
- uhci_urb_dequeue(hcd, u);
- }
-
/* Really disable FSBR */
if (!uhci->fsbr && uhci->fsbrtimeout && time_after_eq(jiffies,
uhci->fsbrtimeout)) {
uhci->fsbrtimeout = 0;
@@ -1680,15 +1588,9 @@ static int init_stall_timer(struct usb_h
static void uhci_free_pending_qhs(struct uhci_hcd *uhci)
{
- struct list_head *tmp, *head;
-
- head = &uhci->qh_remove_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_qh *qh = list_entry(tmp, struct uhci_qh, remove_list);
-
- tmp = tmp->next;
+ struct uhci_qh *qh, *tmp;
+ list_for_each_entry_safe(qh, tmp, &uhci->qh_remove_list, remove_list) {
list_del_init(&qh->remove_list);
uhci_free_qh(uhci, qh);
@@ -1697,15 +1599,9 @@ static void uhci_free_pending_qhs(struct
static void uhci_free_pending_tds(struct uhci_hcd *uhci)
{
- struct list_head *tmp, *head;
-
- head = &uhci->td_remove_list;
- tmp = head->next;
- while (tmp != head) {
- struct uhci_td *td = list_entry(tmp, struct uhci_td, remove_list);
-
- tmp = tmp->next;
+ struct uhci_td *td, *tmp;
+ list_for_each_entry_safe(td, tmp, &uhci->td_remove_list, remove_list) {
list_del_init(&td->remove_list);
uhci_free_td(uhci, td);
@@ -1726,19 +1622,13 @@ static void uhci_finish_urb(struct usb_h
static void uhci_finish_completion(struct usb_hcd *hcd, struct pt_regs *regs)
{
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- struct list_head *tmp, *head;
+ struct urb_priv *urbp, *tmp;
- head = &uhci->complete_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
+ list_for_each_entry_safe(urbp, tmp, &uhci->complete_list, urb_list) {
struct urb *urb = urbp->urb;
list_del_init(&urbp->urb_list);
uhci_finish_urb(hcd, urb, regs);
-
- head = &uhci->complete_list;
- tmp = head->next;
}
}
@@ -1754,7 +1644,7 @@ static irqreturn_t uhci_irq(struct usb_h
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
unsigned long io_addr = uhci->io_addr;
unsigned short status;
- struct list_head *tmp, *head;
+ struct urb_priv *urbp, *tmp;
unsigned int age;
/*
@@ -1801,15 +1691,11 @@ static irqreturn_t uhci_irq(struct usb_h
else
uhci_set_next_interrupt(uhci);
- /* Walk the list of pending URB's to see which ones completed */
- head = &uhci->urb_list;
- tmp = head->next;
- while (tmp != head) {
- struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
+ /* Walk the list of pending URB's to see which ones completed
+ (must be _safe) */
+ list_for_each_entry_safe(urbp, tmp, &uhci->urb_list, urb_list) {
struct urb *urb = urbp->urb;
- tmp = tmp->next;
-
/* Checks the status and does all of the magic necessary */
uhci_transfer_result(uhci, urb);
}
-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
