2010-12-07 14:49:52

by Igor Plyatov

[permalink] [raw]
Subject: [PATCH v2] mach-at91: Support for gms board added

* The gms is a board from GeoSIG Ltd company.
It is based on the Stamp9G20 module from Taskit company.
* This is a second version of the patch with adjustments according
to comments from Ryan Mallon.
* This patch made for Linux 2.6.37-rc5.

Signed-off-by: Igor Plyatov <[email protected]>
---
arch/arm/configs/stamp9g20gms_defconfig | 147 +++++++
arch/arm/mach-at91/Kconfig | 6 +
arch/arm/mach-at91/Makefile | 1 +
arch/arm/mach-at91/board-stamp9g20gms.c | 698 +++++++++++++++++++++++++++++++
arch/arm/mach-at91/include/mach/gms.h | 33 ++
5 files changed, 885 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/configs/stamp9g20gms_defconfig
create mode 100644 arch/arm/mach-at91/board-stamp9g20gms.c
create mode 100644 arch/arm/mach-at91/include/mach/gms.h

diff --git a/arch/arm/configs/stamp9g20gms_defconfig b/arch/arm/configs/stamp9g20gms_defconfig
new file mode 100644
index 0000000..c44057f
--- /dev/null
+++ b/arch/arm/configs/stamp9g20gms_defconfig
@@ -0,0 +1,147 @@
+CONFIG_EXPERIMENTAL=y
+CONFIG_SYSVIPC=y
+CONFIG_LOG_BUF_SHIFT=14
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_SLAB=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_ARCH_AT91=y
+CONFIG_ARCH_AT91SAM9G20=y
+CONFIG_MACH_STAMP9G20GMS=y
+CONFIG_AT91_PROGRAMMABLE_CLOCKS=y
+CONFIG_AT91_SLOW_CLOCK=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_PREEMPT=y
+CONFIG_AEABI=y
+CONFIG_ZBOOT_ROM_TEXT=0x0
+CONFIG_ZBOOT_ROM_BSS=0x0
+CONFIG_CMDLINE="mem=64M console=ttyS0,115200 initrd=0x21100000,3145728 root=/dev/ram0 rw"
+CONFIG_KEXEC=y
+CONFIG_CPU_IDLE=y
+CONFIG_PM=y
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_INET=y
+CONFIG_NETFILTER=y
+CONFIG_NETFILTER_NETLINK_LOG=m
+CONFIG_NF_CONNTRACK=m
+CONFIG_NF_CT_PROTO_SCTP=m
+CONFIG_NF_CT_PROTO_UDPLITE=m
+CONFIG_NF_CONNTRACK_FTP=m
+CONFIG_NF_CONNTRACK_IRC=m
+CONFIG_NF_CONNTRACK_SIP=m
+CONFIG_NF_CT_NETLINK=m
+CONFIG_NETFILTER_XT_MARK=m
+CONFIG_NETFILTER_XT_CONNMARK=m
+CONFIG_NETFILTER_XT_MATCH_STATE=m
+CONFIG_NF_CONNTRACK_IPV4=m
+CONFIG_IP_NF_IPTABLES=m
+CONFIG_IP_NF_MATCH_ADDRTYPE=m
+CONFIG_IP_NF_MATCH_AH=m
+CONFIG_IP_NF_MATCH_ECN=m
+CONFIG_IP_NF_MATCH_TTL=m
+CONFIG_IP_NF_FILTER=m
+CONFIG_IP_NF_TARGET_REJECT=m
+CONFIG_IP_NF_TARGET_LOG=m
+CONFIG_IP_NF_TARGET_ULOG=m
+CONFIG_NF_NAT=m
+CONFIG_IP_NF_TARGET_MASQUERADE=m
+CONFIG_IP_NF_TARGET_NETMAP=m
+CONFIG_IP_NF_TARGET_REDIRECT=m
+CONFIG_NF_NAT_SNMP_BASIC=m
+CONFIG_IP_NF_MANGLE=m
+CONFIG_IP_NF_TARGET_CLUSTERIP=m
+CONFIG_IP_NF_TARGET_ECN=m
+CONFIG_IP_NF_TARGET_TTL=m
+CONFIG_IP_NF_RAW=m
+CONFIG_IP_NF_ARPTABLES=m
+CONFIG_IP_NF_ARPFILTER=m
+CONFIG_IP_NF_ARP_MANGLE=m
+CONFIG_CFG80211=y
+CONFIG_LIB80211=y
+CONFIG_MAC80211=y
+CONFIG_MAC80211_LEDS=y
+CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
+CONFIG_MTD=y
+CONFIG_MTD_CONCAT=y
+CONFIG_MTD_PARTITIONS=y
+CONFIG_MTD_CMDLINE_PARTS=y
+CONFIG_MTD_CHAR=y
+CONFIG_MTD_BLOCK=y
+CONFIG_MTD_DATAFLASH=y
+CONFIG_MTD_NAND=y
+CONFIG_MTD_NAND_ATMEL=y
+CONFIG_MTD_UBI=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_RAM=y
+CONFIG_BLK_DEV_RAM_SIZE=8192
+CONFIG_MISC_DEVICES=y
+CONFIG_EEPROM_AT24=y
+CONFIG_BLK_DEV_SD=y
+CONFIG_SCSI_MULTI_LUN=y
+CONFIG_ATA=y
+CONFIG_PATA_AT91=y
+CONFIG_NETDEVICES=y
+CONFIG_NET_ETHERNET=y
+CONFIG_MACB=y
+CONFIG_RTL8187=m
+CONFIG_RT2X00=y
+CONFIG_RT2500USB=y
+CONFIG_RT73USB=y
+CONFIG_RT2800USB=y
+CONFIG_PPP=y
+CONFIG_PPP_ASYNC=y
+CONFIG_PPP_DEFLATE=y
+CONFIG_PPP_BSDCOMP=y
+CONFIG_PPP_MPPE=y
+CONFIG_INPUT_MOUSEDEV_SCREEN_X=320
+CONFIG_INPUT_MOUSEDEV_SCREEN_Y=240
+CONFIG_INPUT_EVDEV=y
+CONFIG_KEYBOARD_GPIO=y
+CONFIG_SERIAL_ATMEL=y
+CONFIG_SERIAL_ATMEL_CONSOLE=y
+CONFIG_I2C=y
+CONFIG_I2C_CHARDEV=y
+CONFIG_I2C_GPIO=y
+CONFIG_SPI=y
+CONFIG_SPI_ATMEL=y
+CONFIG_SPI_SPIDEV=y
+CONFIG_GPIO_SYSFS=y
+CONFIG_GPIO_PCF857X=y
+CONFIG_W1=y
+CONFIG_W1_MASTER_GPIO=y
+CONFIG_W1_SLAVE_DS2431=y
+CONFIG_USB=y
+CONFIG_USB_DEVICEFS=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_ACM=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_ETH=y
+CONFIG_MMC=y
+CONFIG_MMC_ATMELMCI=y
+CONFIG_NEW_LEDS=y
+CONFIG_LEDS_CLASS=y
+CONFIG_LEDS_GPIO=y
+CONFIG_LEDS_TRIGGER_TIMER=y
+CONFIG_LEDS_TRIGGER_HEARTBEAT=y
+CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_AT91SAM9=y
+CONFIG_EXT2_FS=y
+CONFIG_EXT3_FS=y
+CONFIG_EXT4_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_TMPFS=y
+CONFIG_JFFS2_FS=y
+CONFIG_JFFS2_SUMMARY=y
+CONFIG_UBIFS_FS=y
+CONFIG_NFS_FS=y
+CONFIG_NFS_V3=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_CODEPAGE_850=y
+CONFIG_NLS_ISO8859_1=y
+CONFIG_NLS_ISO8859_15=y
+CONFIG_NLS_UTF8=y
diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index c015b68..6bc9372 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -375,6 +375,12 @@ config MACH_STAMP9G20
evaluation board.
<http://www.taskit.de/en/>

+config MACH_STAMP9G20GMS
+ bool "GeoSIG Stamp9G20 GMS"
+ help
+ Select this if you are using taskit's Stamp9G20 with GeoSIG's GMS.
+ <http://www.geosig.com>
+
config MACH_PCONTROL_G20
bool "PControl G20 CPU module"
help
diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index 62d686f..6eeeba7 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_MACH_AT91SAM9RLEK) += board-sam9rlek.o
obj-$(CONFIG_MACH_AT91SAM9G20EK) += board-sam9g20ek.o
obj-$(CONFIG_MACH_CPU9G20) += board-cpu9krea.o
obj-$(CONFIG_MACH_STAMP9G20) += board-stamp9g20.o
+obj-$(CONFIG_MACH_STAMP9G20GMS) += board-stamp9g20gms.o
obj-$(CONFIG_MACH_PORTUXG20) += board-stamp9g20.o
obj-$(CONFIG_MACH_PCONTROL_G20) += board-pcontrol-g20.o

diff --git a/arch/arm/mach-at91/board-stamp9g20gms.c b/arch/arm/mach-at91/board-stamp9g20gms.c
new file mode 100644
index 0000000..df5a591
--- /dev/null
+++ b/arch/arm/mach-at91/board-stamp9g20gms.c
@@ -0,0 +1,698 @@
+/*
+ * Copyright (C) 2010 Christian Glindkamp <[email protected]>
+ * taskit GmbH
+ * 2010 Igor Plyatov <[email protected]>
+ * GeoSIG Ltd
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/w1-gpio.h>
+#include <linux/i2c/pcf857x.h>
+#include <linux/gpio_keys.h>
+#include <linux/input.h>
+
+#include <asm/mach-types.h>
+#include <asm/mach/arch.h>
+
+#include <mach/board.h>
+#include <mach/at91sam9_smc.h>
+#include <mach/gms.h>
+
+#include "sam9_smc.h"
+#include "generic.h"
+
+static void __init stamp9g20gms_map_io(void)
+{
+ /* Initialize processor: 18.432 MHz crystal */
+ at91sam9260_initialize(18432000);
+
+ /* DGBU on ttyS0. (Rx & Tx only) */
+ at91_register_uart(0, 0, 0);
+
+ /*
+ * USART0 on ttyS1 (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI).
+ * Used for Internal Analog Modem.
+ */
+ at91_register_uart(AT91SAM9260_ID_US0, 1,
+ ATMEL_UART_CTS | ATMEL_UART_RTS
+ | ATMEL_UART_DTR | ATMEL_UART_DSR
+ | ATMEL_UART_DCD | ATMEL_UART_RI);
+ /*
+ * USART1 on ttyS2 (Rx, Tx, CTS, RTS).
+ * Used for GPS or WiFi or Data stream.
+ */
+ at91_register_uart(AT91SAM9260_ID_US1, 2,
+ ATMEL_UART_CTS | ATMEL_UART_RTS);
+ /*
+ * USART2 on ttyS3 (Rx, Tx, CTS, RTS).
+ * Used for External Modem.
+ */
+ at91_register_uart(AT91SAM9260_ID_US2, 3,
+ ATMEL_UART_CTS | ATMEL_UART_RTS);
+ /*
+ * USART3 on ttyS4 (Rx, Tx, RTS).
+ * Used for RS-485.
+ */
+ at91_register_uart(AT91SAM9260_ID_US3, 4, ATMEL_UART_RTS);
+
+ /*
+ * USART4 on ttyS5 (Rx, Tx).
+ * Used for TRX433 Radio Module.
+ */
+ at91_register_uart(AT91SAM9260_ID_US4, 5, 0);
+
+ /* set serial console to ttyS0 (ie, DBGU) */
+ at91_set_serial_console(0);
+}
+
+static void __init init_irq(void)
+{
+ at91sam9260_init_interrupts(NULL);
+}
+
+/*
+ * NAND flash
+ */
+static struct atmel_nand_data __initdata nand_data = {
+ .ale = 21,
+ .cle = 22,
+ .rdy_pin = AT91_PIN_PC13,
+ .enable_pin = AT91_PIN_PC14,
+ .bus_width_16 = 0,
+};
+
+static struct sam9_smc_config __initdata nand_smc_config = {
+ .ncs_read_setup = 0,
+ .nrd_setup = 2,
+ .ncs_write_setup = 0,
+ .nwe_setup = 2,
+
+ .ncs_read_pulse = 4,
+ .nrd_pulse = 4,
+ .ncs_write_pulse = 4,
+ .nwe_pulse = 4,
+
+ .read_cycle = 7,
+ .write_cycle = 7,
+
+ .mode = AT91_SMC_READMODE | \
+ AT91_SMC_WRITEMODE | \
+ AT91_SMC_EXNWMODE_DISABLE | \
+ AT91_SMC_DBW_8,
+ .tdf_cycles = 3,
+};
+
+static void __init add_device_nand(void)
+{
+ /* configure chip-select 3 (NAND) */
+ sam9_smc_configure(3, &nand_smc_config);
+
+ at91_add_device_nand(&nand_data);
+}
+
+/*
+ * MCI (SD/MMC)
+ * det_pin, wp_pin and vcc_pin are not connected
+ */
+#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
+static struct mci_platform_data __initdata mmc_data = {
+ .slot[0] = {
+ .bus_width = 4,
+ },
+};
+#else
+static struct at91_mmc_data __initdata mmc_data = {
+ .slot_b = 0,
+ .wire4 = 1,
+};
+#endif
+
+/*
+ * Two USB Host ports
+ */
+static struct at91_usbh_data __initdata usbh_data = {
+ .ports = 2,
+};
+
+/*
+ * USB Device port
+ */
+static struct at91_udc_data __initdata stamp9g20gms_udc_data = {
+ .vbus_pin = AT91_PIN_PA22,
+ .pullup_pin = 0, /* pull-up driven by UDC */
+};
+
+/*
+ * MACB Ethernet device
+ */
+static struct at91_eth_data __initdata macb_data = {
+ .phy_irq_pin = AT91_PIN_PA28,
+ .is_rmii = 1,
+};
+
+/*
+ * LEDs and GPOs
+ */
+#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpio_led stamp9g20gms_gpio_leds[] = {
+ {
+ .name = "gpo:spi1reset",
+ .gpio = AT91_PIN_PC1,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "gpo:trig_net_out",
+ .gpio = AT91_PIN_PB20,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "gpo:trig_net_dir",
+ .gpio = AT91_PIN_PB19,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "gpo:charge_dis",
+ .gpio = AT91_PIN_PC2,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "led:event",
+ .gpio = AT91_PIN_PB17,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "led:lan",
+ .gpio = AT91_PIN_PB18,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ {
+ .name = "led:error",
+ .gpio = AT91_PIN_PB16,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_ON,
+ }
+};
+
+static struct gpio_led_platform_data stamp9g20gms_gpio_led_info = {
+ .leds = stamp9g20gms_gpio_leds,
+ .num_leds = ARRAY_SIZE(stamp9g20gms_gpio_leds),
+};
+
+static struct platform_device stamp9g20gms_leds = {
+ .name = "leds-gpio",
+ .id = 0,
+ .dev = {
+ .platform_data = &stamp9g20gms_gpio_led_info,
+ }
+};
+
+static void __init stamp9g20gms_leds_init(void)
+{
+ platform_device_register(&stamp9g20gms_leds);
+}
+#else
+static inline void stamp9g20gms_leds_init(void) {}
+#endif
+
+/* PCF8574 0x20 GPIO - U1 on the GS_IA18-CB_V3 board */
+#if (defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)) && \
+ (defined(CONFIG_GPIO_PCF857X) || defined(CONFIG_GPIO_PCF857X_MODULE))
+static struct gpio_led stamp9g20gms_pcf_gpio_leds1[] = {
+ { /* bit 0 */
+ .name = "gpo:hdc_power",
+ .gpio = PCF_GPIO_HDC_POWER,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 1 */
+ .name = "gpo:wifi_setup",
+ .gpio = PCF_GPIO_WIFI_SETUP,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 2 */
+ .name = "gpo:wifi_enable",
+ .gpio = PCF_GPIO_WIFI_ENABLE,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 3 */
+ .name = "gpo:wifi_reset",
+ .gpio = PCF_GPIO_WIFI_RESET,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_ON,
+ },
+ /* bit 4 used as GPI */
+ { /* bit 5 */
+ .name = "gpo:gps_setup",
+ .gpio = PCF_GPIO_GPS_SETUP,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 6 */
+ .name = "gpo:gps_standby",
+ .gpio = PCF_GPIO_GPS_STANDBY,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_ON,
+ },
+ { /* bit 7 */
+ .name = "gpo:gps_power",
+ .gpio = PCF_GPIO_GPS_POWER,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ }
+};
+
+static struct gpio_led_platform_data stamp9g20gms_pcf_gpio_led_info1 = {
+ .leds = stamp9g20gms_pcf_gpio_leds1,
+ .num_leds = ARRAY_SIZE(stamp9g20gms_pcf_gpio_leds1),
+};
+
+static struct platform_device stamp9g20gms_pcf_leds1 = {
+ .name = "leds-gpio", /* GS_IA18-CB_board */
+ .id = 1,
+ .dev = {
+ .platform_data = &stamp9g20gms_pcf_gpio_led_info1,
+ }
+};
+
+/* PCF8574 0x22 GPIO - U1 on the GS_2G_OPT1-A_V0 board (Alarm) */
+static struct gpio_led stamp9g20gms_pcf_gpio_leds2[] = {
+ { /* bit 0 */
+ .name = "gpo:alarm_1",
+ .gpio = PCF_GPIO_ALARM1,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 1 */
+ .name = "gpo:alarm_2",
+ .gpio = PCF_GPIO_ALARM2,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 2 */
+ .name = "gpo:alarm_3",
+ .gpio = PCF_GPIO_ALARM3,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ { /* bit 3 */
+ .name = "gpo:alarm_4",
+ .gpio = PCF_GPIO_ALARM4,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ /* bits 4, 5, 6 not used */
+ { /* bit 7 */
+ .name = "gpo:alarm_v_relay_on",
+ .gpio = PCF_GPIO_ALARM_V_RELAY_ON,
+ .active_low = 0,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+};
+
+static struct gpio_led_platform_data stamp9g20gms_pcf_gpio_led_info2 = {
+ .leds = stamp9g20gms_pcf_gpio_leds2,
+ .num_leds = ARRAY_SIZE(stamp9g20gms_pcf_gpio_leds2),
+};
+
+static struct platform_device stamp9g20gms_pcf_leds2 = {
+ .name = "leds-gpio",
+ .id = 2,
+ .dev = {
+ .platform_data = &stamp9g20gms_pcf_gpio_led_info2,
+ }
+};
+
+/* PCF8574 0x24 GPIO U1 on the GS_2G-OPT23-A_V0 board (Modem) */
+static struct gpio_led stamp9g20gms_pcf_gpio_leds3[] = {
+ { /* bit 0 */
+ .name = "gpo:modem_power",
+ .gpio = PCF_GPIO_MODEM_POWER,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_OFF,
+ },
+ /* bits 1 and 2 not used */
+ { /* bit 3 */
+ .name = "gpo:modem_reset",
+ .gpio = PCF_GPIO_MODEM_RESET,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_ON,
+ },
+ /* bits 4, 5 and 6 not used */
+ { /* bit 7 */
+ .name = "gpo:trx_reset",
+ .gpio = PCF_GPIO_TRX_RESET,
+ .active_low = 1,
+ .default_trigger = "none",
+ .default_state = LEDS_GPIO_DEFSTATE_ON,
+ }
+};
+
+static struct gpio_led_platform_data stamp9g20gms_pcf_gpio_led_info3 = {
+ .leds = stamp9g20gms_pcf_gpio_leds3,
+ .num_leds = ARRAY_SIZE(stamp9g20gms_pcf_gpio_leds3),
+};
+
+static struct platform_device stamp9g20gms_pcf_leds3 = {
+ .name = "leds-gpio",
+ .id = 3,
+ .dev = {
+ .platform_data = &stamp9g20gms_pcf_gpio_led_info3,
+ }
+};
+
+static void __init stamp9g20gms_pcf_leds_init(void)
+{
+ platform_device_register(&stamp9g20gms_pcf_leds1);
+ platform_device_register(&stamp9g20gms_pcf_leds2);
+ platform_device_register(&stamp9g20gms_pcf_leds3);
+}
+#else
+static inline void stamp9g20gms_pcf_leds_init(void) {}
+#endif /* CONFIG_LEDS_GPIO || CONFIG_GPIO_PCF857X */
+
+/*
+ * SPI busses.
+ */
+static struct spi_board_info stamp9g20gms_spi_devices[] = {
+ { /* User accessible spi0, cs0 used for communication with MSP RTC */
+ .modalias = "spidev",
+ .bus_num = 0,
+ .chip_select = 0,
+ .max_speed_hz = 580000,
+ .mode = SPI_MODE_1,
+ },
+ { /* User accessible spi1, cs0 used for communication with int. DSP */
+ .modalias = "spidev",
+ .bus_num = 1,
+ .chip_select = 0,
+ .max_speed_hz = 5600000,
+ .mode = SPI_MODE_0,
+ },
+ { /* User accessible spi1, cs1 used for communication with ext. DSP */
+ .modalias = "spidev",
+ .bus_num = 1,
+ .chip_select = 1,
+ .max_speed_hz = 5600000,
+ .mode = SPI_MODE_0,
+ },
+ { /* User accessible spi1, cs2 used for communication with ext. DSP */
+ .modalias = "spidev",
+ .bus_num = 1,
+ .chip_select = 2,
+ .max_speed_hz = 5600000,
+ .mode = SPI_MODE_0,
+ },
+ { /* User accessible spi1, cs3 used for communication with ext. DSP */
+ .modalias = "spidev",
+ .bus_num = 1,
+ .chip_select = 3,
+ .max_speed_hz = 5600000,
+ .mode = SPI_MODE_0,
+ }
+};
+
+/*
+ * Dallas 1-Wire
+ */
+static struct w1_gpio_platform_data w1_gpio_pdata = {
+ .pin = AT91_PIN_PA29,
+ .is_open_drain = 1,
+};
+
+static struct platform_device w1_device = {
+ .name = "w1-gpio",
+ .id = -1,
+ .dev.platform_data = &w1_gpio_pdata,
+};
+
+void add_w1(void)
+{
+ at91_set_GPIO_periph(w1_gpio_pdata.pin, 1);
+ at91_set_multi_drive(w1_gpio_pdata.pin, 1);
+ platform_device_register(&w1_device);
+}
+
+/*
+ * GPI Buttons
+ */
+#if defined(CONFIG_KEYBOARD_GPIO) || defined(CONFIG_KEYBOARD_GPIO_MODULE)
+static struct gpio_keys_button stamp9g20gms_buttons[] = {
+ {
+ .gpio = GPIO_TRIG_NET_IN,
+ .code = BTN_1,
+ .desc = "TRIG_NET_IN",
+ .type = EV_KEY,
+ .active_low = 0,
+ .wakeup = 1,
+ },
+ { /* SW80 on the GS_IA18_S-MN board*/
+ .gpio = GPIO_CARD_UNMOUNT_0,
+ .code = BTN_2,
+ .desc = "Card umount 0",
+ .type = EV_KEY,
+ .active_low = 1,
+ .wakeup = 1,
+ },
+ { /* SW79 on the GS_IA18_S-MN board*/
+ .gpio = GPIO_CARD_UNMOUNT_1,
+ .code = BTN_3,
+ .desc = "Card umount 1",
+ .type = EV_KEY,
+ .active_low = 1,
+ .wakeup = 1,
+ },
+ { /* SW280 on the GS_IA18-CB board*/
+ .gpio = GPIO_KEY_POWER,
+ .code = KEY_POWER,
+ .desc = "Power Off Button",
+ .type = EV_KEY,
+ .active_low = 0,
+ .wakeup = 1,
+ }
+};
+
+static struct gpio_keys_platform_data stamp9g20gms_button_data = {
+ .buttons = stamp9g20gms_buttons,
+ .nbuttons = ARRAY_SIZE(stamp9g20gms_buttons),
+};
+
+static struct platform_device stamp9g20gms_button_device = {
+ .name = "gpio-keys",
+ .id = -1,
+ .num_resources = 0,
+ .dev = {
+ .platform_data = &stamp9g20gms_button_data,
+ }
+};
+
+static void __init stamp9g20gms_add_device_buttons(void)
+{
+ at91_set_gpio_input(GPIO_TRIG_NET_IN, 1);
+ at91_set_deglitch(GPIO_TRIG_NET_IN, 1);
+ at91_set_gpio_input(GPIO_CARD_UNMOUNT_0, 1);
+ at91_set_deglitch(GPIO_CARD_UNMOUNT_0, 1);
+ at91_set_gpio_input(GPIO_CARD_UNMOUNT_1, 1);
+ at91_set_deglitch(GPIO_CARD_UNMOUNT_1, 1);
+ at91_set_gpio_input(GPIO_KEY_POWER, 0);
+ at91_set_deglitch(GPIO_KEY_POWER, 1);
+
+ platform_device_register(&stamp9g20gms_button_device);
+}
+#else
+static void __init stamp9g20gms_add_device_buttons(void) {}
+#endif
+
+/*
+ * I2C
+ */
+static int pcf8574x_0x20_setup(struct i2c_client *client, int gpio,
+ unsigned int ngpio, void *context)
+{
+ int status;
+
+ status = gpio_request(gpio + PCF_GPIO_ETH_DETECT, "eth_det");
+ if (status < 0) {
+ pr_err("error: can't request GPIO%d\n",
+ gpio + PCF_GPIO_ETH_DETECT);
+ return status;
+ }
+ status = gpio_direction_input(gpio + PCF_GPIO_ETH_DETECT);
+ if (status < 0) {
+ pr_err("error: can't setup GPIO%d as input\n",
+ gpio + PCF_GPIO_ETH_DETECT);
+ return status;
+ }
+ status = gpio_export(gpio + PCF_GPIO_ETH_DETECT, false);
+ if (status < 0) {
+ pr_err("error: can't export GPIO%d\n",
+ gpio + PCF_GPIO_ETH_DETECT);
+ return status;
+ }
+ status = gpio_sysfs_set_active_low(gpio + PCF_GPIO_ETH_DETECT, 1);
+ if (status < 0) {
+ pr_err("error: gpio_sysfs_set active_low(GPIO%d, 1)\n",
+ gpio + PCF_GPIO_ETH_DETECT);
+ return status;
+ }
+
+ return 0;
+}
+
+static int pcf8574x_0x20_teardown(struct i2c_client *client, int gpio,
+ unsigned ngpio, void *context)
+{
+ gpio_free(gpio + 4);
+ return 0;
+}
+
+static struct pcf857x_platform_data stamp9g20_pcf20_pdata = {
+ .gpio_base = GS_IA18_S_PCF_GPIO_BASE0,
+ .n_latch = (1 << 4),
+ .setup = pcf8574x_0x20_setup,
+ .teardown = pcf8574x_0x20_teardown,
+};
+
+static struct pcf857x_platform_data stamp9g20_pcf22_pdata = {
+ .gpio_base = GS_IA18_S_PCF_GPIO_BASE1,
+};
+
+static struct pcf857x_platform_data stamp9g20_pcf24_pdata = {
+ .gpio_base = GS_IA18_S_PCF_GPIO_BASE2,
+};
+
+static struct i2c_board_info __initdata stamp9g20gms_i2c_devices[] = {
+ { /* U1 on the GS_IA18-CB_V3 board */
+ I2C_BOARD_INFO("pcf8574", 0x20),
+ .platform_data = &stamp9g20_pcf20_pdata,
+ },
+ { /* U1 on the GS_2G_OPT1-A_V0 board (Alarm) */
+ I2C_BOARD_INFO("pcf8574", 0x22),
+ .platform_data = &stamp9g20_pcf22_pdata,
+ },
+ { /* U1 on the GS_2G-OPT23-A_V0 board (Modem) */
+ I2C_BOARD_INFO("pcf8574", 0x24),
+ .platform_data = &stamp9g20_pcf24_pdata,
+ },
+ { /* U161 on the GS_IA18_S-MN board */
+ I2C_BOARD_INFO("24c1024", 0x50),
+ },
+ { /* U162 on the GS_IA18_S-MN board */
+ I2C_BOARD_INFO("24c01", 0x53),
+ },
+};
+
+/*
+ * Compact Flash
+ */
+#if defined(CONFIG_PATA_AT91) || defined(CONFIG_PATA_AT91_MODULE)
+static struct at91_cf_data __initdata stamp9g20gms_cf1_data = {
+ .irq_pin = AT91_PIN_PA27,
+ .det_pin = AT91_PIN_PB30,
+ .rst_pin = AT91_PIN_PB31,
+ .chipselect = 5,
+ .flags = AT91_CF_TRUE_IDE,
+};
+#endif /* CONFIG_PATA_AT91 */
+
+/* Power Off by RTC */
+static void stamp9g20gms_power_off(void)
+{
+ pr_notice("Power supply will be switched off automatically now or ");
+ pr_notice("after 60 seconds without ArmDAS.\n");
+ at91_set_gpio_output(AT91_PIN_PA25, 1);
+ /* Spin to death... */
+ while (1)
+ ;
+}
+
+static int __init stamp9g20gms_power_off_init(void)
+{
+ pm_power_off = stamp9g20gms_power_off;
+ return 0;
+}
+
+/* ---------------------------------------------------------------------------*/
+
+static void __init generic_board_init(void)
+{
+ at91_add_device_serial();
+ add_device_nand();
+#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
+ at91_add_device_mci(0, &mmc_data);
+#else
+ at91_add_device_mmc(0, &mmc_data);
+#endif /* CONFIG_MMC_ATMELMCI */
+ add_w1();
+}
+
+static void __init stamp9g20gms_board_init(void)
+{
+ generic_board_init();
+ at91_add_device_usbh(&usbh_data);
+ at91_add_device_udc(&stamp9g20gms_udc_data);
+ at91_add_device_eth(&macb_data);
+ stamp9g20gms_leds_init();
+ stamp9g20gms_pcf_leds_init();
+ stamp9g20gms_add_device_buttons();
+ at91_add_device_i2c(stamp9g20gms_i2c_devices,
+ ARRAY_SIZE(stamp9g20gms_i2c_devices));
+#if defined(CONFIG_PATA_AT91) || defined(CONFIG_PATA_AT91_MODULE)
+ at91_add_device_cf(&stamp9g20gms_cf1_data);
+#endif /* CONFIG_PATA_AT91 */
+ at91_add_device_spi(stamp9g20gms_spi_devices,
+ ARRAY_SIZE(stamp9g20gms_spi_devices));
+ stamp9g20gms_power_off_init();
+}
+
+MACHINE_START(STAMP9G20, "Stamp9G20 on the GeoSIG GS_IA18_S board")
+ /* Maintainer: GeoSIG Ltd */
+ .boot_params = AT91_SDRAM_BASE + 0x100,
+ .timer = &at91sam926x_timer,
+ .map_io = stamp9g20gms_map_io,
+ .init_irq = init_irq,
+ .init_machine = stamp9g20gms_board_init,
+MACHINE_END
diff --git a/arch/arm/mach-at91/include/mach/gms.h b/arch/arm/mach-at91/include/mach/gms.h
new file mode 100644
index 0000000..307c194
--- /dev/null
+++ b/arch/arm/mach-at91/include/mach/gms.h
@@ -0,0 +1,33 @@
+/* Buttons */
+#define GPIO_TRIG_NET_IN AT91_PIN_PB21
+#define GPIO_CARD_UNMOUNT_0 AT91_PIN_PB13
+#define GPIO_CARD_UNMOUNT_1 AT91_PIN_PB12
+#define GPIO_KEY_POWER AT91_PIN_PA25
+
+/* PCF8574 0x20 GPIO - U1 on the GS_IA18-CB_V3 board */
+#define GS_IA18_S_PCF_GPIO_BASE0 NR_BUILTIN_GPIO
+#define PCF_GPIO_HDC_POWER (GS_IA18_S_PCF_GPIO_BASE0 + 0)
+#define PCF_GPIO_WIFI_SETUP (GS_IA18_S_PCF_GPIO_BASE0 + 1)
+#define PCF_GPIO_WIFI_ENABLE (GS_IA18_S_PCF_GPIO_BASE0 + 2)
+#define PCF_GPIO_WIFI_RESET (GS_IA18_S_PCF_GPIO_BASE0 + 3)
+#define PCF_GPIO_ETH_DETECT 4 /* this is a GPI */
+#define PCF_GPIO_GPS_SETUP (GS_IA18_S_PCF_GPIO_BASE0 + 5)
+#define PCF_GPIO_GPS_STANDBY (GS_IA18_S_PCF_GPIO_BASE0 + 6)
+#define PCF_GPIO_GPS_POWER (GS_IA18_S_PCF_GPIO_BASE0 + 7)
+
+/* PCF8574 0x22 GPIO - U1 on the GS_2G_OPT1-A_V0 board (Alarm) */
+#define GS_IA18_S_PCF_GPIO_BASE1 (GS_IA18_S_PCF_GPIO_BASE0 + 8)
+#define PCF_GPIO_ALARM1 (GS_IA18_S_PCF_GPIO_BASE1 + 0)
+#define PCF_GPIO_ALARM2 (GS_IA18_S_PCF_GPIO_BASE1 + 1)
+#define PCF_GPIO_ALARM3 (GS_IA18_S_PCF_GPIO_BASE1 + 2)
+#define PCF_GPIO_ALARM4 (GS_IA18_S_PCF_GPIO_BASE1 + 3)
+/* bits 4, 5, 6 not used */
+#define PCF_GPIO_ALARM_V_RELAY_ON (GS_IA18_S_PCF_GPIO_BASE1 + 7)
+
+/* PCF8574 0x24 GPIO U1 on the GS_2G-OPT23-A_V0 board (Modem) */
+#define GS_IA18_S_PCF_GPIO_BASE2 (GS_IA18_S_PCF_GPIO_BASE1 + 8)
+#define PCF_GPIO_MODEM_POWER (GS_IA18_S_PCF_GPIO_BASE2 + 0)
+#define PCF_GPIO_MODEM_RESET (GS_IA18_S_PCF_GPIO_BASE2 + 3)
+/* bits 1, 2, 4, 5 not used */
+#define PCF_GPIO_TRX_RESET (GS_IA18_S_PCF_GPIO_BASE2 + 6)
+/* bit 7 not used */
--
1.7.0.4


2010-12-07 19:52:24

by Ryan Mallon

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

On 12/08/2010 03:42 AM, Igor Plyatov wrote:
> * The gms is a board from GeoSIG Ltd company.
> It is based on the Stamp9G20 module from Taskit company.
> * This is a second version of the patch with adjustments according
> to comments from Ryan Mallon.
> * This patch made for Linux 2.6.37-rc5.
>
> Signed-off-by: Igor Plyatov <[email protected]>
> ---

Couple more comments below.

> arch/arm/configs/stamp9g20gms_defconfig | 147 +++++++
> arch/arm/mach-at91/Kconfig | 6 +
> arch/arm/mach-at91/Makefile | 1 +
> arch/arm/mach-at91/board-stamp9g20gms.c | 698 +++++++++++++++++++++++++++++++
> arch/arm/mach-at91/include/mach/gms.h | 33 ++
> 5 files changed, 885 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/configs/stamp9g20gms_defconfig
> create mode 100644 arch/arm/mach-at91/board-stamp9g20gms.c
> create mode 100644 arch/arm/mach-at91/include/mach/gms.h
>
> diff --git a/arch/arm/configs/stamp9g20gms_defconfig b/arch/arm/configs/stamp9g20gms_defconfig
> new file mode 100644
> index 0000000..c44057f
> --- /dev/null
> +++ b/arch/arm/configs/stamp9g20gms_defconfig
> @@ -0,0 +1,147 @@
> +CONFIG_EXPERIMENTAL=y
> +CONFIG_SYSVIPC=y
> +CONFIG_LOG_BUF_SHIFT=14
> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_SLAB=y
> +CONFIG_MODULES=y
> +CONFIG_MODULE_UNLOAD=y
> +CONFIG_ARCH_AT91=y
> +CONFIG_ARCH_AT91SAM9G20=y
> +CONFIG_MACH_STAMP9G20GMS=y

This is better and now looks (at a glance) almost identical to
stamp9g20_defconfig. Can you just add CONFIG_MACH_STAMP9G20GMS to
stamp9g20_defconfig and support both boards?

> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> index c015b68..6bc9372 100644
> --- a/arch/arm/mach-at91/Kconfig
> +++ b/arch/arm/mach-at91/Kconfig
> @@ -375,6 +375,12 @@ config MACH_STAMP9G20
> evaluation board.
> <http://www.taskit.de/en/>
>
> +config MACH_STAMP9G20GMS
> + bool "GeoSIG Stamp9G20 GMS"
> + help
> + Select this if you are using taskit's Stamp9G20 with GeoSIG's GMS.
> + <http://www.geosig.com>
> +

Looking at this a bit more closely, the Stamp9G20 is a system on module
(SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
on the PControl carrier board. There is a reasonable amount of code
replication in each of the board files for the UARTs, NAND, MMC, etc.

Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
core support for the Stamp9G20 module and then each of the carrier board
files contain only the setup/devices found on the carrier board?

I don't know what the correct approach for SoMs. We are also interested
in this as we develop SoMs with carrier boards. Cc'ed Russell King for
his opinion.

<snip>

> +
> +static int pcf8574x_0x20_teardown(struct i2c_client *client, int gpio,
> + unsigned ngpio, void *context)
> +{
> + gpio_free(gpio + 4);

gpio_free(gpio + PCF_GPIO_ETH_DETECT)?

<snip>

> +
> +/*
> + * Compact Flash
> + */
> +#if defined(CONFIG_PATA_AT91) || defined(CONFIG_PATA_AT91_MODULE)
> +static struct at91_cf_data __initdata stamp9g20gms_cf1_data = {
> + .irq_pin = AT91_PIN_PA27,
> + .det_pin = AT91_PIN_PB30,
> + .rst_pin = AT91_PIN_PB31,
> + .chipselect = 5,
> + .flags = AT91_CF_TRUE_IDE,
> +};
> +#endif /* CONFIG_PATA_AT91 */

I still think you can do away with some of these ifdef guards.
Registering the device is not a problem, and you are only saving a
handful of bytes by having this ifdef. I think the code is more
readable/maintainable without all the ifdefs.

> +/* Power Off by RTC */
> +static void stamp9g20gms_power_off(void)
> +{
> + pr_notice("Power supply will be switched off automatically now or ");
> + pr_notice("after 60 seconds without ArmDAS.\n");

No. It should all be one call to pr_notice on one single line so that
the full message can easily be grepped. Lines over 80 columns are
allowed for printk functions and checkpatch will not warn about this.

> + at91_set_gpio_output(AT91_PIN_PA25, 1);
> + /* Spin to death... */
> + while (1)
> + ;
> +}

<snip>

~Ryan

--
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934

2010-12-08 09:13:41

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

Hi Igor,

Le 07/12/2010 20:53, Ryan Mallon :
> On 12/08/2010 03:42 AM, Igor Plyatov wrote:
>> * The gms is a board from GeoSIG Ltd company.
>> It is based on the Stamp9G20 module from Taskit company.
>> * This is a second version of the patch with adjustments according
>> to comments from Ryan Mallon.
>> * This patch made for Linux 2.6.37-rc5.

First thank you for submitting this board support.

>> Signed-off-by: Igor Plyatov <[email protected]>
>> ---

[..]

> Couple more comments below.
> Looking at this a bit more closely, the Stamp9G20 is a system on module
> (SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
> taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
> on the PControl carrier board. There is a reasonable amount of code
> replication in each of the board files for the UARTs, NAND, MMC, etc.
>
> Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
> core support for the Stamp9G20 module and then each of the carrier board
> files contain only the setup/devices found on the carrier board?

I have exactly the same feeling as Ryan. We should make sure
to factorize as much code as possible for maintenance reasons.

If you need to distinguish between board features, you can
pass information in system_rev as implemented in this
board merging commit:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a6e016f19d393fbe4e040bee8155b03b840fa689

Best regards,
--
Nicolas Ferre

Subject: Re: [PATCH v2] mach-at91: Support for gms board added

On 09:53 Wed 08 Dec , Nicolas Ferre wrote:
> Hi Igor,
>
> Le 07/12/2010 20:53, Ryan Mallon :
> > On 12/08/2010 03:42 AM, Igor Plyatov wrote:
> >> * The gms is a board from GeoSIG Ltd company.
> >> It is based on the Stamp9G20 module from Taskit company.
> >> * This is a second version of the patch with adjustments according
> >> to comments from Ryan Mallon.
> >> * This patch made for Linux 2.6.37-rc5.
>
> First thank you for submitting this board support.
>
> >> Signed-off-by: Igor Plyatov <[email protected]>
> >> ---
>
> [..]
>
> > Couple more comments below.
> > Looking at this a bit more closely, the Stamp9G20 is a system on module
> > (SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
> > taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
> > on the PControl carrier board. There is a reasonable amount of code
> > replication in each of the board files for the UARTs, NAND, MMC, etc.
> >
> > Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
> > core support for the Stamp9G20 module and then each of the carrier board
> > files contain only the setup/devices found on the carrier board?
>
> I have exactly the same feeling as Ryan. We should make sure
> to factorize as much code as possible for maintenance reasons.
>
> If you need to distinguish between board features, you can
> pass information in system_rev as implemented in this
> board merging commit:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a6e016f19d393fbe4e040bee8155b03b840fa689
I agree with nico not need to have a new board for just few difference

and as we start with the rm9200 we will reduce the number of defconfig first per soc and then for the all sam9

Best Regards,
J.

2010-12-08 14:51:57

by Christian Glindkamp

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

On 2010-12-08 09:53, Nicolas Ferre wrote:
> Hi Igor,
>
> Le 07/12/2010 20:53, Ryan Mallon :
> > On 12/08/2010 03:42 AM, Igor Plyatov wrote:
> >> * The gms is a board from GeoSIG Ltd company.
> >> It is based on the Stamp9G20 module from Taskit company.
> >> * This is a second version of the patch with adjustments according
> >> to comments from Ryan Mallon.
> >> * This patch made for Linux 2.6.37-rc5.
>
> First thank you for submitting this board support.
>
> >> Signed-off-by: Igor Plyatov <[email protected]>
> >> ---
>
> [..]
>
> > Couple more comments below.
> > Looking at this a bit more closely, the Stamp9G20 is a system on module
> > (SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
> > taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
> > on the PControl carrier board. There is a reasonable amount of code
> > replication in each of the board files for the UARTs, NAND, MMC, etc.
> >
> > Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
> > core support for the Stamp9G20 module and then each of the carrier board
> > files contain only the setup/devices found on the carrier board?
>
> I have exactly the same feeling as Ryan. We should make sure
> to factorize as much code as possible for maintenance reasons.
>
> If you need to distinguish between board features, you can
> pass information in system_rev as implemented in this
> board merging commit:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a6e016f19d393fbe4e040bee8155b03b840fa689
>

I don't think system_rev is such a good idea for carrier boards from
different vendors. Somebody would have to control the assigned numbers.

This is what I would do:
1. Refactor board-stamp9g20.c have three board init functions:
- portuxg20_board_init for PortuxG20 SBC (MACH_PORTUXG20)

- stamp9g20_board_init only containing the functions used on the
Stamp9G20 alone

- stamp9g20evb_board_init calling stamp9g20_board_init and adding
functionality of the evaluation board (MACH_STAMP9G20)

2. Modify board-pcontrol-g20.c to use stamp9g20_board_init

This would still duplicate the UART config (there is not much to share,
only the DBGU would be configured on all boards), but share NAND, MMC,
1-wire. Everything else can't be shared as it is carrier board specific.

The gms board would then have to have its own machine number and call
stamp9g20_board_init.

Kind regards,
Christian Glindkamp

2010-12-08 19:26:22

by Igor Plyatov

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

Dear Ryan,

Couple more answers below.

> > +++ b/arch/arm/configs/stamp9g20gms_defconfig
> > @@ -0,0 +1,147 @@
> > +CONFIG_EXPERIMENTAL=y
> > +CONFIG_SYSVIPC=y
> > +CONFIG_LOG_BUF_SHIFT=14
> > +CONFIG_BLK_DEV_INITRD=y
> > +CONFIG_SLAB=y
> > +CONFIG_MODULES=y
> > +CONFIG_MODULE_UNLOAD=y
> > +CONFIG_ARCH_AT91=y
> > +CONFIG_ARCH_AT91SAM9G20=y
> > +CONFIG_MACH_STAMP9G20GMS=y
>
> This is better and now looks (at a glance) almost identical to
> stamp9g20_defconfig. Can you just add CONFIG_MACH_STAMP9G20GMS to
> stamp9g20_defconfig and support both boards?

It is not possible to use the same defconfig for gms and other carrier
boards, because 69 different CONFIG_xxx options required for our machine
(compared with Portux G20) and our options excessive for other devices
based on the Stamp9G20.

> > diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> > index c015b68..6bc9372 100644
> > --- a/arch/arm/mach-at91/Kconfig
> > +++ b/arch/arm/mach-at91/Kconfig
> > @@ -375,6 +375,12 @@ config MACH_STAMP9G20
> > evaluation board.
> > <http://www.taskit.de/en/>
> >
> > +config MACH_STAMP9G20GMS
> > + bool "GeoSIG Stamp9G20 GMS"
> > + help
> > + Select this if you are using taskit's Stamp9G20 with GeoSIG's GMS.
> > + <http://www.geosig.com>
> > +
>
> Looking at this a bit more closely, the Stamp9G20 is a system on module
> (SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
> taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
> on the PControl carrier board. There is a reasonable amount of code
> replication in each of the board files for the UARTs, NAND, MMC, etc.
>
> Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
> core support for the Stamp9G20 module and then each of the carrier board
> files contain only the setup/devices found on the carrier board?
>
> I don't know what the correct approach for SoMs. We are also interested
> in this as we develop SoMs with carrier boards. Cc'ed Russell King for
> his opinion.

You are right about the Stamp9G20 SoM.
The Stamp9G20 SoM can not be used without a carrier board from physical
point of view. It need a power supply and console interface at least.

I will send a new PATCH v3, where code for the Stamp9G20 SoM separated
into the board-stamp9g20.c, and following files will correspond to
carrier boards for Stamp9G20:
* carrier-board-gms.c - "GMS", Seismograph from GeoSIG.
* carrier-board-panelcardevb.c - "Panel Card EVB", Taskit's eval. board.
* carrier-board-portuxg20.c - "Portux G20", derivative from Stamp9G20.
* carrier-board-pcontrol_g20.c - "Pcontrol G20", PORTNER-Elektronik.

All stuff specific to carrier board will be encapsulated into functions:
* carrier_board_map_io()
* carrier_board_init()

I does not see any reasons to invent new machine for each new carrier
board. Better to have a configuration option for carrier boards.

There is only one choice for Stamp9G20 carrier board from:
* CONFIG_CARRIERBOARD_GMS
* CONFIG_CARRIERBOARD_PANELCARDEVB
* CONFIG_CARRIERBOARD_PCONTROL_G20
* CONFIG_CARRIERBOARD_PORTUXG20

> > +static int pcf8574x_0x20_teardown(struct i2c_client *client, int gpio,
> > + unsigned ngpio, void *context)
> > +{
> > + gpio_free(gpio + 4);
>
> gpio_free(gpio + PCF_GPIO_ETH_DETECT)?

Corrected now.

> > +#if defined(CONFIG_PATA_AT91) || defined(CONFIG_PATA_AT91_MODULE)
> > +static struct at91_cf_data __initdata stamp9g20gms_cf1_data = {
> > + .irq_pin = AT91_PIN_PA27,
> > + .det_pin = AT91_PIN_PB30,
> > + .rst_pin = AT91_PIN_PB31,
> > + .chipselect = 5,
> > + .flags = AT91_CF_TRUE_IDE,
> > +};
> > +#endif /* CONFIG_PATA_AT91 */
>
> I still think you can do away with some of these ifdef guards.
> Registering the device is not a problem, and you are only saving a
> handful of bytes by having this ifdef. I think the code is more
> readable/maintainable without all the ifdefs.

Corrected now.

> > +/* Power Off by RTC */
> > +static void stamp9g20gms_power_off(void)
> > +{
> > + pr_notice("Power supply will be switched off automatically now or ");
> > + pr_notice("after 60 seconds without ArmDAS.\n");
>
> No. It should all be one call to pr_notice on one single line so that
> the full message can easily be grepped. Lines over 80 columns are
> allowed for printk functions and checkpatch will not warn about this.

Corrected now.

Best regards!
--
Igor Plyatov


2010-12-08 19:29:35

by Igor Plyatov

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

Dear Jean-Christophe,

> > I have exactly the same feeling as Ryan. We should make sure
> > to factorize as much code as possible for maintenance reasons.
> >
> > If you need to distinguish between board features, you can
> > pass information in system_rev as implemented in this
> > board merging commit:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a6e016f19d393fbe4e040bee8155b03b840fa689
> I agree with nico not need to have a new board for just few difference
>
> and as we start with the rm9200 we will reduce the number of defconfig first per soc and then for the all sam9

I think it does not make sence to invent new machine for each new
carrier board. Better to have a kernel configuration options which will
allow to select required carrier board.

Best regards!
--
Igor Plyatov

2010-12-08 20:06:45

by Ryan Mallon

[permalink] [raw]
Subject: Re: [PATCH v2] mach-at91: Support for gms board added

On 12/09/2010 03:50 AM, Christian Glindkamp wrote:
> On 2010-12-08 09:53, Nicolas Ferre wrote:
>> Hi Igor,
>>
>> Le 07/12/2010 20:53, Ryan Mallon :
>>> On 12/08/2010 03:42 AM, Igor Plyatov wrote:
>>>> * The gms is a board from GeoSIG Ltd company.
>>>> It is based on the Stamp9G20 module from Taskit company.
>>>> * This is a second version of the patch with adjustments according
>>>> to comments from Ryan Mallon.
>>>> * This patch made for Linux 2.6.37-rc5.
>>
>> First thank you for submitting this board support.
>>
>>>> Signed-off-by: Igor Plyatov <[email protected]>
>>>> ---
>>
>> [..]
>>
>>> Couple more comments below.
>>> Looking at this a bit more closely, the Stamp9G20 is a system on module
>>> (SoM) board. The MACH_STAMP9G20 option supports the Stamp9G20 on
>>> taskits's evaluation board and the MACH_PCONTROL_G20 option supports it
>>> on the PControl carrier board. There is a reasonable amount of code
>>> replication in each of the board files for the UARTs, NAND, MMC, etc.
>>>
>>> Would it be better to have MACH_STAMPG20/board-stamp-9g20.c contain the
>>> core support for the Stamp9G20 module and then each of the carrier board
>>> files contain only the setup/devices found on the carrier board?
>>
>> I have exactly the same feeling as Ryan. We should make sure
>> to factorize as much code as possible for maintenance reasons.
>>
>> If you need to distinguish between board features, you can
>> pass information in system_rev as implemented in this
>> board merging commit:
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a6e016f19d393fbe4e040bee8155b03b840fa689
>>
>
> I don't think system_rev is such a good idea for carrier boards from
> different vendors. Somebody would have to control the assigned numbers.
>
> This is what I would do:
> 1. Refactor board-stamp9g20.c have three board init functions:
> - portuxg20_board_init for PortuxG20 SBC (MACH_PORTUXG20)

I don't like the idea of having all of the carrier boards in the same
file as the SoM. Carrier boards may be developed by different people and
if there are a large number of them then the board files will start to
get quite big and confusing.

I think we need a standardised architecture for managing SoMs. A simple,
crude way would be to export the init functions from the SoM, i.e:
stam9g20_map_io and stamp9g20_board_init. Have the core SoM
functionality in those functions, for example in board-stamp9g20.c:

void __init stamp9g20_map_io(void)
{
/* Initialize processor: 18.432 MHz crystal */
at91sam9260_initialize(18432000);

/* DGBU on ttyS0. (Rx & Tx only) */
at91_register_uart(0, 0, 0);

/* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS
| ATMEL_UART_RTS | ATMEL_UART_DTR
| ATMEL_UART_DSR | ATMEL_UART_DCD
| ATMEL_UART_RI);

/* set serial console to ttyS0 (ie, DBGU) */
at91_set_serial_console(0);
}
EXPORT_SYMBOL(stamp9g20_map_io);

and then have the carrier specific initialisation in board-portuxg20.c:

static void __init portuxg20_map_io(void)
{
stamp9g20_map_io();

/* USART1 on ttyS2. (Rx, Tx, CTS, RTS) */
at91_register_uart(AT91SAM9260_ID_US1, 2,
ATMEL_UART_CTS | ATMEL_UART_RTS);

/* USART2 on ttyS3. (Rx, Tx, CTS, RTS) */
at91_register_uart(AT91SAM9260_ID_US2, 3,
ATMEL_UART_CTS | ATMEL_UART_RTS);

/* USART4 on ttyS5. (Rx, Tx only) */
at91_register_uart(AT91SAM9260_ID_US4, 5, 0);

/* USART5 on ttyS6. (Rx, Tx only) */
at91_register_uart(AT91SAM9260_ID_US5, 6, 0);
}

This reduces code reuse, and also means that if the carrier board has
some esoteric setup it can do its own initialisation rather than calling
stamp9g20_map_io. The downsides to this approach are the necessity of
having the exported functions and the relationship between the SoM and
the carrier board is not completely clear.

Maybe having additional macros similar to MACHINE_START for SoMs so that
we can have a machine_desc type struct for the SoM and can do things
like som->map_io()?

> - stamp9g20_board_init only containing the functions used on the
> Stamp9G20 alone

Agreed.

> - stamp9g20evb_board_init calling stamp9g20_board_init and adding
> functionality of the evaluation board (MACH_STAMP9G20)
>
> 2. Modify board-pcontrol-g20.c to use stamp9g20_board_init

Okay, I think we are both for the same approach :-).

> This would still duplicate the UART config (there is not much to share,
> only the DBGU would be configured on all boards), but share NAND, MMC,
> 1-wire. Everything else can't be shared as it is carrier board specific.
>
> The gms board would then have to have its own machine number and call
> stamp9g20_board_init.

~Ryan

--
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934

2010-12-09 10:20:20

by Christian Glindkamp

[permalink] [raw]
Subject: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
SoM initialization and board-pcontrol-g20.c is modified to use it.

Signed-off-by: Christian Glindkamp <[email protected]>
---

How about this approach? Compile tested for PControl G20 and run time tested
for Stamp9G20 EVB and PortuxG20.

Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
shares so much with the evaluation board, that it makes sense to put them both
into the same file. And there is no intention to put other boards into this
file.

arch/arm/mach-at91/Makefile | 2 +-
arch/arm/mach-at91/board-pcontrol-g20.c | 98 +--------------------------
arch/arm/mach-at91/board-stamp9g20.c | 82 ++++++++++++-----------
arch/arm/mach-at91/include/mach/stamp9g20.h | 7 ++
4 files changed, 54 insertions(+), 135 deletions(-)
create mode 100644 arch/arm/mach-at91/include/mach/stamp9g20.h

diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index 62d686f..d13add7 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -65,7 +65,7 @@ obj-$(CONFIG_MACH_AT91SAM9G20EK) += board-sam9g20ek.o
obj-$(CONFIG_MACH_CPU9G20) += board-cpu9krea.o
obj-$(CONFIG_MACH_STAMP9G20) += board-stamp9g20.o
obj-$(CONFIG_MACH_PORTUXG20) += board-stamp9g20.o
-obj-$(CONFIG_MACH_PCONTROL_G20) += board-pcontrol-g20.o
+obj-$(CONFIG_MACH_PCONTROL_G20) += board-pcontrol-g20.o board-stamp9g20.o

# AT91SAM9260/AT91SAM9G20 board-specific support
obj-$(CONFIG_MACH_SNAPPER_9260) += board-snapper9260.o
diff --git a/arch/arm/mach-at91/board-pcontrol-g20.c b/arch/arm/mach-at91/board-pcontrol-g20.c
index bba5a56..feb6578 100644
--- a/arch/arm/mach-at91/board-pcontrol-g20.c
+++ b/arch/arm/mach-at91/board-pcontrol-g20.c
@@ -31,6 +31,7 @@

#include <mach/board.h>
#include <mach/at91sam9_smc.h>
+#include <mach/stamp9g20.h>

#include "sam9_smc.h"
#include "generic.h"
@@ -38,11 +39,7 @@

static void __init pcontrol_g20_map_io(void)
{
- /* Initialize processor: 18.432 MHz crystal */
- at91sam9260_initialize(18432000);
-
- /* DGBU on ttyS0. (Rx, Tx) only TTL -> JTAG connector X7 17,19 ) */
- at91_register_uart(0, 0, 0);
+ stamp9g20_map_io();

/* USART0 on ttyS1. (Rx, Tx, CTS, RTS) piggyback A2 */
at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS
@@ -54,9 +51,6 @@ static void __init pcontrol_g20_map_io(void)

/* USART2 on ttyS3. (Rx, Tx) 9bit-Bus Multidrop-mode X4 */
at91_register_uart(AT91SAM9260_ID_US4, 3, 0);
-
- /* set serial console to ttyS0 (ie, DBGU) */
- at91_set_serial_console(0);
}


@@ -66,38 +60,6 @@ static void __init init_irq(void)
}


-/*
- * NAND flash 512MiB 1,8V 8-bit, sector size 128 KiB
- */
-static struct atmel_nand_data __initdata nand_data = {
- .ale = 21,
- .cle = 22,
- .rdy_pin = AT91_PIN_PC13,
- .enable_pin = AT91_PIN_PC14,
-};
-
-/*
- * Bus timings; unit = 7.57ns
- */
-static struct sam9_smc_config __initdata nand_smc_config = {
- .ncs_read_setup = 0,
- .nrd_setup = 2,
- .ncs_write_setup = 0,
- .nwe_setup = 2,
-
- .ncs_read_pulse = 4,
- .nrd_pulse = 4,
- .ncs_write_pulse = 4,
- .nwe_pulse = 4,
-
- .read_cycle = 7,
- .write_cycle = 7,
-
- .mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE
- | AT91_SMC_EXNWMODE_DISABLE | AT91_SMC_DBW_8,
- .tdf_cycles = 3,
-};
-
static struct sam9_smc_config __initdata pcontrol_smc_config[2] = { {
.ncs_read_setup = 16,
.nrd_setup = 18,
@@ -138,14 +100,6 @@ static struct sam9_smc_config __initdata pcontrol_smc_config[2] = { {
.tdf_cycles = 1,
} };

-static void __init add_device_nand(void)
-{
- /* configure chip-select 3 (NAND) */
- sam9_smc_configure(3, &nand_smc_config);
- at91_add_device_nand(&nand_data);
-}
-
-
static void __init add_device_pcontrol(void)
{
/* configure chip-select 4 (IO compatible to 8051 X4 ) */
@@ -156,23 +110,6 @@ static void __init add_device_pcontrol(void)


/*
- * MCI (SD/MMC)
- * det_pin, wp_pin and vcc_pin are not connected
- */
-#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
-static struct mci_platform_data __initdata mmc_data = {
- .slot[0] = {
- .bus_width = 4,
- },
-};
-#else
-static struct at91_mmc_data __initdata mmc_data = {
- .wire4 = 1,
-};
-#endif
-
-
-/*
* USB Host port
*/
static struct at91_usbh_data __initdata usbh_data = {
@@ -265,42 +202,13 @@ static struct spi_board_info pcontrol_g20_spi_devices[] = {
};


-/*
- * Dallas 1-Wire DS2431
- */
-static struct w1_gpio_platform_data w1_gpio_pdata = {
- .pin = AT91_PIN_PA29,
- .is_open_drain = 1,
-};
-
-static struct platform_device w1_device = {
- .name = "w1-gpio",
- .id = -1,
- .dev.platform_data = &w1_gpio_pdata,
-};
-
-static void add_wire1(void)
-{
- at91_set_GPIO_periph(w1_gpio_pdata.pin, 1);
- at91_set_multi_drive(w1_gpio_pdata.pin, 1);
- platform_device_register(&w1_device);
-}
-
-
static void __init pcontrol_g20_board_init(void)
{
- at91_add_device_serial();
- add_device_nand();
-#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
- at91_add_device_mci(0, &mmc_data);
-#else
- at91_add_device_mmc(0, &mmc_data);
-#endif
+ stamp9g20_board_init();
at91_add_device_usbh(&usbh_data);
at91_add_device_eth(&macb_data);
at91_add_device_i2c(pcontrol_g20_i2c_devices,
ARRAY_SIZE(pcontrol_g20_i2c_devices));
- add_wire1();
add_device_pcontrol();
at91_add_device_spi(pcontrol_g20_spi_devices,
ARRAY_SIZE(pcontrol_g20_spi_devices));
diff --git a/arch/arm/mach-at91/board-stamp9g20.c b/arch/arm/mach-at91/board-stamp9g20.c
index 5206eef..f8902b1 100644
--- a/arch/arm/mach-at91/board-stamp9g20.c
+++ b/arch/arm/mach-at91/board-stamp9g20.c
@@ -32,7 +32,7 @@
#include "generic.h"


-static void __init portuxg20_map_io(void)
+void __init stamp9g20_map_io(void)
{
/* Initialize processor: 18.432 MHz crystal */
at91sam9260_initialize(18432000);
@@ -40,6 +40,24 @@ static void __init portuxg20_map_io(void)
/* DGBU on ttyS0. (Rx & Tx only) */
at91_register_uart(0, 0, 0);

+ /* set serial console to ttyS0 (ie, DBGU) */
+ at91_set_serial_console(0);
+}
+
+static void __init stamp9g20evb_map_io(void)
+{
+ stamp9g20_map_io();
+
+ /* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
+ at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
+ | ATMEL_UART_DTR | ATMEL_UART_DSR
+ | ATMEL_UART_DCD | ATMEL_UART_RI);
+}
+
+static void __init portuxg20_map_io(void)
+{
+ stamp9g20_map_io();
+
/* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
| ATMEL_UART_DTR | ATMEL_UART_DSR
@@ -56,26 +74,6 @@ static void __init portuxg20_map_io(void)

/* USART5 on ttyS6. (Rx, Tx only) */
at91_register_uart(AT91SAM9260_ID_US5, 6, 0);
-
- /* set serial console to ttyS0 (ie, DBGU) */
- at91_set_serial_console(0);
-}
-
-static void __init stamp9g20_map_io(void)
-{
- /* Initialize processor: 18.432 MHz crystal */
- at91sam9260_initialize(18432000);
-
- /* DGBU on ttyS0. (Rx & Tx only) */
- at91_register_uart(0, 0, 0);
-
- /* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
- at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
- | ATMEL_UART_DTR | ATMEL_UART_DSR
- | ATMEL_UART_DCD | ATMEL_UART_RI);
-
- /* set serial console to ttyS0 (ie, DBGU) */
- at91_set_serial_console(0);
}

static void __init init_irq(void)
@@ -156,7 +154,7 @@ static struct at91_udc_data __initdata portuxg20_udc_data = {
.pullup_pin = 0, /* pull-up driven by UDC */
};

-static struct at91_udc_data __initdata stamp9g20_udc_data = {
+static struct at91_udc_data __initdata stamp9g20evb_udc_data = {
.vbus_pin = AT91_PIN_PA22,
.pullup_pin = 0, /* pull-up driven by UDC */
};
@@ -190,7 +188,7 @@ static struct gpio_led portuxg20_leds[] = {
}
};

-static struct gpio_led stamp9g20_leds[] = {
+static struct gpio_led stamp9g20evb_leds[] = {
{
.name = "D8",
.gpio = AT91_PIN_PB18,
@@ -250,7 +248,7 @@ void add_w1(void)
}


-static void __init generic_board_init(void)
+void __init stamp9g20_board_init(void)
{
/* Serial */
at91_add_device_serial();
@@ -262,34 +260,40 @@ static void __init generic_board_init(void)
#else
at91_add_device_mmc(0, &mmc_data);
#endif
- /* USB Host */
- at91_add_device_usbh(&usbh_data);
- /* Ethernet */
- at91_add_device_eth(&macb_data);
- /* I2C */
- at91_add_device_i2c(NULL, 0);
/* W1 */
add_w1();
}

static void __init portuxg20_board_init(void)
{
- generic_board_init();
- /* SPI */
- at91_add_device_spi(portuxg20_spi_devices, ARRAY_SIZE(portuxg20_spi_devices));
+ stamp9g20_board_init();
+ /* USB Host */
+ at91_add_device_usbh(&usbh_data);
/* USB Device */
at91_add_device_udc(&portuxg20_udc_data);
+ /* Ethernet */
+ at91_add_device_eth(&macb_data);
+ /* I2C */
+ at91_add_device_i2c(NULL, 0);
+ /* SPI */
+ at91_add_device_spi(portuxg20_spi_devices, ARRAY_SIZE(portuxg20_spi_devices));
/* LEDs */
at91_gpio_leds(portuxg20_leds, ARRAY_SIZE(portuxg20_leds));
}

-static void __init stamp9g20_board_init(void)
+static void __init stamp9g20evb_board_init(void)
{
- generic_board_init();
+ stamp9g20_board_init();
+ /* USB Host */
+ at91_add_device_usbh(&usbh_data);
/* USB Device */
- at91_add_device_udc(&stamp9g20_udc_data);
+ at91_add_device_udc(&stamp9g20evb_udc_data);
+ /* Ethernet */
+ at91_add_device_eth(&macb_data);
+ /* I2C */
+ at91_add_device_i2c(NULL, 0);
/* LEDs */
- at91_gpio_leds(stamp9g20_leds, ARRAY_SIZE(stamp9g20_leds));
+ at91_gpio_leds(stamp9g20evb_leds, ARRAY_SIZE(stamp9g20evb_leds));
}

MACHINE_START(PORTUXG20, "taskit PortuxG20")
@@ -305,7 +309,7 @@ MACHINE_START(STAMP9G20, "taskit Stamp9G20")
/* Maintainer: taskit GmbH */
.boot_params = AT91_SDRAM_BASE + 0x100,
.timer = &at91sam926x_timer,
- .map_io = stamp9g20_map_io,
+ .map_io = stamp9g20evb_map_io,
.init_irq = init_irq,
- .init_machine = stamp9g20_board_init,
+ .init_machine = stamp9g20evb_board_init,
MACHINE_END
diff --git a/arch/arm/mach-at91/include/mach/stamp9g20.h b/arch/arm/mach-at91/include/mach/stamp9g20.h
new file mode 100644
index 0000000..6120f9c
--- /dev/null
+++ b/arch/arm/mach-at91/include/mach/stamp9g20.h
@@ -0,0 +1,7 @@
+#ifndef __MACH_STAMP9G20_H
+#define __MACH_STAMP9G20_H
+
+void stamp9g20_map_io(void);
+void stamp9g20_board_init(void);
+
+#endif
--
1.7.2.3

Subject: Re: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

HI,

If the hardware are so near I do need to the need to create a new
machine use system_rev to auto detect it will be better

but we need to have only one defconfig as done on rm9200
it's really reduce the maintainance and allow to be sure when we
compile the at91sam9g20_defconfig that we do not brake any board

if a board have incompatible option please the system_rev to specify
them or a specific entry in the Kconfig for this board it will allow
also to known this information for the maintainance

Best Regards,
J.
On 11:15 Thu 09 Dec , Christian Glindkamp wrote:
> As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
> be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
> SoM initialization and board-pcontrol-g20.c is modified to use it.
>
> Signed-off-by: Christian Glindkamp <[email protected]>
> ---
>
> How about this approach? Compile tested for PControl G20 and run time tested
> for Stamp9G20 EVB and PortuxG20.
>
> Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
> shares so much with the evaluation board, that it makes sense to put them both
> into the same file. And there is no intention to put other boards into this
> file.
>
> arch/arm/mach-at91/Makefile | 2 +-
> arch/arm/mach-at91/board-pcontrol-g20.c | 98 +--------------------------
> arch/arm/mach-at91/board-stamp9g20.c | 82 ++++++++++++-----------
> arch/arm/mach-at91/include/mach/stamp9g20.h | 7 ++
> 4 files changed, 54 insertions(+), 135 deletions(-)
> create mode 100644 arch/arm/mach-at91/include/mach/stamp9g20.h
>
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index 62d686f..d13add7 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -65,7 +65,7 @@ obj-$(CONFIG_MACH_AT91SAM9G20EK) += board-sam9g20ek.o
> obj-$(CONFIG_MACH_CPU9G20) += board-cpu9krea.o
> obj-$(CONFIG_MACH_STAMP9G20) += board-stamp9g20.o
> obj-$(CONFIG_MACH_PORTUXG20) += board-stamp9g20.o
> -obj-$(CONFIG_MACH_PCONTROL_G20) += board-pcontrol-g20.o
> +obj-$(CONFIG_MACH_PCONTROL_G20) += board-pcontrol-g20.o board-stamp9g20.o
>
> # AT91SAM9260/AT91SAM9G20 board-specific support
> obj-$(CONFIG_MACH_SNAPPER_9260) += board-snapper9260.o
> diff --git a/arch/arm/mach-at91/board-pcontrol-g20.c b/arch/arm/mach-at91/board-pcontrol-g20.c
> index bba5a56..feb6578 100644
> --- a/arch/arm/mach-at91/board-pcontrol-g20.c
> +++ b/arch/arm/mach-at91/board-pcontrol-g20.c
> @@ -31,6 +31,7 @@
>
> #include <mach/board.h>
> #include <mach/at91sam9_smc.h>
> +#include <mach/stamp9g20.h>
>
> #include "sam9_smc.h"
> #include "generic.h"
> @@ -38,11 +39,7 @@
>
> static void __init pcontrol_g20_map_io(void)
> {
> - /* Initialize processor: 18.432 MHz crystal */
> - at91sam9260_initialize(18432000);
> -
> - /* DGBU on ttyS0. (Rx, Tx) only TTL -> JTAG connector X7 17,19 ) */
> - at91_register_uart(0, 0, 0);
> + stamp9g20_map_io();
>
> /* USART0 on ttyS1. (Rx, Tx, CTS, RTS) piggyback A2 */
> at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS
> @@ -54,9 +51,6 @@ static void __init pcontrol_g20_map_io(void)
>
> /* USART2 on ttyS3. (Rx, Tx) 9bit-Bus Multidrop-mode X4 */
> at91_register_uart(AT91SAM9260_ID_US4, 3, 0);
> -
> - /* set serial console to ttyS0 (ie, DBGU) */
> - at91_set_serial_console(0);
> }
>
>
> @@ -66,38 +60,6 @@ static void __init init_irq(void)
> }
>
>
> -/*
> - * NAND flash 512MiB 1,8V 8-bit, sector size 128 KiB
> - */
> -static struct atmel_nand_data __initdata nand_data = {
> - .ale = 21,
> - .cle = 22,
> - .rdy_pin = AT91_PIN_PC13,
> - .enable_pin = AT91_PIN_PC14,
> -};
> -
> -/*
> - * Bus timings; unit = 7.57ns
> - */
> -static struct sam9_smc_config __initdata nand_smc_config = {
> - .ncs_read_setup = 0,
> - .nrd_setup = 2,
> - .ncs_write_setup = 0,
> - .nwe_setup = 2,
> -
> - .ncs_read_pulse = 4,
> - .nrd_pulse = 4,
> - .ncs_write_pulse = 4,
> - .nwe_pulse = 4,
> -
> - .read_cycle = 7,
> - .write_cycle = 7,
> -
> - .mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE
> - | AT91_SMC_EXNWMODE_DISABLE | AT91_SMC_DBW_8,
> - .tdf_cycles = 3,
> -};
> -
> static struct sam9_smc_config __initdata pcontrol_smc_config[2] = { {
> .ncs_read_setup = 16,
> .nrd_setup = 18,
> @@ -138,14 +100,6 @@ static struct sam9_smc_config __initdata pcontrol_smc_config[2] = { {
> .tdf_cycles = 1,
> } };
>
> -static void __init add_device_nand(void)
> -{
> - /* configure chip-select 3 (NAND) */
> - sam9_smc_configure(3, &nand_smc_config);
> - at91_add_device_nand(&nand_data);
> -}
> -
> -
> static void __init add_device_pcontrol(void)
> {
> /* configure chip-select 4 (IO compatible to 8051 X4 ) */
> @@ -156,23 +110,6 @@ static void __init add_device_pcontrol(void)
>
>
> /*
> - * MCI (SD/MMC)
> - * det_pin, wp_pin and vcc_pin are not connected
> - */
> -#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
> -static struct mci_platform_data __initdata mmc_data = {
> - .slot[0] = {
> - .bus_width = 4,
> - },
> -};
> -#else
> -static struct at91_mmc_data __initdata mmc_data = {
> - .wire4 = 1,
> -};
> -#endif
> -
> -
> -/*
> * USB Host port
> */
> static struct at91_usbh_data __initdata usbh_data = {
> @@ -265,42 +202,13 @@ static struct spi_board_info pcontrol_g20_spi_devices[] = {
> };
>
>
> -/*
> - * Dallas 1-Wire DS2431
> - */
> -static struct w1_gpio_platform_data w1_gpio_pdata = {
> - .pin = AT91_PIN_PA29,
> - .is_open_drain = 1,
> -};
> -
> -static struct platform_device w1_device = {
> - .name = "w1-gpio",
> - .id = -1,
> - .dev.platform_data = &w1_gpio_pdata,
> -};
> -
> -static void add_wire1(void)
> -{
> - at91_set_GPIO_periph(w1_gpio_pdata.pin, 1);
> - at91_set_multi_drive(w1_gpio_pdata.pin, 1);
> - platform_device_register(&w1_device);
> -}
> -
> -
> static void __init pcontrol_g20_board_init(void)
> {
> - at91_add_device_serial();
> - add_device_nand();
> -#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
> - at91_add_device_mci(0, &mmc_data);
> -#else
> - at91_add_device_mmc(0, &mmc_data);
> -#endif
> + stamp9g20_board_init();
> at91_add_device_usbh(&usbh_data);
> at91_add_device_eth(&macb_data);
> at91_add_device_i2c(pcontrol_g20_i2c_devices,
> ARRAY_SIZE(pcontrol_g20_i2c_devices));
> - add_wire1();
> add_device_pcontrol();
> at91_add_device_spi(pcontrol_g20_spi_devices,
> ARRAY_SIZE(pcontrol_g20_spi_devices));
> diff --git a/arch/arm/mach-at91/board-stamp9g20.c b/arch/arm/mach-at91/board-stamp9g20.c
> index 5206eef..f8902b1 100644
> --- a/arch/arm/mach-at91/board-stamp9g20.c
> +++ b/arch/arm/mach-at91/board-stamp9g20.c
> @@ -32,7 +32,7 @@
> #include "generic.h"
>
>
> -static void __init portuxg20_map_io(void)
> +void __init stamp9g20_map_io(void)
> {
> /* Initialize processor: 18.432 MHz crystal */
> at91sam9260_initialize(18432000);
> @@ -40,6 +40,24 @@ static void __init portuxg20_map_io(void)
> /* DGBU on ttyS0. (Rx & Tx only) */
> at91_register_uart(0, 0, 0);
>
> + /* set serial console to ttyS0 (ie, DBGU) */
> + at91_set_serial_console(0);
> +}
> +
> +static void __init stamp9g20evb_map_io(void)
> +{
> + stamp9g20_map_io();
> +
> + /* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
> + at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
> + | ATMEL_UART_DTR | ATMEL_UART_DSR
> + | ATMEL_UART_DCD | ATMEL_UART_RI);
> +}
> +
> +static void __init portuxg20_map_io(void)
> +{
> + stamp9g20_map_io();
> +
> /* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
> at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
> | ATMEL_UART_DTR | ATMEL_UART_DSR
> @@ -56,26 +74,6 @@ static void __init portuxg20_map_io(void)
>
> /* USART5 on ttyS6. (Rx, Tx only) */
> at91_register_uart(AT91SAM9260_ID_US5, 6, 0);
> -
> - /* set serial console to ttyS0 (ie, DBGU) */
> - at91_set_serial_console(0);
> -}
> -
> -static void __init stamp9g20_map_io(void)
> -{
> - /* Initialize processor: 18.432 MHz crystal */
> - at91sam9260_initialize(18432000);
> -
> - /* DGBU on ttyS0. (Rx & Tx only) */
> - at91_register_uart(0, 0, 0);
> -
> - /* USART0 on ttyS1. (Rx, Tx, CTS, RTS, DTR, DSR, DCD, RI) */
> - at91_register_uart(AT91SAM9260_ID_US0, 1, ATMEL_UART_CTS | ATMEL_UART_RTS
> - | ATMEL_UART_DTR | ATMEL_UART_DSR
> - | ATMEL_UART_DCD | ATMEL_UART_RI);
> -
> - /* set serial console to ttyS0 (ie, DBGU) */
> - at91_set_serial_console(0);
> }
>
> static void __init init_irq(void)
> @@ -156,7 +154,7 @@ static struct at91_udc_data __initdata portuxg20_udc_data = {
> .pullup_pin = 0, /* pull-up driven by UDC */
> };
>
> -static struct at91_udc_data __initdata stamp9g20_udc_data = {
> +static struct at91_udc_data __initdata stamp9g20evb_udc_data = {
> .vbus_pin = AT91_PIN_PA22,
> .pullup_pin = 0, /* pull-up driven by UDC */
> };
> @@ -190,7 +188,7 @@ static struct gpio_led portuxg20_leds[] = {
> }
> };
>
> -static struct gpio_led stamp9g20_leds[] = {
> +static struct gpio_led stamp9g20evb_leds[] = {
> {
> .name = "D8",
> .gpio = AT91_PIN_PB18,
> @@ -250,7 +248,7 @@ void add_w1(void)
> }
>
>
> -static void __init generic_board_init(void)
> +void __init stamp9g20_board_init(void)
> {
> /* Serial */
> at91_add_device_serial();
> @@ -262,34 +260,40 @@ static void __init generic_board_init(void)
> #else
> at91_add_device_mmc(0, &mmc_data);
> #endif
> - /* USB Host */
> - at91_add_device_usbh(&usbh_data);
> - /* Ethernet */
> - at91_add_device_eth(&macb_data);
> - /* I2C */
> - at91_add_device_i2c(NULL, 0);
> /* W1 */
> add_w1();
> }
>
> static void __init portuxg20_board_init(void)
> {
> - generic_board_init();
> - /* SPI */
> - at91_add_device_spi(portuxg20_spi_devices, ARRAY_SIZE(portuxg20_spi_devices));
> + stamp9g20_board_init();
> + /* USB Host */
> + at91_add_device_usbh(&usbh_data);
> /* USB Device */
> at91_add_device_udc(&portuxg20_udc_data);
> + /* Ethernet */
> + at91_add_device_eth(&macb_data);
> + /* I2C */
> + at91_add_device_i2c(NULL, 0);
> + /* SPI */
> + at91_add_device_spi(portuxg20_spi_devices, ARRAY_SIZE(portuxg20_spi_devices));
> /* LEDs */
> at91_gpio_leds(portuxg20_leds, ARRAY_SIZE(portuxg20_leds));
> }
>
> -static void __init stamp9g20_board_init(void)
> +static void __init stamp9g20evb_board_init(void)
> {
> - generic_board_init();
> + stamp9g20_board_init();
> + /* USB Host */
> + at91_add_device_usbh(&usbh_data);
> /* USB Device */
> - at91_add_device_udc(&stamp9g20_udc_data);
> + at91_add_device_udc(&stamp9g20evb_udc_data);
> + /* Ethernet */
> + at91_add_device_eth(&macb_data);
> + /* I2C */
> + at91_add_device_i2c(NULL, 0);
> /* LEDs */
> - at91_gpio_leds(stamp9g20_leds, ARRAY_SIZE(stamp9g20_leds));
> + at91_gpio_leds(stamp9g20evb_leds, ARRAY_SIZE(stamp9g20evb_leds));
> }
>
> MACHINE_START(PORTUXG20, "taskit PortuxG20")
> @@ -305,7 +309,7 @@ MACHINE_START(STAMP9G20, "taskit Stamp9G20")
> /* Maintainer: taskit GmbH */
> .boot_params = AT91_SDRAM_BASE + 0x100,
> .timer = &at91sam926x_timer,
> - .map_io = stamp9g20_map_io,
> + .map_io = stamp9g20evb_map_io,
> .init_irq = init_irq,
> - .init_machine = stamp9g20_board_init,
> + .init_machine = stamp9g20evb_board_init,
> MACHINE_END
> diff --git a/arch/arm/mach-at91/include/mach/stamp9g20.h b/arch/arm/mach-at91/include/mach/stamp9g20.h
> new file mode 100644
> index 0000000..6120f9c
> --- /dev/null
> +++ b/arch/arm/mach-at91/include/mach/stamp9g20.h
> @@ -0,0 +1,7 @@
> +#ifndef __MACH_STAMP9G20_H
> +#define __MACH_STAMP9G20_H
> +
> +void stamp9g20_map_io(void);
> +void stamp9g20_board_init(void);
> +
> +#endif
> --
> 1.7.2.3

2010-12-10 06:20:08

by Igor Plyatov

[permalink] [raw]
Subject: Re: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

Dear Christian,

> As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
> be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
> SoM initialization and board-pcontrol-g20.c is modified to use it.
>
> Signed-off-by: Christian Glindkamp <[email protected]>
> ---
>
> How about this approach? Compile tested for PControl G20 and run time tested
> for Stamp9G20 EVB and PortuxG20.
>
> Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
> shares so much with the evaluation board, that it makes sense to put them both
> into the same file. And there is no intention to put other boards into this
> file.

The idea from this patch is clean.
Yours patch does not apply to the kernel from
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git,
but I apply its content by hands and kernels compiled successfully with
stamp9g20_defconfig and pcontrol_g20_defconfig.

Please run ./scripts/checkpatch.pl with you patch and correct all errors
and warnings reported.

If you will correct this patch, to be applicable, then I will vote to
include it into mainline.

Which kernel repository and branch should I use for the AT91 ARM?
I just want to know - whereto send my next patch to support gms machine?

Best regards!
--
Igor Plyatov


2010-12-10 08:47:57

by Christian Glindkamp

[permalink] [raw]
Subject: Re: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

On 2010-12-09 18:39, Igor Plyatov wrote:
> Dear Christian,
>
> > As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
> > be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
> > SoM initialization and board-pcontrol-g20.c is modified to use it.
> >
> > Signed-off-by: Christian Glindkamp <[email protected]>
> > ---
> >
> > How about this approach? Compile tested for PControl G20 and run time tested
> > for Stamp9G20 EVB and PortuxG20.
> >
> > Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
> > shares so much with the evaluation board, that it makes sense to put them both
> > into the same file. And there is no intention to put other boards into this
> > file.
>
> The idea from this patch is clean.
> Yours patch does not apply to the kernel from
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git,
> but I apply its content by hands and kernels compiled successfully with
> stamp9g20_defconfig and pcontrol_g20_defconfig.

I used exactly this repository. Revision 6313e3c21743cc88bb5bd8aa72948ee1e83937b6
to be precise. Maybe your mailer corrupted the patch.

> Please run ./scripts/checkpatch.pl with you patch and correct all errors
> and warnings reported.

I only get some "line over 80 characters" warnings, which can imho be
ignored.

>
> If you will correct this patch, to be applicable, then I will vote to
> include it into mainline.
>
> Which kernel repository and branch should I use for the AT91 ARM?
> I just want to know - whereto send my next patch to support gms machine?
>
> Best regards!
> --
> Igor Plyatov
>

2010-12-10 09:04:58

by Christian Glindkamp

[permalink] [raw]
Subject: Re: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

On 2010-12-10 04:38, Jean-Christophe PLAGNIOL-VILLARD wrote:
> HI,
>
> If the hardware are so near I do need to the need to create a new
> machine use system_rev to auto detect it will be better
>
> but we need to have only one defconfig as done on rm9200
> it's really reduce the maintainance and allow to be sure when we
> compile the at91sam9g20_defconfig that we do not brake any board
>
> if a board have incompatible option please the system_rev to specify
> them or a specific entry in the Kconfig for this board it will allow
> also to known this information for the maintainance

Just because it is near does not mean it is a revision of the other
board. Just compare
http://www.taskit.de/en/products/portuxg20/index.htm
http://www.taskit.de/en/products/stamp9g20/starterkit.htm

Apart from that, both boards are correctly identifiable via the machine
id for a year, respectively one and a half year for the Stamp9G20 EVB.
Why change it for sake of change?

They both have there own machine id to make it clear that these are
really different boards. I could have also submitted two board files
and maybe nobody would have noticed that share a lot, but I thought code
reuse is better so there are in the same file.

And for different carrier boards, system_rev does not make sense at all.

>
> Best Regards,
> J.
> On 11:15 Thu 09 Dec , Christian Glindkamp wrote:
> > As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
> > be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
> > SoM initialization and board-pcontrol-g20.c is modified to use it.
> >
> > Signed-off-by: Christian Glindkamp <[email protected]>
> > ---
> >
> > How about this approach? Compile tested for PControl G20 and run time tested
> > for Stamp9G20 EVB and PortuxG20.
> >
> > Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
> > shares so much with the evaluation board, that it makes sense to put them both
> > into the same file. And there is no intention to put other boards into this
> > file.
> >
> > arch/arm/mach-at91/Makefile | 2 +-
> > arch/arm/mach-at91/board-pcontrol-g20.c | 98 +--------------------------
> > arch/arm/mach-at91/board-stamp9g20.c | 82 ++++++++++++-----------
> > arch/arm/mach-at91/include/mach/stamp9g20.h | 7 ++
> > 4 files changed, 54 insertions(+), 135 deletions(-)
> > create mode 100644 arch/arm/mach-at91/include/mach/stamp9g20.h

2010-12-10 09:45:40

by Nicolas Ferre

[permalink] [raw]
Subject: Re: [PATCH] at91: Refactor Stamp9G20 and PControl G20 board file

Le 10/12/2010 09:44, Christian Glindkamp :
> On 2010-12-09 18:39, Igor Plyatov wrote:
>> Dear Christian,
>>
>>> As PControl G20 is a carrier board for the Stamp9G20 SoM, some code can
>>> be shared. Therefore board-stamp9g20.c is refactored to allow reusing the
>>> SoM initialization and board-pcontrol-g20.c is modified to use it.
>>>
>>> Signed-off-by: Christian Glindkamp <[email protected]>
>>> ---
>>>
>>> How about this approach? Compile tested for PControl G20 and run time tested
>>> for Stamp9G20 EVB and PortuxG20.
>>>
>>> Just a side note: PortuxG20 is not a carrier board for the Stamp9G20. It just
>>> shares so much with the evaluation board, that it makes sense to put them both
>>> into the same file. And there is no intention to put other boards into this
>>> file.
>>
>> The idea from this patch is clean.

Seems clean to me also.

>> Yours patch does not apply to the kernel from
>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git,
>> but I apply its content by hands and kernels compiled successfully with
>> stamp9g20_defconfig and pcontrol_g20_defconfig.
>
> I used exactly this repository. Revision 6313e3c21743cc88bb5bd8aa72948ee1e83937b6
> to be precise. Maybe your mailer corrupted the patch.
>
>> Please run ./scripts/checkpatch.pl with you patch and correct all errors
>> and warnings reported.
>
> I only get some "line over 80 characters" warnings, which can imho be
> ignored.
>
>>
>> If you will correct this patch, to be applicable, then I will vote to
>> include it into mainline.
>>
>> Which kernel repository and branch should I use for the AT91 ARM?
>> I just want to know - whereto send my next patch to support gms machine?

You can build your "gms" patch on top of Linus' + Christian's patches.

Then, I will:
- send the patch series to mailing-list for final review (maybe with one
or two additional patches)
- prepare a pull-request for inclusion in mainline (.37-final may be ok
as it is just board additions)

Bye,
--
Nicolas Ferre