* [PATCH v6 0/1] Add PRUSS UIO driver support @ 2011-02-28 21:01 ` Pratheesh Gangadhar 0 siblings, 0 replies; 18+ messages in thread From: Pratheesh Gangadhar @ 2011-02-28 21:01 UTC (permalink / raw) To: linux-kernel Cc: hjk, gregkh, tglx, sshtylyov, arnd, pratheesh, amit.chatterjee, davinci-linux-open-source, linux-arm-kernel This patch series add support for PRUSS (Programmable Real-time Unit Sub System) UIO driver in Texas Instruments DA850, AM18xx and OMAP-L138 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 Platform/board specific portion of this patch depends on Mistral patches below [PATCH 1/1] davinci: changed SRAM allocator to shared ram : https://patchwork.kernel.org/patch/549351/ [PATCH 1/1] da830: macro rename DA8XX_LPSC0_DMAX to DA8XX_LPSC0_PRUSS : https://patchwork.kernel.org/patch/549331/ [PATCH v2 01/13] mfd: pruss mfd driver : https://patchwork.kernel.org/patch/549531/ [PATCH v2 02/13] da850: pruss platform specific additions : https://patchwork.kernel.org/patch/549521/ I will submit a seperate patch set on top of these patches to support PRUSS UIO driver. Pratheesh Gangadhar (1): PRUSS UIO driver support drivers/uio/Kconfig | 17 ++++ drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 0 deletions(-) create mode 100644 drivers/uio/uio_pruss.c ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 0/1] Add PRUSS UIO driver support @ 2011-02-28 21:01 ` Pratheesh Gangadhar 0 siblings, 0 replies; 18+ messages in thread From: Pratheesh Gangadhar @ 2011-02-28 21:01 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 OMAP-L138 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 Platform/board specific portion of this patch depends on Mistral patches below [PATCH 1/1] davinci: changed SRAM allocator to shared ram : https://patchwork.kernel.org/patch/549351/ [PATCH 1/1] da830: macro rename DA8XX_LPSC0_DMAX to DA8XX_LPSC0_PRUSS : https://patchwork.kernel.org/patch/549331/ [PATCH v2 01/13] mfd: pruss mfd driver : https://patchwork.kernel.org/patch/549531/ [PATCH v2 02/13] da850: pruss platform specific additions : https://patchwork.kernel.org/patch/549521/ I will submit a seperate patch set on top of these patches to support PRUSS UIO driver. Pratheesh Gangadhar (1): PRUSS UIO driver support drivers/uio/Kconfig | 17 ++++ drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 0 deletions(-) create mode 100644 drivers/uio/uio_pruss.c ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support 2011-02-28 21:01 ` Pratheesh Gangadhar @ 2011-02-28 21:01 ` Pratheesh Gangadhar -1 siblings, 0 replies; 18+ messages in thread From: Pratheesh Gangadhar @ 2011-02-28 21:01 UTC (permalink / raw) To: linux-kernel Cc: hjk, gregkh, tglx, sshtylyov, arnd, pratheesh, amit.chatterjee, davinci-linux-open-source, 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 handling system events that have tight real time constraints. This driver is currently supported on Texas Instruments DA850, AM18xx and OMAP-L138 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> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/uio/Kconfig | 17 ++++ drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 255 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..e5ced29 --- /dev/null +++ b/drivers/uio/uio_pruss.c @@ -0,0 +1,237 @@ +/* + * 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/moduleparam.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> +#include <linux/spinlock.h> +#include <mach/sram.h> + + +#define DRV_NAME "pruss_uio" +#define DRV_VERSION "0.50" + + +static int sram_pool_sz = SZ_16K; +module_param(sram_pool_sz, int, 0); +MODULE_PARM_DESC(sram_pool_sz, "sram pool size to allocate "); + +static int extram_pool_sz = SZ_256K; +module_param(extram_pool_sz, int, 0); +MODULE_PARM_DESC(extram_pool_sz, "external ram pool size to allocate"); + + +/* + * 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_EVT 8 + +#define PINTC_HIPIR 0x4900 +#define HIPIR_NOPEND 0x80000000 +#define PINTC_HIER 0x5500 + +static spinlock_t lock; +static struct clk *pruss_clk; +static struct uio_info *info; +static dma_addr_t sram_paddr, ddr_paddr; +static void *prussio_vaddr, *sram_vaddr, *ddr_vaddr; + +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) +{ + unsigned long flags; + int val, intr_mask = (1 << (irq - 1)); + 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); + + spin_lock_irqsave(&lock, flags); + val = ioread32(intren_reg); + /* Is interrupt enabled and active ? */ + if (!(val & intr_mask) && (ioread32(intrstat_reg) & HIPIR_NOPEND)) { + spin_unlock_irqrestore(&lock, flags); + return IRQ_NONE; + } + + /* Disable interrupt */ + iowrite32((val & ~intr_mask), intren_reg); + spin_unlock_irqrestore(&lock, flags); + return IRQ_HANDLED; +} + +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info) +{ + struct uio_info *p = info; + int cnt; + + for (cnt = 0; cnt < MAX_PRUSS_EVT; cnt++, p++) { + uio_unregister_device(p); + kfree(p->name); + } + iounmap(prussio_vaddr); + if (ddr_vaddr) { + dma_free_coherent(&dev->dev, extram_pool_sz, ddr_vaddr, + ddr_paddr); + } + if (sram_vaddr) + sram_free(sram_vaddr, sram_pool_sz); + kfree(info); + clk_put(pruss_clk); +} + +static int __devinit pruss_probe(struct platform_device *dev) +{ + struct uio_info *p; + int ret = -ENODEV, cnt = 0, len; + struct resource *regs_prussio; + + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVT, 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"); + kfree(info); + 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; + } + + if (!regs_prussio->start) { + dev_err(&dev->dev, "Invalid memory resource\n"); + goto out_free; + } + + sram_vaddr = sram_alloc(sram_pool_sz, &sram_paddr); + if (!sram_vaddr) { + dev_err(&dev->dev, "Could not allocate SRAM pool\n"); + goto out_free; + } + + ddr_vaddr = dma_alloc_coherent(&dev->dev, extram_pool_sz, &ddr_paddr, + GFP_KERNEL | GFP_DMA); + if (!ddr_vaddr) { + dev_err(&dev->dev, "Could not allocate external memory\n"); + goto out_free; + } + + len = resource_size(regs_prussio); + prussio_vaddr = ioremap(regs_prussio->start, len); + if (!prussio_vaddr) { + dev_err(&dev->dev, "Can't remap PRUSS I/O address range\n"); + goto out_free; + } + + for (cnt = 0, p = info; cnt < MAX_PRUSS_EVT; cnt++, p++) { + p->mem[0].internal_addr = prussio_vaddr; + 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].internal_addr = sram_vaddr; + p->mem[1].addr = sram_paddr; + p->mem[1].size = sram_pool_sz; + p->mem[1].memtype = UIO_MEM_PHYS; + + p->mem[2].internal_addr = ddr_vaddr; + p->mem[2].addr = ddr_paddr; + p->mem[2].size = extram_pool_sz; + p->mem[2].memtype = UIO_MEM_PHYS; + + p->name = kasprintf(GFP_KERNEL, "pruss_evt%d", cnt); + p->version = "0.50"; + + /* Register PRUSS IRQ lines */ + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; + p->handler = pruss_handler; + + ret = uio_register_device(&dev->dev, p); + + if (ret < 0) + goto out_free; + } + + spin_lock_init(&lock); + 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] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-02-28 21:01 ` Pratheesh Gangadhar 0 siblings, 0 replies; 18+ messages in thread From: Pratheesh Gangadhar @ 2011-02-28 21:01 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 handling system events that have tight real time constraints. This driver is currently supported on Texas Instruments DA850, AM18xx and OMAP-L138 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> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/uio/Kconfig | 17 ++++ drivers/uio/Makefile | 1 + drivers/uio/uio_pruss.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 255 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..e5ced29 --- /dev/null +++ b/drivers/uio/uio_pruss.c @@ -0,0 +1,237 @@ +/* + * 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/moduleparam.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> +#include <linux/spinlock.h> +#include <mach/sram.h> + + +#define DRV_NAME "pruss_uio" +#define DRV_VERSION "0.50" + + +static int sram_pool_sz = SZ_16K; +module_param(sram_pool_sz, int, 0); +MODULE_PARM_DESC(sram_pool_sz, "sram pool size to allocate "); + +static int extram_pool_sz = SZ_256K; +module_param(extram_pool_sz, int, 0); +MODULE_PARM_DESC(extram_pool_sz, "external ram pool size to allocate"); + + +/* + * 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_EVT 8 + +#define PINTC_HIPIR 0x4900 +#define HIPIR_NOPEND 0x80000000 +#define PINTC_HIER 0x5500 + +static spinlock_t lock; +static struct clk *pruss_clk; +static struct uio_info *info; +static dma_addr_t sram_paddr, ddr_paddr; +static void *prussio_vaddr, *sram_vaddr, *ddr_vaddr; + +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) +{ + unsigned long flags; + int val, intr_mask = (1 << (irq - 1)); + 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); + + spin_lock_irqsave(&lock, flags); + val = ioread32(intren_reg); + /* Is interrupt enabled and active ? */ + if (!(val & intr_mask) && (ioread32(intrstat_reg) & HIPIR_NOPEND)) { + spin_unlock_irqrestore(&lock, flags); + return IRQ_NONE; + } + + /* Disable interrupt */ + iowrite32((val & ~intr_mask), intren_reg); + spin_unlock_irqrestore(&lock, flags); + return IRQ_HANDLED; +} + +static void pruss_cleanup(struct platform_device *dev, struct uio_info *info) +{ + struct uio_info *p = info; + int cnt; + + for (cnt = 0; cnt < MAX_PRUSS_EVT; cnt++, p++) { + uio_unregister_device(p); + kfree(p->name); + } + iounmap(prussio_vaddr); + if (ddr_vaddr) { + dma_free_coherent(&dev->dev, extram_pool_sz, ddr_vaddr, + ddr_paddr); + } + if (sram_vaddr) + sram_free(sram_vaddr, sram_pool_sz); + kfree(info); + clk_put(pruss_clk); +} + +static int __devinit pruss_probe(struct platform_device *dev) +{ + struct uio_info *p; + int ret = -ENODEV, cnt = 0, len; + struct resource *regs_prussio; + + info = kzalloc(sizeof(struct uio_info) * MAX_PRUSS_EVT, 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"); + kfree(info); + 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; + } + + if (!regs_prussio->start) { + dev_err(&dev->dev, "Invalid memory resource\n"); + goto out_free; + } + + sram_vaddr = sram_alloc(sram_pool_sz, &sram_paddr); + if (!sram_vaddr) { + dev_err(&dev->dev, "Could not allocate SRAM pool\n"); + goto out_free; + } + + ddr_vaddr = dma_alloc_coherent(&dev->dev, extram_pool_sz, &ddr_paddr, + GFP_KERNEL | GFP_DMA); + if (!ddr_vaddr) { + dev_err(&dev->dev, "Could not allocate external memory\n"); + goto out_free; + } + + len = resource_size(regs_prussio); + prussio_vaddr = ioremap(regs_prussio->start, len); + if (!prussio_vaddr) { + dev_err(&dev->dev, "Can't remap PRUSS I/O address range\n"); + goto out_free; + } + + for (cnt = 0, p = info; cnt < MAX_PRUSS_EVT; cnt++, p++) { + p->mem[0].internal_addr = prussio_vaddr; + 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].internal_addr = sram_vaddr; + p->mem[1].addr = sram_paddr; + p->mem[1].size = sram_pool_sz; + p->mem[1].memtype = UIO_MEM_PHYS; + + p->mem[2].internal_addr = ddr_vaddr; + p->mem[2].addr = ddr_paddr; + p->mem[2].size = extram_pool_sz; + p->mem[2].memtype = UIO_MEM_PHYS; + + p->name = kasprintf(GFP_KERNEL, "pruss_evt%d", cnt); + p->version = "0.50"; + + /* Register PRUSS IRQ lines */ + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; + p->handler = pruss_handler; + + ret = uio_register_device(&dev->dev, p); + + if (ret < 0) + goto out_free; + } + + spin_lock_init(&lock); + 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] 18+ messages in thread
* Re: [PATCH v6 1/1] PRUSS UIO driver support 2011-02-28 21:01 ` Pratheesh Gangadhar @ 2011-02-28 21:26 ` Hans J. Koch -1 siblings, 0 replies; 18+ messages in thread From: Hans J. Koch @ 2011-02-28 21:26 UTC (permalink / raw) To: Pratheesh Gangadhar Cc: linux-kernel, hjk, gregkh, tglx, sshtylyov, arnd, amit.chatterjee, davinci-linux-open-source, linux-arm-kernel On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > + > + /* Register PRUSS IRQ lines */ > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > + p->handler = pruss_handler; > + > + ret = uio_register_device(&dev->dev, p); > + > + if (ret < 0) > + goto out_free; > + } > + > + spin_lock_init(&lock); That's too late. uio_register_device() enables the irq, and your spin_lock is not ready at that time. > + platform_set_drvdata(dev, info); > + return 0; > + > +out_free: > + pruss_cleanup(dev, info); > + return ret; > +} Thanks, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-02-28 21:26 ` Hans J. Koch 0 siblings, 0 replies; 18+ messages in thread From: Hans J. Koch @ 2011-02-28 21:26 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > + > + /* Register PRUSS IRQ lines */ > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > + p->handler = pruss_handler; > + > + ret = uio_register_device(&dev->dev, p); > + > + if (ret < 0) > + goto out_free; > + } > + > + spin_lock_init(&lock); That's too late. uio_register_device() enables the irq, and your spin_lock is not ready at that time. > + platform_set_drvdata(dev, info); > + return 0; > + > +out_free: > + pruss_cleanup(dev, info); > + return ret; > +} Thanks, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v6 1/1] PRUSS UIO driver support 2011-02-28 21:26 ` Hans J. Koch @ 2011-03-01 4:45 ` TK, Pratheesh Gangadhar -1 siblings, 0 replies; 18+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-03-01 4:45 UTC (permalink / raw) To: Hans J. Koch Cc: linux-kernel, gregkh, tglx, sshtylyov, arnd, Chatterjee, Amit, davinci-linux-open-source, linux-arm-kernel Hi, > -----Original Message----- > From: Hans J. Koch [mailto:hjk@hansjkoch.de] > Sent: Tuesday, March 01, 2011 2:57 AM > To: TK, Pratheesh Gangadhar > Cc: linux-kernel@vger.kernel.org; hjk@hansjkoch.de; gregkh@suse.de; > tglx@linutronix.de; sshtylyov@mvista.com; arnd@arndb.de; Chatterjee, Amit; > davinci-linux-open-source@linux.davincidsp.com; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > + > > + /* Register PRUSS IRQ lines */ > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > + p->handler = pruss_handler; > > + > > + ret = uio_register_device(&dev->dev, p); > > + > > + if (ret < 0) > > + goto out_free; > > + } > > + > > + spin_lock_init(&lock); > > That's too late. uio_register_device() enables the irq, and your spin_lock > is not ready at that time. This is ok in this context as "modprobe uio_pruss" is pre-requisite for running PRUSS firmware and without firmware running PRUSS won't generate interrupts. Actually PRUSS INTC is not setup till we start user application. Thanks, Pratheesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 4:45 ` TK, Pratheesh Gangadhar 0 siblings, 0 replies; 18+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-03-01 4:45 UTC (permalink / raw) To: linux-arm-kernel Hi, > -----Original Message----- > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > Sent: Tuesday, March 01, 2011 2:57 AM > To: TK, Pratheesh Gangadhar > Cc: linux-kernel at vger.kernel.org; hjk at hansjkoch.de; gregkh at suse.de; > tglx at linutronix.de; sshtylyov at mvista.com; arnd at arndb.de; Chatterjee, Amit; > davinci-linux-open-source at linux.davincidsp.com; linux-arm- > kernel at lists.infradead.org > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > + > > + /* Register PRUSS IRQ lines */ > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > + p->handler = pruss_handler; > > + > > + ret = uio_register_device(&dev->dev, p); > > + > > + if (ret < 0) > > + goto out_free; > > + } > > + > > + spin_lock_init(&lock); > > That's too late. uio_register_device() enables the irq, and your spin_lock > is not ready at that time. This is ok in this context as "modprobe uio_pruss" is pre-requisite for running PRUSS firmware and without firmware running PRUSS won't generate interrupts. Actually PRUSS INTC is not setup till we start user application. Thanks, Pratheesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v6 1/1] PRUSS UIO driver support 2011-03-01 4:45 ` TK, Pratheesh Gangadhar @ 2011-03-01 9:53 ` Thomas Gleixner -1 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 9:53 UTC (permalink / raw) To: TK, Pratheesh Gangadhar Cc: Hans J. Koch, linux-kernel, gregkh, sshtylyov, arnd, Chatterjee, Amit, davinci-linux-open-source, linux-arm-kernel On Tue, 1 Mar 2011, TK, Pratheesh Gangadhar wrote: > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > + > > > + /* Register PRUSS IRQ lines */ > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > + p->handler = pruss_handler; > > > + > > > + ret = uio_register_device(&dev->dev, p); > > > + > > > + if (ret < 0) > > > + goto out_free; > > > + } > > > + > > > + spin_lock_init(&lock); > > > > That's too late. uio_register_device() enables the irq, and your spin_lock > > is not ready at that time. > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > running PRUSS firmware and without firmware running PRUSS won't > generate interrupts. Actually PRUSS INTC is not setup till we start > user application. No, it's not. If you enable interrupts you have to be prepared for getting one whether the device mask is enabled or not. It's that simple. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 9:53 ` Thomas Gleixner 0 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 9:53 UTC (permalink / raw) To: linux-arm-kernel On Tue, 1 Mar 2011, TK, Pratheesh Gangadhar wrote: > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > + > > > + /* Register PRUSS IRQ lines */ > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > + p->handler = pruss_handler; > > > + > > > + ret = uio_register_device(&dev->dev, p); > > > + > > > + if (ret < 0) > > > + goto out_free; > > > + } > > > + > > > + spin_lock_init(&lock); > > > > That's too late. uio_register_device() enables the irq, and your spin_lock > > is not ready at that time. > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > running PRUSS firmware and without firmware running PRUSS won't > generate interrupts. Actually PRUSS INTC is not setup till we start > user application. No, it's not. If you enable interrupts you have to be prepared for getting one whether the device mask is enabled or not. It's that simple. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v6 1/1] PRUSS UIO driver support 2011-03-01 4:45 ` TK, Pratheesh Gangadhar @ 2011-03-01 18:33 ` Hans J. Koch -1 siblings, 0 replies; 18+ messages in thread From: Hans J. Koch @ 2011-03-01 18:33 UTC (permalink / raw) To: TK, Pratheesh Gangadhar Cc: Hans J. Koch, linux-kernel, gregkh, tglx, sshtylyov, arnd, Chatterjee, Amit, davinci-linux-open-source, linux-arm-kernel On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > Hi, > > > -----Original Message----- > > From: Hans J. Koch [mailto:hjk@hansjkoch.de] > > Sent: Tuesday, March 01, 2011 2:57 AM > > To: TK, Pratheesh Gangadhar > > Cc: linux-kernel@vger.kernel.org; hjk@hansjkoch.de; gregkh@suse.de; > > tglx@linutronix.de; sshtylyov@mvista.com; arnd@arndb.de; Chatterjee, Amit; > > davinci-linux-open-source@linux.davincidsp.com; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > + > > > + /* Register PRUSS IRQ lines */ > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > + p->handler = pruss_handler; > > > + > > > + ret = uio_register_device(&dev->dev, p); > > > + > > > + if (ret < 0) > > > + goto out_free; > > > + } > > > + > > > + spin_lock_init(&lock); > > > > That's too late. uio_register_device() enables the irq, and your spin_lock > > is not ready at that time. > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > running PRUSS firmware and without firmware running PRUSS won't > generate interrupts. Actually PRUSS INTC is not setup till we start > user application. What if the user application is stopped, UIO driver module unloaded and loaded again? Anyway, please don't use that kind of argumentation. The next newbie developer might copy your work as a basis for his new driver, and there it probably won't work. Simply put the spin_lock_init before the loop. Thanks, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 18:33 ` Hans J. Koch 0 siblings, 0 replies; 18+ messages in thread From: Hans J. Koch @ 2011-03-01 18:33 UTC (permalink / raw) To: linux-arm-kernel On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > Hi, > > > -----Original Message----- > > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > > Sent: Tuesday, March 01, 2011 2:57 AM > > To: TK, Pratheesh Gangadhar > > Cc: linux-kernel at vger.kernel.org; hjk at hansjkoch.de; gregkh at suse.de; > > tglx at linutronix.de; sshtylyov at mvista.com; arnd at arndb.de; Chatterjee, Amit; > > davinci-linux-open-source at linux.davincidsp.com; linux-arm- > > kernel at lists.infradead.org > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > + > > > + /* Register PRUSS IRQ lines */ > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > + p->handler = pruss_handler; > > > + > > > + ret = uio_register_device(&dev->dev, p); > > > + > > > + if (ret < 0) > > > + goto out_free; > > > + } > > > + > > > + spin_lock_init(&lock); > > > > That's too late. uio_register_device() enables the irq, and your spin_lock > > is not ready at that time. > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > running PRUSS firmware and without firmware running PRUSS won't > generate interrupts. Actually PRUSS INTC is not setup till we start > user application. What if the user application is stopped, UIO driver module unloaded and loaded again? Anyway, please don't use that kind of argumentation. The next newbie developer might copy your work as a basis for his new driver, and there it probably won't work. Simply put the spin_lock_init before the loop. Thanks, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v6 1/1] PRUSS UIO driver support 2011-03-01 18:33 ` Hans J. Koch @ 2011-03-01 21:19 ` TK, Pratheesh Gangadhar -1 siblings, 0 replies; 18+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-03-01 21:19 UTC (permalink / raw) To: Hans J. Koch Cc: linux-kernel, gregkh, tglx, sshtylyov, arnd, Chatterjee, Amit, davinci-linux-open-source, linux-arm-kernel Hi, > -----Original Message----- > From: Hans J. Koch [mailto:hjk@hansjkoch.de] > Sent: Wednesday, March 02, 2011 12:04 AM > To: TK, Pratheesh Gangadhar > Cc: Hans J. Koch; linux-kernel@vger.kernel.org; gregkh@suse.de; > tglx@linutronix.de; sshtylyov@mvista.com; arnd@arndb.de; Chatterjee, Amit; > davinci-linux-open-source@linux.davincidsp.com; linux-arm- > kernel@lists.infradead.org > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > > Hi, > > > > > -----Original Message----- > > > From: Hans J. Koch [mailto:hjk@hansjkoch.de] > > > Sent: Tuesday, March 01, 2011 2:57 AM > > > To: TK, Pratheesh Gangadhar > > > Cc: linux-kernel@vger.kernel.org; hjk@hansjkoch.de; gregkh@suse.de; > > > tglx@linutronix.de; sshtylyov@mvista.com; arnd@arndb.de; Chatterjee, > Amit; > > > davinci-linux-open-source@linux.davincidsp.com; linux-arm- > > > kernel@lists.infradead.org > > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > > > > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > > + > > > > + /* Register PRUSS IRQ lines */ > > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > > + p->handler = pruss_handler; > > > > + > > > > + ret = uio_register_device(&dev->dev, p); > > > > + > > > > + if (ret < 0) > > > > + goto out_free; > > > > + } > > > > + > > > > + spin_lock_init(&lock); > > > > > > That's too late. uio_register_device() enables the irq, and your > spin_lock > > > is not ready at that time. > > > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > > running PRUSS firmware and without firmware running PRUSS won't > > generate interrupts. Actually PRUSS INTC is not setup till we start > > user application. > > What if the user application is stopped, UIO driver module unloaded > and loaded again? > This is a possible scenario - may be a buggy application. Normally when application is stopped, PRUs are stopped by exit handler. > Anyway, please don't use that kind of argumentation. The next newbie > developer might copy your work as a basis for his new driver, and there > it probably won't work. > > Simply put the spin_lock_init before the loop. > Agree, will fix this in next version. Thanks, Pratheesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 21:19 ` TK, Pratheesh Gangadhar 0 siblings, 0 replies; 18+ messages in thread From: TK, Pratheesh Gangadhar @ 2011-03-01 21:19 UTC (permalink / raw) To: linux-arm-kernel Hi, > -----Original Message----- > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > Sent: Wednesday, March 02, 2011 12:04 AM > To: TK, Pratheesh Gangadhar > Cc: Hans J. Koch; linux-kernel at vger.kernel.org; gregkh at suse.de; > tglx at linutronix.de; sshtylyov at mvista.com; arnd at arndb.de; Chatterjee, Amit; > davinci-linux-open-source at linux.davincidsp.com; linux-arm- > kernel at lists.infradead.org > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > > Hi, > > > > > -----Original Message----- > > > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > > > Sent: Tuesday, March 01, 2011 2:57 AM > > > To: TK, Pratheesh Gangadhar > > > Cc: linux-kernel at vger.kernel.org; hjk at hansjkoch.de; gregkh at suse.de; > > > tglx at linutronix.de; sshtylyov at mvista.com; arnd at arndb.de; Chatterjee, > Amit; > > > davinci-linux-open-source at linux.davincidsp.com; linux-arm- > > > kernel at lists.infradead.org > > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support > > > > > > On Tue, Mar 01, 2011 at 02:31:35AM +0530, Pratheesh Gangadhar wrote: > > > > + > > > > + /* Register PRUSS IRQ lines */ > > > > + p->irq = IRQ_DA8XX_EVTOUT0 + cnt; > > > > + p->handler = pruss_handler; > > > > + > > > > + ret = uio_register_device(&dev->dev, p); > > > > + > > > > + if (ret < 0) > > > > + goto out_free; > > > > + } > > > > + > > > > + spin_lock_init(&lock); > > > > > > That's too late. uio_register_device() enables the irq, and your > spin_lock > > > is not ready at that time. > > > > This is ok in this context as "modprobe uio_pruss" is pre-requisite for > > running PRUSS firmware and without firmware running PRUSS won't > > generate interrupts. Actually PRUSS INTC is not setup till we start > > user application. > > What if the user application is stopped, UIO driver module unloaded > and loaded again? > This is a possible scenario - may be a buggy application. Normally when application is stopped, PRUs are stopped by exit handler. > Anyway, please don't use that kind of argumentation. The next newbie > developer might copy your work as a basis for his new driver, and there > it probably won't work. > > Simply put the spin_lock_init before the loop. > Agree, will fix this in next version. Thanks, Pratheesh ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v6 1/1] PRUSS UIO driver support 2011-03-01 21:19 ` TK, Pratheesh Gangadhar @ 2011-03-01 21:38 ` Thomas Gleixner -1 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 21:38 UTC (permalink / raw) To: TK, Pratheesh Gangadhar Cc: Hans J. Koch, linux-kernel, gregkh, sshtylyov, arnd, Chatterjee, Amit, davinci-linux-open-source, linux-arm-kernel On Wed, 2 Mar 2011, TK, Pratheesh Gangadhar wrote: > Hi, > > -----Original Message----- > > From: Hans J. Koch [mailto:hjk@hansjkoch.de] > > Sent: Wednesday, March 02, 2011 12:04 AM > > To: TK, Pratheesh Gangadhar > > Cc: Hans J. Koch; linux-kernel@vger.kernel.org; gregkh@suse.de; > > tglx@linutronix.de; sshtylyov@mvista.com; arnd@arndb.de; Chatterjee, Amit; > > davinci-linux-open-source@linux.davincidsp.com; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support Sigh, can you please use a mailer which does not repeat the headers for no value and just has a single line like this: > > On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > > Anyway, please don't use that kind of argumentation. The next newbie > > developer might copy your work as a basis for his new driver, and there > > it probably won't work. > > > > Simply put the spin_lock_init before the loop. > > > Agree, will fix this in next version. As I said before, we want stuff initialized when it is possibly used. But first of all we ant people to use the proper mechanisms to achive that. If that's a module global lock then it needs to be instantiated by static DEFINE_SPINLOCK(lock); which implies the initialization of the lock. If it's a lock which is in allocated memory then the spin_lock_init(&lock); wants to be before it can be possibly used. So in your case DEFINE_SPINLOCK is the correct solution. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 21:38 ` Thomas Gleixner 0 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 21:38 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2 Mar 2011, TK, Pratheesh Gangadhar wrote: > Hi, > > -----Original Message----- > > From: Hans J. Koch [mailto:hjk at hansjkoch.de] > > Sent: Wednesday, March 02, 2011 12:04 AM > > To: TK, Pratheesh Gangadhar > > Cc: Hans J. Koch; linux-kernel at vger.kernel.org; gregkh at suse.de; > > tglx at linutronix.de; sshtylyov at mvista.com; arnd at arndb.de; Chatterjee, Amit; > > davinci-linux-open-source at linux.davincidsp.com; linux-arm- > > kernel at lists.infradead.org > > Subject: Re: [PATCH v6 1/1] PRUSS UIO driver support Sigh, can you please use a mailer which does not repeat the headers for no value and just has a single line like this: > > On Tue, Mar 01, 2011 at 10:15:27AM +0530, TK, Pratheesh Gangadhar wrote: > > Anyway, please don't use that kind of argumentation. The next newbie > > developer might copy your work as a basis for his new driver, and there > > it probably won't work. > > > > Simply put the spin_lock_init before the loop. > > > Agree, will fix this in next version. As I said before, we want stuff initialized when it is possibly used. But first of all we ant people to use the proper mechanisms to achive that. If that's a module global lock then it needs to be instantiated by static DEFINE_SPINLOCK(lock); which implies the initialization of the lock. If it's a lock which is in allocated memory then the spin_lock_init(&lock); wants to be before it can be possibly used. So in your case DEFINE_SPINLOCK is the correct solution. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v6 1/1] PRUSS UIO driver support 2011-02-28 21:01 ` Pratheesh Gangadhar @ 2011-03-01 9:51 ` Thomas Gleixner -1 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 9:51 UTC (permalink / raw) To: Pratheesh Gangadhar Cc: linux-kernel, hjk, gregkh, sshtylyov, arnd, amit.chatterjee, davinci-linux-open-source, linux-arm-kernel On Tue, 1 Mar 2011, Pratheesh Gangadhar wrote: > + > +static spinlock_t lock; static DEFINE_SPINLOCK(lock); > +static struct clk *pruss_clk; > +static struct uio_info *info; > +static dma_addr_t sram_paddr, ddr_paddr; > +static void *prussio_vaddr, *sram_vaddr, *ddr_vaddr; > + > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > +{ > + unsigned long flags; > + int val, intr_mask = (1 << (irq - 1)); > + 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); > + > + spin_lock_irqsave(&lock, flags); spin_lock() is enough as we run handlers with interrupts disabled. > + val = ioread32(intren_reg); > + /* Is interrupt enabled and active ? */ > + if (!(val & intr_mask) && (ioread32(intrstat_reg) & HIPIR_NOPEND)) { > + spin_unlock_irqrestore(&lock, flags); > + return IRQ_NONE; > + } > + > + /* Disable interrupt */ > + iowrite32((val & ~intr_mask), intren_reg); > + spin_unlock_irqrestore(&lock, flags); > + return IRQ_HANDLED; > +} So now you still have not solved the problem of user space enabling an interrupt again. That's racy as well and you can solve it by providing an uio->irq_control function which handles the interrupt enable register under the lock as well. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v6 1/1] PRUSS UIO driver support @ 2011-03-01 9:51 ` Thomas Gleixner 0 siblings, 0 replies; 18+ messages in thread From: Thomas Gleixner @ 2011-03-01 9:51 UTC (permalink / raw) To: linux-arm-kernel On Tue, 1 Mar 2011, Pratheesh Gangadhar wrote: > + > +static spinlock_t lock; static DEFINE_SPINLOCK(lock); > +static struct clk *pruss_clk; > +static struct uio_info *info; > +static dma_addr_t sram_paddr, ddr_paddr; > +static void *prussio_vaddr, *sram_vaddr, *ddr_vaddr; > + > +static irqreturn_t pruss_handler(int irq, struct uio_info *dev_info) > +{ > + unsigned long flags; > + int val, intr_mask = (1 << (irq - 1)); > + 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); > + > + spin_lock_irqsave(&lock, flags); spin_lock() is enough as we run handlers with interrupts disabled. > + val = ioread32(intren_reg); > + /* Is interrupt enabled and active ? */ > + if (!(val & intr_mask) && (ioread32(intrstat_reg) & HIPIR_NOPEND)) { > + spin_unlock_irqrestore(&lock, flags); > + return IRQ_NONE; > + } > + > + /* Disable interrupt */ > + iowrite32((val & ~intr_mask), intren_reg); > + spin_unlock_irqrestore(&lock, flags); > + return IRQ_HANDLED; > +} So now you still have not solved the problem of user space enabling an interrupt again. That's racy as well and you can solve it by providing an uio->irq_control function which handles the interrupt enable register under the lock as well. Thanks, tglx ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2011-03-01 21:38 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-28 21:01 [PATCH v6 0/1] Add PRUSS UIO driver support Pratheesh Gangadhar 2011-02-28 21:01 ` Pratheesh Gangadhar 2011-02-28 21:01 ` [PATCH v6 1/1] " Pratheesh Gangadhar 2011-02-28 21:01 ` Pratheesh Gangadhar 2011-02-28 21:26 ` Hans J. Koch 2011-02-28 21:26 ` Hans J. Koch 2011-03-01 4:45 ` TK, Pratheesh Gangadhar 2011-03-01 4:45 ` TK, Pratheesh Gangadhar 2011-03-01 9:53 ` Thomas Gleixner 2011-03-01 9:53 ` Thomas Gleixner 2011-03-01 18:33 ` Hans J. Koch 2011-03-01 18:33 ` Hans J. Koch 2011-03-01 21:19 ` TK, Pratheesh Gangadhar 2011-03-01 21:19 ` TK, Pratheesh Gangadhar 2011-03-01 21:38 ` Thomas Gleixner 2011-03-01 21:38 ` Thomas Gleixner 2011-03-01 9:51 ` Thomas Gleixner 2011-03-01 9:51 ` Thomas Gleixner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.