Skip to contentSkip to conversation

AT91: Add flexibity board support

PATCH Superseded Maxim Osipov 4 participants 9 messages Sep 13, 2010 – Sep 14, 2010

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 AT91: Add flexibity board support

Hello,

This patch adds support for AT91 Flexibity board.

Kind regards,
Maxim

#1 lore raw

Dne Po 13. z??? 2010 12:18:44 Maxim Osipov napsal(a):

Quoted 82 lines

Hello,

This patch adds support for AT91 Flexibity board.

Kind regards,
Maxim

From ccb19f0d6dbda5356e64c1ce5d0fe1396c548f1a Mon Sep 17 00:00:00 2001
From: Maxim Osipov <[email protected]>
Date: Sun, 12 Sep 2010 16:23:48 +0400
Subject: [PATCH v2] AT91: Add flexibity board support

---
 arch/arm/mach-at91/Kconfig           |    6 ++
 arch/arm/mach-at91/Makefile          |    1 +
 arch/arm/mach-at91/board-flexibity.c |  166
++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 0
deletions(-)
 create mode 100644 arch/arm/mach-at91/board-flexibity.c
arch/arm/mach-at91/Kconfig +6 -0
index 939bccd..ca33862 100644
@@ -248,6 +248,12 @@ config MACH_CPU9260
 	  Select this if you are using a Eukrea Electromatique's
 	  CPU9260 Board <http://www.eukrea.com/>
 
+config MACH_FLEXIBITY
+	bool "Flexibity Connect board"
+	help
+	  Select this if you are using Flexibity Connect board
+	  <http://www.flexibity.com>
+
 endif
 
 # ----------------------------------------------------------
arch/arm/mach-at91/Makefile +1 -0
index ca2ac00..7cbe06d 100644
@@ -46,6 +46,7 @@ obj-$(CONFIG_MACH_USB_A9260)	+= board-usb-a9260.o
 obj-$(CONFIG_MACH_QIL_A9260)	+= board-qil-a9260.o
 obj-$(CONFIG_MACH_AFEB9260)	+= board-afeb-9260v1.o
 obj-$(CONFIG_MACH_CPU9260)	+= board-cpu9krea.o
+obj-$(CONFIG_MACH_FLEXIBITY)	+= board-flexibity.o
 
 # AT91SAM9261 board-specific support
 obj-$(CONFIG_MACH_AT91SAM9261EK) += board-sam9261ek.o
(quoted diff) +0 -0

b/arch/arm/mach-at91/board-flexibity.c new file mode 100644 index 0000000..a589077

arch/arm/mach-at91/board-flexibity.c new file +20 -0
@@ -0,0 +1,166 @@
+/*
+ * linux/arch/arm/mach-at91/board-flexibity.c
+ *
+ *  Copyright (C) 2010 Flexibity
+ *  Copyright (C) 2005 SAN People
+ *  Copyright (C) 2006 Atmel
+ *
+ * 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 + */

arch/arm/mach-at91/board-flexibity.c new file +5 -0
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/input.h>

arch/arm/mach-at91/board-flexibity.c

Quoted 10 lines
arch/arm/mach-at91/board-flexibity.c new file +10 -0
+
+#include <asm/mach-types.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+#include <asm/mach/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/board.h>
+#include <mach/gpio.h>

#include <linux/gpio.h>

btw. don't you want to modularize this code a bit ? check how mach-pxa/z2.c or mach-pxa/vpac270.c is done for example.

Cheers

Quoted 130 lines
arch/arm/mach-at91/board-flexibity.c new file +130 -0
+
+#include "generic.h"
+
+
+static void __init flexibity_map_io(void)
+{
+	/* Initialize processor: 18.432 MHz crystal */
+	at91sam9260_initialize(18432000);
+
+	/* DBGU 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 flexibity_init_irq(void)
+{
+	at91sam9260_init_interrupts(NULL);
+}
+
+
+/* USB Host port */
+static struct at91_usbh_data __initdata flexibity_usbh_data = {
+	.ports		= 2,
+};
+
+/* USB Device port */
+static struct at91_udc_data __initdata flexibity_udc_data = {
+	.vbus_pin	= AT91_PIN_PC5,
+	.pullup_pin	= 0,		/* pull-up driven by UDC */
+};
+
+/* SPI devices */
+static struct spi_board_info flexibity_spi_devices[] = {
+	{	/* DataFlash chip */
+		.modalias	= "mtd_dataflash",
+		.chip_select	= 1,
+		.max_speed_hz	= 15 * 1000 * 1000,
+		.bus_num	= 0,
+	},
+};
+
+/* MCI (SD/MMC) */
+static struct at91_mmc_data __initdata flexibity_mmc_data = {
+	.slot_b		= 0,
+	.wire4		= 1,
+	.det_pin	= AT91_PIN_PC9,
+	.wp_pin		= AT91_PIN_PC4,
+};
+
+/* LEDs */
+static struct gpio_led flexibity_leds[] = {
+	{
+		.name			= "usb1:green",
+		.gpio			= AT91_PIN_PA12,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb1:red",
+		.gpio			= AT91_PIN_PA13,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb2:green",
+		.gpio			= AT91_PIN_PB26,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb2:red",
+		.gpio			= AT91_PIN_PB27,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb3:green",
+		.gpio			= AT91_PIN_PC8,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb3:red",
+		.gpio			= AT91_PIN_PC6,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb4:green",
+		.gpio			= AT91_PIN_PB4,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	},
+	{
+		.name			= "usb4:red",
+		.gpio			= AT91_PIN_PB5,
+		.active_low		= 1,
+		.default_trigger	= "default-on",
+	}
+};
+
+static void __init flexibity_board_init(void)
+{
+	/* Serial */
+	at91_add_device_serial();
+	/* USB Host */
+	at91_add_device_usbh(&flexibity_usbh_data);
+	/* USB Device */
+	at91_add_device_udc(&flexibity_udc_data);
+	/* SPI */
+	at91_add_device_spi(flexibity_spi_devices,
+		ARRAY_SIZE(flexibity_spi_devices));
+	/* MMC */
+	at91_add_device_mmc(0, &flexibity_mmc_data);
+	/* LEDs */
+	at91_gpio_leds(flexibity_leds, ARRAY_SIZE(flexibity_leds));
+}
+
+MACHINE_START(FLEXIBITY, "Flexibity Connect")
+	/* Maintainer: Maxim Osipov */
+	.phys_io	= AT91_BASE_SYS,
+	.io_pg_offst	= (AT91_VA_BASE_SYS >> 18) & 0xfffc,
+	.boot_params	= AT91_SDRAM_BASE + 0x100,
+	.timer		= &at91sam926x_timer,
+	.map_io		= flexibity_map_io,
+	.init_irq	= flexibity_init_irq,
+	.init_machine	= flexibity_board_init,
+MACHINE_END
#2 lore raw

Hi!

Quoted 5 lines · Marek Vasut

Dne Po 13. z??? 2010 12:18:44 Maxim Osipov napsal(a):

btw. don't you want to modularize this code a bit ? check how mach-pxa/z2.c or mach-pxa/vpac270.c is done for example.

No, all functionality is needed in the product, so I do not think modularization is necessary.

Kind regards,
Maxim

#3 lore raw

Dne Po 13. z??? 2010 13:02:24 Maxim Osipov napsal(a):

Quoted 5 lines

Hi!

Quoted 2 lines · Marek Vasut

Dne Po 13. z??? 2010 12:18:44 Maxim Osipov napsal(a):

Quoted 8 lines
Quoted 2 lines

btw. don't you want to modularize this code a bit ? check how mach-pxa/z2.c or mach-pxa/vpac270.c is done for example.

No, all functionality is needed in the product, so I do not think modularization is necessary.

Kind regards,
Maxim

Ok, please fix the include then

#4 lore raw
In reply to Maxim Osipov
Quoted 12 lines · Maxim Osipov

Hello,

This patch adds support for AT91 Flexibity board.

Kind regards,
Maxim

Quoted 1 line

From ccb19f0d6dbda5356e64c1ce5d0fe1396c548f1a Mon Sep 17 00:00:00 2001

From: Maxim Osipov <[email protected]>
Date: Sun, 12 Sep 2010 16:23:48 +0400
Subject: [PATCH v2] AT91: Add flexibity board support

Quoted 8 lines
---
 arch/arm/mach-at91/Kconfig           |    6 ++
 arch/arm/mach-at91/Makefile          |    1 +
 arch/arm/mach-at91/board-flexibity.c |  166 ++++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-at91/board-flexibity.c

Best Regards,
J.

#5 lore raw
In reply to Maxim Osipov
Quoted 3 lines · Maxim Osipov

Hello,

This patch adds support for AT91 Flexibity board.

Couple more comments below,

Quoted 12 lines

From ccb19f0d6dbda5356e64c1ce5d0fe1396c548f1a Mon Sep 17 00:00:00 2001
From: Maxim Osipov <[email protected]>
Date: Sun, 12 Sep 2010 16:23:48 +0400
Subject: [PATCH v2] AT91: Add flexibity board support

---

(quoted diff) +4 -0
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+#include <linux/input.h>

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+#include <asm/mach-types.h>
+
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+#include <asm/mach/irq.h>
+
+#include <mach/hardware.h>
+#include <mach/board.h>
+#include <mach/gpio.h>

Use linux/gpio.h

Review

Quoted 4 lines
(quoted diff) +4 -0
+
+#include "generic.h"
+
+

Single blank line here, and between functions.

Quoted 6 lines
(quoted diff) +6 -0
+
+static void __init flexibity_board_init(void)
+{
+	/* Serial */
+	at91_add_device_serial();
+	/* USB Host */

Review

Quoted 10 lines
(quoted diff) +10 -0
+	at91_add_device_usbh(&flexibity_usbh_data);
+	/* USB Device */
+	at91_add_device_udc(&flexibity_udc_data);
+	/* SPI */
+	at91_add_device_spi(flexibity_spi_devices,
+		ARRAY_SIZE(flexibity_spi_devices));
+	/* MMC */
+	at91_add_device_mmc(0, &flexibity_mmc_data);
+	/* LEDs */
+	at91_gpio_leds(flexibity_leds, ARRAY_SIZE(flexibity_leds));

Can we drop all the comments inside this function please. They don't add any information and they make it harder to read the actual code.

~Ryan

Signature · 7 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	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
#6 lore raw

Please see my comments below...

Quoted 12 lines · Ryan Mallon
Quoted 10 lines · Maxim Osipov
(quoted diff) +10 -0
+static void __init flexibity_board_init(void)
+{
+ ? ? /* Serial */
+ ? ? at91_add_device_serial();
+ ? ? /* USB Host */
+ ? ? at91_add_device_usbh(&flexibity_usbh_data);
+ ? ? /* USB Device */
+ ? ? at91_add_device_udc(&flexibity_udc_data);
+ ? ? /* SPI */
+ ? ? at91_add_device_spi(flexibity_spi_devices,
Quoted 9 lines
Quoted 5 lines

+ ? ? ? ? ? ? ARRAY_SIZE(flexibity_spi_devices));
+ ? ? /* MMC */
+ ? ? at91_add_device_mmc(0, &flexibity_mmc_data);
+ ? ? /* LEDs */
+ ? ? at91_gpio_leds(flexibity_leds, ARRAY_SIZE(flexibity_leds));

Can we drop all the comments inside this function please. They don't add any information and they make it harder to read the actual code.

Ryan, thank you for the review. However I would like to keep these comments - for some devices, like UDC - it really provides clarification. Reading the code is not a problem with proper highlighting scheme.

Kind regards,
Maxim

#7 lore raw
Quoted 25 lines · Maxim Osipov

Please see my comments below...

Quoted 12 lines · Ryan Mallon
Quoted 10 lines · Maxim Osipov
(quoted diff) +10 -0
+static void __init flexibity_board_init(void)
+{
+ ? ? /* Serial */
+ ? ? at91_add_device_serial();
+ ? ? /* USB Host */
+ ? ? at91_add_device_usbh(&flexibity_usbh_data);
+ ? ? /* USB Device */
+ ? ? at91_add_device_udc(&flexibity_udc_data);
+ ? ? /* SPI */
+ ? ? at91_add_device_spi(flexibity_spi_devices,
Quoted 9 lines
Quoted 5 lines

+ ? ? ? ? ? ? ARRAY_SIZE(flexibity_spi_devices));
+ ? ? /* MMC */
+ ? ? at91_add_device_mmc(0, &flexibity_mmc_data);
+ ? ? /* LEDs */
+ ? ? at91_gpio_leds(flexibity_leds, ARRAY_SIZE(flexibity_leds));

Can we drop all the comments inside this function please. They don't add any information and they make it harder to read the actual code.

Quoted 4 lines

Ryan, thank you for the review. However I would like to keep these comments - for some devices, like UDC - it really provides clarification. Reading the code is not a problem with proper highlighting scheme.

agreed no need to change

Best Regards,
J.

#8 lore raw
In reply to Maxim Osipov
Quoted 25 lines · Maxim Osipov

Please see my comments below...

Quoted 12 lines · Ryan Mallon
Quoted 10 lines · Maxim Osipov
(quoted diff) +10 -0
+static void __init flexibity_board_init(void)
+{
+     /* Serial */
+     at91_add_device_serial();
+     /* USB Host */
+     at91_add_device_usbh(&flexibity_usbh_data);
+     /* USB Device */
+     at91_add_device_udc(&flexibity_udc_data);
+     /* SPI */
+     at91_add_device_spi(flexibity_spi_devices,
Quoted 9 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+             ARRAY_SIZE(flexibity_spi_devices));
+     /* MMC */
+     at91_add_device_mmc(0, &flexibity_mmc_data);
+     /* LEDs */
+     at91_gpio_leds(flexibity_leds, ARRAY_SIZE(flexibity_leds));

Can we drop all the comments inside this function please. They don't add any information and they make it harder to read the actual code.

Quoted 4 lines

Ryan, thank you for the review. However I would like to keep these comments - for some devices, like UDC - it really provides clarification. Reading the code is not a problem with proper highlighting scheme.

There are already comments above each of the structures so it is clear what all of these are. IMHO, this commenting is similar to doing:

	/* Add 1 to i */
	i++;

We don't have this kind of overly verbose commenting in any of the other ARM variants, it appears to something that is just cut and pasted around the AT91 boards :-). Syntax highlighting or not, reading one for one comments on what should be straight forward device registration is annoying.

~Ryan

Signature · 7 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	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