Hi all,
From the previous discussion, I do not think we have got a clear conclusion
about using maximum timeout value. At least we know from Jae hoon Chung
using 0xE for every case is not a good. So I want to suggest only use 0xE for
busy command. I personally preferred below implementation, which is similar
with a RFC patch submitted by Jae hoon Chung, but only without adding a new
quirk.
I think sdhci_calc_timeout should be left for data transfer since at least we
can get a warning if 0xE is not enough for host to use. And if the host
controller and the card have no bugs, then the calculated timeout should be
safe. Left the old implementation unchanged is also compatible with all
existed host controllers and cards.
But for busy command, we are not clear about how long is safe enough for
waiting and there is also no function to do the calculation for them. So
preferred just using 0xE. Below the patch and comment:
Set the timeout control register for SDHCI host when send some commands which
need busy signal. Use the maximum timeout value 0xE will be safe.
Signed-off-by: Chuanxiao Dong <[email protected]>
---
drivers/mmc/host/sdhci.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 99c372e..8306323 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -659,8 +659,15 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
WARN_ON(host->data);
- if (data == NULL)
+ if (data == NULL) {
+ /*
+ * set the timeout to be maximum value for commands those with
+ * busy signal
+ */
+ if (host->cmd->flags & MMC_RSP_BUSY)
+ sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
return;
+ }
/* Sanity checks */
BUG_ON(data->blksz * data->blocks > 524288);
--
1.6.6.1
Chuanxiao Dong wrote:
> Hi all,
> From the previous discussion, I do not think we have got a clear conclusion
> about using maximum timeout value. At least we know from Jae hoon Chung
> using 0xE for every case is not a good. So I want to suggest only use 0xE for
> busy command. I personally preferred below implementation, which is similar
> with a RFC patch submitted by Jae hoon Chung, but only without adding a new
> quirk.
thanks for remind.
Yes, i tested without quirks, i think that is not problem.
(Just sent RFC patch with quirks, because i want to ask how think about adding quirks or not).
>
> I think sdhci_calc_timeout should be left for data transfer since at least we
> can get a warning if 0xE is not enough for host to use. And if the host
> controller and the card have no bugs, then the calculated timeout should be
> safe. Left the old implementation unchanged is also compatible with all
> existed host controllers and cards.
>
> But for busy command, we are not clear about how long is safe enough for
> waiting and there is also no function to do the calculation for them. So
> preferred just using 0xE. Below the patch and comment:
>
> Set the timeout control register for SDHCI host when send some commands which
> need busy signal. Use the maximum timeout value 0xE will be safe.
>
> Signed-off-by: Chuanxiao Dong <[email protected]>
> ---
> drivers/mmc/host/sdhci.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 99c372e..8306323 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -659,8 +659,15 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
>
> WARN_ON(host->data);
>
> - if (data == NULL)
> + if (data == NULL) {
> + /*
> + * set the timeout to be maximum value for commands those with
> + * busy signal
> + */
> + if (host->cmd->flags & MMC_RSP_BUSY)
> + sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
> return;
> + }
>
> /* Sanity checks */
> BUG_ON(data->blksz * data->blocks > 524288);
On Mar 9, 2011, at 6:29 PM, Jaehoon Chung wrote:
> Chuanxiao Dong wrote:
>> Hi all,
>> From the previous discussion, I do not think we have got a clear conclusion
>> about using maximum timeout value. At least we know from Jae hoon Chung
>> using 0xE for every case is not a good. So I want to suggest only use 0xE for
>> busy command. I personally preferred below implementation, which is similar
>> with a RFC patch submitted by Jae hoon Chung, but only without adding a new
>> quirk.
>
> thanks for remind.
> Yes, i tested without quirks, i think that is not problem.
> (Just sent RFC patch with quirks, because i want to ask how think about adding quirks or not).
Sorry I am confused.
Setting 0x0E all the time does not solve the problem and has side effects ?
What are the side effects ?
Using BUSY patch for 0x0e (below) works ?
>
>>
>> I think sdhci_calc_timeout should be left for data transfer since at least we
>> can get a warning if 0xE is not enough for host to use. And if the host
>> controller and the card have no bugs, then the calculated timeout should be
>> safe. Left the old implementation unchanged is also compatible with all
>> existed host controllers and cards.
>>
>> But for busy command, we are not clear about how long is safe enough for
>> waiting and there is also no function to do the calculation for them. So
>> preferred just using 0xE. Below the patch and comment:
>>
>> Set the timeout control register for SDHCI host when send some commands which
>> need busy signal. Use the maximum timeout value 0xE will be safe.
>>
>> Signed-off-by: Chuanxiao Dong <[email protected]>
>> ---
>> drivers/mmc/host/sdhci.c | 9 ++++++++-
>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 99c372e..8306323 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -659,8 +659,15 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
>>
>> WARN_ON(host->data);
>>
>> - if (data == NULL)
>> + if (data == NULL) {
>> + /*
>> + * set the timeout to be maximum value for commands those with
>> + * busy signal
>> + */
>> + if (host->cmd->flags & MMC_RSP_BUSY)
>> + sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
>> return;
>> + }
>>
>> /* Sanity checks */
>> BUG_ON(data->blksz * data->blocks > 524288);
>
Philip Rakity wrote:
> On Mar 9, 2011, at 6:29 PM, Jaehoon Chung wrote:
>
>> Chuanxiao Dong wrote:
>>> Hi all,
>>> From the previous discussion, I do not think we have got a clear conclusion
>>> about using maximum timeout value. At least we know from Jae hoon Chung
>>> using 0xE for every case is not a good. So I want to suggest only use 0xE for
>>> busy command. I personally preferred below implementation, which is similar
>>> with a RFC patch submitted by Jae hoon Chung, but only without adding a new
>>> quirk.
>> thanks for remind.
>> Yes, i tested without quirks, i think that is not problem.
>> (Just sent RFC patch with quirks, because i want to ask how think about adding quirks or not).
>
>
>
> Sorry I am confused.
>
> Setting 0x0E all the time does not solve the problem and has side effects ?
> What are the side effects ?
Side effect?? i didn't mention "side effect", just not resolved for every case..
That case is SDHCI didn't support the specific cards during suspend/resume.
i didn't know Mr.Chuanxiao's case.
>
> Using BUSY patch for 0x0e (below) works ?
>
>>> I think sdhci_calc_timeout should be left for data transfer since at least we
>>> can get a warning if 0xE is not enough for host to use. And if the host
>>> controller and the card have no bugs, then the calculated timeout should be
>>> safe. Left the old implementation unchanged is also compatible with all
>>> existed host controllers and cards.
>>>
>>> But for busy command, we are not clear about how long is safe enough for
>>> waiting and there is also no function to do the calculation for them. So
>>> preferred just using 0xE. Below the patch and comment:
>>>
>>> Set the timeout control register for SDHCI host when send some commands which
>>> need busy signal. Use the maximum timeout value 0xE will be safe.
>>>
>>> Signed-off-by: Chuanxiao Dong <[email protected]>
>>> ---
>>> drivers/mmc/host/sdhci.c | 9 ++++++++-
>>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 99c372e..8306323 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -659,8 +659,15 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
>>>
>>> WARN_ON(host->data);
>>>
>>> - if (data == NULL)
>>> + if (data == NULL) {
>>> + /*
>>> + * set the timeout to be maximum value for commands those with
>>> + * busy signal
>>> + */
>>> + if (host->cmd->flags & MMC_RSP_BUSY)
>>> + sdhci_writeb(host, 0xE, SDHCI_TIMEOUT_CONTROL);
>>> return;
>>> + }
>>>
>>> /* Sanity checks */
>>> BUG_ON(data->blksz * data->blocks > 524288);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>