On 14/06/16 07:20, Adrian Hunter wrote: > On 13/06/16 13:22, Jon Hunter wrote: >> Adding Adrian and Ulf ... >> >> On 19/05/16 15:29, Jon Hunter wrote: >>> >>> On 13/05/16 18:27, Thierry Reding wrote: >>>> * PGP Signed by an unknown key >>>> >>>> On Fri, May 13, 2016 at 09:25:31AM +0200, Lucas Stach wrote: >>>>> Am Montag, den 29.02.2016, 22:01 +0100 schrieb Lucas Stach: >>>>>> This allows to switch the card signal voltage level to 1.8V, >>>>>> which is needed for any ultra high speed modes to work. >>>>>> >>>>>> Signed-off-by: Lucas Stach <dev@xxxxxxxxxx> >>>>>> --- >>>>>> This needs the SDMMC memcomp pad calibration patches I just >>>>>> sent out to be applied, otherwise the card voltage change will >>>>>> fail with a message in the kernel log and a fall back to >>>>>> high speed operation. >>>>> >>>>> The patches this one depends on have been applied for some time now. >>>>> Please pick up this patch. >>>> >>>> My understanding is that UHS modes currently cause problems on Beaver. >>>> What I don't understand about that is how it will even try those modes >>>> if the voltage regulator can't be set to 1.8 V? Shouldn't that actively >>>> prevent those modes from even being attempted? >>> >>> Looking at the sdhci code, if the regulator is missing then we still >>> attempt to place the controller is 1.8V mode ... >>> >>> static int sdhci_start_signal_voltage_switch(struct mmc_host *mmc, >>> struct mmc_ios *ios) >>> { >>> >>> ... >>> >>> case MMC_SIGNAL_VOLTAGE_180: >>> if (!IS_ERR(mmc->supply.vqmmc)) { >>> ret = regulator_set_voltage(mmc->supply.vqmmc, >>> 1700000, 1950000); >>> if (ret) { >>> pr_warn("%s: Switching to 1.8V signalling voltage failed\n", >>> mmc_hostname(mmc)); >>> return -EIO; >>> } >>> } >>> >>> /* >>> * Enable 1.8V Signal Enable in the Host Control2 >>> * register >>> */ >>> ctrl |= SDHCI_CTRL_VDD_180; >>> sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); >>> >>> /* Some controller need to do more when switching */ >>> if (host->ops->voltage_switch) >>> host->ops->voltage_switch(host); >>> >>> /* 1.8V regulator output should be stable within 5 ms */ >>> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2); >>> if (ctrl & SDHCI_CTRL_VDD_180) >>> return 0; >>> >>> pr_warn("%s: 1.8V regulator output did not became stable\n", >>> mmc_hostname(mmc)); >>> >>> return -EAGAIN; >>> >>> Ideally, the above *should* fail if the regulator is missing. However, what >>> I have found, is that in my case, even though the regulator is missing, the >>> above succeeds and the host thinks we are operating at 1.8V even though we >>> are still at 3.3V! It seems that this does not happen with all SD cards that >>> support UHS. >>> >>> This patch resolves the problems I am seeing on beaver with SD card >>> initialisation failing. I am surprised this is not causing problems for >>> others? >> >> Adrian, Ulf, per the above, I have found that on a Tegra30 beaver board, >> if we enable UHS-I modes for Tegra30 but the device-tree for the board >> is missing the regulator to select 1.8V mode operation, then the above >> code sequence may still return success (ie. SDHCI_CTRL_VDD_180 bit is >> set in SDHCI_HOST_CONTROL2) even though we have not changed the voltage. >> This leads to other problems later on during SD initialisation. >> >> Would you expect that an SDHCI controller should fail to set the >> SDHCI_CTRL_VDD_180 bit in the SDHCI_HOST_CONTROL2 register if we did not >> change the voltage? > > What is meant to happen is that sdhci should wait 5ms and then check > SDHCI_CTRL_VDD_180 - which it used to do but then someone took the 5ms wait > away. Do you plan to add the 5ms delay again? > In any case, if you are using a regulator there is no knowing what sdhci is > meant to do. Ok, seems fragile. >> >> We want to ensure that Tegra devices do not attempt to switch the UHS-I >> modes if the regulator is not present and it is not clear to me if there >> is a problem with the Tegra SDHCI controller or how this should be handled. > > If the driver doesn't support UHS-I modes then it must remove the cap flags. So the controller itself supports UHS-I modes, but a given board may not have the regulator to support them. We need a way to determine if the board can support the UHS-I modes. Now we could check to see if the regulator is present in the Tegra SDHCI driver and if not remove the cap flags. However, I was not sure if this is applicable to other sdhci controllers and so there should be a generic solution for this? Cheers Jon -- nvpublic -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html