Hi Albert,
On Mon, 2015-11-16 at 15:15 +0100, Albert ARIBAUD wrote:
Hello Alexey,
On Mon, 16 Nov 2015 13:43:19 +0000, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Albert,
On Mon, 2015-11-16 at 14:34 +0100, Albert ARIBAUD wrote:
Hello Alexey,
On Mon, 16 Nov 2015 13:12:15 +0000, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Albert,
- /* Allocate and zero GD, update SP */
- mov %r0, %sp
- bl board_init_f_mem
- /* Get reserved area size, update SP and FP */
- bl board_init_f_get_reserve_size /* Update stack- and frame-pointers */
I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely: ---------->8---------- /* Update stack- and frame-pointers */ ---------->8----------
Not sure where you see two SP+FP 'update' comments here; probably you're referring to the 'setup' comment on line 53 and the 'update' one on line 59. If that is what you meant, I tink these comments are different and deserve staying both...
Ok, that's what I have after your patch application:
---------->8---------- /* Setup stack- and frame-pointers */ mov %sp, CONFIG_SYS_INIT_SP_ADDR mov %fp, %sp
/* Get reserved area size, update SP and FP */ bl board_init_f_get_reserve_size /* Update stack- and frame-pointers */ <-- that's already mentioned 2 lines above sub %sp, %sp, %r0 mov %fp, %sp ---------->8----------
My bad, I'd missed that one. I'll turn
/* Get reserved area size, update SP and FP */
Into
/* Get reserved area size */
... However, these comments also pretty much just paraphrase the code which follows them and thus serve little purpose; they could be reworded to show less of what is being done and more of why it is being done:
the "Update stack- and frame-pointer" comment could be turned into "Allocate reserved size on stack and adjust frame pointer accordingly", and
the "Setup stack- and frame-pointers" comment could be turned into "Establish C runtime stack and frame".
Opinions?
Totally agree, care to implement it?
That, and the removal of the repetition. v5 in approach.
-Alexey
Amicalement,
Thanks for doing that!
-Alexey