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

Jaehoon Chung
2012-12-10T02:21:03Z

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
>