diff mbox series

scsi: ufs: Fix registers dump vops caused scheduling while atomic

Message ID 1580882795-29675-1-git-send-email-cang@codeaurora.org
State New
Headers show
Series scsi: ufs: Fix registers dump vops caused scheduling while atomic | expand

Commit Message

Can Guo Feb. 5, 2020, 6:06 a.m. UTC 
Reigsters dump intiated from atomic context should not sleep. To fix it,
add one boolean parameter to register dump vops to inform vendor driver if
sleep is allowed or not.

Signed-off-by: Can Guo <cang@codeaurora.org>

Comments

Can Guo Feb. 5, 2020, 6:31 a.m. UTC | #1 
On 2020-02-05 14:21, Bart Van Assche wrote:
> On 2020-02-04 22:06, Can Guo wrote:
>> @@ -5617,7 +5622,7 @@ static irqreturn_t ufshcd_check_errors(struct
>> 
>>  					__func__, hba->saved_err,
>>  					hba->saved_uic_err);
>> 
>> -				ufshcd_print_host_regs(hba);
>> +				__ufshcd_print_host_regs(hba, true);
>>  				ufshcd_print_pwr_info(hba);
>>  				ufshcd_print_tmrs(hba,
>> 					hba->outstanding_tasks);
>>  				ufshcd_print_trs(hba,
>> 					hba->outstanding_reqs,
> 
> Hi Can,
> 
> Please fix this by splitting ufs_qcom_dump_dbg_regs() into two
> functions: one function that doesn't sleep and a second function that
> behaves identically to the current function. If the function names will
> make it clear which function sleeps and which function doesn't that 
> will
> result in code that is much easier to read than the above code. For the
> above code it is namely impossible to figure out what will happen
> without looking up the caller.
> 
> Thanks,
> 
> Bart.

Hi Bart,

Do you mean by splitting ufshcd_print_host_regs() into two functions?
One behaves identically same to the current function, another one called
ufshcd_print_host_regs_nosleep(). No?

Thanks,
Can Guo.
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 3b5b2d9..c30139c 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1619,13 +1619,17 @@  static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba)
 	kfree(testbus);
 }
 
-static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
+static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba, bool no_sleep)
 {
 	ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4,
 			 "HCI Vendor Specific Registers ");
 
 	/* 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);
+
+	if (no_sleep)
+		return;
+
 	usleep_range(1000, 1100);
 	ufs_qcom_testbus_read(hba);
 	usleep_range(1000, 1100);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0ac5d47..37f1539 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -398,7 +398,7 @@  static void ufshcd_print_err_hist(struct ufs_hba *hba,
 		dev_err(hba->dev, "No record of %s\n", err_name);
 }
 
-static void ufshcd_print_host_regs(struct ufs_hba *hba)
+static inline void __ufshcd_print_host_regs(struct ufs_hba *hba, bool no_sleep)
 {
 	ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: ");
 	dev_err(hba->dev, "hba->ufs_version = 0x%x, hba->capabilities = 0x%x\n",
@@ -430,7 +430,12 @@  static void ufshcd_print_host_regs(struct ufs_hba *hba)
 
 	ufshcd_print_clk_freqs(hba);
 
-	ufshcd_vops_dbg_register_dump(hba);
+	ufshcd_vops_dbg_register_dump(hba, no_sleep);
+}
+
+static void ufshcd_print_host_regs(struct ufs_hba *hba)
+{
+	__ufshcd_print_host_regs(hba, false);
 }
 
 static
@@ -4821,7 +4826,7 @@  static void ufshcd_slave_destroy(struct scsi_device *sdev)
 		dev_err(hba->dev,
 				"OCS error from controller = %x for tag %d\n",
 				ocs, lrbp->task_tag);
-		ufshcd_print_host_regs(hba);
+		__ufshcd_print_host_regs(hba, true);
 		ufshcd_print_host_state(hba);
 		break;
 	} /* end of switch */
@@ -5617,7 +5622,7 @@  static irqreturn_t ufshcd_check_errors(struct ufs_hba *hba)
 					__func__, hba->saved_err,
 					hba->saved_uic_err);
 
-				ufshcd_print_host_regs(hba);
+				__ufshcd_print_host_regs(hba, true);
 				ufshcd_print_pwr_info(hba);
 				ufshcd_print_tmrs(hba, hba->outstanding_tasks);
 				ufshcd_print_trs(hba, hba->outstanding_reqs,
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 2ae6c7c..3de7cbb 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -323,7 +323,7 @@  struct ufs_hba_variant_ops {
 	int	(*apply_dev_quirks)(struct ufs_hba *hba);
 	int     (*suspend)(struct ufs_hba *, enum ufs_pm_op);
 	int     (*resume)(struct ufs_hba *, enum ufs_pm_op);
-	void	(*dbg_register_dump)(struct ufs_hba *hba);
+	void	(*dbg_register_dump)(struct ufs_hba *hba, bool no_sleep);
 	int	(*phy_initialization)(struct ufs_hba *);
 	void	(*device_reset)(struct ufs_hba *hba);
 };
@@ -1078,10 +1078,11 @@  static inline int ufshcd_vops_resume(struct ufs_hba *hba, enum ufs_pm_op op)
 	return 0;
 }
 
-static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
+static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba,
+						 bool no_sleep)
 {
 	if (hba->vops && hba->vops->dbg_register_dump)
-		hba->vops->dbg_register_dump(hba);
+		hba->vops->dbg_register_dump(hba, no_sleep);
 }
 
 static inline void ufshcd_vops_device_reset(struct ufs_hba *hba)