Hi,
On Wed, Nov 10, 2021 at 3:05 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.
I'm not 100% sure about what the part about "the software dto_timer
acts as a backup in cases when this timeout is not long enough" means.
At the moment the software fallback should be set to the same as the
hardware value, right? So the software backup doesn't help make the
timeout longer...
> 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>
> ---
>
> v2:
> - Calculate new value before checking boundaries
> - Include CLKDIV register to get proper value
>
> v3:
> - Use 'if-else' instead of 'goto'
> - Don't touch response field when maximize data field
>
> drivers/mmc/host/dw_mmc.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 6578cc64ae9e..637ae2aea9fa 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1283,6 +1283,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,
> + unsigned int timeout_ns)
> +{
> + unsigned int clk_div, tmp, tmout;
> +
> + clk_div = (mci_readl(host, CLKDIV) & 0xFF) * 2;
> + if (clk_div == 0)
> + clk_div = 1;
> +
> + tmp = DIV_ROUND_UP_ULL((u64)timeout_ns * host->bus_hz,
> + NSEC_PER_SEC * clk_div);
> +
> + /* TMOUT[7:0] (RESPONSE_TIMEOUT) */
> + tmout = 0xFF; /* Set maximum */
> +
> + /* TMOUT[31:8] (DATA_TIMEOUT) */
> + if (!tmp || tmp > 0xFFFFFF)
> + tmout |= (0xFFFFFF << 8);
> + else
> + tmout |= (tmp & 0xFFFFFF) << 8;
> +
> + mci_writel(host, TMOUT, tmout);
> + dev_dbg(host->dev, "timeout_ns: %u => TMOUT[31:8]: 0x%06x",
> + timeout_ns, tmout >> 8);
nit: In theory 0x%06x is slightly better written as %#08x. I wouldn't
spin the patch just for that, though.
In any case, I think this is fine to the best of my knowledge of how
this timeout is supposed to work, so I'm happy with:
Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
