[RFC] libertas: handle hardware events outside ISR to fix starving firmware

Holger Schurig hs4233 at mail.mn-solutions.de
Fri Jun 6 11:30:19 EDT 2008


Don't do this at home, kid. :-)

In the interrupt handler I mix IF_CS_CARD_INT_CAUSE and some bits
from IF_CS_CARD_STATUS together and store it card->int_cause, so
to say a shadow copy of the card interrupt status. I don't do
anything else interesting in the interrupt handler than this. Oh,
no, I send a fake event to the kfifo to wake the sleeping thread.

In the rest of the code, I make sure that I don't read
IF_CS_CARD_STATUS again, as just reading this register would
reset the IF_CS_BIT_EVENT flag.

Then I added a poll method, which the main thread calls just
before his "to sleep or not to sleep" decision orgy.

This if_cs_poll_card() I the simply process the shadow
card->int_cause register.

Signed-off-by: Holger Schurig <hs4233 at mail.mn-solutions.de>


---

This patch is not ready for suspicion, because it seems to
fix my "firmware starves" situation, but is awfully slow. It
also contains #if 0 things, which must be removed.

Dan, feel free to mangle this while I now go home :-)

Index: wireless-testing/drivers/net/wireless/libertas/dev.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/dev.h	2008-06-06 16:03:47.000000000 +0200
+++ wireless-testing/drivers/net/wireless/libertas/dev.h	2008-06-06 16:04:04.000000000 +0200
@@ -154,6 +154,7 @@ struct lbs_private {
 	/** Hardware access */
 	int (*hw_host_to_card) (struct lbs_private *priv, u8 type, u8 *payload, u16 nb);
 	void (*reset_card) (struct lbs_private *priv);
+	void (*poll_card) (struct lbs_private *priv);
 
 	/* Wake On LAN */
 	uint32_t wol_criteria;
@@ -195,6 +196,8 @@ struct lbs_private {
 
 	/* Events sent from hardware to driver */
 	struct kfifo *event_fifo;
+#define LBS_EVENT_MASK 0xff0000
+#define LBS_EVENT_WAKE 0xff0001
 
 	/* nickname */
 	u8 nodename[16];
Index: wireless-testing/drivers/net/wireless/libertas/if_cs.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/if_cs.c	2008-06-06 16:04:04.000000000 +0200
+++ wireless-testing/drivers/net/wireless/libertas/if_cs.c	2008-06-06 16:04:09.000000000 +0200
@@ -59,6 +59,8 @@ struct if_cs_card {
 	struct pcmcia_device *p_dev;
 	struct lbs_private *priv;
 	void __iomem *iobase;
+	spinlock_t lock;
+	u16 int_cause;
 };
 
 
@@ -238,6 +240,7 @@ static int if_cs_poll_while_fw_download(
  */
 #define IF_CS_CARD_STATUS		0x00000020
 #define IF_CS_CARD_STATUS_MASK		0x7f00
+#define IF_CS_CARD_STATUS_EVENT	(IF_CS_CARD_STATUS_MASK | IF_CS_BIT_EVENT)
 
 /*
  * The card int cause register is used by the card/firmware to notify us
@@ -298,22 +301,8 @@ static int if_cs_send_cmd(struct lbs_pri
 {
 	struct if_cs_card *card = (struct if_cs_card *)priv->card;
 	int ret = -1;
-	int loops = 0;
 
 	lbs_deb_enter(LBS_DEB_CS);
-	if_cs_disable_ints(card);
-
-	/* Is hardware ready? */
-	while (1) {
-		u16 status = if_cs_read16(card, IF_CS_CARD_STATUS);
-		if (status & IF_CS_BIT_COMMAND)
-			break;
-		if (++loops > 100) {
-			lbs_pr_err("card not ready for commands\n");
-			goto done;
-		}
-		mdelay(1);
-	}
 
 	if_cs_write16(card, IF_CS_CMD_LEN, nb);
 
@@ -331,8 +320,6 @@ static int if_cs_send_cmd(struct lbs_pri
 	if_cs_write16(card, IF_CS_HOST_INT_CAUSE, IF_CS_BIT_COMMAND);
 	ret = 0;
 
-done:
-	if_cs_enable_ints(card);
 	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
 	return ret;
 }
@@ -343,13 +330,8 @@ done:
 static void if_cs_send_data(struct lbs_private *priv, u8 *buf, u16 nb)
 {
 	struct if_cs_card *card = (struct if_cs_card *)priv->card;
-	u16 status;
 
 	lbs_deb_enter(LBS_DEB_CS);
-	if_cs_disable_ints(card);
-
-	status = if_cs_read16(card, IF_CS_CARD_STATUS);
-	BUG_ON((status & IF_CS_BIT_TX) == 0);
 
 	if_cs_write16(card, IF_CS_WRITE_LEN, nb);
 
@@ -360,30 +342,20 @@ static void if_cs_send_data(struct lbs_p
 
 	if_cs_write16(card, IF_CS_HOST_STATUS, IF_CS_BIT_TX);
 	if_cs_write16(card, IF_CS_HOST_INT_CAUSE, IF_CS_BIT_TX);
-	if_cs_enable_ints(card);
 
 	lbs_deb_leave(LBS_DEB_CS);
 }
 
 /*
- * Get the command result out of the card.
+ * Get the command response out of the card.
  */
-static int if_cs_receive_cmdres(struct lbs_private *priv, u8 *data, u32 *len)
+static int if_cs_receive_resp(struct lbs_private *priv, u8 *data, u32 *len)
 {
 	unsigned long flags;
 	int ret = -1;
-	u16 status;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
-	/* is hardware ready? */
-	status = if_cs_read16(priv->card, IF_CS_CARD_STATUS);
-	if ((status & IF_CS_BIT_RESP) == 0) {
-		lbs_pr_err("no cmd response in card\n");
-		*len = 0;
-		goto out;
-	}
-
 	*len = if_cs_read16(priv->card, IF_CS_RESP_LEN);
 	if ((*len == 0) || (*len > LBS_CMD_BUFFER_SIZE)) {
 		lbs_pr_err("card cmd buffer has invalid # of bytes (%d)\n", *len);
@@ -419,6 +391,7 @@ static struct sk_buff *if_cs_receive_dat
 	lbs_deb_enter(LBS_DEB_CS);
 
 	len = if_cs_read16(priv->card, IF_CS_READ_LEN);
+	lbs_deb_cs("rx %d\n", len);
 	if (len == 0 || len > MRVDRV_ETH_RX_PACKET_BUFFER_SIZE) {
 		lbs_pr_err("card data buffer has invalid # of bytes (%d)\n", len);
 		priv->stats.rx_dropped++;
@@ -450,73 +423,125 @@ static irqreturn_t if_cs_interrupt(int i
 {
 	struct if_cs_card *card = data;
 	struct lbs_private *priv = card->priv;
-	u16 cause;
+	unsigned long flags;
+	u16 cause, status;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
-	/* Ask card interrupt cause register if there is something for us */
+	if_cs_disable_ints(card);
+
 	cause = if_cs_read16(card, IF_CS_CARD_INT_CAUSE);
 	lbs_deb_cs("cause 0x%04x\n", cause);
 
 	if (cause == 0) {
 		/* Not for us */
+		if_cs_enable_ints(card);
 		return IRQ_NONE;
 	}
 
 	if (cause == 0xffff) {
 		/* Read in junk, the card has probably been removed */
 		card->priv->surpriseremoved = 1;
+		if_cs_enable_ints(card);
 		return IRQ_HANDLED;
 	}
 
+	/* read card status to clear event bit */
+	status = if_cs_read16(card, IF_CS_CARD_STATUS);
+	lbs_deb_cs("status 0x%04x\n", status);
+
+	cause &= IF_CS_BIT_MASK;
+
+	/* Store the interupt cause in our shadow */
+	spin_lock_irqsave(&card->lock, flags);
+	card->int_cause |= cause;
+	if (!(cause && IF_CS_BIT_RX) && status & IF_CS_BIT_RX) {
+		/* We sometimes miss an RX interrupt, but we can deduce this
+		 * fact from the status */
+		lbs_deb_cs("forcing IF_CS_BIT_RX\n");
+		card->int_cause |= IF_CS_BIT_RX;
+	}
+	if (status & IF_CS_BIT_EVENT) {
+		/* Store the event bit and the event code into our
+		 * int_cause shadow */
+		lbs_deb_cs("transferring event\n");
+		card->int_cause |= (status & IF_CS_CARD_STATUS_EVENT);
+	}
+	spin_unlock_irqrestore(&card->lock, flags);
+
+	/* Clear interrupt cause */
+	if_cs_write16(card, IF_CS_CARD_INT_CAUSE, cause);
+
+	/* Simple way to keep the main thread from sleeping :-) */
+	lbs_queue_event(priv, LBS_EVENT_WAKE);
+
+	if_cs_enable_ints(card);
+
+	return IRQ_HANDLED;
+}
+
+static void if_cs_poll_status(struct lbs_private *priv)
+{
+	struct if_cs_card *card = (struct if_cs_card *)priv->card;
+	unsigned long flags;
+	u16 cause;
+
+	lbs_deb_enter(LBS_DEB_CS);
+
+	if (priv->surpriseremoved)
+		return;
+
+	spin_lock_irqsave(&card->lock, flags);
+	cause = card->int_cause;
+	card->int_cause = 0;
+	spin_unlock_irqrestore(&card->lock, flags);
+
+	lbs_deb_cs("int_cause 0x%04x\n", cause);
+
+	if (!cause)
+		return;
+
+	if (cause & IF_CS_BIT_TX) {
+		lbs_deb_cs("tx\n");
+		lbs_host_to_card_done(priv);
+	}
+
 	if (cause & IF_CS_BIT_RX) {
 		struct sk_buff *skb;
-		lbs_deb_cs("rx packet\n");
 		skb = if_cs_receive_data(priv);
 		if (skb)
 			lbs_process_rxed_packet(priv, skb);
 	}
 
-	if (cause & IF_CS_BIT_TX) {
-		lbs_deb_cs("tx done\n");
-		lbs_host_to_card_done(priv);
-	}
-
 	if (cause & IF_CS_BIT_RESP) {
-		unsigned long flags;
+		unsigned long driver_flags;
 		u8 i;
 
-		lbs_deb_cs("cmd resp\n");
-		spin_lock_irqsave(&priv->driver_lock, flags);
+		lbs_deb_cs("resp\n");
+		spin_lock_irqsave(&priv->driver_lock, driver_flags);
 		i = (priv->resp_idx == 0) ? 1 : 0;
-		spin_unlock_irqrestore(&priv->driver_lock, flags);
+		spin_unlock_irqrestore(&priv->driver_lock, driver_flags);
 
 		BUG_ON(priv->resp_len[i]);
-		if_cs_receive_cmdres(priv, priv->resp_buf[i],
+		if_cs_receive_resp(priv, priv->resp_buf[i],
 			&priv->resp_len[i]);
 
-		spin_lock_irqsave(&priv->driver_lock, flags);
+		spin_lock_irqsave(&priv->driver_lock, driver_flags);
 		lbs_notify_command_response(priv, i);
-		spin_unlock_irqrestore(&priv->driver_lock, flags);
+		spin_unlock_irqrestore(&priv->driver_lock, driver_flags);
 	}
 
 	if (cause & IF_CS_BIT_EVENT) {
-		u16 status = if_cs_read16(priv->card, IF_CS_CARD_STATUS);
 		if_cs_write16(priv->card, IF_CS_HOST_INT_CAUSE,
 			IF_CS_BIT_EVENT);
-		lbs_queue_event(priv, (status & IF_CS_CARD_STATUS_MASK) >> 8);
+		lbs_queue_event(priv, (cause & IF_CS_CARD_STATUS_MASK) >> 8);
 	}
 
-	/* Clear interrupt cause */
-	if_cs_write16(card, IF_CS_CARD_INT_CAUSE, cause & IF_CS_BIT_MASK);
-
 	lbs_deb_leave(LBS_DEB_CS);
-	return IRQ_HANDLED;
 }
 
 
 
-
 /********************************************************************/
 /* Firmware                                                         */
 /********************************************************************/
@@ -841,6 +866,8 @@ static int if_cs_probe(struct pcmcia_dev
 		}
 	}
 
+	spin_lock_init(&card->lock);
+
 	/* Initialize io access */
 	card->iobase = ioport_map(p_dev->io.BasePort1, p_dev->io.NumPorts1);
 	if (!card->iobase) {
@@ -890,6 +917,7 @@ static int if_cs_probe(struct pcmcia_dev
 	card->priv = priv;
 	priv->card = card;
 	priv->hw_host_to_card = if_cs_host_to_card;
+	priv->poll_card = if_cs_poll_status;
 	priv->fw_ready = 1;
 
 	/* Now actually get the IRQ */
Index: wireless-testing/drivers/net/wireless/libertas/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/libertas/main.c	2008-06-06 16:04:03.000000000 +0200
+++ wireless-testing/drivers/net/wireless/libertas/main.c	2008-06-06 16:04:04.000000000 +0200
@@ -706,6 +706,10 @@ static int lbs_thread(void *data)
 
 		add_wait_queue(&priv->waitq, &wait);
 		set_current_state(TASK_INTERRUPTIBLE);
+
+		if (priv->poll_card)
+			priv->poll_card(priv);
+
 		spin_lock_irq(&priv->driver_lock);
 
 		if (kthread_should_stop())
@@ -812,9 +816,20 @@ static int lbs_thread(void *data)
 
 			__kfifo_get(priv->event_fifo, (unsigned char *) &event,
 				sizeof(event));
-			spin_unlock_irq(&priv->driver_lock);
-			lbs_process_event(priv, event);
-			spin_lock_irq(&priv->driver_lock);
+			/* ignore LBS_EVENT_xxx events */
+			if (event & LBS_EVENT_MASK) {
+#if 0
+				if (priv->poll_card) {
+					spin_unlock_irq(&priv->driver_lock);
+					priv->poll_card(priv);
+					spin_lock_irq(&priv->driver_lock);
+				}
+#endif
+			} else {
+				spin_unlock_irq(&priv->driver_lock);
+				lbs_process_event(priv, event);
+				spin_lock_irq(&priv->driver_lock);
+			}
 		}
 		spin_unlock_irq(&priv->driver_lock);
 



More information about the libertas-dev mailing list