On 12/21/22 06:09, Potthuri, Sai Krishna wrote:
Hi Marek,
Hi,
https://www.xilinx.com/htmldocs/registers/ug1087/sdio___reg_capabilit
ies.html#
Absolute Address 0x00FF160040 (SD0)
Reset Value 0x280737EC6481
really reads 0x200737EC6481 . The interesting part is the top 32
bits, which are SDHCI_CAPABILITIES_1 = 0x2007. The missing 0x800 is
SDHCI_RETUNING_TIMER_COUNT_MASK=0, which makes the SDHCI core
disable
retuning timer.
Fix this up here by explicitly setting tuning_count to 8 as it should
be, otherwise an eMMC might fail in various thermal conditions
Note that the diff is best shown with -w option, this makes it
visible what happened with !sdhci_arasan->has_cqe conditional, which
is placed between sdhci_setup_host() and __sdhci_add_host() calls.
Since sdhci_add_host() is also a sequence of these two calls and
host->tuning_count must be overriden before calling
overriden -> overridden
Fixed
__sdhci_add_host(), call the two calls separately and do all the
adjustments between them in either case.
Signed-off-by: Marek Vasut <marex@xxxxxxx>
---
Cc: Michal Simek <michal.simek@xxxxxxxxxx>
Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
To: linux-mmc@xxxxxxxxxxxxxxx
---
drivers/mmc/host/sdhci-of-arasan.c | 57 ++++++++++++++++++++---------
-
1 file changed, 38 insertions(+), 19 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c
b/drivers/mmc/host/sdhci-of-arasan.c
index 3997cad1f793d..465498f2a7c0f 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1521,37 +1521,56 @@ static int sdhci_arasan_register_sdclk(struct
sdhci_arasan_data *sdhci_arasan,
return 0;
}
-static int sdhci_arasan_add_host(struct sdhci_arasan_data
*sdhci_arasan)
+static int sdhci_arasan_add_host(struct sdhci_arasan_data
*sdhci_arasan,
+ struct device *dev)
{
struct sdhci_host *host = sdhci_arasan->host;
struct cqhci_host *cq_host;
bool dma64;
int ret;
- if (!sdhci_arasan->has_cqe)
- return sdhci_add_host(host);
-
ret = sdhci_setup_host(host);
if (ret)
return ret;
- cq_host = devm_kzalloc(host->mmc->parent,
- sizeof(*cq_host), GFP_KERNEL);
- if (!cq_host) {
- ret = -ENOMEM;
- goto cleanup;
- }
+ /*
+ * On Xilinx ZynqMP, the reg_capabilities (SDIO) Register
+ *
+ *
https://www.xilinx.com/htmldocs/registers/ug1087/sdio___reg_capabilities.
html#
+ * Absolute Address 0x00FF160040 (SD0)
+ * Reset Value 0x280737EC6481
+ *
+ * really reads 0x200737EC6481 . The interesting part is the
+ * top 32 bits, which are SDHCI_CAPABILITIES_1 = 0x2007. The
+ * missing 0x800 is SDHCI_RETUNING_TIMER_COUNT_MASK=0, which
+ * makes the SDHCI core disable retuning timer.
Are you aware that caps can be changed in DT via "sdhci-caps" and
"sdhci-caps-mask" ?
No, I wasn't aware of those.
Is that the preferred approach to this fix, over handling it in the driver ?
I think the driver-side fix would be preferable, because it also fixes systems
which use legacy DTs without the sdhci-caps properties, which would be all
ZynqMP systems thus far.
(and I would also still prefer to get feedback from Xilinx on why does the
value specified in UG1087 not match what is read out of the hardware)
Reset value of the retuning timer count is set to 0x0 via ZynqMP FSBL, we have
an ERRATA for the same.
https://support.xilinx.com/s/article/68550?language=en_US
Xilinx recommendation is to program the appropriate value in the retuning
timer count field based on the specific requirements via DT property.
Why is the retuning timer disabled for HS200 mode ?
