[U-Boot] [PATCH 02/11] at91: Add USART & DBGU base address defines

[Reinhard Meyer]

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.stein at 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