Re: [RFC][PATCH] MMC: Use write timeout value as read from CSR

[Pierre Ossman]

On Wed, 24 Sep 2008 12:16:02 +0100
"Matt Fleming" <mattjfleming@xxxxxxxxxxxxxx> wrote:

> 2008/9/20 Pierre Ossman <drzeus-mmc@xxxxxxxxx>:
> >
> > It shouldn't take that long to write the more proper solution, so get
> > to it and it should be possible to get in even for .27.
> >
> 
> OK, attached is the latest attempt at this patch. Because a lot of
> host drivers manipulate timeout_ns and timeout_clks I decided not
> touch them at all. So, I created a new member of the mmc_data struct
> that has the timeout value as a ktime_t.

Hmm... What do you mean manipulate? They all just read it, so I don't
see a need for a new member.

> I'm still unsure of how
> exactly to tackle the mmc_send_cid and mmc_send_csd() cases, the
> timeout value used in the spec is Ncr, where can I find this value?

Right, they left that out of the simplified spec. Sneaky bastards. It's
defined as 64 clock cycles.

> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 044d84e..b5c6f5f 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -249,8 +249,10 @@ void mmc_set_data_timeout(struct mmc_data *data,
> const struct mmc_card *card)
>          * SDIO cards only define an upper 1 s limit on access.
>          */
>         if (mmc_card_sdio(card)) {
> -               data->timeout_ns = 1000000000;
> -               data->timeout_clks = 0;
> +               if (data->timeout_ns > 1000000000) {
> +                       data->timeout_ns = 1000000000;
> +                       data->timeout_clks = 0;
> +               }
>                 return;
>         }
> 

data->timeout_* is undefined when this function is invoked, so this
code is wrong. It is also unnecessary, so just leave it out.

> @@ -269,6 +271,11 @@ void mmc_set_data_timeout(struct mmc_data *data,
> const struct mmc_card *card)
>         data->timeout_ns = card->csd.tacc_ns * mult;
>         data->timeout_clks = card->csd.tacc_clks * mult;
> 
> +       data->ktimeout = ktime_set(0, 0);
> +       data->ktimeout = ktime_add_ns(data->ktimeout, data->timeout_ns);
> +       data->ktimeout = ktime_add_ns(data->ktimeout,
> +               data->timeout_clks * 1000000 / card->host->ios.clock);
> +
>         /*
>          * SD cards also have an upper limit on the timeout.
>          */

card->host->ios.clock is just the upper bound on the clock, it might be
running slower. This is why the host driver needs to calculate it (well
that, and the fact that different controllers treat timeouts in
different ways so it's not one-size-fits-all).

> @@ -832,9 +832,11 @@ mmc_spi_data_do(struct mmc_spi_host *host, struct
> mmc_command *cmd,
>                                 t->len);
> 
>                         if (direction == DMA_TO_DEVICE)
> -                               status = mmc_spi_writeblock(host, t);
> +                               status = mmc_spi_writeblock(host, t,
> +                                               data->ktimeout);
>                         else
> -                               status = mmc_spi_readblock(host, t);
> +                               status = mmc_spi_readblock(host, t,
> +                                               data->ktimeout);
>                         if (status < 0)
>                                 break;
> 

If you put the calculation somewhere before this chunk instead,
everything should be peachy.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/