[PATCHv3 0/6] Extend UART support for Allwinner SoCs

[PATCHv3 0/6] Extend UART support for Allwinner SoCs

[Maxime Ripard]

Hi,

This patchset reworks a bit the UART support that is found in the DT
for Allwinner SoCs, which was quite limited until now.

It first switches to using the clocks property, then rework a bit the
DTSI to have only the UART available in the A10 and the A13 (the A10
has 8 UARTs, while A13 has only the uart1 and uart3).

Thanks,
Maxime

Changes from v2:
  - Reworked Emilio's patch according to Heikki Krogerus' review to enable the
    use of the clock framework even when not using the dt

Emilio L?pez (1):
  serial: 8250_dw: add support for clk api

Maxime Ripard (5):
  ARM: sunxi: dt: Use clocks property instead of clock-frequency for
    the UARTs
  ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi
  ARM: sunxi: dt: Add uart3 dt node
  ARM: sunxi: dt: Add A10 UARTs to the dtsi.
  ARM: sunxi: hackberry: Add UART muxing

 arch/arm/boot/dts/sun4i-a10-hackberry.dts |    2 +
 arch/arm/boot/dts/sun4i-a10.dtsi          |   61 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/sunxi.dtsi              |   16 ++++----
 drivers/tty/serial/8250/8250_dw.c         |   33 +++++++++++-----
 4 files changed, 95 insertions(+), 17 deletions(-)

-- 
1.7.10.4

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit implements support for using the clk api; this lets us use
the "clocks" property with device tree, instead of having to use
clock-frequency.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/tty/serial/8250/8250_dw.c |   33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index db0e66f..9a834a1 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)
@@ -136,8 +138,13 @@ static int dw8250_probe_of(struct uart_port *p)
 	if (!of_property_read_u32(np, "reg-shift", &val))
 		p->regshift = val;
 
+	/* clock got configured through clk api, all done */
+	if (p->uartclk)
+		return 0;
+
+	/* try to find out clock frequency from DT as fallback */
 	if (of_property_read_u32(np, "clock-frequency", &val)) {
-		dev_err(p->dev, "no clock-frequency property set\n");
+		dev_err(p->dev, "clk or clock-frequency not defined\n");
 		return -EINVAL;
 	}
 	p->uartclk = val;
@@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
 	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);
+	if (IS_ERR(data->clk))
+		data->clk = NULL;
+	else
+		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);
 
@@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 
+	clk_disable_unprepare(data->clk);
+
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit implements support for using the clk api; this lets us use
the "clocks" property with device tree, instead of having to use
clock-frequency.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/tty/serial/8250/8250_dw.c |   33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index db0e66f..9a834a1 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)
@@ -136,8 +138,13 @@ static int dw8250_probe_of(struct uart_port *p)
 	if (!of_property_read_u32(np, "reg-shift", &val))
 		p->regshift = val;
 
+	/* clock got configured through clk api, all done */
+	if (p->uartclk)
+		return 0;
+
+	/* try to find out clock frequency from DT as fallback */
 	if (of_property_read_u32(np, "clock-frequency", &val)) {
-		dev_err(p->dev, "no clock-frequency property set\n");
+		dev_err(p->dev, "clk or clock-frequency not defined\n");
 		return -EINVAL;
 	}
 	p->uartclk = val;
@@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
 	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);
+	if (IS_ERR(data->clk))
+		data->clk = NULL;
+	else
+		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);
 
@@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 
+	clk_disable_unprepare(data->clk);
+
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit implements support for using the clk api; this lets us use
the "clocks" property with device tree, instead of having to use
clock-frequency.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/tty/serial/8250/8250_dw.c |   33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index db0e66f..9a834a1 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)
@@ -136,8 +138,13 @@ static int dw8250_probe_of(struct uart_port *p)
 	if (!of_property_read_u32(np, "reg-shift", &val))
 		p->regshift = val;
 
+	/* clock got configured through clk api, all done */
+	if (p->uartclk)
+		return 0;
+
+	/* try to find out clock frequency from DT as fallback */
 	if (of_property_read_u32(np, "clock-frequency", &val)) {
-		dev_err(p->dev, "no clock-frequency property set\n");
+		dev_err(p->dev, "clk or clock-frequency not defined\n");
 		return -EINVAL;
 	}
 	p->uartclk = val;
@@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
 	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);
+	if (IS_ERR(data->clk))
+		data->clk = NULL;
+	else
+		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);
 
@@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
 
 	serial8250_unregister_port(data->line);
 
+	clk_disable_unprepare(data->clk);
+
 	return 0;
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
> +	/* clock got configured through clk api, all done */
> +	if (p->uartclk)

	if (IS_ERR(p->uartclk))

> +		return 0;
> +
> +	/* try to find out clock frequency from DT as fallback */
>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> -		dev_err(p->dev, "no clock-frequency property set\n");
> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>  		return -EINVAL;
>  	}
>  	p->uartclk = val;
> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>  	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);
> +	if (IS_ERR(data->clk))
> +		data->clk = NULL;
> +	else
> +		clk_prepare_enable(data->clk);

	if (!IS_ERR(data->clk))
		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);

What if data->clk is invalid?

	if (!IS_ERR(data->clk)
		uart.port.uartclk = clk_get_rate(data->clk);

>  
>  	dw8250_setup_port(&uart);
>  
> @@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
>  
>  	serial8250_unregister_port(data->line);
>  

	if (!IS_ERR(data->clk)

> +	clk_disable_unprepare(data->clk);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
> +	/* clock got configured through clk api, all done */
> +	if (p->uartclk)

	if (IS_ERR(p->uartclk))

> +		return 0;
> +
> +	/* try to find out clock frequency from DT as fallback */
>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> -		dev_err(p->dev, "no clock-frequency property set\n");
> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>  		return -EINVAL;
>  	}
>  	p->uartclk = val;
> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>  	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);
> +	if (IS_ERR(data->clk))
> +		data->clk = NULL;
> +	else
> +		clk_prepare_enable(data->clk);

	if (!IS_ERR(data->clk))
		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);

What if data->clk is invalid?

	if (!IS_ERR(data->clk)
		uart.port.uartclk = clk_get_rate(data->clk);

>  
>  	dw8250_setup_port(&uart);
>  
> @@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
>  
>  	serial8250_unregister_port(data->line);
>  

	if (!IS_ERR(data->clk)

> +	clk_disable_unprepare(data->clk);
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Emilio López]

Hello Russell,

El 15/03/13 19:39, Russell King - ARM Linux escribió:
> On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
>> +	/* clock got configured through clk api, all done */
>> +	if (p->uartclk)
> 
> 	if (IS_ERR(p->uartclk))
> 

Isn't IS_ERR for pointers? p->uartclk is an unsigned int

>> +		return 0;
>> +
>> +	/* try to find out clock frequency from DT as fallback */
>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>> -		dev_err(p->dev, "no clock-frequency property set\n");
>> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>>  		return -EINVAL;
>>  	}
>>  	p->uartclk = val;
>> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>>  	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);
>> +	if (IS_ERR(data->clk))
>> +		data->clk = NULL;
>> +	else
>> +		clk_prepare_enable(data->clk);
> 
> 	if (!IS_ERR(data->clk))
> 		clk_prepare_enable(data->clk);
> 

See below

>> +
>>  	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);
> 
> What if data->clk is invalid?
> 
> 	if (!IS_ERR(data->clk)
> 		uart.port.uartclk = clk_get_rate(data->clk);
> 

I'm not sure if it is coincidental or the way it is supposed to be, but
when using the common clock framework, if you pass a NULL to
clk_get_rate, the function explicitly checks for it and returns 0. I
relied on that behaviour when implementing this; see the if..else block
above. Is this not always the case on other clock drivers?

>>  
>>  	dw8250_setup_port(&uart);
>>  
>> @@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
>>  
>>  	serial8250_unregister_port(data->line);
>>  
> 
> 	if (!IS_ERR(data->clk)
> 

I just double checked and clk_enable/disable, clk_prepare/unprepare also
ignore NULLs passed to them on the CCF.

>> +	clk_disable_unprepare(data->clk);
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 1.7.10.4

Thanks for the review,

Emilio

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Emilio López]

Hello Russell,

El 15/03/13 19:39, Russell King - ARM Linux escribi?:
> On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
>> +	/* clock got configured through clk api, all done */
>> +	if (p->uartclk)
> 
> 	if (IS_ERR(p->uartclk))
> 

Isn't IS_ERR for pointers? p->uartclk is an unsigned int

>> +		return 0;
>> +
>> +	/* try to find out clock frequency from DT as fallback */
>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>> -		dev_err(p->dev, "no clock-frequency property set\n");
>> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>>  		return -EINVAL;
>>  	}
>>  	p->uartclk = val;
>> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>>  	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);
>> +	if (IS_ERR(data->clk))
>> +		data->clk = NULL;
>> +	else
>> +		clk_prepare_enable(data->clk);
> 
> 	if (!IS_ERR(data->clk))
> 		clk_prepare_enable(data->clk);
> 

See below

>> +
>>  	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);
> 
> What if data->clk is invalid?
> 
> 	if (!IS_ERR(data->clk)
> 		uart.port.uartclk = clk_get_rate(data->clk);
> 

I'm not sure if it is coincidental or the way it is supposed to be, but
when using the common clock framework, if you pass a NULL to
clk_get_rate, the function explicitly checks for it and returns 0. I
relied on that behaviour when implementing this; see the if..else block
above. Is this not always the case on other clock drivers?

>>  
>>  	dw8250_setup_port(&uart);
>>  
>> @@ -312,12 +331,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 +346,8 @@ static int dw8250_remove(struct platform_device *pdev)
>>  
>>  	serial8250_unregister_port(data->line);
>>  
> 
> 	if (!IS_ERR(data->clk)
> 

I just double checked and clk_enable/disable, clk_prepare/unprepare also
ignore NULLs passed to them on the CCF.

>> +	clk_disable_unprepare(data->clk);
>> +
>>  	return 0;
>>  }
>>  
>> -- 
>> 1.7.10.4

Thanks for the review,

Emilio

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio López wrote:
> Hello Russell,
> 
> El 15/03/13 19:39, Russell King - ARM Linux escribió:
> > On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
> >> +	/* clock got configured through clk api, all done */
> >> +	if (p->uartclk)
> > 
> > 	if (IS_ERR(p->uartclk))
> > 
> 
> Isn't IS_ERR for pointers? p->uartclk is an unsigned int

Right, sorry, ignore that.

> >> +		return 0;
> >> +
> >> +	/* try to find out clock frequency from DT as fallback */
> >>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> >> -		dev_err(p->dev, "no clock-frequency property set\n");
> >> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
> >>  		return -EINVAL;
> >>  	}
> >>  	p->uartclk = val;
> >> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
> >>  	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);
> >> +	if (IS_ERR(data->clk))
> >> +		data->clk = NULL;
> >> +	else
> >> +		clk_prepare_enable(data->clk);
> > 
> > 	if (!IS_ERR(data->clk))
> > 		clk_prepare_enable(data->clk);
> > 
> 
> See below
> 
> >> +
> >>  	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);
> > 
> > What if data->clk is invalid?
> > 
> > 	if (!IS_ERR(data->clk)
> > 		uart.port.uartclk = clk_get_rate(data->clk);
> > 
> 
> I'm not sure if it is coincidental or the way it is supposed to be, but
> when using the common clock framework, if you pass a NULL to
> clk_get_rate, the function explicitly checks for it and returns 0. I
> relied on that behaviour when implementing this; see the if..else block
> above. Is this not always the case on other clock drivers?

That's something that the common clock framework decided to do.  It's
not a defined part of the API though, so drivers shouldn't rely on
this behaviour meaning anything special.

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio L?pez wrote:
> Hello Russell,
> 
> El 15/03/13 19:39, Russell King - ARM Linux escribi?:
> > On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
> >> +	/* clock got configured through clk api, all done */
> >> +	if (p->uartclk)
> > 
> > 	if (IS_ERR(p->uartclk))
> > 
> 
> Isn't IS_ERR for pointers? p->uartclk is an unsigned int

Right, sorry, ignore that.

> >> +		return 0;
> >> +
> >> +	/* try to find out clock frequency from DT as fallback */
> >>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> >> -		dev_err(p->dev, "no clock-frequency property set\n");
> >> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
> >>  		return -EINVAL;
> >>  	}
> >>  	p->uartclk = val;
> >> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
> >>  	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);
> >> +	if (IS_ERR(data->clk))
> >> +		data->clk = NULL;
> >> +	else
> >> +		clk_prepare_enable(data->clk);
> > 
> > 	if (!IS_ERR(data->clk))
> > 		clk_prepare_enable(data->clk);
> > 
> 
> See below
> 
> >> +
> >>  	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);
> > 
> > What if data->clk is invalid?
> > 
> > 	if (!IS_ERR(data->clk)
> > 		uart.port.uartclk = clk_get_rate(data->clk);
> > 
> 
> I'm not sure if it is coincidental or the way it is supposed to be, but
> when using the common clock framework, if you pass a NULL to
> clk_get_rate, the function explicitly checks for it and returns 0. I
> relied on that behaviour when implementing this; see the if..else block
> above. Is this not always the case on other clock drivers?

That's something that the common clock framework decided to do.  It's
not a defined part of the API though, so drivers shouldn't rely on
this behaviour meaning anything special.

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Russell King - ARM Linux]

On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio López wrote:
> Hello Russell,
> 
> El 15/03/13 19:39, Russell King - ARM Linux escribió:
> > On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
> >> +	/* clock got configured through clk api, all done */
> >> +	if (p->uartclk)
> > 
> > 	if (IS_ERR(p->uartclk))
> > 
> 
> Isn't IS_ERR for pointers? p->uartclk is an unsigned int

Right, sorry, ignore that.

> >> +		return 0;
> >> +
> >> +	/* try to find out clock frequency from DT as fallback */
> >>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
> >> -		dev_err(p->dev, "no clock-frequency property set\n");
> >> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
> >>  		return -EINVAL;
> >>  	}
> >>  	p->uartclk = val;
> >> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
> >>  	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);
> >> +	if (IS_ERR(data->clk))
> >> +		data->clk = NULL;
> >> +	else
> >> +		clk_prepare_enable(data->clk);
> > 
> > 	if (!IS_ERR(data->clk))
> > 		clk_prepare_enable(data->clk);
> > 
> 
> See below
> 
> >> +
> >>  	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);
> > 
> > What if data->clk is invalid?
> > 
> > 	if (!IS_ERR(data->clk)
> > 		uart.port.uartclk = clk_get_rate(data->clk);
> > 
> 
> I'm not sure if it is coincidental or the way it is supposed to be, but
> when using the common clock framework, if you pass a NULL to
> clk_get_rate, the function explicitly checks for it and returns 0. I
> relied on that behaviour when implementing this; see the if..else block
> above. Is this not always the case on other clock drivers?

That's something that the common clock framework decided to do.  It's
not a defined part of the API though, so drivers shouldn't rely on
this behaviour meaning anything special.

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Emilio López]

El 15/03/13 21:29, Russell King - ARM Linux escribió:
> On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio López wrote:
>> Hello Russell,
>>
>> El 15/03/13 19:39, Russell King - ARM Linux escribió:
>>> On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
>>>> +	/* clock got configured through clk api, all done */
>>>> +	if (p->uartclk)
>>>
>>> 	if (IS_ERR(p->uartclk))
>>>
>>
>> Isn't IS_ERR for pointers? p->uartclk is an unsigned int
> 
> Right, sorry, ignore that.
> 
>>>> +		return 0;
>>>> +
>>>> +	/* try to find out clock frequency from DT as fallback */
>>>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>>>> -		dev_err(p->dev, "no clock-frequency property set\n");
>>>> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>>>>  		return -EINVAL;
>>>>  	}
>>>>  	p->uartclk = val;
>>>> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>>>>  	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);
>>>> +	if (IS_ERR(data->clk))
>>>> +		data->clk = NULL;
>>>> +	else
>>>> +		clk_prepare_enable(data->clk);
>>>
>>> 	if (!IS_ERR(data->clk))
>>> 		clk_prepare_enable(data->clk);
>>>
>>
>> See below
>>
>>>> +
>>>>  	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);
>>>
>>> What if data->clk is invalid?
>>>
>>> 	if (!IS_ERR(data->clk)
>>> 		uart.port.uartclk = clk_get_rate(data->clk);
>>>
>>
>> I'm not sure if it is coincidental or the way it is supposed to be, but
>> when using the common clock framework, if you pass a NULL to
>> clk_get_rate, the function explicitly checks for it and returns 0. I
>> relied on that behaviour when implementing this; see the if..else block
>> above. Is this not always the case on other clock drivers?
> 
> That's something that the common clock framework decided to do.  It's
> not a defined part of the API though, so drivers shouldn't rely on
> this behaviour meaning anything special.

Ok then, I'll rework the error checking on the clock calls and get a new
patch sent. Thanks for the clarification.

Emilio

[PATCH 1/6] serial: 8250_dw: add support for clk api

[Emilio López]

El 15/03/13 21:29, Russell King - ARM Linux escribi?:
> On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio L?pez wrote:
>> Hello Russell,
>>
>> El 15/03/13 19:39, Russell King - ARM Linux escribi?:
>>> On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
>>>> +	/* clock got configured through clk api, all done */
>>>> +	if (p->uartclk)
>>>
>>> 	if (IS_ERR(p->uartclk))
>>>
>>
>> Isn't IS_ERR for pointers? p->uartclk is an unsigned int
> 
> Right, sorry, ignore that.
> 
>>>> +		return 0;
>>>> +
>>>> +	/* try to find out clock frequency from DT as fallback */
>>>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>>>> -		dev_err(p->dev, "no clock-frequency property set\n");
>>>> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>>>>  		return -EINVAL;
>>>>  	}
>>>>  	p->uartclk = val;
>>>> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>>>>  	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);
>>>> +	if (IS_ERR(data->clk))
>>>> +		data->clk = NULL;
>>>> +	else
>>>> +		clk_prepare_enable(data->clk);
>>>
>>> 	if (!IS_ERR(data->clk))
>>> 		clk_prepare_enable(data->clk);
>>>
>>
>> See below
>>
>>>> +
>>>>  	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);
>>>
>>> What if data->clk is invalid?
>>>
>>> 	if (!IS_ERR(data->clk)
>>> 		uart.port.uartclk = clk_get_rate(data->clk);
>>>
>>
>> I'm not sure if it is coincidental or the way it is supposed to be, but
>> when using the common clock framework, if you pass a NULL to
>> clk_get_rate, the function explicitly checks for it and returns 0. I
>> relied on that behaviour when implementing this; see the if..else block
>> above. Is this not always the case on other clock drivers?
> 
> That's something that the common clock framework decided to do.  It's
> not a defined part of the API though, so drivers shouldn't rely on
> this behaviour meaning anything special.

Ok then, I'll rework the error checking on the clock calls and get a new
patch sent. Thanks for the clarification.

Emilio

Re: [PATCH 1/6] serial: 8250_dw: add support for clk api

[Emilio López]

El 15/03/13 21:29, Russell King - ARM Linux escribió:
> On Fri, Mar 15, 2013 at 09:15:11PM -0300, Emilio López wrote:
>> Hello Russell,
>>
>> El 15/03/13 19:39, Russell King - ARM Linux escribió:
>>> On Fri, Mar 15, 2013 at 09:06:23PM +0100, Maxime Ripard wrote:
>>>> +	/* clock got configured through clk api, all done */
>>>> +	if (p->uartclk)
>>>
>>> 	if (IS_ERR(p->uartclk))
>>>
>>
>> Isn't IS_ERR for pointers? p->uartclk is an unsigned int
> 
> Right, sorry, ignore that.
> 
>>>> +		return 0;
>>>> +
>>>> +	/* try to find out clock frequency from DT as fallback */
>>>>  	if (of_property_read_u32(np, "clock-frequency", &val)) {
>>>> -		dev_err(p->dev, "no clock-frequency property set\n");
>>>> +		dev_err(p->dev, "clk or clock-frequency not defined\n");
>>>>  		return -EINVAL;
>>>>  	}
>>>>  	p->uartclk = val;
>>>> @@ -294,9 +301,21 @@ static int dw8250_probe(struct platform_device *pdev)
>>>>  	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);
>>>> +	if (IS_ERR(data->clk))
>>>> +		data->clk = NULL;
>>>> +	else
>>>> +		clk_prepare_enable(data->clk);
>>>
>>> 	if (!IS_ERR(data->clk))
>>> 		clk_prepare_enable(data->clk);
>>>
>>
>> See below
>>
>>>> +
>>>>  	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);
>>>
>>> What if data->clk is invalid?
>>>
>>> 	if (!IS_ERR(data->clk)
>>> 		uart.port.uartclk = clk_get_rate(data->clk);
>>>
>>
>> I'm not sure if it is coincidental or the way it is supposed to be, but
>> when using the common clock framework, if you pass a NULL to
>> clk_get_rate, the function explicitly checks for it and returns 0. I
>> relied on that behaviour when implementing this; see the if..else block
>> above. Is this not always the case on other clock drivers?
> 
> That's something that the common clock framework decided to do.  It's
> not a defined part of the API though, so drivers shouldn't rely on
> this behaviour meaning anything special.

Ok then, I'll rework the error checking on the clock calls and get a new
patch sent. Thanks for the clarification.

Emilio
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] ARM: sunxi: dt: Use clocks property instead of clock-frequency for the UARTs

It will be especially useful when we will have the clock definitions in
the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio López <emilio@elopez.com.ar>
Tested-by: Emilio López <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sunxi.dtsi |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 8b36abe..791c02a 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -65,7 +65,7 @@
 			interrupts = <1>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clock-frequency = <24000000>;
+			clocks = <&osc>;
 			status = "disabled";
 		};
 
@@ -75,7 +75,7 @@
 			interrupts = <2>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clock-frequency = <24000000>;
+			clocks = <&osc>;
 			status = "disabled";
 		};
 	};
-- 
1.7.10.4

[PATCH 2/6] ARM: sunxi: dt: Use clocks property instead of clock-frequency for the UARTs

[Maxime Ripard]

It will be especially useful when we will have the clock definitions in
the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio L?pez <emilio@elopez.com.ar>
Tested-by: Emilio L?pez <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sunxi.dtsi |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 8b36abe..791c02a 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -65,7 +65,7 @@
 			interrupts = <1>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clock-frequency = <24000000>;
+			clocks = <&osc>;
 			status = "disabled";
 		};
 
@@ -75,7 +75,7 @@
 			interrupts = <2>;
 			reg-shift = <2>;
 			reg-io-width = <4>;
-			clock-frequency = <24000000>;
+			clocks = <&osc>;
 			status = "disabled";
 		};
 	};
-- 
1.7.10.4

[PATCH 3/6] ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi

[Maxime Ripard]

The UART0 is only available on the Allwinner A10 SoCs, and not on the
A13, so move the uart0 node to sun4i-a10.dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio López <emilio@elopez.com.ar>
Tested-by: Emilio López <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   10 ++++++++++
 arch/arm/boot/dts/sunxi.dtsi     |   10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 03d2b53..703e7cb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -47,5 +47,15 @@
 				allwinner,pull = <0>;
 			};
 		};
+
+		uart0: uart@01c28000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28000 0x400>;
+			interrupts = <1>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 791c02a..4f78ef7 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -59,16 +59,6 @@
 			#interrupt-cells = <1>;
 		};
 
-		uart0: uart@01c28000 {
-			compatible = "snps,dw-apb-uart";
-			reg = <0x01c28000 0x400>;
-			interrupts = <1>;
-			reg-shift = <2>;
-			reg-io-width = <4>;
-			clocks = <&osc>;
-			status = "disabled";
-		};
-
 		uart1: uart@01c28400 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x01c28400 0x400>;
-- 
1.7.10.4

[PATCH 3/6] ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi

[Maxime Ripard]

The UART0 is only available on the Allwinner A10 SoCs, and not on the
A13, so move the uart0 node to sun4i-a10.dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio L?pez <emilio@elopez.com.ar>
Tested-by: Emilio L?pez <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   10 ++++++++++
 arch/arm/boot/dts/sunxi.dtsi     |   10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 03d2b53..703e7cb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -47,5 +47,15 @@
 				allwinner,pull = <0>;
 			};
 		};
+
+		uart0: uart at 01c28000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28000 0x400>;
+			interrupts = <1>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 791c02a..4f78ef7 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -59,16 +59,6 @@
 			#interrupt-cells = <1>;
 		};
 
-		uart0: uart at 01c28000 {
-			compatible = "snps,dw-apb-uart";
-			reg = <0x01c28000 0x400>;
-			interrupts = <1>;
-			reg-shift = <2>;
-			reg-io-width = <4>;
-			clocks = <&osc>;
-			status = "disabled";
-		};
-
 		uart1: uart at 01c28400 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x01c28400 0x400>;
-- 
1.7.10.4

[PATCH 4/6] ARM: sunxi: dt: Add uart3 dt node

[Maxime Ripard]

Both A10 and A13 Allwinner SoCs have a Synopsys APB uart3 device
available, so add it to the sunxi.dtsi file

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio López <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sunxi.dtsi |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 4f78ef7..324da45 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -68,5 +68,15 @@
 			clocks = <&osc>;
 			status = "disabled";
 		};
+
+		uart3: uart@01c28c00 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28c00 0x400>;
+			interrupts = <4>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.7.10.4

[PATCH 4/6] ARM: sunxi: dt: Add uart3 dt node

[Maxime Ripard]

Both A10 and A13 Allwinner SoCs have a Synopsys APB uart3 device
available, so add it to the sunxi.dtsi file

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio L?pez <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sunxi.dtsi |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 4f78ef7..324da45 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -68,5 +68,15 @@
 			clocks = <&osc>;
 			status = "disabled";
 		};
+
+		uart3: uart at 01c28c00 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28c00 0x400>;
+			interrupts = <4>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.7.10.4

Re: [PATCH 4/6] ARM: sunxi: dt: Add uart3 dt node

[Sergei Shtylyov]

Hello.

On 03/15/2013 11:06 PM, Maxime Ripard wrote:

> Both A10 and A13 Allwinner SoCs have a Synopsys APB uart3 device
> available, so add it to the sunxi.dtsi file
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Acked-by: Emilio López <emilio@elopez.com.ar>
> ---
>   arch/arm/boot/dts/sunxi.dtsi |   10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
> index 4f78ef7..324da45 100644
> --- a/arch/arm/boot/dts/sunxi.dtsi
> +++ b/arch/arm/boot/dts/sunxi.dtsi
> @@ -68,5 +68,15 @@
>   			clocks = <&osc>;
>   			status = "disabled";
>   		};
> +
> +		uart3: uart@01c28c00 {

    IIRC, historically, the property name should be "serial", not "uart".

WBR, Sergei

[PATCH 4/6] ARM: sunxi: dt: Add uart3 dt node

[Sergei Shtylyov]

Hello.

On 03/15/2013 11:06 PM, Maxime Ripard wrote:

> Both A10 and A13 Allwinner SoCs have a Synopsys APB uart3 device
> available, so add it to the sunxi.dtsi file
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Acked-by: Emilio L?pez <emilio@elopez.com.ar>
> ---
>   arch/arm/boot/dts/sunxi.dtsi |   10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
> index 4f78ef7..324da45 100644
> --- a/arch/arm/boot/dts/sunxi.dtsi
> +++ b/arch/arm/boot/dts/sunxi.dtsi
> @@ -68,5 +68,15 @@
>   			clocks = <&osc>;
>   			status = "disabled";
>   		};
> +
> +		uart3: uart at 01c28c00 {

    IIRC, historically, the property name should be "serial", not "uart".

WBR, Sergei

[PATCH 5/6] ARM: sunxi: dt: Add A10 UARTs to the dtsi.

[Maxime Ripard]

The Allwinner A10 SoC has 8 available UARTs, which is 6 more than on the
A13, so add the missing UARTs to the sun4i-a10 dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio López <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 703e7cb..0142ca0 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -57,5 +57,56 @@
 			clocks = <&osc>;
 			status = "disabled";
 		};
+
+		uart2: uart@01c28800 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28800 0x400>;
+			interrupts = <3>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+
+		uart4: uart@01c29000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29000 0x400>;
+			interrupts = <17>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart5: uart@01c29400 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29400 0x400>;
+			interrupts = <18>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart6: uart@01c29800 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29800 0x400>;
+			interrupts = <19>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart7: uart@01c29c00 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29c00 0x400>;
+			interrupts = <20>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.7.10.4

[PATCH 5/6] ARM: sunxi: dt: Add A10 UARTs to the dtsi.

[Maxime Ripard]

The Allwinner A10 SoC has 8 available UARTs, which is 6 more than on the
A13, so add the missing UARTs to the sun4i-a10 dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio L?pez <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   51 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 703e7cb..0142ca0 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -57,5 +57,56 @@
 			clocks = <&osc>;
 			status = "disabled";
 		};
+
+		uart2: uart at 01c28800 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28800 0x400>;
+			interrupts = <3>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+
+		uart4: uart at 01c29000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29000 0x400>;
+			interrupts = <17>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart5: uart at 01c29400 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29400 0x400>;
+			interrupts = <18>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart6: uart at 01c29800 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29800 0x400>;
+			interrupts = <19>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
+
+		uart7: uart at 01c29c00 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c29c00 0x400>;
+			interrupts = <20>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
-- 
1.7.10.4

[PATCH 6/6] ARM: sunxi: hackberry: Add UART muxing

[Maxime Ripard]

We previously relied on the bootloader to do the muxing of the UART for
the Hackberry. Don't rely on it anymore and use pinctrl.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio López <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10-hackberry.dts |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10-hackberry.dts b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
index f84549a..97703fe 100644
--- a/arch/arm/boot/dts/sun4i-a10-hackberry.dts
+++ b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
@@ -24,6 +24,8 @@
 
 	soc {
 		uart0: uart@01c28000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart0_pins_a>;
 			status = "okay";
 		};
 	};
-- 
1.7.10.4

[PATCH 6/6] ARM: sunxi: hackberry: Add UART muxing

[Maxime Ripard]

We previously relied on the bootloader to do the muxing of the UART for
the Hackberry. Don't rely on it anymore and use pinctrl.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Emilio L?pez <emilio@elopez.com.ar>
---
 arch/arm/boot/dts/sun4i-a10-hackberry.dts |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10-hackberry.dts b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
index f84549a..97703fe 100644
--- a/arch/arm/boot/dts/sun4i-a10-hackberry.dts
+++ b/arch/arm/boot/dts/sun4i-a10-hackberry.dts
@@ -24,6 +24,8 @@
 
 	soc {
 		uart0: uart at 01c28000 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart0_pins_a>;
 			status = "okay";
 		};
 	};
-- 
1.7.10.4

[PATCHv2 0/6] Extend UART support for Allwinner SoCs

[Maxime Ripard]

Hi,

This patchset reworks a bit the UART support that is found in the DT
for Allwinner SoCs, which was quite limited until now.

It first switches to using the clocks property, then rework a bit the
DTSI to have only the UART available in the A10 and the A13 (the A10
has 8 UARTs, while A13 has only the uart1 and uart3).

Thanks,
Maxime

Changes since v1:
  - Add Emilio patch to add support for clock phandle to 8250_dw
  - Change uart3 definition to use the clocks property as well

Emilio L?pez (1):
  serial: 8250_dw: add support for clocks property when using
    DeviceTree

Maxime Ripard (5):
  ARM: sunxi: dt: Use clocks property instead of clock-frequency for
    the UARTs
  ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi
  ARM: sunxi: dt: Add uart3 dt node
  ARM: sunxi: dt: Add A10 UARTs to the dtsi.
  ARM: sunxi: hackberry: Add UART muxing

 arch/arm/boot/dts/sun4i-a10-hackberry.dts |    2 +
 arch/arm/boot/dts/sun4i-a10.dtsi          |   61 +++++++++++++++++++++++++++++
 arch/arm/boot/dts/sunxi.dtsi              |   16 ++++----
 drivers/tty/serial/8250/8250_dw.c         |   32 ++++++++++-----
 4 files changed, 93 insertions(+), 18 deletions(-)

-- 
1.7.10.4

[PATCH 3/6] ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi

[Maxime Ripard]

The UART0 is only available on the Allwinner A10 SoCs, and not on the
A13, so move the uart0 node to sun4i-a10.dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   10 ++++++++++
 arch/arm/boot/dts/sunxi.dtsi     |   10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 03d2b53..703e7cb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -47,5 +47,15 @@
 				allwinner,pull = <0>;
 			};
 		};
+
+		uart0: uart@01c28000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28000 0x400>;
+			interrupts = <1>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 791c02a..4f78ef7 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -59,16 +59,6 @@
 			#interrupt-cells = <1>;
 		};
 
-		uart0: uart@01c28000 {
-			compatible = "snps,dw-apb-uart";
-			reg = <0x01c28000 0x400>;
-			interrupts = <1>;
-			reg-shift = <2>;
-			reg-io-width = <4>;
-			clocks = <&osc>;
-			status = "disabled";
-		};
-
 		uart1: uart@01c28400 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x01c28400 0x400>;
-- 
1.7.10.4

[PATCH 3/6] ARM: sunxi: dt: Move uart0 to sun4i-a10.dtsi

[Maxime Ripard]

The UART0 is only available on the Allwinner A10 SoCs, and not on the
A13, so move the uart0 node to sun4i-a10.dtsi.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi |   10 ++++++++++
 arch/arm/boot/dts/sunxi.dtsi     |   10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 03d2b53..703e7cb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -47,5 +47,15 @@
 				allwinner,pull = <0>;
 			};
 		};
+
+		uart0: uart at 01c28000 {
+			compatible = "snps,dw-apb-uart";
+			reg = <0x01c28000 0x400>;
+			interrupts = <1>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+			clocks = <&osc>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/sunxi.dtsi b/arch/arm/boot/dts/sunxi.dtsi
index 791c02a..4f78ef7 100644
--- a/arch/arm/boot/dts/sunxi.dtsi
+++ b/arch/arm/boot/dts/sunxi.dtsi
@@ -59,16 +59,6 @@
 			#interrupt-cells = <1>;
 		};
 
-		uart0: uart at 01c28000 {
-			compatible = "snps,dw-apb-uart";
-			reg = <0x01c28000 0x400>;
-			interrupts = <1>;
-			reg-shift = <2>;
-			reg-io-width = <4>;
-			clocks = <&osc>;
-			status = "disabled";
-		};
-
 		uart1: uart at 01c28400 {
 			compatible = "snps,dw-apb-uart";
 			reg = <0x01c28400 0x400>;
-- 
1.7.10.4