From: Stephen Boyd <swboyd@chromium.org>
To: Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Douglas Anderson <dianders@chromium.org>
Cc: Maulik Shah <mkshah@codeaurora.org>,
Douglas Anderson <dianders@chromium.org>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] soc: qcom: rpmh-rsc: Don't use ktime for timeout in write_tcs_reg_sync()
Date: Thu, 28 May 2020 15:44:48 -0700 [thread overview]
Message-ID: <159070588846.69627.5268638209383373410@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20200528074530.1.Ib86e5b406fe7d16575ae1bb276d650faa144b63c@changeid>
Quoting Douglas Anderson (2020-05-28 07:48:34)
> The write_tcs_reg_sync() may be called after timekeeping is suspended
> so it's not OK to use ktime. The readl_poll_timeout_atomic() macro
> implicitly uses ktime. This was causing a warning at suspend time.
>
> Change to just loop 1000000 times with a delay of 1 us between loops.
> This may give a timeout of more than 1 second but never less and is
> safe even if timekeeping is suspended.
>
> NOTE: I don't have any actual evidence that we need to loop here.
> It's possibly that all we really need to do is just read the value
> back to ensure that the pipes are cleaned and the looping/comparing is
> totally not needed. I never saw the loop being needed in my tests.
> However, the loop shouldn't hurt.
>
> Fixes: 91160150aba0 ("soc: qcom: rpmh-rsc: Timeout after 1 second in write_tcs_reg_sync()")
> Reported-by: Maulik Shah <mkshah@codeaurora.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Although I don't think ktime_get() inside of readl_poll_timeout_atomic()
is correct. The timekeeping base won't be able to update when a loop is
spinning in an irq disabled region. We need the tick interrupt to come
in and update the base. Spinning for a second with irqs disabled is also
insane for realtime so there's that problem too. Maybe we should try to
kick timekeeping forward from these loops manually. Anyway, not problems
with this patch so not important to fix immediately.
next prev parent reply other threads:[~2020-05-28 22:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-28 14:48 [PATCH] soc: qcom: rpmh-rsc: Don't use ktime for timeout in write_tcs_reg_sync() Douglas Anderson
2020-05-28 22:44 ` Stephen Boyd [this message]
2020-05-29 17:00 ` Doug Anderson
2020-06-18 21:52 ` Doug Anderson
2020-06-24 4:41 ` Maulik Shah
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=159070588846.69627.5268638209383373410@swboyd.mtv.corp.google.com \
--to=swboyd@chromium.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=dianders@chromium.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mkshah@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).