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@xxxxxxxxxxxxxx > > 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 >