On 2020-07-22 22:37, Jeffrey Hugo wrote:
On Mon, Jul 20, 2020 at 12:39 AM Can Guo <cang@xxxxxxxxxxxxxx> wrote:
Dumping testbus registers needs to sleep a bit intermittently as there
are
too many of them. Skip them for those contexts where sleep is not
allowed.
Meanwhile, if ufs_qcom_dump_dbg_regs() calls ufs_qcom_testbus_config()
from
ufshcd_suspend/resume and/or clk gate/ungate context,
pm_runtime_get_sync()
and ufshcd_hold() will cause racing problems. Fix it by removing the
unnecessary calls of pm_runtime_get_sync() and ufshcd_hold().
It sounds like this is two different changes which are clubbed
together into the same patch and really should be two different
patches.
Will split them and give commit msgs accordingly in next version.
Signed-off-by: Can Guo <cang@xxxxxxxxxxxxxx>
---
drivers/scsi/ufs/ufs-qcom.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2e6ddb5..3743c17 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1604,9 +1604,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host
*host)
*/
}
mask <<= offset;
-
- pm_runtime_get_sync(host->hba->dev);
- ufshcd_hold(host->hba, false);
ufshcd_rmwl(host->hba, TEST_BUS_SEL,
(u32)host->testbus.select_major << 19,
REG_UFS_CFG1);
@@ -1619,8 +1616,6 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host
*host)
* committed before returning.
*/
mb();
- ufshcd_release(host->hba);
- pm_runtime_put_sync(host->hba->dev);
return 0;
}
@@ -1658,11 +1653,13 @@ static void ufs_qcom_dump_dbg_regs(struct
ufs_hba *hba)
/* sleep a bit intermittently as we are dumping too much data
*/
ufs_qcom_print_hw_debug_reg_all(hba, NULL,
ufs_qcom_dump_regs_wrapper);
- udelay(1000);
- ufs_qcom_testbus_read(hba);
- udelay(1000);
- ufs_qcom_print_unipro_testbus(hba);
- udelay(1000);
+ if (in_task()) {
+ udelay(1000);
+ ufs_qcom_testbus_read(hba);
+ udelay(1000);
+ ufs_qcom_print_unipro_testbus(hba);
+ udelay(1000);
+ }
Did you run into a specific issue with this? udelay is not a "sleep"
in the sense that it causes scheduling to occur, which is the problem
with atomic contexts.
Here, ufs_qcom_print_unipro_testbus is actually causing the problem as
it has
kmalloc with flag GFP_KERNEL. Even we change the kmalloc flag to ATOMIC,
the
prints are still too heavy for atomic contexts. So we want to mute all
test bus
prints in atomic contexts. Hence the in_task() check. But apprently I
should move
the check up to have all the testbus prints inside the check. I will
modify the
change and the commit msg to tell the true story.
Thanks,
Can Guo.
