Dear Jean-Paul,
in message OF43B05629.5234CA85-ONC1256FB1.003C850C-C1256FB1.003CAA59@philips.com you wrote:
The patch is created against current CVS 2005 feb. 23 and fixes problems for boards that only have word access to the NS16550 UART peripheral.
CHANGELOG:
- Patch by Jean-Paul Saman, 23 Feb 2005 common/cmd_bdinfo.c - fixes bug with negative baudrate: correct use of unsigned long datatype for
bi_baudrate, properly cast bd_t *bd = (bd_t*) gd->bd; make information more clear: print_num("DRAM bank nr", i); include/asm-arm/u-boot.h - fixes bug with negative baudrate: use of unsigned long for bi_baudrate and adding an unsigned char pad[2] makes the struct word aligned changed ulong into unsigned long for consistency include/ns16550.h - fixes alignement bug with UART that only supports word aligned access: removed "__attribute__ ((packed));" for "(CFG_NS16550_REG_SIZE == 4)" GCC generates bytes access when it encounters the packed attribute regardless if the struct is already word aligned for a platform. Peripherals that can only handle word aligned access won't work properly when accessed with byte access. The struct NS16550 is already word aligned for REG_SIZE = 4, so there is no need to packed the struct in that case.
I reject this patch because I cannot read the "CHANGELOG". Please stick to standard netiquette rules for formatting. See also the Coding Style rules in the README.
diff -X ignore -urNpb u-boot-cvs/common/cmd_bdinfo.c u-boot-aps/common/cmd_bdinfo.c --- u-boot-cvs/common/cmd_bdinfo.c 2004-12-31 10:32:50.000000000 +0100 +++ u-boot-aps/common/cmd_bdinfo.c 2005-02-23 09:18:42.000000000 +0100 @@ -216,14 +216,14 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int fl DECLARE_GLOBAL_DATA_PTR;
int i;
bd_t *bd = gd->bd;
bd_t *bd = (bd_t*) gd->bd;
Why do you add a cast here? None is needed - "bd" is already of type "bd_t *". Rejected.
print_num("DRAM bank", i);
print_num("DRAM bank nr", i);
Why do you change the output format here? Rejected.
@@ -236,7 +236,7 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int fl "ip_addr = "); print_IPaddr (bd->bi_ip_addr); printf ("\n"
"baudrate = %d bps\n", bd->bi_baudrate);
"baudrate = %u bps\n", bd->bi_baudrate );
How could you end up with a negative baudrate?
+++ u-boot-aps/include/asm-arm/u-boot.h 2005-02-04 15:24:04.000000000 +0100 @@ -30,16 +30,17 @@ #define _U_BOOT_H_ 1
typedef struct bd_info {
- int bi_baudrate; /* serial console baudrate */
- unsigned long bi_baudrate; /* serial console baudrate */
Not necessary.
unsigned long bi_ip_addr; /* IP Address */ unsigned char bi_enetaddr[6]; /* Ethernet adress */
- unsigned char pad[2];
Not necessary.
struct environment_s *bi_env;
- ulong bi_arch_number; /* unique id for this board */
- ulong bi_boot_params; /* where this board expects params */
- unsigned long bi_arch_number; /* unique id for this board */
- unsigned long bi_boot_params; /* where this board expects params */
Bogus change.
struct /* RAM configuration */ {
ulong start;ulong size;
unsigned long start;unsigned long size;
Bogus change.
Rejected.
Best regards,
Wolfgang Denk