[U-Boot] [RFC] AT91 cleanup, was: Re: [PATCH 02/11] at91: Add USART & DBGU base address defines

[Reinhard Meyer]

Reinhard Meyer schrieb:
> 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>
...

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

I leaped a bit ahead there, and did some cleanups already. Note that they are
not complete and are up for discussion. In order to avoid flooding the list with
patches, you can have a look at u-boot-atmel, at91cleanup branch.

With best regards,
Reinhard