[PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP

Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr --- drivers/mmc/xenon_sdhci.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c index e292f2903d..2f8805096c 100644 --- a/drivers/mmc/xenon_sdhci.c +++ b/drivers/mmc/xenon_sdhci.c @@ -439,6 +439,8 @@ static const struct sdhci_ops xenon_sdhci_ops = { .set_ios_post = xenon_sdhci_set_ios_post };
+static struct dm_mmc_ops xenon_mmc_ops; + static int xenon_sdhci_probe(struct udevice *dev) { struct xenon_sdhci_plat *plat = dev_get_plat(dev); @@ -452,6 +454,9 @@ static int xenon_sdhci_probe(struct udevice *dev) host->mmc->dev = dev; upriv->mmc = host->mmc;
+ xenon_mmc_ops = sdhci_ops; + xenon_mmc_ops.wait_dat0 = NULL; + /* Set quirks */ host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_32BIT_DMA_ADDR;
@@ -568,7 +573,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = { .id = UCLASS_MMC, .of_match = xenon_sdhci_ids, .of_to_plat = xenon_sdhci_of_to_plat, - .ops = &sdhci_ops, + .ops = &xenon_mmc_ops, .bind = xenon_sdhci_bind, .probe = xenon_sdhci_probe, .remove = xenon_sdhci_remove,

+ Marek
Xenon eMMC is broken in U-Boot, could you check / verify it on A3720 eMMC based board?
On Friday 11 March 2022 19:14:07 Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
drivers/mmc/xenon_sdhci.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c index e292f2903d..2f8805096c 100644 --- a/drivers/mmc/xenon_sdhci.c +++ b/drivers/mmc/xenon_sdhci.c @@ -439,6 +439,8 @@ static const struct sdhci_ops xenon_sdhci_ops = { .set_ios_post = xenon_sdhci_set_ios_post };
+static struct dm_mmc_ops xenon_mmc_ops;
static int xenon_sdhci_probe(struct udevice *dev) { struct xenon_sdhci_plat *plat = dev_get_plat(dev); @@ -452,6 +454,9 @@ static int xenon_sdhci_probe(struct udevice *dev) host->mmc->dev = dev; upriv->mmc = host->mmc;
- xenon_mmc_ops = sdhci_ops;
- xenon_mmc_ops.wait_dat0 = NULL;
- /* Set quirks */ host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_32BIT_DMA_ADDR;
@@ -568,7 +573,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = { .id = UCLASS_MMC, .of_match = xenon_sdhci_ids, .of_to_plat = xenon_sdhci_of_to_plat,
- .ops = &sdhci_ops,
- .ops = &xenon_mmc_ops, .bind = xenon_sdhci_bind, .probe = xenon_sdhci_probe, .remove = xenon_sdhci_remove,
-- 2.35.1

On Fri, 11 Mar 2022 19:52:40 +0100 Pali Rohár pali@kernel.org wrote:
- Marek
Xenon eMMC is broken in U-Boot, could you check / verify it on A3720 eMMC based board?
I can confirm this. I also had to workaround this issue, I reverted the commit adding support for wait_dat0 in my internal repo.
For now I am pro this solution, so
Reviewed-by: Marek Behún marek.behun@nic.cz
Marek

On 3/12/22 03:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
drivers/mmc/xenon_sdhci.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c index e292f2903d..2f8805096c 100644 --- a/drivers/mmc/xenon_sdhci.c +++ b/drivers/mmc/xenon_sdhci.c @@ -439,6 +439,8 @@ static const struct sdhci_ops xenon_sdhci_ops = { .set_ios_post = xenon_sdhci_set_ios_post };
+static struct dm_mmc_ops xenon_mmc_ops;
static int xenon_sdhci_probe(struct udevice *dev) { struct xenon_sdhci_plat *plat = dev_get_plat(dev); @@ -452,6 +454,9 @@ static int xenon_sdhci_probe(struct udevice *dev) host->mmc->dev = dev; upriv->mmc = host->mmc;
- xenon_mmc_ops = sdhci_ops;
- xenon_mmc_ops.wait_dat0 = NULL;
- /* Set quirks */ host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_32BIT_DMA_ADDR;
@@ -568,7 +573,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = { .id = UCLASS_MMC, .of_match = xenon_sdhci_ids, .of_to_plat = xenon_sdhci_of_to_plat,
- .ops = &sdhci_ops,
- .ops = &xenon_mmc_ops, .bind = xenon_sdhci_bind, .probe = xenon_sdhci_probe, .remove = xenon_sdhci_remove,

On Mon, Mar 14, 2022 at 06:37:02PM +0900, Jaehoon Chung wrote:
On 3/12/22 03:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Since this is a regression fix, will this be in the PR with the imx fix as well? Thanks!

On Mon, Mar 14, 2022 at 2:10 PM Tom Rini trini@konsulko.com wrote:
On Mon, Mar 14, 2022 at 06:37:02PM +0900, Jaehoon Chung wrote:
On 3/12/22 03:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Since this is a regression fix, will this be in the PR with the imx fix as well? Thanks!
Hi Tom, Was this question directed at me or?
Regards, Robert
-- Tom

On Tue, Mar 15, 2022 at 10:47:57AM +0100, Robert Marko wrote:
On Mon, Mar 14, 2022 at 2:10 PM Tom Rini trini@konsulko.com wrote:
On Mon, Mar 14, 2022 at 06:37:02PM +0900, Jaehoon Chung wrote:
On 3/12/22 03:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Since this is a regression fix, will this be in the PR with the imx fix as well? Thanks!
Hi Tom, Was this question directed at me or?
Sorry, to Jaehoon or Peng.

On 3/15/22 21:22, Tom Rini wrote:
On Tue, Mar 15, 2022 at 10:47:57AM +0100, Robert Marko wrote:
On Mon, Mar 14, 2022 at 2:10 PM Tom Rini trini@konsulko.com wrote:
On Mon, Mar 14, 2022 at 06:37:02PM +0900, Jaehoon Chung wrote:
On 3/12/22 03:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Since this is a regression fix, will this be in the PR with the imx fix as well? Thanks!
Hi Tom, Was this question directed at me or?
Sorry, to Jaehoon or Peng.
Applied u-boot-mmc, Thanks! Sorry for late.
Best Regards, Jaehoon Chung

On 3/11/22 19:14, Robert Marko wrote:
Generic SDHCI driver received support for checking the busy status by polling the DAT[0] level instead of waiting for the worst MMC switch time.
Unfortunately, it appears that this does not work for Xenon controllers despite being a part of the standard SDHCI registers and the Armada 3720 datasheet itself telling that BIT(20) is useful for detecting the DAT[0] busy signal.
I have tried increasing the timeout value, but I have newer managed to catch DAT_LEVEL bits change from 0 at all.
This issue appears to hit most if not all SoC-s supported by Xenon driver, at least A3720, A8040 and CN9130 have non working eMMC currently.
So, until a better solution is found drop the wait_dat0 OP for Xenon. I was able to only test it on A3720, but it should work for others as well.
Fixes: 40e6f52454fc ("drivers: mmc: Add wait_dat0 support for sdhci driver") Signed-off-by: Robert Marko robert.marko@sartura.hr
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/mmc/xenon_sdhci.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/xenon_sdhci.c b/drivers/mmc/xenon_sdhci.c index e292f2903d..2f8805096c 100644 --- a/drivers/mmc/xenon_sdhci.c +++ b/drivers/mmc/xenon_sdhci.c @@ -439,6 +439,8 @@ static const struct sdhci_ops xenon_sdhci_ops = { .set_ios_post = xenon_sdhci_set_ios_post };
+static struct dm_mmc_ops xenon_mmc_ops;
- static int xenon_sdhci_probe(struct udevice *dev) { struct xenon_sdhci_plat *plat = dev_get_plat(dev);
@@ -452,6 +454,9 @@ static int xenon_sdhci_probe(struct udevice *dev) host->mmc->dev = dev; upriv->mmc = host->mmc;
- xenon_mmc_ops = sdhci_ops;
- xenon_mmc_ops.wait_dat0 = NULL;
- /* Set quirks */ host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_32BIT_DMA_ADDR;
@@ -568,7 +573,7 @@ U_BOOT_DRIVER(xenon_sdhci_drv) = { .id = UCLASS_MMC, .of_match = xenon_sdhci_ids, .of_to_plat = xenon_sdhci_of_to_plat,
- .ops = &sdhci_ops,
- .ops = &xenon_mmc_ops, .bind = xenon_sdhci_bind, .probe = xenon_sdhci_probe, .remove = xenon_sdhci_remove,
Viele Grüße, Stefan Roese
participants (6)
-
Jaehoon Chung
-
Marek Behún
-
Pali Rohár
-
Robert Marko
-
Stefan Roese
-
Tom Rini