2011-03-08 13:41:54

by Subhasish Ghosh

[permalink] [raw]
Subject: [PATCH v3 1/7] mfd: add pruss mfd driver.

This patch adds the pruss MFD driver and associated include files.
For details regarding the PRUSS please refer the folowing link:
http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem

The rational behind the MFD driver being the fact that multiple devices can
be implemented on the cores independently. This is determined by the nature
of the program which is loaded into the PRU's instruction memory.
A device may be de-initialized and another loaded or two different devices
can be run simultaneously on the two cores.
It's also possible, as in our case, to implement a single device on both
the PRU's resulting in improved load sharing.

Signed-off-by: Subhasish Ghosh <[email protected]>
---
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/da8xx_pru.c | 560 +++++++++++++++++++++++++++++++
include/linux/mfd/da8xx/da8xx_pru.h | 135 ++++++++
include/linux/mfd/da8xx/da8xx_prucore.h | 129 +++++++
5 files changed, 835 insertions(+), 0 deletions(-)
create mode 100644 drivers/mfd/da8xx_pru.c
create mode 100644 include/linux/mfd/da8xx/da8xx_pru.h
create mode 100644 include/linux/mfd/da8xx/da8xx_prucore.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index fd01836..6c437df 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -81,6 +81,16 @@ config MFD_DM355EVM_MSP
boards. MSP430 firmware manages resets and power sequencing,
inputs from buttons and the IR remote, LEDs, an RTC, and more.

+config MFD_DA8XX_PRUSS
+ tristate "Texas Instruments DA8XX PRUSS support"
+ depends on ARCH_DAVINCI && ARCH_DAVINCI_DA850
+ select MFD_CORE
+ help
+ This driver provides support api's for the programmable
+ realtime unit (PRU) present on TI's da8xx processors. It
+ provides basic read, write, config, enable, disable
+ routines to facilitate devices emulated on it.
+
config HTC_EGPIO
bool "HTC EGPIO support"
depends on GENERIC_HARDIRQS && GPIOLIB && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a54e2c7..670d6b0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o
obj-$(CONFIG_HTC_I2CPLD) += htc-i2cpld.o

obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o
+obj-$(CONFIG_MFD_DA8XX_PRUSS) += da8xx_pru.o
obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o

obj-$(CONFIG_MFD_STMPE) += stmpe.o
diff --git a/drivers/mfd/da8xx_pru.c b/drivers/mfd/da8xx_pru.c
new file mode 100644
index 0000000..0fd9bb2
--- /dev/null
+++ b/drivers/mfd/da8xx_pru.c
@@ -0,0 +1,560 @@
+/*
+ * Copyright (C) 2011 Texas Instruments Incorporated
+ *
+ * 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/init.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/errno.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/mfd/da8xx/da8xx_prucore.h>
+#include <linux/mfd/da8xx/da8xx_pru.h>
+#include <linux/mfd/core.h>
+#include <linux/io.h>
+#include <mach/da8xx.h>
+
+struct da8xx_pruss {
+ struct device *dev;
+ spinlock_t lock;
+ struct resource *res;
+ struct clk *clk;
+ u32 clk_freq;
+ void __iomem *ioaddr;
+};
+
+u32 pruss_get_clk_freq(struct device *dev)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+
+ return pruss->clk_freq;
+}
+EXPORT_SYMBOL(pruss_get_clk_freq);
+
+s32 pruss_disable(struct device *dev, u8 pruss_num)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ struct da8xx_prusscore_regs *h_pruss;
+ struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
+ u32 temp_reg;
+ u32 delay_cnt;
+
+ if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
+ return -EINVAL;
+
+ spin_lock(&pruss->lock);
+ if (pruss_num == DA8XX_PRUCORE_0) {
+ /* pruss deinit */
+ __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT0 & 0xFFFF));
+ /* Disable PRU0 */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_0];
+
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+
+ for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
+
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
+ DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+ }
+
+ /* Reset PRU0 */
+ for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
+ __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
+ &h_pruss->CONTROL);
+
+ } else if (pruss_num == DA8XX_PRUCORE_1) {
+ /* pruss deinit */
+ __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT1 & 0xFFFF));
+ /* Disable PRU1 */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_1];
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+
+ for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
+
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
+ DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+ }
+
+ /* Reset PRU1 */
+ for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
+ __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
+ &h_pruss->CONTROL);
+ }
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_disable);
+
+s32 pruss_enable(struct device *dev, u8 pruss_num)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ struct da8xx_prusscore_regs *h_pruss;
+ struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
+
+ if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
+ return -EINVAL;
+
+ spin_lock(&pruss->lock);
+ if (pruss_num == DA8XX_PRUCORE_0) {
+ /* Reset PRU0 */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_0];
+ __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
+ &h_pruss->CONTROL);
+ } else if (pruss_num == DA8XX_PRUCORE_1) {
+ /* Reset PRU1 */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_1];
+ __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
+ &h_pruss->CONTROL);
+ }
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_enable);
+
+/* Load the specified PRU with code */
+s32 pruss_load(struct device *dev, u8 pruss_num,
+ u32 *pruss_code, u32 code_size_in_words)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
+ u32 *pruss_iram;
+ u32 i;
+
+ if (pruss_num == DA8XX_PRUCORE_0)
+ pruss_iram = (u32 *)&pruss_mmap->iram0;
+ else if (pruss_num == DA8XX_PRUCORE_1)
+ pruss_iram = (u32 *)&pruss_mmap->iram1;
+ else
+ return -EINVAL;
+
+ pruss_enable(dev, pruss_num);
+
+ spin_lock(&pruss->lock);
+ /* Copy dMAX code to its instruction RAM */
+ for (i = 0; i < code_size_in_words; i++)
+ __raw_writel(pruss_code[i], (pruss_iram + i));
+
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_load);
+
+s32 pruss_run(struct device *dev, u8 pruss_num)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ struct da8xx_prusscore_regs *h_pruss;
+ struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
+ u32 temp_reg;
+
+ if (pruss_num == DA8XX_PRUCORE_0) {
+ /* DA8XX_PRUCORE_0_REGS; */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_0];
+ } else if (pruss_num == DA8XX_PRUCORE_1) {
+ /* DA8XX_PRUCORE_1_REGS; */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_1];
+ } else
+ return -EINVAL;
+
+ /* Enable dMAX, let it execute the code we just copied */
+ spin_lock(&pruss->lock);
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_ENABLE <<
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ temp_reg = (temp_reg &
+ ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
+ ((DA8XX_PRUCORE_CONTROL_ENABLE_ENABLE <<
+ DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
+ DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
+ __raw_writel(temp_reg, &h_pruss->CONTROL);
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_run);
+
+s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ struct da8xx_prusscore_regs *h_pruss;
+ struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
+ u32 temp_reg;
+ u32 cnt = timeout;
+
+ if (pruss_num == DA8XX_PRUCORE_0) {
+ /* DA8XX_PRUCORE_0_REGS; */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_0];
+ } else if (pruss_num == DA8XX_PRUCORE_1) {
+ /* DA8XX_PRUCORE_1_REGS; */
+ h_pruss = (struct da8xx_prusscore_regs *)
+ &pruss_mmap->core[DA8XX_PRUCORE_1];
+ } else
+ return -EINVAL;
+
+ while (cnt--) {
+ temp_reg = __raw_readl(&h_pruss->CONTROL);
+ if (((temp_reg & DA8XX_PRUCORE_CONTROL_RUNSTATE_MASK) >>
+ DA8XX_PRUCORE_CONTROL_RUNSTATE_SHIFT) ==
+ DA8XX_PRUCORE_CONTROL_RUNSTATE_HALT)
+ break;
+ }
+ if (cnt == 0)
+ return -EBUSY;
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_wait_for_halt);
+
+s32 pruss_writeb(struct device *dev, u32 offset,
+ u8 *pdatatowrite, u16 bytestowrite)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u8 *paddresstowrite;
+ u16 loop;
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstowrite = (u8 *) (offset);
+
+ for (loop = 0; loop < bytestowrite; loop++)
+ __raw_writeb(*pdatatowrite++, paddresstowrite++);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_writeb);
+
+s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddress;
+ u32 preg_data;
+
+ paddress = (u32 *)((u32)pruss->ioaddr + offset);
+
+ spin_lock(&pruss->lock);
+ preg_data = __raw_readb(paddress);
+ preg_data &= ~mask;
+ preg_data |= val;
+ __raw_writeb(preg_data, paddress);
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_rmwb);
+
+s32 pruss_readb(struct device *dev, u32 offset,
+ u8 *pdatatoread, u16 bytestoread)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u8 *paddresstoread;
+ u16 loop;
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstoread = (u8 *) (offset);
+
+ for (loop = 0; loop < bytestoread; loop++)
+ *pdatatoread++ = __raw_readb(paddresstoread++);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_readb);
+
+s32 pruss_writel(struct device *dev, u32 offset,
+ u32 *pdatatowrite, s16 wordstowrite)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddresstowrite;
+ s16 loop;
+
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstowrite = (u32 *)(offset);
+
+ for (loop = 0; loop < wordstowrite; loop++)
+ __raw_writel(*pdatatowrite++, paddresstowrite++);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_writel);
+
+s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddress;
+ u32 preg_data;
+
+ paddress = (u32 *)((u32)pruss->ioaddr + offset);
+
+ spin_lock(&pruss->lock);
+ preg_data = __raw_readl(paddress);
+ preg_data &= ~mask;
+ preg_data |= val;
+ __raw_writel(preg_data, paddress);
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_rmwl);
+
+s32 pruss_readl(struct device *dev, u32 offset,
+ u32 *pdatatoread, s16 wordstoread)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddresstoread;
+ s16 loop;
+
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstoread = (u32 *)(offset);
+
+ for (loop = 0; loop < wordstoread; loop++)
+ *pdatatoread++ = __raw_readl(paddresstoread++);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_readl);
+
+s32 pruss_writew(struct device *dev, u32 offset,
+ u16 *pdatatowrite, s16 wordstowrite)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddresstowrite;
+ s16 loop;
+
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstowrite = (u32 *)(offset);
+
+ for (loop = 0; loop < wordstowrite; loop++)
+ __raw_writew(*(pdatatowrite++), (paddresstowrite++));
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_writew);
+
+s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddress;
+ u32 preg_data;
+
+ paddress = (u32 *)((u32)pruss->ioaddr + offset);
+
+ spin_lock(&pruss->lock);
+ preg_data = __raw_readw(paddress);
+ preg_data &= ~mask;
+ preg_data |= val;
+ __raw_writew(preg_data, paddress);
+ spin_unlock(&pruss->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_rmww);
+
+s32 pruss_readw(struct device *dev, u32 offset,
+ u16 *pdatatoread, s16 wordstoread)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddresstoread;
+ s16 loop;
+
+ offset = (u32)pruss->ioaddr + offset;
+ paddresstoread = (u32 *)(offset);
+
+ for (loop = 0; loop < wordstoread; loop++)
+ *pdatatoread++ = __raw_readw(paddresstoread++);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_readw);
+
+s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value)
+{
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
+ u32 *paddresstowrite;
+
+ paddresstowrite = (u32 *)(pruss->ioaddr + offset);
+ __raw_writel(value, paddresstowrite);
+
+ return 0;
+}
+EXPORT_SYMBOL(pruss_idx_writel);
+
+static int pruss_mfd_add_devices(struct platform_device *pdev)
+{
+ struct da8xx_pruss_devices *dev_data = pdev->dev.platform_data;
+ struct device *dev = &pdev->dev;
+ struct mfd_cell cell;
+ u32 err, count;
+
+ for (count = 0; dev_data[count].dev_name != NULL; count++) {
+ memset(&cell, 0, sizeof(struct mfd_cell));
+ cell.id = count;
+ cell.name = dev_data[count].dev_name;
+ cell.platform_data = dev_data[count].pdata;
+ cell.data_size = dev_data[count].pdata_size;
+ cell.resources = dev_data[count].resources;
+ cell.num_resources = dev_data[count].num_resources;
+
+ err = mfd_add_devices(dev, 0, &cell, 1, NULL, 0);
+ if (err) {
+ dev_err(dev, "cannot add mfd cells\n");
+ return err;
+ }
+ dev_info(dev, "mfd: added %s device\n",
+ dev_data[count].dev_name);
+ }
+
+ return err;
+}
+
+static int __devinit da8xx_pruss_probe(struct platform_device *pdev)
+{
+ struct da8xx_pruss *pruss_dev = NULL;
+ u32 err;
+
+ pruss_dev = kzalloc(sizeof(struct da8xx_pruss), GFP_KERNEL);
+ if (!pruss_dev)
+ return -ENOMEM;
+
+ pruss_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!pruss_dev->res) {
+ dev_err(&pdev->dev,
+ "unable to get pruss memory resources!\n");
+ err = -ENODEV;
+ goto probe_exit_kfree;
+ }
+
+ if (!request_mem_region(pruss_dev->res->start,
+ resource_size(pruss_dev->res), dev_name(&pdev->dev))) {
+ dev_err(&pdev->dev, "pruss memory region already claimed!\n");
+ err = -EBUSY;
+ goto probe_exit_kfree;
+ }
+
+ pruss_dev->ioaddr = ioremap(pruss_dev->res->start,
+ resource_size(pruss_dev->res));
+ if (!pruss_dev->ioaddr) {
+ dev_err(&pdev->dev, "ioremap failed\n");
+ err = -ENOMEM;
+ goto probe_exit_free_region;
+ }
+
+ pruss_dev->clk = clk_get(NULL, "pruss");
+ if (IS_ERR(pruss_dev->clk)) {
+ dev_err(&pdev->dev, "no clock available: pruss\n");
+ err = -ENODEV;
+ pruss_dev->clk = NULL;
+ goto probe_exit_iounmap;
+ }
+ spin_lock_init(&pruss_dev->lock);
+
+ clk_enable(pruss_dev->clk);
+ pruss_dev->clk_freq = clk_get_rate(pruss_dev->clk);
+
+ err = pruss_mfd_add_devices(pdev);
+ if (err)
+ goto probe_exit_clock;
+
+ platform_set_drvdata(pdev, pruss_dev);
+ pruss_dev->dev = &pdev->dev;
+ return 0;
+
+probe_exit_clock:
+ clk_put(pruss_dev->clk);
+ clk_disable(pruss_dev->clk);
+probe_exit_iounmap:
+ iounmap(pruss_dev->ioaddr);
+probe_exit_free_region:
+ release_mem_region(pruss_dev->res->start,
+ resource_size(pruss_dev->res));
+probe_exit_kfree:
+ kfree(pruss_dev);
+ return err;
+}
+
+static int __devexit da8xx_pruss_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct da8xx_pruss *pruss = dev_get_drvdata(dev);
+
+ mfd_remove_devices(dev);
+ pruss_disable(dev, DA8XX_PRUCORE_0);
+ pruss_disable(dev, DA8XX_PRUCORE_1);
+ clk_disable(pruss->clk);
+ clk_put(pruss->clk);
+ iounmap(pruss->ioaddr);
+ release_mem_region(pruss->res->start, resource_size(pruss->res));
+ kfree(pruss);
+ dev_set_drvdata(dev, NULL);
+ return 0;
+}
+
+static struct platform_driver da8xx_pruss_driver = {
+ .probe = da8xx_pruss_probe,
+ .remove = __devexit_p(da8xx_pruss_remove),
+ .driver = {
+ .name = "pruss_mfd",
+ .owner = THIS_MODULE,
+ }
+};
+
+static int __init da8xx_pruss_init(void)
+{
+ return platform_driver_register(&da8xx_pruss_driver);
+}
+module_init(da8xx_pruss_init);
+
+static void __exit da8xx_pruss_exit(void)
+{
+ platform_driver_unregister(&da8xx_pruss_driver);
+}
+module_exit(da8xx_pruss_exit);
+
+MODULE_DESCRIPTION("Programmable Realtime Unit (PRU) Driver");
+MODULE_AUTHOR("Subhasish Ghosh");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/da8xx/da8xx_pru.h b/include/linux/mfd/da8xx/da8xx_pru.h
new file mode 100644
index 0000000..ba65ec0
--- /dev/null
+++ b/include/linux/mfd/da8xx/da8xx_pru.h
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2010 Texas Instruments Incorporated
+ * Author: Jitendra Kumar <[email protected]>
+ *
+ * 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.
+ */
+
+#ifndef _PRUSS_H_
+#define _PRUSS_H_
+
+#include <linux/types.h>
+#include <linux/platform_device.h>
+#include "da8xx_prucore.h"
+
+#define PRUSS_NUM0 DA8XX_PRUCORE_0
+#define PRUSS_NUM1 DA8XX_PRUCORE_1
+
+#define PRUSS_PRU0_BASE_ADDRESS 0
+#define PRUSS_INTC_BASE_ADDRESS (PRUSS_PRU0_BASE_ADDRESS + 0x4000)
+#define PRUSS_INTC_GLBLEN (PRUSS_INTC_BASE_ADDRESS + 0x10)
+#define PRUSS_INTC_GLBLNSTLVL (PRUSS_INTC_BASE_ADDRESS + 0x1C)
+#define PRUSS_INTC_STATIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x20)
+#define PRUSS_INTC_STATIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x24)
+#define PRUSS_INTC_ENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x28)
+#define PRUSS_INTC_ENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x2C)
+#define PRUSS_INTC_HSTINTENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x34)
+#define PRUSS_INTC_HSTINTENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x38)
+#define PRUSS_INTC_GLBLPRIIDX (PRUSS_INTC_BASE_ADDRESS + 0x80)
+#define PRUSS_INTC_STATSETINT0 (PRUSS_INTC_BASE_ADDRESS + 0x200)
+#define PRUSS_INTC_STATSETINT1 (PRUSS_INTC_BASE_ADDRESS + 0x204)
+#define PRUSS_INTC_STATCLRINT0 (PRUSS_INTC_BASE_ADDRESS + 0x280)
+#define PRUSS_INTC_STATCLRINT1 (PRUSS_INTC_BASE_ADDRESS + 0x284)
+#define PRUSS_INTC_ENABLESET0 (PRUSS_INTC_BASE_ADDRESS + 0x300)
+#define PRUSS_INTC_ENABLESET1 (PRUSS_INTC_BASE_ADDRESS + 0x304)
+#define PRUSS_INTC_ENABLECLR0 (PRUSS_INTC_BASE_ADDRESS + 0x380)
+#define PRUSS_INTC_ENABLECLR1 (PRUSS_INTC_BASE_ADDRESS + 0x384)
+#define PRUSS_INTC_CHANMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x400)
+#define PRUSS_INTC_CHANMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x404)
+#define PRUSS_INTC_CHANMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x408)
+#define PRUSS_INTC_CHANMAP3 (PRUSS_INTC_BASE_ADDRESS + 0x40C)
+#define PRUSS_INTC_CHANMAP4 (PRUSS_INTC_BASE_ADDRESS + 0x410)
+#define PRUSS_INTC_CHANMAP5 (PRUSS_INTC_BASE_ADDRESS + 0x414)
+#define PRUSS_INTC_CHANMAP6 (PRUSS_INTC_BASE_ADDRESS + 0x418)
+#define PRUSS_INTC_CHANMAP7 (PRUSS_INTC_BASE_ADDRESS + 0x41C)
+#define PRUSS_INTC_CHANMAP8 (PRUSS_INTC_BASE_ADDRESS + 0x420)
+#define PRUSS_INTC_CHANMAP9 (PRUSS_INTC_BASE_ADDRESS + 0x424)
+#define PRUSS_INTC_CHANMAP10 (PRUSS_INTC_BASE_ADDRESS + 0x428)
+#define PRUSS_INTC_CHANMAP11 (PRUSS_INTC_BASE_ADDRESS + 0x42C)
+#define PRUSS_INTC_CHANMAP12 (PRUSS_INTC_BASE_ADDRESS + 0x430)
+#define PRUSS_INTC_CHANMAP13 (PRUSS_INTC_BASE_ADDRESS + 0x434)
+#define PRUSS_INTC_CHANMAP14 (PRUSS_INTC_BASE_ADDRESS + 0x438)
+#define PRUSS_INTC_CHANMAP15 (PRUSS_INTC_BASE_ADDRESS + 0x43C)
+#define PRUSS_INTC_HOSTMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x800)
+#define PRUSS_INTC_HOSTMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x804)
+#define PRUSS_INTC_HOSTMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x808)
+#define PRUSS_INTC_POLARITY0 (PRUSS_INTC_BASE_ADDRESS + 0xD00)
+#define PRUSS_INTC_POLARITY1 (PRUSS_INTC_BASE_ADDRESS + 0xD04)
+#define PRUSS_INTC_TYPE0 (PRUSS_INTC_BASE_ADDRESS + 0xD80)
+#define PRUSS_INTC_TYPE1 (PRUSS_INTC_BASE_ADDRESS + 0xD84)
+#define PRUSS_INTC_HOSTINTEN (PRUSS_INTC_BASE_ADDRESS + 0x1500)
+#define PRUSS_INTC_HOSTINTLVL_MAX 9
+
+#define PRU_INTC_HOSTMAP0_CHAN (0x03020100)
+#define PRU_INTC_HOSTMAP1_CHAN (0x07060504)
+#define PRU_INTC_HOSTMAP2_CHAN (0x00000908)
+
+#define PRU_INTC_CHANMAP7_SYS_EVT31 (0x00000000)
+#define PRU_INTC_CHANMAP8_FULL (0x02020100)
+#define PRU_INTC_CHANMAP9_FULL (0x04040303)
+#define PRU_INTC_CHANMAP10_FULL (0x06060505)
+#define PRU_INTC_CHANMAP11_FULL (0x08080707)
+#define PRU_INTC_CHANMAP12_FULL (0x00010909)
+#define PRU_INTC_CHANMAP8_HALF (0x03020100)
+#define PRU_INTC_CHANMAP9_HALF (0x07060504)
+#define PRU_INTC_CHANMAP10_HALF (0x03020908)
+#define PRU_INTC_CHANMAP11_HALF (0x07060504)
+#define PRU_INTC_CHANMAP12_HALF (0x00010908)
+
+#define PRU_INTC_REGMAP_MASK (0xFFFFFFFF)
+
+struct da8xx_pruss_devices {
+ const char *dev_name;
+ void *pdata;
+ size_t pdata_size;
+ int (*setup)(void);
+ u32 num_resources;
+ struct resource *resources;
+};
+
+u32 pruss_get_clk_freq(struct device *dev);
+
+s32 pruss_enable(struct device *dev, u8 pruss_num);
+
+s32 pruss_load(struct device *dev, u8 pruss_num,
+ u32 *pruss_code, u32 code_size_in_words);
+
+s32 pruss_run(struct device *dev, u8 pruss_num);
+
+s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout);
+
+s32 pruss_disable(struct device *dev, u8 pruss_num);
+
+s32 pruss_writeb(struct device *dev, u32 offset,
+ u8 *pdatatowrite, u16 wordstowrite);
+
+s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val);
+
+s32 pruss_readb(struct device *dev, u32 offset,
+ u8 *pdatatoread, u16 wordstoread);
+
+s32 pruss_readl(struct device *dev, u32 offset,
+ u32 *pdatatoread, s16 wordstoread);
+
+s32 pruss_writel(struct device *dev, u32 offset,
+ u32 *pdatatoread, s16 wordstoread);
+
+s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val);
+
+s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value);
+
+s32 pruss_writew(struct device *dev, u32 offset,
+ u16 *datatowrite, s16 wordstowrite);
+
+s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val);
+
+s32 pruss_readw(struct device *dev, u32 offset,
+ u16 *pdatatoread, s16 wordstoread);
+#endif /* End _PRUSS_H_ */
diff --git a/include/linux/mfd/da8xx/da8xx_prucore.h b/include/linux/mfd/da8xx/da8xx_prucore.h
new file mode 100644
index 0000000..913d56f
--- /dev/null
+++ b/include/linux/mfd/da8xx/da8xx_prucore.h
@@ -0,0 +1,129 @@
+/*
+ * Copyright (C) 2010 Texas Instruments Incorporated
+ * Author: Jitendra Kumar <[email protected]>
+ *
+ * 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.
+ */
+
+#ifndef _DA8XX_PRUCORE_H_
+#define _DA8XX_PRUCORE_H_
+
+#include <linux/types.h>
+
+#define DA8XX_PRUCORE_0 (0)
+#define DA8XX_PRUCORE_1 (1)
+
+#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_MASK (0xFFFF0000u)
+#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_SHIFT (0x00000010u)
+#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_RUNSTATE_MASK (0x00008000u)
+#define DA8XX_PRUCORE_CONTROL_RUNSTATE_SHIFT (0x0000000Fu)
+#define DA8XX_PRUCORE_CONTROL_RUNSTATE_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_RUNSTATE_HALT (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_RUNSTATE_RUN (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_MASK (0x00000100u)
+#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_SHIFT (0x00000008u)
+#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_FREERUN (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_SINGLE (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK (0x00000008u)
+#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT (0x00000003u)
+#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_ENABLE (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_SLEEPING_MASK (0x00000004u)
+#define DA8XX_PRUCORE_CONTROL_SLEEPING_SHIFT (0x00000002u)
+#define DA8XX_PRUCORE_CONTROL_SLEEPING_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SLEEPING_NOTASLEEP (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SLEEPING_ASLEEP (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_ENABLE_MASK (0x00000002u)
+#define DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_ENABLE_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_ENABLE_ENABLE (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_SOFTRESET_MASK (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_SOFTRESET_SHIFT (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SOFTRESET_RESETVAL (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SOFTRESET_RESET (0x00000000u)
+#define DA8XX_PRUCORE_CONTROL_SOFTRESET_OUT_OF_RESET (0x00000001u)
+#define DA8XX_PRUCORE_CONTROL_RESETVAL (0x00000000u)
+
+struct da8xx_prusscore_regs {
+ u32 CONTROL;
+ u32 STATUS;
+ u32 WAKEUP;
+ u32 CYCLECNT;
+ u32 STALLCNT;
+ u8 RSVD0[12];
+ u32 CONTABBLKIDX0;
+ u32 CONTABBLKIDX1;
+ u32 CONTABPROPTR0;
+ u32 CONTABPROPTR1;
+ u8 RSVD1[976];
+ u32 INTGPR[32];
+ u32 INTCTER[32];
+ u8 RSVD2[768];
+};
+
+struct pruss_intc_regs {
+ u32 REVID;
+ u32 CONTROL;
+ u8 RES1[8];
+ u32 GLBLEN;
+ u8 RES2[8];
+ u32 GLBLNSTLVL;
+ u32 STATIDXSET;
+ u32 STATIDXCLR;
+ u32 ENIDXSET;
+ u32 ENIDXCLR;
+ u8 RES3[4];
+ u32 HOSTINTENIDXSET;
+ u32 HOSTINTENIDXCLR;
+ u8 RES4[68];
+ u32 GLBLPRIIDX;
+ u8 RES5[380];
+ u32 STATSETINT[2];
+ u8 RES6[120];
+ u32 STATCLRINT[2];
+ u8 RES7[120];
+ u32 ENABLESET[2];
+ u8 RES8[120];
+ u32 ENABLECLR[2];
+ u8 RES9[120];
+ u32 CHANMAP[16];
+ u8 RES10[960];
+ u32 HOSTMAP[2];
+ u8 RES11[248];
+ u32 HOSTINTPRIIDX[10];
+ u8 RES12[984];
+ u32 POLARITY[2];
+ u8 RES13[120];
+ u32 TYPE[2];
+ u8 RES14[888];
+ u32 HOSTINTNSTLVL[10];
+ u8 RES15[984];
+ u32 HOSTINTEN;
+ u8 RES16[6907];
+};
+
+struct pruss_map {
+ u8 dram0[512];
+ u8 res1[7680];
+ u8 dram1[512];
+ u8 res2[7680];
+ struct pruss_intc_regs intc;
+ struct da8xx_prusscore_regs core[2];
+ u8 iram0[4096];
+ u8 res3[12288];
+ u8 iram1[4096];
+ u8 res4[12288];
+};
+
+#endif
--
1.7.2.3


2011-03-09 11:57:22

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On 03/08/2011 02:57 PM, Subhasish Ghosh wrote:
> This patch adds the pruss MFD driver and associated include files.
> For details regarding the PRUSS please refer the folowing link:
> http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem
>
> The rational behind the MFD driver being the fact that multiple devices can
> be implemented on the cores independently. This is determined by the nature
> of the program which is loaded into the PRU's instruction memory.
> A device may be de-initialized and another loaded or two different devices
> can be run simultaneously on the two cores.
> It's also possible, as in our case, to implement a single device on both
> the PRU's resulting in improved load sharing.

Make you driver compile with sparse "make C=1", you cast way too much
when accessing io mem. Use void __iomem * instead of u32 *.

See more comments inline.

>
> Signed-off-by: Subhasish Ghosh <[email protected]>
> ---
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/da8xx_pru.c | 560 +++++++++++++++++++++++++++++++
> include/linux/mfd/da8xx/da8xx_pru.h | 135 ++++++++
> include/linux/mfd/da8xx/da8xx_prucore.h | 129 +++++++
> 5 files changed, 835 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mfd/da8xx_pru.c
> create mode 100644 include/linux/mfd/da8xx/da8xx_pru.h
> create mode 100644 include/linux/mfd/da8xx/da8xx_prucore.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index fd01836..6c437df 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -81,6 +81,16 @@ config MFD_DM355EVM_MSP
> boards. MSP430 firmware manages resets and power sequencing,
> inputs from buttons and the IR remote, LEDs, an RTC, and more.
>
> +config MFD_DA8XX_PRUSS
> + tristate "Texas Instruments DA8XX PRUSS support"
> + depends on ARCH_DAVINCI && ARCH_DAVINCI_DA850
> + select MFD_CORE
> + help
> + This driver provides support api's for the programmable
> + realtime unit (PRU) present on TI's da8xx processors. It
> + provides basic read, write, config, enable, disable
> + routines to facilitate devices emulated on it.
> +
> config HTC_EGPIO
> bool "HTC EGPIO support"
> depends on GENERIC_HARDIRQS && GPIOLIB && ARM
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a54e2c7..670d6b0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o
> obj-$(CONFIG_HTC_I2CPLD) += htc-i2cpld.o
>
> obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o
> +obj-$(CONFIG_MFD_DA8XX_PRUSS) += da8xx_pru.o
> obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o
>
> obj-$(CONFIG_MFD_STMPE) += stmpe.o
> diff --git a/drivers/mfd/da8xx_pru.c b/drivers/mfd/da8xx_pru.c
> new file mode 100644
> index 0000000..0fd9bb2
> --- /dev/null
> +++ b/drivers/mfd/da8xx_pru.c
> @@ -0,0 +1,560 @@
> +/*
> + * Copyright (C) 2011 Texas Instruments Incorporated
> + *
> + * 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/bitops.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/errno.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/da8xx/da8xx_prucore.h>
> +#include <linux/mfd/da8xx/da8xx_pru.h>
> +#include <linux/mfd/core.h>
> +#include <linux/io.h>
> +#include <mach/da8xx.h>
> +
> +struct da8xx_pruss {
> + struct device *dev;
> + spinlock_t lock;
> + struct resource *res;
> + struct clk *clk;
> + u32 clk_freq;
> + void __iomem *ioaddr;
> +};
> +
> +u32 pruss_get_clk_freq(struct device *dev)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> +
> + return pruss->clk_freq;
> +}
> +EXPORT_SYMBOL(pruss_get_clk_freq);
> +
> +s32 pruss_disable(struct device *dev, u8 pruss_num)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 temp_reg;
> + u32 delay_cnt;
> +
> + if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
> + return -EINVAL;
> +
> + spin_lock(&pruss->lock);
> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT0 & 0xFFFF));
> + /* Disable PRU0 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU0 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> +
> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT1 & 0xFFFF));
> + /* Disable PRU1 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU1 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> + }

unneeded code duplication

> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_disable);
> +
> +s32 pruss_enable(struct device *dev, u8 pruss_num)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> +
> + if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
> + return -EINVAL;
> +
> + spin_lock(&pruss->lock);

just use pruss_num in &pruss_mmap->core[DA8XX_PRUCORE_0], then remove
the "if..else if"

> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* Reset PRU0 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* Reset PRU1 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> + }
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_enable);
> +
> +/* Load the specified PRU with code */
> +s32 pruss_load(struct device *dev, u8 pruss_num,
> + u32 *pruss_code, u32 code_size_in_words)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 *pruss_iram;
> + u32 i;
> +
> + if (pruss_num == DA8XX_PRUCORE_0)
> + pruss_iram = (u32 *)&pruss_mmap->iram0;
> + else if (pruss_num == DA8XX_PRUCORE_1)
> + pruss_iram = (u32 *)&pruss_mmap->iram1;

u32 * looks fishy, should be probavly a void __iomem
also make pruss_mmap->iram0,1 an array

> + else
> + return -EINVAL;
> +
> + pruss_enable(dev, pruss_num);
> +
> + spin_lock(&pruss->lock);
> + /* Copy dMAX code to its instruction RAM */
> + for (i = 0; i < code_size_in_words; i++)
> + __raw_writel(pruss_code[i], (pruss_iram + i));
> +
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_load);
> +
> +s32 pruss_run(struct device *dev, u8 pruss_num)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 temp_reg;
> +
> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* DA8XX_PRUCORE_0_REGS; */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* DA8XX_PRUCORE_1_REGS; */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];

if you first check if pruss_num is correct, then you can just use
pruss_num in "pruss_mmap->core[DA8XX_PRUCORE_1]"

> + } else
> + return -EINVAL;
> +
> + /* Enable dMAX, let it execute the code we just copied */

is it possible to use the _rmw function you defined later?

> + spin_lock(&pruss->lock);
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_ENABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_ENABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_run);
> +
> +s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 temp_reg;
> + u32 cnt = timeout;
> +
> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* DA8XX_PRUCORE_0_REGS; */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* DA8XX_PRUCORE_1_REGS; */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];

dito

> + } else
> + return -EINVAL;
> +
> + while (cnt--) {
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + if (((temp_reg & DA8XX_PRUCORE_CONTROL_RUNSTATE_MASK) >>
> + DA8XX_PRUCORE_CONTROL_RUNSTATE_SHIFT) ==
> + DA8XX_PRUCORE_CONTROL_RUNSTATE_HALT)
> + break;
> + }
> + if (cnt == 0)
> + return -EBUSY;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_wait_for_halt);
> +
> +s32 pruss_writeb(struct device *dev, u32 offset,
> + u8 *pdatatowrite, u16 bytestowrite)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u8 *paddresstowrite;
> + u16 loop;
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstowrite = (u8 *) (offset);

Why all this casing? Checck your driver with sparse, please compile your
driver with "make C=1".

> +
> + for (loop = 0; loop < bytestowrite; loop++)
> + __raw_writeb(*pdatatowrite++, paddresstowrite++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_writeb);
> +
> +s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddress;
> + u32 preg_data;
> +
> + paddress = (u32 *)((u32)pruss->ioaddr + offset);
same here
> +
> + spin_lock(&pruss->lock);
> + preg_data = __raw_readb(paddress);
> + preg_data &= ~mask;
> + preg_data |= val;
> + __raw_writeb(preg_data, paddress);
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_rmwb);
> +
> +s32 pruss_readb(struct device *dev, u32 offset,
> + u8 *pdatatoread, u16 bytestoread)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u8 *paddresstoread;
> + u16 loop;
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstoread = (u8 *) (offset);
same here
> +
> + for (loop = 0; loop < bytestoread; loop++)
> + *pdatatoread++ = __raw_readb(paddresstoread++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_readb);
> +
> +s32 pruss_writel(struct device *dev, u32 offset,
> + u32 *pdatatowrite, s16 wordstowrite)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddresstowrite;
> + s16 loop;
> +
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstowrite = (u32 *)(offset);
dito
> +
> + for (loop = 0; loop < wordstowrite; loop++)
> + __raw_writel(*pdatatowrite++, paddresstowrite++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_writel);
> +
> +s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddress;
> + u32 preg_data;
> +
> + paddress = (u32 *)((u32)pruss->ioaddr + offset);
dito
> +
> + spin_lock(&pruss->lock);
> + preg_data = __raw_readl(paddress);
> + preg_data &= ~mask;
> + preg_data |= val;
> + __raw_writel(preg_data, paddress);
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_rmwl);
> +
> +s32 pruss_readl(struct device *dev, u32 offset,
> + u32 *pdatatoread, s16 wordstoread)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddresstoread;
> + s16 loop;
> +
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstoread = (u32 *)(offset);

dito

> +
> + for (loop = 0; loop < wordstoread; loop++)
> + *pdatatoread++ = __raw_readl(paddresstoread++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_readl);
> +
> +s32 pruss_writew(struct device *dev, u32 offset,
> + u16 *pdatatowrite, s16 wordstowrite)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddresstowrite;
> + s16 loop;
> +
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstowrite = (u32 *)(offset);

dito

> +
> + for (loop = 0; loop < wordstowrite; loop++)
> + __raw_writew(*(pdatatowrite++), (paddresstowrite++));
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_writew);
> +
> +s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddress;
> + u32 preg_data;
> +
> + paddress = (u32 *)((u32)pruss->ioaddr + offset);

dito

> +
> + spin_lock(&pruss->lock);
> + preg_data = __raw_readw(paddress);
> + preg_data &= ~mask;
> + preg_data |= val;
> + __raw_writew(preg_data, paddress);
> + spin_unlock(&pruss->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_rmww);
> +
> +s32 pruss_readw(struct device *dev, u32 offset,
> + u16 *pdatatoread, s16 wordstoread)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddresstoread;
> + s16 loop;
> +
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstoread = (u32 *)(offset);
> +

dito

> + for (loop = 0; loop < wordstoread; loop++)
use "i" as loop variable
> + *pdatatoread++ = __raw_readw(paddresstoread++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_readw);
> +
> +s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u32 *paddresstowrite;
> +
> + paddresstowrite = (u32 *)(pruss->ioaddr + offset);

dito
> + __raw_writel(value, paddresstowrite);



> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_idx_writel);
> +
> +static int pruss_mfd_add_devices(struct platform_device *pdev)
> +{
> + struct da8xx_pruss_devices *dev_data = pdev->dev.platform_data;
> + struct device *dev = &pdev->dev;
> + struct mfd_cell cell;
> + u32 err, count;
> +
> + for (count = 0; dev_data[count].dev_name != NULL; count++) {

nitpick:
we usually use "i" as counter variable

> + memset(&cell, 0, sizeof(struct mfd_cell));
> + cell.id = count;
> + cell.name = dev_data[count].dev_name;
> + cell.platform_data = dev_data[count].pdata;
> + cell.data_size = dev_data[count].pdata_size;
> + cell.resources = dev_data[count].resources;
> + cell.num_resources = dev_data[count].num_resources;
> +
> + err = mfd_add_devices(dev, 0, &cell, 1, NULL, 0);
> + if (err) {
> + dev_err(dev, "cannot add mfd cells\n");
> + return err;

you forget to clean up already registered mfd devices. I suggest to
rearrange your code so that you can use mfd_add_devices() to register
all of your devices at once.

> + }
> + dev_info(dev, "mfd: added %s device\n",
> + dev_data[count].dev_name);
> + }
> +
> + return err;
> +}
> +
> +static int __devinit da8xx_pruss_probe(struct platform_device *pdev)
> +{
> + struct da8xx_pruss *pruss_dev = NULL;
> + u32 err;
> +
> + pruss_dev = kzalloc(sizeof(struct da8xx_pruss), GFP_KERNEL);
> + if (!pruss_dev)
> + return -ENOMEM;
> +
> + pruss_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!pruss_dev->res) {
> + dev_err(&pdev->dev,
> + "unable to get pruss memory resources!\n");
> + err = -ENODEV;
> + goto probe_exit_kfree;
> + }
> +
> + if (!request_mem_region(pruss_dev->res->start,
> + resource_size(pruss_dev->res), dev_name(&pdev->dev))) {
> + dev_err(&pdev->dev, "pruss memory region already claimed!\n");
> + err = -EBUSY;
> + goto probe_exit_kfree;
> + }
> +
> + pruss_dev->ioaddr = ioremap(pruss_dev->res->start,
> + resource_size(pruss_dev->res));
> + if (!pruss_dev->ioaddr) {
> + dev_err(&pdev->dev, "ioremap failed\n");
> + err = -ENOMEM;
> + goto probe_exit_free_region;
> + }
> +
> + pruss_dev->clk = clk_get(NULL, "pruss");
> + if (IS_ERR(pruss_dev->clk)) {
> + dev_err(&pdev->dev, "no clock available: pruss\n");
> + err = -ENODEV;
> + pruss_dev->clk = NULL;
> + goto probe_exit_iounmap;c
> + }
> + spin_lock_init(&pruss_dev->lock);
> +
> + clk_enable(pruss_dev->clk);
> + pruss_dev->clk_freq = clk_get_rate(pruss_dev->clk);

pruss_dev->clk_freq in an u32, but clk_get_rate returns an unsigned long

> +
> + err = pruss_mfd_add_devices(pdev);
> + if (err)
> + goto probe_exit_clock;
> +
> + platform_set_drvdata(pdev, pruss_dev);
> + pruss_dev->dev = &pdev->dev;
> + return 0;
> +
> +probe_exit_clock:
> + clk_put(pruss_dev->clk);
> + clk_disable(pruss_dev->clk);
> +probe_exit_iounmap:
> + iounmap(pruss_dev->ioaddr);
> +probe_exit_free_region:
> + release_mem_region(pruss_dev->res->start,
> + resource_size(pruss_dev->res));
> +probe_exit_kfree:
> + kfree(pruss_dev);
> + return err;
> +}
> +
> +static int __devexit da8xx_pruss_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev);
> +
> + mfd_remove_devices(dev);
> + pruss_disable(dev, DA8XX_PRUCORE_0);
> + pruss_disable(dev, DA8XX_PRUCORE_1);
> + clk_disable(pruss->clk);
> + clk_put(pruss->clk);
> + iounmap(pruss->ioaddr);
> + release_mem_region(pruss->res->start, resource_size(pruss->res));
> + kfree(pruss);
> + dev_set_drvdata(dev, NULL);
> + return 0;
> +}
> +
> +static struct platform_driver da8xx_pruss_driver = {
> + .probe = da8xx_pruss_probe,
> + .remove = __devexit_p(da8xx_pruss_remove),
> + .driver = {
> + .name = "pruss_mfd",
> + .owner = THIS_MODULE,
> + }
> +};
> +
> +static int __init da8xx_pruss_init(void)
> +{
> + return platform_driver_register(&da8xx_pruss_driver);
> +}
> +module_init(da8xx_pruss_init);
> +
> +static void __exit da8xx_pruss_exit(void)
> +{
> + platform_driver_unregister(&da8xx_pruss_driver);
> +}
> +module_exit(da8xx_pruss_exit);
> +
> +MODULE_DESCRIPTION("Programmable Realtime Unit (PRU) Driver");
> +MODULE_AUTHOR("Subhasish Ghosh");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/da8xx/da8xx_pru.h b/include/linux/mfd/da8xx/da8xx_pru.h
> new file mode 100644
> index 0000000..ba65ec0
> --- /dev/null
> +++ b/include/linux/mfd/da8xx/da8xx_pru.h
> @@ -0,0 +1,135 @@
> +/*
> + * Copyright (C) 2010 Texas Instruments Incorporated
> + * Author: Jitendra Kumar <[email protected]>
> + *
> + * 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.
> + */
> +
> +#ifndef _PRUSS_H_
> +#define _PRUSS_H_
> +
> +#include <linux/types.h>
> +#include <linux/platform_device.h>
> +#include "da8xx_prucore.h"
> +
> +#define PRUSS_NUM0 DA8XX_PRUCORE_0
> +#define PRUSS_NUM1 DA8XX_PRUCORE_1
> +
> +#define PRUSS_PRU0_BASE_ADDRESS 0
> +#define PRUSS_INTC_BASE_ADDRESS (PRUSS_PRU0_BASE_ADDRESS + 0x4000)
> +#define PRUSS_INTC_GLBLEN (PRUSS_INTC_BASE_ADDRESS + 0x10)
> +#define PRUSS_INTC_GLBLNSTLVL (PRUSS_INTC_BASE_ADDRESS + 0x1C)
> +#define PRUSS_INTC_STATIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x20)
> +#define PRUSS_INTC_STATIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x24)
> +#define PRUSS_INTC_ENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x28)
> +#define PRUSS_INTC_ENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x2C)
> +#define PRUSS_INTC_HSTINTENIDXSET (PRUSS_INTC_BASE_ADDRESS + 0x34)
> +#define PRUSS_INTC_HSTINTENIDXCLR (PRUSS_INTC_BASE_ADDRESS + 0x38)
> +#define PRUSS_INTC_GLBLPRIIDX (PRUSS_INTC_BASE_ADDRESS + 0x80)
> +#define PRUSS_INTC_STATSETINT0 (PRUSS_INTC_BASE_ADDRESS + 0x200)
> +#define PRUSS_INTC_STATSETINT1 (PRUSS_INTC_BASE_ADDRESS + 0x204)
> +#define PRUSS_INTC_STATCLRINT0 (PRUSS_INTC_BASE_ADDRESS + 0x280)
> +#define PRUSS_INTC_STATCLRINT1 (PRUSS_INTC_BASE_ADDRESS + 0x284)
> +#define PRUSS_INTC_ENABLESET0 (PRUSS_INTC_BASE_ADDRESS + 0x300)
> +#define PRUSS_INTC_ENABLESET1 (PRUSS_INTC_BASE_ADDRESS + 0x304)
> +#define PRUSS_INTC_ENABLECLR0 (PRUSS_INTC_BASE_ADDRESS + 0x380)
> +#define PRUSS_INTC_ENABLECLR1 (PRUSS_INTC_BASE_ADDRESS + 0x384)
> +#define PRUSS_INTC_CHANMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x400)
> +#define PRUSS_INTC_CHANMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x404)
> +#define PRUSS_INTC_CHANMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x408)
> +#define PRUSS_INTC_CHANMAP3 (PRUSS_INTC_BASE_ADDRESS + 0x40C)
> +#define PRUSS_INTC_CHANMAP4 (PRUSS_INTC_BASE_ADDRESS + 0x410)
> +#define PRUSS_INTC_CHANMAP5 (PRUSS_INTC_BASE_ADDRESS + 0x414)
> +#define PRUSS_INTC_CHANMAP6 (PRUSS_INTC_BASE_ADDRESS + 0x418)
> +#define PRUSS_INTC_CHANMAP7 (PRUSS_INTC_BASE_ADDRESS + 0x41C)
> +#define PRUSS_INTC_CHANMAP8 (PRUSS_INTC_BASE_ADDRESS + 0x420)
> +#define PRUSS_INTC_CHANMAP9 (PRUSS_INTC_BASE_ADDRESS + 0x424)
> +#define PRUSS_INTC_CHANMAP10 (PRUSS_INTC_BASE_ADDRESS + 0x428)
> +#define PRUSS_INTC_CHANMAP11 (PRUSS_INTC_BASE_ADDRESS + 0x42C)
> +#define PRUSS_INTC_CHANMAP12 (PRUSS_INTC_BASE_ADDRESS + 0x430)
> +#define PRUSS_INTC_CHANMAP13 (PRUSS_INTC_BASE_ADDRESS + 0x434)
> +#define PRUSS_INTC_CHANMAP14 (PRUSS_INTC_BASE_ADDRESS + 0x438)
> +#define PRUSS_INTC_CHANMAP15 (PRUSS_INTC_BASE_ADDRESS + 0x43C)
> +#define PRUSS_INTC_HOSTMAP0 (PRUSS_INTC_BASE_ADDRESS + 0x800)
> +#define PRUSS_INTC_HOSTMAP1 (PRUSS_INTC_BASE_ADDRESS + 0x804)
> +#define PRUSS_INTC_HOSTMAP2 (PRUSS_INTC_BASE_ADDRESS + 0x808)
> +#define PRUSS_INTC_POLARITY0 (PRUSS_INTC_BASE_ADDRESS + 0xD00)
> +#define PRUSS_INTC_POLARITY1 (PRUSS_INTC_BASE_ADDRESS + 0xD04)
> +#define PRUSS_INTC_TYPE0 (PRUSS_INTC_BASE_ADDRESS + 0xD80)
> +#define PRUSS_INTC_TYPE1 (PRUSS_INTC_BASE_ADDRESS + 0xD84)
> +#define PRUSS_INTC_HOSTINTEN (PRUSS_INTC_BASE_ADDRESS + 0x1500)
> +#define PRUSS_INTC_HOSTINTLVL_MAX 9
> +
> +#define PRU_INTC_HOSTMAP0_CHAN (0x03020100)
> +#define PRU_INTC_HOSTMAP1_CHAN (0x07060504)
> +#define PRU_INTC_HOSTMAP2_CHAN (0x00000908)
> +
> +#define PRU_INTC_CHANMAP7_SYS_EVT31 (0x00000000)
> +#define PRU_INTC_CHANMAP8_FULL (0x02020100)
> +#define PRU_INTC_CHANMAP9_FULL (0x04040303)
> +#define PRU_INTC_CHANMAP10_FULL (0x06060505)
> +#define PRU_INTC_CHANMAP11_FULL (0x08080707)
> +#define PRU_INTC_CHANMAP12_FULL (0x00010909)
> +#define PRU_INTC_CHANMAP8_HALF (0x03020100)
> +#define PRU_INTC_CHANMAP9_HALF (0x07060504)
> +#define PRU_INTC_CHANMAP10_HALF (0x03020908)
> +#define PRU_INTC_CHANMAP11_HALF (0x07060504)
> +#define PRU_INTC_CHANMAP12_HALF (0x00010908)
> +
> +#define PRU_INTC_REGMAP_MASK (0xFFFFFFFF)
> +
> +struct da8xx_pruss_devices {
> + const char *dev_name;
> + void *pdata;
> + size_t pdata_size;
> + int (*setup)(void);
> + u32 num_resources;
> + struct resource *resources;
> +};
> +
> +u32 pruss_get_(struct device *dev);
> +
> +s32 pruss_enable(struct device *dev, u8 pruss_num);
> +
> +s32 pruss_load(struct device *dev, u8 pruss_num,
> + u32 *pruss_code, u32 code_size_in_words);
> +
> +s32 pruss_run(struct device *dev, u8 pruss_num);
> +
> +s32 pruss_wait_for_halt(struct device *dev, u8 pruss_num, u32 timeout);
> +
> +s32 pruss_disable(struct device *dev, u8 pruss_num);
> +
> +s32 pruss_writeb(struct device *dev, u32 offset,
> + u8 *pdatatowrite, u16 wordstowrite);
> +
> +s32 pruss_rmwb(struct device *dev, u32 offset, u8 mask, u8 val);
> +
> +s32 pruss_readb(struct device *dev, u32 offset,
> + u8 *pdatatoread, u16 wordstoread);
> +
> +s32 pruss_readl(struct device *dev, u32 offset,
> + u32 *pdatatoread, s16 wordstoread);
> +
> +s32 pruss_writel(struct device *dev, u32 offset,
> + u32 *pdatatoread, s16 wordstoread);
> +
> +s32 pruss_rmwl(struct device *dev, u32 offset, u32 mask, u32 val);
> +
> +s32 pruss_idx_writel(struct device *dev, u32 offset, u32 value);
> +
> +s32 pruss_writew(struct device *dev, u32 offset,
> + u16 *datatowrite, s16 wordstowrite);
> +
> +s32 pruss_rmww(struct device *dev, u32 offset, u16 mask, u16 val);
> +
> +s32 pruss_readw(struct device *dev, u32 offset,
> + u16 *pdatatoread, s16 wordstoread);
> +#endif /* End _PRUSS_H_ */
> diff --git a/include/linux/mfd/da8xx/da8xx_prucore.h b/include/linux/mfd/da8xx/da8xx_prucore.h
> new file mode 100644
> index 0000000..913d56f
> --- /dev/null
> +++ b/include/linux/mfd/da8xx/da8xx_prucore.h
> @@ -0,0 +1,129 @@
> +/*
> + * Copyright (C) 2010 Texas Instruments Incorporated
> + * Author: Jitendra Kumar <[email protected]>
> + *
> + * 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.
> + */
> +
> +#ifndef _DA8XX_PRUCORE_H_
> +#define _DA8XX_PRUCORE_H_
> +
> +#include <linux/types.h>
> +
> +#define DA8XX_PRUCORE_0 (0)
> +#define DA8XX_PRUCORE_1 (1)
> +
> +#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_MASK (0xFFFF0000u)
> +#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_SHIFT (0x00000010u)
> +#define DA8XX_PRUCORE_CONTROL_PCRESETVAL_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_RUNSTATE_MASK (0x00008000u)
> +#define DA8XX_PRUCORE_CONTROL_RUNSTATE_SHIFT (0x0000000Fu)
> +#define DA8XX_PRUCORE_CONTROL_RUNSTATE_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_RUNSTATE_HALT (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_RUNSTATE_RUN (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_MASK (0x00000100u)
> +#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_SHIFT (0x00000008u)
> +#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_FREERUN (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SINGLESTEP_SINGLE (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK (0x00000008u)
> +#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT (0x00000003u)
> +#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_COUNTENABLE_ENABLE (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_SLEEPING_MASK (0x00000004u)
> +#define DA8XX_PRUCORE_CONTROL_SLEEPING_SHIFT (0x00000002u)
> +#define DA8XX_PRUCORE_CONTROL_SLEEPING_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SLEEPING_NOTASLEEP (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SLEEPING_ASLEEP (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_ENABLE_MASK (0x00000002u)
> +#define DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_ENABLE_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_ENABLE_ENABLE (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_SOFTRESET_MASK (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_SOFTRESET_SHIFT (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SOFTRESET_RESETVAL (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SOFTRESET_RESET (0x00000000u)
> +#define DA8XX_PRUCORE_CONTROL_SOFTRESET_OUT_OF_RESET (0x00000001u)
> +#define DA8XX_PRUCORE_CONTROL_RESETVAL (0x00000000u)
> +
> +struct da8xx_prusscore_regs {
> + u32 CONTROL;
> + u32 STATUS;
> + u32 WAKEUP;
> + u32 CYCLECNT;
> + u32 STALLCNT;
> + u8 RSVD0[12];
> + u32 CONTABBLKIDX0;
> + u32 CONTABBLKIDX1;
> + u32 CONTABPROPTR0;
> + u32 CONTABPROPTR1;
> + u8 RSVD1[976];
> + u32 INTGPR[32];
> + u32 INTCTER[32];
> + u8 RSVD2[768];
> +};

struct members are usually lowercase

> +
> +struct pruss_intc_regs {
> + u32 REVID;
> + u32 CONTROL;
> + u8 RES1[8];
> + u32 GLBLEN;
> + u8 RES2[8];
> + u32 GLBLNSTLVL;
> + u32 STATIDXSET;
> + u32 STATIDXCLR;
> + u32 ENIDXSET;
> + u32 ENIDXCLR;
> + u8 RES3[4];
> + u32 HOSTINTENIDXSET;
> + u32 HOSTINTENIDXCLR;
> + u8 RES4[68];
> + u32 GLBLPRIIDX;
> + u8 RES5[380];
> + u32 STATSETINT[2];
> + u8 RES6[120];
> + u32 STATCLRINT[2];
> + u8 RES7[120];
> + u32 ENABLESET[2];
> + u8 RES8[120];
> + u32 ENABLECLR[2];
> + u8 RES9[120];
> + u32 CHANMAP[16];
> + u8 RES10[960];
> + u32 HOSTMAP[2];
> + u8 RES11[248];
> + u32 HOSTINTPRIIDX[10];
> + u8 RES12[984];
> + u32 POLARITY[2];
> + u8 RES13[120];
> + u32 TYPE[2];
> + u8 RES14[888];
> + u32 HOSTINTNSTLVL[10];
> + u8 RES15[984];
> + u32 HOSTINTEN;
> + u8 RES16[6907];
> +};
> +
> +struct pruss_map {
> + u8 dram0[512];
> + u8 res1[7680];
> + u8 dram1[512];
> + u8 res2[7680];
> + struct pruss_intc_regs intc;
> + struct da8xx_prusscore_regs core[2];
> + u8 iram0[4096];
> + u8 res3[12288];
> + u8 iram1[4096];
> + u8 res4[12288];
> +};
> +
> +#endif


--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (262.00 B)
OpenPGP digital signature

2011-03-11 15:29:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Tuesday 08 March 2011, Subhasish Ghosh wrote:

> +struct da8xx_pruss {
> + struct device *dev;
> + spinlock_t lock;
> + struct resource *res;
> + struct clk *clk;
> + u32 clk_freq;
> + void __iomem *ioaddr;
> +};

> +s32 pruss_disable(struct device *dev, u8 pruss_num)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 temp_reg;
> + u32 delay_cnt;

Can you explain the significance of pruss_num? As far as I
can tell, you always pass constants in here, so it should
be possible to determine the number from the device.

> + if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
> + return -EINVAL;
> +
> + spin_lock(&pruss->lock);
> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT0 & 0xFFFF));
> + /* Disable PRU0 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);

Better use readl/writel, the __raw_ variants are not reliable in general.

> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU0 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);

Why do you need to reset it 65536 times? Is once not enough?

> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT1 & 0xFFFF));
> + /* Disable PRU1 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU1 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> + }
> + spin_unlock(&pruss->lock);

This is almost the exact same code as for the DA8XX_PRUCORE_0 case.
Please be a little more creative in order to avoid such code duplication.

> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_disable);

EXPORT_SYMBOL_GPL, please. Also for the other symbols.

> +s32 pruss_writeb(struct device *dev, u32 offset,
> + u8 *pdatatowrite, u16 bytestowrite)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + u8 *paddresstowrite;
> + u16 loop;
> + offset = (u32)pruss->ioaddr + offset;
> + paddresstowrite = (u8 *) (offset);
> +
> + for (loop = 0; loop < bytestowrite; loop++)
> + __raw_writeb(*pdatatowrite++, paddresstowrite++);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(pruss_writeb);

I would recommend providing a simpler variant of your all I/O accessors,
which write a single word. Most of the users of these simply
pass bytestowrite=1 anyway, so the caller can become more readable.

Also, my comments about __raw_* and Marc's comments about the
type cast apply to all of these.

> +static int pruss_mfd_add_devices(struct platform_device *pdev)
> +{
> + struct da8xx_pruss_devices *dev_data = pdev->dev.platform_data;
> + struct device *dev = &pdev->dev;
> + struct mfd_cell cell;
> + u32 err, count;
> +
> + for (count = 0; dev_data[count].dev_name != NULL; count++) {
> + memset(&cell, 0, sizeof(struct mfd_cell));
> + cell.id = count;
> + cell.name = dev_data[count].dev_name;
> + cell.platform_data = dev_data[count].pdata;
> + cell.data_size = dev_data[count].pdata_size;
> + cell.resources = dev_data[count].resources;
> + cell.num_resources = dev_data[count].num_resources;
> +
> + err = mfd_add_devices(dev, 0, &cell, 1, NULL, 0);
> + if (err) {
> + dev_err(dev, "cannot add mfd cells\n");
> + return err;
> + }
> + dev_info(dev, "mfd: added %s device\n",
> + dev_data[count].dev_name);
> + }
> +
> + return err;
> +}

This would get much simpler if you just replaced the da8xx_pruss_devices
array with an mfd_cell array.

Arnd

2011-03-18 12:00:00

by Sekhar Nori

[permalink] [raw]
Subject: RE: [PATCH v3 1/7] mfd: add pruss mfd driver.

Hi Subhasish,

On Tue, Mar 08, 2011 at 19:27:40, Subhasish Ghosh wrote:
> This patch adds the pruss MFD driver and associated include files.
> For details regarding the PRUSS please refer the folowing link:
> http://processors.wiki.ti.com/index.php/Programmable_Realtime_Unit_Subsystem
>
> The rational behind the MFD driver being the fact that multiple devices can
> be implemented on the cores independently. This is determined by the nature
> of the program which is loaded into the PRU's instruction memory.
> A device may be de-initialized and another loaded or two different devices
> can be run simultaneously on the two cores.
> It's also possible, as in our case, to implement a single device on both
> the PRU's resulting in improved load sharing.
>
> Signed-off-by: Subhasish Ghosh <[email protected]>
> ---
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/da8xx_pru.c | 560 +++++++++++++++++++++++++++++++
> include/linux/mfd/da8xx/da8xx_pru.h | 135 ++++++++
> include/linux/mfd/da8xx/da8xx_prucore.h | 129 +++++++
> 5 files changed, 835 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mfd/da8xx_pru.c
> create mode 100644 include/linux/mfd/da8xx/da8xx_pru.h
> create mode 100644 include/linux/mfd/da8xx/da8xx_prucore.h

PRUSS as an IP is not really tied to the DA8xx SoC architecture.
So, can you please name the files ti-pruss* or even just pruss if
MFD folks are okay with it?

>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index fd01836..6c437df 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -81,6 +81,16 @@ config MFD_DM355EVM_MSP
> boards. MSP430 firmware manages resets and power sequencing,
> inputs from buttons and the IR remote, LEDs, an RTC, and more.
>
> +config MFD_DA8XX_PRUSS
> + tristate "Texas Instruments DA8XX PRUSS support"
> + depends on ARCH_DAVINCI && ARCH_DAVINCI_DA850

Just ARCH_DAVINCI_DA850 should be okay since ARCH_DAVINCI_DA850
Already depends on ARCH_DAVINCI

> + select MFD_CORE
> + help
> + This driver provides support api's for the programmable

"API" would be preferred. Also please remove the apostrophe.

> + realtime unit (PRU) present on TI's da8xx processors. It
> + provides basic read, write, config, enable, disable
> + routines to facilitate devices emulated on it.
> +
> config HTC_EGPIO
> bool "HTC EGPIO support"
> depends on GENERIC_HARDIRQS && GPIOLIB && ARM
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a54e2c7..670d6b0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o
> obj-$(CONFIG_HTC_I2CPLD) += htc-i2cpld.o
>
> obj-$(CONFIG_MFD_DAVINCI_VOICECODEC) += davinci_voicecodec.o
> +obj-$(CONFIG_MFD_DA8XX_PRUSS) += da8xx_pru.o
> obj-$(CONFIG_MFD_DM355EVM_MSP) += dm355evm_msp.o
>
> obj-$(CONFIG_MFD_STMPE) += stmpe.o
> diff --git a/drivers/mfd/da8xx_pru.c b/drivers/mfd/da8xx_pru.c
> new file mode 100644
> index 0000000..0fd9bb2
> --- /dev/null
> +++ b/drivers/mfd/da8xx_pru.c
> @@ -0,0 +1,560 @@
> +/*
> + * Copyright (C) 2011 Texas Instruments Incorporated
> + *
> + * 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/bitops.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/errno.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/mfd/da8xx/da8xx_prucore.h>
> +#include <linux/mfd/da8xx/da8xx_pru.h>
> +#include <linux/mfd/core.h>
> +#include <linux/io.h>
> +#include <mach/da8xx.h>

Hmm, this means the driver is tied to the platform.
This should not be the case.

I was able to get this patch compiled even
after removing this include. Please remove it
in the next version.

> +
> +struct da8xx_pruss {

I would prefer if the variables and data structures
and includes are not pre-fixed with da8xx as well.

> + struct device *dev;
> + spinlock_t lock;
> + struct resource *res;
> + struct clk *clk;
> + u32 clk_freq;
> + void __iomem *ioaddr;
> +};
> +
> +u32 pruss_get_clk_freq(struct device *dev)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> +
> + return pruss->clk_freq;
> +}
> +EXPORT_SYMBOL(pruss_get_clk_freq);

This looks strange. Why do we need this? There
is clk_get_rate() API in the kernel would would
seem more suitable.

> +
> +s32 pruss_disable(struct device *dev, u8 pruss_num)
> +{
> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> + struct da8xx_prusscore_regs *h_pruss;
> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> + u32 temp_reg;
> + u32 delay_cnt;
> +
> + if ((pruss_num != DA8XX_PRUCORE_0) && (pruss_num != DA8XX_PRUCORE_1))
> + return -EINVAL;
> +
> + spin_lock(&pruss->lock);
> + if (pruss_num == DA8XX_PRUCORE_0) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT0 & 0xFFFF));

Why not use writel instead? Per my understanding
The __raw_ variants are unsafe to use on ARMv6
and above.

> + /* Disable PRU0 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_0];
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU0 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> +
> + } else if (pruss_num == DA8XX_PRUCORE_1) {
> + /* pruss deinit */
> + __raw_writel(0xFFFFFFFF, (PRUSS_INTC_STATCLRINT1 & 0xFFFF));
> + /* Disable PRU1 */
> + h_pruss = (struct da8xx_prusscore_regs *)
> + &pruss_mmap->core[DA8XX_PRUCORE_1];
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> +
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--) {
> +
> + temp_reg = __raw_readl(&h_pruss->CONTROL);
> + temp_reg = (temp_reg &
> + ~DA8XX_PRUCORE_CONTROL_ENABLE_MASK) |
> + ((DA8XX_PRUCORE_CONTROL_ENABLE_DISABLE <<
> + DA8XX_PRUCORE_CONTROL_ENABLE_SHIFT) &
> + DA8XX_PRUCORE_CONTROL_ENABLE_MASK);
> + __raw_writel(temp_reg, &h_pruss->CONTROL);
> + }
> +
> + /* Reset PRU1 */
> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
> + &h_pruss->CONTROL);
> + }

There is a lot of code repeated between the if and else and
I am sure it will be possible to remove the if-else block
altogether and use the pruss_num as an index know which
core to operate on. Doing this will also help add more cores
later.

I really couldn't complete this review (got to rush now),
but I thought I will send you what I have. You can wait
for review to complete before posting another version.

Thanks,
Sekhar

2011-03-18 12:39:29

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Fri, Mar 18, 2011 at 05:29:39PM +0530, Nori, Sekhar wrote:

> PRUSS as an IP is not really tied to the DA8xx SoC architecture.
> So, can you please name the files ti-pruss* or even just pruss if
> MFD folks are okay with it?

This...

> > +EXPORT_SYMBOL(pruss_get_clk_freq);

> This looks strange. Why do we need this? There
> is clk_get_rate() API in the kernel would would
> seem more suitable.

...unfortunately conflicts with this - currently all clock API
implementations are unique to the platform and there's no way of adding
clocks that works over multiple platforms.

2011-03-22 09:35:29

by Sekhar Nori

[permalink] [raw]
Subject: RE: [PATCH v3 1/7] mfd: add pruss mfd driver.

Hi Mark,

On Fri, Mar 18, 2011 at 18:09:17, Mark Brown wrote:
> On Fri, Mar 18, 2011 at 05:29:39PM +0530, Nori, Sekhar wrote:
>
> > PRUSS as an IP is not really tied to the DA8xx SoC architecture.
> > So, can you please name the files ti-pruss* or even just pruss if
> > MFD folks are okay with it?
>
> This...
>
> > > +EXPORT_SYMBOL(pruss_get_clk_freq);
>
> > This looks strange. Why do we need this? There
> > is clk_get_rate() API in the kernel would would
> > seem more suitable.
>
> ...unfortunately conflicts with this - currently all clock API
> implementations are unique to the platform and there's no way of adding
> clocks that works over multiple platforms.

Hmm, I guess I have a limited view of this. We do have drivers which
use clock API and work across OMAP and DaVinci series of ARM SoCs.
The clock implementation in this case is under mach-omap and mach-davinci
respectively.

Thanks,
Sekhar

2011-03-22 09:44:08

by Sekhar Nori

[permalink] [raw]
Subject: RE: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Tue, Mar 22, 2011 at 15:05:07, Nori, Sekhar wrote:
> Hi Mark,
>
> On Fri, Mar 18, 2011 at 18:09:17, Mark Brown wrote:
> > On Fri, Mar 18, 2011 at 05:29:39PM +0530, Nori, Sekhar wrote:
> >
> > > PRUSS as an IP is not really tied to the DA8xx SoC architecture.
> > > So, can you please name the files ti-pruss* or even just pruss if
> > > MFD folks are okay with it?
> >
> > This...
> >
> > > > +EXPORT_SYMBOL(pruss_get_clk_freq);
> >
> > > This looks strange. Why do we need this? There
> > > is clk_get_rate() API in the kernel would would
> > > seem more suitable.
> >
> > ...unfortunately conflicts with this - currently all clock API
> > implementations are unique to the platform and there's no way of adding
> > clocks that works over multiple platforms.
>
> Hmm, I guess I have a limited view of this. We do have drivers which
> use clock API and work across OMAP and DaVinci series of ARM SoCs.
> The clock implementation in this case is under mach-omap and mach-davinci

This should have read "under plat-omap/mach-omap2 and mach-davinci"

Thanks,
Sekhar

2011-03-22 16:52:54

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Tue, Mar 22, 2011 at 03:05:07PM +0530, Nori, Sekhar wrote:
> On Fri, Mar 18, 2011 at 18:09:17, Mark Brown wrote:

> > ...unfortunately conflicts with this - currently all clock API
> > implementations are unique to the platform and there's no way of adding
> > clocks that works over multiple platforms.

> Hmm, I guess I have a limited view of this. We do have drivers which
> use clock API and work across OMAP and DaVinci series of ARM SoCs.
> The clock implementation in this case is under mach-omap and mach-davinci
> respectively.

It's mostly fine from the point of view of using clocks, it's if you're
trying to provide a clock that there's no API you can rely on - the
clock API itself only includes the user side and every platform gets to
provide its own implementation.

2011-03-24 13:14:05

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

Hello,

> A device may be de-initialized and another loaded or two different devices
> can be run simultaneously on the two cores.
> It's also possible, as in our case, to implement a single device on both
> the PRU's resulting in improved load sharing.

>Make you driver compile with sparse "make C=1", you cast way too much
>when accessing io mem. Use void __iomem * instead of u32 *.


With make C=1 I am receiving some warnings such as:

warning: cast removes address space of expression
drivers/mfd/da8xx_pru.c:61:17: warning: incorrect type in argument 1
(different base types)
drivers/mfd/da8xx_pru.c:61:17: expected void const volatile [noderef]
<asn:2>*<noident>
drivers/mfd/da8xx_pru.c:61:17: got int
drivers/mfd/da8xx_pru.c:66:28: warning: incorrect type in argument 1
(different address spaces)

I can remove all of these by casting to (__force void __iomem *) but is
this correct.

-SG

2011-03-24 13:25:08

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On 03/24/2011 02:16 PM, Subhasish Ghosh wrote:
> Hello,
>
>> A device may be de-initialized and another loaded or two different
>> devices
>> can be run simultaneously on the two cores.
>> It's also possible, as in our case, to implement a single device on both
>> the PRU's resulting in improved load sharing.
>
>> Make you driver compile with sparse "make C=1", you cast way too much
>> when accessing io mem. Use void __iomem * instead of u32 *.
>
>
> With make C=1 I am receiving some warnings such as:
>
> warning: cast removes address space of expression
> drivers/mfd/da8xx_pru.c:61:17: warning: incorrect type in argument 1
> (different base types)
> drivers/mfd/da8xx_pru.c:61:17: expected void const volatile [noderef]
> <asn:2>*<noident>
> drivers/mfd/da8xx_pru.c:61:17: got int
> drivers/mfd/da8xx_pru.c:66:28: warning: incorrect type in argument 1
> (different address spaces)

(Re)read the warning carefully:

You have to fix drivers/mfd/da8xx_pru.c, the first argument should be
__void iomem *, not an int. Fix the type of the first argument. Don't
use any casts at all, follow all warnings, eventually you will have void
__iomem * (or struct * instead of void *) and no warnings.

> I can remove all of these by casting to (__force void __iomem *) but is
> this correct.

No evil casts, please :)

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |


Attachments:
signature.asc (262.00 B)
OpenPGP digital signature

2011-03-24 13:57:04

by Kurt Van Dijck

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Thu, Mar 24, 2011 at 02:24:51PM +0100, Marc Kleine-Budde wrote:
> On 03/24/2011 02:16 PM, Subhasish Ghosh wrote:
> > Hello,
> >
> > With make C=1 I am receiving some warnings such as:
> >
> > warning: cast removes address space of expression
> > drivers/mfd/da8xx_pru.c:61:17: warning: incorrect type in argument 1
> > (different base types)
> > drivers/mfd/da8xx_pru.c:61:17: expected void const volatile [noderef]
> > <asn:2>*<noident>
> > drivers/mfd/da8xx_pru.c:61:17: got int
> > drivers/mfd/da8xx_pru.c:66:28: warning: incorrect type in argument 1
> > (different address spaces)
>
> (Re)read the warning carefully:
>
> You have to fix drivers/mfd/da8xx_pru.c, the first argument should be
> __void iomem *, not an int. Fix the type of the first argument. Don't
> use any casts at all, follow all warnings, eventually you will have void
> __iomem * (or struct * instead of void *) and no warnings.
>
> > I can remove all of these by casting to (__force void __iomem *) but is
> > this correct.
>
> No evil casts, please :)
In addition, I learned that if a driver is not bound to a specific architecture,
you better use ioread/iowrite & friends, rather that accessing the iomemory
directly.
>
Kurt

2011-03-30 07:13:42

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.


>> +s32 pruss_disable(struct device *dev, u8 pruss_num)
>> +{
>> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
>> + struct da8xx_prusscore_regs *h_pruss;
>> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
>> + u32 temp_reg;
>> + u32 delay_cnt;
>
> Can you explain the significance of pruss_num? As far as I
> can tell, you always pass constants in here, so it should
> be possible to determine the number from the device.

SG - The number is not programmed in the device, I need something to decide
which PRU to disable
or enable.

>> + ~DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK) |
>> + ((DA8XX_PRUCORE_CONTROL_COUNTENABLE_DISABLE <<
>> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_SHIFT) &
>> + DA8XX_PRUCORE_CONTROL_COUNTENABLE_MASK);
>> + __raw_writel(temp_reg, &h_pruss->CONTROL);
>
> Better use readl/writel, the __raw_ variants are not reliable in general.

SG - Have changed this to iowrite32 and variants.


>> +
>> + /* Reset PRU0 */
>> + for (delay_cnt = 0x10000; delay_cnt > 0; delay_cnt--)
>> + __raw_writel(DA8XX_PRUCORE_CONTROL_RESETVAL,
>> + &h_pruss->CONTROL);
>
> Why do you need to reset it 65536 times? Is once not enough?

SG - I am not sure why this was done, but I have removed it now.

>> + }
>> + spin_unlock(&pruss->lock);
>
> This is almost the exact same code as for the DA8XX_PRUCORE_0 case.
> Please be a little more creative in order to avoid such code duplication.

SG - Ok, cleanup done.


>> +
>> + for (loop = 0; loop < bytestowrite; loop++)
>> + __raw_writeb(*pdatatowrite++, paddresstowrite++);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(pruss_writeb);
>
> I would recommend providing a simpler variant of your all I/O accessors,
> which write a single word. Most of the users of these simply
> pass bytestowrite=1 anyway, so the caller can become more readable.

SG - At some sections in the code I am using upto 8 bytescount.
If its ok, I would want to keep it as is.

>
> Also, my comments about __raw_* and Marc's comments about the
> type cast apply to all of these.

SG - Changed to iowrite32 variants and also used make C=1


>> + err = mfd_add_devices(dev, 0, &cell, 1, NULL, 0);
>> + if (err) {
>> + dev_err(dev, "cannot add mfd cells\n");
>> + return err;
>> + }
>> + dev_info(dev, "mfd: added %s device\n",
>> + dev_data[count].dev_name);
>> + }
>> +
>> + return err;
>> +}
>
> This would get much simpler if you just replaced the da8xx_pruss_devices
> array with an mfd_cell array.

SG - Will do.

2011-03-30 09:12:51

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

Hi Arnd,

I am using a .setup to initialize pin mux etc. To use the mfd_cells directly
should I
use .enable and .disable of the mfd_cells instead of the .setup.

>> +static int pruss_mfd_add_devices(struct platform_device *pdev)
>> +{
>> + struct da8xx_pruss_devices *dev_data = pdev->dev.platform_data;
>> + struct device *dev = &pdev->dev;
>> + struct mfd_cell cell;
>> + u32 err, count;
>> +
>> + for (count = 0; dev_data[count].dev_name != NULL; count++) {
>> + memset(&cell, 0, sizeof(struct mfd_cell));
>> + cell.id = count;
>> + cell.name = dev_data[count].dev_name;
>> + cell.platform_data = dev_data[count].pdata;
>> + cell.data_size = dev_data[count].pdata_size;
>> + cell.resources = dev_data[count].resources;
>> + cell.num_resources = dev_data[count].num_resources;
>> +
>> + err = mfd_add_devices(dev, 0, &cell, 1, NULL, 0);
>> + if (err) {
>> + dev_err(dev, "cannot add mfd cells\n");
>> + return err;
>> + }
>> + dev_info(dev, "mfd: added %s device\n",
>> + dev_data[count].dev_name);
>> + }
>> +
>> + return err;
>> +}
>
> This would get much simpler if you just replaced the da8xx_pruss_devices
> array with an mfd_cell array.

2011-03-30 10:59:39

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Wednesday 30 March 2011, Subhasish Ghosh wrote:
>
> >> +s32 pruss_disable(struct device *dev, u8 pruss_num)
> >> +{
> >> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
> >> + struct da8xx_prusscore_regs *h_pruss;
> >> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
> >> + u32 temp_reg;
> >> + u32 delay_cnt;
> >
> > Can you explain the significance of pruss_num? As far as I
> > can tell, you always pass constants in here, so it should
> > be possible to determine the number from the device.
>
> SG - The number is not programmed in the device, I need something to decide
> which PRU to disable or enable.

I still don't understand. Please explain how the devices
relate to the multiple PRUs in hardware.

> >> + for (loop = 0; loop < bytestowrite; loop++)
> >> + __raw_writeb(*pdatatowrite++, paddresstowrite++);
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL(pruss_writeb);
> >
> > I would recommend providing a simpler variant of your all I/O accessors,
> > which write a single word. Most of the users of these simply
> > pass bytestowrite=1 anyway, so the caller can become more readable.
>
> SG - At some sections in the code I am using upto 8 bytescount.
> If its ok, I would want to keep it as is.

You can of course have both, but I would recommend making the common
case simpler by providing a version that writes just one word.

Arnd

2011-03-30 11:39:22

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.

On Wednesday 30 March 2011, Subhasish Ghosh wrote:
> I am using a .setup to initialize pin mux etc. To use the mfd_cells directly
> should I
> use .enable and .disable of the mfd_cells instead of the .setup.

I think it would be best not to define any per-board functions that
are needed to set up the cell, but rather to initialize it from
the ->probe function of the cell specific driver.

If you need board specific data such as GPIO lists, you can pass
them in the mfd_cell platform_data.

Arnd

2011-04-05 06:37:59

by Subhasish Ghosh

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] mfd: add pruss mfd driver.


> On Wednesday 30 March 2011, Subhasish Ghosh wrote:
>>
>> >> +s32 pruss_disable(struct device *dev, u8 pruss_num)
>> >> +{
>> >> + struct da8xx_pruss *pruss = dev_get_drvdata(dev->parent);
>> >> + struct da8xx_prusscore_regs *h_pruss;
>> >> + struct pruss_map *pruss_mmap = (struct pruss_map *)pruss->ioaddr;
>> >> + u32 temp_reg;
>> >> + u32 delay_cnt;
>> >
>> > Can you explain the significance of pruss_num? As far as I
>> > can tell, you always pass constants in here, so it should
>> > be possible to determine the number from the device.
>>
>> SG - The number is not programmed in the device, I need something to
>> decide
>> which PRU to disable or enable.
>
> I still don't understand. Please explain how the devices
> relate to the multiple PRUs in hardware.

There are two devices, CAN and UART, in our case we use the PRU as follows:
1. CAN-TX on PRU0, CAN-RX on PRU1
2. SUART-TX on PRU0, SUART-RX on PRU1
3. SUART-TXRX on PRU0, SUART-TXRX on PRU1