[U-Boot] [PATCH 0/8] Fix PPC issues introduced by debug() macro change
From: Marek Vasut marex@pollux.denx.de
Due to the conversion of debug() macro to inline function, the type-checking happens even if DEBUG isn't defined. This made a few problems surface. These fixes here address the ones found so far.
Marek Vasut (8): PPC: Fix tqm8xx_pcmcia.c by removing condition around pcmp PPC: Fix mem_to_mem_idma2intr.c by renaming "debug" to "mmdebug" PPC: Fix i82365.c by removing ifdef around "buf" PPC: Fix relative path in galileo/core.h PPC: Fix pd67290.c by removing ifdef around "buf" PPC: Fix eepro100_eeprom.c by renaming "debug" to "eedebug" PPC: Fix fsl_upm.c by renaming nand handling functions PPC: Remove #ifdef DEBUG around step_to_string
arch/powerpc/cpu/mpc8xxx/ddr/main.c | 2 -- board/cpc45/pd67290.c | 2 -- drivers/mtd/nand/fsl_upm.c | 16 ++++++++-------- drivers/pcmcia/i82365.c | 2 -- drivers/pcmcia/tqm8xx_pcmcia.c | 2 -- examples/standalone/eepro100_eeprom.c | 10 +++++----- examples/standalone/mem_to_mem_idma2intr.c | 4 ++-- include/galileo/core.h | 2 +- 8 files changed, 16 insertions(+), 24 deletions(-)
From: Marek Vasut marex@pollux.denx.de
This fix has no impact on u-boot.bin size (tested with ELDK 4.2).
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- drivers/pcmcia/tqm8xx_pcmcia.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/pcmcia/tqm8xx_pcmcia.c b/drivers/pcmcia/tqm8xx_pcmcia.c index ca1a9fe..2e997dd 100644 --- a/drivers/pcmcia/tqm8xx_pcmcia.c +++ b/drivers/pcmcia/tqm8xx_pcmcia.c @@ -227,10 +227,8 @@ int pcmcia_voltage_set(int slot, int vcc, int vpp) { #ifndef CONFIG_NSCU u_long reg; -# ifdef DEBUG volatile pcmconf8xx_t *pcmp = (pcmconf8xx_t *)(&(((immap_t *)CONFIG_SYS_IMMR)->im_pcmcia)); -# endif
debug ("voltage_set: " PCMCIA_BOARD_MSG " Slot %c, Vcc=%d.%d, Vpp=%d.%d\n",
Dear Marek Vasut,
In message 1317603450-7527-2-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Please make sure to use only valid mail addresses.
This fix has no impact on u-boot.bin size (tested with ELDK 4.2).
Signed-off-by: Marek Vasut marek.vasut@gmail.com
drivers/pcmcia/tqm8xx_pcmcia.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
Um... this modification increases the code size;
before (commit b30d41c):
$ size u-boot text data bss dec hex filename 260519 13960 25704 300183 49497 /work/wd/tmp-ppc/u-boot $ size drivers/pcmcia/tqm8xx_pcmcia.o text data bss dec hex filename 655 16 0 671 29f /work/wd/tmp-ppc/drivers/pcmcia/tqm8xx_pcmcia.o
After:
$ size u-boot text data bss dec hex filename 260543 13960 25704 300207 494af /work/wd/tmp-ppc/u-boot $ size drivers/pcmcia/tqm8xx_pcmcia.o text data bss dec hex filename 683 16 0 699 2bb /work/wd/tmp-ppc/drivers/pcmcia/tqm8xx_pcmcia.o
I'm not willing to accept a code size increase (especially when DEBUG is not even defined!) for such a change.
Please find a better way to fix the problems introduced by commit 60ce53c... GCC4.6: Convert various empty macros to inline functions
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
Also, squash a checkpatch warning in if(debug != 0) part.
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- examples/standalone/mem_to_mem_idma2intr.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/standalone/mem_to_mem_idma2intr.c b/examples/standalone/mem_to_mem_idma2intr.c index d0a75ea..8a986d3 100644 --- a/examples/standalone/mem_to_mem_idma2intr.c +++ b/examples/standalone/mem_to_mem_idma2intr.c @@ -44,10 +44,10 @@ DECLARE_GLOBAL_DATA_PTR; } #endif /* STANDALONE */
-static int debug = 1; +static int mmdebug = 1;
#define DEBUG(fmt, args...) { \ - if(debug != 0) { \ + if (mmdebug != 0) { \ printf("[%s %d %s]: ",__FILE__,__LINE__,__FUNCTION__); \ printf(fmt, ##args); \ } \
Dear Marek Vasut,
In message 1317603450-7527-3-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Please FIX your git setup. This is NOT a valid address.
Also, squash a checkpatch warning in if(debug != 0) part.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
examples/standalone/mem_to_mem_idma2intr.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/standalone/mem_to_mem_idma2intr.c b/examples/standalone/mem_to_mem_idma2intr.c index d0a75ea..8a986d3 100644 --- a/examples/standalone/mem_to_mem_idma2intr.c +++ b/examples/standalone/mem_to_mem_idma2intr.c @@ -44,10 +44,10 @@ DECLARE_GLOBAL_DATA_PTR; } #endif /* STANDALONE */
-static int debug = 1; +static int mmdebug = 1;
#define DEBUG(fmt, args...) { \
- if(debug != 0) { \
- if (mmdebug != 0) { \ printf("[%s %d %s]: ",__FILE__,__LINE__,__FUNCTION__); \ printf(fmt, ##args); \ } \
All of this is bogus. The #define DEBUG itself should be fixed here. And when debug resp. mmdebug is set to 1 anyway, and there is no place anywhere to change that value, we can omit the "if (mmdebug != 0)", too.
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
As the "buf" is cleared at runtime, this might introduce a slight overhead.
Signed-off-by: Marek Vasut marex@pollux.denx.de --- drivers/pcmcia/i82365.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
Q: Can this be squashed into char buf[200] = {0}; ? That'd help the optimization to remove the overhead altogether again.
diff --git a/drivers/pcmcia/i82365.c b/drivers/pcmcia/i82365.c index 1bcb3a5..968d35a 100644 --- a/drivers/pcmcia/i82365.c +++ b/drivers/pcmcia/i82365.c @@ -272,11 +272,9 @@ static u_int cirrus_set_opts (socket_info_t * s) { cirrus_state_t *p = &s->c_state; u_int mask = 0xffff; -#if DEBUG char buf[200];
memset (buf, 0, 200); -#endif
if (has_ring == -1) has_ring = 1;
Dear Marek Vasut,
In message 1317603450-7527-4-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Fix...
As the "buf" is cleared at runtime, this might introduce a slight overhead.
Signed-off-by: Marek Vasut marex@pollux.denx.de
drivers/pcmcia/i82365.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
NAK due to code size increase.
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
Change #include "gt64260R.h" to #include <galileo/gt64260R.h>
Signed-off-by: Marek Vasut marex@pollux.denx.de --- include/galileo/core.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/galileo/core.h b/include/galileo/core.h index c277509..eee0cce 100644 --- a/include/galileo/core.h +++ b/include/galileo/core.h @@ -13,7 +13,7 @@ space). The macros take care of Big/Little endian conversions. #define __INCcoreh
/* includes */ -#include "gt64260R.h" +#include <galileo/gt64260R.h>
extern unsigned int INTERNAL_REG_BASE_ADDR;
Dear Marek Vasut,
In message 1317603450-7527-5-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Change #include "gt64260R.h" to #include <galileo/gt64260R.h>
What is the actual problem / build issue you are fixing here?
Best regards,
Wolfgang Denk
On Monday, October 03, 2011 04:31:36 PM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317603450-7527-5-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Change #include "gt64260R.h" to #include <galileo/gt64260R.h>
What is the actual problem / build issue you are fixing here?
Not an issue, just stabilize the path.
Cheers
From: Marek Vasut marex@pollux.denx.de
As the "buf" is cleared at runtime, this might introduce a slight overhead.
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- board/cpc45/pd67290.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/board/cpc45/pd67290.c b/board/cpc45/pd67290.c index 0d8ef23..96d7289 100644 --- a/board/cpc45/pd67290.c +++ b/board/cpc45/pd67290.c @@ -225,11 +225,9 @@ static u_int cirrus_set_opts (socket_info_t * s) { cirrus_state_t *p = &s->c_state; u_int mask = 0xffff; -#if DEBUG char buf[200];
memset (buf, 0, 200); -#endif
if (has_ring == -1) has_ring = 1;
Dear Marek Vasut,
In message 1317603450-7527-6-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Fix...
As the "buf" is cleared at runtime, this might introduce a slight overhead.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
board/cpc45/pd67290.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/board/cpc45/pd67290.c b/board/cpc45/pd67290.c index 0d8ef23..96d7289 100644 --- a/board/cpc45/pd67290.c +++ b/board/cpc45/pd67290.c @@ -225,11 +225,9 @@ static u_int cirrus_set_opts (socket_info_t * s) { cirrus_state_t *p = &s->c_state; u_int mask = 0xffff; -#if DEBUG char buf[200];
memset (buf, 0, 200); -#endif
NAK due to code size increase.
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
Also, squash a warning about initialized static data.
Signed-off-by: Marek Vasut marek.vasut@gmail.com --- examples/standalone/eepro100_eeprom.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/examples/standalone/eepro100_eeprom.c b/examples/standalone/eepro100_eeprom.c index 3c7f380..889ab55 100644 --- a/examples/standalone/eepro100_eeprom.c +++ b/examples/standalone/eepro100_eeprom.c @@ -59,7 +59,7 @@ static unsigned short eeprom[256]; static int eeprom_size = 64; static int eeprom_addr_size = 6;
-static int debug = 0; +static int eedebug;
static inline unsigned short swap16(unsigned short x) { @@ -123,7 +123,7 @@ static int do_eeprom_cmd(long ioaddr, int cmd, int cmd_len) unsigned retval = 0; long ee_addr = ioaddr + EE_OFFSET;
- if (debug > 1) + if (eedebug > 1) printf(" EEPROM op 0x%x: ", cmd);
outw(EE_ENB | EE_SHIFT_CLK, ee_addr); @@ -133,7 +133,7 @@ static int do_eeprom_cmd(long ioaddr, int cmd, int cmd_len) short dataval = (cmd & (1 << cmd_len)) ? EE_WRITE_1 : EE_WRITE_0; outw(dataval, ee_addr); eeprom_delay(ee_addr); - if (debug > 2) + if (eedebug > 2) printf("%X", inw(ee_addr) & 15); outw(dataval | EE_SHIFT_CLK, ee_addr); eeprom_delay(ee_addr); @@ -144,7 +144,7 @@ static int do_eeprom_cmd(long ioaddr, int cmd, int cmd_len) #endif /* Terminate the EEPROM access. */ outw(EE_ENB & ~EE_CS, ee_addr); - if (debug > 1) + if (eedebug > 1) printf(" EEPROM result is 0x%5.5x.\n", retval); return retval; } @@ -170,7 +170,7 @@ static void write_eeprom(long ioaddr, int index, int value, int addr_len) 3 + addr_len + 16); /* Poll for write finished. */ i = eeprom_busy_poll(ee_ioaddr); /* Typical 2000 ticks */ - if (debug) + if (eedebug) printf(" Write finished after %d ticks.\n", i); /* Disable programming. This command is not instantaneous, so we check for busy before the next op. */
Dear Marek Vasut,
In message 1317603450-7527-7-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Fix...
Also, squash a warning about initialized static data.
Signed-off-by: Marek Vasut marek.vasut@gmail.com
examples/standalone/eepro100_eeprom.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/examples/standalone/eepro100_eeprom.c b/examples/standalone/eepro100_eeprom.c index 3c7f380..889ab55 100644 --- a/examples/standalone/eepro100_eeprom.c +++ b/examples/standalone/eepro100_eeprom.c @@ -59,7 +59,7 @@ static unsigned short eeprom[256]; static int eeprom_size = 64; static int eeprom_addr_size = 6;
-static int debug = 0; +static int eedebug;
It appears that there is no code anywhere that would ever change the value of debug resp. eedebug; so this should probably be cleaned up differently ?
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
This avoids colision with nand subsystem's functions.
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Scott Wood scottwood@freescale.com --- drivers/mtd/nand/fsl_upm.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c index c33e278..31c174b 100644 --- a/drivers/mtd/nand/fsl_upm.c +++ b/drivers/mtd/nand/fsl_upm.c @@ -124,14 +124,14 @@ static void fun_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl) fun_wait(fun); }
-static u8 nand_read_byte(struct mtd_info *mtd) +static u8 upm_nand_read_byte(struct mtd_info *mtd) { struct nand_chip *chip = mtd->priv;
return in_8(chip->IO_ADDR_R); }
-static void nand_write_buf(struct mtd_info *mtd, const u_char *buf, int len) +static void upm_nand_write_buf(struct mtd_info *mtd, const u_char *buf, int len) { int i; struct nand_chip *chip = mtd->priv; @@ -147,7 +147,7 @@ static void nand_write_buf(struct mtd_info *mtd, const u_char *buf, int len) fun_wait(fun); }
-static void nand_read_buf(struct mtd_info *mtd, u_char *buf, int len) +static void upm_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len) { int i; struct nand_chip *chip = mtd->priv; @@ -156,7 +156,7 @@ static void nand_read_buf(struct mtd_info *mtd, u_char *buf, int len) buf[i] = in_8(chip->IO_ADDR_R); }
-static int nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len) +static int upm_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len) { int i; struct nand_chip *chip = mtd->priv; @@ -191,10 +191,10 @@ int fsl_upm_nand_init(struct nand_chip *chip, struct fsl_upm_nand *fun) #if CONFIG_SYS_NAND_MAX_CHIPS > 1 chip->select_chip = fun_select_chip; #endif - chip->read_byte = nand_read_byte; - chip->read_buf = nand_read_buf; - chip->write_buf = nand_write_buf; - chip->verify_buf = nand_verify_buf; + chip->read_byte = upm_nand_read_byte; + chip->read_buf = upm_nand_read_buf; + chip->write_buf = upm_nand_write_buf; + chip->verify_buf = upm_nand_verify_buf; if (fun->dev_ready) chip->dev_ready = nand_dev_ready;
Dear Marek Vasut,
In message 1317603450-7527-8-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Needs fixing...
Best regards,
Wolfgang Denk
From: Marek Vasut marex@pollux.denx.de
This fixes a type-checking issue.
The growth of u-boot.bin isn't measurable here as the resulting binary is always 512k big. But the u-boot executable is 500 bytes bigger.
Signed-off-by: Marek Vasut marex@pollux.denx.de --- arch/powerpc/cpu/mpc8xxx/ddr/main.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xxx/ddr/main.c b/arch/powerpc/cpu/mpc8xxx/ddr/main.c index 249fd7d..eb70535 100644 --- a/arch/powerpc/cpu/mpc8xxx/ddr/main.c +++ b/arch/powerpc/cpu/mpc8xxx/ddr/main.c @@ -132,7 +132,6 @@ void fsl_ddr_get_spd(generic_spd_eeprom_t *ctrl_dimms_spd, * | interleaving */
-#ifdef DEBUG const char *step_string_tbl[] = { "STEP_GET_SPD", "STEP_COMPUTE_DIMM_PARMS", @@ -153,7 +152,6 @@ const char * step_to_string(unsigned int step) {
return step_string_tbl[s]; } -#endif
int step_assign_addresses(fsl_ddr_info_t *pinfo, unsigned int dbw_cap_adj[],
Dear Marek Vasut,
In message 1317603450-7527-9-git-send-email-marek.vasut@gmail.com you wrote:
From: Marek Vasut marex@pollux.denx.de
Fix...
The growth of u-boot.bin isn't measurable here as the resulting binary is always 512k big. But the u-boot executable is 500 bytes bigger.
You must be joking. Does your computer overflow at 19 bits?? Of course you can measure the size changes in bytes.
And a code size increase of 1% just for nothing is cleanly not acceptable.
Best regards,
Wolfgang Denk
participants (2)
-
Marek Vasut -
Wolfgang Denk