[PATCH] plat-omap changes for 2430 SDP

[PATCH] plat-omap changes for 2430 SDP

[Syed Mohammed, Khasim]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

Hi,

The attached patch adds 2430 support to arch/arm/plat-omap folder.

The following files are modified devices.c, dmtimer.c, gpio.c, mcbsp.c

Since the changes were very minimal I thought of sending them as a
single patch.

Changes:
--------
devices.c : Initialization of devices are commented for 2430, will
enable the relevant once as and when required.
dmtimer.c : Base address for Timer 1 is different from 2420.
gpio.c    : The code is modified in few places to support GPIO 5th
block.
mcbsp.c   : MUX settings are disabled for 2430 SDP.

Thanks & Regards, 
Khasim

[-- Attachment #2: plat-omap_for2430.patch --]
[-- Type: application/octet-stream, Size: 6348 bytes --]

This patch adds minimal OMAP2430 support to plat-omap files to get the kernel booting on 2430SDP.

Signed-off-by: Syed Mohammed Khasim  <x0khasim@ti.com>

Files Changed:
 devices.c |    6 +++++-
 dmtimer.c |    5 +++++
 gpio.c    |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 mcbsp.c   |   15 ++++++++++-----
 4 files changed, 70 insertions(+), 13 deletions(-)

=============================================================================
diff -purN linux-omap/arch/arm/plat-omap/devices.c mydev-linux-omap/arch/arm/plat-omap/devices.c
--- linux-omap/arch/arm/plat-omap/devices.c	2006-11-11 21:48:05.000000000 -0600
+++ mydev-linux-omap/arch/arm/plat-omap/devices.c	2006-11-13 18:07:09.000000000 -0600
@@ -493,6 +493,10 @@ static inline void omap_init_rng(void) {
  */
 static int __init omap_init_devices(void)
 {
+/*
+ * Need to enable relevant once for 2430 SDP
+ */ 
+#ifndef CONFIG_MACH_OMAP_2430SDP
 	/* please keep these calls, and their implementations above,
 	 * in alphabetical order so they're easier to sort through.
 	 */
@@ -503,7 +507,7 @@ static int __init omap_init_devices(void
 	omap_init_uwire();
 	omap_init_wdt();
 	omap_init_rng();
-
+#endif
 	return 0;
 }
 arch_initcall(omap_init_devices);
diff -purN linux-omap/arch/arm/plat-omap/dmtimer.c mydev-linux-omap/arch/arm/plat-omap/dmtimer.c
--- linux-omap/arch/arm/plat-omap/dmtimer.c	2006-11-11 21:48:05.000000000 -0600
+++ mydev-linux-omap/arch/arm/plat-omap/dmtimer.c	2006-11-13 18:08:34.000000000 -0600
@@ -100,7 +100,12 @@ static struct omap_dm_timer dm_timers[] 
 #define omap_dm_clk_disable(x) clk_disable(x)
 
 static struct omap_dm_timer dm_timers[] = {
+#ifdef CONFIG_ARCH_OMAP2420
 	{ .phys_base = 0x48028000, .irq = INT_24XX_GPTIMER1 },
+#endif
+#ifdef CONFIG_ARCH_OMAP2430
+	{ .phys_base = 0x49018000, .irq = INT_24XX_GPTIMER1 },
+#endif		
 	{ .phys_base = 0x4802a000, .irq = INT_24XX_GPTIMER2 },
 	{ .phys_base = 0x48078000, .irq = INT_24XX_GPTIMER3 },
 	{ .phys_base = 0x4807a000, .irq = INT_24XX_GPTIMER4 },
diff -purN linux-omap/arch/arm/plat-omap/gpio.c mydev-linux-omap/arch/arm/plat-omap/gpio.c
--- linux-omap/arch/arm/plat-omap/gpio.c	2006-11-11 21:48:06.000000000 -0600
+++ mydev-linux-omap/arch/arm/plat-omap/gpio.c	2006-11-13 18:56:48.000000000 -0600
@@ -86,10 +86,19 @@
 /*
  * omap24xx specific GPIO registers
  */
+#if defined(CONFIG_ARCH_OMAP2420)
 #define OMAP24XX_GPIO1_BASE		(void __iomem *)0x48018000
 #define OMAP24XX_GPIO2_BASE		(void __iomem *)0x4801a000
 #define OMAP24XX_GPIO3_BASE		(void __iomem *)0x4801c000
 #define OMAP24XX_GPIO4_BASE		(void __iomem *)0x4801e000
+#elif defined(CONFIG_ARCH_OMAP2430)
+#define OMAP24XX_GPIO1_BASE		(void __iomem *)0x4900C000
+#define OMAP24XX_GPIO2_BASE		(void __iomem *)0x4900E000
+#define OMAP24XX_GPIO3_BASE		(void __iomem *)0x49010000
+#define OMAP24XX_GPIO4_BASE		(void __iomem *)0x49012000
+#define OMAP24XX_GPIO5_BASE		(void __iomem *)0x480B6000
+#endif
+	
 #define OMAP24XX_GPIO_REVISION		0x0000
 #define OMAP24XX_GPIO_SYSCONFIG		0x0010
 #define OMAP24XX_GPIO_SYSSTATUS		0x0014
@@ -169,12 +178,23 @@ static struct gpio_bank gpio_bank_730[7]
 #endif
 
 #ifdef CONFIG_ARCH_OMAP24XX
-static struct gpio_bank gpio_bank_24xx[4] = {
-	{ OMAP24XX_GPIO1_BASE, INT_24XX_GPIO_BANK1, IH_GPIO_BASE,	METHOD_GPIO_24XX },
-	{ OMAP24XX_GPIO2_BASE, INT_24XX_GPIO_BANK2, IH_GPIO_BASE + 32,	METHOD_GPIO_24XX },
-	{ OMAP24XX_GPIO3_BASE, INT_24XX_GPIO_BANK3, IH_GPIO_BASE + 64,	METHOD_GPIO_24XX },
-	{ OMAP24XX_GPIO4_BASE, INT_24XX_GPIO_BANK4, IH_GPIO_BASE + 96,	METHOD_GPIO_24XX },
+
+#if defined(CONFIG_ARCH_OMAP2420)
+#define OMAP2_GPIO_MAX_BANK		4
+#elif defined(CONFIG_ARCH_OMAP2430)
+#define OMAP2_GPIO_MAX_BANK		5
+#endif
+
+static struct gpio_bank gpio_bank_24xx[OMAP2_GPIO_MAX_BANK] = {
+	{ OMAP24XX_GPIO1_BASE, INT_24XX_GPIO_BANK1, IH_GPIO_BASE,       METHOD_GPIO_24XX },
+	{ OMAP24XX_GPIO2_BASE, INT_24XX_GPIO_BANK2, IH_GPIO_BASE + 32,  METHOD_GPIO_24XX },
+	{ OMAP24XX_GPIO3_BASE, INT_24XX_GPIO_BANK3, IH_GPIO_BASE + 64,  METHOD_GPIO_24XX },
+	{ OMAP24XX_GPIO4_BASE, INT_24XX_GPIO_BANK4, IH_GPIO_BASE + 96,  METHOD_GPIO_24XX },
+#ifdef CONFIG_ARCH_OMAP2430
+	{ OMAP24XX_GPIO5_BASE, INT_24XX_GPIO_BANK5, IH_GPIO_BASE + 128, METHOD_GPIO_24XX },
+#endif
 };
+
 #endif
 
 static struct gpio_bank *gpio_bank;
@@ -1114,6 +1134,11 @@ static int initialized;
 static struct clk * gpio_ick;
 static struct clk * gpio_fck;
 
+#ifdef CONFIG_ARCH_OMAP2430
+static struct clk * gpio5_ick;
+static struct clk * gpio5_fck;
+#endif
+
 static int __init _omap_gpio_init(void)
 {
 	int i;
@@ -1139,7 +1164,25 @@ static int __init _omap_gpio_init(void)
 			printk("Could not get gpios_fck\n");
 		else
 			clk_enable(gpio_fck);
-	}
+
+		/*
+		 * On 2430 GPIO 5 uses CORE L4 ICLK
+		 */
+#ifdef CONFIG_ARCH_OMAP2430
+		if (cpu_is_omap2430()) {
+			gpio5_ick = clk_get(NULL, "gpio5_ick");
+			if (IS_ERR(gpio5_ick))
+				printk("Could not get gpio5_ick\n");
+			else
+				clk_enable(gpio5_ick);			
+			gpio5_fck = clk_get(NULL, "gpio5_fck");
+			if (IS_ERR(gpio5_fck))
+				printk("Could not get gpio5_fck\n");
+			else
+				clk_enable(gpio5_fck);
+		}
+#endif
+}
 
 #ifdef CONFIG_ARCH_OMAP15XX
 	if (cpu_is_omap15xx()) {
@@ -1170,7 +1213,7 @@ static int __init _omap_gpio_init(void)
 	if (cpu_is_omap24xx()) {
 		int rev;
 
-		gpio_bank_count = 4;
+		gpio_bank_count = OMAP2_GPIO_MAX_BANK;
 		gpio_bank = gpio_bank_24xx;
 		rev = omap_readl(gpio_bank[0].base + OMAP24XX_GPIO_REVISION);
 		printk(KERN_INFO "OMAP24xx GPIO hardware version %d.%d\n",
diff -purN linux-omap/arch/arm/plat-omap/mcbsp.c mydev-linux-omap/arch/arm/plat-omap/mcbsp.c
--- linux-omap/arch/arm/plat-omap/mcbsp.c	2006-11-11 21:48:06.000000000 -0600
+++ mydev-linux-omap/arch/arm/plat-omap/mcbsp.c	2006-11-13 18:21:31.000000000 -0600
@@ -227,11 +227,16 @@ static void omap_mcbsp_dsp_free(void)
 #ifdef CONFIG_ARCH_OMAP2
 static void omap2_mcbsp2_mux_setup(void)
 {
-	omap_cfg_reg(Y15_24XX_MCBSP2_CLKX);
-	omap_cfg_reg(R14_24XX_MCBSP2_FSX);
-	omap_cfg_reg(W15_24XX_MCBSP2_DR);
-	omap_cfg_reg(V15_24XX_MCBSP2_DX);
-	omap_cfg_reg(V14_24XX_GPIO117);
+	if (cpu_is_omap2420()){
+		omap_cfg_reg(Y15_24XX_MCBSP2_CLKX);
+		omap_cfg_reg(R14_24XX_MCBSP2_FSX);
+		omap_cfg_reg(W15_24XX_MCBSP2_DR);
+		omap_cfg_reg(V15_24XX_MCBSP2_DX);
+		omap_cfg_reg(V14_24XX_GPIO117);
+	}
+	/*
+	 * Need to add MUX settings for OMAP 2430 SDP
+	 */
 }
 #endif
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] plat-omap changes for 2430 SDP

[David Brownell]

Someone should also add an "musb_hdrc" platform device for
the high speed OTG controller ... I figure that would basically
live in mach-omap2/devices.c and be called from omap2430sdp_init().
(I'm assuming that controller is accessible on the SDP...)

- Dave

RE: [PATCH] plat-omap changes for 2430 SDP

[Syed Mohammed, Khasim]

Hi Dave,

At present in our tree, we have "musb_plat" 
(musb_hdrc_platform_data musb_plat) in our board_2430sdp.c. 

It's treated like flash or smc91.x device and added as another 2430SDP
device to "__initdata " list. Do you see any issues with this approach?

I think we need to discuss more on these sorts of placements. 

Thanks & Regards,
Khasim

-----Original Message-----
From: David Brownell [mailto:david-b@pacbell.net] 
Sent: Monday, November 13, 2006 8:47 PM
To: linux-omap-open-source@linux.omap.com
Cc: Syed Mohammed, Khasim
Subject: Re: [PATCH] plat-omap changes for 2430 SDP

Someone should also add an "musb_hdrc" platform device for
the high speed OTG controller ... I figure that would basically
live in mach-omap2/devices.c and be called from omap2430sdp_init().
(I'm assuming that controller is accessible on the SDP...)

- Dave

Re: [PATCH] plat-omap changes for 2430 SDP

[David Brownell]

On Monday 13 November 2006 9:26 pm, Syed Mohammed, Khasim wrote:
> Hi Dave,
> 
> At present in our tree, we have "musb_plat" 
> (musb_hdrc_platform_data musb_plat) in our board_2430sdp.c. 

That's not in current linux-omap GIT.


> It's treated like flash or smc91.x device and added as another 2430SDP
> device to "__initdata " list. Do you see any issues with this approach?

Does it bind to the musb_hdrc driver?

The basic goal is to have the musb_hdrc driver bind to that platform
device, with its pins muxed appropriately, and able to talk propery
to the external transceiver.  The approach that's been taken so far is
to wrap the pin muxing, transceiver setup, and platform device setup
into one setup routine (in "devices.c"), and call that from the various
board-specific setup routines.  That's a bit more complex than fits well
into the simple "add to static device list" ... or at least it has been
for other USB controllers (e.g. the integrated full speed ones).

- Dave




> I think we need to discuss more on these sorts of placements. 
> 
> Thanks & Regards,
> Khasim
> 
> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net] 
> Sent: Monday, November 13, 2006 8:47 PM
> To: linux-omap-open-source@linux.omap.com
> Cc: Syed Mohammed, Khasim
> Subject: Re: [PATCH] plat-omap changes for 2430 SDP
> 
> Someone should also add an "musb_hdrc" platform device for
> the high speed OTG controller ... I figure that would basically
> live in mach-omap2/devices.c and be called from omap2430sdp_init().
> (I'm assuming that controller is accessible on the SDP...)
> 
> - Dave
> 

RE: [PATCH] plat-omap changes for 2430 SDP

[Syed Mohammed, Khasim]

Dave,

>> That's not in current linux-omap GIT.

Sorry, its TI internal tree.

>>The basic goal is to have the musb_hdrc driver bind to that platform
>>device, with its pins muxed appropriately, and able to talk propery
>>to the external transceiver.  The approach that's been taken so far is
>>to wrap the pin muxing, transceiver setup, and platform device setup
>>into one setup routine (in "devices.c"), and call that from the
various
>>board-specific setup routines.  That's a bit more complex than fits
well
>>into the simple "add to static device list" ... or at least it has
been
>>for other USB controllers (e.g. the integrated full speed ones).

Thanks, will try to align to the approach described. Later this week I
will probably send our code for a quick review. 

Thanks & Regards,

Khasim

-----Original Message-----
From: David Brownell [mailto:david-b@pacbell.net] 
Sent: Tuesday, November 14, 2006 12:44 AM
To: Syed Mohammed, Khasim
Cc: linux-omap-open-source@linux.omap.com
Subject: Re: [PATCH] plat-omap changes for 2430 SDP

On Monday 13 November 2006 9:26 pm, Syed Mohammed, Khasim wrote:
> Hi Dave,
> 
> At present in our tree, we have "musb_plat" 
> (musb_hdrc_platform_data musb_plat) in our board_2430sdp.c. 

That's not in current linux-omap GIT.


> It's treated like flash or smc91.x device and added as another 2430SDP
> device to "__initdata " list. Do you see any issues with this
approach?

Does it bind to the musb_hdrc driver?

The basic goal is to have the musb_hdrc driver bind to that platform
device, with its pins muxed appropriately, and able to talk propery
to the external transceiver.  The approach that's been taken so far is
to wrap the pin muxing, transceiver setup, and platform device setup
into one setup routine (in "devices.c"), and call that from the various
board-specific setup routines.  That's a bit more complex than fits well
into the simple "add to static device list" ... or at least it has been
for other USB controllers (e.g. the integrated full speed ones).

- Dave




> I think we need to discuss more on these sorts of placements. 
> 
> Thanks & Regards,
> Khasim
> 
> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net] 
> Sent: Monday, November 13, 2006 8:47 PM
> To: linux-omap-open-source@linux.omap.com
> Cc: Syed Mohammed, Khasim
> Subject: Re: [PATCH] plat-omap changes for 2430 SDP
> 
> Someone should also add an "musb_hdrc" platform device for
> the high speed OTG controller ... I figure that would basically
> live in mach-omap2/devices.c and be called from omap2430sdp_init().
> (I'm assuming that controller is accessible on the SDP...)
> 
> - Dave
> 

Re: [PATCH] plat-omap changes for 2430 SDP

[Trilok Soni]

On 11/14/06, Syed Mohammed, Khasim <x0khasim@ti.com> wrote:
> Hi,
>
> The attached patch adds 2430 support to arch/arm/plat-omap folder.
>
> The following files are modified devices.c, dmtimer.c, gpio.c, mcbsp.c
>
> Since the changes were very minimal I thought of sending them as a
> single patch.
>
> Changes:
> --------
> devices.c : Initialization of devices are commented for 2430, will
> enable the relevant once as and when required.
> dmtimer.c : Base address for Timer 1 is different from 2420.
> gpio.c    : The code is modified in few places to support GPIO 5th

+#ifdef CONFIG_ARCH_OMAP2430
+		if (cpu_is_omap2430()) {
+			gpio5_ick = clk_get(NULL, "gpio5_ick");
+			if (IS_ERR(gpio5_ick))
+				printk("Could not get gpio5_ick\n");
+			else
+				clk_enable(gpio5_ick);			
+			gpio5_fck = clk_get(NULL, "gpio5_fck");
+			if (IS_ERR(gpio5_fck))
+				printk("Could not get gpio5_fck\n");
+			else
+				clk_enable(gpio5_fck);
+		}
+#endif
+}

As clk_get gpio5_ick can really fail, we should also put proper error
return path if there is an error in getting gpio5_fck. Actually this
is required in general for other OMAP1/2 clocks in gpio.c, also printk
with KERN_INFO will be good :)

--Trilok Soni

RE: [PATCH] plat-omap changes for 2430 SDP

[Syed Mohammed, Khasim]

Trilok,

Thanks for the review.

This file does need some minor cleanups. I was trying to get a base line
code (for 2430) with less number of differences. 

Will try to submit another patch for code cleanup (once this gets into
the tree :)

Regards,
Khasim

-----Original Message-----
From: Trilok Soni [mailto:soni.trilok@gmail.com] 
Sent: Tuesday, November 14, 2006 3:21 AM
To: Syed Mohammed, Khasim
Cc: linux-omap-open-source@linux.omap.com
Subject: Re: [PATCH] plat-omap changes for 2430 SDP

On 11/14/06, Syed Mohammed, Khasim <x0khasim@ti.com> wrote:
> Hi,
>
> The attached patch adds 2430 support to arch/arm/plat-omap folder.
>
> The following files are modified devices.c, dmtimer.c, gpio.c, mcbsp.c
>
> Since the changes were very minimal I thought of sending them as a
> single patch.
>
> Changes:
> --------
> devices.c : Initialization of devices are commented for 2430, will
> enable the relevant once as and when required.
> dmtimer.c : Base address for Timer 1 is different from 2420.
> gpio.c    : The code is modified in few places to support GPIO 5th

+#ifdef CONFIG_ARCH_OMAP2430
+		if (cpu_is_omap2430()) {
+			gpio5_ick = clk_get(NULL, "gpio5_ick");
+			if (IS_ERR(gpio5_ick))
+				printk("Could not get gpio5_ick\n");
+			else
+				clk_enable(gpio5_ick);			
+			gpio5_fck = clk_get(NULL, "gpio5_fck");
+			if (IS_ERR(gpio5_fck))
+				printk("Could not get gpio5_fck\n");
+			else
+				clk_enable(gpio5_fck);
+		}
+#endif
+}

As clk_get gpio5_ick can really fail, we should also put proper error
return path if there is an error in getting gpio5_fck. Actually this
is required in general for other OMAP1/2 clocks in gpio.c, also printk
with KERN_INFO will be good :)

--Trilok Soni

Re: [PATCH] plat-omap changes for 2430 SDP

[Tony Lindgren]

Hi,

Some comments below.

* Syed Mohammed, Khasim <x0khasim@ti.com> [061114 03:20]:
> Hi,
> 
> The attached patch adds 2430 support to arch/arm/plat-omap folder.
> 
> The following files are modified devices.c, dmtimer.c, gpio.c, mcbsp.c
> 
> Since the changes were very minimal I thought of sending them as a
> single patch.
...

> diff -purN linux-omap/arch/arm/plat-omap/dmtimer.c mydev-linux-omap/arch/arm/plat-omap/dmtimer.c
> --- linux-omap/arch/arm/plat-omap/dmtimer.c	2006-11-11 21:48:05.000000000 -0600
> +++ mydev-linux-omap/arch/arm/plat-omap/dmtimer.c	2006-11-13 18:08:34.000000000 -0600
> @@ -100,7 +100,12 @@ static struct omap_dm_timer dm_timers[] 
>  #define omap_dm_clk_disable(x) clk_disable(x)
>  
>  static struct omap_dm_timer dm_timers[] = {
> +#ifdef CONFIG_ARCH_OMAP2420
>  	{ .phys_base = 0x48028000, .irq = INT_24XX_GPTIMER1 },
> +#endif
> +#ifdef CONFIG_ARCH_OMAP2430
> +	{ .phys_base = 0x49018000, .irq = INT_24XX_GPTIMER1 },
> +#endif		
>  	{ .phys_base = 0x4802a000, .irq = INT_24XX_GPTIMER2 },
>  	{ .phys_base = 0x48078000, .irq = INT_24XX_GPTIMER3 },
>  	{ .phys_base = 0x4807a000, .irq = INT_24XX_GPTIMER4 },

Please change this to use just if (cpu_is_omap2430()) in init instead of
ifdefs. Almost all the code already uses base address + offsets, so
compiling in all the similar platforms should be easy.

Actually considering how easy it's to do, I'd rather see the existing patch
boot with both 2420 and 2430 compiled in rather than adding more ifdefs
that would make the changes more complicated later.

> diff -purN linux-omap/arch/arm/plat-omap/gpio.c mydev-linux-omap/arch/arm/plat-omap/gpio.c
> --- linux-omap/arch/arm/plat-omap/gpio.c	2006-11-11 21:48:06.000000000 -0600
> +++ mydev-linux-omap/arch/arm/plat-omap/gpio.c	2006-11-13 18:56:48.000000000 -0600
> @@ -86,10 +86,19 @@
>  /*
>   * omap24xx specific GPIO registers
>   */
> +#if defined(CONFIG_ARCH_OMAP2420)
>  #define OMAP24XX_GPIO1_BASE		(void __iomem *)0x48018000
>  #define OMAP24XX_GPIO2_BASE		(void __iomem *)0x4801a000
>  #define OMAP24XX_GPIO3_BASE		(void __iomem *)0x4801c000
>  #define OMAP24XX_GPIO4_BASE		(void __iomem *)0x4801e000
> +#elif defined(CONFIG_ARCH_OMAP2430)
> +#define OMAP24XX_GPIO1_BASE		(void __iomem *)0x4900C000
> +#define OMAP24XX_GPIO2_BASE		(void __iomem *)0x4900E000
> +#define OMAP24XX_GPIO3_BASE		(void __iomem *)0x49010000
> +#define OMAP24XX_GPIO4_BASE		(void __iomem *)0x49012000
> +#define OMAP24XX_GPIO5_BASE		(void __iomem *)0x480B6000
> +#endif
> +	
>  #define OMAP24XX_GPIO_REVISION		0x0000
>  #define OMAP24XX_GPIO_SYSCONFIG		0x0010
>  #define OMAP24XX_GPIO_SYSSTATUS		0x0014
> @@ -169,12 +178,23 @@ static struct gpio_bank gpio_bank_730[7]
>  #endif
>  
>  #ifdef CONFIG_ARCH_OMAP24XX
> -static struct gpio_bank gpio_bank_24xx[4] = {
> -	{ OMAP24XX_GPIO1_BASE, INT_24XX_GPIO_BANK1, IH_GPIO_BASE,	METHOD_GPIO_24XX },
> -	{ OMAP24XX_GPIO2_BASE, INT_24XX_GPIO_BANK2, IH_GPIO_BASE + 32,	METHOD_GPIO_24XX },
> -	{ OMAP24XX_GPIO3_BASE, INT_24XX_GPIO_BANK3, IH_GPIO_BASE + 64,	METHOD_GPIO_24XX },
> -	{ OMAP24XX_GPIO4_BASE, INT_24XX_GPIO_BANK4, IH_GPIO_BASE + 96,	METHOD_GPIO_24XX },
> +
> +#if defined(CONFIG_ARCH_OMAP2420)
> +#define OMAP2_GPIO_MAX_BANK		4
> +#elif defined(CONFIG_ARCH_OMAP2430)
> +#define OMAP2_GPIO_MAX_BANK		5
> +#endif
> +
> +static struct gpio_bank gpio_bank_24xx[OMAP2_GPIO_MAX_BANK] = {
> +	{ OMAP24XX_GPIO1_BASE, INT_24XX_GPIO_BANK1, IH_GPIO_BASE,       METHOD_GPIO_24XX },
> +	{ OMAP24XX_GPIO2_BASE, INT_24XX_GPIO_BANK2, IH_GPIO_BASE + 32,  METHOD_GPIO_24XX },
> +	{ OMAP24XX_GPIO3_BASE, INT_24XX_GPIO_BANK3, IH_GPIO_BASE + 64,  METHOD_GPIO_24XX },
> +	{ OMAP24XX_GPIO4_BASE, INT_24XX_GPIO_BANK4, IH_GPIO_BASE + 96,  METHOD_GPIO_24XX },
> +#ifdef CONFIG_ARCH_OMAP2430
> +	{ OMAP24XX_GPIO5_BASE, INT_24XX_GPIO_BANK5, IH_GPIO_BASE + 128, METHOD_GPIO_24XX },
> +#endif
>  };
> +
>  #endif
>  
>  static struct gpio_bank *gpio_bank;

Same here, please just define gpio banks for 2430 instead of ifdefs.

Regards,

Tony

RE: [PATCH] plat-omap changes for 2430 SDP

[Woodruff, Richard]

> As clk_get gpio5_ick can really fail, we should also put proper error
> return path if there is an error in getting gpio5_fck. Actually this
> is required in general for other OMAP1/2 clocks in gpio.c, also printk
> with KERN_INFO will be good :)

That is the convention and it should be followed...  But practically for
an internal clock wrapped in an is_cpu_xyz macro the only way for it to
fail today is for the name parameter to be wrong (or system corruption).
Given the API there are many possibilities which could do the get in a
more creative way, requiring a check, but that isn't usage today.  A lot
of verbosity inside of a is_cpu_xyz doesn't feel like it adds much for
its size.

Regards,
Richard W.