Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 12 Sep 2018 14:51:13 -0700
From:   Matthias Kaehlcke <mka@chromium.org>
To:     Raju P L S S S N <rplsssn@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, ilina@codeaurora.org
Subject: Re: [PATCH v2 4/6] drivers: qcom: rpmh-rsc: clear active mode
 configuration for waketcs
Message-ID: <20180912215113.GJ22824@google.com>
References: <1532685889-31345-1-git-send-email-rplsssn@codeaurora.org>
 <1532685889-31345-5-git-send-email-rplsssn@codeaurora.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1532685889-31345-5-git-send-email-rplsssn@codeaurora.org>
User-Agent: Mutt/1.9.2 (2017-12-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jul 27, 2018 at 03:34:47PM +0530, Raju P L S S S N wrote:
> From: "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org>
> 
> For RSCs that have sleep & wake TCS but no dedicated active TCS, wake
> TCS can be re-purposed to send active requests. Once the active requests
> are sent and response is received, the active mode configuration needs
> to be cleared so that controller can use wake TCS for sending wake
> requests in 'solver' state while executing low power modes.
> 
> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
> ---
>  drivers/soc/qcom/rpmh-rsc.c | 77 +++++++++++++++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> index 42d0041..19616c9 100644
> --- a/drivers/soc/qcom/rpmh-rsc.c
> +++ b/drivers/soc/qcom/rpmh-rsc.c
> @@ -199,6 +199,42 @@ static const struct tcs_request *get_req_from_tcs(struct rsc_drv *drv,
>  	return NULL;
>  }
>  
> +static void __tcs_trigger(struct rsc_drv *drv, int tcs_id, bool trigger)
> +{
> +	u32 enable;
> +
> +	/*
> +	 * HW req: Clear the DRV_CONTROL and enable TCS again
> +	 * While clearing ensure that the AMC mode trigger is cleared
> +	 * and then the mode enable is cleared.
> +	 */
> +	enable = read_tcs_reg(drv, RSC_DRV_CONTROL, tcs_id, 0);
> +	enable &= ~TCS_AMC_MODE_TRIGGER;
> +	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> +	enable &= ~TCS_AMC_MODE_ENABLE;
> +	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> +
> +	if (trigger) {
> +		/* Enable the AMC mode on the TCS and then trigger the TCS */
> +		enable = TCS_AMC_MODE_ENABLE;
> +		write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> +		enable |= TCS_AMC_MODE_TRIGGER;
> +		write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> +	}
> +}
> +
> +static inline void enable_tcs_irq(struct rsc_drv *drv, int tcs_id, bool enable)
> +{
> +	u32 data;
> +
> +	data = read_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, 0);
> +	if (enable)
> +		data |= BIT(tcs_id);
> +	else
> +		data &= ~BIT(tcs_id);
> +	write_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, data);
> +}
> +
>  /**
>   * tcs_tx_done: TX Done interrupt handler
>   */
> @@ -235,6 +271,21 @@ static irqreturn_t tcs_tx_done(int irq, void *p)
>  		}
>  
>  		trace_rpmh_tx_done(drv, i, req, err);
> +
> +		/*
> +		 * if wake tcs was re-purposed for sending active
> +		 * votes, clear AMC trigger & enable modes and
> +		 * disable interrupt for this TCS
> +		 */
> +		if (!drv->tcs[ACTIVE_TCS].num_tcs) {
> +			__tcs_trigger(drv, i, false);
> +			/*
> +			 * Disable interrupt for this TCS to avoid being
> +			 * spammed with interrupts coming when the solver
> +			 * sends its wake votes.
> +			 */
> +			enable_tcs_irq(drv, i, false);
> +		}
>  skip:
>  		/* Reclaim the TCS */
>  		write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i, 0);
> @@ -282,28 +333,6 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int tcs_id, int cmd_id,
>  	write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, tcs_id, cmd_enable);
>  }
>  
> -static void __tcs_trigger(struct rsc_drv *drv, int tcs_id)
> -{
> -	u32 enable;
> -
> -	/*
> -	 * HW req: Clear the DRV_CONTROL and enable TCS again
> -	 * While clearing ensure that the AMC mode trigger is cleared
> -	 * and then the mode enable is cleared.
> -	 */
> -	enable = read_tcs_reg(drv, RSC_DRV_CONTROL, tcs_id, 0);
> -	enable &= ~TCS_AMC_MODE_TRIGGER;
> -	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> -	enable &= ~TCS_AMC_MODE_ENABLE;
> -	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> -
> -	/* Enable the AMC mode on the TCS and then trigger the TCS */
> -	enable = TCS_AMC_MODE_ENABLE;
> -	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> -	enable |= TCS_AMC_MODE_TRIGGER;
> -	write_tcs_reg_sync(drv, RSC_DRV_CONTROL, tcs_id, enable);
> -}
> -
>  static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group *tcs,
>  				  const struct tcs_request *msg)
>  {
> @@ -374,10 +403,12 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg)
>  
>  	tcs->req[tcs_id - tcs->offset] = msg;
>  	set_bit(tcs_id, drv->tcs_in_use);
> +	if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type != ACTIVE_TCS)
> +		enable_tcs_irq(drv, tcs_id, true);

You might want to add a short comment here. In the context of this
patch it is (relatively) clear that the wake TCS is re-purposed for
active requests, and the interrupt is disabled by default for
non-active TCSes. However on it's own this isn't necessarily
evident without looking through the code.

>  	spin_unlock(&drv->lock);
>  
>  	__tcs_buffer_write(drv, tcs_id, 0, msg);
> -	__tcs_trigger(drv, tcs_id);
> +	__tcs_trigger(drv, tcs_id, true);
>  
>  done_write:
>  	spin_unlock_irqrestore(&tcs->lock, flags);

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>