Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Hello Prafulla, tahnks for your quick response! Prafulla Wadaskar wrote: >> -Original Message- >> From: Heiko Schocher [mailto:h...@denx.de] >> Sent: Wednesday, February 10, 2010 12:39 PM >> To: Prafulla Wadaskar >> Cc: U-Boot user list >> Subject: Re: [PATCH 2/2 v3] arm: suen3, suen3_v1, >> mgcoge2_arm_p1a support >> >> Hello Prafulla, >> >> do you have some comments on the following 2 points? >> (If they are clarified, I can post v4 of the patch ...) >> >> Heiko Schocher wrote: >>> Prafulla Wadaskar wrote: > -Original Message- > From: Heiko Schocher [mailto:h...@denx.de] > Sent: Monday, February 01, 2010 1:07 PM > To: U-Boot user list > Cc: Wolfgang Denk; Prafulla Wadaskar; Tom > Subject: [PATCH 2/2 v3] arm: suen3, suen3_v1, >> mgcoge2_arm_p1a support > ...snip... the include/config files indicates that there are five >> board supports. Please provide one patch for each board, may be first will >> be master one. >>> This question also asked Tom, see: >>> >>> http://lists.denx.de/pipermail/u-boot/2010-January/067182.html >>> >>> But if you prefer to split this in 5 patches, I can do it. >> Is it OK in one patch, or should I split it in 4 patches? > > Dear Heiko > > You should split it in patches as per boards supported, if you are supporting > four board then there should be four different patches. OK. >> [...] > diff --git a/board/keymile/km_arm/sdramregs.txt > b/board/keymile/km_arm/sdramregs.txt > new file mode 100644 > index 000..68c53a7 > --- /dev/null > +++ b/board/keymile/km_arm/sdramregs.txt > @@ -0,0 +1,31 @@ What is this file? Which license? Who is using it? >>> Ok, you are right, some comments are here necessary. >>> >>> On this boards is a preloader, which initializes >>> the RAM. Therefore the preloader reads the RAM settings >>> from the image he should load, through an hear. This >>> header is created with a tool doimage (I think it is >>> from marvell), and this tool needs this file ... > > Marvell Kirkwood has internal bootROM and it may be active through h/w > configuration on you board. > In such case bootROM tries to read Kirkwood boot image (i.e. kwbimage) from > boot media (i.e. NAND/SPI/Sata etc). > > So kwbimage.cfg (the above file that you have created) should be present in > board folder and this will be used by mkimage tool if you create u-boot.kwb > target. Ah, OK! >>> So, I have no idea where to put this files, and think >>> they are in the board directory on the right place ... > > Please refer docs/README.kwbimage Ok, found it, thanks >>> I found something similiar in current mainline: >>> >>> board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt > > There is kwbimage.cfg and not dramregs_333h.txt, may be you are referring > very early post Yep, sorry. 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
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
> -Original Message- > From: Heiko Schocher [mailto:h...@denx.de] > Sent: Wednesday, February 10, 2010 12:39 PM > To: Prafulla Wadaskar > Cc: U-Boot user list > Subject: Re: [PATCH 2/2 v3] arm: suen3, suen3_v1, > mgcoge2_arm_p1a support > > Hello Prafulla, > > do you have some comments on the following 2 points? > (If they are clarified, I can post v4 of the patch ...) > > Heiko Schocher wrote: > > Prafulla Wadaskar wrote: > >>> -Original Message- > >>> From: Heiko Schocher [mailto:h...@denx.de] > >>> Sent: Monday, February 01, 2010 1:07 PM > >>> To: U-Boot user list > >>> Cc: Wolfgang Denk; Prafulla Wadaskar; Tom > >>> Subject: [PATCH 2/2 v3] arm: suen3, suen3_v1, > mgcoge2_arm_p1a support ...snip... > >> the include/config files indicates that there are five > board supports. > >> Please provide one patch for each board, may be first will > be master one. > > > > This question also asked Tom, see: > > > > http://lists.denx.de/pipermail/u-boot/2010-January/067182.html > > > > But if you prefer to split this in 5 patches, I can do it. > > Is it OK in one patch, or should I split it in 4 patches? Dear Heiko You should split it in patches as per boards supported, if you are supporting four board then there should be four different patches. > > [...] > >>> diff --git a/board/keymile/km_arm/sdramregs.txt > >>> b/board/keymile/km_arm/sdramregs.txt > >>> new file mode 100644 > >>> index 000..68c53a7 > >>> --- /dev/null > >>> +++ b/board/keymile/km_arm/sdramregs.txt > >>> @@ -0,0 +1,31 @@ > >> What is this file? > >> Which license? > >> Who is using it? > > > > Ok, you are right, some comments are here necessary. > > > > On this boards is a preloader, which initializes > > the RAM. Therefore the preloader reads the RAM settings > > from the image he should load, through an hear. This > > header is created with a tool doimage (I think it is > > from marvell), and this tool needs this file ... Marvell Kirkwood has internal bootROM and it may be active through h/w configuration on you board. In such case bootROM tries to read Kirkwood boot image (i.e. kwbimage) from boot media (i.e. NAND/SPI/Sata etc). So kwbimage.cfg (the above file that you have created) should be present in board folder and this will be used by mkimage tool if you create u-boot.kwb target. > > > > So, I have no idea where to put this files, and think > > they are in the board directory on the right place ... Please refer docs/README.kwbimage > > > > I found something similiar in current mainline: > > > > board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt There is kwbimage.cfg and not dramregs_333h.txt, may be you are referring very early post > > > > This file is also without comments, license info ... > > Maybe this tool don;t accept comments? > > What should I do with this file? Please see sheevaplug implementation in latest release Regards.. Prafulla . . > > 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
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Hello Prafulla, do you have some comments on the following 2 points? (If they are clarified, I can post v4 of the patch ...) Heiko Schocher wrote: > Prafulla Wadaskar wrote: >>> -Original Message- >>> From: Heiko Schocher [mailto:h...@denx.de] >>> Sent: Monday, February 01, 2010 1:07 PM >>> To: U-Boot user list >>> Cc: Wolfgang Denk; Prafulla Wadaskar; Tom >>> Subject: [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support >>> >>> This patch adds support for the Keymile SUEN3 board variants which >>> are based on the Marvell Kirkwood (88F6281) SoC. All variants >>> uses common code stored in board/keymile/km_arm/km_arm.c >>> >>> mgcoge2_arm_p1a board: >>> This adds support for the ARM part of the mgcoge2. The suen3 >>> target was moved to the correct suen3 p1b version. There is a >>> difference between the GPIO configuration between suen3 and mgcoge2. >>> >>> Signed-off-by: Holger Brunck >>> Signed-off-by: Stefan Roese >>> Signed-off-by: Heiko Schocher >>> --- >>> - changes since v1: >>> added comments from Wolfgang Denk: >>> get rid of flash_info_t define in board config >>> (to get this working patch 1/2 is introduced/needed) >>> - changes since v2: >>> added comments from Wolfgang Denk >>> - rearranged if/else in do_spi_toggle() >>> - added I/O accessor functions for bootcounter >>> >>> MAINTAINERS |2 + >>> MAKEALL |4 + >>> Makefile |8 + >>> board/keymile/common/common.c | 23 ++- >>> board/keymile/km_arm/Makefile | 51 + >>> board/keymile/km_arm/config.mk| 25 +++ >>> board/keymile/km_arm/km_arm.c | 343 >>> + >>> board/keymile/km_arm/sdramregs.txt| 31 +++ >>> board/keymile/km_arm/sdramregs_v1.txt | 28 +++ >>> include/configs/km-arm.h | 189 ++ [...] >>> include/configs/mgcoge2_arm_p1a.h | 96 + >>> include/configs/suen3.h | 103 ++ >>> include/configs/suen3_p1a.h | 82 >>> include/configs/suen3_p1b_p1c.h | 110 +++ >> ...snip... >> >> the include/config files indicates that there are five board supports. >> Please provide one patch for each board, may be first will be master one. > > This question also asked Tom, see: > > http://lists.denx.de/pipermail/u-boot/2010-January/067182.html > > But if you prefer to split this in 5 patches, I can do it. Is it OK in one patch, or should I split it in 4 patches? [...] >>> diff --git a/board/keymile/km_arm/sdramregs.txt >>> b/board/keymile/km_arm/sdramregs.txt >>> new file mode 100644 >>> index 000..68c53a7 >>> --- /dev/null >>> +++ b/board/keymile/km_arm/sdramregs.txt >>> @@ -0,0 +1,31 @@ >> What is this file? >> Which license? >> Who is using it? > > Ok, you are right, some comments are here necessary. > > On this boards is a preloader, which initializes > the RAM. Therefore the preloader reads the RAM settings > from the image he should load, through an header. This > header is created with a tool doimage (I think it is > from marvell), and this tool needs this file ... > > So, I have no idea where to put this files, and think > they are in the board directory on the right place ... > > I found something similiar in current mainline: > > board/Marvell/mv88f6281gtw_ge/dramregs_333h.txt > > This file is also without comments, license info ... > Maybe this tool don;t accept comments? What should I do with this file? 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
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Heiko Schocher wrote: > Scott Wood wrote: >> That's not how nand_init() is meant to be used. It is meant to be called >> once on system init. There is probably at least a memory leak here, e.g. >> chip->buffers. > > Oh, Ok. How could/should this then be solved? Well, for a hackish solution that avoids the leak on chip->buffers, see omap_nand_switch_ecc(). > (Some weak function, maybe: int nand_available(void)?, that board specific > code can overwrite, and this function is checked before a nand command > is executed?) I'd rather make the callback specific to the driver -- you're not disabling any possible NAND controller that might be attached through whatever odd means, you're disabling the kirkwood NAND controller. > In the First step, I don;t call nand_init() again, there is also a > warning message, that NAND is disabled, so the user should know, that > he don;t have longer access to it, is this Okay for you? > >> Even as a hack, it looks like these boards use the kirkwood nand controller, >> and its board_nand_init() will unconditionally return 0, telling > > Yep, but ... > >> nand_init_chip that it does indeed have NAND available. Or is there a patch >> somewhere changing that? > > ... nand_get_flash_type() returns -ENODEV, if no manufacturer or/and > id could be read from nand (And if using this u-boot command, the nand > is not longer visible, because the nand is disabled, and the pins are > used to access a SPI Flash) -> nand_scan_ident returns this error, and > so nand_scan ... Will the attempt to do a read ID command cause bad things to go on the SPI pins when configured as SPI? Or will the NAND controller hardware know that it doesn't have access to the pins and just return dummy values? -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Hello Scott,
Scott Wood wrote:
> On Wed, Feb 03, 2010 at 04:52:05PM +0100, Heiko Schocher wrote:
+ if ((strcmp(argv[1], "off") == 0)) {
+ printf("SPI FLASH disabled, NAND enabled\n");
+ /* Multi-Purpose Pins Functionality configuration */
+ kwmpp_config[0] = MPP0_NF_IO2;
+ kwmpp_config[1] = MPP1_NF_IO3;
+ kwmpp_config[2] = MPP2_NF_IO4;
+ kwmpp_config[3] = MPP3_NF_IO5;
+
+ kirkwood_mpp_conf(kwmpp_config);
+ tmp = readl(KW_GPIO0_BASE);
+ writel(tmp | FLASH_GPIO_PIN , KW_GPIO0_BASE);
+
+ nand_init();
+ } else if ((strcmp(argv[1], "on") == 0)) {
+ printf("SPI FLASH enabled, NAND disabled\n");
+ /* Multi-Purpose Pins Functionality configuration */
+ kwmpp_config[0] = MPP0_SPI_SCn;
+ kwmpp_config[1] = MPP1_SPI_MOSI;
+ kwmpp_config[2] = MPP2_SPI_SCK;
+ kwmpp_config[3] = MPP3_SPI_MISO;
+
+ kirkwood_mpp_conf(kwmpp_config);
+ tmp = readl(KW_GPIO0_BASE);
+ writel(tmp & (~FLASH_GPIO_PIN) , KW_GPIO0_BASE);
+
+ nand_init();
>>> What do you need nand_init for disabled nand operation?
>> With it, the nand subsystem knows, that there is no longer
>> the nand availiable.
>
> That's not how nand_init() is meant to be used. It is meant to be called
> once on system init. There is probably at least a memory leak here, e.g.
> chip->buffers.
Oh, Ok. How could/should this then be solved?
(Some weak function, maybe: int nand_available(void)?, that board specific
code can overwrite, and this function is checked before a nand command
is executed?)
In the First step, I don;t call nand_init() again, there is also a
warning message, that NAND is disabled, so the user should know, that
he don;t have longer access to it, is this Okay for you?
> Even as a hack, it looks like these boards use the kirkwood nand controller,
> and its board_nand_init() will unconditionally return 0, telling
Yep, but ...
> nand_init_chip that it does indeed have NAND available. Or is there a patch
> somewhere changing that?
... nand_get_flash_type() returns -ENODEV, if no manufacturer or/and
id could be read from nand (And if using this u-boot command, the nand
is not longer visible, because the nand is disabled, and the pins are
used to access a SPI Flash) -> nand_scan_ident returns this error, and
so nand_scan ...
Actually, I get this message, when running this code:
No NAND device found!!!
Thanks for your comment.
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
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
On Wed, Feb 03, 2010 at 04:52:05PM +0100, Heiko Schocher wrote:
> >> + if ((strcmp(argv[1], "off") == 0)) {
> >> + printf("SPI FLASH disabled, NAND enabled\n");
> >> + /* Multi-Purpose Pins Functionality configuration */
> >> + kwmpp_config[0] = MPP0_NF_IO2;
> >> + kwmpp_config[1] = MPP1_NF_IO3;
> >> + kwmpp_config[2] = MPP2_NF_IO4;
> >> + kwmpp_config[3] = MPP3_NF_IO5;
> >> +
> >> + kirkwood_mpp_conf(kwmpp_config);
> >> + tmp = readl(KW_GPIO0_BASE);
> >> + writel(tmp | FLASH_GPIO_PIN , KW_GPIO0_BASE);
> >> +
> >> + nand_init();
> >> + } else if ((strcmp(argv[1], "on") == 0)) {
> >> + printf("SPI FLASH enabled, NAND disabled\n");
> >> + /* Multi-Purpose Pins Functionality configuration */
> >> + kwmpp_config[0] = MPP0_SPI_SCn;
> >> + kwmpp_config[1] = MPP1_SPI_MOSI;
> >> + kwmpp_config[2] = MPP2_SPI_SCK;
> >> + kwmpp_config[3] = MPP3_SPI_MISO;
> >> +
> >> + kirkwood_mpp_conf(kwmpp_config);
> >> + tmp = readl(KW_GPIO0_BASE);
> >> + writel(tmp & (~FLASH_GPIO_PIN) , KW_GPIO0_BASE);
> >> +
> >> + nand_init();
> >
> > What do you need nand_init for disabled nand operation?
>
> With it, the nand subsystem knows, that there is no longer
> the nand availiable.
That's not how nand_init() is meant to be used. It is meant to be called
once on system init. There is probably at least a memory leak here, e.g.
chip->buffers.
Even as a hack, it looks like these boards use the kirkwood nand controller,
and its board_nand_init() will unconditionally return 0, telling
nand_init_chip that it does indeed have NAND available. Or is there a patch
somewhere changing that?
-Scott
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
On Wednesday 03 February 2010 16:52:05 Heiko Schocher wrote:
> >> +
> >> + str = getenv("mach_type");
> >> + if (str != NULL) {
> >> + mach_type = simple_strtoul(str, NULL, 10);
> >> + printf("Overwriting MACH_TYPE with %d!!!\n", mach_type);
> >> + gd->bd->bi_arch_number = mach_type;
> >> + }
> >> + return 0;
> >
> > Pls avoid this, the machine types should be predefined and registered
> > first for the board support that you are adding. Why do you need this in
> > environment?.
>
> Stefan, can you answer this?
Yes.
The SUEN3 mach-type is already registered. But we wanted to use one U-Boot
image on this system (early suen3), and boot the "normal" mainline Linux
kernel (with the registered mach-type) and a specific Linux kernel image from
the Linux kernel distributed from Marvell (2.6.22 based). And this kernel
image had a different MACH_TYPE number. So we added this method to dynamically
change the mach-type passed to the Linux kernel.
I have no idea if this is still needed for Keymile. But I also think this
doesn't "hurt". So I'm voting to keep it for now.
Cheers,
Stefan
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Hello Prafulla,
Prafulla Wadaskar wrote:
> First of all sorry for delayed feedback
No problem, thanks for reviewing!
>> -Original Message-
>> From: Heiko Schocher [mailto:h...@denx.de]
>> Sent: Monday, February 01, 2010 1:07 PM
>> To: U-Boot user list
>> Cc: Wolfgang Denk; Prafulla Wadaskar; Tom
>> Subject: [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
>>
>> This patch adds support for the Keymile SUEN3 board variants which
>> are based on the Marvell Kirkwood (88F6281) SoC. All variants
>> uses common code stored in board/keymile/km_arm/km_arm.c
>>
>> mgcoge2_arm_p1a board:
>> This adds support for the ARM part of the mgcoge2. The suen3
>> target was moved to the correct suen3 p1b version. There is a
>> difference between the GPIO configuration between suen3 and mgcoge2.
>>
>> Signed-off-by: Holger Brunck
>> Signed-off-by: Stefan Roese
>> Signed-off-by: Heiko Schocher
>> ---
>> - changes since v1:
>> added comments from Wolfgang Denk:
>> get rid of flash_info_t define in board config
>> (to get this working patch 1/2 is introduced/needed)
>> - changes since v2:
>> added comments from Wolfgang Denk
>> - rearranged if/else in do_spi_toggle()
>> - added I/O accessor functions for bootcounter
>>
>> MAINTAINERS |2 +
>> MAKEALL |4 +
>> Makefile |8 +
>> board/keymile/common/common.c | 23 ++-
>> board/keymile/km_arm/Makefile | 51 +
>> board/keymile/km_arm/config.mk| 25 +++
>> board/keymile/km_arm/km_arm.c | 343
>> +
>> board/keymile/km_arm/sdramregs.txt| 31 +++
>> board/keymile/km_arm/sdramregs_v1.txt | 28 +++
>> include/configs/km-arm.h | 189 ++
>
> The associated board name and c file is named as kw_arm, pls sync on this
> This file name should be km_arm.h
Yep, you are right! I fix this.
>> include/configs/mgcoge2_arm_p1a.h | 96 +
>> include/configs/suen3.h | 103 ++
>> include/configs/suen3_p1a.h | 82
>> include/configs/suen3_p1b_p1c.h | 110 +++
> ...snip...
>
> the include/config files indicates that there are five board supports.
> Please provide one patch for each board, may be first will be master one.
This question also asked Tom, see:
http://lists.denx.de/pipermail/u-boot/2010-January/067182.html
But if you prefer to split this in 5 patches, I can do it.
> ...snip...
>> diff --git a/board/keymile/common/common.c
>> b/board/keymile/common/common.c
>> index ec27bda..33857c7 100644
>> --- a/board/keymile/common/common.c
>> +++ b/board/keymile/common/common.c
>> @@ -35,6 +35,7 @@
>> #include
>> #endif
>>
>> +#include "../common/common.h"
>> #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
>> #include
>>
>> @@ -395,7 +396,12 @@ static void setports (int gpio)
>> #endif
>> #endif
>>
>> -#if defined(CONFIG_KM8XX)
>> +#if (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_HARD_I2C))
>> +#error I2C bus resetsequence not implemented yet.
>> +#endif
>> +
>> +#if defined(CONFIG_KM8XX) || \
>> +(defined(CONFIG_MACH_SUEN3) && defined(CONFIG_SOFT_I2C))
>> static void set_sda (int state)
>> {
>> I2C_SDA(state);
>> @@ -411,6 +417,12 @@ static int get_sda (void)
>> return I2C_READ;
>> }
>>
>> +#if defined(CONFIG_MACH_SUEN3)
>> +static int get_scl (void)
>> +{
>> +return (kw_gpio_get_value(SUEN3_SCL_PIN) ? 1 : 0);
>> +}
>> +#else
>> static int get_scl (void)
>> {
>> int val;
>> @@ -421,10 +433,11 @@ static int get_scl (void)
>>
>> return ((val & SCL_BIT) == SCL_BIT);
>> }
>> +#endif
>>
>> #endif
>>
>> -#if !defined(CONFIG_KMETER1)
>> +#if !defined(CONFIG_KMETER1) && !defined(CONFIG_SUVD3)
>> static void writeStartSeq (void)
>> {
>> set_sda (1);
>> @@ -483,7 +496,7 @@ static int i2c_make_abort (void)
>> */
>> void i2c_init_board(void)
>> {
>> -#if defined(CONFIG_KMETER1)
>> +#if defined(CONFIG_KMETER1) || defined(CONFIG_SUVD3)
>> struct fsl_i2c *dev;
>> dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR +
>> CONFIG_SYS_I2C_OFFSET);
>> uchar dummy;
>> @@ -500,7 +513,7 @@ void i2c_init_board(void)
>> out_8 (&dev->cr, (I2C_CR_MEN));
>>
>> #else
>> -#if defined(CONFIG_HARD_I2C)
>> +#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_MACH_SUEN3)
>> volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR ;
>> volatile i2c8260_t *i2c = (i2c8260_t *)&immap->im_i2c;
>>
>> @@ -578,10 +591,12 @@ int fdt_get_node_and_value (void *blob,
>> }
>> #endif
>>
>> +#if !defined(CONFIG_MACH_SUEN3)
>> int ethernet_present (void)
>> {
>> return (in_8((u8 *)CONFIG_SYS_PIGGY_BASE +
>> CONFIG_SYS_SLOT_ID_OFF) & 0x80);
>> }
>> +#endif
>>
>
> In general common file should have code common to all, but the patch for this
> file looks like the code is ifdefed for specific boards. You can keep such
> code in km_arm.c
Hmm.
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Dear Prafulla Wadaskar, In message <73173d32e9439e4abb5151606c3e19e2030a566...@sc-vexch1.marvell.com> you wrote: > ... > > +/* include common defines/options for all arm based Keymile boards */ > > +#include "km-arm.h" > > Further includes are not allowed in board config header. > May be Wolfgang can comment on this.. I am not aware of any such restriction. We actually do this on many boards - and we intend to use this more frequently to factor out common definitions. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Q: How many DEC repairman does it take to fix a flat? A: Five; four to hold the car up and one to swap tires. Q: How long does it take? A: It's indeterminate. It will depend upon how many flats they've brought with them. Q: What happens if you've got TWO flats? A: They replace your generator. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
Hi Heiko
First of all sorry for delayed feedback
> -Original Message-
> From: Heiko Schocher [mailto:h...@denx.de]
> Sent: Monday, February 01, 2010 1:07 PM
> To: U-Boot user list
> Cc: Wolfgang Denk; Prafulla Wadaskar; Tom
> Subject: [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
>
> This patch adds support for the Keymile SUEN3 board variants which
> are based on the Marvell Kirkwood (88F6281) SoC. All variants
> uses common code stored in board/keymile/km_arm/km_arm.c
>
> mgcoge2_arm_p1a board:
> This adds support for the ARM part of the mgcoge2. The suen3
> target was moved to the correct suen3 p1b version. There is a
> difference between the GPIO configuration between suen3 and mgcoge2.
>
> Signed-off-by: Holger Brunck
> Signed-off-by: Stefan Roese
> Signed-off-by: Heiko Schocher
> ---
> - changes since v1:
> added comments from Wolfgang Denk:
> get rid of flash_info_t define in board config
> (to get this working patch 1/2 is introduced/needed)
> - changes since v2:
> added comments from Wolfgang Denk
> - rearranged if/else in do_spi_toggle()
> - added I/O accessor functions for bootcounter
>
> MAINTAINERS |2 +
> MAKEALL |4 +
> Makefile |8 +
> board/keymile/common/common.c | 23 ++-
> board/keymile/km_arm/Makefile | 51 +
> board/keymile/km_arm/config.mk| 25 +++
> board/keymile/km_arm/km_arm.c | 343
> +
> board/keymile/km_arm/sdramregs.txt| 31 +++
> board/keymile/km_arm/sdramregs_v1.txt | 28 +++
> include/configs/km-arm.h | 189 ++
The associated board name and c file is named as kw_arm, pls sync on this
This file name should be km_arm.h
> include/configs/mgcoge2_arm_p1a.h | 96 +
> include/configs/suen3.h | 103 ++
> include/configs/suen3_p1a.h | 82
> include/configs/suen3_p1b_p1c.h | 110 +++
...snip...
the include/config files indicates that there are five board supports.
Please provide one patch for each board, may be first will be master one.
...snip...
> diff --git a/board/keymile/common/common.c
> b/board/keymile/common/common.c
> index ec27bda..33857c7 100644
> --- a/board/keymile/common/common.c
> +++ b/board/keymile/common/common.c
> @@ -35,6 +35,7 @@
> #include
> #endif
>
> +#include "../common/common.h"
> #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
> #include
>
> @@ -395,7 +396,12 @@ static void setports (int gpio)
> #endif
> #endif
>
> -#if defined(CONFIG_KM8XX)
> +#if (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_HARD_I2C))
> +#error I2C bus resetsequence not implemented yet.
> +#endif
> +
> +#if defined(CONFIG_KM8XX) || \
> + (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_SOFT_I2C))
> static void set_sda (int state)
> {
> I2C_SDA(state);
> @@ -411,6 +417,12 @@ static int get_sda (void)
> return I2C_READ;
> }
>
> +#if defined(CONFIG_MACH_SUEN3)
> +static int get_scl (void)
> +{
> + return (kw_gpio_get_value(SUEN3_SCL_PIN) ? 1 : 0);
> +}
> +#else
> static int get_scl (void)
> {
> int val;
> @@ -421,10 +433,11 @@ static int get_scl (void)
>
> return ((val & SCL_BIT) == SCL_BIT);
> }
> +#endif
>
> #endif
>
> -#if !defined(CONFIG_KMETER1)
> +#if !defined(CONFIG_KMETER1) && !defined(CONFIG_SUVD3)
> static void writeStartSeq (void)
> {
> set_sda (1);
> @@ -483,7 +496,7 @@ static int i2c_make_abort (void)
> */
> void i2c_init_board(void)
> {
> -#if defined(CONFIG_KMETER1)
> +#if defined(CONFIG_KMETER1) || defined(CONFIG_SUVD3)
> struct fsl_i2c *dev;
> dev = (struct fsl_i2c *) (CONFIG_SYS_IMMR +
> CONFIG_SYS_I2C_OFFSET);
> uchar dummy;
> @@ -500,7 +513,7 @@ void i2c_init_board(void)
> out_8 (&dev->cr, (I2C_CR_MEN));
>
> #else
> -#if defined(CONFIG_HARD_I2C)
> +#if defined(CONFIG_HARD_I2C) && !defined(CONFIG_MACH_SUEN3)
> volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR ;
> volatile i2c8260_t *i2c = (i2c8260_t *)&immap->im_i2c;
>
> @@ -578,10 +591,12 @@ int fdt_get_node_and_value (void *blob,
> }
> #endif
>
> +#if !defined(CONFIG_MACH_SUEN3)
> int ethernet_present (void)
> {
> return (in_8((u8 *)CONFIG_SYS_PIGGY_BASE +
> CONFIG_SYS_SLOT_ID_OFF) & 0x80);
> }
> +#endif
>
In general common file should have code common to all, but the patch for this
file looks like the code is ifdefed for specific boards. You can keep such code
in km_arm.c
> int board_eth_init (bd_t *bis)
> {
> diff --git a/board/keymile/km_arm/km_arm.c
> b/board/keymile/km_arm/km_arm.c
> new file mode 100644
> index 000..0edf3b2
> --- /dev/null
> +++ b/board/keymile/km_arm/km_arm.c
> @@ -0,0 +1,343 @@
> +/*
> + * (C) Copyright 2009
> + * Marvell Semiconductor
> + * Prafulla Wadaskar
> + *
> + * (C) Copyright 2009
> + * Stefan Roese, DENX Softwa
[U-Boot] [PATCH 2/2 v3] arm: suen3, suen3_v1, mgcoge2_arm_p1a support
This patch adds support for the Keymile SUEN3 board variants which
are based on the Marvell Kirkwood (88F6281) SoC. All variants
uses common code stored in board/keymile/km_arm/km_arm.c
mgcoge2_arm_p1a board:
This adds support for the ARM part of the mgcoge2. The suen3
target was moved to the correct suen3 p1b version. There is a
difference between the GPIO configuration between suen3 and mgcoge2.
Signed-off-by: Holger Brunck
Signed-off-by: Stefan Roese
Signed-off-by: Heiko Schocher
---
- changes since v1:
added comments from Wolfgang Denk:
get rid of flash_info_t define in board config
(to get this working patch 1/2 is introduced/needed)
- changes since v2:
added comments from Wolfgang Denk
- rearranged if/else in do_spi_toggle()
- added I/O accessor functions for bootcounter
MAINTAINERS |2 +
MAKEALL |4 +
Makefile |8 +
board/keymile/common/common.c | 23 ++-
board/keymile/km_arm/Makefile | 51 +
board/keymile/km_arm/config.mk| 25 +++
board/keymile/km_arm/km_arm.c | 343 +
board/keymile/km_arm/sdramregs.txt| 31 +++
board/keymile/km_arm/sdramregs_v1.txt | 28 +++
include/configs/km-arm.h | 189 ++
include/configs/mgcoge2_arm_p1a.h | 96 +
include/configs/suen3.h | 103 ++
include/configs/suen3_p1a.h | 82
include/configs/suen3_p1b_p1c.h | 110 +++
14 files changed, 1091 insertions(+), 4 deletions(-)
create mode 100644 board/keymile/km_arm/Makefile
create mode 100644 board/keymile/km_arm/config.mk
create mode 100644 board/keymile/km_arm/km_arm.c
create mode 100644 board/keymile/km_arm/sdramregs.txt
create mode 100644 board/keymile/km_arm/sdramregs_v1.txt
create mode 100644 include/configs/km-arm.h
create mode 100644 include/configs/mgcoge2_arm_p1a.h
create mode 100644 include/configs/suen3.h
create mode 100644 include/configs/suen3_p1a.h
create mode 100644 include/configs/suen3_p1b_p1c.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 9734b1d..6ca5a21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -385,6 +385,8 @@ Stefan Roese
P3M750 PPC750FX/GX/GL
+ suen3 ARM926EJS (Kirkwood SoC)
+
Yusdi Santoso
HIDDEN_DRAGON MPC8241/MPC8245
diff --git a/MAKEALL b/MAKEALL
index ab1bb6f..17f277b 100755
--- a/MAKEALL
+++ b/MAKEALL
@@ -554,6 +554,7 @@ LIST_ARM9=" \
da830evm\
imx27lite \
lpd7a400\
+ mgcoge2_arm_p1a \
mv88f6281gtw_ge \
mx1ads \
mx1fs2 \
@@ -572,6 +573,9 @@ LIST_ARM9=" \
sheevaplug \
smdk2400\
smdk2410\
+ suen3_p1a \
+ suen3_p1b_p1c \
+ suen3 \
trab\
VCMA9 \
versatile \
diff --git a/Makefile b/Makefile
index ed6156f..968d8a9 100644
--- a/Makefile
+++ b/Makefile
@@ -2934,6 +2934,9 @@ lpd7a400_config \
lpd7a404_config: unconfig
@$(MKCONFIG) $(@:_config=) arm lh7a40x lpd7a40x
+mgcoge2_arm_p1a_config: unconfig
+ @$(MKCONFIG) $(@:_config=) arm arm926ejs km_arm keymile kirkwood
+
mv88f6281gtw_ge_config: unconfig
@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood
@@ -3023,6 +3026,11 @@ smdk2400_config : unconfig
smdk2410_config: unconfig
@$(MKCONFIG) $(@:_config=) arm arm920t smdk2410 samsung s3c24x0
+suen3_config \
+suen3_p1a_config \
+suen3_p1b_p1c_config: unconfig
+ @$(MKCONFIG) $(@:_config=) arm arm926ejs km_arm keymile kirkwood
+
SX1_stdout_serial_config \
SX1_config:unconfig
@mkdir -p $(obj)include
diff --git a/board/keymile/common/common.c b/board/keymile/common/common.c
index ec27bda..33857c7 100644
--- a/board/keymile/common/common.c
+++ b/board/keymile/common/common.c
@@ -35,6 +35,7 @@
#include
#endif
+#include "../common/common.h"
#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
#include
@@ -395,7 +396,12 @@ static void setports (int gpio)
#endif
#endif
-#if defined(CONFIG_KM8XX)
+#if (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_HARD_I2C))
+#error I2C bus resetsequence not implemented yet.
+#endif
+
+#if defined(CONFIG_KM8XX) || \
+ (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_SOFT_I2C))
static void set_sda (int state)
{
I2C_SDA(state);
@@ -411,6 +417,12 @@ static int get_sda (void)
return I2C_READ;
}
+#if defined(CONFIG_MACH_SUEN3)
+static int get_scl (void)
+{
+ return (kw_gpio_get_value(SUEN3_SCL_PIN) ? 1 : 0);
+}
+#else
static int get_scl (void)
{
int val;
@@ -421,10 +433,11 @@ static int get_s
