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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-12-01 11:19, Asutosh Das (asd) wrote:
On 11/30/2020 6:53 PM, Bjorn Andersson wrote:
On Mon 30 Nov 17:54 CST 2020, Asutosh Das (asd) wrote:

On 11/30/2020 3:14 PM, Bjorn Andersson wrote:
On Mon 30 Nov 16:51 CST 2020, Asutosh Das (asd) wrote:

On 11/30/2020 1:16 AM, Stanley Chu wrote:
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 <stanley.chu@xxxxxxxxxxxx>
---
    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

-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?

Am open to other ideas though.


But that means that for a board where we don't know (don't want to know)
if we have a 2.x or 3.x device we need to set:

   regulator-min-microvolt = <2.4V>
   regulator-max-microvolt = <3.6V>

And the 2.5V and the two ranges should be hard coded into the ufshcd (in
particular if they come from the specification).

For devices with only 2.x or 3.x devices, regulator-{min,max}-microvolt
should be adjusted accordingly.

Note that driving the regulators outside these ranges will either damage the hardware or cause it to misbehave, so these values should be defined
in the board.dts anyways.

Also note that regulator_set_voltage(2.4V, 3.6V) won't give you "a
voltage between 2.4V and 3.6V, it will most likely give either 2.4V or
any more specific voltage that we've specified in the board file because the regulator happens to be shared with some other consumer and changing
it in runtime would be bad.

Regards,
Bjorn


Understood.
I also understand that assumptions on the regulator limits in the
driver is a bad idea. I'm not sure how it's designed, but I should
think the power-grid design should take care of regulator sharing; if
it's being shared and the platform supports both 2.x and 3.x. Perhaps,
such platforms be identified using a dts flag - not sure if that's
such a good idea though.

I like Stanley's proposal of a vops and let vendors handle it, until
specs or someone has a better suggestion.

Agree, vops is all we need as of now, please upload a change to add one properly.

Thanks,

Can Guo.


-asd



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux