From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Andy Gross <agross@kernel.org>,
Maulik Shah <mkshah@codeaurora.org>,
swboyd@chromium.org, mka@chromium.org,
Rajendra Nayak <rnayak@codeaurora.org>,
evgreen@chromium.org, Lina Iyer <ilina@codeaurora.org>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 00/10] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments
Date: Mon, 13 Apr 2020 22:28:51 -0700 [thread overview]
Message-ID: <20200414052851.GI20625@builder.lan> (raw)
In-Reply-To: <20200413170415.32463-1-dianders@chromium.org>
On Mon 13 Apr 10:04 PDT 2020, Douglas Anderson wrote:
> In order to review Maulik's latest "rpmh_flush for non OSI targets"
> patch series I've found myself trying to understand rpmh-rsc better.
> To make it easier for others to do this in the future, add a whole lot
> of comments / documentation.
>
> As part of this there are a very small number of functional changes.
> - We'll get a tiny performance boost by getting rid of the "cmd_cache"
> which I believe was unnecessary.
> - We now assume someone else is in charge of exclusivity for
> tcs_invalidate() and have removed a lock in there. As per the
> comments in the patch, this isn't expected to cause problems.
> - tcs_is_free() no longer checks hardware state, but we think it
> didn't need to.
>
> These changes touch a lot of code in rpmh-rsc. Luckily Maulik has
> reported that he's tested them and they work fine for him. They
> should be ready to go.
>
> I've tried to structure the patches so that simpler / less
> controversial patches are first. Those could certainly land on their
> own without later patches. Many of the patches could also be dropped
> and the others would still apply if they are controversial. If you
> need help doing this then please yell.
>
> These patches are based on Maulik's v17 series, AKA:
> https://lore.kernel.org/r/1586703004-13674-1-git-send-email-mkshah@codeaurora.org
>
> There are still more cleanups that we need to do, but to avoid having
> too many patches flying through the air at once we'll do them after
> Maulik's v17 and this series lands.
>
> With all that, enjoy.
>
Applied the series, with the typos pointed out by Stephen fixed.
Please follow up on the additional suggestions with incremental patches.
Regards,
Bjorn
> Changes in v4:
> - Add "payload" to end of ("Don't double-check rpmh") patch subject
> - Removed extra "make sure" in commit message.
>
> Changes in v3:
> - ("...are not for IRQ") is new for v3.
> - ("Don't double-check rpmh") replaces ("Warning if tcs_write...")
> - Add "TCS" in title (Maulik).
> - Adjusted comments for rpmh_rsc_write_ctrl_data().
> - Comments for new enable_tcs_irq() function.
> - Comments for new rpmh_rsc_cpu_pm_callback() function.
> - Extra blank line removed (Maulik).
> - IRQ registers aren't in TCS0 (Maulik).
> - Kill find_match moves from patch #9 to patch #5 (Maulik).
> - Mention in message that I also fixed up kernel-doc stuff.
> - Moved comments patch after ("Kill cmd_cache and find_match...").
> - One space after a period now (Maulik).
> - Plural of TCS fixed to TCSes following Maulik's example.
> - Re-added comment in tcs_write() about checking for same address.
> - Rebased atop v16 ('Invoke rpmh_flush...') series.
> - Replace ("...warn if state mismatch") w/ ("...just check tcs_in_use")
> - Replaced ("irqsave()...") + ("...never -EBUSY") w/ ("Caller handles...")
> - Rewrote commit message to adjust for patch order.
> - __tcs_set_trigger() comments adjusted now that it can set or unset.
> - get_tcs_for_msg() documents why it's safe to borrow the wake TCS.
> - get_tcs_for_msg() no longer returns -EAGAIN.
>
> Changes in v2:
> - Comment tcs_is_free() new for v2; replaces old patch 6.
> - Document bug of tcs_write() not handling -EAGAIN.
> - Document get_tcs_for_msg() => -EAGAIN only for ACTIVE_ONLY.
> - Document locks for updating "tcs_in_use" more.
> - Document tcs_is_free() without drv->lock OK for tcs_invalidate().
> - Document that rpmh_rsc_send_data() can be an implicit invalidate.
> - Document two get_tcs_for_msg() issues if zero-active TCS.
> - Fixed documentation of "tcs" param in find_slots().
> - Got rid of useless "if (x) continue" at end of for loop.
> - More clear that active-only xfers can happen on wake TCS sometimes.
> - Now prose in comments instead of struct definitions.
> - Pretty ASCII art from Stephen.
> - Reword tcs_write() doc a bit.
>
> Douglas Anderson (10):
> drivers: qcom: rpmh-rsc: Clean code reading/writing TCS regs/cmds
> drivers: qcom: rpmh-rsc: Document the register layout better
> drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller
> drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction
> drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire
> drivers: qcom: rpmh-rsc: A lot of comments
> drivers: qcom: rpmh-rsc: tcs_is_free() can just check tcs_in_use
> drivers: qcom: rpmh-rsc: Don't double-check rpmh payload
> drivers: qcom: rpmh-rsc: Caller handles tcs_invalidate() exclusivity
> drivers: qcom: rpmh-rsc: read_tcs_reg()/write_tcs_reg() are not for
> IRQ
>
> drivers/soc/qcom/rpmh-internal.h | 66 +++--
> drivers/soc/qcom/rpmh-rsc.c | 465 +++++++++++++++++++++----------
> drivers/soc/qcom/rpmh.c | 5 +-
> 3 files changed, 363 insertions(+), 173 deletions(-)
>
> --
> 2.26.0.110.g2183baf09c-goog
>
prev parent reply other threads:[~2020-04-14 5:28 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-13 17:04 [PATCH v4 00/10] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Douglas Anderson
2020-04-13 17:04 ` [PATCH v4 01/10] drivers: qcom: rpmh-rsc: Clean code reading/writing TCS regs/cmds Douglas Anderson
2020-04-13 18:19 ` Joe Perches
2020-04-13 21:18 ` Doug Anderson
2020-04-13 21:33 ` Joe Perches
2020-04-14 17:43 ` Doug Anderson
2020-04-13 21:20 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 02/10] drivers: qcom: rpmh-rsc: Document the register layout better Douglas Anderson
2020-04-13 21:23 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 03/10] drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller Douglas Anderson
2020-04-13 21:36 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 04/10] drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction Douglas Anderson
2020-04-13 21:37 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 05/10] drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire Douglas Anderson
2020-04-13 21:41 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 06/10] drivers: qcom: rpmh-rsc: A lot of comments Douglas Anderson
2020-04-13 21:58 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 07/10] drivers: qcom: rpmh-rsc: tcs_is_free() can just check tcs_in_use Douglas Anderson
2020-04-13 21:58 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 08/10] drivers: qcom: rpmh-rsc: Don't double-check rpmh payload Douglas Anderson
2020-04-13 23:02 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 09/10] drivers: qcom: rpmh-rsc: Caller handles tcs_invalidate() exclusivity Douglas Anderson
2020-04-13 23:03 ` Stephen Boyd
2020-04-13 17:04 ` [PATCH v4 10/10] drivers: qcom: rpmh-rsc: read_tcs_reg()/write_tcs_reg() are not for IRQ Douglas Anderson
2020-04-13 23:03 ` Stephen Boyd
2020-04-14 5:28 ` Bjorn Andersson [this message]
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=20200414052851.GI20625@builder.lan \
--to=bjorn.andersson@linaro.org \
--cc=agross@kernel.org \
--cc=dianders@chromium.org \
--cc=evgreen@chromium.org \
--cc=ilina@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mka@chromium.org \
--cc=mkshah@codeaurora.org \
--cc=rnayak@codeaurora.org \
--cc=swboyd@chromium.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).