[RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[Paulius Zaleckas]

Signed-off-by: Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
---

 arch/arm/mach-mx1/devices.c |   43 ++++++++++++++
 arch/arm/mach-mx1/mx1ads.c  |   45 ---------------
 arch/arm/mach-mx2/mx27ads.c |  131 -------------------------------------------
 arch/arm/mach-mx2/pcm038.c  |   64 ---------------------
 arch/arm/mach-mx2/serial.c  |  127 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 170 insertions(+), 240 deletions(-)


diff --git a/arch/arm/mach-mx1/devices.c b/arch/arm/mach-mx1/devices.c
index a956441..5fd4ee3 100644
--- a/arch/arm/mach-mx1/devices.c
+++ b/arch/arm/mach-mx1/devices.c
@@ -26,6 +26,7 @@
 
 #include <mach/irqs.h>
 #include <mach/hardware.h>
+#include <mach/iomux-mx1-mx2.h>
 
 static struct resource imx_csi_resources[] = {
 	[0] = {
@@ -96,11 +97,32 @@ static struct resource imx_uart1_resources[] = {
 	},
 };
 
+static int mxc_uart1_pins[] = {
+	PC9_PF_UART1_CTS,
+	PC10_PF_UART1_RTS,
+	PC11_PF_UART1_TXD,
+	PC12_PF_UART1_RXD,
+};
+
+static int uart1_mxc_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
+			ARRAY_SIZE(mxc_uart1_pins), "UART1");
+}
+
+static void uart1_mxc_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
+			ARRAY_SIZE(mxc_uart1_pins));
+}
+
 struct platform_device imx_uart1_device = {
 	.name		= "imx-uart",
 	.id		= 0,
 	.num_resources	= ARRAY_SIZE(imx_uart1_resources),
 	.resource	= imx_uart1_resources,
+	.init		= uart1_mxc_init,
+	.exit		= uart1_mxc_exit,
 };
 
 static struct resource imx_uart2_resources[] = {
@@ -126,11 +148,32 @@ static struct resource imx_uart2_resources[] = {
 	},
 };
 
+static int mxc_uart2_pins[] = {
+	PB28_PF_UART2_CTS,
+	PB29_PF_UART2_RTS,
+	PB30_PF_UART2_TXD,
+	PB31_PF_UART2_RXD,
+};
+
+static int uart2_mxc_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart2_pins,
+			ARRAY_SIZE(mxc_uart2_pins), "UART2");
+}
+
+static void uart2_mxc_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart2_pins,
+			ARRAY_SIZE(mxc_uart2_pins));
+}
+
 struct platform_device imx_uart2_device = {
 	.name		= "imx-uart",
 	.id		= 1,
 	.num_resources	= ARRAY_SIZE(imx_uart2_resources),
 	.resource	= imx_uart2_resources,
+	.init		= uart2_mxc_init,
+	.exit		= uart2_mxc_exit,
 };
 
 static struct resource imx_rtc_resources[] = {
diff --git a/arch/arm/mach-mx1/mx1ads.c b/arch/arm/mach-mx1/mx1ads.c
index 3200cf6..086202f 100644
--- a/arch/arm/mach-mx1/mx1ads.c
+++ b/arch/arm/mach-mx1/mx1ads.c
@@ -25,60 +25,15 @@
 #include <mach/hardware.h>
 #include <mach/common.h>
 #include <mach/imx-uart.h>
-#include <mach/iomux-mx1-mx2.h>
 #include "devices.h"
 
 /*
  * UARTs platform data
  */
-static int mxc_uart1_pins[] = {
-	PC9_PF_UART1_CTS,
-	PC10_PF_UART1_RTS,
-	PC11_PF_UART1_TXD,
-	PC12_PF_UART1_RXD,
-};
-
-static int uart1_mxc_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins), "UART1");
-}
-
-static int uart1_mxc_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins));
-	return 0;
-}
-
-static int mxc_uart2_pins[] = {
-	PB28_PF_UART2_CTS,
-	PB29_PF_UART2_RTS,
-	PB30_PF_UART2_TXD,
-	PB31_PF_UART2_RXD,
-};
-
-static int uart2_mxc_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins), "UART2");
-}
-
-static int uart2_mxc_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins));
-	return 0;
-}
-
 static struct imxuart_platform_data uart_pdata[] = {
 	{
-		.init = uart1_mxc_init,
-		.exit = uart1_mxc_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart2_mxc_init,
-		.exit = uart2_mxc_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	},
 };
diff --git a/arch/arm/mach-mx2/mx27ads.c b/arch/arm/mach-mx2/mx27ads.c
index 2b5c67f..7681558 100644
--- a/arch/arm/mach-mx2/mx27ads.c
+++ b/arch/arm/mach-mx2/mx27ads.c
@@ -58,125 +58,6 @@ static struct platform_device mx27ads_nor_mtd_device = {
 	.resource = &mx27ads_flash_resource,
 };
 
-static int mxc_uart0_pins[] = {
-	PE12_PF_UART1_TXD,
-	PE13_PF_UART1_RXD,
-	PE14_PF_UART1_CTS,
-	PE15_PF_UART1_RTS
-};
-
-static int uart_mxc_port0_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart0_pins,
-			ARRAY_SIZE(mxc_uart0_pins), "UART0");
-}
-
-static int uart_mxc_port0_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart0_pins,
-			ARRAY_SIZE(mxc_uart0_pins));
-	return 0;
-}
-
-static int mxc_uart1_pins[] = {
-	PE3_PF_UART2_CTS,
-	PE4_PF_UART2_RTS,
-	PE6_PF_UART2_TXD,
-	PE7_PF_UART2_RXD
-};
-
-static int uart_mxc_port1_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins), "UART1");
-}
-
-static int uart_mxc_port1_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins));
-	return 0;
-}
-
-static int mxc_uart2_pins[] = {
-	PE8_PF_UART3_TXD,
-	PE9_PF_UART3_RXD,
-	PE10_PF_UART3_CTS,
-	PE11_PF_UART3_RTS
-};
-
-static int uart_mxc_port2_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins), "UART2");
-}
-
-static int uart_mxc_port2_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins));
-	return 0;
-}
-
-static int mxc_uart3_pins[] = {
-	PB26_AF_UART4_RTS,
-	PB28_AF_UART4_TXD,
-	PB29_AF_UART4_CTS,
-	PB31_AF_UART4_RXD
-};
-
-static int uart_mxc_port3_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart3_pins,
-			ARRAY_SIZE(mxc_uart3_pins), "UART3");
-}
-
-static int uart_mxc_port3_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart3_pins,
-			ARRAY_SIZE(mxc_uart3_pins));
-}
-
-static int mxc_uart4_pins[] = {
-	PB18_AF_UART5_TXD,
-	PB19_AF_UART5_RXD,
-	PB20_AF_UART5_CTS,
-	PB21_AF_UART5_RTS
-};
-
-static int uart_mxc_port4_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart4_pins,
-			ARRAY_SIZE(mxc_uart4_pins), "UART4");
-}
-
-static int uart_mxc_port4_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart4_pins,
-			ARRAY_SIZE(mxc_uart4_pins));
-	return 0;
-}
-
-static int mxc_uart5_pins[] = {
-	PB10_AF_UART6_TXD,
-	PB12_AF_UART6_CTS,
-	PB11_AF_UART6_RXD,
-	PB13_AF_UART6_RTS
-};
-
-static int uart_mxc_port5_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart5_pins,
-			ARRAY_SIZE(mxc_uart5_pins), "UART5");
-}
-
-static int uart_mxc_port5_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart5_pins,
-			ARRAY_SIZE(mxc_uart5_pins));
-	return 0;
-}
-
 static struct platform_device *platform_devices[] __initdata = {
 	&mx27ads_nor_mtd_device,
 };
@@ -216,28 +97,16 @@ static void gpio_fec_inactive(void)
 
 static struct imxuart_platform_data uart_pdata[] = {
 	{
-		.init = uart_mxc_port0_init,
-		.exit = uart_mxc_port0_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port1_init,
-		.exit = uart_mxc_port1_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port2_init,
-		.exit = uart_mxc_port2_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port3_init,
-		.exit = uart_mxc_port3_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port4_init,
-		.exit = uart_mxc_port4_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port5_init,
-		.exit = uart_mxc_port5_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	},
 };
diff --git a/arch/arm/mach-mx2/pcm038.c b/arch/arm/mach-mx2/pcm038.c
index dfd4156..c2747b4 100644
--- a/arch/arm/mach-mx2/pcm038.c
+++ b/arch/arm/mach-mx2/pcm038.c
@@ -81,76 +81,12 @@ static struct platform_device pcm038_nor_mtd_device = {
 	.resource = &pcm038_flash_resource,
 };
 
-static int mxc_uart0_pins[] = {
-	PE12_PF_UART1_TXD,
-	PE13_PF_UART1_RXD,
-	PE14_PF_UART1_CTS,
-	PE15_PF_UART1_RTS
-};
-
-static int uart_mxc_port0_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart0_pins,
-			ARRAY_SIZE(mxc_uart0_pins), "UART0");
-}
-
-static int uart_mxc_port0_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart0_pins,
-			ARRAY_SIZE(mxc_uart0_pins));
-	return 0;
-}
-
-static int mxc_uart1_pins[] = {
-	PE3_PF_UART2_CTS,
-	PE4_PF_UART2_RTS,
-	PE6_PF_UART2_TXD,
-	PE7_PF_UART2_RXD
-};
-
-static int uart_mxc_port1_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins), "UART1");
-}
-
-static int uart_mxc_port1_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
-			ARRAY_SIZE(mxc_uart1_pins));
-	return 0;
-}
-
-static int mxc_uart2_pins[] = { PE10_PF_UART3_CTS,
-				PE9_PF_UART3_RXD,
-				PE10_PF_UART3_CTS,
-				PE9_PF_UART3_RXD };
-
-static int uart_mxc_port2_init(struct platform_device *pdev)
-{
-	return mxc_gpio_setup_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins), "UART2");
-}
-
-static int uart_mxc_port2_exit(struct platform_device *pdev)
-{
-	mxc_gpio_release_multiple_pins(mxc_uart2_pins,
-			ARRAY_SIZE(mxc_uart2_pins));
-	return 0;
-}
-
 static struct imxuart_platform_data uart_pdata[] = {
 	{
-		.init = uart_mxc_port0_init,
-		.exit = uart_mxc_port0_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port1_init,
-		.exit = uart_mxc_port1_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	}, {
-		.init = uart_mxc_port2_init,
-		.exit = uart_mxc_port2_exit,
 		.flags = IMXUART_HAVE_RTSCTS,
 	},
 };
diff --git a/arch/arm/mach-mx2/serial.c b/arch/arm/mach-mx2/serial.c
index 16debc2..8c79664 100644
--- a/arch/arm/mach-mx2/serial.c
+++ b/arch/arm/mach-mx2/serial.c
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/serial.h>
 #include <mach/hardware.h>
+#include <mach/iomux-mx1-mx2.h>
 #include <mach/imx-uart.h>
 
 static struct resource uart0[] = {
@@ -35,11 +36,32 @@ static struct resource uart0[] = {
 	},
 };
 
+static int mxc_uart0_pins[] = {
+	PE12_PF_UART1_TXD,
+	PE13_PF_UART1_RXD,
+	PE14_PF_UART1_CTS,
+	PE15_PF_UART1_RTS
+};
+
+static int uart_mxc_port0_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart0_pins,
+			ARRAY_SIZE(mxc_uart0_pins), "UART0");
+}
+
+static void uart_mxc_port0_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart0_pins,
+			ARRAY_SIZE(mxc_uart0_pins));
+}
+
 struct platform_device mxc_uart_device0 = {
 	.name = "imx-uart",
 	.id = 0,
 	.resource = uart0,
 	.num_resources = ARRAY_SIZE(uart0),
+	.init = uart_mxc_port0_init,
+	.exit = uart_mxc_port0_exit,
 };
 
 static struct resource uart1[] = {
@@ -54,11 +76,32 @@ static struct resource uart1[] = {
 	},
 };
 
+static int mxc_uart1_pins[] = {
+	PE3_PF_UART2_CTS,
+	PE4_PF_UART2_RTS,
+	PE6_PF_UART2_TXD,
+	PE7_PF_UART2_RXD
+};
+
+static int uart_mxc_port1_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
+			ARRAY_SIZE(mxc_uart1_pins), "UART1");
+}
+
+static void uart_mxc_port1_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
+			ARRAY_SIZE(mxc_uart1_pins));
+}
+
 struct platform_device mxc_uart_device1 = {
 	.name = "imx-uart",
 	.id = 1,
 	.resource = uart1,
 	.num_resources = ARRAY_SIZE(uart1),
+	.init = uart_mxc_port1_init,
+	.exit = uart_mxc_port1_exit,
 };
 
 static struct resource uart2[] = {
@@ -73,11 +116,32 @@ static struct resource uart2[] = {
 	},
 };
 
+static int mxc_uart2_pins[] = {
+	PE8_PF_UART3_TXD,
+	PE9_PF_UART3_RXD,
+	PE10_PF_UART3_CTS,
+	PE11_PF_UART3_RTS
+};
+
+static int uart_mxc_port2_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart2_pins,
+			ARRAY_SIZE(mxc_uart2_pins), "UART2");
+}
+
+static void uart_mxc_port2_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart2_pins,
+			ARRAY_SIZE(mxc_uart2_pins));
+}
+
 struct platform_device mxc_uart_device2 = {
 	.name = "imx-uart",
 	.id = 2,
 	.resource = uart2,
 	.num_resources = ARRAY_SIZE(uart2),
+	.init = uart_mxc_port2_init,
+	.exit = uart_mxc_port2_exit,
 };
 
 static struct resource uart3[] = {
@@ -92,11 +156,32 @@ static struct resource uart3[] = {
 	},
 };
 
+static int mxc_uart3_pins[] = {
+	PB26_AF_UART4_RTS,
+	PB28_AF_UART4_TXD,
+	PB29_AF_UART4_CTS,
+	PB31_AF_UART4_RXD
+};
+
+static int uart_mxc_port3_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart3_pins,
+			ARRAY_SIZE(mxc_uart3_pins), "UART3");
+}
+
+static void uart_mxc_port3_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart3_pins,
+			ARRAY_SIZE(mxc_uart3_pins));
+}
+
 struct platform_device mxc_uart_device3 = {
 	.name = "imx-uart",
 	.id = 3,
 	.resource = uart3,
 	.num_resources = ARRAY_SIZE(uart3),
+	.init = uart_mxc_port3_init,
+	.exit = uart_mxc_port3_exit,
 };
 
 static struct resource uart4[] = {
@@ -111,11 +196,32 @@ static struct resource uart4[] = {
 	},
 };
 
+static int mxc_uart4_pins[] = {
+	PB18_AF_UART5_TXD,
+	PB19_AF_UART5_RXD,
+	PB20_AF_UART5_CTS,
+	PB21_AF_UART5_RTS
+};
+
+static int uart_mxc_port4_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart4_pins,
+			ARRAY_SIZE(mxc_uart4_pins), "UART4");
+}
+
+static void uart_mxc_port4_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart4_pins,
+			ARRAY_SIZE(mxc_uart4_pins));
+}
+
 struct platform_device mxc_uart_device4 = {
 	.name = "imx-uart",
 	.id = 4,
 	.resource = uart4,
 	.num_resources = ARRAY_SIZE(uart4),
+	.init = uart_mxc_port4_init,
+	.exit = uart_mxc_port4_exit,
 };
 
 static struct resource uart5[] = {
@@ -130,9 +236,30 @@ static struct resource uart5[] = {
 	},
 };
 
+static int mxc_uart5_pins[] = {
+	PB10_AF_UART6_TXD,
+	PB12_AF_UART6_CTS,
+	PB11_AF_UART6_RXD,
+	PB13_AF_UART6_RTS
+};
+
+static int uart_mxc_port5_init(struct platform_device *pdev)
+{
+	return mxc_gpio_setup_multiple_pins(mxc_uart5_pins,
+			ARRAY_SIZE(mxc_uart5_pins), "UART5");
+}
+
+static void uart_mxc_port5_exit(struct platform_device *pdev)
+{
+	mxc_gpio_release_multiple_pins(mxc_uart5_pins,
+			ARRAY_SIZE(mxc_uart5_pins));
+}
+
 struct platform_device mxc_uart_device5 = {
 	.name = "imx-uart",
 	.id = 5,
 	.resource = uart5,
 	.num_resources = ARRAY_SIZE(uart5),
+	.init = uart_mxc_port5_init,
+	.exit = uart_mxc_port5_exit,
 };

Re: [RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[Russell King - ARM Linux]

On Tue, Feb 24, 2009 at 05:57:37PM +0200, Paulius Zaleckas wrote:
> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
> ---
> 
>  arch/arm/mach-mx1/devices.c |   43 ++++++++++++++
>  arch/arm/mach-mx1/mx1ads.c  |   45 ---------------
>  arch/arm/mach-mx2/mx27ads.c |  131 -------------------------------------------
>  arch/arm/mach-mx2/pcm038.c  |   64 ---------------------
>  arch/arm/mach-mx2/serial.c  |  127 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 170 insertions(+), 240 deletions(-)
> 
> 
> diff --git a/arch/arm/mach-mx1/devices.c b/arch/arm/mach-mx1/devices.c
> index a956441..5fd4ee3 100644
> --- a/arch/arm/mach-mx1/devices.c
> +++ b/arch/arm/mach-mx1/devices.c
> @@ -26,6 +26,7 @@
>  
>  #include <mach/irqs.h>
>  #include <mach/hardware.h>
> +#include <mach/iomux-mx1-mx2.h>
>  
>  static struct resource imx_csi_resources[] = {
>  	[0] = {
> @@ -96,11 +97,32 @@ static struct resource imx_uart1_resources[] = {
>  	},
>  };
>  
> +static int mxc_uart1_pins[] = {
> +	PC9_PF_UART1_CTS,
> +	PC10_PF_UART1_RTS,
> +	PC11_PF_UART1_TXD,
> +	PC12_PF_UART1_RXD,
> +};
> +
> +static int uart1_mxc_init(struct platform_device *pdev)
> +{
> +	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
> +			ARRAY_SIZE(mxc_uart1_pins), "UART1");
> +}
> +
> +static void uart1_mxc_exit(struct platform_device *pdev)
> +{
> +	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
> +			ARRAY_SIZE(mxc_uart1_pins));
> +}
> +
>  struct platform_device imx_uart1_device = {
>  	.name		= "imx-uart",
>  	.id		= 0,
>  	.num_resources	= ARRAY_SIZE(imx_uart1_resources),
>  	.resource	= imx_uart1_resources,
> +	.init		= uart1_mxc_init,
> +	.exit		= uart1_mxc_exit,

I really don't like this approach to controlling multiplex pins, which
is to setup the SoC pin configuration when the driver is being bound and
to remove it when the driver is unbound.

Let's take the issue of a serial driver.

The outputs of a serial port have defined inactive levels - for TXD, RTS
and DTR, that's logic one.  If a driver is not loaded and you have a
peripheral connected to this port, you probably don't want them to see
a break level on TXD, or active RTS or DTR signal.

However, by hooking the SoC pin configuration into the binding and
unbinding of the driver, the state of the pins are indeterminent until
the driver is initialised.

I can think of other cases in hardware I've dealt with which required
careful handling of SSP signals to ensure that a flip-flop in a FPGA is
correctly set to ensure that left/right channels aren't swapped.

Basically, my point is that for 99.9% of the time, SoC pin configuration
is determined by the platform board layout, and the right place to set
this configuration up is in the board support file, just like we do on
PXA platforms.

For the 0.1% of cases where a board needs to manipulate the SoC pin
configuration depending on which drivers are loaded, doing so at driver
probe time _may_ make sense, but it feels all together cumbersome,
especially as unloading drivers has historically had question marks
over it.

Surely, for this 0.1% of cases, the right solution would be to have an
interface which allows a platform device to be unregistered, the SoC pin
mux settings changed by platform code, and the new device registered?

Finally, note that the approach of putting init/exit into struct
platform_device doesn't cater for all cases - we're still going to need
to have init/exit pointers in platform data for some platform devices,
such as MMC drivers, which have to pass parameters to those hooks.

Re: [RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[Guennadi Liakhovetski]

On Tue, 24 Feb 2009, Russell King - ARM Linux wrote:

> I really don't like this approach to controlling multiplex pins, which
> is to setup the SoC pin configuration when the driver is being bound and
> to remove it when the driver is unbound.

What do you think about the reasoning given by Sascha here: 
http://marc.info/?l=linux-arm-kernel&m=123453175927569&w=2 which is power 
saving?

> 
> Let's take the issue of a serial driver.
> 
> The outputs of a serial port have defined inactive levels - for TXD, RTS
> and DTR, that's logic one.  If a driver is not loaded and you have a
> peripheral connected to this port, you probably don't want them to see
> a break level on TXD, or active RTS or DTR signal.
> 
> However, by hooking the SoC pin configuration into the binding and
> unbinding of the driver, the state of the pins are indeterminent until
> the driver is initialised.
> 
> I can think of other cases in hardware I've dealt with which required
> careful handling of SSP signals to ensure that a flip-flop in a FPGA is
> correctly set to ensure that left/right channels aren't swapped.
> 
> Basically, my point is that for 99.9% of the time, SoC pin configuration
> is determined by the platform board layout, and the right place to set
> this configuration up is in the board support file, just like we do on
> PXA platforms.
> 
> For the 0.1% of cases where a board needs to manipulate the SoC pin
> configuration depending on which drivers are loaded, doing so at driver
> probe time _may_ make sense, but it feels all together cumbersome,
> especially as unloading drivers has historically had question marks
> over it.
> 
> Surely, for this 0.1% of cases, the right solution would be to have an
> interface which allows a platform device to be unregistered, the SoC pin
> mux settings changed by platform code, and the new device registered?
> 
> Finally, note that the approach of putting init/exit into struct
> platform_device doesn't cater for all cases - we're still going to need
> to have init/exit pointers in platform data for some platform devices,
> such as MMC drivers, which have to pass parameters to those hooks.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[Russell King - ARM Linux]

On Tue, Feb 24, 2009 at 11:09:00PM +0100, Guennadi Liakhovetski wrote:
> On Tue, 24 Feb 2009, Russell King - ARM Linux wrote:
> 
> > I really don't like this approach to controlling multiplex pins, which
> > is to setup the SoC pin configuration when the driver is being bound and
> > to remove it when the driver is unbound.
> 
> What do you think about the reasoning given by Sascha here: 
> http://marc.info/?l=linux-arm-kernel&m=123453175927569&w=2 which is power 
> saving?

That kind of power saving methodology is far better being triggered from
the use of the driver, not whether it is loaded or not if and only if it
is appropriate and necessary for the hardware.

For example, if you do this with a serial port which is connected to the
external world, and you're monitoring it, you don't want your monitoring
to see a break condition every time the port is closed and re-opened.

If you talk about an I2S codec, you'd want to configure the SoCs outputs
to the codec to be logic 0 when powering it down because it isn't being
used.

That's all unrelated to whether the driver is loaded or not, or bound to
a device or not.

Re: [RFC PATCH 2/3] mxc: move serial driver init()/exit() to platform_device

[Paulius Zaleckas]

Russell King - ARM Linux wrote:
> On Tue, Feb 24, 2009 at 05:57:37PM +0200, Paulius Zaleckas wrote:
>> Signed-off-by: Paulius Zaleckas <paulius.zaleckas@teltonika.lt>
>> ---
>>
>>  arch/arm/mach-mx1/devices.c |   43 ++++++++++++++
>>  arch/arm/mach-mx1/mx1ads.c  |   45 ---------------
>>  arch/arm/mach-mx2/mx27ads.c |  131 -------------------------------------------
>>  arch/arm/mach-mx2/pcm038.c  |   64 ---------------------
>>  arch/arm/mach-mx2/serial.c  |  127 ++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 170 insertions(+), 240 deletions(-)
>>
>>
>> diff --git a/arch/arm/mach-mx1/devices.c b/arch/arm/mach-mx1/devices.c
>> index a956441..5fd4ee3 100644
>> --- a/arch/arm/mach-mx1/devices.c
>> +++ b/arch/arm/mach-mx1/devices.c
>> @@ -26,6 +26,7 @@
>>  
>>  #include <mach/irqs.h>
>>  #include <mach/hardware.h>
>> +#include <mach/iomux-mx1-mx2.h>
>>  
>>  static struct resource imx_csi_resources[] = {
>>  	[0] = {
>> @@ -96,11 +97,32 @@ static struct resource imx_uart1_resources[] = {
>>  	},
>>  };
>>  
>> +static int mxc_uart1_pins[] = {
>> +	PC9_PF_UART1_CTS,
>> +	PC10_PF_UART1_RTS,
>> +	PC11_PF_UART1_TXD,
>> +	PC12_PF_UART1_RXD,
>> +};
>> +
>> +static int uart1_mxc_init(struct platform_device *pdev)
>> +{
>> +	return mxc_gpio_setup_multiple_pins(mxc_uart1_pins,
>> +			ARRAY_SIZE(mxc_uart1_pins), "UART1");
>> +}
>> +
>> +static void uart1_mxc_exit(struct platform_device *pdev)
>> +{
>> +	mxc_gpio_release_multiple_pins(mxc_uart1_pins,
>> +			ARRAY_SIZE(mxc_uart1_pins));
>> +}
>> +
>>  struct platform_device imx_uart1_device = {
>>  	.name		= "imx-uart",
>>  	.id		= 0,
>>  	.num_resources	= ARRAY_SIZE(imx_uart1_resources),
>>  	.resource	= imx_uart1_resources,
>> +	.init		= uart1_mxc_init,
>> +	.exit		= uart1_mxc_exit,
> 
> I really don't like this approach to controlling multiplex pins, which
> is to setup the SoC pin configuration when the driver is being bound and
> to remove it when the driver is unbound.
> 
> Let's take the issue of a serial driver.
> 
> The outputs of a serial port have defined inactive levels - for TXD, RTS
> and DTR, that's logic one.  If a driver is not loaded and you have a
> peripheral connected to this port, you probably don't want them to see
> a break level on TXD, or active RTS or DTR signal.
> 
> However, by hooking the SoC pin configuration into the binding and
> unbinding of the driver, the state of the pins are indeterminent until
> the driver is initialised.
> 
> I can think of other cases in hardware I've dealt with which required
> careful handling of SSP signals to ensure that a flip-flop in a FPGA is
> correctly set to ensure that left/right channels aren't swapped.
> 
> Basically, my point is that for 99.9% of the time, SoC pin configuration
> is determined by the platform board layout, and the right place to set
> this configuration up is in the board support file, just like we do on
> PXA platforms.

I see your point and have to agree with you.
After all that is why it was RFC!
It was quick idea by looking at MXC drivers and amount of platform_data
with init()/exit()...
Now it seems for me, like this was just bad approach.

Thanks for comments!

> For the 0.1% of cases where a board needs to manipulate the SoC pin
> configuration depending on which drivers are loaded, doing so at driver
> probe time _may_ make sense, but it feels all together cumbersome,
> especially as unloading drivers has historically had question marks
> over it.
> 
> Surely, for this 0.1% of cases, the right solution would be to have an
> interface which allows a platform device to be unregistered, the SoC pin
> mux settings changed by platform code, and the new device registered?
> 
> Finally, note that the approach of putting init/exit into struct
> platform_device doesn't cater for all cases - we're still going to need
> to have init/exit pointers in platform data for some platform devices,
> such as MMC drivers, which have to pass parameters to those hooks.