On 09-07-21, 09:57, Ionela Voinescu wrote:

On Thursday 08 Jul 2021 at 13:23:53 (+0530), Viresh Kumar wrote:

quoted

On 07-07-21, 11:24, Ionela Voinescu wrote:

quoted

Now comes the interesting part: what seems to fix it is a call to
clk_get_rate(opp_table->clk) in _set_opp(), which is what basically
happened before this patch, as _find_current_opp() was always called.
I do not need to do anything with the returned frequency.

Wow, thanks for narrowing it down this far :)

I had a quick look and this is what I think is the problem here.

This platform uses mailbox API to send its frequency change requests to another
processor.  And the way it is written currently, I don't see any guarantee
whatsoever which say

  "once clk_set_rate() returns, the frequency would have already changed".

I think what was strange to me was that the frequency never seems to
change, there isn't just a delay in the new frequency taking effect, as
I would expect in these cases. Or if there is a delay, that's quite large
- at least a second.

No idea on what the firmware is doing behind the scene :)

quoted

And this may exactly be the thing you are able to hit, luckily because of this
patchset :)

As a quick way of checking if that is right or not, this may make it work:

diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
index 395ddc250828..9856c1c84dcf 100644
--- a/drivers/mailbox/hi3660-mailbox.c
+++ b/drivers/mailbox/hi3660-mailbox.c

@@ -201,6 +201,9 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, void *msg)

        /* Trigger data transferring */
        writel(BIT(mchan->ack_irq), base + MBOX_SEND_REG);
+
+       hi3660_mbox_check_state(chan);
+

I gave this a try an it does work for me.

Good, so that kind of proves what I was suspecting. The mailbox driver looks
buggy here.

quoted

-------------------------8<-------------------------

As a proper fix, something like this (not even compile tested) is required I
believe as I don't see the clients would know if the transfer is over. Cc'ing
mailbox guys to see what can be done.

I'll give this a try as well when there is consensus. I might even try to
review it, if the time allows.

Sure, lets see what the platform guys think about this first.

Kevin, Kaihua ?

-- 
viresh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel