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

[Jaehoon Chung <jh80.chung@samsung.com>]

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@samsung.com> wrote:
>> It looks good to me.
>> Added minor comment.
>>
>> Acked-by: Jaehoon Chung <jh80.chung@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@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
>