lkml.org 
[lkml]   [2018]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v2] mailbox: Hi3660: Fixup mailbox state machine malfunction issue
From
Date
Hi,

On 03/12/2018 06:13, Kevin Wangtao wrote:
> Current mailbox driver of Hi3660 release the mailbox directly
> before sending a new message which may cause last message lost
> and next message sending doesn't take effect actually.
>
> This patch fixes this issue by following the right process below,
> each time before sending a message, mailbox driver will check
> whether the mailbox is in ready state, if last message has been
> acknowledged, mailbox driver will clear the ack state to turn
> the mailbox to ready state again.
>
> Signed-off-by: Kevin Wangtao <kevin.wangtao@hisilicon.com>

Validated using sysbench at each OPP ([1]). Test passes with patch and fails
without:

| CPU | OPP | Base | Patch |
|------+---------+------+-------|
| CPU0 | 533000 | 104 | 104 |
| CPU0 | 999000 | 104 | 201 |
| CPU0 | 140200 | 285 | 285 |
| CPU0 | 1709000 | 285 | 349 |
| CPU0 | 1844000 | 377 | 377 |
|------+---------+------+-------|
| CPU4 | 903000 | 249 | 248 |
| CPU4 | 1421000 | 249 | 394 |
| CPU4 | 1805000 | 500 | 500 |
| CPU4 | 2112000 | 499 | 583 |
| CPU4 | 2362000 | 653 | 654 |

We need this pretty badly, otherwise frequency setting is a coin toss.

Tested-by: Valentin Schneider <valentin.schneider@arm.com>

Thanks,
Valentin



[1]: https://github.com/ARM-software/lisa/blob/next/lisa/tests/kernel/cpufreq/sanity.py#L23

Ran the above with this script on each kernel:

----->8-----
#!/usr/bin/env python3

from lisa.env import TestEnv
from lisa.tests.kernel.cpufreq.sanity import UserspaceSanity

te = TestEnv.from_cli()

print(UserspaceSanity.from_testenv(te).test_performance_sanity())
-----8<-----


> ---
> Changes v1 -> v2:
> - update commit message
>
> drivers/mailbox/hi3660-mailbox.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
> index 3eea6b6..035b71a 100644
> --- a/drivers/mailbox/hi3660-mailbox.c
> +++ b/drivers/mailbox/hi3660-mailbox.c
> @@ -38,6 +38,7 @@
> #define MBOX_AUTOMATIC_ACK 1
>
> #define MBOX_STATE_IDLE BIT(4)
> +#define MBOX_STATE_READY BIT(5)
> #define MBOX_STATE_ACK BIT(7)
>
> #define MBOX_MSG_LEN 8
> @@ -91,8 +92,8 @@ static int hi3660_mbox_check_state(struct mbox_chan *chan)
> unsigned long val;
> unsigned int ret;
>
> - /* Mailbox is idle so directly bail out */
> - if (readl(base + MBOX_MODE_REG) & MBOX_STATE_IDLE)
> + /* Mailbox is ready to use */
> + if (readl(base + MBOX_MODE_REG) & MBOX_STATE_READY)
> return 0;
>
> /* Wait for acknowledge from remote */
> @@ -103,9 +104,9 @@ static int hi3660_mbox_check_state(struct mbox_chan *chan)
> return ret;
> }
>
> - /* Ensure channel is released */
> - writel(0xffffffff, base + MBOX_IMASK_REG);
> - writel(BIT(mchan->ack_irq), base + MBOX_SRC_REG);
> + /* clear ack state, mailbox will get back to ready state */
> + writel(BIT(mchan->ack_irq), base + MBOX_ICLR_REG);
> +
> return 0;
> }
>
> @@ -160,10 +161,6 @@ static int hi3660_mbox_startup(struct mbox_chan *chan)
> {
> int ret;
>
> - ret = hi3660_mbox_check_state(chan);
> - if (ret)
> - return ret;
> -
> ret = hi3660_mbox_unlock(chan);
> if (ret)
> return ret;
> @@ -183,10 +180,11 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)
> void __iomem *base = MBOX_BASE(mbox, ch);
> u32 *buf = msg;
> unsigned int i;
> + int ret;
>
> - /* Ensure channel is released */
> - writel_relaxed(0xffffffff, base + MBOX_IMASK_REG);
> - writel_relaxed(BIT(mchan->ack_irq), base + MBOX_SRC_REG);
> + ret = hi3660_mbox_check_state(chan);
> + if (ret)
> + return ret;
>
> /* Clear mask for destination interrupt */
> writel_relaxed(~BIT(mchan->dst_irq), base + MBOX_IMASK_REG);
>

\
 
 \ /
  Last update: 2018-12-10 19:32    [W:0.064 / U:6.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site