Re: [patch 2/3] s3cmci - call pio_tasklet from IRQ

Previous thread: [patch 0/3] s3cmci SDIO patches by Christer Weinigel on Monday, September 8, 2008 - 5:48 am. (1 message)

Next thread: [patch 1/3] s3cmci -- support odd block transfers by Christer Weinigel on Monday, September 8, 2008 - 5:48 am. (1 message)
From: Christer Weinigel
Date: Monday, September 8, 2008 - 5:48 am

Scheduling a tasklet to perform the pio transfer introduces a bit of
extra processing, just call pio_tasklet directly from the interrupt
instead.  Writing up to 64 bytes to a FIFO is probably uses less CPU
than scheduling a tasklet anyway.

Signed-off-by: Christer Weinigel <christer@weinigel.se>

Index: linux-2.6.26.2/drivers/mmc/host/s3cmci.c
===================================================================
--- linux-2.6.26.2.orig/drivers/mmc/host/s3cmci.c
+++ linux-2.6.26.2/drivers/mmc/host/s3cmci.c
@@ -361,11 +361,8 @@ static void do_pio_write(struct s3cmci_h
 	enable_imask(host, S3C2410_SDIIMSK_TXFIFOHALF);
 }
 
-static void pio_tasklet(unsigned long data)
+static void pio_tasklet(struct s3cmci_host *host)
 {
-	struct s3cmci_host *host = (struct s3cmci_host *) data;
-
-
 	disable_irq(host->irq);
 
 	if (host->pio_active == XFER_WRITE)
@@ -460,10 +457,10 @@ static irqreturn_t s3cmci_irq(int irq, v
 	if (!host->dodma) {
 		if ((host->pio_active == XFER_WRITE) &&
 		    (mci_fsta & S3C2410_SDIFSTA_TFDET)) {
-
 			disable_imask(host, S3C2410_SDIIMSK_TXFIFOHALF);
-			tasklet_schedule(&host->pio_tasklet);
+			pio_tasklet(host);
 			host->status = "pio tx";
+			goto irq_out;
 		}
 
 		if ((host->pio_active == XFER_READ) &&
@@ -473,8 +470,9 @@ static irqreturn_t s3cmci_irq(int irq, v
 				      S3C2410_SDIIMSK_RXFIFOHALF |
 				      S3C2410_SDIIMSK_RXFIFOLAST);
 
-			tasklet_schedule(&host->pio_tasklet);
+			pio_tasklet(host);
 			host->status = "pio rx";
+			goto irq_out;
 		}
 	}
 
@@ -595,7 +593,7 @@ close_transfer:
 	host->complete_what = COMPLETION_FINALIZE;
 
 	clear_imask(host);
-	tasklet_schedule(&host->pio_tasklet);
+	pio_tasklet(host);
 
 	goto irq_out;
 
@@ -666,7 +664,7 @@ void s3cmci_dma_done_callback(struct s3c
 	host->complete_what = COMPLETION_FINALIZE;
 
 out:
-	tasklet_schedule(&host->pio_tasklet);
+	pio_tasklet(host);
 	spin_unlock_irqrestore(&host->complete_lock, iflags);
 	return;
 
@@ -1198,7 +1196,6 @@ static int __devinit ...
From: Ben Dooks
Date: Monday, September 8, 2008 - 6:46 am

Hmm, i'd be interested to find out how long these are taking... I might
try and rig up something to test the time being taken via an SMDK.

If the fifo read/writes are taking significant amounts of time, then the
pio tasklet will at least improve the interrupt latencies invloved, as
iirc we're currently running the main irq handler in IRQ_DISABLED mode
to stop any problems with re-enternancy.... I'll check this and see what

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

--

From: Christer Weinigel
Date: Monday, September 8, 2008 - 7:04 am

It should be possible to set a flag and then call the pio_task after the 
spin_unlock_irqrestore instead.  I didn't want to do that to change as 
little of the logic as possible, but it's probably better to do that.

I'm also thinking of changing send_request to do a busy wait for 
commands without data, that will probably need a bit larger changes.

   /Christer
--

From: Christer Weinigel
Date: Monday, September 8, 2008 - 8:12 am

Ok, I just measured this on the 200 MHz S3C24A0, when running the SDIO 
bus at 10 MHz, the longest time I saw the driver spend in the pio_read 
function was ~10us.  I guess that means that the hardware managed to 
empty the fifo enough to do yet another spin through the loop.  So with 
a faster SDIO clock the time spent in pio_read ought to go up, and for a 
long transfer it could grow without bounds.

I also tried to run the code with the schedule_tasklet still there, and 
then I saw ~13us as the longest time spent in the loop, and every now 
and then there was a ~10ms gap when the clock stopped.

If we have working DMA, I think the PIO tasklet is unneccesary, then 
we'll do PIO for short transfers which won't affect latency much, and 
use DMA for long transfers that would have affected latency if done with 
PIO from interrupt context.

   /Christer

--

Previous thread: [patch 0/3] s3cmci SDIO patches by Christer Weinigel on Monday, September 8, 2008 - 5:48 am. (1 message)

Next thread: [patch 1/3] s3cmci -- support odd block transfers by Christer Weinigel on Monday, September 8, 2008 - 5:48 am. (1 message)