linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Maulik Shah <mkshah@codeaurora.org>
Cc: mka@chromium.org, Rajendra Nayak <rnayak@codeaurora.org>,
	evgreen@chromium.org, Lina Iyer <ilina@codeaurora.org>,
	swboyd@chromium.org, Douglas Anderson <dianders@chromium.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFT PATCH v2 00/10] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments
Date: Wed, 11 Mar 2020 16:13:38 -0700	[thread overview]
Message-ID: <20200311231348.129254-1-dianders@chromium.org> (raw)

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 (though just to be sure, best to try
  this atop Maulik's patches where it should be super obvious that we
  always invalidate before writing sleep/wake TCSs.
- I think I've eliminated a possible deadlock on "nosmp" systems,
  though it was mostly theoretical.
- Possibly we could get a warning in some cases if I misunderstood how
  tcs_is_free() works.  It'd be easy to remove the warning, though.

These changes touch a lot of code in rpmh-rsc, so hopefully someone at
Qualcomm can test them out better than I did (I don't have every last
client of RPMH in my tree) and review them soon-ish so they can land
and future patches can be based on them.

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.

At the end of this patch series there is still one TODO left in the
code.  Specifically I think we still have some problems when we try to
borrow a wakeup TCS for an active-only transfer.  The problems I think
are there aren't new, just the comment.  It's unclear if we should
land with the TODO in place or wait till that gets resolved first.

With all that, enjoy.

Changes in v2:
- Now prose in comments instead of struct definitions.
- Pretty ASCII art from Stephen.
- More clear that active-only xfers can happen on wake TCS sometimes.
- Document locks for updating "tcs_in_use" more.
- Document tcs_is_free() without drv->lock OK for tcs_invalidate().
- Document bug of tcs_write() not handling -EAGAIN.
- Document get_tcs_for_msg() => -EAGAIN only for ACTIVE_ONLY.
- Reword tcs_write() doc a bit.
- Document two get_tcs_for_msg() issues if zero-active TCS.
- Document that rpmh_rsc_send_data() can be an implicit invalidate.
- Fixed documentation of "tcs" param in find_slots().
- Comment tcs_is_free() new for v2; replaces old patch 6.
- Got rid of useless "if (x) continue" at end of for loop.
- ("Always use -EAGAIN, never -EBUSY") new for v2.

Douglas Anderson (10):
  drivers: qcom: rpmh-rsc: Clean code reading/writing 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: A lot of comments
  drivers: qcom: rpmh-rsc: Comment tcs_is_free() + warn if state
    mismatch
  drivers: qcom: rpmh-rsc: Warning if tcs_write() used for non-active
  drivers: qcom: rpmh-rsc: spin_lock_irqsave() for tcs_invalidate()
  drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire
  drivers: qcom: rpmh-rsc: Always use -EAGAIN, never -EBUSY

 drivers/soc/qcom/rpmh-internal.h |  52 ++--
 drivers/soc/qcom/rpmh-rsc.c      | 443 +++++++++++++++++++++++--------
 2 files changed, 358 insertions(+), 137 deletions(-)

-- 
2.25.1.481.gfbce0eb801-goog


             reply	other threads:[~2020-03-11 23:14 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-11 23:13 Douglas Anderson [this message]
2020-03-11 23:13 ` [RFT PATCH v2 01/10] drivers: qcom: rpmh-rsc: Clean code reading/writing regs/cmds Douglas Anderson
2020-04-01  8:12   ` Maulik Shah
2020-03-11 23:13 ` [RFT PATCH v2 02/10] drivers: qcom: rpmh-rsc: Document the register layout better Douglas Anderson
2020-04-01  8:14   ` Maulik Shah
2020-04-02 20:15     ` Doug Anderson
2020-03-11 23:13 ` [RFT PATCH v2 03/10] drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller Douglas Anderson
2020-04-01  8:17   ` Maulik Shah
2020-03-11 23:13 ` [RFT PATCH v2 04/10] drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction Douglas Anderson
2020-04-01  8:18   ` Maulik Shah
2020-03-11 23:13 ` [RFT PATCH v2 05/10] drivers: qcom: rpmh-rsc: A lot of comments Douglas Anderson
2020-04-01 11:29   ` Maulik Shah
2020-04-02 20:18     ` Doug Anderson
2020-04-08 11:10       ` Maulik Shah
2020-03-11 23:13 ` [RFT PATCH v2 06/10] drivers: qcom: rpmh-rsc: Comment tcs_is_free() + warn if state mismatch Douglas Anderson
2020-04-01 11:38   ` Maulik Shah
2020-04-02 20:19     ` Doug Anderson
2020-03-11 23:13 ` [RFT PATCH v2 07/10] drivers: qcom: rpmh-rsc: Warning if tcs_write() used for non-active Douglas Anderson
2020-04-01 12:40   ` Maulik Shah
2020-04-02 20:19     ` Doug Anderson
2020-03-11 23:13 ` [RFT PATCH v2 08/10] drivers: qcom: rpmh-rsc: spin_lock_irqsave() for tcs_invalidate() Douglas Anderson
2020-03-26 21:44   ` Doug Anderson
2020-03-11 23:13 ` [RFT PATCH v2 09/10] drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire Douglas Anderson
2020-03-11 23:13 ` [RFT PATCH v2 10/10] drivers: qcom: rpmh-rsc: Always use -EAGAIN, never -EBUSY Douglas Anderson

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=20200311231348.129254-1-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.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).