| Message ID | 20230823154413.23788-6-quic_nitirawa@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: qcom: Align programming sequence as per HW spec | expand |
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 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 >> >
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); }