mach-pxa: add AM300 platform driver
RFC 2/2 mach-pxa: add AM300 platform driver
This patch adds support for the AM300 platform driver.
--- arch/arm/mach-pxa/Kconfig | 3 + arch/arm/mach-pxa/Makefile | 1 + arch/arm/mach-pxa/am300epd.c | 335 ++++++++++++++++++++++++++++++ arch/arm/mach-pxa/gumstix.c | 6 + arch/arm/mach-pxa/include/mach/gumstix.h | 1 + 5 files changed, 346 insertions(+), 0 deletions(-) create mode 100644 arch/arm/mach-pxa/am300epd.c
arch/arm/mach-pxa/Kconfig +3 -0
@@ -40,6 +40,9 @@ choice config GUMSTIX_AM200EPD bool "Enable AM200EPD board support" +config GUMSTIX_AM300EPD + bool "Enable AM300EPD board support" + endchoice config MACH_INTELMOTE2
arch/arm/mach-pxa/Makefile +1 -0
@@ -28,6 +28,7 @@ obj-$(CONFIG_CPU_PXA930) += pxa930.o # Specific board support obj-$(CONFIG_ARCH_GUMSTIX) += gumstix.o obj-$(CONFIG_GUMSTIX_AM200EPD) += am200epd.o +obj-$(CONFIG_GUMSTIX_AM300EPD) += am300epd.o obj-$(CONFIG_ARCH_LUBBOCK) += lubbock.o obj-$(CONFIG_MACH_LOGICPD_PXA270) += lpd270.o obj-$(CONFIG_MACH_MAINSTONE) += mainstone.o
arch/arm/mach-pxa/am300epd.c new file +335 -0
335 lines not on this page · raw message
arch/arm/mach-pxa/gumstix.c +6 -0
@@ -191,6 +191,11 @@ int __attribute__((weak)) am200_init(void)return 0 ; } +int __attribute__((weak))am300_init (void ) +{ +return 0 ; +} +static void __init carrier_board_init (void ) {/*
@@ -198,6 +203,7 @@ static void __init carrier_board_init(void)* they cannot be detected programatically */ am200_init (); +am300_init (); }static void __init gumstix_init (void )
arch/arm/mach-pxa/include/mach/gumstix.h +1 -0
@@ -97,4 +97,5 @@ has detected a cable insertion; driven low otherwise. *//* for expansion boards that can't be programatically detected */ extern int am200_init (void ); +extern int am300_init (void );
Signature · 4 lines
1.5.2.3 ------------------------------------------------------------------------------
Quoted 2 lines · Jaya Kumar
This patch adds support for the AM300 platform driver.
Patch looks good, I feel I'm nit-picking. Anyway , see my comments below:
Quoted 5 lines
arch/arm/mach-pxa/am300epd.c new file +5
-0+/* register offsets for gpio control */ +#define PWR_GPIO_PIN 16 +#define CFG_GPIO_PIN 17 +#define RDY_GPIO_PIN 32 +#define DC_GPIO_PIN 48
arch/arm/mach-pxa/am300epd.c line 91
Quoted 10 lines
arch/arm/mach-pxa/am300epd.c new file +10
-0+#define RST_GPIO_PIN 49 +#define LED_GPIO_PIN 51 +#define RD_GPIO_PIN 74 +#define WR_GPIO_PIN 75 +#define CS_GPIO_PIN 76 +#define IRQ_GPIO_PIN 77 + +/* hdb bus */ +#define DB0_GPIO_PIN 58 +#define DB15_GPIO_PIN 73
indentation?
Quoted 13 lines
arch/arm/mach-pxa/am300epd.c new file +13
-0+ +static int gpios[] = {PWR_GPIO_PIN ,CFG_GPIO_PIN ,RDY_GPIO_PIN ,DC_GPIO_PIN , +RST_GPIO_PIN ,RD_GPIO_PIN ,WR_GPIO_PIN ,CS_GPIO_PIN , +IRQ_GPIO_PIN ,LED_GPIO_PIN }; +static char *gpio_names[] = {"PWR" ,"CFG" ,"RDY" ,"DC" ,"RST" ,"RD" ,"WR" , +"CS" ,"IRQ" ,"LED" }; + +static int am300_wait_for_rdy (struct broadsheetfb_par *par) +{ +unsigned long flags; +DEFINE_WAIT (wait); + +spin_lock_irqsave (par->lock, flags);
arch/arm/mach-pxa/am300epd.c line 114
Quoted 10 lines
arch/arm/mach-pxa/am300epd.c new file +10
-0+while (!gpio_get_value (RDY_GPIO_PIN )) { +prepare_to_wait (&par->waitq, &wait,TASK_INTERRUPTIBLE ); + +spin_unlock_irqrestore (par->lock, flags); +schedule (); +spin_lock_irqsave (par->lock, flags); + } +finish_wait (&par->waitq, &wait); + +spin_unlock_irqrestore (par->lock, flags);
wait_event() might be the perfect choice for the above case, unless your spin_lock_* is really doing something useful, which I doubt it is actually protecting something.
Quoted 44 lines
arch/arm/mach-pxa/am300epd.c new file +44
-0+return 0 ; + +} + +static int am300_init_gpio_regs (struct broadsheetfb_par *par) +{ +int i; +int err; +char dbname[8 ]; + +for (i =0 ; i <ARRAY_SIZE (gpios); i++) { + err =gpio_request (gpios[i], gpio_names[i]); +if (err) { +dev_err (&am300_device->dev,"failed requesting " +"gpio %s, err=%d\n" , gpio_names[i], err); + goto err_req_gpio; + } + } + +/* we also need to take care of the hdb bus */ +for (i =DB0_GPIO_PIN ; i <=DB15_GPIO_PIN ; i++) { +sprintf (dbname,"DB%d" , i); + err =gpio_request (i, dbname); +if (err) { +dev_err (&am300_device->dev,"failed requesting " +"gpio %d, err=%d\n" , i, err); +while (i >=DB0_GPIO_PIN ) +gpio_free (i--); + i =ARRAY_SIZE (gpios) -1 ; + goto err_req_gpio; + } + } + +/* setup the outputs and init values */ +gpio_direction_output (PWR_GPIO_PIN ,0 ); +gpio_direction_output (CFG_GPIO_PIN ,1 ); +gpio_direction_output (DC_GPIO_PIN ,0 ); +gpio_direction_output (RD_GPIO_PIN ,1 ); +gpio_direction_output (WR_GPIO_PIN ,1 ); +gpio_direction_output (CS_GPIO_PIN ,1 ); +gpio_direction_output (RST_GPIO_PIN ,0 ); + +/* setup the inputs */ +gpio_direction_input (RDY_GPIO_PIN );
arch/arm/mach-pxa/am300epd.c line 168
Quoted 10 lines
arch/arm/mach-pxa/am300epd.c new file +10
-0+gpio_direction_input (IRQ_GPIO_PIN ); + +/* start the hdb bus as an input */ +for (i =DB0_GPIO_PIN ; i <=DB15_GPIO_PIN ; i++) +gpio_direction_output (i,0 ); + +/* go into command mode */ +gpio_set_value (CFG_GPIO_PIN ,1 ); +gpio_set_value (RST_GPIO_PIN ,0 ); +mdelay (10 );
which deserves a msleep(10) if the timing is relaxed enough.
Quoted 14 lines
arch/arm/mach-pxa/am300epd.c new file +14
-0+gpio_set_value (RST_GPIO_PIN ,1 ); +mdelay (10 ); +am300_wait_for_rdy (par); + +return 0 ; + +err_req_gpio: +while (i >0 ) +gpio_free (gpios[i--]); + +return err; +} + +static int am300_init_board (struct broadsheetfb_par *par)
arch/arm/mach-pxa/am300epd.c line 192
Quoted 10 lines
arch/arm/mach-pxa/am300epd.c new file +10
-0+{ +int ret; + +/* setup pins */ + ret =am300_init_gpio_regs (par); +if (ret) +return ret; + +return 0 ; +}
which can be shortened to one statement 'return am300_init_gpio_regs(par)'
Quoted 1 line
+
------------------------------------------------------------------------------
Hi,
Quoted 6 lines · Eric Miao
Quoted 2 lines · Jaya Kumar
This patch adds support for the AM300 platform driver.
Patch looks good, I feel I'm nit-picking. Anyway , see my comments below:
[...]
Quoted 8 lines
arch/arm/mach-pxa/am300epd.c line 106
Quoted 8 lines
arch/arm/mach-pxa/am300epd.c new file +8
-0+static int am300_wait_for_rdy (struct broadsheetfb_par *par) +{ +unsigned long flags; +DEFINE_WAIT (wait); + +spin_lock_irqsave (par->lock, flags); +while (!gpio_get_value (RDY_GPIO_PIN )) { +prepare_to_wait (&par->waitq, &wait,TASK_INTERRUPTIBLE );
^^^^^^^^^^^^^
This should be TASK_UNINTERRUPTIBLE, or the case of an interrupted wait need to be handled in some way!
Quoted 2 lines
wait_event()might be the perfect choice for the above case, unless your
... which would also fix the above bug.
Lothar Wa���mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra���e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch���ftsf���hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | [email protected] ___________________________________________________________
-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
Quoted 15 lines · Eric Miao
Quoted 2 lines · Jaya Kumar
This patch adds support for the AM300 platform driver.
Patch looks good, I feel I'm nit-picking. Anyway , see my comments below:
Quoted 9 lines
arch/arm/mach-pxa/am300epd.c new file +9
-0+/* register offsets for gpio control */ +#define PWR_GPIO_PIN 16 +#define CFG_GPIO_PIN 17 +#define RDY_GPIO_PIN 32 +#define DC_GPIO_PIN 48 +#define RST_GPIO_PIN 49 +#define LED_GPIO_PIN 51 +#define RD_GPIO_PIN 74 +#define WR_GPIO_PIN 75
Quoted 8 lines
arch/arm/mach-pxa/am300epd.c line 91
Quoted 6 lines
arch/arm/mach-pxa/am300epd.c new file +6
-0+#define CS_GPIO_PIN 76 +#define IRQ_GPIO_PIN 77 + +/* hdb bus */ +#define DB0_GPIO_PIN 58 +#define DB15_GPIO_PIN 73indentation?
Ok, will fix.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ gpio_set_value(RST_GPIO_PIN, 0); + mdelay(10);which deserves a msleep(10) if the timing is relaxed enough.
True. Will fix.
Quoted 19 lines
Quoted 19 lines
arch/arm/mach-pxa/am300epd.c new file +18
-0+gpio_set_value (RST_GPIO_PIN ,1 ); +mdelay (10 ); +am300_wait_for_rdy (par); + +return 0 ; + +err_req_gpio: +while (i >0 ) +gpio_free (gpios[i--]); + +return err; +} + +static int am300_init_board (struct broadsheetfb_par *par) +{ +int ret; + +/* setup pins */
Quoted 9 lines
arch/arm/mach-pxa/am300epd.c line 192
Quoted 6 lines
arch/arm/mach-pxa/am300epd.c new file +6
-0+ ret =am300_init_gpio_regs (par); +if (ret) +return ret; + +return 0 ; +}which can be shortened to one statement 'return
am300_init_gpio_regs(par)'
Yes, sorry, there was other stuff I had been doing in that function while trying to trim down the board init to the minimum needed for things to work. Okay, will fix.
Thanks,
jaya
------------------------------------------------------------------------------