Assorted PXA3xx DT patches

This patch adds generic device tree bindings to the PXA RTC driver. Documentation for bindings were also added.

Signed-off-by Daniel Mack <[email protected]>

Cc Robert Jarzmik <[email protected]>

Cc Alessandro Zummo <[email protected]>

---
  Documentation/devicetree/bindings/rtc/pxa-rtc.txt |   12 ++++++++++++
  drivers/rtc/rtc-pxa.c                             |   11 +++++++++++
  2 files changed, 23 insertions(+)

create mode 100644 Documentation/devicetree/bindings/rtc/pxa-rtc.txt

Documentation/devicetree/bindings/rtc/pxa-rtc.txt new file +0 -0

new file mode 100644
index 0000000..acba256

Signed-off-by Daniel Mack <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Chris Ball <[email protected]>

---
 Documentation/devicetree/bindings/mmc/pxa-mmc.txt |   24 ++++++++++
 drivers/mmc/host/pxamci.c                         |   50 +++++++++++++++++++++
 2 files changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/pxa-mmc.txt

Documentation/devicetree/bindings/mmc/pxa-mmc.txt new file +5 -0

new file mode 100644
index 0000000..8f0ea58
@@ -0,0 +1,24 @@
+* PXA MMC drivers
+
+Driver bindings for the PXA MCI (MMC/SDIO) interfaces
+
+Required properties:

Documentation/devicetree/bindings/mmc/pxa-mmc.txt new file +8 -0

+Examples:
+
+mmc0: mmc at 41100000 {
+	compatible = "mrvl,pxa-mmc";
+	reg = <0x41100000 0x1000>;
+	interrupts = <23>;
+};
+

drivers/mmc/host/pxamci.c +49 -0

index cb2dc0e..39ddc00 100644
@@ -30,6 +30,9 @@
 #include <linux/regulator/consumer.h>
 #include <linux/gpio.h>
 #include <linux/gfp.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/of_device.h>
 
 #include <asm/sizes.h>
 

@@ -573,6 +576,48 @@ static irqreturn_t pxamci_detect_irq(int irq, void *devid)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id pxa_mmc_dt_ids[] = {
+        { .compatible = "mrvl,pxa-mmc" },
+        { }
+};
+
+MODULE_DEVICE_TABLE(of, pxa_mmc_dt_ids);
+
+static int __devinit pxamci_of_init(struct platform_device *pdev)
+{
+        struct device_node *np = pdev->dev.of_node;
+        struct pxamci_platform_data *pdata;
+        u32 tmp;
+
+        if (!np)
+                return 0;
+
+        pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+        if (!pdata)
+                return -ENOMEM;
+
+	pdata->gpio_card_detect =
+		of_get_named_gpio(np, "pxa-mmc,gpio-card-detect", 0);
+	pdata->gpio_card_ro =
+		of_get_named_gpio(np, "pxa-mmc,gpio-card-readonly", 0);
+	pdata->gpio_power =
+		of_get_named_gpio(np, "pxa-mmc,gpio-power", 0);
+
+	if (of_property_read_u32(np, "pxa-mmc,detect-delay-ms", &tmp) == 0)
+		pdata->detect_delay_ms = tmp;
+
+        pdev->dev.platform_data = pdata;
+
+        return 0;
+}
+#else
+static int __devinit pxamci_of_init(struct platform_device *pdev)
+{
+        return 0;
+}
+#endif
+
 static int pxamci_probe(struct platform_device *pdev)
 {
 	struct mmc_host *mmc;

@@ -580,6 +625,10 @@ static int pxamci_probe(struct platform_device *pdev)
 	struct resource *r, *dmarx, *dmatx;
 	int ret, irq, gpio_cd = -1, gpio_ro = -1, gpio_power = -1;
 
+	ret = pxamci_of_init(pdev);
+	if (ret)
+		return ret;
+
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
 	if (!r || irq < 0)

Hi Daniel,

Quoted 20 lines · Daniel Mack

Signed-off-by Daniel Mack <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Chris Ball <[email protected]>

---
 Documentation/devicetree/bindings/mmc/pxa-mmc.txt |   24 ++++++++++
 drivers/mmc/host/pxamci.c                         |   50 +++++++++++++++++++++
 2 files changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/pxa-mmc.txt

Documentation/devicetree/bindings/mmc/pxa-mmc.txt new file +5 -0

new file mode 100644
index 0000000..8f0ea58
@@ -0,0 +1,24 @@
+* PXA MMC drivers
+
+Driver bindings for the PXA MCI (MMC/SDIO) interfaces
+
+Required properties:

Documentation/devicetree/bindings/mmc/pxa-mmc.txt line 15

Quoted 10 lines

Documentation/devicetree/bindings/mmc/pxa-mmc.txt new file +10 -0

+- compatible: Should be "mrvl,pxa-mmc".
+- reg: Should contain registers location and length
+- interrupts: Should contain the interrupt
+- vmmc-supply: A regulator for VMMC
+
+Optional properties:
+- mrvl,detect-delay-ms: sets the detection delay timeout in ms.
+- mrvl,gpio-card-detect: GPIO spec for the card detect pin
+- mrvl,gpio-card-readonly: GPIO spec for the card write protection pin
+- mrvl,gpio-power: GPIO spec for the card power enable pin

Please see Documentation/devicetree/bindings/mmc/mmc.txt. It looks like you should be using cd-gpios and wp-gpios, and you should refer to mmc.txt instead of specifying reg/interrupts, with:

This file documents differences between the core properties in mmc.txt and the properties used by the pxa-mmc driver.

I saw Arnd mention that we're moving from "mrvl," to "marvell,", but maybe that's not finalized yet. (And there are other drivers using mrvl, so it'll presumably involve a large renaming patch.)

Documentation/devicetree/bindings/mmc/pxa-mmc.txt static struct platform_driver pxamci_driver = { · line 918

Quoted 5 lines

Documentation/devicetree/bindings/mmc/pxa-mmc.txt new file +1 -0

@@ -866,6 +915,7 @@ static struct platform_driver pxamci_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(pxa_mmc_dt_ids),

Have you tried compiling without CONFIG_OF? This doesn't look to be inside #ifdef CONFIG_OF, which I think would cause a compile error.

This patch contains a hack to get the DMA resources of the device when probed from a devicetree node. This can be removed once a generic DMA controller framework lands.

A mtd_part_parser_data is passed mtd_device_parse_register which contains a reference to the device node, so MTD partitions can be added as children.

Signed-off-by Daniel Mack <[email protected]>

Cc David Woodhouse <[email protected]>

---
 .../devicetree/bindings/mtd/pxa3xx-nand.txt        |   31 ++++++++
 drivers/mtd/nand/pxa3xx_nand.c                     |   83
++++++++++++++++---- 2 files changed, 100 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt new file mode 100644
index 0000000..6e28cef

Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt new file +11 -0

@@ -0,0 +1,31 @@
+PXA3xx NAND DT bindings
+
+Required properties:
+
+ - compatible:		Should be "mrvl,pxa3xx-nand"
+ - reg: 		The register base for the controller
+ - interrupts:		The interrupt to map
+ - #address-cells:	Set to <1> if the node includes partitions
+
+Optional properties:
+

Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt new file +10 -0

+		interrupts = <45>;
+		#address-cells = <1>;
+
+		mrvl,nand-enable-arbiter;
+		mrvl,nand-keep-config;
+		num-cs = <1>;
+
+		/* partitions (optional) */
+	};
+

(quoted diff) +0 -0

b/drivers/mtd/nand/pxa3xx_nand.c index 252aaef..fb592a9 100644

drivers/mtd/nand/pxa3xx_nand.c +2 -5

@@ -22,6 +22,8 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <mach/dma.h>
 #include <plat/pxa3xx_nand.h>

@@ -1081,21 +1083,29 @@ static int alloc_nand_resource(struct
 platform_device *pdev) }
 	clk_enable(info->clk);
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (r == NULL) {
-		dev_err(&pdev->dev, "no resource defined for data DMA\n");
-		ret = -ENXIO;
-		goto fail_put_clk;

drivers/mtd/nand/pxa3xx_nand.c +17 -1

+			ret = -ENXIO;
+			goto fail_put_clk;
+		}
 	}
-	info->drcmr_cmd = r->start;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {

@@ -1200,12 +1210,55 @@ static int pxa3xx_nand_remove(struct
 platform_device *pdev) return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id pxa3xx_nand_dt_ids[] = {
+	{ .compatible = "mrvl,pxa3xx-nand" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
+
+static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
+{
+	struct pxa3xx_nand_platform_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *of_id =
+			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
+

drivers/mtd/nand/pxa3xx_nand.c +22 -1

+		pdata->keep_config = 1;
+	of_property_read_u32(np, "num-cs", &pdata->num_cs);
+
+	pdev->dev.platform_data = pdata;
+
+	return 0;
+}
+#else
+static inline int pxa3xx_nand_probe_dt(struct platform_device *)
+{
+	return 0;
+}
+#endif
+
 static int pxa3xx_nand_probe(struct platform_device *pdev)
 {
 	struct pxa3xx_nand_platform_data *pdata;
+	struct mtd_part_parser_data ppdata = {};
 	struct pxa3xx_nand_info *info;
 	int ret, cs, probe_success;
 
+	ret = pxa3xx_nand_probe_dt(pdev);
+	if (ret)
+		return ret;
+
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
 		dev_err(&pdev->dev, "no platform data defined\n");

@@ -1229,8 +1282,9 @@ static int pxa3xx_nand_probe(struct platform_device
 *pdev) continue;
 		}
 
+		ppdata.of_node = pdev->dev.of_node;
 		ret = mtd_device_parse_register(info->host[cs]->mtd, NULL,
-						NULL, pdata->parts[cs],
+						&ppdata, pdata->parts[cs],
 						pdata->nr_parts[cs]);
 		if (!ret)
 			probe_success = 1;

@@ -1306,6 +1360,7 @@ static int pxa3xx_nand_resume(struct platform_device
 *pdev) static struct platform_driver pxa3xx_nand_driver = {
 	.driver = {
 		.name	= "pxa3xx-nand",
+		.of_match_table = of_match_ptr(pxa3xx_nand_dt_ids),
 	},
 	.probe		= pxa3xx_nand_probe,
 	.remove		= pxa3xx_nand_remove,

Quoted 5 lines

drivers/mtd/nand/pxa3xx_nand.c +0 -5

-     r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-     if (r == NULL) {
-             dev_err(&pdev->dev, "no resource defined for data DMA\n");
-             ret = -ENXIO;
-             goto fail_put_clk;

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1093

Quoted 10 lines

drivers/mtd/nand/pxa3xx_nand.c +8 -2

-     }
-     info->drcmr_dat = r->start;
+     /*
+      * This is a dirty hack to make this driver work from devicetree
+      * bindings. It can be removed once we have a prober DMA controller
+      * framework for DT.
+      */
+     if (pdev->dev.of_node && cpu_is_pxa3xx()) {
+             info->drcmr_dat = 97;
+             info->drcmr_cmd = 99;

cpu_is_() stuff should begone ... besides, what are these numbers here?

Quoted 16 lines · Marek Vasut

Quoted 15 lines

drivers/mtd/nand/pxa3xx_nand.c +8 -7

-     r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-     if (r == NULL) {
-             dev_err(&pdev->dev, "no resource defined for data DMA\n");
-             ret = -ENXIO;
-             goto fail_put_clk;
-     }
-     info->drcmr_dat = r->start;
+     /*
+      * This is a dirty hack to make this driver work from devicetree
+      * bindings. It can be removed once we have a prober DMA controller
+      * framework for DT.
+      */
+     if (pdev->dev.of_node && cpu_is_pxa3xx()) {
+             info->drcmr_dat = 97;
+             info->drcmr_cmd = 99;

Quoted 1 line

cpu_is_() stuff should begone ... besides, what are these numbers here?

They are the numbers from the DMA resource. It's ugly but I think reasonable to do this here. We will clean it up soon, once the bindings are in place.

I did notice though the the two "info->drcmr_dat = r->start" assignments in the non-DT case are removed here. I think that's a bug.

Quoted 8 lines

This patch contains a hack to get the DMA resources of the device when probed from a devicetree node. This can be removed once a generic DMA controller framework lands.

A mtd_part_parser_data is passed mtd_device_parse_register which contains a reference to the device node, so MTD partitions can be added as children.

Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt

Quoted 4 lines

Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt new file +4 -0

+
+	nand0: nand at 43100000 {
+		compatible = "mrvl,pxa3xx-nand";
+		reg = <0x43100000 90> ;

0x90 maybe?

Quoted 37 lines

Documentation/devicetree/bindings/mtd/pxa3xx-nand.txt new file +10 -0

+		interrupts = <45>;
+		#address-cells = <1>;
+
+		mrvl,nand-enable-arbiter;
+		mrvl,nand-keep-config;
+		num-cs = <1>;
+
+		/* partitions (optional) */
+	};
+

(quoted diff) +0 -0

b/drivers/mtd/nand/pxa3xx_nand.c index 252aaef..fb592a9 100644

drivers/mtd/nand/pxa3xx_nand.c +4 -7

@@ -22,6 +22,8 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <mach/dma.h>
 #include <plat/pxa3xx_nand.h>

@@ -1081,21 +1083,29 @@ static int alloc_nand_resource(struct
 platform_device *pdev) }
 	clk_enable(info->clk);
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (r == NULL) {
-		dev_err(&pdev->dev, "no resource defined for data DMA\n");
-		ret = -ENXIO;
-		goto fail_put_clk;
-	}
-	info->drcmr_dat = r->start;
+	/*
+	 * This is a dirty hack to make this driver work from devicetree

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1093

Quoted 6 lines

drivers/mtd/nand/pxa3xx_nand.c +6 -0

+	 * bindings. It can be removed once we have a prober DMA controller
+	 * framework for DT.
+	 */
+	if (pdev->dev.of_node && cpu_is_pxa3xx()) {
+		info->drcmr_dat = 97;
+		info->drcmr_cmd = 99;

cpu_is_() stuff should begone ... besides, what are these numbers here?

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev)

Quoted 5 lines

drivers/mtd/nand/pxa3xx_nand.c +4 -0

+	} else {
+		r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+		if (r == NULL) {
+			dev_err(&pdev->dev, "no resource defined for data 

DMA\n");

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev)

Quoted 12 lines

drivers/mtd/nand/pxa3xx_nand.c +6 -5

+			ret = -ENXIO;
+			goto fail_put_clk;
+		}
 
-	r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-	if (r == NULL) {
-		dev_err(&pdev->dev, "no resource defined for command DMA\n");
-		ret = -ENXIO;
-		goto fail_put_clk;
+		r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
+		if (r == NULL) {
+			dev_err(&pdev->dev, "no resource defined for command 

DMA\n");

Quoted 26 lines

drivers/mtd/nand/pxa3xx_nand.c +17 -1

+			ret = -ENXIO;
+			goto fail_put_clk;
+		}
 	}
-	info->drcmr_cmd = r->start;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {

@@ -1200,12 +1210,55 @@ static int pxa3xx_nand_remove(struct
 platform_device *pdev) return 0;
 }
 
+#ifdef CONFIG_OF
+static struct of_device_id pxa3xx_nand_dt_ids[] = {
+	{ .compatible = "mrvl,pxa3xx-nand" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, i2c_pxa_dt_ids);
+
+static int pxa3xx_nand_probe_dt(struct platform_device *pdev)
+{
+	struct pxa3xx_nand_platform_data *pdata;
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *of_id =
+			of_match_device(pxa3xx_nand_dt_ids, &pdev->dev);
+

drivers/mtd/nand/pxa3xx_nand.c static int pxa3xx_nand_remove(struct · line 1236

Quoted 10 lines

drivers/mtd/nand/pxa3xx_nand.c +10 -0

+	if (!of_id)
+		return 0;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	if (of_get_property(np, "mrvl,nand-enable-arbiter", NULL))
+		pdata->enable_arbiter = 1;
+	if (of_get_property(np, "mrvl,nand-keep-config", NULL))

of_property_read_bool() please.

Quoted 47 lines

drivers/mtd/nand/pxa3xx_nand.c +22 -1

+		pdata->keep_config = 1;
+	of_property_read_u32(np, "num-cs", &pdata->num_cs);
+
+	pdev->dev.platform_data = pdata;
+
+	return 0;
+}
+#else
+static inline int pxa3xx_nand_probe_dt(struct platform_device *)
+{
+	return 0;
+}
+#endif
+
 static int pxa3xx_nand_probe(struct platform_device *pdev)
 {
 	struct pxa3xx_nand_platform_data *pdata;
+	struct mtd_part_parser_data ppdata = {};
 	struct pxa3xx_nand_info *info;
 	int ret, cs, probe_success;
 
+	ret = pxa3xx_nand_probe_dt(pdev);
+	if (ret)
+		return ret;
+
 	pdata = pdev->dev.platform_data;
 	if (!pdata) {
 		dev_err(&pdev->dev, "no platform data defined\n");

@@ -1229,8 +1282,9 @@ static int pxa3xx_nand_probe(struct platform_device
 *pdev) continue;
 		}
 
+		ppdata.of_node = pdev->dev.of_node;
 		ret = mtd_device_parse_register(info->host[cs]->mtd, NULL,
-						NULL, pdata->parts[cs],
+						&ppdata, pdata->parts[cs],
 						pdata->nr_parts[cs]);
 		if (!ret)
 			probe_success = 1;

@@ -1306,6 +1360,7 @@ static int pxa3xx_nand_resume(struct platform_device
 *pdev) static struct platform_driver pxa3xx_nand_driver = {
 	.driver = {
 		.name	= "pxa3xx-nand",
+		.of_match_table = of_match_ptr(pxa3xx_nand_dt_ids),
 	},
 	.probe		= pxa3xx_nand_probe,
 	.remove		= pxa3xx_nand_remove,

Add a DT_MACHINE_START entry for PXA3xx machines and a auxdata table for some of the devices. This file can be extended to also support pxa2xx boards.

Signed-off-by Daniel Mack <[email protected]>

---
 arch/arm/mach-pxa/Kconfig  |   12 +++++++++
 arch/arm/mach-pxa/Makefile |    3 +++
 arch/arm/mach-pxa/pxa-dt.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

Quoted 10 lines · Daniel Mack

Add a DT_MACHINE_START entry for PXA3xx machines and a auxdata table for some of the devices. This file can be extended to also support pxa2xx boards.

Signed-off-by Daniel Mack <[email protected]>

---
 arch/arm/mach-pxa/Kconfig  |   12 +++++++++
 arch/arm/mach-pxa/Makefile |    3 +++
 arch/arm/mach-pxa/pxa-dt.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

The code looks good, but I wonder if it would be better to add it to the pxa3xx.c file instead. One more comment:

arch/arm/mach-pxa/pxa-dt.c line 48

Quoted 3 lines

arch/arm/mach-pxa/pxa-dt.c new file +3 -0

+static const char *pxa3xx_dt_board_compat[] __initdata = {
+       "mrvl,pxa3xx",
+};

We should try to avoid wildcards in compatible properties but rather be more specific. I would use separate values for pxa300/310/320/920/930/935 that you could all list here if they have the same auxdata lookup table.

Quoted 12 lines · Arnd Bergmann

Quoted 10 lines · Daniel Mack

Add a DT_MACHINE_START entry for PXA3xx machines and a auxdata table for some of the devices. This file can be extended to also support pxa2xx boards.

Signed-off-by Daniel Mack <[email protected]>

---
 arch/arm/mach-pxa/Kconfig  |   12 +++++++++
 arch/arm/mach-pxa/Makefile |    3 +++
 arch/arm/mach-pxa/pxa-dt.c |   61 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

Quoted 2 lines

The code looks good, but I wonder if it would be better to add it to the pxa3xx.c file instead. One more comment:

Well, I though having tham separated from the generic code will make things cleaner and also save us one #ifdef.

Quoted 7 lines

arch/arm/mach-pxa/pxa-dt.c line 48

Quoted 3 lines

arch/arm/mach-pxa/pxa-dt.c new file +3 -0

+static const char *pxa3xx_dt_board_compat[] __initdata = {
+       "mrvl,pxa3xx",
+};

We should try to avoid wildcards in compatible properties but rather be more specific. I would use separate values for pxa300/310/320/920/930/935 that you could all list here if they have the same auxdata lookup table.

Ok, makes sense. I'm not sure about 920/930/935, so I'll leave them out for now. Whoever successfully tests these can easly add that lines.