Skip to contentSkip to conversation

ARM: mmp: support OF on pxa168

PATCH Superseded Haojian Zhuang 8 participants 32 messages Mar 5, 2012 – Apr 9, 2012

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

v2 cover ARM: mmp: support OF on pxa168

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.

#1 lore raw

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

Parse uart device id from alias in DTS file.

---
 drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +53 -12
index 5c8e3bb..a71855b 100644
@@ -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,
 	},
 };
 
Signature · 1 line
1.7.0.4
#2 lore raw

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

drivers/tty/serial/pxa.c line 52

Quoted 6 lines · Haojian Zhuang
drivers/tty/serial/pxa.c +2 -0
+#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?

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 792

Quoted 10 lines
drivers/tty/serial/pxa.c +6 -0
@@ -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

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 797

Quoted 4 lines
drivers/tty/serial/pxa.c +4 -0
+#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.

drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 854

Quoted 5 lines
drivers/tty/serial/pxa.c +4 -0
+	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.

drivers/tty/serial/pxa.c static struct platform_driver serial_pxa_driver = { · line 911

Quoted 3 lines
drivers/tty/serial/pxa.c +1 -0
+		.of_match_table = serial_pxa_dt_ids,
 	},
 };

.of_match_table = of_match_ptr(serial_pxa_dt_ids),

Arnd

#3 lore raw

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

Quoted 41 lines · Arnd Bergmann
Quoted 6 lines · Haojian Zhuang
(quoted diff) +2 -0
+#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?

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 792

Quoted 10 lines
(quoted diff) +0 -0
@@ -781,6 +784,39 @@ static const struct dev_pm_ops serial_pxa_pm_ops = {
 ?};

?#endif

drivers/tty/serial/pxa.c +6 -0
+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

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 797

Quoted 4 lines
drivers/tty/serial/pxa.c +4 -0
+#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.

Quoted 5 lines

+ ? ? 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.

Quoted 8 lines
Quoted 3 lines

+ ? ? ? ? ? ? .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

#4 lore raw

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

In reply to Arnd Bergmann

Parse uart device id from alias in DTS file.

---
 drivers/tty/serial/pxa.c |   52 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 40 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +40 -12
index 5c8e3bb..23d32f6 100644
@@ -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,
 	},
 };
 
Signature · 1 line
1.7.0.4
#5 lore raw

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

drivers/tty/serial/pxa.c struct uart_pxa_port { · line 58

Quoted 7 lines · Haojian Zhuang
drivers/tty/serial/pxa.c +1 -1
@@ -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

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 794

Quoted 13 lines
drivers/tty/serial/pxa.c +8 -0
 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.

drivers/tty/serial/pxa.c static int serial_pxa_remove(struct platform_device *dev) · line 882

Quoted 8 lines
drivers/tty/serial/pxa.c +1 -0
@@ -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

#6 lore raw

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

Quoted 41 lines · Arnd Bergmann

Review struct uart_pxa_port { · line 58

Quoted 7 lines · Haojian Zhuang
(quoted diff) +1 -1
@@ -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

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 794

Quoted 13 lines
(quoted diff) +0 -0
 ?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

drivers/tty/serial/pxa.c +8 -0
+#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.

Review static int serial_pxa_remove(struct platform_device *dev) · line 882

Quoted 8 lines
(quoted diff) +1 -0
@@ -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.

Quoted 2 lines

? ? ? ?Arnd

Ah. It's a bug. I'll fix it tomorrow.

Thanks
Haojian

#7 lore raw

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

In reply to Arnd Bergmann

From: Haojian Zhuang <[email protected]>

Parse uart device id from alias in DTS file.

---
 drivers/tty/serial/pxa.c |   49 ++++++++++++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +37 -12
index 5c8e3bb..cdf4b2b 100644
@@ -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,
 	},
 };
 
Signature · 1 line
1.7.5.4
#9 lore raw

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

In reply to Haojian Zhuang
Quoted 54 lines · Haojian Zhuang

Parse uart device id from alias in DTS file.

---
 drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 12 deletions(-)
drivers/tty/serial/pxa.c +25 -0
index 5c8e3bb..a71855b 100644
@@ -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;
+	}

drivers/tty/serial/pxa.c static const struct dev_pm_ops serial_pxa_pm_ops = { · line 818

Quoted 10 lines
drivers/tty/serial/pxa.c +10 -0
+	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.

Quoted 12 lines
drivers/tty/serial/pxa.c +1 -1
+
 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);

drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 850

Quoted 10 lines
drivers/tty/serial/pxa.c +1 -9
-
-	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.

drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 861

Quoted 11 lines
drivers/tty/serial/pxa.c +9 -0
+	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.

Quoted 30 lines
drivers/tty/serial/pxa.c +6 -2
+
 	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);
 

drivers/tty/serial/pxa.c static struct platform_driver serial_pxa_driver = { · line 909

Quoted 4 lines
drivers/tty/serial/pxa.c +1 -0
@@ -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.

#10 lore raw

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

In reply to Haojian Zhuang
Quoted 6 lines · Haojian Zhuang

Parse uart device id from alias in DTS file.

---
 drivers/tty/serial/pxa.c |   65 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 12 deletions(-)

Binding documentation!

Quoted 70 lines
drivers/tty/serial/pxa.c +36 -1
index 5c8e3bb..a71855b 100644
@@ -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);

drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 850

Quoted 10 lines
drivers/tty/serial/pxa.c +1 -9
-
-	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()

drivers/tty/serial/pxa.c static int serial_pxa_probe(struct platform_device *dev) · line 861

Quoted 11 lines
drivers/tty/serial/pxa.c +9 -0
+	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?

Quoted 40 lines
drivers/tty/serial/pxa.c +7 -2
+
 	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,
 	},
 };
 
Signature · 1 line
1.7.0.4
Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies,Ltd.
#11 lore raw

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

In reply to Haojian Zhuang

Add OF support on rtc-sa1100.

---
 drivers/rtc/rtc-sa1100.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
drivers/rtc/rtc-sa1100.c +9 -0
index 962510c..e443b78 100644
@@ -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,
 	},
 };
 
Signature · 1 line
1.7.0.4
#13 lore raw

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

In reply to Haojian Zhuang

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

Still keep slave, slave_addr and class in platform data.

---
 drivers/i2c/busses/i2c-pxa.c |   95 ++++++++++++++++++++++++++++++++---------
 1 files changed, 74 insertions(+), 21 deletions(-)
drivers/i2c/busses/i2c-pxa.c +74 -21
index d603646..f673326 100644
@@ -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,
 };
Signature · 1 line
1.7.0.4
#14 lore raw

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

Quoted 7 lines · Haojian Zhuang

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

Still keep slave, slave_addr and class in platform data.

Just one tiny detail:

Quoted 1 line

+ *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

#15 lore raw

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

In reply to Haojian Zhuang

Enable clk of rtc-sa1100 device in pxa168.

---
 arch/arm/mach-mmp/pxa168.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mach-mmp/pxa168.c +2 -0
index 9f61256..aa62d0c 100644
@@ -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)
Signature · 1 line
1.7.0.4
#17 lore raw

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

In reply to Haojian Zhuang

Enable PXA168 and aspenite support.

---
 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
arch/arm/mach-mmp/Kconfig +10 -0
index 323d4c9..5a90b9a 100644
@@ -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
arch/arm/mach-mmp/Makefile +1 -0
index ba254a7..4fc0ff5 100644
@@ -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
arch/arm/mach-mmp/mmp-dt.c new file +75 -0

75 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#19 lore raw

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

In reply to Haojian Zhuang

Hi,

Quoted 18 lines · Haojian Zhuang

Enable PXA168 and aspenite support.

---
 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
arch/arm/mach-mmp/Kconfig +0 -0
index 323d4c9..5a90b9a 100644
@@ -2,6 +2,16 @@ if ARCH_MMP
 
 menu "Marvell PXA168/910/MMP2 Implmentations"
 

arch/arm/mach-mmp/Kconfig if ARCH_MMP · line 14

Quoted 10 lines
arch/arm/mach-mmp/Kconfig +10 -0
+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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#20 lore raw

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

Quoted 36 lines · Chris Ball

Hi,

Quoted 18 lines · Haojian Zhuang

Enable PXA168 and aspenite support.

---

?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

arch/arm/mach-mmp/Kconfig +0 -0
index 323d4c9..5a90b9a 100644
@@ -2,6 +2,16 @@ if ARCH_MMP
 

?menu "Marvell PXA168/910/MMP2 Implmentations"

arch/arm/mach-mmp/Kconfig

Quoted 10 lines
arch/arm/mach-mmp/Kconfig +10 -0
+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:

Quoted 9 lines

?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

#21 lore raw

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

Hi,

Quoted 3 lines · Haojian Zhuang

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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#22 lore raw

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

Quoted 10 lines · Chris Ball

Hi,

Quoted 3 lines · Haojian Zhuang

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

#23 lore raw

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

In reply to Haojian Zhuang

Hi,

arch/arm/mach-mmp/mmp-dt.c line 47

Quoted 13 lines · Haojian Zhuang
arch/arm/mach-mmp/mmp-dt.c new file +13 -0
+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.

Signature · 2 lines
Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
#24 lore raw

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

Quoted 20 lines · Chris Ball

Hi,

arch/arm/mach-mmp/mmp-dt.c line 47

Quoted 13 lines · Haojian Zhuang
arch/arm/mach-mmp/mmp-dt.c new file +13 -0
+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

#25 lore raw

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

In reply to Haojian Zhuang

DTS files of both PXA168 and aspenite are appended.

---
 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
arch/arm/boot/dts/pxa168-aspenite.dts new file +38 -0
new file mode 100644
index 0000000..e762fac
@@ -0,0 +1,38 @@
+/*
+ *  Copyright (C) 2012 Marvell Technology Group Ltd.
+ *  Author: Haojian Zhuang <[email protected]>
+ *
+ *  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";
+			};
+		};
+	};
+};
arch/arm/boot/dts/pxa168.dtsi new file +98 -0

98 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#27 lore raw

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

In reply to Haojian Zhuang

Add OF support in Document/devicetree directory.

---
 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
Documentation/devicetree/bindings/arm/mrvl.txt new file +6 -0
new file mode 100644
index 0000000..d8de933
@@ -0,0 +1,6 @@
+Marvell Platforms Device Tree Bindings
+----------------------------------------------------
+
+PXA168 Aspenite Board
+Required root node properties:
+	- compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +23 -0
new file mode 100644
index 0000000..1e34cfe
@@ -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>;
+      };
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +37 -0
new file mode 100644
index 0000000..071eb3c
@@ -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>;
+	};
+
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +17 -0
new file mode 100644
index 0000000..0cda19a
@@ -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";
+	};
Documentation/devicetree/bindings/serial/mrvl-serial.txt new file +4 -0
new file mode 100644
index 0000000..d744340
@@ -0,0 +1,4 @@
+PXA UART controller
+
+Required properties:
+- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".
Signature · 1 line
1.7.0.4
#28 lore raw

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

Quoted 3 lines · Haojian Zhuang

Add OF support in Document/devicetree directory.

One question for Benoit, Grant or Rob:

Quoted 5 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +5 -0
+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

Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17

Quoted 10 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +10 -0
+  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

#29 lore raw

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

Quoted 21 lines · Arnd Bergmann
Quoted 3 lines · Haojian Zhuang

Add OF support in Document/devicetree directory.

One question for Benoit, Grant or Rob:

Quoted 12 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +12 -0
+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>;
Quoted 8 lines

Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17

Quoted 3 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +3 -0
+		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

#30 lore raw

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

In reply to Arnd Bergmann
Quoted 21 lines · Arnd Bergmann
Quoted 3 lines · Haojian Zhuang

Add OF support in Document/devicetree directory.

One question for Benoit, Grant or Rob:

Quoted 12 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +12 -0
+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>;
Quoted 8 lines

Documentation/devicetree/bindings/rtc/sa1100-rtc.txt line 17

Quoted 3 lines
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +3 -0
+		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

#31 lore raw

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

In reply to Haojian Zhuang

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

Quoted 34 lines · Haojian Zhuang

Add OF support in Document/devicetree directory.

---
  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

Documentation/devicetree/bindings/arm/mrvl.txt new file +6 -0
new file mode 100644
index 0000000..d8de933
@@ -0,0 +1,6 @@
+Marvell Platforms Device Tree Bindings
+----------------------------------------------------
+
+PXA168 Aspenite Board
+Required root node properties:
+	- compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +0 -0
new file mode 100644
index 0000000..1e34cfe

Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +6 -0
@@ -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?

Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 7

Quoted 2 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1 -0
+- 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.

Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 8

Quoted 2 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1 -0
+  one number.

See above.

Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 15

Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +7 -0
+- 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.

Documentation/devicetree/bindings/gpio/mrvl-gpio.txt line 22

Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +7 -0
+		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.

Quoted 8 lines
Documentation/devicetree/bindings/gpio/mrvl-gpio.txt new file +1 -0
+      };
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +0 -0
new file mode 100644
index 0000000..071eb3c

Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 6

Quoted 6 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +6 -0
@@ -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".

Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 17

Quoted 12 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +11 -0
+   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.

Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 19

Quoted 3 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +2 -0
+ - 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.

Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 26

Quoted 8 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +7 -0
+ - 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.

Documentation/devicetree/bindings/i2c/mrvl-i2c.txt line 33

Quoted 8 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +7 -0
+		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.

Quoted 38 lines
Documentation/devicetree/bindings/i2c/mrvl-i2c.txt new file +4 -0
+		reg =<0xd4025000 0x1000>;
+		interrupts =<58>;
+	};
+
Documentation/devicetree/bindings/rtc/sa1100-rtc.txt new file +17 -0
new file mode 100644
index 0000000..0cda19a
@@ -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";
+	};
Documentation/devicetree/bindings/serial/mrvl-serial.txt new file +4 -0
new file mode 100644
index 0000000..d744340
@@ -0,0 +1,4 @@
+PXA UART controller
+
+Required properties:
+- compatible : should be "mrvl,mmp-uart" or "mrvl,pxa-uart".