[patch v4 00/10] efikamx support improvements - take 4

[patch v4 00/10] efikamx support improvements - take 4

[Arnaud Patard (Rtp)]

Hi,

This patchset aims at improving current efikamx support. It adds support for :
- board id detection
- sdhc
- leds
- power button
- spi nor
- reset

I also had to fix/improve some imx51 iomux header so there are patches for
iomux-mx51.h.

v4:
- reorder selects for the machine Kconfig entry
- drop linux/gpio_keys.h include
- mark mx51_efikamx_powerkey_data as const & __initconst
- add missing return after calling mx51_efikamx_reset()

v3:
- use imx51_add_gpio_keys
- fix patch description for power key patch

v2:
- split iomux patch 3 into 2 patches
- reorder patches
- fix space comment
- fix Amit's comments
- decrease board id msleep

Arnaud

[patch v4 01/10] efikamx: read board id

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_boardid.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/0c293691/attachment.ksh>

[patch v4 02/10] imx51: fix iomux configuration

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: imx51_fix_iomux_gpio_1_x.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/43174a4b/attachment.ksh>

[patch v4 03/10] imx51: enhance iomux configuration for esdhc support

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: imx51_add_esdhc_pad_mode.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/195f1a4e/attachment.ksh>

[patch v4 04/10] efikamx: add mmc support

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efika_mmc.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/0970d28f/attachment.ksh>

[patch v4 05/10] imx51: add gpio mode for csi1 {h,v}sync

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: imx51_add_csi1_hvsync_gpio.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/cbd20a7f/attachment.ksh>

[patch v4 06/10] efikamx: add leds support

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efikamx_add_leds.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/e0e8b099/attachment.ksh>

[patch v4 07/10] efikamx: add support for power key

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efikamx_add_powerkey.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/0ef2db73/attachment.ksh>

[patch v4 08/10] imx51: fix gpio_4_24 and gpio_4_25 pad configuration

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: imx51_fix_iomux_gpio_4_24_25.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/4c44397e/attachment.ksh>

[patch v4 09/10] efikamx: add spi nor support

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efikamx_add_nor.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/73a883ed/attachment.ksh>

[patch v4 10/10] efikamx: add reset

[Arnaud Patard (Rtp)]

An embedded and charset-unspecified text was scrubbed...
Name: efikamx_reset.patch
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20101027/dfecaead/attachment.ksh>

[patch v4 01/10] efikamx: read board id

[Marek Vasut]

On Wednesday 27 October 2010 14:40:46 Arnaud Patard wrote:
> read board id value from the GPIO3_16/17/11
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> ===================================================================

This is not git, is it ... what do you use to generate this stuff ?

> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-21
> 08:29:23.000000000 +0200 +++
> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-21
> 08:29:46.000000000 +0200 @@ -39,12 +39,26 @@
> 
>  #define	MX51_USB_PLL_DIV_24_MHZ	0x01
> 
> +#define EFIKAMX_PCBID0		(2*32 + 16)
> +#define EFIKAMX_PCBID1		(2*32 + 17)
> +#define EFIKAMX_PCBID2		(2*32 + 11)
> +
> +/* the pci ids pin have pull up. they're driven low according to board id
> */ +#define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> PAD_CTL_PUS_100K_UP) +#define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> 0x0,   0, PAD_CTL_PUS_100K_UP) +#define MX51_PAD_PCBID2	IOMUX_PAD(0x504,
> 0x128, 3, 0x0,   0, PAD_CTL_PUS_100K_UP) +
>  static struct pad_desc mx51efikamx_pads[] = {
>  	/* UART1 */
>  	MX51_PAD_UART1_RXD__UART1_RXD,
>  	MX51_PAD_UART1_TXD__UART1_TXD,
>  	MX51_PAD_UART1_RTS__UART1_RTS,
>  	MX51_PAD_UART1_CTS__UART1_CTS,
> +
> +	/* board id */
> +	MX51_PAD_PCBID0,
> +	MX51_PAD_PCBID1,
> +	MX51_PAD_PCBID2,
>  };
> 
>  /* Serial ports */
> @@ -92,10 +106,62 @@
>  	.flags  = MXC_EHCI_INTERNAL_PHY,
>  };
> 
> +/*   PCBID2  PCBID1 PCBID0  STATE
> +	1       1      1    ER1:rev1.1
> +	1       1      0    ER2:rev1.2
> +	1       0      1    ER3:rev1.3
> +	1       0      0    ER4:rev1.4
> +*/
> +static void __init mx51_efikamx_board_id(void)
> +{
> +	int id;
> +
> +	/* things are taking time to settle */
> +	msleep(150);
> +
> +	gpio_request(EFIKAMX_PCBID0, "pcbid0");
> +	gpio_direction_input(EFIKAMX_PCBID0);
> +	gpio_request(EFIKAMX_PCBID1, "pcbid1");
> +	gpio_direction_input(EFIKAMX_PCBID1);
> +	gpio_request(EFIKAMX_PCBID2, "pcbid2");
> +	gpio_direction_input(EFIKAMX_PCBID2);
> +
> +	id = gpio_get_value(EFIKAMX_PCBID0);
> +	id |= gpio_get_value(EFIKAMX_PCBID1) << 1;
> +	id |= gpio_get_value(EFIKAMX_PCBID2) << 2;
> +
> +	switch (id) {
> +	case 7:
> +		system_rev = 0x11;
> +		break;
> +	case 6:
> +		system_rev = 0x12;
> +		break;
> +	case 5:
> +		system_rev = 0x13;
> +		break;
> +	case 4:
> +		system_rev = 0x14;
> +		break;
> +	default:
> +		system_rev = 0x10;
> +		break;
> +	}
> +
> +	if ((system_rev == 0x10)
> +		|| (system_rev == 0x12)
> +		|| (system_rev == 0x14)) {
> +		printk(KERN_WARNING
> +			"EfikaMX: Unsupported board revision 1.%u!\n",
> +			system_rev & 0xf);

Use rather Unknown than Unsupported ... or make it sound more like a warning 
than like a utter crash.

Also, maybe you should free the GPIOs here again ?

> +	}
> +}
> +
>  static void __init mxc_board_init(void)
>  {
>  	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
>  					ARRAY_SIZE(mx51efikamx_pads));
> +	mx51_efikamx_board_id();
>  	mxc_register_device(&mxc_usbdr_host_device, &dr_utmi_config);
>  	mxc_init_imx_uart();
>  }
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[patch v4 02/10] imx51: fix iomux configuration

[Marek Vasut]

On Wednesday 27 October 2010 14:40:47 Arnaud Patard wrote:
> - ALT0 is used to set GPIO mode of GPIO_1_{2,3,4,5,6,7,8,9} but it's ALT1
> for GPIO_1_{0,1}.
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h
> ===================================================================
> ---
> linux-2.6-submit.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h	2010-10-
> 20 18:30:30.000000000 +0200 +++
> linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h	2010-10-20
> 18:30:42.000000000 +0200 @@ -368,18 +368,18 @@
>  							MX51_SDHCI_PAD_CTRL)
>  #define MX51_PAD_GPIO_1_0__GPIO_1_0		IOMUX_PAD(0x7B4, 0x3AC, 1, 0x0,   
0,
> MX51_GPIO_PAD_CTRL) #define MX51_PAD_GPIO_1_1__GPIO_1_1		IOMUX_PAD(0x7B8,
> 0x3B0, 1, 0x0,   0, MX51_GPIO_PAD_CTRL) -#define
> MX51_PAD_GPIO_1_2__GPIO_1_2		IOMUX_PAD(0x7D4, 0x3CC, 1, 0x0,   0,
> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_2__GPIO_1_2		
IOMUX_PAD(0x7D4,
> 0x3CC, 0, 0x0,   0, MX51_GPIO_PAD_CTRL) #define
> MX51_PAD_GPIO_1_2__I2C2_SCL		IOMUX_PAD(0x7D4, 0x3CC, (2 |
> IOMUX_CONFIG_SION), \ 0x9b8,   3, MX51_I2C_PAD_CTRL)
> -#define MX51_PAD_GPIO_1_3__GPIO_1_3		IOMUX_PAD(0x7D8, 0x3D0, 1, 0x0,   
0,
> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_3__GPIO_1_3		
IOMUX_PAD(0x7D8,
> 0x3D0, 0, 0x0,   0, MX51_GPIO_PAD_CTRL) #define
> MX51_PAD_GPIO_1_3__I2C2_SDA		IOMUX_PAD(0x7D8, 0x3D0, (2 |
> IOMUX_CONFIG_SION), \ 0x9bc,   3, MX51_I2C_PAD_CTRL)
>  #define MX51_PAD_PMIC_INT_REQ__PMIC_INT_REQ	IOMUX_PAD(0x7FC, 0x3D4, 0,
> 0x0,   0, NO_PAD_CTRL) -#define
> MX51_PAD_GPIO_1_4__GPIO_1_4		IOMUX_PAD(0x804, 0x3D8, 1, 0x0,   0,
> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_5__GPIO_1_5		IOMUX_PAD(0x808,
> 0x3DC, 1, 0x0,   0, MX51_GPIO_PAD_CTRL) -#define
> MX51_PAD_GPIO_1_6__GPIO_1_6		IOMUX_PAD(0x80C, 0x3E0, 1, 0x0,   0,
> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_7__GPIO_1_7		IOMUX_PAD(0x810,
> 0x3E4, 1, 0x0,   0, MX51_GPIO_PAD_CTRL) -#define
> MX51_PAD_GPIO_1_8__GPIO_1_8		IOMUX_PAD(0x814, 0x3E8, 1, 0x0,   0,
> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_9__GPIO_1_9		IOMUX_PAD(0x818,
> 0x3EC, 1, 0x0,   0, MX51_GPIO_PAD_CTRL) +#define
> MX51_PAD_GPIO_1_4__GPIO_1_4		IOMUX_PAD(0x804, 0x3D8, 0, 0x0,   0,
> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_5__GPIO_1_5		
IOMUX_PAD(0x808,
> 0x3DC, 0, 0x0,   0, MX51_GPIO_PAD_CTRL) +#define
> MX51_PAD_GPIO_1_6__GPIO_1_6		IOMUX_PAD(0x80C, 0x3E0, 0, 0x0,   0,
> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_7__GPIO_1_7		
IOMUX_PAD(0x810,
> 0x3E4, 0, 0x0,   0, MX51_GPIO_PAD_CTRL) +#define
> MX51_PAD_GPIO_1_8__GPIO_1_8		IOMUX_PAD(0x814, 0x3E8, 0, 0x0,   0,
> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_9__GPIO_1_9		
IOMUX_PAD(0x818,
> 0x3EC, 0, 0x0,   0, MX51_GPIO_PAD_CTRL)
> 
>  #endif /* __MACH_IOMUX_MX51_H__ */
> 

Maybe you should CC the freescale guy on this, CCed
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[patch v4 05/10] imx51: add gpio mode for csi1 {h,v}sync

[Marek Vasut]

On Wednesday 27 October 2010 14:40:50 Arnaud Patard wrote:
> Add definitions for configuring CSI1_{H,V}SYNC as GPIO
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h
> ===================================================================
> ---
> linux-2.6-submit.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h	2010-10-
> 20 18:30:44.000000000 +0200 +++
> linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h	2010-10-20
> 18:30:48.000000000 +0200 @@ -200,7 +200,9 @@
>  #define MX51_PAD_CSI1_D18__CSI1_D18             IOMUX_PAD(0x5A4, 0x1BC, 0,
> 0x0,   0, NO_PAD_CTRL) #define MX51_PAD_CSI1_D19__CSI1_D19            
> IOMUX_PAD(0x5A8, 0x1C0, 0, 0x0,   0, NO_PAD_CTRL) #define
> MX51_PAD_CSI1_VSYNC__CSI1_VSYNC         IOMUX_PAD(0x5AC, 0x1C4, 0, 0x0,  
> 0, NO_PAD_CTRL) +#define MX51_PAD_CSI1_VSYNC__GPIO_3_14         
> IOMUX_PAD(0x5AC, 0x1C4, 3, 0x0,   0, MX51_GPIO_PAD_CTRL) #define
> MX51_PAD_CSI1_HSYNC__CSI1_HSYNC         IOMUX_PAD(0x5B0, 0x1C8, 0, 0x0,  
> 0, NO_PAD_CTRL) +#define MX51_PAD_CSI1_HSYNC__GPIO_3_15         
> IOMUX_PAD(0x5B0, 0x1C8, 3, 0x0,   0, MX51_GPIO_PAD_CTRL) #define
> MX51_PAD_CSI1_PIXCLK__CSI1_PIXCLK       IOMUX_PAD(0x5B4, 0x000, 0, 0x0,  
> 0, NO_PAD_CTRL) #define MX51_PAD_CSI1_MCLK__CSI1_MCLK          
> IOMUX_PAD(0x5B8, 0x000, 0, 0x0,   0, NO_PAD_CTRL) #define
> MX51_PAD_CSI1_PKE0__CSI1_PKE0           IOMUX_PAD(0x860, 0x000, 0, 0x0,  
> 0, NO_PAD_CTRL)
> 
> 

DTTO for 03/10 ... CC the freescale people here
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[patch v4 06/10] efikamx: add leds support

[Marek Vasut]

On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
> The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
> 13/14/15. Also, some special care is done for default trigger of blue led
> for mmc as the mmc host used is different between hw revisions
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> 11:26:16.000000000 +0200 +++
> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/i2c.h>
>  #include <linux/gpio.h>
> +#include <linux/leds.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/fsl_devices.h>
> @@ -43,6 +44,10 @@
>  #define EFIKAMX_PCBID1		(2*32 + 17)
>  #define EFIKAMX_PCBID2		(2*32 + 11)
> 
> +#define EFIKAMX_BLUE_LED	(2*32 + 13)
> +#define EFIKAMX_GREEN_LED	(2*32 + 14)
> +#define EFIKAMX_RED_LED		(2*32 + 15)
> +
>  /* the pci ids pin have pull up. they're driven low according to board id
> */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
>  	MX51_PAD_GPIO_1_1__ESDHC1_WP,
>  	MX51_PAD_GPIO_1_7__ESDHC2_WP,
>  	MX51_PAD_GPIO_1_8__ESDHC2_CD,
> +
> +	/* leds */
> +	MX51_PAD_CSI1_D9__GPIO_3_13,
> +	MX51_PAD_CSI1_VSYNC__GPIO_3_14,
> +	MX51_PAD_CSI1_HSYNC__GPIO_3_15,
>  };
> 
>  /* Serial ports */
> @@ -179,6 +189,37 @@
>  	}
>  }

Maybe this could be modularized ?

#ifdef CONFIG_LEDS_GPIO
... the platform_data stuff below ...

efikamx_register_leds()
{
	platform_device_register();
}
#else
static inline void efikamx_register_leds() {}
#endif

board_init()
{
...
efikamx_register_leds();
...
}

What do you think ? Cheers
> 
> +static struct gpio_led mx51_efikamx_leds[] = {
> +	{
> +		.name = "efikamx:green",
> +		.default_trigger = "default-on",
> +		.gpio = EFIKAMX_GREEN_LED,
> +	},
> +	{
> +		.name = "efikamx:red",
> +		.default_trigger = "ide-disk",
> +		.gpio = EFIKAMX_RED_LED,
> +	},
> +	{
> +		.name = "efikamx:blue",
> +		.default_trigger = "mmc0",
> +		.gpio = EFIKAMX_BLUE_LED,
> +	},
> +};
> +
> +static struct gpio_led_platform_data mx51_efikamx_leds_data = {
> +	.leds = mx51_efikamx_leds,
> +	.num_leds = ARRAY_SIZE(mx51_efikamx_leds),
> +};
> +
> +static struct platform_device mx51_efikamx_leds_device = {
> +	.name = "leds-gpio",
> +	.id = -1,
> +	.dev = {
> +		.platform_data = &mx51_efikamx_leds_data,
> +	},
> +};
> +
>  static void __init mxc_board_init(void)
>  {
>  	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
> @@ -189,8 +230,12 @@
>  	imx51_add_esdhc(0, NULL);
> 
>  	/* on < 1.2 boards both SD controllers are used */
> -	if (system_rev < 0x12)
> +	if (system_rev < 0x12) {
>  		imx51_add_esdhc(1, NULL);
> +		mx51_efikamx_leds[2].default_trigger = "mmc1";
> +	}
> +
> +	platform_device_register(&mx51_efikamx_leds_device);
>  }
> 
>  static void __init mx51_efikamx_timer_init(void)
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[patch v4 09/10] efikamx: add spi nor support

[Marek Vasut]

On Wednesday 27 October 2010 14:40:54 Arnaud Patard wrote:
> On efikamx, uboot is stored on a nor spi flash. Add support for it
> 
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> 11:39:54.000000000 +0200 +++
> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> 11:43:11.000000000 +0200 @@ -23,6 +23,8 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/fsl_devices.h>
> +#include <linux/spi/flash.h>
> +#include <linux/spi/spi.h>
> 
>  #include <mach/common.h>
>  #include <mach/hardware.h>
> @@ -51,6 +53,9 @@
> 
>  #define EFIKAMX_POWER_KEY	(1*32 + 31)
> 
> +#define EFIKAMX_SPI_CS0		(3*32 + 24)
> +#define EFIKAMX_SPI_CS1		(3*32 + 25)
> +
>  /* the pci ids pin have pull up. they're driven low according to board id
> */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -98,6 +103,14 @@
> 
>  	/* power key */
>  	MX51_PAD_PWRKEY,
> +
> +	/* spi */
> +	MX51_PAD_CSPI1_MOSI__ECSPI1_MOSI,
> +	MX51_PAD_CSPI1_MISO__ECSPI1_MISO,
> +	MX51_PAD_CSPI1_SS0__GPIO_4_24,
> +	MX51_PAD_CSPI1_SS1__GPIO_4_25,
> +	MX51_PAD_CSPI1_RDY__ECSPI1_RDY,
> +	MX51_PAD_CSPI1_SCLK__ECSPI1_SCLK,
>  };
> 
>  /* Serial ports */
> @@ -243,6 +256,47 @@
>  	.nbuttons = ARRAY_SIZE(mx51_efikamx_powerkey),
>  };
> 
> +static struct mtd_partition mx51_efikamx_spi_nor_partitions[] = {
> +	{
> +	 .name = "u-boot",
> +	 .offset = 0,
> +	 .size = SZ_256K,
> +	},
> +	{
> +	  .name = "config",
> +	  .offset = MTDPART_OFS_APPEND,
> +	  .size = SZ_64K,
> +	},
> +};
> +
> +static struct flash_platform_data mx51_efikamx_spi_flash_data = {
> +	.name		= "spi_flash",
> +	.parts		= mx51_efikamx_spi_nor_partitions,
> +	.nr_parts	= ARRAY_SIZE(mx51_efikamx_spi_nor_partitions),
> +	.type		= "sst25vf032b",
> +};

If you use this kind of indent here, why don't you use it also everywhere else 
(indent structures with TAB) ?

btw. please run checkpatch on your patches (it's in scripts/checkpatch.pl)
> +
> +static struct spi_board_info mx51_efikamx_spi_board_info[] __initdata = {
> +	{
> +		.modalias = "m25p80",
> +		.max_speed_hz = 25000000,
> +		.bus_num = 0,
> +		.chip_select = 1,
> +		.platform_data = &mx51_efikamx_spi_flash_data,
> +		.irq = -1,
> +	},
> +};
> +
> +static int mx51_efikamx_spi_cs[] = {
> +	EFIKAMX_SPI_CS0,
> +	EFIKAMX_SPI_CS1,
> +};
> +
> +static const struct spi_imx_master mx51_efikamx_spi_pdata __initconst = {
> +	.chipselect     = mx51_efikamx_spi_cs,
> +	.num_chipselect = ARRAY_SIZE(mx51_efikamx_spi_cs),
> +};
> +
>  static void __init mxc_board_init(void)
>  {
>  	mxc_iomux_v3_setup_multiple_pads(mx51efikamx_pads,
> @@ -260,6 +314,10 @@
> 
>  	platform_device_register(&mx51_efikamx_leds_device);
>  	imx51_add_gpio_keys(&mx51_efikamx_powerkey_data);
> +
> +	spi_register_board_info(mx51_efikamx_spi_board_info,
> +		ARRAY_SIZE(mx51_efikamx_spi_board_info));
> +	imx51_add_ecspi(0, &mx51_efikamx_spi_pdata);
>  }
> 
>  static void __init mx51_efikamx_timer_init(void)
> Index: linux-2.6-submit/arch/arm/mach-mx5/Kconfig
> ===================================================================
> --- linux-2.6-submit.orig/arch/arm/mach-mx5/Kconfig	2010-10-27
> 11:39:39.000000000 +0200 +++
> linux-2.6-submit/arch/arm/mach-mx5/Kconfig	2010-10-27 11:43:44.000000000
> +0200 @@ -83,6 +83,7 @@
>  	bool "Support MX51 Genesi Efika MX nettop"
>  	select IMX_HAVE_PLATFORM_ESDHC
>  	select IMX_HAVE_PLATFORM_IMX_UART
> +	select IMX_HAVE_PLATFORM_SPI_IMX
>  	help
>  	  Include support for Genesi Efika MX nettop. This includes specific
>  	  configurations for the board and its peripherals.
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[patch v4 02/10] imx51: fix iomux configuration

[Xinyu Chen]

On Sat, Oct 30, 2010 at 3:50 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Wednesday 27 October 2010 14:40:47 Arnaud Patard wrote:
>> - ALT0 is used to set GPIO mode of GPIO_1_{2,3,4,5,6,7,8,9} but it's ALT1
>> for GPIO_1_{0,1}.
>>
>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> Index: linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h
>> ===================================================================
>> ---
>> linux-2.6-submit.orig/arch/arm/plat-mxc/include/mach/iomux-mx51.h ? ? 2010-10-
>> 20 18:30:30.000000000 +0200 +++
>> linux-2.6-submit/arch/arm/plat-mxc/include/mach/iomux-mx51.h ?2010-10-20
>> 18:30:42.000000000 +0200 @@ -368,18 +368,18 @@
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? MX51_SDHCI_PAD_CTRL)
>> ?#define MX51_PAD_GPIO_1_0__GPIO_1_0 ? ? ? ? ?IOMUX_PAD(0x7B4, 0x3AC, 1, 0x0,
> 0,
>> MX51_GPIO_PAD_CTRL) #define MX51_PAD_GPIO_1_1__GPIO_1_1 ? ? ? ? ? ? ? IOMUX_PAD(0x7B8,
>> 0x3B0, 1, 0x0, ? 0, MX51_GPIO_PAD_CTRL) -#define
>> MX51_PAD_GPIO_1_2__GPIO_1_2 ? ? ? ? ? IOMUX_PAD(0x7D4, 0x3CC, 1, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_2__GPIO_1_2
> IOMUX_PAD(0x7D4,
>> 0x3CC, 0, 0x0, ? 0, MX51_GPIO_PAD_CTRL) #define
>> MX51_PAD_GPIO_1_2__I2C2_SCL ? ? ? ? ? IOMUX_PAD(0x7D4, 0x3CC, (2 |
>> IOMUX_CONFIG_SION), \ 0x9b8, ? 3, MX51_I2C_PAD_CTRL)
>> -#define MX51_PAD_GPIO_1_3__GPIO_1_3 ? ? ? ? ?IOMUX_PAD(0x7D8, 0x3D0, 1, 0x0,
> 0,
>> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_3__GPIO_1_3
> IOMUX_PAD(0x7D8,
>> 0x3D0, 0, 0x0, ? 0, MX51_GPIO_PAD_CTRL) #define
>> MX51_PAD_GPIO_1_3__I2C2_SDA ? ? ? ? ? IOMUX_PAD(0x7D8, 0x3D0, (2 |
>> IOMUX_CONFIG_SION), \ 0x9bc, ? 3, MX51_I2C_PAD_CTRL)
>> ?#define MX51_PAD_PMIC_INT_REQ__PMIC_INT_REQ ?IOMUX_PAD(0x7FC, 0x3D4, 0,
>> 0x0, ? 0, NO_PAD_CTRL) -#define
>> MX51_PAD_GPIO_1_4__GPIO_1_4 ? ? ? ? ? IOMUX_PAD(0x804, 0x3D8, 1, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_5__GPIO_1_5 ? ? ? ? ? ? ?IOMUX_PAD(0x808,
>> 0x3DC, 1, 0x0, ? 0, MX51_GPIO_PAD_CTRL) -#define
>> MX51_PAD_GPIO_1_6__GPIO_1_6 ? ? ? ? ? IOMUX_PAD(0x80C, 0x3E0, 1, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_7__GPIO_1_7 ? ? ? ? ? ? ?IOMUX_PAD(0x810,
>> 0x3E4, 1, 0x0, ? 0, MX51_GPIO_PAD_CTRL) -#define
>> MX51_PAD_GPIO_1_8__GPIO_1_8 ? ? ? ? ? IOMUX_PAD(0x814, 0x3E8, 1, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) -#define MX51_PAD_GPIO_1_9__GPIO_1_9 ? ? ? ? ? ? ?IOMUX_PAD(0x818,
>> 0x3EC, 1, 0x0, ? 0, MX51_GPIO_PAD_CTRL) +#define
>> MX51_PAD_GPIO_1_4__GPIO_1_4 ? ? ? ? ? IOMUX_PAD(0x804, 0x3D8, 0, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_5__GPIO_1_5
> IOMUX_PAD(0x808,
>> 0x3DC, 0, 0x0, ? 0, MX51_GPIO_PAD_CTRL) +#define
>> MX51_PAD_GPIO_1_6__GPIO_1_6 ? ? ? ? ? IOMUX_PAD(0x80C, 0x3E0, 0, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_7__GPIO_1_7
> IOMUX_PAD(0x810,
>> 0x3E4, 0, 0x0, ? 0, MX51_GPIO_PAD_CTRL) +#define
>> MX51_PAD_GPIO_1_8__GPIO_1_8 ? ? ? ? ? IOMUX_PAD(0x814, 0x3E8, 0, 0x0, ? 0,
>> MX51_GPIO_PAD_CTRL) +#define MX51_PAD_GPIO_1_9__GPIO_1_9
> IOMUX_PAD(0x818,
>> 0x3EC, 0, 0x0, ? 0, MX51_GPIO_PAD_CTRL)
>>
>> ?#endif /* __MACH_IOMUX_MX51_H__ */
>>
>
> Maybe you should CC the freescale guy on this, CCed

This change is correct.

-- 
Best Regards
Xinyu Chen
Freescale Semiconductor
Linux BSP Team

[patch v4 06/10] efikamx: add leds support

[Uwe Kleine-König]

Hello,

On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
> > The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
> > 13/14/15. Also, some special care is done for default trigger of blue led
> > for mmc as the mmc host used is different between hw revisions
> > 
> > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> > Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> > ===================================================================
> > --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> > 11:26:16.000000000 +0200 +++
> > linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> > 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/i2c.h>
> >  #include <linux/gpio.h>
> > +#include <linux/leds.h>
> >  #include <linux/delay.h>
> >  #include <linux/io.h>
> >  #include <linux/fsl_devices.h>
> > @@ -43,6 +44,10 @@
> >  #define EFIKAMX_PCBID1		(2*32 + 17)
> >  #define EFIKAMX_PCBID2		(2*32 + 11)
> > 
> > +#define EFIKAMX_BLUE_LED	(2*32 + 13)
> > +#define EFIKAMX_GREEN_LED	(2*32 + 14)
> > +#define EFIKAMX_RED_LED		(2*32 + 15)
> > +
> >  /* the pci ids pin have pull up. they're driven low according to board id
> > */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> > PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> > 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
> >  	MX51_PAD_GPIO_1_1__ESDHC1_WP,
> >  	MX51_PAD_GPIO_1_7__ESDHC2_WP,
> >  	MX51_PAD_GPIO_1_8__ESDHC2_CD,
> > +
> > +	/* leds */
> > +	MX51_PAD_CSI1_D9__GPIO_3_13,
> > +	MX51_PAD_CSI1_VSYNC__GPIO_3_14,
> > +	MX51_PAD_CSI1_HSYNC__GPIO_3_15,
> >  };
> > 
> >  /* Serial ports */
> > @@ -179,6 +189,37 @@
> >  	}
> >  }
> 
> Maybe this could be modularized ?
> 
> #ifdef CONFIG_LEDS_GPIO
> ... the platform_data stuff below ...
> 
> efikamx_register_leds()
> {
> 	platform_device_register();
> }
> #else
> static inline void efikamx_register_leds() {}
> #endif
> 
> board_init()
> {
> ...
> efikamx_register_leds();
> ...
> }
> 
> What do you think ? Cheers
IMHO it's better to register all devices independently of the available
drivers.  So for me the used approach is OK.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch v4 01/10] efikamx: read board id

[Arnaud Patard (Rtp)]

Marek Vasut <marek.vasut@gmail.com> writes:

> On Wednesday 27 October 2010 14:40:46 Arnaud Patard wrote:
>> read board id value from the GPIO3_16/17/11
>> 
>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
>> ===================================================================
>
> This is not git, is it ... what do you use to generate this stuff ?

I'm using quilt (http://savannah.nongnu.org/projects/quilt). 

>
>> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-21
>> 08:29:23.000000000 +0200 +++
>> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-21
>> 08:29:46.000000000 +0200 @@ -39,12 +39,26 @@
>> 
>>  #define	MX51_USB_PLL_DIV_24_MHZ	0x01
>> 
>> +#define EFIKAMX_PCBID0		(2*32 + 16)
>> +#define EFIKAMX_PCBID1		(2*32 + 17)
>> +#define EFIKAMX_PCBID2		(2*32 + 11)
>> +
>> +/* the pci ids pin have pull up. they're driven low according to board id
>> */ +#define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
>> PAD_CTL_PUS_100K_UP) +#define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
>> 0x0,   0, PAD_CTL_PUS_100K_UP) +#define MX51_PAD_PCBID2	IOMUX_PAD(0x504,
>> 0x128, 3, 0x0,   0, PAD_CTL_PUS_100K_UP) +
>>  static struct pad_desc mx51efikamx_pads[] = {
>>  	/* UART1 */
>>  	MX51_PAD_UART1_RXD__UART1_RXD,
>>  	MX51_PAD_UART1_TXD__UART1_TXD,
>>  	MX51_PAD_UART1_RTS__UART1_RTS,
>>  	MX51_PAD_UART1_CTS__UART1_CTS,
>> +
>> +	/* board id */
>> +	MX51_PAD_PCBID0,
>> +	MX51_PAD_PCBID1,
>> +	MX51_PAD_PCBID2,
>>  };
>> 
>>  /* Serial ports */
>> @@ -92,10 +106,62 @@
>>  	.flags  = MXC_EHCI_INTERNAL_PHY,
>>  };
>> 
>> +/*   PCBID2  PCBID1 PCBID0  STATE
>> +	1       1      1    ER1:rev1.1
>> +	1       1      0    ER2:rev1.2
>> +	1       0      1    ER3:rev1.3
>> +	1       0      0    ER4:rev1.4
>> +*/
>> +static void __init mx51_efikamx_board_id(void)
>> +{
>> +	int id;
>> +
>> +	/* things are taking time to settle */
>> +	msleep(150);
>> +
>> +	gpio_request(EFIKAMX_PCBID0, "pcbid0");
>> +	gpio_direction_input(EFIKAMX_PCBID0);
>> +	gpio_request(EFIKAMX_PCBID1, "pcbid1");
>> +	gpio_direction_input(EFIKAMX_PCBID1);
>> +	gpio_request(EFIKAMX_PCBID2, "pcbid2");
>> +	gpio_direction_input(EFIKAMX_PCBID2);
>> +
>> +	id = gpio_get_value(EFIKAMX_PCBID0);
>> +	id |= gpio_get_value(EFIKAMX_PCBID1) << 1;
>> +	id |= gpio_get_value(EFIKAMX_PCBID2) << 2;
>> +
>> +	switch (id) {
>> +	case 7:
>> +		system_rev = 0x11;
>> +		break;
>> +	case 6:
>> +		system_rev = 0x12;
>> +		break;
>> +	case 5:
>> +		system_rev = 0x13;
>> +		break;
>> +	case 4:
>> +		system_rev = 0x14;
>> +		break;
>> +	default:
>> +		system_rev = 0x10;
>> +		break;
>> +	}
>> +
>> +	if ((system_rev == 0x10)
>> +		|| (system_rev == 0x12)
>> +		|| (system_rev == 0x14)) {
>> +		printk(KERN_WARNING
>> +			"EfikaMX: Unsupported board revision 1.%u!\n",
>> +			system_rev & 0xf);
>
> Use rather Unknown than Unsupported ... or make it sound more like a warning 
> than like a utter crash.

It's not unknown, iiuc, it's just that other boards have not been
available so they do exist but using them likely lead to troubles.

>
> Also, maybe you should free the GPIOs here again ?
>

afaik theses GPIOs are used only for that so it's not a big problem to
not free them and also this allows to look at their value through
/sys/kernel/debug/gpio easily.


Arnaud

[patch v4 06/10] efikamx: add leds support

[Arnaud Patard (Rtp)]

Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:

Hi,
> Hello,
>
> On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
>> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
>> > The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
>> > 13/14/15. Also, some special care is done for default trigger of blue led
>> > for mmc as the mmc host used is different between hw revisions
>> > 
>> > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>> > Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
>> > ===================================================================
>> > --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
>> > 11:26:16.000000000 +0200 +++
>> > linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
>> > 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
>> >  #include <linux/platform_device.h>
>> >  #include <linux/i2c.h>
>> >  #include <linux/gpio.h>
>> > +#include <linux/leds.h>
>> >  #include <linux/delay.h>
>> >  #include <linux/io.h>
>> >  #include <linux/fsl_devices.h>
>> > @@ -43,6 +44,10 @@
>> >  #define EFIKAMX_PCBID1		(2*32 + 17)
>> >  #define EFIKAMX_PCBID2		(2*32 + 11)
>> > 
>> > +#define EFIKAMX_BLUE_LED	(2*32 + 13)
>> > +#define EFIKAMX_GREEN_LED	(2*32 + 14)
>> > +#define EFIKAMX_RED_LED		(2*32 + 15)
>> > +
>> >  /* the pci ids pin have pull up. they're driven low according to board id
>> > */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
>> > PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
>> > 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
>> >  	MX51_PAD_GPIO_1_1__ESDHC1_WP,
>> >  	MX51_PAD_GPIO_1_7__ESDHC2_WP,
>> >  	MX51_PAD_GPIO_1_8__ESDHC2_CD,
>> > +
>> > +	/* leds */
>> > +	MX51_PAD_CSI1_D9__GPIO_3_13,
>> > +	MX51_PAD_CSI1_VSYNC__GPIO_3_14,
>> > +	MX51_PAD_CSI1_HSYNC__GPIO_3_15,
>> >  };
>> > 
>> >  /* Serial ports */
>> > @@ -179,6 +189,37 @@
>> >  	}
>> >  }
>> 
>> Maybe this could be modularized ?
>> 
>> #ifdef CONFIG_LEDS_GPIO
>> ... the platform_data stuff below ...
>> 
>> efikamx_register_leds()
>> {
>> 	platform_device_register();
>> }
>> #else
>> static inline void efikamx_register_leds() {}
>> #endif
>> 
>> board_init()
>> {
>> ...
>> efikamx_register_leds();
>> ...
>> }
>> 
>> What do you think ? Cheers
> IMHO it's better to register all devices independently of the available
> drivers.  So for me the used approach is OK.

I don't really see the point of doing that. It just looks a good way to
make the code harder to read imho. Moreover, it doesn't bring a lot by
doing that - if you don't want the leds-gpio stuff, just disable it at
kernel configuration and you're done.

Arnaud

[patch v4 06/10] efikamx: add leds support

[Matt Sealey]

I cant imagine any scenario where you'd not want a power light so the modular "no led gpio class" register function would better set the green one on anyway.

If you did that then I think you'd be defeating the object of modularizing. Efika mx should automatically select kconfig on led gpio and led trigger.

Matt Sealey
Product Development Analyst
Genesi USA, Inc.

On Nov 2, 2010, at 3:38 AM, Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> wrote:

> Hello,
> 
> On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
>> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
>>> The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
>>> 13/14/15. Also, some special care is done for default trigger of blue led
>>> for mmc as the mmc host used is different between hw revisions
>>> 
>>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>>> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
>>> ===================================================================
>>> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
>>> 11:26:16.000000000 +0200 +++
>>> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
>>> 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
>>> #include <linux/platform_device.h>
>>> #include <linux/i2c.h>
>>> #include <linux/gpio.h>
>>> +#include <linux/leds.h>
>>> #include <linux/delay.h>
>>> #include <linux/io.h>
>>> #include <linux/fsl_devices.h>
>>> @@ -43,6 +44,10 @@
>>> #define EFIKAMX_PCBID1        (2*32 + 17)
>>> #define EFIKAMX_PCBID2        (2*32 + 11)
>>> 
>>> +#define EFIKAMX_BLUE_LED    (2*32 + 13)
>>> +#define EFIKAMX_GREEN_LED    (2*32 + 14)
>>> +#define EFIKAMX_RED_LED        (2*32 + 15)
>>> +
>>> /* the pci ids pin have pull up. they're driven low according to board id
>>> */ #define MX51_PAD_PCBID0    IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
>>> PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1    IOMUX_PAD(0x51C, 0x134, 3,
>>> 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
>>>    MX51_PAD_GPIO_1_1__ESDHC1_WP,
>>>    MX51_PAD_GPIO_1_7__ESDHC2_WP,
>>>    MX51_PAD_GPIO_1_8__ESDHC2_CD,
>>> +
>>> +    /* leds */
>>> +    MX51_PAD_CSI1_D9__GPIO_3_13,
>>> +    MX51_PAD_CSI1_VSYNC__GPIO_3_14,
>>> +    MX51_PAD_CSI1_HSYNC__GPIO_3_15,
>>> };
>>> 
>>> /* Serial ports */
>>> @@ -179,6 +189,37 @@
>>>    }
>>> }
>> 
>> Maybe this could be modularized ?
>> 
>> #ifdef CONFIG_LEDS_GPIO
>> ... the platform_data stuff below ...
>> 
>> efikamx_register_leds()
>> {
>>    platform_device_register();
>> }
>> #else
>> static inline void efikamx_register_leds() {}
>> #endif
>> 
>> board_init()
>> {
>> ...
>> efikamx_register_leds();
>> ...
>> }
>> 
>> What do you think ? Cheers
> IMHO it's better to register all devices independently of the available
> drivers.  So for me the used approach is OK.
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-K?nig            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[patch v4 06/10] efikamx: add leds support

[Alberto Panizzo]

Hi,
On mar, 2010-11-02 at 10:35 +0100, Arnaud Patard wrote:
> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:
> 
> Hi,
> > Hello,
> >
> > On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
> >> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
> >> > The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
> >> > 13/14/15. Also, some special care is done for default trigger of blue led
> >> > for mmc as the mmc host used is different between hw revisions
> >> > 
> >> > Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> >> > Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> >> > ===================================================================
> >> > --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> >> > 11:26:16.000000000 +0200 +++
> >> > linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c	2010-10-27
> >> > 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
> >> >  #include <linux/platform_device.h>
> >> >  #include <linux/i2c.h>
> >> >  #include <linux/gpio.h>
> >> > +#include <linux/leds.h>
> >> >  #include <linux/delay.h>
> >> >  #include <linux/io.h>
> >> >  #include <linux/fsl_devices.h>
> >> > @@ -43,6 +44,10 @@
> >> >  #define EFIKAMX_PCBID1		(2*32 + 17)
> >> >  #define EFIKAMX_PCBID2		(2*32 + 11)
> >> > 
> >> > +#define EFIKAMX_BLUE_LED	(2*32 + 13)
> >> > +#define EFIKAMX_GREEN_LED	(2*32 + 14)
> >> > +#define EFIKAMX_RED_LED		(2*32 + 15)
> >> > +
> >> >  /* the pci ids pin have pull up. they're driven low according to board id
> >> > */ #define MX51_PAD_PCBID0	IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> >> > PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1	IOMUX_PAD(0x51C, 0x134, 3,
> >> > 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
> >> >  	MX51_PAD_GPIO_1_1__ESDHC1_WP,
> >> >  	MX51_PAD_GPIO_1_7__ESDHC2_WP,
> >> >  	MX51_PAD_GPIO_1_8__ESDHC2_CD,
> >> > +
> >> > +	/* leds */
> >> > +	MX51_PAD_CSI1_D9__GPIO_3_13,
> >> > +	MX51_PAD_CSI1_VSYNC__GPIO_3_14,
> >> > +	MX51_PAD_CSI1_HSYNC__GPIO_3_15,
> >> >  };
> >> > 
> >> >  /* Serial ports */
> >> > @@ -179,6 +189,37 @@
> >> >  	}
> >> >  }
> >> 
> >> Maybe this could be modularized ?
> >> 
> >> #ifdef CONFIG_LEDS_GPIO
> >> ... the platform_data stuff below ...
> >> 
> >> efikamx_register_leds()
> >> {
> >> 	platform_device_register();
> >> }
> >> #else
> >> static inline void efikamx_register_leds() {}
> >> #endif
> >> 
> >> board_init()
> >> {
> >> ...
> >> efikamx_register_leds();
> >> ...
> >> }
> >> 
> >> What do you think ? Cheers
> > IMHO it's better to register all devices independently of the available
> > drivers.  So for me the used approach is OK.
> 
> I don't really see the point of doing that. It just looks a good way to
> make the code harder to read imho. Moreover, it doesn't bring a lot by
> doing that - if you don't want the leds-gpio stuff, just disable it at
> kernel configuration and you're done.
> 
> Arnaud

The great advantage of having all the supported devices registered
even if the corresponding driver is not currently built is that if you 
flash the kernel image to the board building this image with a subset 
of supported drivers and you need for some reason to change the 
software configuration (adding another driver) you don't have to rebuild
the kernel image and re-flash it on the board. Instead you can add the
new module on the filesystem and load it at runtime.

Lot of efforts were and are done to build the kernel more general as it 
can be to reduce the versioning issues with a really minimal difference
of footprint.

Best Regards,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

[patch v4 06/10] efikamx: add leds support

[Matt Sealey]

On Nov 2, 2010, at 8:46 AM, Alberto Panizzo <maramaopercheseimorto@gmail.com> wrote:

> Hi,
> On mar, 2010-11-02 at 10:35 +0100, Arnaud Patard wrote:
>> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:
>> 
>> Hi,
>>> Hello,
>>> 
>>> On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
>>>> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
>>>>> The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
>>>>> 13/14/15. Also, some special care is done for default trigger of blue led
>>>>> for mmc as the mmc host used is different between hw revisions
>>>>> 
>>>>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
>>>>> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
>>>>> ===================================================================
>>>>> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
>>>>> 11:26:16.000000000 +0200 +++
>>>>> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
>>>>> 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
>>>>> #include <linux/platform_device.h>
>>>>> #include <linux/i2c.h>
>>>>> #include <linux/gpio.h>
>>>>> +#include <linux/leds.h>
>>>>> #include <linux/delay.h>
>>>>> #include <linux/io.h>
>>>>> #include <linux/fsl_devices.h>
>>>>> @@ -43,6 +44,10 @@
>>>>> #define EFIKAMX_PCBID1        (2*32 + 17)
>>>>> #define EFIKAMX_PCBID2        (2*32 + 11)
>>>>> 
>>>>> +#define EFIKAMX_BLUE_LED    (2*32 + 13)
>>>>> +#define EFIKAMX_GREEN_LED    (2*32 + 14)
>>>>> +#define EFIKAMX_RED_LED        (2*32 + 15)
>>>>> +
>>>>> /* the pci ids pin have pull up. they're driven low according to board id
>>>>> */ #define MX51_PAD_PCBID0    IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
>>>>> PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1    IOMUX_PAD(0x51C, 0x134, 3,
>>>>> 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
>>>>>    MX51_PAD_GPIO_1_1__ESDHC1_WP,
>>>>>    MX51_PAD_GPIO_1_7__ESDHC2_WP,
>>>>>    MX51_PAD_GPIO_1_8__ESDHC2_CD,
>>>>> +
>>>>> +    /* leds */
>>>>> +    MX51_PAD_CSI1_D9__GPIO_3_13,
>>>>> +    MX51_PAD_CSI1_VSYNC__GPIO_3_14,
>>>>> +    MX51_PAD_CSI1_HSYNC__GPIO_3_15,
>>>>> };
>>>>> 
>>>>> /* Serial ports */
>>>>> @@ -179,6 +189,37 @@
>>>>>    }
>>>>> }
>>>> 
>>>> Maybe this could be modularized ?
>>>> 
>>>> #ifdef CONFIG_LEDS_GPIO
>>>> ... the platform_data stuff below ...
>>>> 
>>>> efikamx_register_leds()
>>>> {
>>>>    platform_device_register();
>>>> }
>>>> #else
>>>> static inline void efikamx_register_leds() {}
>>>> #endif
>>>> 
>>>> board_init()
>>>> {
>>>> ...
>>>> efikamx_register_leds();
>>>> ...
>>>> }
>>>> 
>>>> What do you think ? Cheers
>>> IMHO it's better to register all devices independently of the available
>>> drivers.  So for me the used approach is OK.
>> 
>> I don't really see the point of doing that. It just looks a good way to
>> make the code harder to read imho. Moreover, it doesn't bring a lot by
>> doing that - if you don't want the leds-gpio stuff, just disable it at
>> kernel configuration and you're done.
>> 
>> Arnaud
> 
> The great advantage of having all the supported devices registered
> even if the corresponding driver is not currently built is that if you 
> flash the kernel image to the board building this image with a subset 
> of supported drivers and you need for some reason to change the 
> software configuration (adding another driver) you don't have to rebuild
> the kernel image and re-flash it on the board. Instead you can add the
> new module on the filesystem and load it at runtime.
> 
> Lot of efforts were and are done to build the kernel more general as it 
> can be to reduce the versioning issues with a really minimal difference
> of footprint.

There really isn't any need to flash a kernel on the efika, it will boot from fat, ext2, mmc, pata and maybe before too long USB drives..

Matt Sealey
Product Development Analyst
Genesi USA, Inc.

[patch v4 06/10] efikamx: add leds support

[Alberto Panizzo]

On mar, 2010-11-02 at 09:16 -0500, Matt Sealey wrote:
> 
> 
> On Nov 2, 2010, at 8:46 AM, Alberto Panizzo <maramaopercheseimorto@gmail.com> wrote:
> 
> > Hi,
> > On mar, 2010-11-02 at 10:35 +0100, Arnaud Patard wrote:
> >> Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de> writes:
> >> 
> >> Hi,
> >>> Hello,
> >>> 
> >>> On Sat, Oct 30, 2010 at 09:54:47AM +0200, Marek Vasut wrote:
> >>>> On Wednesday 27 October 2010 14:40:51 Arnaud Patard wrote:
> >>>>> The efika mx a 3 leds (1 blue, 1 red, 1 green) connected on GPIOS 3
> >>>>> 13/14/15. Also, some special care is done for default trigger of blue led
> >>>>> for mmc as the mmc host used is different between hw revisions
> >>>>> 
> >>>>> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> >>>>> Index: linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c
> >>>>> ===================================================================
> >>>>> --- linux-2.6-submit.orig/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
> >>>>> 11:26:16.000000000 +0200 +++
> >>>>> linux-2.6-submit/arch/arm/mach-mx5/board-mx51_efikamx.c    2010-10-27
> >>>>> 11:27:38.000000000 +0200 @@ -18,6 +18,7 @@
> >>>>> #include <linux/platform_device.h>
> >>>>> #include <linux/i2c.h>
> >>>>> #include <linux/gpio.h>
> >>>>> +#include <linux/leds.h>
> >>>>> #include <linux/delay.h>
> >>>>> #include <linux/io.h>
> >>>>> #include <linux/fsl_devices.h>
> >>>>> @@ -43,6 +44,10 @@
> >>>>> #define EFIKAMX_PCBID1        (2*32 + 17)
> >>>>> #define EFIKAMX_PCBID2        (2*32 + 11)
> >>>>> 
> >>>>> +#define EFIKAMX_BLUE_LED    (2*32 + 13)
> >>>>> +#define EFIKAMX_GREEN_LED    (2*32 + 14)
> >>>>> +#define EFIKAMX_RED_LED        (2*32 + 15)
> >>>>> +
> >>>>> /* the pci ids pin have pull up. they're driven low according to board id
> >>>>> */ #define MX51_PAD_PCBID0    IOMUX_PAD(0x518, 0x130, 3, 0x0,   0,
> >>>>> PAD_CTL_PUS_100K_UP) #define MX51_PAD_PCBID1    IOMUX_PAD(0x51C, 0x134, 3,
> >>>>> 0x0,   0, PAD_CTL_PUS_100K_UP) @@ -81,6 +86,11 @@
> >>>>>    MX51_PAD_GPIO_1_1__ESDHC1_WP,
> >>>>>    MX51_PAD_GPIO_1_7__ESDHC2_WP,
> >>>>>    MX51_PAD_GPIO_1_8__ESDHC2_CD,
> >>>>> +
> >>>>> +    /* leds */
> >>>>> +    MX51_PAD_CSI1_D9__GPIO_3_13,
> >>>>> +    MX51_PAD_CSI1_VSYNC__GPIO_3_14,
> >>>>> +    MX51_PAD_CSI1_HSYNC__GPIO_3_15,
> >>>>> };
> >>>>> 
> >>>>> /* Serial ports */
> >>>>> @@ -179,6 +189,37 @@
> >>>>>    }
> >>>>> }
> >>>> 
> >>>> Maybe this could be modularized ?
> >>>> 
> >>>> #ifdef CONFIG_LEDS_GPIO
> >>>> ... the platform_data stuff below ...
> >>>> 
> >>>> efikamx_register_leds()
> >>>> {
> >>>>    platform_device_register();
> >>>> }
> >>>> #else
> >>>> static inline void efikamx_register_leds() {}
> >>>> #endif
> >>>> 
> >>>> board_init()
> >>>> {
> >>>> ...
> >>>> efikamx_register_leds();
> >>>> ...
> >>>> }
> >>>> 
> >>>> What do you think ? Cheers
> >>> IMHO it's better to register all devices independently of the available
> >>> drivers.  So for me the used approach is OK.
> >> 
> >> I don't really see the point of doing that. It just looks a good way to
> >> make the code harder to read imho. Moreover, it doesn't bring a lot by
> >> doing that - if you don't want the leds-gpio stuff, just disable it at
> >> kernel configuration and you're done.
> >> 
> >> Arnaud
> > 
> > The great advantage of having all the supported devices registered
> > even if the corresponding driver is not currently built is that if you 
> > flash the kernel image to the board building this image with a subset 
> > of supported drivers and you need for some reason to change the 
> > software configuration (adding another driver) you don't have to rebuild
> > the kernel image and re-flash it on the board. Instead you can add the
> > new module on the filesystem and load it at runtime.
> > 
> > Lot of efforts were and are done to build the kernel more general as it 
> > can be to reduce the versioning issues with a really minimal difference
> > of footprint.
> 
> There really isn't any need to flash a kernel on the efika, it will boot from fat, ext2, mmc, pata and maybe before too long USB drives..
> 
> Matt Sealey

Yes, now this is your point of view, but within the time this can 
change and when you will code the next machine support maybe you will
code it in another way because it logically best fit the status of the 
moment.
In my short experience in kernel coding, I've seen different machine 
code leaving the "#ifdef" way to the more general "register all that can
be supported" so this is the past experience that talk.
But anyway, your way is not wrong, it is only a little more difficult to
maintain for you and make a little more harder this task for persons 
that have to apply platform wide changes if needed.

Best Regards,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

[patch v4 06/10] efikamx: add leds support

[Sascha Hauer]

On Tue, Nov 02, 2010 at 09:38:58AM +0100, Uwe Kleine-K?nig wrote:
> > >  /* Serial ports */
> > > @@ -179,6 +189,37 @@
> > >  	}
> > >  }
> > 
> > Maybe this could be modularized ?
> > 
> > #ifdef CONFIG_LEDS_GPIO
> > ... the platform_data stuff below ...
> > 
> > efikamx_register_leds()
> > {
> > 	platform_device_register();
> > }
> > #else
> > static inline void efikamx_register_leds() {}
> > #endif
> > 
> > board_init()
> > {
> > ...
> > efikamx_register_leds();
> > ...
> > }
> > 
> > What do you think ? Cheers
> IMHO it's better to register all devices independently of the available
> drivers.  So for me the used approach is OK.

For me this approach is also ok. I just applied this whole series.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |