[U-Boot] [PATCH 1/4] powerpc: Correct im_dprambase cast

When casting im_dprambase we must cast to unsigned char not unsigned short.
Signed-off-by: Tom Rini trini@ti.com --- arch/powerpc/cpu/mpc8260/commproc.c | 2 +- arch/powerpc/cpu/mpc8260/cpu.c | 2 +- arch/powerpc/cpu/mpc8260/i2c.c | 8 ++++---- arch/powerpc/cpu/mpc8260/serial_smc.c | 4 ++-- arch/powerpc/cpu/mpc8260/spi.c | 2 +- common/cmd_immap.c | 2 +- examples/standalone/mem_to_mem_idma2intr.c | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8260/commproc.c b/arch/powerpc/cpu/mpc8260/commproc.c index 22cef3e..9d6c8e2 100644 --- a/arch/powerpc/cpu/mpc8260/commproc.c +++ b/arch/powerpc/cpu/mpc8260/commproc.c @@ -43,7 +43,7 @@ m8260_cpm_reset(void) } while ((immr->im_cpm.cp_cpcr & CPM_CR_FLG) && ++count < 1000000);
#ifdef CONFIG_HARD_I2C - *((unsigned short*)(&immr->im_dprambase[PROFF_I2C_BASE])) = 0; + *((unsigned char*)(&immr->im_dprambase[PROFF_I2C_BASE])) = 0; #endif }
diff --git a/arch/powerpc/cpu/mpc8260/cpu.c b/arch/powerpc/cpu/mpc8260/cpu.c index f8bc5a9..26b3e8d 100644 --- a/arch/powerpc/cpu/mpc8260/cpu.c +++ b/arch/powerpc/cpu/mpc8260/cpu.c @@ -106,7 +106,7 @@ int checkcpu (void) * in the mask. */ m = immr & (IMMR_PARTNUM_MSK | IMMR_MASKNUM_MSK); - k = *((ushort *) & immap->im_dprambase[PROFF_REVNUM]); + k = *((uchar *) & immap->im_dprambase[PROFF_REVNUM]);
switch (m) { case 0x0000: diff --git a/arch/powerpc/cpu/mpc8260/i2c.c b/arch/powerpc/cpu/mpc8260/i2c.c index b720b1f..48917ea 100644 --- a/arch/powerpc/cpu/mpc8260/i2c.c +++ b/arch/powerpc/cpu/mpc8260/i2c.c @@ -221,14 +221,14 @@ void i2c_init(int speed, int slaveadd) i2c_init_board(); #endif
- dpaddr = *((unsigned short *) (&immap->im_dprambase[PROFF_I2C_BASE])); + dpaddr = *((unsigned char *) (&immap->im_dprambase[PROFF_I2C_BASE])); if (dpaddr == 0) { /* need to allocate dual port ram */ dpaddr = m8260_cpm_dpalloc(64 + (NUM_RX_BDS * sizeof(I2C_BD)) + (NUM_TX_BDS * sizeof(I2C_BD)) + MAX_TX_SPACE, 64); - *((unsigned short *)(&immap->im_dprambase[PROFF_I2C_BASE])) = + *((unsigned char *)(&immap->im_dprambase[PROFF_I2C_BASE])) = dpaddr; }
@@ -305,7 +305,7 @@ void i2c_newio(i2c_state_t *state)
debug("[I2C] i2c_newio\n");
- dpaddr = *((unsigned short *)(&immap->im_dprambase[PROFF_I2C_BASE])); + dpaddr = *((unsigned char *)(&immap->im_dprambase[PROFF_I2C_BASE])); iip = (iic_t *)&immap->im_dprambase[dpaddr]; state->rx_idx = 0; state->tx_idx = 0; @@ -480,7 +480,7 @@ int i2c_doio(i2c_state_t *state) return I2CERR_QUEUE_EMPTY; }
- dpaddr = *((unsigned short *)(&immap->im_dprambase[PROFF_I2C_BASE])); + dpaddr = *((unsigned char *)(&immap->im_dprambase[PROFF_I2C_BASE])); iip = (iic_t *)&immap->im_dprambase[dpaddr]; iip->iic_rbptr = iip->iic_rbase; iip->iic_tbptr = iip->iic_tbase; diff --git a/arch/powerpc/cpu/mpc8260/serial_smc.c b/arch/powerpc/cpu/mpc8260/serial_smc.c index feba1f6..d57c44e 100644 --- a/arch/powerpc/cpu/mpc8260/serial_smc.c +++ b/arch/powerpc/cpu/mpc8260/serial_smc.c @@ -105,7 +105,7 @@ static int mpc8260_smc_serial_init(void) /* initialize pointers to SMC */
sp = (smc_t *) &(im->im_smc[SMC_INDEX]); - *(ushort *)(&im->im_dprambase[PROFF_SMC_BASE]) = PROFF_SMC; + *(uchar *)(&im->im_dprambase[PROFF_SMC_BASE]) = PROFF_SMC; up = (smc_uart_t *)&im->im_dprambase[PROFF_SMC];
/* Disable transmitter/receiver. */ @@ -331,7 +331,7 @@ kgdb_serial_init (void) /* initialize pointers to SMC */
sp = (smc_t *) &(im->im_smc[KGDB_SMC_INDEX]); - *(ushort *)(&im->im_dprambase[KGDB_PROFF_SMC_BASE]) = KGDB_PROFF_SMC; + *(uchar *)(&im->im_dprambase[KGDB_PROFF_SMC_BASE]) = KGDB_PROFF_SMC; up = (smc_uart_t *)&im->im_dprambase[KGDB_PROFF_SMC];
/* Disable transmitter/receiver. */ diff --git a/arch/powerpc/cpu/mpc8260/spi.c b/arch/powerpc/cpu/mpc8260/spi.c index dc98ea7..b5ead9f 100644 --- a/arch/powerpc/cpu/mpc8260/spi.c +++ b/arch/powerpc/cpu/mpc8260/spi.c @@ -146,7 +146,7 @@ void spi_init_f (void) immr = (immap_t *) CONFIG_SYS_IMMR; cp = (cpm8260_t *) &immr->im_cpm;
- *(ushort *)(&immr->im_dprambase[PROFF_SPI_BASE]) = PROFF_SPI; + *(uchar *)(&immr->im_dprambase[PROFF_SPI_BASE]) = PROFF_SPI; spi = (spi_t *)&immr->im_dprambase[PROFF_SPI];
/* 1 */ diff --git a/common/cmd_immap.c b/common/cmd_immap.c index fdf9489..2cee082 100644 --- a/common/cmd_immap.c +++ b/common/cmd_immap.c @@ -535,7 +535,7 @@ do_i2cinfo (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) volatile iic_t *iip; uint dpaddr;
- dpaddr = *((unsigned short *) (&immap->im_dprambase[PROFF_I2C_BASE])); + dpaddr = *((unsigned char *) (&immap->im_dprambase[PROFF_I2C_BASE])); if (dpaddr == 0) iip = NULL; else diff --git a/examples/standalone/mem_to_mem_idma2intr.c b/examples/standalone/mem_to_mem_idma2intr.c index e466c90..ee9ca76 100644 --- a/examples/standalone/mem_to_mem_idma2intr.c +++ b/examples/standalone/mem_to_mem_idma2intr.c @@ -309,7 +309,7 @@ int idma_init (void)
memaddr = dpalloc (sizeof (pram_idma_t), 64);
- *(volatile ushort *) &immap->im_dprambase[PROFF_IDMA2_BASE] = memaddr; + *(volatile uchar *) &immap->im_dprambase[PROFF_IDMA2_BASE] = memaddr; piptr = (volatile pram_idma_t *) ((uint) (immap) + memaddr);
piptr->pi_resv1 = 0; /* manual says: clear it */

We must cast this to unsigned char not unsigned short to avoid warnings.
Cc: Wolfgang Denk wd@denx.de Signed-off-by: Tom Rini trini@ti.com --- arch/powerpc/cpu/mpc8xx/cpu.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xx/cpu.c b/arch/powerpc/cpu/mpc8xx/cpu.c index b6b733d..db98e82 100644 --- a/arch/powerpc/cpu/mpc8xx/cpu.c +++ b/arch/powerpc/cpu/mpc8xx/cpu.c @@ -78,7 +78,7 @@ static int check_CPU (long clock, uint pvr, uint immr) if ((pvr >> 16) != 0x0050) return -1;
- k = (immr << 16) | *((ushort *) & immap->im_cpm.cp_dparam[0xB0]); + k = (immr << 16) | *((uchar *) & immap->im_cpm.cp_dparam[0xB0]); m = 0; suf = "";
@@ -194,7 +194,7 @@ static int check_CPU (long clock, uint pvr, uint immr) if ((pvr >> 16) != 0x0050) return -1;
- k = (immr << 16) | *((ushort *) & immap->im_cpm.cp_dparam[0xB0]); + k = (immr << 16) | *((uchar *) & immap->im_cpm.cp_dparam[0xB0]); m = 0;
switch (k) { @@ -253,7 +253,7 @@ static int check_CPU (long clock, uint pvr, uint immr) if ((pvr >> 16) != 0x0050) return -1;
- k = (immr << 16) | in_be16((ushort *)&immap->im_cpm.cp_dparam[0xB0]); + k = (immr << 16) | in_be16((uchar *)&immap->im_cpm.cp_dparam[0xB0]); m = 0;
switch (k) { @@ -312,7 +312,7 @@ static int check_CPU (long clock, uint pvr, uint immr) if ((pvr >> 16) != 0x0050) return -1;
- k = (immr << 16) | *((ushort *) & immap->im_cpm.cp_dparam[0xB0]); + k = (immr << 16) | *((uchar *) & immap->im_cpm.cp_dparam[0xB0]); m = 0;
switch (k) {

Dear Tom Rini,
In message 1368477117-32669-2-git-send-email-trini@ti.com you wrote:
We must cast this to unsigned char not unsigned short to avoid warnings.
But this is wrong. We're actually reading a 16 bit value here.
I think there was previous discussion, and Scott Wodd has a patch out there, which just needed some minor fixing...
Best regards,
Wolfgang Denk

Dear Tom,
In message 20130513223447.5D86C3804AF@gemini.denx.de I wrote:
In message 1368477117-32669-2-git-send-email-trini@ti.com you wrote:
We must cast this to unsigned char not unsigned short to avoid warnings.
But this is wrong. We're actually reading a 16 bit value here.
I think there was previous discussion, and Scott Wodd has a patch out there, which just needed some minor fixing...
Scott's patch is here:
http://patchwork.ozlabs.org/patch/210599/
Scott, do you have any plans to submit an updated version?
Best regards,
Wolfgang Denk

On 05/14/2013 05:26:19 AM, Wolfgang Denk wrote:
Dear Tom,
In message 20130513223447.5D86C3804AF@gemini.denx.de I wrote:
In message 1368477117-32669-2-git-send-email-trini@ti.com you
wrote:
We must cast this to unsigned char not unsigned short to avoid
warnings.
But this is wrong. We're actually reading a 16 bit value here.
I think there was previous discussion, and Scott Wodd has a patch
out
there, which just needed some minor fixing...
Scott's patch is here:
http://patchwork.ozlabs.org/patch/210599/
Scott, do you have any plans to submit an updated version?
I'll try to get to it soon.
-Scott

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 05/13/2013 06:34 PM, Wolfgang Denk wrote:
Dear Tom Rini,
In message 1368477117-32669-2-git-send-email-trini@ti.com you wrote:
We must cast this to unsigned char not unsigned short to avoid warnings.
But this is wrong. We're actually reading a 16 bit value here.
Oh derp, I was mis-recalling the type rules here.
- -- Tom

We need to write each segment of the buffer directly rather than using shorts.
Cc: Frank Gottschling fgottschling@eltec.de Signed-off-by: Tom Rini trini@ti.com --- board/eltec/elppc/misc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/board/eltec/elppc/misc.c b/board/eltec/elppc/misc.c index 89f1b1d..a7d8486 100644 --- a/board/eltec/elppc/misc.c +++ b/board/eltec/elppc/misc.c @@ -207,9 +207,12 @@ int misc_init_r (void) buf[4] = eerev.etheraddr[5]; buf[5] = eerev.etheraddr[4];
- *(unsigned short *) &buf[20] = 0x48B2; - *(unsigned short *) &buf[22] = 0x0004; - *(unsigned short *) &buf[24] = 0x1433; + buf[20] = 0x48; + buf[21] = 0xB2; + buf[22] = 0x00; + buf[23] = 0x04; + buf[24] = 0x14; + buf[25] = 0x33;
printf ("\nSRom: Writing i82559 info ........ "); if (eepro100_srom_store ((unsigned short *) buf) == -1)

Dear Tom Rini,
In message 1368477117-32669-3-git-send-email-trini@ti.com you wrote:
We need to write each segment of the buffer directly rather than using shorts.
Cc: Frank Gottschling fgottschling@eltec.de Signed-off-by: Tom Rini trini@ti.com
board/eltec/elppc/misc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/board/eltec/elppc/misc.c b/board/eltec/elppc/misc.c index 89f1b1d..a7d8486 100644 --- a/board/eltec/elppc/misc.c +++ b/board/eltec/elppc/misc.c @@ -207,9 +207,12 @@ int misc_init_r (void) buf[4] = eerev.etheraddr[5]; buf[5] = eerev.etheraddr[4];
*(unsigned short *) &buf[20] = 0x48B2;
*(unsigned short *) &buf[22] = 0x0004;
*(unsigned short *) &buf[24] = 0x1433;
buf[20] = 0x48;
buf[21] = 0xB2;
buf[22] = 0x00;
buf[23] = 0x04;
buf[24] = 0x14;
buf[25] = 0x33;
I guess the 16 bit numbers originally written had some meaning, so there should be at least a comment explaining the code.
Best regards,
Wolfgang Denk

We need to cast to unsigned char, not unsigned short here to avoid a warning.
Cc: Michael Barkowski michael.barkowski@freescale.com Signed-off-by: Tom Rini trini@ti.com --- board/freescale/mpc8323erdb/mpc8323erdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mpc8323erdb/mpc8323erdb.c b/board/freescale/mpc8323erdb/mpc8323erdb.c index f29b2f4..ba4993e 100644 --- a/board/freescale/mpc8323erdb/mpc8323erdb.c +++ b/board/freescale/mpc8323erdb/mpc8323erdb.c @@ -195,7 +195,7 @@ int mac_read_from_eeprom(void) printf("\nEEPROM @ 0x%02x read FAILED!!!\n", CONFIG_SYS_I2C_EEPROM_ADDR); } else { - if (crc32(crc, buf, 24) == *(unsigned int *)&buf[24]) { + if (crc32(crc, buf, 24) == *(unsigned char *)&buf[24]) { printf("Reading MAC from EEPROM\n"); for (i = 0; i < 4; i++) { if (memcmp(&buf[i * 6], "\0\0\0\0\0\0", 6)) {

Dear Tom Rini,
In message 1368477117-32669-4-git-send-email-trini@ti.com you wrote:
We need to cast to unsigned char, not unsigned short here to avoid a warning.
You fix the warning, but break the code.
Cc: Michael Barkowski michael.barkowski@freescale.com Signed-off-by: Tom Rini trini@ti.com
board/freescale/mpc8323erdb/mpc8323erdb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/freescale/mpc8323erdb/mpc8323erdb.c b/board/freescale/mpc8323erdb/mpc8323erdb.c index f29b2f4..ba4993e 100644 --- a/board/freescale/mpc8323erdb/mpc8323erdb.c +++ b/board/freescale/mpc8323erdb/mpc8323erdb.c @@ -195,7 +195,7 @@ int mac_read_from_eeprom(void) printf("\nEEPROM @ 0x%02x read FAILED!!!\n", CONFIG_SYS_I2C_EEPROM_ADDR); } else {
if (crc32(crc, buf, 24) == *(unsigned int *)&buf[24]) {
if (crc32(crc, buf, 24) == *(unsigned char *)&buf[24]) {
We need to read a 32 bit number here.
Best regards,
Wolfgang Denk

Dear Tom Rini,
In message 1368477117-32669-1-git-send-email-trini@ti.com you wrote:
When casting im_dprambase we must cast to unsigned char not unsigned short.
No, this is wrong. Now you write only 8 bit where 16 bit needs to be written.
Best regards,
Wolfgang Denk
participants (3)
-
Scott Wood
-
Tom Rini
-
Wolfgang Denk