[U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code

[Kim, Heung Jun]

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 at 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 at gmail.com>
>>> Signed-off-by: Dirk Behme <dirk.behme at 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
>