Re: [PATCH 14/15] scsi: ufs: ufs-qcom: Add support for finding HS gear on new UFS versions

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	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
Subject: Re: [PATCH 14/15] scsi: ufs: ufs-qcom: Add support for finding HS gear on new UFS versions
Date: Thu, 3 Nov 2022 17:48:30 +0530	[thread overview]
Message-ID: <20221103121830.GC8434@thinkpad> (raw)
In-Reply-To: <d11609d5-963a-de41-86f9-7451f460b6fa@linaro.org>

On Wed, Nov 02, 2022 at 04:05:34PM -0400, Krzysztof Kozlowski wrote:
> On 31/10/2022 10:56, Manivannan Sadhasivam wrote:
> >>>   		if (hs_gear > UFS_HS_G2)
> >>>   			return UFS_HS_G2;
> >>> +	} else if (host->hw_ver.major > 0x3) {
> >>> +		/*
> >>> +		 * Starting from UFS controller v4, Qcom supports dual gear mode (i.e., the
> >>> +		 * controller/PHY can be configured to run in two gear speeds). But that
> >>> +		 * requires an agreement between the UFS controller and the device. Below
> >>> +		 * code tries to find the max gear of both and decides which gear to use.
> >>> +		 *
> >>> +		 * First get the max gear supported by the UFS device if available.
> >>> +		 * If the property is not defined in devicetree, then use the default gear.
> >>> +		 */
> >>> +		ret = of_property_read_u32(dev->of_node, "max-gear", &max_gear);
> >>> +		if (ret)
> >>> +			goto err_out;
> >>
> >> Can we detect the UFS device's max gear somehow? If not, the 'max-gear'
> >> property name doesn't sound good. Maybe calling it 'device-gear' would be
> >> better.
> >>
> > 
> > UFS device probing depends on PHY init sequence. So technically we cannot know
> > the max gear of the device without using an init sequence, but this information
> > is static and should be known to a board manufacturer. That's why I decided to
> > use this property. Another option is to use a fixed init sequence for probing
> > the device and do a re-init after knowing it's max gear but that is not
> > recommended.
> > 
> 
> Why it is not recommended? By whom? You init on some default low gear
> (support for some is mandated by UFS spec) and then allow faster gears
> while you know the capabilities.
> 

This approach is what used in Qcom downstream. I learned that when they tried
submitting to mailing list, it got rejected. So I came up with this approach.

Thanks,
Mani

> > We need "max" keyword because this property specifies the maximum gear at which
> > the device could operate and not necessarily the gear at which it operates.
> > Maybe, "max-device-gear" would make it clear.
> 
> 
> Best regards,
> Krzysztof
> 

-- 
மணிவண்ணன் சதாசிவம்