Skip to contentSkip to conversation

mmc: sunxi: Don't start commands while the card is busy

PATCH Superseded Hans de Goede 5 participants 11 messages Jul 10, 2015 – Aug 25, 2015

Subsystems (2)
MMC, SD and SDIO Arm/Allwinner sunXi SoC support
lore
Share & tools
lore

Hans de Goede

author
#0 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>
Cc Arend van Spriel <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:
-Properly accredit Eugene K for coming up with the fix for this

---
 drivers/mmc/host/sunxi-mmc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
drivers/mmc/host/sunxi-mmc.c +32 -0
index 4d3e1ff..daa90b7 100644
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host *mmc)
 	return 0;
 }
 
+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+	unsigned long expire = jiffies + msecs_to_jiffies(500);
+	u32 rval;
+
+	do {
+		rval = mmc_readl(host, REG_STAS);
+	} while (time_before(jiffies, expire) && (rval & SDXC_CARD_DATA_BUSY));
+
+	if (rval & SDXC_CARD_DATA_BUSY) {
+		dev_err(mmc_dev(host->mmc), "Error R1 ready timeout\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
 				    struct mmc_data *data)
 {

@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct sunxi_mmc_host *host,
 	u32 arg, cmd_val, ri;
 	unsigned long expire = jiffies + msecs_to_jiffies(1000);
 
+	sunxi_mmc_wait_card_ready(host);
+
 	cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
 		  SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;
 

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
 {
 	unsigned long expire = jiffies + msecs_to_jiffies(250);
 	u32 rval;
+	int ret;
+
+	ret = sunxi_mmc_wait_card_ready(host);
+	if (ret)
+		return ret;
 
 	rval = mmc_readl(host, REG_CLKCR);
 	rval &= ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);

@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 		return;
 	}
 
+	ret = sunxi_mmc_wait_card_ready(host);
+	if (ret) {
+		mrq->cmd->error = ret;
+		mmc_request_done(mmc, mrq);
+		return;
+	}
+
 	if (data) {
 		ret = sunxi_mmc_map_dma(host, data);
 		if (ret < 0) {
Signature · 1 line
2.4.3
#1 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Hi,

Quoted 10 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>

You should use the "real" name here, and not some shortening.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150720/af9c8cfa/attachment.sig>
author
#2 lore raw

Hi,

Quoted 7 lines · Maxime Ripard

Hi,

Quoted 4 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Quoted 8 lines
Quoted 6 lines

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>

You should use the "real" name here, and not some shortening.

I don't know the real name, and AFAIK this is really only important for S-o-b lines.

Eugene, care to share your full name with us?

Regards,

Hans

#3 lore raw
Quoted 13 lines · Hans de Goede

Hi,

Quoted 10 lines · Maxime Ripard

Hi,

Quoted 7 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>

You should use the "real" name here, and not some shortening.

I don't know the real name, and AFAIK this is really only important for S-o-b lines.

This is mandatory only for SoB, but it's pointless to have it in the other tags. This is to identify the developper that sets these tags, and we can't do that without a full name.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150724/d5018f03/attachment.sig>
#4 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

In reply to Hans de Goede
Quoted 3 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Which are those commands? Or more interesting, which response types do these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

Quoted 81 lines

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>
Cc Arend van Spriel <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:

(quoted diff) +0 -2
-Properly accredit Eugene K for coming up with the fix for this
---
 drivers/mmc/host/sunxi-mmc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
drivers/mmc/host/sunxi-mmc.c +32 -1
index 4d3e1ff..daa90b7 100644
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host *mmc)
        return 0;
 }
 
+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+       unsigned long expire = jiffies + msecs_to_jiffies(500);
+       u32 rval;
+
+       do {
+               rval = mmc_readl(host, REG_STAS);
+       } while (time_before(jiffies, expire) && (rval & SDXC_CARD_DATA_BUSY));
+
+       if (rval & SDXC_CARD_DATA_BUSY) {
+               dev_err(mmc_dev(host->mmc), "Error R1 ready timeout\n");
+               return -EIO;
+       }
+
+       return 0;
+}
+
 static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
                                    struct mmc_data *data)
 {

@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct sunxi_mmc_host *host,
        u32 arg, cmd_val, ri;
        unsigned long expire = jiffies + msecs_to_jiffies(1000);
 
+       sunxi_mmc_wait_card_ready(host);
+
        cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
                  SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;
 

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
 {
        unsigned long expire = jiffies + msecs_to_jiffies(250);
        u32 rval;
+       int ret;
+
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret)
+               return ret;
 
        rval = mmc_readl(host, REG_CLKCR);
        rval &= ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);

@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
                return;
        }
 
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret) {
+               mrq->cmd->error = ret;
+               mmc_request_done(mmc, mrq);
+               return;
+       }
+
        if (data) {
                ret = sunxi_mmc_map_dma(host, data);
                if (ret < 0) {
--
Quoted 2 lines

2.4.3

Kind regards
Uffe

#5 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Quoted 9 lines · Ulf Hansson
Quoted 3 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Which are those commands? Or more interesting, which response types do these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Indeed. A similar fix was needed for dw_mmc host controller.

Quoted 4 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

The problem here is that there are a number of host controllers out there not implementing that callback. That may be because the hardware is dealing properly with busy signalling, but I would not bet on that.

Regards,
Arend

Quoted 90 lines

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

Quoted 81 lines

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>
Cc Arend van Spriel <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:

(quoted diff) +0 -2
-Properly accredit Eugene K for coming up with the fix for this
---
  drivers/mmc/host/sunxi-mmc.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
drivers/mmc/host/sunxi-mmc.c +32 -1
index 4d3e1ff..daa90b7 100644
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host *mmc)
         return 0;
  }
 
+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+       unsigned long expire = jiffies + msecs_to_jiffies(500);
+       u32 rval;
+
+       do {
+               rval = mmc_readl(host, REG_STAS);
+       } while (time_before(jiffies, expire) && (rval & SDXC_CARD_DATA_BUSY));
+
+       if (rval & SDXC_CARD_DATA_BUSY) {
+               dev_err(mmc_dev(host->mmc), "Error R1 ready timeout\n");
+               return -EIO;
+       }
+
+       return 0;
+}
+
  static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
                                     struct mmc_data *data)
  {

@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct sunxi_mmc_host *host,
         u32 arg, cmd_val, ri;
         unsigned long expire = jiffies + msecs_to_jiffies(1000);
 
+       sunxi_mmc_wait_card_ready(host);
+
         cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
                   SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;
 

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
  {
         unsigned long expire = jiffies + msecs_to_jiffies(250);
         u32 rval;
+       int ret;
+
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret)
+               return ret;
 
         rval = mmc_readl(host, REG_CLKCR);
         rval &= ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);

@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
                 return;
         }
 
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret) {
+               mrq->cmd->error = ret;
+               mmc_request_done(mmc, mrq);
+               return;
+       }
+
         if (data) {
                 ret = sunxi_mmc_map_dma(host, data);
                 if (ret < 0) {
--
Quoted 2 lines

2.4.3

Kind regards
Uffe

#6 lore raw
Quoted 6 lines · Arend van Spriel
Quoted 5 lines · Ulf Hansson
Quoted 3 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Quoted 6 lines
Quoted 4 lines

Which are those commands? Or more interesting, which response types do these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Indeed. A similar fix was needed for dw_mmc host controller.

bit 10 in STATUS register of sysnopsys dw_mmc should be issued each time you start a new cmd. It's hard to say whetehr a local hack or not, because it's a mandatory requirement from IP databook(refer to "Sysnopsys DesignWare cores Mobile Storage Host Databook" Chapter 7- Programming the DWC_mobile_storage).

Bit 10: indicate state-machine is ready, controller MUST guarantee all I/O used in "free state", actually high.

If dw_mmc's state-machine isn't ready, NO cmd can be issued, which means even we configure cmd and cmdarg, start bit cannot be auto-clean by controller that leads to no cmd_done interrupt generated which dw_mmc's tasklet state-machine need.

Quoted 116 lines
Quoted 5 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

The problem here is that there are a number of host controllers out there not implementing that callback. That may be because the hardware is dealing properly with busy signalling, but I would not bet on that.

Regards,
Arend

Quoted 78 lines

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

Quoted 75 lines

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>
Cc Arend van Spriel <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:

(quoted diff) +0 -2
-Properly accredit Eugene K for coming up with the fix for this
---
  drivers/mmc/host/sunxi-mmc.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
drivers/mmc/host/sunxi-mmc.c +18 -0
index 4d3e1ff..daa90b7 100644
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host
 *mmc)
         return 0;
  }
 
+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+       unsigned long expire = jiffies + msecs_to_jiffies(500);
+       u32 rval;
+
+       do {
+               rval = mmc_readl(host, REG_STAS);
+       } while (time_before(jiffies, expire) && (rval &
 SDXC_CARD_DATA_BUSY));
+
+       if (rval & SDXC_CARD_DATA_BUSY) {
+               dev_err(mmc_dev(host->mmc), "Error R1 ready timeout\n");
+               return -EIO;
+       }
+
+       return 0;
+}
+
  static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,

struct mmc_data *data)
{

drivers/mmc/host/sunxi-mmc.c +7 -0
@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct
 sunxi_mmc_host *host,
         u32 arg, cmd_val, ri;
         unsigned long expire = jiffies + msecs_to_jiffies(1000);
 
+       sunxi_mmc_wait_card_ready(host);
+
         cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
                   SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct
 sunxi_mmc_host *host, u32 oclk_en)
  {
         unsigned long expire = jiffies + msecs_to_jiffies(250);
         u32 rval;
+       int ret;
+
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret)
+               return ret;
 
         rval = mmc_readl(host, REG_CLKCR);

         rval &= ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);
drivers/mmc/host/sunxi-mmc.c +0 -0
@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host
 *mmc, struct mmc_request *mrq)
                 return;
         }
Quoted 17 lines
Quoted 10 lines
drivers/mmc/host/sunxi-mmc.c +7 -0
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret) {
+               mrq->cmd->error = ret;
+               mmc_request_done(mmc, mrq);
+               return;
+       }
+
         if (data) {
                 ret = sunxi_mmc_map_dma(host, data);

                 if (ret < 0) {
Quoted 3 lines

--
2.4.3

Kind regards
Uffe

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Shawn Lin
author
#7 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

In reply to Ulf Hansson

Hi,

Quoted 6 lines · Ulf Hansson
Quoted 3 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Which are those commands?

The code were things get stuck when not waiting for the busy signal uses the following sdio helper functions:

sdio_readb/sdio_writeb
sdio_f0_readb/sdio_f0_writeb
sdio_readw/sdio_writew
sdio_readl/sdio_writel
sdio_readsb
sdio_memcpy_fromio/sdio_memcpy_toio

And directly issues the following command:

         mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
         mmc_cmd.opcode = SD_IO_RW_EXTENDED;
         mmc_cmd.arg = write ? 1<<31 : 0;        /* write flag  */
         mmc_cmd.arg |= (fn & 0x7) << 28;        /* SDIO func num */
         mmc_cmd.arg |= 1<<27;                   /* block mode */
         /* for function 1 the addr will be incremented */
         mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
         mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;

I do not know if the busy wait is necessary for all of these, but the hack in the android kernel code, which inserts calls to a wait_card_busy function directly
into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

Does so for all of these.

Quoted 4 lines

Or more interesting, which response types do
these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Agreed, recent dw-mmc patches address the same issue, also involving broadcom sdio wifi chips.

Quoted 4 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

I'm fine with solving this either way, implementing host_ops->card_busy() for sunxi is easy, and if the core os modified to call that function before issue sdio io ops (which seems to be the common thing here), then that indeed is better then having the sunxi code always do the busy check.

Quoted 2 lines

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

I'm afraid that we need to poll the status register. I've been unable to find an irq flag corresponding to this.

Regards,

Hans

Quoted 86 lines
Quoted 80 lines

This commit fixes this, thereby fixing the wifi reliability issues on the Cubietruck and other sunxi boards using sdio wifi.

Reported-by Eugene K <[email protected]>
Suggested-by Eugene K <[email protected]>
Cc Eugene K <[email protected]>
Cc Arend van Spriel <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:

(quoted diff) +0 -2
-Properly accredit Eugene K for coming up with the fix for this
---
  drivers/mmc/host/sunxi-mmc.c | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)
drivers/mmc/host/sunxi-mmc.c +32 -1
index 4d3e1ff..daa90b7 100644
@@ -289,6 +289,24 @@ static int sunxi_mmc_init_host(struct mmc_host *mmc)
         return 0;
  }
 
+/* Wait for card to report ready before starting a new cmd */
+static int sunxi_mmc_wait_card_ready(struct sunxi_mmc_host *host)
+{
+       unsigned long expire = jiffies + msecs_to_jiffies(500);
+       u32 rval;
+
+       do {
+               rval = mmc_readl(host, REG_STAS);
+       } while (time_before(jiffies, expire) && (rval & SDXC_CARD_DATA_BUSY));
+
+       if (rval & SDXC_CARD_DATA_BUSY) {
+               dev_err(mmc_dev(host->mmc), "Error R1 ready timeout\n");
+               return -EIO;
+       }
+
+       return 0;
+}
+
  static void sunxi_mmc_init_idma_des(struct sunxi_mmc_host *host,
                                     struct mmc_data *data)
  {

@@ -383,6 +401,8 @@ static void sunxi_mmc_send_manual_stop(struct sunxi_mmc_host *host,
         u32 arg, cmd_val, ri;
         unsigned long expire = jiffies + msecs_to_jiffies(1000);
 
+       sunxi_mmc_wait_card_ready(host);
+
         cmd_val = SDXC_START | SDXC_RESP_EXPIRE |
                   SDXC_STOP_ABORT_CMD | SDXC_CHECK_RESPONSE_CRC;
 

@@ -597,6 +617,11 @@ static int sunxi_mmc_oclk_onoff(struct sunxi_mmc_host *host, u32 oclk_en)
  {
         unsigned long expire = jiffies + msecs_to_jiffies(250);
         u32 rval;
+       int ret;
+
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret)
+               return ret;
 
         rval = mmc_readl(host, REG_CLKCR);
         rval &= ~(SDXC_CARD_CLOCK_ON | SDXC_LOW_POWER_ON);

@@ -785,6 +810,13 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
                 return;
         }
 
+       ret = sunxi_mmc_wait_card_ready(host);
+       if (ret) {
+               mrq->cmd->error = ret;
+               mmc_request_done(mmc, mrq);
+               return;
+       }
+
         if (data) {
                 ret = sunxi_mmc_map_dma(host, data);
                 if (ret < 0) {
--
Quoted 2 lines

2.4.3

Kind regards
Uffe

#8 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Quoted 58 lines · Hans de Goede

Hi,

Quoted 9 lines · Ulf Hansson
Quoted 4 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Which are those commands?

The code were things get stuck when not waiting for the busy signal uses the following sdio helper functions:

sdio_readb/sdio_writeb
sdio_f0_readb/sdio_f0_writeb
sdio_readw/sdio_writew
sdio_readl/sdio_writel
sdio_readsb
sdio_memcpy_fromio/sdio_memcpy_toio

And directly issues the following command:

        mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
        mmc_cmd.opcode = SD_IO_RW_EXTENDED;
        mmc_cmd.arg = write ? 1<<31 : 0;        /* write flag  */
        mmc_cmd.arg |= (fn & 0x7) << 28;        /* SDIO func num */
        mmc_cmd.arg |= 1<<27;                   /* block mode */
        /* for function 1 the addr will be incremented */
        mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
        mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;

I do not know if the busy wait is necessary for all of these, but the hack in the android kernel code, which inserts calls to a wait_card_busy function directly
into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

Does so for all of these.

Quoted 5 lines

Or more interesting, which response types do

these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Agreed, recent dw-mmc patches address the same issue, also involving broadcom sdio wifi chips.

Quoted 4 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

Quoted 5 lines

I'm fine with solving this either way, implementing host_ops->card_busy() for sunxi is easy, and if the core os modified to call that function before issue sdio io ops (which seems to be the common thing here), then that indeed is better then having the sunxi code always do the busy check.

Okay, so let's make the core to call the ->card_busy() callback and then it's up to each host driver to implement that callback.

As an optimization, we might consider (in a separate step) to add MMC_RSP_BUSY to the response type. I realize that would somewhat be a violation of the spec, but apparently the SDIO spec isn't really clear on this area.

Quoted 7 lines
Quoted 3 lines

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

I'm afraid that we need to poll the status register. I've been unable to find an irq flag corresponding to this.

Okay, thanks for the info.

[...]

Kind regards
Uffe

author
#9 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Hi,

Quoted 59 lines · Ulf Hansson
Quoted 58 lines · Hans de Goede

Hi,

Quoted 9 lines · Ulf Hansson
Quoted 4 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc host code. This turns out to be caused by it starting new commands while the card signals that it is still busy processing a previous command.

Which are those commands?

The code were things get stuck when not waiting for the busy signal uses the following sdio helper functions:

sdio_readb/sdio_writeb
sdio_f0_readb/sdio_f0_writeb
sdio_readw/sdio_writew
sdio_readl/sdio_writel
sdio_readsb
sdio_memcpy_fromio/sdio_memcpy_toio

And directly issues the following command:

         mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
         mmc_cmd.opcode = SD_IO_RW_EXTENDED;
         mmc_cmd.arg = write ? 1<<31 : 0;        /* write flag  */
         mmc_cmd.arg |= (fn & 0x7) << 28;        /* SDIO func num */
         mmc_cmd.arg |= 1<<27;                   /* block mode */
         /* for function 1 the addr will be incremented */
         mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
         mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;

I do not know if the busy wait is necessary for all of these, but the hack in the android kernel code, which inserts calls to a wait_card_busy function directly
into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

Does so for all of these.

Quoted 5 lines

Or more interesting, which response types do

these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Agreed, recent dw-mmc patches address the same issue, also involving broadcom sdio wifi chips.

Quoted 4 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

Quoted 8 lines
Quoted 5 lines

I'm fine with solving this either way, implementing host_ops->card_busy() for sunxi is easy, and if the core os modified to call that function before issue sdio io ops (which seems to be the common thing here), then that indeed is better then having the sunxi code always do the busy check.

Okay, so let's make the core to call the ->card_busy() callback and then it's up to each host driver to implement that callback.

Ok, do you plan to do a patch for this, or do you expect to get one submitted to you ?

Regards,

Hans

Quoted 20 lines

As an optimization, we might consider (in a separate step) to add MMC_RSP_BUSY to the response type. I realize that would somewhat be a violation of the spec, but apparently the SDIO spec isn't really clear on this area.

Quoted 7 lines
Quoted 3 lines

Within this context, could I ask whether you controller supports IRQ based HW-busy detection? Or you need polling of the status register?

I'm afraid that we need to poll the status register. I've been unable to find an irq flag corresponding to this.

Okay, thanks for the info.

[...]

Kind regards
Uffe

#10 lore raw

PATCH v2 1/1 mmc: sunxi: Don't start commands while the card is busy

Quoted 80 lines · Hans de Goede

Hi,

Quoted 76 lines · Ulf Hansson
Quoted 72 lines · Hans de Goede

Hi,

Quoted 13 lines · Ulf Hansson
Quoted 7 lines · Hans de Goede

Some sdio wifi modules have not been working reliable with the sunxi-mmc
host code. This turns out to be caused by it starting new commands while
the card signals that it is still busy processing a previous command.

Quoted 1 line

Which are those commands?

The code were things get stuck when not waiting for the busy signal uses the following sdio helper functions:

sdio_readb/sdio_writeb
sdio_f0_readb/sdio_f0_writeb
sdio_readw/sdio_writew
sdio_readl/sdio_writel
sdio_readsb
sdio_memcpy_fromio/sdio_memcpy_toio

And directly issues the following command:

         mmc_dat.flags = write ? MMC_DATA_WRITE : MMC_DATA_READ;
         mmc_cmd.opcode = SD_IO_RW_EXTENDED;
         mmc_cmd.arg = write ? 1<<31 : 0;        /* write flag  */
         mmc_cmd.arg |= (fn & 0x7) << 28;        /* SDIO func num */
         mmc_cmd.arg |= 1<<27;                   /* block mode */
         /* for function 1 the addr will be incremented */
         mmc_cmd.arg |= (fn == 1) ? 1<<26 : 0;
         mmc_cmd.flags = MMC_RSP_SPI_R5 | MMC_RSP_R5 | MMC_CMD_ADTC;

I do not know if the busy wait is necessary for all of these, but the hack in the android kernel code, which inserts calls to a wait_card_busy function directly
into: drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c

Does so for all of these.

Quoted 5 lines

Or more interesting, which response types do

these commands expect?
I don't think this is a sunxi specific issue, I have seen similar issues for other host controllers.

Agreed, recent dw-mmc patches address the same issue, also involving broadcom sdio wifi chips.

Quoted 4 lines

I am thinking that perhaps this should be managed by the mmc core instead of local hacks to sunxi. Potentially we could make the core to invoke the already existing host_ops->card_busy() callback when needed...

I'm fine with solving this either way, implementing host_ops->card_busy() for sunxi is easy, and if the core os modified to call that function before issue sdio io ops (which seems to be the common thing here), then that indeed is better then having the sunxi code always do the busy check.

Quoted 6 lines
Quoted 2 lines

Okay, so let's make the core to call the ->card_busy() callback and then it's up to each host driver to implement that callback.

Ok, do you plan to do a patch for this, or do you expect to get one submitted to you ?

I prefer to get a patch submitted to me. My bandwidth has been limited and still have a lot to catch up with...

Kind regards
Uffe