[PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
As noted in commit 3a6383207be ("mmc: sdhci: add the quirk for broken r1b response"), some MMC controllers don't always set the transfer complete bit with R1b responses.
According to the SD Host Controller Simplified Specification v4.20,
In the case of a command pairing with response-with-busy[, Transfer Complete] is set when busy is de-asserted. Refer to DAT Line Active and Command Inhibit (DAT) in the Present State register.
By polling the DAT Line Active bit in the present state register, we can detect when we are no longer busy, without waiting for a long timeout. This results in much faster reads/writes on buggy controllers.
Signed-off-by: Sean Anderson sean.anderson@seco.com --- I've CC'd a few people who've added this quirk to controllers recently. Please let me know if your hardware still works. It's possible that my hardware is buggy in a different way.
drivers/mmc/sdhci.c | 19 ++++++++++++------- include/sdhci.h | 1 + 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index fc9c6c37996..0178ed8a11e 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -306,14 +306,19 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break;
- if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) { - if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) { + if (host->quirks & SDHCI_QUIRK_BROKEN_R1B && + cmd->resp_type & MMC_RSP_BUSY && !data) { + unsigned int state = + sdhci_readl(host, SDHCI_PRESENT_STATE); + + if (!(state & SDHCI_DAT_ACTIVE)) return 0; - } else { - printf("%s: Timeout for status update!\n", - __func__); - return -ETIMEDOUT; - } + } + + if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) { + printf("%s: Timeout for status update: %08x %08x\n", + __func__, stat, mask); + return -ETIMEDOUT; } } while ((stat & mask) != mask);
diff --git a/include/sdhci.h b/include/sdhci.h index 70fefca2a97..a1b74e3bd79 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -57,6 +57,7 @@ #define SDHCI_PRESENT_STATE 0x24 #define SDHCI_CMD_INHIBIT BIT(0) #define SDHCI_DATA_INHIBIT BIT(1) +#define SDHCI_DAT_ACTIVE BIT(2) #define SDHCI_DOING_WRITE BIT(8) #define SDHCI_DOING_READ BIT(9) #define SDHCI_SPACE_AVAILABLE BIT(10)
Hi Sean,
Tested on odroid-u2 (exynos4412-odroid) on top of 2024.01-rc1, still works fine, so feel free to add:
Tested-by: Henrik Grimler henrik@grimler.se
Best regards, Henrik Grimler
On Fri, Oct 27, 2023 at 04:57:03PM -0400, Sean Anderson wrote:
As noted in commit 3a6383207be ("mmc: sdhci: add the quirk for broken r1b response"), some MMC controllers don't always set the transfer complete bit with R1b responses.
According to the SD Host Controller Simplified Specification v4.20,
In the case of a command pairing with response-with-busy[, Transfer Complete] is set when busy is de-asserted. Refer to DAT Line Active and Command Inhibit (DAT) in the Present State register.
By polling the DAT Line Active bit in the present state register, we can detect when we are no longer busy, without waiting for a long timeout. This results in much faster reads/writes on buggy controllers.
Signed-off-by: Sean Anderson sean.anderson@seco.com
I've CC'd a few people who've added this quirk to controllers recently. Please let me know if your hardware still works. It's possible that my hardware is buggy in a different way.
drivers/mmc/sdhci.c | 19 ++++++++++++------- include/sdhci.h | 1 + 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index fc9c6c37996..0178ed8a11e 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -306,14 +306,19 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break;
if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) {
if (host->quirks & SDHCI_QUIRK_BROKEN_R1B &&cmd->resp_type & MMC_RSP_BUSY && !data) {unsigned int state =sdhci_readl(host, SDHCI_PRESENT_STATE);if (!(state & SDHCI_DAT_ACTIVE)) return 0;
} else {printf("%s: Timeout for status update!\n",__func__);return -ETIMEDOUT;}
}if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {printf("%s: Timeout for status update: %08x %08x\n",__func__, stat, mask);return -ETIMEDOUT;diff --git a/include/sdhci.h b/include/sdhci.h index 70fefca2a97..a1b74e3bd79 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -57,6 +57,7 @@ #define SDHCI_PRESENT_STATE 0x24 #define SDHCI_CMD_INHIBIT BIT(0) #define SDHCI_DATA_INHIBIT BIT(1) +#define SDHCI_DAT_ACTIVE BIT(2) #define SDHCI_DOING_WRITE BIT(8) #define SDHCI_DOING_READ BIT(9)
#define SDHCI_SPACE_AVAILABLE BIT(10)
2.35.1.1320.gc452695387.dirty
Hi,
-----Original Message----- From: Henrik Grimler henrik@grimler.se Sent: Sunday, October 29, 2023 5:31 AM To: Sean Anderson sean.anderson@seco.com Cc: u-boot@lists.denx.de; Jaehoon Chung jh80.chung@samsung.com; Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com; Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Subject: Re: [PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
Hi Sean,
Tested on odroid-u2 (exynos4412-odroid) on top of 2024.01-rc1, still works fine, so feel free to add:
Tested-by: Henrik Grimler henrik@grimler.se
Best regards, Henrik Grimler
On Fri, Oct 27, 2023 at 04:57:03PM -0400, Sean Anderson wrote:
As noted in commit 3a6383207be ("mmc: sdhci: add the quirk for broken r1b response"), some MMC controllers don't always set the transfer complete bit with R1b responses.
According to the SD Host Controller Simplified Specification v4.20,
In the case of a command pairing with response-with-busy[, Transfer Complete] is set when busy is de-asserted. Refer to DAT Line Active and Command Inhibit (DAT) in the Present State register.
By polling the DAT Line Active bit in the present state register, we can detect when we are no longer busy, without waiting for a long timeout. This results in much faster reads/writes on buggy controllers.
Signed-off-by: Sean Anderson sean.anderson@seco.com
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
I've CC'd a few people who've added this quirk to controllers recently. Please let me know if your hardware still works. It's possible that my hardware is buggy in a different way.
drivers/mmc/sdhci.c | 19 ++++++++++++------- include/sdhci.h | 1 + 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index fc9c6c37996..0178ed8a11e 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -306,14 +306,19 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break;
if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) {
if (host->quirks & SDHCI_QUIRK_BROKEN_R1B &&cmd->resp_type & MMC_RSP_BUSY && !data) {unsigned int state =sdhci_readl(host, SDHCI_PRESENT_STATE);if (!(state & SDHCI_DAT_ACTIVE)) return 0;
} else {printf("%s: Timeout for status update!\n",__func__);return -ETIMEDOUT;}
}if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {printf("%s: Timeout for status update: %08x %08x\n",__func__, stat, mask);return -ETIMEDOUT;diff --git a/include/sdhci.h b/include/sdhci.h index 70fefca2a97..a1b74e3bd79 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -57,6 +57,7 @@ #define SDHCI_PRESENT_STATE 0x24 #define SDHCI_CMD_INHIBIT BIT(0) #define SDHCI_DATA_INHIBIT BIT(1) +#define SDHCI_DAT_ACTIVE BIT(2) #define SDHCI_DOING_WRITE BIT(8) #define SDHCI_DOING_READ BIT(9)
#define SDHCI_SPACE_AVAILABLE BIT(10)
2.35.1.1320.gc452695387.dirty
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Jaehoon Chung Sent: Wednesday, November 1, 2023 10:09 AM To: 'Henrik Grimler' henrik@grimler.se; 'Sean Anderson' sean.anderson@seco.com Cc: u-boot@lists.denx.de; 'Rayagonda Kokatanur' rayagonda.kokatanur@broadcom.com; 'Bharat Kumar Reddy Gooty' bharat.gooty@broadcom.com Subject: RE: [PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
Hi,
-----Original Message----- From: Henrik Grimler henrik@grimler.se Sent: Sunday, October 29, 2023 5:31 AM To: Sean Anderson sean.anderson@seco.com Cc: u-boot@lists.denx.de; Jaehoon Chung jh80.chung@samsung.com; Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com; Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Subject: Re: [PATCH] mmc: sdhci: Rework SDHCI_QUIRK_BROKEN_R1B
Hi Sean,
Tested on odroid-u2 (exynos4412-odroid) on top of 2024.01-rc1, still works fine, so feel free to add:
Tested-by: Henrik Grimler henrik@grimler.se
Best regards, Henrik Grimler
On Fri, Oct 27, 2023 at 04:57:03PM -0400, Sean Anderson wrote:
As noted in commit 3a6383207be ("mmc: sdhci: add the quirk for broken r1b response"), some MMC controllers don't always set the transfer complete bit with R1b responses.
According to the SD Host Controller Simplified Specification v4.20,
In the case of a command pairing with response-with-busy[, Transfer Complete] is set when busy is de-asserted. Refer to DAT Line Active and Command Inhibit (DAT) in the Present State register.
By polling the DAT Line Active bit in the present state register, we can detect when we are no longer busy, without waiting for a long timeout. This results in much faster reads/writes on buggy controllers.
Signed-off-by: Sean Anderson sean.anderson@seco.com
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Applied to u-boot-mmc/master. Thanks!
Best Regards, Jaehoon Chung
Best Regards, Jaehoon Chung
I've CC'd a few people who've added this quirk to controllers recently. Please let me know if your hardware still works. It's possible that my hardware is buggy in a different way.
drivers/mmc/sdhci.c | 19 ++++++++++++------- include/sdhci.h | 1 + 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index fc9c6c37996..0178ed8a11e 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -306,14 +306,19 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd, if (stat & SDHCI_INT_ERROR) break;
if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {if (host->quirks & SDHCI_QUIRK_BROKEN_R1B) {
if (host->quirks & SDHCI_QUIRK_BROKEN_R1B &&cmd->resp_type & MMC_RSP_BUSY && !data) {unsigned int state =sdhci_readl(host, SDHCI_PRESENT_STATE);if (!(state & SDHCI_DAT_ACTIVE)) return 0;
} else {printf("%s: Timeout for status update!\n",__func__);return -ETIMEDOUT;}
}if (get_timer(start) >= SDHCI_READ_STATUS_TIMEOUT) {printf("%s: Timeout for status update: %08x %08x\n",__func__, stat, mask);return -ETIMEDOUT;diff --git a/include/sdhci.h b/include/sdhci.h index 70fefca2a97..a1b74e3bd79 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -57,6 +57,7 @@ #define SDHCI_PRESENT_STATE 0x24 #define SDHCI_CMD_INHIBIT BIT(0) #define SDHCI_DATA_INHIBIT BIT(1) +#define SDHCI_DAT_ACTIVE BIT(2) #define SDHCI_DOING_WRITE BIT(8) #define SDHCI_DOING_READ BIT(9)
#define SDHCI_SPACE_AVAILABLE BIT(10)
2.35.1.1320.gc452695387.dirty
participants (3)
-
Henrik Grimler -
Jaehoon Chung -
Sean Anderson