Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 16 Jan 2023 09:43:01 -0700
From:   Mathieu Poirier <mathieu.poirier@linaro.org>
To:     Devarsh Thakkar <devarsht@ti.com>
Cc:     andersson@kernel.org, devicetree@vger.kernel.org,
        p.zabel@pengutronix.de, linux-remoteproc@vger.kernel.org,
        robh+dt@kernel.org, linux-kernel@vger.kernel.org,
        krzysztof.kozlowski+dt@linaro.org, s-anna@ti.com, hnagalla@ti.com,
        praneeth@ti.com, nm@ti.com, vigneshr@ti.com, a-bhatia1@ti.com,
        j-luthra@ti.com
Subject: Re: [PATCH v6 1/3] remoteproc: k3-r5: Simplify single core config set
Message-ID: <20230116164301.GA3239767@p14s>
References: <20230116151906.549384-1-devarsht@ti.com>
 <20230116151906.549384-2-devarsht@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230116151906.549384-2-devarsht@ti.com>
Precedence: bulk

On Mon, Jan 16, 2023 at 08:49:04PM +0530, Devarsh Thakkar wrote:
> The config PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE
> is set only when cluster mode is set to
> CLUSTER_MODE_SINGLECPU and cluster mode
> is already configured before setting this config.
> 
> So directly check for cluster mode instead of checking
> soc_data->single_cpu_mode first and then checking
> cluster mode.
> 
> Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
> ---
> V6: No change
> ---
>  drivers/remoteproc/ti_k3_r5_remoteproc.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> index 0481926c6975..036c9dc217f3 100644
> --- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
> +++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
> @@ -875,15 +875,14 @@ static int k3_r5_rproc_configure(struct k3_r5_rproc *kproc)
>  	boot_vec = 0x0;
>  	if (core == core0) {
>  		clr_cfg = PROC_BOOT_CFG_FLAG_R5_TEINIT;
> -		if (cluster->soc_data->single_cpu_mode) {
> -			/*
> -			 * Single-CPU configuration bit can only be configured
> -			 * on Core0 and system firmware will NACK any requests
> -			 * with the bit configured, so program it only on
> -			 * permitted cores
> -			 */
> -			if (cluster->mode == CLUSTER_MODE_SINGLECPU)
> -				set_cfg = PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE;
> +		/*
> +		 * Single-CPU configuration bit can only be configured
> +		 * on Core0 and system firmware will NACK any requests
> +		 * with the bit configured, so program it only on
> +		 * permitted cores
> +		 */
> +		if (cluster->mode == CLUSTER_MODE_SINGLECPU) {
> +			set_cfg = PROC_BOOT_CFG_FLAG_R5_SINGLE_CORE;

This is exactly the kind of buggy situation I want to avoid by asking to use
soc_data->single_cpu_mode only in probe().

>  		} else {
>  			/*
>  			 * LockStep configuration bit is Read-only on Split-mode
> -- 
> 2.34.1
>