This patch series add support for PRUSS (Programmable Real-time Unit Sub
System) UIO driver in Texas Instruments DA850, AM18xx and OMAPL1-38 processors.
PRUSS is programmable RISC core which can be used to implement Soft IPs
(eg:- DMA, CAN, UART,SmartCard) and Industrial communications data link layers
(eg:- PROFIBUS). UIO driver exposes PRUSS resources like memory and interrupts
to user space application.PRUSS UIO application API can be used to control PRUs
in PRUSS, setup PRU INTC, load firmware to PRUs and implement IPC between Host
processor and PRUs. More information on PRUSS and UIO linux user space API
available in the links below
http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem
http://processors.wiki.ti.com/index.php/PRU_Linux_Application_Loader
http://processors.wiki.ti.com/index.php/PRU_Linux_Application_Loader_API_Guide
Pratheesh Gangadhar (2):
PRUSS UIO driver support
Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver
arch/arm/mach-davinci/board-da850-evm.c | 4 +
arch/arm/mach-davinci/da850.c | 35 +++++
arch/arm/mach-davinci/devices-da8xx.c | 73 +++++++++
arch/arm/mach-davinci/include/mach/da8xx.h | 3 +
drivers/uio/Kconfig | 17 ++
drivers/uio/Makefile | 1 +
drivers/uio/uio_pruss.c | 223 ++++++++++++++++++++++++++++
7 files changed, 356 insertions(+), 0 deletions(-)
create mode 100644 drivers/uio/uio_pruss.c
This patch implements PRUSS (Programmable Real-time Unit Sub System)
UIO driver which exports SOC resources associated with PRUSS like
I/O, memories and IRQs to user space. PRUSS is dual 32-bit RISC
processors which is efficient in performing embedded tasks that
require manipulation of packed memory mapped data structures and
efficient in handling system events that have tight real time
constraints. This driver is currently supported on Texas Instruments
DA850, AM18xx and OMAPL1-38 devices.
For example, PRUSS runs firmware for real-time critical industrial
communication data link layer and communicates with application stack
running in user space via shared memory and IRQs.
Signed-off-by: Pratheesh Gangadhar <[email protected]>
---
drivers/uio/Kconfig | 17 ++++
drivers/uio/Makefile | 1 +
drivers/uio/uio_pruss.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 241 insertions(+), 0 deletions(-)
create mode 100644 drivers/uio/uio_pruss.c
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index bb44079..6f3ea9b 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -94,4 +94,21 @@ config UIO_NETX
To compile this driver as a module, choose M here; the module
will be called uio_netx.
+config UIO_PRUSS
+ tristate "Texas Instruments PRUSS driver"
+ depends on ARCH_DAVINCI_DA850
+ help
+ PRUSS driver for OMAPL138/DA850/AM18XX devices
+ PRUSS driver requires user space components, examples and user space
+ driver is available from below SVN repo - you may use anonymous login
+
+ https://gforge.ti.com/gf/project/pru_sw/
+
+ More info on API is available at below wiki
+
+ http://processors.wiki.ti.com/index.php/PRU_Linux_Application_Loader
+
+ To compile this driver as a module, choose M here: the module
+ will be called uio_pruss.
+
endif
diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
index 18fd818..d4dd9a5 100644
--- a/drivers/uio/Makefile
+++ b/drivers/uio/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_UIO_AEC) += uio_aec.o
obj-$(CONFIG_UIO_SERCOS3) += uio_sercos3.o
obj-$(CONFIG_UIO_PCI_GENERIC) += uio_pci_generic.o
obj-$(CONFIG_UIO_NETX) += uio_netx.o
+obj-$(CONFIG_UIO_PRUSS) += uio_pruss.o
diff --git a/drivers/uio/uio_pruss.c b/drivers/uio/uio_pruss.c
new file mode 100644
index 0000000..f2b2f4b
--- /dev/null
+++ b/drivers/uio/uio_pruss.c
@@ -0,0 +1,223 @@
+/*
+ * Programmable Real-Time Unit Sub System (PRUSS) UIO driver (uio_pruss)
+ *
+ * This driver exports PRUSS host event out interrupts and PRUSS, L3 RAM,
+ * and DDR RAM to user space for applications interacting with PRUSS firmware
+ *
+ * Copyright (C) 2010-11 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/uio_driver.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+
+#define DRV_NAME "pruss"
+#define DRV_VERSION "0.50"
+
+/*
+ * Host event IRQ numbers from PRUSS - PRUSS can generate upto 8 interrupt
+ * events to AINTC of ARM host processor - which can be used for IPC b/w PRUSS
+ * firmware and user space application, async notification from PRU firmware
+ * to user space application
+ * 3 PRU_EVTOUT0
+ * 4 PRU_EVTOUT1
+ * 5 PRU_EVTOUT2
+ * 6 PRU_EVTOUT3
+ * 7 PRU_EVTOUT4
+ * 8 PRU_EVTOUT5
+ * 9 PRU_EVTOUT6
+ * 10 PRU_EVTOUT7
+*/
+
+#define MAX_PRUSS_EVTOUT_INSTANCE 8
+
+#define PINTC_HIPIR 0x4900
+#define PINTC_HIPIR_NO_PEND_MASK 0x80000000
+#define PINTC_HIER 0x5500
+
+struct clk *pruss_clk;
+struct uio_info *info;
+void *ddr_virt_addr = NULL, *prussio_virt_addr = NULL;
+dma_addr_t ddr_phy_addr;
+
+static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info)
+{
+ void __iomem *base = dev_info->mem[0].internal_addr;
+ void __iomem *intren_reg = base + PINTC_HIER;
+ void __iomem *intrstat_reg = base + PINTC_HIPIR + ((irq - 1) << 2);
+ int intren_regval = ioread32(intren_reg), intr_mask = (1 << (irq - 1));
+
+ /* Is interrupt enabled and active ? */
+ if (!(intren_regval & intr_mask)
+ && (ioread32(intrstat_reg) & PINTC_HIPIR_NO_PEND_MASK))
+ return IRQ_NONE;
+
+ /* Disable interrupt */
+ iowrite32((intren_regval & ~intr_mask), intren_reg);
+ return IRQ_HANDLED;
+}
+
+static void pruss_cleanup(struct platform_device *dev, struct uio_info *info)
+{
+ int count;
+ struct uio_info *p;
+
+ for (count = 0, p = info; count < MAX_PRUSS_EVTOUT_INSTANCE;
+ count++, p++) {
+ uio_unregister_device(p);
+ kfree(p->name);
+ }
+ iounmap(prussio_virt_addr);
+ dma_free_coherent(&dev->dev, info[0].mem[2].size,
+ info[0].mem[2].internal_addr, info[0].mem[2].addr);
+
+ kfree(info);
+ clk_put(pruss_clk);
+}
+
+static int __devinit pruss_probe(struct platform_device *dev)
+{
+ int ret = -ENODEV, count = 0;
+ struct resource *regs_prussio, *regs_l3ram, *regs_ddr;
+ struct uio_info *p;
+
+ info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
+ GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ /* Power on PRU in case its not done as part of boot-loader */
+ pruss_clk = clk_get(&dev->dev, "pruss");
+ if (IS_ERR(pruss_clk)) {
+ dev_err(&dev->dev, "Failed to get clock\n");
+ ret = PTR_ERR(pruss_clk);
+ return ret;
+ } else {
+ clk_enable(pruss_clk);
+ }
+
+ regs_prussio = platform_get_resource(dev, IORESOURCE_MEM, 0);
+ if (!regs_prussio) {
+ dev_err(&dev->dev, "No PRUSS I/O resource specified\n");
+ goto out_free;
+ }
+
+ regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1);
+ if (!regs_l3ram) {
+ dev_err(&dev->dev, "No L3 RAM resource specified\n");
+ goto out_free;
+ }
+
+ regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2);
+ if (!regs_ddr) {
+ dev_err(&dev->dev, "No External RAM resource specified\n");
+ goto out_free;
+ }
+
+ if (!regs_prussio->start || !regs_l3ram->start) {
+ dev_err(&dev->dev, "Invalid memory resource\n");
+ goto out_free;
+ }
+
+ ddr_virt_addr =
+ dma_alloc_coherent(&dev->dev, regs_ddr->end - regs_ddr->start + 1,
+ &ddr_phy_addr, GFP_KERNEL | GFP_DMA);
+ if (!ddr_virt_addr) {
+ dev_err(&dev->dev, "Could not allocate external memory\n");
+ goto out_free;
+ }
+
+ prussio_virt_addr =
+ ioremap(regs_prussio->start,
+ regs_prussio->end - regs_prussio->start + 1);
+ if (prussio_virt_addr) {
+ dev_err(&dev->dev, "Can't remap PRUSS I/O address range\n");
+ goto out_free;
+ }
+
+ for (count = 0, p = info; count < MAX_PRUSS_EVTOUT_INSTANCE;
+ count++, p++) {
+ p->mem[0].internal_addr = prussio_virt_addr;
+ p->mem[0].addr = regs_prussio->start;
+ p->mem[0].size = regs_prussio->end - regs_prussio->start + 1;
+ p->mem[0].memtype = UIO_MEM_PHYS;
+
+ p->mem[1].addr = regs_l3ram->start;
+ p->mem[1].size = regs_l3ram->end - regs_l3ram->start + 1;
+ p->mem[1].memtype = UIO_MEM_PHYS;
+
+ p->mem[2].internal_addr = ddr_virt_addr;
+ p->mem[2].addr = ddr_phy_addr;
+ p->mem[2].size = regs_ddr->end - regs_ddr->start + 1;
+ p->mem[2].memtype = UIO_MEM_PHYS;
+
+ p->name = kasprintf(GFP_KERNEL, "pruss_evt%d", count);
+ p->version = "0.50";
+
+ /* Register PRUSS IRQ lines */
+ p->irq = IRQ_DA8XX_EVTOUT0 + count;
+ p->handler = pruss_handler;
+
+ ret = uio_register_device(&dev->dev, p);
+
+ if (ret < 0)
+ goto out_free;
+ }
+
+ platform_set_drvdata(dev, info);
+ return 0;
+
+out_free:
+ pruss_cleanup(dev, info);
+ return ret;
+}
+
+static int __devexit pruss_remove(struct platform_device *dev)
+{
+ struct uio_info *info = platform_get_drvdata(dev);
+
+ pruss_cleanup(dev, info);
+ platform_set_drvdata(dev, NULL);
+ return 0;
+}
+
+static struct platform_driver pruss_driver = {
+ .probe = pruss_probe,
+ .remove = __devexit_p(pruss_remove),
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init pruss_init_module(void)
+{
+ return platform_driver_register(&pruss_driver);
+}
+
+module_init(pruss_init_module);
+
+static void __exit pruss_exit_module(void)
+{
+ platform_driver_unregister(&pruss_driver);
+}
+
+module_exit(pruss_exit_module);
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION(DRV_VERSION);
+MODULE_AUTHOR("Amit Chatterjee <[email protected]>");
+MODULE_AUTHOR("Pratheesh Gangadhar <[email protected]>");
--
1.6.0.6
This patch defines PRUSS, ECAP clocks, memory and IRQ resources
used by PRUSS UIO driver in DA850/AM18xx/OMAPL1-38 devices. UIO
driver exports 64K I/O region of PRUSS, 128KB L3 RAM and 256KB
DDR buffer to user space. PRUSS has 8 host event interrupt lines
mapped to IRQ_DA8XX_EVTOUT0..7 of ARM9 INTC.These in conjunction
with shared memory can be used to implement IPC between ARM9 and
PRUSS.
Signed-off-by: Pratheesh Gangadhar <[email protected]>
---
arch/arm/mach-davinci/board-da850-evm.c | 4 ++
arch/arm/mach-davinci/da850.c | 35 +++++++++++++
arch/arm/mach-davinci/devices-da8xx.c | 73 ++++++++++++++++++++++++++++
arch/arm/mach-davinci/include/mach/da8xx.h | 3 +
4 files changed, 115 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
index 11f986b..bd85aa3 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -1077,6 +1077,10 @@ static __init void da850_evm_init(void)
pr_warning("da850_evm_init: i2c0 registration failed: %d\n",
ret);
+ ret = da8xx_register_pruss();
+ if (ret)
+ pr_warning("da850_evm_init: pruss registration failed: %d\n",
+ ret);
ret = da8xx_register_watchdog();
if (ret)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 3443d97..0096d4f 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -238,6 +238,13 @@ static struct clk tptc2_clk = {
.flags = ALWAYS_ENABLED,
};
+static struct clk pruss_clk = {
+ .name = "pruss",
+ .parent = &pll0_sysclk2,
+ .lpsc = DA8XX_LPSC0_DMAX,
+ .flags = ALWAYS_ENABLED,
+};
+
static struct clk uart0_clk = {
.name = "uart0",
.parent = &pll0_sysclk2,
@@ -359,6 +366,30 @@ static struct clk usb20_clk = {
.gpsc = 1,
};
+static struct clk ecap0_clk = {
+ .name = "ecap0",
+ .parent = &pll0_sysclk2,
+ .lpsc = DA8XX_LPSC1_ECAP,
+ .flags = DA850_CLK_ASYNC3,
+ .gpsc = 1,
+};
+
+static struct clk ecap1_clk = {
+ .name = "ecap1",
+ .parent = &pll0_sysclk2,
+ .lpsc = DA8XX_LPSC1_ECAP,
+ .flags = DA850_CLK_ASYNC3,
+ .gpsc = 1,
+};
+
+static struct clk ecap2_clk = {
+ .name = "ecap2",
+ .parent = &pll0_sysclk2,
+ .lpsc = DA8XX_LPSC1_ECAP,
+ .flags = DA850_CLK_ASYNC3,
+ .gpsc = 1,
+};
+
static struct clk_lookup da850_clks[] = {
CLK(NULL, "ref", &ref_clk),
CLK(NULL, "pll0", &pll0_clk),
@@ -386,6 +417,7 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, "tptc1", &tptc1_clk),
CLK(NULL, "tpcc1", &tpcc1_clk),
CLK(NULL, "tptc2", &tptc2_clk),
+ CLK(NULL, "pruss", &pruss_clk),
CLK(NULL, "uart0", &uart0_clk),
CLK(NULL, "uart1", &uart1_clk),
CLK(NULL, "uart2", &uart2_clk),
@@ -403,6 +435,9 @@ static struct clk_lookup da850_clks[] = {
CLK(NULL, "aemif", &aemif_clk),
CLK(NULL, "usb11", &usb11_clk),
CLK(NULL, "usb20", &usb20_clk),
+ CLK(NULL, "ecap0", &ecap0_clk),
+ CLK(NULL, "ecap1", &ecap1_clk),
+ CLK(NULL, "ecap2", &ecap2_clk),
CLK(NULL, NULL, NULL),
};
diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c
index beda8a4..4ea3d1f 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -725,3 +725,76 @@ int __init da8xx_register_cpuidle(void)
return platform_device_register(&da8xx_cpuidle_device);
}
+static struct resource pruss_resources[] = {
+ [0] = {
+ .start = DA8XX_PRUSS_BASE,
+ .end = DA8XX_PRUSS_BASE + SZ_64K - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = DA8XX_L3RAM_BASE,
+ .end = DA8XX_L3RAM_BASE + SZ_128K - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ [2] = {
+ .start = 0,
+ .end = SZ_256K - 1,
+ .flags = IORESOURCE_MEM,
+ },
+
+ [3] = {
+ .start = IRQ_DA8XX_EVTOUT0,
+ .end = IRQ_DA8XX_EVTOUT0,
+ .flags = IORESOURCE_IRQ,
+ },
+ [4] = {
+ .start = IRQ_DA8XX_EVTOUT1,
+ .end = IRQ_DA8XX_EVTOUT1,
+ .flags = IORESOURCE_IRQ,
+ },
+ [5] = {
+ .start = IRQ_DA8XX_EVTOUT2,
+ .end = IRQ_DA8XX_EVTOUT2,
+ .flags = IORESOURCE_IRQ,
+ },
+ [6] = {
+ .start = IRQ_DA8XX_EVTOUT3,
+ .end = IRQ_DA8XX_EVTOUT3,
+ .flags = IORESOURCE_IRQ,
+ },
+ [7] = {
+ .start = IRQ_DA8XX_EVTOUT4,
+ .end = IRQ_DA8XX_EVTOUT4,
+ .flags = IORESOURCE_IRQ,
+ },
+ [8] = {
+ .start = IRQ_DA8XX_EVTOUT5,
+ .end = IRQ_DA8XX_EVTOUT5,
+ .flags = IORESOURCE_IRQ,
+ },
+ [9] = {
+ .start = IRQ_DA8XX_EVTOUT6,
+ .end = IRQ_DA8XX_EVTOUT6,
+ .flags = IORESOURCE_IRQ,
+ },
+ [10] = {
+ .start = IRQ_DA8XX_EVTOUT7,
+ .end = IRQ_DA8XX_EVTOUT7,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct platform_device pruss_device = {
+ .name = "pruss",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(pruss_resources),
+ .resource = pruss_resources,
+ .dev = {
+ .coherent_dma_mask = 0xffffffff,
+ }
+};
+
+int __init da8xx_register_pruss()
+{
+ return platform_device_register(&pruss_device);
+}
diff --git a/arch/arm/mach-davinci/include/mach/da8xx.h b/arch/arm/mach-davinci/include/mach/da8xx.h
index cfcb223..3ed6ee0 100644
--- a/arch/arm/mach-davinci/include/mach/da8xx.h
+++ b/arch/arm/mach-davinci/include/mach/da8xx.h
@@ -60,6 +60,7 @@ extern unsigned int da850_max_speed;
#define DA8XX_PLL0_BASE 0x01c11000
#define DA8XX_TIMER64P0_BASE 0x01c20000
#define DA8XX_TIMER64P1_BASE 0x01c21000
+#define DA8XX_PRUSS_BASE 0x01c30000
#define DA8XX_GPIO_BASE 0x01e26000
#define DA8XX_PSC1_BASE 0x01e27000
#define DA8XX_LCD_CNTRL_BASE 0x01e13000
@@ -68,6 +69,7 @@ extern unsigned int da850_max_speed;
#define DA8XX_AEMIF_CS2_BASE 0x60000000
#define DA8XX_AEMIF_CS3_BASE 0x62000000
#define DA8XX_AEMIF_CTL_BASE 0x68000000
+#define DA8XX_L3RAM_BASE 0x80000000
#define DA8XX_DDR2_CTL_BASE 0xb0000000
#define DA8XX_ARM_RAM_BASE 0xffff0000
@@ -90,6 +92,7 @@ int da850_register_cpufreq(char *async_clk);
int da8xx_register_cpuidle(void);
void __iomem * __init da8xx_get_mem_ctlr(void);
int da850_register_pm(struct platform_device *pdev);
+int da8xx_register_pruss(void);
extern struct platform_device da8xx_serial_device;
extern struct emac_platform_data da8xx_emac_pdata;
--
1.6.0.6
On Wed, Feb 23, 2011 at 07:22:40PM +0530, Pratheesh Gangadhar wrote:
> This patch implements PRUSS (Programmable Real-time Unit Sub System)
> UIO driver which exports SOC resources associated with PRUSS like
> I/O, memories and IRQs to user space. PRUSS is dual 32-bit RISC
> processors which is efficient in performing embedded tasks that
> require manipulation of packed memory mapped data structures and
> efficient in handling system events that have tight real time
> constraints. This driver is currently supported on Texas Instruments
> DA850, AM18xx and OMAPL1-38 devices.
> For example, PRUSS runs firmware for real-time critical industrial
> communication data link layer and communicates with application stack
> running in user space via shared memory and IRQs.
>
> Signed-off-by: Pratheesh Gangadhar <[email protected]>
> ---
> drivers/uio/Kconfig | 17 ++++
> drivers/uio/Makefile | 1 +
> drivers/uio/uio_pruss.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 241 insertions(+), 0 deletions(-)
> create mode 100644 drivers/uio/uio_pruss.c
>
[...]
> +
> +static int __devinit pruss_probe(struct platform_device *dev)
> +{
> + int ret = -ENODEV, count = 0;
> + struct resource *regs_prussio, *regs_l3ram, *regs_ddr;
> + struct uio_info *p;
> +
> + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + /* Power on PRU in case its not done as part of boot-loader */
> + pruss_clk = clk_get(&dev->dev, "pruss");
> + if (IS_ERR(pruss_clk)) {
> + dev_err(&dev->dev, "Failed to get clock\n");
> + ret = PTR_ERR(pruss_clk);
> + return ret;
You leak memory here. What about freeing "info"?
> + } else {
> + clk_enable(pruss_clk);
> + }
Thanks,
Hans
On Wed, 23 Feb 2011, Pratheesh Gangadhar wrote:
Is it actually too much of an hassle to cc the people who spent their
time to review previous versions of your patch ?
> +struct clk *pruss_clk;
> +struct uio_info *info;
> +void *ddr_virt_addr = NULL, *prussio_virt_addr = NULL;
Grrr. We do not initialize with NULL.
> +dma_addr_t ddr_phy_addr;
Also all of these want to be static.
> +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info)
> +{
> + void __iomem *base = dev_info->mem[0].internal_addr;
> + void __iomem *intren_reg = base + PINTC_HIER;
> + void __iomem *intrstat_reg = base + PINTC_HIPIR + ((irq - 1) << 2);
> + int intren_regval = ioread32(intren_reg), intr_mask = (1 << (irq - 1));
> +
> + /* Is interrupt enabled and active ? */
> + if (!(intren_regval & intr_mask)
> + && (ioread32(intrstat_reg) & PINTC_HIPIR_NO_PEND_MASK))
> + return IRQ_NONE;
You could avoid all these ugly line breaks by chosing sensible short
names for variables and defines.
> + /* Disable interrupt */
> + iowrite32((intren_regval & ~intr_mask), intren_reg);
> + return IRQ_HANDLED;
> +}
> +
> +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info)
> +{
> + int count;
> + struct uio_info *p;
Nit: I prefer the longer declarations above the shorter ones. It's
easier to parse, but that's really my personal preference.
struct uio_info *p;
int count;
Compare and contrast it and decide yourself.
> +
> + for (count = 0, p = info; count < MAX_PRUSS_EVTOUT_INSTANCE;
> + count++, p++) {
Aside of making that constant shorter, you could assign info to p in
the variable declaration already. So avoid that line break.
> + uio_unregister_device(p);
> + kfree(p->name);
> + }
> + iounmap(prussio_virt_addr);
> + dma_free_coherent(&dev->dev, info[0].mem[2].size,
> + info[0].mem[2].internal_addr, info[0].mem[2].addr);
You sure, that iounmap and dma_free_coherent are too happy about being
called with NULL pointers?
> +
> + kfree(info);
> + clk_put(pruss_clk);
> +}
> +
> +static int __devinit pruss_probe(struct platform_device *dev)
> +{
> + int ret = -ENODEV, count = 0;
> + struct resource *regs_prussio, *regs_l3ram, *regs_ddr;
> + struct uio_info *p;
> +
> + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> + GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + /* Power on PRU in case its not done as part of boot-loader */
> + pruss_clk = clk_get(&dev->dev, "pruss");
> + if (IS_ERR(pruss_clk)) {
> + dev_err(&dev->dev, "Failed to get clock\n");
> + ret = PTR_ERR(pruss_clk);
> + return ret;
Memory leak.
> + } else {
> + clk_enable(pruss_clk);
> + }
> +
> + regs_prussio = platform_get_resource(dev, IORESOURCE_MEM, 0);
> + if (!regs_prussio) {
> + dev_err(&dev->dev, "No PRUSS I/O resource specified\n");
> + goto out_free;
> + }
> +
> + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1);
> + if (!regs_l3ram) {
> + dev_err(&dev->dev, "No L3 RAM resource specified\n");
> + goto out_free;
> + }
> +
> + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2);
> + if (!regs_ddr) {
> + dev_err(&dev->dev, "No External RAM resource specified\n");
> + goto out_free;
> + }
> +
> + if (!regs_prussio->start || !regs_l3ram->start) {
> + dev_err(&dev->dev, "Invalid memory resource\n");
> + goto out_free;
> + }
> +
> + ddr_virt_addr =
> + dma_alloc_coherent(&dev->dev, regs_ddr->end - regs_ddr->start + 1,
> + &ddr_phy_addr, GFP_KERNEL | GFP_DMA);
> + if (!ddr_virt_addr) {
> + dev_err(&dev->dev, "Could not allocate external memory\n");
> + goto out_free;
> + }
> +
> + prussio_virt_addr =
> + ioremap(regs_prussio->start,
> + regs_prussio->end - regs_prussio->start + 1);
Either make those variable names shorter or do:
len = regs_prussio->end - regs_prussio->start + 1;
prussio_virt_addr = ioremap(regs_prussio->start, len);
Those multi line breaks are irritating and not necessary at all.
They result from mindlessly converting code following an unfortunately
popular codingstyle which is only readable on extra wide screen
monitors. I really wonder when people start to understand that the
human eye can only parse up to a certain line width easily. There is a
reason why newspapers are printed in columns.
Please be a bit more careful about this. Just mechanically converting
existing horrible code to 80 chars does not make it more readable and
less horrible than not converting it at all.
> + if (prussio_virt_addr) {
> + dev_err(&dev->dev, "Can't remap PRUSS I/O address range\n");
> + goto out_free;
> + }
> +
> + for (count = 0, p = info; count < MAX_PRUSS_EVTOUT_INSTANCE;
> + count++, p++) {
See above.
for (cnt = 0, p = info; cnt < MAX_PRUSS_EVT; cnt++, p++) {
is better readable and contains exactly the same amount of
information.
Thanks,
tglx
Hello.
On 23-02-2011 23:25, Thomas Gleixner wrote:
>> + prussio_virt_addr =
>> + ioremap(regs_prussio->start,
>> + regs_prussio->end - regs_prussio->start + 1);
> Either make those variable names shorter or do:
> len = regs_prussio->end - regs_prussio->start + 1;
Or even:
len = resource_size(regs_prussio);
> prussio_virt_addr = ioremap(regs_prussio->start, len);
WBR, Sergei
On Wed, 23 Feb 2011, Sergei Shtylyov wrote:
> Hello.
>
> On 23-02-2011 23:25, Thomas Gleixner wrote:
>
> > > + prussio_virt_addr =
> > > + ioremap(regs_prussio->start,
> > > + regs_prussio->end - regs_prussio->start + 1);
>
> > Either make those variable names shorter or do:
>
> > len = regs_prussio->end - regs_prussio->start + 1;
>
> Or even:
>
> len = resource_size(regs_prussio);
>
> > prussio_virt_addr = ioremap(regs_prussio->start, len);
Good point. Forgot about that one :)
Thanks,
tglx
> -----Original Message-----
> From: Thomas Gleixner [mailto:[email protected]]
> Sent: Thursday, February 24, 2011 1:56 AM
> To: TK, Pratheesh Gangadhar
> Cc: [email protected]; [email protected];
> [email protected]; Chatterjee, Amit; [email protected]; linux-arm-
> [email protected]
> Subject: Re: [PATCH v3 1/2] PRUSS UIO driver support
>
> On Wed, 23 Feb 2011, Pratheesh Gangadhar wrote:
>
> Is it actually too much of an hassle to cc the people who spent their
> time to review previous versions of your patch ?
>
Sorry. Will do...
> > +struct clk *pruss_clk;
> > +struct uio_info *info;
> > +void *ddr_virt_addr = NULL, *prussio_virt_addr = NULL;
>
> Grrr. We do not initialize with NULL.
>
> > +dma_addr_t ddr_phy_addr;
>
> Also all of these want to be static.
>
Ok.
>
> > + uio_unregister_device(p);
> > + kfree(p->name);
> > + }
> > + iounmap(prussio_virt_addr);
> > + dma_free_coherent(&dev->dev, info[0].mem[2].size,
> > + info[0].mem[2].internal_addr, info[0].mem[2].addr);
>
> You sure, that iounmap and dma_free_coherent are too happy about being
> called with NULL pointers?
>
For iounmap its ok, not ok for dma_free_coherent. Will fix.
> > +
> > + kfree(info);
> > + clk_put(pruss_clk);
> > +}
> > +
> > +static int __devinit pruss_probe(struct platform_device *dev)
> > +{
> > + int ret = -ENODEV, count = 0;
> > + struct resource *regs_prussio, *regs_l3ram, *regs_ddr;
> > + struct uio_info *p;
> > +
> > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> > + GFP_KERNEL);
> > + if (!info)
> > + return -ENOMEM;
> > +
> > + /* Power on PRU in case its not done as part of boot-loader */
> > + pruss_clk = clk_get(&dev->dev, "pruss");
> > + if (IS_ERR(pruss_clk)) {
> > + dev_err(&dev->dev, "Failed to get clock\n");
> > + ret = PTR_ERR(pruss_clk);
> > + return ret;
>
> Memory leak.
Will fix.
Thanks,
Pratheesh
> -----Original Message-----
> From: Thomas Gleixner [mailto:[email protected]]
> Sent: Thursday, February 24, 2011 2:24 AM
> To: Sergei Shtylyov
> Cc: TK, Pratheesh Gangadhar; davinci-linux-open-
> [email protected]; Chatterjee, Amit; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]
> Subject: Re: [PATCH v3 1/2] PRUSS UIO driver support
>
> On Wed, 23 Feb 2011, Sergei Shtylyov wrote:
> > Hello.
> >
> > On 23-02-2011 23:25, Thomas Gleixner wrote:
> >
> > > > + prussio_virt_addr =
> > > > + ioremap(regs_prussio->start,
> > > > + regs_prussio->end - regs_prussio->start + 1);
> >
> > > Either make those variable names shorter or do:
> >
> > > len = regs_prussio->end - regs_prussio->start + 1;
> >
> > Or even:
> >
> > len = resource_size(regs_prussio);
> >
> > > prussio_virt_addr = ioremap(regs_prussio->start, len);
>
> Good point. Forgot about that one :)
>
Thanks will do this.
Pratheesh
> -----Original Message-----
> From: Hans J. Koch [mailto:[email protected]]
> Sent: Wednesday, February 23, 2011 11:07 PM
> To: TK, Pratheesh Gangadhar
> Cc: [email protected]; [email protected];
> [email protected]; Chatterjee, Amit; [email protected]; linux-arm-
> [email protected]
> Subject: Re: [PATCH v3 1/2] PRUSS UIO driver support
>
> On Wed, Feb 23, 2011 at 07:22:40PM +0530, Pratheesh Gangadhar wrote:
> > This patch implements PRUSS (Programmable Real-time Unit Sub System)
> > UIO driver which exports SOC resources associated with PRUSS like
> > I/O, memories and IRQs to user space. PRUSS is dual 32-bit RISC
> > processors which is efficient in performing embedded tasks that
> > require manipulation of packed memory mapped data structures and
> > efficient in handling system events that have tight real time
> > constraints. This driver is currently supported on Texas Instruments
> > DA850, AM18xx and OMAPL1-38 devices.
> > For example, PRUSS runs firmware for real-time critical industrial
> > communication data link layer and communicates with application stack
> > running in user space via shared memory and IRQs.
> >
> > Signed-off-by: Pratheesh Gangadhar <[email protected]>
> > ---
> > drivers/uio/Kconfig | 17 ++++
> > drivers/uio/Makefile | 1 +
> > drivers/uio/uio_pruss.c | 223
> +++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 241 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/uio/uio_pruss.c
> >
>
> [...]
>
> > +
> > +static int __devinit pruss_probe(struct platform_device *dev)
> > +{
> > + int ret = -ENODEV, count = 0;
> > + struct resource *regs_prussio, *regs_l3ram, *regs_ddr;
> > + struct uio_info *p;
> > +
> > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE,
> > + GFP_KERNEL);
> > + if (!info)
> > + return -ENOMEM;
> > +
> > + /* Power on PRU in case its not done as part of boot-loader */
> > + pruss_clk = clk_get(&dev->dev, "pruss");
> > + if (IS_ERR(pruss_clk)) {
> > + dev_err(&dev->dev, "Failed to get clock\n");
> > + ret = PTR_ERR(pruss_clk);
> > + return ret;
>
> You leak memory here. What about freeing "info"?
Ok. Will fix.
Thanks,
Pratheesh