The attached patch adds support for the Amstrad Delta (E3) Videophone. Currently the Amstrad primary bootloader provides the main hardware initialisation and then loads u-boot to handle user interaction and kernel loading.
I'd appreciate any comments you may have. Patch is against current git.
J.
On Tue, Jun 13, 2006 at 05:59:27PM +0100, Jonathan McDowell wrote:
diff --git a/Makefile b/Makefile index 440ab2c..659e9b1 100644 --- a/Makefile +++ b/Makefile @@ -1565,6 +1565,9 @@ netstar_config: unconfig omap1510inn_config : unconfig @./mkconfig $(@:_config=) arm arm925t omap1510inn
+amsdelta_config : unconfig
- @./mkconfig $(@:_config=) arm arm925t amsdelta
It would be nice to keep machines sorted alphabeticaly...
omap5912osk_config : unconfig @./mkconfig $(@:_config=) arm arm926ejs omap5912osk NULL omap
[snip]
diff --git a/cpu/arm925t/start.S b/cpu/arm925t/start.S index acd7742..c2c2adb 100644 --- a/cpu/arm925t/start.S +++ b/cpu/arm925t/start.S @@ -125,6 +125,7 @@ reset: orr r0,r0,#0xd3 msr cpsr,r0
+#ifndef CONFIG_AMSDELTA /* * Set up 925T mode */ @@ -159,6 +160,7 @@ poll1: ldrh r1, [r0] ands r1, r1, #0x01 beq poll1 +#endif
Why it this needed?
/* * we do sys-critical inits only at reboot,
Jonathan, you will need a bit more patience. It tooks several months to get patches accepted :-)
Best regards, ladis (the patient ;-))
On Fri, Sep 01, 2006 at 01:34:52PM +0200, Ladislav Michl wrote:
On Tue, Jun 13, 2006 at 05:59:27PM +0100, Jonathan McDowell wrote:
diff --git a/Makefile b/Makefile index 440ab2c..659e9b1 100644 --- a/Makefile +++ b/Makefile @@ -1565,6 +1565,9 @@ netstar_config: unconfig omap1510inn_config : unconfig @./mkconfig $(@:_config=) arm arm925t omap1510inn
+amsdelta_config : unconfig
- @./mkconfig $(@:_config=) arm arm925t amsdelta
It would be nice to keep machines sorted alphabeticaly...
Sure, that seems reasonable.
omap5912osk_config : unconfig @./mkconfig $(@:_config=) arm arm926ejs omap5912osk NULL omap
[snip]
diff --git a/cpu/arm925t/start.S b/cpu/arm925t/start.S index acd7742..c2c2adb 100644 --- a/cpu/arm925t/start.S +++ b/cpu/arm925t/start.S @@ -125,6 +125,7 @@ reset: orr r0,r0,#0xd3 msr cpsr,r0
+#ifndef CONFIG_AMSDELTA /* * Set up 925T mode */ @@ -159,6 +160,7 @@ poll1: ldrh r1, [r0] ands r1, r1, #0x01 beq poll1 +#endif
Why it this needed?
Without it the boot process is incredibly slow. This section was isolated as different between the 925 and 926 init with no obvious extra benefit and once removed the boot process speeds up significantly (loading the kernel from NAND is about 10 times faster, for example, though it can be seen even just in terms of interactive u-boot use).
I don't have any other 925 platform available to determine if this can be left out for them all, hence only disabling it for the Delta.
/* * we do sys-critical inits only at reboot,
Jonathan, you will need a bit more patience. It tooks several months to get patches accepted :-)
I can be patient about acceptance as long as there's some indication they've actually been seen and will get there eventually - I may have another board I want to port u-boot to, but there's no point if it proves impossible to push changes upstream.
J.
In message 20060901115834.GS26829@earth.li you wrote:
I can be patient about acceptance as long as there's some indication they've actually been seen and will get there eventually - I may have another board I want to port u-boot to, but there's no point if it proves impossible to push changes upstream.
Sorry, but reviewing and applying (or rejecting) a patch is usally one step. Most patches will not be checked thoroughly before they are "ripe" for merging.
Best regards,
Wolfgang Denk
On Fri, Sep 01, 2006 at 03:41:19PM +0200, Wolfgang Denk wrote:
In message 20060901115834.GS26829@earth.li you wrote:
I can be patient about acceptance as long as there's some indication they've actually been seen and will get there eventually - I may have another board I want to port u-boot to, but there's no point if it proves impossible to push changes upstream.
Sorry, but reviewing and applying (or rejecting) a patch is usally one step. Most patches will not be checked thoroughly before they are "ripe" for merging.
Ok. As long as I know I'll get a definite statement either way eventually I'll just learn to be more patient for u-boot; I've had too many experiences where mails just end up getting lost in the noise.
Thanks, J.
Dear Jonathan,
in message 20060613165927.GE14634@earth.li you wrote:
The attached patch adds support for the Amstrad Delta (E3) Videophone. Currently the Amstrad primary bootloader provides the main hardware initialisation and then loads u-boot to handle user interaction and kernel loading.
I'd appreciate any comments you may have. Patch is against current git.
I'm afraid I have to reject his patch.
The major reason is that I don't want to see any board specific code in files like cpu/arm925t/start.S ; maybe you have a good reason for your modifiations of such a file, but then you must explain it, and try to implement it in a board-independent way.
Also, there are a few formal resons for the reject: you missed to include a CHANGELOG entry, and your files violate the Coding Style requirements (trailing white space, C++ comments, etc.).
Please clean up and resubmit (and while doing this, please also adapt the makefiles to the new style to allow for building in another directory).
Best regards,
Wolfgang Denk
-
Jonathan McDowell -
Ladislav Michl -
Wolfgang Denk