| Message ID | 20230901114336.31339-7-quic_nitirawa@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: ufs: qcom: Align programming sequence as per HW spec | expand |
On Fri, Sep 01, 2023 at 05:13:36PM +0530, Nitin Rawat wrote: > a) Enable internal HW division of unipro core_clk for scale up condition. > b) Clear internal HW division of unipro core_clk for scale down condition. > This commit message isn't good. > Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> > Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@quicinc.com> > Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> > --- > drivers/ufs/host/ufs-qcom.c | 31 +++++++++++++++++-------------- > drivers/ufs/host/ufs-qcom.h | 2 +- > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index c251c98a74f0..2ddda9356abc 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -1389,18 +1389,21 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, > core_clk_ctrl_reg |= FIELD_PREP(CLK_1US_CYCLES_MASK, cycles_in_1us); > } > > - /* Clear CORE_CLK_DIV_EN */ > - core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; > + /* Enable CORE_CLK_DIV_EN for scale up condition */ > + if (is_max_freq) > + core_clk_ctrl_reg |= CORE_CLK_DIV_EN_BIT; > > ret = ufshcd_dme_set(hba, > UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), > core_clk_ctrl_reg); > + if (ret) > + return ret; Nice, but don't mix logical and stylistic changes in the same patch. > /* > * UFS host controller V4.0.0 onwards needs to program > * PA_VS_CORE_CLK_40NS_CYCLES attribute per programmed > * frequency of unipro core clk of UFS host controller. > */ > - if (!ret && (host->hw_ver.major >= 4)) { > + if (host->hw_ver.major >= 4) { > if (cycles_in_40ns > PA_VS_CORE_CLK_40NS_CYCLES_MASK) > return -EINVAL; > > @@ -1451,26 +1454,26 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > - int err; > - u32 core_clk_ctrl_reg; > + int ret; > + u32 reg; This just obfuscates the logical changes you're making. Frankly, I've been staring at this for several minutes now. The commit message says there is a change here (but fails to describe what problem is solves or what the change is)...but I can't find the change! Regards, Bjorn > > if (!ufs_qcom_cap_qunipro(host)) > return 0; > > - err = ufshcd_dme_get(hba, > + ret = ufshcd_dme_get(hba, > UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), > - &core_clk_ctrl_reg); > + ®); > + if (ret) > + return ret; > > /* make sure CORE_CLK_DIV_EN is cleared */ > - if (!err && > - (core_clk_ctrl_reg & DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT)) { > - core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; > - err = ufshcd_dme_set(hba, > + if (reg & CORE_CLK_DIV_EN_BIT) { > + reg &= ~CORE_CLK_DIV_EN_BIT; > + ret = ufshcd_dme_set(hba, > UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), > - core_clk_ctrl_reg); > + reg); > } > - > - return err; > + return ret; > } > > static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) > diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h > index bc176ef58e3e..bf0c370c79c7 100644 > --- a/drivers/ufs/host/ufs-qcom.h > +++ b/drivers/ufs/host/ufs-qcom.h > @@ -141,7 +141,7 @@ enum { > /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */ > #define CLK_1US_CYCLES_MASK_V4 GENMASK(27, 16) > #define CLK_1US_CYCLES_MASK GENMASK(7, 0) > -#define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT BIT(8) > +#define CORE_CLK_DIV_EN_BIT BIT(8) > #define PA_VS_CORE_CLK_40NS_CYCLES 0x9007 > #define PA_VS_CORE_CLK_40NS_CYCLES_MASK GENMASK(6, 0) > > -- > 2.17.1 >
diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c index c251c98a74f0..2ddda9356abc 100644 --- a/drivers/ufs/host/ufs-qcom.c +++ b/drivers/ufs/host/ufs-qcom.c @@ -1389,18 +1389,21 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, core_clk_ctrl_reg |= FIELD_PREP(CLK_1US_CYCLES_MASK, cycles_in_1us); } - /* Clear CORE_CLK_DIV_EN */ - core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; + /* Enable CORE_CLK_DIV_EN for scale up condition */ + if (is_max_freq) + core_clk_ctrl_reg |= CORE_CLK_DIV_EN_BIT; ret = ufshcd_dme_set(hba, UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), core_clk_ctrl_reg); + if (ret) + return ret; /* * UFS host controller V4.0.0 onwards needs to program * PA_VS_CORE_CLK_40NS_CYCLES attribute per programmed * frequency of unipro core clk of UFS host controller. */ - if (!ret && (host->hw_ver.major >= 4)) { + if (host->hw_ver.major >= 4) { if (cycles_in_40ns > PA_VS_CORE_CLK_40NS_CYCLES_MASK) return -EINVAL; @@ -1451,26 +1454,26 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) { struct ufs_qcom_host *host = ufshcd_get_variant(hba); - int err; - u32 core_clk_ctrl_reg; + int ret; + u32 reg; if (!ufs_qcom_cap_qunipro(host)) return 0; - err = ufshcd_dme_get(hba, + ret = ufshcd_dme_get(hba, UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), - &core_clk_ctrl_reg); + ®); + if (ret) + return ret; /* make sure CORE_CLK_DIV_EN is cleared */ - if (!err && - (core_clk_ctrl_reg & DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT)) { - core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; - err = ufshcd_dme_set(hba, + if (reg & CORE_CLK_DIV_EN_BIT) { + reg &= ~CORE_CLK_DIV_EN_BIT; + ret = ufshcd_dme_set(hba, UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), - core_clk_ctrl_reg); + reg); } - - return err; + return ret; } static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h index bc176ef58e3e..bf0c370c79c7 100644 --- a/drivers/ufs/host/ufs-qcom.h +++ b/drivers/ufs/host/ufs-qcom.h @@ -141,7 +141,7 @@ enum { /* bit and mask definitions for DME_VS_CORE_CLK_CTRL attribute */ #define CLK_1US_CYCLES_MASK_V4 GENMASK(27, 16) #define CLK_1US_CYCLES_MASK GENMASK(7, 0) -#define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT BIT(8) +#define CORE_CLK_DIV_EN_BIT BIT(8) #define PA_VS_CORE_CLK_40NS_CYCLES 0x9007 #define PA_VS_CORE_CLK_40NS_CYCLES_MASK GENMASK(6, 0)