* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-11 18:14 ` [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP Robert Marko
@ 2022-03-11 18:52 ` Pali Rohár
2022-03-12 17:39 ` Marek Behún
2022-03-14 9:37 ` Jaehoon Chung
2022-03-14 12:03 ` Stefan Roese
2 siblings, 1 reply; 9+ messages in thread
From: Pali Rohár @ 2022-03-11 18:52 UTC (permalink / raw)
To: Marek Behún; +Cc: Robert Marko, sr, peng.fan, jh80.chung, stcarlso, u-boot
+ 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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-11 18:52 ` Pali Rohár
@ 2022-03-12 17:39 ` Marek Behún
0 siblings, 0 replies; 9+ messages in thread
From: Marek Behún @ 2022-03-12 17:39 UTC (permalink / raw)
To: Pali Rohár; +Cc: Robert Marko, sr, peng.fan, jh80.chung, stcarlso, u-boot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-11 18:14 ` [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP Robert Marko
2022-03-11 18:52 ` Pali Rohár
@ 2022-03-14 9:37 ` Jaehoon Chung
2022-03-14 13:10 ` Tom Rini
2022-03-14 12:03 ` Stefan Roese
2 siblings, 1 reply; 9+ messages in thread
From: Jaehoon Chung @ 2022-03-14 9:37 UTC (permalink / raw)
To: Robert Marko, sr, peng.fan, stcarlso, u-boot, pali
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,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-14 9:37 ` Jaehoon Chung
@ 2022-03-14 13:10 ` Tom Rini
2022-03-15 9:47 ` Robert Marko
0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2022-03-14 13:10 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Robert Marko, sr, peng.fan, stcarlso, u-boot, pali
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
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!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-14 13:10 ` Tom Rini
@ 2022-03-15 9:47 ` Robert Marko
2022-03-15 12:22 ` Tom Rini
0 siblings, 1 reply; 9+ messages in thread
From: Robert Marko @ 2022-03-15 9:47 UTC (permalink / raw)
To: Tom Rini
Cc: Jaehoon Chung, Stefan Roese, peng.fan, stcarlso, U-Boot-Denx,
Pali Rohár
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
--
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-15 9:47 ` Robert Marko
@ 2022-03-15 12:22 ` Tom Rini
2022-03-16 7:48 ` Jaehoon Chung
0 siblings, 1 reply; 9+ messages in thread
From: Tom Rini @ 2022-03-15 12:22 UTC (permalink / raw)
To: Robert Marko
Cc: Jaehoon Chung, Stefan Roese, peng.fan, stcarlso, U-Boot-Denx,
Pali Rohár
[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]
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.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-15 12:22 ` Tom Rini
@ 2022-03-16 7:48 ` Jaehoon Chung
0 siblings, 0 replies; 9+ messages in thread
From: Jaehoon Chung @ 2022-03-16 7:48 UTC (permalink / raw)
To: Tom Rini, Robert Marko
Cc: Stefan Roese, peng.fan, stcarlso, U-Boot-Denx, Pali Rohár
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP
2022-03-11 18:14 ` [PATCH] mmc: xenon_sdhci: remove wait_dat0 SDHCI OP Robert Marko
2022-03-11 18:52 ` Pali Rohár
2022-03-14 9:37 ` Jaehoon Chung
@ 2022-03-14 12:03 ` Stefan Roese
2 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2022-03-14 12:03 UTC (permalink / raw)
To: Robert Marko, peng.fan, jh80.chung, stcarlso, u-boot, pali
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
--
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: sr@denx.de
^ permalink raw reply [flat|nested] 9+ messages in thread