public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Reinhard Meyer <u-boot@emk-elektronik.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 02/11] at91: Add USART & DBGU base address	defines
Date: Tue, 02 Nov 2010 11:42:08 +0100	[thread overview]
Message-ID: <4CCFEB00.6000603@emk-elektronik.de> (raw)
In-Reply-To: <4CCEAFF1.3080603@emk-elektronik.de>

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.

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

  reply	other threads:[~2010-11-02 10:42 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-01  8:29 [U-Boot] various at91 patches to make some boards compilable again Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 01/11] at91sam9260: Replace PHYS_SDRAM with CONFIG_SYS_SDRAM_BASE Alexander Stein
2010-11-01  9:33   ` Wolfgang Denk
2010-11-01  8:29 ` [U-Boot] [PATCH 02/11] at91: Add USART & DBGU base address defines Alexander Stein
2010-11-01 10:57   ` Andreas Bießmann
2010-11-01 11:05     ` Alexander Stein
2010-11-01 11:26       ` Andreas Bießmann
2010-11-01 12:17         ` Reinhard Meyer
2010-11-02 10:42           ` Reinhard Meyer [this message]
2010-11-02 14:11             ` [U-Boot] [RFC] AT91 cleanup, was: " Reinhard Meyer
2010-11-03  7:18               ` Alexander Stein
2010-11-03  9:17                 ` Reinhard Meyer
2010-11-03  9:29                   ` Andreas Bießmann
2010-11-03 10:03                     ` Reinhard Meyer
2010-11-03 10:25                       ` Andreas Bießmann
2010-11-03 10:20               ` [U-Boot] [PATCH 1/2] [RFC] AVR32: move memory-map.h to hardware.h Andreas Bießmann
2010-11-03 15:02                 ` Reinhard Meyer
2010-11-03 15:37                 ` Reinhard Meyer
2010-11-05  9:15                 ` [U-Boot] [PATCH v2 0/2] adopt avr32 to latest changes in u-boot-atmel/at91cleanup-rework Andreas Bießmann
2010-11-05  9:15                 ` [U-Boot] [PATCH v2 1/2] avr32: rename memory-map.h -> hardware.h Andreas Bießmann
2011-04-11  8:55                   ` Reinhard Meyer
2010-11-05  9:15                 ` [U-Boot] [PATCH v2 2/2] avr32: fixup definitions to ATMEL_BASE_xxx Andreas Bießmann
2010-11-05 10:34                   ` Reinhard Meyer
2011-04-11  8:58                   ` Reinhard Meyer
2010-11-03 10:20               ` [U-Boot] [PATCH 2/2] [RFC] drivers:atmel-related: fixup include to hardware.h Andreas Bießmann
2010-11-03 15:04                 ` Reinhard Meyer
2010-11-01  8:29 ` [U-Boot] [PATCH 03/11] at91: Use AT91_USART0_BASE instead of AT91_USART0 Alexander Stein
2010-11-01 10:56   ` Andreas Bießmann
2010-11-01  8:29 ` [U-Boot] [PATCH 04/11] at91sam9260ek: Convert to SoC Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 05/11] at91: Fix pdc register names and add some defines Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 06/11] at91: Add defines for spi SoC access Alexander Stein
2010-11-01  8:43   ` Reinhard Meyer
2010-11-01  8:52     ` Alexander Stein
2010-11-01  9:36     ` Wolfgang Denk
2010-11-01 12:31       ` Reinhard Meyer
2010-11-01 13:34         ` Alexander Stein
2010-11-04 16:07         ` Thomas Petazzoni
2010-11-01  8:29 ` [U-Boot] [PATCH 07/11] at91: Add spi base addresses Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 08/11] atmel_dataflash: Update to SoC access Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 09/11] at91sam9261ek: make compilable again Alexander Stein
2010-11-01  8:29 ` [U-Boot] [PATCH 10/11] at91sam9263ek: " Alexander Stein
2010-11-01  9:39   ` Wolfgang Denk
2010-11-01  8:29 ` [U-Boot] [PATCH 11/11] at91cap9adk: " Alexander Stein
2010-11-01  8:40 ` [U-Boot] various at91 patches to make some boards " Reinhard Meyer
2010-11-01  8:50   ` Alexander Stein
2010-11-01  9:30     ` Wolfgang Denk
2010-11-01 11:09     ` Andreas Bießmann
2011-01-27 19:18   ` Remy Bohmer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CCFEB00.6000603@emk-elektronik.de \
    --to=u-boot@emk-elektronik.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox