* [PATCH v2 0/2] Add PRUSS UIO driver support @ 2011-02-21 17:35 Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 1/2] " Pratheesh Gangadhar 0 siblings, 1 reply; 7+ messages in thread From: Pratheesh Gangadhar @ 2011-02-21 17:35 UTC (permalink / raw) To: linux-arm-kernel 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 | 9 + drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 231 ++++++++++++++++++++++++++++ 7 files changed, 356 insertions(+), 0 deletions(-) create mode 100644 drivers/uio/uio_pruss.c ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] PRUSS UIO driver support 2011-02-21 17:35 [PATCH v2 0/2] Add PRUSS UIO driver support Pratheesh Gangadhar @ 2011-02-21 17:35 ` Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 2/2] Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver Pratheesh Gangadhar ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Pratheesh Gangadhar @ 2011-02-21 17:35 UTC (permalink / raw) To: linux-arm-kernel 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 <pratheesh@ti.com> --- drivers/uio/Kconfig | 9 ++ drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++ 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..f92f20d 100644 --- a/drivers/uio/Kconfig +++ b/drivers/uio/Kconfig @@ -94,4 +94,13 @@ 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 + 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..b402197 --- /dev/null +++ b/drivers/uio/uio_pruss.c @@ -0,0 +1,231 @@ +/* + * 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 PRUSS_INTC_HIPIR 0x4900 +#define PRUSS_INTC_HIPIR_INTPEND_MASK 0x80000000 +#define PRUSS_INTC_HIER 0x5500 + +struct clk *pruss_clk; +struct uio_info *info; +void *ddr_virt_addr; +dma_addr_t ddr_phy_addr; + +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) +{ + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr + + PRUSS_INTC_HIER; + void __iomem *int_status_reg = dev_info->mem[0].internal_addr + + PRUSS_INTC_HIPIR+((irq-1) << 2); + /* Is interrupt enabled and active ? */ + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) && + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK)) + return IRQ_NONE; + + /* Disable interrupt */ + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)), + int_enable_reg); + return IRQ_HANDLED; +} + +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info) +{ + int count; + if (info) { + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { + uio_unregister_device(&info[count]); + kfree(info[count].name); + iounmap(info[count].mem[0].internal_addr); + + } + if (ddr_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); + } + if (pruss_clk != NULL) + clk_put(pruss_clk); +} + +static int __devinit pruss_probe(struct platform_device *dev) +{ + int ret = -ENODEV, count = 0; + struct resource *regs_pruram, *regs_l3ram, *regs_ddr; + char *string; + + /* 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); + } + + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE, + GFP_KERNEL); + if (info == NULL) + return -ENOMEM; + + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0); + if (!regs_pruram) { + dev_err(&dev->dev, "No memory resource specified\n"); + goto out_free; + } + + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1); + if (!regs_l3ram) { + dev_err(&dev->dev, "No memory resource specified\n"); + goto out_free; + } + + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2); + if (!regs_ddr) { + dev_err(&dev->dev, "No memory resource specified\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; + } + + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { + info[count].mem[0].addr = regs_pruram->start; + info[count].mem[0].size = + regs_pruram->end - regs_pruram->start + 1; + if (!info[count].mem[0].addr || + !(info[count].mem[0].size - 1)) { + dev_err(&dev->dev, "Invalid memory resource\n"); + goto out_free; + } + info[count].mem[0].internal_addr = + ioremap(regs_pruram->start, info[count].mem[0].size); + if (!info[count].mem[0].internal_addr) { + dev_err(&dev->dev, + "Can't remap memory address range\n"); + goto out_free; + } + info[count].mem[0].memtype = UIO_MEM_PHYS; + + info[count].mem[1].addr = regs_l3ram->start; + info[count].mem[1].size = + regs_l3ram->end - regs_l3ram->start + 1; + if (!info[count].mem[1].addr || + !(info[count].mem[1].size - 1)) { + dev_err(&dev->dev, "Invalid memory resource\n"); + goto out_free; + } + info[count].mem[1].memtype = UIO_MEM_PHYS; + + info[count].mem[2].internal_addr = ddr_virt_addr; + info[count].mem[2].addr = ddr_phy_addr; + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1; + info[count].mem[2].memtype = UIO_MEM_PHYS; + + string = kzalloc(20, GFP_KERNEL); + sprintf(string, "pruss_evt%d", count); + info[count].name = string; + info[count].version = "0.50"; + + /* Register PRUSS IRQ lines */ + info[count].irq = IRQ_DA8XX_EVTOUT0 + count; + + info[count].irq_flags = 0; + info[count].handler = pruss_handler; + + ret = uio_register_device(&dev->dev, &info[count]); + + 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 <amit.chatterjee@ti.com>"); +MODULE_AUTHOR("Pratheesh Gangadhar <pratheesh@ti.com>"); -- 1.6.0.6 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver 2011-02-21 17:35 ` [PATCH v2 1/2] " Pratheesh Gangadhar @ 2011-02-21 17:35 ` Pratheesh Gangadhar 2011-02-21 18:44 ` [PATCH v2 1/2] PRUSS UIO driver support Hans J. Koch 2011-02-21 18:54 ` Thomas Gleixner 2 siblings, 0 replies; 7+ messages in thread From: Pratheesh Gangadhar @ 2011-02-21 17:35 UTC (permalink / raw) To: linux-arm-kernel 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 <pratheesh@ti.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] PRUSS UIO driver support 2011-02-21 17:35 ` [PATCH v2 1/2] " Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 2/2] Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver Pratheesh Gangadhar @ 2011-02-21 18:44 ` Hans J. Koch 2011-02-22 12:01 ` TK, Pratheesh Gangadhar 2011-02-21 18:54 ` Thomas Gleixner 2 siblings, 1 reply; 7+ messages in thread From: Hans J. Koch @ 2011-02-21 18:44 UTC (permalink / raw) To: linux-arm-kernel On Mon, Feb 21, 2011 at 11:05:14PM +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 <pratheesh@ti.com> Looks much better now. Just some minor issues, see below. Thanks, Hans > --- > drivers/uio/Kconfig | 9 ++ > drivers/uio/Makefile | 1 + > drivers/uio/uio_pruss.c | 231 +++++++++++++++++++++++++++++++++++++++++++++++ > 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..f92f20d 100644 > --- a/drivers/uio/Kconfig > +++ b/drivers/uio/Kconfig > @@ -94,4 +94,13 @@ 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 It would be nice to mention a link here where these user space components are available. > + 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..b402197 > --- /dev/null > +++ b/drivers/uio/uio_pruss.c > @@ -0,0 +1,231 @@ > +/* > + * 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 PRUSS_INTC_HIPIR 0x4900 > +#define PRUSS_INTC_HIPIR_INTPEND_MASK 0x80000000 > +#define PRUSS_INTC_HIER 0x5500 > + > +struct clk *pruss_clk; > +struct uio_info *info; > +void *ddr_virt_addr; > +dma_addr_t ddr_phy_addr; > + > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > +{ > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr > + + PRUSS_INTC_HIER; > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr > + + PRUSS_INTC_HIPIR+((irq-1) << 2); Blank line between variable definitions and code, please. > + /* Is interrupt enabled and active ? */ > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) && Hmm. I'd prefer blanks around operands, like (1 << (irq - 1))). I won't be too picky about that, noticing that checkpatch.pl doesn't complain. If you want to do me a favor, you can fix it ;-) > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK)) > + return IRQ_NONE; > + > + /* Disable interrupt */ > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)), If you save the return value of the first ioread32(int_enable_reg) in a variable, you don't need the second hardware access. > + int_enable_reg); > + return IRQ_HANDLED; > +} > + > +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info) > +{ > + int count; Blank line between variable definitions and code, please. > + if (info) { > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > + uio_unregister_device(&info[count]); > + kfree(info[count].name); > + iounmap(info[count].mem[0].internal_addr); > + > + } > + if (ddr_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); > + } > + if (pruss_clk != NULL) > + clk_put(pruss_clk); > +} > + > +static int __devinit pruss_probe(struct platform_device *dev) > +{ > + int ret = -ENODEV, count = 0; > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr; > + char *string; > + > + /* 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); > + } > + > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE, > + GFP_KERNEL); > + if (info == NULL) if (!info) looks better. > + return -ENOMEM; > + > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0); > + if (!regs_pruram) { > + dev_err(&dev->dev, "No memory resource specified\n"); > + goto out_free; > + } > + > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1); > + if (!regs_l3ram) { > + dev_err(&dev->dev, "No memory resource specified\n"); > + goto out_free; > + } > + > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2); > + if (!regs_ddr) { > + dev_err(&dev->dev, "No memory resource specified\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; > + } > + > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > + info[count].mem[0].addr = regs_pruram->start; > + info[count].mem[0].size = > + regs_pruram->end - regs_pruram->start + 1; > + if (!info[count].mem[0].addr || > + !(info[count].mem[0].size - 1)) { > + dev_err(&dev->dev, "Invalid memory resource\n"); > + goto out_free; > + } > + info[count].mem[0].internal_addr = > + ioremap(regs_pruram->start, info[count].mem[0].size); > + if (!info[count].mem[0].internal_addr) { > + dev_err(&dev->dev, > + "Can't remap memory address range\n"); > + goto out_free; > + } > + info[count].mem[0].memtype = UIO_MEM_PHYS; > + > + info[count].mem[1].addr = regs_l3ram->start; > + info[count].mem[1].size = > + regs_l3ram->end - regs_l3ram->start + 1; > + if (!info[count].mem[1].addr || > + !(info[count].mem[1].size - 1)) { > + dev_err(&dev->dev, "Invalid memory resource\n"); > + goto out_free; > + } > + info[count].mem[1].memtype = UIO_MEM_PHYS; > + > + info[count].mem[2].internal_addr = ddr_virt_addr; > + info[count].mem[2].addr = ddr_phy_addr; > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1; > + info[count].mem[2].memtype = UIO_MEM_PHYS; > + > + string = kzalloc(20, GFP_KERNEL); > + sprintf(string, "pruss_evt%d", count); > + info[count].name = string; > + info[count].version = "0.50"; > + > + /* Register PRUSS IRQ lines */ > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count; > + > + info[count].irq_flags = 0; > + info[count].handler = pruss_handler; > + > + ret = uio_register_device(&dev->dev, &info[count]); > + > + 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); Blank line, please. > + 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 <amit.chatterjee@ti.com>"); > +MODULE_AUTHOR("Pratheesh Gangadhar <pratheesh@ti.com>"); > -- > 1.6.0.6 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] PRUSS UIO driver support 2011-02-21 18:44 ` [PATCH v2 1/2] PRUSS UIO driver support Hans J. Koch @ 2011-02-22 12:01 ` TK, Pratheesh Gangadhar 0 siblings, 0 replies; 7+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-02-22 12:01 UTC (permalink / raw) To: linux-arm-kernel Hi, > -----Original Message----- > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > Sent: Tuesday, February 22, 2011 12:15 AM > To: TK, Pratheesh Gangadhar > Cc: davinci-linux-open-source at linux.davincidsp.com; hjk at hansjkoch.de; > gregkh at suse.de; Chatterjee, Amit; linux-kernel at vger.kernel.org; linux-arm- > kernel at lists.infradead.org > Subject: Re: [PATCH v2 1/2] PRUSS UIO driver support > > On Mon, Feb 21, 2011 at 11:05:14PM +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 <pratheesh@ti.com> > > Looks much better now. Just some minor issues, see below. > > Thanks, > Hans > > > --- > > drivers/uio/Kconfig | 9 ++ > > drivers/uio/Makefile | 1 + > > drivers/uio/uio_pruss.c | 231 > +++++++++++++++++++++++++++++++++++++++++++++++ > > 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..f92f20d 100644 > > --- a/drivers/uio/Kconfig > > +++ b/drivers/uio/Kconfig > > @@ -94,4 +94,13 @@ 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 > > It would be nice to mention a link here where these user space components > are > available. Ok, will do. > > > + 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..b402197 > > --- /dev/null > > +++ b/drivers/uio/uio_pruss.c > > @@ -0,0 +1,231 @@ > > +/* > > + * 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 PRUSS_INTC_HIPIR 0x4900 > > +#define PRUSS_INTC_HIPIR_INTPEND_MASK 0x80000000 > > +#define PRUSS_INTC_HIER 0x5500 > > + > > +struct clk *pruss_clk; > > +struct uio_info *info; > > +void *ddr_virt_addr; > > +dma_addr_t ddr_phy_addr; > > + > > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > > +{ > > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr > > + + PRUSS_INTC_HIER; > > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr > > + + PRUSS_INTC_HIPIR+((irq-1) << 2); > > Blank line between variable definitions and code, please. > Ok. > > + /* Is interrupt enabled and active ? */ > > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) && > > Hmm. I'd prefer blanks around operands, like (1 << (irq - 1))). > I won't be too picky about that, noticing that checkpatch.pl doesn't > complain. > If you want to do me a favor, you can fix it ;-) > Ok. > > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK)) > > + return IRQ_NONE; > > + > > + /* Disable interrupt */ > > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)), > > If you save the return value of the first ioread32(int_enable_reg) in a > variable, > you don't need the second hardware access. > Will do. > > + int_enable_reg); > > + return IRQ_HANDLED; > > +} > > + > > +static void pruss_cleanup(struct platform_device *dev, struct uio_info > *info) > > +{ > > + int count; > > Blank line between variable definitions and code, please. > Ok. > > + if (info) { > > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > > + uio_unregister_device(&info[count]); > > + kfree(info[count].name); > > + iounmap(info[count].mem[0].internal_addr); > > + > > + } > > + if (ddr_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); > > + } > > + if (pruss_clk != NULL) > > + clk_put(pruss_clk); > > +} > > + > > +static int __devinit pruss_probe(struct platform_device *dev) > > +{ > > + int ret = -ENODEV, count = 0; > > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr; > > + char *string; > > + > > + /* 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); > > + } > > + > > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE, > > + GFP_KERNEL); > > + if (info == NULL) > > if (!info) looks better. > > > + return -ENOMEM; > > + > > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0); > > + if (!regs_pruram) { > > + dev_err(&dev->dev, "No memory resource specified\n"); > > + goto out_free; > > + } > > + > > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1); > > + if (!regs_l3ram) { > > + dev_err(&dev->dev, "No memory resource specified\n"); > > + goto out_free; > > + } > > + > > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2); > > + if (!regs_ddr) { > > + dev_err(&dev->dev, "No memory resource specified\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; > > + } > > + > > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > > + info[count].mem[0].addr = regs_pruram->start; > > + info[count].mem[0].size = > > + regs_pruram->end - regs_pruram->start + 1; > > + if (!info[count].mem[0].addr || > > + !(info[count].mem[0].size - 1)) { > > + dev_err(&dev->dev, "Invalid memory resource\n"); > > + goto out_free; > > + } > > + info[count].mem[0].internal_addr = > > + ioremap(regs_pruram->start, info[count].mem[0].size); > > + if (!info[count].mem[0].internal_addr) { > > + dev_err(&dev->dev, > > + "Can't remap memory address range\n"); > > + goto out_free; > > + } > > + info[count].mem[0].memtype = UIO_MEM_PHYS; > > + > > + info[count].mem[1].addr = regs_l3ram->start; > > + info[count].mem[1].size = > > + regs_l3ram->end - regs_l3ram->start + 1; > > + if (!info[count].mem[1].addr || > > + !(info[count].mem[1].size - 1)) { > > + dev_err(&dev->dev, "Invalid memory resource\n"); > > + goto out_free; > > + } > > + info[count].mem[1].memtype = UIO_MEM_PHYS; > > + > > + info[count].mem[2].internal_addr = ddr_virt_addr; > > + info[count].mem[2].addr = ddr_phy_addr; > > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1; > > + info[count].mem[2].memtype = UIO_MEM_PHYS; > > + > > + string = kzalloc(20, GFP_KERNEL); > > + sprintf(string, "pruss_evt%d", count); > > + info[count].name = string; > > + info[count].version = "0.50"; > > + > > + /* Register PRUSS IRQ lines */ > > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count; > > + > > + info[count].irq_flags = 0; > > + info[count].handler = pruss_handler; > > + > > + ret = uio_register_device(&dev->dev, &info[count]); > > + > > + 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); > > Blank line, please. > Ok. Thanks, Pratheesh ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] PRUSS UIO driver support 2011-02-21 17:35 ` [PATCH v2 1/2] " Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 2/2] Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver Pratheesh Gangadhar 2011-02-21 18:44 ` [PATCH v2 1/2] PRUSS UIO driver support Hans J. Koch @ 2011-02-21 18:54 ` Thomas Gleixner 2011-02-22 11:58 ` TK, Pratheesh Gangadhar 2 siblings, 1 reply; 7+ messages in thread From: Thomas Gleixner @ 2011-02-21 18:54 UTC (permalink / raw) To: linux-arm-kernel On Mon, 21 Feb 2011, Pratheesh Gangadhar wrote: > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > +{ > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr > + + PRUSS_INTC_HIER; > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr > + + PRUSS_INTC_HIPIR+((irq-1) << 2); void __iomem *base = dev_info->mem[0].internal_addr; void __iomem *int_enable_reg = base + PRUSS_INTC_HIER; .... Makes that readable. > + /* Is interrupt enabled and active ? */ > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) && > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK)) > + return IRQ_NONE; That returns when the interrupt is disabled _AND_ pending. It should return when the interrupt is disabled _OR_ not pending. > + > + /* Disable interrupt */ > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)), > + int_enable_reg); Chosing shorter variable names avoid those line breaks all over the place. > + return IRQ_HANDLED; > +} > + > +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info) > +{ > + int count; New line between variables and code please > + if (info) { This check is silly. pruss_probe() returns right away when it cannot allocate info. pruss_remove() is never called when info == NULL > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > + uio_unregister_device(&info[count]); > + kfree(info[count].name); > + iounmap(info[count].mem[0].internal_addr); Why do you map/unmap the same physical address 8 times ???? > + } > + if (ddr_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); > + } > + if (pruss_clk != NULL) Silly check as well. > + clk_put(pruss_clk); > +} > + > +static int __devinit pruss_probe(struct platform_device *dev) > +{ > + int ret = -ENODEV, count = 0; > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr; > + char *string; > + > + /* 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); > + } > + > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE, > + GFP_KERNEL); > + if (info == NULL) if (!info) > + return -ENOMEM; Leaves the clock enabled > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0); > + if (!regs_pruram) { > + dev_err(&dev->dev, "No memory resource specified\n"); > + goto out_free; > + } > + > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1); > + if (!regs_l3ram) { > + dev_err(&dev->dev, "No memory resource specified\n"); > + goto out_free; > + } > + > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2); > + if (!regs_ddr) { > + dev_err(&dev->dev, "No memory resource specified\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; > + } > + > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { Sigh. Can't you have a pointer struct uio_info *p and do the following. for (cnt = 0; p = info; cnt < MAX_PRUSS_EVTOUT_INSTANCE; cnt++, p++) { p->mem[0] ... > + info[count].mem[0].addr = regs_pruram->start; > + info[count].mem[0].size = > + regs_pruram->end - regs_pruram->start + 1; > + if (!info[count].mem[0].addr || > + !(info[count].mem[0].size - 1)) { All you catch is size == 0. So with size == 1 it works ??? > + dev_err(&dev->dev, "Invalid memory resource\n"); > + goto out_free; > + } > + info[count].mem[0].internal_addr = > + ioremap(regs_pruram->start, info[count].mem[0].size); That's redundant to remap the same address 8 times. That and the check above should be done before the loop and the result used in the loop. > + if (!info[count].mem[0].internal_addr) { > + dev_err(&dev->dev, > + "Can't remap memory address range\n"); > + goto out_free; > + } > + info[count].mem[0].memtype = UIO_MEM_PHYS; > + > + info[count].mem[1].addr = regs_l3ram->start; > + info[count].mem[1].size = > + regs_l3ram->end - regs_l3ram->start + 1; > + if (!info[count].mem[1].addr || > + !(info[count].mem[1].size - 1)) { > + dev_err(&dev->dev, "Invalid memory resource\n"); > + goto out_free; > + } No need to check the same thing over and over. > + info[count].mem[1].memtype = UIO_MEM_PHYS; > + > + info[count].mem[2].internal_addr = ddr_virt_addr; > + info[count].mem[2].addr = ddr_phy_addr; > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1; > + info[count].mem[2].memtype = UIO_MEM_PHYS; > + > + string = kzalloc(20, GFP_KERNEL); > + sprintf(string, "pruss_evt%d", count); > + info[count].name = string; kasprintf() please > + info[count].version = "0.50"; > + > + /* Register PRUSS IRQ lines */ > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count; > + > + info[count].irq_flags = 0; Is already zero > + info[count].handler = pruss_handler; > + > + ret = uio_register_device(&dev->dev, &info[count]); > + > + 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); Empty line between variables and code. > + pruss_cleanup(dev, info); > + platform_set_drvdata(dev, NULL); > + return 0; Thanks, tglx ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] PRUSS UIO driver support 2011-02-21 18:54 ` Thomas Gleixner @ 2011-02-22 11:58 ` TK, Pratheesh Gangadhar 0 siblings, 0 replies; 7+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-02-22 11:58 UTC (permalink / raw) To: linux-arm-kernel Hi, > -----Original Message----- > From: Thomas Gleixner [mailto:tglx at linutronix.de] > Sent: Tuesday, February 22, 2011 12:24 AM > To: TK, Pratheesh Gangadhar > Cc: davinci-linux-open-source at linux.davincidsp.com; hjk at hansjkoch.de; > gregkh at suse.de; Chatterjee, Amit; linux-kernel at vger.kernel.org; linux-arm- > kernel at lists.infradead.org > Subject: Re: [PATCH v2 1/2] PRUSS UIO driver support > > On Mon, 21 Feb 2011, Pratheesh Gangadhar wrote: > > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > > +{ > > + void __iomem *int_enable_reg = dev_info->mem[0].internal_addr > > + + PRUSS_INTC_HIER; > > + void __iomem *int_status_reg = dev_info->mem[0].internal_addr > > + + PRUSS_INTC_HIPIR+((irq-1) << 2); > > void __iomem *base = dev_info->mem[0].internal_addr; > void __iomem *int_enable_reg = base + PRUSS_INTC_HIER; > .... > > Makes that readable. Ok. > > > + /* Is interrupt enabled and active ? */ > > + if (!(ioread32(int_enable_reg) & (1<<(irq-1))) && > > + (ioread32(int_status_reg) & PRUSS_INTC_HIPIR_INTPEND_MASK)) > > + return IRQ_NONE; > > That returns when the interrupt is disabled _AND_ pending. It should > return when the interrupt is disabled _OR_ not pending. I should have named it _NOPEND_MASK, basically mask is set when there is no pending interrupt. > > + > > + /* Disable interrupt */ > > + iowrite32(ioread32(int_enable_reg) & ~(1<<(irq-1)), > > + int_enable_reg); > > Chosing shorter variable names avoid those line breaks all over the > place. > Ok. > > + return IRQ_HANDLED; > > +} > > + > > +static void pruss_cleanup(struct platform_device *dev, struct uio_info > *info) > > +{ > > + int count; > > New line between variables and code please > Ok. > > + if (info) { > > This check is silly. pruss_probe() returns right away when it cannot > allocate info. pruss_remove() is never called when info == NULL > Ok. > > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > > + uio_unregister_device(&info[count]); > > + kfree(info[count].name); > > + iounmap(info[count].mem[0].internal_addr); > > Why do you map/unmap the same physical address 8 times ???? Will fix this. > > > + } > > + if (ddr_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); > > + } > > + if (pruss_clk != NULL) > > Silly check as well. > Ok. > > + clk_put(pruss_clk); > > +} > > + > > +static int __devinit pruss_probe(struct platform_device *dev) > > +{ > > + int ret = -ENODEV, count = 0; > > + struct resource *regs_pruram, *regs_l3ram, *regs_ddr; > > + char *string; > > + > > + /* 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); > > + } > > + > > + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVTOUT_INSTANCE, > > + GFP_KERNEL); > > + if (info == NULL) > > if (!info) > > > + return -ENOMEM; > > Leaves the clock enabled > Will fix this. > > + regs_pruram = platform_get_resource(dev, IORESOURCE_MEM, 0); > > + if (!regs_pruram) { > > + dev_err(&dev->dev, "No memory resource specified\n"); > > + goto out_free; > > + } > > + > > + regs_l3ram = platform_get_resource(dev, IORESOURCE_MEM, 1); > > + if (!regs_l3ram) { > > + dev_err(&dev->dev, "No memory resource specified\n"); > > + goto out_free; > > + } > > + > > + regs_ddr = platform_get_resource(dev, IORESOURCE_MEM, 2); > > + if (!regs_ddr) { > > + dev_err(&dev->dev, "No memory resource specified\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; > > + } > > + > > + for (count = 0; count < MAX_PRUSS_EVTOUT_INSTANCE; count++) { > > Sigh. Can't you have a pointer struct uio_info *p and do the following. > Ok. > for (cnt = 0; p = info; cnt < MAX_PRUSS_EVTOUT_INSTANCE; cnt++, p++) > { > > p->mem[0] ... > > > + info[count].mem[0].addr = regs_pruram->start; > > + info[count].mem[0].size = > > + regs_pruram->end - regs_pruram->start + 1; > > + if (!info[count].mem[0].addr || > > + !(info[count].mem[0].size - 1)) { > > All you catch is size == 0. So with size == 1 it works ??? I can drop this check as well as normally one won't specify improbable size for mapping memory during configuration. > > > + dev_err(&dev->dev, "Invalid memory resource\n"); > > + goto out_free; > > + } > > + info[count].mem[0].internal_addr = > > + ioremap(regs_pruram->start, info[count].mem[0].size); > > That's redundant to remap the same address 8 times. That and the check > above should be done before the loop and the result used in the loop. > Ok, will do. > > + if (!info[count].mem[0].internal_addr) { > > + dev_err(&dev->dev, > > + "Can't remap memory address range\n"); > > + goto out_free; > > + } > > + info[count].mem[0].memtype = UIO_MEM_PHYS; > > + > > + info[count].mem[1].addr = regs_l3ram->start; > > + info[count].mem[1].size = > > + regs_l3ram->end - regs_l3ram->start + 1; > > + if (!info[count].mem[1].addr || > > + !(info[count].mem[1].size - 1)) { > > + dev_err(&dev->dev, "Invalid memory resource\n"); > > + goto out_free; > > + } > > No need to check the same thing over and over. > > > + info[count].mem[1].memtype = UIO_MEM_PHYS; > > + > > + info[count].mem[2].internal_addr = ddr_virt_addr; > > + info[count].mem[2].addr = ddr_phy_addr; > > + info[count].mem[2].size = regs_ddr->end - regs_ddr->start + 1; > > + info[count].mem[2].memtype = UIO_MEM_PHYS; > > + > > + string = kzalloc(20, GFP_KERNEL); > > + sprintf(string, "pruss_evt%d", count); > > + info[count].name = string; > > kasprintf() please Ok. > > > + info[count].version = "0.50"; > > + > > + /* Register PRUSS IRQ lines */ > > + info[count].irq = IRQ_DA8XX_EVTOUT0 + count; > > + > > + info[count].irq_flags = 0; > > Is already zero > Ok. > > + info[count].handler = pruss_handler; > > + > > + ret = uio_register_device(&dev->dev, &info[count]); > > + > > + 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); > > Empty line between variables and code. > Ok. > > + pruss_cleanup(dev, info); > > + platform_set_drvdata(dev, NULL); > > + return 0; > Thanks, Pratheesh ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-22 12:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-21 17:35 [PATCH v2 0/2] Add PRUSS UIO driver support Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 1/2] " Pratheesh Gangadhar 2011-02-21 17:35 ` [PATCH v2 2/2] Defines DA850/AM18xx/OMAPL1-38 SOC resources used by PRUSS UIO driver Pratheesh Gangadhar 2011-02-21 18:44 ` [PATCH v2 1/2] PRUSS UIO driver support Hans J. Koch 2011-02-22 12:01 ` TK, Pratheesh Gangadhar 2011-02-21 18:54 ` Thomas Gleixner 2011-02-22 11:58 ` TK, Pratheesh Gangadhar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).