[PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[David Vrabel]

These patches add support for SDIO cards to the omap_hsmmc driver.  Less
restrictions on the sizes of transfers, power management changes to 
prevent SDIO cards from being turned off and losing all state, and card
interrupts.

I've been unable to test these exact patches as I only have an N900 for 
testing and the N900 support in mainline is incomplete.

David Vrabel (3):
  mmc: omap_hsmmc: use packet sync'd DMA
  mmc: omap_hsmmc: don't turn SDIO cards off when idle
  mmc: omap_hsmmc: enable SDIO card interrupts

 drivers/mmc/host/omap_hsmmc.c |  123 ++++++++++++++++++++++++++---------------
 1 files changed, 78 insertions(+), 45 deletions(-)

-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

[PATCH 1/3] mmc: omap_hsmmc: use packet sync'd DMA

[David Vrabel]

Use packet sync'd DMA in the omap_hsmmc driver to avoid the
restriction that transfers must be a multiple of the block size.  This
is required for byte-mode transfers to SDIO cards.

Signed-off-by: David Vrabel <david.vrabel@csr.com>
---
 drivers/mmc/host/omap_hsmmc.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4b23225..e7142a2 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -114,6 +114,7 @@
 
 #define MMC_TIMEOUT_MS		20
 #define OMAP_MMC_MASTER_CLOCK	96000000
+#define OMAP_HSMMC_FIFO_WORDS	(512/4)
 #define DRIVER_NAME		"mmci-omap-hs"
 
 /* Timeouts for entering power saving states on inactivity, msec */
@@ -887,12 +888,12 @@ static void omap_hsmmc_config_dma_params(struct omap_hsmmc_host *host,
 	dma_ch = host->dma_ch;
 	if (data->flags & MMC_DATA_WRITE) {
 		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
+			(host->mapbase + OMAP_HSMMC_DATA), 0, OMAP_HSMMC_FIFO_WORDS);
 		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
 			sg_dma_address(sgl), 0, 0);
 	} else {
 		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
+			(host->mapbase + OMAP_HSMMC_DATA), 0, OMAP_HSMMC_FIFO_WORDS);
 		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
 			sg_dma_address(sgl), 0, 0);
 	}
@@ -901,7 +902,7 @@ static void omap_hsmmc_config_dma_params(struct omap_hsmmc_host *host,
 	nblk = sg_dma_len(sgl) / blksz;
 
 	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
-			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
+			blksz / 4, nblk, OMAP_DMA_SYNC_PACKET,
 			omap_hsmmc_get_dma_sync_dev(host, data),
 			!(data->flags & MMC_DATA_WRITE));
 
@@ -944,17 +945,9 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status, void *data)
 static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 					struct mmc_request *req)
 {
-	int dma_ch = 0, ret = 0, err = 1, i;
+	int dma_ch = 0, ret = 0, err = 1;
 	struct mmc_data *data = req->data;
 
-	/* Sanity check: all the SG entries must be aligned by block size. */
-	for (i = 0; i < data->sg_len; i++) {
-		struct scatterlist *sgl;
-
-		sgl = data->sg + i;
-		if (sgl->length % data->blksz)
-			return -EINVAL;
-	}
 	if ((data->blksz % 4) != 0)
 		/* REVISIT: The MMC buffer increments only when MSB is written.
 		 * Return error for blksz which is non multiple of four.
-- 
1.6.3.3

[PATCH 2/3] mmc: omap_hsmmc: don't turn SDIO cards off when idle

[David Vrabel]

Don't turn SDIO cards off to save power.  Doing so will lose all
internal state in the card.

Signed-off-by: David Vrabel <david.vrabel@csr.com>
---
 drivers/mmc/host/omap_hsmmc.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index e7142a2..e6d8cb3 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -28,6 +28,7 @@
 #include <linux/clk.h>
 #include <linux/mmc/host.h>
 #include <linux/mmc/core.h>
+#include <linux/mmc/card.h>
 #include <linux/io.h>
 #include <linux/semaphore.h>
 #include <plat/dma.h>
@@ -1359,8 +1360,12 @@ static int omap_hsmmc_sleep_to_off(struct omap_hsmmc_host *host)
 	      mmc_slot(host).card_detect ||
 	      (mmc_slot(host).get_cover_state &&
 	       mmc_slot(host).get_cover_state(host->dev, host->slot_id)))) {
-		mmc_release_host(host->mmc);
-		return 0;
+		goto out;
+	}
+
+	/* Don't turn SDIO cards off. */
+	if (host->mmc->card && mmc_card_sdio(host->mmc->card)) {
+		goto out;
 	}
 
 	mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
@@ -1371,9 +1376,8 @@ static int omap_hsmmc_sleep_to_off(struct omap_hsmmc_host *host)
 		host->dpm_state == CARDSLEEP ? "CARDSLEEP" : "REGSLEEP");
 
 	host->dpm_state = OFF;
-
+out:
 	mmc_release_host(host->mmc);
-
 	return 0;
 }
 
-- 
1.6.3.3

[PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
smart-idle mode.

FCLK must be enabled while SDIO interrupts are enabled or the MMC block
won't wake-up.

The writes to STAT and ISE when starting a command are unnecessary and
have been removed.

Signed-off-by: David Vrabel <david.vrabel@csr.com>
---
As noted in the FIXME comment, I think the correct thing to do is to
always leave FCLK enabled.  The clock/clockdomain subsystems should be
configuring smart-idle mode making explicit calls to disable FCLK
unnecessary.  I couldn't follow the clock subsystem to see if it was
actually doing this, though.
---
 drivers/mmc/host/omap_hsmmc.c |   94 ++++++++++++++++++++++++++++-------------
 1 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index e6d8cb3..ad0f867 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -64,6 +64,7 @@
 #define SDVS_MASK		0x00000E00
 #define SDVSCLR			0xFFFFF1FF
 #define SDVSDET			0x00000400
+#define ENAWAKEUP		(1 << 2)
 #define AUTOIDLE		0x1
 #define SDBP			(1 << 8)
 #define DTO			0xe
@@ -74,9 +75,11 @@
 #define CLKD_SHIFT		6
 #define DTO_MASK		0x000F0000
 #define DTO_SHIFT		16
+#define CIRQ_ENABLE		(1 << 8)
 #define INT_EN_MASK		0x307F0033
 #define BWR_ENABLE		(1 << 4)
 #define BRR_ENABLE		(1 << 5)
+#define CTPL			(1 << 11)
 #define INIT_STREAM		(1 << 1)
 #define DP_SELECT		(1 << 21)
 #define DDIR			(1 << 4)
@@ -84,10 +87,12 @@
 #define MSBS			(1 << 5)
 #define BCE			(1 << 1)
 #define FOUR_BIT		(1 << 1)
+#define IWE			(1 << 24)
 #define DW8			(1 << 5)
 #define CC			0x1
 #define TC			0x02
 #define OD			0x1
+#define CIRQ			(1 << 8)
 #define ERR			(1 << 15)
 #define CMD_TIMEOUT		(1 << 16)
 #define DATA_TIMEOUT		(1 << 20)
@@ -228,7 +233,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
 		;
 
 	OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
-			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
+			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE | ENAWAKEUP);
 
 	if (host->id == OMAP_MMC1_DEVID) {
 		if (host->power_mode != MMC_POWER_OFF &&
@@ -243,7 +248,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
 	}
 
 	OMAP_HSMMC_WRITE(host->base, HCTL,
-			OMAP_HSMMC_READ(host->base, HCTL) | hctl);
+			OMAP_HSMMC_READ(host->base, HCTL) | hctl | IWE);
 
 	OMAP_HSMMC_WRITE(host->base, CAPA,
 			OMAP_HSMMC_READ(host->base, CAPA) | capa);
@@ -257,7 +262,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
 		;
 
 	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
-	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
+	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
 	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
 
 	/* Do not initialize card-specific things if the power is off */
@@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host *host, struct mmc_command *cmd,
 		mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
 	host->cmd = cmd;
 
-	/*
-	 * Clear status bits and enable interrupts
-	 */
-	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
-	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
-
 	if (host->use_dma)
 		OMAP_HSMMC_WRITE(host->base, IE,
 				 INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));
@@ -638,18 +637,21 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
 {
 	struct omap_hsmmc_host *host = dev_id;
 	struct mmc_data *data;
-	int end_cmd = 0, end_trans = 0, status;
+	u32 status;
+	int end_cmd = 0, end_trans = 0;
+	bool card_irq = false;
 
 	spin_lock(&host->irq_lock);
 
-	if (host->mrq == NULL) {
-		OMAP_HSMMC_WRITE(host->base, STAT,
-			OMAP_HSMMC_READ(host->base, STAT));
-		/* Flush posted write */
-		OMAP_HSMMC_READ(host->base, STAT);
-		spin_unlock(&host->irq_lock);
-		return IRQ_HANDLED;
-	}
+	status = OMAP_HSMMC_READ(host->base, STAT);
+	OMAP_HSMMC_WRITE(host->base, STAT, status);
+	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
+
+	if (status & CIRQ)
+		card_irq = true;
+
+	if (host->mrq == NULL)
+		goto out;
 
 	data = host->data;
 	status = OMAP_HSMMC_READ(host->base, STAT);
@@ -704,17 +706,16 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
 		}
 	}
 
-	OMAP_HSMMC_WRITE(host->base, STAT, status);
-	/* Flush posted write */
-	OMAP_HSMMC_READ(host->base, STAT);
-
 	if (end_cmd || ((status & CC) && host->cmd))
 		omap_hsmmc_cmd_done(host, host->cmd);
 	if ((end_trans || (status & TC)) && host->mrq)
 		omap_hsmmc_xfer_done(host, data);
-
+out:
 	spin_unlock(&host->irq_lock);
 
+	if (card_irq)
+		mmc_signal_sdio_irq(host->mmc);
+
 	return IRQ_HANDLED;
 }
 
@@ -1248,6 +1249,41 @@ static int omap_hsmmc_get_ro(struct mmc_host *mmc)
 	return mmc_slot(host).get_ro(host->dev, 0);
 }
 
+static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
+{
+	struct omap_hsmmc_host *host = mmc_priv(mmc);
+	u32 ie, con;
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->irq_lock, flags);
+
+	/*
+	 * When interrupts are enabled, CTPL must be set to enable
+	 * DAT1 input buffer (or the card interrupt is always
+	 * asserted) and FCLK must be enabled else wakeup does not
+	 * work.
+	 *
+	 * FIXME: the power save code probably shouldn't be playing
+	 * with FCLK and allow the clock subsystem to put it into
+	 * smart-idle mode instead.
+	 */
+	con = OMAP_HSMMC_READ(host->base, CON);
+	ie = OMAP_HSMMC_READ(host->base, IE);
+	if (enable) {
+		clk_enable(host->fclk);
+		ie |= CIRQ_ENABLE;
+		con |= CTPL;
+	} else {
+		clk_disable(host->fclk);
+		ie &= ~CIRQ_ENABLE;
+		con &= ~CTPL;
+	}
+	OMAP_HSMMC_WRITE(host->base, CON, con);
+	OMAP_HSMMC_WRITE(host->base, IE, ie);
+
+	spin_unlock_irqrestore(&host->irq_lock, flags);
+}
+
 static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
 {
 	u32 hctl, capa, value;
@@ -1262,14 +1298,14 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
 	}
 
 	value = OMAP_HSMMC_READ(host->base, HCTL) & ~SDVS_MASK;
-	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl);
+	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl | IWE);
 
 	value = OMAP_HSMMC_READ(host->base, CAPA);
 	OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
 
 	/* Set the controller to AUTO IDLE mode */
 	value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
-	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
+	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE | ENAWAKEUP);
 
 	/* Set SD bus power bit */
 	set_sd_bus_power(host);
@@ -1516,7 +1552,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = {
 	.set_ios = omap_hsmmc_set_ios,
 	.get_cd = omap_hsmmc_get_cd,
 	.get_ro = omap_hsmmc_get_ro,
-	/* NYET -- enable_sdio_irq */
+	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
 };
 
 static const struct mmc_host_ops omap_hsmmc_ps_ops = {
@@ -1526,7 +1562,7 @@ static const struct mmc_host_ops omap_hsmmc_ps_ops = {
 	.set_ios = omap_hsmmc_set_ios,
 	.get_cd = omap_hsmmc_get_cd,
 	.get_ro = omap_hsmmc_get_ro,
-	/* NYET -- enable_sdio_irq */
+	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -1731,7 +1767,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	mmc->max_seg_size = mmc->max_req_size;
 
 	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
-		     MMC_CAP_WAIT_WHILE_BUSY;
+		     MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_SDIO_IRQ;
 
 	if (mmc_slot(host).wires >= 8)
 		mmc->caps |= MMC_CAP_8_BIT_DATA;
@@ -1802,7 +1838,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 		}
 	}
 
-	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
+	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
 	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
 
 	mmc_host_lazy_disable(host->mmc);
-- 
1.6.3.3

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

Hi David,

David Vrabel wrote:
> These patches add support for SDIO cards to the omap_hsmmc driver.  Less
> restrictions on the sizes of transfers, power management changes to 
> prevent SDIO cards from being turned off and losing all state, and card
> interrupts.
> 
> I've been unable to test these exact patches as I only have an N900 for 
> testing and the N900 support in mainline is incomplete.

I've tried your patches on CM-T35 that has 8686 SDIO and the result was
absence of MMC/SDIO devices at all :( Morover, kmmcd hangs:

[  399.427764] INFO: task kmmcd:149 blocked for more than 120 seconds.

[  399.434082] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.

[  399.442016] kmmcd         D c027f804     0   149      2 0x00000000

[  399.448333] [<c027f804>] (schedule+0x274/0x2ac) from [<c027fa08>]
(schedule_timeout+0x18/0x17c)

[  399.457122] [<c027fa08>] (schedule_timeout+0x18/0x17c) from
[<c027f484>] (wait_for_common+0xc0/0x14c)

[  399.466461] [<c027f484>] (wait_for_common+0xc0/0x14c) from
[<c01e61e0>] (mmc_wait_for_req+0x1f4/0x214)

[  399.475860] [<c01e61e0>] (mmc_wait_for_req+0x1f4/0x214) from
[<c01e6264>] (mmc_wait_for_cmd+0x64/0x74)

[  399.485290] [<c01e6264>] (mmc_wait_for_cmd+0x64/0x74) from
[<c01e86b0>] (mmc_go_idle+0x60/0xac)

[  399.494079] [<c01e86b0>] (mmc_go_idle+0x60/0xac) from [<c01e6bb8>]
(mmc_rescan+0x150/0x208)

[  399.502532] [<c01e6bb8>] (mmc_rescan+0x150/0x208) from [<c005e288>]
(worker_thread+0xf0/0x168)

[  399.511260] [<c005e288>] (worker_thread+0xf0/0x168) from [<c0060dbc>]
(kthread+0x7c/0x84)

[  399.519531] [<c0060dbc>] (kthread+0x7c/0x84) from [<c002ae50>]
(kernel_thread_exit+0x0/0x8)


I'll try to debug it more today and see what is going on wrong...

> David Vrabel (3):
>   mmc: omap_hsmmc: use packet sync'd DMA
>   mmc: omap_hsmmc: don't turn SDIO cards off when idle
>   mmc: omap_hsmmc: enable SDIO card interrupts
> 
>  drivers/mmc/host/omap_hsmmc.c |  123 ++++++++++++++++++++++++++---------------
>  1 files changed, 78 insertions(+), 45 deletions(-)
> 


-- 
Sincerely yours,
Mike.

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[David Vrabel]

Mike Rapoport wrote:
> Hi David,
> 
> David Vrabel wrote:
>> These patches add support for SDIO cards to the omap_hsmmc driver.  Less
>> restrictions on the sizes of transfers, power management changes to 
>> prevent SDIO cards from being turned off and losing all state, and card
>> interrupts.
>>
>> I've been unable to test these exact patches as I only have an N900 for 
>> testing and the N900 support in mainline is incomplete.
> 
> I've tried your patches on CM-T35 that has 8686 SDIO and the result was
> absence of MMC/SDIO devices at all :( Morover, kmmcd hangs:
> 
> [  399.427764] INFO: task kmmcd:149 blocked for more than 120 seconds.
> [...]
> [  399.485290] [<c01e6264>] (mmc_wait_for_cmd+0x64/0x74) from
> [<c01e86b0>] (mmc_go_idle+0x60/0xac)

I suspect interrupts aren't enabled correctly as this command does not
complete.  Try reverting this hunk.

@@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
*host, struct mmc_command *cmd,
                mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
        host->cmd = cmd;

-       /*
-        * Clear status bits and enable interrupts
-        */
-       OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
-       OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
-
        if (host->use_dma)
                OMAP_HSMMC_WRITE(host->base, IE,
                                 INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

David Vrabel wrote:
> Mike Rapoport wrote:
>> Hi David,
>>
>> David Vrabel wrote:
>>> These patches add support for SDIO cards to the omap_hsmmc driver.  Less
>>> restrictions on the sizes of transfers, power management changes to 
>>> prevent SDIO cards from being turned off and losing all state, and card
>>> interrupts.
>>>
>>> I've been unable to test these exact patches as I only have an N900 for 
>>> testing and the N900 support in mainline is incomplete.
>> I've tried your patches on CM-T35 that has 8686 SDIO and the result was
>> absence of MMC/SDIO devices at all :( Morover, kmmcd hangs:
>>
>> [  399.427764] INFO: task kmmcd:149 blocked for more than 120 seconds.
>> [...]
>> [  399.485290] [<c01e6264>] (mmc_wait_for_cmd+0x64/0x74) from
>> [<c01e86b0>] (mmc_go_idle+0x60/0xac)
> 
> I suspect interrupts aren't enabled correctly as this command does not
> complete.  Try reverting this hunk.

I did. No changes...

I've started to apply the patches one by one and after the first patch
is applied, the SD card works Ok, but libertas fails to initialize with
the following messages:

> modprobe libertas_sdio
[   82.233489] lib80211: common routines for IEEE802.11 drivers
[   82.833251] cfg80211: Calling CRDA to update world regulatory domain
[   83.327911] libertas_sdio: Libertas SDIO driver
[   83.332489] libertas_sdio: Copyright Pierre Ossman
[   83.348510] libertas_sdio mmc1:0001:1: firmware: requesting
sd8686_helper.bin
[   83.497619] libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
[   83.679229] DMA synchronization event drop occurred with device 47

And then modprobe hangs:

[  258.700561] INFO: task modprobe:1191 blocked for more than 120
seconds.
[  258.707336] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[  258.715270] modprobe      D c027f794     0  1191   1182 0x00000003
[  258.721588] [<c027f794>] (schedule+0x274/0x2ac) from [<c027f998>]
(schedule_timeout+0x18/0x17c)
[  258.730377] [<c027f998>] (schedule_timeout+0x18/0x17c) from
[<c027f414>] (wait_for_common+0xc0/0x14c)
[  258.739746] [<c027f414>] (wait_for_common+0xc0/0x14c) from
[<c01e61e0>] (mmc_wait_for_req+0x1f4/0x214)
[  258.749145] [<c01e61e0>] (mmc_wait_for_req+0x1f4/0x214) from
[<c01ea2a0>] (mmc_io_rw_extended+0x188/0x1f0)
[  258.758911] [<c01ea2a0>] (mmc_io_rw_extended+0x188/0x1f0) from
[<c01eb1c4>] (sdio_io_rw_ext_helper+0xc4/0x184)
[  258.769012] [<c01eb1c4>] (sdio_io_rw_ext_helper+0xc4/0x184) from
[<c01eb29c>] (sdio_writesb+0x18/0x1c)
[  258.778442] [<c01eb29c>] (sdio_writesb+0x18/0x1c) from [<bf0638dc>]
(if_sdio_probe+0x63c/0x938 [libertas_sdio])
[  258.788665] [<bf0638dc>] (if_sdio_probe+0x63c/0x938 [libertas_sdio])
from [<c01ea724>] (sdio_bus_probe+0x5c/0x68)
[  258.799041] [<c01ea724>] (sdio_bus_probe+0x5c/0x68) from [<c01a67a4>]
(driver_probe_device+0xa0/0x14c)
[  258.808441] [<c01a67a4>] (driver_probe_device+0xa0/0x14c) from
[<c01a68b0>] (__driver_attach+0x60/0x84)
[  258.817932] [<c01a68b0>] (__driver_attach+0x60/0x84) from
[<c01a5c8c>] (bus_for_each_dev+0x44/0x78)
[  258.827087] [<c01a5c8c>] (bus_for_each_dev+0x44/0x78) from
[<c01a6224>] (bus_add_driver+0x9c/0x224)
[  258.836212] [<c01a6224>] (bus_add_driver+0x9c/0x224) from
[<c01a6c08>] (driver_register+0xa8/0x130)
[  258.845367] [<c01a6c08>] (driver_register+0xa8/0x130) from
[<bf06701c>] (if_sdio_init_module+0x1c/0x3c [libertas_sdio])
[  258.856292] [<bf06701c>] (if_sdio_init_module+0x1c/0x3c
[libertas_sdio]) from [<c0029344>] (do_one_initcall+0x54/0x190)
[  258.867187] [<c0029344>] (do_one_initcall+0x54/0x190) from
[<c0073130>] (sys_init_module+0xb8/0x1e4)
[  258.876403] [<c0073130>] (sys_init_module+0xb8/0x1e4) from
[<c0029e80>] (ret_fast_syscall+0x0/0x2c)


> @@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
> *host, struct mmc_command *cmd,
>                 mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
>         host->cmd = cmd;
> 
> -       /*
> -        * Clear status bits and enable interrupts
> -        */
> -       OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -       OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> -
>         if (host->use_dma)
>                 OMAP_HSMMC_WRITE(host->base, IE,
>                                  INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));
> 
> David


-- 
Sincerely yours,
Mike.

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[David Vrabel]

Mike Rapoport wrote:
> 
> I've started to apply the patches one by one and after the first patch
> is applied, the SD card works Ok, but libertas fails to initialize with
> the following messages:
> 
>> modprobe libertas_sdio
> [   82.233489] lib80211: common routines for IEEE802.11 drivers
> [   82.833251] cfg80211: Calling CRDA to update world regulatory domain
> [   83.327911] libertas_sdio: Libertas SDIO driver
> [   83.332489] libertas_sdio: Copyright Pierre Ossman
> [   83.348510] libertas_sdio mmc1:0001:1: firmware: requesting
> sd8686_helper.bin
> [   83.497619] libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
> [   83.679229] DMA synchronization event drop occurred with device 47

What block size are you using?  It looks like you're using something
less than 512. I've checked the spec again and the DMA packet size
should be the block size in words (and not the FIFO depth).

Also, keep in mind that the buffers for transfers must begin and end on
a word boundary.  The OMAP's DMA controller can only transfer whole
words to the MMC FIFO.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

David Vrabel wrote:
> Mike Rapoport wrote:
>> I've started to apply the patches one by one and after the first patch
>> is applied, the SD card works Ok, but libertas fails to initialize with
>> the following messages:
>>
>>> modprobe libertas_sdio
>> [   82.233489] lib80211: common routines for IEEE802.11 drivers
>> [   82.833251] cfg80211: Calling CRDA to update world regulatory domain
>> [   83.327911] libertas_sdio: Libertas SDIO driver
>> [   83.332489] libertas_sdio: Copyright Pierre Ossman
>> [   83.348510] libertas_sdio mmc1:0001:1: firmware: requesting
>> sd8686_helper.bin
>> [   83.497619] libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
>> [   83.679229] DMA synchronization event drop occurred with device 47
> 
> What block size are you using?  It looks like you're using something
> less than 512. I've checked the spec again and the DMA packet size
> should be the block size in words (and not the FIFO depth).

It's quite possible. I haven't dig too much and I really don't know how exactly
libertas driver sends/receives the data... I'll continue testing and will try to get
some more meaningful info

> Also, keep in mind that the buffers for transfers must begin and end on
> a word boundary.  The OMAP's DMA controller can only transfer whole
> words to the MMC FIFO.
> 
> David


-- 
Sincerely yours,
Mike.

RE: [PATCH 1/3] mmc: omap_hsmmc: use packet sync'd DMA

[Madhusudhan]

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@csr.com]
> Sent: Wednesday, February 10, 2010 5:52 AM
> To: linux-mmc@vger.kernel.org
> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> Subject: [PATCH 1/3] mmc: omap_hsmmc: use packet sync'd DMA
> 
> Use packet sync'd DMA in the omap_hsmmc driver to avoid the
> restriction that transfers must be a multiple of the block size.  This
> is required for byte-mode transfers to SDIO cards.
> 
What is the typical scenario here? Is it IO_RW_EXTENDED with block_mode
turned off? 

> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c |   17 +++++------------
>  1 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4b23225..e7142a2 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -114,6 +114,7 @@
> 
>  #define MMC_TIMEOUT_MS		20
>  #define OMAP_MMC_MASTER_CLOCK	96000000
> +#define OMAP_HSMMC_FIFO_WORDS	(512/4)
>  #define DRIVER_NAME		"mmci-omap-hs"
> 
>  /* Timeouts for entering power saving states on inactivity, msec */
> @@ -887,12 +888,12 @@ static void omap_hsmmc_config_dma_params(struct
> omap_hsmmc_host *host,
>  	dma_ch = host->dma_ch;
>  	if (data->flags & MMC_DATA_WRITE) {
>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
> +			(host->mapbase + OMAP_HSMMC_DATA), 0,
> OMAP_HSMMC_FIFO_WORDS);
>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>  			sg_dma_address(sgl), 0, 0);
>  	} else {
>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
> +			(host->mapbase + OMAP_HSMMC_DATA), 0,
> OMAP_HSMMC_FIFO_WORDS);
>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>  			sg_dma_address(sgl), 0, 0);
>  	}
> @@ -901,7 +902,7 @@ static void omap_hsmmc_config_dma_params(struct
> omap_hsmmc_host *host,
>  	nblk = sg_dma_len(sgl) / blksz;
> 
>  	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
> -			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
> +			blksz / 4, nblk, OMAP_DMA_SYNC_PACKET,
>  			omap_hsmmc_get_dma_sync_dev(host, data),
>  			!(data->flags & MMC_DATA_WRITE));
> 

How does this configuration help? Isn't the PACKET again a multiple of
blksz?
 
> @@ -944,17 +945,9 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status,
> void *data)
>  static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
>  					struct mmc_request *req)
>  {
> -	int dma_ch = 0, ret = 0, err = 1, i;
> +	int dma_ch = 0, ret = 0, err = 1;
>  	struct mmc_data *data = req->data;
> 
> -	/* Sanity check: all the SG entries must be aligned by block size.
> */
> -	for (i = 0; i < data->sg_len; i++) {
> -		struct scatterlist *sgl;
> -
> -		sgl = data->sg + i;
> -		if (sgl->length % data->blksz)
> -			return -EINVAL;
> -	}
>  	if ((data->blksz % 4) != 0)
>  		/* REVISIT: The MMC buffer increments only when MSB is
> written.
>  		 * Return error for blksz which is non multiple of four.
> --
> 1.6.3.3

Re: [PATCH 1/3] mmc: omap_hsmmc: use packet sync'd DMA

[David Vrabel]

Madhusudhan wrote:
> 
>> -----Original Message-----
>> From: David Vrabel [mailto:david.vrabel@csr.com]
>> Sent: Wednesday, February 10, 2010 5:52 AM
>> To: linux-mmc@vger.kernel.org
>> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
>> Subject: [PATCH 1/3] mmc: omap_hsmmc: use packet sync'd DMA
>>
>> Use packet sync'd DMA in the omap_hsmmc driver to avoid the
>> restriction that transfers must be a multiple of the block size.  This
>> is required for byte-mode transfers to SDIO cards.
>>
> What is the typical scenario here? Is it IO_RW_EXTENDED with block_mode
> turned off? 

This patch isn't necessary.  With byte mode transfers you get a single
sg element and the transfer's block size is set to the length of the
transfer so frame sync mode works fine.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

David Vrabel wrote:
> 
> +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> +	struct omap_hsmmc_host *host = mmc_priv(mmc);
> +	u32 ie, con;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&host->irq_lock, flags);
> +
> +	/*
> +	 * When interrupts are enabled, CTPL must be set to enable
> +	 * DAT1 input buffer (or the card interrupt is always
> +	 * asserted) and FCLK must be enabled else wakeup does not
> +	 * work.
> +	 *
> +	 * FIXME: the power save code probably shouldn't be playing
> +	 * with FCLK and allow the clock subsystem to put it into
> +	 * smart-idle mode instead.
> +	 */
> +	con = OMAP_HSMMC_READ(host->base, CON);
> +	ie = OMAP_HSMMC_READ(host->base, IE);
> +	if (enable) {
> +		clk_enable(host->fclk);
> +		ie |= CIRQ_ENABLE;
> +		con |= CTPL;
> +	} else {
> +		clk_disable(host->fclk);
> +		ie &= ~CIRQ_ENABLE;
> +		con &= ~CTPL;
> +	}
> +	OMAP_HSMMC_WRITE(host->base, CON, con);
> +	OMAP_HSMMC_WRITE(host->base, IE, ie);
> +
> +	spin_unlock_irqrestore(&host->irq_lock, flags);
> +}

This isn't doing a posted write flush but when it's added this ends up
looking like:

	spin_lock_irqsave(&host->lock, flags);

	/*
	 * When interrupts are enabled, CTPL must be set to enable
	 * DAT1 input buffer (or the card interrupt is always
	 * asserted) and FCLK must be enabled else wakeup does not
	 * work.  Take care to disable FCLK after all the register
	 * accesses as they might not complete if FCLK is off.
	 *
	 * FIXME: the power save code probably shouldn't be playing
	 * with FCLK and allow the clock subsystem to put it into
	 * smart-idle mode instead.
	 */
	con = OMAP_HSMMC_READ(host->base, CON);
	ie = OMAP_HSMMC_READ(host->base, IE);
	if (enable) {
		clk_enable(host->fclk);
		ie |= CIRQ_ENABLE;
		con |= CTPL;
	} else {
		ie &= ~CIRQ_ENABLE;
		con &= ~CTPL;
	}
	OMAP_HSMMC_WRITE(host->base, CON, con);
	OMAP_HSMMC_WRITE(host->base, IE, ie);
	OMAP_HSMMC_READ(host->base, IE); /* flush posted write */
        if (!enable) {
            clk_disable(host->fclk);
        }

	spin_unlock_irqrestore(&host->lock, flags);

Could someone from TI could comment on why disabling FCLK affects
register read?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@csr.com]
> Sent: Wednesday, February 10, 2010 5:52 AM
> To: linux-mmc@vger.kernel.org
> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
> in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
> smart-idle mode.
> 
> FCLK must be enabled while SDIO interrupts are enabled or the MMC block
> won't wake-up.
> 

I am curious to know the system behavior with this patch. Does the FCLK
remain enabled forever if a SDIO card is detected on the bus?

The "mmc_signal_sdio_irq" fn seems to disable the irq and then wake up the
sdio_irq_thread which would result in enabling the FCLK back. Hence the
above question.

> The writes to STAT and ISE when starting a command are unnecessary and
> have been removed.
> 
> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
> As noted in the FIXME comment, I think the correct thing to do is to
> always leave FCLK enabled.  The clock/clockdomain subsystems should be
> configuring smart-idle mode making explicit calls to disable FCLK
> unnecessary.  I couldn't follow the clock subsystem to see if it was
> actually doing this, though.
> ---
>  drivers/mmc/host/omap_hsmmc.c |   94 ++++++++++++++++++++++++++++--------
> -----
>  1 files changed, 65 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index e6d8cb3..ad0f867 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -64,6 +64,7 @@
>  #define SDVS_MASK		0x00000E00
>  #define SDVSCLR			0xFFFFF1FF
>  #define SDVSDET			0x00000400
> +#define ENAWAKEUP		(1 << 2)
>  #define AUTOIDLE		0x1
>  #define SDBP			(1 << 8)
>  #define DTO			0xe
> @@ -74,9 +75,11 @@
>  #define CLKD_SHIFT		6
>  #define DTO_MASK		0x000F0000
>  #define DTO_SHIFT		16
> +#define CIRQ_ENABLE		(1 << 8)
>  #define INT_EN_MASK		0x307F0033
>  #define BWR_ENABLE		(1 << 4)
>  #define BRR_ENABLE		(1 << 5)
> +#define CTPL			(1 << 11)
>  #define INIT_STREAM		(1 << 1)
>  #define DP_SELECT		(1 << 21)
>  #define DDIR			(1 << 4)
> @@ -84,10 +87,12 @@
>  #define MSBS			(1 << 5)
>  #define BCE			(1 << 1)
>  #define FOUR_BIT		(1 << 1)
> +#define IWE			(1 << 24)
>  #define DW8			(1 << 5)
>  #define CC			0x1
>  #define TC			0x02
>  #define OD			0x1
> +#define CIRQ			(1 << 8)
>  #define ERR			(1 << 15)
>  #define CMD_TIMEOUT		(1 << 16)
>  #define DATA_TIMEOUT		(1 << 20)
> @@ -228,7 +233,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  		;
> 
>  	OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
> -			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
> +			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE |
> ENAWAKEUP);
> 
>  	if (host->id == OMAP_MMC1_DEVID) {
>  		if (host->power_mode != MMC_POWER_OFF &&
> @@ -243,7 +248,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  	}
> 
>  	OMAP_HSMMC_WRITE(host->base, HCTL,
> -			OMAP_HSMMC_READ(host->base, HCTL) | hctl);
> +			OMAP_HSMMC_READ(host->base, HCTL) | hctl | IWE);
> 
>  	OMAP_HSMMC_WRITE(host->base, CAPA,
>  			OMAP_HSMMC_READ(host->base, CAPA) | capa);
> @@ -257,7 +262,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  		;
> 
>  	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
>  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> 
>  	/* Do not initialize card-specific things if the power is off */
> @@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
> *host, struct mmc_command *cmd,
>  		mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
>  	host->cmd = cmd;
> 
> -	/*
> -	 * Clear status bits and enable interrupts
> -	 */
> -	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> -
>  	if (host->use_dma)
>  		OMAP_HSMMC_WRITE(host->base, IE,
>  				 INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));
> @@ -638,18 +637,21 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> *dev_id)
>  {
>  	struct omap_hsmmc_host *host = dev_id;
>  	struct mmc_data *data;
> -	int end_cmd = 0, end_trans = 0, status;
> +	u32 status;
> +	int end_cmd = 0, end_trans = 0;
> +	bool card_irq = false;
> 
>  	spin_lock(&host->irq_lock);
> 
> -	if (host->mrq == NULL) {
> -		OMAP_HSMMC_WRITE(host->base, STAT,
> -			OMAP_HSMMC_READ(host->base, STAT));
> -		/* Flush posted write */
> -		OMAP_HSMMC_READ(host->base, STAT);
> -		spin_unlock(&host->irq_lock);
> -		return IRQ_HANDLED;
> -	}
> +	status = OMAP_HSMMC_READ(host->base, STAT);
> +	OMAP_HSMMC_WRITE(host->base, STAT, status);
> +	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
> +
> +	if (status & CIRQ)
> +		card_irq = true;
> +
> +	if (host->mrq == NULL)
> +		goto out;
> 
>  	data = host->data;
>  	status = OMAP_HSMMC_READ(host->base, STAT);
> @@ -704,17 +706,16 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> *dev_id)
>  		}
>  	}
> 
> -	OMAP_HSMMC_WRITE(host->base, STAT, status);
> -	/* Flush posted write */
> -	OMAP_HSMMC_READ(host->base, STAT);
> -
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
>  	if ((end_trans || (status & TC)) && host->mrq)
>  		omap_hsmmc_xfer_done(host, data);
> -
> +out:
>  	spin_unlock(&host->irq_lock);
> 
> +	if (card_irq)
> +		mmc_signal_sdio_irq(host->mmc);
> +
>  	return IRQ_HANDLED;
>  }
> 
> @@ -1248,6 +1249,41 @@ static int omap_hsmmc_get_ro(struct mmc_host *mmc)
>  	return mmc_slot(host).get_ro(host->dev, 0);
>  }
> 
> +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> +	struct omap_hsmmc_host *host = mmc_priv(mmc);
> +	u32 ie, con;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&host->irq_lock, flags);
> +
> +	/*
> +	 * When interrupts are enabled, CTPL must be set to enable
> +	 * DAT1 input buffer (or the card interrupt is always
> +	 * asserted) and FCLK must be enabled else wakeup does not
> +	 * work.
> +	 *
> +	 * FIXME: the power save code probably shouldn't be playing
> +	 * with FCLK and allow the clock subsystem to put it into
> +	 * smart-idle mode instead.
> +	 */
> +	con = OMAP_HSMMC_READ(host->base, CON);
> +	ie = OMAP_HSMMC_READ(host->base, IE);
> +	if (enable) {
> +		clk_enable(host->fclk);
> +		ie |= CIRQ_ENABLE;
> +		con |= CTPL;
> +	} else {
> +		clk_disable(host->fclk);
> +		ie &= ~CIRQ_ENABLE;
> +		con &= ~CTPL;
> +	}
> +	OMAP_HSMMC_WRITE(host->base, CON, con);
> +	OMAP_HSMMC_WRITE(host->base, IE, ie);
> +
> +	spin_unlock_irqrestore(&host->irq_lock, flags);
> +}
> +
>  static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
>  {
>  	u32 hctl, capa, value;
> @@ -1262,14 +1298,14 @@ static void omap_hsmmc_conf_bus_power(struct
> omap_hsmmc_host *host)
>  	}
> 
>  	value = OMAP_HSMMC_READ(host->base, HCTL) & ~SDVS_MASK;
> -	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl);
> +	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl | IWE);
> 
>  	value = OMAP_HSMMC_READ(host->base, CAPA);
>  	OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
> 
>  	/* Set the controller to AUTO IDLE mode */
>  	value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
> -	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
> +	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE |
> ENAWAKEUP);
> 
>  	/* Set SD bus power bit */
>  	set_sd_bus_power(host);
> @@ -1516,7 +1552,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = {
>  	.set_ios = omap_hsmmc_set_ios,
>  	.get_cd = omap_hsmmc_get_cd,
>  	.get_ro = omap_hsmmc_get_ro,
> -	/* NYET -- enable_sdio_irq */
> +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
>  };
> 
>  static const struct mmc_host_ops omap_hsmmc_ps_ops = {
> @@ -1526,7 +1562,7 @@ static const struct mmc_host_ops omap_hsmmc_ps_ops =
> {
>  	.set_ios = omap_hsmmc_set_ios,
>  	.get_cd = omap_hsmmc_get_cd,
>  	.get_ro = omap_hsmmc_get_ro,
> -	/* NYET -- enable_sdio_irq */
> +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
>  };
> 
>  #ifdef CONFIG_DEBUG_FS
> @@ -1731,7 +1767,7 @@ static int __init omap_hsmmc_probe(struct
> platform_device *pdev)
>  	mmc->max_seg_size = mmc->max_req_size;
> 
>  	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> -		     MMC_CAP_WAIT_WHILE_BUSY;
> +		     MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_SDIO_IRQ;
> 
>  	if (mmc_slot(host).wires >= 8)
>  		mmc->caps |= MMC_CAP_8_BIT_DATA;
> @@ -1802,7 +1838,7 @@ static int __init omap_hsmmc_probe(struct
> platform_device *pdev)
>  		}
>  	}
> 
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
>  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> 
>  	mmc_host_lazy_disable(host->mmc);
> --
> 1.6.3.3

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

Madhusudhan wrote:
> 
>> -----Original Message-----
>> From: David Vrabel [mailto:david.vrabel@csr.com]
>> Sent: Wednesday, February 10, 2010 5:52 AM
>> To: linux-mmc@vger.kernel.org
>> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
>> Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
>>
>> Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
>> in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
>> smart-idle mode.
>>
>> FCLK must be enabled while SDIO interrupts are enabled or the MMC block
>> won't wake-up.
>>
> 
> I am curious to know the system behavior with this patch. Does the FCLK
> remain enabled forever if a SDIO card is detected on the bus?

Only if card interrupts are enabled.  The card irq is disabled when the 
sdio_irq_thread exits.

This is why I think that smart-idle mode needs to be used (it's turned
on by default on the MMC controller) but I don't understand the
clock/clockdomain code to see if that's doing the right thing.

If smart-idle mode is used then we don't need to disable FCLK manually 
to save power.  This will simplify the driver a bit.

David

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Paul Walmsley]

On Wed, 17 Feb 2010, David Vrabel wrote:

> Madhusudhan wrote:
> > 
> > > -----Original Message-----
> > > From: David Vrabel [mailto:david.vrabel@csr.com]
> > > Sent: Wednesday, February 10, 2010 5:52 AM
> > > To: linux-mmc@vger.kernel.org
> > > Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> > > Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> > > 
> > > Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
> > > in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
> > > smart-idle mode.
> > > 
> > > FCLK must be enabled while SDIO interrupts are enabled or the MMC block
> > > won't wake-up.
> > > 
> > 
> > I am curious to know the system behavior with this patch. Does the FCLK
> > remain enabled forever if a SDIO card is detected on the bus?
> 
> Only if card interrupts are enabled.  The card irq is disabled when the
> sdio_irq_thread exits.
> 
> This is why I think that smart-idle mode needs to be used (it's turned
> on by default on the MMC controller) but I don't understand the
> clock/clockdomain code to see if that's doing the right thing.

The easiest way to observe the operation of the clock and clockdomain code 
is to change the "#undef DEBUG" to "#define DEBUG" at the top of the clock 
or clockdomain code files that you're interested in observing, recompile 
the kernel, and boot it.  You'll want to use an "ignore_loglevel" kernel 
command line parameter or something similar.  This will alter the behavior 
of some power management situations, since the output of the debug 
messages will keep a UART active, but should be sufficient for the MMC 
work.

> If smart-idle mode is used then we don't need to disable FCLK manually to save
> power.  This will simplify the driver a bit.

Perhaps I'm misunderstanding what you're referring to, but it seems there 
is some confusion as to what target smart-idle mode does.  It's the PRCM 
that requests that the module enter idle.  This process is not dependent 
on the target idle mode bits, and happens when the PRCM concludes that 
clocks can potentially be disabled.  In the case of functional clocks, 
this process is triggered when some code (i.e., the clock code) disables 
the CM_FCLKEN_x.EN_y bit.

After the PRCM requests the module enter idle, depending on the 
OCP_SYSCONFIG.SIDLEMODE bits, the module either does not acknowledge the 
idle request (no-idle mode), always acknowledges the request (force-idle 
mode), or, acknowledges the idle request when the module believes it is 
internally quiescent (smart-idle mode).  This acknowledgement signal 
originates from the module and goes back to the PRCM.  Once this happens, 
the PRCM can switch off some clocks.  As can be seen in the HSMMC TRM 
docs, the HSMMC module expects the PRCM to disable both the interface and 
the functional clock when it acknowledges the idle request.

My point is that the target idle mode is unlikely to have any impact on 
the need to disable and enable the functional clock in software.

That said, I don't really know the details of the SDIO wakeup issue that 
you mention, but it might be worth observing that if your SDIO card 
doesn't generate its own internal clock, then it's unlikely to be able to 
provide an interrupt to wake up the controller when the HSMMC functional 
clock is off.  This is because the only other available clock for the card 
would come from the HSMMC's functional clock.

It seems to me that this issue could be diagnosed fairly easily by 
triggering the SDIO interrupt line by hand while the HSMMC functional 
clock is kept off.  If a wakeup is generated, then it's a card issue.  
Otherwise, you'll need to do some more intensive diagnostics.  For 
example, if the CORE powerdomain isn't on when you do the test, presumably 
you'll need to make sure that an IO ring wakeup is set for the SDIO card's 
interrupt line.  (In these situations, the interrupt response latency will 
be quite high, since the chip will need to boot from off-mode.  The driver 
can request a lower bound on the interrupt response latency via an OMAP PM 
layer function, which will likely keep the chip from entering off mode.)  

But if the CORE powerdomain is on or in retention when you do the test, 
then perhaps there is indeed some issue with the HSMMC controller where it 
cannot generate wakeups when its functional clock is off.  You'd want to 
verify that your HSMMC register settings are correct, though, before 
coming to that conclusion.


- Paul

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Cousson, Benoit]

>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner@vger.kernel.org] On Behalf Of David Vrabel
>Sent: Wednesday, February 17, 2010 7:10 PM
>
>David Vrabel wrote:
>>
>> +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>> +{
>> +    struct omap_hsmmc_host *host = mmc_priv(mmc);
>> +    u32 ie, con;
>> +    unsigned long flags;
>> +
>> +    spin_lock_irqsave(&host->irq_lock, flags);
>> +
>> +    /*
>> +     * When interrupts are enabled, CTPL must be set to enable
>> +     * DAT1 input buffer (or the card interrupt is always
>> +     * asserted) and FCLK must be enabled else wakeup does not
>> +     * work.
>> +     *
>> +     * FIXME: the power save code probably shouldn't be playing
>> +     * with FCLK and allow the clock subsystem to put it into
>> +     * smart-idle mode instead.
>> +     */
>> +    con = OMAP_HSMMC_READ(host->base, CON);
>> +    ie = OMAP_HSMMC_READ(host->base, IE);
>> +    if (enable) {
>> +            clk_enable(host->fclk);
>> +            ie |= CIRQ_ENABLE;
>> +            con |= CTPL;
>> +    } else {
>> +            clk_disable(host->fclk);
>> +            ie &= ~CIRQ_ENABLE;
>> +            con &= ~CTPL;
>> +    }
>> +    OMAP_HSMMC_WRITE(host->base, CON, con);
>> +    OMAP_HSMMC_WRITE(host->base, IE, ie);
>> +
>> +    spin_unlock_irqrestore(&host->irq_lock, flags);
>> +}
>
>This isn't doing a posted write flush but when it's added this ends up
>looking like:
>
>       spin_lock_irqsave(&host->lock, flags);
>
>       /*
>        * When interrupts are enabled, CTPL must be set to enable
>        * DAT1 input buffer (or the card interrupt is always
>        * asserted) and FCLK must be enabled else wakeup does not
>        * work.  Take care to disable FCLK after all the register
>        * accesses as they might not complete if FCLK is off.
>        *
>        * FIXME: the power save code probably shouldn't be playing
>        * with FCLK and allow the clock subsystem to put it into
>        * smart-idle mode instead.
>        */
>       con = OMAP_HSMMC_READ(host->base, CON);
>       ie = OMAP_HSMMC_READ(host->base, IE);
>       if (enable) {
>               clk_enable(host->fclk);
>               ie |= CIRQ_ENABLE;
>               con |= CTPL;
>       } else {
>               ie &= ~CIRQ_ENABLE;
>               con &= ~CTPL;
>       }
>       OMAP_HSMMC_WRITE(host->base, CON, con);
>       OMAP_HSMMC_WRITE(host->base, IE, ie);
>       OMAP_HSMMC_READ(host->base, IE); /* flush posted write */
>        if (!enable) {
>            clk_disable(host->fclk);
>        }
>
>       spin_unlock_irqrestore(&host->lock, flags);
>
>Could someone from TI could comment on why disabling FCLK affects
>register read?

Most IPs will require both iclk and fclk to be active in order to access them. Some simple IP can work with only an iclk.
Otherwise, accessing a register while enabling only iclk will lead to unpredictable behavior.

Regards,
Benoit

Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@csr.com]
> Sent: Wednesday, February 17, 2010 1:40 PM
> To: Madhusudhan
> Cc: linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> Madhusudhan wrote:
> >
> >> -----Original Message-----
> >> From: David Vrabel [mailto:david.vrabel@csr.com]
> >> Sent: Wednesday, February 10, 2010 5:52 AM
> >> To: linux-mmc@vger.kernel.org
> >> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> >> Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> >>
> >> Enable the use of SDIO card interrupts.  This requires setting
> ENAWAKEUP
> >> in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
> >> smart-idle mode.
> >>
> >> FCLK must be enabled while SDIO interrupts are enabled or the MMC block
> >> won't wake-up.
> >>
> >
> > I am curious to know the system behavior with this patch. Does the FCLK
> > remain enabled forever if a SDIO card is detected on the bus?
> 
> Only if card interrupts are enabled.  The card irq is disabled when the
> sdio_irq_thread exits.
> 

Your patch sets up the capability to enable sdio irqs anyway. My question
was more related to the idle path. In a scenario where you start using the
SDIO functionality and after some time you leave the SDIO card idle, does
the FCLK still remain enabled? Or do you see that sdio_irq_thread exits and
disables the FCLK?

Regards,
Madhu


> This is why I think that smart-idle mode needs to be used (it's turned
> on by default on the MMC controller) but I don't understand the
> clock/clockdomain code to see if that's doing the right thing.
> 
> If smart-idle mode is used then we don't need to disable FCLK manually
> to save power.  This will simplify the driver a bit.
> 
> David

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

Mike Rapoport wrote:
> David Vrabel wrote:
>> Mike Rapoport wrote:
>>> I've started to apply the patches one by one and after the first patch
>>> is applied, the SD card works Ok, but libertas fails to initialize with
>>> the following messages:
>>>
>>>> modprobe libertas_sdio
>>> [   82.233489] lib80211: common routines for IEEE802.11 drivers
>>> [   82.833251] cfg80211: Calling CRDA to update world regulatory domain
>>> [   83.327911] libertas_sdio: Libertas SDIO driver
>>> [   83.332489] libertas_sdio: Copyright Pierre Ossman
>>> [   83.348510] libertas_sdio mmc1:0001:1: firmware: requesting
>>> sd8686_helper.bin
>>> [   83.497619] libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
>>> [   83.679229] DMA synchronization event drop occurred with device 47
>> What block size are you using?  It looks like you're using something
>> less than 512. I've checked the spec again and the DMA packet size
>> should be the block size in words (and not the FIFO depth).
> 
> It's quite possible. I haven't dig too much and I really don't know how exactly
> libertas driver sends/receives the data... I'll continue testing and will try to get
> some more meaningful info
> 
>> Also, keep in mind that the buffers for transfers must begin and end on
>> a word boundary.  The OMAP's DMA controller can only transfer whole
>> words to the MMC FIFO.

I've slightly modified your patch "mmc: omap_hsmmc: use packet sync'd DMA" and it
seems to work now, at least with SD card and libertas_sdio:

---
 drivers/mmc/host/omap_hsmmc.c |   23 ++++++++---------------
 1 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 4b23225..5408bcb 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -114,6 +114,7 @@

 #define MMC_TIMEOUT_MS		20
 #define OMAP_MMC_MASTER_CLOCK	96000000
+#define OMAP_HSMMC_FIFO_WORDS	(512/4)
 #define DRIVER_NAME		"mmci-omap-hs"

 /* Timeouts for entering power saving states on inactivity, msec */
@@ -884,24 +885,24 @@ static void omap_hsmmc_config_dma_params(struct omap_hsmmc_host *host,
 {
 	int blksz, nblk, dma_ch;

+	blksz = host->data->blksz;
+	nblk = sg_dma_len(sgl) / blksz;
+
 	dma_ch = host->dma_ch;
 	if (data->flags & MMC_DATA_WRITE) {
 		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
+			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
 		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
 			sg_dma_address(sgl), 0, 0);
 	} else {
 		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
-			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
+			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
 		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
 			sg_dma_address(sgl), 0, 0);
 	}

-	blksz = host->data->blksz;
-	nblk = sg_dma_len(sgl) / blksz;
-
 	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
-			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
+			blksz / 4, nblk, OMAP_DMA_SYNC_PACKET,
 			omap_hsmmc_get_dma_sync_dev(host, data),
 			!(data->flags & MMC_DATA_WRITE));

@@ -944,17 +945,9 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status, void *data)
 static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
 					struct mmc_request *req)
 {
-	int dma_ch = 0, ret = 0, err = 1, i;
+	int dma_ch = 0, ret = 0, err = 1;
 	struct mmc_data *data = req->data;

-	/* Sanity check: all the SG entries must be aligned by block size. */
-	for (i = 0; i < data->sg_len; i++) {
-		struct scatterlist *sgl;
-
-		sgl = data->sg + i;
-		if (sgl->length % data->blksz)
-			return -EINVAL;
-	}
 	if ((data->blksz % 4) != 0)
 		/* REVISIT: The MMC buffer increments only when MSB is written.
 		 * Return error for blksz which is non multiple of four.
-- 
1.6.4.4



>> David
> 
> 


-- 
Sincerely yours,
Mike.

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

David Vrabel wrote:
> Mike Rapoport wrote:
>> Hi David,
>>
>> David Vrabel wrote:
>>> These patches add support for SDIO cards to the omap_hsmmc driver.  Less
>>> restrictions on the sizes of transfers, power management changes to 
>>> prevent SDIO cards from being turned off and losing all state, and card
>>> interrupts.
>>>
>>> I've been unable to test these exact patches as I only have an N900 for 
>>> testing and the N900 support in mainline is incomplete.
>> I've tried your patches on CM-T35 that has 8686 SDIO and the result was
>> absence of MMC/SDIO devices at all :( Morover, kmmcd hangs:
>>
>> [  399.427764] INFO: task kmmcd:149 blocked for more than 120 seconds.
>> [...]
>> [  399.485290] [<c01e6264>] (mmc_wait_for_cmd+0x64/0x74) from
>> [<c01e86b0>] (mmc_go_idle+0x60/0xac)
> 
> I suspect interrupts aren't enabled correctly as this command does not
> complete.  Try reverting this hunk.
> 
> @@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
> *host, struct mmc_command *cmd,
>                 mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
>         host->cmd = cmd;
> 
> -       /*
> -        * Clear status bits and enable interrupts
> -        */
> -       OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -       OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> -
>         if (host->use_dma)
>                 OMAP_HSMMC_WRITE(host->base, IE,
>                                  INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));

Reverting this hunk and reverting/adjusting changes to IRQ handler makes
SD card recognizable again. No luck with SDIO interrupts though...

I've managed to get libertas_sdio loading properly, but I get lookups when there's
some traffic on the interface:

[ 1277.755859] BUG: soft lockup - CPU#0 stuck for 61s! [libertas_sdio/0:1212]
[ 1277.762786] Modules linked in: libertas_sdio libertas cfg80211 lib80211
firmware_class omap_hsmmc ads7846 twl4030_keypad
[ 1277.773803]
[ 1277.775299] Pid: 1212, comm:      libertas_sdio/0
[ 1277.780029] CPU: 0    Not tainted  (2.6.33-rc4-07945-gd19df2c-dirty #173)
[ 1277.786895] PC is at handle_IRQ_event+0x20/0xf0
[ 1277.791442] LR is at handle_level_irq+0xdc/0xf4
[ 1277.796020] pc : [<c0074ea8>]    lr : [<c0076e10>]    psr: 40000113
[ 1277.796020] sp : cfb61ce8  ip : fb058000  fp : fb058018
[ 1277.807556] r10: c0382508  r9 : 00000000  r8 : 00000022
[ 1277.812805] r7 : 00000001  r6 : 00000000  r5 : 00000143  r4 : cf990c80
[ 1277.819366] r3 : 00000088  r2 : c03c815c  r1 : cf990c80  r0 : 00000143
[ 1277.825958] Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
[ 1277.833282] Control: 10c5387d  Table: 8fa1c019  DAC: 00000017


> 
> David


-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

Madhusudhan wrote:
> 
> My question was more related to the idle path. In a scenario where
> you start using the SDIO functionality and after some time you leave
> the SDIO card idle, does the FCLK still remain enabled? Or do you see
> that sdio_irq_thread exits and disables the FCLK?

What do you mean by "leave the SDIO card idle"?

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

Paul Walmsley wrote:
> 
> Perhaps I'm misunderstanding what you're referring to, but it seems there 
> is some confusion as to what target smart-idle mode does.

The TI OMAP 34xx TRM is the one of the most obtuse document I've had to
read.  An even after multiple reads of relevant sections it still
doesn't make sense.  Parts are contradictory, parts are poorly explained
and some bit are just plain incorrect.

I don't have time to sort out any of this properly -- it works well
enough for me.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

RE: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Madhusudhan]

<snip>
> >
> >> Also, keep in mind that the buffers for transfers must begin and end on
> >> a word boundary.  The OMAP's DMA controller can only transfer whole
> >> words to the MMC FIFO.
> 
> I've slightly modified your patch "mmc: omap_hsmmc: use packet sync'd DMA"
> and it
> seems to work now, at least with SD card and libertas_sdio:
> 

Mike,

As per the latest discussion, David replied that this patch is not needed
and FRAME sync just works fine. So, without this patch does SDIO int
functionality work for you?

Regards,
Madhu

> ---
>  drivers/mmc/host/omap_hsmmc.c |   23 ++++++++---------------
>  1 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 4b23225..5408bcb 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -114,6 +114,7 @@
> 
>  #define MMC_TIMEOUT_MS		20
>  #define OMAP_MMC_MASTER_CLOCK	96000000
> +#define OMAP_HSMMC_FIFO_WORDS	(512/4)
>  #define DRIVER_NAME		"mmci-omap-hs"
> 
>  /* Timeouts for entering power saving states on inactivity, msec */
> @@ -884,24 +885,24 @@ static void omap_hsmmc_config_dma_params(struct
> omap_hsmmc_host *host,
>  {
>  	int blksz, nblk, dma_ch;
> 
> +	blksz = host->data->blksz;
> +	nblk = sg_dma_len(sgl) / blksz;
> +
>  	dma_ch = host->dma_ch;
>  	if (data->flags & MMC_DATA_WRITE) {
>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
> +			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>  			sg_dma_address(sgl), 0, 0);
>  	} else {
>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
> +			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>  			sg_dma_address(sgl), 0, 0);
>  	}
> 
> -	blksz = host->data->blksz;
> -	nblk = sg_dma_len(sgl) / blksz;
> -
>  	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
> -			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
> +			blksz / 4, nblk, OMAP_DMA_SYNC_PACKET,
>  			omap_hsmmc_get_dma_sync_dev(host, data),
>  			!(data->flags & MMC_DATA_WRITE));
> 
> @@ -944,17 +945,9 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status,
> void *data)
>  static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
>  					struct mmc_request *req)
>  {
> -	int dma_ch = 0, ret = 0, err = 1, i;
> +	int dma_ch = 0, ret = 0, err = 1;
>  	struct mmc_data *data = req->data;
> 
> -	/* Sanity check: all the SG entries must be aligned by block size.
> */
> -	for (i = 0; i < data->sg_len; i++) {
> -		struct scatterlist *sgl;
> -
> -		sgl = data->sg + i;
> -		if (sgl->length % data->blksz)
> -			return -EINVAL;
> -	}
>  	if ((data->blksz % 4) != 0)
>  		/* REVISIT: The MMC buffer increments only when MSB is
> written.
>  		 * Return error for blksz which is non multiple of four.
> --
> 1.6.4.4
> 
> 
> 
> >> David
> >
> >
> 
> 
> --
> Sincerely yours,
> Mike.
> 

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@csr.com]
> Sent: Thursday, February 18, 2010 6:16 AM
> To: Madhusudhan
> Cc: linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> Madhusudhan wrote:
> >
> > My question was more related to the idle path. In a scenario where
> > you start using the SDIO functionality and after some time you leave
> > the SDIO card idle, does the FCLK still remain enabled? Or do you see
> > that sdio_irq_thread exits and disables the FCLK?
> 
> What do you mean by "leave the SDIO card idle"?
> 

I meant a state where the SDIO card is not doing any active transfers. Does
it work that way?

Regards,
Madhu

> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
> 
> 
> Member of the CSR plc group of companies. CSR plc registered in England
> and Wales, registered number 4187346, registered office Churchill House,
> Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Paul Walmsley]

On Thu, 18 Feb 2010, David Vrabel wrote:

> Paul Walmsley wrote:
> > 
> > Perhaps I'm misunderstanding what you're referring to, but it seems there 
> > is some confusion as to what target smart-idle mode does.
> 
> The TI OMAP 34xx TRM is the one of the most obtuse document I've had to
> read.  An even after multiple reads of relevant sections it still
> doesn't make sense.  Parts are contradictory, parts are poorly explained
> and some bit are just plain incorrect.

Yes.  TI needs to also release the functional specifications for each IP 
block; the information in the current TRM is generally insufficient to 
write high-quality drivers.

TI should also host a process, open to the public, to submit TRM revision 
and clarification requests, and to track their disposition.

> I don't have time to sort out any of this properly -- it works well
> enough for me.

Thanks very much for the SDIO patches.  They are appreciated.


- Paul

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[David Vrabel]

Mike Rapoport wrote:
> 
> Reverting this hunk and reverting/adjusting changes to IRQ handler makes
> SD card recognizable again. No luck with SDIO interrupts though...

Your card might need CLKEXTFREE enabled in MMCHSi_CON if it cannot
generate card interrupts asynchronously.  If that doesn't help you'll
have to see if one of the TI people has any suggestions.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Thursday, February 18, 2010 11:44 AM
> To: David Vrabel
> Cc: Madhusudhan; linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> On Thu, 18 Feb 2010, David Vrabel wrote:
> 
> > Paul Walmsley wrote:
> > >
> > > Perhaps I'm misunderstanding what you're referring to, but it seems
> there
> > > is some confusion as to what target smart-idle mode does.
> >
> > The TI OMAP 34xx TRM is the one of the most obtuse document I've had to
> > read.  An even after multiple reads of relevant sections it still
> > doesn't make sense.  Parts are contradictory, parts are poorly explained
> > and some bit are just plain incorrect.
> 
> Yes.  TI needs to also release the functional specifications for each IP
> block; the information in the current TRM is generally insufficient to
> write high-quality drivers.
> 
> TI should also host a process, open to the public, to submit TRM revision
> and clarification requests, and to track their disposition.
> 
Paul, David,

Not to add further confusion but as far as I understand SDIO should be able
to asynchronously wakeup OMAP regardless of MMC clocks being OFF . As per
the SDIO spec DAT1 line is used for signaling the card interrupt to the
host. The TRM clearly shows a GPIO connected to DAT1 line for wakeup through
the SDIO interrupt. Wouldn't this help?   

> > I don't have time to sort out any of this properly -- it works well
> > enough for me.
> 
> Thanks very much for the SDIO patches.  They are appreciated.

Definitely these are good features that are being added. Just that there are
confusions around and not everyone has a setup to validate these features.
If I look in the mainline kernel today there is only one SDIO card driver "
sdio_uart.c". And at least I don't see a way I can test any of these
features myself.

Regards,
Madhu   
> 
> 
> - Paul

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Nicolas Pitre]

On Thu, 18 Feb 2010, Madhusudhan wrote:

> If I look in the mainline kernel today there is only one SDIO card driver "
> sdio_uart.c". And at least I don't see a way I can test any of these
> features myself.

There are many other SDIO drivers in the kernel today:

 - The libertas wireless driver supports many interfaces, including 
   PCMCIA/CF, USB, SDIO and SPI.  Chips known to work with the SDIO 
   interface are the Marvell 8686 and 8688.

 - There is a BlueTooth HCI SDIO driver implementing the standard SDIO
   BlueTooth class.

 - The Marvell 8688 mentioned above is also a BlueTooth device through
   an additional SDIO "function".

 - Intel Wireless WiMAX Connection 2400 over SDIO.

And the sdio_uart.c driver can be used with SDIO GPS receivers for 
example.


Nicolas

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: Nicolas Pitre [mailto:nico@fluxnic.net]
> Sent: Thursday, February 18, 2010 1:27 PM
> To: Madhusudhan
> Cc: 'Paul Walmsley'; 'David Vrabel'; linux-mmc@vger.kernel.org; linux-
> omap@vger.kernel.org
> Subject: RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> On Thu, 18 Feb 2010, Madhusudhan wrote:
> 
> > If I look in the mainline kernel today there is only one SDIO card
> driver "
> > sdio_uart.c". And at least I don't see a way I can test any of these
> > features myself.
> 
> There are many other SDIO drivers in the kernel today:
> 
>  - The libertas wireless driver supports many interfaces, including
>    PCMCIA/CF, USB, SDIO and SPI.  Chips known to work with the SDIO
>    interface are the Marvell 8686 and 8688.
> 
>  - There is a BlueTooth HCI SDIO driver implementing the standard SDIO
>    BlueTooth class.
> 
>  - The Marvell 8688 mentioned above is also a BlueTooth device through
>    an additional SDIO "function".
> 
>  - Intel Wireless WiMAX Connection 2400 over SDIO.
> 
> And the sdio_uart.c driver can be used with SDIO GPS receivers for
> example.
> 

This is good information. Any known SDIO cards commercially available using
the above chips?

Thanks,
Madhu
> 
> Nicolas

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Paul Walmsley]

Hi,

(cc'ing Steve, Richard, Anand)

On Thu, 18 Feb 2010, Madhusudhan wrote:

> Not to add further confusion but as far as I understand SDIO should be 
> able to asynchronously wakeup OMAP regardless of MMC clocks being OFF . 
> As per the SDIO spec DAT1 line is used for signaling the card interrupt 
> to the host. The TRM clearly shows a GPIO connected to DAT1 line for 
> wakeup through the SDIO interrupt. Wouldn't this help?

Is the integration code in arch/arm/*omap* set to enable wakeup on the 
DAT1 ball on the OMAP?  I don't see any trace of that in the code.  That 
would surely prevent wakeup from succeeding on an off-mode enabled kernel.  
Sounds like there may be other problems.  Why not give it a try with an 
SDIO card and see if you can get it to work?

> Definitely these are good features that are being added. Just that there 
> are confusions around and not everyone has a setup to validate these 
> features.

Hopefully someone inside TI is validating SDIO wakeup on Linux?

> If I look in the mainline kernel today there is only one SDIO card 
> driver " sdio_uart.c".

[paul@twilight current]$ find . -name "*sdio*c" -print | fgrep -v /core/
./drivers/ssb/sdio.c
./drivers/bluetooth/btsdio.c
./drivers/bluetooth/btmrvl_sdio.c
./drivers/mmc/host/mvsdio.c
./drivers/net/wimax/i2400m/sdio-rx.c
./drivers/net/wimax/i2400m/sdio-tx.c
./drivers/net/wimax/i2400m/sdio.c
./drivers/net/wimax/i2400m/sdio-fw.c
./drivers/net/wireless/iwmc3200wifi/sdio.c
./drivers/net/wireless/libertas/if_sdio.c
./drivers/net/wireless/wl12xx/wl1251_sdio.c
./drivers/net/wireless/b43/sdio.c
./drivers/media/dvb/siano/smssdio.c
[paul@twilight current]$

There appear to be least seven SDIO card drivers in 2.6.34-rc7. At least 
one of these is for a TI chip - the wl1251.  I think some of the 
Gumstix Overo OMAP3 boards are using an SDIO-connected Marvell Libertas:

http://www.gumstix.com/store/catalog/product_info.php?products_id=252
  
David is probably testing with a Bluetooth card - maybe he can comment 
further.

> And at least I don't see a way I can test any of these features myself.

Could you clarify?


- Paul

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Steve Sakoman]

On Thu, Feb 18, 2010 at 12:21 PM, Paul Walmsley <paul@pwsan.com> wrote:

> (cc'ing Steve, Richard, Anand)

> There appear to be least seven SDIO card drivers in 2.6.34-rc7. At least
> one of these is for a TI chip - the wl1251.  I think some of the
> Gumstix Overo OMAP3 boards are using an SDIO-connected Marvell Libertas:
>
> http://www.gumstix.com/store/catalog/product_info.php?products_id=252

That is correct, a Wi2Wi wifi module is connected via mmc2 on the
Overo Air and Fire products.

Data rates are pretty low with the existing OMAP SDIO driver.   Any
chance this patch would improve that situation?

Just added applying the patch and retesting data rates to my "to do" list :-)

Steve

> David is probably testing with a Bluetooth card - maybe he can comment
> further.
>
>> And at least I don't see a way I can test any of these features myself.
>
> Could you clarify?
>
>
> - Paul
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 890 bytes --]

On Thu, 18 Feb 2010, Steve Sakoman wrote:

> On Thu, Feb 18, 2010 at 12:21 PM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> > (cc'ing Steve, Richard, Anand)
> 
> > There appear to be least seven SDIO card drivers in 2.6.34-rc7. At least
> > one of these is for a TI chip - the wl1251.  I think some of the
> > Gumstix Overo OMAP3 boards are using an SDIO-connected Marvell Libertas:
> >
> > http://www.gumstix.com/store/catalog/product_info.php?products_id=252
> 
> That is correct, a Wi2Wi wifi module is connected via mmc2 on the
> Overo Air and Fire products.
> 
> Data rates are pretty low with the existing OMAP SDIO driver.   Any
> chance this patch would improve that situation?

Without proper SDIO interrupt support at the controller level, the core 
SDIO code does poll for pending SDIO interrupts periodically.  So yes, 
this patch is certainly going to help throughput.


Nicolas

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: David Vrabel [mailto:david.vrabel@csr.com]
> Sent: Wednesday, February 10, 2010 5:52 AM
> To: linux-mmc@vger.kernel.org
> Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
> in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
> smart-idle mode.
> 
> FCLK must be enabled while SDIO interrupts are enabled or the MMC block
> won't wake-up.
> 
> The writes to STAT and ISE when starting a command are unnecessary and
> have been removed.
> 
> Signed-off-by: David Vrabel <david.vrabel@csr.com>
> ---
> As noted in the FIXME comment, I think the correct thing to do is to
> always leave FCLK enabled.  The clock/clockdomain subsystems should be
> configuring smart-idle mode making explicit calls to disable FCLK
> unnecessary.  I couldn't follow the clock subsystem to see if it was
> actually doing this, though.

I Just noticed that SD cards stopped working on Zoom2 if I apply this patch.
I will come back with details on what's going on in a bit unless someone
else beats me on that.

Regards,
Madhu


> ---
>  drivers/mmc/host/omap_hsmmc.c |   94 ++++++++++++++++++++++++++++--------
> -----
>  1 files changed, 65 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index e6d8cb3..ad0f867 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -64,6 +64,7 @@
>  #define SDVS_MASK		0x00000E00
>  #define SDVSCLR			0xFFFFF1FF
>  #define SDVSDET			0x00000400
> +#define ENAWAKEUP		(1 << 2)
>  #define AUTOIDLE		0x1
>  #define SDBP			(1 << 8)
>  #define DTO			0xe
> @@ -74,9 +75,11 @@
>  #define CLKD_SHIFT		6
>  #define DTO_MASK		0x000F0000
>  #define DTO_SHIFT		16
> +#define CIRQ_ENABLE		(1 << 8)
>  #define INT_EN_MASK		0x307F0033
>  #define BWR_ENABLE		(1 << 4)
>  #define BRR_ENABLE		(1 << 5)
> +#define CTPL			(1 << 11)
>  #define INIT_STREAM		(1 << 1)
>  #define DP_SELECT		(1 << 21)
>  #define DDIR			(1 << 4)
> @@ -84,10 +87,12 @@
>  #define MSBS			(1 << 5)
>  #define BCE			(1 << 1)
>  #define FOUR_BIT		(1 << 1)
> +#define IWE			(1 << 24)
>  #define DW8			(1 << 5)
>  #define CC			0x1
>  #define TC			0x02
>  #define OD			0x1
> +#define CIRQ			(1 << 8)
>  #define ERR			(1 << 15)
>  #define CMD_TIMEOUT		(1 << 16)
>  #define DATA_TIMEOUT		(1 << 20)
> @@ -228,7 +233,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  		;
> 
>  	OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
> -			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
> +			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE |
> ENAWAKEUP);
> 
>  	if (host->id == OMAP_MMC1_DEVID) {
>  		if (host->power_mode != MMC_POWER_OFF &&
> @@ -243,7 +248,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  	}
> 
>  	OMAP_HSMMC_WRITE(host->base, HCTL,
> -			OMAP_HSMMC_READ(host->base, HCTL) | hctl);
> +			OMAP_HSMMC_READ(host->base, HCTL) | hctl | IWE);
> 
>  	OMAP_HSMMC_WRITE(host->base, CAPA,
>  			OMAP_HSMMC_READ(host->base, CAPA) | capa);
> @@ -257,7 +262,7 @@ static int omap_hsmmc_context_restore(struct
> omap_hsmmc_host *host)
>  		;
> 
>  	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
>  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> 
>  	/* Do not initialize card-specific things if the power is off */
> @@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
> *host, struct mmc_command *cmd,
>  		mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
>  	host->cmd = cmd;
> 
> -	/*
> -	 * Clear status bits and enable interrupts
> -	 */
> -	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> -
>  	if (host->use_dma)
>  		OMAP_HSMMC_WRITE(host->base, IE,
>  				 INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));
> @@ -638,18 +637,21 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> *dev_id)
>  {
>  	struct omap_hsmmc_host *host = dev_id;
>  	struct mmc_data *data;
> -	int end_cmd = 0, end_trans = 0, status;
> +	u32 status;
> +	int end_cmd = 0, end_trans = 0;
> +	bool card_irq = false;
> 
>  	spin_lock(&host->irq_lock);
> 
> -	if (host->mrq == NULL) {
> -		OMAP_HSMMC_WRITE(host->base, STAT,
> -			OMAP_HSMMC_READ(host->base, STAT));
> -		/* Flush posted write */
> -		OMAP_HSMMC_READ(host->base, STAT);
> -		spin_unlock(&host->irq_lock);
> -		return IRQ_HANDLED;
> -	}
> +	status = OMAP_HSMMC_READ(host->base, STAT);
> +	OMAP_HSMMC_WRITE(host->base, STAT, status);
> +	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
> +
> +	if (status & CIRQ)
> +		card_irq = true;
> +
> +	if (host->mrq == NULL)
> +		goto out;
> 
>  	data = host->data;
>  	status = OMAP_HSMMC_READ(host->base, STAT);
> @@ -704,17 +706,16 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> *dev_id)
>  		}
>  	}
> 
> -	OMAP_HSMMC_WRITE(host->base, STAT, status);
> -	/* Flush posted write */
> -	OMAP_HSMMC_READ(host->base, STAT);
> -
>  	if (end_cmd || ((status & CC) && host->cmd))
>  		omap_hsmmc_cmd_done(host, host->cmd);
>  	if ((end_trans || (status & TC)) && host->mrq)
>  		omap_hsmmc_xfer_done(host, data);
> -
> +out:
>  	spin_unlock(&host->irq_lock);
> 
> +	if (card_irq)
> +		mmc_signal_sdio_irq(host->mmc);
> +
>  	return IRQ_HANDLED;
>  }
> 
> @@ -1248,6 +1249,41 @@ static int omap_hsmmc_get_ro(struct mmc_host *mmc)
>  	return mmc_slot(host).get_ro(host->dev, 0);
>  }
> 
> +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> +	struct omap_hsmmc_host *host = mmc_priv(mmc);
> +	u32 ie, con;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&host->irq_lock, flags);
> +
> +	/*
> +	 * When interrupts are enabled, CTPL must be set to enable
> +	 * DAT1 input buffer (or the card interrupt is always
> +	 * asserted) and FCLK must be enabled else wakeup does not
> +	 * work.
> +	 *
> +	 * FIXME: the power save code probably shouldn't be playing
> +	 * with FCLK and allow the clock subsystem to put it into
> +	 * smart-idle mode instead.
> +	 */
> +	con = OMAP_HSMMC_READ(host->base, CON);
> +	ie = OMAP_HSMMC_READ(host->base, IE);
> +	if (enable) {
> +		clk_enable(host->fclk);
> +		ie |= CIRQ_ENABLE;
> +		con |= CTPL;
> +	} else {
> +		clk_disable(host->fclk);
> +		ie &= ~CIRQ_ENABLE;
> +		con &= ~CTPL;
> +	}
> +	OMAP_HSMMC_WRITE(host->base, CON, con);
> +	OMAP_HSMMC_WRITE(host->base, IE, ie);
> +
> +	spin_unlock_irqrestore(&host->irq_lock, flags);
> +}
> +
>  static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
>  {
>  	u32 hctl, capa, value;
> @@ -1262,14 +1298,14 @@ static void omap_hsmmc_conf_bus_power(struct
> omap_hsmmc_host *host)
>  	}
> 
>  	value = OMAP_HSMMC_READ(host->base, HCTL) & ~SDVS_MASK;
> -	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl);
> +	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl | IWE);
> 
>  	value = OMAP_HSMMC_READ(host->base, CAPA);
>  	OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
> 
>  	/* Set the controller to AUTO IDLE mode */
>  	value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
> -	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
> +	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE |
> ENAWAKEUP);
> 
>  	/* Set SD bus power bit */
>  	set_sd_bus_power(host);
> @@ -1516,7 +1552,7 @@ static const struct mmc_host_ops omap_hsmmc_ops = {
>  	.set_ios = omap_hsmmc_set_ios,
>  	.get_cd = omap_hsmmc_get_cd,
>  	.get_ro = omap_hsmmc_get_ro,
> -	/* NYET -- enable_sdio_irq */
> +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
>  };
> 
>  static const struct mmc_host_ops omap_hsmmc_ps_ops = {
> @@ -1526,7 +1562,7 @@ static const struct mmc_host_ops omap_hsmmc_ps_ops =
> {
>  	.set_ios = omap_hsmmc_set_ios,
>  	.get_cd = omap_hsmmc_get_cd,
>  	.get_ro = omap_hsmmc_get_ro,
> -	/* NYET -- enable_sdio_irq */
> +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
>  };
> 
>  #ifdef CONFIG_DEBUG_FS
> @@ -1731,7 +1767,7 @@ static int __init omap_hsmmc_probe(struct
> platform_device *pdev)
>  	mmc->max_seg_size = mmc->max_req_size;
> 
>  	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> -		     MMC_CAP_WAIT_WHILE_BUSY;
> +		     MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_SDIO_IRQ;
> 
>  	if (mmc_slot(host).wires >= 8)
>  		mmc->caps |= MMC_CAP_8_BIT_DATA;
> @@ -1802,7 +1838,7 @@ static int __init omap_hsmmc_probe(struct
> platform_device *pdev)
>  		}
>  	}
> 
> -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
>  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> 
>  	mmc_host_lazy_disable(host->mmc);
> --
> 1.6.3.3

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Thursday, February 18, 2010 2:21 PM
> To: Madhusudhan
> Cc: 'David Vrabel'; sakoman@gmail.com; r-woodruff2@ti.com; sawant@ti.com;
> linux-mmc@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> Hi,
> 
> (cc'ing Steve, Richard, Anand)
> 
> On Thu, 18 Feb 2010, Madhusudhan wrote:
> 
> > Not to add further confusion but as far as I understand SDIO should be
> > able to asynchronously wakeup OMAP regardless of MMC clocks being OFF .
> > As per the SDIO spec DAT1 line is used for signaling the card interrupt
> > to the host. The TRM clearly shows a GPIO connected to DAT1 line for
> > wakeup through the SDIO interrupt. Wouldn't this help?
> 
> Is the integration code in arch/arm/*omap* set to enable wakeup on the
> DAT1 ball on the OMAP?  I don't see any trace of that in the code.  That
> would surely prevent wakeup from succeeding on an off-mode enabled kernel.
> Sounds like there may be other problems.  Why not give it a try with an
> SDIO card and see if you can get it to work?
> 
> > Definitely these are good features that are being added. Just that there
> > are confusions around and not everyone has a setup to validate these
> > features.
> 
> Hopefully someone inside TI is validating SDIO wakeup on Linux?
> 
> > If I look in the mainline kernel today there is only one SDIO card
> > driver " sdio_uart.c".
> 
> [paul@twilight current]$ find . -name "*sdio*c" -print | fgrep -v /core/
> ./drivers/ssb/sdio.c
> ./drivers/bluetooth/btsdio.c
> ./drivers/bluetooth/btmrvl_sdio.c
> ./drivers/mmc/host/mvsdio.c
> ./drivers/net/wimax/i2400m/sdio-rx.c
> ./drivers/net/wimax/i2400m/sdio-tx.c
> ./drivers/net/wimax/i2400m/sdio.c
> ./drivers/net/wimax/i2400m/sdio-fw.c
> ./drivers/net/wireless/iwmc3200wifi/sdio.c
> ./drivers/net/wireless/libertas/if_sdio.c
> ./drivers/net/wireless/wl12xx/wl1251_sdio.c
> ./drivers/net/wireless/b43/sdio.c
> ./drivers/media/dvb/siano/smssdio.c
> [paul@twilight current]$
> 
> There appear to be least seven SDIO card drivers in 2.6.34-rc7. At least
> one of these is for a TI chip - the wl1251.  I think some of the
> Gumstix Overo OMAP3 boards are using an SDIO-connected Marvell Libertas:
> 
> http://www.gumstix.com/store/catalog/product_info.php?products_id=252
> 
> David is probably testing with a Bluetooth card - maybe he can comment
> further.
> 
> > And at least I don't see a way I can test any of these features myself.
> 
> Could you clarify?
> 
I meant that right now I am not equipped with a setup that has a SDIO device
that works to a level that this wakeup feature can be validated.

Regards,
Madhu
> 
> - Paul

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

[-- Attachment #1: Type: text/plain, Size: 10043 bytes --]



> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Madhusudhan
> Sent: Friday, February 19, 2010 3:05 PM
> To: 'David Vrabel'; linux-mmc@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Subject: RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> 
> 
> > -----Original Message-----
> > From: David Vrabel [mailto:david.vrabel@csr.com]
> > Sent: Wednesday, February 10, 2010 5:52 AM
> > To: linux-mmc@vger.kernel.org
> > Cc: David Vrabel; linux-omap@vger.kernel.org; madhu.cr@ti.com
> > Subject: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> >
> > Enable the use of SDIO card interrupts.  This requires setting ENAWAKEUP
> > in SYSCONFIG and IWE in HTCL to allow the MMC block to wake-up when in
> > smart-idle mode.
> >
> > FCLK must be enabled while SDIO interrupts are enabled or the MMC block
> > won't wake-up.
> >
> > The writes to STAT and ISE when starting a command are unnecessary and
> > have been removed.
> >
> > Signed-off-by: David Vrabel <david.vrabel@csr.com>
> > ---
> > As noted in the FIXME comment, I think the correct thing to do is to
> > always leave FCLK enabled.  The clock/clockdomain subsystems should be
> > configuring smart-idle mode making explicit calls to disable FCLK
> > unnecessary.  I couldn't follow the clock subsystem to see if it was
> > actually doing this, though.
> 
> I Just noticed that SD cards stopped working on Zoom2 if I apply this
> patch.
> I will come back with details on what's going on in a bit unless someone
> else beats me on that.
> 

A little bit of rearranging the David's changes to the irq handler gets the
MMC/SD cards to work fine again. Changes are shown in the attached patch for
now and it should not hurt the CIRQ handling as well. 

Regards,
Madhu

> Regards,
> Madhu
> 
> 
> > ---
> >  drivers/mmc/host/omap_hsmmc.c |   94 ++++++++++++++++++++++++++++------
> --
> > -----
> >  1 files changed, 65 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/mmc/host/omap_hsmmc.c
> b/drivers/mmc/host/omap_hsmmc.c
> > index e6d8cb3..ad0f867 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -64,6 +64,7 @@
> >  #define SDVS_MASK		0x00000E00
> >  #define SDVSCLR			0xFFFFF1FF
> >  #define SDVSDET			0x00000400
> > +#define ENAWAKEUP		(1 << 2)
> >  #define AUTOIDLE		0x1
> >  #define SDBP			(1 << 8)
> >  #define DTO			0xe
> > @@ -74,9 +75,11 @@
> >  #define CLKD_SHIFT		6
> >  #define DTO_MASK		0x000F0000
> >  #define DTO_SHIFT		16
> > +#define CIRQ_ENABLE		(1 << 8)
> >  #define INT_EN_MASK		0x307F0033
> >  #define BWR_ENABLE		(1 << 4)
> >  #define BRR_ENABLE		(1 << 5)
> > +#define CTPL			(1 << 11)
> >  #define INIT_STREAM		(1 << 1)
> >  #define DP_SELECT		(1 << 21)
> >  #define DDIR			(1 << 4)
> > @@ -84,10 +87,12 @@
> >  #define MSBS			(1 << 5)
> >  #define BCE			(1 << 1)
> >  #define FOUR_BIT		(1 << 1)
> > +#define IWE			(1 << 24)
> >  #define DW8			(1 << 5)
> >  #define CC			0x1
> >  #define TC			0x02
> >  #define OD			0x1
> > +#define CIRQ			(1 << 8)
> >  #define ERR			(1 << 15)
> >  #define CMD_TIMEOUT		(1 << 16)
> >  #define DATA_TIMEOUT		(1 << 20)
> > @@ -228,7 +233,7 @@ static int omap_hsmmc_context_restore(struct
> > omap_hsmmc_host *host)
> >  		;
> >
> >  	OMAP_HSMMC_WRITE(host->base, SYSCONFIG,
> > -			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE);
> > +			OMAP_HSMMC_READ(host->base, SYSCONFIG) | AUTOIDLE |
> > ENAWAKEUP);
> >
> >  	if (host->id == OMAP_MMC1_DEVID) {
> >  		if (host->power_mode != MMC_POWER_OFF &&
> > @@ -243,7 +248,7 @@ static int omap_hsmmc_context_restore(struct
> > omap_hsmmc_host *host)
> >  	}
> >
> >  	OMAP_HSMMC_WRITE(host->base, HCTL,
> > -			OMAP_HSMMC_READ(host->base, HCTL) | hctl);
> > +			OMAP_HSMMC_READ(host->base, HCTL) | hctl | IWE);
> >
> >  	OMAP_HSMMC_WRITE(host->base, CAPA,
> >  			OMAP_HSMMC_READ(host->base, CAPA) | capa);
> > @@ -257,7 +262,7 @@ static int omap_hsmmc_context_restore(struct
> > omap_hsmmc_host *host)
> >  		;
> >
> >  	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> > -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> > +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
> >  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> >
> >  	/* Do not initialize card-specific things if the power is off */
> > @@ -426,12 +431,6 @@ omap_hsmmc_start_command(struct omap_hsmmc_host
> > *host, struct mmc_command *cmd,
> >  		mmc_hostname(host->mmc), cmd->opcode, cmd->arg);
> >  	host->cmd = cmd;
> >
> > -	/*
> > -	 * Clear status bits and enable interrupts
> > -	 */
> > -	OMAP_HSMMC_WRITE(host->base, STAT, STAT_CLEAR);
> > -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> > -
> >  	if (host->use_dma)
> >  		OMAP_HSMMC_WRITE(host->base, IE,
> >  				 INT_EN_MASK & ~(BRR_ENABLE | BWR_ENABLE));
> > @@ -638,18 +637,21 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> > *dev_id)
> >  {
> >  	struct omap_hsmmc_host *host = dev_id;
> >  	struct mmc_data *data;
> > -	int end_cmd = 0, end_trans = 0, status;
> > +	u32 status;
> > +	int end_cmd = 0, end_trans = 0;
> > +	bool card_irq = false;
> >
> >  	spin_lock(&host->irq_lock);
> >
> > -	if (host->mrq == NULL) {
> > -		OMAP_HSMMC_WRITE(host->base, STAT,
> > -			OMAP_HSMMC_READ(host->base, STAT));
> > -		/* Flush posted write */
> > -		OMAP_HSMMC_READ(host->base, STAT);
> > -		spin_unlock(&host->irq_lock);
> > -		return IRQ_HANDLED;
> > -	}
> > +	status = OMAP_HSMMC_READ(host->base, STAT);
> > +	OMAP_HSMMC_WRITE(host->base, STAT, status);
> > +	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
> > +
> > +	if (status & CIRQ)
> > +		card_irq = true;
> > +
> > +	if (host->mrq == NULL)
> > +		goto out;
> >
> >  	data = host->data;
> >  	status = OMAP_HSMMC_READ(host->base, STAT);
> > @@ -704,17 +706,16 @@ static irqreturn_t omap_hsmmc_irq(int irq, void
> > *dev_id)
> >  		}
> >  	}
> >
> > -	OMAP_HSMMC_WRITE(host->base, STAT, status);
> > -	/* Flush posted write */
> > -	OMAP_HSMMC_READ(host->base, STAT);
> > -
> >  	if (end_cmd || ((status & CC) && host->cmd))
> >  		omap_hsmmc_cmd_done(host, host->cmd);
> >  	if ((end_trans || (status & TC)) && host->mrq)
> >  		omap_hsmmc_xfer_done(host, data);
> > -
> > +out:
> >  	spin_unlock(&host->irq_lock);
> >
> > +	if (card_irq)
> > +		mmc_signal_sdio_irq(host->mmc);
> > +
> >  	return IRQ_HANDLED;
> >  }
> >
> > @@ -1248,6 +1249,41 @@ static int omap_hsmmc_get_ro(struct mmc_host
> *mmc)
> >  	return mmc_slot(host).get_ro(host->dev, 0);
> >  }
> >
> > +static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int
> enable)
> > +{
> > +	struct omap_hsmmc_host *host = mmc_priv(mmc);
> > +	u32 ie, con;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&host->irq_lock, flags);
> > +
> > +	/*
> > +	 * When interrupts are enabled, CTPL must be set to enable
> > +	 * DAT1 input buffer (or the card interrupt is always
> > +	 * asserted) and FCLK must be enabled else wakeup does not
> > +	 * work.
> > +	 *
> > +	 * FIXME: the power save code probably shouldn't be playing
> > +	 * with FCLK and allow the clock subsystem to put it into
> > +	 * smart-idle mode instead.
> > +	 */
> > +	con = OMAP_HSMMC_READ(host->base, CON);
> > +	ie = OMAP_HSMMC_READ(host->base, IE);
> > +	if (enable) {
> > +		clk_enable(host->fclk);
> > +		ie |= CIRQ_ENABLE;
> > +		con |= CTPL;
> > +	} else {
> > +		clk_disable(host->fclk);
> > +		ie &= ~CIRQ_ENABLE;
> > +		con &= ~CTPL;
> > +	}
> > +	OMAP_HSMMC_WRITE(host->base, CON, con);
> > +	OMAP_HSMMC_WRITE(host->base, IE, ie);
> > +
> > +	spin_unlock_irqrestore(&host->irq_lock, flags);
> > +}
> > +
> >  static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
> >  {
> >  	u32 hctl, capa, value;
> > @@ -1262,14 +1298,14 @@ static void omap_hsmmc_conf_bus_power(struct
> > omap_hsmmc_host *host)
> >  	}
> >
> >  	value = OMAP_HSMMC_READ(host->base, HCTL) & ~SDVS_MASK;
> > -	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl);
> > +	OMAP_HSMMC_WRITE(host->base, HCTL, value | hctl | IWE);
> >
> >  	value = OMAP_HSMMC_READ(host->base, CAPA);
> >  	OMAP_HSMMC_WRITE(host->base, CAPA, value | capa);
> >
> >  	/* Set the controller to AUTO IDLE mode */
> >  	value = OMAP_HSMMC_READ(host->base, SYSCONFIG);
> > -	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE);
> > +	OMAP_HSMMC_WRITE(host->base, SYSCONFIG, value | AUTOIDLE |
> > ENAWAKEUP);
> >
> >  	/* Set SD bus power bit */
> >  	set_sd_bus_power(host);
> > @@ -1516,7 +1552,7 @@ static const struct mmc_host_ops omap_hsmmc_ops =
> {
> >  	.set_ios = omap_hsmmc_set_ios,
> >  	.get_cd = omap_hsmmc_get_cd,
> >  	.get_ro = omap_hsmmc_get_ro,
> > -	/* NYET -- enable_sdio_irq */
> > +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
> >  };
> >
> >  static const struct mmc_host_ops omap_hsmmc_ps_ops = {
> > @@ -1526,7 +1562,7 @@ static const struct mmc_host_ops omap_hsmmc_ps_ops
> =
> > {
> >  	.set_ios = omap_hsmmc_set_ios,
> >  	.get_cd = omap_hsmmc_get_cd,
> >  	.get_ro = omap_hsmmc_get_ro,
> > -	/* NYET -- enable_sdio_irq */
> > +	.enable_sdio_irq = omap_hsmmc_enable_sdio_irq,
> >  };
> >
> >  #ifdef CONFIG_DEBUG_FS
> > @@ -1731,7 +1767,7 @@ static int __init omap_hsmmc_probe(struct
> > platform_device *pdev)
> >  	mmc->max_seg_size = mmc->max_req_size;
> >
> >  	mmc->caps |= MMC_CAP_MMC_HIGHSPEED | MMC_CAP_SD_HIGHSPEED |
> > -		     MMC_CAP_WAIT_WHILE_BUSY;
> > +		     MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_SDIO_IRQ;
> >
> >  	if (mmc_slot(host).wires >= 8)
> >  		mmc->caps |= MMC_CAP_8_BIT_DATA;
> > @@ -1802,7 +1838,7 @@ static int __init omap_hsmmc_probe(struct
> > platform_device *pdev)
> >  		}
> >  	}
> >
> > -	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK);
> > +	OMAP_HSMMC_WRITE(host->base, ISE, INT_EN_MASK | CIRQ);
> >  	OMAP_HSMMC_WRITE(host->base, IE, INT_EN_MASK);
> >
> >  	mmc_host_lazy_disable(host->mmc);
> > --
> > 1.6.3.3
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: 0001-OMAP-Hsmmc-Get-the-SD-cards-to-work-again.patch --]
[-- Type: application/octet-stream, Size: 1556 bytes --]

>From f18edd943659d4a9632cd0c0c64f4c6a4aa2f206 Mon Sep 17 00:00:00 2001
From: Madhusudhan Chikkature <madhu.cr@ti.com>
Date: Fri, 19 Feb 2010 19:52:14 -0500
Subject: [PATCH] OMAP Hsmmc:Get the SD cards to work again.

Signed-off-by: Madhusudhan Chikkature <madhu.cr@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 27f1426..6c508bd 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -977,19 +977,18 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
 	struct mmc_data *data;
 	u32 status;
 	int end_cmd = 0, end_trans = 0;
-	bool card_irq = false;
 
 	spin_lock(&host->irq_lock);
 
-	status = OMAP_HSMMC_READ(host->base, STAT);
-	OMAP_HSMMC_WRITE(host->base, STAT, status);
-	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
-
-	if (status & CIRQ)
-		card_irq = true;
+	if (host->mrq == NULL) {
+		status = OMAP_HSMMC_READ(host->base, STAT);
+		OMAP_HSMMC_WRITE(host->base, STAT,
+			status);
+		/* Flush posted write */
+		OMAP_HSMMC_READ(host->base, STAT);
 
-	if (host->mrq == NULL)
 		goto out;
+	}
 
 	data = host->data;
 	status = OMAP_HSMMC_READ(host->base, STAT);
@@ -1051,7 +1050,7 @@ static irqreturn_t omap_hsmmc_irq(int irq, void *dev_id)
 out:
 	spin_unlock(&host->irq_lock);
 
-	if (card_irq)
+	if (status & CIRQ)
 		mmc_signal_sdio_irq(host->mmc);
 
 	return IRQ_HANDLED;
-- 
1.6.0.4

Re: [PATCH 0/3] mmc: omap_hsmmc: support SDIO cards

[Mike Rapoport]

Madhusudhan wrote:
> <snip>
>>>> Also, keep in mind that the buffers for transfers must begin and end on
>>>> a word boundary.  The OMAP's DMA controller can only transfer whole
>>>> words to the MMC FIFO.
>> I've slightly modified your patch "mmc: omap_hsmmc: use packet sync'd DMA"
>> and it
>> seems to work now, at least with SD card and libertas_sdio:
>>
> 
> Mike,
> 
> As per the latest discussion, David replied that this patch is not needed
> and FRAME sync just works fine. So, without this patch does SDIO int
> functionality work for you?

I had no luck with David's patches for SDIO IRQ, however with patches previously 
sent by Phaneendra ([1]) and fclk hack ([2]) SDIO IRQ works...
Without the fclk hack I get kernel panic in the omap_hsmmc:

Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa0b4130 

Internal error: : 1028 [#1]
last sysfs file: /sys/class/net/lo/operstate
Modules linked in: libertas_sdio libertas cfg80211 lib80211 firmware_class 
ads7846 twl4030_keypad 

CPU: 0    Not tainted  (2.6.33-rc4-07078-gd8ebff3-dirty #8)
PC is at omap_hsmmc_irq+0xa8/0x59c
LR is at try_to_wake_up+0xd8/0xf4
pc : [<c021c49c>]    lr : [<c004f37c>]    psr: 60000193
sp : c03b5e28  ip : c03c3e08  fp : 00000000
r10: 0000001f  r9 : 411fc083  r8 : 00000000
r7 : cf9b79c0  r6 : 00000100  r5 : 00000056  r4 : cf9b7800
r3 : 00000000  r2 : fa0b4000  r1 : 20000193  r0 : 00000001
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c5387d  Table: 8faf0019  DAC: 00000017
Process swapper (pid: 0, stack limit = 0xc03b42e8)
Stack: (0xc03b5e28 to 0xc03b6000)
5e20:                   00000000 4c400e5f 000020b6 c006b8ec c03c2190 00000000
5e40: c03c2190 00000000 c03c1ce8 c03c2190 c03b5e74 c004e020 c03c2190 c03c2190
5e60: c03c2190 01dcd817 00000000 0000004b 00000000 515f5afc 000020b8 c006b8ec
5e80: 518247cf 00000000 c040b9a0 00000000 c03b5ed8 00000034 515f5afc 00000000
5ea0: 000225c1 00000000 1235a688 c0036e14 c03c0dc8 c006bb88 0000004a 00000000
5ec0: ffff6c00 ffffffff 15e77dcf 00000016 40000000 ffffff40 15e77dcf ffff84af
5ee0: 00000000 ffff84b1 00000000 00000001 00000000 ffff84af 00000000 c005b4ac
5f00: 00773594 00000000 00773594 00000000 00000001 c006f51c 00773594 00000000
5f20: 16d14154 00000035 2ff68e8c cf988f40 00000056 00000000 00000000 80023164
5f40: 0000001f c007972c c03c5b00 00000056 c0025014 c03b7bd8 80023164 c007b680
5f60: 00000056 00000000 c0025014 c002b070 ffffffff fa200000 c0025014 c002ba30
5f80: 00000000 1235a688 40000013 c03e94d4 c03b4000 c03e8adc c0025014 c03b7bd8
5fa0: 80023164 411fc083 0000001f 00000000 cfbfdae0 c03b5fc8 c003bff4 c003c02c
5fc0: 60000013 ffffffff c03b4000 c002d2e0 c040d250 c0008b1c c00084e8 00000000
5fe0: 00000000 c0025018 00000000 10c53c7d c03e8c04 80008034 00000000 00000000
[<c021c49c>] (omap_hsmmc_irq+0xa8/0x59c) from [<c007972c>] 
(handle_IRQ_event+0x34/0xf0)
[<c007972c>] (handle_IRQ_event+0x34/0xf0) from [<c007b680>] 
(handle_level_irq+0xdc/0xf4)
[<c007b680>] (handle_level_irq+0xdc/0xf4) from [<c002b070>] (asm_do_IRQ+0x70/0x90)
[<c002b070>] (asm_do_IRQ+0x70/0x90) from [<c002ba30>] (__irq_svc+0x30/0x80)
Exception stack(0xc03b5f80 to 0xc03b5fc8)
5f80: 00000000 1235a688 40000013 c03e94d4 c03b4000 c03e8adc c0025014 c03b7bd8
5fa0: 80023164 411fc083 0000001f 00000000 cfbfdae0 c03b5fc8 c003bff4 c003c02c
5fc0: 60000013 ffffffff
[<c002ba30>] (__irq_svc+0x30/0x80) from [<c003c02c>] (omap3_pm_idle+0x48/0x4c)
[<c003c02c>] (omap3_pm_idle+0x48/0x4c) from [<c002d2e0>] (cpu_idle+0x48/0x88)
[<c002d2e0>] (cpu_idle+0x48/0x88) from [<c0008b1c>] (start_kernel+0x24c/0x2a4)
[<c0008b1c>] (start_kernel+0x24c/0x2a4) from [<80008034>] (0x80008034)
Code: e5973008 e3530000 1a000005 e597203c (e5923130)
---[ end trace 2011509d5458c456 ]---
Kernel panic - not syncing: Fatal exception in interrupt


[1] http://thread.gmane.org/gmane.linux.kernel.mmc/967
[2] http://thread.gmane.org/gmane.linux.kernel.mmc/1107/focus=1109

> Regards,
> Madhu
> 
>> ---
>>  drivers/mmc/host/omap_hsmmc.c |   23 ++++++++---------------
>>  1 files changed, 8 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
>> index 4b23225..5408bcb 100644
>> --- a/drivers/mmc/host/omap_hsmmc.c
>> +++ b/drivers/mmc/host/omap_hsmmc.c
>> @@ -114,6 +114,7 @@
>>
>>  #define MMC_TIMEOUT_MS		20
>>  #define OMAP_MMC_MASTER_CLOCK	96000000
>> +#define OMAP_HSMMC_FIFO_WORDS	(512/4)
>>  #define DRIVER_NAME		"mmci-omap-hs"
>>
>>  /* Timeouts for entering power saving states on inactivity, msec */
>> @@ -884,24 +885,24 @@ static void omap_hsmmc_config_dma_params(struct
>> omap_hsmmc_host *host,
>>  {
>>  	int blksz, nblk, dma_ch;
>>
>> +	blksz = host->data->blksz;
>> +	nblk = sg_dma_len(sgl) / blksz;
>> +
>>  	dma_ch = host->dma_ch;
>>  	if (data->flags & MMC_DATA_WRITE) {
>>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
>> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
>> +			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
>>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>>  			sg_dma_address(sgl), 0, 0);
>>  	} else {
>>  		omap_set_dma_src_params(dma_ch, 0, OMAP_DMA_AMODE_CONSTANT,
>> -			(host->mapbase + OMAP_HSMMC_DATA), 0, 0);
>> +			(host->mapbase + OMAP_HSMMC_DATA), 0, blksz / 4);
>>  		omap_set_dma_dest_params(dma_ch, 0, OMAP_DMA_AMODE_POST_INC,
>>  			sg_dma_address(sgl), 0, 0);
>>  	}
>>
>> -	blksz = host->data->blksz;
>> -	nblk = sg_dma_len(sgl) / blksz;
>> -
>>  	omap_set_dma_transfer_params(dma_ch, OMAP_DMA_DATA_TYPE_S32,
>> -			blksz / 4, nblk, OMAP_DMA_SYNC_FRAME,
>> +			blksz / 4, nblk, OMAP_DMA_SYNC_PACKET,
>>  			omap_hsmmc_get_dma_sync_dev(host, data),
>>  			!(data->flags & MMC_DATA_WRITE));
>>
>> @@ -944,17 +945,9 @@ static void omap_hsmmc_dma_cb(int lch, u16 ch_status,
>> void *data)
>>  static int omap_hsmmc_start_dma_transfer(struct omap_hsmmc_host *host,
>>  					struct mmc_request *req)
>>  {
>> -	int dma_ch = 0, ret = 0, err = 1, i;
>> +	int dma_ch = 0, ret = 0, err = 1;
>>  	struct mmc_data *data = req->data;
>>
>> -	/* Sanity check: all the SG entries must be aligned by block size.
>> */
>> -	for (i = 0; i < data->sg_len; i++) {
>> -		struct scatterlist *sgl;
>> -
>> -		sgl = data->sg + i;
>> -		if (sgl->length % data->blksz)
>> -			return -EINVAL;
>> -	}
>>  	if ((data->blksz % 4) != 0)
>>  		/* REVISIT: The MMC buffer increments only when MSB is
>> written.
>>  		 * Return error for blksz which is non multiple of four.
>> --
>> 1.6.4.4
>>
>>
>>
>>>> David
>>>
>>
>> --
>> Sincerely yours,
>> Mike.
>>
> 
> 


-- 
Sincerely yours,
Mike.

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[David Vrabel]

Madhusudhan wrote:
> 
> A little bit of rearranging the David's changes to the irq handler gets the
> MMC/SD cards to work fine again. Changes are shown in the attached patch for
> now and it should not hurt the CIRQ handling as well. 

This breaks detecting card interrupts unless a command is in progress.

>>> +	status = OMAP_HSMMC_READ(host->base, STAT);
>>> +	OMAP_HSMMC_WRITE(host->base, STAT, status);
>>> +	OMAP_HSMMC_READ(host->base, STAT); /* Flush posted write. */
>>> +
>>> +	if (status & CIRQ)
>>> +		card_irq = true;
>>> +
>>> +	if (host->mrq == NULL)
>>> +		goto out;
>>>
>>>  	data = host->data;
>>>  	status = OMAP_HSMMC_READ(host->base, STAT);

This line is the real problem.  We re-read MMCi_STAT after acknowledging
 (clearing) it.

Please see the version #2 of the patchset.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

RE: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Madhusudhan]

> -----Original Message-----
> From: Steve Sakoman [mailto:sakoman@gmail.com]
> Sent: Thursday, February 18, 2010 4:16 PM
> To: Paul Walmsley
> Cc: Madhusudhan; David Vrabel; r-woodruff2@ti.com; sawant@ti.com; linux-
> mmc@vger.kernel.org; linux-omap@vger.kernel.org
> Subject: Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts
> 
> On Thu, Feb 18, 2010 at 12:21 PM, Paul Walmsley <paul@pwsan.com> wrote:
> 
> > (cc'ing Steve, Richard, Anand)
> 
> > There appear to be least seven SDIO card drivers in 2.6.34-rc7. At least
> > one of these is for a TI chip - the wl1251.  I think some of the
> > Gumstix Overo OMAP3 boards are using an SDIO-connected Marvell Libertas:
> >
> > http://www.gumstix.com/store/catalog/product_info.php?products_id=252
> 
> That is correct, a Wi2Wi wifi module is connected via mmc2 on the
> Overo Air and Fire products.
> 

Steve,

A dumb question. Is this Wi2Wi wifi module available as a standard SDIO card
which can be plugged into a SD card cage? Or is it specifically designed for
the gumstix board?

Regards,
Madhu

> Data rates are pretty low with the existing OMAP SDIO driver.   Any
> chance this patch would improve that situation?
> 
> Just added applying the patch and retesting data rates to my "to do" list
> :-)
> 
> Steve
> 
> > David is probably testing with a Bluetooth card - maybe he can comment
> > further.
> >
> >> And at least I don't see a way I can test any of these features myself.
> >
> > Could you clarify?
> >
> >
> > - Paul
> >

Re: [PATCH 3/3] mmc: omap_hsmmc: enable SDIO card interrupts

[Steve Sakoman]

On Tue, Mar 2, 2010 at 2:08 PM, Madhusudhan <madhu.cr@ti.com> wrote:

>> That is correct, a Wi2Wi wifi module is connected via mmc2 on the
>> Overo Air and Fire products.

> A dumb question. Is this Wi2Wi wifi module available as a standard SDIO card
> which can be plugged into a SD card cage? Or is it specifically designed for
> the gumstix board?

The module on Overo is soldered directly to the board. It is a
standard part, not custom.  I am not aware if wi2wi also sells a SD
version.

Steve