[U-Boot] [PATCH] AT91RM9200: real pointer variable for PORT A to D configuration
Wolfgang Denk
wd at denx.de
Wed Oct 28 15:39:05 CET 2009
Dear Jens Scharsig,
In message <4AE832F4.2090601 at bus-elektronik.de> you wrote:
> adds real pointer variable for use with use I/O accessors
>
> * defines for PORT A to D configuration registers
> * defines for SMC configuration registers
>
> Signed-off-by: Jens Scharsig <esw at bus-elektronik.de>
> ---
NAK.
> diff --git a/include/asm-arm/arch-at91rm9200/AT91RM9200.h b/include/asm-arm/arch-at91rm9200/AT91RM9200.h
> index 00bae1c..1b4667e 100644
> --- a/include/asm-arm/arch-at91rm9200/AT91RM9200.h
> +++ b/include/asm-arm/arch-at91rm9200/AT91RM9200.h
> @@ -251,6 +251,15 @@ typedef struct _AT91S_SMC2
> AT91_REG SMC2_CSR[8]; /* SMC2 Chip Select Register */
> } AT91S_SMC2, *AT91PS_SMC2;
>
> +#define AT91C_SMC_CSR0 ((AT91_REG *) 0xFFFFFF70)
> +#define AT91C_SMC_CSR1 ((AT91_REG *) 0xFFFFFF74)
> +#define AT91C_SMC_CSR2 ((AT91_REG *) 0xFFFFFF78)
> +#define AT91C_SMC_CSR3 ((AT91_REG *) 0xFFFFFF7C)
> +#define AT91C_SMC_CSR4 ((AT91_REG *) 0xFFFFFF80)
> +#define AT91C_SMC_CSR5 ((AT91_REG *) 0xFFFFFF84)
> +#define AT91C_SMC_CSR6 ((AT91_REG *) 0xFFFFFF88)
> +#define AT91C_SMC_CSR7 ((AT91_REG *) 0xFFFFFF8C)
Please declare this as a C struct.
> /* -------- SMC2_CSR : (SMC2 Offset: 0x0) SMC2 Chip Select Register -------- */
> #define AT91C_SMC2_NWS ((unsigned int) 0x7F << 0) /* (SMC2) Number of Wait States */
> #define AT91C_SMC2_WSEN ((unsigned int) 0x1 << 7) /* (SMC2) Wait State Enable */
> @@ -586,6 +595,7 @@ typedef struct _AT91S_PDC
> #define AT91C_PMC_PCDR ((AT91_REG *) 0xFFFFFC14) /* (PMC) Peripheral Clock Enable Register */
> #define AT91C_PMC_SCER ((AT91_REG *) 0xFFFFFC00) /* (PMC) Peripheral Clock Enable Register */
> #define AT91C_PMC_SCDR ((AT91_REG *) 0xFFFFFC04) /* (PMC) Peripheral Clock Enable Register */
> +
> #define AT91C_PIOA_PER ((AT91_REG *) 0xFFFFF400) /* (PIOA) PIO Enable Register */
> #define AT91C_PIOA_PDR ((AT91_REG *) 0xFFFFF404) /* (PIOA) PIO Disable Register */
> #define AT91C_PIOA_PSR ((AT91_REG *) 0xFFFFF408) /* (PIOA) PIO Status Register */
> @@ -615,7 +625,96 @@ typedef struct _AT91S_PDC
> #define AT91C_PIOA_OWER ((AT91_REG *) 0xFFFFF4A0) /* (PIOA) PIO Output Write Enable Register */
> #define AT91C_PIOA_OWDR ((AT91_REG *) 0xFFFFF4A4) /* (PIOA) PIO Output Write Disable Register */
> #define AT91C_PIOA_OWSR ((AT91_REG *) 0xFFFFF4A8) /* (PIOA) PIO Output Write Status Register */
> -#define AT91C_PIOB_PDR ((AT91_REG *) 0xFFFFF604) /* (PIOB) PIO Disable Register */
> +
> +#define AT91C_PIOB_PER ((AT91_REG *) 0xFFFFF600)
> +#define AT91C_PIOB_PDR ((AT91_REG *) 0xFFFFF604)
> +#define AT91C_PIOB_PSR ((AT91_REG *) 0xFFFFF608)
> +#define AT91C_PIOB_OER ((AT91_REG *) 0xFFFFF610)
> +#define AT91C_PIOB_ODR ((AT91_REG *) 0xFFFFF614)
> +#define AT91C_PIOB_OSR ((AT91_REG *) 0xFFFFF618)
> +#define AT91C_PIOB_IFER ((AT91_REG *) 0xFFFFF620)
> +#define AT91C_PIOB_IFDR ((AT91_REG *) 0xFFFFF624)
> +#define AT91C_PIOB_IFSR ((AT91_REG *) 0xFFFFF628)
> +#define AT91C_PIOB_SODR ((AT91_REG *) 0xFFFFF630)
> +#define AT91C_PIOB_CODR ((AT91_REG *) 0xFFFFF634)
> +#define AT91C_PIOB_ODSR ((AT91_REG *) 0xFFFFF638)
> +#define AT91C_PIOB_PDSR ((AT91_REG *) 0xFFFFF63C)
> +#define AT91C_PIOB_IER ((AT91_REG *) 0xFFFFF640)
> +#define AT91C_PIOB_IDR ((AT91_REG *) 0xFFFFF644)
> +#define AT91C_PIOB_IMR ((AT91_REG *) 0xFFFFF648)
> +#define AT91C_PIOB_ISR ((AT91_REG *) 0xFFFFF64C)
> +#define AT91C_PIOB_MDER ((AT91_REG *) 0xFFFFF650)
> +#define AT91C_PIOB_MDDR ((AT91_REG *) 0xFFFFF654)
> +#define AT91C_PIOB_MDSR ((AT91_REG *) 0xFFFFF658)
> +#define AT91C_PIOB_PUDR ((AT91_REG *) 0xFFFFF660)
> +#define AT91C_PIOB_PUER ((AT91_REG *) 0xFFFFF664)
> +#define AT91C_PIOB_PUSR ((AT91_REG *) 0xFFFFF668)
> +#define AT91C_PIOB_ASR ((AT91_REG *) 0xFFFFF670)
> +#define AT91C_PIOB_BSR ((AT91_REG *) 0xFFFFF674)
> +#define AT91C_PIOB_ABSR ((AT91_REG *) 0xFFFFF678)
> +#define AT91C_PIOB_OWER ((AT91_REG *) 0xFFFFF6A0)
> +#define AT91C_PIOB_OWDR ((AT91_REG *) 0xFFFFF6A4)
> +#define AT91C_PIOB_OWSR ((AT91_REG *) 0xFFFFF6A8)
> +
> +#define AT91C_PIOC_PER ((AT91_REG *) 0xFFFFF800)
> +#define AT91C_PIOC_PDR ((AT91_REG *) 0xFFFFF804)
> +#define AT91C_PIOC_PSR ((AT91_REG *) 0xFFFFF808)
> +#define AT91C_PIOC_OER ((AT91_REG *) 0xFFFFF810)
> +#define AT91C_PIOC_ODR ((AT91_REG *) 0xFFFFF814)
> +#define AT91C_PIOC_OSR ((AT91_REG *) 0xFFFFF818)
> +#define AT91C_PIOC_IFER ((AT91_REG *) 0xFFFFF820)
> +#define AT91C_PIOC_IFDR ((AT91_REG *) 0xFFFFF824)
> +#define AT91C_PIOC_IFSR ((AT91_REG *) 0xFFFFF828)
> +#define AT91C_PIOC_SODR ((AT91_REG *) 0xFFFFF830)
> +#define AT91C_PIOC_CODR ((AT91_REG *) 0xFFFFF834)
> +#define AT91C_PIOC_ODSR ((AT91_REG *) 0xFFFFF838)
> +#define AT91C_PIOC_PDSR ((AT91_REG *) 0xFFFFF83C)
> +#define AT91C_PIOC_IER ((AT91_REG *) 0xFFFFF840)
> +#define AT91C_PIOC_IDR ((AT91_REG *) 0xFFFFF844)
> +#define AT91C_PIOC_IMR ((AT91_REG *) 0xFFFFF848)
> +#define AT91C_PIOC_ISR ((AT91_REG *) 0xFFFFF84C)
> +#define AT91C_PIOC_MDER ((AT91_REG *) 0xFFFFF850)
> +#define AT91C_PIOC_MDDR ((AT91_REG *) 0xFFFFF854)
> +#define AT91C_PIOC_MDSR ((AT91_REG *) 0xFFFFF858)
> +#define AT91C_PIOC_PUDR ((AT91_REG *) 0xFFFFF860)
> +#define AT91C_PIOC_PUER ((AT91_REG *) 0xFFFFF864)
> +#define AT91C_PIOC_PUSR ((AT91_REG *) 0xFFFFF868)
> +#define AT91C_PIOC_ASR ((AT91_REG *) 0xFFFFF870)
> +#define AT91C_PIOC_BSR ((AT91_REG *) 0xFFFFF874)
> +#define AT91C_PIOC_ABSR ((AT91_REG *) 0xFFFFF878)
> +#define AT91C_PIOC_OWER ((AT91_REG *) 0xFFFFF8A0)
> +#define AT91C_PIOC_OWDR ((AT91_REG *) 0xFFFFF8A4)
> +#define AT91C_PIOC_OWSR ((AT91_REG *) 0xFFFFF8A8)
> +
> +#define AT91C_PIOD_PER ((AT91_REG *) 0xFFFFFA00)
> +#define AT91C_PIOD_PDR ((AT91_REG *) 0xFFFFFA04)
> +#define AT91C_PIOD_PSR ((AT91_REG *) 0xFFFFFA08)
> +#define AT91C_PIOD_OER ((AT91_REG *) 0xFFFFFA10)
> +#define AT91C_PIOD_ODR ((AT91_REG *) 0xFFFFFA14)
> +#define AT91C_PIOD_OSR ((AT91_REG *) 0xFFFFFA18)
> +#define AT91C_PIOD_IFER ((AT91_REG *) 0xFFFFFA20)
> +#define AT91C_PIOD_IFDR ((AT91_REG *) 0xFFFFFA24)
> +#define AT91C_PIOD_IFSR ((AT91_REG *) 0xFFFFFA28)
> +#define AT91C_PIOD_SODR ((AT91_REG *) 0xFFFFFA30)
> +#define AT91C_PIOD_CODR ((AT91_REG *) 0xFFFFFA34)
> +#define AT91C_PIOD_ODSR ((AT91_REG *) 0xFFFFFA38)
> +#define AT91C_PIOD_PDSR ((AT91_REG *) 0xFFFFFA3C)
> +#define AT91C_PIOD_IER ((AT91_REG *) 0xFFFFFA40)
> +#define AT91C_PIOD_IDR ((AT91_REG *) 0xFFFFFA44)
> +#define AT91C_PIOD_IMR ((AT91_REG *) 0xFFFFFA48)
> +#define AT91C_PIOD_ISR ((AT91_REG *) 0xFFFFFA4C)
> +#define AT91C_PIOD_MDER ((AT91_REG *) 0xFFFFFA50)
> +#define AT91C_PIOD_MDDR ((AT91_REG *) 0xFFFFFA54)
> +#define AT91C_PIOD_MDSR ((AT91_REG *) 0xFFFFFA58)
> +#define AT91C_PIOD_PUDR ((AT91_REG *) 0xFFFFFA60)
> +#define AT91C_PIOD_PUER ((AT91_REG *) 0xFFFFFA64)
> +#define AT91C_PIOD_PUSR ((AT91_REG *) 0xFFFFFA68)
> +#define AT91C_PIOD_ASR ((AT91_REG *) 0xFFFFFA70)
> +#define AT91C_PIOD_BSR ((AT91_REG *) 0xFFFFFA74)
> +#define AT91C_PIOD_ABSR ((AT91_REG *) 0xFFFFFA78)
> +#define AT91C_PIOD_OWER ((AT91_REG *) 0xFFFFFAA0)
> +#define AT91C_PIOD_OWDR ((AT91_REG *) 0xFFFFFAA4)
> +#define AT91C_PIOD_OWSR ((AT91_REG *) 0xFFFFFAA8)
I really have to wonder how people can write teh same thing several
times without noticing they are just repeting the same stuff, and
without doiung something about it?
For all these items a _single_ C struct would be sufficient. Please do
this!
> +#define AT91C_PIO_PD0 ((unsigned int) 1 << 0)
> +#define AT91C_PIO_PD1 ((unsigned int) 1 << 1)
> +#define AT91C_PIO_PD2 ((unsigned int) 1 << 2)
> +#define AT91C_PIO_PD3 ((unsigned int) 1 << 3)
> +#define AT91C_PIO_PD4 ((unsigned int) 1 << 4)
> +#define AT91C_PIO_PD5 ((unsigned int) 1 << 5)
> +#define AT91C_PIO_PD6 ((unsigned int) 1 << 6)
> +#define AT91C_PIO_PD7 ((unsigned int) 1 << 7)
> +#define AT91C_PIO_PD8 ((unsigned int) 1 << 8)
> +#define AT91C_PIO_PD9 ((unsigned int) 1 << 9)
> +#define AT91C_PIO_PD10 ((unsigned int) 1 << 10)
> +#define AT91C_PIO_PD11 ((unsigned int) 1 << 11)
> +#define AT91C_PIO_PD12 ((unsigned int) 1 << 12)
> +#define AT91C_PIO_PD13 ((unsigned int) 1 << 13)
> +#define AT91C_PIO_PD14 ((unsigned int) 1 << 14)
> +#define AT91C_PIO_PD15 ((unsigned int) 1 << 15)
> +#define AT91C_PIO_PD16 ((unsigned int) 1 << 16)
> +#define AT91C_PIO_PD17 ((unsigned int) 1 << 17)
> +#define AT91C_PIO_PD18 ((unsigned int) 1 << 18)
> +#define AT91C_PIO_PD19 ((unsigned int) 1 << 19)
> +#define AT91C_PIO_PD20 ((unsigned int) 1 << 20)
> +#define AT91C_PIO_PD21 ((unsigned int) 1 << 21)
> +#define AT91C_PIO_PD22 ((unsigned int) 1 << 22)
> +#define AT91C_PIO_PD23 ((unsigned int) 1 << 23)
> +#define AT91C_PIO_PD24 ((unsigned int) 1 << 24)
> +#define AT91C_PIO_PD25 ((unsigned int) 1 << 25)
> +#define AT91C_PIO_PD26 ((unsigned int) 1 << 26)
> +#define AT91C_PIO_PD27 ((unsigned int) 1 << 27)
Um... what good is this supposed to do? Do you think this makes the
code any better readable or easier to understand?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I'm what passes for a Unix guru in my office. This is a frightening
concept. - Lee Ann Goldstein, in <3k55ba$c43 at butch.lmsc.lockheed.com>
More information about the U-Boot
mailing list