[U-Boot] [PATCH] Add Boundary Devices Nitrogen6X boards

[Eric Nelson]

Hi Benoît,

On 02/20/2013 06:28 AM, Benoît Thébaudeau wrote:
> Hi Eric,
>
> On Wednesday, February 20, 2013 1:05:04 PM, Benoît Thébaudeau wrote:
>> Hi Eric,
>>
>> On Wednesday, February 20, 2013 12:01:15 AM, Eric Nelson wrote:
>>> Hi Benoît,
>>>
>>> On 02/19/2013 03:31 PM, Benoît Thébaudeau wrote:
>>>> On Tuesday, February 19, 2013 10:53:48 PM, Eric Nelson wrote:
>>>>> On 02/19/2013 01:52 PM, Benoît Thébaudeau wrote:
>>>>>> Hi Eric,
>>>>>>
>>>>>> On Tuesday, February 19, 2013 9:20:48 PM, Eric Nelson wrote:
>>>>>> [--snip--]
>>>>>>> diff --git a/board/boundary/nitrogen6x/1066mhz_4x128mx16.cfg
>>>>>>> b/board/boundary/nitrogen6x/1066mhz_4x128mx16.cfg
>>>>>>> new file mode 100644
>>>>>>> index 0000000..45b8879
>>>>>>> --- /dev/null
>>>>>>> +++ b/board/boundary/nitrogen6x/1066mhz_4x128mx16.cfg
>>>>>> [--snip--]
>>>>>>> +DATA 4, MX6_MMDC_P0_MDPDC, 0x00020036
>>>>>>> +DATA 4, MX6_MMDC_P0_MDCFG0, 0x555B7974
>>>>>>                                    ^A?
>>>>>>> +DATA 4, MX6_MMDC_P0_MDCFG1, 0xDB538F64
>>>>>>> +DATA 4, MX6_MMDC_P0_MDCFG2, 0x01FF00DB
>>>>>>> +DATA 4, MX6_MMDC_P0_MDRWD, 0x000026D2
>>>>>>> +DATA 4, MX6_MMDC_P0_MDOR, 0x005B1023
>>>>>>                                  ^A?
>>>>>>> +DATA 4, MX6_MMDC_P0_MDOTC, 0x09444040
>>>>>>> +DATA 4, MX6_MMDC_P0_MDPDC, 0x00025576
>>>>>> [--snip--]
>>>>>>
>>>>>> tXS = tXPR = 170 ns -> 91 nCK -> 91 - 1 -> 0x5A.
>>>>>>
>>>>>
>>>>> Thanks Benoît,
>>>>>
>>>>> I was going to bring this up in a separate thread.
>>>>>
>>>>> While working through the details of our 800MHz
>>>>> variants (Solo, Dual-Lite), and x256mx16 variants,
>>>>> I re-worked these numbers and it seems that we
>>>>> have an off-by-one issue with those fields.
>>>>
>>>> Probably because it has been missed that the bit-field
>>>   > value is the number of clock cycles minus 1.
>>>>
>>> Right. All of these fields are plus 1.
>>>
>>> 	MDCFG0.tRFC
>>> 	MDCFG0.tXS
>>> 	MDOR.tXPR
>>>
>>> Since they're all in the same units, the requirements
>>> are:
>>> 	MDCFG0.tXS >= MDCFG0.tRFC + 10nS
>>> and
>>> 	MDOR.tXPR >= MDCFG0.tRFC + 10nS
>>>
>>> Since we operate at ~528MHz, each clock is less than
>>> 2 nS, and we need 6 more clocks for each.
>>>
>>>>> According to the JEDEC spec and data sheets,
>>>>> both tXS and tXPR should be 10nS greater than tRFC.
>>>>
>>>> Indeed, or more precisely, max(5 nCK, tRFC + 10 ns).
>>>>
>>>
>>> Yep. I shortened because nothing approaches 5nCK.
>>>
>>> And note that this is the minimum spec, not the
>>> target.
>>>
>>>>> Since the nominal clock for i.MX6 is 528MHz (1.89nS),
>>>>
>>>> I used 532 MHz because this is a more standard value, and I found several
>>>> close
>>>> different values in the documentation, so in the doubt, I chose the worst
>>>> case.
>>>> With 528 MHz, the bit-field value would be 0x59.
>>>>
>>>
>>> Either way, we need 6 clocks to get > 10nS.
>>>
>>>>> this should be a delta of 6 clocks, not 5.
>>>>
>>>> Delta with what?
>>>>
>>>
>>> Sorry. I meant the Delta between MDCFG0.tRFC and the
>>> other two fields.
>>
>> OK, now I see what you mean and how you got these values. But I disagree.
>>
>> tXS(min) and tXPR(min) are defined by the JEDEC DDR3 specification as
>> max(5 nCK, tRFC(min) + 10 ns). tRFC(min) is used here, not tRFC.
>>
>> Moreover, tXS and tXPR are timings depending on internal features of the RAM,
>> not on external operations from the host processor. It's not as if they were
>> the
>> result of the combination of two external operations. E.g., see tXS on
>> figures
>> 14, 15 and 62 in JESD79-3F.
>
> I don't know if that's clear enough, but here I mean that tXS and tXPR are
> intrinsic RAM timings, and that there is no way tRFC(MMDC) can interfere with
> either tXS/XPR(DDR3) or tXS/XPR(MMDC), so there is no reason to take tRFC(MMDC)
> into account to determine tXS or tXPR. Only tRFC(min) should be used here.
>

You're right of course. And this description was very clear. We don't
communicate __our__ tRFC to the device, so there's no reason our tRFC
couldn't be higher than tXS/XPR.

>> Hence, tXS and tXPR should not be considered as tRFC(MMDC) + 10 ns, but
>> really
>> as tRFC(min) + 10 ns, i.e. a single 170 ns timing (for 2-Gib density) to be
>> converted into a number of clock cycles.
>>
>> I don't feel like it's possible to interpret the specification in a different
>> way. But perhaps I'm wrong.

I also have no evidence of failures because of these settings and will
drop the +1 in a V2 patch set.

Thanks again for your detailed review.

Regards,


Eric