diff mbox

[1/2] ARM: OMAP2+: move mailbox.h out of plat-omap headers

Message ID 1351530381-11459-2-git-send-email-omar.luna@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Omar Ramirez Luna Oct. 29, 2012, 5:06 p.m. UTC
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: devel@driverdev.osuosl.org
Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
---
 arch/arm/mach-omap2/mailbox.c                      |    2 +-
 arch/arm/plat-omap/include/plat/mailbox.h          |  105 --------------------
 arch/arm/plat-omap/mailbox.c                       |    2 +-
 .../tidspbridge/include/dspbridge/host_os.h        |    2 +-
 include/linux/platform_data/omap_mailbox.h         |  105 ++++++++++++++++++++
 5 files changed, 108 insertions(+), 108 deletions(-)
 delete mode 100644 arch/arm/plat-omap/include/plat/mailbox.h
 create mode 100644 include/linux/platform_data/omap_mailbox.h

Comments

Tony Lindgren Oct. 29, 2012, 5:52 p.m. UTC | #1
Hi,

> --- /dev/null
> +++ b/include/linux/platform_data/omap_mailbox.h
> @@ -0,0 +1,105 @@

This file should only contain pure platform data needed
by the core omap code to pass to the mailbox driver.

The mailbox API header should be somewhere else,
like include/linux/mailbox/mailbox-omap.h or similar.

But shouldn't this all now be handled by using the
remoteproc framework?

Regards,

Tony
Omar Ramirez Luna Oct. 30, 2012, 12:18 p.m. UTC | #2
Tony,

On 29 October 2012 12:52, Tony Lindgren <tony@atomide.com> wrote:
>> --- /dev/null
>> +++ b/include/linux/platform_data/omap_mailbox.h
>> @@ -0,0 +1,105 @@
>
> This file should only contain pure platform data needed
> by the core omap code to pass to the mailbox driver.

Ok, looking at it closely, this header file is related to the API
itself, there is nothing that could be actually considered as pure
platform data, the structures are related with the mailbox framework
and even if I split this file into two, the additional header would
end up including the "platform_data" header unless I move
save/restore_ctx functions and then export them as symbols for the
API.

So, it might be better for the entire file to sit in
linux/include/mailbox/ then.

> The mailbox API header should be somewhere else,
> like include/linux/mailbox/mailbox-omap.h or similar.

Ok.

> But shouldn't this all now be handled by using the
> remoteproc framework?

Remoteproc doesn't handle the mailbox hardware directly, it still
relies in the mailbox framework for the low level communications.
E.g.: Proc1 has a message (virtqueue msg) queued to Proc2, uses
mailbox msg to generate an interrupt to Proc2, Proc2 queries the
message (virtqueue) based on the mailbox message received.

Regards,

Omar
Tony Lindgren Oct. 30, 2012, 4:24 p.m. UTC | #3
* Omar Ramirez Luna <omar.luna@linaro.org> [121030 05:20]:
> Tony,
> 
> On 29 October 2012 12:52, Tony Lindgren <tony@atomide.com> wrote:
> >> --- /dev/null
> >> +++ b/include/linux/platform_data/omap_mailbox.h
> >> @@ -0,0 +1,105 @@
> >
> > This file should only contain pure platform data needed
> > by the core omap code to pass to the mailbox driver.
> 
> Ok, looking at it closely, this header file is related to the API
> itself, there is nothing that could be actually considered as pure
> platform data, the structures are related with the mailbox framework
> and even if I split this file into two, the additional header would
> end up including the "platform_data" header unless I move
> save/restore_ctx functions and then export them as symbols for the
> API.
> 
> So, it might be better for the entire file to sit in
> linux/include/mailbox/ then.

OK to me.
 
> > The mailbox API header should be somewhere else,
> > like include/linux/mailbox/mailbox-omap.h or similar.
> 
> Ok.
> 
> > But shouldn't this all now be handled by using the
> > remoteproc framework?
> 
> Remoteproc doesn't handle the mailbox hardware directly, it still
> relies in the mailbox framework for the low level communications.
> E.g.: Proc1 has a message (virtqueue msg) queued to Proc2, uses
> mailbox msg to generate an interrupt to Proc2, Proc2 queries the
> message (virtqueue) based on the mailbox message received.

OK.

Greg, do these patches look OK to you to move to live under
drivers/mailbox?

Regards,

Tony
Greg KH Oct. 30, 2012, 9:02 p.m. UTC | #4
On Tue, Oct 30, 2012 at 09:24:42AM -0700, Tony Lindgren wrote:
> * Omar Ramirez Luna <omar.luna@linaro.org> [121030 05:20]:
> > Tony,
> > 
> > On 29 October 2012 12:52, Tony Lindgren <tony@atomide.com> wrote:
> > >> --- /dev/null
> > >> +++ b/include/linux/platform_data/omap_mailbox.h
> > >> @@ -0,0 +1,105 @@
> > >
> > > This file should only contain pure platform data needed
> > > by the core omap code to pass to the mailbox driver.
> > 
> > Ok, looking at it closely, this header file is related to the API
> > itself, there is nothing that could be actually considered as pure
> > platform data, the structures are related with the mailbox framework
> > and even if I split this file into two, the additional header would
> > end up including the "platform_data" header unless I move
> > save/restore_ctx functions and then export them as symbols for the
> > API.
> > 
> > So, it might be better for the entire file to sit in
> > linux/include/mailbox/ then.
> 
> OK to me.
>  
> > > The mailbox API header should be somewhere else,
> > > like include/linux/mailbox/mailbox-omap.h or similar.
> > 
> > Ok.
> > 
> > > But shouldn't this all now be handled by using the
> > > remoteproc framework?
> > 
> > Remoteproc doesn't handle the mailbox hardware directly, it still
> > relies in the mailbox framework for the low level communications.
> > E.g.: Proc1 has a message (virtqueue msg) queued to Proc2, uses
> > mailbox msg to generate an interrupt to Proc2, Proc2 queries the
> > message (virtqueue) based on the mailbox message received.
> 
> OK.
> 
> Greg, do these patches look OK to you to move to live under
> drivers/mailbox?

Um, I don't know, I wasn't paying attention here, sorry.

greg k-h
Omar Ramirez Luna Oct. 31, 2012, 7:22 a.m. UTC | #5
Hi Greg,

On 30 October 2012 16:02, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>> OK.
>>
>> Greg, do these patches look OK to you to move to live under
>> drivers/mailbox?
>
> Um, I don't know, I wasn't paying attention here, sorry.

As part of plat-omap code cleanup, I was planning to move omap-mailbox
framework to a newly drivers/mailbox folder, right now this code is
specific to OMAP platforms, but with some clean up it could be the
base for a generic framework. And living under drivers/mailbox could
give it some exposure for interested developers to give feedback and
propose changes.

In the past there was a mailbox-like driver in mach-ux500, I was
hoping both could live under the same folder, however the platform
using it, was dropped a couple of kernel releases back and with it the
other similar mailbox implementation.

So, here it is the thread with the patches:
http://thread.gmane.org/gmane.linux.kernel/1384603, if you think it is
OK for them to be moved under drivers/mailbox, I just need to re-spin
them to move the mailbox header file to include/linux/mailbox instead
of include/linux/platform_data.

Cheers,

Omar
Loic PALLARDY Oct. 31, 2012, 8:45 a.m. UTC | #6
Hi Omar,

On 10/31/2012 08:22 AM, Omar Ramirez Luna wrote:
>
> As part of plat-omap code cleanup, I was planning to move omap-mailbox
> framework to a newly drivers/mailbox folder, right now this code is
> specific to OMAP platforms, but with some clean up it could be the
> base for a generic framework. And living under drivers/mailbox could
> give it some exposure for interested developers to give feedback and
> propose changes.
>
> In the past there was a mailbox-like driver in mach-ux500, I was
> hoping both could live under the same folder, however the platform
> using it, was dropped a couple of kernel releases back and with it the
> other similar mailbox implementation.

On STE side, we are working on a mailbox driver which will rely on 
mailbox framework.
However some modifications in current Omap mailbox framework are needed 
to fit STE HW specificities.
- API naming should be generic (replace omap prefix by mailbox)
- message type should be enhanced
- fifo mechanism is linked to Omap maibox, not needed in STE case since 
it relies on cross interrupt + shared memory
I already have modifications I'll send you to see how we can create a 
common and generic mailbox framework.

Regards
Loic
Tony Lindgren Oct. 31, 2012, 6:24 p.m. UTC | #7
* Loic PALLARDY <loic.pallardy@st.com> [121031 01:48]:
> 
> Hi Omar,
> 
> On 10/31/2012 08:22 AM, Omar Ramirez Luna wrote:
> >
> > As part of plat-omap code cleanup, I was planning to move omap-mailbox
> > framework to a newly drivers/mailbox folder, right now this code is
> > specific to OMAP platforms, but with some clean up it could be the
> > base for a generic framework. And living under drivers/mailbox could
> > give it some exposure for interested developers to give feedback and
> > propose changes.
> >
> > In the past there was a mailbox-like driver in mach-ux500, I was
> > hoping both could live under the same folder, however the platform
> > using it, was dropped a couple of kernel releases back and with it the
> > other similar mailbox implementation.
> 
> On STE side, we are working on a mailbox driver which will rely on 
> mailbox framework.
> However some modifications in current Omap mailbox framework are needed 
> to fit STE HW specificities.
> - API naming should be generic (replace omap prefix by mailbox)
> - message type should be enhanced
> - fifo mechanism is linked to Omap maibox, not needed in STE case since 
> it relies on cross interrupt + shared memory
> I already have modifications I'll send you to see how we can create a 
> common and generic mailbox framework.

OK cool. How about Omar first posts the fixed minimal patches
to move things to live under drivers so can fix up omap for
ARCH_MULTIPLATFORM?

I can then put just those into an immutable branch that people
can pull in as needed, and you guys can continue from there with
the common mailbox framework (and I can continue work on the
core omap code independently of the mailbox framwork work).

Regards,

Tony
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 0d97456..f69e659 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -17,7 +17,7 @@ 
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
 
-#include <plat/mailbox.h>
+#include <linux/platform_data/omap_mailbox.h>
 
 #include "soc.h"
 
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
deleted file mode 100644
index cc3921e..0000000
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ /dev/null
@@ -1,105 +0,0 @@ 
-/* mailbox.h */
-
-#ifndef MAILBOX_H
-#define MAILBOX_H
-
-#include <linux/spinlock.h>
-#include <linux/workqueue.h>
-#include <linux/interrupt.h>
-#include <linux/device.h>
-#include <linux/kfifo.h>
-
-typedef u32 mbox_msg_t;
-struct omap_mbox;
-
-typedef int __bitwise omap_mbox_irq_t;
-#define IRQ_TX ((__force omap_mbox_irq_t) 1)
-#define IRQ_RX ((__force omap_mbox_irq_t) 2)
-
-typedef int __bitwise omap_mbox_type_t;
-#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
-#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
-
-struct omap_mbox_ops {
-	omap_mbox_type_t	type;
-	int		(*startup)(struct omap_mbox *mbox);
-	void		(*shutdown)(struct omap_mbox *mbox);
-	/* fifo */
-	mbox_msg_t	(*fifo_read)(struct omap_mbox *mbox);
-	void		(*fifo_write)(struct omap_mbox *mbox, mbox_msg_t msg);
-	int		(*fifo_empty)(struct omap_mbox *mbox);
-	int		(*fifo_full)(struct omap_mbox *mbox);
-	/* irq */
-	void		(*enable_irq)(struct omap_mbox *mbox,
-						omap_mbox_irq_t irq);
-	void		(*disable_irq)(struct omap_mbox *mbox,
-						omap_mbox_irq_t irq);
-	void		(*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
-	int		(*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
-	/* ctx */
-	void		(*save_ctx)(struct omap_mbox *mbox);
-	void		(*restore_ctx)(struct omap_mbox *mbox);
-};
-
-struct omap_mbox_queue {
-	spinlock_t		lock;
-	struct kfifo		fifo;
-	struct work_struct	work;
-	struct tasklet_struct	tasklet;
-	struct omap_mbox	*mbox;
-	bool full;
-};
-
-struct omap_mbox {
-	char			*name;
-	unsigned int		irq;
-	struct omap_mbox_queue	*txq, *rxq;
-	struct omap_mbox_ops	*ops;
-	struct device		*dev;
-	void			*priv;
-	int			use_count;
-	struct blocking_notifier_head   notifier;
-};
-
-int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
-void omap_mbox_init_seq(struct omap_mbox *);
-
-struct omap_mbox *omap_mbox_get(const char *, struct notifier_block *nb);
-void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb);
-
-int omap_mbox_register(struct device *parent, struct omap_mbox **);
-int omap_mbox_unregister(void);
-
-static inline void omap_mbox_save_ctx(struct omap_mbox *mbox)
-{
-	if (!mbox->ops->save_ctx) {
-		dev_err(mbox->dev, "%s:\tno save\n", __func__);
-		return;
-	}
-
-	mbox->ops->save_ctx(mbox);
-}
-
-static inline void omap_mbox_restore_ctx(struct omap_mbox *mbox)
-{
-	if (!mbox->ops->restore_ctx) {
-		dev_err(mbox->dev, "%s:\tno restore\n", __func__);
-		return;
-	}
-
-	mbox->ops->restore_ctx(mbox);
-}
-
-static inline void omap_mbox_enable_irq(struct omap_mbox *mbox,
-					omap_mbox_irq_t irq)
-{
-	mbox->ops->enable_irq(mbox, irq);
-}
-
-static inline void omap_mbox_disable_irq(struct omap_mbox *mbox,
-					 omap_mbox_irq_t irq)
-{
-	mbox->ops->disable_irq(mbox, irq);
-}
-
-#endif /* MAILBOX_H */
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 42377ef..cae8692 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@ 
 #include <linux/notifier.h>
 #include <linux/module.h>
 
-#include <plat/mailbox.h>
+#include <linux/platform_data/omap_mailbox.h>
 
 static struct omap_mbox **mboxes;
 
diff --git a/drivers/staging/tidspbridge/include/dspbridge/host_os.h b/drivers/staging/tidspbridge/include/dspbridge/host_os.h
index 896f157..bff9e3a 100644
--- a/drivers/staging/tidspbridge/include/dspbridge/host_os.h
+++ b/drivers/staging/tidspbridge/include/dspbridge/host_os.h
@@ -41,11 +41,11 @@ 
 #include <linux/ioport.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
-#include <plat/mailbox.h>
 #include <linux/pagemap.h>
 #include <asm/cacheflush.h>
 #include <linux/dma-mapping.h>
 
+#include <linux/platform_data/omap_mailbox.h>
 /* TODO -- Remove, once BP defines them */
 #define INT_DSP_MMU_IRQ        28
 
diff --git a/include/linux/platform_data/omap_mailbox.h b/include/linux/platform_data/omap_mailbox.h
new file mode 100644
index 0000000..cc3921e
--- /dev/null
+++ b/include/linux/platform_data/omap_mailbox.h
@@ -0,0 +1,105 @@ 
+/* mailbox.h */
+
+#ifndef MAILBOX_H
+#define MAILBOX_H
+
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/kfifo.h>
+
+typedef u32 mbox_msg_t;
+struct omap_mbox;
+
+typedef int __bitwise omap_mbox_irq_t;
+#define IRQ_TX ((__force omap_mbox_irq_t) 1)
+#define IRQ_RX ((__force omap_mbox_irq_t) 2)
+
+typedef int __bitwise omap_mbox_type_t;
+#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
+#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+
+struct omap_mbox_ops {
+	omap_mbox_type_t	type;
+	int		(*startup)(struct omap_mbox *mbox);
+	void		(*shutdown)(struct omap_mbox *mbox);
+	/* fifo */
+	mbox_msg_t	(*fifo_read)(struct omap_mbox *mbox);
+	void		(*fifo_write)(struct omap_mbox *mbox, mbox_msg_t msg);
+	int		(*fifo_empty)(struct omap_mbox *mbox);
+	int		(*fifo_full)(struct omap_mbox *mbox);
+	/* irq */
+	void		(*enable_irq)(struct omap_mbox *mbox,
+						omap_mbox_irq_t irq);
+	void		(*disable_irq)(struct omap_mbox *mbox,
+						omap_mbox_irq_t irq);
+	void		(*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+	int		(*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+	/* ctx */
+	void		(*save_ctx)(struct omap_mbox *mbox);
+	void		(*restore_ctx)(struct omap_mbox *mbox);
+};
+
+struct omap_mbox_queue {
+	spinlock_t		lock;
+	struct kfifo		fifo;
+	struct work_struct	work;
+	struct tasklet_struct	tasklet;
+	struct omap_mbox	*mbox;
+	bool full;
+};
+
+struct omap_mbox {
+	char			*name;
+	unsigned int		irq;
+	struct omap_mbox_queue	*txq, *rxq;
+	struct omap_mbox_ops	*ops;
+	struct device		*dev;
+	void			*priv;
+	int			use_count;
+	struct blocking_notifier_head   notifier;
+};
+
+int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
+void omap_mbox_init_seq(struct omap_mbox *);
+
+struct omap_mbox *omap_mbox_get(const char *, struct notifier_block *nb);
+void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb);
+
+int omap_mbox_register(struct device *parent, struct omap_mbox **);
+int omap_mbox_unregister(void);
+
+static inline void omap_mbox_save_ctx(struct omap_mbox *mbox)
+{
+	if (!mbox->ops->save_ctx) {
+		dev_err(mbox->dev, "%s:\tno save\n", __func__);
+		return;
+	}
+
+	mbox->ops->save_ctx(mbox);
+}
+
+static inline void omap_mbox_restore_ctx(struct omap_mbox *mbox)
+{
+	if (!mbox->ops->restore_ctx) {
+		dev_err(mbox->dev, "%s:\tno restore\n", __func__);
+		return;
+	}
+
+	mbox->ops->restore_ctx(mbox);
+}
+
+static inline void omap_mbox_enable_irq(struct omap_mbox *mbox,
+					omap_mbox_irq_t irq)
+{
+	mbox->ops->enable_irq(mbox, irq);
+}
+
+static inline void omap_mbox_disable_irq(struct omap_mbox *mbox,
+					 omap_mbox_irq_t irq)
+{
+	mbox->ops->disable_irq(mbox, irq);
+}
+
+#endif /* MAILBOX_H */