Hi again Prafulla,
Prafulla Wadaskar a écrit :
Break this in to basic orion5x Soc support patch And individual drivers patches like gpio, uart, etc..
Will do. What exact criteria should I use for splitting patches? Obviously some patches will never be compiled alone, e.g. basic Orion5x support won't be compiled until board support patches come in; also, having SoC and board support without uart support, if it even compiles, will be pretty useless as the resulting u-boot won't even have a console.
It should be patch series with properly addressed dependencies i.e. (1/4)basic orion soc support patch (2/4)orion gpio driver (3/4)orion UART support (4/4) Board support patch
All patches when applied in series should build u-boot for respective board. The spilt has to be done carefully because individual patch may be applied to different repositories, like nandf to u-boot-nand.git, basic orion support to u-boot-marvell.git, egiga driver patch will go in u-boot-net.git etc
Duly noted. Currently, I only add support for SoC, UART and NOT FLASH (not NAND) so only u-boot-marvell.git needs to be considered; I'll keep other repos in mind for upcoming patches like egiga and others.
+TEXT_BASE = 0x00600000
Is this valid for your board? BTW: how much DRAM you have
on this board?
Yes it works for the board--I'm using this value for testing. However, in very old and non-reuseable u-boot code, it was 0x00F00000. I'll switch to this in order to minimize the difference between these older u-boots and the current mainline one.
I will suggest to use lower part of available ram for this
Understood. Note the 0x00600000 value comes from kirkwood too, in case this matters.
if (dev == MV88F5181_DEV_ID) {dev_name = "MV88F5181";if (rev == MV88F5181_REV_B1) {rev_name = "B1";} else if (rev == MV88F5181L_REV_A1) {dev_name = "MV88F5181L";rev_name = "A1";} else if (rev == MV88F5181L_REV_A0) {dev_name = "MV88F5181L";rev_name = "A0";} else {sprintf(rev_str,"0x%02x", rev);}} else if (dev == MV88F5182_DEV_ID) {dev_name = "MV88F5182";if (rev == MV88F5182_REV_A2) {rev_name = "A2";} else {sprintf(rev_str,"0x%02x", rev);}} else if (dev == MV88F5281_DEV_ID) {dev_name = "MV88F5281";if (rev == MV88F5281_REV_D2) {rev_name = "D2";} else if (rev == MV88F5281_REV_D1) {rev_name = "D1";} else if (rev == MV88F5281_REV_D0) {rev_name = "D0";} else {sprintf(rev_str,"0x%02x", rev);}} else if (dev == MV88F6183_DEV_ID) {dev_name = "MV88F6183";if (rev == MV88F6183_REV_B0) {rev_name = "B0";} else {sprintf(rev_str,"0x%02x", rev);}} else {sprintf(dev_str,"0x%04x", rev);sprintf(rev_str,"0x%02x", rev);This is common line, pls take out of if-else
Can you be more specific? The outer 'if' sequence deals with device id
Okay I will take this back Whereas I found
sprintf(rev_str,"0x%02x", rev);At so many places, can you reduce it?
Hmm yes, I think I can start by setting dev_name/rev_name tu NULL, go through the ifs and set dev_name/rev_name only if appropriate, and then do the sprintf()s only if dev_name/rev_name are still NULL after the ifs.
Is this Marvell custom board ? If not, even you can choose to keep in in boards instead of
boards/Marvell/
No, it's not a Marvell custom board, it's a LaCie product board. I'll move the board to boards/ directly (or maybe "boards/lacie", to provide a home for other lacie boards? Any best pratice here to follow?) and change the prompt to "EDMiniV2".
That's good approach for this board, if you are planning to put support for more Lacie boards, then its better to have boards/lacie
I guess there's at least a second one to come, so I'll go for the boards/lacie/* approach.
Regards.. Prafulla . .
Thanks again!
Amicalement,