* [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible
@ 2024-04-23 6:22 Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 2/3] mci: core: fix fallback when host doesn't support HS200 Ahmad Fatoum
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2024-04-23 6:22 UTC (permalink / raw)
To: barebox; +Cc: str, mtr, Ahmad Fatoum
mci_mmc_select_hs_ddr() will try DDR52 and if that fails, revert to
SDR operation. In that case, it returns the bus width, which would
be forwarded as if it were an error code:
WARNING: mmc2: Card's startup fails with 3
Fix this by translating successful return values to 0 in
mci_startup_mmc().
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/mci/mci-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 1d383e6449e9..57b4c5b99c9c 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1566,7 +1566,7 @@ static int mci_startup_mmc(struct mci *mci)
}
}
- return ret;
+ return ret >= MMC_BUS_WIDTH_1 ? 0 : ret;
}
/**
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH master 2/3] mci: core: fix fallback when host doesn't support HS200
2024-04-23 6:22 [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Ahmad Fatoum
@ 2024-04-23 6:22 ` Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 3/3] mci: core: make execute_tuning mandatory for HS200 Ahmad Fatoum
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2024-04-23 6:22 UTC (permalink / raw)
To: barebox; +Cc: str, mtr, Ahmad Fatoum
If switching the card into HS200 mode via mmc_select_timing() succeeds,
but switching the host fails in mmc_hs200_tuning(), we are left with
wrong timing information, which prevents us from falling back to a lower
speed mode. Restore the high speed timing on failure to resolve this.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/mci/mci-core.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 57b4c5b99c9c..af0980308866 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1551,6 +1551,11 @@ static int mci_startup_mmc(struct mci *mci)
if (mmc_card_hs200(mci))
ret = mmc_hs200_tuning(mci);
+
+ if (ret) {
+ host->timing = MMC_TIMING_MMC_HS;
+ mci_switch(mci, EXT_CSD_HS_TIMING, EXT_CSD_TIMING_HS);
+ }
}
if (ret || !IS_ENABLED(CONFIG_MCI_TUNING)) {
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH master 3/3] mci: core: make execute_tuning mandatory for HS200
2024-04-23 6:22 [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 2/3] mci: core: fix fallback when host doesn't support HS200 Ahmad Fatoum
@ 2024-04-23 6:22 ` Ahmad Fatoum
2024-04-23 6:44 ` [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Sascha Hauer
2024-04-30 5:44 ` Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2024-04-23 6:22 UTC (permalink / raw)
To: barebox; +Cc: str, mtr, Ahmad Fatoum
We assume that any host controller that doesn't implement execute_tuning
will have no issue with HS200 operation. This is likely untrue for most
host drivers that we have and it will confuse users:
- Enable CONFIG_MCI_TUNING, e.g. for i.MX
- devinfo shows HS200 operation
- Actually reading blocks will fail with EIO
Therefore, let's make execute_tuning mandatory for HS200. If some driver
doesn't need it, it can always define an empty stub for it.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
drivers/mci/mci-core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index af0980308866..f6565b2b64dd 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1349,8 +1349,14 @@ int mci_execute_tuning(struct mci *mci)
struct mci_host *host = mci->host;
u32 opcode;
- if (!host->execute_tuning)
- return 0;
+ if (!host->execute_tuning) {
+ /*
+ * For us, implementing ->execute_tuning is mandatory to
+ * support higher speed modes
+ */
+ dev_warn(&mci->dev, "tuning failed: no host diver support\n");
+ return -EOPNOTSUPP;
+ }
/* Tuning is only supported for MMC / HS200 */
if (mmc_card_hs200(mci))
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible
2024-04-23 6:22 [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 2/3] mci: core: fix fallback when host doesn't support HS200 Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 3/3] mci: core: make execute_tuning mandatory for HS200 Ahmad Fatoum
@ 2024-04-23 6:44 ` Sascha Hauer
2024-04-23 6:52 ` Ahmad Fatoum
2024-04-30 5:44 ` Sascha Hauer
3 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2024-04-23 6:44 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: barebox, str, mtr
On Tue, Apr 23, 2024 at 08:22:04AM +0200, Ahmad Fatoum wrote:
> mci_mmc_select_hs_ddr() will try DDR52 and if that fails, revert to
> SDR operation. In that case, it returns the bus width, which would
> be forwarded as if it were an error code:
>
> WARNING: mmc2: Card's startup fails with 3
>
> Fix this by translating successful return values to 0 in
> mci_startup_mmc().
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> drivers/mci/mci-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 1d383e6449e9..57b4c5b99c9c 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1566,7 +1566,7 @@ static int mci_startup_mmc(struct mci *mci)
> }
> }
>
> - return ret;
> + return ret >= MMC_BUS_WIDTH_1 ? 0 : ret;
> }
Can we do it like below instead? It makes clear where a MMC_BUS_WIDTH_x
return value is expected.
Sascha
----------------------------------8<---------------------------
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 083d2f4ed1..da892a5f84 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -1558,8 +1558,12 @@ static int mci_startup_mmc(struct mci *mci)
/* find out maximum bus width and then try DDR if supported */
ret = mci_mmc_select_bus_width(mci);
- if (ret > MMC_BUS_WIDTH_1 && mci->tran_speed == 52000000)
- ret = mci_mmc_select_hs_ddr(mci);
+ if (ret > MMC_BUS_WIDTH_1) {
+ ret = 0;
+
+ if (mci->tran_speed == 52000000)
+ ret = mci_mmc_select_hs_ddr(mci);
+ }
if (ret < 0) {
dev_warn(&mci->dev, "Changing MMC bus width failed: %d\n", ret);
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible
2024-04-23 6:44 ` [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Sascha Hauer
@ 2024-04-23 6:52 ` Ahmad Fatoum
0 siblings, 0 replies; 6+ messages in thread
From: Ahmad Fatoum @ 2024-04-23 6:52 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox, str, mtr
On 23.04.24 08:44, Sascha Hauer wrote:
> On Tue, Apr 23, 2024 at 08:22:04AM +0200, Ahmad Fatoum wrote:
>> mci_mmc_select_hs_ddr() will try DDR52 and if that fails, revert to
>> SDR operation. In that case, it returns the bus width, which would
>> be forwarded as if it were an error code:
>>
>> WARNING: mmc2: Card's startup fails with 3
>>
>> Fix this by translating successful return values to 0 in
>> mci_startup_mmc().
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> drivers/mci/mci-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>> index 1d383e6449e9..57b4c5b99c9c 100644
>> --- a/drivers/mci/mci-core.c
>> +++ b/drivers/mci/mci-core.c
>> @@ -1566,7 +1566,7 @@ static int mci_startup_mmc(struct mci *mci)
>> }
>> }
>>
>> - return ret;
>> + return ret >= MMC_BUS_WIDTH_1 ? 0 : ret;
>> }
>
> Can we do it like below instead? It makes clear where a MMC_BUS_WIDTH_x
> return value is expected.
mci_mmc_select_hs_ddr() may return MMC_BUS_WIDTH_*, which isn't addressed
by your patch.
>
> Sascha
>
>
> ----------------------------------8<---------------------------
>
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 083d2f4ed1..da892a5f84 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -1558,8 +1558,12 @@ static int mci_startup_mmc(struct mci *mci)
>
> /* find out maximum bus width and then try DDR if supported */
> ret = mci_mmc_select_bus_width(mci);
> - if (ret > MMC_BUS_WIDTH_1 && mci->tran_speed == 52000000)
> - ret = mci_mmc_select_hs_ddr(mci);
> + if (ret > MMC_BUS_WIDTH_1) {
> + ret = 0;
> +
> + if (mci->tran_speed == 52000000)
> + ret = mci_mmc_select_hs_ddr(mci);
> + }
>
> if (ret < 0) {
> dev_warn(&mci->dev, "Changing MMC bus width failed: %d\n", ret);
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible
2024-04-23 6:22 [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Ahmad Fatoum
` (2 preceding siblings ...)
2024-04-23 6:44 ` [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Sascha Hauer
@ 2024-04-30 5:44 ` Sascha Hauer
3 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2024-04-30 5:44 UTC (permalink / raw)
To: barebox, Ahmad Fatoum; +Cc: str, mtr
On Tue, 23 Apr 2024 08:22:04 +0200, Ahmad Fatoum wrote:
> mci_mmc_select_hs_ddr() will try DDR52 and if that fails, revert to
> SDR operation. In that case, it returns the bus width, which would
> be forwarded as if it were an error code:
>
> WARNING: mmc2: Card's startup fails with 3
>
> Fix this by translating successful return values to 0 in
> mci_startup_mmc().
>
> [...]
Applied, thanks!
[1/3] mci: core: fix fallback when eMMC DDR52 is not possible
https://git.pengutronix.de/cgit/barebox/commit/?id=02a40f3ed0ac (link may not be stable)
[2/3] mci: core: fix fallback when host doesn't support HS200
https://git.pengutronix.de/cgit/barebox/commit/?id=ebb5a3204630 (link may not be stable)
[3/3] mci: core: make execute_tuning mandatory for HS200
https://git.pengutronix.de/cgit/barebox/commit/?id=8ca910b92188 (link may not be stable)
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-30 5:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-23 6:22 [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 2/3] mci: core: fix fallback when host doesn't support HS200 Ahmad Fatoum
2024-04-23 6:22 ` [PATCH master 3/3] mci: core: make execute_tuning mandatory for HS200 Ahmad Fatoum
2024-04-23 6:44 ` [PATCH master 1/3] mci: core: fix fallback when eMMC DDR52 is not possible Sascha Hauer
2024-04-23 6:52 ` Ahmad Fatoum
2024-04-30 5:44 ` Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox