by Hiroshi DOYU

[permalink] [raw]

Subject: Re: [PATCH 01/10] omap mailbox: cleanup omap2 register definition with macro

From: "ext roel kluin" <[email protected]>
Subject: Re: [PATCH 01/10] omap mailbox: cleanup omap2 register definition with macro
Date: Fri, 16 Jan 2009 10:56:01 +0100

[...]
> > +#define MAILBOX_SYSCONFIG 0x010
> > +#define MAILBOX_SYSSTATUS 0x014
> > +#define MAILBOX_MESSAGE(m) (0x040 + 4 * (m))
> > +#define MAILBOX_FIFOSTATUS(m) (0x080 + 4 * (m))
> > +#define MAILBOX_MSGSTATUS(m) (0x0c0 + 4 * (m))
> > +#define MAILBOX_IRQSTATUS(u) (0x100 + 8 * (u))
> > +#define MAILBOX_IRQENABLE(u) (0x108 + 8 * (u))
> ^^^^^^^
> shouldn't this be
> #define MAILBOX_IRQENABLE(u) (0x104 + 8 * (u))

Right. I'll fix, thanks.

2009-01-16 13:21:37

omap mailbox: add save_/restore_ctx() for PM

From: Hiroshi DOYU <[email protected]>

To preserve the registers during off-mode

Signed-off-by: Hiroshi DOYU <[email protected]>
---

arch/arm/mach-omap2/mailbox.c | 32 +++++++++++++++++++++++++++++
arch/arm/plat-omap/include/mach/mailbox.h | 23 +++++++++++++++++++++
2 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 5751013..0a88243 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -32,6 +32,9 @@
#define MAILBOX_IRQ_NEWMSG(u) (1 << (2 * (u)))
#define MAILBOX_IRQ_NOTFULL(u) (1 << (2 * (u) + 1))

+#define MBOX_REG_SIZE 0x120
+#define MBOX_NR_REGS (MBOX_REG_SIZE / sizeof(u32))
+
static void __iomem *mbox_base;

struct omap_mbox2_fifo {
@@ -47,6 +50,7 @@ struct omap_mbox2_priv {
unsigned long irqstatus;
u32 newmsg_bit;
u32 notfull_bit;
+ u32 ctx[MBOX_NR_REGS];
};

static struct clk *mbox_ick_handle;
@@ -167,6 +171,32 @@ static int omap2_mbox_is_irq(struct omap_mbox *mbox,
return (enable & status & bit);
}

+static void omap2_mbox_save_ctx(struct omap_mbox *mbox)
+{
+ int i;
+ struct omap_mbox2_priv *p = mbox->priv;
+
+ for (i = 0; i < MBOX_NR_REGS; i++) {
+ p->ctx[i] = mbox_read_reg(i * sizeof(u32));
+
+ dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
+ i, p->ctx[i]);
+ }
+}
+
+static void omap2_mbox_restore_ctx(struct omap_mbox *mbox)
+{
+ int i;
+ struct omap_mbox2_priv *p = mbox->priv;
+
+ for (i = 0; i < MBOX_NR_REGS; i++) {
+ mbox_write_reg(p->ctx[i], i * sizeof(u32));
+
+ dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
+ i, p->ctx[i]);
+ }
+}
+
static struct omap_mbox_ops omap2_mbox_ops = {
.type = OMAP_MBOX_TYPE2,
.startup = omap2_mbox_startup,
@@ -179,6 +209,8 @@ static struct omap_mbox_ops omap2_mbox_ops = {
.disable_irq = omap2_mbox_disable_irq,
.ack_irq = omap2_mbox_ack_irq,
.is_irq = omap2_mbox_is_irq,
+ .save_ctx = omap2_mbox_save_ctx,
+ .restore_ctx = omap2_mbox_restore_ctx,
};

/*
diff --git a/arch/arm/plat-omap/include/mach/mailbox.h b/arch/arm/plat-omap/include/mach/mailbox.h
index 577db68..b7a6991 100644
--- a/arch/arm/plat-omap/include/mach/mailbox.h
+++ b/arch/arm/plat-omap/include/mach/mailbox.h
@@ -33,6 +33,9 @@ struct omap_mbox_ops {
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 {
@@ -70,4 +73,24 @@ void omap_mbox_put(struct omap_mbox *);
int omap_mbox_register(struct device *parent, struct omap_mbox *);
int omap_mbox_unregister(struct omap_mbox *);

+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);
+}
+
#endif /* MAILBOX_H */

Attachments:

omap-mailbox-add-save_-restore (3.20 kB)

2009-01-19 22:00:36

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH 07/10] omap mailbox: add save_/restore_ctx() for PM

On Mon, Jan 19, 2009 at 12:36:12PM +0200, Hiroshi DOYU wrote:
> Hi Russell, Thank you for your review.
>
> From: "ext Russell King - ARM Linux" <[email protected]>
> Subject: Re: [PATCH 07/10] omap mailbox: add save_/restore_ctx() for PM
> Date: Sat, 17 Jan 2009 17:20:37 +0000
>
> > On Sat, Jan 17, 2009 at 05:10:44PM +0000, Russell King - ARM Linux wrote:
> > > On Fri, Jan 16, 2009 at 10:27:37AM +0200, Hiroshi DOYU wrote:
> > > > diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> > > > index a877305..544dde9 100644
> > > > --- a/arch/arm/mach-omap2/mailbox.c
> > > > +++ b/arch/arm/mach-omap2/mailbox.c
>
> [...]
>
> > Hmm, don't forget 'i' is an offset not an index, so...
> >
> > ?define MBOX_NR_REGS (MBOX_REG_SIZE / sizeof(u32))
> >
> > for (i = 0; i < MBOX_NR_REGS; i++) {
> > p->ctx[i] = mbox_read_reg(i * sizeof(u32));
>
> The updated one is attached.

... which looks much better, thanks.

2009-01-28 14:50:25

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH 05/10] omap mailbox: fix empty struct device for omap1

On Fri, Jan 16, 2009 at 10:27:27AM +0200, Hiroshi DOYU wrote:
> @@ -17,6 +17,8 @@
> #include <mach/mailbox.h>
> #include <mach/irqs.h>
>
> +#define DRV_NAME "omap1-mailbox"

I don't see any need for this define - it just adds an additional level of
indirection when trying to find out the name of the driver. It's only
used in two places as is, so it's value is rather limited:

> @@ -184,9 +184,9 @@ static int omap1_mbox_remove(struct platform_device *pdev)
>
> static struct platform_driver omap1_mbox_driver = {
> .probe = omap1_mbox_probe,
> - .remove = omap1_mbox_remove,
> + .remove = __devexit_p(omap1_mbox_remove),
> .driver = {
> - .name = "mailbox",
> + .name = DRV_NAME,
> },
> };
>
> @@ -203,4 +203,7 @@ static void __exit omap1_mbox_exit(void)
> module_init(omap1_mbox_init);
> module_exit(omap1_mbox_exit);
>
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("omap mailbox: omap1 architecture specific functions");
> +MODULE_AUTHOR("Hiroshi DOYU" <[email protected]>);
> +MODULE_ALIAS("platform:"DRV_NAME);

2009-01-28 15:34:33

>From f6efca787d09564c0cfd8ad1cb21d29213df8862 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <[email protected]>
Date: Wed, 28 Jan 2009 21:18:07 +0200
Subject: [PATCH 05/10] omap mailbox: fix empty struct device for omap1

Signed-off-by: Hiroshi DOYU <[email protected]>
---
arch/arm/mach-omap1/devices.c | 2 +-
arch/arm/mach-omap1/mailbox.c | 29 +++++++++++++++--------------
2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap1/devices.c b/arch/arm/mach-omap1/devices.c
index 77382d8..b4b6106 100644
--- a/arch/arm/mach-omap1/devices.c
+++ b/arch/arm/mach-omap1/devices.c
@@ -86,7 +86,7 @@ static struct resource mbox_resources[] = {
};

static struct platform_device mbox_device = {
- .name = "mailbox",
+ .name = "omap1-mailbox",
.id = -1,
.num_resources = ARRAY_SIZE(mbox_resources),
.resource = mbox_resources,
diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 59abbf3..0af4d6c 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -1,7 +1,7 @@
/*
* Mailbox reservation modules for DSP
*
- * Copyright (C) 2006 Nokia Corporation
+ * Copyright (C) 2006-2009 Nokia Corporation
* Written by: Hiroshi DOYU <[email protected]>
*
* This file is subject to the terms and conditions of the GNU General Public
@@ -27,7 +27,7 @@
#define MAILBOX_DSP2ARM1_Flag 0x1c
#define MAILBOX_DSP2ARM2_Flag 0x20

-unsigned long mbox_base;
+static void __iomem *mbox_base;

struct omap_mbox1_fifo {
unsigned long cmd;
@@ -40,14 +40,14 @@ struct omap_mbox1_priv {
struct omap_mbox1_fifo rx_fifo;
};

-static inline int mbox_read_reg(unsigned int reg)
+static inline int mbox_read_reg(size_t ofs)
{
- return __raw_readw(mbox_base + reg);
+ return __raw_readw(mbox_base + ofs);
}

-static inline void mbox_write_reg(unsigned int val, unsigned int reg)
+static inline void mbox_write_reg(u32 val, size_t ofs)
{
- __raw_writew(val, mbox_base + reg);
+ __raw_writew(val, mbox_base + ofs);
}

/* msg */
@@ -143,7 +143,7 @@ struct omap_mbox mbox_dsp_info = {
};
EXPORT_SYMBOL(mbox_dsp_info);

-static int __init omap1_mbox_probe(struct platform_device *pdev)
+static int __devinit omap1_mbox_probe(struct platform_device *pdev)
{
struct resource *res;
int ret = 0;
@@ -170,12 +170,10 @@ static int __init omap1_mbox_probe(struct platform_device *pdev)
}
mbox_dsp_info.irq = res->start;

- ret = omap_mbox_register(&mbox_dsp_info);
-
- return ret;
+ return omap_mbox_register(&pdev->dev, &mbox_dsp_info);
}

-static int omap1_mbox_remove(struct platform_device *pdev)
+static int __devexit omap1_mbox_remove(struct platform_device *pdev)
{
omap_mbox_unregister(&mbox_dsp_info);

@@ -184,9 +182,9 @@ static int omap1_mbox_remove(struct platform_device *pdev)

static struct platform_driver omap1_mbox_driver = {
.probe = omap1_mbox_probe,
- .remove = omap1_mbox_remove,
+ .remove = __devexit_p(omap1_mbox_remove),
.driver = {
- .name = "mailbox",
+ .name = "omap1-mailbox",
},
};

@@ -203,4 +201,7 @@ static void __exit omap1_mbox_exit(void)
module_init(omap1_mbox_init);
module_exit(omap1_mbox_exit);

-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("omap mailbox: omap1 architecture specific functions");
+MODULE_AUTHOR("Hiroshi DOYU" <[email protected]>);
+MODULE_ALIAS("platform:omap1-mailbox");
--
1.6.1.rc4

Attachments:

0006-omap-mailbox-fix-empty-struct-device-for-omap2.patch (2.53 kB)
0005-omap-mailbox-fix-empty-struct-device-for-omap1.patch (3.32 kB)
Download all attachments

2009-01-28 19:42:33

by Russell King - ARM Linux

[permalink] [raw]

Subject: Re: [PATCH 05/10] omap mailbox: fix empty struct device for omap1

On Wed, Jan 28, 2009 at 09:21:22PM +0200, Hiroshi DOYU wrote:
> It seems that I misunderstood your comment in the following mail.
>
> http://lkml.indiana.edu/hypermail/linux/kernel/0901.2/00610.html

Ah, that comment was more about that specific include in that specific
file - I couldn't see anything in there which required it.

The general rule is: if both asm/xxx.h and linux/xxx.h exists, then use
the linux/xxx.h version. I stress _general_ rule - there are exceptions.
For example, linux/irq.h and linux/memory.h. A clue would be if linux/xxx.h
doesn't include asm/xxx.h, then the rule above probably doesn't apply.

> Reverted both header file changes. Update attached patches.

Much better, thanks. This I'm happy with.

2009-01-30 18:57:36

by Tony Lindgren

[permalink] [raw]

Subject: Re: [PATCH 00/10] arm: omap mailbox: add omap3 support with some fixes

* Hiroshi DOYU <[email protected]> [090116 00:29]:
> The following patches add omap3 support and some cleanups.
>
> This "omap mailbox" device driver has been used for the interprocessor
> communication between ARM and some co-processors(DSP) inside of omap
> SoC, which is a kind of interrupt driven FIFO.

Adding the mailbox series to the omap for-next branch. Ignoring the
iommu series for now as it still needs some updates.

Tony

> ---
>
> Hiroshi DOYU (10):
> omap mailbox: remove unnecessary header file inclusion
> omap mailbox: convert sequence bit checking to module paramter
> omap mailbox: move mailbox.h into mailbox.c
> omap mailbox: add save_/restore_ctx() for PM
> omap mailbox: fix empty struct device for omap2
> omap mailbox: fix empty struct device for omap1
> omap mailbox: fix empty struct device for omap_mbox
> omap mailbox: print hardware revision at startup
> omap mailbox: add initial omap3 support
> omap mailbox: cleanup omap2 register definition with macro
>
>
> arch/arm/mach-omap1/devices.c | 2
> arch/arm/mach-omap1/mailbox.c | 31 ++--
> arch/arm/mach-omap2/devices.c | 38 ++++-
> arch/arm/mach-omap2/mailbox.c | 195 +++++++++++++++++-----------
> arch/arm/plat-omap/Kconfig | 8 +
> arch/arm/plat-omap/include/mach/mailbox.h | 27 ++++
> arch/arm/plat-omap/include/mach/omap34xx.h | 1
> arch/arm/plat-omap/mailbox.c | 152 ++++++++++++++++------
> arch/arm/plat-omap/mailbox.h | 100 --------------
> 9 files changed, 311 insertions(+), 243 deletions(-)
> delete mode 100644 arch/arm/plat-omap/mailbox.h
>
> --
> Hiroshi DOYU
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html