diff mbox series

[RESEND,v3,3/5] scsi: ufs: Fix regulator load and icc-level configuration

Message ID 1553680707-28579-4-git-send-email-stanley.chu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series scsi: ufs: Fix regulator operations and remove "<name>-fixed-regulator" device tree property | expand

Commit Message

Stanley Chu March 27, 2019, 9:58 a.m. UTC
Currently if a regulator has "<name>-fixed-regulator"
property in device tree, it will skip current limit initialization.
This lead to a zero "max_uA" value in struct ufs_vreg.

However, "regulator_set_load" operation shall be required
on regulators which have valid current limits, otherwise a zero
"max_uA" set by "regulator_set_load" may cause unexpected behavior
when this regulator is enabled or set as high power mode.

Similarly, in device's icc_level configuration flow, the target
icc_level shall be updated if regulator also has valid current limit,
otherwise a wrong icc_level will be calculated by zero "max_uA" and
thus causes unexpected results after it is written to device.

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

Comments

Avri Altman March 27, 2019, 10:55 a.m. UTC | #1
> 
> Currently if a regulator has "<name>-fixed-regulator"
> property in device tree, it will skip current limit initialization.
> This lead to a zero "max_uA" value in struct ufs_vreg.
> 
> However, "regulator_set_load" operation shall be required
> on regulators which have valid current limits, otherwise a zero
> "max_uA" set by "regulator_set_load" may cause unexpected behavior
> when this regulator is enabled or set as high power mode.
> 
> Similarly, in device's icc_level configuration flow, the target
> icc_level shall be updated if regulator also has valid current limit,
> otherwise a wrong icc_level will be calculated by zero "max_uA" and
> thus causes unexpected results after it is written to device.
> 
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 81d99aebb867..5ba49c8cd2a3 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6294,19 +6294,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,
@@ -7004,6 +7004,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",