[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Amarendra Reddy]

Hi Jaehoon Chung,

I will update the patches as per your review comments.

As suggested by Mr.Simon, I will create macros similar to the below macros
in header file.

 #define RX_WMARK_SHIFT 16
 #define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)


Thanks & Regards
Amarendra

On 10 December 2012 07:51, Jaehoon Chung <jh80.chung at samsung.com> wrote:

> Hi Simon,
>
> On 12/09/2012 04:49 AM, Simon Glass wrote:
> > Hi Jaehoon,
> >
> > On Wed, Dec 5, 2012 at 7:59 PM, Jaehoon Chung <jh80.chung at samsung.com>
> wrote:
> >> It looks good to me.
> >> Added minor comment.
> >>
> >> Acked-by: Jaehoon Chung <jh80.chung at samsung.com>
> >>
> >> On 12/05/2012 10:31 PM, Amar wrote:
> >>> The current implementation of fifo size computation was giving improper
> >>> values for eMMC channel. Modified the computation as per user manual.
> >>>
> >>> Signed-off-by: Amarendra Reddy <amarendra.xt at samsung.com>
> >>> ---
> >>>  drivers/mmc/dw_mmc.c |    2 ++
> >>>  1 files changed, 2 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> >>> index 4070d4e..62dc152 100644
> >>> --- a/drivers/mmc/dw_mmc.c
> >>> +++ b/drivers/mmc/dw_mmc.c
> >>> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
> >>>       dwmci_writel(host, DWMCI_BMOD, 1);
> >>>
> >>>       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
> >>> +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
> >> How about using like FIFO_SIZE_MASK?
> >
> > It might be better to avoid macros in header files which shift and
> > mask x, since they obscure the operation, and just define the amount
> > of shift and mask in the header file. Also if you have a #define for
> > the mask you should probably also have one for the shift, otherwise
> > you have the information in two places. So maybe:
> I want to add the macro into header file like your suggestion.
> RX_WMARK() is used to set with user input or pdata.
> If Amarendra will change this patch, it will looks great to me.
>
> Best Regards,
> Jaehoon Chung
>  >
> > #define RX_WMARK_SHIFT 16
> > #define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
> >
> > fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1;
> >
> >>> +
> >>>       if (host->fifoth_val)
> >>>               fifoth_val = host->fifoth_val;
> >>>       else
> >>>
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Regards,
> > Simon
> > _______________________________________________
> > 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
>