[patch 0/3] s3cmci SDIO patches

[patch 0/3] s3cmci SDIO patches

[Christer Weinigel]

Hi Ben,

here are the s3cmci SDIO patches that I've done so far.

The first patch modifies the s3cmci driver so that it supports
non-word-sized transfers which is needed by SDIO.

The second patch removes calls the pio_tasklet directly from the
interrupt handler instead of scheduling it as a tasklet.  This reduces
the CPU load for the driver sligthtly.

The third patch adds experimental SDIO interrupt support.  Since there
are issues with spurious interrupts on some S3C processors, I think
this needs more testing before going into the kernel.

I'm working on trying to get the SDIO driver to go faster and use less
CPU, so I'm mainly doing two things right now, first of all I'd like
to try to make DMA work again.  There are hooks in the code for DMA,
but when i tried turning it on, it didn't work, so I'll try to chase
that down.

I'll probably make some other random changes, I'd like to make the
driver support non-word-aligned block transfers for example, something
that it doesn't support right now.  It doesn't seem to be needed for
SDIO Bluetooth Type A, but it would be nice to have it for
correctness.

And I'll try to clean up my patches to add asynchronous operations to
the SDIO layer, which might cause some fallout on the s3cmci driver.

Is there anything you'd like me to do with the s3cmci driver while I'm
at it?

  /Christer

-- 
"Just how much can I get away with and still go to heaven?"

Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

[patch 1/3] s3cmci -- support odd block transfers

[Christer Weinigel]

[-- Attachment #1: s3c_mci-odd-sizes.patch --]
[-- Type: text/plain, Size: 6647 bytes --]

To be able to do SDIO the s3cmci driver has to support non-word-sized
transfers.  Change pio_words into pio_bytes and fix up all the places
where it is used.  

This variant of the patch will not overrun the buffer when reading an
odd number of bytes.  When writing, this variant will still read past
the end of the buffer, but since the driver can't support non-word-
aligned transfers anyway, this should not be a problem, since a
word-aligned transfer will never cross a page boundary.

This has been tested with a CSR SDIO Bluetooth Type A device on a
Samsung S3C24A0 processor.

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
@@ -191,7 +191,7 @@ static inline void clear_imask(struct s3
 }
 
 static inline int get_data_buffer(struct s3cmci_host *host,
-				  u32 *words, u32 **pointer)
+				  u32 *bytes, u32 **pointer)
 {
 	struct scatterlist *sg;
 
@@ -208,7 +208,7 @@ static inline int get_data_buffer(struct
 	}
 	sg = &host->mrq->data->sg[host->pio_sgptr];
 
-	*words = sg->length >> 2;
+	*bytes = sg->length;
 	*pointer = sg_virt(sg);
 
 	host->pio_sgptr++;
@@ -224,7 +224,7 @@ static inline u32 fifo_count(struct s3cm
 	u32 fifostat = readl(host->base + S3C2410_SDIFSTA);
 
 	fifostat &= S3C2410_SDIFSTA_COUNTMASK;
-	return fifostat >> 2;
+	return fifostat;
 }
 
 static inline u32 fifo_free(struct s3cmci_host *host)
@@ -232,13 +232,14 @@ static inline u32 fifo_free(struct s3cmc
 	u32 fifostat = readl(host->base + S3C2410_SDIFSTA);
 
 	fifostat &= S3C2410_SDIFSTA_COUNTMASK;
-	return (63 - fifostat) >> 2;
+	return 63 - fifostat;
 }
 
 static void do_pio_read(struct s3cmci_host *host)
 {
 	int res;
 	u32 fifo;
+	u32 fifo_words;
 	void __iomem *from_ptr;
 
 	/* write real prescaler to host, it might be set slow to fix */
@@ -247,8 +248,8 @@ static void do_pio_read(struct s3cmci_ho
 	from_ptr = host->base + host->sdidata;
 
 	while ((fifo = fifo_count(host))) {
-		if (!host->pio_words) {
-			res = get_data_buffer(host, &host->pio_words,
+		if (!host->pio_bytes) {
+			res = get_data_buffer(host, &host->pio_bytes,
 					      &host->pio_ptr);
 			if (res) {
 				host->pio_active = XFER_NONE;
@@ -261,26 +262,45 @@ static void do_pio_read(struct s3cmci_ho
 
 			dbg(host, dbg_pio,
 			    "pio_read(): new target: [%i]@[%p]\n",
-			    host->pio_words, host->pio_ptr);
+			    host->pio_bytes, host->pio_ptr);
 		}
 
 		dbg(host, dbg_pio,
 		    "pio_read(): fifo:[%02i] buffer:[%03i] dcnt:[%08X]\n",
-		    fifo, host->pio_words,
+		    fifo, host->pio_bytes,
 		    readl(host->base + S3C2410_SDIDCNT));
 
-		if (fifo > host->pio_words)
-			fifo = host->pio_words;
+		/* If we have reached the end of the block, we can
+		 * read a word and get 1 to 3 bytes.  If we in the
+		 * middle of the block, we have to read full words,
+		 * otherwise we will write garbage, so round down to
+		 * an even multiple of 4. */
+		if (fifo >= host->pio_bytes)
+			fifo = host->pio_bytes;
+		else
+			fifo -= fifo & 3;
 
-		host->pio_words -= fifo;
+		host->pio_bytes -= fifo;
 		host->pio_count += fifo;
 
-		while (fifo--)
+		fifo_words = fifo >> 2;
+		while (fifo_words--)
 			*(host->pio_ptr++) = readl(from_ptr);
+
+		if (fifo & 3) {
+			u32 n = fifo & 3;
+			u32 data = readl(from_ptr);
+			u8 *p = (u8 *)host->pio_ptr;
+
+			while (n--) {
+				*p++ = data;
+				data >>= 8;
+			}
+		}
 	}
 
-	if (!host->pio_words) {
-		res = get_data_buffer(host, &host->pio_words, &host->pio_ptr);
+	if (!host->pio_bytes) {
+		res = get_data_buffer(host, &host->pio_bytes, &host->pio_ptr);
 		if (res) {
 			dbg(host, dbg_pio,
 			    "pio_read(): complete (no more buffers).\n");
@@ -304,8 +324,8 @@ static void do_pio_write(struct s3cmci_h
 	to_ptr = host->base + host->sdidata;
 
 	while ((fifo = fifo_free(host))) {
-		if (!host->pio_words) {
-			res = get_data_buffer(host, &host->pio_words,
+		if (!host->pio_bytes) {
+			res = get_data_buffer(host, &host->pio_bytes,
 							&host->pio_ptr);
 			if (res) {
 				dbg(host, dbg_pio,
@@ -317,16 +337,23 @@ static void do_pio_write(struct s3cmci_h
 
 			dbg(host, dbg_pio,
 			    "pio_write(): new source: [%i]@[%p]\n",
-			    host->pio_words, host->pio_ptr);
+			    host->pio_bytes, host->pio_ptr);
 
 		}
 
-		if (fifo > host->pio_words)
-			fifo = host->pio_words;
+		/* If we have reached the end of the block, we have to
+		 * write exactly the remaining number of bytes.  If we
+		 * in the middle of the block, we have to write full
+		 * words, so round down to an even multiple of 4. */
+		if (fifo >= host->pio_bytes)
+			fifo = host->pio_bytes;
+		else
+			fifo -= fifo & 3;
 
-		host->pio_words -= fifo;
+		host->pio_bytes -= fifo;
 		host->pio_count += fifo;
 
+		fifo = (fifo + 3) >> 2;
 		while (fifo--)
 			writel(*(host->pio_ptr++), to_ptr);
 	}
@@ -351,9 +378,9 @@ static void pio_tasklet(unsigned long da
 		clear_imask(host);
 		if (host->pio_active != XFER_NONE) {
 			dbg(host, dbg_err, "unfinished %s "
-			    "- pio_count:[%u] pio_words:[%u]\n",
+			    "- pio_count:[%u] pio_bytes:[%u]\n",
 			    (host->pio_active == XFER_READ) ? "read" : "write",
-			    host->pio_count, host->pio_words);
+			    host->pio_count, host->pio_bytes);
 
 			if (host->mrq->data)
 				host->mrq->data->error = -EINVAL;
@@ -813,11 +840,10 @@ static int s3cmci_setup_data(struct s3cm
 		/* We cannot deal with unaligned blocks with more than
 		 * one block being transfered. */
 
-		if (data->blocks > 1)
+		if (data->blocks > 1) {
+			pr_warning("%s: can't do non-word sized block transfers (blksz %d)\n", __func__, data->blksz);
 			return -EINVAL;
-
-		/* No support yet for non-word block transfers. */
-		return -EINVAL;
+		}
 	}
 
 	while (readl(host->base + S3C2410_SDIDSTA) &
@@ -897,7 +923,7 @@ static int s3cmci_prepare_pio(struct s3c
 	BUG_ON((data->flags & BOTH_DIR) == BOTH_DIR);
 
 	host->pio_sgptr = 0;
-	host->pio_words = 0;
+	host->pio_bytes = 0;
 	host->pio_count = 0;
 	host->pio_active = rw ? XFER_WRITE : XFER_READ;
 
Index: linux-2.6.26.2/drivers/mmc/host/s3cmci.h
===================================================================
--- linux-2.6.26.2.orig/drivers/mmc/host/s3cmci.h
+++ linux-2.6.26.2/drivers/mmc/host/s3cmci.h
@@ -51,7 +51,7 @@ struct s3cmci_host {
 	int			dma_complete;
 
 	u32			pio_sgptr;
-	u32			pio_words;
+	u32			pio_bytes;
 	u32			pio_count;
 	u32			*pio_ptr;
 #define XFER_NONE 0

-- 
"Just how much can I get away with and still go to heaven?"

Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

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

[Christer Weinigel]

[-- Attachment #1: s3c_mci-no-tasklet.patch --]
[-- Type: text/plain, Size: 2851 bytes --]

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 s3cmci_probe(struct
 	}
 
 	spin_lock_init(&host->complete_lock);
-	tasklet_init(&host->pio_tasklet, pio_tasklet, (unsigned long) host);
 
 	if (is2440) {
 		host->sdiimsk	= S3C2440_SDIIMSK;
@@ -1381,7 +1378,6 @@ static int __devexit s3cmci_remove(struc
 
 	clk_put(host->clk);
 
-	tasklet_disable(&host->pio_tasklet);
 	s3c2410_dma_free(S3CMCI_DMA, &s3cmci_dma_client);
 
 	free_irq(host->irq, host);
Index: linux-2.6.26.2/drivers/mmc/host/s3cmci.h
===================================================================
--- linux-2.6.26.2.orig/drivers/mmc/host/s3cmci.h
+++ linux-2.6.26.2/drivers/mmc/host/s3cmci.h
@@ -66,5 +66,4 @@ struct s3cmci_host {
 	char			*status;
 
 	unsigned int		ccnt, dcnt;
-	struct tasklet_struct	pio_tasklet;
 };

-- 
"Just how much can I get away with and still go to heaven?"

Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

[patch 3/3] s3cmci - add SDIO interrupt support

[Christer Weinigel]

[-- Attachment #1: s3c_mci-sdio-irq.patch --]
[-- Type: text/plain, Size: 5611 bytes --]

This adds support for the SDIO irq to the s3cmci driver.

The driver calls to enable_irq and disable_irq are not matched
properly when doing SDIO, so I've worked around this by counting the
number of enables and disables.  This is not the proper way to do it,
but makes things work for me the moment on a Samsung S3C24A0
processor.  There are supposed to be issues with spurious interrupts
on some S3C processors, so this needs more investigation and testing,
but I thought this patch may be useful 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
@@ -187,7 +187,11 @@ static inline u32 disable_imask(struct s
 
 static inline void clear_imask(struct s3cmci_host *host)
 {
-	writel(0, host->base + host->sdiimsk);
+	u32 newmask;
+
+	newmask = readl(host->base + host->sdiimsk);
+	newmask &= S3C2410_SDIIMSK_SDIOIRQ;
+	writel(newmask, host->base + host->sdiimsk);
 }
 
 static inline int get_data_buffer(struct s3cmci_host *host,
@@ -364,6 +368,7 @@ static void do_pio_write(struct s3cmci_h
 static void pio_tasklet(struct s3cmci_host *host)
 {
 	disable_irq(host->irq);
+	host->irq_disabled++;
 
 	if (host->pio_active == XFER_WRITE)
 		do_pio_write(host);
@@ -384,8 +389,12 @@ static void pio_tasklet(struct s3cmci_ho
 		}
 
 		finalize_request(host);
-	} else
+	}
+
+	if (host->irq_disabled) {
 		enable_irq(host->irq);
+		host->irq_disabled--;
+	}
 }
 
 /*
@@ -422,6 +431,7 @@ static irqreturn_t s3cmci_irq(int irq, v
 	u32 mci_csta, mci_dsta, mci_fsta, mci_dcnt, mci_imsk;
 	u32 mci_cclear, mci_dclear;
 	unsigned long iflags;
+	int cardint = 0;
 
 	spin_lock_irqsave(&host->complete_lock, iflags);
 
@@ -433,6 +443,13 @@ static irqreturn_t s3cmci_irq(int irq, v
 	mci_cclear = 0;
 	mci_dclear = 0;
 
+	if (mci_dsta & S3C2410_SDIDSTA_SDIOIRQDETECT &&
+	    mci_imsk & S3C2410_SDIIMSK_SDIOIRQ) {
+		cardint = 1;
+		mci_dclear = S3C2410_SDIDSTA_SDIOIRQDETECT;
+		goto clear_status_bits;
+	}
+
 	if ((host->complete_what == COMPLETION_NONE) ||
 	    (host->complete_what == COMPLETION_FINALIZE)) {
 		host->status = "nothing to complete";
@@ -603,8 +620,12 @@ irq_out:
 	    mci_csta, mci_dsta, mci_fsta, mci_dcnt, host->status);
 
 	spin_unlock_irqrestore(&host->complete_lock, iflags);
-	return IRQ_HANDLED;
 
+	/* We have to delay this as it calls back into the driver. */
+	if (cardint)
+		mmc_signal_sdio_irq(host->mmc);
+
+	return IRQ_HANDLED;
 }
 
 /*
@@ -671,7 +692,7 @@ out:
 fail_request:
 	host->mrq->data->error = -EINVAL;
 	host->complete_what = COMPLETION_FINALIZE;
-	writel(0, host->base + host->sdiimsk);
+	clear_imask(host);
 	goto out;
 
 }
@@ -716,7 +737,7 @@ static void finalize_request(struct s3cm
 	writel(0, host->base + S3C2410_SDICMDARG);
 	writel(S3C2410_SDIDCON_STOP, host->base + S3C2410_SDIDCON);
 	writel(0, host->base + S3C2410_SDICMDCON);
-	writel(0, host->base + host->sdiimsk);
+	clear_imask(host);
 
 	if (cmd->data && cmd->error)
 		cmd->data->error = cmd->error;
@@ -1026,7 +1047,10 @@ static void s3cmci_send_request(struct m
 	s3cmci_send_command(host, cmd);
 
 	/* Enable Interrupt */
-	enable_irq(host->irq);
+	if (host->irq_disabled) {
+		enable_irq(host->irq);
+		host->irq_disabled--;
+	}
 }
 
 static int s3cmci_card_present(struct s3cmci_host *host)
@@ -1119,9 +1143,9 @@ static void s3cmci_set_ios(struct mmc_ho
 
 	/* Set CLOCK_ENABLE */
 	if (ios->clock)
-		mci_con |= S3C2410_SDICON_CLOCKTYPE;
+		mci_con |= S3C2410_SDICON_CLOCKTYPE | S3C2410_SDICON_SDIOIRQ;
 	else
-		mci_con &= ~S3C2410_SDICON_CLOCKTYPE;
+		mci_con &= ~(S3C2410_SDICON_CLOCKTYPE | S3C2410_SDICON_SDIOIRQ);
 
 	writel(mci_con, host->base + S3C2410_SDICON);
 
@@ -1161,10 +1185,26 @@ static int s3cmci_get_ro(struct mmc_host
 	return ret;
 }
 
+static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+	struct s3cmci_host *host = mmc_priv(mmc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->complete_lock, flags);
+
+	if (enable)
+		enable_imask(host, S3C2410_SDIIMSK_SDIOIRQ);
+	else
+		disable_imask(host, S3C2410_SDIIMSK_SDIOIRQ);
+
+	spin_unlock_irqrestore(&host->complete_lock, flags);
+}
+
 static struct mmc_host_ops s3cmci_ops = {
 	.request	= s3cmci_request,
 	.set_ios	= s3cmci_set_ios,
 	.get_ro		= s3cmci_get_ro,
+	.enable_sdio_irq = s3cmci_enable_sdio_irq,
 };
 
 static struct s3c24xx_mci_pdata s3cmci_def_pdata = {
@@ -1256,6 +1296,7 @@ static int __devinit s3cmci_probe(struct
 	 * ensure we don't lock the system with un-serviceable requests. */
 
 	disable_irq(host->irq);
+	host->irq_disabled++;
 
 	host->irq_cd = s3c2410_gpio_getirq(host->pdata->gpio_detect);
 
@@ -1301,7 +1342,7 @@ static int __devinit s3cmci_probe(struct
 
 	mmc->ops 	= &s3cmci_ops;
 	mmc->ocr_avail	= MMC_VDD_32_33 | MMC_VDD_33_34;
-	mmc->caps	= MMC_CAP_4_BIT_DATA;
+	mmc->caps	= MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ;
 	mmc->f_min 	= host->clk_rate / (host->clk_div * 256);
 	mmc->f_max 	= host->clk_rate / host->clk_div;
 
Index: linux-2.6.26.2/drivers/mmc/host/s3cmci.h
===================================================================
--- linux-2.6.26.2.orig/drivers/mmc/host/s3cmci.h
+++ linux-2.6.26.2/drivers/mmc/host/s3cmci.h
@@ -41,6 +41,7 @@ struct s3cmci_host {
 	unsigned		sdidata;
 	int			dodma;
 	int			dmatogo;
+	int                     irq_disabled;
 
 	struct mmc_request	*mrq;
 	int			cmd_is_stop;

-- 
"Just how much can I get away with and still go to heaven?"

Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

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

[Ben Dooks]

On Mon, Sep 08, 2008 at 02:48:50PM +0200, Christer Weinigel wrote:
> 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.

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
is going on.
 
> 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 s3cmci_probe(struct
>  	}
>  
>  	spin_lock_init(&host->complete_lock);
> -	tasklet_init(&host->pio_tasklet, pio_tasklet, (unsigned long) host);
>  
>  	if (is2440) {
>  		host->sdiimsk	= S3C2440_SDIIMSK;
> @@ -1381,7 +1378,6 @@ static int __devexit s3cmci_remove(struc
>  
>  	clk_put(host->clk);
>  
> -	tasklet_disable(&host->pio_tasklet);
>  	s3c2410_dma_free(S3CMCI_DMA, &s3cmci_dma_client);
>  
>  	free_irq(host->irq, host);
> Index: linux-2.6.26.2/drivers/mmc/host/s3cmci.h
> ===================================================================
> --- linux-2.6.26.2.orig/drivers/mmc/host/s3cmci.h
> +++ linux-2.6.26.2/drivers/mmc/host/s3cmci.h
> @@ -66,5 +66,4 @@ struct s3cmci_host {
>  	char			*status;
>  
>  	unsigned int		ccnt, dcnt;
> -	struct tasklet_struct	pio_tasklet;
>  };
> 
> -- 
> "Just how much can I get away with and still go to heaven?"
> 
> Christer Weinigel <christer@weinigel.se>  http://www.weinigel.se

-- 
-- 
Ben

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

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

[Christer Weinigel]

Ben Dooks wrote:
> On Mon, Sep 08, 2008 at 02:48:50PM +0200, Christer Weinigel wrote:
>> 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.
> 
> 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
> is going on.

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

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

[Christer Weinigel]

Ben Dooks wrote:
> On Mon, Sep 08, 2008 at 02:48:50PM +0200, Christer Weinigel wrote:
>> 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.
> 
> 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
> is going on.

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