[RFC] cf rework to fix locking issues

Dan Williams dcbw at redhat.com
Tue Feb 26 10:52:00 EST 2008


A while ago I reworked the CF flow to match that of the SDIO and USB
drivers with respect to interrupt handling and callbacks into the core.
This was in response to locking issues reported by Holger.  I got as far
as this; unfortunately the card seems to stall during large transfers
with this patch, but I'm not enough of a CF person to know how to debug
it.  Any comments or thoughts about the issue or patch?

Signed-off-by: Dan Williams <dcbw at redhat.com>

diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 030dbe2..dfed4ec 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -59,8 +59,13 @@ struct if_cs_card {
 	struct pcmcia_device *p_dev;
 	struct lbs_private *priv;
 	void __iomem *iobase;
+
+	u8 buffer[LBS_UPLD_SIZE];
+	u8 int_cause;
+	u32 event;
 };
 
+static void if_cs_card_to_host(struct lbs_private *priv);
 
 
 /********************************************************************/
@@ -245,14 +250,15 @@ static irqreturn_t if_cs_interrupt(int irq, void *data)
 {
 	struct if_cs_card *card = (struct if_cs_card *)data;
 	u16 int_cause;
+	irqreturn_t ret = IRQ_HANDLED;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
 	int_cause = if_cs_read16(card, IF_CS_C_INT_CAUSE);
-	if(int_cause == 0x0) {
+	if (int_cause == 0x0) {
 		/* Not for us */
-		return IRQ_NONE;
-
+		ret = IRQ_NONE;
+		goto out;
 	} else if (int_cause == 0xffff) {
 		/* Read in junk, the card has probably been removed */
 		card->priv->surpriseremoved = 1;
@@ -263,12 +269,15 @@ static irqreturn_t if_cs_interrupt(int irq, void *data)
 
 		/* clear interrupt */
 		if_cs_write16(card, IF_CS_C_INT_CAUSE, int_cause & IF_CS_C_IC_MASK);
+
+		/* Handle stuff the card wants us to do */
+lbs_deb_cs("%s():%d int_cause is 0x%04x\n", __func__, __LINE__, int_cause);
+		if_cs_card_to_host(card->priv);
 	}
-	spin_lock(&card->priv->driver_lock);
-	lbs_interrupt(card->priv);
-	spin_unlock(&card->priv->driver_lock);
 
-	return IRQ_HANDLED;
+out:
+	lbs_deb_leave_args(LBS_DEB_CS, "irqreturn %d", ret);
+	return ret;
 }
 
 
@@ -349,46 +358,64 @@ static void if_cs_send_data(struct lbs_private *priv, u8 *buf, u16 nb)
 /*
  * Get the command result out of the card.
  */
-static int if_cs_receive_cmdres(struct lbs_private *priv, u8 *data, u32 *len)
+static int if_cs_handle_cmd(struct lbs_private *priv)
 {
+	struct if_cs_card *card = (struct if_cs_card *)priv->card;
 	int ret = -1;
 	u16 val;
+	u32 len = 0;
+	unsigned long flags;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
 	/* is hardware ready? */
-	val = if_cs_read16(priv->card, IF_CS_C_STATUS);
+	val = if_cs_read16(card, IF_CS_C_STATUS);
 	if ((val & IF_CS_C_S_CMD_UPLD_RDY) == 0) {
 		lbs_pr_err("card not ready for CMD\n");
 		goto out;
 	}
 
-	*len = if_cs_read16(priv->card, IF_CS_C_CMD_LEN);
-	if ((*len == 0) || (*len > LBS_CMD_BUFFER_SIZE)) {
-		lbs_pr_err("card cmd buffer has invalid # of bytes (%d)\n", *len);
+	len = if_cs_read16(card, IF_CS_C_CMD_LEN);
+	if ((len == 0) || (len > LBS_CMD_BUFFER_SIZE)) {
+		lbs_pr_err("card cmd buffer has invalid # of bytes (%d)\n", len);
 		goto out;
 	}
 
 	/* read even number of bytes, then odd byte if necessary */
-	if_cs_read16_rep(priv->card, IF_CS_C_CMD, data, *len/sizeof(u16));
-	if (*len & 1)
-		data[*len-1] = if_cs_read8(priv->card, IF_CS_C_CMD);
+	if_cs_read16_rep(card, IF_CS_C_CMD, card->buffer, len / sizeof(u16));
+	if (len & 1)
+		card->buffer[len-1] = if_cs_read8(card, IF_CS_C_CMD);
+
+//	spin_lock_irqsave(&priv->driver_lock, flags);
+	spin_lock(&priv->driver_lock);
+
+	memcpy(priv->upld_buf, card->buffer, len);
 
 	/* This is a workaround for a firmware that reports too much
 	 * bytes */
-	*len -= 8;
+	priv->upld_len = len - 8;
+	card->int_cause |= MRVDRV_CMD_UPLD_RDY;
+
+lbs_deb_cs("%s():%d calling lbs_interrupt() intcounter %d\n", __func__, __LINE__, priv->intcounter);
+	lbs_interrupt(priv);
+
+//	spin_unlock_irqrestore(&priv->driver_lock, flags);
+	spin_unlock(&priv->driver_lock);
+
 	ret = 0;
+
 out:
-	lbs_deb_leave_args(LBS_DEB_CS, "ret %d, len %d", ret, *len);
+	lbs_deb_leave_args(LBS_DEB_CS, "ret %d, len %d", ret, len - 8);
 	return ret;
 }
 
 
-static struct sk_buff *if_cs_receive_data(struct lbs_private *priv)
+static int if_cs_handle_data(struct lbs_private *priv)
 {
 	struct sk_buff *skb = NULL;
 	u16 len;
 	u8 *data;
+	int ret = 0;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
@@ -397,32 +424,64 @@ static struct sk_buff *if_cs_receive_data(struct lbs_private *priv)
 		lbs_pr_err("card data buffer has invalid # of bytes (%d)\n", len);
 		priv->stats.rx_dropped++;
 		printk(KERN_INFO "##HS %s:%d TODO\n", __FUNCTION__, __LINE__);
+		ret = -E2BIG;
 		goto dat_err;
 	}
 
 	//TODO: skb = dev_alloc_skb(len+ETH_FRAME_LEN+MRVDRV_SNAP_HEADER_LEN+EXTRA_LEN);
 	skb = dev_alloc_skb(MRVDRV_ETH_RX_PACKET_BUFFER_SIZE + 2);
-	if (!skb)
+	if (!skb) {
+		ret = -ENOMEM;
 		goto out;
-	skb_put(skb, len);
-	skb_reserve(skb, 2);/* 16 byte align */
-	data = skb->data;
+	}
+
+	skb_reserve(skb, NET_IP_ALIGN);
+
+	data = skb_put(skb, len);
 
 	/* read even number of bytes, then odd byte if necessary */
 	if_cs_read16_rep(priv->card, IF_CS_H_READ, data, len/sizeof(u16));
 	if (len & 1)
 		data[len-1] = if_cs_read8(priv->card, IF_CS_H_READ);
 
+	lbs_process_rxed_packet(priv, skb);
+
 dat_err:
 	if_cs_write16(priv->card, IF_CS_H_STATUS, IF_CS_H_STATUS_RX_OVER);
 	if_cs_write16(priv->card, IF_CS_H_INT_CAUSE, IF_CS_H_IC_RX_OVER);
 
 out:
-	lbs_deb_leave_args(LBS_DEB_CS, "ret %p", skb);
-	return skb;
+	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
+	return ret;
 }
 
 
+static int if_cs_handle_event(struct lbs_private *priv, u8 status)
+{
+	struct if_cs_card *card = (struct if_cs_card *)priv->card;
+	int ret = 0;
+	unsigned long flags;
+
+	lbs_deb_enter(LBS_DEB_CS);
+
+	if_cs_write16(card, IF_CS_H_INT_CAUSE, IF_CS_H_IC_HOST_EVENT);
+
+//	spin_lock_irqsave(&priv->driver_lock, flags);
+	spin_lock(&priv->driver_lock);
+
+	card->event = (status & IF_CS_C_S_STATUS_MASK) >> 5;
+	card->int_cause |= MRVDRV_CARDEVENT;
+
+lbs_deb_cs("%s():%d calling lbs_interrupt() intcounter %d\n", __func__, __LINE__, priv->intcounter);
+	lbs_interrupt(priv);
+
+//	spin_unlock_irqrestore(&priv->driver_lock, flags);
+	spin_unlock(&priv->driver_lock);
+
+	lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
+	return 0;
+}
+
 
 /********************************************************************/
 /* Firmware                                                         */
@@ -642,61 +701,69 @@ static int if_cs_host_to_card(struct lbs_private *priv,
 }
 
 
-static int if_cs_get_int_status(struct lbs_private *priv, u8 *ireg)
+static void if_cs_card_to_host(struct lbs_private *priv)
 {
 	struct if_cs_card *card = (struct if_cs_card *)priv->card;
+	u8 status;
+	u8 type;
 	int ret = 0;
-	u16 int_cause;
-	*ireg = 0;
 
 	lbs_deb_enter(LBS_DEB_CS);
 
-	if (priv->surpriseremoved)
+	status = if_cs_read16(card, IF_CS_C_STATUS);
+	type = status & IF_CS_C_S_MASK;
+lbs_deb_cs("%s():%d status is 0x%04x", __func__, __LINE__, status);
+	if (type == 0)
 		goto out;
 
-	int_cause = if_cs_read16(card, IF_CS_C_INT_CAUSE) & IF_CS_C_IC_MASK;
-	if_cs_write16(card, IF_CS_C_INT_CAUSE, int_cause);
-
-	*ireg = if_cs_read16(card, IF_CS_C_STATUS) & IF_CS_C_S_MASK;
-
-	if (!*ireg)
-		goto sbi_get_int_status_exit;
-
-sbi_get_int_status_exit:
-
-	/* is there a data packet for us? */
-	if (*ireg & IF_CS_C_S_RX_UPLD_RDY) {
-		struct sk_buff *skb = if_cs_receive_data(priv);
-		lbs_process_rxed_packet(priv, skb);
-		*ireg &= ~IF_CS_C_S_RX_UPLD_RDY;
+	/* Card has a command result for us */
+	if (type & IF_CS_C_S_CMD_UPLD_RDY) {
+		ret = if_cs_handle_cmd(priv);
+		if (ret < 0)
+			lbs_pr_err("could not receive cmd from card: %d\n", ret);
 	}
 
-	if (*ireg & IF_CS_C_S_TX_DNLD_RDY) {
-		priv->dnld_sent = DNLD_RES_RECEIVED;
+	if (type & IF_CS_C_S_RX_UPLD_RDY) {
+		ret = if_cs_handle_data(priv);
+		if (ret < 0)
+			lbs_pr_err("could not receive data from card: %d\n", ret);
 	}
 
-	/* Card has a command result for us */
-	if (*ireg & IF_CS_C_S_CMD_UPLD_RDY) {
-		spin_lock(&priv->driver_lock);
-		ret = if_cs_receive_cmdres(priv, priv->upld_buf, &priv->upld_len);
-		spin_unlock(&priv->driver_lock);
-		if (ret < 0)
-			lbs_pr_err("could not receive cmd from card\n");
+	if (type & IF_CS_C_S_TX_DNLD_RDY)
+		lbs_host_to_card_done (priv);
+
+	if (type & IF_CS_C_S_CARDEVENT) {
+		/* Event handler needs entire status register */
+		if_cs_handle_event(priv, status);
 	}
 
 out:
-	lbs_deb_leave_args(LBS_DEB_CS, "ret %d, ireg 0x%x, hisregcpy 0x%x", ret, *ireg, priv->hisregcpy);
-	return ret;
+	lbs_deb_leave(LBS_DEB_CS);
+}
+
+static int if_cs_get_int_status(struct lbs_private *priv, u8 *ireg)
+{
+	struct if_cs_card *card = (struct if_cs_card *)priv->card;
+
+	lbs_deb_enter(LBS_DEB_CS);
+
+	*ireg = card->int_cause;
+	card->int_cause = 0;
+
+	lbs_deb_leave(LBS_DEB_CS);
+	return 0;
 }
 
 
 static int if_cs_read_event_cause(struct lbs_private *priv)
 {
+	struct if_cs_card *card = (struct if_cs_card *)priv->card;
+
 	lbs_deb_enter(LBS_DEB_CS);
 
-	priv->eventcause = (if_cs_read16(priv->card, IF_CS_C_STATUS) & IF_CS_C_S_STATUS_MASK) >> 5;
-	if_cs_write16(priv->card, IF_CS_H_INT_CAUSE, IF_CS_H_IC_HOST_EVENT);
+	priv->eventcause = card->event;
 
+	lbs_deb_leave(LBS_DEB_CS);
 	return 0;
 }
 





More information about the libertas-dev mailing list