All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
@ 2016-08-29 17:26 ` Jacob Chen
  2016-08-29 17:26   ` [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036 Jacob Chen
                     ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Jacob Chen @ 2016-08-29 17:26 UTC (permalink / raw)
  To: u-boot

From: "jacob2.chen" <jacob2.chen@rock-chips.com>

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


Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
---

 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;
 					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)) {
+				while (size) {
 					len = dwmci_readl(host, DWMCI_STATUS);
 					len = fifo_depth - ((len >>
-						   DWMCI_FIFO_SHIFT) &
-						   DWMCI_FIFO_MASK);
+							     DWMCI_FIFO_SHIFT) &
+							    DWMCI_FIFO_MASK);
 					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. */
-- 
2.7.4

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

* [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036
  2016-08-29 17:26 ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jacob Chen
@ 2016-08-29 17:26   ` Jacob Chen
  2016-09-06  1:03     ` Simon Glass
  2016-08-29 17:26   ` [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map Jacob Chen
  2016-08-30  9:54   ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jaehoon Chung
  2 siblings, 1 reply; 17+ messages in thread
From: Jacob Chen @ 2016-08-29 17:26 UTC (permalink / raw)
  To: u-boot

From: "jacob2.chen" <jacob2.chen@rock-chips.com>

Enable ums feature for rk3036 boards, so that we can mount the mmc
device to PC.

Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
---

 include/configs/rk3036_common.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h
index 21d4683..dea4acb 100644
--- a/include/configs/rk3036_common.h
+++ b/include/configs/rk3036_common.h
@@ -74,6 +74,10 @@
 #define CONFIG_FASTBOOT_BUF_ADDR	CONFIG_SYS_LOAD_ADDR
 #define CONFIG_FASTBOOT_BUF_SIZE	0x08000000
 
+/* usb mass storage */
+#define CONFIG_USB_FUNCTION_MASS_STORAGE
+#define CONFIG_CMD_USB_MASS_STORAGE
+
 #define CONFIG_USB_GADGET_DOWNLOAD
 #define CONFIG_G_DNL_MANUFACTURER	"Rockchip"
 #define CONFIG_G_DNL_VENDOR_NUM		0x2207
-- 
2.7.4

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

* [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map
  2016-08-29 17:26 ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jacob Chen
  2016-08-29 17:26   ` [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036 Jacob Chen
@ 2016-08-29 17:26   ` Jacob Chen
  2016-09-06  1:03     ` Simon Glass
  2016-08-30  9:54   ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jaehoon Chung
  2 siblings, 1 reply; 17+ messages in thread
From: Jacob Chen @ 2016-08-29 17:26 UTC (permalink / raw)
  To: u-boot

From: "jacob2.chen" <jacob2.chen@rock-chips.com>

Below link contains documents about rockchip linux partitions.
http://rockchip.wikidot.com/partitions

Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
---

 include/configs/kylin_rk3036.h  | 25 +++++++++----------------
 include/configs/rk3288_common.h |  7 ++++++-
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h
index e8ca76d..e08654b 100644
--- a/include/configs/kylin_rk3036.h
+++ b/include/configs/kylin_rk3036.h
@@ -18,7 +18,6 @@
 #undef CONFIG_ENV_IS_NOWHERE
 #define CONFIG_ENV_IS_IN_MMC
 #define CONFIG_SYS_MMC_ENV_DEV		0 /* emmc */
-#define CONFIG_SYS_MMC_ENV_PART		0 /* user area */
 #define CONFIG_ENV_OFFSET		(SZ_4M - SZ_64K) /* reserved area */
 #define CONFIG_ENV_OFFSET_REDUND	(CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE)
 #define CONFIG_SYS_REDUNDAND_ENVIRONMENT
@@ -28,25 +27,19 @@
 #define CONFIG_RANDOM_UUID
 #define PARTS_DEFAULT \
 	"uuid_disk=${uuid_gpt_disk};" \
-	"name=loader,start=32K,size=4000K,uuid=${uuid_gpt_loader};" \
-	"name=reserved,size=64K,uuid=${uuid_gpt_reserved};" \
-	"name=misc,size=4M,uuid=${uuid_gpt_misc};" \
-	"name=recovery,size=32M,uuid=${uuid_gpt_recovery};" \
-	"name=boot_a,size=32M,uuid=${uuid_gpt_boot_a};" \
-	"name=boot_b,size=32M,uuid=${uuid_gpt_boot_b};" \
-	"name=system_a,size=818M,uuid=${uuid_gpt_system_a};" \
-	"name=system_b,size=818M,uuid=${uuid_gpt_system_b};" \
-	"name=vendor_a,size=50M,uuid=${uuid_gpt_vendor_a};" \
-	"name=vendor_b,size=50M,uuid=${uuid_gpt_vendor_b};" \
-	"name=cache,size=100M,uuid=${uuid_gpt_cache};" \
-	"name=metadata,size=16M,uuid=${uuid_gpt_metadata};" \
-	"name=persist,size=4M,uuid=${uuid_gpt_persist};" \
-	"name=userdata,size=-,uuid=${uuid_gpt_userdata};\0" \
+	"name=loader1,start=32K,size=4000K,uuid=${uuid_gpt_loader1};" \
+	"name=reserved1,size=64K,uuid=${uuid_gpt_reserved1};" \
+	"name=reserved2,size=4M,uuid=${uuid_gpt_reserved2};" \
+	"name=loader2,size=4MB,uuid=${uuid_gpt_loader2};" \
+	"name=atf,size=4M,uuid=${uuid_gpt_atf};" \
+	"name=boot,size=128M,bootable,uuid=${uuid_gpt_boot};" \
+	"name=rootfs,size=-,uuid=${uuid_gpt_rootfs};\0" \
 
 #undef CONFIG_EXTRA_ENV_SETTINGS
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	"partitions=" PARTS_DEFAULT \
-
+	ENV_MEM_LAYOUT_SETTINGS \
+	BOOTENV
 #endif
 
 #define CONFIG_BOARD_LATE_INIT
diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
index d3d4c68..cf5ab59 100644
--- a/include/configs/rk3288_common.h
+++ b/include/configs/rk3288_common.h
@@ -120,7 +120,12 @@
 #define CONFIG_RANDOM_UUID
 #define PARTS_DEFAULT \
 	"uuid_disk=${uuid_gpt_disk};" \
-	"name=boot,start=8M,size=64M,bootable,uuid=${uuid_gpt_boot};" \
+	"name=loader1,start=32K,size=4000K,uuid=${uuid_gpt_loader1};" \
+	"name=reserved1,size=64K,uuid=${uuid_gpt_reserved1};" \
+	"name=reserved2,size=4M,uuid=${uuid_gpt_reserved2};" \
+	"name=loader2,size=4MB,uuid=${uuid_gpt_loader2};" \
+	"name=atf,size=4M,uuid=${uuid_gpt_atf};" \
+	"name=boot,size=128M,bootable,uuid=${uuid_gpt_boot};" \
 	"name=rootfs,size=-,uuid=${uuid_gpt_rootfs};\0" \
 
 /* First try to boot from SD (index 0), then eMMC (index 1 */
-- 
2.7.4

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-08-29 17:26 ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jacob Chen
  2016-08-29 17:26   ` [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036 Jacob Chen
  2016-08-29 17:26   ` [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map Jacob Chen
@ 2016-08-30  9:54   ` Jaehoon Chung
  2016-08-30 13:56     ` 陈豪
  2016-09-07  6:14     ` Ziyuan Xu
  2 siblings, 2 replies; 17+ messages in thread
From: Jaehoon Chung @ 2016-08-30  9:54 UTC (permalink / raw)
  To: u-boot

Hi Jacob,

On 08/30/2016 02:26 AM, Jacob Chen wrote:
> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
> 
> 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 <jacob2.chen@rock-chips.com>

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. */
> 

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-08-30  9:54   ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jaehoon Chung
@ 2016-08-30 13:56     ` 陈豪
  2016-08-30 14:11       ` Ziyuan Xu
  2016-09-07  6:14     ` Ziyuan Xu
  1 sibling, 1 reply; 17+ messages in thread
From: 陈豪 @ 2016-08-30 13:56 UTC (permalink / raw)
  To: u-boot

Hi jaehoon,


2016-08-30 17:54 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
> Hi Jacob,
>
> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>>
>> 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 <jacob2.chen@rock-chips.com>
>
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-08-30 13:56     ` 陈豪
@ 2016-08-30 14:11       ` Ziyuan Xu
  2016-08-30 15:59         ` 陈豪
  0 siblings, 1 reply; 17+ messages in thread
From: Ziyuan Xu @ 2016-08-30 14:11 UTC (permalink / raw)
  To: u-boot



On 2016?08?30? 21:56, ?? wrote:
> Hi jaehoon,
>
>
> 2016-08-30 17:54 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
>> Hi Jacob,
>>
>> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>>> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>>>
>>> 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 <jacob2.chen@rock-chips.com>
>> 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
>

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-08-30 14:11       ` Ziyuan Xu
@ 2016-08-30 15:59         ` 陈豪
  0 siblings, 0 replies; 17+ messages in thread
From: 陈豪 @ 2016-08-30 15:59 UTC (permalink / raw)
  To: u-boot

Hi  Ziyuan,

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

2016-08-30 22:11 GMT+08:00 Ziyuan Xu <xzy.xu@rock-chips.com>:
>
>
> On 2016?08?30? 21:56, ?? wrote:
>>
>> Hi jaehoon,
>>
>>
>> 2016-08-30 17:54 GMT+08:00 Jaehoon Chung <jh80.chung@samsung.com>:
>>>
>>> Hi Jacob,
>>>
>>> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>>>>
>>>> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>>>>
>>>> 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 <jacob2.chen@rock-chips.com>
>>>
>>> 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 at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>>
>
>

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

* [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036
  2016-08-29 17:26   ` [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036 Jacob Chen
@ 2016-09-06  1:03     ` Simon Glass
  2016-09-09  2:59       ` 陈豪
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2016-09-06  1:03 UTC (permalink / raw)
  To: u-boot

On 29 August 2016 at 11:26, Jacob Chen <jacob2.chen@rock-chips.com> wrote:
> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>
> Enable ums feature for rk3036 boards, so that we can mount the mmc
> device to PC.
>
> Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
> ---
>
>  include/configs/rk3036_common.h | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Simon Glass <sjg@chromium.org>

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

* [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map
  2016-08-29 17:26   ` [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map Jacob Chen
@ 2016-09-06  1:03     ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2016-09-06  1:03 UTC (permalink / raw)
  To: u-boot

Hi,

On 29 August 2016 at 11:26, Jacob Chen <jacob2.chen@rock-chips.com> wrote:
> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>
> Below link contains documents about rockchip linux partitions.
> http://rockchip.wikidot.com/partitions
>

Please add to the commit message some info about why this change is needed.

Please can you update README.rockchip with this info and the link.

Regards,
Simon

> Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
> ---
>
>  include/configs/kylin_rk3036.h  | 25 +++++++++----------------
>  include/configs/rk3288_common.h |  7 ++++++-
>  2 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h
> index e8ca76d..e08654b 100644
> --- a/include/configs/kylin_rk3036.h
> +++ b/include/configs/kylin_rk3036.h
> @@ -18,7 +18,6 @@
>  #undef CONFIG_ENV_IS_NOWHERE
>  #define CONFIG_ENV_IS_IN_MMC
>  #define CONFIG_SYS_MMC_ENV_DEV         0 /* emmc */
> -#define CONFIG_SYS_MMC_ENV_PART                0 /* user area */
>  #define CONFIG_ENV_OFFSET              (SZ_4M - SZ_64K) /* reserved area */
>  #define CONFIG_ENV_OFFSET_REDUND       (CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE)
>  #define CONFIG_SYS_REDUNDAND_ENVIRONMENT
> @@ -28,25 +27,19 @@
>  #define CONFIG_RANDOM_UUID
>  #define PARTS_DEFAULT \
>         "uuid_disk=${uuid_gpt_disk};" \
> -       "name=loader,start=32K,size=4000K,uuid=${uuid_gpt_loader};" \
> -       "name=reserved,size=64K,uuid=${uuid_gpt_reserved};" \
> -       "name=misc,size=4M,uuid=${uuid_gpt_misc};" \
> -       "name=recovery,size=32M,uuid=${uuid_gpt_recovery};" \
> -       "name=boot_a,size=32M,uuid=${uuid_gpt_boot_a};" \
> -       "name=boot_b,size=32M,uuid=${uuid_gpt_boot_b};" \
> -       "name=system_a,size=818M,uuid=${uuid_gpt_system_a};" \
> -       "name=system_b,size=818M,uuid=${uuid_gpt_system_b};" \
> -       "name=vendor_a,size=50M,uuid=${uuid_gpt_vendor_a};" \
> -       "name=vendor_b,size=50M,uuid=${uuid_gpt_vendor_b};" \
> -       "name=cache,size=100M,uuid=${uuid_gpt_cache};" \
> -       "name=metadata,size=16M,uuid=${uuid_gpt_metadata};" \
> -       "name=persist,size=4M,uuid=${uuid_gpt_persist};" \
> -       "name=userdata,size=-,uuid=${uuid_gpt_userdata};\0" \
> +       "name=loader1,start=32K,size=4000K,uuid=${uuid_gpt_loader1};" \
> +       "name=reserved1,size=64K,uuid=${uuid_gpt_reserved1};" \
> +       "name=reserved2,size=4M,uuid=${uuid_gpt_reserved2};" \
> +       "name=loader2,size=4MB,uuid=${uuid_gpt_loader2};" \
> +       "name=atf,size=4M,uuid=${uuid_gpt_atf};" \
> +       "name=boot,size=128M,bootable,uuid=${uuid_gpt_boot};" \
> +       "name=rootfs,size=-,uuid=${uuid_gpt_rootfs};\0" \
>
>  #undef CONFIG_EXTRA_ENV_SETTINGS
>  #define CONFIG_EXTRA_ENV_SETTINGS \
>         "partitions=" PARTS_DEFAULT \
> -
> +       ENV_MEM_LAYOUT_SETTINGS \
> +       BOOTENV
>  #endif
>
>  #define CONFIG_BOARD_LATE_INIT
> diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
> index d3d4c68..cf5ab59 100644
> --- a/include/configs/rk3288_common.h
> +++ b/include/configs/rk3288_common.h
> @@ -120,7 +120,12 @@
>  #define CONFIG_RANDOM_UUID
>  #define PARTS_DEFAULT \
>         "uuid_disk=${uuid_gpt_disk};" \
> -       "name=boot,start=8M,size=64M,bootable,uuid=${uuid_gpt_boot};" \
> +       "name=loader1,start=32K,size=4000K,uuid=${uuid_gpt_loader1};" \
> +       "name=reserved1,size=64K,uuid=${uuid_gpt_reserved1};" \
> +       "name=reserved2,size=4M,uuid=${uuid_gpt_reserved2};" \
> +       "name=loader2,size=4MB,uuid=${uuid_gpt_loader2};" \
> +       "name=atf,size=4M,uuid=${uuid_gpt_atf};" \
> +       "name=boot,size=128M,bootable,uuid=${uuid_gpt_boot};" \
>         "name=rootfs,size=-,uuid=${uuid_gpt_rootfs};\0" \
>
>  /* First try to boot from SD (index 0), then eMMC (index 1 */
> --
> 2.7.4
>

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-08-30  9:54   ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jaehoon Chung
  2016-08-30 13:56     ` 陈豪
@ 2016-09-07  6:14     ` Ziyuan Xu
  2016-09-07  6:50       ` Jaehoon Chung
  1 sibling, 1 reply; 17+ messages in thread
From: Ziyuan Xu @ 2016-09-07  6:14 UTC (permalink / raw)
  To: u-boot

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" <jacob2.chen@rock-chips.com>
>>
>> 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 <jacob2.chen@rock-chips.com>
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
>

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-09-07  6:14     ` Ziyuan Xu
@ 2016-09-07  6:50       ` Jaehoon Chung
  2016-09-08  3:43         ` Ziyuan Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Jaehoon Chung @ 2016-09-07  6:50 UTC (permalink / raw)
  To: u-boot

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" <jacob2.chen@rock-chips.com>
>>>
>>> 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 <jacob2.chen@rock-chips.com>
>> 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>>
> 
> 
> 
> 
> 

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-09-07  6:50       ` Jaehoon Chung
@ 2016-09-08  3:43         ` Ziyuan Xu
  2016-09-08  3:54           ` Jaehoon Chung
  0 siblings, 1 reply; 17+ messages in thread
From: Ziyuan Xu @ 2016-09-08  3:43 UTC (permalink / raw)
  To: u-boot



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" <jacob2.chen@rock-chips.com>
>>>>
>>>> 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 <jacob2.chen@rock-chips.com>
>>> 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 at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>
>>>
>>>
>>
>>
>>
>>
>
>
>

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-09-08  3:43         ` Ziyuan Xu
@ 2016-09-08  3:54           ` Jaehoon Chung
  2016-09-23  2:39             ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: Jaehoon Chung @ 2016-09-08  3:54 UTC (permalink / raw)
  To: u-boot

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" <jacob2.chen@rock-chips.com>
>>>>>
>>>>> 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 <jacob2.chen@rock-chips.com>
>>>> 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,
>>>>> +                         DWMCI_INTMSK_TXDR);
>>>>>                }
>>>>> -            size = size > len ? (size - len) : 0;
>>>>>            }
>>>>>              /* Data arrived correctly. */
>>>>>
>>>> _______________________________________________
>>>> U-Boot mailing list
>>>> U-Boot at lists.denx.de
>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>>
>>
> 
> 
> 
> 
> 

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

* [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036
  2016-09-06  1:03     ` Simon Glass
@ 2016-09-09  2:59       ` 陈豪
  2016-09-24  0:03         ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: 陈豪 @ 2016-09-09  2:59 UTC (permalink / raw)
  To: u-boot

Hi,


2016-09-06 9:03 GMT+08:00 Simon Glass <sjg@chromium.org>:
> On 29 August 2016 at 11:26, Jacob Chen <jacob2.chen@rock-chips.com> wrote:
>> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>>
>> Enable ums feature for rk3036 boards, so that we can mount the mmc
>> device to PC.
>>
>> Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
>> ---
>>
>>  include/configs/rk3036_common.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>
> Acked-by: Simon Glass <sjg@chromium.org>

This patch has no relations with other patchs.
Can we merge it alone?


> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-09-08  3:54           ` Jaehoon Chung
@ 2016-09-23  2:39             ` Simon Glass
  2016-09-23  2:51               ` Ziyuan Xu
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2016-09-23  2:39 UTC (permalink / raw)
  To: u-boot

Hi,

On 7 September 2016 at 21:54, Jaehoon Chung <jh80.chung@samsung.com> 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" <jacob2.chen@rock-chips.com>
>>>>>>
>>>>>> 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 <jacob2.chen@rock-chips.com>
>>>>> 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

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

* [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode
  2016-09-23  2:39             ` Simon Glass
@ 2016-09-23  2:51               ` Ziyuan Xu
  0 siblings, 0 replies; 17+ messages in thread
From: Ziyuan Xu @ 2016-09-23  2:51 UTC (permalink / raw)
  To: u-boot

hi Simon,


On 2016?09?23? 10:39, Simon Glass wrote:
> Hi,
>
> On 7 September 2016 at 21:54, Jaehoon Chung <jh80.chung@samsung.com> 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" <jacob2.chen@rock-chips.com>
>>>>>>>
>>>>>>> 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 <jacob2.chen@rock-chips.com>
>>>>>> 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
>
>
>

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

* [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036
  2016-09-09  2:59       ` 陈豪
@ 2016-09-24  0:03         ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2016-09-24  0:03 UTC (permalink / raw)
  To: u-boot

On 8 September 2016 at 20:59, ?? <jacobchen110@gmail.com> wrote:
> Hi,
>
>
> 2016-09-06 9:03 GMT+08:00 Simon Glass <sjg@chromium.org>:
>> On 29 August 2016 at 11:26, Jacob Chen <jacob2.chen@rock-chips.com> wrote:
>>> From: "jacob2.chen" <jacob2.chen@rock-chips.com>
>>>
>>> Enable ums feature for rk3036 boards, so that we can mount the mmc
>>> device to PC.
>>>
>>> Signed-off-by: jacob2.chen <jacob2.chen@rock-chips.com>
>>> ---
>>>
>>>  include/configs/rk3036_common.h | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>
> This patch has no relations with other patchs.
> Can we merge it alone?

Applied to u-boot-rockchip, thanks!

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

end of thread, other threads:[~2016-09-24  0:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20160829172627epcas1p3be811bc3802eca452e5fa577fe191cad@epcas1p3.samsung.com>
2016-08-29 17:26 ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jacob Chen
2016-08-29 17:26   ` [U-Boot] [PATCH 2/3] rockchip: add usb mass storage feature support for rk3036 Jacob Chen
2016-09-06  1:03     ` Simon Glass
2016-09-09  2:59       ` 陈豪
2016-09-24  0:03         ` Simon Glass
2016-08-29 17:26   ` [U-Boot] [PATCH 3/3] rockchip: use rockchip linux partitions map Jacob Chen
2016-09-06  1:03     ` Simon Glass
2016-08-30  9:54   ` [U-Boot] [PATCH 1/3] mmc: dw_mmc: change the read/write order under fifo mode Jaehoon Chung
2016-08-30 13:56     ` 陈豪
2016-08-30 14:11       ` Ziyuan Xu
2016-08-30 15:59         ` 陈豪
2016-09-07  6:14     ` Ziyuan Xu
2016-09-07  6:50       ` Jaehoon Chung
2016-09-08  3:43         ` Ziyuan Xu
2016-09-08  3:54           ` Jaehoon Chung
2016-09-23  2:39             ` Simon Glass
2016-09-23  2:51               ` Ziyuan Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.