[PATCH] Voipac PXA270

[PATCH] Voipac PXA270

[Marek Vasut]

Hi

This patch adds basic support for the Voipac PXA270 SBC.
The device consists of the following hardware:
- PXA270 @ 520 MHz
- 256MB RAM (sparsemem, 2*128MB regions)
- 64MB NOR flash
- 640x480 LCD
- Ports: 2xUHC, 1xUDC, 1xPCMCIA, VGA, FFUART, 2xPS2, Speaker, MIC

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Voipac-PXA270.patch
Type: text/x-patch
Size: 10549 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100310/42e9225d/attachment.bin>

[PATCH] Voipac PXA270

[Mike Rapoport]

Hi Marek,

Marek Vasut wrote:
> Hi
> 
> This patch adds basic support for the Voipac PXA270 SBC.
> The device consists of the following hardware:
> - PXA270 @ 520 MHz
> - 256MB RAM (sparsemem, 2*128MB regions)
> - 64MB NOR flash
> - 640x480 LCD
> - Ports: 2xUHC, 1xUDC, 1xPCMCIA, VGA, FFUART, 2xPS2, Speaker, MIC
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

The patch looks good to me. Some nitpicking though :)

> From c2c517d966726c570a3a4fa6d7df029df4bac481 Mon Sep 17 00:00:00 2001
> From: Marek <marek.vasut@gmail.com>
> Date: Tue, 9 Mar 2010 04:04:12 +0100
> Subject: [PATCH] Voipac PXA270
> 
> This patch adds basic support for the Voipac PXA270 SBC.
> The device consists of the following hardware:
> - PXA270 @ 520 MHz
> - 256MB RAM (sparsemem, 2*128MB regions)
> - 64MB NOR flash
> - 640x480 LCD
> - Ports: 2xUHC, 1xUDC, 1xPCMCIA, VGA, FFUART, 2xPS2, Speaker, MIC
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  arch/arm/mach-pxa/Kconfig                |    6 +
>  arch/arm/mach-pxa/Makefile               |    1 +
>  arch/arm/mach-pxa/include/mach/vpac270.h |   26 +++
>  arch/arm/mach-pxa/vpac270.c              |  306 ++++++++++++++++++++++++++++++
>  4 files changed, 339 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-pxa/include/mach/vpac270.h
>  create mode 100644 arch/arm/mach-pxa/vpac270.c
> 
> diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> index bf852b0..dc871aa 100644
> --- a/arch/arm/mach-pxa/Kconfig
> +++ b/arch/arm/mach-pxa/Kconfig
> @@ -255,6 +255,12 @@ config MACH_COLIBRI320
>  	select PXA3xx
>  	select CPU_PXA320
>  
> +config MACH_VPAC270
> +	bool "Voipac PXA270"
> +	select PXA27x
> +	help
> +	  PXA270 based Single Board Computer.
> +
>  comment "End-user Products (sorted by vendor name)"
>  
>  config MACH_H4700
> diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
> index c53397e..042bc54 100644
> --- a/arch/arm/mach-pxa/Makefile
> +++ b/arch/arm/mach-pxa/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_MACH_PCM990_BASEBOARD)	+= pcm990-baseboard.o
>  obj-$(CONFIG_MACH_COLIBRI)	+= colibri-pxa270.o
>  obj-$(CONFIG_MACH_COLIBRI300)	+= colibri-pxa3xx.o colibri-pxa300.o
>  obj-$(CONFIG_MACH_COLIBRI320)	+= colibri-pxa3xx.o colibri-pxa320.o
> +obj-$(CONFIG_MACH_VPAC270)	+= vpac270.o
>  
>  # End-user Products
>  obj-$(CONFIG_MACH_H4700)	+= hx4700.o
> diff --git a/arch/arm/mach-pxa/include/mach/vpac270.h b/arch/arm/mach-pxa/include/mach/vpac270.h
> new file mode 100644
> index 0000000..8f8eaed
> --- /dev/null
> +++ b/arch/arm/mach-pxa/include/mach/vpac270.h
> @@ -0,0 +1,26 @@
> +/*
> + * GPIOs and interrupts for Voipac PXA270
> + *
> + * Copyright (C) 2010
> + * Marek Vasut <marek.vasut@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#ifndef _INCLUDE_VPAC270_H_
> +#define _INCLUDE_VPAC270_H_
> +
> +#define	GPIO1_VPAC270_USER_BTN		1
> +
> +#define	GPIO15_VPAC270_LED_ORANGE	15
> +
> +#define	GPIO81_VPAC270_BKL_ON		81
> +#define	GPIO83_VPAC270_NL_ON		83
> +
> +#define	GPIO52_VPAC270_SD_READONLY	52
> +#define	GPIO53_VPAC270_SD_DETECT_N	53
> +
> +#endif

Do these defines really deserve dedicated .h file? Why cannot they live
in the arch/arm/mach-pxa/vpac270.c?

The same applies to additional defines in arch/arm/mach-pxa/include/mach/vpac270.h
in other patches in the series. The GPIO definitions can be moved to the *vpac270.c
thus reducing pollution in arch/arm/mach-pxa/include/mach.

> diff --git a/arch/arm/mach-pxa/vpac270.c b/arch/arm/mach-pxa/vpac270.c
> new file mode 100644
> index 0000000..c83abf6
> --- /dev/null
> +++ b/arch/arm/mach-pxa/vpac270.c
> @@ -0,0 +1,306 @@
> +/*
> + * Hardware definitions for Voipac PXA270
> + *
> + * Copyright (C) 2010
> + * Marek Vasut <marek.vasut@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/gpio_keys.h>
> +#include <linux/input.h>
> +#include <linux/gpio.h>
> +#include <linux/sysdev.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/partitions.h>
> +#include <linux/mtd/physmap.h>
> +
> +#include <asm/mach-types.h>
> +#include <asm/mach/arch.h>
> +
> +#include <mach/pxa27x.h>
> +#include <mach/vpac270.h>
> +#include <mach/mmc.h>
> +#include <mach/pxafb.h>
> +
> +#include "generic.h"
> +#include "devices.h"
> +
> +/******************************************************************************
> + * Pin configuration
> + ******************************************************************************/
> +static unsigned long vpac270_pin_config[] __initdata = {
> +	/* MMC */
> +	GPIO32_MMC_CLK,
> +	GPIO92_MMC_DAT_0,
> +	GPIO109_MMC_DAT_1,
> +	GPIO110_MMC_DAT_2,
> +	GPIO111_MMC_DAT_3,
> +	GPIO112_MMC_CMD,
> +	GPIO53_GPIO,	/* SD detect */
> +	GPIO52_GPIO,	/* SD r/o switch */
> +
> +	/* GPIO KEYS */
> +	GPIO1_GPIO,	/* USER BTN */
> +
> +	/* LEDs */
> +	GPIO15_GPIO,	/* orange led */
> +
> +	/* FFUART */
> +	GPIO34_FFUART_RXD,
> +	GPIO39_FFUART_TXD,
> +	GPIO27_FFUART_RTS,
> +	GPIO100_FFUART_CTS,
> +	GPIO33_FFUART_DSR,
> +	GPIO40_FFUART_DTR,
> +	GPIO10_FFUART_DCD,
> +	GPIO38_FFUART_RI,
> +
> +	/* LCD */
> +	GPIO58_LCD_LDD_0,
> +	GPIO59_LCD_LDD_1,
> +	GPIO60_LCD_LDD_2,
> +	GPIO61_LCD_LDD_3,
> +	GPIO62_LCD_LDD_4,
> +	GPIO63_LCD_LDD_5,
> +	GPIO64_LCD_LDD_6,
> +	GPIO65_LCD_LDD_7,
> +	GPIO66_LCD_LDD_8,
> +	GPIO67_LCD_LDD_9,
> +	GPIO68_LCD_LDD_10,
> +	GPIO69_LCD_LDD_11,
> +	GPIO70_LCD_LDD_12,
> +	GPIO71_LCD_LDD_13,
> +	GPIO72_LCD_LDD_14,
> +	GPIO73_LCD_LDD_15,
> +	GPIO86_LCD_LDD_16,
> +	GPIO87_LCD_LDD_17,
> +	GPIO74_LCD_FCLK,
> +	GPIO75_LCD_LCLK,
> +	GPIO76_LCD_PCLK,
> +	GPIO77_LCD_BIAS,
> +};
> +
> +/******************************************************************************
> + * NOR Flash
> + ******************************************************************************/
> +#if defined(CONFIG_MTD_PHYSMAP) || defined(CONFIG_MTD_PHYSMAP_MODULE)
> +static struct mtd_partition vpac270_partitions[] = {
> +	{
> +		.name		= "Flash",
> +		.offset		= 0x00000000,
> +		.size		= MTDPART_SIZ_FULL,
> +		.mask_flags	= 0

not necessary

> +	}
> +};
> +
> +static struct physmap_flash_data vpac270_flash_data[] = {
> +	{
> +		.width		= 2,	/* bankwidth in bytes */
> +		.parts		= vpac270_partitions,
> +		.nr_parts	= ARRAY_SIZE(vpac270_partitions)
> +	}
> +};
> +
> +static struct resource vpac270_flash_resource = {
> +	.start	= PXA_CS0_PHYS,
> +	.end	= PXA_CS0_PHYS + SZ_64M - 1,
> +	.flags	= IORESOURCE_MEM,
> +};
> +
> +static struct platform_device vpac270_flash = {
> +	.name		= "physmap-flash",
> +	.id		= 0,
> +	.resource	= &vpac270_flash_resource,
> +	.num_resources	= 1,
> +	.dev 		= {
> +		.platform_data = vpac270_flash_data,
> +	},
> +};
> +static void __init vpac270_nor_init(void)
> +{
> +	platform_device_register(&vpac270_flash);
> +}
> +#else
> +static inline void vpac270_nor_init(void) {}
> +#endif
> +
> +/******************************************************************************
> + * SD/MMC card controller
> + ******************************************************************************/
> +#if defined(CONFIG_MMC_PXA) || defined(CONFIG_MMC_PXA_MODULE)
> +static struct pxamci_platform_data vpac270_mci_platform_data = {
> +	.ocr_mask		= MMC_VDD_32_33 | MMC_VDD_33_34,
> +	.gpio_card_detect	= GPIO53_VPAC270_SD_DETECT_N,
> +	.gpio_card_ro		= GPIO52_VPAC270_SD_READONLY,
> +	.detect_delay		= 20,
> +};
> +
> +static void __init vpac270_mmc_init(void)
> +{
> +	pxa_set_mci_info(&vpac270_mci_platform_data);
> +}
> +#else
> +static inline void vpac270_mmc_init(void) {}
> +#endif
> +
> +/******************************************************************************
> + * GPIO keys
> + ******************************************************************************/
> +#if defined(CONFIG_KEYBOARD_GPIO) || defined(CONFIG_KEYBOARD_GPIO_MODULE)
> +static struct gpio_keys_button vpac270_pxa_buttons[] = {
> +	{KEY_POWER, GPIO1_VPAC270_USER_BTN, 0, "USER BTN"},
> +};
> +
> +static struct gpio_keys_platform_data vpac270_pxa_keys_data = {
> +	.buttons	= vpac270_pxa_buttons,
> +	.nbuttons	= ARRAY_SIZE(vpac270_pxa_buttons),
> +};
> +
> +static struct platform_device vpac270_pxa_keys = {
> +	.name	= "gpio-keys",
> +	.id	= -1,
> +	.dev	= {
> +		.platform_data = &vpac270_pxa_keys_data,
> +	},
> +};
> +
> +static void __init vpac270_keys_init(void)
> +{
> +	platform_device_register(&vpac270_pxa_keys);
> +}
> +#else
> +static inline void vpac270_keys_init(void) {}
> +#endif
> +
> +/******************************************************************************
> + * LED
> + ******************************************************************************/
> +#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
> +struct gpio_led vpac270_gpio_leds[] = {
> +{
> +	.name			= "vpac270:orange:user",
> +	.default_trigger	= "none",
> +	.gpio			= GPIO15_VPAC270_LED_ORANGE,
> +	.active_low		= 1,
> +}
> +};
> +
> +static struct gpio_led_platform_data vpac270_gpio_led_info = {
> +	.leds		= vpac270_gpio_leds,
> +	.num_leds	= ARRAY_SIZE(vpac270_gpio_leds),
> +};
> +
> +static struct platform_device vpac270_leds = {
> +	.name	= "leds-gpio",
> +	.id	= -1,
> +	.dev	= {
> +		.platform_data	= &vpac270_gpio_led_info,
> +	}
> +};
> +
> +static void __init vpac270_leds_init(void)
> +{
> +	platform_device_register(&vpac270_leds);
> +}
> +#else
> +static inline void vpac270_leds_init(void) {}
> +#endif
> +
> +/******************************************************************************
> + * Framebuffer
> + ******************************************************************************/
> +#if defined(CONFIG_FB_PXA) || defined(CONFIG_FB_PXA_MODULE)
> +static struct pxafb_mode_info vpac270_lcd_modes[] = {
> +{
> +	.pixclock	= 57692,
> +	.xres		= 640,
> +	.yres		= 480,
> +	.bpp		= 32,
> +	.depth		= 18,
> +
> +	.left_margin	= 144,
> +	.right_margin	= 32,
> +	.upper_margin	= 13,
> +	.lower_margin	= 30,
> +
> +	.hsync_len	= 32,
> +	.vsync_len	= 2,
> +
> +	.sync		= FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
> +},
> +};
> +
> +static struct pxafb_mach_info vpac270_lcd_screen = {
> +	.modes		= vpac270_lcd_modes,
> +	.num_modes	= ARRAY_SIZE(vpac270_lcd_modes),
> +	.lcd_conn	= LCD_COLOR_TFT_18BPP,
> +};
> +
> +static void vpac270_lcd_power(int on, struct fb_var_screeninfo *info)
> +{
> +	gpio_set_value(GPIO81_VPAC270_BKL_ON, on);
> +}
> +
> +static void __init vpac270_lcd_init(void)
> +{
> +	int ret;
> +
> +	ret = gpio_request(GPIO81_VPAC270_BKL_ON, "BKL-ON");
> +	if (ret) {
> +		pr_err("Requesting BKL-ON GPIO failed!\n");
> +		goto err;
> +	}
> +
> +	ret = gpio_direction_output(GPIO81_VPAC270_BKL_ON, 1);
> +	if (ret) {
> +		pr_err("Setting BKL-ON GPIO direction failed!\n");
> +		goto err2;
> +	}
> +
> +	vpac270_lcd_screen.pxafb_lcd_power = vpac270_lcd_power;
> +	set_pxa_fb_info(&vpac270_lcd_screen);
> +	return;
> +
> +err2:
> +	gpio_free(GPIO81_VPAC270_BKL_ON);
> +err:
> +	return;
> +}
> +#else
> +static inline void vpac270_lcd_init(void) {}
> +#endif
> +
> +/******************************************************************************
> + * Machine init
> + ******************************************************************************/
> +static void __init vpac270_init(void)
> +{
> +	pxa2xx_mfp_config(ARRAY_AND_SIZE(vpac270_pin_config));
> +
> +	pxa_set_ffuart_info(NULL);
> +	pxa_set_btuart_info(NULL);
> +	pxa_set_stuart_info(NULL);
> +
> +	vpac270_lcd_init();
> +	vpac270_mmc_init();
> +	vpac270_nor_init();
> +	vpac270_leds_init();
> +	vpac270_keys_init();
> +}
> +
> +MACHINE_START(VPAC270, "Voipac PXA270")
> +	.phys_io	= 0x40000000,
> +	.io_pg_offst	= (io_p2v(0x40000000) >> 18) & 0xfffc,
> +	.boot_params	= 0xa0000100,
> +	.map_io		= pxa_map_io,
> +	.init_irq	= pxa27x_init_irq,
> +	.timer		= &pxa_timer,
> +	.init_machine	= vpac270_init
> +MACHINE_END
> -- 
> 1.7.0
> 

-- 
Sincerely yours,
Mike.

[PATCH] Voipac PXA270

[Marek Vasut]

Dne St 10. b?ezna 2010 07:18:40 Mike Rapoport napsal(a):
> Hi Marek,
> 
> Marek Vasut wrote:
> > Hi
> >
> > This patch adds basic support for the Voipac PXA270 SBC.
> > The device consists of the following hardware:
> > - PXA270 @ 520 MHz
> > - 256MB RAM (sparsemem, 2*128MB regions)
> > - 64MB NOR flash
> > - 640x480 LCD
> > - Ports: 2xUHC, 1xUDC, 1xPCMCIA, VGA, FFUART, 2xPS2, Speaker, MIC
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> The patch looks good to me. Some nitpicking though :)
> 
> > From c2c517d966726c570a3a4fa6d7df029df4bac481 Mon Sep 17 00:00:00 2001
> > From: Marek <marek.vasut@gmail.com>
> > Date: Tue, 9 Mar 2010 04:04:12 +0100
> > Subject: [PATCH] Voipac PXA270
> >
> > This patch adds basic support for the Voipac PXA270 SBC.
> > The device consists of the following hardware:
> > - PXA270 @ 520 MHz
> > - 256MB RAM (sparsemem, 2*128MB regions)
> > - 64MB NOR flash
> > - 640x480 LCD
> > - Ports: 2xUHC, 1xUDC, 1xPCMCIA, VGA, FFUART, 2xPS2, Speaker, MIC
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > ---
> >  arch/arm/mach-pxa/Kconfig                |    6 +
> >  arch/arm/mach-pxa/Makefile               |    1 +
> >  arch/arm/mach-pxa/include/mach/vpac270.h |   26 +++
> >  arch/arm/mach-pxa/vpac270.c              |  306
> > ++++++++++++++++++++++++++++++ 4 files changed, 339 insertions(+), 0
> > deletions(-)
> >  create mode 100644 arch/arm/mach-pxa/include/mach/vpac270.h
> >  create mode 100644 arch/arm/mach-pxa/vpac270.c
> >
> > diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig
> > index bf852b0..dc871aa 100644
> > --- a/arch/arm/mach-pxa/Kconfig
> > +++ b/arch/arm/mach-pxa/Kconfig
> > @@ -255,6 +255,12 @@ config MACH_COLIBRI320
> >  	select PXA3xx
> >  	select CPU_PXA320
> >
> > +config MACH_VPAC270
> > +	bool "Voipac PXA270"
> > +	select PXA27x
> > +	help
> > +	  PXA270 based Single Board Computer.
> > +
> >  comment "End-user Products (sorted by vendor name)"
> >
> >  config MACH_H4700
> > diff --git a/arch/arm/mach-pxa/Makefile b/arch/arm/mach-pxa/Makefile
> > index c53397e..042bc54 100644
> > --- a/arch/arm/mach-pxa/Makefile
> > +++ b/arch/arm/mach-pxa/Makefile
> > @@ -62,6 +62,7 @@ obj-$(CONFIG_MACH_PCM990_BASEBOARD)	+=
> > pcm990-baseboard.o obj-$(CONFIG_MACH_COLIBRI)	+= colibri-pxa270.o
> >  obj-$(CONFIG_MACH_COLIBRI300)	+= colibri-pxa3xx.o colibri-pxa300.o
> >  obj-$(CONFIG_MACH_COLIBRI320)	+= colibri-pxa3xx.o colibri-pxa320.o
> > +obj-$(CONFIG_MACH_VPAC270)	+= vpac270.o
> >
> >  # End-user Products
> >  obj-$(CONFIG_MACH_H4700)	+= hx4700.o
> > diff --git a/arch/arm/mach-pxa/include/mach/vpac270.h
> > b/arch/arm/mach-pxa/include/mach/vpac270.h new file mode 100644
> > index 0000000..8f8eaed
> > --- /dev/null
> > +++ b/arch/arm/mach-pxa/include/mach/vpac270.h
> > @@ -0,0 +1,26 @@
> > +/*
> > + * GPIOs and interrupts for Voipac PXA270
> > + *
> > + * Copyright (C) 2010
> > + * Marek Vasut <marek.vasut@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + */
> > +
> > +#ifndef _INCLUDE_VPAC270_H_
> > +#define _INCLUDE_VPAC270_H_
> > +
> > +#define	GPIO1_VPAC270_USER_BTN		1
> > +
> > +#define	GPIO15_VPAC270_LED_ORANGE	15
> > +
> > +#define	GPIO81_VPAC270_BKL_ON		81
> > +#define	GPIO83_VPAC270_NL_ON		83
> > +
> > +#define	GPIO52_VPAC270_SD_READONLY	52
> > +#define	GPIO53_VPAC270_SD_DETECT_N	53
> > +
> > +#endif
> 
> Do these defines really deserve dedicated .h file? Why cannot they live
> in the arch/arm/mach-pxa/vpac270.c?

I'm not moving this in. I dont want to polute the .c file with stuff that should 
obviously go into the header file. Besides there'll be more GPIOs defined and 
that'd make the .c file less readable, which I want to avoid.

> 
> The same applies to additional defines in
>  arch/arm/mach-pxa/include/mach/vpac270.h in other patches in the series.
>  The GPIO definitions can be moved to the *vpac270.c thus reducing
>  pollution in arch/arm/mach-pxa/include/mach.
> 
> > diff --git a/arch/arm/mach-pxa/vpac270.c b/arch/arm/mach-pxa/vpac270.c
> > new file mode 100644
> > index 0000000..c83abf6
> > --- /dev/null
> > +++ b/arch/arm/mach-pxa/vpac270.c
> > @@ -0,0 +1,306 @@
> > +/*
> > + * Hardware definitions for Voipac PXA270
> > + *
> > + * Copyright (C) 2010
> > + * Marek Vasut <marek.vasut@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <linux/delay.h>
> > +#include <linux/irq.h>
> > +#include <linux/gpio_keys.h>
> > +#include <linux/input.h>
> > +#include <linux/gpio.h>
> > +#include <linux/sysdev.h>
> > +#include <linux/mtd/mtd.h>
> > +#include <linux/mtd/partitions.h>
> > +#include <linux/mtd/physmap.h>
> > +
> > +#include <asm/mach-types.h>
> > +#include <asm/mach/arch.h>
> > +
> > +#include <mach/pxa27x.h>
> > +#include <mach/vpac270.h>
> > +#include <mach/mmc.h>
> > +#include <mach/pxafb.h>
> > +
> > +#include "generic.h"
> > +#include "devices.h"
> > +
> > +/***********************************************************************
> >******* + * Pin configuration
> > +
> > *************************************************************************
> >*****/ +static unsigned long vpac270_pin_config[] __initdata = {
> > +	/* MMC */
> > +	GPIO32_MMC_CLK,
> > +	GPIO92_MMC_DAT_0,
> > +	GPIO109_MMC_DAT_1,
> > +	GPIO110_MMC_DAT_2,
> > +	GPIO111_MMC_DAT_3,
> > +	GPIO112_MMC_CMD,
> > +	GPIO53_GPIO,	/* SD detect */
> > +	GPIO52_GPIO,	/* SD r/o switch */
> > +
> > +	/* GPIO KEYS */
> > +	GPIO1_GPIO,	/* USER BTN */
> > +
> > +	/* LEDs */
> > +	GPIO15_GPIO,	/* orange led */
> > +
> > +	/* FFUART */
> > +	GPIO34_FFUART_RXD,
> > +	GPIO39_FFUART_TXD,
> > +	GPIO27_FFUART_RTS,
> > +	GPIO100_FFUART_CTS,
> > +	GPIO33_FFUART_DSR,
> > +	GPIO40_FFUART_DTR,
> > +	GPIO10_FFUART_DCD,
> > +	GPIO38_FFUART_RI,
> > +
> > +	/* LCD */
> > +	GPIO58_LCD_LDD_0,
> > +	GPIO59_LCD_LDD_1,
> > +	GPIO60_LCD_LDD_2,
> > +	GPIO61_LCD_LDD_3,
> > +	GPIO62_LCD_LDD_4,
> > +	GPIO63_LCD_LDD_5,
> > +	GPIO64_LCD_LDD_6,
> > +	GPIO65_LCD_LDD_7,
> > +	GPIO66_LCD_LDD_8,
> > +	GPIO67_LCD_LDD_9,
> > +	GPIO68_LCD_LDD_10,
> > +	GPIO69_LCD_LDD_11,
> > +	GPIO70_LCD_LDD_12,
> > +	GPIO71_LCD_LDD_13,
> > +	GPIO72_LCD_LDD_14,
> > +	GPIO73_LCD_LDD_15,
> > +	GPIO86_LCD_LDD_16,
> > +	GPIO87_LCD_LDD_17,
> > +	GPIO74_LCD_FCLK,
> > +	GPIO75_LCD_LCLK,
> > +	GPIO76_LCD_PCLK,
> > +	GPIO77_LCD_BIAS,
> > +};
> > +
> > +/***********************************************************************
> >******* + * NOR Flash
> > +
> > *************************************************************************
> >*****/ +#if defined(CONFIG_MTD_PHYSMAP) ||
> > defined(CONFIG_MTD_PHYSMAP_MODULE) +static struct mtd_partition
> > vpac270_partitions[] = {
> > +	{
> > +		.name		= "Flash",
> > +		.offset		= 0x00000000,
> > +		.size		= MTDPART_SIZ_FULL,
> > +		.mask_flags	= 0
> 
> not necessary
> 
ok, updated patch attached
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Voipac-PXA270.patch
Type: text/x-patch
Size: 10530 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100310/4a12f8a1/attachment-0001.bin>

[PATCH] Voipac PXA270

[Daniel Mack]

On Wed, Mar 10, 2010 at 12:37:26PM +0100, Marek Vasut wrote:
> Dne St 10. b?ezna 2010 07:18:40 Mike Rapoport napsal(a):
> > Do these defines really deserve dedicated .h file? Why cannot they live
> > in the arch/arm/mach-pxa/vpac270.c?
> 
> I'm not moving this in. I dont want to polute the .c file with stuff that should 
> obviously go into the header file. Besides there'll be more GPIOs defined and 
> that'd make the .c file less readable, which I want to avoid.

The question is - are they shared? If they're not and will never be used
by anything else then this .c file, I'd also vote for not having an
extra header file for them.

Daniel

[PATCH] Voipac PXA270

[Marek Vasut]

Dne St 10. b?ezna 2010 14:35:29 Daniel Mack napsal(a):
> On Wed, Mar 10, 2010 at 12:37:26PM +0100, Marek Vasut wrote:
> > Dne St 10. b?ezna 2010 07:18:40 Mike Rapoport napsal(a):
> > > Do these defines really deserve dedicated .h file? Why cannot they live
> > > in the arch/arm/mach-pxa/vpac270.c?
> >
> > I'm not moving this in. I dont want to polute the .c file with stuff that
> > should obviously go into the header file. Besides there'll be more GPIOs
> > defined and that'd make the .c file less readable, which I want to avoid.
> 
> The question is - are they shared? If they're not and will never be used
> by anything else then this .c file, I'd also vote for not having an
> extra header file for them.
> 
> Daniel
> 
Yes, they are used in the PCMCIA driver. And I'd rather avoid having GPIO 
definitions scattered across multiple files.

[PATCH] Voipac PXA270

[Eric Miao]

On Wed, Mar 10, 2010 at 11:20 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dne St 10. b?ezna 2010 14:35:29 Daniel Mack napsal(a):
>> On Wed, Mar 10, 2010 at 12:37:26PM +0100, Marek Vasut wrote:
>> > Dne St 10. b?ezna 2010 07:18:40 Mike Rapoport napsal(a):
>> > > Do these defines really deserve dedicated .h file? Why cannot they live
>> > > in the arch/arm/mach-pxa/vpac270.c?
>> >
>> > I'm not moving this in. I dont want to polute the .c file with stuff that
>> > should obviously go into the header file. Besides there'll be more GPIOs
>> > defined and that'd make the .c file less readable, which I want to avoid.
>>
>> The question is - are they shared? If they're not and will never be used
>> by anything else then this .c file, I'd also vote for not having an
>> extra header file for them.
>>
>> Daniel
>>
> Yes, they are used in the PCMCIA driver. And I'd rather avoid having GPIO
> definitions scattered across multiple files.
>

Mmm... that's because PCMCIA driver is not abstracted well enough to
allow all platform specific stuffs back into board file. Once that's done,
these shared definitions could be avoided to live in a separate header
file. For now, it's OK.

[PATCH] Voipac PXA270

[Baurzhan Ismagulov]

On Mon, Mar 22, 2010 at 10:10:52AM +0800, Eric Miao wrote:
> On Wed, Mar 10, 2010 at 11:20 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Yes, they are used in the PCMCIA driver. And I'd rather avoid having GPIO
> > definitions scattered across multiple files.
> 
> Mmm... that's because PCMCIA driver is not abstracted well enough to
> allow all platform specific stuffs back into board file. Once that's done,
> these shared definitions could be avoided to live in a separate header
> file.

What is the recommended method to abstract that? For example, I have two
boards with different GPIO pins assigned to mean certain external
signals. I also ended up with GPIO definitions in a header, which is
used by the driver, and I'd like to clean this up.

Thanks in advance,
-- 
Baurzhan Ismagulov
http://www.kz-easy.com/

[PATCH] Voipac PXA270

[Daniel Mack]

On Tue, Mar 23, 2010 at 02:21:09PM +0100, Baurzhan Ismagulov wrote:
> On Mon, Mar 22, 2010 at 10:10:52AM +0800, Eric Miao wrote:
> > On Wed, Mar 10, 2010 at 11:20 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > > Yes, they are used in the PCMCIA driver. And I'd rather avoid having GPIO
> > > definitions scattered across multiple files.
> > 
> > Mmm... that's because PCMCIA driver is not abstracted well enough to
> > allow all platform specific stuffs back into board file. Once that's done,
> > these shared definitions could be avoided to live in a separate header
> > file.
> 
> What is the recommended method to abstract that? For example, I have two
> boards with different GPIO pins assigned to mean certain external
> signals. I also ended up with GPIO definitions in a header, which is
> used by the driver, and I'd like to clean this up.

Make up a struct to pass as platform_data. Fill it in your board code
and pass down a reference to the actual driver. The driver code should
then only use resources referenced by the platform data.

This is common practice, just have a look how most other driver do it.

Daniel

[PATCH] Voipac PXA270

[Baurzhan Ismagulov]

On Tue, Mar 23, 2010 at 03:07:27PM +0100, Daniel Mack wrote:
> > What is the recommended method to abstract that? For example, I have two
> > boards with different GPIO pins assigned to mean certain external
> > signals. I also ended up with GPIO definitions in a header, which is
> > used by the driver, and I'd like to clean this up.
> 
> Make up a struct to pass as platform_data. Fill it in your board code
> and pass down a reference to the actual driver. The driver code should
> then only use resources referenced by the platform data.

Thanks! This is what I was looking for.


> This is common practice, just have a look how most other driver do it.

Yes, easy when one knows what to grep for ;) . My driver didn't have
platform_device yet, so I'll have to do that first.


With kind regards,
-- 
Baurzhan Ismagulov
http://www.kz-easy.com/

[PATCH] Voipac PXA270

[Eric Miao]

> ok, updated patch attached
>

Applied to 'devel'.