RE: [RFC 4/6] omap: introduce common remoteproc module

2010-07-27 Thread Premi, Sanjeev
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org
 [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Ohad Ben-Cohen
 Sent: Friday, July 02, 2010 3:53 AM
 To: linux-omap@vger.kernel.org
 Cc: Kanigeri, Hari; Ben-cohen, Ohad
 Subject: [RFC 4/6] omap: introduce common remoteproc module

 From: Ohad Ben-Cohen oh...@ti.com

 The OMAP remote processor module decouples
 machine-specific code from TI's IPC
 drivers (e.g. dspbridge and syslink).

 While dspbridge calls the remoteproc handlers
 from the kernel, syslink calls them from
 user space. Hence remoteproc supports both
 interfaces.

 Signed-off-by: Ohad Ben-Cohen oh...@ti.com
 Signed-off-by: Hari Kanigeri h-kanige...@ti.com
 ---
  arch/arm/plat-omap/include/plat/remoteproc.h |   75 ++
  arch/arm/plat-omap/remoteproc.c  |  316
 ++
  2 files changed, 391 insertions(+), 0 deletions(-)
  create mode 100644 arch/arm/plat-omap/include/plat/remoteproc.h
  create mode 100644 arch/arm/plat-omap/remoteproc.c

 diff --git a/arch/arm/plat-omap/include/plat/remoteproc.h
 b/arch/arm/plat-omap/include/plat/remoteproc.h
 new file mode 100644
 index 000..1cedd08
 --- /dev/null
 +++ b/arch/arm/plat-omap/include/plat/remoteproc.h
 @@ -0,0 +1,75 @@
 +/*
 + * OMAP Remote Processor driver
 + *
 + * Copyright (C) 2010 Texas Instruments Inc.
 + *
 + * Written by Ohad Ben-Cohen o...@wizery.com
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License
 + * version 2 as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be
 useful, but
 + * WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 + * 02110-1301 USA
 + *
 + */
 +
 +#ifndef REMOTEPROC_H
 +#define REMOTEPROC_H
 +
 +#include linux/ioctl.h
 +#include linux/cdev.h
 +
 +#define RPROC_IOC_MAGIC  'P'

[sp] I notice that there is already a conflict with this magic number in
 Documentation/ioctl/ioctl-number.txt.

 Suggest reserving a different code as per the steps mentioned in the
 same file.

 +
 +#define RPROC_IOCSTART   _IOW(RPROC_IOC_MAGIC, 1, int)
 +#define RPROC_IOCSTOP_IO(RPROC_IOC_MAGIC, 2)
 +#define RPROC_IOCGETSTATE_IOR(RPROC_IOC_MAGIC, 3, int)
 +
 +#define RPROC_IOC_MAXNR  (3)
 +
 +struct omap_rproc;
 +
 +struct omap_rproc_ops {
 + int (*start)(struct device *dev, u32 start_addr);
 + int (*stop)(struct device *dev);
 + int (*get_state)(struct device *dev);
 +};
 +
 +struct omap_rproc_clk_t {
 + void *clk_handle;
 + const char *dev_id;
 + const char *con_id;
 +};
 +

[sp] Small description of the fields in the structure will help understand the 
code better.
 I couldn't locate usage of con_id in this patch; not sure what it 
signifies.
 ...hope it becomes clear in one of the other patches in the series.
 (after one review) The structure is not used anywhere in this file.

 +struct omap_rproc_platform_data {
 + struct omap_rproc_ops *ops;
 + char *name;
 + char *oh_name;
 +};

[sp] same comment here.

 +
 +struct omap_rproc {
 + struct device *dev;
 + struct cdev cdev;
 + atomic_t count;

[sp] I believe this field is for use-counts. If so, suggest renaming to
 usecount.

 + int minor;
 +};
 +
 +struct omap_rproc_start_args {
 + u32 start_addr;
 +};
 +
 +struct omap_rproc_platform_data *omap3_get_rproc_data(void);
 +int omap3_get_rproc_data_size(void);
 +struct omap_rproc_platform_data *omap4_get_rproc_data(void);
 +int omap4_get_rproc_data_size(void);
[sp] The forward declaration seems to be used for plugging
 device specific data (haven't looked at complet code, so
 it is currently a speculation).

 The contents of this header have so far been device independent.
 Can we look to reorganiza this code such that this header remains
 generic?

 +int omap_get_num_of_remoteproc(void);

[sp] Don't see this being implemented in the C file below.
 If this function is expected to be implemented in the omap3/4
 specific file, then I suspect multi-omap will break for compilation.
 Same function will be defined multiple times.

 +
 +#endif /* REMOTEPROC_H */
 diff --git a/arch/arm/plat-omap/remoteproc.c
 b/arch/arm/plat-omap/remoteproc.c
 new file mode 100644
 index 000..7a9862e
 --- /dev/null
 +++ b/arch/arm/plat-omap/remoteproc.c
 @@ -0,0 +1,316 @@
 +/*
 + * OMAP Remote Processor driver
 + *
 + * Copyright (C) 2010 Texas Instruments Inc.
 + *
 + * Authors:
 + * Ohad Ben-Cohen o...@wizery.com
 + * Hari Kanigeri h-kanige...@ti.com

[RFC 4/6] omap: introduce common remoteproc module

2010-07-01 Thread Ohad Ben-Cohen
From: Ohad Ben-Cohen oh...@ti.com

The OMAP remote processor module decouples
machine-specific code from TI's IPC
drivers (e.g. dspbridge and syslink).

While dspbridge calls the remoteproc handlers
from the kernel, syslink calls them from
user space. Hence remoteproc supports both
interfaces.

Signed-off-by: Ohad Ben-Cohen oh...@ti.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
---
 arch/arm/plat-omap/include/plat/remoteproc.h |   75 ++
 arch/arm/plat-omap/remoteproc.c  |  316 ++
 2 files changed, 391 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/remoteproc.h
 create mode 100644 arch/arm/plat-omap/remoteproc.c

diff --git a/arch/arm/plat-omap/include/plat/remoteproc.h 
b/arch/arm/plat-omap/include/plat/remoteproc.h
new file mode 100644
index 000..1cedd08
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/remoteproc.h
@@ -0,0 +1,75 @@
+/*
+ * OMAP Remote Processor driver
+ *
+ * Copyright (C) 2010 Texas Instruments Inc.
+ *
+ * Written by Ohad Ben-Cohen o...@wizery.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef REMOTEPROC_H
+#define REMOTEPROC_H
+
+#include linux/ioctl.h
+#include linux/cdev.h
+
+#define RPROC_IOC_MAGIC'P'
+
+#define RPROC_IOCSTART _IOW(RPROC_IOC_MAGIC, 1, int)
+#define RPROC_IOCSTOP  _IO(RPROC_IOC_MAGIC, 2)
+#define RPROC_IOCGETSTATE  _IOR(RPROC_IOC_MAGIC, 3, int)
+
+#define RPROC_IOC_MAXNR(3)
+
+struct omap_rproc;
+
+struct omap_rproc_ops {
+   int (*start)(struct device *dev, u32 start_addr);
+   int (*stop)(struct device *dev);
+   int (*get_state)(struct device *dev);
+};
+
+struct omap_rproc_clk_t {
+   void *clk_handle;
+   const char *dev_id;
+   const char *con_id;
+};
+
+struct omap_rproc_platform_data {
+   struct omap_rproc_ops *ops;
+   char *name;
+   char *oh_name;
+};
+
+struct omap_rproc {
+   struct device *dev;
+   struct cdev cdev;
+   atomic_t count;
+   int minor;
+};
+
+struct omap_rproc_start_args {
+   u32 start_addr;
+};
+
+struct omap_rproc_platform_data *omap3_get_rproc_data(void);
+int omap3_get_rproc_data_size(void);
+struct omap_rproc_platform_data *omap4_get_rproc_data(void);
+int omap4_get_rproc_data_size(void);
+int omap_get_num_of_remoteproc(void);
+
+#endif /* REMOTEPROC_H */
diff --git a/arch/arm/plat-omap/remoteproc.c b/arch/arm/plat-omap/remoteproc.c
new file mode 100644
index 000..7a9862e
--- /dev/null
+++ b/arch/arm/plat-omap/remoteproc.c
@@ -0,0 +1,316 @@
+/*
+ * OMAP Remote Processor driver
+ *
+ * Copyright (C) 2010 Texas Instruments Inc.
+ *
+ * Authors:
+ * Ohad Ben-Cohen o...@wizery.com
+ * Hari Kanigeri h-kanige...@ti.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/interrupt.h
+#include linux/device.h
+#include linux/delay.h
+#include linux/file.h
+#include linux/poll.h
+#include linux/mm.h
+#include linux/platform_device.h
+
+#include plat/remoteproc.h
+
+#define OMAP_RPROC_NAME omap-rproc
+
+static struct class *omap_rproc_class;
+static dev_t omap_rproc_dev;
+static atomic_t num_of_rprocs;
+
+static inline int rproc_start(struct omap_rproc *rproc, const void __user *arg)
+{
+   struct omap_rproc_platform_data *pdata;
+   struct omap_rproc_start_args start_args;
+
+   if (!rproc-dev)
+   return -EINVAL;
+
+   pdata = rproc-dev-platform_data;
+   if (!pdata-ops)
+   return -EINVAL;
+
+   if (copy_from_user(start_args, arg, sizeof(start_args)))
+   return -EFAULT;
+
+   return pdata-ops-start(rproc-dev, start_args.start_addr);
+}
+
+static inline int