[U-Boot] [PATCH 05/14] sunxi: dram: Code cleanup for the impedance calibration

[Ian Campbell <ijc@hellion.org.uk>]

On Fri, 2014-07-18 at 19:22 +0300, Siarhei Siamashka wrote:
> Moved the impedance setup code part into a separate function. Added explicit
> wait for ZQ calibration completion before proceeding to the next initialization
> steps. Removed the CONFIG_SUN7I ifdef guard around the code, which has identical
> behaviour on sun4i/sun5i/sun7i. And if 'odt_en' is set in the 'dram_para' struct,
> then ODT now actually gets enabled in the DRAM_IOCR register (which the older
> code failed to do and was always running without ODT no matter the settings).

There's a few aspects of this code which don't seem to be explained
here.

Firstly if odt_en is not enabled we now skip setting the impedance.
Which seems logical but should me mentioned. It's also worth noting that
none of the platforms in u-boot-sunxi.git#master set odt_en

Secondly the weird sun7i magic has changed from
-       setbits_le32(&dram->zqcr1, (0x1 << 24) | (0x1 << 1));
-       if (para->tpr4 & 0x2)
-               clrsetbits_le32(&dram->zqcr1, (0x1 << 24), (0x1 << 1));
into just a write of the raw value. This should be mentioned. Also this
now occurs after the call to dramc_clock_output_en().

Thirdly why do we not wait for ZQ calibration on sun7i?

Lastly it now seems to support calibration using an external resistor.
And neither half of that new if (zdata) seems to correspond to the old
code.

I think part of the problem here is that this patch is trying to do too
much in one go. If separating things out isn't possible (e.g. because
these changes are all interdependent) then it is important that the
commit message describes them. I'd also steer clear of describing this
as a code cleanup when it also has functional changes.

> + * Wait up to 1s for mask to be set in given reg.
> + */
> +static void await_bits_set(u32 *reg, u32 mask)

This could be combined with the existing await_completion into a
function which takes a mask and a val. Perhaps with convenience wrappers
for the two cases.

Ian.