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

[Dirk Behme]

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