| Message ID | 1369314900-16412-1-git-send-email-rajeshwari.s@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi Rajeshwari On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde <rajeshwari.s@samsung.com > wrote: > Current DWMMC driver used to give FIFO underrun/overrun error every 3rd > time > for mmc rescan command. > In current code FIFO_DEPTH is getting calculated after reading the FIFOTH > register and extracting the RX_WMARK bits from it i.e (RX_WMARK = > FIFO_DEPTH/2 -1). > Instead of storing the correct value, we were recalculating the FIFO_DEPT > each > time which is not correct. > > Signed-off-by: Hatim Ali <hatim.rv@samsung.com> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> > --- > drivers/mmc/dw_mmc.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 4070d4e..be590a4 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) > dwmci_writel(host, DWMCI_BMOD, 1); > > fifo_size = dwmci_readl(host, DWMCI_FIFOTH); > - if (host->fifoth_val) > + if (host->fifoth_val) { > fifoth_val = host->fifoth_val; > - else > + } else { > fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | > TX_WMARK(fifo_size/2); > + host->fifoth_val = fifoth_val; > + } > Looks Good to me. Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com> > dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); > > dwmci_writel(host, DWMCI_CLKENA, 0); > -- > 1.7.4.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde <rajeshwari.s@samsung.com> wrote: > Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time > for mmc rescan command. > In current code FIFO_DEPTH is getting calculated after reading the FIFOTH > register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). > Instead of storing the correct value, we were recalculating the FIFO_DEPT each > time which is not correct. > > Signed-off-by: Hatim Ali <hatim.rv@samsung.com> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> > --- > drivers/mmc/dw_mmc.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 4070d4e..be590a4 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) > dwmci_writel(host, DWMCI_BMOD, 1); > > fifo_size = dwmci_readl(host, DWMCI_FIFOTH); > - if (host->fifoth_val) > + if (host->fifoth_val) { What is the inital value for host->fifoth_val, for the first time call.? > fifoth_val = host->fifoth_val; > - else > + } else { > fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | > TX_WMARK(fifo_size/2); > + host->fifoth_val = fifoth_val; > + } > dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); > > dwmci_writel(host, DWMCI_CLKENA, 0); > -- > 1.7.4.4 > Thanks, Jagan.
On 05/24/2013 03:27 AM, Jagan Teki wrote: > On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde > <rajeshwari.s@samsung.com> wrote: >> Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time >> for mmc rescan command. >> In current code FIFO_DEPTH is getting calculated after reading the FIFOTH >> register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). >> Instead of storing the correct value, we were recalculating the FIFO_DEPT each >> time which is not correct. >> >> Signed-off-by: Hatim Ali <hatim.rv@samsung.com> >> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >> --- >> drivers/mmc/dw_mmc.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >> index 4070d4e..be590a4 100644 >> --- a/drivers/mmc/dw_mmc.c >> +++ b/drivers/mmc/dw_mmc.c >> @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) >> dwmci_writel(host, DWMCI_BMOD, 1); >> >> fifo_size = dwmci_readl(host, DWMCI_FIFOTH); >> - if (host->fifoth_val) >> + if (host->fifoth_val) { > > What is the inital value for host->fifoth_val, for the first time call.? It should be set into board-specific file(dw-mmc_exynos.c) or others. Best Regards, Jaehoon Chung > >> fifoth_val = host->fifoth_val; >> - else >> + } else { >> fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | >> TX_WMARK(fifo_size/2); >> + host->fifoth_val = fifoth_val; >> + } >> dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); >> >> dwmci_writel(host, DWMCI_CLKENA, 0); >> -- >> 1.7.4.4 >> > > Thanks, > Jagan. >
Acked-by: Jaehoon Chung <jh80.chung@samsung.com> On 05/23/2013 10:15 PM, Rajeshwari Shinde wrote: > Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time > for mmc rescan command. > In current code FIFO_DEPTH is getting calculated after reading the FIFOTH > register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). > Instead of storing the correct value, we were recalculating the FIFO_DEPT each > time which is not correct. > > Signed-off-by: Hatim Ali <hatim.rv@samsung.com> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> > --- > drivers/mmc/dw_mmc.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 4070d4e..be590a4 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) > dwmci_writel(host, DWMCI_BMOD, 1); > > fifo_size = dwmci_readl(host, DWMCI_FIFOTH); > - if (host->fifoth_val) > + if (host->fifoth_val) { > fifoth_val = host->fifoth_val; > - else > + } else { > fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | > TX_WMARK(fifo_size/2); > + host->fifoth_val = fifoth_val; > + } > dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); > > dwmci_writel(host, DWMCI_CLKENA, 0); >
On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > On 05/24/2013 03:27 AM, Jagan Teki wrote: >> On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde >> <rajeshwari.s@samsung.com> wrote: >>> Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time >>> for mmc rescan command. >>> In current code FIFO_DEPTH is getting calculated after reading the FIFOTH >>> register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). >>> Instead of storing the correct value, we were recalculating the FIFO_DEPT each >>> time which is not correct. >>> >>> Signed-off-by: Hatim Ali <hatim.rv@samsung.com> >>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>> --- >>> drivers/mmc/dw_mmc.c | 6 ++++-- >>> 1 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>> index 4070d4e..be590a4 100644 >>> --- a/drivers/mmc/dw_mmc.c >>> +++ b/drivers/mmc/dw_mmc.c >>> @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) >>> dwmci_writel(host, DWMCI_BMOD, 1); >>> >>> fifo_size = dwmci_readl(host, DWMCI_FIFOTH); >>> - if (host->fifoth_val) >>> + if (host->fifoth_val) { >> >> What is the inital value for host->fifoth_val, for the first time call.? > It should be set into board-specific file(dw-mmc_exynos.c) or others. I am unable to find dw-mmc_exynos.c on the master, and also I haven't see fifoth_val used other than dw_mmc.c Could you please help me out. Thanks, Jagan. > > Best Regards, > Jaehoon Chung >> >>> fifoth_val = host->fifoth_val; >>> - else >>> + } else { >>> fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | >>> TX_WMARK(fifo_size/2); >>> + host->fifoth_val = fifoth_val; >>> + } >>> dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); >>> >>> dwmci_writel(host, DWMCI_CLKENA, 0); >>> -- >>> 1.7.4.4 >>> >> >> Thanks, >> Jagan. >> >
Hi Jagan, Please find my comments below. On Fri, May 24, 2013 at 10:31 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >> On 05/24/2013 03:27 AM, Jagan Teki wrote: >>> On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde >>> <rajeshwari.s@samsung.com> wrote: >>>> Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time >>>> for mmc rescan command. >>>> In current code FIFO_DEPTH is getting calculated after reading the FIFOTH >>>> register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). >>>> Instead of storing the correct value, we were recalculating the FIFO_DEPT each >>>> time which is not correct. >>>> >>>> Signed-off-by: Hatim Ali <hatim.rv@samsung.com> >>>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>>> --- >>>> drivers/mmc/dw_mmc.c | 6 ++++-- >>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>> index 4070d4e..be590a4 100644 >>>> --- a/drivers/mmc/dw_mmc.c >>>> +++ b/drivers/mmc/dw_mmc.c >>>> @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) >>>> dwmci_writel(host, DWMCI_BMOD, 1); >>>> >>>> fifo_size = dwmci_readl(host, DWMCI_FIFOTH); >>>> - if (host->fifoth_val) >>>> + if (host->fifoth_val) { >>> >>> What is the inital value for host->fifoth_val, for the first time call.? >> It should be set into board-specific file(dw-mmc_exynos.c) or others. > > I am unable to find dw-mmc_exynos.c on the master, and also I haven't > see fifoth_val used other than dw_mmc.c > Could you please help me out. dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c > > Thanks, > Jagan. > >> >> Best Regards, >> Jaehoon Chung >>> >>>> fifoth_val = host->fifoth_val; >>>> - else >>>> + } else { >>>> fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | >>>> TX_WMARK(fifo_size/2); >>>> + host->fifoth_val = fifoth_val; >>>> + } >>>> dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); >>>> >>>> dwmci_writel(host, DWMCI_CLKENA, 0); >>>> -- >>>> 1.7.4.4 >>>> >>> >>> Thanks, >>> Jagan. >>> >> > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Jagan, On 05/24/2013 03:15 PM, Rajeshwari Birje wrote: > Hi Jagan, > > Please find my comments below. > On Fri, May 24, 2013 at 10:31 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >> On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>> On 05/24/2013 03:27 AM, Jagan Teki wrote: >>>> On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde >>>> <rajeshwari.s@samsung.com> wrote: >>>>> Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time >>>>> for mmc rescan command. >>>>> In current code FIFO_DEPTH is getting calculated after reading the FIFOTH >>>>> register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). >>>>> Instead of storing the correct value, we were recalculating the FIFO_DEPT each >>>>> time which is not correct. >>>>> >>>>> Signed-off-by: Hatim Ali <hatim.rv@samsung.com> >>>>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>>>> --- >>>>> drivers/mmc/dw_mmc.c | 6 ++++-- >>>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>>> index 4070d4e..be590a4 100644 >>>>> --- a/drivers/mmc/dw_mmc.c >>>>> +++ b/drivers/mmc/dw_mmc.c >>>>> @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) >>>>> dwmci_writel(host, DWMCI_BMOD, 1); >>>>> >>>>> fifo_size = dwmci_readl(host, DWMCI_FIFOTH); >>>>> - if (host->fifoth_val) >>>>> + if (host->fifoth_val) { >>>> >>>> What is the inital value for host->fifoth_val, for the first time call.? >>> It should be set into board-specific file(dw-mmc_exynos.c) or others. >> >> I am unable to find dw-mmc_exynos.c on the master, and also I haven't >> see fifoth_val used other than dw_mmc.c >> Could you please help me out. > dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c host->fifoth_val should be set to hard-coding by developer. In general case, it's right that fifoth_Val is calculated with register value. But sometime, it needs to set to other value, not register value. Then we can use the host->fifoth_val in board-specific file. Maybe it didn't use the host->fifoth_val anywhere. In future, we can use this. Best Regards, Jaehoon Chung >> >> Thanks, >> Jagan. >> >>> >>> Best Regards, >>> Jaehoon Chung >>>> >>>>> fifoth_val = host->fifoth_val; >>>>> - else >>>>> + } else { >>>>> fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | >>>>> TX_WMARK(fifo_size/2); >>>>> + host->fifoth_val = fifoth_val; >>>>> + } >>>>> dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); >>>>> >>>>> dwmci_writel(host, DWMCI_CLKENA, 0); >>>>> -- >>>>> 1.7.4.4 >>>>> >>>> >>>> Thanks, >>>> Jagan. >>>> >>> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot >
On Fri, May 24, 2013 at 12:16 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Hi Jagan, > > On 05/24/2013 03:15 PM, Rajeshwari Birje wrote: >> Hi Jagan, >> >> Please find my comments below. >> On Fri, May 24, 2013 at 10:31 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >>> On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote: >>>> On 05/24/2013 03:27 AM, Jagan Teki wrote: >>>>> On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde >>>>> <rajeshwari.s@samsung.com> wrote: >>>>>> Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time >>>>>> for mmc rescan command. >>>>>> In current code FIFO_DEPTH is getting calculated after reading the FIFOTH >>>>>> register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). >>>>>> Instead of storing the correct value, we were recalculating the FIFO_DEPT each >>>>>> time which is not correct. >>>>>> >>>>>> Signed-off-by: Hatim Ali <hatim.rv@samsung.com> >>>>>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>>>>> --- >>>>>> drivers/mmc/dw_mmc.c | 6 ++++-- >>>>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >>>>>> index 4070d4e..be590a4 100644 >>>>>> --- a/drivers/mmc/dw_mmc.c >>>>>> +++ b/drivers/mmc/dw_mmc.c >>>>>> @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) >>>>>> dwmci_writel(host, DWMCI_BMOD, 1); >>>>>> >>>>>> fifo_size = dwmci_readl(host, DWMCI_FIFOTH); >>>>>> - if (host->fifoth_val) >>>>>> + if (host->fifoth_val) { >>>>> >>>>> What is the inital value for host->fifoth_val, for the first time call.? >>>> It should be set into board-specific file(dw-mmc_exynos.c) or others. >>> >>> I am unable to find dw-mmc_exynos.c on the master, and also I haven't >>> see fifoth_val used other than dw_mmc.c >>> Could you please help me out. >> dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c > host->fifoth_val should be set to hard-coding by developer. > In general case, it's right that fifoth_Val is calculated with register value. > But sometime, it needs to set to other value, not register value. > Then we can use the host->fifoth_val in board-specific file. > Maybe it didn't use the host->fifoth_val anywhere. In future, we can use this. Ok, thanks for your response. If that is the case may be we can initialize host->fifoth_val on add_dwmci() for as of now. Once the proper init of this is done in board specific file in future may be we can remove this. Thanks, Jagan. > > Best Regards, > Jaehoon Chung >>> >>> Thanks, >>> Jagan. >>> >>>> >>>> Best Regards, >>>> Jaehoon Chung >>>>> >>>>>> fifoth_val = host->fifoth_val; >>>>>> - else >>>>>> + } else { >>>>>> fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | >>>>>> TX_WMARK(fifo_size/2); >>>>>> + host->fifoth_val = fifoth_val; >>>>>> + } >>>>>> dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); >>>>>> >>>>>> dwmci_writel(host, DWMCI_CLKENA, 0); >>>>>> -- >>>>>> 1.7.4.4 >>>>>> >>>>> >>>>> Thanks, >>>>> Jagan. >>>>> >>>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >> >
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1); fifo_size = dwmci_readl(host, DWMCI_FIFOTH); - if (host->fifoth_val) + if (host->fifoth_val) { fifoth_val = host->fifoth_val; - else + } else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2); + host->fifoth_val = fifoth_val; + } dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);