Thanks for replaying, I did have a long wait. A question about board version. Is it good to do a hardware check to find out hardware version? Like function static void __init omap3_evm_get_revision(void) in board-omap3evm.c Best regards, Jason > -----Original Message----- > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > Sent: 2010年5月5日 7:00 > To: Jason > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] Add support for OMAP3Stalker boards > > Hi, > > Sorry for the delay in replying. Few comments, see below. > > * Jason <lzg@xxxxxxxxxxxx> [100303 06:16]: > > This patche add omap3 board support for the EMA-Tech StalkerBoards. Base > on > > TI's EVM. > > <snip> > > > --- a/arch/arm/mach-omap2/Kconfig > > +++ b/arch/arm/mach-omap2/Kconfig > > @@ -82,6 +82,26 @@ config MACH_OMAP3517EVM > > depends on ARCH_OMAP3 > > select OMAP_PACKAGE_CBB > > > > +config MACH_SBC3530 > > + bool "OMAP3 SBC STALKER board" > > + depends on ARCH_OMAP3 > > + > > +choice > > + prompt STALKER_BOARD_TYPE > > + depends on MACH_SBC3530 > > + default STALKER_EVM > > + > > +config STALKER_BOARD_TYPE_EVM > > + bool "Stalker EVM board" > > + help > > + Select this option if you have a Stalker EVM board > > + > > +config STALKER_BOARD_TYPE_LK_S > > + bool "Stalker LK-S board" > > + help > > + Select this option if you have a Stalker LK-S board > > +endchoice > > + > > config MACH_OMAP3_PANDORA > > bool "OMAP3 Pandora" > > depends on ARCH_OMAP3 > > Things like this are best done by looking at the ATAG_REVISION passed by > the bootloader. Then you can dynamically initialize the platform data > based on the revision. > > > --- /dev/null > > +++ b/arch/arm/mach-omap2/board-omap3stalker.c > > @@ -0,0 +1,922 @@ > > +/* > > + * linux/arch/arm/mach-omap2/board-omap3evm.c > > + * > > + * Copyright (C) 2008 Guangzhou EMA-Tech > > + * > > + * Modified from mach-omap2/board-omap3evm.c > > + * > > + * Initial code: Syed Mohammed Khasim > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/init.h> > > +#include <linux/platform_device.h> > > +#include <linux/delay.h> > > +#include <linux/err.h> > > +#include <linux/clk.h> > > +#include <linux/gpio.h> > > +#include <linux/input.h> > > +#include <linux/input/matrix_keypad.h> > > +#include <linux/leds.h> > > +#include <linux/interrupt.h> > > + > > +#include <linux/spi/spi.h> > > +#include <linux/spi/ads7846.h> > > +#include <linux/i2c/twl.h> > > +#include <linux/usb/otg.h> > > +#if defined(CONFIG_STALKER_BOARD_TYPE_EVM) > > +#include <linux/dm9000.h> > > +#elif defined(CONFIG_STALKER_BOARD_TYPE_LK_S) > > We should not ifdef includes, it should be OK to include them automatically. > > > +#include <linux/smsc911x.h> > > +#include <linux/gpio_keys.h> > > +#include <linux/i2c/at24.h> > > +#endif > > + > > +#include <linux/regulator/machine.h> > > + > > +#include <linux/mtd/mtd.h> > > +#include <linux/mtd/partitions.h> > > +#include <linux/mtd/nand.h> > > + > > +#include <mach/hardware.h> > > +#include <asm/mach-types.h> > > +#include <asm/mach/arch.h> > > +#include <asm/mach/map.h> > > +#include <asm/mach/flash.h> > > + > > +#include <plat/board.h> > > +#include <plat/mux.h> > > +#include <plat/gpmc.h> > > +#include <plat/nand.h> > > +#include <plat/usb.h> > > +#include <plat/common.h> > > +#include <plat/control.h> > > +#include <plat/mcspi.h> > > +#include <plat/clock.h> > > +#include <plat/omap-pm.h> > > +#include <plat/display.h> > > +#include <plat/timer-gp.h> > > + > > +#include "mux.h" > > +#include "sdram-micron-mt46h32m32lf-6.h" > > +#include "hsmmc.h" > > +#include "pm.h" > > That's a lot of include files, are they all needed? > > > +static struct mtd_partition omap3stalker_nand_partitions[] = { > > + /* All the partition sizes are listed in terms of NAND block size */ > > + { > > + .name = "X-Loader", > > + .offset = 0, > > + .size = 4 * (SZ_128K), > > + .mask_flags = MTD_WRITEABLE, > > + }, > > + { > > + .name = "U-Boot", > > + .offset = MTDPART_OFS_APPEND, > > + .size = 15 * (SZ_128K), > > + .mask_flags = MTD_WRITEABLE, > > + }, > > + { > > + .name = "U-Boot Env", > > + .offset = MTDPART_OFS_APPEND, > > + .size = 1 * (SZ_128K)}, > > + { > > + .name = "Kernel", > > + .offset = MTDPART_OFS_APPEND, > > + .size = 32 * (SZ_128K)}, > > + { > > + .name = "File System", > > + .size = MTDPART_SIZ_FULL, > > + .offset = MTDPART_OFS_APPEND, > > + }, > > +}; > > Please use tabs instead of spaces. > > > +void __init omap3stalker_flash_init(void) > > +{ > > + u8 cs = 0; > > + u8 nandcs = GPMC_CS_NUM + 1; > > + u32 gpmc_base_add = OMAP34XX_GPMC_VIRT; > > Please leave out the NAND stuff for now, we need to first fix the > NAND driver before adding more of the OMAP34XX_GPMC_VIRT.. > > > +static struct resource omap3stalker_smsc911x_resources[] = { > > + [0] = { > > + .start = OMAP3STALKER_ETHR_START, > > + .end = > > + (OMAP3STALKER_ETHR_START + OMAP3STALKER_ETHR_SIZE - > 1), > > + .flags = IORESOURCE_MEM, > > + }, > > + [1] = { > > + .start = OMAP_GPIO_IRQ(OMAP3STALKER_ETHR_GPIO_IRQ), > > + .end = OMAP_GPIO_IRQ(OMAP3STALKER_ETHR_GPIO_IRQ), > > + .flags = (IORESOURCE_IRQ | IRQF_TRIGGER_LOW), > > + }, > > +}; > > Please use tabs here too instead of spaces. > > > +static struct omap2_hsmmc_info mmc[] = { > > + { > > + .mmc = 1, > > + .wires = 4, > > + .gpio_cd = -EINVAL, > > + .gpio_wp = 23, > > + }, > > + {} /* Terminator */ > > +}; > > + > > +#if defined(CONFIG_STALKER_BOARD_TYPE_LK_S) > > +static struct gpio_keys_button gpio_buttons[] = { > > + { > > + .code = BTN_EXTRA, > > + .gpio = 18, > > + .desc = "user", > > + .wakeup = 1, > > + }, > > +}; > > Here too. > > > + .irq_base = TWL4030_IRQ_BASE, > > + .irq_end = TWL4030_IRQ_END, > > + > > + /* platform_data for children goes here */ > > + .keypad = &omap3stalker_kp_data, > > + .madc = &omap3stalker_madc_data, > > + .usb = &omap3stalker_usb_data, > > + .gpio = &omap3stalker_gpio_data, > > + .codec = &omap3stalker_codec_data, > > + .vdac = &omap3_stalker_vdac, > > + .vpll2 = &omap3_stalker_vpll2, > > +}; > > Please align with tabs: > > .keypad = &omap3stalker_kp_data, > .madc = &omap3stalker_madc_data, > > > +static struct i2c_board_info __initdata omap3stalker_i2c_boardinfo[] = { > > + { > > + I2C_BOARD_INFO("twl4030", 0x48), > > + .flags = I2C_CLIENT_WAKE, > > + .irq = INT_34XX_SYS_NIRQ, > > + .platform_data = &omap3stalker_twldata, > > + }, > > +}; > > Tabify > > > +#ifdef CONFIG_STALKER_BOARD_TYPE_LK_S > > +static struct at24_platform_data fram_info = { > > + .byte_len = (64 * 1024) / 8, > > + .page_size = 8192, > > + .flags = AT24_FLAG_ADDR16 | AT24_FLAG_IRUGO, > > +}; > > + > > +static struct i2c_board_info __initdata omap3stalker_i2c_boardinfo3[] = { > > + { > > + I2C_BOARD_INFO("24c64", 0x50), > > + .flags = I2C_CLIENT_WAKE, > > + .platform_data = &fram_info, > > + }, > > +}; > > +#endif > > Tabify > > Anyways, mostly minor stuff except for the preference for dynamic detection > of the board. > > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
