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

[陈豪]

Hi jaehoon,


2016-08-30 17:54 GMT+08:00 Jaehoon Chung <jh80.chung at samsung.com>:
> Hi Jacob,
>
> On 08/30/2016 02:26 AM, Jacob Chen wrote:
>> From: "jacob2.chen" <jacob2.chen at 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 at 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