Re: [PATCH v3] mmc: dw_mmc: Allow lower TMOUT value than maximum

[Marten Lindahl <martenli@axis.com>]

On Mon, Nov 15, 2021 at 03:54:19PM +0100, Ulf Hansson wrote:
> On Wed, 10 Nov 2021 at 12:05, Mårten Lindahl <marten.lindahl@axis.com> 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@axis.com>
> 
> Applied for next, thanks!
> 
> Kind regards
> Uffe
>

Hi Ulf!

During tests on some units I found that there is a potential overflow
of the 32bit divider in this patch (if clk_div > 4).

Is it possible to stop this patch, until I created a new version of it?
I guess it's better to update this patch, in favour of sending a new
correction patch for it?

Kind regards
Mårten

> 
> > ---
> >
> > 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);
> > +}
> > +
> >  static void __dw_mci_start_request(struct dw_mci *host,
> >                                    struct dw_mci_slot *slot,
> >                                    struct mmc_command *cmd)
> > @@ -1303,7 +1329,7 @@ static void __dw_mci_start_request(struct dw_mci *host,
> >
> >         data = cmd->data;
> >         if (data) {
> > -               mci_writel(host, TMOUT, 0xFFFFFFFF);
> > +               dw_mci_set_data_timeout(host, data->timeout_ns);
> >                 mci_writel(host, BYTCNT, data->blksz*data->blocks);
> >                 mci_writel(host, BLKSIZ, data->blksz);
> >         }
> > --
> > 2.20.1
> >