[GIT PULL v3] Fixes/cleanup for vt8500 serial

[GIT PULL v3] Fixes/cleanup for vt8500 serial

[Tony Prisk]

Hi Greg,

This should be the final pull request for this series, unless there are
other review comments. Changelog included.

v2:
Restore the setting of vt8500_port->uart.uartclk which was dropped in v1.
Corrected the return-on-fail of devm_request_and_ioremap to -EADDRNOTAVAIL.

v3:
Corrected the commit message for patch 3


The following changes since commit 9931faca02c604c22335f5a935a501bb2ace6e20:

  Linux 3.8-rc3 (2013-01-09 18:59:55 -0800)

are available in the git repository at:

  git://server.prisktech.co.nz/git/linuxwmt.git tags/vt8500/serial-fixes

for you to fetch changes up to 08bab1720e19e4f980e9e93536add4a7e497b38e:

  serial: tty: Cleanup code using devm_ function (2013-01-15 17:36:50 +1300)

----------------------------------------------------------------
Series of fixes/cleanup for arch-vt8500 serial driver

----------------------------------------------------------------
Tony Prisk (4):
      serial: vt8500: Fix range-checking on vt8500_uart_ports
      serial: vt8500: ioremap'd resource is never freed
      serial: vt8500: UART uses gated clock rather than 24Mhz reference
      serial: tty: Cleanup code using devm_ function

 arch/arm/boot/dts/vt8500.dtsi      |   40 +++++++++++++++++++++---
 arch/arm/boot/dts/wm8505.dtsi      |   60 ++++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/wm8650.dtsi      |   20 ++++++++++--
 drivers/tty/serial/vt8500_serial.c |   45 +++++++++++++--------------
 4 files changed, 130 insertions(+), 35 deletions(-)

[PATCH 1/4 v3] serial: vt8500: Fix range-checking on vt8500_uart_ports

[Tony Prisk]

Fix two instances where the index to vt8500_uart_ports is tested
against > VT8500_MAX_PORTS. Correct usage should be >= VT8500_MAX_PORTS.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
 drivers/tty/serial/vt8500_serial.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 8fd1814..4c4a58d 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -569,7 +569,7 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 
 	if (np)
 		port = of_alias_get_id(np, "serial");
-		if (port > VT8500_MAX_PORTS)
+		if (port >= VT8500_MAX_PORTS)
 			port = -1;
 	else
 		port = -1;
@@ -580,7 +580,7 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 					sizeof(vt8500_ports_in_use));
 	}
 
-	if (port > VT8500_MAX_PORTS)
+	if (port >= VT8500_MAX_PORTS)
 		return -ENODEV;
 
 	/* reserve the port id */
-- 
1.7.9.5

[PATCH 2/4 v3] serial: vt8500: ioremap'd resource is never freed

[Tony Prisk]

Memory mapped via ioremap call is never released. Rather than add an
iounmap call, change allocation function to devm_request_and_ioremap.

Also, change the error on failure for this call to -EADDRNOTAVAIL rather than
-ENOMEM.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
 drivers/tty/serial/vt8500_serial.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 4c4a58d..3e76dff 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -615,9 +615,9 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	snprintf(vt8500_port->name, sizeof(vt8500_port->name),
 		 "VT8500 UART%d", pdev->id);
 
-	vt8500_port->uart.membase = ioremap(mmres->start, resource_size(mmres));
+	vt8500_port->uart.membase = devm_request_and_ioremap(&pdev->dev, mmres);
 	if (!vt8500_port->uart.membase) {
-		ret = -ENOMEM;
+		ret = -EADDRNOTAVAIL;
 		goto err;
 	}
 
-- 
1.7.9.5

[PATCH 3/4 v3] serial: vt8500: UART uses gated clock rather than 24Mhz reference

[Tony Prisk]

UART modules on Wondermedia SoCs are connected via a gated clock
source, rather than directly to the 24Mhz reference clock. While
uboot enables UART0 for debugging, other UART ports are unavailable
until the clock is enabled.

This patch checks that a valid clock is actually passed from devicetree,
enables the clock in probe. This change removes the fallback when a
clock was not specified as it doesn't apply any longer (and would only
work if the UART clock was already enabled).

DTSI files are updated for VT8500, WM8505 and WM8650.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
 arch/arm/boot/dts/vt8500.dtsi      |   40 +++++++++++++++++++++---
 arch/arm/boot/dts/wm8505.dtsi      |   60 ++++++++++++++++++++++++++++++++----
 arch/arm/boot/dts/wm8650.dtsi      |   20 ++++++++++--
 drivers/tty/serial/vt8500_serial.c |   34 +++++++++++---------
 4 files changed, 127 insertions(+), 27 deletions(-)

diff --git a/arch/arm/boot/dts/vt8500.dtsi b/arch/arm/boot/dts/vt8500.dtsi
index d8645e9..cf31ced 100644
--- a/arch/arm/boot/dts/vt8500.dtsi
+++ b/arch/arm/boot/dts/vt8500.dtsi
@@ -45,6 +45,38 @@
 					compatible = "fixed-clock";
 					clock-frequency = <24000000>;
 				};
+
+				clkuart0: uart0 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <1>;
+				};
+
+				clkuart1: uart1 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <2>;
+				};
+
+				clkuart2: uart2 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <3>;
+				};
+
+				clkuart3: uart3 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <4>;
+				};
 			};
 		};
 
@@ -83,28 +115,28 @@
 			compatible = "via,vt8500-uart";
 			reg = <0xd8200000 0x1040>;
 			interrupts = <32>;
-			clocks = <&ref24>;
+			clocks = <&clkuart0>;
 		};
 
 		uart at d82b0000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd82b0000 0x1040>;
 			interrupts = <33>;
-			clocks = <&ref24>;
+			clocks = <&clkuart1>;
 		};
 
 		uart at d8210000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd8210000 0x1040>;
 			interrupts = <47>;
-			clocks = <&ref24>;
+			clocks = <&clkuart2>;
 		};
 
 		uart at d82c0000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd82c0000 0x1040>;
 			interrupts = <50>;
-			clocks = <&ref24>;
+			clocks = <&clkuart3>;
 		};
 
 		rtc at d8100000 {
diff --git a/arch/arm/boot/dts/wm8505.dtsi b/arch/arm/boot/dts/wm8505.dtsi
index 330f833..e74a1c0 100644
--- a/arch/arm/boot/dts/wm8505.dtsi
+++ b/arch/arm/boot/dts/wm8505.dtsi
@@ -59,6 +59,54 @@
 					compatible = "fixed-clock";
 					clock-frequency = <24000000>;
 				};
+
+				clkuart0: uart0 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <1>;
+				};
+
+				clkuart1: uart1 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <2>;
+				};
+
+				clkuart2: uart2 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <3>;
+				};
+
+				clkuart3: uart3 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <4>;
+				};
+
+				clkuart4: uart4 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <22>;
+				};
+
+				clkuart5: uart5 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <23>;
+				};
 			};
 		};
 
@@ -96,42 +144,42 @@
 			compatible = "via,vt8500-uart";
 			reg = <0xd8200000 0x1040>;
 			interrupts = <32>;
-			clocks = <&ref24>;
+			clocks = <&clkuart0>;
 		};
 
 		uart at d82b0000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd82b0000 0x1040>;
 			interrupts = <33>;
-			clocks = <&ref24>;
+			clocks = <&clkuart1>;
 		};
 
 		uart at d8210000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd8210000 0x1040>;
 			interrupts = <47>;
-			clocks = <&ref24>;
+			clocks = <&clkuart2>;
 		};
 
 		uart at d82c0000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd82c0000 0x1040>;
 			interrupts = <50>;
-			clocks = <&ref24>;
+			clocks = <&clkuart3>;
 		};
 
 		uart at d8370000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd8370000 0x1040>;
 			interrupts = <31>;
-			clocks = <&ref24>;
+			clocks = <&clkuart4>;
 		};
 
 		uart at d8380000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd8380000 0x1040>;
 			interrupts = <30>;
-			clocks = <&ref24>;
+			clocks = <&clkuart5>;
 		};
 
 		rtc at d8100000 {
diff --git a/arch/arm/boot/dts/wm8650.dtsi b/arch/arm/boot/dts/wm8650.dtsi
index 83b9467..e0c42ff 100644
--- a/arch/arm/boot/dts/wm8650.dtsi
+++ b/arch/arm/boot/dts/wm8650.dtsi
@@ -75,6 +75,22 @@
 					reg = <0x204>;
 				};
 
+				clkuart0: uart0 {
+ 					#clock-cells = <0>;
+ 					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <1>;
+ 				};
+ 
+				clkuart1: uart1 {
+					#clock-cells = <0>;
+					compatible = "via,vt8500-device-clock";
+					clocks = <&ref24>;
+					enable-reg = <0x250>;
+					enable-bit = <2>;
+				};
+
 				arm: arm {
 					#clock-cells = <0>;
 					compatible = "via,vt8500-device-clock";
@@ -128,14 +144,14 @@
 			compatible = "via,vt8500-uart";
 			reg = <0xd8200000 0x1040>;
 			interrupts = <32>;
-			clocks = <&ref24>;
+			clocks = <&clkuart0>;
 		};
 
 		uart at d82b0000 {
 			compatible = "via,vt8500-uart";
 			reg = <0xd82b0000 0x1040>;
 			interrupts = <33>;
-			clocks = <&ref24>;
+			clocks = <&clkuart1>;
 		};
 
 		rtc at d8100000 {
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 3e76dff..7f9e578 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -593,6 +593,23 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	if (!vt8500_port)
 		return -ENOMEM;
 
+	vt8500_port->uart.membase = devm_request_and_ioremap(&pdev->dev, mmres);
+	if (!vt8500_port->uart.membase)
+		return -EADDRNOTAVAIL;
+
+	vt8500_port->clk = of_clk_get(pdev->dev.of_node, 0);
+	if (IS_ERR(vt8500_port->clk)) {
+		dev_err(&pdev->dev, "failed to get clock\n");
+		ret = -EINVAL;
+		goto err;
+	}
+
+	ret = clk_prepare_enable(vt8500_port->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable clock\n");
+ 		goto err;
+ 	}
+
 	vt8500_port->uart.type = PORT_VT8500;
 	vt8500_port->uart.iotype = UPIO_MEM;
 	vt8500_port->uart.mapbase = mmres->start;
@@ -602,25 +619,11 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	vt8500_port->uart.line = port;
 	vt8500_port->uart.dev = &pdev->dev;
 	vt8500_port->uart.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
-
-	vt8500_port->clk = of_clk_get(pdev->dev.of_node, 0);
-	if (vt8500_port->clk) {
-		vt8500_port->uart.uartclk = clk_get_rate(vt8500_port->clk);
-	} else {
-		/* use the default of 24Mhz if not specified and warn */
-		pr_warn("%s: serial clock source not specified\n", __func__);
-		vt8500_port->uart.uartclk = 24000000;
-	}
+	vt8500_port->uart.uartclk = clk_get_rate(vt8500_port->clk);
 
 	snprintf(vt8500_port->name, sizeof(vt8500_port->name),
 		 "VT8500 UART%d", pdev->id);
 
-	vt8500_port->uart.membase = devm_request_and_ioremap(&pdev->dev, mmres);
-	if (!vt8500_port->uart.membase) {
-		ret = -EADDRNOTAVAIL;
-		goto err;
-	}
-
 	vt8500_uart_ports[port] = vt8500_port;
 
 	uart_add_one_port(&vt8500_uart_driver, &vt8500_port->uart);
@@ -639,6 +642,7 @@ static int vt8500_serial_remove(struct platform_device *pdev)
 	struct vt8500_port *vt8500_port = platform_get_drvdata(pdev);
 
 	platform_set_drvdata(pdev, NULL);
+	clk_disable_unprepare(vt8500_port->clk);
 	uart_remove_one_port(&vt8500_uart_driver, &vt8500_port->uart);
 	kfree(vt8500_port);
 
-- 
1.7.9.5

[PATCH 4/4 v3] serial: tty: Cleanup code using devm_ function

[Tony Prisk]

Convert the last memory allocation (vt8500_port) to use devm_kzalloc
and remove the fail path cleanup code from vt8500_serial_probe.

Reorder iomem mapping above clk_enable to simplify fail code. The
clock is only enabled if all other resources are available.

Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
---
 drivers/tty/serial/vt8500_serial.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 7f9e578..1fc6f3d 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -589,7 +589,8 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	vt8500_port = kzalloc(sizeof(struct vt8500_port), GFP_KERNEL);
+	vt8500_port = devm_kzalloc(&pdev->dev, sizeof(struct vt8500_port),
+				   GFP_KERNEL);
 	if (!vt8500_port)
 		return -ENOMEM;
 
@@ -600,14 +601,13 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	vt8500_port->clk = of_clk_get(pdev->dev.of_node, 0);
 	if (IS_ERR(vt8500_port->clk)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
-		ret = -EINVAL;
-		goto err;
+		return  -EINVAL;
 	}
 
 	ret = clk_prepare_enable(vt8500_port->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to enable clock\n");
- 		goto err;
+ 		return ret;
  	}
 
 	vt8500_port->uart.type = PORT_VT8500;
@@ -631,10 +631,6 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, vt8500_port);
 
 	return 0;
-
-err:
-	kfree(vt8500_port);
-	return ret;
 }
 
 static int vt8500_serial_remove(struct platform_device *pdev)
@@ -644,7 +640,6 @@ static int vt8500_serial_remove(struct platform_device *pdev)
 	platform_set_drvdata(pdev, NULL);
 	clk_disable_unprepare(vt8500_port->clk);
 	uart_remove_one_port(&vt8500_uart_driver, &vt8500_port->uart);
-	kfree(vt8500_port);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 3/4 v3] serial: vt8500: UART uses gated clock rather than 24Mhz reference

[Greg KH]

On Thu, Jan 17, 2013 at 08:05:42AM +1300, Tony Prisk wrote:
> UART modules on Wondermedia SoCs are connected via a gated clock
> source, rather than directly to the 24Mhz reference clock. While
> uboot enables UART0 for debugging, other UART ports are unavailable
> until the clock is enabled.
> 
> This patch checks that a valid clock is actually passed from devicetree,
> enables the clock in probe. This change removes the fallback when a
> clock was not specified as it doesn't apply any longer (and would only
> work if the UART clock was already enabled).
> 
> DTSI files are updated for VT8500, WM8505 and WM8650.
> 
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
>  arch/arm/boot/dts/vt8500.dtsi      |   40 +++++++++++++++++++++---
>  arch/arm/boot/dts/wm8505.dtsi      |   60 ++++++++++++++++++++++++++++++++----
>  arch/arm/boot/dts/wm8650.dtsi      |   20 ++++++++++--
>  drivers/tty/serial/vt8500_serial.c |   34 +++++++++++---------
>  4 files changed, 127 insertions(+), 27 deletions(-)

PLEASE always use checkpatch.pl on your patches before sending them, so
people don't have to write emails saying "why didn't you fix the
checkpatch.pl errors in your patch" much like this one.

Sorry, not applied.

greg k-h

[PATCH 4/4 v3] serial: vt8500: Cleanup code using devm_ function

[Greg KH]

On Thu, Jan 17, 2013 at 08:05:43AM +1300, Tony Prisk wrote:
> Convert the last memory allocation (vt8500_port) to use devm_kzalloc
> and remove the fail path cleanup code from vt8500_serial_probe.
> 
> Reorder iomem mapping above clk_enable to simplify fail code. The
> clock is only enabled if all other resources are available.
> 
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
>  drivers/tty/serial/vt8500_serial.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)

As I dropped patch 3, I can't take this one either.

greg k-h

[GIT PULL v3] Fixes/cleanup for vt8500 serial

[Greg KH]

On Thu, Jan 17, 2013 at 08:05:39AM +1300, Tony Prisk wrote:
> Hi Greg,
> 
> This should be the final pull request for this series, unless there are
> other review comments. Changelog included.
> 
> v2:
> Restore the setting of vt8500_port->uart.uartclk which was dropped in v1.
> Corrected the return-on-fail of devm_request_and_ioremap to -EADDRNOTAVAIL.
> 
> v3:
> Corrected the commit message for patch 3
> 
> 
> The following changes since commit 9931faca02c604c22335f5a935a501bb2ace6e20:
> 
>   Linux 3.8-rc3 (2013-01-09 18:59:55 -0800)
> 
> are available in the git repository at:
> 
>   git://server.prisktech.co.nz/git/linuxwmt.git tags/vt8500/serial-fixes

I don't take git pull requests for tiny stuff like this, sorry, only
patches through email.

But 2 of these patches had checkpatch issues, so I didn't take patches 3
and 4, please redo them.

thanks,

greg k-h