On 2/1/07, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hmmm. You know that the signedness of char is not defined?
Yes, but if functions like strlen() take a char *, then shouldn't the parameter be signed rather than unsigned?
Besides that, we definitely should enable -fno-strict-aliasing in the gcc flags until we have audited all casts.
Agreed.
Please remove the magic constant 48 when you change this line. Same for all other magic 48.
Oh boy. You're asking me to understand the code I'm tidying? :-)
Index: LinuxBIOSv2-2540/src/southbridge/nvidia/mcp55/mcp55_aza.c
--- LinuxBIOSv2-2540.orig/src/southbridge/nvidia/mcp55/mcp55_aza.c +++ LinuxBIOSv2-2540/src/southbridge/nvidia/mcp55/mcp55_aza.c @@ -27,6 +27,7 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <arch/io.h> +#include <delay.h> #include "mcp55.h"
static int set_bits(uint8_t *port, uint32_t mask, uint32_t val) @@ -228,7 +229,7 @@ static void aza_init(struct device *dev) if(!res) return;
base =(uint8_t *) res->base;
base =(uint8_t *) ((uint32_t) res->base); printk_debug("base = %08x\n", base); codec_mask = codec_detect(base);Are you really sure you want to cast a uint64_t to uint32_t to uint8_t * ?
I'm not sure. The current implementation truncates base to 32 bits, so I suppose there ought to be a check that base doesn't exceed 0xffffffff. But in this case the double-cast is still necessary to avoid gcc's complaint about truncating while casting to a pointer.
=================================================================== --- LinuxBIOSv2-2540.orig/src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c +++ LinuxBIOSv2-2540/src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c @@ -175,7 +175,8 @@ static void mcp55_early_pcie_setup(unsig pci_write_config32(dev, 0xe4, dword);
// need to wait 100ms
delayx(1000);
for (i=0; i<10; i++)delayx(100);}
static void mcp55_early_setup(unsigned mcp55_num, unsigned *busn, unsigned *devn, unsigned *io_base, unsigned *pci_e_x)
Nack. You increased the delay by a factor of 10.
This alters the existing behavior, yes. But if the comments in the code are to be believed, delayx(1) waits 100 usec, so for a 100-msec delay we have to call delayx(100) 10 times.
--Ed