subject:"Re\: \[U\-Boot\] \[PATCH 1\/3\] mmc\: dw_mmc\: change the read\/write order under fifo mode"

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-22 Thread Ziyuan Xu


hi Simon,


On 2016年09月23日 10:39, Simon Glass wrote:

Hi,

On 7 September 2016 at 21:54, Jaehoon Chung  wrote:

On 09/08/2016 12:43 PM, Ziyuan Xu wrote:


On 2016年09月07日 14:50, Jaehoon Chung wrote:

On 09/07/2016 03:14 PM, Ziyuan Xu wrote:

hi Jaehoon,


On 2016年08月30日 17:54, Jaehoon Chung wrote:

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:

From: "jacob2.chen" 

The former implement have a bug.
It will cause wrong data reading sometimes.

Could you explain what bug is there?

Signed-off-by: jacob2.chen 

Could you change from jacob2.chen to your name?


---

drivers/mmc/dw_mmc.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index afc674d..f072739 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host *host, 
struct mmc_data *data)
  if (host->fifo_mode && size) {
len = 0;
-if (data->flags == MMC_DATA_READ) {
-if ((dwmci_readl(host, DWMCI_RINTSTS) &
- DWMCI_INTMSK_RXDR)) {
+if (data->flags == MMC_DATA_READ &&
+(mask & DWMCI_INTMSK_RXDR)) {
+while (size) {
len = dwmci_readl(host, DWMCI_STATUS);
len = (len >> DWMCI_FIFO_SHIFT) &
-DWMCI_FIFO_MASK;
+DWMCI_FIFO_MASK;

What is the status of this patch please?


I have sent patch v2, and Jaehoon applied it. Pls see 
http://patchwork.ozlabs.org/patch/671533/.

Thanks for your attention.



[...]

Regards,
Simon






___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-22 Thread Simon Glass

Hi,

On 7 September 2016 at 21:54, Jaehoon Chung  wrote:
> On 09/08/2016 12:43 PM, Ziyuan Xu wrote:
>>
>>
>> On 2016年09月07日 14:50, Jaehoon Chung wrote:
>>> On 09/07/2016 03:14 PM, Ziyuan Xu wrote:
 hi Jaehoon,


 On 2016年08月30日 17:54, Jaehoon Chung wrote:
> Hi Jacob,
>
> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>> From: "jacob2.chen" 
>>
>> The former implement have a bug.
>> It will cause wrong data reading sometimes.
> Could you explain what bug is there?
>> Signed-off-by: jacob2.chen 
> Could you change from jacob2.chen to your name?
>
>> ---
>>
>>drivers/mmc/dw_mmc.c | 32 +---
>>1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>> index afc674d..f072739 100644
>> --- a/drivers/mmc/dw_mmc.c
>> +++ b/drivers/mmc/dw_mmc.c
>> @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host 
>> *host, struct mmc_data *data)
>>  if (host->fifo_mode && size) {
>>len = 0;
>> -if (data->flags == MMC_DATA_READ) {
>> -if ((dwmci_readl(host, DWMCI_RINTSTS) &
>> - DWMCI_INTMSK_RXDR)) {
>> +if (data->flags == MMC_DATA_READ &&
>> +(mask & DWMCI_INTMSK_RXDR)) {
>> +while (size) {
>>len = dwmci_readl(host, DWMCI_STATUS);
>>len = (len >> DWMCI_FIFO_SHIFT) &
>> -DWMCI_FIFO_MASK;
>> +DWMCI_FIFO_MASK;

What is the status of this patch please?

[...]

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-08 Thread Jaehoon Chung

On 09/08/2016 12:43 PM, Ziyuan Xu wrote:
> 
> 
> On 2016年09月07日 14:50, Jaehoon Chung wrote:
>> On 09/07/2016 03:14 PM, Ziyuan Xu wrote:
>>> hi Jaehoon,
>>>
>>>
>>> On 2016年08月30日 17:54, Jaehoon Chung wrote:
 Hi Jacob,

 On 08/30/2016 02:26 AM, Jacob Chen wrote:
> From: "jacob2.chen" 
>
> The former implement have a bug.
> It will cause wrong data reading sometimes.
 Could you explain what bug is there?
> Signed-off-by: jacob2.chen 
 Could you change from jacob2.chen to your name?

> ---
>
>drivers/mmc/dw_mmc.c | 32 +---
>1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index afc674d..f072739 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host 
> *host, struct mmc_data *data)
>  if (host->fifo_mode && size) {
>len = 0;
> -if (data->flags == MMC_DATA_READ) {
> -if ((dwmci_readl(host, DWMCI_RINTSTS) &
> - DWMCI_INTMSK_RXDR)) {
> +if (data->flags == MMC_DATA_READ &&
> +(mask & DWMCI_INTMSK_RXDR)) {
> +while (size) {
>len = dwmci_readl(host, DWMCI_STATUS);
>len = (len >> DWMCI_FIFO_SHIFT) &
> -DWMCI_FIFO_MASK;
> +DWMCI_FIFO_MASK;
 this changing is related with bug?
>>> I just hit this bug on rk3036 board.
>> This changing is just an indentation, isn't?
>> It looks like unnecessary modifying.
>>
>>> When DTO interrupt occurred, there are any remaining data still in FIFO due 
>>> to RX FIFO threshold is larger than remaining data. It also causes that 
>>> dwmmc didn't trigger RXDR interrupt.
>>> It's responsibility of driver to read remaining bytes on seeing DTO 
>>> interrupt. So we need rework dwmci_data_transfer w/ pio mode.
>> Sure, your bug is possible to occur..but my mean is that modified 
>> DWMCI_FIFO_MASK isn't related with your entire bug.
> 
> Okay I see. If I understand correctly, you recommend that set fifo-depth to 
> 1, so that host can trigger RXDR interrupt as soon as 1 bytes data come in.
> I think it's inefficient.

Well, my meaning is " len = (len >> DWMCI_FIFO_SHIFT) & DWMCI_FIFO_MASK;"

> -DWMCI_FIFO_MASK;
> +DWMCI_FIFO_MASK;

It doesn't need to modify. There is no reason to change.

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>len = min(size, len);
>for (i = 0; i < len; i++)
>*buf++ =
> -dwmci_readl(host, DWMCI_DATA);
> -dwmci_writel(host, DWMCI_RINTSTS,
> - DWMCI_INTMSK_RXDR);
> +dwmci_readl(host,
> +DWMCI_DATA);
> +size = size > len ? (size - len) : 0;
>}
> -} else {
> -if ((dwmci_readl(host, DWMCI_RINTSTS) &
> - DWMCI_INTMSK_TXDR)) {
> +dwmci_writel(host, DWMCI_RINTSTS,
> + DWMCI_INTMSK_RXDR);
> +} else if (data->flags == MMC_DATA_WRITE &&
> +   (mask & DWMCI_INTMSK_TXDR)) {
 data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
 one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.

> +while (size) {
>len = dwmci_readl(host, DWMCI_STATUS);
>len = fifo_depth - ((len >>
> -   DWMCI_FIFO_SHIFT) &
> -   DWMCI_FIFO_MASK);
> + DWMCI_FIFO_SHIFT) &
> +DWMCI_FIFO_MASK);
 ditto.


> -   DWMCI_FIFO_SHIFT) &
> -   DWMCI_FIFO_MASK);
> + DWMCI_FIFO_SHIFT) &
> +DWMCI_FIFO_MASK);

Ditto.

Best Regards,
Jaehoon Chung


 Best Regards,
 Jaehoon Chung

>len = min(size, len);
>for (i = 0; i < len; i++)
>dwmci_writel(host, DWMCI_DATA,
> *buf++);
> -dwmci_writel(host, DWMCI_RINTSTS,
> - DWMCI_INTMSK_TXDR);
> +size = size > len ? (size - len) : 0;
>}
> +dwmci_writel(host, DWMCI_RINTSTS,
> +

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-08 Thread Ziyuan Xu




On 2016年09月07日 14:50, Jaehoon Chung wrote:

On 09/07/2016 03:14 PM, Ziyuan Xu wrote:

hi Jaehoon,


On 2016年08月30日 17:54, Jaehoon Chung wrote:

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:

From: "jacob2.chen" 

The former implement have a bug.
It will cause wrong data reading sometimes.

Could you explain what bug is there?

Signed-off-by: jacob2.chen 

Could you change from jacob2.chen to your name?


---

   drivers/mmc/dw_mmc.c | 32 +---
   1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index afc674d..f072739 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host *host, 
struct mmc_data *data)
 if (host->fifo_mode && size) {
   len = 0;
-if (data->flags == MMC_DATA_READ) {
-if ((dwmci_readl(host, DWMCI_RINTSTS) &
- DWMCI_INTMSK_RXDR)) {
+if (data->flags == MMC_DATA_READ &&
+(mask & DWMCI_INTMSK_RXDR)) {
+while (size) {
   len = dwmci_readl(host, DWMCI_STATUS);
   len = (len >> DWMCI_FIFO_SHIFT) &
-DWMCI_FIFO_MASK;
+DWMCI_FIFO_MASK;

this changing is related with bug?

I just hit this bug on rk3036 board.

This changing is just an indentation, isn't?
It looks like unnecessary modifying.


When DTO interrupt occurred, there are any remaining data still in FIFO due to 
RX FIFO threshold is larger than remaining data. It also causes that dwmmc 
didn't trigger RXDR interrupt.
It's responsibility of driver to read remaining bytes on seeing DTO interrupt. 
So we need rework dwmci_data_transfer w/ pio mode.

Sure, your bug is possible to occur..but my mean is that modified 
DWMCI_FIFO_MASK isn't related with your entire bug.


Okay I see. If I understand correctly, you recommend that set fifo-depth 
to 1, so that host can trigger RXDR interrupt as soon as 1 bytes data 
come in.

I think it's inefficient.



Best Regards,
Jaehoon Chung


   len = min(size, len);
   for (i = 0; i < len; i++)
   *buf++ =
-dwmci_readl(host, DWMCI_DATA);
-dwmci_writel(host, DWMCI_RINTSTS,
- DWMCI_INTMSK_RXDR);
+dwmci_readl(host,
+DWMCI_DATA);
+size = size > len ? (size - len) : 0;
   }
-} else {
-if ((dwmci_readl(host, DWMCI_RINTSTS) &
- DWMCI_INTMSK_TXDR)) {
+dwmci_writel(host, DWMCI_RINTSTS,
+ DWMCI_INTMSK_RXDR);
+} else if (data->flags == MMC_DATA_WRITE &&
+   (mask & DWMCI_INTMSK_TXDR)) {

data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.


+while (size) {
   len = dwmci_readl(host, DWMCI_STATUS);
   len = fifo_depth - ((len >>
-   DWMCI_FIFO_SHIFT) &
-   DWMCI_FIFO_MASK);
+ DWMCI_FIFO_SHIFT) &
+DWMCI_FIFO_MASK);

ditto.

Best Regards,
Jaehoon Chung


   len = min(size, len);
   for (i = 0; i < len; i++)
   dwmci_writel(host, DWMCI_DATA,
*buf++);
-dwmci_writel(host, DWMCI_RINTSTS,
- DWMCI_INTMSK_TXDR);
+size = size > len ? (size - len) : 0;
   }
+dwmci_writel(host, DWMCI_RINTSTS,
+ DWMCI_INTMSK_TXDR);
   }
-size = size > len ? (size - len) : 0;
   }
 /* Data arrived correctly. */


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot















___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-07 Thread Jaehoon Chung

On 09/07/2016 03:14 PM, Ziyuan Xu wrote:
> hi Jaehoon,
> 
> 
> On 2016年08月30日 17:54, Jaehoon Chung wrote:
>> Hi Jacob,
>>
>> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>>> From: "jacob2.chen" 
>>>
>>> The former implement have a bug.
>>> It will cause wrong data reading sometimes.
>> Could you explain what bug is there?
>>>
>>> Signed-off-by: jacob2.chen 
>> Could you change from jacob2.chen to your name?
>>
>>> ---
>>>
>>>   drivers/mmc/dw_mmc.c | 32 +---
>>>   1 file changed, 17 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>>> index afc674d..f072739 100644
>>> --- a/drivers/mmc/dw_mmc.c
>>> +++ b/drivers/mmc/dw_mmc.c
>>> @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host 
>>> *host, struct mmc_data *data)
>>> if (host->fifo_mode && size) {
>>>   len = 0;
>>> -if (data->flags == MMC_DATA_READ) {
>>> -if ((dwmci_readl(host, DWMCI_RINTSTS) &
>>> - DWMCI_INTMSK_RXDR)) {
>>> +if (data->flags == MMC_DATA_READ &&
>>> +(mask & DWMCI_INTMSK_RXDR)) {
>>> +while (size) {
>>>   len = dwmci_readl(host, DWMCI_STATUS);
>>>   len = (len >> DWMCI_FIFO_SHIFT) &
>>> -DWMCI_FIFO_MASK;
>>> +DWMCI_FIFO_MASK;
>> this changing is related with bug?
> 
> I just hit this bug on rk3036 board.

This changing is just an indentation, isn't?
It looks like unnecessary modifying.

> 
> When DTO interrupt occurred, there are any remaining data still in FIFO due 
> to RX FIFO threshold is larger than remaining data. It also causes that dwmmc 
> didn't trigger RXDR interrupt.
> It's responsibility of driver to read remaining bytes on seeing DTO 
> interrupt. So we need rework dwmci_data_transfer w/ pio mode.

Sure, your bug is possible to occur..but my mean is that modified 
DWMCI_FIFO_MASK isn't related with your entire bug.

Best Regards,
Jaehoon Chung

> 
>>
>>>   len = min(size, len);
>>>   for (i = 0; i < len; i++)
>>>   *buf++ =
>>> -dwmci_readl(host, DWMCI_DATA);
>>> -dwmci_writel(host, DWMCI_RINTSTS,
>>> - DWMCI_INTMSK_RXDR);
>>> +dwmci_readl(host,
>>> +DWMCI_DATA);
>>> +size = size > len ? (size - len) : 0;
>>>   }
>>> -} else {
>>> -if ((dwmci_readl(host, DWMCI_RINTSTS) &
>>> - DWMCI_INTMSK_TXDR)) {
>>> +dwmci_writel(host, DWMCI_RINTSTS,
>>> + DWMCI_INTMSK_RXDR);
>>> +} else if (data->flags == MMC_DATA_WRITE &&
>>> +   (mask & DWMCI_INTMSK_TXDR)) {
>> data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
>> one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.
>>
>>> +while (size) {
>>>   len = dwmci_readl(host, DWMCI_STATUS);
>>>   len = fifo_depth - ((len >>
>>> -   DWMCI_FIFO_SHIFT) &
>>> -   DWMCI_FIFO_MASK);
>>> + DWMCI_FIFO_SHIFT) &
>>> +DWMCI_FIFO_MASK);
>> ditto.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>   len = min(size, len);
>>>   for (i = 0; i < len; i++)
>>>   dwmci_writel(host, DWMCI_DATA,
>>>*buf++);
>>> -dwmci_writel(host, DWMCI_RINTSTS,
>>> - DWMCI_INTMSK_TXDR);
>>> +size = size > len ? (size - len) : 0;
>>>   }
>>> +dwmci_writel(host, DWMCI_RINTSTS,
>>> + DWMCI_INTMSK_TXDR);
>>>   }
>>> -size = size > len ? (size - len) : 0;
>>>   }
>>> /* Data arrived correctly. */
>>>
>> ___
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>>
> 
> 
> 
> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-09-07 Thread Ziyuan Xu


hi Jaehoon,


On 2016年08月30日 17:54, Jaehoon Chung wrote:

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:

From: "jacob2.chen" 

The former implement have a bug.
It will cause wrong data reading sometimes.

Could you explain what bug is there?


Signed-off-by: jacob2.chen 

Could you change from jacob2.chen to your name?


---

  drivers/mmc/dw_mmc.c | 32 +---
  1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index afc674d..f072739 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host *host, 
struct mmc_data *data)
  
  		if (host->fifo_mode && size) {

len = 0;
-   if (data->flags == MMC_DATA_READ) {
-   if ((dwmci_readl(host, DWMCI_RINTSTS) &
-DWMCI_INTMSK_RXDR)) {
+   if (data->flags == MMC_DATA_READ &&
+   (mask & DWMCI_INTMSK_RXDR)) {
+   while (size) {
len = dwmci_readl(host, DWMCI_STATUS);
len = (len >> DWMCI_FIFO_SHIFT) &
-   DWMCI_FIFO_MASK;
+   DWMCI_FIFO_MASK;

this changing is related with bug?


I just hit this bug on rk3036 board.

When DTO interrupt occurred, there are any remaining data still in FIFO 
due to RX FIFO threshold is larger than remaining data. It also causes 
that dwmmc didn't trigger RXDR interrupt.
It's responsibility of driver to read remaining bytes on seeing DTO 
interrupt. So we need rework dwmci_data_transfer w/ pio mode.





len = min(size, len);
for (i = 0; i < len; i++)
*buf++ =
-   dwmci_readl(host, DWMCI_DATA);
-   dwmci_writel(host, DWMCI_RINTSTS,
-DWMCI_INTMSK_RXDR);
+   dwmci_readl(host,
+   DWMCI_DATA);
+   size = size > len ? (size - len) : 0;
}
-   } else {
-   if ((dwmci_readl(host, DWMCI_RINTSTS) &
-DWMCI_INTMSK_TXDR)) {
+   dwmci_writel(host, DWMCI_RINTSTS,
+DWMCI_INTMSK_RXDR);
+   } else if (data->flags == MMC_DATA_WRITE &&
+  (mask & DWMCI_INTMSK_TXDR)) {

data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.


+   while (size) {
len = dwmci_readl(host, DWMCI_STATUS);
len = fifo_depth - ((len >>
-  DWMCI_FIFO_SHIFT) &
-  DWMCI_FIFO_MASK);
+DWMCI_FIFO_SHIFT) &
+   DWMCI_FIFO_MASK);

ditto.

Best Regards,
Jaehoon Chung


len = min(size, len);
for (i = 0; i < len; i++)
dwmci_writel(host, DWMCI_DATA,
 *buf++);
-   dwmci_writel(host, DWMCI_RINTSTS,
-DWMCI_INTMSK_TXDR);
+   size = size > len ? (size - len) : 0;
}
+   dwmci_writel(host, DWMCI_RINTSTS,
+DWMCI_INTMSK_TXDR);
}
-   size = size > len ? (size - len) : 0;
}
  
  		/* Data arrived correctly. */



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot






___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-08-30 Thread 陈豪

Hi  Ziyuan,

It was rk3036-kylin board.
This bug will make distro_boot failed.

2016-08-30 22:11 GMT+08:00 Ziyuan Xu :
>
>
> On 2016年08月30日 21:56, 陈豪 wrote:
>>
>> Hi jaehoon,
>>
>>
>> 2016-08-30 17:54 GMT+08:00 Jaehoon Chung :
>>>
>>> Hi Jacob,
>>>
>>> On 08/30/2016 02:26 AM, Jacob Chen wrote:

 From: "jacob2.chen" 

 The former implement have a bug.
 It will cause wrong data reading sometimes.
>>>
>>> Could you explain what bug is there?
>>
>> This bug affects data reading and make board fail to boot.
>> There will be some errors like,
>> "Warning - bad CRC, using the default environmen"
>
>
> It's not cause by mmc device, U-Boot will read env from media device, and do
> CRC checking.
>
>> "ERROR: Can 't read GPT header"
>
>
> Which board do you use? RK3288 use DMA-mode for dw_mmc, not pio. I'm
> interest in it, show me more information.
>
>
>>
>> Actually I am not very familiar with the MMC hardware and i don't know
>> why former implemen will cause this bug.
>> I just rewrite it according to the driver in kernel.
>>

 Signed-off-by: jacob2.chen 
>>>
>>> Could you change from jacob2.chen to your name?
>>>
 ---

   drivers/mmc/dw_mmc.c | 32 +---
   1 file changed, 17 insertions(+), 15 deletions(-)

 diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
 index afc674d..f072739 100644
 --- a/drivers/mmc/dw_mmc.c
 +++ b/drivers/mmc/dw_mmc.c
 @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host
 *host, struct mmc_data *data)

if (host->fifo_mode && size) {
len = 0;
 - if (data->flags == MMC_DATA_READ) {
 - if ((dwmci_readl(host, DWMCI_RINTSTS) &
 -  DWMCI_INTMSK_RXDR)) {
 + if (data->flags == MMC_DATA_READ &&
 + (mask & DWMCI_INTMSK_RXDR)) {
 + while (size) {
len = dwmci_readl(host,
 DWMCI_STATUS);
len = (len >> DWMCI_FIFO_SHIFT) &
 - DWMCI_FIFO_MASK;
 + DWMCI_FIFO_MASK;
>>>
>>> this changing is related with bug?
>>>
len = min(size, len);
for (i = 0; i < len; i++)
*buf++ =
 - dwmci_readl(host,
 DWMCI_DATA);
 - dwmci_writel(host, DWMCI_RINTSTS,
 -  DWMCI_INTMSK_RXDR);
 + dwmci_readl(host,
 +
 DWMCI_DATA);
 + size = size > len ? (size - len) :
 0;
}
 - } else {
 - if ((dwmci_readl(host, DWMCI_RINTSTS) &
 -  DWMCI_INTMSK_TXDR)) {
 + dwmci_writel(host, DWMCI_RINTSTS,
 +  DWMCI_INTMSK_RXDR);
 + } else if (data->flags == MMC_DATA_WRITE &&
 +(mask & DWMCI_INTMSK_TXDR)) {
>>>
>>> data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
>>> one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.
>>
>> The reason why i write it so strange is that it warning "Too many
>> leading tabs"
>>
 + while (size) {
len = dwmci_readl(host,
 DWMCI_STATUS);
len = fifo_depth - ((len >>
 -DWMCI_FIFO_SHIFT) &
 -DWMCI_FIFO_MASK);
 +
 DWMCI_FIFO_SHIFT) &
 +
 DWMCI_FIFO_MASK);
>>>
>>> ditto.
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
len = min(size, len);
for (i = 0; i < len; i++)
dwmci_writel(host,
 DWMCI_DATA,
 *buf++);
 - dwmci_writel(host, DWMCI_RINTSTS,
 -  DWMCI_INTMSK_TXDR);
 + size = size > len ? (size - len) :
 0;
}
 + dwmci_writel(host, DWMCI_RINTSTS,
 +

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-08-30 Thread Ziyuan Xu




On 2016年08月30日 21:56, 陈豪 wrote:

Hi jaehoon,


2016-08-30 17:54 GMT+08:00 Jaehoon Chung :

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:

From: "jacob2.chen" 

The former implement have a bug.
It will cause wrong data reading sometimes.

Could you explain what bug is there?

This bug affects data reading and make board fail to boot.
There will be some errors like,
"Warning - bad CRC, using the default environmen"


It's not cause by mmc device, U-Boot will read env from media device, 
and do CRC checking.



"ERROR: Can 't read GPT header"


Which board do you use? RK3288 use DMA-mode for dw_mmc, not pio. I'm 
interest in it, show me more information.




Actually I am not very familiar with the MMC hardware and i don't know
why former implemen will cause this bug.
I just rewrite it according to the driver in kernel.



Signed-off-by: jacob2.chen 

Could you change from jacob2.chen to your name?


---

  drivers/mmc/dw_mmc.c | 32 +---
  1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index afc674d..f072739 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host *host, 
struct mmc_data *data)

   if (host->fifo_mode && size) {
   len = 0;
- if (data->flags == MMC_DATA_READ) {
- if ((dwmci_readl(host, DWMCI_RINTSTS) &
-  DWMCI_INTMSK_RXDR)) {
+ if (data->flags == MMC_DATA_READ &&
+ (mask & DWMCI_INTMSK_RXDR)) {
+ while (size) {
   len = dwmci_readl(host, DWMCI_STATUS);
   len = (len >> DWMCI_FIFO_SHIFT) &
- DWMCI_FIFO_MASK;
+ DWMCI_FIFO_MASK;

this changing is related with bug?


   len = min(size, len);
   for (i = 0; i < len; i++)
   *buf++ =
- dwmci_readl(host, DWMCI_DATA);
- dwmci_writel(host, DWMCI_RINTSTS,
-  DWMCI_INTMSK_RXDR);
+ dwmci_readl(host,
+ DWMCI_DATA);
+ size = size > len ? (size - len) : 0;
   }
- } else {
- if ((dwmci_readl(host, DWMCI_RINTSTS) &
-  DWMCI_INTMSK_TXDR)) {
+ dwmci_writel(host, DWMCI_RINTSTS,
+  DWMCI_INTMSK_RXDR);
+ } else if (data->flags == MMC_DATA_WRITE &&
+(mask & DWMCI_INTMSK_TXDR)) {

data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.

The reason why i write it so strange is that it warning "Too many
leading tabs"


+ while (size) {
   len = dwmci_readl(host, DWMCI_STATUS);
   len = fifo_depth - ((len >>
-DWMCI_FIFO_SHIFT) &
-DWMCI_FIFO_MASK);
+  DWMCI_FIFO_SHIFT) &
+ DWMCI_FIFO_MASK);

ditto.

Best Regards,
Jaehoon Chung


   len = min(size, len);
   for (i = 0; i < len; i++)
   dwmci_writel(host, DWMCI_DATA,
*buf++);
- dwmci_writel(host, DWMCI_RINTSTS,
-  DWMCI_INTMSK_TXDR);
+ size = size > len ? (size - len) : 0;
   }
+ dwmci_writel(host, DWMCI_RINTSTS,
+  DWMCI_INTMSK_TXDR);
   }
- size = size > len ? (size - len) : 0;
   }

   /* Data arrived correctly. */


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-08-30 Thread 陈豪

Hi jaehoon,


2016-08-30 17:54 GMT+08:00 Jaehoon Chung :
> Hi Jacob,
>
> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>> From: "jacob2.chen" 
>>
>> The former implement have a bug.
>> It will cause wrong data reading sometimes.
>
> Could you explain what bug is there?

This bug affects data reading and make board fail to boot.
There will be some errors like,
"Warning - bad CRC, using the default environmen"
"ERROR: Can 't read GPT header"

Actually I am not very familiar with the MMC hardware and i don't know
why former implemen will cause this bug.
I just rewrite it according to the driver in kernel.

>>
>>
>> Signed-off-by: jacob2.chen 
>
> Could you change from jacob2.chen to your name?
>
>> ---
>>
>>  drivers/mmc/dw_mmc.c | 32 +---
>>  1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>> index afc674d..f072739 100644
>> --- a/drivers/mmc/dw_mmc.c
>> +++ b/drivers/mmc/dw_mmc.c
>> @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host 
>> *host, struct mmc_data *data)
>>
>>   if (host->fifo_mode && size) {
>>   len = 0;
>> - if (data->flags == MMC_DATA_READ) {
>> - if ((dwmci_readl(host, DWMCI_RINTSTS) &
>> -  DWMCI_INTMSK_RXDR)) {
>> + if (data->flags == MMC_DATA_READ &&
>> + (mask & DWMCI_INTMSK_RXDR)) {
>> + while (size) {
>>   len = dwmci_readl(host, DWMCI_STATUS);
>>   len = (len >> DWMCI_FIFO_SHIFT) &
>> - DWMCI_FIFO_MASK;
>> + DWMCI_FIFO_MASK;
>
> this changing is related with bug?
>
>>   len = min(size, len);
>>   for (i = 0; i < len; i++)
>>   *buf++ =
>> - dwmci_readl(host, DWMCI_DATA);
>> - dwmci_writel(host, DWMCI_RINTSTS,
>> -  DWMCI_INTMSK_RXDR);
>> + dwmci_readl(host,
>> + DWMCI_DATA);
>> + size = size > len ? (size - len) : 0;
>>   }
>> - } else {
>> - if ((dwmci_readl(host, DWMCI_RINTSTS) &
>> -  DWMCI_INTMSK_TXDR)) {
>> + dwmci_writel(host, DWMCI_RINTSTS,
>> +  DWMCI_INTMSK_RXDR);
>> + } else if (data->flags == MMC_DATA_WRITE &&
>> +(mask & DWMCI_INTMSK_TXDR)) {
>
> data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
> one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.

The reason why i write it so strange is that it warning "Too many
leading tabs"

>
>> + while (size) {
>>   len = dwmci_readl(host, DWMCI_STATUS);
>>   len = fifo_depth - ((len >>
>> -DWMCI_FIFO_SHIFT) &
>> -DWMCI_FIFO_MASK);
>> +  DWMCI_FIFO_SHIFT) 
>> &
>> + DWMCI_FIFO_MASK);
>
> ditto.
>
> Best Regards,
> Jaehoon Chung
>
>>   len = min(size, len);
>>   for (i = 0; i < len; i++)
>>   dwmci_writel(host, DWMCI_DATA,
>>*buf++);
>> - dwmci_writel(host, DWMCI_RINTSTS,
>> -  DWMCI_INTMSK_TXDR);
>> + size = size > len ? (size - len) : 0;
>>   }
>> + dwmci_writel(host, DWMCI_RINTSTS,
>> +  DWMCI_INTMSK_TXDR);
>>   }
>> - size = size > len ? (size - len) : 0;
>>   }
>>
>>   /* Data arrived correctly. */
>>
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

2016-08-30 Thread Jaehoon Chung

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:
> From: "jacob2.chen" 
> 
> The former implement have a bug.
> It will cause wrong data reading sometimes.

Could you explain what bug is there?
> 
> 
> Signed-off-by: jacob2.chen 

Could you change from jacob2.chen to your name?

> ---
> 
>  drivers/mmc/dw_mmc.c | 32 +---
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index afc674d..f072739 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -120,35 +120,37 @@ static int dwmci_data_transfer(struct dwmci_host *host, 
> struct mmc_data *data)
>  
>   if (host->fifo_mode && size) {
>   len = 0;
> - if (data->flags == MMC_DATA_READ) {
> - if ((dwmci_readl(host, DWMCI_RINTSTS) &
> -  DWMCI_INTMSK_RXDR)) {
> + if (data->flags == MMC_DATA_READ &&
> + (mask & DWMCI_INTMSK_RXDR)) {
> + while (size) {
>   len = dwmci_readl(host, DWMCI_STATUS);
>   len = (len >> DWMCI_FIFO_SHIFT) &
> - DWMCI_FIFO_MASK;
> + DWMCI_FIFO_MASK;

this changing is related with bug?

>   len = min(size, len);
>   for (i = 0; i < len; i++)
>   *buf++ =
> - dwmci_readl(host, DWMCI_DATA);
> - dwmci_writel(host, DWMCI_RINTSTS,
> -  DWMCI_INTMSK_RXDR);
> + dwmci_readl(host,
> + DWMCI_DATA);
> + size = size > len ? (size - len) : 0;
>   }
> - } else {
> - if ((dwmci_readl(host, DWMCI_RINTSTS) &
> -  DWMCI_INTMSK_TXDR)) {
> + dwmci_writel(host, DWMCI_RINTSTS,
> +  DWMCI_INTMSK_RXDR);
> + } else if (data->flags == MMC_DATA_WRITE &&
> +(mask & DWMCI_INTMSK_TXDR)) {

data->flags == MMC_DATA_WRITE doesn't need..flags are only two..
one is MMC_DATA_READ, otherwise it's MMC_DATA_WRITE.

> + while (size) {
>   len = dwmci_readl(host, DWMCI_STATUS);
>   len = fifo_depth - ((len >>
> -DWMCI_FIFO_SHIFT) &
> -DWMCI_FIFO_MASK);
> +  DWMCI_FIFO_SHIFT) &
> + DWMCI_FIFO_MASK);

ditto.

Best Regards,
Jaehoon Chung

>   len = min(size, len);
>   for (i = 0; i < len; i++)
>   dwmci_writel(host, DWMCI_DATA,
>*buf++);
> - dwmci_writel(host, DWMCI_RINTSTS,
> -  DWMCI_INTMSK_TXDR);
> + size = size > len ? (size - len) : 0;
>   }
> + dwmci_writel(host, DWMCI_RINTSTS,
> +  DWMCI_INTMSK_TXDR);
>   }
> - size = size > len ? (size - len) : 0;
>   }
>  
>   /* Data arrived correctly. */
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

Re: [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode

10 matches

Site Navigation

Mail list logo

Footer information