LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Doug Anderson <email@example.com> To: Andy Gross <firstname.lastname@example.org>, Bjorn Andersson <email@example.com>, Maulik Shah <firstname.lastname@example.org> Cc: Rajendra Nayak <email@example.com>, Matthias Kaehlcke <firstname.lastname@example.org>, Evan Green <email@example.com>, Stephen Boyd <firstname.lastname@example.org>, Lina Iyer <email@example.com>, linux-arm-msm <firstname.lastname@example.org>, LKML <email@example.com> Subject: Re: [RFT PATCH 6/9] drivers: qcom: rpmh-rsc: Only use "tcs_in_use" for ACTIVE_ONLY Date: Tue, 10 Mar 2020 17:33:24 -0700 [thread overview] Message-ID: <CAD=FV=U0OX-HFK8jcj5HU9_f9tPq13GEZpc7er2knMYS6sdDNA@mail.gmail.com> (raw) In-Reply-To: <20200306155707.RFT.6.Icf2213131ea652087f100129359052c83601f8b0@changeid> Hi, On Fri, Mar 6, 2020 at 4:00 PM Douglas Anderson <firstname.lastname@example.org> wrote: > > From trawling through the code (see the "A lot of comments" change) I > found that "tcs_in_use" was only kept up-to-date for ACTIVE_ONLY TCSs. > ...yet tcs_is_free() was checking the variable called from > tcs_invalidate() and tcs_invalidate() is only used for non-ACTIVE_ONLY > TCSs. > > Let's change tcs_invalidate() to just check the "RSC_DRV_STATUS" > register, which was presumably the important part. > > It also feels like for ACTIVE_ONLY TCSs that it probably wasn't > important to check the "RSC_DRV_STATUS". We'll keep doing it just in > case but we'll add a warning if it ever actually mattered. > > Signed-off-by: Douglas Anderson <email@example.com> > --- > > drivers/soc/qcom/rpmh-rsc.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) After other RPMH email threads, it's possible that this patch isn't quite right. ...but also the code wasn't quite right before. Specifically if we have 0 active TCSs then it's possible that we've used a wake TCS to send an active-only request. That would be a case where "tcs_in_use" might be set and we'd need to make sure that tcs_invalidate() checks it. However: 1. We need to add locking to tcs_invalidate() since "tcs_in_use" is protected by drv->lock and tcs_invalidate() didn't grab that lock. 2. We should add documentation explaining the situation. I'm still waiting on overall review / testing of my series, but I'll put it on my list to address this for v2. -Doug
next prev parent reply other threads:[~2020-03-11 0:33 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-06 23:59 [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 1/9] drivers: qcom: rpmh-rsc: Clean code reading/writing regs/cmds Douglas Anderson 2020-03-11 8:47 ` Maulik Shah 2020-03-11 15:03 ` Doug Anderson 2020-03-11 16:17 ` Matthias Kaehlcke 2020-03-11 19:30 ` Stephen Boyd 2020-03-06 23:59 ` [RFT PATCH 2/9] drivers: qcom: rpmh-rsc: Document the register layout better Douglas Anderson 2020-03-11 9:35 ` Maulik Shah 2020-03-11 15:27 ` Doug Anderson 2020-03-11 18:49 ` Evan Green 2020-03-11 20:08 ` Stephen Boyd 2020-03-11 22:35 ` Doug Anderson 2020-03-06 23:59 ` [RFT PATCH 3/9] drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller Douglas Anderson 2020-03-11 9:50 ` Maulik Shah 2020-03-06 23:59 ` [RFT PATCH 4/9] drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction Douglas Anderson 2020-03-11 12:02 ` Maulik Shah 2020-03-06 23:59 ` [RFT PATCH 5/9] drivers: qcom: rpmh-rsc: A lot of comments Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 6/9] drivers: qcom: rpmh-rsc: Only use "tcs_in_use" for ACTIVE_ONLY Douglas Anderson 2020-03-11 0:33 ` Doug Anderson [this message] 2020-03-06 23:59 ` [RFT PATCH 7/9] drivers: qcom: rpmh-rsc: Warning if tcs_write() used for non-active Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 8/9] drivers: qcom: rpmh-rsc: spin_lock_irqsave() for tcs_invalidate() Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 9/9] drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire Douglas Anderson 2020-03-11 0:35 ` Doug Anderson 2020-03-11 9:48 ` [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Maulik Shah
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='CAD=FV=U0OX-HFK8jcj5HU9_f9tPq13GEZpc7er2knMYS6sdDNA@mail.gmail.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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: linkBe 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).