Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
* [RFT PATCH v2 00/10] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments
@ 2020-03-11 23:13 Douglas Anderson
  2020-03-11 23:13 ` [RFT PATCH v2 01/10] drivers: qcom: rpmh-rsc: Clean code reading/writing regs/cmds Douglas Anderson
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Douglas Anderson @ 2020-03-11 23:13 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Maulik Shah
  Cc: mka, Rajendra Nayak, evgreen, Lina Iyer, swboyd,
	Douglas Anderson, linux-arm-msm, linux-kernel

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 23:13 [RFT PATCH v2 00/10] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Douglas Anderson
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

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git