On 30/12/22 08:42, Marek Vasut wrote: > 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 . That is a bit sad. You might want to push for changing that situation. Send an updated patch then.
