| Message ID | 1351530381-11459-2-git-send-email-omar.luna@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
* 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
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
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
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
* 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 --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 */
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