Dear Adam Graham,
In message 1231887261-30490-1-git-send-email-agraham@amcc.com you wrote:
--- a/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c +++ b/cpu/ppc4xx/4xx_ibm_ddr2_autocalib.c @@ -61,6 +61,12 @@ #define NUMLOOPS 1 /* configure as you deem approporiate */ #define NUMMEMWORDS 16
+#define DDR_VERBOSE_PRT(dbg, lvl, fmt, args...) \
- do { \
- if (lvl > dbg) \
printf(fmt, ##args); \- } while (0)
We have a creaping featurism here. I seriously doubt if all this debug code is really needed in a production release. I recommend to get rid of all this dynamic run-time verbosity switching code, and simply use debug() resp. debugX() instead. Keep in mind that this is a boot loader, and memory footprint counts.
[Also, indentation was wrong, too.]
...
DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best result: 0x%04x\n",wdtr, clkp, best_result);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best WRDTR: 0x%04x\n",wdtr, clkp, tcal.clocks.wrdtr);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best CLKTR: 0x%04x\n",wdtr, clkp, tcal.clocks.clktr);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best RQDC: 0x%04x\n",wdtr, clkp, tcal.autocal.rqfd);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best RFDC: 0x%04x\n",wdtr, clkp, tcal.autocal.rffd);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best RDCC: 0x%08x\n",wdtr, clkp, tcal.clocks.rdcc);mfsdram(SDRAM_RTSR, val);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best loop RTSR: 0x%08x\n",wdtr, clkp, val);mfsdram(SDRAM_FCSR, val);DDR_VERBOSE_PRT(2, verbose_lvl,"** (%d)(%d) best loop FCSR: 0x%08x\n",wdtr, clkp, val);
In your implementation above - does it really make sense to test the verbosity level against the very same constant again and again and again? I don't think so. This code is just difficult to read.
Please simplyfy. All this complexity of debug code has no place in the production version.
Best regards,
Wolfgang Denk