Revert "scsi: ufs: disable vccq if it's not needed by UFS device"
authorMarc Gonzalez <marc.w.gonzalez@free.fr>
Wed, 27 Feb 2019 10:41:45 +0000 (11:41 +0100)
committerMartin K. Petersen <martin.petersen@oracle.com>
Wed, 27 Feb 2019 13:53:42 +0000 (08:53 -0500)
This reverts commit 60f0187031c05e04cbadffb62f557d0ff3564490.

There was one conflict in drivers/scsi/ufs/ufshcd.c

<<<<<<< HEAD
/* Init check for device descriptor sizes */
ufshcd_init_desc_sizes(hba);

ret = ufs_get_device_desc(hba, &card);
if (ret) {
dev_err(hba->dev, "%s: Failed getting device info. err = %d\n",
__func__, ret);
goto out;
}

ufs_fixup_device_setup(hba, &card);
ufshcd_tune_unipro_params(hba);

ret = ufshcd_set_vccq_rail_unused(hba,
(hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
if (ret)
goto out;

=======
ufs_advertise_fixup_device(hba);
>>>>>>> parent of 60f0187031c0... scsi: ufs: disable vccq if it's not needed by UFS device

Resolution: keep HEAD, and delete the ufshcd_set_vccq_rail_unused() call
and corresponding error-handling code.

Clean up loose ends in a follow-up patch.

60f0187031c0 introduced a small power optimization: ignore the vccq load
specified in the UFSHC DT node when said host controller is connected to
specific Flash chips (currently, Samsung and Hynix).

Unfortunately, this optimization breaks UFS on systems where vccq powers
not only the Flash chip, but the host controller as well, such as APQ8098
MEDIABOX or MTP8998:

[    3.929877] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[    5.433815] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[    6.937771] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr: opcode 0x04 for idn 13 failed, index 0, err = -11
[    6.937866] ufshcd-qcom 1da4000.ufshc: ufshcd_query_attr_retry: query attribute, idn 13, failed with error -11 after 3 retires
[    6.946412] ufshcd-qcom 1da4000.ufshc: ufshcd_disable_auto_bkops: failed to enable exception event -11
[    6.957972] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1587 failed 3 retries
[    6.967181] ufshcd-qcom 1da4000.ufshc: dme-peer-get: attr-id 0x1586 failed 3 retries
[    6.975025] ufshcd-qcom 1da4000.ufshc: ufshcd_get_max_pwr_mode: invalid max pwm tx gear read = 0
[    6.982755] ufshcd-qcom 1da4000.ufshc: ufshcd_probe_hba: Failed getting max supported power mode
[    8.505770] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[   10.009807] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[   11.513766] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag: Sending flag query for idn 3 failed, err = -11
[   11.513861] ufshcd-qcom 1da4000.ufshc: ufshcd_query_flag_retry: query attribute, opcode 5, idn 3, failed with error -11 after 3 retires
[   13.049807] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[   14.553768] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[   16.057767] ufshcd-qcom 1da4000.ufshc: __ufshcd_query_descriptor: opcode 0x01 for idn 8 failed, index 0, err = -11
[   16.057872] ufshcd-qcom 1da4000.ufshc: ufshcd_read_desc_param: Failed reading descriptor. desc_id 8, desc_index 0, param_offset 0, ret -11
[   16.067109] ufshcd-qcom 1da4000.ufshc: ufshcd_init_icc_levels: Failed reading power descriptor.len = 98 ret = -11
[   37.073787] ufshcd-qcom 1da4000.ufshc: link startup failed 1

In my opinion, the rationale for the original patch is questionable.  If
neither the UFSHC, nor the Flash chip, require any load from vccq, then
that power rail should simply not be specified at all in the DT.

Working around that fact in the driver is detrimental, as evidenced by the
failure to initialize the host controller on MSM8998.

Acked-by: Avri Altman <avri.altman@wdc.com>
Acked-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Marc Gonzalez <marc.w.gonzalez@free.fr>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/ufs/ufs.h
drivers/scsi/ufs/ufshcd.c

index dd65fea07687dd2611b58cc000ef2b2646a16ff6..7da7318eb6a625f6423ac279d32a3d3e1948b5c4 100644 (file)
@@ -514,7 +514,6 @@ struct ufs_vreg {
        struct regulator *reg;
        const char *name;
        bool enabled;
        struct regulator *reg;
        const char *name;
        bool enabled;
-       bool unused;
        int min_uV;
        int max_uV;
        int min_uA;
        int min_uV;
        int max_uV;
        int min_uA;
index f90badcb83184e2d448e98e1db9ca4f5dc7ec9e5..59b980712e7ce84ecff8acd4fc9cba6a8b523a41 100644 (file)
@@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
 static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
                                 bool skip_ref_clk);
 static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
 static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
                                 bool skip_ref_clk);
 static int ufshcd_setup_clocks(struct ufs_hba *hba, bool on);
-static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused);
 static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
 static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
 static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
 static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba);
 static int ufshcd_uic_hibern8_enter(struct ufs_hba *hba);
 static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba);
@@ -6824,11 +6823,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
        ufs_fixup_device_setup(hba, &card);
        ufshcd_tune_unipro_params(hba);
 
        ufs_fixup_device_setup(hba, &card);
        ufshcd_tune_unipro_params(hba);
 
-       ret = ufshcd_set_vccq_rail_unused(hba,
-               (hba->dev_quirks & UFS_DEVICE_NO_VCCQ) ? true : false);
-       if (ret)
-               goto out;
-
        /* UFS device is also active now */
        ufshcd_set_ufs_dev_active(hba);
        ufshcd_force_reset_auto_bkops(hba);
        /* UFS device is also active now */
        ufshcd_set_ufs_dev_active(hba);
        ufshcd_force_reset_auto_bkops(hba);
@@ -7012,24 +7006,13 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
 static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
                                         struct ufs_vreg *vreg)
 {
 static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
                                         struct ufs_vreg *vreg)
 {
-       if (!vreg)
-               return 0;
-       else if (vreg->unused)
-               return 0;
-       else
-               return ufshcd_config_vreg_load(hba->dev, vreg,
-                                              UFS_VREG_LPM_LOAD_UA);
+       return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
 }
 
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
                                         struct ufs_vreg *vreg)
 {
 }
 
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
                                         struct ufs_vreg *vreg)
 {
-       if (!vreg)
-               return 0;
-       else if (vreg->unused)
-               return 0;
-       else
-               return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
+       return ufshcd_config_vreg_load(hba->dev, vreg, vreg->max_uA);
 }
 
 static int ufshcd_config_vreg(struct device *dev,
 }
 
 static int ufshcd_config_vreg(struct device *dev,
@@ -7067,9 +7050,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
 {
        int ret = 0;
 
 {
        int ret = 0;
 
-       if (!vreg)
-               goto out;
-       else if (vreg->enabled || vreg->unused)
+       if (!vreg || vreg->enabled)
                goto out;
 
        ret = ufshcd_config_vreg(dev, vreg, true);
                goto out;
 
        ret = ufshcd_config_vreg(dev, vreg, true);
@@ -7089,9 +7070,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
 {
        int ret = 0;
 
 {
        int ret = 0;
 
-       if (!vreg)
-               goto out;
-       else if (!vreg->enabled || vreg->unused)
+       if (!vreg || !vreg->enabled)
                goto out;
 
        ret = regulator_disable(vreg->reg);
                goto out;
 
        ret = regulator_disable(vreg->reg);
@@ -7197,36 +7176,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba)
        return 0;
 }
 
        return 0;
 }
 
-static int ufshcd_set_vccq_rail_unused(struct ufs_hba *hba, bool unused)
-{
-       int ret = 0;
-       struct ufs_vreg_info *info = &hba->vreg_info;
-
-       if (!info)
-               goto out;
-       else if (!info->vccq)
-               goto out;
-
-       if (unused) {
-               /* shut off the rail here */
-               ret = ufshcd_toggle_vreg(hba->dev, info->vccq, false);
-               /*
-                * Mark this rail as no longer used, so it doesn't get enabled
-                * later by mistake
-                */
-               if (!ret)
-                       info->vccq->unused = true;
-       } else {
-               /*
-                * rail should have been already enabled hence just make sure
-                * that unused flag is cleared.
-                */
-               info->vccq->unused = false;
-       }
-out:
-       return ret;
-}
-
 static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
                                        bool skip_ref_clk)
 {
 static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
                                        bool skip_ref_clk)
 {