[PATCH 0/4] OMAP mailbox suspend/resume support

[PATCH 0/4] OMAP mailbox suspend/resume support

[Suman Anna]

Hi,

The following series adds the suspend/resume support through the PM
callbacks to the OMAP mailbox driver. The functionality is added mainly
in Patch 2. Patch 4 kills the legacy API exported to the mailbox clients
to save and restore the context.

The series is based on 4.6-rc2 + the OMAP Mailbox legacy platform device
support removal series [1].

regards
Suman

[1] http://marc.info/?l=linux-omap&m=145996429717184&w=2

Suman Anna (4):
  mailbox/omap: store mailbox interrupt type in omap_mbox_device
  mailbox/omap: add support for suspend/resume
  mailbox/omap: check for any unread messages during suspend
  mailbox/omap: kill omap_mbox_{save/restore}_ctx() functions

 drivers/mailbox/omap-mailbox.c | 106 +++++++++++++++++++++--------------------
 include/linux/omap-mailbox.h   |   2 -
 2 files changed, 55 insertions(+), 53 deletions(-)

-- 
2.7.4

[PATCH 1/4] mailbox/omap: store mailbox interrupt type in omap_mbox_device

[Suman Anna]

The interrupt type used for identifying the layout of the interrupt
configuration registers between OMAP4+ SoCs and older SoCs is stored
only in the sub-mailbox structures for easier access. Store this type
in the the omap_mbox_device structure as well along with the other
global variables. This is being done to facilitate the context save
and restore of appropriate registers during system suspend/resume.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/mailbox/omap-mailbox.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 84cafc02e761..adf37f494690 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -90,6 +90,7 @@ struct omap_mbox_device {
 	void __iomem *mbox_base;
 	u32 num_users;
 	u32 num_fifos;
+	u32 intr_type;
 	struct omap_mbox **mboxes;
 	struct mbox_controller controller;
 	struct list_head elem;
@@ -828,6 +829,7 @@ static int omap_mbox_probe(struct platform_device *pdev)
 	mdev->dev = &pdev->dev;
 	mdev->num_users = num_users;
 	mdev->num_fifos = num_fifos;
+	mdev->intr_type = intr_type;
 	mdev->mboxes = list;
 
 	/* OMAP does not have a Tx-Done IRQ, but rather a Tx-Ready IRQ */
-- 
2.7.4

[PATCH 2/4] mailbox/omap: add support for suspend/resume

[Suman Anna]

Support has been added to the OMAP mailbox driver to allow it
to work across a system suspend/resume. The OMAP mailbox driver
requires only the interrupt configuration registers to be saved
and restored, and this is done in the suspend/resume callbacks.
The registers need to be saved only if there are active clients
at the time of suspend. The enabling and disabling of the mailbox
clocks is done automatically by the omap_device layer.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/mailbox/omap-mailbox.c | 45 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index adf37f494690..1b90eb380d44 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -88,6 +88,7 @@ struct omap_mbox_device {
 	struct device *dev;
 	struct mutex cfg_lock;
 	void __iomem *mbox_base;
+	u32 *irq_ctx;
 	u32 num_users;
 	u32 num_fifos;
 	u32 intr_type;
@@ -650,6 +651,44 @@ static const struct mbox_chan_ops omap_mbox_chan_ops = {
 	.shutdown       = omap_mbox_chan_shutdown,
 };
 
+#ifdef CONFIG_PM_SLEEP
+static int omap_mbox_suspend(struct device *dev)
+{
+	struct omap_mbox_device *mdev = dev_get_drvdata(dev);
+	u32 usr, reg;
+
+	if (pm_runtime_status_suspended(dev))
+		return 0;
+
+	for (usr = 0; usr < mdev->num_users; usr++) {
+		reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
+		mdev->irq_ctx[usr] = mbox_read_reg(mdev, reg);
+	}
+
+	return 0;
+}
+
+static int omap_mbox_resume(struct device *dev)
+{
+	struct omap_mbox_device *mdev = dev_get_drvdata(dev);
+	u32 usr, reg;
+
+	if (pm_runtime_status_suspended(dev))
+		return 0;
+
+	for (usr = 0; usr < mdev->num_users; usr++) {
+		reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
+		mbox_write_reg(mdev, mdev->irq_ctx[usr], reg);
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops omap_mbox_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(omap_mbox_suspend, omap_mbox_resume)
+};
+
 static const struct of_device_id omap_mailbox_of_match[] = {
 	{
 		.compatible	= "ti,omap2-mailbox",
@@ -777,6 +816,11 @@ static int omap_mbox_probe(struct platform_device *pdev)
 	if (IS_ERR(mdev->mbox_base))
 		return PTR_ERR(mdev->mbox_base);
 
+	mdev->irq_ctx = devm_kzalloc(&pdev->dev, num_users * sizeof(u32),
+				     GFP_KERNEL);
+	if (!mdev->irq_ctx)
+		return -ENOMEM;
+
 	/* allocate one extra for marking end of list */
 	list = devm_kzalloc(&pdev->dev, (info_count + 1) * sizeof(*list),
 			    GFP_KERNEL);
@@ -887,6 +931,7 @@ static struct platform_driver omap_mbox_driver = {
 	.remove	= omap_mbox_remove,
 	.driver	= {
 		.name = "omap-mailbox",
+		.pm = &omap_mbox_pm_ops,
 		.of_match_table = of_match_ptr(omap_mailbox_of_match),
 	},
 };
-- 
2.7.4

[PATCH 3/4] mailbox/omap: check for any unread messages during suspend

[Suman Anna]

The OMAP mailbox driver is used by clients to communicate with remote
processors in general. The mailbox clients are expected to have stopped
communicating with these remote processors during a system suspend. The
OMAP mailbox fifos are expected to not have any messages as such. Add a
check for any pending unprocessed messages in the suspend callback, to
detect any communication protocol issues of the mailbox clients. The
system suspend is aborted if any messages are found.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/mailbox/omap-mailbox.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index 1b90eb380d44..b61c6e8654b5 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -655,11 +655,19 @@ static const struct mbox_chan_ops omap_mbox_chan_ops = {
 static int omap_mbox_suspend(struct device *dev)
 {
 	struct omap_mbox_device *mdev = dev_get_drvdata(dev);
-	u32 usr, reg;
+	u32 usr, fifo, reg;
 
 	if (pm_runtime_status_suspended(dev))
 		return 0;
 
+	for (fifo = 0; fifo < mdev->num_fifos; fifo++) {
+		if (mbox_read_reg(mdev, MAILBOX_MSGSTATUS(fifo))) {
+			dev_err(mdev->dev, "fifo %d has unexpected unread messages\n",
+				fifo);
+			return -EBUSY;
+		}
+	}
+
 	for (usr = 0; usr < mdev->num_users; usr++) {
 		reg = MAILBOX_IRQENABLE(mdev->intr_type, usr);
 		mdev->irq_ctx[usr] = mbox_read_reg(mdev, reg);
-- 
2.7.4

[PATCH 4/4] mailbox/omap: kill omap_mbox_{save/restore}_ctx() functions

[Suman Anna]

The omap_mbox_save_ctx() and omap_mbox_restore_ctx() API were
previously provided to OMAP mailbox clients to save and restore
the mailbox context during system suspend/resume. The save and
restore functionality is now implemented through System PM driver
callbacks, and there is no need for these functions, so kill these
API.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/mailbox/omap-mailbox.c | 51 ------------------------------------------
 include/linux/omap-mailbox.h   |  2 --
 2 files changed, 53 deletions(-)

diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
index b61c6e8654b5..1e0963ed6a25 100644
--- a/drivers/mailbox/omap-mailbox.c
+++ b/drivers/mailbox/omap-mailbox.c
@@ -55,13 +55,6 @@
 #define MAILBOX_IRQ_NEWMSG(m)		(1 << (2 * (m)))
 #define MAILBOX_IRQ_NOTFULL(m)		(1 << (2 * (m) + 1))
 
-#define MBOX_REG_SIZE			0x120
-
-#define OMAP4_MBOX_REG_SIZE		0x130
-
-#define MBOX_NR_REGS			(MBOX_REG_SIZE / sizeof(u32))
-#define OMAP4_MBOX_NR_REGS		(OMAP4_MBOX_REG_SIZE / sizeof(u32))
-
 /* Interrupt register configuration types */
 #define MBOX_INTR_CFG_TYPE1		(0)
 #define MBOX_INTR_CFG_TYPE2		(1)
@@ -118,7 +111,6 @@ struct omap_mbox {
 	struct omap_mbox_device *parent;
 	struct omap_mbox_fifo	tx_fifo;
 	struct omap_mbox_fifo	rx_fifo;
-	u32			ctx[OMAP4_MBOX_NR_REGS];
 	u32			intr_type;
 	struct mbox_chan	*chan;
 	bool			send_no_irq;
@@ -209,49 +201,6 @@ static int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
 	return (int)(enable & status & bit);
 }
 
-void omap_mbox_save_ctx(struct mbox_chan *chan)
-{
-	int i;
-	int nr_regs;
-	struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan);
-
-	if (WARN_ON(!mbox))
-		return;
-
-	if (mbox->intr_type)
-		nr_regs = OMAP4_MBOX_NR_REGS;
-	else
-		nr_regs = MBOX_NR_REGS;
-	for (i = 0; i < nr_regs; i++) {
-		mbox->ctx[i] = mbox_read_reg(mbox->parent, i * sizeof(u32));
-
-		dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
-			i, mbox->ctx[i]);
-	}
-}
-EXPORT_SYMBOL(omap_mbox_save_ctx);
-
-void omap_mbox_restore_ctx(struct mbox_chan *chan)
-{
-	int i;
-	int nr_regs;
-	struct omap_mbox *mbox = mbox_chan_to_omap_mbox(chan);
-
-	if (WARN_ON(!mbox))
-		return;
-
-	if (mbox->intr_type)
-		nr_regs = OMAP4_MBOX_NR_REGS;
-	else
-		nr_regs = MBOX_NR_REGS;
-	for (i = 0; i < nr_regs; i++) {
-		mbox_write_reg(mbox->parent, mbox->ctx[i], i * sizeof(u32));
-		dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
-			i, mbox->ctx[i]);
-	}
-}
-EXPORT_SYMBOL(omap_mbox_restore_ctx);
-
 static void _omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
 {
 	u32 l;
diff --git a/include/linux/omap-mailbox.h b/include/linux/omap-mailbox.h
index 587bbdd31f5a..c726bd833761 100644
--- a/include/linux/omap-mailbox.h
+++ b/include/linux/omap-mailbox.h
@@ -21,8 +21,6 @@ struct mbox_client;
 struct mbox_chan *omap_mbox_request_channel(struct mbox_client *cl,
 					    const char *chan_name);
 
-void omap_mbox_save_ctx(struct mbox_chan *chan);
-void omap_mbox_restore_ctx(struct mbox_chan *chan);
 void omap_mbox_enable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq);
 void omap_mbox_disable_irq(struct mbox_chan *chan, omap_mbox_irq_t irq);
 
-- 
2.7.4

Re: [PATCH 1/4] mailbox/omap: store mailbox interrupt type in omap_mbox_device

[Grygorii Strashko]

On 04/07/2016 02:37 AM, Suman Anna wrote:
> The interrupt type used for identifying the layout of the interrupt
> configuration registers between OMAP4+ SoCs and older SoCs is stored
> only in the sub-mailbox structures for easier access. Store this type
> in the the omap_mbox_device structure as well along with the other
> global variables. This is being done to facilitate the context save
> and restore of appropriate registers during system suspend/resume.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>   drivers/mailbox/omap-mailbox.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
> index 84cafc02e761..adf37f494690 100644
> --- a/drivers/mailbox/omap-mailbox.c
> +++ b/drivers/mailbox/omap-mailbox.c
> @@ -90,6 +90,7 @@ struct omap_mbox_device {
>   	void __iomem *mbox_base;
>   	u32 num_users;
>   	u32 num_fifos;
> +	u32 intr_type;
>   	struct omap_mbox **mboxes;
>   	struct mbox_controller controller;
>   	struct list_head elem;
> @@ -828,6 +829,7 @@ static int omap_mbox_probe(struct platform_device *pdev)
>   	mdev->dev = &pdev->dev;
>   	mdev->num_users = num_users;
>   	mdev->num_fifos = num_fifos;
> +	mdev->intr_type = intr_type;
>   	mdev->mboxes = list;
>
>   	/* OMAP does not have a Tx-Done IRQ, but rather a Tx-Ready IRQ */
>

This smth. which definitely has to be done using compatible props.
Why not to pass more info through of_device_id->data?



-- 
regards,
-grygorii

Re: [PATCH 1/4] mailbox/omap: store mailbox interrupt type in omap_mbox_device

[Suman Anna]

Hi Grygorii,

On 04/07/2016 05:32 AM, Grygorii Strashko wrote:
> On 04/07/2016 02:37 AM, Suman Anna wrote:
>> The interrupt type used for identifying the layout of the interrupt
>> configuration registers between OMAP4+ SoCs and older SoCs is stored
>> only in the sub-mailbox structures for easier access. Store this type
>> in the the omap_mbox_device structure as well along with the other
>> global variables. This is being done to facilitate the context save
>> and restore of appropriate registers during system suspend/resume.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>>   drivers/mailbox/omap-mailbox.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mailbox/omap-mailbox.c
>> b/drivers/mailbox/omap-mailbox.c
>> index 84cafc02e761..adf37f494690 100644
>> --- a/drivers/mailbox/omap-mailbox.c
>> +++ b/drivers/mailbox/omap-mailbox.c
>> @@ -90,6 +90,7 @@ struct omap_mbox_device {
>>       void __iomem *mbox_base;
>>       u32 num_users;
>>       u32 num_fifos;
>> +    u32 intr_type;
>>       struct omap_mbox **mboxes;
>>       struct mbox_controller controller;
>>       struct list_head elem;
>> @@ -828,6 +829,7 @@ static int omap_mbox_probe(struct platform_device
>> *pdev)
>>       mdev->dev = &pdev->dev;
>>       mdev->num_users = num_users;
>>       mdev->num_fifos = num_fifos;
>> +    mdev->intr_type = intr_type;
>>       mdev->mboxes = list;
>>
>>       /* OMAP does not have a Tx-Done IRQ, but rather a Tx-Ready IRQ */
>>
> 
> This smth. which definitely has to be done using compatible props.
> Why not to pass more info through of_device_id->data?
> 

Yeah, that's already the case, intr_type is a local variable assigned
from compatible match data. This patch merely caches it in the
omap_mbox_device structure so that it can be leveraged by the PM
callback functions in the next patch.

regards
Suman

Re: [PATCH 1/4] mailbox/omap: store mailbox interrupt type in omap_mbox_device

[Grygorii Strashko]

On 04/07/2016 03:24 PM, Suman Anna wrote:
> Hi Grygorii,
>
> On 04/07/2016 05:32 AM, Grygorii Strashko wrote:
>> On 04/07/2016 02:37 AM, Suman Anna wrote:
>>> The interrupt type used for identifying the layout of the interrupt
>>> configuration registers between OMAP4+ SoCs and older SoCs is stored
>>> only in the sub-mailbox structures for easier access. Store this type
>>> in the the omap_mbox_device structure as well along with the other
>>> global variables. This is being done to facilitate the context save
>>> and restore of appropriate registers during system suspend/resume.
>>>
>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>> ---
>>>    drivers/mailbox/omap-mailbox.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mailbox/omap-mailbox.c
>>> b/drivers/mailbox/omap-mailbox.c
>>> index 84cafc02e761..adf37f494690 100644
>>> --- a/drivers/mailbox/omap-mailbox.c
>>> +++ b/drivers/mailbox/omap-mailbox.c
>>> @@ -90,6 +90,7 @@ struct omap_mbox_device {
>>>        void __iomem *mbox_base;
>>>        u32 num_users;
>>>        u32 num_fifos;
>>> +    u32 intr_type;
>>>        struct omap_mbox **mboxes;
>>>        struct mbox_controller controller;
>>>        struct list_head elem;
>>> @@ -828,6 +829,7 @@ static int omap_mbox_probe(struct platform_device
>>> *pdev)
>>>        mdev->dev = &pdev->dev;
>>>        mdev->num_users = num_users;
>>>        mdev->num_fifos = num_fifos;
>>> +    mdev->intr_type = intr_type;
>>>        mdev->mboxes = list;
>>>
>>>        /* OMAP does not have a Tx-Done IRQ, but rather a Tx-Ready IRQ */
>>>
>>
>> This smth. which definitely has to be done using compatible props.
>> Why not to pass more info through of_device_id->data?
>>
>
> Yeah, that's already the case, intr_type is a local variable assigned
> from compatible match data. This patch merely caches it in the
> omap_mbox_device structure so that it can be leveraged by the PM
> callback functions in the next patch.
>

Ah ok. Thanks for explanation.

-- 
regards,
-grygorii