diff for duplicates of <3f7e85c7-b0ba-e6a8-8d9f-091413614773@ti.com> diff --git a/a/1.txt b/N1/1.txt index 8b13789..7981eb0 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1 +1,109 @@ +Hi Adrian, +On 05/08/20 3:16 pm, Adrian Hunter wrote: +> On 5/08/20 11:22 am, Faiz Abbas wrote: +>> Hi Adrian, +>> +>> On 05/08/20 1:44 pm, Adrian Hunter wrote: +>>> On 30/07/20 2:41 am, Faiz Abbas wrote: +>>>> There is a one time delay because of a card detect debounce timer in the +>>>> controller IP. This timer runs as soon as power is applied to the module +>>>> regardless of whether a card is present or not and any writes to +>>>> SDHCI_POWER_ON will return 0 before it expires. This timeout has been +>>>> measured to be about 1 second in am654x and j721e. +>>>> +>>>> Write-and-read-back in a loop on SDHCI_POWER_ON for a maximum of +>>>> 1.5 seconds to make sure that the controller actually powers on. +>>>> +>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com> +>>>> --- +>>>> drivers/mmc/host/sdhci_am654.c | 21 +++++++++++++++++++++ +>>>> 1 file changed, 21 insertions(+) +>>>> +>>>> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c +>>>> index 1718b9e8af63..55cff9de2f3e 100644 +>>>> --- a/drivers/mmc/host/sdhci_am654.c +>>>> +++ b/drivers/mmc/host/sdhci_am654.c +>>>> @@ -272,6 +272,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host, +>>>> sdhci_set_clock(host, clock); +>>>> } +>>>> +>>>> +#define MAX_POWER_ON_TIMEOUT 1500 /* ms */ +>>>> static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg) +>>>> { +>>>> unsigned char timing = host->mmc->ios.timing; +>>>> @@ -291,6 +292,26 @@ static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg) +>>>> } +>>>> +>>>> writeb(val, host->ioaddr + reg); +>>>> + if (reg == SDHCI_POWER_CONTROL && (val & SDHCI_POWER_ON)) { +>>>> + /* +>>>> + * Power on will not happen until the card detect debounce +>>>> + * timer expires. Wait at least 1.5 seconds for the power on +>>>> + * bit to be set +>>>> + */ +>>> +>>> Can you use readb_poll_timeout() here? +>>> +>> +>> The loop is write -> readback -> check for set bit -> write again and so on until timeout +>> so poll_timeout() calls will not work. +> +> I mentioned it because pedantically you need to check the condition +> again after a timeout. Alternatively, the read_poll_timeout macro + +Ideally, the timeout will never happen because the internal timer always expires at +1 second. The actual time spent in the loop can be anything less than 1 second +depending on when clocks were enabled + +> can be used with a function, something like below (not even compile +> tested!) +> +> static u8 write_power_on(struct sdhci_host *host, u8 val, int reg) +> { +> writeb(val, host->ioaddr + reg); +> usleep_range(1000, 10000); +> return readb(host->ioaddr + reg); +> } +> +> #define MAX_POWER_ON_TIMEOUT 1500000 /* us */ +> static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg) +> { +> unsigned char timing = host->mmc->ios.timing; +> u8 pwr; +> +> if (reg == SDHCI_HOST_CONTROL) { +> switch (timing) { +> /* +> * According to the data manual, HISPD bit +> * should not be set in these speed modes. +> */ +> case MMC_TIMING_SD_HS: +> case MMC_TIMING_MMC_HS: +> case MMC_TIMING_UHS_SDR12: +> case MMC_TIMING_UHS_SDR25: +> val &= ~SDHCI_CTRL_HISPD; +> } +> } +> +> writeb(val, host->ioaddr + reg); +> +> /* +> * Power on will not happen until the card detect debounce +> * timer expires. Wait at least 1.5 seconds for the power on +> * bit to be set +> */ +> if (reg == SDHCI_POWER_CONTROL && (val & SDHCI_POWER_ON) && +> read_poll_timeout(write_power_on, pwr, (pwr & SDHCI_POWER_ON), 0, +> MAX_POWER_ON_TIMEOUT, false, host, val, reg)) +> dev_warn(mmc_dev(host->mmc), "Power on failed\n"); +> return; +> } +> } +> } +> + +Looks good. Let me add this in v2. + +Thanks, +Faiz diff --git a/a/content_digest b/N1/content_digest index 0f76f60..02b3037 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -11,5 +11,114 @@ "Cc\0<ulf.hansson@linaro.org>\0" "\00:1\0" "b\0" + "Hi Adrian,\n" + "\n" + "On 05/08/20 3:16 pm, Adrian Hunter wrote:\n" + "> On 5/08/20 11:22 am, Faiz Abbas wrote:\n" + ">> Hi Adrian,\n" + ">>\n" + ">> On 05/08/20 1:44 pm, Adrian Hunter wrote:\n" + ">>> On 30/07/20 2:41 am, Faiz Abbas wrote:\n" + ">>>> There is a one time delay because of a card detect debounce timer in the\n" + ">>>> controller IP. This timer runs as soon as power is applied to the module\n" + ">>>> regardless of whether a card is present or not and any writes to\n" + ">>>> SDHCI_POWER_ON will return 0 before it expires. This timeout has been\n" + ">>>> measured to be about 1 second in am654x and j721e.\n" + ">>>>\n" + ">>>> Write-and-read-back in a loop on SDHCI_POWER_ON for a maximum of\n" + ">>>> 1.5 seconds to make sure that the controller actually powers on.\n" + ">>>>\n" + ">>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>\n" + ">>>> ---\n" + ">>>> drivers/mmc/host/sdhci_am654.c | 21 +++++++++++++++++++++\n" + ">>>> 1 file changed, 21 insertions(+)\n" + ">>>>\n" + ">>>> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c\n" + ">>>> index 1718b9e8af63..55cff9de2f3e 100644\n" + ">>>> --- a/drivers/mmc/host/sdhci_am654.c\n" + ">>>> +++ b/drivers/mmc/host/sdhci_am654.c\n" + ">>>> @@ -272,6 +272,7 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,\n" + ">>>> \tsdhci_set_clock(host, clock);\n" + ">>>> }\n" + ">>>> \n" + ">>>> +#define MAX_POWER_ON_TIMEOUT\t1500 /* ms */\n" + ">>>> static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)\n" + ">>>> {\n" + ">>>> \tunsigned char timing = host->mmc->ios.timing;\n" + ">>>> @@ -291,6 +292,26 @@ static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)\n" + ">>>> \t}\n" + ">>>> \n" + ">>>> \twriteb(val, host->ioaddr + reg);\n" + ">>>> +\tif (reg == SDHCI_POWER_CONTROL && (val & SDHCI_POWER_ON)) {\n" + ">>>> +\t\t/*\n" + ">>>> +\t\t * Power on will not happen until the card detect debounce\n" + ">>>> +\t\t * timer expires. Wait at least 1.5 seconds for the power on\n" + ">>>> +\t\t * bit to be set\n" + ">>>> +\t\t */\n" + ">>>\n" + ">>> Can you use readb_poll_timeout() here?\n" + ">>>\n" + ">>\n" + ">> The loop is write -> readback -> check for set bit -> write again and so on until timeout\n" + ">> so poll_timeout() calls will not work.\n" + "> \n" + "> I mentioned it because pedantically you need to check the condition\n" + "> again after a timeout. Alternatively, the read_poll_timeout macro\n" + "\n" + "Ideally, the timeout will never happen because the internal timer always expires at\n" + "1 second. The actual time spent in the loop can be anything less than 1 second\n" + "depending on when clocks were enabled\n" + "\n" + "> can be used with a function, something like below (not even compile\n" + "> tested!)\n" + "> \n" + "> static u8 write_power_on(struct sdhci_host *host, u8 val, int reg)\n" + "> {\n" + "> \twriteb(val, host->ioaddr + reg);\n" + "> \tusleep_range(1000, 10000);\n" + "> \treturn readb(host->ioaddr + reg);\n" + "> }\n" + "> \n" + "> #define MAX_POWER_ON_TIMEOUT\t1500000 /* us */\n" + "> static void sdhci_am654_write_b(struct sdhci_host *host, u8 val, int reg)\n" + "> {\n" + "> \tunsigned char timing = host->mmc->ios.timing;\n" + "> \tu8 pwr;\n" + "> \n" + "> \tif (reg == SDHCI_HOST_CONTROL) {\n" + "> \t\tswitch (timing) {\n" + "> \t\t/*\n" + "> \t\t * According to the data manual, HISPD bit\n" + "> \t\t * should not be set in these speed modes.\n" + "> \t\t */\n" + "> \t\tcase MMC_TIMING_SD_HS:\n" + "> \t\tcase MMC_TIMING_MMC_HS:\n" + "> \t\tcase MMC_TIMING_UHS_SDR12:\n" + "> \t\tcase MMC_TIMING_UHS_SDR25:\n" + "> \t\t\tval &= ~SDHCI_CTRL_HISPD;\n" + "> \t\t}\n" + "> \t}\n" + "> \n" + "> \twriteb(val, host->ioaddr + reg);\n" + "> \n" + "> \t/*\n" + "> \t * Power on will not happen until the card detect debounce\n" + "> \t * timer expires. Wait at least 1.5 seconds for the power on\n" + "> \t * bit to be set\n" + "> \t */\n" + "> \tif (reg == SDHCI_POWER_CONTROL && (val & SDHCI_POWER_ON) &&\n" + "> \t read_poll_timeout(write_power_on, pwr, (pwr & SDHCI_POWER_ON), 0,\n" + "> \t\t\t MAX_POWER_ON_TIMEOUT, false, host, val, reg))\n" + "> \t\t\tdev_warn(mmc_dev(host->mmc), \"Power on failed\\n\");\n" + "> \t\t\treturn;\n" + "> \t\t}\n" + "> \t}\n" + "> }\n" + "> \n" + "\n" + "Looks good. Let me add this in v2.\n" + "\n" + "Thanks,\n" + Faiz -92b9c285fee8eaf6d31df8f70b8b4a3cc11850ed4ed3b8ad0dd0bd04654182c1 +70bd58da1e2d2b5eba710018d6fc9ef6dbfd3ff0d53f5d5931133c4fdcc70f82
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox