All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
@ 2012-07-06  8:15 Yann Vernier
  2012-07-06  8:43 ` Andreas Bießmann
  0 siblings, 1 reply; 10+ messages in thread
From: Yann Vernier @ 2012-07-06  8:15 UTC (permalink / raw)
  To: u-boot

On Thursday 05 July 2012 16:48:22 you wrote:
> On 05.07.2012 14:11, Yann Vernier wrote:
> > Leave dram_init_banksize to set up the bank info data.
> > ram_size was previously uninitialized. Also, generalize
> > lowlevel assembly to support more RAM options.
> > 
> > Signed-off-by: Yann Vernier <yann.vernier@orsoc.se>
> > ---
> > 
> > Changes for v2:
> >    - Update to use CONFIG_SYS_SDRAM_ constants
> >    - Update cm41xx also
> >    - Map SDRAM to match configuration
> > 
> > ---
> > 
> >  arch/arm/cpu/arm920t/ks8695/lowlevel_init.S |    8 +++-----
> >  board/cm4008/cm4008.c                       |    5 +++--
> >  board/cm41xx/cm41xx.c                       |    5 +++--
> >  include/configs/cm4008.h                    |    5 ++---
> >  include/configs/cm41xx.h                    |    5 ++---
> >  5 files changed, 13 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> > b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S index e9f1227..df13de6
> > 100644
> > --- a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> > +++ b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> > 
> > @@ -131,11 +131,9 @@ highflash:
> >  	 * before relocating, we have to setup RAM timing
> >  	 */
> >  	
> >  	ldr	r1, =(KS8695_IO_BASE+KS8695_SDRAM_CTRL0)
> > 
> > -#if (PHYS_SDRAM_1_SIZE == 0x02000000)
> > -	ldr	r2, =0x7fc0000e		/* 32MB */
> > -#else
> > -	ldr	r2, =0x3fc0000e		/* 16MB */
> > -#endif
> > +	/* 8 column address bits, 4 banks, 32 bits data width */
> > +	ldr	r2,
> > =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +		
> >      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
> 
> ugh ... magic. Will it work for every possible setting?
> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
> related to register content in any way?
> 
> Would be great to have a little documentation here why it is shifted
> that way, unfortunately I can not find a KS8695 spec in the net. Don't
> get me wrong it is ok to do so. I think it is way better than hard
> coding the values like before but would be great to understand why it is
> done that way.
> 
> Best regards
> 
> Andreas Bie?mann


It's not a guarded secret, although it is far from obvious where to find it. 
One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design 
Kit. Within that zip file, 
KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf

My personal thought on the matter is that the existing code is rather heavy on 
magic numbers, ergo the comment once I figured out what the first ROM 
reconfiguration was even for. 


The memory mapping registers allow you to reconfigure the address recognition 
for each memory region. The low 16 bits are not configurable, so you can only 
relocate to 64KiB aligned addresses. Bits 25-16 are configured as an inclusive 
range, with the top value in bits 31-22 of the register, and the bottom in 
bits 21-12. The lower bits configure things like SDRAM geometry or SRAM/ROM 
access times, with one value meaning disabled. 

Come to think of it, I fear I've miscalculated where the high region actually 
is. That comment in lowlevel_init should read 48MB and 32MB, not 768 and 512. 
I'll fix that and insert a little more explanatory text. 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-06  8:15 [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init Yann Vernier
@ 2012-07-06  8:43 ` Andreas Bießmann
  2012-07-06  9:14   ` Yann Vernier
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Bießmann @ 2012-07-06  8:43 UTC (permalink / raw)
  To: u-boot

Dear Yann Vernier,

On 06.07.2012 10:15, Yann Vernier wrote:
> On Thursday 05 July 2012 16:48:22 you wrote:
>> On 05.07.2012 14:11, Yann Vernier wrote:

<snip>

>>> +	/* 8 column address bits, 4 banks, 32 bits data width */
>>> +	ldr	r2,
>>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +		
>>>      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
>>
>> ugh ... magic. Will it work for every possible setting?
>> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
>> related to register content in any way?
>>
>> Would be great to have a little documentation here why it is shifted
>> that way, unfortunately I can not find a KS8695 spec in the net. Don't
>> get me wrong it is ok to do so. I think it is way better than hard
>> coding the values like before but would be great to understand why it is
>> done that way.
>>
>> Best regards
>>
>> Andreas Bie?mann
> 
> 
> It's not a guarded secret, although it is far from obvious where to find it. 
> One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design 
> Kit. Within that zip file, 
> KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf

With this information I understand your equation. I think you should not
do it that way. You may solve the current state (all devices have 8
column, 4 bank and 32 bit). But one adding (well, if that will ever come
;) another board with different setting will get in trouble here and
need to find another solution. Maybe more sophisticated by doing another
equation.

I think a straight forward solution here would be to add another special
define in the board config, write the magic number down there and maybe
describe what the number stands for. You can then just use the
previously defined value in start.S.
You may have a look for at91 lowlevel_init, there it is done that way.

> My personal thought on the matter is that the existing code is rather heavy on 
> magic numbers, ergo the comment once I figured out what the first ROM 
> reconfiguration was even for. 

Well, thats true.

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-06  8:43 ` Andreas Bießmann
@ 2012-07-06  9:14   ` Yann Vernier
  2012-07-06 10:25     ` Andreas Bießmann
  0 siblings, 1 reply; 10+ messages in thread
From: Yann Vernier @ 2012-07-06  9:14 UTC (permalink / raw)
  To: u-boot

On Friday 06 July 2012 10:43:40 you wrote:
> >>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
> >>>      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
> >> 
> >> ugh ... magic. Will it work for every possible setting?
> >> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
> >> related to register content in any way?
> > 
> > It's not a guarded secret, although it is far from obvious where to find
> > it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's -
> > HW Design Kit. Within that zip file,
> > KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description
> > v1.1.pdf
> 
> With this information I understand your equation. I think you should not
> do it that way. You may solve the current state (all devices have 8
> column, 4 bank and 32 bit). But one adding (well, if that will ever come
> ;) another board with different setting will get in trouble here and
> need to find another solution. Maybe more sophisticated by doing another
> equation.
> 
> I think a straight forward solution here would be to add another special
> define in the board config, write the magic number down there and maybe
> describe what the number stands for. You can then just use the
> previously defined value in start.S.
> You may have a look for at91 lowlevel_init, there it is done that way.

I shall. In fact, I may already have this problem as I need to verify the 
timing on the flash memory (I have the demo board, not a cm4xxx), and the 
reason I work on this is that we may design another board soon.

I'm a little hesitant about how to fit these changes together, though; for now 
I have three barely separated patches, but overhauling all those magic numbers 
changes them all yet again. These three patches are all necessary to make my 
u-boot work in the first place, while fixing the magic values is a code cleanup 
change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which 
currently contains a few meaningless constants like KS8695_SDRAM_START and a 
consistent misspelling of definitions. 

Could I save the value decoding and corresponding configuration changes for a 
fourth patch?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-06  9:14   ` Yann Vernier
@ 2012-07-06 10:25     ` Andreas Bießmann
  2012-07-09 21:58       ` Albert ARIBAUD
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Bießmann @ 2012-07-06 10:25 UTC (permalink / raw)
  To: u-boot

Dear Yann Vernier,

On 06.07.2012 11:14, Yann Vernier wrote:
> On Friday 06 July 2012 10:43:40 you wrote:
>>>>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
>>>>>      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
>>>>
>>>> ugh ... magic. Will it work for every possible setting?
>>>> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
>>>> related to register content in any way?
>>>
>>> It's not a guarded secret, although it is far from obvious where to find
>>> it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's -
>>> HW Design Kit. Within that zip file,
>>> KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description
>>> v1.1.pdf
>>
>> With this information I understand your equation. I think you should not
>> do it that way. You may solve the current state (all devices have 8
>> column, 4 bank and 32 bit). But one adding (well, if that will ever come
>> ;) another board with different setting will get in trouble here and
>> need to find another solution. Maybe more sophisticated by doing another
>> equation.
>>
>> I think a straight forward solution here would be to add another special
>> define in the board config, write the magic number down there and maybe
>> describe what the number stands for. You can then just use the
>> previously defined value in start.S.
>> You may have a look for at91 lowlevel_init, there it is done that way.
> 
> I shall. In fact, I may already have this problem as I need to verify the 
> timing on the flash memory (I have the demo board, not a cm4xxx), and the 
> reason I work on this is that we may design another board soon.
> 
> I'm a little hesitant about how to fit these changes together, though; for now 
> I have three barely separated patches, but overhauling all those magic numbers 
> changes them all yet again. These three patches are all necessary to make my 
> u-boot work in the first place, while fixing the magic values is a code cleanup 
> change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which 
> currently contains a few meaningless constants like KS8695_SDRAM_START and a 
> consistent misspelling of definitions. 
> 
> Could I save the value decoding and corresponding configuration changes for a 
> fourth patch?

I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm
related I guess Albert is the one in question.
I think these three patches are all fixes to get a board already in
mainline working. So I think we should try to get these in -rc1.
Albert, can you please comment?

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-06 10:25     ` Andreas Bießmann
@ 2012-07-09 21:58       ` Albert ARIBAUD
  2012-07-10 12:19         ` Greg Ungerer
  2012-07-12  9:30         ` Andreas Bießmann
  0 siblings, 2 replies; 10+ messages in thread
From: Albert ARIBAUD @ 2012-07-09 21:58 UTC (permalink / raw)
  To: u-boot

Hi Andreas,

On Fri, 06 Jul 2012 12:25:00 +0200, "Andreas Bie?mann" <andreas.devel@googlemail.com> wrote:
> Dear Yann Vernier,
> 
> On 06.07.2012 11:14, Yann Vernier wrote:
> > On Friday 06 July 2012 10:43:40 you wrote:
> >>>>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
> >>>>>      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
> >>>>
> >>>> ugh ... magic. Will it work for every possible setting?
> >>>> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
> >>>> related to register content in any way?
> >>>
> >>> It's not a guarded secret, although it is far from obvious where to find
> >>> it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's -
> >>> HW Design Kit. Within that zip file,
> >>> KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description
> >>> v1.1.pdf
> >>
> >> With this information I understand your equation. I think you should not
> >> do it that way. You may solve the current state (all devices have 8
> >> column, 4 bank and 32 bit). But one adding (well, if that will ever come
> >> ;) another board with different setting will get in trouble here and
> >> need to find another solution. Maybe more sophisticated by doing another
> >> equation.
> >>
> >> I think a straight forward solution here would be to add another special
> >> define in the board config, write the magic number down there and maybe
> >> describe what the number stands for. You can then just use the
> >> previously defined value in start.S.
> >> You may have a look for at91 lowlevel_init, there it is done that way.
> > 
> > I shall. In fact, I may already have this problem as I need to verify the 
> > timing on the flash memory (I have the demo board, not a cm4xxx), and the 
> > reason I work on this is that we may design another board soon.
> > 
> > I'm a little hesitant about how to fit these changes together, though; for now 
> > I have three barely separated patches, but overhauling all those magic numbers 
> > changes them all yet again. These three patches are all necessary to make my 
> > u-boot work in the first place, while fixing the magic values is a code cleanup 
> > change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which 
> > currently contains a few meaningless constants like KS8695_SDRAM_START and a 
> > consistent misspelling of definitions. 
> > 
> > Could I save the value decoding and corresponding configuration changes for a 
> > fourth patch?
> 
> I'm fine with this suggestion.
> So the next question is who would pull it in mainline? Since this is arm
> related I guess Albert is the one in question.

Ah, board-related patches. :)

> I think these three patches are all fixes to get a board already in
> mainline working. So I think we should try to get these in -rc1.
> Albert, can you please comment?

If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is Greg Ungerer,
Cc:) green-lights it, then I'm ok for pulling it in.

> Best regards
> 
> Andreas Bie?mann

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-09 21:58       ` Albert ARIBAUD
@ 2012-07-10 12:19         ` Greg Ungerer
  2012-07-12  9:32           ` Andreas Bießmann
  2012-07-12  9:30         ` Andreas Bießmann
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Ungerer @ 2012-07-10 12:19 UTC (permalink / raw)
  To: u-boot

On 07/10/2012 07:58 AM, Albert ARIBAUD wrote:
> Hi Andreas,
>
> On Fri, 06 Jul 2012 12:25:00 +0200, "Andreas Bie?mann" <andreas.devel@googlemail.com> wrote:
>> Dear Yann Vernier,
>>
>> On 06.07.2012 11:14, Yann Vernier wrote:
>>> On Friday 06 July 2012 10:43:40 you wrote:
>>>>>>> =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
>>>>>>>       (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
>>>>>>
>>>>>> ugh ... magic. Will it work for every possible setting?
>>>>>> How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
>>>>>> related to register content in any way?
>>>>>
>>>>> It's not a guarded secret, although it is far from obvious where to find
>>>>> it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's -
>>>>> HW Design Kit. Within that zip file,
>>>>> KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description
>>>>> v1.1.pdf
>>>>
>>>> With this information I understand your equation. I think you should not
>>>> do it that way. You may solve the current state (all devices have 8
>>>> column, 4 bank and 32 bit). But one adding (well, if that will ever come
>>>> ;) another board with different setting will get in trouble here and
>>>> need to find another solution. Maybe more sophisticated by doing another
>>>> equation.
>>>>
>>>> I think a straight forward solution here would be to add another special
>>>> define in the board config, write the magic number down there and maybe
>>>> describe what the number stands for. You can then just use the
>>>> previously defined value in start.S.
>>>> You may have a look for at91 lowlevel_init, there it is done that way.
>>>
>>> I shall. In fact, I may already have this problem as I need to verify the
>>> timing on the flash memory (I have the demo board, not a cm4xxx), and the
>>> reason I work on this is that we may design another board soon.
>>>
>>> I'm a little hesitant about how to fit these changes together, though; for now
>>> I have three barely separated patches, but overhauling all those magic numbers
>>> changes them all yet again. These three patches are all necessary to make my
>>> u-boot work in the first place, while fixing the magic values is a code cleanup
>>> change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which
>>> currently contains a few meaningless constants like KS8695_SDRAM_START and a
>>> consistent misspelling of definitions.
>>>
>>> Could I save the value decoding and corresponding configuration changes for a
>>> fourth patch?
>>
>> I'm fine with this suggestion.
>> So the next question is who would pull it in mainline? Since this is arm
>> related I guess Albert is the one in question.
>
> Ah, board-related patches. :)
>
>> I think these three patches are all fixes to get a board already in
>> mainline working.

Fixes or improvements?


> So I think we should try to get these in -rc1.
>> Albert, can you please comment?
>
> If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is Greg Ungerer,
> Cc:) green-lights it, then I'm ok for pulling it in.

I have no problem with them. I have only done a visual inspection of the
revised patches in the mail archives, and not tested them on the real
hardware.

I would suggest defining the proper meanings of the remaining bits in
the SDRAM setup registers that are still magic numbers, but that can be
a future change.

Acked-by: Greg Ungerer greg.ungerer at opengear.com

Regards
Greg

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-09 21:58       ` Albert ARIBAUD
  2012-07-10 12:19         ` Greg Ungerer
@ 2012-07-12  9:30         ` Andreas Bießmann
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Bießmann @ 2012-07-12  9:30 UTC (permalink / raw)
  To: u-boot

Dear Albert Aribaud,

On 09.07.2012 23:58, Albert ARIBAUD wrote:
> Hi Andreas,
> 
> On Fri, 06 Jul 2012 12:25:00 +0200, "Andreas Bie?mann" <andreas.devel@googlemail.com> wrote:
>> Dear Yann Vernier,
>>
>> On 06.07.2012 11:14, Yann Vernier wrote:
>>> On Friday 06 July 2012 10:43:40 you wrote:

<snip>

>>> Could I save the value decoding and corresponding configuration changes for a 
>>> fourth patch?
>>
>> I'm fine with this suggestion.
>> So the next question is who would pull it in mainline? Since this is arm
>> related I guess Albert is the one in question.
> 
> Ah, board-related patches. :)
> 
>> I think these three patches are all fixes to get a board already in
>> mainline working. So I think we should try to get these in -rc1.
>> Albert, can you please comment?
> 
> If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is Greg Ungerer,
> Cc:) green-lights it, then I'm ok for pulling it in.

Ouch, I should have added the board maintainer in cc earlier ... still a
lot to learn for me.

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-10 12:19         ` Greg Ungerer
@ 2012-07-12  9:32           ` Andreas Bießmann
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Bießmann @ 2012-07-12  9:32 UTC (permalink / raw)
  To: u-boot

On 10.07.2012 14:19, Greg Ungerer wrote:
> On 07/10/2012 07:58 AM, Albert ARIBAUD wrote:
>> Hi Andreas,
>>
>> On Fri, 06 Jul 2012 12:25:00 +0200, "Andreas Bie?mann"
>> <andreas.devel@googlemail.com> wrote:
>>> Dear Yann Vernier,
>>>
>>> On 06.07.2012 11:14, Yann Vernier wrote:
>>>> On Friday 06 July 2012 10:43:40 you wrote:

<snip>

>>>>
>>>> Could I save the value decoding and corresponding configuration
>>>> changes for a
>>>> fourth patch?
>>>
>>> I'm fine with this suggestion.
>>> So the next question is who would pull it in mainline? Since this is arm
>>> related I guess Albert is the one in question.
>>
>> Ah, board-related patches. :)
>>
>>> I think these three patches are all fixes to get a board already in
>>> mainline working.
> 
> Fixes or improvements?

I think these are fixes cause the board seems to not working without
these changes.

>> So I think we should try to get these in -rc1.
>>> Albert, can you please comment?
>>
>> If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is
>> Greg Ungerer,
>> Cc:) green-lights it, then I'm ok for pulling it in.
> 
> I have no problem with them. I have only done a visual inspection of the
> revised patches in the mail archives, and not tested them on the real
> hardware.
> 
> I would suggest defining the proper meanings of the remaining bits in
> the SDRAM setup registers that are still magic numbers, but that can be
> a future change.

Definitely.

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-05 12:11 ` [U-Boot] [PATCH v2] arm: cm4008, cm41xx: " Yann Vernier
@ 2012-07-05 14:48   ` Andreas Bießmann
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Bießmann @ 2012-07-05 14:48 UTC (permalink / raw)
  To: u-boot

On 05.07.2012 14:11, Yann Vernier wrote:
> Leave dram_init_banksize to set up the bank info data.
> ram_size was previously uninitialized. Also, generalize
> lowlevel assembly to support more RAM options.
> 
> Signed-off-by: Yann Vernier <yann.vernier@orsoc.se>
> ---
> Changes for v2:
>    - Update to use CONFIG_SYS_SDRAM_ constants
>    - Update cm41xx also
>    - Map SDRAM to match configuration
> ---
>  arch/arm/cpu/arm920t/ks8695/lowlevel_init.S |    8 +++-----
>  board/cm4008/cm4008.c                       |    5 +++--
>  board/cm41xx/cm41xx.c                       |    5 +++--
>  include/configs/cm4008.h                    |    5 ++---
>  include/configs/cm41xx.h                    |    5 ++---
>  5 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> index e9f1227..df13de6 100644
> --- a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> +++ b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
> @@ -131,11 +131,9 @@ highflash:
>  	 * before relocating, we have to setup RAM timing
>  	 */
>  	ldr	r1, =(KS8695_IO_BASE+KS8695_SDRAM_CTRL0)
> -#if (PHYS_SDRAM_1_SIZE == 0x02000000)
> -	ldr	r2, =0x7fc0000e		/* 32MB */
> -#else
> -	ldr	r2, =0x3fc0000e		/* 16MB */
> -#endif
> +	/* 8 column address bits, 4 banks, 32 bits data width */
> +	ldr	r2, =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \
> +		      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))

ugh ... magic. Will it work for every possible setting?
How about one setting CONFIG_SYS_SDRAM_BASE != 0x0? I the base address
related to register content in any way?

Would be great to have a little documentation here why it is shifted
that way, unfortunately I can not find a KS8695 spec in the net. Don't
get me wrong it is ok to do so. I think it is way better than hard
coding the values like before but would be great to understand why it is
done that way.

Best regards

Andreas Bie?mann

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init
  2012-07-05 10:02 [U-Boot] [PATCH 1/3] cm4008: " Andreas Bießmann
@ 2012-07-05 12:11 ` Yann Vernier
  2012-07-05 14:48   ` Andreas Bießmann
  0 siblings, 1 reply; 10+ messages in thread
From: Yann Vernier @ 2012-07-05 12:11 UTC (permalink / raw)
  To: u-boot

Leave dram_init_banksize to set up the bank info data.
ram_size was previously uninitialized. Also, generalize
lowlevel assembly to support more RAM options.

Signed-off-by: Yann Vernier <yann.vernier@orsoc.se>
---
Changes for v2:
   - Update to use CONFIG_SYS_SDRAM_ constants
   - Update cm41xx also
   - Map SDRAM to match configuration
---
 arch/arm/cpu/arm920t/ks8695/lowlevel_init.S |    8 +++-----
 board/cm4008/cm4008.c                       |    5 +++--
 board/cm41xx/cm41xx.c                       |    5 +++--
 include/configs/cm4008.h                    |    5 ++---
 include/configs/cm41xx.h                    |    5 ++---
 5 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
index e9f1227..df13de6 100644
--- a/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
+++ b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.S
@@ -131,11 +131,9 @@ highflash:
 	 * before relocating, we have to setup RAM timing
 	 */
 	ldr	r1, =(KS8695_IO_BASE+KS8695_SDRAM_CTRL0)
-#if (PHYS_SDRAM_1_SIZE == 0x02000000)
-	ldr	r2, =0x7fc0000e		/* 32MB */
-#else
-	ldr	r2, =0x3fc0000e		/* 16MB */
-#endif
+	/* 8 column address bits, 4 banks, 32 bits data width */
+	ldr	r2, =((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \
+		      (CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))
 	str	r2, [r1]		/* configure sdram bank0 setup */
 	ldr	r1, =(KS8695_IO_BASE+KS8695_SDRAM_CTRL1)
 	mov	r2, #0
diff --git a/board/cm4008/cm4008.c b/board/cm4008/cm4008.c
index ed493a8..6c0da9a 100644
--- a/board/cm4008/cm4008.c
+++ b/board/cm4008/cm4008.c
@@ -97,8 +97,9 @@ int board_init (void)
 
 int dram_init (void)
 {
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size  = PHYS_SDRAM_1_SIZE;
+	/* dram_init must store complete ramsize in gd->ram_size */
+	gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
+			CONFIG_SYS_SDRAM_SIZE);
 
 	return (0);
 }
diff --git a/board/cm41xx/cm41xx.c b/board/cm41xx/cm41xx.c
index d9dff4e..6aef617 100644
--- a/board/cm41xx/cm41xx.c
+++ b/board/cm41xx/cm41xx.c
@@ -97,8 +97,9 @@ int board_init (void)
 
 int dram_init (void)
 {
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size  = PHYS_SDRAM_1_SIZE;
+	/* dram_init must store complete ramsize in gd->ram_size */
+	gd->ram_size = get_ram_size((long *)CONFIG_SYS_SDRAM_BASE,
+			CONFIG_SYS_SDRAM_SIZE);
 
 	return (0);
 }
diff --git a/include/configs/cm4008.h b/include/configs/cm4008.h
index 408e918..58b0f4b 100644
--- a/include/configs/cm4008.h
+++ b/include/configs/cm4008.h
@@ -110,9 +110,8 @@
  * Physical Memory Map
  */
 #define CONFIG_NR_DRAM_BANKS	1	   /* we have 1 bank of DRAM */
-#define PHYS_SDRAM_1		0x00000000 /* SDRAM Bank #1 */
-#define PHYS_SDRAM_1_SIZE	0x01000000 /* 16 MB */
-#define CONFIG_SYS_SDRAM_BASE	PHYS_SDRAM_1
+#define CONFIG_SYS_SDRAM_BASE	0x00000000 /* SDRAM Bank #1 */
+#define CONFIG_SYS_SDRAM_SIZE	0x01000000 /* 16 MB */
 
 #define CONFIG_SYS_INIT_SP_ADDR	0x00020000 /* lowest 128k of RAM */
 
diff --git a/include/configs/cm41xx.h b/include/configs/cm41xx.h
index d85a600..d29040c 100644
--- a/include/configs/cm41xx.h
+++ b/include/configs/cm41xx.h
@@ -110,9 +110,8 @@
  * Physical Memory Map
  */
 #define CONFIG_NR_DRAM_BANKS	1	   /* we have 1 bank of DRAM */
-#define PHYS_SDRAM_1		0x00000000 /* SDRAM Bank #1 */
-#define PHYS_SDRAM_1_SIZE	0x02000000 /* 32 MB */
-#define CONFIG_SYS_SDRAM_BASE	PHYS_SDRAM_1
+#define CONFIG_SYS_SDRAM_BASE	0x00000000 /* SDRAM Bank #1 */
+#define CONFIG_SYS_SDRAM_SIZE	0x02000000 /* 32 MB */
 
 #define CONFIG_SYS_INIT_SP_ADDR	0x00020000 /* lowest 128k of RAM */
 
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-07-12  9:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-06  8:15 [U-Boot] [PATCH v2] arm: cm4008, cm41xx: set gd->ram_size in dram_init Yann Vernier
2012-07-06  8:43 ` Andreas Bießmann
2012-07-06  9:14   ` Yann Vernier
2012-07-06 10:25     ` Andreas Bießmann
2012-07-09 21:58       ` Albert ARIBAUD
2012-07-10 12:19         ` Greg Ungerer
2012-07-12  9:32           ` Andreas Bießmann
2012-07-12  9:30         ` Andreas Bießmann
  -- strict thread matches above, loose matches on Subject: below --
2012-07-05 10:02 [U-Boot] [PATCH 1/3] cm4008: " Andreas Bießmann
2012-07-05 12:11 ` [U-Boot] [PATCH v2] arm: cm4008, cm41xx: " Yann Vernier
2012-07-05 14:48   ` Andreas Bießmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.