From 59413659e7f07931151bf646c6c614465fce27b2 Mon Sep 17 00:00:00 2001 From: Jaegeuk Kim Date: Fri, 16 Aug 2019 08:38:20 -0700 Subject: [PATCH] Revert "Revert "Revert "scsi: ufs-qcom: Add flag to keep track of PHY's power state""" This reverts commit 28652f3b71d2eeb780d61085907c8a04d5ced116. Change log from v1: - add is_binary_power_count to toggle the power_count under phy mutex - no funcitonal changes on other components = v1 description: This can contribute a race condition between is_phy_pwr_on and phy_power_on/off. Instead of it, we must rely on mutex with power_count in phy_power_on/off more precisely. And, in order to fix the origina issue caused by multiple power on/ off, we must use power_count as a flag. So, the final approach is same as moving "is_phy_pwr_on" under mutex. Bug: 139262967 Change-Id: Ia2ca0a1187b97e0f5b9e46d8e8465ac0b28d2862 Signed-off-by: Jaegeuk Kim Signed-off-by: Alexander Winkowski --- drivers/phy/phy-core.c | 8 +++++-- drivers/scsi/ufs/ufs-qcom.c | 46 ++++++++++++++++--------------------- drivers/scsi/ufs/ufs-qcom.h | 1 - include/linux/phy/phy.h | 1 + 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index 48a365e303e5..1a306664324e 100755 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -299,8 +299,10 @@ int phy_power_on(struct phy *phy) dev_err(&phy->dev, "phy poweron failed --> %d\n", ret); goto err_pwr_on; } + ++phy->power_count; + } else if (!phy->is_binary_power_count) { + ++phy->power_count; } - ++phy->power_count; mutex_unlock(&phy->mutex); return 0; @@ -330,8 +332,10 @@ int phy_power_off(struct phy *phy) mutex_unlock(&phy->mutex); return ret; } + --phy->power_count; + } else if (!phy->is_binary_power_count) { + --phy->power_count; } - --phy->power_count; mutex_unlock(&phy->mutex); phy_pm_runtime_put(phy); diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 7adb6552d33a..5ecc3cd08806 100755 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -830,10 +830,8 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) */ if (!ufs_qcom_is_link_active(hba)) { ufs_qcom_disable_lane_clks(host); - if (host->is_phy_pwr_on) { - phy_power_off(phy); - host->is_phy_pwr_on = false; - } + phy_power_off(phy); + if (host->vddp_ref_clk && ufs_qcom_is_link_off(hba)) ret = ufs_qcom_disable_vreg(hba->dev, host->vddp_ref_clk); @@ -860,15 +858,13 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) struct phy *phy = host->generic_phy; int err; - if (!host->is_phy_pwr_on) { - err = phy_power_on(phy); - if (err) { - dev_err(hba->dev, "%s: failed enabling regs, err = %d\n", - __func__, err); - goto out; - } - host->is_phy_pwr_on = true; + err = phy_power_on(phy); + if (err) { + dev_err(hba->dev, "%s: failed enabling regs, err = %d\n", + __func__, err); + goto out; } + if (host->vddp_ref_clk && (hba->rpm_lvl > UFS_PM_LVL_3 || hba->spm_lvl > UFS_PM_LVL_3)) ufs_qcom_enable_vreg(hba->dev, @@ -1524,10 +1520,8 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on, return 0; if (on && (status == POST_CHANGE)) { - if (!host->is_phy_pwr_on) { - phy_power_on(host->generic_phy); - host->is_phy_pwr_on = true; - } + phy_power_on(host->generic_phy); + /* enable the device ref clock for HS mode*/ if (ufshcd_is_hs_mode(&hba->pwr_info)) ufs_qcom_dev_ref_clk_ctrl(host, true); @@ -1544,10 +1538,7 @@ static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on, ufs_qcom_dev_ref_clk_ctrl(host, false); /* powering off PHY during aggressive clk gating */ - if (host->is_phy_pwr_on) { - phy_power_off(host->generic_phy); - host->is_phy_pwr_on = false; - } + phy_power_off(host->generic_phy); } } @@ -2148,6 +2139,8 @@ static int ufs_qcom_init(struct ufs_hba *hba) host->dev_ref_clk_en_mask = BIT(5); } } + /* use binary power_count to avoid race condition of phy on/off */ + host->generic_phy->is_binary_power_count = true; /* update phy revision information before calling phy_init() */ ufs_qcom_phy_save_controller_version(host->generic_phy, @@ -2156,13 +2149,15 @@ static int ufs_qcom_init(struct ufs_hba *hba) err = ufs_qcom_parse_reg_info(host, "qcom,vddp-ref-clk", &host->vddp_ref_clk); phy_init(host->generic_phy); - + err = phy_power_on(host->generic_phy); + if (err) + goto out_unregister_bus; if (host->vddp_ref_clk) { err = ufs_qcom_enable_vreg(dev, host->vddp_ref_clk); if (err) { dev_err(dev, "%s: failed enabling ref clk supply: %d\n", __func__, err); - goto out_unregister_bus; + goto out_disable_phy; } } @@ -2209,6 +2204,8 @@ out_set_load_vccq_parent: out_disable_vddp: if (host->vddp_ref_clk) ufs_qcom_disable_vreg(dev, host->vddp_ref_clk); +out_disable_phy: + phy_power_off(host->generic_phy); out_unregister_bus: phy_exit(host->generic_phy); msm_bus_scale_unregister_client(host->bus_vote.client_handle); @@ -2232,10 +2229,7 @@ static void ufs_qcom_exit(struct ufs_hba *hba) msm_bus_scale_unregister_client(host->bus_vote.client_handle); ufs_qcom_disable_lane_clks(host); - if (host->is_phy_pwr_on) { - phy_power_off(host->generic_phy); - host->is_phy_pwr_on = false; - } + phy_power_off(host->generic_phy); phy_exit(host->generic_phy); ufs_qcom_pm_qos_remove(host); } diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h index f3118d6a1113..0c97544ce891 100755 --- a/drivers/scsi/ufs/ufs-qcom.h +++ b/drivers/scsi/ufs/ufs-qcom.h @@ -361,7 +361,6 @@ struct ufs_qcom_host { struct ufs_vreg *vddp_ref_clk; struct ufs_vreg *vccq_parent; bool work_pending; - bool is_phy_pwr_on; /* hw reset info. */ unsigned int hw_reset_count; diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index e694d4008c4a..04e875300817 100755 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -77,6 +77,7 @@ struct phy { struct mutex mutex; int init_count; int power_count; + bool is_binary_power_count; struct phy_attrs attrs; struct regulator *pwr; };