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>
>> ---
>> - 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
>>
>> - changes since v3:
>> added comment Scott Wood
>> - removed nand_init in do_spi_toggle()
>>
>> added comments from Prafulla Wadagaskar
>> - km-arm.h renamed to km_arm.h
>> - reworked eeprom_write_enable() (deleted it)
>> (when reviewing this function, it cropped up, that
>> this pin is connected through a gpio pin, not as
>> in previous version, through the boco (a FPGA))
>> - moved set_sda(), set_scl(), get_sda(), get_scl()
>> to km_arm.c
>> - split patch in 4 patches (for each board an extra patch)
>> - renamed sdramregs.txt in kwbimage.cfg, also license
>> info added.
>>
>> MAINTAINERS | 2 +
>> MAKEALL | 1 +
>> Makefile | 3 +
>> board/keymile/common/common.c | 6 +-
>> board/keymile/km_arm/Makefile | 51 ++++++
>> board/keymile/km_arm/config.mk | 28 +++
>> board/keymile/km_arm/km_arm.c | 339
>> +++++++++++++++++++++++++++++++++++++
>> board/keymile/km_arm/kwbimage.cfg | 59 +++++++
>> include/configs/km_arm.h | 191 +++++++++++++++++++++
>> include/configs/mgcoge2_arm_p1a.h | 96 +++++++++++
>> 10 files changed, 774 insertions(+), 2 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/kwbimage.cfg
>> create mode 100644 include/configs/km_arm.h
>> create mode 100644 include/configs/mgcoge2_arm_p1a.h
[...]
>> 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 ...
>> #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
>> #include <i2c.h>
>>
>> @@ -421,7 +422,6 @@ static int get_scl (void)
>>
>> return ((val & SCL_BIT) == SCL_BIT);
>> }
>> -
>> #endif
>>
>> #if !defined(CONFIG_KMETER1)
>> @@ -500,7 +500,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)
>
> This is not with reference to this board
It is defined in include/configs/km_arm.h line 41
(As I said, this "4" boards are all the same, just different
hardwarerevisions ...)
>> volatile immap_t *immap = (immap_t *)CONFIG_SYS_IMMR ;
>> volatile i2c8260_t *i2c = (i2c8260_t *)&immap->im_i2c;
>>
>> @@ -578,10 +578,12 @@ int fdt_get_node_and_value (void *blob,
>> }
>> #endif
>>
>> +#if !defined(CONFIG_MACH_SUEN3)
>
> Ditto
See comment above.
[...]
>> diff --git a/board/keymile/km_arm/km_arm.c
>> b/board/keymile/km_arm/km_arm.c
>> new file mode 100644
>> index 0000000..e1cf379
>> --- /dev/null
>> +++ b/board/keymile/km_arm/km_arm.c
[...]
>> +/* Multi-Purpose Pins Functionality configuration */
>> +u32 kwmpp_config[] = {
>> + MPP0_NF_IO2,
>> + MPP1_NF_IO3,
>> + MPP2_NF_IO4,
>> + MPP3_NF_IO5,
>> + MPP4_NF_IO6,
>> + MPP5_NF_IO7,
>> + MPP6_SYSRST_OUTn,
>> + MPP7_PEX_RST_OUTn,
>> +#if defined(CONFIG_KIRKWOOD_GPIO)
>
> Why does you need this ifdef here? for a particular board this should be
> pre-defined.
Yep, this define should be CONFIG_SOFT_I2C.
>> + MPP8_GPIO, /* SDA */
>> + MPP9_GPIO, /* SCL */
>> +#else
>> + MPP8_TW_SDA,
>> + MPP9_TW_SCK,
>> +#endif
>> + MPP10_UART0_TXD,
>> + MPP11_UART0_RXD,
>> + MPP12_GPO, /* Reserved */
>> + MPP13_UART1_TXD,
>> + MPP14_UART1_RXD,
>> + MPP15_GPIO, /* Not used */
>> + MPP16_GPIO, /* Not used */
>> + MPP17_GPIO, /* Reserved */
>> + MPP18_NF_IO0,
>> + MPP19_NF_IO1,
>> + MPP20_GPIO,
>> + MPP21_GPIO,
>> + MPP22_GPIO,
>> + MPP23_GPIO,
>> + MPP24_GPIO,
>> + MPP25_GPIO,
>> + MPP26_GPIO,
>> + MPP27_GPIO,
>> + MPP28_GPIO,
>> + MPP29_GPIO,
>> + MPP30_GPIO,
>> + MPP31_GPIO,
>> + MPP32_GPIO,
>> + MPP33_GPIO,
>> + MPP34_GPIO, /* CDL1 (input) */
>> + MPP35_GPIO, /* CDL2 (input) */
>> + MPP36_GPIO, /* MAIN_IRQ (input) */
>> + MPP37_GPIO, /* BOARD_LED */
>> + MPP38_GPIO, /* Piggy3 LED[1] */
>> + MPP39_GPIO, /* Piggy3 LED[2] */
>> + MPP40_GPIO, /* Piggy3 LED[3] */
>> + MPP41_GPIO, /* Piggy3 LED[4] */
>> + MPP42_GPIO, /* Piggy3 LED[5] */
>> + MPP43_GPIO, /* Piggy3 LED[6] */
>> + MPP44_GPIO, /* Piggy3 LED[7] */
>> + MPP45_GPIO, /* Piggy3 LED[8] */
>> + MPP46_GPIO, /* Reserved */
>> + MPP47_GPIO, /* Reserved */
>> + MPP48_GPIO, /* Reserved */
>> + MPP49_GPIO, /* SW_INTOUTn */
>> + 0
>> +};
>> +
>> +int ethernet_present(void)
>> +{
>> + uchar buf;
>> + int ret = 0;
>> +
>> +#if defined(CONFIG_SUEN_P1A)
>
> Ditto
I correct this.
>
>> + int oldbusnum = i2c_get_bus_num();
>> +
>> + i2c_set_bus_num(io_dev);
>> + if (i2c_read(0x74, 0, 1, &buf, 1) != 0) {
>> + printf ("%s: Error reading EEprom\n", __FUNCTION__);
>> + i2c_set_bus_num(oldbusnum);
>> + return -1;
>> + }
>> + if ((buf & 0x80) == 0x80) {
>> + ret = 1;
>> + }
>> + i2c_set_bus_num(oldbusnum);
>> +#else
>> + if (i2c_read(0x10, 2, 1, &buf, 1) != 0) {
>> + printf ("%s: Error reading Boco\n", __FUNCTION__);
>> + return -1;
>> + }
>> + if ((buf & 0x40) == 0x40) {
>> + ret = 1;
>> + }
>> +#endif
>> + return ret;
>> +}
>> +
>> +int misc_init_r(void)
>> +{
>> + I2C_MUX_DEVICE *i2cdev;
>> + char *str;
>> + int mach_type;
>> +
>> + /* add I2C Bus for I/O Expander */
>> + i2cdev = i2c_mux_ident_muxstring((uchar *)"pca9554a:70:a");
>> + io_dev = i2cdev->busid;
>> + puts("Piggy:");
>> + if (ethernet_present() == 0)
>> + puts (" not");
>> + puts(" present\n");
>> +
>> + 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;
>> +}
>> +
>> +int board_init(void)
>> +{
>> + u32 tmp;
>> +
>> + kirkwood_mpp_conf(kwmpp_config);
>> +
>> + /*
>> + * The FLASH_GPIO_PIN switches between using a
>> + * NAND or a SPI FLASH. Set this pin on start
>> + * to NAND mode.
>> + */
>> + tmp = readl(KW_GPIO0_BASE);
>> + writel(tmp | FLASH_GPIO_PIN , KW_GPIO0_BASE);
>> + tmp = readl(KW_GPIO0_BASE + 4);
>> + writel(tmp & (~FLASH_GPIO_PIN) , KW_GPIO0_BASE + 4);
>> + printf("KM: setting NAND mode\n");
>> +
>> + /*
>> + * 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_ ...
>> +
>> + /* 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 ...
>> + /* init the GPIO for I2C Bitbang driver */
>> + kw_gpio_set_valid(SUEN3_SDA_PIN, 1);
>> + kw_gpio_set_valid(SUEN3_SCL_PIN, 1);
>> + kw_gpio_direction_output(SUEN3_SDA_PIN, 0);
>> + kw_gpio_direction_output(SUEN3_SCL_PIN, 0);
>> +#if defined(CONFIG_SYS_EEPROM_WREN)
>> + kw_gpio_set_valid(SUEN3_ENV_WP, 38);
>> + kw_gpio_direction_output(SUEN3_ENV_WP, 1);
>> +#endif
>> +#endif
>> + return 0;
>> +}
[...]
>> +
>> +#if (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_HARD_I2C))
>
> Ditto..
What do you mean here?
>> +#error I2C bus resetsequence not implemented yet.
>> +#endif
>> +
>> +#if (defined(CONFIG_MACH_SUEN3) && defined(CONFIG_SOFT_I2C))
>
> Ditto..
What do you mean here?
>> +void set_sda (int state)
>> +{
>> + I2C_ACTIVE;
>> + I2C_SDA(state);
>> +}
>> +
>> +void set_scl (int state)
>> +{
>> + I2C_SCL(state);
>> +}
>> +
>> +int get_sda (void)
>> +{
>> + I2C_TRISTATE;
>> + return I2C_READ;
>> +}
>> +
>> +int get_scl (void)
>> +{
>> + return (kw_gpio_get_value(SUEN3_SCL_PIN) ? 1 : 0);
>> +}
>> +#endif
>
> Where these are used, I could not find any reference in this patch
They are used in board/keymile/common/common.c for
the I2C deblocking sequence. (This is one of the common code
used for all keymile boards)
>> +
>> +#if defined(CONFIG_SYS_EEPROM_WREN)
>> +int eeprom_write_enable (unsigned dev_addr, int state)
>> +{
>> + kw_gpio_set_value(SUEN3_ENV_WP, !state);
>> +
>> + return !kw_gpio_get_value(SUEN3_ENV_WP);
>> +}
>> +#endif
>> diff --git a/board/keymile/km_arm/kwbimage.cfg
>> b/board/keymile/km_arm/kwbimage.cfg
>> new file mode 100644
>> index 0000000..bfe0889
>> --- /dev/null
>> +++ b/board/keymile/km_arm/kwbimage.cfg
>> @@ -0,0 +1,59 @@
>> +#
>> +# (C) Copyright 2010
>> +# Heiko Schocher, DENX Software Engineering, h...@denx.de.
>> +#
>> +# See file CREDITS for list of people who contributed to this
>> +# project.
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation; either version 2 of
>> +# the License, or (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
>> +# MA 02110-1301 USA
>> +#
>> +# Refer docs/README.kwimage for more details about how-to configure
>> +# and create kirkwood boot image
>> +#
>> +
>> +# Boot Media configurations
>> +BOOT_FROM spi # Boot from SPI flash
>> +
>> +DATA 0xFFD10000 0x01111111
>> +DATA 0xFFD10008 0x00001100
>> +DATA 0xFFD100E0 0x1B1B1B1B
>> +DATA 0xFFD20134 0xBBBBBBBB
>> +DATA 0xFFD20138 0x00BBBBBB
>> +DATA 0xFFD20154 0x00000200
>> +DATA 0xFFD2014C 0x00001C00
>> +DATA 0xFFD20148 0x00000001
>> +DATA 0xFFD01400 0x43000400
>> +DATA 0xFFD01404 0x36343000
>> +DATA 0xFFD01408 0x2302544B
>> +DATA 0xFFD0140C 0x00000032
>> +DATA 0xFFD01410 0x0000000D
>> +DATA 0xFFD01414 0x00000000
>> +DATA 0xFFD01418 0x00000000
>> +DATA 0xFFD0141C 0x00000642
>> +DATA 0xFFD01420 0x00000040
>> +DATA 0xFFD01424 0x0000F07F
>> +DATA 0xFFD01504 0x07FFFFF1
>> +DATA 0xFFD01508 0x00000000
>> +DATA 0xFFD0150C 0x00000000
>> +DATA 0xFFD01514 0x00000000
>> +DATA 0xFFD0151C 0x00000000
>> +DATA 0xFFD01494 0x00000000
>> +DATA 0xFFD01498 0x00000000
>> +DATA 0xFFD0149C 0x0000E90F
>> +DATA 0xFFD01480 0x00000001
>
> It is better to explain each settings here about what exactly you are
> programming
Ok, I try it.
>> +
>> +# End of Header extension
>> +DATA 0x0 0x0
>> diff --git a/include/configs/km_arm.h b/include/configs/km_arm.h
>> new file mode 100644
>> index 0000000..091e941
>> --- /dev/null
>> +++ b/include/configs/km_arm.h
>> @@ -0,0 +1,191 @@
[...]
>> + * I2C related stuff
>> + */
>> +#undef CONFIG_HARD_I2C /* I2C with hardware support */
>> +#define CONFIG_SOFT_I2C /* I2C bit-banged */
>> +
>> +#if defined(CONFIG_HARD_I2C)
>> +#define CONFIG_I2C_KIRKWOOD
>> +#define CONFIG_I2C_KW_REG_BASE KW_TWSI_BASE
>> +#define CONFIG_SYS_I2C_SLAVE 0x0
>> +#define CONFIG_SYS_I2C_SPEED 100000
>> +#endif
>> +
>> +#if defined(CONFIG_SOFT_I2C)
>> +#define CONFIG_KIRKWOOD_GPIO /* Enable GPIO
>> Support */
>> +#ifndef __ASSEMBLY__
>> +#include <asm/arch-kirkwood/gpio.h>
>> +extern void __set_direction(unsigned pin, int high);
>> +void set_sda (int state);
>> +void set_scl (int state);
>> +int get_sda (void);
>> +int get_scl (void);
>> +#define SUEN3_SDA_PIN 8
>> +#define SUEN3_SCL_PIN 9
>> +#define SUEN3_ENV_WP 38
>
> Not valid for this board, this patch should explain only about mgcoge2
No, they are valid, because it uses the i2c bitbang driver...
(This file is used by *all* 4 boards ...)
I know the board name mgcoge2_arm_p1a is a little bit
confusing, but this is because this was the first hardware
version of the "suen3" board, which is a part of a hardware,
where also is a powerpc piece named "mgcoge2", and so
the first version was named to mgcoge2_arm_p1a ... the
next hardware revison was named "suen3" ...
>> +
>> +#define I2C_ACTIVE __set_direction(SUEN3_SDA_PIN, 0)
>> +#define I2C_TRISTATE __set_direction(SUEN3_SDA_PIN, 1)
>> +#define I2C_READ (kw_gpio_get_value(SUEN3_SDA_PIN) ? 1 : 0)
>> +#define I2C_SDA(bit) kw_gpio_set_value(SUEN3_SDA_PIN, bit);
>> +#define I2C_SCL(bit) kw_gpio_set_value(SUEN3_SCL_PIN, bit);
>> +#endif
>> +
>> +#define I2C_DELAY udelay(3) /* 1/4 I2C clock duration */
>> +#define I2C_SOFT_DECLARATIONS
>> +
>> +#define CONFIG_SYS_I2C_SLAVE 0x0
>> +#define CONFIG_SYS_I2C_SPEED 100000
>> +#endif
>> +
>> +#define CONFIG_SYS_I2C_EEPROM_ADDR 0x50
>> +#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
>> +
>> +#if defined(CONFIG_SYS_NO_FLASH)
>> +#define CONFIG_KM_UBI_PARTITION_NAME "ubi0"
>> +#undef CONFIG_FLASH_CFI_MTD
>> +#undef CONFIG_JFFS2_CMDLINE
>> +#endif
>> +
>> +#endif /* _CONFIG_KM_ARM_H */
>> diff --git a/include/configs/mgcoge2_arm_p1a.h
>> b/include/configs/mgcoge2_arm_p1a.h
>> new file mode 100644
>> index 0000000..db38915
>> --- /dev/null
>> +++ b/include/configs/mgcoge2_arm_p1a.h
>> @@ -0,0 +1,96 @@
[...]
>> + ""
>> +
>> +#endif /* _CONFIG_SUEN3_H */
>
> Wrong comment...
Yep, correct it.
Thanks for reviewing!
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
