[PATCH 00/13] uio_pruss: add support for devicetree and am33xx

Hi,

this series adds PRUv2 support to uio_pruss through devicetree, makes the
device usable on am33xx and enables it on beaglebone black.
Inspired by old patches from Matt Porter found in a downstream tree.

To archieve that this series:
* adds a flag to omap_hwmod.c to get PRUSS out of hardreset (patch 5 and 6)
* adds devicetree support to uio_pruss (patch 7 and 9)
* adds the device to the am33xx dtsi and boneblack dts (patch 12 and 13)

Bits and pieces:
* some cleanup (patch 1-4)
* take care of a fact that SRAM on am33xx is not exposed through UIO (patch 8)
* add runtime pm support to enable clocks (patch 10)
* allow the driver to be compiled on SOC_AM33XX (patch 11)

This is only tested on beaglebone black (as that's the only hardware of the
PRUSS enabled families I have) with some basic GPIO and IRQ tests.

Notes:
* I just got this hardware and I don't know if this UIO PRUSS business is
  desired. Looking at the userspace driver I'd guess not so much ;), but this
  interface is there for older generations anyway, and this small series lets
  me use the device.
* is the hardreset thing I did there the right thing to do? I think the
  proper way would be a reset controller (which apparently doesn't yet exist
  for this SoC?) and let the driver deassert/assert on probe/remove?
* the platform device path has a clk_enable() / clk_put() calls. Are those
  now redundant with the introduced pm_runtime_enable() pm_runtime_disable()
  calls?

Thanks,
Andre

Andre Heider (13):
  uio: uio_pruss: use struct device
  uio: uio_pruss: use devm_kzalloc()
  uio: uio_pruss: use devm_ioremap_resource()
  uio: uio_pruss: use dmam_alloc_coherent()
  ARM: OMAP2+: hwmod: Introduce a flag to deassert the HW reset line
  ARM: AM33XX: hwmod: deassert PRUSS' hardreset lines
  Documentation: devicetree: add bindings for TI PRUSS
  uio: uio_pruss: make the UIO SRAM memory region optional
  uio: uio_pruss: add devicetree support
  uio: uio_pruss: add runtime pm support
  uio: uio_pruss: enable the driver for am33xx SoCs
  ARM: dts: am33xx: add the PRUSSv2 device
  ARM: dts: am335x-boneblack: enable the PRUSSv2 device

 .../devicetree/bindings/misc/ti,pruss.txt          |  19 +++
 arch/arm/boot/dts/am335x-boneblack.dts             |   4 +
 arch/arm/boot/dts/am33xx.dtsi                      |   9 ++
 arch/arm/mach-omap2/omap_hwmod.c                   |   2 +
 arch/arm/mach-omap2/omap_hwmod.h                   |   2 +
 .../mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c |   1 +
 drivers/uio/Kconfig                                |   4 +-
 drivers/uio/uio_pruss.c                            | 144 ++++++++++++---------
 8 files changed, 123 insertions(+), 62 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/ti,pruss.txt

-- 
2.0.0

Replace kzalloc() by devm_kzalloc() and remove the kfree() calls.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index c28d6e2..f07545b 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -109,9 +109,7 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev)
 		gen_pool_free(gdev->sram_pool,
 			      gdev->sram_vaddr,
 			      sram_pool_sz);
-	kfree(gdev->info);
 	clk_put(gdev->pruss_clk);
-	kfree(gdev);
 }
 
 static int pruss_probe(struct platform_device *pdev)

@@ -123,24 +121,19 @@ static int pruss_probe(struct platform_device *pdev)
 	int ret = -ENODEV, cnt = 0, len;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
 
-	gdev = kzalloc(sizeof(struct uio_pruss_dev), GFP_KERNEL);
+	gdev = devm_kzalloc(dev, sizeof(struct uio_pruss_dev), GFP_KERNEL);
 	if (!gdev)
 		return -ENOMEM;
 
-	gdev->info = kzalloc(sizeof(*p) * MAX_PRUSS_EVT, GFP_KERNEL);
-	if (!gdev->info) {
-		kfree(gdev);
+	gdev->info = devm_kzalloc(dev, sizeof(*p) * MAX_PRUSS_EVT, GFP_KERNEL);
+	if (!gdev->info)
 		return -ENOMEM;
-	}
 
 	/* Power on PRU in case its not done as part of boot-loader */
 	gdev->pruss_clk = clk_get(dev, "pruss");
 	if (IS_ERR(gdev->pruss_clk)) {
 		dev_err(dev, "Failed to get clock\n");
-		ret = PTR_ERR(gdev->pruss_clk);
-		kfree(gdev->info);
-		kfree(gdev);
-		return ret;
+		return PTR_ERR(gdev->pruss_clk);
 	} else {
 		clk_enable(gdev->pruss_clk);
 	}

-- 
2.0.0

Replace resource_size() followed by ioremap() with
devm_ioremap_resource() and remove the iounmap() call.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index f07545b..310598a 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -100,7 +100,6 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev)
 		uio_unregister_device(p);
 		kfree(p->name);
 	}
-	iounmap(gdev->prussio_vaddr);
 	if (gdev->ddr_vaddr) {
 		dma_free_coherent(dev, extram_pool_sz, gdev->ddr_vaddr,
 			gdev->ddr_paddr);

@@ -118,7 +117,7 @@ static int pruss_probe(struct platform_device *pdev)
 	struct uio_pruss_dev *gdev;
 	struct resource *regs_prussio;
 	struct device *dev = &pdev->dev;
-	int ret = -ENODEV, cnt = 0, len;
+	int ret = -ENODEV, cnt = 0;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
 
 	gdev = devm_kzalloc(dev, sizeof(struct uio_pruss_dev), GFP_KERNEL);

@@ -139,13 +138,9 @@ static int pruss_probe(struct platform_device *pdev)
 	}
 
 	regs_prussio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!regs_prussio) {
-		dev_err(dev, "No PRUSS I/O resource specified\n");
-		goto out_free;
-	}
-
-	if (!regs_prussio->start) {
-		dev_err(dev, "Invalid memory resource\n");
+	gdev->prussio_vaddr = devm_ioremap_resource(dev, regs_prussio);
+	if (IS_ERR(gdev->prussio_vaddr)) {
+		ret = PTR_ERR(gdev->prussio_vaddr);
 		goto out_free;
 	}
 

@@ -167,13 +162,6 @@ static int pruss_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	len = resource_size(regs_prussio);
-	gdev->prussio_vaddr = ioremap(regs_prussio->start, len);
-	if (!gdev->prussio_vaddr) {
-		dev_err(dev, "Can't remap PRUSS I/O  address range\n");
-		goto out_free;
-	}
-
 	gdev->pintc_base = pdata->pintc_base;
 	gdev->hostirq_start = platform_get_irq(pdev, 0);
 

-- 
2.0.0

Replace dma_alloc_coherent() with dmam_alloc_coherent() and remove the
dma_free_coherent() call.

This shaves off 2 vars in the driver data struct.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index 310598a..50ff206 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -63,10 +63,8 @@ struct uio_pruss_dev {
 	struct uio_info *info;
 	struct clk *pruss_clk;
 	dma_addr_t sram_paddr;
-	dma_addr_t ddr_paddr;
 	void __iomem *prussio_vaddr;
 	unsigned long sram_vaddr;
-	void *ddr_vaddr;
 	unsigned int hostirq_start;
 	unsigned int pintc_base;
 	struct gen_pool *sram_pool;

@@ -100,10 +98,6 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev)
 		uio_unregister_device(p);
 		kfree(p->name);
 	}
-	if (gdev->ddr_vaddr) {
-		dma_free_coherent(dev, extram_pool_sz, gdev->ddr_vaddr,
-			gdev->ddr_paddr);
-	}
 	if (gdev->sram_vaddr)
 		gen_pool_free(gdev->sram_pool,
 			      gdev->sram_vaddr,

@@ -119,6 +113,7 @@ static int pruss_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int ret = -ENODEV, cnt = 0;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
+	dma_addr_t ddr_paddr;
 
 	gdev = devm_kzalloc(dev, sizeof(struct uio_pruss_dev), GFP_KERNEL);
 	if (!gdev)

@@ -155,9 +150,8 @@ static int pruss_probe(struct platform_device *pdev)
 		}
 	}
 
-	gdev->ddr_vaddr = dma_alloc_coherent(dev, extram_pool_sz,
-				&(gdev->ddr_paddr), GFP_KERNEL | GFP_DMA);
-	if (!gdev->ddr_vaddr) {
+	if (!dmam_alloc_coherent(dev, extram_pool_sz, &ddr_paddr,
+				 GFP_KERNEL | GFP_DMA)) {
 		dev_err(dev, "Could not allocate external memory\n");
 		goto out_free;
 	}

@@ -174,7 +168,7 @@ static int pruss_probe(struct platform_device *pdev)
 		p->mem[1].size = sram_pool_sz;
 		p->mem[1].memtype = UIO_MEM_PHYS;
 
-		p->mem[2].addr = gdev->ddr_paddr;
+		p->mem[2].addr = ddr_paddr;
 		p->mem[2].size = extram_pool_sz;
 		p->mem[2].memtype = UIO_MEM_PHYS;
 

-- 
2.0.0

Signed-off-by: Andre Heider <redacted>
---
 Documentation/devicetree/bindings/misc/ti,pruss.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/ti,pruss.txt

diff --git a/Documentation/devicetree/bindings/misc/ti,pruss.txt b/Documentation/devicetree/bindings/misc/ti,pruss.txt
new file mode 100644
index 0000000..4eacc41
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti,pruss.txt

@@ -0,0 +1,19 @@
+TI Programmable Real-Time Unit Sub System (PRUSS)
+
+Required properties:
+- compatible :
+	- "ti,pruss-v1" - for PRUv1 as found on the OMAPL138/DA850/AM18xx SoC families
+	- "ti,pruss-v2" - for PRUv2 as found on the AM33xx SoC family
+- ti,hwmods: Name of the hwmod associated to the PRUSS
+- reg: Address range of rtc register set
+- interrupts: host event interrupts in order
+- interrupt-parent: phandle for the interrupt controller
+
+Example:
+pruss: pruss at 4a300000 {
+	compatible = "ti,pruss-v2";
+	ti,hwmods = "pruss";
+	reg = <0x4a300000 0x080000>;
+	interrupts = <20 21 22 23 24 25 26 27>;
+	interrupt-parent = <&intc>;
+};

-- 
2.0.0

Skip creating the UIO SRAM memory region if no SRAM genalloc has been
passed along. This will be the case for am33xx SoCs.

The order of the memory regions is not changed for already supported
platforms. That is to keep the current behavior for existing userland
drivers.

For am33x this gives one memory region less. This behavior is in line
with downstream patches and userland driver support for this SoC family.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index 50ff206..afaf726 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -111,7 +111,7 @@ static int pruss_probe(struct platform_device *pdev)
 	struct uio_pruss_dev *gdev;
 	struct resource *regs_prussio;
 	struct device *dev = &pdev->dev;
-	int ret = -ENODEV, cnt = 0;
+	int ret = -ENODEV, cnt = 0, i;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
 	dma_addr_t ddr_paddr;
 

@@ -160,17 +160,24 @@ static int pruss_probe(struct platform_device *pdev)
 	gdev->hostirq_start = platform_get_irq(pdev, 0);
 
 	for (cnt = 0, p = gdev->info; cnt < MAX_PRUSS_EVT; cnt++, p++) {
-		p->mem[0].addr = regs_prussio->start;
-		p->mem[0].size = resource_size(regs_prussio);
-		p->mem[0].memtype = UIO_MEM_PHYS;
-
-		p->mem[1].addr = gdev->sram_paddr;
-		p->mem[1].size = sram_pool_sz;
-		p->mem[1].memtype = UIO_MEM_PHYS;
+		i = 0;
+
+		p->mem[i].addr = regs_prussio->start;
+		p->mem[i].size = resource_size(regs_prussio);
+		p->mem[i].memtype = UIO_MEM_PHYS;
+		i++;
+
+		if (gdev->sram_vaddr) {
+			p->mem[i].addr = gdev->sram_paddr;
+			p->mem[i].size = sram_pool_sz;
+			p->mem[i].memtype = UIO_MEM_PHYS;
+			i++;
+		}
 
-		p->mem[2].addr = ddr_paddr;
-		p->mem[2].size = extram_pool_sz;
-		p->mem[2].memtype = UIO_MEM_PHYS;
+		p->mem[i].addr = ddr_paddr;
+		p->mem[i].size = extram_pool_sz;
+		p->mem[i].memtype = UIO_MEM_PHYS;
+		i++;
 
 		p->name = kasprintf(GFP_KERNEL, "pruss_evt%d", cnt);
 		p->version = DRV_VERSION;

-- 
2.0.0

Add support to probe via devicetree.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index afaf726..2df54ab 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -26,6 +26,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <linux/genalloc.h>
+#include <linux/of_device.h>
 
 #define DRV_NAME "pruss_uio"
 #define DRV_VERSION "1.0"

@@ -70,6 +71,27 @@ struct uio_pruss_dev {
 	struct gen_pool *sram_pool;
 };
 
+#ifdef CONFIG_OF
+struct uio_pruss_params {
+	u32 pintc_offset;
+};
+
+static const struct uio_pruss_params uio_pruss_v1_params = {
+	.pintc_offset = 0x4000,
+};
+
+static const struct uio_pruss_params uio_pruss_v2_params = {
+	.pintc_offset = 0x20000,
+};
+
+static const struct of_device_id pruss_of_match_table[] = {
+	{ .compatible = "ti,pruss-v1", .data = &uio_pruss_v1_params, },
+	{ .compatible = "ti,pruss-v2", .data = &uio_pruss_v2_params, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, pruss_of_match_table);
+#endif
+
 static irqreturn_t pruss_handler(int irq, struct uio_info *info)
 {
 	struct uio_pruss_dev *gdev = info->priv;

@@ -111,6 +133,8 @@ static int pruss_probe(struct platform_device *pdev)
 	struct uio_pruss_dev *gdev;
 	struct resource *regs_prussio;
 	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+	const struct uio_pruss_params *params;
 	int ret = -ENODEV, cnt = 0, i;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
 	dma_addr_t ddr_paddr;

@@ -123,13 +147,21 @@ static int pruss_probe(struct platform_device *pdev)
 	if (!gdev->info)
 		return -ENOMEM;
 
-	/* Power on PRU in case its not done as part of boot-loader */
-	gdev->pruss_clk = clk_get(dev, "pruss");
-	if (IS_ERR(gdev->pruss_clk)) {
-		dev_err(dev, "Failed to get clock\n");
-		return PTR_ERR(gdev->pruss_clk);
+	if (dev->of_node) {
+		match = of_match_device(pruss_of_match_table, dev);
+		params = match->data;
+		gdev->pintc_base = params->pintc_offset;
 	} else {
+		/* Power on PRU in case its not done as part of boot-loader */
+		gdev->pruss_clk = clk_get(dev, "pruss");
+		if (IS_ERR(gdev->pruss_clk)) {
+			dev_err(dev, "Failed to get clock\n");
+			return PTR_ERR(gdev->pruss_clk);
+		}
+
 		clk_enable(gdev->pruss_clk);
+
+		gdev->pintc_base = pdata->pintc_base;
 	}
 
 	regs_prussio = platform_get_resource(pdev, IORESOURCE_MEM, 0);

@@ -139,7 +171,7 @@ static int pruss_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	if (pdata->sram_pool) {
+	if (pdata && pdata->sram_pool) {
 		gdev->sram_pool = pdata->sram_pool;
 		gdev->sram_vaddr =
 			(unsigned long)gen_pool_dma_alloc(gdev->sram_pool,

@@ -156,7 +188,6 @@ static int pruss_probe(struct platform_device *pdev)
 		goto out_free;
 	}
 
-	gdev->pintc_base = pdata->pintc_base;
 	gdev->hostirq_start = platform_get_irq(pdev, 0);
 
 	for (cnt = 0, p = gdev->info; cnt < MAX_PRUSS_EVT; cnt++, p++) {

@@ -214,6 +245,7 @@ static struct platform_driver pruss_driver = {
 	.driver = {
 		   .name = DRV_NAME,
 		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(pruss_of_match_table),
 		   },
 };
 

-- 
2.0.0

This enables the hwmod's associated clocks and gets the device in a
working state.

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/uio_pruss.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
index 2df54ab..28a1c1f 100644
--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/genalloc.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 
 #define DRV_NAME "pruss_uio"
 #define DRV_VERSION "1.0"

@@ -125,6 +126,7 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev)
 			      gdev->sram_vaddr,
 			      sram_pool_sz);
 	clk_put(gdev->pruss_clk);
+	pm_runtime_disable(dev);
 }
 
 static int pruss_probe(struct platform_device *pdev)

@@ -164,6 +166,13 @@ static int pruss_probe(struct platform_device *pdev)
 		gdev->pintc_base = pdata->pintc_base;
 	}
 
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (IS_ERR_VALUE(ret)) {
+		dev_err(dev, "pm_runtime_get_sync() failed\n");
+		goto out_free;
+	}
+
 	regs_prussio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	gdev->prussio_vaddr = devm_ioremap_resource(dev, regs_prussio);
 	if (IS_ERR(gdev->prussio_vaddr)) {

-- 
2.0.0

Signed-off-by: Andre Heider <redacted>
---
 drivers/uio/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index 5a90914..1678387 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig

@@ -106,10 +106,10 @@ config UIO_NETX
 
 config UIO_PRUSS
 	tristate "Texas Instruments PRUSS driver"
-	depends on ARCH_DAVINCI_DA850
+	depends on ARCH_DAVINCI_DA850 || SOC_AM33XX
 	select GENERIC_ALLOCATOR
 	help
-	  PRUSS driver for OMAPL138/DA850/AM18XX devices
+	  PRUSS driver for OMAPL138/DA850/AM18XX/AM33XX devices
 	  PRUSS driver requires user space components, examples and user space
 	  driver is available from below SVN repo - you may use anonymous login
 

-- 
2.0.0

Signed-off-by: Andre Heider <redacted>
---
 arch/arm/boot/dts/am33xx.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 4a4e02d..28a7e5d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi

@@ -409,6 +409,15 @@
 			ti,hwmods = "rtc";
 		};
 
+		pruss: pruss at 4a300000 {
+			compatible = "ti,pruss-v2";
+			ti,hwmods = "pruss";
+			reg = <0x4a300000 0x080000>;
+			interrupts = <20 21 22 23 24 25 26 27>;
+			interrupt-parent = <&intc>;
+			status = "disabled";
+		};
+
 		spi0: spi at 48030000 {
 			compatible = "ti,omap4-mcspi";
 			#address-cells = <1>;

-- 
2.0.0

Signed-off-by: Andre Heider <redacted>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 305975d..eeb5c2e 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts

@@ -75,3 +75,7 @@
 		status = "okay";
 	};
 };
+
+&pruss {
+	status = "okay";
+};

-- 
2.0.0

On Sun, Jun 29, 2014 at 05:21:41PM +0100, Andre Heider wrote:

How many of these do we expect?

+- interrupt-parent: phandle for the interrupt controller
+
+Example:
+pruss: pruss at 4a300000 {
+	compatible = "ti,pruss-v2";
+	ti,hwmods = "pruss";
+	reg = <0x4a300000 0x080000>;
+	interrupts = <20 21 22 23 24 25 26 27>;

Assuming these represent more than one interrupt, could you please
bracket them individually? e.g.

	interrupts = <20 21>, <22 23>, <24 25>, <26 27>;

It makes it far clearer that it's a list of multi-cell elements rather
than a giant binary blob, and usually makes it easier to read a dts.

Thanks,
Mark.

On Sun, Jun 29, 2014 at 05:21:43PM +0100, Andre Heider wrote:

The "pruss" clock was not documented in the binding.

Is the clock really called "pruss", or is it given a specific name in
the manual?

Cheers,
Mark.

On Sun, Jun 29, 2014 at 05:21:46PM +0100, Andre Heider wrote:

Could this please have entries individually bracketed?

Thanks,
Mark.

+			interrupt-parent = <&intc>;
+			status = "disabled";
+		};
+
 		spi0: spi at 48030000 {
 			compatible = "ti,omap4-mcspi";
 			#address-cells = <1>;
-- 
2.0.0

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

On Sun, Jun 29, 2014 at 05:21:36PM +0100, Andre Heider wrote:

If this is changing anyway, how about:

	gdev = devm_kzalloc(dev, sizeof(*gdev), GFP_KERNEL);

Cheers,
Mark.

On Mon, Jun 30, 2014 at 10:33:32AM +0100, Mark Rutland wrote:

On Sun, Jun 29, 2014 at 05:21:41PM +0100, Andre Heider wrote:

quoted

diff --git a/Documentation/devicetree/bindings/misc/ti,pruss.txt b/Documentation/devicetree/bindings/misc/ti,pruss.txt
new file mode 100644
index 0000000..4eacc41
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti,pruss.txt

@@ -0,0 +1,19 @@
+TI Programmable Real-Time Unit Sub System (PRUSS)
+
+Required properties:
+- compatible :
+	- "ti,pruss-v1" - for PRUv1 as found on the OMAPL138/DA850/AM18xx SoC families
+	- "ti,pruss-v2" - for PRUv2 as found on the AM33xx SoC family
+- ti,hwmods: Name of the hwmod associated to the PRUSS
+- reg: Address range of rtc register set

Just noticed the "rtc" c&p error, will fix that.

quoted

+- interrupts: host event interrupts in order

How many of these do we expect?

Exactly 8, which correspond to the /dev/uio* devices the driver creates.
I'll change that to make it clear.

Additionally, the driver currently expects those 8 to be sequential.
In fact, it just gets the first irq and increments from there on, I'll
add a patch to the series to improve that too.

quoted

+- interrupt-parent: phandle for the interrupt controller
+
+Example:
+pruss: pruss at 4a300000 {
+	compatible = "ti,pruss-v2";
+	ti,hwmods = "pruss";
+	reg = <0x4a300000 0x080000>;
+	interrupts = <20 21 22 23 24 25 26 27>;

Assuming these represent more than one interrupt, could you please
bracket them individually? e.g.

	interrupts = <20 21>, <22 23>, <24 25>, <26 27>;

It makes it far clearer that it's a list of multi-cell elements rather
than a giant binary blob, and usually makes it easier to read a dts.

This interrupt controller has one cell, so I assume you meant:

	interrupts = <20>, <21>, <22>, <23>, <24>, <25>, <26>, <27>;

I can do that, but it would be out of line with the rest on the file. The
audio devices are the only ones using that format, but they also got
"interrupt-names".

Thanks,
Andre

On Mon, Jun 30, 2014 at 10:36:06AM +0100, Mark Rutland wrote:

On Sun, Jun 29, 2014 at 05:21:43PM +0100, Andre Heider wrote:

quoted

-	/* Power on PRU in case its not done as part of boot-loader */
-	gdev->pruss_clk = clk_get(dev, "pruss");
-	if (IS_ERR(gdev->pruss_clk)) {
-		dev_err(dev, "Failed to get clock\n");
-		return PTR_ERR(gdev->pruss_clk);
+	if (dev->of_node) {
+		match = of_match_device(pruss_of_match_table, dev);
+		params = match->data;
+		gdev->pintc_base = params->pintc_offset;
 	} else {
+		/* Power on PRU in case its not done as part of boot-loader */
+		gdev->pruss_clk = clk_get(dev, "pruss");
+		if (IS_ERR(gdev->pruss_clk)) {
+			dev_err(dev, "Failed to get clock\n");
+			return PTR_ERR(gdev->pruss_clk);
+		}

The "pruss" clock was not documented in the binding.

Is the clock really called "pruss", or is it given a specific name in
the manual?

That hunk was moved, see above. It's not the devicetree path, it's for
non-DT TI DaVinci:

	arch/arm/mach-davinci/da850.c:static struct clk pruss_clk = {
	arch/arm/mach-davinci/da850.c:  .name       = "pruss",

The DT path already has a clocks associated at the hwmod level.

From arch/arm/mach-omap2/omap_hwmod_33xx_43xx_ipblock_data.c:

		struct omap_hwmod am33xx_pruss_hwmod = {
					.name		= "pruss",
					.class		= &am33xx_pruss_hwmod_class,
					.clkdm_name	= "pruss_ocp_clkdm",
					.flags		= HWMOD_INIT_DEASSERT_HARD_RESET,
					.main_clk	= "pruss_ocp_gclk",

and doesn't require any additional clock entries as far as the binding is
concerned.

Thanks,
Andre

On Mon, Jun 30, 2014 at 10:36:42AM +0100, Mark Rutland wrote:

On Sun, Jun 29, 2014 at 05:21:46PM +0100, Andre Heider wrote:

quoted

--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi

@@ -409,6 +409,15 @@
 			ti,hwmods = "rtc";
 		};
 
+		pruss: pruss at 4a300000 {
+			compatible = "ti,pruss-v2";
+			ti,hwmods = "pruss";
+			reg = <0x4a300000 0x080000>;
+			interrupts = <20 21 22 23 24 25 26 27>;

Could this please have entries individually bracketed?

I'll sync that to with what we end up with on path 7.

Thanks,
Andre

On Mon, Jun 30, 2014 at 10:38:09AM +0100, Mark Rutland wrote:

On Sun, Jun 29, 2014 at 05:21:36PM +0100, Andre Heider wrote:

quoted

--- a/drivers/uio/uio_pruss.c
+++ b/drivers/uio/uio_pruss.c

@@ -109,9 +109,7 @@ static void pruss_cleanup(struct device *dev, struct uio_pruss_dev *gdev)
 		gen_pool_free(gdev->sram_pool,
 			      gdev->sram_vaddr,
 			      sram_pool_sz);
-	kfree(gdev->info);
 	clk_put(gdev->pruss_clk);
-	kfree(gdev);
 }
 
 static int pruss_probe(struct platform_device *pdev)

@@ -123,24 +121,19 @@ static int pruss_probe(struct platform_device *pdev)
 	int ret = -ENODEV, cnt = 0, len;
 	struct uio_pruss_pdata *pdata = dev_get_platdata(dev);
 
-	gdev = kzalloc(sizeof(struct uio_pruss_dev), GFP_KERNEL);
+	gdev = devm_kzalloc(dev, sizeof(struct uio_pruss_dev), GFP_KERNEL);

If this is changing anyway, how about:

	gdev = devm_kzalloc(dev, sizeof(*gdev), GFP_KERNEL);

Sounds good, will do just that.

Thanks for having at look at the series, Mark!

Regards,
Andre

On Sun, Jun 29, 2014 at 06:21:34PM +0200, Andre Heider wrote:

Hi,

this series adds PRUv2 support to uio_pruss through devicetree, makes the
device usable on am33xx and enables it on beaglebone black.
Inspired by old patches from Matt Porter found in a downstream tree.

To archieve that this series:
* adds a flag to omap_hwmod.c to get PRUSS out of hardreset (patch 5 and 6)
* adds devicetree support to uio_pruss (patch 7 and 9)
* adds the device to the am33xx dtsi and boneblack dts (patch 12 and 13)

Bits and pieces:
* some cleanup (patch 1-4)
* take care of a fact that SRAM on am33xx is not exposed through UIO (patch 8)
* add runtime pm support to enable clocks (patch 10)
* allow the driver to be compiled on SOC_AM33XX (patch 11)

This is only tested on beaglebone black (as that's the only hardware of the
PRUSS enabled families I have) with some basic GPIO and IRQ tests.

Notes:
* I just got this hardware and I don't know if this UIO PRUSS business is
  desired. Looking at the userspace driver I'd guess not so much ;), but this
  interface is there for older generations anyway, and this small series lets
  me use the device.
* is the hardreset thing I did there the right thing to do? I think the
  proper way would be a reset controller (which apparently doesn't yet exist
  for this SoC?) and let the driver deassert/assert on probe/remove?
* the platform device path has a clk_enable() / clk_put() calls. Are those
  now redundant with the introduced pm_runtime_enable() pm_runtime_disable()
  calls?

@OMAP guys: any comments? The series depends on patch 5 and 6; both touch
common hwmod code.

I noticed that AM437x now comes with 4 PRUSS cores, maybe you had something
different in mind on how to expose these?

Thanks in advance,
Andre

On Mon, 7 Jul 2014, Andre Heider wrote:

On Sun, Jun 29, 2014 at 06:21:34PM +0200, Andre Heider wrote:

quoted

this series adds PRUv2 support to uio_pruss through devicetree, makes the
device usable on am33xx and enables it on beaglebone black.
Inspired by old patches from Matt Porter found in a downstream tree.

To archieve that this series:
* adds a flag to omap_hwmod.c to get PRUSS out of hardreset (patch 5 and 6)

...

quoted

* is the hardreset thing I did there the right thing to do? I think the
  proper way would be a reset controller (which apparently doesn't yet exist
  for this SoC?) and let the driver deassert/assert on probe/remove?
* the platform device path has a clk_enable() / clk_put() calls. Are those
  now redundant with the introduced pm_runtime_enable() pm_runtime_disable()
  calls?

Probably you only need pm_runtime_{get,put}_*() calls, unless you're 
changing clock parents or rates in your driver code.

@OMAP guys: any comments? The series depends on patch 5 and 6; both touch
common hwmod code.

I'd suggest splitting the series into three independent pieces if 
possible:

1. UIO code, for the UIO maintainer(s)
2. DT pieces for Tony
3. hwmod pieces for me

That way they can be cleanly merged by the respective maintainers.

As far as the hwmod piece goes, I'd be willing to merge your code as a 
temporary workaround for the issue, and marking it as such; but I'd be 
concerned about power management-related interactions (i.e., does the 
PRUSS need to be reset upon return from deep idle states, etc.)


- Paul

On Mon, Jul 07, 2014 at 05:50:09PM +0000, Paul Walmsley wrote:

On Mon, 7 Jul 2014, Andre Heider wrote:

quoted

On Sun, Jun 29, 2014 at 06:21:34PM +0200, Andre Heider wrote:

quoted

this series adds PRUv2 support to uio_pruss through devicetree, makes the
device usable on am33xx and enables it on beaglebone black.
Inspired by old patches from Matt Porter found in a downstream tree.

To archieve that this series:
* adds a flag to omap_hwmod.c to get PRUSS out of hardreset (patch 5 and 6)

...

quoted

quoted

* is the hardreset thing I did there the right thing to do? I think the
  proper way would be a reset controller (which apparently doesn't yet exist
  for this SoC?) and let the driver deassert/assert on probe/remove?
* the platform device path has a clk_enable() / clk_put() calls. Are those
  now redundant with the introduced pm_runtime_enable() pm_runtime_disable()
  calls?

Probably you only need pm_runtime_{get,put}_*() calls, unless you're 
changing clock parents or rates in your driver code.

quoted

@OMAP guys: any comments? The series depends on patch 5 and 6; both touch
common hwmod code.

I'd suggest splitting the series into three independent pieces if 
possible:

1. UIO code, for the UIO maintainer(s)
2. DT pieces for Tony
3. hwmod pieces for me

That way they can be cleanly merged by the respective maintainers.

I second that. At first sight, the UIO parts look OK to me, but please
make it a new patch series.

Thanks,
Hans

On Mon, Jul 07, 2014 at 05:50:09PM +0000, Paul Walmsley wrote:

On Mon, 7 Jul 2014, Andre Heider wrote:

quoted

On Sun, Jun 29, 2014 at 06:21:34PM +0200, Andre Heider wrote:

quoted

this series adds PRUv2 support to uio_pruss through devicetree, makes the
device usable on am33xx and enables it on beaglebone black.
Inspired by old patches from Matt Porter found in a downstream tree.

To archieve that this series:
* adds a flag to omap_hwmod.c to get PRUSS out of hardreset (patch 5 and 6)

...

quoted

quoted

* is the hardreset thing I did there the right thing to do? I think the
  proper way would be a reset controller (which apparently doesn't yet exist
  for this SoC?) and let the driver deassert/assert on probe/remove?
* the platform device path has a clk_enable() / clk_put() calls. Are those
  now redundant with the introduced pm_runtime_enable() pm_runtime_disable()
  calls?

Probably you only need pm_runtime_{get,put}_*() calls, unless you're 
changing clock parents or rates in your driver code.

No, the driver doesn't do that. So I can clean that up, nice.

quoted

@OMAP guys: any comments? The series depends on patch 5 and 6; both touch
common hwmod code.

I'd suggest splitting the series into three independent pieces if 
possible:

1. UIO code, for the UIO maintainer(s)
2. DT pieces for Tony
3. hwmod pieces for me

That way they can be cleanly merged by the respective maintainers.

As far as the hwmod piece goes, I'd be willing to merge your code as a 
temporary workaround for the issue, and marking it as such; but I'd be 
concerned about power management-related interactions (i.e., does the 
PRUSS need to be reset upon return from deep idle states, etc.)

Alright, thanks Paul.

About the deep idle states... I'm not sure, I couldn't find any explicit
wording about it in the am335x technical reference manual nor in in the
boneblack system reference manual.

According to the TRM the PRUSS lies in the PD_PER power domain, which is
powered down for the "Deepsleep0" power mode. So I *guess* the PRUSS also
needs to be taken out of hard reset when waking up from such a state.

But there's no upstream support for these power modes on am33xx anyway,
and I'd assume that HWMOD_INIT_DEASSERT_HARD_RESET gets removed or
replaced once that lands. Which is probably what you meant by "temporary".
FWIW, I'd be willing to look into that when the time comes and PRUSS gets
left behind.

Thanks,
Andre

Hi,

I'm a bit of a rookie with the kernel,  but I'm trying to get the pruss 
working in kernel 3.15

can you help me pull together the various bits and pieces of these patches?

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/270086.html


thanks!!

Bob Bailey