[PATCH v2 0/7] ARM: mmp: support OF on pxa168

[PATCH v2 0/7] ARM: mmp: support OF on pxa168

[Haojian Zhuang]

Add OF support on gpio/serial/rtc/i2c components in pxa168 aspenite.

Changelog v2:
1. Append interrupt-names property in rtc-sa1100, gpio-pxa driver.
2. Append alias in i2c-pxa driver.
3. Avoid embed CONFIG_OF in i2c-pxa driver.

[PATCH v2 1/7] serial: pxa: add OF support

[Haojian Zhuang]

Parse uart device id from alias in DTS file.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 5c8e3bb..a71855b 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -36,6 +36,7 @@
 #include <linux/circ_buf.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 
+#define PXA_NAME_LEN		8
+
 struct uart_pxa_port {
 	struct uart_port        port;
 	unsigned char           ier;
@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
 };
 #endif
 
+static struct of_device_id serial_pxa_dt_ids[] = {
+	{ .compatible = "mrvl,pxa-uart", },
+	{ .compatible = "mrvl,mmp-uart", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
+
+#ifdef CONFIG_OF
+static int serial_pxa_probe_dt(struct platform_device *pdev,
+			       struct uart_pxa_port *sport)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np)
+		return 1;
+
+	ret = of_alias_get_id(np, "serial");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+	sport->port.line = ret;
+	return 0;
+}
+#else
+static int serial_pxa_probe_dt(struct platform_device *pdev,
+			       struct uart_pxa_port *sport)
+{
+	return 1;
+}
+#endif
+
 static int serial_pxa_probe(struct platform_device *dev)
 {
 	struct uart_pxa_port *sport;
@@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev)
 	sport->port.irq = irqres->start;
 	sport->port.fifosize = 64;
 	sport->port.ops = &serial_pxa_pops;
-	sport->port.line = dev->id;
 	sport->port.dev = &dev->dev;
 	sport->port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
 	sport->port.uartclk = clk_get_rate(sport->clk);
-
-	switch (dev->id) {
-	case 0: sport->name = "FFUART"; break;
-	case 1: sport->name = "BTUART"; break;
-	case 2: sport->name = "STUART"; break;
-	case 3: sport->name = "HWUART"; break;
-	default:
-		sport->name = "???";
-		break;
+	sport->name = kzalloc(PXA_NAME_LEN, GFP_KERNEL);
+	if (!sport->name) {
+		ret = -ENOMEM;
+		goto err_clk;
 	}
 
+	ret = serial_pxa_probe_dt(dev, sport);
+	if (ret > 0)
+		sport->port.line = dev->id;
+	else if (ret < 0)
+		goto err_clk;
+	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
+
 	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
 	if (!sport->port.membase) {
 		ret = -ENOMEM;
-		goto err_clk;
+		goto err_name;
 	}
 
-	serial_pxa_ports[dev->id] = sport;
+	serial_pxa_ports[sport->port.line] = sport;
 
 	uart_add_one_port(&serial_pxa_reg, &sport->port);
 	platform_set_drvdata(dev, sport);
 
 	return 0;
 
+ err_name:
+	kfree(sport->name);
  err_clk:
 	clk_put(sport->clk);
  err_free:
@@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)
 	platform_set_drvdata(dev, NULL);
 
 	uart_remove_one_port(&serial_pxa_reg, &sport->port);
+	kfree(sport->name);
 	clk_put(sport->clk);
 	kfree(sport);
 
@@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {
 #ifdef CONFIG_PM
 		.pm	= &serial_pxa_pm_ops,
 #endif
+		.of_match_table = serial_pxa_dt_ids,
 	},
 };
 
-- 
1.7.0.4

[PATCH v2 1/7] serial: pxa: add OF support

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:

>  
> +#define PXA_NAME_LEN		8
> +
>  struct uart_pxa_port {
>  	struct uart_port        port;
>  	unsigned char           ier;

Why didn't you just add a field here with that length?

> @@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>  };
>  #endif
>  
> +static struct of_device_id serial_pxa_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-uart", },
> +	{ .compatible = "mrvl,mmp-uart", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);

This one should have an #ifdef CONFIG_OF

> +#ifdef CONFIG_OF
> +static int serial_pxa_probe_dt(struct platform_device *pdev,
> +			       struct uart_pxa_port *sport)
> +{

While this one does not need it: it will already compile to nothing
if you check the error value correctly.

> +	sport->name = kzalloc(PXA_NAME_LEN, GFP_KERNEL);
> +	if (!sport->name) {
> +		ret = -ENOMEM;
> +		goto err_clk;
>  	}

No need for this allocation if you put the name into uart_pxa_port
as a member instead of a pointer.

> +		.of_match_table = serial_pxa_dt_ids,
>  	},
>  };

		.of_match_table = of_match_ptr(serial_pxa_dt_ids),

	Arnd

[PATCH v2 1/7] serial: pxa: add OF support

[Haojian Zhuang]

On Mon, Mar 5, 2012 at 8:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 05 March 2012, Haojian Zhuang wrote:
>
>>
>> +#define PXA_NAME_LEN ? ? ? ? 8
>> +
>> ?struct uart_pxa_port {
>> ? ? ? struct uart_port ? ? ? ?port;
>> ? ? ? unsigned char ? ? ? ? ? ier;
>
> Why didn't you just add a field here with that length?
>
>> @@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>> ?};
>> ?#endif
>>
>> +static struct of_device_id serial_pxa_dt_ids[] = {
>> + ? ? { .compatible = "mrvl,pxa-uart", },
>> + ? ? { .compatible = "mrvl,mmp-uart", },
>> + ? ? {}
>> +};
>> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
>
> This one should have an #ifdef CONFIG_OF
>
>> +#ifdef CONFIG_OF
>> +static int serial_pxa_probe_dt(struct platform_device *pdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct uart_pxa_port *sport)
>> +{
>
> While this one does not need it: it will already compile to nothing
> if you check the error value correctly.
>
>> + ? ? sport->name = kzalloc(PXA_NAME_LEN, GFP_KERNEL);
>> + ? ? if (!sport->name) {
>> + ? ? ? ? ? ? ret = -ENOMEM;
>> + ? ? ? ? ? ? goto err_clk;
>> ? ? ? }
>
> No need for this allocation if you put the name into uart_pxa_port
> as a member instead of a pointer.
>
>> + ? ? ? ? ? ? .of_match_table = serial_pxa_dt_ids,
>> ? ? ? },
>> ?};
>
> ? ? ? ? ? ? ? ?.of_match_table = of_match_ptr(serial_pxa_dt_ids),
>
> ? ? ? ?Arnd
>
Thanks. I'll update it.

Best Regards
Haojian

[PATCH v3 1/7] serial: pxa: add OF support

[Haojian Zhuang]

Parse uart device id from alias in DTS file.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/tty/serial/pxa.c |   52 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 5c8e3bb..23d32f6 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -36,6 +36,7 @@
 #include <linux/circ_buf.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 
+#define PXA_NAME_LEN		8
+
 struct uart_pxa_port {
 	struct uart_port        port;
 	unsigned char           ier;
@@ -51,7 +54,7 @@ struct uart_pxa_port {
 	unsigned char           mcr;
 	unsigned int            lsr_break_flag;
 	struct clk		*clk;
-	char			*name;
+	char			name[PXA_NAME_LEN];
 };
 
 static inline unsigned int serial_in(struct uart_pxa_port *up, int offset)
@@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
 };
 #endif
 
+#ifdef CONFIG_OF
+static struct of_device_id serial_pxa_dt_ids[] = {
+	{ .compatible = "mrvl,pxa-uart", },
+	{ .compatible = "mrvl,mmp-uart", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
+#endif
+
+static int serial_pxa_probe_dt(struct platform_device *pdev,
+			       struct uart_pxa_port *sport)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np)
+		return 1;
+
+	ret = of_alias_get_id(np, "serial");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+	sport->port.line = ret;
+	return 0;
+}
+
 static int serial_pxa_probe(struct platform_device *dev)
 {
 	struct uart_pxa_port *sport;
@@ -808,20 +838,16 @@ static int serial_pxa_probe(struct platform_device *dev)
 	sport->port.irq = irqres->start;
 	sport->port.fifosize = 64;
 	sport->port.ops = &serial_pxa_pops;
-	sport->port.line = dev->id;
 	sport->port.dev = &dev->dev;
 	sport->port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
 	sport->port.uartclk = clk_get_rate(sport->clk);
 
-	switch (dev->id) {
-	case 0: sport->name = "FFUART"; break;
-	case 1: sport->name = "BTUART"; break;
-	case 2: sport->name = "STUART"; break;
-	case 3: sport->name = "HWUART"; break;
-	default:
-		sport->name = "???";
-		break;
-	}
+	ret = serial_pxa_probe_dt(dev, sport);
+	if (ret > 0)
+		sport->port.line = dev->id;
+	else if (ret < 0)
+		goto err_clk;
+	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
 
 	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
 	if (!sport->port.membase) {
@@ -829,7 +855,7 @@ static int serial_pxa_probe(struct platform_device *dev)
 		goto err_clk;
 	}
 
-	serial_pxa_ports[dev->id] = sport;
+	serial_pxa_ports[sport->port.line] = sport;
 
 	uart_add_one_port(&serial_pxa_reg, &sport->port);
 	platform_set_drvdata(dev, sport);
@@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev)
 	platform_set_drvdata(dev, NULL);
 
 	uart_remove_one_port(&serial_pxa_reg, &sport->port);
+	kfree(sport->name);
 	clk_put(sport->clk);
 	kfree(sport);
 
@@ -866,6 +893,7 @@ static struct platform_driver serial_pxa_driver = {
 #ifdef CONFIG_PM
 		.pm	= &serial_pxa_pm_ops,
 #endif
+		.of_match_table = serial_pxa_dt_ids,
 	},
 };
 
-- 
1.7.0.4

[PATCH v3 1/7] serial: pxa: add OF support

[Arnd Bergmann]

On Tuesday 06 March 2012, Haojian Zhuang wrote:

> @@ -51,7 +54,7 @@ struct uart_pxa_port {
>  	unsigned char           mcr;
>  	unsigned int            lsr_break_flag;
>  	struct clk		*clk;
> -	char			*name;
> +	char			name[PXA_NAME_LEN];
>  };

This looks better now

>  static inline unsigned int serial_in(struct uart_pxa_port *up, int offset)
> @@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>  };
>  #endif
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id serial_pxa_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-uart", },
> +	{ .compatible = "mrvl,mmp-uart", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
> +#endif

If you do this, you have to use of_match_ptr for referencing
serial_pxa_dt_ids or you will get a link error when CONFIG_OF is
disabled.

> @@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev)
>  	platform_set_drvdata(dev, NULL);
>  
>  	uart_remove_one_port(&serial_pxa_reg, &sport->port);
> +	kfree(sport->name);
>  	clk_put(sport->clk);
>  	kfree(sport);
>  

And this must be removed now that sport->name is allocated as part of sport.

	Arnd

[PATCH v3 1/7] serial: pxa: add OF support

[Haojian Zhuang]

On Tue, Mar 6, 2012 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 06 March 2012, Haojian Zhuang wrote:
>
>> @@ -51,7 +54,7 @@ struct uart_pxa_port {
>> ? ? ? unsigned char ? ? ? ? ? mcr;
>> ? ? ? unsigned int ? ? ? ? ? ?lsr_break_flag;
>> ? ? ? struct clk ? ? ? ? ? ? ?*clk;
>> - ? ? char ? ? ? ? ? ? ? ? ? ?*name;
>> + ? ? char ? ? ? ? ? ? ? ? ? ?name[PXA_NAME_LEN];
>> ?};
>
> This looks better now
>
>> ?static inline unsigned int serial_in(struct uart_pxa_port *up, int offset)
>> @@ -781,6 +784,33 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>> ?};
>> ?#endif
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id serial_pxa_dt_ids[] = {
>> + ? ? { .compatible = "mrvl,pxa-uart", },
>> + ? ? { .compatible = "mrvl,mmp-uart", },
>> + ? ? {}
>> +};
>> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
>> +#endif
>
> If you do this, you have to use of_match_ptr for referencing
> serial_pxa_dt_ids or you will get a link error when CONFIG_OF is
> disabled.
>
>> @@ -850,6 +876,7 @@ static int serial_pxa_remove(struct platform_device *dev)
>> ? ? ? platform_set_drvdata(dev, NULL);
>>
>> ? ? ? uart_remove_one_port(&serial_pxa_reg, &sport->port);
>> + ? ? kfree(sport->name);
>> ? ? ? clk_put(sport->clk);
>> ? ? ? kfree(sport);
>>
>
> And this must be removed now that sport->name is allocated as part of sport.
>
> ? ? ? ?Arnd
>
Ah. It's a bug. I'll fix it tomorrow.

Thanks
Haojian

[PATCH v3 1/6] serial: pxa: add OF support

[Haojian Zhuang]

From: Haojian Zhuang <haojian.zhuang@marvell.com>

Parse uart device id from alias in DTS file.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/tty/serial/pxa.c |   49 ++++++++++++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 5c8e3bb..cdf4b2b 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -36,6 +36,7 @@
 #include <linux/circ_buf.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -44,6 +45,8 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 
+#define PXA_NAME_LEN		8
+
 struct uart_pxa_port {
 	struct uart_port        port;
 	unsigned char           ier;
@@ -51,7 +54,7 @@ struct uart_pxa_port {
 	unsigned char           mcr;
 	unsigned int            lsr_break_flag;
 	struct clk		*clk;
-	char			*name;
+	char			name[PXA_NAME_LEN];
 };
 
 static inline unsigned int serial_in(struct uart_pxa_port *up, int offset)
@@ -781,6 +784,31 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
 };
 #endif
 
+static struct of_device_id serial_pxa_dt_ids[] = {
+	{ .compatible = "mrvl,pxa-uart", },
+	{ .compatible = "mrvl,mmp-uart", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
+
+static int serial_pxa_probe_dt(struct platform_device *pdev,
+			       struct uart_pxa_port *sport)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np)
+		return 1;
+
+	ret = of_alias_get_id(np, "serial");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+	sport->port.line = ret;
+	return 0;
+}
+
 static int serial_pxa_probe(struct platform_device *dev)
 {
 	struct uart_pxa_port *sport;
@@ -808,20 +836,16 @@ static int serial_pxa_probe(struct platform_device *dev)
 	sport->port.irq = irqres->start;
 	sport->port.fifosize = 64;
 	sport->port.ops = &serial_pxa_pops;
-	sport->port.line = dev->id;
 	sport->port.dev = &dev->dev;
 	sport->port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
 	sport->port.uartclk = clk_get_rate(sport->clk);
 
-	switch (dev->id) {
-	case 0: sport->name = "FFUART"; break;
-	case 1: sport->name = "BTUART"; break;
-	case 2: sport->name = "STUART"; break;
-	case 3: sport->name = "HWUART"; break;
-	default:
-		sport->name = "???";
-		break;
-	}
+	ret = serial_pxa_probe_dt(dev, sport);
+	if (ret > 0)
+		sport->port.line = dev->id;
+	else if (ret < 0)
+		goto err_clk;
+	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
 
 	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
 	if (!sport->port.membase) {
@@ -829,7 +853,7 @@ static int serial_pxa_probe(struct platform_device *dev)
 		goto err_clk;
 	}
 
-	serial_pxa_ports[dev->id] = sport;
+	serial_pxa_ports[sport->port.line] = sport;
 
 	uart_add_one_port(&serial_pxa_reg, &sport->port);
 	platform_set_drvdata(dev, sport);
@@ -866,6 +890,7 @@ static struct platform_driver serial_pxa_driver = {
 #ifdef CONFIG_PM
 		.pm	= &serial_pxa_pm_ops,
 #endif
+		.of_match_table = serial_pxa_dt_ids,
 	},
 };
 
-- 
1.7.5.4

[PATCH v3 1/6] serial: pxa: add OF support

[Arnd Bergmann]

On Wednesday 07 March 2012, Haojian Zhuang wrote:
> From: Haojian Zhuang <haojian.zhuang@marvell.com>
> 
> Parse uart device id from alias in DTS file.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Looks good now,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 1/7] serial: pxa: add OF support

[Grant Likely]

On Mon,  5 Mar 2012 20:21:45 +0800, Haojian Zhuang <haojian.zhuang@marvell.com> wrote:
> Parse uart device id from alias in DTS file.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 53 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
> index 5c8e3bb..a71855b 100644
> --- a/drivers/tty/serial/pxa.c
> +++ b/drivers/tty/serial/pxa.c
> @@ -36,6 +36,7 @@
>  #include <linux/circ_buf.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/tty.h>
>  #include <linux/tty_flip.h>
> @@ -44,6 +45,8 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  
> +#define PXA_NAME_LEN		8
> +
>  struct uart_pxa_port {
>  	struct uart_port        port;
>  	unsigned char           ier;
> @@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>  };
>  #endif
>  
> +static struct of_device_id serial_pxa_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-uart", },
> +	{ .compatible = "mrvl,mmp-uart", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
> +
> +#ifdef CONFIG_OF
> +static int serial_pxa_probe_dt(struct platform_device *pdev,
> +			       struct uart_pxa_port *sport)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	int ret;
> +
> +	if (!np)
> +		return 1;
> +
> +	ret = of_alias_get_id(np, "serial");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> +		return ret;
> +	}
> +	sport->port.line = ret;
> +	return 0;
> +}
> +#else
> +static int serial_pxa_probe_dt(struct platform_device *pdev,
> +			       struct uart_pxa_port *sport)
> +{
> +	return 1;
> +}
> +#endif

Returning 1 on success is rather odd... see comment below.

> +
>  static int serial_pxa_probe(struct platform_device *dev)
>  {
>  	struct uart_pxa_port *sport;
> @@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev)
>  	sport->port.irq = irqres->start;
>  	sport->port.fifosize = 64;
>  	sport->port.ops = &serial_pxa_pops;
> -	sport->port.line = dev->id;
>  	sport->port.dev = &dev->dev;
>  	sport->port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
>  	sport->port.uartclk = clk_get_rate(sport->clk);
> -
> -	switch (dev->id) {
> -	case 0: sport->name = "FFUART"; break;
> -	case 1: sport->name = "BTUART"; break;
> -	case 2: sport->name = "STUART"; break;
> -	case 3: sport->name = "HWUART"; break;
> -	default:
> -		sport->name = "???";
> -		break;
> +	sport->name = kzalloc(PXA_NAME_LEN, GFP_KERNEL);

devm_kzalloc().  Then you don't need to unwind the allocation on failure.

> +	if (!sport->name) {
> +		ret = -ENOMEM;
> +		goto err_clk;
>  	}
>  
> +	ret = serial_pxa_probe_dt(dev, sport);
> +	if (ret > 0)
> +		sport->port.line = dev->id;
> +	else if (ret < 0)
> +		goto err_clk;
> +	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);

returning a -/0/+ value from serial_pxa_probe_dt() is rather odd.  It's easier
to follow code which sticks to the convention of <0 for error, 0 for success.
I would do:

	sport->port.line = dev->id;
	ret = serial_pxa_probe_dt(dev, sport);
	if (ret < 0)
		goto err_clk;

Then serial_pxa_probe_dt() can return an error code on failure, or 0 on
success or no-devicetree.


> +
>  	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
>  	if (!sport->port.membase) {
>  		ret = -ENOMEM;
> -		goto err_clk;
> +		goto err_name;
>  	}
>  
> -	serial_pxa_ports[dev->id] = sport;
> +	serial_pxa_ports[sport->port.line] = sport;
>  
>  	uart_add_one_port(&serial_pxa_reg, &sport->port);
>  	platform_set_drvdata(dev, sport);
>  
>  	return 0;
>  
> + err_name:
> +	kfree(sport->name);
>   err_clk:
>  	clk_put(sport->clk);
>   err_free:
> @@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)
>  	platform_set_drvdata(dev, NULL);
>  
>  	uart_remove_one_port(&serial_pxa_reg, &sport->port);
> +	kfree(sport->name);
>  	clk_put(sport->clk);
>  	kfree(sport);
>  
> @@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {
>  #ifdef CONFIG_PM
>  		.pm	= &serial_pxa_pm_ops,
>  #endif
> +		.of_match_table = serial_pxa_dt_ids,

	.of_match_table = of_match_ptr(serial_pxa_dt_ids),

Otherwise looks great to me.

g.

[PATCH v2 1/7] serial: pxa: add OF support

[Grant Likely]

On Mon,  5 Mar 2012 20:21:45 +0800, Haojian Zhuang <haojian.zhuang@marvell.com> wrote:
> Parse uart device id from alias in DTS file.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 53 insertions(+), 12 deletions(-)

Binding documentation!

> 
> diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
> index 5c8e3bb..a71855b 100644
> --- a/drivers/tty/serial/pxa.c
> +++ b/drivers/tty/serial/pxa.c
> @@ -36,6 +36,7 @@
>  #include <linux/circ_buf.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/tty.h>
>  #include <linux/tty_flip.h>
> @@ -44,6 +45,8 @@
>  #include <linux/io.h>
>  #include <linux/slab.h>
>  
> +#define PXA_NAME_LEN		8
> +
>  struct uart_pxa_port {
>  	struct uart_port        port;
>  	unsigned char           ier;
> @@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
>  };
>  #endif
>  
> +static struct of_device_id serial_pxa_dt_ids[] = {
> +	{ .compatible = "mrvl,pxa-uart", },
> +	{ .compatible = "mrvl,mmp-uart", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, serial_pxa_dt_ids);
> +
> +#ifdef CONFIG_OF
> +static int serial_pxa_probe_dt(struct platform_device *pdev,
> +			       struct uart_pxa_port *sport)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	int ret;
> +
> +	if (!np)
> +		return 1;
> +
> +	ret = of_alias_get_id(np, "serial");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> +		return ret;
> +	}
> +	sport->port.line = ret;
> +	return 0;
> +}
> +#else
> +static int serial_pxa_probe_dt(struct platform_device *pdev,
> +			       struct uart_pxa_port *sport)
> +{
> +	return 1;
> +}
> +#endif
> +
>  static int serial_pxa_probe(struct platform_device *dev)
>  {
>  	struct uart_pxa_port *sport;
> @@ -808,34 +844,37 @@ static int serial_pxa_probe(struct platform_device *dev)
>  	sport->port.irq = irqres->start;
>  	sport->port.fifosize = 64;
>  	sport->port.ops = &serial_pxa_pops;
> -	sport->port.line = dev->id;
>  	sport->port.dev = &dev->dev;
>  	sport->port.flags = UPF_IOREMAP | UPF_BOOT_AUTOCONF;
>  	sport->port.uartclk = clk_get_rate(sport->clk);
> -
> -	switch (dev->id) {
> -	case 0: sport->name = "FFUART"; break;
> -	case 1: sport->name = "BTUART"; break;
> -	case 2: sport->name = "STUART"; break;
> -	case 3: sport->name = "HWUART"; break;
> -	default:
> -		sport->name = "???";
> -		break;
> +	sport->name = kzalloc(PXA_NAME_LEN, GFP_KERNEL);

devm_kzalloc()

> +	if (!sport->name) {
> +		ret = -ENOMEM;
> +		goto err_clk;
>  	}
>  
> +	ret = serial_pxa_probe_dt(dev, sport);
> +	if (ret > 0)
> +		sport->port.line = dev->id;
> +	else if (ret < 0)
> +		goto err_clk;
> +	snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);

Why +1 here?  Won't that just cause confusion when compared with the
DT aliases?

> +
>  	sport->port.membase = ioremap(mmres->start, resource_size(mmres));
>  	if (!sport->port.membase) {
>  		ret = -ENOMEM;
> -		goto err_clk;
> +		goto err_name;
>  	}
>  
> -	serial_pxa_ports[dev->id] = sport;
> +	serial_pxa_ports[sport->port.line] = sport;
>  
>  	uart_add_one_port(&serial_pxa_reg, &sport->port);
>  	platform_set_drvdata(dev, sport);
>  
>  	return 0;
>  
> + err_name:
> +	kfree(sport->name);
>   err_clk:
>  	clk_put(sport->clk);
>   err_free:
> @@ -850,6 +889,7 @@ static int serial_pxa_remove(struct platform_device *dev)
>  	platform_set_drvdata(dev, NULL);
>  
>  	uart_remove_one_port(&serial_pxa_reg, &sport->port);
> +	kfree(sport->name);
>  	clk_put(sport->clk);
>  	kfree(sport);
>  
> @@ -866,6 +906,7 @@ static struct platform_driver serial_pxa_driver = {
>  #ifdef CONFIG_PM
>  		.pm	= &serial_pxa_pm_ops,
>  #endif
> +		.of_match_table = serial_pxa_dt_ids,
>  	},
>  };
>  
> -- 
> 1.7.0.4
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.

[PATCH v2 2/7] rtc: sa1100: add OF support

[Haojian Zhuang]

Add OF support on rtc-sa1100.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/rtc/rtc-sa1100.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index 962510c..e443b78 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -30,6 +30,7 @@
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/of.h>
 #include <linux/pm.h>
 #include <linux/bitops.h>
 
@@ -349,6 +350,13 @@ static const struct dev_pm_ops sa1100_rtc_pm_ops = {
 };
 #endif
 
+static struct of_device_id sa1100_rtc_dt_ids[] = {
+	{ .compatible = "mrvl,sa1100-rtc", },
+	{ .compatible = "mrvl,mmp-rtc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sa1100_rtc_dt_ids);
+
 static struct platform_driver sa1100_rtc_driver = {
 	.probe		= sa1100_rtc_probe,
 	.remove		= sa1100_rtc_remove,
@@ -357,6 +365,7 @@ static struct platform_driver sa1100_rtc_driver = {
 #ifdef CONFIG_PM
 		.pm	= &sa1100_rtc_pm_ops,
 #endif
+		.of_match_table = sa1100_rtc_dt_ids,
 	},
 };
 
-- 
1.7.0.4

[PATCH v2 2/7] rtc: sa1100: add OF support

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> Add OF support on rtc-sa1100.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 3/7] i2c: pxa: add OF support

[Haojian Zhuang]

Append these properties in below.
mrvl,i2c-polling
mrvl,i2c-fast-mode

Still keep slave, slave_addr and class in platform data.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/i2c/busses/i2c-pxa.c |   95 ++++++++++++++++++++++++++++++++---------
 1 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index d603646..f673326 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -29,6 +29,8 @@
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/i2c-pxa.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_i2c.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
@@ -1044,23 +1046,60 @@ static const struct i2c_algorithm i2c_pxa_pio_algorithm = {
 	.functionality	= i2c_pxa_functionality,
 };
 
-static int i2c_pxa_probe(struct platform_device *dev)
+static struct of_device_id i2c_pxa_dt_ids[] = {
+	{ .compatible = "mrvl,pxa-i2c", .data = (void *)REGS_PXA2XX },
+	{ .compatible = "mrvl,pwri2c", .data = (void *)REGS_PXA3XX },
+	{ .compatible = "mrvl,mmp-twsi", .data = (void *)REGS_PXA2XX },
+	{}
+};
+MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
+
+static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
+			    enum pxa_i2c_types *i2c_types)
 {
-	struct pxa_i2c *i2c;
-	struct resource *res;
-	struct i2c_pxa_platform_data *plat = dev->dev.platform_data;
-	const struct platform_device_id *id = platform_get_device_id(dev);
-	enum pxa_i2c_types i2c_type = id->driver_data;
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *of_id =
+			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
 	int ret;
-	int irq;
 
-	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
-	irq = platform_get_irq(dev, 0);
-	if (res == NULL || irq < 0)
-		return -ENODEV;
+	if (!of_id)
+		return 1;
+	ret = of_alias_get_id(np, "i2c");
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+	pdev->id = ret;
+	if (of_get_property(np, "mrvl,i2c-polling", NULL))
+		i2c->use_pio = 1;
+	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
+		i2c->fast_mode = 1;
+	*i2c_types = (u32)(of_id->data);
+	return 0;
+}
 
-	if (!request_mem_region(res->start, resource_size(res), res->name))
-		return -ENOMEM;
+static int i2c_pxa_probe_pdata(struct platform_device *pdev,
+			       struct pxa_i2c *i2c,
+			       enum pxa_i2c_types *i2c_types)
+{
+	struct i2c_pxa_platform_data *plat = pdev->dev.platform_data;
+	const struct platform_device_id *id = platform_get_device_id(pdev);
+
+	*i2c_types = id->driver_data;
+	if (plat) {
+		i2c->use_pio = plat->use_pio;
+		i2c->fast_mode = plat->fast_mode;
+	}
+	return 0;
+}
+
+static int i2c_pxa_probe(struct platform_device *dev)
+{
+	struct i2c_pxa_platform_data *plat = dev->dev.platform_data;
+	enum pxa_i2c_types i2c_type;
+	struct pxa_i2c *i2c;
+	struct resource *res = NULL;
+	int ret, irq;
 
 	i2c = kzalloc(sizeof(struct pxa_i2c), GFP_KERNEL);
 	if (!i2c) {
@@ -1068,6 +1107,24 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		goto emalloc;
 	}
 
+	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
+	if (ret > 0)
+		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
+	if (ret < 0)
+		goto eclk;
+
+	res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(dev, 0);
+	if (res == NULL || irq < 0) {
+		ret = -ENODEV;
+		goto eclk;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res), res->name)) {
+		ret = -ENOMEM;
+		goto eclk;
+	}
+
 	i2c->adap.owner   = THIS_MODULE;
 	i2c->adap.retries = 5;
 
@@ -1109,21 +1166,16 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	i2c->slave_addr = I2C_PXA_SLAVE_ADDR;
 
-#ifdef CONFIG_I2C_PXA_SLAVE
 	if (plat) {
+#ifdef CONFIG_I2C_PXA_SLAVE
 		i2c->slave_addr = plat->slave_addr;
 		i2c->slave = plat->slave;
-	}
 #endif
-
-	clk_enable(i2c->clk);
-
-	if (plat) {
 		i2c->adap.class = plat->class;
-		i2c->use_pio = plat->use_pio;
-		i2c->fast_mode = plat->fast_mode;
 	}
 
+	clk_enable(i2c->clk);
+
 	if (i2c->use_pio) {
 		i2c->adap.algo = &i2c_pxa_pio_algorithm;
 	} else {
@@ -1234,6 +1286,7 @@ static struct platform_driver i2c_pxa_driver = {
 		.name	= "pxa2xx-i2c",
 		.owner	= THIS_MODULE,
 		.pm	= I2C_PXA_DEV_PM_OPS,
+		.of_match_table = i2c_pxa_dt_ids,
 	},
 	.id_table	= i2c_pxa_id_table,
 };
-- 
1.7.0.4

[PATCH v2 3/7] i2c: pxa: add OF support

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> Append these properties in below.
> mrvl,i2c-polling
> mrvl,i2c-fast-mode
> 
> Still keep slave, slave_addr and class in platform data.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

Just one tiny detail:

> +       *i2c_types = (u32)(of_id->data);

By convention, please use (unsigned long) as the cast from pointer to an
integer type. While this doesn't make any difference on 32 bit systems,
it's better to always write code that is 64-bit clean.

	Arnd

[PATCH v2 4/7] ARM: mmp: enable rtc clk in pxa168

[Haojian Zhuang]

Enable clk of rtc-sa1100 device in pxa168.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 arch/arm/mach-mmp/pxa168.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mmp/pxa168.c b/arch/arm/mach-mmp/pxa168.c
index 9f61256..aa62d0c 100644
--- a/arch/arm/mach-mmp/pxa168.c
+++ b/arch/arm/mach-mmp/pxa168.c
@@ -65,6 +65,7 @@ static APBC_CLK(ssp4, PXA168_SSP4, 4, 0);
 static APBC_CLK(ssp5, PXA168_SSP5, 4, 0);
 static APBC_CLK(gpio, PXA168_GPIO, 0, 13000000);
 static APBC_CLK(keypad, PXA168_KPC, 0, 32000);
+static APBC_CLK(rtc, PXA168_RTC, 8, 32768);
 
 static APMU_CLK(nand, NAND, 0x19b, 156000000);
 static APMU_CLK(lcd, LCD, 0x7f, 312000000);
@@ -93,6 +94,7 @@ static struct clk_lookup pxa168_clkregs[] = {
 	INIT_CLKREG(&clk_keypad, "pxa27x-keypad", NULL),
 	INIT_CLKREG(&clk_eth, "pxa168-eth", "MFUCLK"),
 	INIT_CLKREG(&clk_usb, "pxa168-ehci", "PXA168-USBCLK"),
+	INIT_CLKREG(&clk_rtc, "sa1100-rtc", NULL),
 };
 
 static int __init pxa168_init(void)
-- 
1.7.0.4

[PATCH v2 4/7] ARM: mmp: enable rtc clk in pxa168

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> Enable clk of rtc-sa1100 device in pxa168.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Haojian Zhuang]

Enable PXA168 and aspenite support.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 arch/arm/mach-mmp/Kconfig  |   10 ++++++
 arch/arm/mach-mmp/Makefile |    1 +
 arch/arm/mach-mmp/mmp-dt.c |   75 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-mmp/mmp-dt.c

diff --git a/arch/arm/mach-mmp/Kconfig b/arch/arm/mach-mmp/Kconfig
index 323d4c9..5a90b9a 100644
--- a/arch/arm/mach-mmp/Kconfig
+++ b/arch/arm/mach-mmp/Kconfig
@@ -2,6 +2,16 @@ if ARCH_MMP
 
 menu "Marvell PXA168/910/MMP2 Implmentations"
 
+config MACH_MMP_DT
+	bool "Support MMP2 platforms from device tree"
+	select CPU_PXA168
+	select CPU_PXA910
+	select USE_OF
+	help
+	  Include support for Marvell MMP2 based platforms using
+	  the device tree. Needn't select any other machine while
+	  MACH_MMP_DT is enabled.
+
 config MACH_ASPENITE
 	bool "Marvell's PXA168 Aspenite Development Board"
 	select CPU_PXA168
diff --git a/arch/arm/mach-mmp/Makefile b/arch/arm/mach-mmp/Makefile
index ba254a7..4fc0ff5 100644
--- a/arch/arm/mach-mmp/Makefile
+++ b/arch/arm/mach-mmp/Makefile
@@ -18,5 +18,6 @@ obj-$(CONFIG_MACH_TTC_DKB)	+= ttc_dkb.o
 obj-$(CONFIG_MACH_BROWNSTONE)	+= brownstone.o
 obj-$(CONFIG_MACH_FLINT)	+= flint.o
 obj-$(CONFIG_MACH_MARVELL_JASPER) += jasper.o
+obj-$(CONFIG_MACH_MMP_DT)	+= mmp-dt.o
 obj-$(CONFIG_MACH_TETON_BGA)	+= teton_bga.o
 obj-$(CONFIG_MACH_GPLUGD)	+= gplugd.o
diff --git a/arch/arm/mach-mmp/mmp-dt.c b/arch/arm/mach-mmp/mmp-dt.c
new file mode 100644
index 0000000..6707539
--- /dev/null
+++ b/arch/arm/mach-mmp/mmp-dt.c
@@ -0,0 +1,75 @@
+/*
+ *  linux/arch/arm/mach-mmp/mmp-dt.c
+ *
+ *  Copyright (C) 2012 Marvell Technology Group Ltd.
+ *  Author: Haojian Zhuang <haojian.zhuang@marvell.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <asm/mach/arch.h>
+#include <mach/irqs.h>
+
+#include "common.h"
+
+extern struct sys_timer pxa168_timer;
+extern void __init icu_init_irq(void);
+
+static const struct of_dev_auxdata mmp_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4017000, "pxa2xx-uart.0", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4018000, "pxa2xx-uart.1", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-uart", 0xd4026000, "pxa2xx-uart.2", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-twsi", 0xd4011000, "pxa2xx-i2c.0", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-twsi", 0xd4025000, "pxa2xx-i2c.1", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-gpio", 0xd4019000, "pxa-gpio", NULL),
+	OF_DEV_AUXDATA("mrvl,mmp-rtc", 0xd4010000, "sa1100-rtc", NULL),
+	{}
+};
+
+static int __init mmp_intc_add_irq_domain(struct device_node *np,
+					   struct device_node *parent)
+{
+	irq_domain_add_simple(np, 0);
+	return 0;
+}
+
+static int __init mmp_gpio_add_irq_domain(struct device_node *np,
+					   struct device_node *parent)
+{
+	irq_domain_add_simple(np, IRQ_GPIO_START);
+	return 0;
+}
+
+static const struct of_device_id mmp_irq_match[] __initconst = {
+	{ .compatible = "mrvl,mmp-intc", .data = mmp_intc_add_irq_domain, },
+	{ .compatible = "mrvl,mmp-gpio", .data = mmp_gpio_add_irq_domain, },
+	{}
+};
+
+static void __init mmp_dt_init(void)
+{
+
+	of_irq_init(mmp_irq_match);
+
+	of_platform_populate(NULL, of_default_bus_match_table,
+			     mmp_auxdata_lookup, NULL);
+}
+
+static const char *pxa168_dt_board_compat[] __initdata = {
+	"mrvl,pxa168-aspenite",
+	NULL,
+};
+
+DT_MACHINE_START(PXA168_DT, "Marvell PXA168 (Device Tree Support)")
+	.map_io		= mmp_map_io,
+	.init_irq	= icu_init_irq,
+	.timer		= &pxa168_timer,
+	.init_machine	= mmp_dt_init,
+	.dt_compat	= pxa168_dt_board_compat,
+MACHINE_END
-- 
1.7.0.4

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> Enable PXA168 and aspenite support.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Chris Ball]

Hi,

On Mon, Mar 05 2012, Haojian Zhuang wrote:
> Enable PXA168 and aspenite support.
>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  arch/arm/mach-mmp/Kconfig  |   10 ++++++
>  arch/arm/mach-mmp/Makefile |    1 +
>  arch/arm/mach-mmp/mmp-dt.c |   75 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-mmp/mmp-dt.c
>
> diff --git a/arch/arm/mach-mmp/Kconfig b/arch/arm/mach-mmp/Kconfig
> index 323d4c9..5a90b9a 100644
> --- a/arch/arm/mach-mmp/Kconfig
> +++ b/arch/arm/mach-mmp/Kconfig
> @@ -2,6 +2,16 @@ if ARCH_MMP
>  
>  menu "Marvell PXA168/910/MMP2 Implmentations"
>  
> +config MACH_MMP_DT
> +	bool "Support MMP2 platforms from device tree"
> +	select CPU_PXA168
> +	select CPU_PXA910
> +	select USE_OF
> +	help
> +	  Include support for Marvell MMP2 based platforms using
> +	  the device tree. Needn't select any other machine while
> +	  MACH_MMP_DT is enabled.
> +

This patch breaks the build with MACH_MMP_DT=y in 3.4-rc2 for me:

MACH_MMP_DT selects CPU_PXA168 which selects CPU_32v5, which results in:

  armv7-unknown-linux-gnueabi-gcc -Wp,-MD,arch/arm/mm/.dma-mapping.o.d  -nostdinc -isystem /opt/crosstool/gcc-4.6.0/lib/gcc/armv7-unknown-linux-gnueabi/4.6.0/include -I/home/cjb/git/olpc-kernel/arch/arm/include -Iarch/arm/include/generated -Iinclude  -I/home/cjb/git/olpc-kernel/include -include /home/cjb/git/olpc-kernel/include/linux/kconfig.h  -I/home/cjb/git/olpc-kernel/arch/arm/mm -Iarch/arm/mm -D__KERNEL__ -mlittle-endian   -I/home/cjb/git/olpc-kernel/arch/arm/mach-mmp/include   -I/home/cjb/git/olpc-kernel/arch/arm/plat-pxa/include -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-delete-null-pointer-checks -Os -marm -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mno-thumb-interwork -funwind-tables -D__LINUX_ARM_ARCH__=5 -march=armv5te -msoft-float -Uarm -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -g -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fconserve-stack    -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(dma_mapping)"  -D"KBUILD_MODNAME=KBUILD_STR(dma_mapping)" -c -o arch/arm/mm/dma-mapping.o /home/cjb/git/olpc-kernel/arch/arm/mm/dma-mapping.c
/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S: Assembler messages:
/home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S:207: Error: selected processor does not support ARM mode `clrex'

CPU_32v5=y adds:

arch/arm/Makefile:arch-$(CONFIG_CPU_32v5) :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Haojian Zhuang]

On Mon, Apr 9, 2012 at 9:36 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Mon, Mar 05 2012, Haojian Zhuang wrote:
>> Enable PXA168 and aspenite support.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>> ---
>> ?arch/arm/mach-mmp/Kconfig ?| ? 10 ++++++
>> ?arch/arm/mach-mmp/Makefile | ? ?1 +
>> ?arch/arm/mach-mmp/mmp-dt.c | ? 75 ++++++++++++++++++++++++++++++++++++++++++++
>> ?3 files changed, 86 insertions(+), 0 deletions(-)
>> ?create mode 100644 arch/arm/mach-mmp/mmp-dt.c
>>
>> diff --git a/arch/arm/mach-mmp/Kconfig b/arch/arm/mach-mmp/Kconfig
>> index 323d4c9..5a90b9a 100644
>> --- a/arch/arm/mach-mmp/Kconfig
>> +++ b/arch/arm/mach-mmp/Kconfig
>> @@ -2,6 +2,16 @@ if ARCH_MMP
>>
>> ?menu "Marvell PXA168/910/MMP2 Implmentations"
>>
>> +config MACH_MMP_DT
>> + ? ? bool "Support MMP2 platforms from device tree"
>> + ? ? select CPU_PXA168
>> + ? ? select CPU_PXA910
>> + ? ? select USE_OF
>> + ? ? help
>> + ? ? ? Include support for Marvell MMP2 based platforms using
>> + ? ? ? the device tree. Needn't select any other machine while
>> + ? ? ? MACH_MMP_DT is enabled.
>> +
>
> This patch breaks the build with MACH_MMP_DT=y in 3.4-rc2 for me:
>
> MACH_MMP_DT selects CPU_PXA168 which selects CPU_32v5, which results in:
>
> ?armv7-unknown-linux-gnueabi-gcc -Wp,-MD,arch/arm/mm/.dma-mapping.o.d ?-nostdinc -isystem /opt/crosstool/gcc-4.6.0/lib/gcc/armv7-unknown-linux-gnueabi/4.6.0/include -I/home/cjb/git/olpc-kernel/arch/arm/include -Iarch/arm/include/generated -Iinclude ?-I/home/cjb/git/olpc-kernel/include -include /home/cjb/git/olpc-kernel/include/linux/kconfig.h ?-I/home/cjb/git/olpc-kernel/arch/arm/mm -Iarch/arm/mm -D__KERNEL__ -mlittle-endian ? -I/home/cjb/git/olpc-kernel/arch/arm/mach-mmp/include ? -I/home/cjb/git/olpc-kernel/arch/arm/plat-pxa/include -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-delete-null-pointer-checks -Os -marm -fno-dwarf2-cfi-asm -mabi=aapcs-linux -mno-thumb-interwork -funwind-tables -D__LINUX_ARM_ARCH__=5 -march=armv5te -msoft-float -Uarm -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -g -Wdeclaration-after-statement -Wno-pointer
> ?-sign -fno-strict-overflow -fconserve-stack ? ?-D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(dma_mapping)" ?-D"KBUILD_MODNAME=KBUILD_STR(dma_mapping)" -c -o arch/arm/mm/dma-mapping.o /home/cjb/git/olpc-kernel/arch/arm/mm/dma-mapping.c
> /home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S: Assembler messages:
> /home/cjb/git/olpc-kernel/arch/arm/kernel/entry-armv.S:207: Error: selected processor does not support ARM mode `clrex'
>
> CPU_32v5=y adds:
>
> arch/arm/Makefile:arch-$(CONFIG_CPU_32v5) :=-D__LINUX_ARM_ARCH__=5 $(call cc-option,-march=armv5te,-march=armv4t)
>

There's a typo error in comments. MMP_DT is only for PXA168 & PXA910.
We need to append MMP2_DT for ARMv7 in arch-mmp directory. Since we
are not intended to build ARMv5 and ARMv7 together.

Thanks
Haojian

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Chris Ball]

Hi,

On Sun, Apr 08 2012, Haojian Zhuang wrote:
> There's a typo error in comments. MMP_DT is only for PXA168 & PXA910.
> We need to append MMP2_DT for ARMv7 in arch-mmp directory. Since we
> are not intended to build ARMv5 and ARMv7 together.

Okay.  In that case, you should make sure that MMP_DT cannot be selected
when CPU_MMP2 is selected, by encoding that dependency in the Kconfig.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Haojian Zhuang]

On Mon, Apr 9, 2012 at 9:51 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Sun, Apr 08 2012, Haojian Zhuang wrote:
>> There's a typo error in comments. MMP_DT is only for PXA168 & PXA910.
>> We need to append MMP2_DT for ARMv7 in arch-mmp directory. Since we
>> are not intended to build ARMv5 and ARMv7 together.
>
> Okay. ?In that case, you should make sure that MMP_DT cannot be selected
> when CPU_MMP2 is selected, by encoding that dependency in the Kconfig.
>
Sure. I'll protect it in Kconfig.

Thanks
Haojian

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Chris Ball]

Hi,

On Mon, Mar 05 2012, Haojian Zhuang wrote:
> +static int __init mmp_intc_add_irq_domain(struct device_node *np,
> +					   struct device_node *parent)
> +{
> +	irq_domain_add_simple(np, 0);
> +	return 0;
> +}
> +
> +static int __init mmp_gpio_add_irq_domain(struct device_node *np,
> +					   struct device_node *parent)
> +{
> +	irq_domain_add_simple(np, IRQ_GPIO_START);
> +	return 0;
> +}

This patch was merged for 3.4-rc1, but it fails to compile because Grant
removed irq_domain_add_simple() in 3.4-rc1.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH v2 5/7] ARM: mmp: append OF support on pxa168

[Haojian Zhuang]

On Mon, Apr 9, 2012 at 9:43 AM, Chris Ball <cjb@laptop.org> wrote:
> Hi,
>
> On Mon, Mar 05 2012, Haojian Zhuang wrote:
>> +static int __init mmp_intc_add_irq_domain(struct device_node *np,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device_node *parent)
>> +{
>> + ? ? irq_domain_add_simple(np, 0);
>> + ? ? return 0;
>> +}
>> +
>> +static int __init mmp_gpio_add_irq_domain(struct device_node *np,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct device_node *parent)
>> +{
>> + ? ? irq_domain_add_simple(np, IRQ_GPIO_START);
>> + ? ? return 0;
>> +}
>
> This patch was merged for 3.4-rc1, but it fails to compile because Grant
> removed irq_domain_add_simple() in 3.4-rc1.
>
Noticed. I'll fix it.

Best Regards
Haojian

[PATCH v2 6/7] ARM: dts: append DTS file of pxa168

[Haojian Zhuang]

DTS files of both PXA168 and aspenite are appended.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 arch/arm/boot/dts/pxa168-aspenite.dts |   38 +++++++++++++
 arch/arm/boot/dts/pxa168.dtsi         |   98 +++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/pxa168-aspenite.dts
 create mode 100644 arch/arm/boot/dts/pxa168.dtsi

diff --git a/arch/arm/boot/dts/pxa168-aspenite.dts b/arch/arm/boot/dts/pxa168-aspenite.dts
new file mode 100644
index 0000000..e762fac
--- /dev/null
+++ b/arch/arm/boot/dts/pxa168-aspenite.dts
@@ -0,0 +1,38 @@
+/*
+ *  Copyright (C) 2012 Marvell Technology Group Ltd.
+ *  Author: Haojian Zhuang <haojian.zhuang@marvell.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/dts-v1/;
+/include/ "pxa168.dtsi"
+
+/ {
+	model = "Marvell PXA168 Aspenite Development Board";
+	compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
+
+	chosen {
+		bootargs = "console=ttyS0,115200 root=/dev/nfs nfsroot=192.168.1.100:/nfsroot/ ip=192.168.1.101:192.168.1.100::255.255.255.0::eth0:on";
+	};
+
+	memory {
+		reg = <0x00000000 0x04000000>;
+	};
+
+	soc {
+		apb at d4000000 {
+			uart1: uart at d4017000 {
+				status = "okay";
+			};
+			twsi1: i2c at d4011000 {
+				status = "okay";
+			};
+			rtc: rtc at d4010000 {
+				status = "okay";
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/pxa168.dtsi b/arch/arm/boot/dts/pxa168.dtsi
new file mode 100644
index 0000000..d32d512
--- /dev/null
+++ b/arch/arm/boot/dts/pxa168.dtsi
@@ -0,0 +1,98 @@
+/*
+ *  Copyright (C) 2012 Marvell Technology Group Ltd.
+ *  Author: Haojian Zhuang <haojian.zhuang@marvell.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  publishhed by the Free Software Foundation.
+ */
+
+/include/ "skeleton.dtsi"
+
+/ {
+	aliases {
+		serial0 = &uart1;
+		serial1 = &uart2;
+		serial2 = &uart3;
+		i2c0 = &twsi1;
+		i2c1 = &twsi2;
+	};
+
+	intc: intc-interrupt-controller at d4282000 {
+		compatible = "mrvl,mmp-intc", "mrvl,intc";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = <0xd4282000 0x1000>;
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&intc>;
+		ranges;
+
+		apb at d4000000 {	/* APB */
+			compatible = "mrvl,apb-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0xd4000000 0x00200000>;
+			ranges;
+
+			uart1: uart at d4017000 {
+				compatible = "mrvl,mmp-uart", "mrvl,pxa-uart";
+				reg = <0xd4017000 0x1000>;
+				interrupts = <27>;
+				status = "disabled";
+			};
+
+			uart2: uart at d4018000 {
+				compatible = "mrvl,mmp-uart", "mrvl,pxa-uart";
+				reg = <0xd4018000 0x1000>;
+				interrupts = <28>;
+				status = "disabled";
+			};
+
+			uart3: uart at d4026000 {
+				compatible = "mrvl,mmp-uart", "mrvl,pxa-uart";
+				reg = <0xd4026000 0x1000>;
+				interrupts = <29>;
+				status = "disabled";
+			};
+
+			gpio: gpio at d4019000 {
+				compatible = "mrvl,mmp-gpio", "mrvl,pxa-gpio";
+				reg = <0xd4019000 0x1000>;
+				interrupts = <49>;
+				interrupt-names = "gpio_mux";
+				gpio-controller;
+				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+
+			twsi1: i2c at d4011000 {
+				compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
+				reg = <0xd4011000 0x1000>;
+				interrupts = <7>;
+				mrvl,i2c-fast-mode;
+				status = "disabled";
+			};
+
+			twsi2: i2c at d4025000 {
+				compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
+				reg = <0xd4025000 0x1000>;
+				interrupts = <58>;
+				status = "disabled";
+			};
+
+			rtc: rtc at d4010000 {
+				compatible = "mrvl,mmp-rtc";
+				reg = <0xd4010000 0x1000>;
+				interrupts = <5 6>;
+				interrupt-names = "rtc 1Hz", "rtc alarm";
+				status = "disabled";
+			};
+		};
+	};
+};
-- 
1.7.0.4

[PATCH v2 6/7] ARM: dts: append DTS file of pxa168

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> 
> DTS files of both PXA168 and aspenite are appended.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 7/7] Document: devicetree: add OF documents for arch-mmp

[Haojian Zhuang]

Add OF support in Document/devicetree directory.

Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 Documentation/devicetree/bindings/arm/mrvl.txt     |    6 +++
 .../devicetree/bindings/gpio/mrvl-gpio.txt         |   23 ++++++++++++
 Documentation/devicetree/bindings/i2c/mrvl-i2c.txt |   37 ++++++++++++++++++++
 .../devicetree/bindings/rtc/sa1100-rtc.txt         |   17 +++++++++
 .../devicetree/bindings/serial/mrvl-serial.txt     |    4 ++
 5 files changed, 87 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/mrvl.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
 create mode 100644 Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
 create mode 100644 Documentation/devicetree/bindings/serial/mrvl-serial.txt

diff --git a/Documentation/devicetree/bindings/arm/mrvl.txt b/Documentation/devicetree/bindings/arm/mrvl.txt
new file mode 100644
index 0000000..d8de933
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mrvl.txt
@@ -0,0 +1,6 @@
+Marvell Platforms Device Tree Bindings
+----------------------------------------------------
+
+PXA168 Aspenite Board
+Required root node properties:
+	- compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
diff --git a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
new file mode 100644
index 0000000..1e34cfe
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
@@ -0,0 +1,23 @@
+* Marvell PXA GPIO controller
+
+Required properties:
+- compatible : Should be "mrvl,pxa-gpio" or "mrvl,mmp-gpio"
+- reg : Address and length of the register set for the device
+- interrupts : Should be the port interrupt shared by all gpio pins, if
+- interrupt-name : Should be the name of irq resource.
+  one number.
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be one.  It is the pin number.
+
+Example:
+
+	gpio: gpio at d4019000 {
+		compatible = "mrvl,mmp-gpio", "mrvl,pxa-gpio";
+		reg = <0xd4019000 0x1000>;
+		interrupts = <49>, <17>, <18>;
+		interrupt-name = "gpio_mux", "gpio0", "gpio1";
+		gpio-controller;
+		#gpio-cells = <1>;
+		interrupt-controller;
+		#interrupt-cells = <1>;
+      };
diff --git a/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt b/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
new file mode 100644
index 0000000..071eb3c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
@@ -0,0 +1,37 @@
+* I2C
+
+Required properties :
+
+ - reg : Offset and length of the register set for the device
+ - compatible : should be "mrvl,mmp-twsi" where CHIP is the name of a
+   compatible processor, e.g. pxa168, pxa910, mmp2, mmp3.
+   For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required
+   as shown in the example below.
+
+Recommended properties :
+
+ - interrupts : <a b> where a is the interrupt number and b is a
+   field that represents an encoding of the sense and level
+   information for the interrupt.  This should be encoded based on
+   the information in section 2) depending on the type of interrupt
+   controller you have.
+ - interrupt-parent : the phandle for the interrupt controller that
+   services interrupts for this device.
+ - mrvl,i2c-polling : Disable interrupt of i2c controller. Polling
+   status register of i2c controller instead.
+ - mrvl,i2c-fast-mode : Enable fast mode of i2c controller.
+
+Examples:
+	twsi1: i2c at d4011000 {
+		compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
+		reg = <0xd4011000 0x1000>;
+		interrupts = <7>;
+		mrvl,i2c-fast-mode;
+	};
+	
+	twsi2: i2c at d4025000 {
+		compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";
+		reg = <0xd4025000 0x1000>;
+		interrupts = <58>;
+	};
+
diff --git a/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt b/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
new file mode 100644
index 0000000..0cda19a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
@@ -0,0 +1,17 @@
+* Marvell Real Time Clock controller
+
+Required properties:
+- compatible: should be "mrvl,sa1100-rtc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: Should be two. The first interrupt number is the rtc alarm
+  interrupt and the second interrupt number is the rtc hz interrupt.
+- interrupt-names: Assign name of irq resource.
+
+Example:
+	rtc: rtc at d4010000 {
+		compatible = "mrvl,mmp-rtc";
+		reg = <0xd4010000 0x1000>;
+		interrupts = <5>, <6>;
+		interrupt-name = "rtc 1Hz", "rtc alarm";
+	};
diff --git a/Documentation/devicetree/bindings/serial/mrvl-serial.txt b/Documentation/devicetree/bindings/serial/mrvl-serial.txt
new file mode 100644
index 0000000..d744340
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/mrvl-serial.txt
@@ -0,0 +1,4 @@
+PXA UART controller
+
+Required properties:
+- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".
-- 
1.7.0.4

[PATCH v2 7/7] Document: devicetree: add OF documents for arch-mmp

[Arnd Bergmann]

On Monday 05 March 2012, Haojian Zhuang wrote:
> Add OF support in Document/devicetree directory.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>

Acked-by: Arnd Bergmann <arnd@arndb.de>

One question for Benoit, Grant or Rob:

> +Required properties:
> +- compatible: should be "mrvl,sa1100-rtc"
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +- interrupts: Should be two. The first interrupt number is the rtc alarm
> +  interrupt and the second interrupt number is the rtc hz interrupt.
> +- interrupt-names: Assign name of irq resource.
> +
> +Example:
> +	rtc: rtc at d4010000 {
> +		compatible = "mrvl,mmp-rtc";
> +		reg = <0xd4010000 0x1000>;
> +		interrupts = <5>, <6>;
> +		interrupt-name = "rtc 1Hz", "rtc alarm";
> +	};

When the binding for a device lists the interrupt-names property, should it
also list the specific values for it? This one says that which ones they are,
but I think we should actually mandate the exact string in the binding if
we want to allow drivers to get the resource by its name.

	Arnd

[PATCH v2 7/7] Document: devicetree: add OF documents for arch-mmp

[Rob Herring]

On 03/05/2012 08:46 AM, Arnd Bergmann wrote:
> On Monday 05 March 2012, Haojian Zhuang wrote:
>> Add OF support in Document/devicetree directory.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> One question for Benoit, Grant or Rob:
> 
>> +Required properties:
>> +- compatible: should be "mrvl,sa1100-rtc"
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +- interrupts: Should be two. The first interrupt number is the rtc alarm
>> +  interrupt and the second interrupt number is the rtc hz interrupt.
>> +- interrupt-names: Assign name of irq resource.
>> +
>> +Example:
>> +	rtc: rtc at d4010000 {
>> +		compatible = "mrvl,mmp-rtc";
>> +		reg = <0xd4010000 0x1000>;
>> +		interrupts = <5>, <6>;
>> +		interrupt-name = "rtc 1Hz", "rtc alarm";
>> +	};
> 
> When the binding for a device lists the interrupt-names property, should it
> also list the specific values for it? This one says that which ones they are,
> but I think we should actually mandate the exact string in the binding if
> we want to allow drivers to get the resource by its name.

Sounds good to me. Requiring documentation is a good way to discourage
its use. :)

Ideally we would discourage spaces in the names, but since the whole
point is using the existing names from Linux we can't really enforce that.

Rob

[PATCH v2 7/7] Document: devicetree: add OF documents for arch-mmp

[Cousson, Benoit]

On 3/5/2012 3:46 PM, Arnd Bergmann wrote:
> On Monday 05 March 2012, Haojian Zhuang wrote:
>> Add OF support in Document/devicetree directory.
>>
>> Signed-off-by: Haojian Zhuang<haojian.zhuang@marvell.com>
>
> Acked-by: Arnd Bergmann<arnd@arndb.de>
>
> One question for Benoit, Grant or Rob:
>
>> +Required properties:
>> +- compatible: should be "mrvl,sa1100-rtc"
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +- interrupts: Should be two. The first interrupt number is the rtc alarm
>> +  interrupt and the second interrupt number is the rtc hz interrupt.
>> +- interrupt-names: Assign name of irq resource.
>> +
>> +Example:
>> +	rtc: rtc at d4010000 {
>> +		compatible = "mrvl,mmp-rtc";
>> +		reg =<0xd4010000 0x1000>;
>> +		interrupts =<5>,<6>;
>> +		interrupt-name = "rtc 1Hz", "rtc alarm";
>> +	};
>
> When the binding for a device lists the interrupt-names property, should it
> also list the specific values for it? This one says that which ones they are,
> but I think we should actually mandate the exact string in the binding if
> we want to allow drivers to get the resource by its name.

Yes, indeed. The names are not some random strings, the driver will use 
get_resource_by_name and expect the name to be aligned with HW 
documentation and thus should be well documented in the binding.

Regards,
Benoit

[PATCH v2 7/7] Document: devicetree: add OF documents for arch-mmp

[Mitch Bradley]

There are some inconsistencies in this patch as indicated in-line below:


On 3/5/2012 2:21 AM, Haojian Zhuang wrote:
> Add OF support in Document/devicetree directory.
>
> Signed-off-by: Haojian Zhuang<haojian.zhuang@marvell.com>
> ---
>   Documentation/devicetree/bindings/arm/mrvl.txt     |    6 +++
>   .../devicetree/bindings/gpio/mrvl-gpio.txt         |   23 ++++++++++++
>   Documentation/devicetree/bindings/i2c/mrvl-i2c.txt |   37 ++++++++++++++++++++
>   .../devicetree/bindings/rtc/sa1100-rtc.txt         |   17 +++++++++
>   .../devicetree/bindings/serial/mrvl-serial.txt     |    4 ++
>   5 files changed, 87 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/arm/mrvl.txt
>   create mode 100644 Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
>   create mode 100644 Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
>   create mode 100644 Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
>   create mode 100644 Documentation/devicetree/bindings/serial/mrvl-serial.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/mrvl.txt b/Documentation/devicetree/bindings/arm/mrvl.txt
> new file mode 100644
> index 0000000..d8de933
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mrvl.txt
> @@ -0,0 +1,6 @@
> +Marvell Platforms Device Tree Bindings
> +----------------------------------------------------
> +
> +PXA168 Aspenite Board
> +Required root node properties:
> +	- compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
> diff --git a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
> new file mode 100644
> index 0000000..1e34cfe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
> @@ -0,0 +1,23 @@
> +* Marvell PXA GPIO controller
> +
> +Required properties:
> +- compatible : Should be "mrvl,pxa-gpio" or "mrvl,mmp-gpio"
> +- reg : Address and length of the register set for the device
> +- interrupts : Should be the port interrupt shared by all gpio pins, if

This sentence is incomplete.  Maybe the "one number." two lines below is 
part of this sentence?  If so, I'm not sure I understand the meaning.  
Did you mean to say "If a single interrupt is shared by all GPIO pins, 
the value is a single integer." ?  What is the meaning if there are 
multiple integers in the value?
>
> +- interrupt-name : Should be the name of irq resource.

"The name" implies that there is a single value, but the example below 
shows multiple values.

>
> +  one number.

See above.

>
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be one.  It is the pin number.
> +
> +Example:
> +
> +	gpio: gpio at d4019000 {
> +		compatible = "mrvl,mmp-gpio", "mrvl,pxa-gpio";

The text above says "mrvl,pxa-gpio" *or* "mrvl,mmp-gpio", but the 
example has both.  If both are necessary, the word "and" would be more 
appropriate.

What is the point of having both names in the device tree?  If the Linux 
driver is known to support the MMP version of the GPIO hardware, there 
is no need for a "fallback" to the PXA version.   Fallback names are 
rarely needed for Linux, because new customized Linux kernels are easy 
to develop for new hardware.  Fallback names were intended for 
commercial operating systems where the hardware vendor must work with an 
OS version that is already in the field.

>
> +		reg =<0xd4019000 0x1000>;
> +		interrupts =<49>,<17>,<18>;
> +		interrupt-name = "gpio_mux", "gpio0", "gpio1";
> +		gpio-controller;
> +		#gpio-cells =<1>;
> +		interrupt-controller;
> +		#interrupt-cells =<1>;

The text above does not mention "interrupt-controller" and 
"#interrupt-cells" properties for this node.

>
> +      };
> diff --git a/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt b/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
> new file mode 100644
> index 0000000..071eb3c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/mrvl-i2c.txt
> @@ -0,0 +1,37 @@
> +* I2C
> +
> +Required properties :
> +
> + - reg : Offset and length of the register set for the device
> + - compatible : should be "mrvl,mmp-twsi" where CHIP is the name of a

"CHIP" makes no sense in this sentence.  I presume that you meant to 
refer to a template name that contains "CHIP", but "mrvl,mmp-twsi" does 
not contain "CHIP".

>
> +   compatible processor, e.g. pxa168, pxa910, mmp2, mmp3.
> +   For the pxa2xx/pxa3xx, an additional node "mrvl,pxa-i2c" is required
> +   as shown in the example below.
> +
> +Recommended properties :
> +
> + - interrupts :<a b>  where a is the interrupt number and b is a
> +   field that represents an encoding of the sense and level
> +   information for the interrupt.  This should be encoded based on
> +   the information in section 2) depending on the type of interrupt
> +   controller you have.

Was this text copied from somewhere else and not edited?  It doesn't 
make sense here, and the example below has only a single number, so <a 
b> cannot be correct.

>
> + - interrupt-parent : the phandle for the interrupt controller that
> +   services interrupts for this device.

There is no interrupt-parent property in the example below.  I presume 
you mean to use the default where the device tree parent node is the 
interrupt parent.  If so, you should say that.
>
> + - mrvl,i2c-polling : Disable interrupt of i2c controller. Polling
> +   status register of i2c controller instead.
> + - mrvl,i2c-fast-mode : Enable fast mode of i2c controller.
> +
> +Examples:
> +	twsi1: i2c at d4011000 {
> +		compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";

No need for two different values, as discussed above.

>
> +		reg =<0xd4011000 0x1000>;
> +		interrupts =<7>;
> +		mrvl,i2c-fast-mode;
> +	};
> +	
> +	twsi2: i2c at d4025000 {
> +		compatible = "mrvl,mmp-twsi", "mrvl,pxa-i2c";

No need for two different values, as discussed above.

>
> +		reg =<0xd4025000 0x1000>;
> +		interrupts =<58>;
> +	};
> +
> diff --git a/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt b/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
> new file mode 100644
> index 0000000..0cda19a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
> @@ -0,0 +1,17 @@
> +* Marvell Real Time Clock controller
> +
> +Required properties:
> +- compatible: should be "mrvl,sa1100-rtc"
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +- interrupts: Should be two. The first interrupt number is the rtc alarm
> +  interrupt and the second interrupt number is the rtc hz interrupt.
> +- interrupt-names: Assign name of irq resource.
> +
> +Example:
> +	rtc: rtc at d4010000 {
> +		compatible = "mrvl,mmp-rtc";
> +		reg =<0xd4010000 0x1000>;
> +		interrupts =<5>,<6>;
> +		interrupt-name = "rtc 1Hz", "rtc alarm";
> +	};
> diff --git a/Documentation/devicetree/bindings/serial/mrvl-serial.txt b/Documentation/devicetree/bindings/serial/mrvl-serial.txt
> new file mode 100644
> index 0000000..d744340
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/mrvl-serial.txt
> @@ -0,0 +1,4 @@
> +PXA UART controller
> +
> +Required properties:
> +- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".