Re: Marvell NFC timings on CN9130

[Chris Packham]

On 17/05/23 05:25, Miquel Raynal wrote:
> Hi Chris!
>
> Chris.Packham@xxxxxxxxxxxxxxxxxxx wrote on Tue, 16 May 2023 04:46:38
> +0000:
>
>> Hi Miquel, Thomas,
>>
>> A hardware colleague reported a concern to me about a new design we have
>> using the Marvell CN9130 SoC (which I think was called Armada-8K before
>> they rebranded).
>>
>> Basically their concern is that the tWC timing they observe is faster
>> (~18ns) than the documented minimum in the hardware datasheet for the
>> CN9130 (25ns). Aside from not meeting the datasheet spec we've not
>> observed any other issue (yet).
> I would have expected the controller to support almost any kind of
> timings, including SDR EDO mode 5. tWC is 25ns with mode 4, but 20 on
> mode 5 (ONFI). So I believe you're running a system with a chip that is
> not compatible with the fastest mode. If that is the case, it may
> explain why you don't see errors with this chip: it may support
> slightly faster timings than it advertises.
>
> Anyway, if your findings are true, it means the current implementation
> is slightly out of spec and the timing calculation might require to be
> tweaked a little bit to reduce tWC.
>
>> I notice in the marvell_nand.c driver that marvell_nfc_init() sets the
>> NAND Clock Frequency Select bit (0xF2440700:0) to 1 which runs according
>> to the datasheet the NAND flash at 400MHz . But the calculations in
>> marvell_nfc_setup_interface() use the value from
>> clk_get_rate(nfc->core_clk) which is still 250MHz so I'm wondering if
>> maybe the fact that the NAND flash is being run faster is having an
>> impact on timings that are calculated around the core_clk frequency.
> What if you reset this bit? Do you observe different timings? I hope
> you do, otherwise this is a dead-end.
Yes if we clear the bit the timings go from ~18ns to about 30ns.
> The timings are derived from this clock but I remember seeing different
> rates than the ones I expected with no obvious explanation (see the "*
> 2" in the calculation of period_ns and the comment right below). So
> maybe this is due to the 400MHz vs. 250MHz issue you are reporting, or
> there is an undocumented pre-scaler in-between (this is my original
> guess).

I wondered if the * 2 was because of this or because of the comment that 
the ECC_CLK is 2*NF_CLK. That probably also means that a number of SoCs 
are running with an extra *2 that don't need to be (e.g. Armada-385).

>> Do you think that the timings calculations should take the NAND Clock
>> Frequency Select setting into account?
> There is not much about this clock in the manual, so if the clock is
> feeding the logic of the controller generating the signals on the bus,
> then yes. You can verify this with the test mentioned above.
>
> Could you check the values set to tWP and tWH with and without the bit
> and probe the signals in both cases? Maybe the "* 2" in the
> period_ns calculation will vanish if we use 400MHz as input clock rather
> than clk_get_rate() (or better, expose the bit as a mux-clock and use
> it to tell the CCF the right frequency) and you'll get a sharper tWC in
> the end, which hopefully should match the spec this time.

I was going to have a look to see if I can get the NAND clock to 
correctly reflect the value when the NAND Clock Frequency Select bit is 
set. In the meantime I'll also do some experiments removing the * 2 and 
hard-coding the frequency at 400MHz.