From: Shawn Guo <shawn.guo@linaro.org>
To: Thara Gopinath <thara.gopinath@linaro.org>
Cc: agross@kernel.org, bjorn.andersson@linaro.org,
dan.j.williams@intel.com, vkoul@kernel.org,
srinivas.kandagatla@linaro.org, linux-arm-msm@vger.kernel.org,
dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drivers: dma: qcom: bam_dma: Manage clocks when controlled_remotely is set
Date: Sat, 23 Jan 2021 15:19:25 +0800 [thread overview]
Message-ID: <20210123071924.GF2479@dragon> (raw)
In-Reply-To: <d1f1724c-39f1-7b6e-8cd4-638a44608d9c@linaro.org>
On Fri, Jan 22, 2021 at 10:44:09AM -0500, Thara Gopinath wrote:
> Hi Shawn,
>
> Thanks for the review
>
> On 1/22/21 12:10 AM, Shawn Guo wrote:
> > On Thu, Jan 21, 2021 at 09:52:51PM -0500, Thara Gopinath wrote:
> > > When bam dma is "controlled remotely", thus far clocks were not controlled
> > > from the Linux. In this scenario, Linux was disabling runtime pm in bam dma
> > > driver and not doing any clock management in suspend/resume hooks.
> > >
> > > With introduction of crypto engine bam dma, the clock is a rpmh resource
> > > that can be controlled from both Linux and TZ/remote side. Now bam dma
> > > clock is getting enabled during probe even though the bam dma can be
> > > "controlled remotely". But due to clocks not being handled properly,
> > > bam_suspend generates a unbalanced clk_unprepare warning during system
> > > suspend.
> > >
> > > To fix the above issue and to enable proper clock-management, this patch
> > > enables runtim-pm and handles bam dma clocks in suspend/resume hooks if
> > > the clock node is present irrespective of controlled_remotely property.
> >
> > Shouldn't the following probe code need some update? Now we have both
> > controlled_remotely and clocks handle for cryptobam node. For example,
> > if devm_clk_get() returns -EPROBE_DEFER, we do not want to continue with
> > bamclk forcing to be NULL, right?
>
> We still will have to set bdev->bamclk to NULL in certain scenarios. For eg
> slimbus bam dma is controlled-remotely and the clocks are handled by the
> remote s/w. Linux does not handle the clocks at all and there is no clock
> specified in the dt node.This is the norm for the devices that are also
> controlled by remote s/w. Crypto bam dma is a special case where the clock
> is actually a rpmh resource and hence can be independently handled from both
> remote side and Linux by voting. In this case, the dma is controlled
> remotely but clock can be turned off and on in Linux. Hence the need for
> this patch.
So is it correct to say that clock is mandatory for !controlled-remotely
BAM, while it's optional for controlled-remotely one. If yes, maybe we
can do something like below to make the code a bit easier to read?
if (controlled-remotely)
bdev->bamclk = devm_clk_get_optional();
else
bdev->bamclk = devm_clk_get();
> Yes, the probe code needs updating to handle -EPROBE_DEFER (esp if the clock
> driver is built in as a module) I am not sure if the clock framework handles
> -EPROBE_DEFER properly either. So that
> might need updating too. This is a separate activity and not part of this
> patch.
As the patch breaks the assumption that for controlled-remotely BAM
there is no clock to be managed, the probe code becomes buggy right
away.
Shawn
next prev parent reply other threads:[~2021-01-23 7:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-22 2:52 [PATCH] drivers: dma: qcom: bam_dma: Manage clocks when controlled_remotely is set Thara Gopinath
2021-01-22 5:10 ` Shawn Guo
2021-01-22 15:44 ` Thara Gopinath
2021-01-23 7:19 ` Shawn Guo [this message]
2021-01-25 13:33 ` Thara Gopinath
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=20210123071924.GF2479@dragon \
--to=shawn.guo@linaro.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=dan.j.williams@intel.com \
--cc=dmaengine@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=srinivas.kandagatla@linaro.org \
--cc=thara.gopinath@linaro.org \
--cc=vkoul@kernel.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).