Date: Fri, 1 Aug 2008 15:49:31 +0200
From: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
To: Gerard Kam <gerardk5@verizon.net>
Cc: David Brownell <david-b@pacbell.net>,
       spi-devel-general@lists.sourceforge.net,
       Lars Steubesand <lars.steubesand@philips.com>,
       linux-kernel@vger.kernel.org
Subject: Re: [PATCH] atmel_spi: fix hang due to missed interrupt
Message-ID: <20080801154931.120632f1@hskinnemo-gx745.norway.atmel.com>
In-Reply-To: <1217524213-4027-1-git-send-email-haavard.skinnemoen@atmel.com>
References: <1217524213-4027-1-git-send-email-haavard.skinnemoen@atmel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1764
Lines: 37

Haavard Skinnemoen <haavard.skinnemoen@atmel.com> wrote:
>  		spi_writel(as, RNCR, 0);
>  		spi_writel(as, TNCR, 0);
> +		ieval = SPI_BIT(RXBUFF) | SPI_BIT(ENDRX) | SPI_BIT(OVRES);

Actually, I think the real bug happens right here: Writing RNCR to 0
will clear any pending ENDRX interrupt, so if the transfer is completed
before this, we won't see any interrupt. These writes are also
completely pointless -- RNCR is zeroed automatically after it gets
shifted into RCR. TNCR works the same way.

The RXBUFF interrupt is only cleared by writing a nonzero RCR or RNCR,
so your patch should fix it. But I'm wondering if there may be another
race left to fix: If we queue two transfers, and both of them complete
before we handle the interrupt, I think we only consider one of them to
be complete. If RXBUFF is set, we should complete any "next" transfer
we have queued up as well.

It could be your patch fixes this last case too -- when this happens,
RXBUFF stays set when we return from the interrupt handler, so the
interrupt gets retriggered immediately. We could handle this more
efficiently, but I think it's handled correctly with your patch applied.

I'll see if I can find a way to clean up the somewhat headache-inducing
control flow in this driver, but until then, your patch should
definitely improve things.

As for the overruns, I'm beginning to suspect that the only way to get
rid of those and still maintain a reasonable transfer rate is to use
bounce buffers in faster RAM (e.g. on-chip SRAM).

Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/