[U-Boot-Users] "2.mach_types":(1 of 1) [PATCH][ARM] Create mach-types for various at91 boards

[Grant Likely]

On 3/27/07, Ulf Samuelsson <ulf at atmel.com> wrote:
> diff -urN u-boot-1.2.0/include/asm-arm/mach-types.h u-boot-1.2.0-atmel/include/asm-arm/mach-types.h
> --- u-boot-1.2.0/include/asm-arm/mach-types.h   2007-01-07 00:13:11.000000000 +0100
> +++ u-boot-1.2.0-atmel/include/asm-arm/mach-types.h     2007-03-24 20:07:34.000000000 +0100
> @@ -736,7 +736,11 @@
>  #define MACH_TYPE_LN2410SBC            725
>  #define MACH_TYPE_CB3RUFC              726
>  #define MACH_TYPE_MP2USB               727
> +#define MACH_TYPE_AT91SAM9261EK        848
>  #define MACH_TYPE_PDNB3               1002
> +#define MACH_TYPE_AT91SAM9260EK       1099
> +#define MACH_TYPE_AT91RM9200DF 1119
> +#define MACH_TYPE_AT91SAM9263EK        1202
>
>  #ifdef CONFIG_ARCH_EBSA110
>  # ifdef machine_arch_type
> @@ -9402,6 +9406,66 @@
>  # define machine_is_mp2usb()   (0)
>  #endif
>
> +#ifdef CONFIG_MACH_AT91SAM9261EK
> +# ifdef machine_arch_type
> +#  undef machine_arch_type
> +#  define machine_arch_type    __machine_arch_type
> +# else
> +#  define machine_arch_type    MACH_TYPE_AT91SAM9261EK
> +# endif
> +# define machine_is_at91sam9261ek()    (machine_arch_type == MACH_TYPE_AT91SAM9261EK)
> +#else
> +# define machine_is_at91sam9261ek()    (0)
> +#endif

(Note: I realize this is a comment on the entire file, and your change
just matches the existing code, but I'm going to bring it up
anyway...)

I'm confused by this construct.  What is the reason for the large
#ifdef block?  As I read it, there are two conditions for each board;
1. #ifdef CONFIG_MACH_XXXX
2. #ifdef machine_arch_type

Regardless of the result of condition 2, 'machine_arch_type' is always
defined to something.  That something either matches the current
board, or it does not, and therefore the test in the first definition
of machine_is_XXXXX() should always evaluate correctly.

To me, it seems like quite a verbose construct when something simpler
could be used.  For example:

#ifdef machine_arch_type
# undef machine_arch_type
# define machine_arch_type    __machine_arch_type
#else
# ifdef CONFIG_MACH_ARMBOARD1
#  define machine_arch_type    MACH_TYPE_ARMBOARD1
# elif CONFIG_MACH_ARMBOARD2
#  define machine_arch_type    MACH_TYPE_ARMBOARD1
...etc for all boards...
# endif
#endif

#define machine_is_board1()    (machine_arch_type == MACH_TYPE_BOARD1)
#define machine_is_board2()    (machine_arch_type == MACH_TYPE_BOARD2)
...etc

Am I missing something?

Cheers,
g.

-- 
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195