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, 31 Oct 2022 15:31:33 -0500
From:   Andrew Halaney <ahalaney@redhat.com>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc:     martin.petersen@oracle.com, jejb@linux.ibm.com,
        andersson@kernel.org, vkoul@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, konrad.dybcio@somainline.org,
        robh+dt@kernel.org, quic_cang@quicinc.com,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-phy@lists.infradead.org,
        linux-scsi@vger.kernel.org, dmitry.baryshkov@linaro.org
Subject: Re: [PATCH v2 13/15] scsi: ufs: ufs-qcom: Factor out the logic
 finding the HS Gear
Message-ID: <20221031203133.vmqqig7wlryrgiwv@halaney-x13s>
References: <20221031180217.32512-1-manivannan.sadhasivam@linaro.org>
 <20221031180217.32512-14-manivannan.sadhasivam@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221031180217.32512-14-manivannan.sadhasivam@linaro.org>
Precedence: bulk

On Mon, Oct 31, 2022 at 11:32:15PM +0530, Manivannan Sadhasivam wrote:
> In the preparation of adding support for new gears, let's move the
> logic that finds the gear for each platform to a new function. This helps
> with code readability and also allows the logic to be used in other places
> of the driver in future.
> 
> While at it, let's make it clear that this driver only supports symmetric
> gear setting (hs_tx_gear == hs_rx_gear).
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>

> ---
>  drivers/ufs/host/ufs-qcom.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 38e2ed749d75..c93d2d38b43e 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -278,6 +278,26 @@ static int ufs_qcom_host_reset(struct ufs_hba *hba)
>  	return 0;
>  }
>  
> +static u32 ufs_qcom_get_hs_gear(struct ufs_hba *hba, u32 hs_gear)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +
> +	if (host->hw_ver.major == 0x1) {
> +		/*
> +		 * HS-G3 operations may not reliably work on legacy QCOM
> +		 * UFS host controller hardware even though capability
> +		 * exchange during link startup phase may end up
> +		 * negotiating maximum supported gear as G3.
> +		 * Hence downgrade the maximum supported gear to HS-G2.
> +		 */
> +		if (hs_gear > UFS_HS_G2)
> +			return UFS_HS_G2;
> +	}
> +
> +	/* Default is HS-G3 */
> +	return UFS_HS_G3;
> +}
> +
>  static int ufs_qcom_power_up_sequence(struct ufs_hba *hba)
>  {
>  	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> @@ -692,19 +712,9 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba,
>  		ufshcd_init_pwr_dev_param(&ufs_qcom_cap);
>  		ufs_qcom_cap.hs_rate = UFS_QCOM_LIMIT_HS_RATE;
>  
> -		if (host->hw_ver.major == 0x1) {
> -			/*
> -			 * HS-G3 operations may not reliably work on legacy QCOM
> -			 * UFS host controller hardware even though capability
> -			 * exchange during link startup phase may end up
> -			 * negotiating maximum supported gear as G3.
> -			 * Hence downgrade the maximum supported gear to HS-G2.
> -			 */
> -			if (ufs_qcom_cap.hs_tx_gear > UFS_HS_G2)
> -				ufs_qcom_cap.hs_tx_gear = UFS_HS_G2;
> -			if (ufs_qcom_cap.hs_rx_gear > UFS_HS_G2)
> -				ufs_qcom_cap.hs_rx_gear = UFS_HS_G2;
> -		}
> +		/* This driver only supports symmetic gear setting i.e., hs_tx_gear == hs_rx_gear */
> +		ufs_qcom_cap.hs_tx_gear = ufs_qcom_cap.hs_rx_gear = ufs_qcom_get_hs_gear(hba,
> +									ufs_qcom_cap.hs_tx_gear);
>  
>  		ret = ufshcd_get_pwr_dev_param(&ufs_qcom_cap,
>  					       dev_max_params,
> -- 
> 2.25.1
>