Commit 73067981 authored by Marc Gonzalez's avatar Marc Gonzalez Committed by Martin K. Petersen

Revert "scsi: ufs: disable vccq if it's not needed by UFS device"

This reverts commit 60f01870.

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 60f01870... 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.

60f01870 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: default avatarAvri Altman <avri.altman@wdc.com>
Acked-by: default avatarAlim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: default avatarMarc Gonzalez <marc.w.gonzalez@free.fr>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 379c003f
...@@ -514,7 +514,6 @@ struct ufs_vreg { ...@@ -514,7 +514,6 @@ struct ufs_vreg {
struct regulator *reg; struct regulator *reg;
const char *name; const char *name;
bool enabled; bool enabled;
bool unused;
int min_uV; int min_uV;
int max_uV; int max_uV;
int min_uA; int min_uA;
......
...@@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba); ...@@ -245,7 +245,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba);
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); 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);
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_exit(struct ufs_hba *hba);
static int ufshcd_uic_hibern8_enter(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 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) ...@@ -6824,11 +6823,6 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
ufs_fixup_device_setup(hba, &card); ufs_fixup_device_setup(hba, &card);
ufshcd_tune_unipro_params(hba); 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 */ /* UFS device is also active now */
ufshcd_set_ufs_dev_active(hba); ufshcd_set_ufs_dev_active(hba);
ufshcd_force_reset_auto_bkops(hba); ufshcd_force_reset_auto_bkops(hba);
...@@ -7012,23 +7006,12 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg, ...@@ -7012,23 +7006,12 @@ static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba, static inline int ufshcd_config_vreg_lpm(struct ufs_hba *hba,
struct ufs_vreg *vreg) struct ufs_vreg *vreg)
{ {
if (!vreg) return ufshcd_config_vreg_load(hba->dev, vreg, UFS_VREG_LPM_LOAD_UA);
return 0;
else if (vreg->unused)
return 0;
else
return ufshcd_config_vreg_load(hba->dev, vreg,
UFS_VREG_LPM_LOAD_UA);
} }
static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba, static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
struct ufs_vreg *vreg) 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);
} }
...@@ -7067,9 +7050,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg) ...@@ -7067,9 +7050,7 @@ static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
{ {
int ret = 0; int ret = 0;
if (!vreg) if (!vreg || vreg->enabled)
goto out;
else if (vreg->enabled || vreg->unused)
goto out; goto out;
ret = ufshcd_config_vreg(dev, vreg, true); ret = ufshcd_config_vreg(dev, vreg, true);
...@@ -7089,9 +7070,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg) ...@@ -7089,9 +7070,7 @@ static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
{ {
int ret = 0; int ret = 0;
if (!vreg) if (!vreg || !vreg->enabled)
goto out;
else if (!vreg->enabled || vreg->unused)
goto out; goto out;
ret = regulator_disable(vreg->reg); ret = regulator_disable(vreg->reg);
...@@ -7197,36 +7176,6 @@ static int ufshcd_init_hba_vreg(struct ufs_hba *hba) ...@@ -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, static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
bool skip_ref_clk) bool skip_ref_clk)
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment