| Message ID | 20230823154413.23788-1-quic_nitirawa@quicinc.com |
|---|---|
| Headers | show |
| Series | scsi: ufs: qcom: Align programming sequence as per HW spec | expand |
Nitin, > This patch aligns programming sequence as per Qualcomm UFS hardware > specification. Which tree is this against? Doesn't apply to 6.6/scsi-queue...
On Wed, Aug 23, 2023 at 09:14:09PM +0530, Nitin Rawat wrote: > For Qualcomm UFS controller V4.0 and above PA_VS_CORE_CLK_40NS_CYCLES > attribute needs to be programmed with frequency of unipro core clk. > Hence Configure PA_VS_CORE_CLK_40NS_CYCLES attribute for Unipro core clk. > Same comment applies as patch 1. - Mani > 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 | 45 ++++++++++++++++++++++++++++--------- > drivers/ufs/host/ufs-qcom.h | 2 ++ > 2 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 1108b0cd43b3..abc0e7f7d1b0 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -93,8 +93,9 @@ static const struct __ufs_qcom_bw_table { > static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS]; > > static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); > -static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, > - u32 clk_cycles); > +static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, > + u32 clk_cycles, > + u32 clk_40ns_cycles); > > static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) > { > @@ -690,8 +691,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba, > * set unipro core clock cycles to 150 & clear clock > * divider > */ > - err = ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, > - 150); > + err = ufs_qcom_set_core_clk_ctrl(hba, 150, 6); > > /* > * Some UFS devices (and may be host) have issues if LCC is > @@ -1296,12 +1296,13 @@ static void ufs_qcom_exit(struct ufs_hba *hba) > phy_exit(host->generic_phy); > } > > -static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, > - u32 clk_1us_cycles) > +static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, > + u32 clk_1us_cycles, > + u32 clk_40ns_cycles) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > u32 mask = DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK; > - u32 core_clk_ctrl_reg; > + u32 core_clk_ctrl_reg, reg; > u32 offset = 0; > int err; > > @@ -1326,9 +1327,33 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, > /* Clear CORE_CLK_DIV_EN */ > core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; > > - return ufshcd_dme_set(hba, > + err = ufshcd_dme_set(hba, > UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), > core_clk_ctrl_reg); > + /* > + * 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 (!err && (host->hw_ver.major >= 4)) { > + if (clk_40ns_cycles > PA_VS_CORE_CLK_40NS_CYCLES_MASK) > + return -EINVAL; > + > + err = ufshcd_dme_get(hba, > + UIC_ARG_MIB(PA_VS_CORE_CLK_40NS_CYCLES), > + ®); > + if (err) > + return err; > + > + reg &= ~PA_VS_CORE_CLK_40NS_CYCLES_MASK; > + reg |= clk_40ns_cycles; > + > + err = ufshcd_dme_set(hba, > + UIC_ARG_MIB(PA_VS_CORE_CLK_40NS_CYCLES), > + reg); > + } > + > + return err; > } > > static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) > @@ -1345,7 +1370,7 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > return 0; > > /* set unipro core clock cycles to 150 and clear clock divider */ > - return ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, 150); > + return ufs_qcom_set_core_clk_ctrl(hba, 150, 6); > } > > static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) > @@ -1381,7 +1406,7 @@ static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) > return 0; > > /* set unipro core clock cycles to 75 and clear clock divider */ > - return ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, 75); > + return ufs_qcom_set_core_clk_ctrl(hba, 75, 3); > } > > static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, > diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h > index a829296e11bb..325f08aca260 100644 > --- a/drivers/ufs/host/ufs-qcom.h > +++ b/drivers/ufs/host/ufs-qcom.h > @@ -133,6 +133,8 @@ enum { > #define MAX_CORE_CLK_1US_CYCLES_OFFSET_V4 0x10 > #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT BIT(8) > #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK 0xFF > +#define PA_VS_CORE_CLK_40NS_CYCLES 0x9007 > +#define PA_VS_CORE_CLK_40NS_CYCLES_MASK 0x3F > > static inline void > ufs_qcom_get_controller_revision(struct ufs_hba *hba, > -- > 2.17.1 >
On Wed, Aug 23, 2023 at 09:14:11PM +0530, Nitin Rawat wrote: > Currently CORE_CLK_1US_CYCLES, PA_VS_CORE_CLK_40NS_CYCLES are configured > in clk scaling post change ops. Move this to clk scaling pre change ops to > align with the hardware specification. > Does this mean, the driver was doing the clk scaling at the wrong time? If so, this patch should be moved ahead of all patches, should have fixes tag and CC stable list. - Mani > 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 | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 8162b19191a9..491c0173603e 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -1399,12 +1399,6 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, > } > > static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) > -{ > - /* nothing to do as of now */ > - return 0; > -} > - > -static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > @@ -1414,6 +1408,11 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > return ufs_qcom_cfg_core_clk_ctrl(hba); > } > > +static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > +{ > + return 0; > +} > + > static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > -- > 2.17.1 >
On Wed, Aug 23, 2023 at 09:14:12PM +0530, Nitin Rawat wrote: > This change configures SYS1CLK_1US_REG for pre scale up condition. Also > move ufs_qcom_cfg_timers from clk scaling post change ops to clk scaling > pre change ops to align with the hardware specification. > Same comment as previous patch. This looks like a bug fix to me. Also, this patch should be splitted into 2. SYS1CLK_1US_REG and ufs_qcom_cfg_timers change. - Mani > 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 | 61 +++++++++++++++++++++++++------------ > 1 file changed, 42 insertions(+), 19 deletions(-) > > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c > index 491c0173603e..82cf3ac4193a 100644 > --- a/drivers/ufs/host/ufs-qcom.c > +++ b/drivers/ufs/host/ufs-qcom.c > @@ -533,7 +533,8 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, > * Return: zero for success and non-zero in case of a failure. > */ > static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, > - u32 hs, u32 rate, bool update_link_startup_timer) > + u32 hs, u32 rate, bool link_startup, > + bool is_pre_scale_up) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct ufs_clk_info *clki; > @@ -564,11 +565,16 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, > /* > * The Qunipro controller does not use following registers: > * SYS1CLK_1US_REG, TX_SYMBOL_CLK_1US_REG, CLK_NS_REG & > - * UFS_REG_PA_LINK_STARTUP_TIMER > - * But UTP controller uses SYS1CLK_1US_REG register for Interrupt > - * Aggregation logic. > - */ > - if (ufs_qcom_cap_qunipro(host) && !ufshcd_is_intr_aggr_allowed(hba)) > + * UFS_REG_PA_LINK_STARTUP_TIMER. > + * However UTP controller uses SYS1CLK_1US_REG register for Interrupt > + * Aggregation logic and Auto hibern8 logic. > + * It is mandatory to write SYS1CLK_1US_REG register on UFS host > + * controller V4.0.0 onwards. > + */ > + if (ufs_qcom_cap_qunipro(host) && > + !(ufshcd_is_intr_aggr_allowed(hba) || > + ufshcd_is_auto_hibern8_supported(hba) || > + host->hw_ver.major >= 4)) > return 0; > > if (gear == 0) { > @@ -577,8 +583,14 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, > } > > list_for_each_entry(clki, &hba->clk_list_head, list) { > - if (!strcmp(clki->name, "core_clk")) > - core_clk_rate = clk_get_rate(clki->clk); > + if (!strcmp(clki->name, "core_clk")) { > + if (is_pre_scale_up) > + core_clk_rate = clki->max_freq; > + else > + core_clk_rate = clk_get_rate(clki->clk); > + break; > + } > + > } > > /* If frequency is smaller than 1MHz, set to 1MHz */ > @@ -658,7 +670,7 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, > mb(); > } > > - if (update_link_startup_timer && host->hw_ver.major != 0x5) { > + if (link_startup && host->hw_ver.major != 0x5) { > ufshcd_writel(hba, ((core_clk_rate / MSEC_PER_SEC) * 100), > REG_UFS_CFG0); > /* > @@ -719,7 +731,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba, > switch (status) { > case PRE_CHANGE: > if (ufs_qcom_cfg_timers(hba, UFS_PWM_G1, SLOWAUTO_MODE, > - 0, true)) { > + 0, true, false)) { > dev_err(hba->dev, "%s: ufs_qcom_cfg_timers() failed\n", > __func__); > return -EINVAL; > @@ -968,7 +980,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba, > case POST_CHANGE: > if (ufs_qcom_cfg_timers(hba, dev_req_params->gear_rx, > dev_req_params->pwr_rx, > - dev_req_params->hs_rate, false)) { > + dev_req_params->hs_rate, false, false)) { > dev_err(hba->dev, "%s: ufs_qcom_cfg_timers() failed\n", > __func__); > /* > @@ -1401,11 +1413,24 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, > static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > + struct ufs_pa_layer_attr *attr = &host->dev_req_params; > + int err; > > if (!ufs_qcom_cap_qunipro(host)) > - return 0; > + goto out; > + > + if (attr) { > + err = ufs_qcom_cfg_timers(hba, attr->gear_rx, > + attr->pwr_rx, attr->hs_rate, > + false, true); > + if (err) > + dev_err(hba->dev, "%s ufs cfg timer failed\n", > + __func__); > + } > > - return ufs_qcom_cfg_core_clk_ctrl(hba); > + err = ufs_qcom_cfg_core_clk_ctrl(hba); > +out: > + return err; > } > > static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) > @@ -1441,6 +1466,7 @@ static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) > static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > + struct ufs_pa_layer_attr *attr = &host->dev_req_params; > struct list_head *head = &hba->clk_list_head; > struct ufs_clk_info *clki; > u32 curr_freq = 0; > @@ -1449,6 +1475,9 @@ static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) > if (!ufs_qcom_cap_qunipro(host)) > return 0; > > + if (attr) > + ufs_qcom_cfg_timers(hba, attr->gear_rx, attr->pwr_rx, > + attr->hs_rate, false, false); > > list_for_each_entry(clki, head, list) { > if (!IS_ERR_OR_NULL(clki->clk) && > @@ -1480,7 +1509,6 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, > bool scale_up, enum ufs_notify_change_status status) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > - struct ufs_pa_layer_attr *dev_req_params = &host->dev_req_params; > int err = 0; > > /* check the host controller state before sending hibern8 cmd */ > @@ -1510,11 +1538,6 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, > return err; > } > > - ufs_qcom_cfg_timers(hba, > - dev_req_params->gear_rx, > - dev_req_params->pwr_rx, > - dev_req_params->hs_rate, > - false); > ufs_qcom_icc_update_bw(host); > ufshcd_uic_hibern8_exit(hba); > } > -- > 2.17.1 >
On Fri, Aug 25, 2023 at 05:44:01PM -0400, Martin K. Petersen wrote: > > Nitin, > > > This patch aligns programming sequence as per Qualcomm UFS hardware > > specification. > > Which tree is this against? Doesn't apply to 6.6/scsi-queue... > Martin, thanks for considering this series. However, I'd request you to wait for an Ack from atleast one reviewer/maintainer before merging the Qcom patches. Reviews might be delayed, but for sure we are keeping an eye on all patches. - Mani > -- > Martin K. Petersen Oracle Linux Engineering
On 8/28/2023 1:10 PM, Manivannan Sadhasivam wrote: > On Wed, Aug 23, 2023 at 09:14:09PM +0530, Nitin Rawat wrote: >> For Qualcomm UFS controller V4.0 and above PA_VS_CORE_CLK_40NS_CYCLES >> attribute needs to be programmed with frequency of unipro core clk. >> Hence Configure PA_VS_CORE_CLK_40NS_CYCLES attribute for Unipro core clk. >> > > Same comment applies as patch 1. > > - Mani Sure...Will take care of this in next patchset. Thanks -Nitin > >> 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 | 45 ++++++++++++++++++++++++++++--------- >> drivers/ufs/host/ufs-qcom.h | 2 ++ >> 2 files changed, 37 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index 1108b0cd43b3..abc0e7f7d1b0 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -93,8 +93,9 @@ static const struct __ufs_qcom_bw_table { >> static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS]; >> >> static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); >> -static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, >> - u32 clk_cycles); >> +static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, >> + u32 clk_cycles, >> + u32 clk_40ns_cycles); >> >> static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) >> { >> @@ -690,8 +691,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba, >> * set unipro core clock cycles to 150 & clear clock >> * divider >> */ >> - err = ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, >> - 150); >> + err = ufs_qcom_set_core_clk_ctrl(hba, 150, 6); >> >> /* >> * Some UFS devices (and may be host) have issues if LCC is >> @@ -1296,12 +1296,13 @@ static void ufs_qcom_exit(struct ufs_hba *hba) >> phy_exit(host->generic_phy); >> } >> >> -static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, >> - u32 clk_1us_cycles) >> +static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, >> + u32 clk_1us_cycles, >> + u32 clk_40ns_cycles) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> u32 mask = DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK; >> - u32 core_clk_ctrl_reg; >> + u32 core_clk_ctrl_reg, reg; >> u32 offset = 0; >> int err; >> >> @@ -1326,9 +1327,33 @@ static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, >> /* Clear CORE_CLK_DIV_EN */ >> core_clk_ctrl_reg &= ~DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT; >> >> - return ufshcd_dme_set(hba, >> + err = ufshcd_dme_set(hba, >> UIC_ARG_MIB(DME_VS_CORE_CLK_CTRL), >> core_clk_ctrl_reg); >> + /* >> + * 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 (!err && (host->hw_ver.major >= 4)) { >> + if (clk_40ns_cycles > PA_VS_CORE_CLK_40NS_CYCLES_MASK) >> + return -EINVAL; >> + >> + err = ufshcd_dme_get(hba, >> + UIC_ARG_MIB(PA_VS_CORE_CLK_40NS_CYCLES), >> + ®); >> + if (err) >> + return err; >> + >> + reg &= ~PA_VS_CORE_CLK_40NS_CYCLES_MASK; >> + reg |= clk_40ns_cycles; >> + >> + err = ufshcd_dme_set(hba, >> + UIC_ARG_MIB(PA_VS_CORE_CLK_40NS_CYCLES), >> + reg); >> + } >> + >> + return err; >> } >> >> static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) >> @@ -1345,7 +1370,7 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) >> return 0; >> >> /* set unipro core clock cycles to 150 and clear clock divider */ >> - return ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, 150); >> + return ufs_qcom_set_core_clk_ctrl(hba, 150, 6); >> } >> >> static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) >> @@ -1381,7 +1406,7 @@ static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) >> return 0; >> >> /* set unipro core clock cycles to 75 and clear clock divider */ >> - return ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(hba, 75); >> + return ufs_qcom_set_core_clk_ctrl(hba, 75, 3); >> } >> >> static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, >> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h >> index a829296e11bb..325f08aca260 100644 >> --- a/drivers/ufs/host/ufs-qcom.h >> +++ b/drivers/ufs/host/ufs-qcom.h >> @@ -133,6 +133,8 @@ enum { >> #define MAX_CORE_CLK_1US_CYCLES_OFFSET_V4 0x10 >> #define DME_VS_CORE_CLK_CTRL_CORE_CLK_DIV_EN_BIT BIT(8) >> #define DME_VS_CORE_CLK_CTRL_MAX_CORE_CLK_1US_CYCLES_MASK 0xFF >> +#define PA_VS_CORE_CLK_40NS_CYCLES 0x9007 >> +#define PA_VS_CORE_CLK_40NS_CYCLES_MASK 0x3F >> >> static inline void >> ufs_qcom_get_controller_revision(struct ufs_hba *hba, >> -- >> 2.17.1 >> >
On 8/28/2023 1:38 PM, Manivannan Sadhasivam wrote: > On Wed, Aug 23, 2023 at 09:14:11PM +0530, Nitin Rawat wrote: >> Currently CORE_CLK_1US_CYCLES, PA_VS_CORE_CLK_40NS_CYCLES are configured >> in clk scaling post change ops. Move this to clk scaling pre change ops to >> align with the hardware specification. >> > > Does this mean, the driver was doing the clk scaling at the wrong time? If so, > this patch should be moved ahead of all patches, should have fixes tag and CC > stable list. > > - Mani > -- Functionality wise there is no affect with this patch. Our Qcom internal Hardware specification suggest to program these timers in pre clk scale. SO we wanted to align with the HPG. -Nitin >> 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 | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index 8162b19191a9..491c0173603e 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -1399,12 +1399,6 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, >> } >> >> static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) >> -{ >> - /* nothing to do as of now */ >> - return 0; >> -} >> - >> -static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> >> @@ -1414,6 +1408,11 @@ static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) >> return ufs_qcom_cfg_core_clk_ctrl(hba); >> } >> >> +static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) >> +{ >> + return 0; >> +} >> + >> static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> -- >> 2.17.1 >> >
On 8/28/2023 1:47 PM, Manivannan Sadhasivam wrote: > On Wed, Aug 23, 2023 at 09:14:12PM +0530, Nitin Rawat wrote: >> This change configures SYS1CLK_1US_REG for pre scale up condition. Also >> move ufs_qcom_cfg_timers from clk scaling post change ops to clk scaling >> pre change ops to align with the hardware specification. >> > > Same comment as previous patch. This looks like a bug fix to me. > > Also, this patch should be splitted into 2. SYS1CLK_1US_REG and > ufs_qcom_cfg_timers change. > > - Mani > In this patch we are trying to refactor ufs_qcom_cfg_timers function and added extra argument to this function. Since it is just refactoring code, IMO it's better to not split in to 2 patches. We will update the commit message to explain more in detail --Nitin >> 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 | 61 +++++++++++++++++++++++++------------ >> 1 file changed, 42 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c >> index 491c0173603e..82cf3ac4193a 100644 >> --- a/drivers/ufs/host/ufs-qcom.c >> +++ b/drivers/ufs/host/ufs-qcom.c >> @@ -533,7 +533,8 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, >> * Return: zero for success and non-zero in case of a failure. >> */ >> static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, >> - u32 hs, u32 rate, bool update_link_startup_timer) >> + u32 hs, u32 rate, bool link_startup, >> + bool is_pre_scale_up) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> struct ufs_clk_info *clki; >> @@ -564,11 +565,16 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, >> /* >> * The Qunipro controller does not use following registers: >> * SYS1CLK_1US_REG, TX_SYMBOL_CLK_1US_REG, CLK_NS_REG & >> - * UFS_REG_PA_LINK_STARTUP_TIMER >> - * But UTP controller uses SYS1CLK_1US_REG register for Interrupt >> - * Aggregation logic. >> - */ >> - if (ufs_qcom_cap_qunipro(host) && !ufshcd_is_intr_aggr_allowed(hba)) >> + * UFS_REG_PA_LINK_STARTUP_TIMER. >> + * However UTP controller uses SYS1CLK_1US_REG register for Interrupt >> + * Aggregation logic and Auto hibern8 logic. >> + * It is mandatory to write SYS1CLK_1US_REG register on UFS host >> + * controller V4.0.0 onwards. >> + */ >> + if (ufs_qcom_cap_qunipro(host) && >> + !(ufshcd_is_intr_aggr_allowed(hba) || >> + ufshcd_is_auto_hibern8_supported(hba) || >> + host->hw_ver.major >= 4)) >> return 0; >> >> if (gear == 0) { >> @@ -577,8 +583,14 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, >> } >> >> list_for_each_entry(clki, &hba->clk_list_head, list) { >> - if (!strcmp(clki->name, "core_clk")) >> - core_clk_rate = clk_get_rate(clki->clk); >> + if (!strcmp(clki->name, "core_clk")) { >> + if (is_pre_scale_up) >> + core_clk_rate = clki->max_freq; >> + else >> + core_clk_rate = clk_get_rate(clki->clk); >> + break; >> + } >> + >> } >> >> /* If frequency is smaller than 1MHz, set to 1MHz */ >> @@ -658,7 +670,7 @@ static int ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, >> mb(); >> } >> >> - if (update_link_startup_timer && host->hw_ver.major != 0x5) { >> + if (link_startup && host->hw_ver.major != 0x5) { >> ufshcd_writel(hba, ((core_clk_rate / MSEC_PER_SEC) * 100), >> REG_UFS_CFG0); >> /* >> @@ -719,7 +731,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba *hba, >> switch (status) { >> case PRE_CHANGE: >> if (ufs_qcom_cfg_timers(hba, UFS_PWM_G1, SLOWAUTO_MODE, >> - 0, true)) { >> + 0, true, false)) { >> dev_err(hba->dev, "%s: ufs_qcom_cfg_timers() failed\n", >> __func__); >> return -EINVAL; >> @@ -968,7 +980,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba *hba, >> case POST_CHANGE: >> if (ufs_qcom_cfg_timers(hba, dev_req_params->gear_rx, >> dev_req_params->pwr_rx, >> - dev_req_params->hs_rate, false)) { >> + dev_req_params->hs_rate, false, false)) { >> dev_err(hba->dev, "%s: ufs_qcom_cfg_timers() failed\n", >> __func__); >> /* >> @@ -1401,11 +1413,24 @@ static int ufs_qcom_set_core_clk_ctrl(struct ufs_hba *hba, >> static int ufs_qcom_clk_scale_up_pre_change(struct ufs_hba *hba) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> + struct ufs_pa_layer_attr *attr = &host->dev_req_params; >> + int err; >> >> if (!ufs_qcom_cap_qunipro(host)) >> - return 0; >> + goto out; >> + >> + if (attr) { >> + err = ufs_qcom_cfg_timers(hba, attr->gear_rx, >> + attr->pwr_rx, attr->hs_rate, >> + false, true); >> + if (err) >> + dev_err(hba->dev, "%s ufs cfg timer failed\n", >> + __func__); >> + } >> >> - return ufs_qcom_cfg_core_clk_ctrl(hba); >> + err = ufs_qcom_cfg_core_clk_ctrl(hba); >> +out: >> + return err; >> } >> >> static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba) >> @@ -1441,6 +1466,7 @@ static int ufs_qcom_clk_scale_down_pre_change(struct ufs_hba *hba) >> static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> + struct ufs_pa_layer_attr *attr = &host->dev_req_params; >> struct list_head *head = &hba->clk_list_head; >> struct ufs_clk_info *clki; >> u32 curr_freq = 0; >> @@ -1449,6 +1475,9 @@ static int ufs_qcom_clk_scale_down_post_change(struct ufs_hba *hba) >> if (!ufs_qcom_cap_qunipro(host)) >> return 0; >> >> + if (attr) >> + ufs_qcom_cfg_timers(hba, attr->gear_rx, attr->pwr_rx, >> + attr->hs_rate, false, false); >> >> list_for_each_entry(clki, head, list) { >> if (!IS_ERR_OR_NULL(clki->clk) && >> @@ -1480,7 +1509,6 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, >> bool scale_up, enum ufs_notify_change_status status) >> { >> struct ufs_qcom_host *host = ufshcd_get_variant(hba); >> - struct ufs_pa_layer_attr *dev_req_params = &host->dev_req_params; >> int err = 0; >> >> /* check the host controller state before sending hibern8 cmd */ >> @@ -1510,11 +1538,6 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba, >> return err; >> } >> >> - ufs_qcom_cfg_timers(hba, >> - dev_req_params->gear_rx, >> - dev_req_params->pwr_rx, >> - dev_req_params->hs_rate, >> - false); >> ufs_qcom_icc_update_bw(host); >> ufshcd_uic_hibern8_exit(hba); >> } >> -- >> 2.17.1 >> >
This patch aligns programming sequence as per Qualcomm UFS hardware specification. Changes from v4: - Addressed bjorn comment to split single patch to multiple patches. Changes from v3: -Addressed bjorn comment to update commit msg to capture change details. Changes from v2: - Addressed bao comment, removed duplicate clock timer cfg API call Changes from v1: - Addressed bao comment, removed wrapper function - Tab alignment Nitin Rawat (6): scsi: ufs: qcom: Update offset for core_clk_1us_cycles scsi: ufs: qcom: Configure PA_VS_CORE_CLK_40NS_CYCLES for Unipro core clk scsi: ufs: qcom: Add multiple frequency support for unipro clk attributes scsi: ufs: qcom: Align unipro clk attributes as per Hardware specification scsi: ufs: qcom: Refactor ufs_qcom_cfg_timers function. scsi: ufs: qcom: Handle unipro clk HW division based on scaling conditions. drivers/ufs/host/ufs-qcom.c | 239 +++++++++++++++++++++++++++--------- drivers/ufs/host/ufs-qcom.h | 15 ++- 2 files changed, 197 insertions(+), 57 deletions(-) -- 2.17.1