diff mbox series

[V6,4/6] scsi: ufs: qcom: Align unipro clk attributes configuration as per HPG

Message ID 20230901114336.31339-5-quic_nitirawa@quicinc.com (mailing list archive)
State Superseded
Headers show
Series scsi: ufs: qcom: Align programming sequence as per HW spec | expand

Commit Message

Nitin Rawat Sept. 1, 2023, 11:43 a.m. UTC 
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 completely with hardware
specification. This doesn't bring any functionality change.

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(-)

--
2.17.1

Comments

Bjorn Andersson Sept. 1, 2023, 3:43 p.m. UTC | #1 
On Fri, Sep 01, 2023 at 05:13:34PM +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 completely with hardware
> specification. This doesn't bring any functionality change.
> 

How can applying the clock scaling configuration, and "aligning with
hardware specification" not "bring any functionality change"?

If the code is called in a way where there is no difference between pre
and post callbacks, then state that - but it begs the question, why do
we have this "flexible" (complex) callback scheme if it doesn't matter.

Regards,
Bjorn
Nitin Rawat Sept. 4, 2023, 2:44 p.m. UTC | #2 
On 9/1/2023 9:13 PM, Bjorn Andersson wrote:
> On Fri, Sep 01, 2023 at 05:13:34PM +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 completely with hardware
>> specification. This doesn't bring any functionality change.
>>
> 
> How can applying the clock scaling configuration, and "aligning with
> hardware specification" not "bring any functionality change"?
> 
> If the code is called in a way where there is no difference between pre
> and post callbacks, then state that - but it begs the question, why do
> we have this "flexible" (complex) callback scheme if it doesn't matter.
> 
> Regards,
> Bjorn

Hi Bjorn,

Here my intention is to align the sequence completely with HPG. 
Functionality w.r.t to clock scaling is not impacted here.
I'll update the commit text to capture more details in next patchset.

Thanks,
Nitin
diff mbox series

Patch

diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
index e3648e936498..d670fcc27ffb 100644
--- a/drivers/ufs/host/ufs-qcom.c
+++ b/drivers/ufs/host/ufs-qcom.c
@@ -1410,12 +1410,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);

@@ -1425,6 +1419,11 @@  static int ufs_qcom_clk_scale_up_post_change(struct ufs_hba *hba)
 	return ufs_qcom_set_core_clk_ctrl(hba, true);
 }

+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);