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

[陈豪]

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 at rock-chips.com>:
>
>
> On 2016年08月30日 21:56, 陈豪 wrote:
>>
>> 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"
>
>
> 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 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
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>>
>>
>
>