On Fri, 2021-09-24 at 13:07 +0300, Adrian Hunter wrote: > On 24/09/21 12:17 pm, Bean Huo wrote: > > On Fri, 2021-09-24 at 08:29 +0300, Adrian Hunter wrote: > > > > If the data transmission timeout value required by the device > > > > exceeds > > > > the maximum timeout value of the host HW timer, we still use > > > > the HW > > > > timer with the maximum timeout value of the HW timer. This > > > > setting > > > > is > > > > suitable for most R/W situations. But sometimes, the device > > > > will > > > > complete > > > > the R/W task within its required timeout value (greater than > > > > the HW > > > > timer). > > > > In this case, the HW timer for data transmission will time out. > > > > Currently, in this condition, we disable the HW timer and use > > > > the > > > > SW > > > > timer only when the SDHCI_QUIRK2_DISABLE_HW_TIMEOUT quirk is > > > > set by > > > > the > > > > host driver. The patch is to remove this if statement > > > > restriction > > > > and > > > > allow data transmission to use the SW timer when the hardware > > > > timer > > > > cannot > > > > meet the required timeout value. > > > > > > The reason it is a quirk is because it does not work for all > > > hardware. > > > > > > For some controllers the timeout cannot really be disabled, only > > > the > > > > > > interrupt is disabled, and then the controller never indicates > > > completion > > > > > > if the timeout is exceeded. > > > > Hi Adrian, > > Thanks for your review. > > > > Yes, you are right. But this quirk prevents disabling the hardware > > timeoutIRQ. The purpose of this patch is to disable the hardware > > timeout IRQ and > > select the software timeout. > > > > void __sdhci_set_timeout(struct sdhci_host *host, struct > > mmc_command > > *cmd) > > { > > bool too_big = false; > > u8 count = sdhci_calc_timeout(host, cmd, &too_big); > > > > if (too_big) { > > sdhci_calc_sw_timeout(host, cmd); > > sdhci_set_data_timeout_irq(host, false); // disable > > IRQ > > } else if (!(host->ier & SDHCI_INT_DATA_TIMEOUT)) { > > sdhci_set_data_timeout_irq(host, true); > > } > > > > sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL); > > } > > > > > > The driver has detected that the hardware timer cannot meet the > > timeout > > requirements of the device, but we still use the hardware timer, > > which will > > allow potential timeout issuea . Rather than allowing a potential > > problem to exist, why can’t software timing be used to avoid this > > problem? > > Timeouts aren't that accurate. The maximum is assumed still to work. > mmc->max_busy_timeout is used to tell the core what the maximum is. > mmc->max_busy_timeout is still a representation of Host HW timer maximum timeout count, isn't it? Bean