public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
@ 2016-07-12 12:18 Jaehoon Chung
  2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-07-12 12:18 UTC (permalink / raw)
  To: u-boot

There is no data, it doesn't needs to wait for completing data transfer.
(It seems that it can be removed.)
Almost all timeout error is occured from stop command without data.
After applied this patch, I hope that we don't need to increase timeout value anymore.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 604f18d..0a38a56 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
 		flags = SDHCI_CMD_RESP_LONG;
 	else if (cmd->resp_type & MMC_RSP_BUSY) {
 		flags = SDHCI_CMD_RESP_SHORT_BUSY;
-		mask |= SDHCI_INT_DATA_END;
+		if (data)
+			mask |= SDHCI_INT_DATA_END;
 	} else
 		flags = SDHCI_CMD_RESP_SHORT;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B
  2016-07-12 12:18 [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Jaehoon Chung
@ 2016-07-12 12:18 ` Jaehoon Chung
  2016-07-12 12:55   ` Lukasz Majewski
  2016-08-05  2:24   ` [U-Boot] [U-Boot, " Jaehoon Chung
  2016-07-12 12:55 ` [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Lukasz Majewski
  2016-08-05  2:24 ` [U-Boot] [U-Boot, " Jaehoon Chung
  2 siblings, 2 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-07-12 12:18 UTC (permalink / raw)
  To: u-boot

Unset the SDHCI_QUIRK_BROKEN_R1B for exynos SoC.
(Tested on Exynos4 Boards.)

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/s5p_sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
index 44353c7..3bace21 100644
--- a/drivers/mmc/s5p_sdhci.c
+++ b/drivers/mmc/s5p_sdhci.c
@@ -68,7 +68,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
 	host->name = S5P_NAME;
 
 	host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
-		SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR |
+		SDHCI_QUIRK_32BIT_DMA_ADDR |
 		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_USE_WIDE8;
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
 	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-12 12:18 [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Jaehoon Chung
  2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
@ 2016-07-12 12:55 ` Lukasz Majewski
  2016-07-25  4:55   ` Jaehoon Chung
  2016-08-05  2:24 ` [U-Boot] [U-Boot, " Jaehoon Chung
  2 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2016-07-12 12:55 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

> There is no data, it doesn't needs to wait for completing data
> transfer. (It seems that it can be removed.)
> Almost all timeout error is occured from stop command without data.
> After applied this patch, I hope that we don't need to increase
> timeout value anymore.

This patch fixes issue (in a better way) for Odroid U3 write
performance (http://patchwork.ozlabs.org/patch/646932/) and hence
should be used.


> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/sdhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 604f18d..0a38a56 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
>  	else if (cmd->resp_type & MMC_RSP_BUSY) {
>  		flags = SDHCI_CMD_RESP_SHORT_BUSY;
> -		mask |= SDHCI_INT_DATA_END;
> +		if (data)
> +			mask |= SDHCI_INT_DATA_END;
>  	} else
>  		flags = SDHCI_CMD_RESP_SHORT;
>  

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

Test HW: Odroid U3 (Exynos4):

Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
32505856 bytes read in 1471 ms (21.1 MiB/s)
Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
File System is consistent
update journal finished
32505856 bytes written in 3528 ms (8.8 MiB/s)

Performance improvement is even better than with previously proposed
patches.

Test HW: Odroid XU3 (Exynos5):

ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
32505856 bytes read in 6309 ms (4.9 MiB/s)
ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
File System is consistent
update journal finished
32505856 bytes written in 4884 ms (6.3 MiB/s)


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B
  2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
@ 2016-07-12 12:55   ` Lukasz Majewski
  2016-08-05  2:24   ` [U-Boot] [U-Boot, " Jaehoon Chung
  1 sibling, 0 replies; 15+ messages in thread
From: Lukasz Majewski @ 2016-07-12 12:55 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

> Unset the SDHCI_QUIRK_BROKEN_R1B for exynos SoC.
> (Tested on Exynos4 Boards.)
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/s5p_sdhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
> index 44353c7..3bace21 100644
> --- a/drivers/mmc/s5p_sdhci.c
> +++ b/drivers/mmc/s5p_sdhci.c
> @@ -68,7 +68,7 @@ static int s5p_sdhci_core_init(struct sdhci_host
> *host) host->name = S5P_NAME;
>  
>  	host->quirks = SDHCI_QUIRK_NO_HISPD_BIT |
> SDHCI_QUIRK_BROKEN_VOLTAGE |
> -		SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR |
> +		SDHCI_QUIRK_32BIT_DMA_ADDR |
>  		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_USE_WIDE8;
>  	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 |
> MMC_VDD_165_195; host->version = sdhci_readw(host,
> SDHCI_HOST_VERSION);

Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Lukasz Majewski <l.majewski@samsung.com>

Test HW: Odroid U3 (Exynos4):

Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
32505856 bytes read in 1471 ms (21.1 MiB/s)
Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
File System is consistent
update journal finished
32505856 bytes written in 3528 ms (8.8 MiB/s)

Performance improvement is even better than with previously proposed
patches.

Test HW: Odroid XU3 (Exynos5):

ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
32505856 bytes read in 6309 ms (4.9 MiB/s)
ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
File System is consistent
update journal finished
32505856 bytes written in 4884 ms (6.3 MiB/s)

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-12 12:55 ` [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Lukasz Majewski
@ 2016-07-25  4:55   ` Jaehoon Chung
  2016-07-25 10:10     ` Minkyu Kang
                       ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-07-25  4:55 UTC (permalink / raw)
  To: u-boot

Hi All,

On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
> Hi Jaehoon,
> 
>> There is no data, it doesn't needs to wait for completing data
>> transfer. (It seems that it can be removed.)
>> Almost all timeout error is occured from stop command without data.
>> After applied this patch, I hope that we don't need to increase
>> timeout value anymore.
> 
> This patch fixes issue (in a better way) for Odroid U3 write
> performance (http://patchwork.ozlabs.org/patch/646932/) and hence
> should be used.
> 

Is there any other opinion for this patch?
Who is maintaining u-boot-mmc? Is Pantelis still maintaining u-boot-mmc?

Who can apply this patch and patch relevant to mmc?

Best Regards,
Jaehoon Chung

> 
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/sdhci.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 604f18d..0a38a56 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
>> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
>>  	else if (cmd->resp_type & MMC_RSP_BUSY) {
>>  		flags = SDHCI_CMD_RESP_SHORT_BUSY;
>> -		mask |= SDHCI_INT_DATA_END;
>> +		if (data)
>> +			mask |= SDHCI_INT_DATA_END;
>>  	} else
>>  		flags = SDHCI_CMD_RESP_SHORT;
>>  
> 
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> 
> Test HW: Odroid U3 (Exynos4):
> 
> Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
> 32505856 bytes read in 1471 ms (21.1 MiB/s)
> Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
> File System is consistent
> update journal finished
> 32505856 bytes written in 3528 ms (8.8 MiB/s)
> 
> Performance improvement is even better than with previously proposed
> patches.
> 
> Test HW: Odroid XU3 (Exynos5):
> 
> ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
> 32505856 bytes read in 6309 ms (4.9 MiB/s)
> ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
> File System is consistent
> update journal finished
> 32505856 bytes written in 4884 ms (6.3 MiB/s)
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25  4:55   ` Jaehoon Chung
@ 2016-07-25 10:10     ` Minkyu Kang
  2016-07-25 10:21     ` Lukasz Majewski
  2016-08-01 10:05     ` Lukasz Majewski
  2 siblings, 0 replies; 15+ messages in thread
From: Minkyu Kang @ 2016-07-25 10:10 UTC (permalink / raw)
  To: u-boot

Hi,

On 25/07/16 13:55, Jaehoon Chung wrote:
> Hi All,
> 
> On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
>> Hi Jaehoon,
>>
>>> There is no data, it doesn't needs to wait for completing data
>>> transfer. (It seems that it can be removed.)
>>> Almost all timeout error is occured from stop command without data.
>>> After applied this patch, I hope that we don't need to increase
>>> timeout value anymore.
>>
>> This patch fixes issue (in a better way) for Odroid U3 write
>> performance (http://patchwork.ozlabs.org/patch/646932/) and hence
>> should be used.
>>
> 
> Is there any other opinion for this patch?

looks reasonable.

Acked-by: Minkyu Kang <mk7.kang@samsung.com>

> Who is maintaining u-boot-mmc? Is Pantelis still maintaining u-boot-mmc?
> 
> Who can apply this patch and patch relevant to mmc?
> 

Pantelis seems to away from mailing list.
If so, I think we need discuss about new custodian for mmc.

Thanks,
Minkyu Kang.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25  4:55   ` Jaehoon Chung
  2016-07-25 10:10     ` Minkyu Kang
@ 2016-07-25 10:21     ` Lukasz Majewski
  2016-07-25 23:23       ` Steve Rae
  2016-08-01 10:05     ` Lukasz Majewski
  2 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2016-07-25 10:21 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

> Hi All,
> 
> On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
> > Hi Jaehoon,
> > 
> >> There is no data, it doesn't needs to wait for completing data
> >> transfer. (It seems that it can be removed.)
> >> Almost all timeout error is occured from stop command without data.
> >> After applied this patch, I hope that we don't need to increase
> >> timeout value anymore.
> > 
> > This patch fixes issue (in a better way) for Odroid U3 write
> > performance (http://patchwork.ozlabs.org/patch/646932/) and hence
> > should be used.
> > 
> 
> Is there any other opinion for this patch?
> Who is maintaining u-boot-mmc? Is Pantelis still maintaining
> u-boot-mmc?
> 
> Who can apply this patch and patch relevant to mmc?

To be honest, I do look forward to see this patch included to u-boot
master branch.

Addition of some extra Odroid U3 tests hinge on it.

> 
> Best Regards,
> Jaehoon Chung
> 
> > 
> >>
> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> ---
> >>  drivers/mmc/sdhci.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> >> index 604f18d..0a38a56 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
> >> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
> >>  	else if (cmd->resp_type & MMC_RSP_BUSY) {
> >>  		flags = SDHCI_CMD_RESP_SHORT_BUSY;
> >> -		mask |= SDHCI_INT_DATA_END;
> >> +		if (data)
> >> +			mask |= SDHCI_INT_DATA_END;
> >>  	} else
> >>  		flags = SDHCI_CMD_RESP_SHORT;
> >>  
> > 
> > Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> > 
> > Test HW: Odroid U3 (Exynos4):
> > 
> > Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
> > 32505856 bytes read in 1471 ms (21.1 MiB/s)
> > Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
> > File System is consistent
> > update journal finished
> > 32505856 bytes written in 3528 ms (8.8 MiB/s)
> > 
> > Performance improvement is even better than with previously proposed
> > patches.
> > 
> > Test HW: Odroid XU3 (Exynos5):
> > 
> > ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
> > 32505856 bytes read in 6309 ms (4.9 MiB/s)
> > ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
> > File System is consistent
> > update journal finished
> > 32505856 bytes written in 4884 ms (6.3 MiB/s)
> > 
> > 
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25 10:21     ` Lukasz Majewski
@ 2016-07-25 23:23       ` Steve Rae
  2016-07-25 23:28         ` Steve Rae
  2016-07-26  9:13         ` Lukasz Majewski
  0 siblings, 2 replies; 15+ messages in thread
From: Steve Rae @ 2016-07-25 23:23 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

On Mon, Jul 25, 2016 at 3:21 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Jaehoon,
>
>> Hi All,
>>
>> On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
>> > Hi Jaehoon,
>> >
>> >> There is no data, it doesn't needs to wait for completing data
>> >> transfer. (It seems that it can be removed.)
>> >> Almost all timeout error is occured from stop command without data.
>> >> After applied this patch, I hope that we don't need to increase
>> >> timeout value anymore.
>> >
>> > This patch fixes issue (in a better way) for Odroid U3 write
>> > performance (http://patchwork.ozlabs.org/patch/646932/) and hence
>> > should be used.
>> >
>>
>> Is there any other opinion for this patch?
>> Who is maintaining u-boot-mmc? Is Pantelis still maintaining
>> u-boot-mmc?
>>
>> Who can apply this patch and patch relevant to mmc?
>
> To be honest, I do look forward to see this patch included to u-boot
> master branch.
>
> Addition of some extra Odroid U3 tests hinge on it.
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> >
>> >>
>> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> >> ---
>> >>  drivers/mmc/sdhci.c | 3 ++-
>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> >> index 604f18d..0a38a56 100644
>> >> --- a/drivers/mmc/sdhci.c
>> >> +++ b/drivers/mmc/sdhci.c
>> >> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
>> >> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
>> >>    else if (cmd->resp_type & MMC_RSP_BUSY) {
>> >>            flags = SDHCI_CMD_RESP_SHORT_BUSY;
>> >> -          mask |= SDHCI_INT_DATA_END;
>> >> +          if (data)
>> >> +                  mask |= SDHCI_INT_DATA_END;
>> >>    } else
>> >>            flags = SDHCI_CMD_RESP_SHORT;
>> >>
>> >
>> > Acked-by: Lukasz Majewski <l.majewski@samsung.com>
>> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
>> >
>> > Test HW: Odroid U3 (Exynos4):
>> >
>> > Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
>> > 32505856 bytes read in 1471 ms (21.1 MiB/s)
>> > Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
>> > File System is consistent
>> > update journal finished
>> > 32505856 bytes written in 3528 ms (8.8 MiB/s)
>> >
>> > Performance improvement is even better than with previously proposed
>> > patches.
>> >
>> > Test HW: Odroid XU3 (Exynos5):
>> >
>> > ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
>> > 32505856 bytes read in 6309 ms (4.9 MiB/s)
>> > ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
>> > File System is consistent
>> > update journal finished
>> > 32505856 bytes written in 4884 ms (6.3 MiB/s)
>> >
>> >
>>
>
>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

with this change, I can also set the following back to 100:

diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index de8d8ea..d593dc6 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -128,7 +128,7 @@ static int sdhci_transfer_data(struct sdhci_host
*host, struct mmc_data *data,
 #define CONFIG_SDHCI_CMD_MAX_TIMEOUT           3200
 #endif
 #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT       100
-#define SDHCI_READ_STATUS_TIMEOUT              1000
+#define SDHCI_READ_STATUS_TIMEOUT              100

 #ifdef CONFIG_DM_MMC_OPS
 static int sdhci_send_command(struct udevice *dev, struct mmc_cmd *cmd,

And it still works on my board !  Thanks !

Tested-by: Steve Rae <steve.rae@raedomain.com> [Test HW: bcm235xx board]

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25 23:23       ` Steve Rae
@ 2016-07-25 23:28         ` Steve Rae
  2016-07-26  9:13         ` Lukasz Majewski
  1 sibling, 0 replies; 15+ messages in thread
From: Steve Rae @ 2016-07-25 23:28 UTC (permalink / raw)
  To: u-boot

On Mon, Jul 25, 2016 at 4:23 PM, Steve Rae <steve.rae@raedomain.com> wrote:
> Hi Jaehoon,
>
> On Mon, Jul 25, 2016 at 3:21 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
>> Hi Jaehoon,
>>
>>> Hi All,
>>>
>>> On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
>>> > Hi Jaehoon,
>>> >
>>> >> There is no data, it doesn't needs to wait for completing data
>>> >> transfer. (It seems that it can be removed.)
>>> >> Almost all timeout error is occured from stop command without data.
>>> >> After applied this patch, I hope that we don't need to increase
>>> >> timeout value anymore.
>>> >
>>> > This patch fixes issue (in a better way) for Odroid U3 write
>>> > performance (http://patchwork.ozlabs.org/patch/646932/) and hence
>>> > should be used.
>>> >
>>>
>>> Is there any other opinion for this patch?
>>> Who is maintaining u-boot-mmc? Is Pantelis still maintaining
>>> u-boot-mmc?
>>>
>>> Who can apply this patch and patch relevant to mmc?
>>
>> To be honest, I do look forward to see this patch included to u-boot
>> master branch.
>>
>> Addition of some extra Odroid U3 tests hinge on it.
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>> >
>>> >>
>>> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> >> ---
>>> >>  drivers/mmc/sdhci.c | 3 ++-
>>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> >> index 604f18d..0a38a56 100644
>>> >> --- a/drivers/mmc/sdhci.c
>>> >> +++ b/drivers/mmc/sdhci.c
>>> >> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
>>> >> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
>>> >>    else if (cmd->resp_type & MMC_RSP_BUSY) {
>>> >>            flags = SDHCI_CMD_RESP_SHORT_BUSY;
>>> >> -          mask |= SDHCI_INT_DATA_END;
>>> >> +          if (data)
>>> >> +                  mask |= SDHCI_INT_DATA_END;
>>> >>    } else
>>> >>            flags = SDHCI_CMD_RESP_SHORT;
>>> >>
>>> >
>>> > Acked-by: Lukasz Majewski <l.majewski@samsung.com>
>>> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
>>> >
>>> > Test HW: Odroid U3 (Exynos4):
>>> >
>>> > Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
>>> > 32505856 bytes read in 1471 ms (21.1 MiB/s)
>>> > Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
>>> > File System is consistent
>>> > update journal finished
>>> > 32505856 bytes written in 3528 ms (8.8 MiB/s)
>>> >
>>> > Performance improvement is even better than with previously proposed
>>> > patches.
>>> >
>>> > Test HW: Odroid XU3 (Exynos5):
>>> >
>>> > ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
>>> > 32505856 bytes read in 6309 ms (4.9 MiB/s)
>>> > ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
>>> > File System is consistent
>>> > update journal finished
>>> > 32505856 bytes written in 4884 ms (6.3 MiB/s)
>>> >
>>> >
>>>
>>
>>
>>
>> --
>> Best regards,
>>
>> Lukasz Majewski
>>
>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>
> with this change, I can also set the following back to 100:
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index de8d8ea..d593dc6 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -128,7 +128,7 @@ static int sdhci_transfer_data(struct sdhci_host
> *host, struct mmc_data *data,
>  #define CONFIG_SDHCI_CMD_MAX_TIMEOUT           3200
>  #endif
>  #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT       100
> -#define SDHCI_READ_STATUS_TIMEOUT              1000
> +#define SDHCI_READ_STATUS_TIMEOUT              100
>
>  #ifdef CONFIG_DM_MMC_OPS
>  static int sdhci_send_command(struct udevice *dev, struct mmc_cmd *cmd,
>
> And it still works on my board !  Thanks !
>
> Tested-by: Steve Rae <steve.rae@raedomain.com> [Test HW: bcm235xx board]

patchworks added a new entry; so try again:
Tested-by: Steve Rae <steve.rae@raedomain.com> [Test HW: bcm235xx board]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25 23:23       ` Steve Rae
  2016-07-25 23:28         ` Steve Rae
@ 2016-07-26  9:13         ` Lukasz Majewski
  2016-07-26 16:10           ` Steve Rae
  1 sibling, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2016-07-26  9:13 UTC (permalink / raw)
  To: u-boot

Hi Steve,

> with this change, I can also set the following back to 100:
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index de8d8ea..d593dc6 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -128,7 +128,7 @@ static int sdhci_transfer_data(struct sdhci_host
> *host, struct mmc_data *data,
>  #define CONFIG_SDHCI_CMD_MAX_TIMEOUT           3200
>  #endif
>  #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT       100
> -#define SDHCI_READ_STATUS_TIMEOUT              1000
> +#define SDHCI_READ_STATUS_TIMEOUT              100
> 
>  #ifdef CONFIG_DM_MMC_OPS
>  static int sdhci_send_command(struct udevice *dev, struct mmc_cmd
> *cmd,
> 
> And it still works on my board !  Thanks !

Could you prepare proper revert patch?

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-26  9:13         ` Lukasz Majewski
@ 2016-07-26 16:10           ` Steve Rae
  2016-07-27  5:11             ` Jaehoon Chung
  0 siblings, 1 reply; 15+ messages in thread
From: Steve Rae @ 2016-07-26 16:10 UTC (permalink / raw)
  To: u-boot

HI Lukasz,

On Tue, Jul 26, 2016 at 2:13 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Steve,
>
>> with this change, I can also set the following back to 100:
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index de8d8ea..d593dc6 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -128,7 +128,7 @@ static int sdhci_transfer_data(struct sdhci_host
>> *host, struct mmc_data *data,
>>  #define CONFIG_SDHCI_CMD_MAX_TIMEOUT           3200
>>  #endif
>>  #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT       100
>> -#define SDHCI_READ_STATUS_TIMEOUT              1000
>> +#define SDHCI_READ_STATUS_TIMEOUT              100
>>
>>  #ifdef CONFIG_DM_MMC_OPS
>>  static int sdhci_send_command(struct udevice *dev, struct mmc_cmd
>> *cmd,
>>
>> And it still works on my board !  Thanks !
>
> Could you prepare proper revert patch?
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Looking at the code, I don't think there is any value changing the
SDHCI_READ_STATUS_TIMEOUT from 1000 to 100.
But maybe someone (Jaehoon ?) could comment on the impact of this
SDHCI_READ_STATUS_TIMEOUT value in the SDHCI_QUIRK_BROKEN_R1B case...
Does it affect performance in anyway?
If it does, the I'll prepare a patch....

Thanks, Steve

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-26 16:10           ` Steve Rae
@ 2016-07-27  5:11             ` Jaehoon Chung
  0 siblings, 0 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-07-27  5:11 UTC (permalink / raw)
  To: u-boot

Hi 

On 07/27/2016 01:10 AM, Steve Rae wrote:
> HI Lukasz,
> 
> On Tue, Jul 26, 2016 at 2:13 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
>> Hi Steve,
>>
>>> with this change, I can also set the following back to 100:
>>>
>>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>>> index de8d8ea..d593dc6 100644
>>> --- a/drivers/mmc/sdhci.c
>>> +++ b/drivers/mmc/sdhci.c
>>> @@ -128,7 +128,7 @@ static int sdhci_transfer_data(struct sdhci_host
>>> *host, struct mmc_data *data,
>>>  #define CONFIG_SDHCI_CMD_MAX_TIMEOUT           3200
>>>  #endif
>>>  #define CONFIG_SDHCI_CMD_DEFAULT_TIMEOUT       100
>>> -#define SDHCI_READ_STATUS_TIMEOUT              1000
>>> +#define SDHCI_READ_STATUS_TIMEOUT              100
>>>
>>>  #ifdef CONFIG_DM_MMC_OPS
>>>  static int sdhci_send_command(struct udevice *dev, struct mmc_cmd
>>> *cmd,
>>>
>>> And it still works on my board !  Thanks !
>>
>> Could you prepare proper revert patch?
>>
>> --
>> Best regards,
>>
>> Lukasz Majewski
>>
>> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
> 
> Looking at the code, I don't think there is any value changing the
> SDHCI_READ_STATUS_TIMEOUT from 1000 to 100.
> But maybe someone (Jaehoon ?) could comment on the impact of this
> SDHCI_READ_STATUS_TIMEOUT value in the SDHCI_QUIRK_BROKEN_R1B case...
> Does it affect performance in anyway?

There is no performance effect whatever SDHCI_READ_STATUS_TIMEOUT is using value.
So i think that we don't need to revert it..

SDHCI_QUIRK_BROKEN_R1B had been added from me.
At that time, i didn't know why added SDHCI_INT_DAT_END.
I'm not sure but if remove the SDHCI_INT_DAT_END, 
I guess that SDHCI_QUIRK_BROKEN_R1B can be also removed.
(quirks is workaround flags, so if it can be removed, it's best.)

In future, I want to remove SDHCI_QUIRK_BROKEN_R1B.

Best Regards,
Jaehoon Chung

> If it does, the I'll prepare a patch....
> 
> Thanks, Steve
> 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-25  4:55   ` Jaehoon Chung
  2016-07-25 10:10     ` Minkyu Kang
  2016-07-25 10:21     ` Lukasz Majewski
@ 2016-08-01 10:05     ` Lukasz Majewski
  2 siblings, 0 replies; 15+ messages in thread
From: Lukasz Majewski @ 2016-08-01 10:05 UTC (permalink / raw)
  To: u-boot

Dear All,

> Hi All,
> 
> On 07/12/2016 09:55 PM, Lukasz Majewski wrote:
> > Hi Jaehoon,
> > 
> >> There is no data, it doesn't needs to wait for completing data
> >> transfer. (It seems that it can be removed.)
> >> Almost all timeout error is occured from stop command without data.
> >> After applied this patch, I hope that we don't need to increase
> >> timeout value anymore.
> > 
> > This patch fixes issue (in a better way) for Odroid U3 write
> > performance (http://patchwork.ozlabs.org/patch/646932/) and hence
> > should be used.
> > 
> 
> Is there any other opinion for this patch?
> Who is maintaining u-boot-mmc? Is Pantelis still maintaining
> u-boot-mmc?
> 
> Who can apply this patch and patch relevant to mmc?

Gentle ping .....

Pantelis could you pull this patch to u-boot-mmc tree?

> 
> Best Regards,
> Jaehoon Chung
> 
> > 
> >>
> >> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> >> ---
> >>  drivers/mmc/sdhci.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> >> index 604f18d..0a38a56 100644
> >> --- a/drivers/mmc/sdhci.c
> >> +++ b/drivers/mmc/sdhci.c
> >> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc,
> >> struct mmc_cmd *cmd, flags = SDHCI_CMD_RESP_LONG;
> >>  	else if (cmd->resp_type & MMC_RSP_BUSY) {
> >>  		flags = SDHCI_CMD_RESP_SHORT_BUSY;
> >> -		mask |= SDHCI_INT_DATA_END;
> >> +		if (data)
> >> +			mask |= SDHCI_INT_DATA_END;
> >>  	} else
> >>  		flags = SDHCI_CMD_RESP_SHORT;
> >>  
> > 
> > Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> > Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> > 
> > Test HW: Odroid U3 (Exynos4):
> > 
> > Odroid # ext4load mmc 0:2 0x41000000 dat_31M.img
> > 32505856 bytes read in 1471 ms (21.1 MiB/s)
> > Odroid # ext4write mmc 0:2 0x41000000 /dat_w55.img 0x1f00000
> > File System is consistent
> > update journal finished
> > 32505856 bytes written in 3528 ms (8.8 MiB/s)
> > 
> > Performance improvement is even better than with previously proposed
> > patches.
> > 
> > Test HW: Odroid XU3 (Exynos5):
> > 
> > ODROID-XU3 # ext4load mmc 0:2 0x41000000 dat_31M.img
> > 32505856 bytes read in 6309 ms (4.9 MiB/s)
> > ODROID-XU3 # ext4write mmc 0:2 0x41000000 /dat_w1.img 0x1f00000
> > File System is consistent
> > update journal finished
> > 32505856 bytes written in 4884 ms (6.3 MiB/s)
> > 
> > 
> 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot, 1/2] mmc: sdhci: set to INT_DATA_END when there are data
  2016-07-12 12:18 [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Jaehoon Chung
  2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
  2016-07-12 12:55 ` [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Lukasz Majewski
@ 2016-08-05  2:24 ` Jaehoon Chung
  2 siblings, 0 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-08-05  2:24 UTC (permalink / raw)
  To: u-boot

On 07/12/2016 09:18 PM, Jaehoon Chung wrote:
> There is no data, it doesn't needs to wait for completing data transfer.
> (It seems that it can be removed.)
> Almost all timeout error is occured from stop command without data.
> After applied this patch, I hope that we don't need to increase timeout value anymore.
> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> Acked-by: Minkyu Kang <mk7.kang@samsung.com>

Applied on u-boot-mmc. Thanks!

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/sdhci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 604f18d..0a38a56 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -175,7 +175,8 @@ static int sdhci_send_command(struct mmc *mmc, struct mmc_cmd *cmd,
>  		flags = SDHCI_CMD_RESP_LONG;
>  	else if (cmd->resp_type & MMC_RSP_BUSY) {
>  		flags = SDHCI_CMD_RESP_SHORT_BUSY;
> -		mask |= SDHCI_INT_DATA_END;
> +		if (data)
> +			mask |= SDHCI_INT_DATA_END;
>  	} else
>  		flags = SDHCI_CMD_RESP_SHORT;
>  
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot, 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B
  2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
  2016-07-12 12:55   ` Lukasz Majewski
@ 2016-08-05  2:24   ` Jaehoon Chung
  1 sibling, 0 replies; 15+ messages in thread
From: Jaehoon Chung @ 2016-08-05  2:24 UTC (permalink / raw)
  To: u-boot

On 07/12/2016 09:18 PM, Jaehoon Chung wrote:
> Unset the SDHCI_QUIRK_BROKEN_R1B for exynos SoC.
> (Tested on Exynos4 Boards.)

Applied on u-boot-mmc. Thanks!

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
> Tested-by: Lukasz Majewski <l.majewski@samsung.com>
> ---
>  drivers/mmc/s5p_sdhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c
> index 44353c7..3bace21 100644
> --- a/drivers/mmc/s5p_sdhci.c
> +++ b/drivers/mmc/s5p_sdhci.c
> @@ -68,7 +68,7 @@ static int s5p_sdhci_core_init(struct sdhci_host *host)
>  	host->name = S5P_NAME;
>  
>  	host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
> -		SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR |
> +		SDHCI_QUIRK_32BIT_DMA_ADDR |
>  		SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_USE_WIDE8;
>  	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
>  	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-08-05  2:24 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-12 12:18 [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Jaehoon Chung
2016-07-12 12:18 ` [U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B Jaehoon Chung
2016-07-12 12:55   ` Lukasz Majewski
2016-08-05  2:24   ` [U-Boot] [U-Boot, " Jaehoon Chung
2016-07-12 12:55 ` [U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data Lukasz Majewski
2016-07-25  4:55   ` Jaehoon Chung
2016-07-25 10:10     ` Minkyu Kang
2016-07-25 10:21     ` Lukasz Majewski
2016-07-25 23:23       ` Steve Rae
2016-07-25 23:28         ` Steve Rae
2016-07-26  9:13         ` Lukasz Majewski
2016-07-26 16:10           ` Steve Rae
2016-07-27  5:11             ` Jaehoon Chung
2016-08-01 10:05     ` Lukasz Majewski
2016-08-05  2:24 ` [U-Boot] [U-Boot, " Jaehoon Chung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox