Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 19 Aug 2021 22:33:01 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To:     Bhaumik Bhatt <bbhatt@codeaurora.org>
Cc:     linux-arm-msm@vger.kernel.org, hemantk@codeaurora.org,
        linux-kernel@vger.kernel.org, loic.poulain@linaro.org,
        quic_jhugo@quicinc.com
Subject: Re: [PATCH v1 2/2] bus: mhi: core: Optimize and update MMIO register
 write method
Message-ID: <20210819170301.GF200135@thinkpad>
References: <1629330634-36465-1-git-send-email-bbhatt@codeaurora.org>
 <1629330634-36465-3-git-send-email-bbhatt@codeaurora.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1629330634-36465-3-git-send-email-bbhatt@codeaurora.org>
Precedence: bulk

On Wed, Aug 18, 2021 at 04:50:34PM -0700, Bhaumik Bhatt wrote:
> As of now, MMIO writes done after ready state transition use the
> mhi_write_reg_field() API even though the whole register is being
> written in most cases. Optimize this process by using mhi_write_reg()
> API instead for those writes and use the mhi_write_reg_field()
> API for MHI config registers only.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/init.c | 64 ++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 0917465..e4be171 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -433,75 +433,65 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
>  	struct device *dev = &mhi_cntrl->mhi_dev->dev;
>  	struct {
>  		u32 offset;
> -		u32 mask;
> -		u32 shift;
>  		u32 val;
>  	} reg_info[] = {
>  		{
> -			CCABAP_HIGHER, U32_MAX, 0,
> +			CCABAP_HIGHER,
>  			upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
>  		},
>  		{
> -			CCABAP_LOWER, U32_MAX, 0,
> +			CCABAP_LOWER,
>  			lower_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
>  		},
>  		{
> -			ECABAP_HIGHER, U32_MAX, 0,
> +			ECABAP_HIGHER,
>  			upper_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr),
>  		},
>  		{
> -			ECABAP_LOWER, U32_MAX, 0,
> +			ECABAP_LOWER,
>  			lower_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr),
>  		},
>  		{
> -			CRCBAP_HIGHER, U32_MAX, 0,
> +			CRCBAP_HIGHER,
>  			upper_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr),
>  		},
>  		{
> -			CRCBAP_LOWER, U32_MAX, 0,
> +			CRCBAP_LOWER,
>  			lower_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr),
>  		},
>  		{
> -			MHICFG, MHICFG_NER_MASK, MHICFG_NER_SHIFT,
> -			mhi_cntrl->total_ev_rings,
> -		},
> -		{
> -			MHICFG, MHICFG_NHWER_MASK, MHICFG_NHWER_SHIFT,
> -			mhi_cntrl->hw_ev_rings,
> -		},
> -		{
> -			MHICTRLBASE_HIGHER, U32_MAX, 0,
> +			MHICTRLBASE_HIGHER,
>  			upper_32_bits(mhi_cntrl->iova_start),
>  		},
>  		{
> -			MHICTRLBASE_LOWER, U32_MAX, 0,
> +			MHICTRLBASE_LOWER,
>  			lower_32_bits(mhi_cntrl->iova_start),
>  		},
>  		{
> -			MHIDATABASE_HIGHER, U32_MAX, 0,
> +			MHIDATABASE_HIGHER,
>  			upper_32_bits(mhi_cntrl->iova_start),
>  		},
>  		{
> -			MHIDATABASE_LOWER, U32_MAX, 0,
> +			MHIDATABASE_LOWER,
>  			lower_32_bits(mhi_cntrl->iova_start),
>  		},
>  		{
> -			MHICTRLLIMIT_HIGHER, U32_MAX, 0,
> +			MHICTRLLIMIT_HIGHER,
>  			upper_32_bits(mhi_cntrl->iova_stop),
>  		},
>  		{
> -			MHICTRLLIMIT_LOWER, U32_MAX, 0,
> +			MHICTRLLIMIT_LOWER,
>  			lower_32_bits(mhi_cntrl->iova_stop),
>  		},
>  		{
> -			MHIDATALIMIT_HIGHER, U32_MAX, 0,
> +			MHIDATALIMIT_HIGHER,
>  			upper_32_bits(mhi_cntrl->iova_stop),
>  		},
>  		{
> -			MHIDATALIMIT_LOWER, U32_MAX, 0,
> +			MHIDATALIMIT_LOWER,
>  			lower_32_bits(mhi_cntrl->iova_stop),
>  		},
> -		{ 0, 0, 0 }
> +		{0, 0}
>  	};
>  
>  	dev_dbg(dev, "Initializing MHI registers\n");
> @@ -544,14 +534,22 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
>  	mhi_cntrl->mhi_cmd[PRIMARY_CMD_RING].ring.db_addr = base + CRDB_LOWER;
>  
>  	/* Write to MMIO registers */
> -	for (i = 0; reg_info[i].offset; i++) {
> -		ret = mhi_write_reg_field(mhi_cntrl, base, reg_info[i].offset,
> -					  reg_info[i].mask, reg_info[i].shift,
> -					  reg_info[i].val);
> -		if (ret) {
> -			dev_err(dev, "Unable to write to MMIO registers");
> -			return ret;
> -		}
> +	for (i = 0; reg_info[i].offset; i++)
> +		mhi_write_reg(mhi_cntrl, base, reg_info[i].offset,
> +			      reg_info[i].val);
> +
> +	ret = mhi_write_reg_field(mhi_cntrl, base, MHICFG, MHICFG_NER_MASK,
> +				  MHICFG_NER_SHIFT, mhi_cntrl->total_ev_rings);
> +	if (ret) {
> +		dev_err(dev, "Unable to read MHICFG register\n");
> +		return ret;
> +	}
> +
> +	ret = mhi_write_reg_field(mhi_cntrl, base, MHICFG, MHICFG_NHWER_MASK,
> +				  MHICFG_NHWER_SHIFT, mhi_cntrl->hw_ev_rings);
> +	if (ret) {
> +		dev_err(dev, "Unable to read MHICFG register\n");
> +		return ret;
>  	}
>  
>  	return 0;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>