Re: MMC_CAP_SDIO_IRQ for omap 3430

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



John Rigby wrote:
First the disclaimers:
The board is new board that has had hardware mmc/sdio problems before.
 It is based closely on beagle but uses a different package which has
some package specific issues.  We think we have found them all but who
knows.

My kernel is a few months old 2.6.29.

Then you can't apply my patch 1:1. Steve already found that there are a lot of differences between recent git head and 2.6.31. A lot of omap_hsmmc changes were done since 2.6.31.

Ok, given the above issues, it looks to me like I am getting
interrupted every time  CIRQ is enabled.

Infinite interrupts. This is what I assumed :(

Nor sure if the libertas
driver is not disabling as it should or what.

I'm not sure if libertas has to do something with this.

So I end up in this
death spiral:

irq handler is called and status indicates CIRQ is pending

mmc_signal_sdio_irq is called which calls omap_hsmmc_enable_sdio_irq
with via mmc_host_ops, en is zero so CIRQ gets disabled

mmc_signal_sdio_irq then wakes up sdio_irq_thread which does its thing
then before going to sleep calls omap_hsmmc_enable_irq with en set to
1 so CIRC gets turned back on

Sounds ok. And thanks for giving the call stack. As mentioned, I did only compile check ;)

with CIRC enabled the irq handler gets called and we are back where we started.

The issue then is not acknowledging the interrupt correctly. The TRM (spruf98.c) tells us:

-- cut --
Card interrupt Enable
A clear of this bit also clears the corresponding status bit.
During 1-bit mode, if the interrupt routine does not remove the
source of a card interrupt in the SDIO card, the status bit is
reasserted when this bit is set to 1.
-- cut --

The clear card interrupt enable should be done by ~(CIRQ_ENABLE) in

+if (status & CIRQ) {
+	dev_dbg(mmc_dev(host->mmc), "SDIO interrupt");
+	OMAP_HSMMC_WRITE(host->base, IE, OMAP_HSMMC_READ(host->base, IE)
+			 & ~(CIRQ_ENABLE));
+	mmc_signal_sdio_irq(host->mmc);
+	spin_unlock(&host->irq_lock);
+	return IRQ_HANDLED;
+}

Anybody sees anything wrong here? Maybe we missed something?

I assume that we are not in 1-bit mode. We should be in 4 bit mode. It could be checked by some test code that we are in 4 bit mode and IBG is set.

Additionally, what I wonder regarding this, is the davinci driver [1] additionally checking for DAT1 level in enable_sdio_irq

1114         if (enable) {
1115                 if (!((readl(host->base + DAVINCI_SDIOST0))
1116                             & SDIOST0_DAT1_HI)) {
1117                         writel(SDIOIST_IOINT,
1118 host->base + DAVINCI_SDIOIST);
1119                         mmc_signal_sdio_irq(host->mmc);
1120                 } else {
... enable int
1124                 }
1125         } else {
... disable int
1129         }

And it does similar stuff in xfer_done

 873         if (host->mmc->caps & MMC_CAP_SDIO_IRQ) {
874 if (host->sdio_int && (!((readl(host->base + DAVINCI_SDIOST0))
 875                                         & SDIOST0_DAT1_HI))) {
876 writel(SDIOIST_IOINT, host->base + DAVINCI_SDIOIST);
 877                         mmc_signal_sdio_irq(host->mmc);
 878                 }
 879         }

Most probably there is a reason for doing so?

Best regards

Dirk

[1] http://arago-project.org/git/projects/?p=linux-davinci.git;a=blob;f=drivers/mmc/host/davinci_mmc.c;h=1bf0587250614c6d8abfe02028b96e0e47148ac8;hb=HEAD

Looks like either the libertas driver is not disabling the irq or the
CIRQ irq is somehow getting stuck or I have a hw problem.

One thing I did try was to have the sdio_irq_thread basically ignore
the MMC_CAP_SDIO_IRQ flag except for calling
host->ops->enable_sdio_irq(host, 1) and I changed mmc_signal_sdio_irq
to not reschedule sdio_irq_thread.  In shows that the mere act of
enabling CIRQ does not break anything.  In this mode I get one CIRQ
irq for each time around the sdio_irq_thread loop.

I guess I should try checking the CIRQ bit in the status register
after calling the libertas handler which should clear the irq?


John


On Fri, Oct 16, 2009 at 10:06 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxxxx> wrote:
John Rigby wrote:
Dirk,

Thanks for the update.  After looking more closely at your patch I
found that the only thing my attempt was missing was the IBG setting.
I added that to mine with the result that the system hangs on loading
the libertas modules.
On which board are you working? libertas does mean you are working on wifi
connected via SDIO?

The last thing i see is the firmware request
message.
Ok, you see a hang. Below we have a timeout.

I would vote for some interrupt issues. For timeouts I would vote for
missing interrupts, while system hang might be infinite interrupts
(something like this was reported in [1]).

Could you add some debug code in omap_hsmmc_enable_sdio_irq() to trace if
enable/disable is called in the correct order? And in omap_hsmmc_irq() in if
(status & CIRQ) { to check if and how often it is called?

Best regards

Dirk

[1] http://groups.google.com/group/beagleboard/msg/5cdfe2a319531937

On Fri, Oct 16, 2009 at 9:02 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxxxx>
wrote:
John,

John Rigby wrote:
Dirk,

Many thanks, after sending the request yesterday I made my own attempt
which failed miserably.  My patch looks like a subset of yours so that
is to be expected.  I'll try yours out today and report back my
experience.
I got already some (private) feedback (thanks!):

- Accessing host->mmc->card in omap_hsmmc_set_ios() results in a null
pointer :( Seems that mmc->card isn't set yet while calling
omap_hsmmc_set_ios().

As workaround, for testing only, I hard coded setting IBG with something
like

      case MMC_BUS_WIDTH_4:
              OMAP_HSMMC_WRITE(host->base, CON, con & ~DW8);
-               OMAP_HSMMC_WRITE(host->base, HCTL,
-                       OMAP_HSMMC_READ(host->base, HCTL) | FOUR_BIT);
+                       OMAP_HSMMC_WRITE(host->base, HCTL,
+                                        OMAP_HSMMC_READ(host->base,
HCTL) |
IBG | FOUR_BIT);
             break;

If you test this, be careful that this doesn't hurt any other 4 bit cards
except the SDIO you want to test.

Later, we have to find a way how to detect that we are in SDIO mode. We
want
to set IBG only if in SDIO and 4 bit mode.

-  After this, I got the report that null pointer crash is gone. But SDIO
doesn't seem to work any more:

libertas_sdio: Libertas SDIO driver
libertas_sdio: Copyright Pierre Ossman
libertas_sdio mmc1:0001:1: firmware: requesting sd8686_helper.bin
libertas_sdio mmc1:0001:1: firmware: requesting sd8686.bin
libertas: command 0x0003 timed out
libertas: requeueing command 0x0003 due to timeout (#1)
libertas: command 0x0003 timed out
libertas: requeueing command 0x0003 due to timeout (#2)
libertas: command 0x0003 timed out
libertas: requeueing command 0x0003 due to timeout (#3)
libertas: command 0x0003 timed out
libertas: Excessive timeouts submitting command 0x0003
libertas: PREP_CMD: command 0x0003 failed: -110
libertas_sdio: probe of mmc1:0001:1 failed with error -110

I now start to think about it ;)

Again, any help would be appreciated!

Best regards

Dirk

On Fri, Oct 16, 2009 at 1:16 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxxxx>
wrote:
John Rigby wrote:
I have seen several discussions about lack of sdio irq support in the
hsmmc driver but no patches.  Has anyone on this list implemented this
and/or can anyone point me to patches?
What a coincidence ;)

I'm currently working on this. See attachment what I currently have. It
is
compile tested only against recent omap linux head. I don't have a
board
using SDIO at the moment, so no real testing possible :(

Some background, maybe it helps people to step in:

Gumstix OMAP3 based Overo air board connects Marvell 88W8686 wifi by
MMC
port 2 in 4 bit configuration [1]. The wifi performance is quite bad
(~100kB/s). There is some rumor that this might be SDIO irq related
[2].
There was an attempt to fix this [3] already, but this doesn't work
[4].
Having this, I started to look into it.

I used [3], the TI Davinci driver [5] (supporting SDIO irq), the SDIO
Simplified Specification [6] and the OMAP35x TRM [7] as starting
points.

Unfortunately, the Davinci MMC registers and irqs are different
(Davinci
has
a dedicated SDIO irq). But combining [3] and [5] helps to get an idea
what
has to be done.

I think the main issues of [3] were that it doesn't enable IBG for 4
bit
mode ([6] chapter 8.1.2) and that mmc_omap_irq() doesn't reset the irq
bits.

Topics I still open:

- Is it always necessary to deal with IE _and_ ISE register? I'm not
totally
clear what the difference between these two registers are ;) And in
which
order they have to be set.

- Davinci driver [5] in line 1115 checks for data line to call
mmc_signal_sdio_irq() for irq enable.

- Davinci driver deals with SDIO in xfer_done() (line 873)

- Davinci driver sets block size to 64 if SDIO in line 701

It would be quite nice if anybody likes to comment on attachment and
help
testing.

Many thanks and best regards

Dirk

[1] http://gumstix.net/wiki/index.php?title=Overo_Wifi

[2] http://groups.google.com/group/beagleboard/msg/14e822778c5eeb56

[3] http://groups.google.com/group/beagleboard/msg/d0eb69f4c20673be

[4] http://groups.google.com/group/beagleboard/msg/5cdfe2a319531937

[5]


http://arago-project.org/git/projects/?p=linux-davinci.git;a=blob;f=drivers/mmc/host/davinci_mmc.c;h=1bf0587250614c6d8abfe02028b96e0e47148ac8;hb=HEAD

[6] http://www.sdcard.org/developers/tech/sdio/sd_bluetooth_spec/

[7] http://focus.ti.com/lit/ug/spruf98c/spruf98c.pdf


Subject: [PATCH][RFC] OMAP HSMMC: Add SDIO interrupt support
Form: Dirk Behme <dirk.behme@xxxxxxxxxxxxxx>

At the moment, OMAP HSMMC driver supports only SDIO polling, resulting
in
poor
performance. Add support for SDIO interrupt handling.

Signed-off-by: Dirk Behme <dirk.behme@xxxxxxxxxxxxxx>
---

Patch against recent omap-linux head "Linux omap got rebuilt from
scratch"
274c94b29ee7c53609a756acca974e4742c59559

Compile tested only. Please comment and help testing.

 drivers/mmc/host/omap_hsmmc.c |   48
+++++++++++++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 5 deletions(-)

Index: linux-beagle/drivers/mmc/host/omap_hsmmc.c
===================================================================
--- linux-beagle.orig/drivers/mmc/host/omap_hsmmc.c
+++ linux-beagle/drivers/mmc/host/omap_hsmmc.c
@@ -27,6 +27,7 @@
 #include <linux/timer.h>
 #include <linux/clk.h>
 #include <linux/mmc/host.h>
+#include <linux/mmc/card.h>
 #include <linux/mmc/core.h>
 #include <linux/io.h>
 #include <linux/semaphore.h>
@@ -65,6 +66,7 @@
 #define SDVSDET                        0x00000400
 #define AUTOIDLE               0x1
 #define SDBP                   (1 << 8)
+#define IBG                    (1 << 19)
 #define DTO                    0xe
 #define ICE                    0x1
 #define ICS                    0x2
@@ -76,6 +78,7 @@
 #define INT_EN_MASK            0x307F0033
 #define BWR_ENABLE             (1 << 4)
 #define BRR_ENABLE             (1 << 5)
+#define CIRQ_ENABLE            (1 << 8)
 #define INIT_STREAM            (1 << 1)
 #define DP_SELECT              (1 << 21)
 #define DDIR                   (1 << 4)
@@ -87,6 +90,7 @@
 #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)
@@ -653,6 +657,15 @@ static irqreturn_t omap_hsmmc_irq(int ir
     status = OMAP_HSMMC_READ(host->base, STAT);
     dev_dbg(mmc_dev(host->mmc), "IRQ Status is %x\n", status);

+       if (status & CIRQ) {
+               dev_dbg(mmc_dev(host->mmc), "SDIO interrupt");
+               OMAP_HSMMC_WRITE(host->base, IE,
OMAP_HSMMC_READ(host->base,
IE)
+                                & ~(CIRQ_ENABLE));
+               mmc_signal_sdio_irq(host->mmc);
+               spin_unlock(&host->irq_lock);
+               return IRQ_HANDLED;
+       }
+
     if (status & ERR) {
 #ifdef CONFIG_MMC_DEBUG
             omap_hsmmc_report_irq(host, status);
@@ -1165,8 +1178,15 @@ static void omap_hsmmc_set_ios(struct mm
             break;
     case MMC_BUS_WIDTH_4:
             OMAP_HSMMC_WRITE(host->base, CON, con & ~DW8);
-               OMAP_HSMMC_WRITE(host->base, HCTL,
-                       OMAP_HSMMC_READ(host->base, HCTL) | FOUR_BIT);
+               if (mmc_card_sdio(host->mmc->card)) {
+                       OMAP_HSMMC_WRITE(host->base, HCTL,
+                                        OMAP_HSMMC_READ(host->base,
HCTL)
+                                        | IBG | FOUR_BIT);
+               } else {
+                       OMAP_HSMMC_WRITE(host->base, HCTL,
+                                        OMAP_HSMMC_READ(host->base,
HCTL)
+                                        | FOUR_BIT);
+               }
             break;
     case MMC_BUS_WIDTH_1:
             OMAP_HSMMC_WRITE(host->base, CON, con & ~DW8);
@@ -1512,6 +1532,24 @@ static int omap_hsmmc_disable_fclk(struc
     return 0;
 }

+static void omap_hsmmc_enable_sdio_irq(struct mmc_host *mmc, int
enable)
+{
+       struct omap_hsmmc_host *host = mmc_priv(mmc);
+       u32 ie, ise;
+
+       ise = OMAP_HSMMC_READ(host->base, ISE);
+       ie  = OMAP_HSMMC_READ(host->base, IE);
+
+       if (enable) {
+               OMAP_HSMMC_WRITE(host->base, ISE, ise | CIRQ_ENABLE);
+               OMAP_HSMMC_WRITE(host->base, IE,  ie  | CIRQ_ENABLE);
+       } else {
+               OMAP_HSMMC_WRITE(host->base, ISE, ise & ~CIRQ_ENABLE);
+               OMAP_HSMMC_WRITE(host->base, IE,  ie  & ~CIRQ_ENABLE);
+       }
+
+}
+
 static const struct mmc_host_ops omap_hsmmc_ops = {
     .enable = omap_hsmmc_enable_fclk,
     .disable = omap_hsmmc_disable_fclk,
@@ -1519,7 +1557,7 @@ static const struct mmc_host_ops omap_hs
     .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 = {
@@ -1529,7 +1567,7 @@ static const struct mmc_host_ops omap_hs
     .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
@@ -1734,7 +1772,7 @@ static int __init omap_hsmmc_probe(struc
     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;





--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux