Messages in this thread | | | Date | Mon, 12 Jul 2021 09:44:08 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 03/13] opp: Keep track of currently programmed OPP |
| |
On 09-07-21, 09:57, Ionela Voinescu wrote: > On Thursday 08 Jul 2021 at 13:23:53 (+0530), Viresh Kumar wrote: > > On 07-07-21, 11:24, Ionela Voinescu wrote: > > > 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 :)
> > 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.
> > -------------------------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
| |