On 11/13/2013 12:12 PM, Dong Aisheng wrote:
Hi Arend,
On Wed, Nov 13, 2013 at 6:21 PM, Arend van Spriel <ar...@broadcom.com> wrote:
On 11/13/2013 06:02 AM, Dong Aisheng wrote:
Hi Arend,
On Mon, Nov 11, 2013 at 5:49 PM, Arend van Spriel <ar...@broadcom.com>
wrote:
When the host->tuning_count is zero it means that the
retuning is disabled. This is checked on the first
run of sdhci_execute_tuning() by the if statement below:
if (!(host->flags & SDHCI_NEEDS_RETUNING) && host->tuning_count
&&
(host->tuning_mode == SDHCI_TUNING_MODE_1)) {
So only when tuning_count is non-zero it will set the host
flag SDHCI_USING_RETUNING_TIMER. The else statement is only
for re-programming the timer, which means that flag must be
set. Because that is not checked the else statement is executed
in the first run when tuning_count is zero.
This was seen on a host controller which indicated
SDHCI_TUNING_MODE_1 (0) and tuning_count being zero. Suspect
that (one of) these registers is not properly set.
Signed-off-by: Arend van Spriel <ar...@broadcom.com>
---
This patch applies to the mmc-next branch.
V2:
- add more explanation to the commit message
- check host flag SDHCI_USING_RETUNING_TIMER
---
drivers/mmc/host/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index bd8a098..5974599 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2014,7 +2014,7 @@ out:
host->tuning_count * HZ);
/* Tuning mode 1 limits the maximum data length to 4MB
*/
mmc->max_blk_count = (4 * 1024 * 1024) /
mmc->max_blk_size;
- } else {
+ } else if (host->flags & SDHCI_USING_RETUNING_TIMER) {
host->flags &= ~SDHCI_NEEDS_RETUNING;
/* Reload the new initial value for timer */
if (host->tuning_mode == SDHCI_TUNING_MODE_1)
I wonder if we could also remove this line?
It looks to me it's not neccesary to check the tuning_mode again since
we already check the flag
above and SDHCI_TUNING_MODE_1 seems like the prerequisite of
SDHCI_USING_RETUNING_TIMER.
According the spec the other tuning modes also can use retuning timer.
Currently, the mmc stack in upstream linux only supports tuning mode 1. When
adding the other modes this if statement will probably go.
For currently code, it looks like also not necessary to check it since
SDHCI_USING_RETUNING_TIMER will only be set when tunning_mode is
SDHCI_TUNING_MODE_1.
And SDHCI_TUNING_MODE_1 just indicates the tuning mode while the flag
SDHCI_USING_RETUNING_TIMER represents the retuning timer implementation.
So check the flag to invoke the timer seems make more sense to me.
do you agree?
The flag SDHCI_USING_RETUNING_TIMER is only set after the initial tuning
run so in the if-statement. So currently in the else-statement the fact
that SDHCI_USING_RETUNING_TIMER is set implies SDHCI_TUNING_MODE_1.
Regards,
Arend
Regards
Dong Aisheng
Regards,
Arend
Regards
Dong Aisheng
--
1.7.10.4
--
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
--
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
