Am 22.04.2010 14:34, schrieb Wolfgang Denk:
> Dear Matthias Weisser,
>
> In message<1271932257-14618-2-git-send-email-weiss...@arcor.de> you
wrote:
>> This patch adds support for MB86R0x SoCs from Fujitsu
>>
>> Signed-off-by: Matthias Weisser<weiss...@arcor.de>
> ...
>> --- /dev/null
>> +++ b/arch/arm/cpu/arm926ejs/mb86r0x/reset.c
> ...
>> +void reset_cpu(ulong ignored)
>> +{
>> + writel(0x00000002, MB86R0x_CRG_PHYS_BASE + CRG_CRSR);
>
> Please define some symbolic name for the magic constant 0x00000002
>
> Also, we do not accept code based on a "base + offset" notation.
> Please use C structures instead. [Please fix globally.]
I will fix this
>> +#define TIMER_LOAD_VAL 0xffffffff
>> +#define TIMER_BASE MB86R0x_TIMER_PHYS_BASE
>> +
>> +#define TIMER_REG_LOAD (TIMER_BASE + 0)
>> +#define TIMER_REG_VALUE (TIMER_BASE + 4)
>> +#define TIMER_REG_CONTROL (TIMER_BASE + 8)
>
> Create a proper C struct, please!
and this
>> +/*
>> + * Offset definitions for DRAM controller
>> + */
>> +#define DDR2C_DRIC 0x00
>> +#define DDR2C_DRIC1 0x02
>> +#define DDR2C_DRIC2 0x04
>> +#define DDR2C_DRCA 0x06
>> +#define DDR2C_DRCM 0x08
>> +#define DDR2C_DRCST1 0x0a
>> +#define DDR2C_DRCST2 0x0c
>> +#define DDR2C_DRCR 0x0e
>> +#define DDR2C_DRCF 0x20
>> +#define DDR2C_DRASR 0x30
>> +#define DDR2C_DRIMS 0x50
>> +#define DDR2C_DROS 0x60
>> +#define DDR2C_DRIBSLI 0x62
>> +#define DDR2C_DRIBSODT1 0x64
>> +#define DDR2C_DRIBSOCD 0x66
>> +#define DDR2C_DRIBSOCD2 0x68
>> +#define DDR2C_DROABA 0x70
>> +#define DDR2C_DROBV 0x80
>> +#define DDR2C_DROBS 0x84
>> +#define DDR2C_DROBSR1 0x86
>> +#define DDR2C_DROBSR2 0x88
>> +#define DDR2C_DROBSR3 0x8a
>> +#define DDR2C_DROBSR4 0x8c
>> +#define DDR2C_DRIMR1 0x90
>> +#define DDR2C_DRIMR2 0x92
>> +#define DDR2C_DRIMR3 0x94
>> +#define DDR2C_DRIMR4 0x96
>> +#define DDR2C_DROISR1 0x98
>> +#define DDR2C_DROISR2 0x9a
>
> C struct, please.
>
>> +/*
>> + * Offset definitions Chip Control Module
>> + */
>> +#define CCNT_CCID 0x00
>> +#define CCNT_CSRST 0x1c
>> +#define CCNT_CIST 0x20
>> +#define CCNT_CISTM 0x24
>> +#define CCNT_CMUX_MD 0x30
>> +#define CCNT_CDCRC 0xec
>
> Ditto.
>
>> +/*
>> + * Offset definitions clock reset generator
>> + */
>> +#define CRG_CRPR 0x00
>> +#define CRG_CRWR 0x08
>> +#define CRG_CRSR 0x0c
>> +#define CRG_CRDA 0x10
>> +#define CRG_CRDB 0x14
>> +#define CRG_CRHA 0x18
>> +#define CRG_CRPA 0x1c
>> +#define CRG_CRPB 0x20
>> +#define CRG_CRHB 0x24
>> +#define CRG_CRAM 0x28
>
> Ditto.
Well, the above three modules are used in assembler code only
(lowlevel_init.S) and I didn't found a way to use C structs here. What
would be the right approach in this case? Defining all these registers
as absolute addresses?
I have a also a couple of magic values in the mentioned .S file. Do I
have to move them also to some symbolic constants?
Thanks for the quick review.
Regards,
Matthias
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
