LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: ilina@codeaurora.org
Cc: andy.gross@linaro.org, david.brown@linaro.org,
	linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org,
	rnayak@codeaurora.org, bjorn.andersson@linaro.org,
	linux-kernel@vger.kernel.org, sboyd@kernel.org,
	evgreen@chromium.org, dianders@chromium.org
Subject: Re: [PATCH v7 06/10] drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS
Date: Tue, 8 May 2018 10:25:47 -0700	[thread overview]
Message-ID: <20180508172547.GG19594@google.com> (raw)
In-Reply-To: <65db1bdd4cf9337337b41745179eb84b@codeaurora.org>

Hi Lina,

On Tue, May 08, 2018 at 10:14:33AM -0600, ilina@codeaurora.org wrote:
> On 2018-05-03 16:06, Matthias Kaehlcke wrote:
> Hi Matthias,
> 
> > Hi Lina,
> > 
> > On Wed, May 02, 2018 at 01:37:45PM -0600, Lina Iyer wrote:
> > > Allow sleep and wake commands to be cleared from the respective TCSes,
> > > so that they can be re-populated.
> > > 
> > > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> > > ---
> > > 
> > > Changes in v7:
> > > 	- Move bitmap_zero() outside the loop
> > > 
> > > Changes in v6:
> > > 	- remove unnecessary locks around __tcs_invalidate
> > > 	- rename function to tcs_invaldiate
> > > 
> > > Changes in v4:
> > > 	- refactored the rphm_rsc_invalidate()
> > > ---
> > >  drivers/soc/qcom/rpmh-rsc.c | 45
> > > +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > > 
> > > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> > > index 4e2144a14c31..42aedf2d80fe 100644
> > > --- a/drivers/soc/qcom/rpmh-rsc.c
> > > +++ b/drivers/soc/qcom/rpmh-rsc.c
> > > @@ -104,6 +104,51 @@ static struct tcs_group *get_tcs_of_type(struct
> > > rsc_drv *drv, int type)
> > >  	return &drv->tcs[type];
> > >  }
> > > 
> > > +static int tcs_invalidate(struct rsc_drv *drv, int type)
> > > +{
> > > +	int m;
> > 
> > nit: use tcs_id?
> > 
> It's an iterator. Hence didn't change this.
> 
> > > +	struct tcs_group *tcs;
> > > +
> > > +	tcs = get_tcs_of_type(drv, type);
> > > +	if (IS_ERR(tcs))
> > > +		return PTR_ERR(tcs);
> > > +
> > > +	spin_lock(&tcs->lock);
> > > +	if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) {
> > > +		spin_unlock(&tcs->lock);
> > > +		return 0;
> > > +	}
> > > +
> > > +	for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) {
> > > +		if (!tcs_is_free(drv, m)) {
> > > +			spin_unlock(&tcs->lock);
> > > +			return -EAGAIN;
> > > +		}
> > > +		write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0);
> > > +	}
> > > +	bitmap_zero(tcs->slots, MAX_TCS_SLOTS);
> > 
> > You didn't reply to (or address) my comment on v6:
> > 
> > It could occur that one or more TCSes are disabled, then tcs_is_free()
> > returns false for the next TCS and the function returns without having
> > updated tcs->slots for the TCSes that have been disabled.
> How do you mean TCS are disabled? I think I asked that question in my mail.
> Sorry, if I forgot to ask.
> 
> TCSes are either available for sending requests or they are busy sending
> requests. They cannot be disabled if they are present.
> Individual commands, however are enabled or disabled based on whether they
> have active requests or not.

Sorry for conflating terminologies, I don't have a firm grasp on that
yet, what I meant is a TCS being busy sending requests.

> What we are trying to do here is to cleanup the TCSes of their existing
> requests. Generally, sleep and wake TCSes are not used to send active state
> requests, they are sent through AMC/Active TCS. So they will be free.
> However, in the case of the Display RSC, there is no explicit TCS available
> for sending active state requests. So we overload the wake TCS to send the
> active state requests. Even in that case, TCS would have finished and should
> be free when this function is called. The tcs_is_free() check is addition to
> make sure the requirement is not violated.

If you are sure that tcs_is_free(tcs) basically should never return
false when tcs_invalidate() is called I won't insist. If it is a
requirement violation should the code path include a WARN?

  reply	other threads:[~2018-05-08 17:25 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02 19:37 [PATCH v7 00/10] drivers/qcom: add RPMH communication support Lina Iyer
2018-05-02 19:37 ` [PATCH v7 01/10] drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs Lina Iyer
2018-05-02 19:37 ` [PATCH v7 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs Lina Iyer
2018-05-02 20:37   ` Stephen Boyd
2018-05-02 19:37 ` [PATCH v7 03/10] drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE Lina Iyer
2018-05-02 19:45   ` Steven Rostedt
2018-05-02 19:37 ` [PATCH v7 04/10] drivers: qcom: rpmh: add RPMH helper functions Lina Iyer
2018-05-03 20:26   ` Doug Anderson
2018-05-04 20:50     ` Matthias Kaehlcke
2018-05-08 16:05     ` ilina
2018-05-10 22:37       ` Doug Anderson
2018-05-11 15:06         ` Lina Iyer
2018-05-11 20:14           ` Doug Anderson
2018-05-14 15:00             ` Lina Iyer
2018-05-02 19:37 ` [PATCH v7 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS Lina Iyer
2018-05-03 21:35   ` Matthias Kaehlcke
2018-05-08 16:16     ` ilina
2018-05-02 19:37 ` [PATCH v7 06/10] drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS Lina Iyer
2018-05-03 22:06   ` Matthias Kaehlcke
2018-05-08 16:14     ` ilina
2018-05-08 17:25       ` Matthias Kaehlcke [this message]
2018-05-02 19:37 ` [PATCH v7 07/10] drivers: qcom: rpmh: cache sleep/wake state requests Lina Iyer
2018-05-04 21:39   ` Matthias Kaehlcke
2018-05-02 19:37 ` [PATCH v7 08/10] drivers: qcom: rpmh: allow requests to be sent asynchronously Lina Iyer
2018-05-02 19:37 ` [PATCH v7 09/10] drivers: qcom: rpmh: add support for batch RPMH request Lina Iyer
2018-05-02 19:37 ` [PATCH v7 10/10] drivers: qcom: rpmh-rsc: allow active requests from wake TCS Lina Iyer

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=20180508172547.GG19594@google.com \
    --to=mka@chromium.org \
    --cc=andy.gross@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=david.brown@linaro.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=linux-soc@vger.kernel.org \
    --cc=rnayak@codeaurora.org \
    --cc=sboyd@kernel.org \
    --subject='Re: [PATCH v7 06/10] drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS' \
    /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

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).