mmc: sdhci: increase the timeout value for data transfer

PATCH New Jaehoon Chung 3 participants 7 messages Sep 21, 2012 – Sep 25, 2012 MMC, SD and SDIO

Share & tools

Jaehoon Chung

author

2012-09-21 06:31 UTC #0 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Timeout value is tunable.
When run read/write operation, sometime returned the timeout error. Because the timeout value is too short.
So increased the enough timeout value.
(This timeout value is used to prevent the infinite loop.)

Signed-off-by Jaehoon Chung <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/mmc/sdhci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/mmc/sdhci.c +1 -1

index 2e3c408..9329874 100644
@@ -83,7 +83,7 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data,
 {
 	unsigned int stat, rdy, mask, timeout, block = 0;
 
-	timeout = 10000;
+	timeout = 1000000;
 	rdy = SDHCI_INT_SPACE_AVAIL | SDHCI_INT_DATA_AVAIL;
 	mask = SDHCI_DATA_AVAILABLE | SDHCI_SPACE_AVAILABLE;
 	do {

Signature · 1 line

1.7.4.1

Rommel Custodio

2012-09-21 09:48 UTC #1 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Quoted 4 lines · Jaehoon Chung <jh80.chung <at> samsung.com> writes:

Timeout value is tunable.
When run read/write operation, sometime returned the timeout error. Because the timeout value is too short.

Hello,

I think it is better to fine tune CONFIG_SYS_MMC_MAX_BLK_COUNT. This gets assigned to mmc->b_max, unless you specifically set mmc->b_max value during mmc_register().

b_max is important since when you adjust b_max properly, mmc_bread() and mmc_bwrite() will properly partition the read/write operation (in b_max blocks) so that the timeout does not occur.

All the best,
Rommel

(replying via gmane since i just joined the list)

Jaehoon Chung

author

2012-09-24 01:32 UTC #2 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Hi Rommel,

I didn't think so..Our environment is support the CONFIG_SYS_MMC_MAX_BLK_COUNT. Did you know how get the timeout value "1000"?

If the timeout value "1000" is reasonable, i want to know what basis. Well, i don't think that my timeout value is reasonable.

Actually i want to remove the timeout value in that function. But then we should be prevent the infinite loop.

Anyway, thanks for your comment. I will check the your opinion.

Best Regards,
Jaehoon Chung

Quoted 30 lines · Rommel Custodio

Quoted 4 lines · Jaehoon Chung <jh80.chung <at> samsung.com> writes:
Timeout value is tunable.
When run read/write operation, sometime returned the timeout error. Because the timeout value is too short.
Hello,
I think it is better to fine tune CONFIG_SYS_MMC_MAX_BLK_COUNT. This gets assigned to mmc->b_max, unless you specifically set mmc->b_max value during mmc_register().
b_max is important since when you adjust b_max properly, mmc_bread() and mmc_bwrite() will properly partition the read/write operation (in b_max blocks) so that the timeout does not occur.
All the best,
Rommel
(replying via gmane since i just joined the list)
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Rommel Custodio

2012-09-24 02:34 UTC #3 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Hello Jaehoon,

Quoted 2 lines

I didn't think so..Our environment is support the CONFIG_SYS_MMC_MAX_BLK_COUNT.

This is defined in mmc.c right after the include definitions. The comment says that:
Set block count limit because of 16 bit
register limit on some hardware

So my use of this define is a bit of a hack too.

Quoted 5 lines

Did you know how get the timeout value "1000"?
If the timeout value "1000" is reasonable, i want to know what basis. Well, i don't think that my timeout value is reasonable.

I think timeout value was from original creator/maintainer of SD/MMC code.

When trying to read big data off of the SD card, I get the timeout too on my platform (ml507). My test consist of reading a 26Mb file from the SD card in PIO and DMA mode.

When the timeout is displayed in the serial console, the block count register is not yet zero but there is no error in the interrupt status register. The block count value shows the same value when I perform the same test.

For my use, CONFIG_SYS_MMC_MAX_BLK_COUNT = 0x1000 in my environment. I did not modify any of the timeout values.

Quoted 3 lines

Actually i want to remove the timeout value in that function. But then we should be prevent the infinite loop.

The infinite loop that you mention does not occur in my situation.

Quoted 2 lines

Anyway, thanks for your comment. I will check the your opinion.

HTH

(replying via gmane since i just joined the list)

Jaehoon Chung

author

2012-09-24 05:23 UTC #4 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Hi Rommel,

Quoted 9 lines · Rommel Custodio

Hello Jaehoon,
Quoted 2 lines
I didn't think so..Our environment is support the CONFIG_SYS_MMC_MAX_BLK_COUNT.
This is defined in mmc.c right after the include definitions. The comment says that:
Set block count limit because of 16 bit
register limit on some hardware

I known that defined into mmc.c.
After changed the MMC_MAX_BLK_COUNT, it also produced "Transfer data timeout". When MMC_MAX_BLOCK_COUNT is set to 1, didn't produce the error log. But it's too late.

Quoted 4 lines

So my use of this define is a bit of a hack too.

Quoted 8 lines

Quoted 5 lines
Did you know how get the timeout value "1000"?
If the timeout value "1000" is reasonable, i want to know what basis. Well, i don't think that my timeout value is reasonable.
I think timeout value was from original creator/maintainer of SD/MMC code.

Right, the timeout value has set from original code.

Quoted 9 lines

When trying to read big data off of the SD card, I get the timeout too on my platform (ml507). My test consist of reading a 26Mb file from the SD card in PIO and DMA mode.
When the timeout is displayed in the serial console, the block count register is not yet zero but there is no error in the interrupt status register. The block count value shows the same value when I perform the same test.

This timeout value isn't timeout value related with TIMEOUT control register. Your comment seems to mention the TIMEOUT control register.
(As my understanding..if my understanding is wrong, sorry..)

Quoted 11 lines

For my use, CONFIG_SYS_MMC_MAX_BLK_COUNT = 0x1000 in my environment. I did not modify any of the timeout values.
Quoted 3 lines
Actually i want to remove the timeout value in that function. But then we should be prevent the infinite loop.
The infinite loop that you mention does not occur in my situation.

I also didn't occur the infinite loop.
So i think that we can remove the timeout value in that function.

Best Regards,
Jaehoon Chung

Quoted 13 lines

Quoted 3 lines
Anyway, thanks for your comment. I will check the your opinion.
HTH
(replying via gmane since i just joined the list)
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Mela Custodio

2012-09-24 19:57 UTC #5 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Quoted 4 lines · Jaehoon Chung

Quoted 2 lines
The infinite loop that you mention does not occur in my situation.
I also didn't occur the infinite loop.
So i think that we can remove the timeout value in that function.

Hi,

I have no comment about that. In your local environment you could remove it but I doubt that that will be good in mainline u-boot though.

All the best,
RgC

Jaehoon Chung

author

2012-09-25 01:33 UTC #6 lore raw

PATCH 1/3 mmc: sdhci: increase the timeout value for data transfer

Hi,

Quoted 9 lines · Mela Custodio

Quoted 4 lines · Jaehoon Chung
Quoted 2 lines
The infinite loop that you mention does not occur in my situation.
I also didn't occur the infinite loop.
So i think that we can remove the timeout value in that function.
Hi,
I have no comment about that. In your local environment you could remove it but I doubt that that will be good in mainline u-boot though.

Yes, In local environment, i can remove it. but in mainline, i agreed your opinion. As your mention, it would be not good that remove the timeout.
So i increased the timeout value from 1000 to 100000.
then we can get more chance to check the interrupt status register.
My problem is that host is returned the timeout error before changing the interrupt status register.

Best Regards,
Jaehoon Chung

Quoted 4 lines

All the best,
RgC