From: Dirk Behme <dirk.behme@googlemail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code
Date: Sun, 31 May 2009 17:56:45 +0200 [thread overview]
Message-ID: <4A22A8BD.4020302@googlemail.com> (raw)
In-Reply-To: <20090531115103.GA20818@game.jcrosoft.org>
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
next prev parent reply other threads:[~2009-05-31 15:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-30 7:30 [U-Boot] [PATCH] ARM Cortex A8: Move OMAP3 specific reset handler to OMAP3 code Dirk Behme
2009-05-31 11:51 ` Jean-Christophe PLAGNIOL-VILLARD
2009-05-31 15:56 ` Dirk Behme [this message]
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
-- strict thread matches above, loose matches on Subject: below --
2009-07-17 1:48 Minkyu Kang
2009-07-17 13:36 ` Dirk Behme
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=4A22A8BD.4020302@googlemail.com \
--to=dirk.behme@googlemail.com \
--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