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

Dear Adam Graham,

In message 1229633739-23318-1-git-send-email-agraham@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