On 12/29/22 13:51, Adrian Hunter wrote:
On 26/10/22 12:20, Marek Vasut wrote:
On 10/26/22 08:07, Adrian Hunter wrote:
On 25/10/22 22:15, Marek Vasut wrote:
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.
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 guess ideally. Mainline does not really need the driver
fix because it seems it can be done by DT. Older kernels
are a separate issue really.
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.
You could backport support of the properties "sdhci-caps"
and "sdhci-caps-mask".
This won't help. Vivado (the xilinx FPGA design tool) is capable of
generating DTs, so you can end up with a combination of new Linux kernel
and old generated DT, which is still missing the
sdhci-caps/sdhci-caps-mask .
