Patchwork [1/6] serial: 8250_dw: add support for clocks property when using DeviceTree

login
register
mail settings
Submitter Maxime Ripard
Date March 7, 2013, 10:14 p.m.
Message ID <1362694460-1919-2-git-send-email-maxime.ripard@free-electrons.com>
Download mbox | patch
Permalink /patch/225984/
State New
Headers show

Comments

Maxime Ripard - March 7, 2013, 10:14 p.m.
From: Emilio López <emilio@elopez.com.ar>

This commit implements support for using the "clocks" DT property, instead
of having to use clock-frequency.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
---
 drivers/tty/serial/8250/8250_dw.c |   32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)
Heikki Krogerus - March 15, 2013, 12:56 p.m.
Hi,

On Thu, Mar 07, 2013 at 11:14:15PM +0100, Maxime Ripard wrote:
> From: Emilio López <emilio@elopez.com.ar>
> 
> This commit implements support for using the "clocks" DT property, instead
> of having to use clock-frequency.
> 
> Signed-off-by: Emilio López <emilio@elopez.com.ar>
> ---
>  drivers/tty/serial/8250/8250_dw.c |   32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)

<snip>

> @@ -137,8 +140,15 @@ static int dw8250_probe_of(struct uart_port *p)
>  		p->regshift = val;
>  
>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> -		dev_err(p->dev, "no clock-frequency property set\n");
> -		return -EINVAL;
> +		/* Get clk rate through clk driver if present */
> +		data->clk = clk_get(p->dev, NULL);
> +		if (IS_ERR(data->clk)) {
> +			dev_err(p->dev, "clk or clock-frequency not defined\n");
> +			return -EINVAL;
> +		}
> +
> +		clk_prepare_enable(data->clk);
> +		val = clk_get_rate(data->clk);

Don't get the clk here..

>  	}
>  	p->uartclk = val;
>  
> @@ -300,6 +310,12 @@ static int dw8250_probe(struct platform_device *pdev)

Get it here so it is then handled for also others besides DT users.
And prefer devm_clk_get(). Something like this:

        ...
        uart.port.membase = ioremap(regs->start, resource_size(regs));
        if (!uart.port.membase)
                return -ENOMEM;

+       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+       if (!data)
+       	return -ENOMEM;
+        
+       data->clk = devm_clk_get(&pdev->dev, NULL);
+       clk_prepare_enable(data->clk);
+
        uart.port.iotype = UPIO_MEM;
        uart.port.serial_in = dw8250_serial_in;
        uart.port.serial_out = dw8250_serial_out;
+       uart.port.private_data = data;
+       uart.port.uartclk = clk_get_rate(data->clk);

  	dw8250_setup_port(&uart);
        ... 

Then in dw8250_probe_of() you need to use the "clock-frequency"
property only when p->uartclk is 0.

Thanks,
Maxime Ripard - March 15, 2013, 7:48 p.m.
Hi Heikki

Le 15/03/2013 13:56, Heikki Krogerus a écrit :
>> @@ -137,8 +140,15 @@ static int dw8250_probe_of(struct uart_port *p)
>>  		p->regshift = val;
>>  
>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>> -		dev_err(p->dev, "no clock-frequency property set\n");
>> -		return -EINVAL;
>> +		/* Get clk rate through clk driver if present */
>> +		data->clk = clk_get(p->dev, NULL);
>> +		if (IS_ERR(data->clk)) {
>> +			dev_err(p->dev, "clk or clock-frequency not defined\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		clk_prepare_enable(data->clk);
>> +		val = clk_get_rate(data->clk);
> 
> Don't get the clk here..
> 
>>  	}
>>  	p->uartclk = val;
>>  
>> @@ -300,6 +310,12 @@ static int dw8250_probe(struct platform_device *pdev)
> 
> Get it here so it is then handled for also others besides DT users.
> And prefer devm_clk_get(). Something like this:
> 
>         ...
>         uart.port.membase = ioremap(regs->start, resource_size(regs));
>         if (!uart.port.membase)
>                 return -ENOMEM;
> 
> +       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +       	return -ENOMEM;
> +        
> +       data->clk = devm_clk_get(&pdev->dev, NULL);
> +       clk_prepare_enable(data->clk);
> +
>         uart.port.iotype = UPIO_MEM;
>         uart.port.serial_in = dw8250_serial_in;
>         uart.port.serial_out = dw8250_serial_out;
> +       uart.port.private_data = data;
> +       uart.port.uartclk = clk_get_rate(data->clk);
> 
>   	dw8250_setup_port(&uart);
>         ... 
> 
> Then in dw8250_probe_of() you need to use the "clock-frequency"
> property only when p->uartclk is 0.

Thanks for your feedback.
Emilio made a new version of this patch, I'll send it soon.

Maxime

Patch

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index db0e66f..8f03800 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -26,6 +26,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
+#include <linux/clk.h>
 
 #include "8250.h"
 
@@ -55,8 +56,9 @@ 
 
 
 struct dw8250_data {
-	int	last_lcr;
-	int	line;
+	int		last_lcr;
+	int		line;
+	struct clk	*clk;
 };
 
 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
@@ -116,6 +118,7 @@  static int dw8250_handle_irq(struct uart_port *p)
 static int dw8250_probe_of(struct uart_port *p)
 {
 	struct device_node	*np = p->dev->of_node;
+	struct dw8250_data	*data = p->private_data;
 	u32			val;
 
 	if (!of_property_read_u32(np, "reg-io-width", &val)) {
@@ -137,8 +140,15 @@  static int dw8250_probe_of(struct uart_port *p)
 		p->regshift = val;
 
 	if (of_property_read_u32(np, "clock-frequency", &val)) {
-		dev_err(p->dev, "no clock-frequency property set\n");
-		return -EINVAL;
+		/* Get clk rate through clk driver if present */
+		data->clk = clk_get(p->dev, NULL);
+		if (IS_ERR(data->clk)) {
+			dev_err(p->dev, "clk or clock-frequency not defined\n");
+			return -EINVAL;
+		}
+
+		clk_prepare_enable(data->clk);
+		val = clk_get_rate(data->clk);
 	}
 	p->uartclk = val;
 
@@ -300,6 +310,12 @@  static int dw8250_probe(struct platform_device *pdev)
 
 	dw8250_setup_port(&uart);
 
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	uart.port.private_data = data;
+
 	if (pdev->dev.of_node) {
 		err = dw8250_probe_of(&uart.port);
 		if (err)
@@ -312,12 +328,6 @@  static int dw8250_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	uart.port.private_data = data;
-
 	data->line = serial8250_register_8250_port(&uart);
 	if (data->line < 0)
 		return data->line;
@@ -333,6 +343,8 @@  static int dw8250_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 
+	clk_disable_unprepare(data->clk);
+
 	return 0;
 }