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

Reinhard Meyer
2010-11-01T12:17:53Z

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@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.

With best regards,
Reinhard