[PATCH] Fixes for interrupts in cf8385 driver

[Dan Williams]

On Tue, 2007-08-21 at 08:57 +1200, Ryan Mallon wrote:
> Hey all,
> 
> First off thanks to Holger Schurig for his work on the cf8385 driver, we
> have successfully replaced the binary drivers supplied by Marvell with
> his code, which makes our lives a little easier.
> 
> The following patch fixes the tx transmit timeout problem, which is
> caused by the interrupts being incorrectly check and masked. The patch
> moves the interrupt masking code so that interrupts are enabled only
> when the driver is registered and only disabled when the driver is
> unregistered. The patch also corrects a minor bug with priv->dnld_sent
> being set incorrectly in if_cs_host_to_card.
> 
> I have tested this with transfer of a 1mb file using both an unsecured
> network and WPA security, and received no transmit timeout errors. I
> will be testing/timing transfers of some larger files later.

Thanks for the patch, given that I have no hardware I'd like to get
Holger to comment on it first before I commit any bits of it.  If he
doesn't get to it by next week or so, I guess I'll just push it.

The dnld_sent bits seem correct, but I'm unsure about when CF interrupts
need to be enabled/disabled...

Cheers,
Dan

> Cheers,
> Ryan
> 
> --- 
> 
> diff -Naur a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
> --- a/drivers/net/wireless/libertas/if_cs.c	2007-08-07 14:59:27.000000000 +1200
> +++ b/drivers/net/wireless/libertas/if_cs.c	2007-08-08 15:52:22.000000000 +1200
> @@ -248,22 +248,26 @@
>  	lbs_deb_enter(LBS_DEB_CS);
>  
>  	int_cause = if_cs_read16(card, IF_CS_C_INT_CAUSE);
> -	switch (int_cause) {
> -	case 0x0000:
> -		/* not for us */
> +	if(int_cause == 0x0) {
> +		/* Not for us */
>  		return IRQ_NONE;
> -	case 0xffff:
> -		/* if one reads junk, then probably the card was removed */
> +	
> +	} else if(int_cause == 0xffff) {
> +		/* Read in junk, the card has probably been removed */
>  		card->priv->adapter->surpriseremoved = 1;
> -		break;
> -	case IF_CS_H_IC_TX_OVER:
> -		if (card->priv->adapter->connect_status == LIBERTAS_CONNECTED)
> -			netif_wake_queue(card->priv->dev);
> -		/* fallthrought */
> -	default:
> +	
> +	} else {
> +		if(int_cause & IF_CS_H_IC_TX_OVER) {
> +			card->priv->dnld_sent = DNLD_RES_RECEIVED;
> +			if (!card->priv->adapter->cur_cmd) 
> +				wake_up_interruptible(&card->priv->waitq);
> +			
> +			if (card->priv->adapter->connect_status == LIBERTAS_CONNECTED) 
> +				netif_wake_queue(card->priv->dev);			
> +		}
> +		
>  		/* clear interrupt */
>  		if_cs_write16(card, IF_CS_C_INT_CAUSE, int_cause & IF_CS_C_IC_MASK);
> -		if_cs_disable_ints(card);
>  	}
>  
>  	libertas_interrupt(card->priv->dev);
> @@ -615,6 +619,7 @@
>  	lbs_deb_enter(LBS_DEB_CS);
>  
>  	card->priv = priv;
> +	if_cs_enable_ints(card);
>  
>  	return 0;
>  }
> @@ -622,12 +627,15 @@
>  
>  static int if_cs_unregister_dev(wlan_private *priv)
>  {
> +    	struct if_cs_card *card = (struct if_cs_card *)priv->card;
> +
>  	lbs_deb_enter(LBS_DEB_CS);
>  
>  	/*
>  	 * Nothing special here. Because the device's power gets turned off
>  	 * anyway, there's no need to send a RESET command like in if_usb.c
>  	 */
> +	if_cs_disable_ints(card);
>  
>  	return 0;
>  }
> @@ -662,11 +670,12 @@
>  
>  	switch (type) {
>  	case MVMS_DAT:
> -		priv->dnld_sent = DNLD_CMD_SENT;
> +		priv->dnld_sent = DNLD_DATA_SENT;
>  		if_cs_send_data(priv, buf, nb);
>  		ret = 0;
>  		break;
>  	case MVMS_CMD:
> +		priv->dnld_sent = DNLD_CMD_SENT;
>  		ret = if_cs_send_cmd(priv, buf, nb);
>  		break;
>  	default:
> @@ -696,7 +705,6 @@
>  	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_cs_enable_ints(card);
>  
>  	if (!*ireg)
>  		goto sbi_get_int_status_exit;
> @@ -885,7 +893,6 @@
>  	       p_dev->irq.AssignedIRQ, p_dev->io.BasePort1,
>  	       p_dev->io.BasePort1 + p_dev->io.NumPorts1 - 1);
>  
> -	if_cs_enable_ints(card);
>  
>  	/* Load the firmware early, before calling into libertas.ko */
>  	ret = if_cs_prog_helper(card);
> 
> 
> 
> _______________________________________________
> libertas-dev mailing list
> libertas-dev at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libertas-dev