| Message ID | 20200729234130.25056-1-faiz_abbas@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci_am654: Add workaround for card detect debounce timer | expand |
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? > + ktime_t timeout = ktime_add_ms(ktime_get(), > + MAX_POWER_ON_TIMEOUT); > + do { > + if (ktime_compare(ktime_get(), timeout) > 0) { > + dev_warn(mmc_dev(host->mmc), > + "Power on failed\n"); > + > + return; > + } > + > + writeb(val, host->ioaddr + reg); > + usleep_range(1000, 10000); > + } while (!(readb(host->ioaddr + reg) & SDHCI_POWER_ON)); > + } > } > > static int sdhci_am654_execute_tuning(struct mmc_host *mmc, u32 opcode) >
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. Thanks, Faiz
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 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; } } }
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/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 + */ + ktime_t timeout = ktime_add_ms(ktime_get(), + MAX_POWER_ON_TIMEOUT); + do { + if (ktime_compare(ktime_get(), timeout) > 0) { + dev_warn(mmc_dev(host->mmc), + "Power on failed\n"); + + return; + } + + writeb(val, host->ioaddr + reg); + usleep_range(1000, 10000); + } while (!(readb(host->ioaddr + reg) & SDHCI_POWER_ON)); + } } static int sdhci_am654_execute_tuning(struct mmc_host *mmc, u32 opcode)
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(+)