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!
Alan Stern
Patching drivers/usb/host/uhci-hcd.c:
@@ -481,17 +463,12 @@ static void uhci_append_queued_urb(struc
/* Find the first URB in the queue */
if (eurbp->queued) {
- struct list_head *head = &eurbp->queue_list;
-
- tmp = head->next;
- while (tmp != head) {
+ list_for_each(tmp, &eurbp->queue_list) {
struct urb_priv *turbp =
list_entry(tmp, struct urb_priv, queue_list);
if (!turbp->queued)
break;
-
- tmp = tmp->next;
}
} else
tmp = &eurbp->queue_list;
This is not as efficient as it could be. Remove tmp and turbp, and use
list_for_each_entry() with furbp as the loop variable. The last line above
can be replaced simply with:
furbp = eurbp;
and this can be moved to the top, just before the test of eurbp->queued.
@@ -1083,21 +1045,14 @@ 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;
+ struct uhci_td *td, *tmp;
unsigned int status = 0;
int ret = 0;
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_safe(td, tmp, &urbp->td_list, list) {
status = uhci_status_bits(td_status(td));
if (status & TD_CTRL_ACTIVE)
return -EINPROGRESS;
This doesn't need to use list_for_each_entry_safe(). list_for_each_entry()
will work okay.
@@ -1280,14 +1230,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_safe(td, tmp, &urbp->td_list, list) {
int actlength;
- tmp = tmp->next;
-
if (td_status(td) & TD_CTRL_ACTIVE)
return -EINPROGRESS;
This doesn't need _safe either.
@@ -1311,20 +1256,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, *tmp;
/* 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_safe(up, tmp, &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) &&
And this doesn't need _safe.
@@ -1475,7 +1415,7 @@ out:
static void uhci_unlink_generic(struct uhci_hcd *uhci, struct urb *urb)
{
- struct list_head *head, *tmp;
+ struct list_head *head, *tmp, *next;
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
int prevactive = 1;
@@ -1495,15 +1435,12 @@ static void uhci_unlink_generic(struct u
* for all types
*/
head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
+ list_for_each_safe(tmp, next, head) {
struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
- tmp = tmp->next;
-
if (!(td_status(td) & TD_CTRL_ACTIVE) &&
(uhci_actual_length(td_status(td)) <
uhci_expected_length(td_token(td)) ||
- tmp == head))
+ next == head))
usb_settoggle(urb->dev, uhci_endpoint(td_token(td)),
uhci_packetout(td_token(td)),
uhci_toggle(td_token(td)) ^ 1);
This doesn't need _safe, and it could be changed to list_for_each_entry().
The test for tmp == head would change to td->list.next == head.
@@ -1570,18 +1507,15 @@ static int uhci_fsbr_timeout(struct uhci
*/
head = &urbp->td_list;
- tmp = head->next;
- while (tmp != head) {
+ list_for_each(tmp, head) {
struct uhci_td *td = list_entry(tmp, struct uhci_td, list);
- tmp = tmp->next;
-
/*
* 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 (tmp->next != head && (count % DEPTH_INTERVAL) == (DEPTH_INTERVAL -
1))
td->link |= UHCI_PTR_DEPTH;
count++;
This could be list_for_each_entry(). Replace tmp with td->list.next.
And feel free to fold long lines.
@@ -1620,14 +1555,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_safe(up, tmp, &uhci->urb_list, urb_list) {
struct urb *u = up->urb;
- tmp = tmp->next;
-
spin_lock(&u->lock);
/* Check if the FSBR timed out */
This doesn't need _safe.
@@ -1642,14 +1572,9 @@ 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);
+ list_for_each_entry_safe(up, tmp, &list, urb_list) {
struct urb *u = up->urb;
- tmp = tmp->next;
-
uhci_urb_dequeue(hcd, u);
}
Ironically, not only does this not need _safe -- it isn't needed at all!
This whole section is dead code, as is the "list" variable. They can be
removed completely.
@@ -1680,15 +1605,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);
This can be done more directly. Since every member of the list is removed,
you don't need list_for_each_entry_safe(). Instead use
while (!list_empty(&uhci->qh_remove_list)) {...
@@ -1697,15 +1616,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);
This same comment as above applies here.
@@ -1726,19 +1639,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;
}
}
This same comment as above applies here.
@@ -1802,14 +1709,9 @@ static irqreturn_t uhci_irq(struct usb_h
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);
+ list_for_each_entry(urbp, &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 loop _does_ need to use _safe.
-------------------------------------------------------
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
