[U-Boot-Users] [PATCH] word alignment fixes for word aligned NS16550 UART peripheral

[Wolfgang Denk]

Dear Jean-Paul,

in message <OF43B05629.5234CA85-ONC1256FB1.003C850C-C1256FB1.003CAA59 at 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

-- 
See us @ Embedded World, Nuremberg, Feb 22 - 24,  Hall 10.0 Booth 310
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Humans do claim a great deal for that particular emotion (love).
	-- Spock, "The Lights of Zetar", stardate 5725.6