Re: [RFC PATCH v1] scsi: ufs: Remove pre-defined initial VCC voltage values

On Mon, 2020-11-30 at 15:54 -0800, Asutosh Das (asd) wrote:

On 11/30/2020 3:14 PM, Bjorn Andersson wrote:

quoted

On Mon 30 Nov 16:51 CST 2020, Asutosh Das (asd) wrote:

quoted

On 11/30/2020 1:16 AM, Stanley Chu wrote:

quoted

UFS specficication allows different VCC configurations for UFS devices,
for example,
	(1). 2.70V - 3.60V (By default)
	(2). 1.70V - 1.95V (Activated if "vcc-supply-1p8" is declared in
                            device tree)
	(3). 2.40V - 2.70V (Supported since UFS 3.x)

With the introduction of UFS 3.x products, an issue is happening that
UFS driver will use wrong "min_uV/max_uV" configuration to toggle VCC
regulator on UFU 3.x products with VCC configuration (3) used.

To solve this issue, we simply remove pre-defined initial VCC voltage
values in UFS driver with below reasons,

1. UFS specifications do not define how to detect the VCC configuration
     supported by attached device.

2. Device tree already supports standard regulator properties.

Therefore VCC voltage shall be defined correctly in device tree, and
shall not be changed by UFS driver. What UFS driver needs to do is simply
enabling or disabling the VCC regulator only.

This is a RFC conceptional patch. Please help review this and feel
free to feedback any ideas. Once this concept is accepted, and then
I would post a more completed patch series to fix this issue.

Signed-off-by: Stanley Chu <redacted>
---
   drivers/scsi/ufs/ufshcd-pltfrm.c | 10 +---------
   1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index a6f76399b3ae..3965be03c136 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c

@@ -133,15 +133,7 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
   		vreg->max_uA = 0;
   	}
-	if (!strcmp(name, "vcc")) {
-		if (of_property_read_bool(np, "vcc-supply-1p8")) {
-			vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV;
-			vreg->max_uV = UFS_VREG_VCC_1P8_MAX_UV;
-		} else {
-			vreg->min_uV = UFS_VREG_VCC_MIN_UV;
-			vreg->max_uV = UFS_VREG_VCC_MAX_UV;
-		}
-	} else if (!strcmp(name, "vccq")) {
+	if (!strcmp(name, "vccq")) {
   		vreg->min_uV = UFS_VREG_VCCQ_MIN_UV;
   		vreg->max_uV = UFS_VREG_VCCQ_MAX_UV;
   	} else if (!strcmp(name, "vccq2")) {

Hi Stanley

Thanks for the patch. Bao (nguyenb) was also working towards something
similar.
Would it be possible for you to take into account the scenario in which the
same platform supports both 2.x and 3.x UFS devices?

These've different voltage requirements, 2.4v-3.6v.
I'm not sure if standard dts regulator properties can support this.

What is the actual voltage requirement for these devices and how does
the software know what voltage to pick in this range?

Regards,
Bjorn

quoted

-asd


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
Linux Foundation Collaborative Project

For platforms that support both 2.x (2.7v-3.6v) and 3.x (2.4v-2.7v), the 
voltage requirements (Vcc) are 2.4v-3.6v. The software initializes the 
ufs device at 2.95v & reads the version and if the device is 3.x, it may 
do the following:
- Set the device power mode to SLEEP
- Disable the Vcc
- Enable the Vcc and set it to 2.5v
- Set the device power mode to ACTIVE

All of the above may be done at HS-G1 & moved to max supported gear 
based on the device version, perhaps?

Hi Asutosh,

Thanks for sharing this idea.

1. I did not see above flow defined in UFS specifications, please
correct me if I was wrong.

2. For above flow, the concern is that I am not sure if all devices
supporting VCC (2.4v - 2.7v) can accept higher voltage, say 2.95v, for
version detection.

3. For version detection, another concern is that I am not sure if all
3.x devices support VCC (2.4v - 2.7v) only, or in other words, I am not
sure if all 2.x devices support VCC (2.7v - 3.6v) only. The above rule
will break any devices not obeying this "conventions".

For platforms that support both 2.x (2.7v-3.6v) and 3.x (2.4v-2.7v),

It would be good for UFS drivers detecting the correct voltage if the
protocol is well-defined in specifications. Until that day, any
"non-standard" way may be better implemented in vendor's ops?

If the vop concept works on your platform, we could still keep struct
ufs_vreg and allow vendors to configure proper min_uV and max_uV to make
regulator_set_voltage() works during VCC toggling flow. Without specific
vendor configurations, min_uV and max_uV would be NULL by default and
UFS core driver will only enable/disasble VCC regulator only without
adjusting its voltage.

Maybe one possible another idea is to decide the correct voltage and
configure regulator properly before kernel?

Thanks,
Stanley Chu

Am open to other ideas though.

-asd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel