Re: [PATCH v2 05/10] scsi: ufshcd: fix possible unclocked register access

[Subhash Jadavani]

On 2018-05-03 04:07, Asutosh Das wrote:

From: Subhash Jadavani 

vendor specific setup_clocks ops may depend on clocks managed by ufshcd
driver so if the vendor specific setup_clocks callback is called when
the required clocks are turned off, it results into unclocked register
access.

This change make sure that required clocks are enabled before vendor
specific setup_clocks callback is called.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Can Guo 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 838ba8f0..dfeb194 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6780,9 +6780,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba
*hba, bool on,
if (list_empty(head))
goto out;

-   ret = ufshcd_vops_setup_clocks(hba, on, PRE_CHANGE);
-   if (ret)
-   return ret;
+   /*
+* vendor specific setup_clocks ops may depend on clocks managed by
+* this standard driver hence call the vendor specific setup_clocks
+* before disabling the clocks managed here.
+*/
+   if (!on) {
+   ret = ufshcd_vops_setup_clocks(hba, on, PRE_CHANGE);
+   if (ret)
+   return ret;
+   }

list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
@@ -6806,9 +6813,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba
*hba, bool on,
}
}

-   ret = ufshcd_vops_setup_clocks(hba, on, POST_CHANGE);
-   if (ret)
-   return ret;
+   /*
+* vendor specific setup_clocks ops may depend on clocks managed by
+* this standard driver hence call the vendor specific setup_clocks
+* after enabling the clocks managed here.
+*/
+   if (on) {
+   ret = ufshcd_vops_setup_clocks(hba, on, POST_CHANGE);
+   if (ret)
+   return ret;
+   }

 out:
if (ret) {


Looks good to me.

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 05/10] scsi: ufshcd: fix possible unclocked register access

[Asutosh Das]

From: Subhash Jadavani 

vendor specific setup_clocks ops may depend on clocks managed by ufshcd
driver so if the vendor specific setup_clocks callback is called when
the required clocks are turned off, it results into unclocked register
access.

This change make sure that required clocks are enabled before vendor
specific setup_clocks callback is called.

Signed-off-by: Subhash Jadavani 
Signed-off-by: Venkat Gopalakrishnan 
Signed-off-by: Can Guo 
Signed-off-by: Asutosh Das 
---
 drivers/scsi/ufs/ufshcd.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 838ba8f0..dfeb194 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6780,9 +6780,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, 
bool on,
if (list_empty(head))
goto out;
 
-   ret = ufshcd_vops_setup_clocks(hba, on, PRE_CHANGE);
-   if (ret)
-   return ret;
+   /*
+* vendor specific setup_clocks ops may depend on clocks managed by
+* this standard driver hence call the vendor specific setup_clocks
+* before disabling the clocks managed here.
+*/
+   if (!on) {
+   ret = ufshcd_vops_setup_clocks(hba, on, PRE_CHANGE);
+   if (ret)
+   return ret;
+   }
 
list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
@@ -6806,9 +6813,16 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, 
bool on,
}
}
 
-   ret = ufshcd_vops_setup_clocks(hba, on, POST_CHANGE);
-   if (ret)
-   return ret;
+   /*
+* vendor specific setup_clocks ops may depend on clocks managed by
+* this standard driver hence call the vendor specific setup_clocks
+* after enabling the clocks managed here.
+*/
+   if (on) {
+   ret = ufshcd_vops_setup_clocks(hba, on, POST_CHANGE);
+   if (ret)
+   return ret;
+   }
 
 out:
if (ret) {
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project.