Hi,
On Wed, Nov 3, 2021 at 8:24 AM Mårten Lindahl <marten.lindahl@xxxxxxxx> wrote:
>
> The TMOUT register is always set with a full value for every transfer,
> which (with a 200MHz clock) will give a full DRTO of ~84 milliseconds.
> Since the software dto_timer acts as a backup in cases when this timeout
> is not long enough, it is normally not a problem. But setting a full
> value makes it impossible to test shorter timeouts, when for example
> testing data read times on different SD cards.
>
> Add a function to set any value smaller than the maximum of 0xFFFFFF.
>
> Signed-off-by: Mårten Lindahl <marten.lindahl@xxxxxxxx>
> ---
> drivers/mmc/host/dw_mmc.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 6578cc64ae9e..0d23b8ed9403 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -54,6 +54,7 @@
>
> #define DW_MCI_FREQ_MAX 200000000 /* unit: HZ */
> #define DW_MCI_FREQ_MIN 100000 /* unit: HZ */
> +#define DW_MCI_DATA_TMOUT_NS_MAX 83886075
>
> #define IDMAC_INT_CLR (SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
> SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
> @@ -1283,6 +1284,32 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
> mci_writel(host, CTYPE, (slot->ctype << slot->id));
> }
>
> +static void dw_mci_set_data_timeout(struct dw_mci *host, u32 timeout_ns)
The type of "timeout_ns" should match `struct mmc_data`, which is
unsigned int, not u32.
> +{
> + u32 timeout, freq_mhz, tmp, tmout;
> +
> + if (!timeout_ns || timeout_ns > DW_MCI_DATA_TMOUT_NS_MAX) {
> + /* Set maximum */
> + tmout = 0xFFFFFFFF;
> + goto tmout_done;
> + }
I don't think that the above is right. If the card clock is 50 Hz
instead of 200 Hz then 0xffffffff is actually ~83 ms * 4 = ~332 ms. It
would be better to attempt to program it correctly.
Can you just do the math below and if the number is greater than can
be represented then you can just put in the max?
Interestingly enough, in `struct mmc_data` this is documented as a max
of 80 ms, though I don't think your code should care about that. Just
cap to the maximum value after your math.
> + timeout = timeout_ns;
> + freq_mhz = DIV_ROUND_UP(host->bus_hz, NSEC_PER_MSEC);
> +
> + /* TMOUT[7:0] (RESPONSE_TIMEOUT) */
> + tmout = 0xFF; /* Set maximum */
> +
> + /* TMOUT[31:8] (DATA_TIMEOUT) */
> + tmp = DIV_ROUND_UP_ULL((u64)timeout * freq_mhz, MSEC_PER_SEC);
> + tmout |= (tmp & 0xFFFFFF) << 8;
Combining your two calculations, I guess you have:
tmp = timeout * (bus_hz / 1000000) / 1000
Why isn't this just:
tmp = (timeout * bus_hz) / 1000000000
Since you're doing 64-bit math anyway I don't think you need to worry
about that calculation overflowing. Multiplying two 32-bit numbers
can't exceed 64-bits, right?
Also: I think "bus_hz" is the wrong thing to be using here. You need
to take CLKDIV into account like dw_mci_set_drto() does.
-Doug
