On Feb 25, 2011, at 10:02 AM, Wolfram Sang wrote:
> Hi Philip,
>
> On Fri, Feb 25, 2011 at 09:54:35AM -0800, Philip Rakity wrote:
>> Rather then special case busy etc .. lets just use the max value.
>
> OK.
>
>>
>> Did not remove BROKEN_TIMEOUT QUIRK so existing code will compile
>> we can remove this once existing platform drivers delete usage and get
>> quirk back.
>
> If we wait for that, we'll probably wait till eternity ;) I'd vote that
> removing the quirk should be part of the patch.
I concur (see below)
>
>>
>> Patch starts after ====
>> =====
>
> The usual nomenclature is that such comments simply go between '---' and the
> diffstat. Most tools are prepared for this...
>
>> The card/host controller may sometimes return a value that is
>> too low and cause the h/w to timeout a transfer that would have
>> worked. Using the maximum value avoids this.
>>
>> Signed-off-by: Philip Rakity <prak...@marvell.com>
>> ---
>
> ... to handle them here.
>
>> drivers/mmc/host/sdhci.c | 48 ++++-----------------------------------------
>> 1 files changed, 5 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 655617c..dd99da8 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -592,53 +592,15 @@ static void sdhci_adma_table_post(struct sdhci_host
>> *host,
>> data->sg_len, direction);
>> }
>>
>> -static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_data *data)
>> +static inline u8 sdhci_calc_timeout(void)
>> {
>> - u8 count;
>> - unsigned target_timeout, current_timeout;
>> -
>> /*
>> - * If the host controller provides us with an incorrect timeout
>> - * value, just skip the check and use 0xE. The hardware may take
>> + * The host controller/card can provide us with an incorrect timeout
>> + * value, just use the maximum value 0xE. The hardware may take
>> * longer to time out, but that's much better than having a too-short
>> * timeout value.
>> */
>> - if (host->quirks & SDHCI_QUIRK_BROKEN_TIMEOUT_VAL)
>> - return 0xE;
>> -
>> - /* timeout in us */
>> - target_timeout = data->timeout_ns / 1000 +
>> - data->timeout_clks / host->clock;
>> -
>> - if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)
>> - host->timeout_clk = host->clock / 1000;
>
> This quirk could go then as well?
I am all for that -- did not want to touch other drivers but will remove for
sdhci-pxa if
patch is okay.
>
>> -
>> - /*
>> - * Figure out needed cycles.
>> - * We do this in steps in order to fit inside a 32 bit int.
>> - * The first step is the minimum timeout, which will have a
>> - * minimum resolution of 6 bits:
>> - * (1) 2^13*1000 > 2^22,
>> - * (2) host->timeout_clk < 2^16
>> - * =>
>> - * (1) / (2) > 2^6
>> - */
>> - count = 0;
>> - current_timeout = (1 << 13) * 1000 / host->timeout_clk;
>> - while (current_timeout < target_timeout) {
>> - count++;
>> - current_timeout <<= 1;
>> - if (count >= 0xF)
>> - break;
>> - }
>> -
>> - if (count >= 0xF) {
>> - printk(KERN_WARNING "%s: Too large timeout requested!\n",
>> - mmc_hostname(host->mmc));
>> - count = 0xE;
>> - }
>> -
>> - return count;
>> + return 0xE;
>
> Why don't you remove the function entirely?
better to rename it --- to set_maximum_timeout since a little clearer.
left the old name for historical reasons -- if no need I will change it
>
>> }
>>
>> static void sdhci_set_transfer_irqs(struct sdhci_host *host)
>> @@ -671,7 +633,7 @@ static void sdhci_prepare_data(struct sdhci_host *host,
>> struct mmc_data *data)
>> host->data = data;
>> host->data_early = 0;
>>
>> - count = sdhci_calc_timeout(host, data);
>> + count = sdhci_calc_timeout();
>> sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
>>
>> if (host->flags & (SDHCI_USE_SDMA | SDHCI_USE_ADMA))
>
> Thanks,
>
> Wolfram
>
> --
> Pengutronix e.K. | Wolfram Sang |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
