Hello Prafulla, Prafulla Wadaskar wrote: >> -----Original Message----- >> From: Heiko Schocher [mailto:h...@denx.de] >> Sent: Monday, February 15, 2010 1:37 PM >> To: Prafulla Wadaskar >> Cc: U-Boot user list; Wolfgang Denk; Scott Wood; Stefan Roese >> Subject: Re: [PATCH 1/4 v4] arm: add support for the >> mgcoge2_arm_p1a board from keymile >> >> Hello Prafulla, >> >> Prafulla Wadaskar wrote: >>>> -----Original Message----- >>>> From: Heiko Schocher [mailto:h...@denx.de] >>>> Sent: Friday, February 12, 2010 1:36 PM >>>> To: U-Boot user list >>>> Cc: Wolfgang Denk; Prafulla Wadaskar; Scott Wood; Stefan Roese >>>> Subject: [PATCH 1/4 v4] arm: add support for the >>>> mgcoge2_arm_p1a board from keymile >>>> >>>> Add support for the ARM part of the mgcoge2. This board >>>> is based on the Marvell Kirkwood (88F6281) SoC. As there >>>> come more board variants, common code is stored in >>>> board/keymile/km_arm/km_arm.c >>>> >>>> Signed-off-by: Holger Brunck <holger.bru...@keymile.com> >>>> Signed-off-by: Stefan Roese <s...@denx.de> >>>> Signed-off-by: Heiko Schocher <h...@denx.de> > > ...snip... > >>>> diff --git a/board/keymile/common/common.c >>>> b/board/keymile/common/common.c >>>> index ec27bda..7b4eefd 100644 >>>> --- a/board/keymile/common/common.c >>>> +++ b/board/keymile/common/common.c >>>> @@ -35,6 +35,7 @@ >>>> #include <libfdt.h> >>>> #endif >>>> >>>> +#include "../common/common.h" >>> Can't you use "common.h" here ? >> No, this is "just" a keymile specific header for including >> functions, which are "common" for all keymile boards ... > > More important common.h that you are referring here is missing in this patch.
No, it is in mainline, see: http://git.denx.de/?p=u-boot.git;a=blob;f=board/keymile/common/common.h;h=a38c72772ce75f4659c50378c8d16c4098ec2b6c;hb=77e7273c40315abd2f3c17ad8d46a78950e3e65f > Secondly, since you are in the same folder you should use "common.h" instead > of absolute path" This is a header common for all keymile boards, so it sit in the board/manufacturer/common/ directory. This is used also used for some ppc based boards. > ...snip... >>>> + >>>> + /* >>>> + * arch number of board >>>> + */ >>>> + gd->bd->bi_arch_number = MACH_TYPE_SUEN3; >>> This does not match with the board you are supporting, >> better to use similar name >> >> As I said above, this boards are all the same, just different >> hardwarerevisions, so theys all have one MACH_TYPE_ ... > > To me, two boards are two different hardware and must have associated with > two different machine ids. > How will you manage picking different board setups in kernel for same machine > ID? They all use the *same* kernel! > If not Machine ID, you can think of using EEPROM available on board to store > board version info and use it selectively. > >>>> + >>>> + /* address of boot parameters */ >>>> + gd->bd->bi_boot_params = kw_sdram_bar(0) + 0x100; >>>> + >>>> +#if defined(CONFIG_KIRKWOOD_GPIO) >>> Avoid this at least for this board patch >> Don;t understand this! Why? The board uses this pins for >> I2C bitbang, so I must initialize the pins ... > > why you need to ifdef this? For this particular board it should be hard > coded, I think you can do this for other board support if needed. > > First patch in this series has to be clean standard board support without any > ifdefs, subsequent patches add support for different hardware version, there > if you use ifdef it makes more sense Ok, I delete this. bye Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
