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.c...@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.c...@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@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