diff mbox series

[v1,2/2] scsi: ufs: Support reading UFS's Vcc voltage from device tree

Message ID 69db325a09d5c3fa7fc260db031b1e498b601c25.1598939393.git.nguyenb@codeaurora.org
State New
Headers show
Series Supports Reading UFS's Vcc Voltage Levels from DT | expand

Commit Message

Bao D. Nguyen Sept. 1, 2020, 6 a.m. UTC 
The UFS specifications supports a range of Vcc operating voltage
from 2.4-3.6V depending on the device and manufacturers.
Allows selecting the UFS Vcc voltage level by setting the
UFS's entry vcc-voltage-level in the device tree. If UFS's
vcc-voltage-level setting is not found in the device tree,
use default values provided by the driver.

Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Bao D. Nguyen Sept. 14, 2020, 6:43 p.m. UTC | #1 
On 2020-09-13 02:37, Avri Altman wrote:
>> 

>> The UFS specifications supports a range of Vcc operating voltage

>> from 2.4-3.6V depending on the device and manufacturers.

>> Allows selecting the UFS Vcc voltage level by setting the

>> UFS's entry vcc-voltage-level in the device tree. If UFS's

>> vcc-voltage-level setting is not found in the device tree,

>> use default values provided by the driver.

>> 

>> Signed-off-by: Can Guo <cang@codeaurora.org>

>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>

>> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>

>> ---

>>  drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++---

>>  1 file changed, 12 insertions(+), 3 deletions(-)

>> 

>> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c 

>> b/drivers/scsi/ufs/ufshcd-pltfrm.c

>> index 3db0af6..48f429c 100644

>> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c

>> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c

>> @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct 

>> ufs_hba

>> *hba)

>>  static int ufshcd_populate_vreg(struct device *dev, const char *name,

>>                 struct ufs_vreg **out_vreg)

>>  {

>> -       int ret = 0;

>> +       int len, ret = 0;

>>         char prop_name[MAX_PROP_SIZE];

>>         struct ufs_vreg *vreg = NULL;

>>         struct device_node *np = dev->of_node;

>> +       const __be32 *prop;

>> 

>>         if (!np) {

>>                 dev_err(dev, "%s: non DT initialization\n", __func__);

>> @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device 

>> *dev,

>> const char *name,

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

>> +                       prop = of_get_property(np, 

>> "vcc-voltage-level", &len);

>> +                       if (!prop || (len != (2 * sizeof(__be32)))) {

>> +                               dev_warn(dev, "%s vcc-voltage-level 

>> property.\n",

>> +                                       prop ? "invalid format" : 

>> "no");

>> +                               vreg->min_uV = UFS_VREG_VCC_MIN_UV;

>> +                               vreg->max_uV = UFS_VREG_VCC_MAX_UV;

>> +                       } else {

>> +                               vreg->min_uV = be32_to_cpup(&prop[0]);

>> +                               vreg->max_uV = be32_to_cpup(&prop[1]);

>> +                       }

>>                 }

>>         } else if (!strcmp(name, "vccq")) {

>>                 vreg->min_uV = UFS_VREG_VCCQ_MIN_UV;

>> --

> Maybe instead call ufshcd_populate_vreg with the new name,

> To not break the function flow, and just add another else if ?

Could you please clarify your comments? Are you suggesting to create a 
new function?
Thank you.
Avri Altman Sept. 15, 2020, 6:51 a.m. UTC | #2 
> > Maybe instead call ufshcd_populate_vreg with the new name,

> > To not break the function flow, and just add another else if ?

> Could you please clarify your comments? Are you suggesting to create a

> new function?

> Thank you.

No, just call ufshcd_populate_vreg with the new name, e.g. something like:

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 3db0af6..9798d4c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -141,6 +141,8 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
                        vreg->min_uV = UFS_VREG_VCC_MIN_UV;
                        vreg->max_uV = UFS_VREG_VCC_MAX_UV;
                }
+       } else if (!strcmp(name, "vcc-voltage-level")) {
+               /* add your changes here */
        } else if (!strcmp(name, "vccq")) {
                vreg->min_uV = UFS_VREG_VCCQ_MIN_UV;
                vreg->max_uV = UFS_VREG_VCCQ_MAX_UV;
@@ -177,8 +179,12 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
                goto out;
 
        err = ufshcd_populate_vreg(dev, "vcc", &info->vcc);
-       if (err)
-               goto out;
+       if (err) {
+               err = ufshcd_populate_vreg(dev, "vcc-voltage-level",
+                                          &info->vcc);
+               if (err)
+                       goto out;
+       }
 
        err = ufshcd_populate_vreg(dev, "vccq", &info->vccq);
        if (err)
Bjorn Andersson Sept. 15, 2020, 1:46 p.m. UTC | #3 
On Tue 01 Sep 01:00 CDT 2020, Bao D. Nguyen wrote:

> The UFS specifications supports a range of Vcc operating voltage

> from 2.4-3.6V depending on the device and manufacturers.

> Allows selecting the UFS Vcc voltage level by setting the

> UFS's entry vcc-voltage-level in the device tree. If UFS's

> vcc-voltage-level setting is not found in the device tree,

> use default values provided by the driver.

> 


As stated in the reply to patch 1, this is not the right approach.  As
long as you have static min/max values requested by the driver you
should rely on the board's constraints for the regulator levels and
instead remove the min_uV/max_uV code from the driver.

Thanks,
Bjorn

> Signed-off-by: Can Guo <cang@codeaurora.org>

> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>

> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>

> ---

>  drivers/scsi/ufs/ufshcd-pltfrm.c | 15 ++++++++++++---

>  1 file changed, 12 insertions(+), 3 deletions(-)

> 

> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c

> index 3db0af6..48f429c 100644

> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c

> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c

> @@ -104,10 +104,11 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)

>  static int ufshcd_populate_vreg(struct device *dev, const char *name,

>  		struct ufs_vreg **out_vreg)

>  {

> -	int ret = 0;

> +	int len, ret = 0;

>  	char prop_name[MAX_PROP_SIZE];

>  	struct ufs_vreg *vreg = NULL;

>  	struct device_node *np = dev->of_node;

> +	const __be32 *prop;

>  

>  	if (!np) {

>  		dev_err(dev, "%s: non DT initialization\n", __func__);

> @@ -138,8 +139,16 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,

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

> +			prop = of_get_property(np, "vcc-voltage-level", &len);

> +			if (!prop || (len != (2 * sizeof(__be32)))) {

> +				dev_warn(dev, "%s vcc-voltage-level property.\n",

> +					prop ? "invalid format" : "no");

> +				vreg->min_uV = UFS_VREG_VCC_MIN_UV;

> +				vreg->max_uV = UFS_VREG_VCC_MAX_UV;

> +			} else {

> +				vreg->min_uV = be32_to_cpup(&prop[0]);

> +				vreg->max_uV = be32_to_cpup(&prop[1]);

> +			}

>  		}

>  	} else if (!strcmp(name, "vccq")) {

>  		vreg->min_uV = UFS_VREG_VCCQ_MIN_UV;

> -- 

> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,

> a Linux Foundation Collaborative Project

>
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 3db0af6..48f429c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -104,10 +104,11 @@  static int ufshcd_parse_clock_info(struct ufs_hba *hba)
 static int ufshcd_populate_vreg(struct device *dev, const char *name,
 		struct ufs_vreg **out_vreg)
 {
-	int ret = 0;
+	int len, ret = 0;
 	char prop_name[MAX_PROP_SIZE];
 	struct ufs_vreg *vreg = NULL;
 	struct device_node *np = dev->of_node;
+	const __be32 *prop;
 
 	if (!np) {
 		dev_err(dev, "%s: non DT initialization\n", __func__);
@@ -138,8 +139,16 @@  static int ufshcd_populate_vreg(struct device *dev, const char *name,
 			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;
+			prop = of_get_property(np, "vcc-voltage-level", &len);
+			if (!prop || (len != (2 * sizeof(__be32)))) {
+				dev_warn(dev, "%s vcc-voltage-level property.\n",
+					prop ? "invalid format" : "no");
+				vreg->min_uV = UFS_VREG_VCC_MIN_UV;
+				vreg->max_uV = UFS_VREG_VCC_MAX_UV;
+			} else {
+				vreg->min_uV = be32_to_cpup(&prop[0]);
+				vreg->max_uV = be32_to_cpup(&prop[1]);
+			}
 		}
 	} else if (!strcmp(name, "vccq")) {
 		vreg->min_uV = UFS_VREG_VCCQ_MIN_UV;