drivers/net/wan/wanxl.c: time_before(timeout, jiffies) -> jiffies, timeout

fix reversal of timeout and jiffies

drivers/net/wan/wanxl.c +0 -0

index d4aab8a..f61413b 100644

Quoted 1 line · Roel Kluin

fix reversal of timeout and jiffies

Wouldn't it be better to have a schedule() in those while loops too?

Maybe a more generic macro / statement expression
would be more readable?

perhaps something like (compiled/untested):

 drivers/net/wan/wanxl.c |   86 ++++++++++++++++------------------------------
 1 files changed, 30 insertions(+), 56 deletions(-)

drivers/net/wan/wanxl.c +0 -0

index d4aab8a..db1eabe 100644

drivers/net/wan/wanxl.c +7 -0

+		if (t)					\
+			break;				\
+		schedule();				\
+	} while (time_before(jiffies, timeout));	\
+	t;						\
+})
 
+
 typedef struct {
 	struct net_device *dev;

drivers/net/wan/wanxl.c +12 -29

 	struct card_t *card;

@@ -396,7 +409,6 @@ static int wanxl_open(struct net_device *dev)
 {
 	port_t *port = dev_to_port(dev);
 	u8 __iomem *dbr = port->card->plx + PLX_DOORBELL_TO_CARD;
-	unsigned long timeout;
 	int i;
 
 	if (get_status(port)->open) {

@@ -412,18 +424,15 @@ static int wanxl_open(struct net_device *dev)
 	/* signal the card */
 	writel(1 << (DOORBELL_TO_CARD_OPEN_0 + port->node), dbr);
 
-	timeout = jiffies + HZ;
-	do
-		if (get_status(port)->open) {
-			netif_start_queue(dev);
-			return 0;
-		}
-	while (time_after(timeout, jiffies));
+	if (!TEST_UNTIL_TIMEOUT((get_status(port)->open), HZ)) {
+		printk(KERN_ERR "%s: unable to open port\n", dev->name);
+		/* ask the card to close the port, should it be still alive */
+		writel(1 << (DOORBELL_TO_CARD_CLOSE_0 + port->node), dbr);
+		return -EFAULT;
+	}
 
-	printk(KERN_ERR "%s: unable to open port\n", dev->name);
-	/* ask the card to close the port, should it be still alive */
-	writel(1 << (DOORBELL_TO_CARD_CLOSE_0 + port->node), dbr);
-	return -EFAULT;
+	netif_start_queue(dev);
+	return 0;
 }
 
 

@@ -431,7 +440,6 @@ static int wanxl_open(struct net_device *dev)
 static int wanxl_close(struct net_device *dev)
 {
 	port_t *port = dev_to_port(dev);
-	unsigned long timeout;
 	int i;
 
 	hdlc_close(dev);

@@ -439,13 +447,7 @@ static int wanxl_close(struct net_device *dev)
 	writel(1 << (DOORBELL_TO_CARD_CLOSE_0 + port->node),
 	       port->card->plx + PLX_DOORBELL_TO_CARD);
 
-	timeout = jiffies + HZ;
-	do
-		if (!get_status(port)->open)
-			break;
-	while (time_after(timeout, jiffies));
-
-	if (get_status(port)->open)
+	if (TEST_UNTIL_TIMEOUT((!get_status(port)->open), HZ))
 		printk(KERN_ERR "%s: unable to close port\n", dev->name);
 
 	netif_stop_queue(dev);

@@ -481,17 +483,11 @@ static struct net_device_stats *wanxl_get_stats(struct net_device *dev)
 
 static int wanxl_puts_command(card_t *card, u32 cmd)
 {
-	unsigned long timeout = jiffies + 5 * HZ;
-
 	writel(cmd, card->plx + PLX_MAILBOX_1);
-	do {
-		if (readl(card->plx + PLX_MAILBOX_1) == 0)
-			return 0;
-
-		schedule();
-	}while (time_after(timeout, jiffies));
+	if (TEST_UNTIL_TIMEOUT((readl(card->plx + PLX_MAILBOX_1)), 5 * HZ))
+		return -1;
 
-	return -1;
+	return 0;
 }
 
 

drivers/net/wan/wanxl.c +0 -11

-	while ((stat = readl(card->plx + PLX_MAILBOX_0)) != 0) {
-		if (time_before(timeout, jiffies)) {
-			printk(KERN_WARNING "wanXL %s: timeout waiting for"
-			       " PUTS to complete\n", pci_name(pdev));
-			wanxl_pci_remove_one(pdev);
-			return -ENODEV;
-		}
-
-		switch(stat & 0xC0) {
-		case 0x00:	/* hmm - PUTS completed with non-zero code? */
-		case 0x80:	/* PUTS still testing the hardware */

drivers/net/wan/wanxl.c +2 -12

-		}
-
-		schedule();
 	}
 
 	/* get on-board memory size (PUTS detects no more than 4 MB) */

@@ -734,15 +715,8 @@ static int __devinit wanxl_pci_init_one(struct pci_dev *pdev,
 		return -ENODEV;
 	}
 
-	stat = 0;
-	timeout = jiffies + 5 * HZ;
-	do {
-		if ((stat = readl(card->plx + PLX_MAILBOX_5)) != 0)
-			break;
-		schedule();
-	}while (time_after(timeout, jiffies));
-
-	if (!stat) {
+	if (TEST_UNTIL_TIMEOUT(((stat = readl(card->plx + PLX_MAILBOX_5)) != 0),
+			       5 * HZ)) {
 		printk(KERN_WARNING "wanXL %s: timeout while initializing card "
 		       "firmware\n", pci_name(pdev));
 		wanxl_pci_remove_one(pdev);

Quoted 15 lines

drivers/net/wan/wanxl.c +0 -15

-	while ((stat = readl(card->plx + PLX_MAILBOX_0)) != 0) {
-		if (time_before(timeout, jiffies)) {
-			printk(KERN_WARNING "wanXL %s: timeout waiting for"
-			       " PUTS to complete\n", pci_name(pdev));
-			wanxl_pci_remove_one(pdev);
-			return -ENODEV;
-		}
-
-		switch(stat & 0xC0) {
-		case 0x00:	/* hmm - PUTS completed with non-zero code? */
-		case 0x80:	/* PUTS still testing the hardware */
-			break;
-
-		default:
-			printk(KERN_WARNING "wanXL %s: PUTS test 0x%X"

drivers/net/wan/wanxl.c static int __devinit wanxl_pci_init_one(struct pci_dev *pdev, · line 653

Quoted 6 lines

drivers/net/wan/wanxl.c +3 -0

+	if (TEST_UNTIL_TIMEOUT(((stat = readl(card->plx + PLX_MAILBOX_0)) != 0),
+			       20 * HZ)) {
+		printk(KERN_WARNING "wanXL %s: PUTS test 0x%X"
 			       " failed\n", pci_name(pdev), stat & 0x30);
 			wanxl_pci_remove_one(pdev);
 			return -ENODEV;

Doesn't this change behavior for (stat & 0xC0) != 0x00 or 0x80?

Review

Quoted 6 lines

(quoted diff) +1 -1

 	while ((stat = readl(card->plx + PLX_MAILBOX_0)) != 0) {
-		if (time_before(timeout, jiffies)) {
+		if (time_before(jiffies, timeout)) {
 			printk(KERN_WARNING "wanXL %s: timeout waiting for"
 			       " PUTS to complete\n", pci_name(pdev));
 			wanxl_pci_remove_one(pdev);

Wouldn't it be better to have a schedule() in those while loops too?

Maybe a more generic macro / statement expression
would be more readable?

Quoted 2 lines · Joe Perches

Wouldn't it be better to have a schedule() in those while loops too?

There is a schedule() here:

What's the value of not having schedule() in the
wanxl_open and wanxl_close routines?