Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
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 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
Reviewed-by: Jaehoon Chung
>>>
>>> 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
>
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
On Tue, Mar 15, 2022 at 10:47:57AM +0100, Robert Marko wrote:
> On Mon, Mar 14, 2022 at 2:10 PM Tom Rini 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
> > >
> > > Reviewed-by: Jaehoon Chung
> >
> > 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.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
On Mon, Mar 14, 2022 at 2:10 PM Tom Rini 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
> >
> > Reviewed-by: Jaehoon Chung
>
> 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
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
1 Zagreb, Croatia
Email: robert.ma...@sartura.hr
Web: www.sartura.hr
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
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
>
> Reviewed-by: Jaehoon Chung
Since this is a regression fix, will this be in the PR with the imx fix
as well? Thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
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
Reviewed-by: Stefan Roese
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= _ops,
+ .ops= _mmc_ops,
.bind = xenon_sdhci_bind,
.probe = xenon_sdhci_probe,
.remove = xenon_sdhci_remove,
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
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
Reviewed-by: Jaehoon Chung
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= _ops,
> + .ops= _mmc_ops,
> .bind = xenon_sdhci_bind,
> .probe = xenon_sdhci_probe,
> .remove = xenon_sdhci_remove,
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
On Fri, 11 Mar 2022 19:52:40 +0100 Pali Rohár 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
Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
+ 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
> ---
> 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= _ops,
> + .ops= _mmc_ops,
> .bind = xenon_sdhci_bind,
> .probe = xenon_sdhci_probe,
> .remove = xenon_sdhci_remove,
> --
> 2.35.1
>
[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
---
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= _ops,
+ .ops= _mmc_ops,
.bind = xenon_sdhci_bind,
.probe = xenon_sdhci_probe,
.remove = xenon_sdhci_remove,
--
2.35.1
