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,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
