Dear All concerned, >>> On Monday 01 November 2010, 11:57:14 Andreas Bießmann wrote: >>>> Am 01.11.2010 um 09:29 schrieb Alexander Stein: >>>>> Signed-off-by: Alexander Stein<alexander.st...@systec-electronic.com> >>>>> --- >>>>> arch/arm/include/asm/arch-at91/at91cap9.h | 12 ++++++++---- >>>>> arch/arm/include/asm/arch-at91/at91sam9260.h | 7 +++++++ >>>>> arch/arm/include/asm/arch-at91/at91sam9261.h | 4 ++++ >>>>> arch/arm/include/asm/arch-at91/at91sam9263.h | 3 +++ >>>>> arch/arm/include/asm/arch-at91/at91sam9g45.h | 5 +++++ >>>>> arch/arm/include/asm/arch-at91/at91sam9rl.h | 4 ++++ >>>>> 6 files changed, 31 insertions(+), 4 deletions(-) >>>>> >>>>> --- a/arch/arm/include/asm/arch-at91/at91sam9g45.h >>>>> +++ b/arch/arm/include/asm/arch-at91/at91sam9g45.h >>>>> @@ -51,9 +51,14 @@ >>>>> #define AT91SAM9G45_ID_VDEC 30 /* Video Decoder */ >>>>> #define AT91SAM9G45_ID_IRQ0 31 /* Advanced Interrupt >>>>> Controller */ >>>>> >>>>> +#define AT91_USART0_BASE 0xfff8c000 >>>>> +#define AT91_USART1_BASE 0xfff90000 >>>>> +#define AT91_USART2_BASE 0xfff94000 >>>>> +#define AT91_USART3_BASE 0xfff98000 >>>>> #define AT91_EMAC_BASE 0xfffbc000 >>>>> #define AT91_SMC_BASE 0xffffe800 >>>>> #define AT91_MATRIX_BASE 0xffffea00 >>>>> +#define AT91_DBGU_BASE 0xffffee00 >>>>> #define AT91_PIO_BASE 0xfffff200 >>>>> #define AT91_PMC_BASE 0xfffffc00 >>>>> #define AT91_RSTC_BASE 0xfffffd00 >>>>> diff --git a/arch/arm/include/asm/arch-at91/at91sam9rl.h >>>>> b/arch/arm/include/asm/arch-at91/at91sam9rl.h index 8eb0d4f..ffa6687 >>>>> 100644 >>>>> >>>>> >>>>> --- a/arch/arm/include/asm/arch-at91/at91sam9rl.h >>>>> +++ b/arch/arm/include/asm/arch-at91/at91sam9rl.h >>>>> @@ -44,6 +44,10 @@ >>>>> #define AT91SAM9RL_ID_AC97C 24 /* AC97 Controller */ >>>>> #define AT91SAM9RL_ID_IRQ0 31 /* Advanced Interrupt >>>>> Controller (IRQ0) >>> */ >>>>> +#define AT91_US0_BASE 0xfffb0000 >>>>> +#define AT91_US1_BASE 0xfffb4000 >>>>> +#define AT91_US2_BASE 0xfffb8000 >>>>> +#define AT91_US3_BASE 0xfffbc000 >>>>> #define AT91_SDRAMC_BASE 0xffffea00 >>>>> #define AT91_SMC_BASE 0xffffec00 >>>>> #define AT91_MATRIX_BASE 0xffffee00 >>>> can we just use one naming scheme here? I dunno whether it should be >>>> AT91_USx or AT91_USARTx but it should be the same in any case. >>> Yes, sure. I justed copied the dfine and reworded it to match the >>> AT91_$COMPONENT_BASE scheme. Always using USARTx is fine though. >> Hmm ... I just thought they have renamed their registers in spec, but all >> checked datasheets use US_xx for USART register names. >> I also prefer USART here but Reinhard can you please give a comment? > Honestly I would prefer a much more thorough cleanup on the long run > (or instantly): > > As was pointed out long ago and just now: a triple indirection of the defines > until used in the driver is bad. > > Since only one of the at91samxxxx.h files is included, all files should > directly define the address of a component like follows: > > #define ATMEL_USART0_BASE 0x'something' > > Note: that I used ATMEL, not AT91, since the same components are used in > AVR32 as well. > > The name should be descriptive enough, and preferably adhere to what the > component is called in the datasheet(s). (Hopefully its consistent there...) > > Another note: even if currently there is only one incarnation of some > peripherals like emac or mci we should try to number them starting from 0, > e.g. ATMEL_EMAC0_BASE. > > As a way to go there I can offer to make a branch at91cleanup where I > collect all work and which allows everyone to do incremental improvements > which can be squashed into fewer patches later. Then we don't need to base > all patch revisions on current master. > > If you see no other priority, I would delay "build-fixing" individual boards, > but concentrate on cleanup work. Right now is the best opportunity for that > since most boards are not building anyway and do not need to be utterly > concerned with breaking them even more. After the cleanup is done, we can > attend to fixing certain boards. > > Work could therefore start with cleaning up the at91samxxxx.h files. > I am also game with throwing out all LEGACY stuff and fix build problems > that occur when they occur. > > If you agree, the cleanup in the *.h files should entail: > > 1. remove legacy parts > 2. rename AT91xxxx_<component>_BASE consistently into ATMEL_<component>_BASE > and make for example EMAC into EMAC0, I2C into I2C0. I am currently unsure if > that needs to be done for SDRAMC, SMC. I think here we can live with SDRAMC > and having SDRAMC1 _if_ there is a second one. > 3. get rid of hardware.h and have a simple #if-elif-endif chain in > memory_map.h > instead. So that when memory_map.h is included and the proper #define > AT91SAMxxxx > is set we get exactly the defines valid for that SoC. memory_map.h is > consistent > with the AVR32 case. The common drivers all include memory_map.h. > > This *.h cleanup could be patch 1/n of a series. Each individual driver > cleanup > to remove legacy and adapt to the naming would be 2/n, 3/n, etc. I am willing > to collect them in the above mentioned branch and also squash incremental > improvements > later to produce a clean patch series later.
Another optical issue: We have ATxxx_ID_yyy consistently. but both ATxxx_BASE_yyy or ATxxx_yyy_BASE. ATxxx_BASE_yyy would be consistent with the ID variant and look better: ATMEL_BASE_USART0 ATMEL_BASE_SPI0 ... With best regards, Reinhard _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
