scsi: ufs: fix regulator operations and remove "<name>-fixed-regulator" device tree property

scsi: ufs: fix regulator operations and remove "<name>-fixed-regulator" device tree property

[Stanley Chu]

Hi,

This patch series fixes UFS regulator operations and removes "<name>-fixed-regulator" device tree property.

Stanley Chu (4):
  scsi: ufs: remove unused min_uA field in struct ufs_vreg
  scsi: ufs: fix regulator set load and icc-level configuration
  scsi: ufs: change "<name>-max-microamp" to non-mandatory property
  scsi: ufs: remove "<name>-fixed-regulator" device tree property

 drivers/scsi/ufs/ufs.h           |  1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 14 +++-----------
 drivers/scsi/ufs/ufshcd.c        | 15 ++++++++++++---
 3 files changed, 15 insertions(+), 15 deletions(-)

[PATCH v2 0/4] scsi: ufs: fix regulator operations and remove "<name>-fixed-regulator" device tree property

[Stanley Chu]

Hi,

This patch series fixes UFS regulator operations and removes "<name>-fixed-regulator" device tree property.

Stanley Chu (4):
  scsi: ufs: remove unused min_uA field in struct ufs_vreg
  scsi: ufs: fix regulator set load and icc-level configuration
  scsi: ufs: change "<name>-max-microamp" to non-mandatory property
  scsi: ufs: remove "<name>-fixed-regulator" device tree property

 drivers/scsi/ufs/ufs.h           |  1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 14 +++-----------
 drivers/scsi/ufs/ufshcd.c        | 15 ++++++++++++---
 3 files changed, 15 insertions(+), 15 deletions(-)

-- 
2.18.0

[PATCH v2 1/4] scsi: ufs: remove unused min_uA field in struct ufs_vreg

[Stanley Chu]

There are two fields related to regulator current limit in struct
ufs_vreg: "min_uA" and "max_uA".

"max_uA" is probed by "<name>-max-microamp" property from device
tree and used for

- regulator_set_load operations, and
- icc_level configuration in device.

However "min_uA" field is not used anywhere, thus we can remove it.

Signed-off-by: Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Reviewed-by: Marc Gonzalez <marc.w.gonzalez-GANU6spQydw@public.gmane.org>
---
 drivers/scsi/ufs/ufs.h           | 1 -
 drivers/scsi/ufs/ufshcd-pltfrm.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 7da7318eb6a6..0f23ac80bacd 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -516,7 +516,6 @@ struct ufs_vreg {
 	bool enabled;
 	int min_uV;
 	int max_uV;
-	int min_uA;
 	int max_uA;
 };
 
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 895a9b5ac989..588079286e8a 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -164,7 +164,6 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
 		goto out;
 	}
 
-	vreg->min_uA = 0;
 	if (!strcmp(name, "vcc")) {
 		if (of_property_read_bool(np, "vcc-supply-1p8")) {
 			vreg->min_uV = UFS_VREG_VCC_1P8_MIN_UV;
-- 
2.18.0

[PATCH v2 2/4] scsi: ufs: fix regulator set load and icc-level configuration

[Stanley Chu]

Currently if a regulator has "<name>-fixed-regulator"
property in device tree, it will skip current limit configuration.
This lead to a zero "max_uA" value in struct ufs_vreg.

However, "regulator_set_load" operation shall be required
on those regulators which specifically configured current
limit, otherwise a zero max_uA value may cause unexpected behavior
when this regulator is enabled or set as high power mode.

Similarly, in icc_level configuration flow, icc_level shall be
updated if specified regulator has also configured current limit,
otherwise a zero max_uA will lead to wrong icc_level which may
cause unexpected results after written to device.

Signed-off-by: Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/scsi/ufs/ufshcd.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8b9a01073d62..61cdae74de62 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6279,19 +6279,19 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
 		goto out;
 	}
 
-	if (hba->vreg_info.vcc)
+	if (hba->vreg_info.vcc && hba->vreg_info.vcc->max_uA)
 		icc_level = ufshcd_get_max_icc_level(
 				hba->vreg_info.vcc->max_uA,
 				POWER_DESC_MAX_ACTV_ICC_LVLS - 1,
 				&desc_buf[PWR_DESC_ACTIVE_LVLS_VCC_0]);
 
-	if (hba->vreg_info.vccq)
+	if (hba->vreg_info.vccq && hba->vreg_info.vccq->max_uA)
 		icc_level = ufshcd_get_max_icc_level(
 				hba->vreg_info.vccq->max_uA,
 				icc_level,
 				&desc_buf[PWR_DESC_ACTIVE_LVLS_VCCQ_0]);
 
-	if (hba->vreg_info.vccq2)
+	if (hba->vreg_info.vccq2 && hba->vreg_info.vccq2->max_uA)
 		icc_level = ufshcd_get_max_icc_level(
 				hba->vreg_info.vccq2->max_uA,
 				icc_level,
@@ -6989,6 +6989,15 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
 	if (!vreg)
 		return 0;
 
+	/*
+	 * "set_load" operation shall be required on those regulators
+	 * which specifically configured current limitation. Otherwise
+	 * zero max_uA may cause unexpected behavior when regulator is
+	 * enabled or set as high power mode.
+	 */
+	if (!vreg->max_uA)
+		return 0;
+
 	ret = regulator_set_load(vreg->reg, ua);
 	if (ret < 0) {
 		dev_err(dev, "%s: %s set load (ua=%d) failed, err=%d\n",
-- 
2.18.0

[PATCH v2 3/4] scsi: ufs: change "<name>-max-microamp" to non-mandatory property

[Stanley Chu]

In some platforms, vcc, vccq and vccq2 regulators may not need
to define their current limit but need to define their voltage
range for correct regulator_set_voltage behaviors.

However, missing "<name>-max-microamp" property in device tree
will lead to initialization fail. This limitation shall be
resolved to tolerate such kind of regulators.

Because we do bypass regulator_set_load operations in case
current limit is undefined, so this change shall be safe.

Signed-off-by: Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 588079286e8a..2f244d388ca8 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -157,11 +157,9 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
 		goto out;
 
 	snprintf(prop_name, MAX_PROP_SIZE, "%s-max-microamp", name);
-	ret = of_property_read_u32(np, prop_name, &vreg->max_uA);
-	if (ret) {
-		dev_err(dev, "%s: unable to find %s err %d\n",
-				__func__, prop_name, ret);
-		goto out;
+	if (of_property_read_u32(np, prop_name, &vreg->max_uA)) {
+		dev_info(dev, "%s: unable to find %s\n", __func__, prop_name);
+		vreg->max_uA = 0;
 	}
 
 	if (!strcmp(name, "vcc")) {
-- 
2.18.0

[PATCH v2 4/4] scsi: ufs: remove "<name>-fixed-regulator" device tree property

[Stanley Chu]

"<name>-fixed-regulator" device tree property can be
safely removed because below things are fixed or resolved,

- "<name>-max-microamp" becomes optional property: Undefined
  "<name>-max-microamp" will not cause initialization fail.

- regulator_set_load operation now has rules: Only those regulators
  which have configured current limit from "<name>-max-microamp"
  property is allowed to change its load.

The difference of regulators which define "<name>-fixed-regulator"
or not is listed as below,

- "<name>-max-microamp": If an existed regulator which defined
  "<name>-fixed-regulator", it shall be lack of "<name>-max-microamp"
  property in device tree thus regulator_set_load behaviors will be
  the same as before this patch.

- "vcc-supply-1p8": This only impacts "vcc-supply" regulator. However
  vcc shall not define "<name>-fixed-regulator" in device tree
  otherwise ufshcd_config_vreg() will use zero voltage values as
  request to regulator_set_voltage() and may lead to unexpected
  results.

Therefore this patch is safe for all existed regulators with
"<name>-fixed-regulator" property already used.

Signed-off-by: Stanley Chu <stanley.chu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 2f244d388ca8..a667e7ba1c8b 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -151,11 +151,6 @@ static int ufshcd_populate_vreg(struct device *dev, const char *name,
 
 	vreg->name = kstrdup(name, GFP_KERNEL);
 
-	/* if fixed regulator no need further initialization */
-	snprintf(prop_name, MAX_PROP_SIZE, "%s-fixed-regulator", name);
-	if (of_property_read_bool(np, prop_name))
-		goto out;
-
 	snprintf(prop_name, MAX_PROP_SIZE, "%s-max-microamp", name);
 	if (of_property_read_u32(np, prop_name, &vreg->max_uA)) {
 		dev_info(dev, "%s: unable to find %s\n", __func__, prop_name);
-- 
2.18.0