Skip to contentSkip to conversation

Fixes/cleanup for vt8500 serial

GIT PULL In review Tony Prisk 2 participants 8 messages Jan 16, 2013 – Jan 18, 2013

Subsystems (3)
Device tree bindings TTY layer and serial drivers Arm/VT8500 Arm architecture
lore
Share & tools
lore

Tony Prisk

author
#0 lore raw

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(-)
author
#1 lore raw

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

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 <[email protected]>
---
 drivers/tty/serial/vt8500_serial.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/tty/serial/vt8500_serial.c +2 -2
index 8fd1814..4c4a58d 100644
@@ -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 */
Signature · 1 line
1.7.9.5
author
#2 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/tty/serial/vt8500_serial.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/tty/serial/vt8500_serial.c +2 -2
index 4c4a58d..3e76dff 100644
@@ -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;
 	}
Signature · 1 line
1.7.9.5
author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 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(-)
arch/arm/boot/dts/vt8500.dtsi +36 -4
index d8645e9..cf31ced 100644
@@ -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 {
arch/arm/boot/dts/wm8505.dtsi +54 -6

102 lines not on this page · raw message

arch/arm/boot/dts/wm8650.dtsi +18 -2
index 83b9467..e0c42ff 100644
@@ -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 {
drivers/tty/serial/vt8500_serial.c +19 -15
index 3e76dff..7f9e578 100644
@@ -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);
Signature · 1 line
1.7.9.5
maintainer
#4 lore raw

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

Quoted 12 lines · 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.

Quoted 7 lines
Signed-off-by Tony Prisk <[email protected]>
---
 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

author
#5 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/tty/serial/vt8500_serial.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)
drivers/tty/serial/vt8500_serial.c +4 -9
index 7f9e578..1fc6f3d 100644
@@ -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;
 }
Signature · 1 line
1.7.9.5
maintainer
#6 lore raw

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

Quoted 10 lines · 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 <[email protected]>
---
 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

maintainer
#7 lore raw
In reply to Tony Prisk
Quoted 13 lines · 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

Quoted 7 lines

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