Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <beda2d5ecc3c15e9bf9aa18383c22c2a90d31dab.camel@gmail.com>
Subject: Re: [PATCH v1 2/2] mmc: sdhci: Use the SW timer when the HW timer
 cannot meet the timeout value required by the device
From:   Bean Huo <huobean@gmail.com>
To:     Adrian Hunter <adrian.hunter@intel.com>,
        Ulf Hansson <ulf.hansson@linaro.org>
Cc:     Bean Huo <beanhuo@micron.com>, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Fri, 24 Sep 2021 11:17:05 +0200
In-Reply-To: <fc14d8e1-9438-d4b0-80f4-ccf9055ab7d3@intel.com>
References: <20210917172727.26834-1-huobean@gmail.com>
         <20210917172727.26834-3-huobean@gmail.com>
         <fc14d8e1-9438-d4b0-80f4-ccf9055ab7d3@intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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?


Kind regards,
Bean