public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
@ 2009-05-30  7:30 Dirk Behme
  2009-05-31 11:51 ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 9+ messages in thread
From: Dirk Behme @ 2009-05-30  7:30 UTC (permalink / raw)
  To: u-boot

Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
code to OMAP3 SoC specific file.

CC: "Kim, Heung Jun" <riverful@gmail.com>
Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>

---

This patches fixes the second issue found by riverful in

http://lists.denx.de/pipermail/u-boot/2009-May/053433.html

The first issue is fixed by

http://lists.denx.de/pipermail/u-boot/2009-May/053444.html

 cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
 cpu/arm_cortexa8/start.S               |   14 --------------
 2 files changed, 12 insertions(+), 14 deletions(-)

Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
===================================================================
--- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
+++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
@@ -181,6 +181,18 @@ lowlevel_init:
 	/* back to arch calling code */
 	mov	pc, lr
 
+.global reset_cpu
+reset_cpu:
+	ldr	r1, rstctl			@ get addr for global reset
+						@ reg
+	mov	r3, #0x2			@ full reset pll + mpu
+	str	r3, [r1]			@ force reset
+	mov	r0, r0
+_loop_forever:
+	b	_loop_forever
+rstctl:
+	.word	PRM_RSTCTRL
+
 	/* the literal pools origin */
 	.ltorg
 
Index: u-boot-arm/cpu/arm_cortexa8/start.S
===================================================================
--- u-boot-arm.orig/cpu/arm_cortexa8/start.S
+++ u-boot-arm/cpu/arm_cortexa8/start.S
@@ -500,17 +500,3 @@ finished_inval:
 						@ but we compile with armv5
 
 	ldmfd	r13!, {r0 - r5, r7, r9 - r12, pc}
-
-
-	.align	5
-.global reset_cpu
-reset_cpu:
-	ldr	r1, rstctl			@ get addr for global reset
-						@ reg
-	mov	r3, #0x2			@ full reset pll + mpu
-	str	r3, [r1]			@ force reset
-	mov	r0, r0
-_loop_forever:
-	b	_loop_forever
-rstctl:
-	.word	PRM_RSTCTRL

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-05-30  7:30 Dirk Behme
@ 2009-05-31 11:51 ` Jean-Christophe PLAGNIOL-VILLARD
  2009-05-31 15:56   ` Dirk Behme
  0 siblings, 1 reply; 9+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-31 11:51 UTC (permalink / raw)
  To: u-boot

On 09:30 Sat 30 May     , Dirk Behme wrote:
> Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
> code to OMAP3 SoC specific file.
> 
> CC: "Kim, Heung Jun" <riverful@gmail.com>
> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
> 
> ---
> 
> This patches fixes the second issue found by riverful in
> 
> http://lists.denx.de/pipermail/u-boot/2009-May/053433.html
> 
> The first issue is fixed by
> 
> http://lists.denx.de/pipermail/u-boot/2009-May/053444.html
> 
>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>  cpu/arm_cortexa8/start.S               |   14 --------------
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
> ===================================================================
> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
> @@ -181,6 +181,18 @@ lowlevel_init:
>  	/* back to arch calling code */
>  	mov	pc, lr
>  
> +.global reset_cpu
> +reset_cpu:
> +	ldr	r1, rstctl			@ get addr for global reset
> +						@ reg
> +	mov	r3, #0x2			@ full reset pll + mpu
> +	str	r3, [r1]			@ force reset
> +	mov	r0, r0
> +_loop_forever:
> +	b	_loop_forever
> +rstctl:
> +	.word	PRM_RSTCTRL
> +
please move this to reset.S other wise fine

Best Regards,
J.

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-05-31 11:51 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-05-31 15:56   ` Dirk Behme
  2009-06-01  7:14     ` Kim, Heung Jun
  2009-06-01 15:09     ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 2 replies; 9+ messages in thread
From: Dirk Behme @ 2009-05-31 15:56 UTC (permalink / raw)
  To: u-boot

Dear Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:30 Sat 30 May     , Dirk Behme wrote:
>> Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
>> code to OMAP3 SoC specific file.
>>
>> CC: "Kim, Heung Jun" <riverful@gmail.com>
>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>>
>> ---
>>
>> This patches fixes the second issue found by riverful in
>>
>> http://lists.denx.de/pipermail/u-boot/2009-May/053433.html
>>
>> The first issue is fixed by
>>
>> http://lists.denx.de/pipermail/u-boot/2009-May/053444.html
>>
>>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>>  cpu/arm_cortexa8/start.S               |   14 --------------
>>  2 files changed, 12 insertions(+), 14 deletions(-)
>>
>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>> ===================================================================
>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>> @@ -181,6 +181,18 @@ lowlevel_init:
>>  	/* back to arch calling code */
>>  	mov	pc, lr
>>  
>> +.global reset_cpu
>> +reset_cpu:
>> +	ldr	r1, rstctl			@ get addr for global reset
>> +						@ reg
>> +	mov	r3, #0x2			@ full reset pll + mpu
>> +	str	r3, [r1]			@ force reset
>> +	mov	r0, r0
>> +_loop_forever:
>> +	b	_loop_forever
>> +rstctl:
>> +	.word	PRM_RSTCTRL
>> +
> please move this to reset.S other wise fine

Most probably your idea is that each file should only contain 
functionality which fits 100% (120%?) what the file name implies (?). 
While from general point of view this is correct, it makes no sense to 
create new files again and again just to follow this rule. We already 
created a cache.c on your request, now you request a new file reset.S 
for ~5 assembly lines. This new file would contain more comments (e.g. 
GPL header) than useful code.

So while in general case having file names reflecting more or less the 
functionality in these files, in this case it doesn't make sense. It 
doesn't make sense to pollute the source tree with a new file 
containing ~5 assembly lines just to make your rules apply. For such 
small code, re-using existing file is the better way to go. So NACK in 
this case.

Best regards

Dirk

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-05-31 15:56   ` Dirk Behme
@ 2009-06-01  7:14     ` Kim, Heung Jun
  2009-06-01  7:26       ` Wolfgang Denk
  2009-06-01 15:09     ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 1 reply; 9+ messages in thread
From: Kim, Heung Jun @ 2009-06-01  7:14 UTC (permalink / raw)
  To: u-boot

Dear Jean & Dirk,

Jean's opinion seems that file naming & func naming must match
for soruce maintaining, & definitely I agree with that.
Moreover it's a very first time to implement the new arm_cortexa8
code for u-boot source. So, the matching works is needed.

On the other hand, Dirk's opinion seems that it dosen't make sense
in this case, if there is many files when the developer make the code.
I think that too, especially when the code is a little simple like the
boot code.
The u-boot's architecture & SoC directory tree is already brilliant,
so it's alright we just look at this directories what arch & SoC ever
we develop.

So, my opinions is re-using the files seems to be right in this case.
the Dirk's patch code is not difficult to understand & use.
After the code polluted cause of not-matching the file & func naming,
I believe that the developers seem be able to fix this issue easily,
like my case.

BTW, when am I able to re-update my new patch??
I wanna do that :)

Best Regards,
riverful

2009/6/1 Dirk Behme <dirk.behme@googlemail.com>:
> Dear Jean-Christophe,
>
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 09:30 Sat 30 May ? ? , Dirk Behme wrote:
>>> Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
>>> code to OMAP3 SoC specific file.
>>>
>>> CC: "Kim, Heung Jun" <riverful@gmail.com>
>>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>>>
>>> ---
>>>
>>> This patches fixes the second issue found by riverful in
>>>
>>> http://lists.denx.de/pipermail/u-boot/2009-May/053433.html
>>>
>>> The first issue is fixed by
>>>
>>> http://lists.denx.de/pipermail/u-boot/2009-May/053444.html
>>>
>>> ?cpu/arm_cortexa8/omap3/lowlevel_init.S | ? 12 ++++++++++++
>>> ?cpu/arm_cortexa8/start.S ? ? ? ? ? ? ? | ? 14 --------------
>>> ?2 files changed, 12 insertions(+), 14 deletions(-)
>>>
>>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> ===================================================================
>>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> @@ -181,6 +181,18 @@ lowlevel_init:
>>> ? ? ?/* back to arch calling code */
>>> ? ? ?mov ? ? pc, lr
>>>
>>> +.global reset_cpu
>>> +reset_cpu:
>>> + ? ?ldr ? ? r1, rstctl ? ? ? ? ? ? ? ? ? ? ?@ get addr for global reset
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?@ reg
>>> + ? ?mov ? ? r3, #0x2 ? ? ? ? ? ? ? ? ? ? ? ?@ full reset pll + mpu
>>> + ? ?str ? ? r3, [r1] ? ? ? ? ? ? ? ? ? ? ? ?@ force reset
>>> + ? ?mov ? ? r0, r0
>>> +_loop_forever:
>>> + ? ?b ? ? ? _loop_forever
>>> +rstctl:
>>> + ? ?.word ? PRM_RSTCTRL
>>> +
>> please move this to reset.S other wise fine
>
> Most probably your idea is that each file should only contain
> functionality which fits 100% (120%?) what the file name implies (?).
> While from general point of view this is correct, it makes no sense to
> create new files again and again just to follow this rule. We already
> created a cache.c on your request, now you request a new file reset.S
> for ~5 assembly lines. This new file would contain more comments (e.g.
> GPL header) than useful code.
>
> So while in general case having file names reflecting more or less the
> functionality in these files, in this case it doesn't make sense. It
> doesn't make sense to pollute the source tree with a new file
> containing ~5 assembly lines just to make your rules apply. For such
> small code, re-using existing file is the better way to go. So NACK in
> this case.
>
> Best regards
>
> Dirk
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-06-01  7:14     ` Kim, Heung Jun
@ 2009-06-01  7:26       ` Wolfgang Denk
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfgang Denk @ 2009-06-01  7:26 UTC (permalink / raw)
  To: u-boot

Dear Riverful,

In message <b64afca20906010014r321bd3fas3539d97f6667e70d@mail.gmail.com> you wrote:
> 
> Jean's opinion seems that file naming & func naming must match
> for soruce maintaining, & definitely I agree with that.

I do not agree.

Yes, they _should_ match. That means we should try to acchieve this
whenever it makes sense.

But there is no strict must that we need to create a new source file
for each and every function. Maintaining tons of tiny files does not
make much sense either.

In this case, I agree with Dirk that splitting of a new source file
for a small (10 lines or so including comments) function is overkill.

Jean-Christiphe sent a note about this code, and Dirk probvided a
reasonable explanation why the code was written as is.

I think that should be enough in this case. From my point of view, the
code can go in as is.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The algorithm to do that is extremely nasty. You might want  to  mug
someone with it."                   - M. Devine, Computer Science 340

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-05-31 15:56   ` Dirk Behme
  2009-06-01  7:14     ` Kim, Heung Jun
@ 2009-06-01 15:09     ` Jean-Christophe PLAGNIOL-VILLARD
  2009-06-02 15:12       ` Dirk Behme
  1 sibling, 1 reply; 9+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-01 15:09 UTC (permalink / raw)
  To: u-boot

On 17:56 Sun 31 May     , Dirk Behme wrote:
> Dear Jean-Christophe,
>
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 09:30 Sat 30 May     , Dirk Behme wrote:
>>> Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
>>> code to OMAP3 SoC specific file.
>>>
>>> CC: "Kim, Heung Jun" <riverful@gmail.com>
>>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>>>
>>> ---
>>>
>>> This patches fixes the second issue found by riverful in
>>>
>>> http://lists.denx.de/pipermail/u-boot/2009-May/053433.html
>>>
>>> The first issue is fixed by
>>>
>>> http://lists.denx.de/pipermail/u-boot/2009-May/053444.html
>>>
>>>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>>>  cpu/arm_cortexa8/start.S               |   14 --------------
>>>  2 files changed, 12 insertions(+), 14 deletions(-)
>>>
>>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> ===================================================================
>>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>> @@ -181,6 +181,18 @@ lowlevel_init:
>>>  	/* back to arch calling code */
>>>  	mov	pc, lr
>>>  +.global reset_cpu
>>> +reset_cpu:
>>> +	ldr	r1, rstctl			@ get addr for global reset
>>> +						@ reg
>>> +	mov	r3, #0x2			@ full reset pll + mpu
>>> +	str	r3, [r1]			@ force reset
>>> +	mov	r0, r0
>>> +_loop_forever:
>>> +	b	_loop_forever
>>> +rstctl:
>>> +	.word	PRM_RSTCTRL
>>> +
>> please move this to reset.S other wise fine
>
> Most probably your idea is that each file should only contain  
> functionality which fits 100% (120%?) what the file name implies (?).  
> While from general point of view this is correct, it makes no sense to  
> create new files again and again just to follow this rule. We already  
> created a cache.c on your request, now you request a new file reset.S  
> for ~5 assembly lines. This new file would contain more comments (e.g.  
> GPL header) than useful code.
the idea is different here
I want to have only code in lowlevel_init.S that can be disable by
CONFIG_SKIP_LOWLEVEL_INIT and do it via Makefile

Best Regards,
J.

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-06-01 15:09     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2009-06-02 15:12       ` Dirk Behme
  0 siblings, 0 replies; 9+ messages in thread
From: Dirk Behme @ 2009-06-02 15:12 UTC (permalink / raw)
  To: u-boot

Dear Jean-Christophe,

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:56 Sun 31 May     , Dirk Behme wrote:
>> Dear Jean-Christophe,
>>
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 09:30 Sat 30 May     , Dirk Behme wrote:
>>>> Reset is SoC specific and not ARM Cortex A8 generic. Move it from generic
>>>> code to OMAP3 SoC specific file.
>>>>
>>>> CC: "Kim, Heung Jun" <riverful@gmail.com>
>>>> Signed-off-by: Dirk Behme <dirk.behme@googlemail.com>
>>>>
>>>> ---
>>>>
>>>> This patches fixes the second issue found by riverful in
>>>>
>>>> http://lists.denx.de/pipermail/u-boot/2009-May/053433.html
>>>>
>>>> The first issue is fixed by
>>>>
>>>> http://lists.denx.de/pipermail/u-boot/2009-May/053444.html
>>>>
>>>>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>>>>  cpu/arm_cortexa8/start.S               |   14 --------------
>>>>  2 files changed, 12 insertions(+), 14 deletions(-)
>>>>
>>>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>> ===================================================================
>>>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>> @@ -181,6 +181,18 @@ lowlevel_init:
>>>>  	/* back to arch calling code */
>>>>  	mov	pc, lr
>>>>  +.global reset_cpu
>>>> +reset_cpu:
>>>> +	ldr	r1, rstctl			@ get addr for global reset
>>>> +						@ reg
>>>> +	mov	r3, #0x2			@ full reset pll + mpu
>>>> +	str	r3, [r1]			@ force reset
>>>> +	mov	r0, r0
>>>> +_loop_forever:
>>>> +	b	_loop_forever
>>>> +rstctl:
>>>> +	.word	PRM_RSTCTRL
>>>> +
>>> please move this to reset.S other wise fine
>> Most probably your idea is that each file should only contain  
>> functionality which fits 100% (120%?) what the file name implies (?).  
>> While from general point of view this is correct, it makes no sense to  
>> create new files again and again just to follow this rule. We already  
>> created a cache.c on your request, now you request a new file reset.S  
>> for ~5 assembly lines. This new file would contain more comments (e.g.  
>> GPL header) than useful code.
> the idea is different here
> I want to have only code in lowlevel_init.S that can be disable by
> CONFIG_SKIP_LOWLEVEL_INIT and do it via Makefile

Looking at recent OMAP3 lowlevel_init.S most probably some other stuff 
has to be moved to make this work, too. So for the moment, the 
cleanest way is to move above reset_cpu to low_levelinit.S. And then 
later, after thorough investigation and testing, move the stuff needed 
for your idea to an appropriate place. This move will be consistent 
then and will avoid polluting source tree with unnecessary files until 
then.

So let's do it in two steps:

a) Now, move reset_cpu to lowlevel_init.S so that Riverful can go on 
with his work

b) Later, move everything necessary in one consistent patch set while 
you implement your "CONFIG_SKIP_LOWLEVEL_INIT via Makefile" idea

Best regards

Dirk

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
@ 2009-07-17  1:48 Minkyu Kang
  2009-07-17 13:36 ` Dirk Behme
  0 siblings, 1 reply; 9+ messages in thread
From: Minkyu Kang @ 2009-07-17  1:48 UTC (permalink / raw)
  To: u-boot

Dear Jean and Dirk,

>>>>>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>>>>>  cpu/arm_cortexa8/start.S               |   14 --------------
>>>>>  2 files changed, 12 insertions(+), 14 deletions(-)
>>>>>
>>>>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>> ===================================================================
>>>>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>> @@ -181,6 +181,18 @@ lowlevel_init:
>>>>>    /* back to arch calling code */
>>>>>    mov     pc, lr
>>>>>  +.global reset_cpu
>>>>> +reset_cpu:
>>>>> +  ldr     r1, rstctl                      @ get addr for global reset
>>>>> +                                          @ reg
>>>>> +  mov     r3, #0x2                        @ full reset pll + mpu
>>>>> +  str     r3, [r1]                        @ force reset
>>>>> +  mov     r0, r0
>>>>> +_loop_forever:
>>>>> +  b       _loop_forever
>>>>> +rstctl:
>>>>> +  .word   PRM_RSTCTRL
>>>>> +
>>>> please move this to reset.S other wise fine
>>> Most probably your idea is that each file should only contain
>>> functionality which fits 100% (120%?) what the file name implies (?).
>>> While from general point of view this is correct, it makes no sense to
>>> create new files again and again just to follow this rule. We already
>>> created a cache.c on your request, now you request a new file reset.S
>>> for ~5 assembly lines. This new file would contain more comments (e.g.
>>> GPL header) than useful code.
>> the idea is different here
>> I want to have only code in lowlevel_init.S that can be disable by
>> CONFIG_SKIP_LOWLEVEL_INIT and do it via Makefile
>
> Looking at recent OMAP3 lowlevel_init.S most probably some other stuff
> has to be moved to make this work, too. So for the moment, the
> cleanest way is to move above reset_cpu to low_levelinit.S. And then
> later, after thorough investigation and testing, move the stuff needed
> for your idea to an appropriate place. This move will be consistent
> then and will avoid polluting source tree with unnecessary files until
> then.
>
> So let's do it in two steps:
>
> a) Now, move reset_cpu to lowlevel_init.S so that Riverful can go on
> with his work
>
> b) Later, move everything necessary in one consistent patch set while
> you implement your "CONFIG_SKIP_LOWLEVEL_INIT via Makefile" idea
>

As you known riverful and me prepare the new SOC (s5pc100) patch.
so, we've been waiting for this issue to be resolved.
Please let me know how do you solve this problem.

I think... as Wolfgang said.. it would be better make new file.

I hope to be progressed this issue :)
thanks

-- 
from. prom.
www.promsoft.net

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

* [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
  2009-07-17  1:48 [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code Minkyu Kang
@ 2009-07-17 13:36 ` Dirk Behme
  0 siblings, 0 replies; 9+ messages in thread
From: Dirk Behme @ 2009-07-17 13:36 UTC (permalink / raw)
  To: u-boot

Minkyu Kang wrote:
> Dear Jean and Dirk,
> 
>>>>>>  cpu/arm_cortexa8/omap3/lowlevel_init.S |   12 ++++++++++++
>>>>>>  cpu/arm_cortexa8/start.S               |   14 --------------
>>>>>>  2 files changed, 12 insertions(+), 14 deletions(-)
>>>>>>
>>>>>> Index: u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>>> ===================================================================
>>>>>> --- u-boot-arm.orig/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>>> +++ u-boot-arm/cpu/arm_cortexa8/omap3/lowlevel_init.S
>>>>>> @@ -181,6 +181,18 @@ lowlevel_init:
>>>>>>    /* back to arch calling code */
>>>>>>    mov     pc, lr
>>>>>>  +.global reset_cpu
>>>>>> +reset_cpu:
>>>>>> +  ldr     r1, rstctl                      @ get addr for global reset
>>>>>> +                                          @ reg
>>>>>> +  mov     r3, #0x2                        @ full reset pll + mpu
>>>>>> +  str     r3, [r1]                        @ force reset
>>>>>> +  mov     r0, r0
>>>>>> +_loop_forever:
>>>>>> +  b       _loop_forever
>>>>>> +rstctl:
>>>>>> +  .word   PRM_RSTCTRL
>>>>>> +
>>>>> please move this to reset.S other wise fine
>>>> Most probably your idea is that each file should only contain
>>>> functionality which fits 100% (120%?) what the file name implies (?).
>>>> While from general point of view this is correct, it makes no sense to
>>>> create new files again and again just to follow this rule. We already
>>>> created a cache.c on your request, now you request a new file reset.S
>>>> for ~5 assembly lines. This new file would contain more comments (e.g.
>>>> GPL header) than useful code.
>>> the idea is different here
>>> I want to have only code in lowlevel_init.S that can be disable by
>>> CONFIG_SKIP_LOWLEVEL_INIT and do it via Makefile
>> Looking at recent OMAP3 lowlevel_init.S most probably some other stuff
>> has to be moved to make this work, too. So for the moment, the
>> cleanest way is to move above reset_cpu to low_levelinit.S. And then
>> later, after thorough investigation and testing, move the stuff needed
>> for your idea to an appropriate place. This move will be consistent
>> then and will avoid polluting source tree with unnecessary files until
>> then.
>>
>> So let's do it in two steps:
>>
>> a) Now, move reset_cpu to lowlevel_init.S so that Riverful can go on
>> with his work
>>
>> b) Later, move everything necessary in one consistent patch set while
>> you implement your "CONFIG_SKIP_LOWLEVEL_INIT via Makefile" idea
>>
> 
> As you known riverful and me prepare the new SOC (s5pc100) patch.
> so, we've been waiting for this issue to be resolved.
> Please let me know how do you solve this problem.
> 
> I think... as Wolfgang said.. it would be better make new file.

Do you like to send a patch for this?

> I hope to be progressed this issue :)

Me too :)

Best regards

Dirk

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

end of thread, other threads:[~2009-07-17 13:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-17  1:48 [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code Minkyu Kang
2009-07-17 13:36 ` Dirk Behme
  -- strict thread matches above, loose matches on Subject: below --
2009-05-30  7:30 Dirk Behme
2009-05-31 11:51 ` Jean-Christophe PLAGNIOL-VILLARD
2009-05-31 15:56   ` Dirk Behme
2009-06-01  7:14     ` Kim, Heung Jun
2009-06-01  7:26       ` Wolfgang Denk
2009-06-01 15:09     ` Jean-Christophe PLAGNIOL-VILLARD
2009-06-02 15:12       ` Dirk Behme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox