[U-Boot] [PATCH 1/1] ppc4xx: Autocalibration can set RDCC to over aggressive value.

[Wolfgang Denk]

Dear Adam Graham,

In message <1229633739-23318-1-git-send-email-agraham at amcc.com> you wrote:
> The criteria of the AMCC SDRAM Controller DDR autocalibration U-Boot code is to pick the largest passing write/read/compare window that also has the smallest SDRAM_RDCC.[RDSS] Read Sample Cycle Select value.

Please adhere to the netiquette, and to coding style and patch
requirements.

Maximum line length is some 70+ characters.

Your lines are up to 450 characters.

Such a patch can only be rejected.

Also pay attention to code like this:

> +					     (val <= best_rdcc))) {
> +						best_rdcc = val;
> +						tcal.clocks.wrdtr = wdtr;
> +						tcal.clocks.clktr = clkp;
> +						tcal.clocks.rdcc = (val << 30);
> +						tcal.autocal.rqfd = ddrcal.rqfd;
> +						tcal.autocal.rffd = ddrcal.rffd;
> +						best_result = result;
> +
> +						if (verbose_lvl > 2) {
> +							printf("** (%d)(%d)  "
> +							    "best result: "
> +							    "0x%04x\n",
> +							    wdtr, clkp,
> +							    best_result);
> +							printf("** (%d)(%d)  "
> +							    "best WRDTR: "
> +							    "0x%04x\n",
> +							    wdtr, clkp,
> +							    tcal.clocks.wrdtr);
> +							printf("** (%d)(%d)  "
> +							    "best CLKTR: "
> +							    "0x%04x\n",
> +							    wdtr, clkp,
> +							    tcal.clocks.clktr);
> +							printf("** (%d)(%d)  "
> +							    "best RQDC: "
> +							    "0x%04x\n",
> +							    wdtr, clkp,
> +							    tcal.autocal.rqfd);
> +							printf("** (%d)(%d)  "
> +							    "best RFDC: "
> +							    "0x%04x\n",
> +							    wdtr, clkp,
> +							    tcal.autocal.rffd);
> +							printf("** (%d)(%d)  "
> +							    "best RDCC: "
> +							    "0x%08x\n",
> +							    wdtr, clkp,
> +							    tcal.clocks.rdcc);
> +							mfsdram(SDRAM_RTSR,
> +								val);
> +							printf("** (%d)(%d)  "
> +							    "best loop RTSR: "
> +							    "0x%08x\n",
> +							    wdtr, clkp, val);
> +							mfsdram(SDRAM_FCSR,
> +								val);
> +							printf("** (%d)(%d)  "
> +							    "best loop FCSR: "
> +							    "0x%08x\n",
> +							    wdtr, clkp, val);
> +						}

This violates coding style requirements because indentation is not by
TABs. But even more important, the following part of the Coding Style
document aplies:

        Now, some people will claim that having 8-character
        indentations makes the code move too far to the right, and
        makes it hard to read on a 80-character terminal screen. The
        answer to that is that if you need more than 3 levels of
        indentation, you're screwed anyway, and should fix your
        program.

        In short, 8-char indents make things easier to read, and have
        the added benefit of warning you when you're nesting your
        functions too deep. Heed that warning.

How deep is your nesting?

Please clean up and resubmit.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If in any problem you find yourself doing an immense amount of  work,
the answer can be obtained by simple inspection.