[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Jaehoon Chung]

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

[U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B

[Jaehoon Chung]

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

[U-Boot] [PATCH 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B

[Lukasz Majewski]

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

[U-Boot] [U-Boot, 2/2] mmc: s5p_sdhci: unset the SDHCI_QUIRK_BROKEN_R1B

[Jaehoon Chung]

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);
> 

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Lukasz Majewski]

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

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Jaehoon Chung]

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)
> 
> 

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Minkyu Kang]

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.

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Lukasz Majewski]

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

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Steve Rae]

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]

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Steve Rae]

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]

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Lukasz Majewski]

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

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Steve Rae]

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

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Jaehoon Chung]

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
> 
> 
> 

[U-Boot] [PATCH 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Lukasz Majewski]

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

[U-Boot] [U-Boot, 1/2] mmc: sdhci: set to INT_DATA_END when there are data

[Jaehoon Chung]

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;
>  
>