[PATCH v2 00/10] UFS patches for kernel v5.16

[PATCH v2 00/10] UFS patches for kernel v5.16

[Bart Van Assche]

Hi Martin,

This patch series includes the UFS driver kernel patches I would like to see
included kernel v5.16. Please consider these patches for kernel v5.16.

Thank you,

Bart.

Changes compared to v1:
- Converted the sysfs attribute for triggering the error handler into two
  debugfs attributes.
- Use ufshcd_schedule_eh_work(hba) instead of scsi_schedule_eh(hba->host) to
  trigger the error handler.
- Added three patches that implement a small performance optimization.

Bart Van Assche (10):
  scsi: ufs: Revert "Retry aborted SCSI commands instead of completing
    these successfully"
  scsi: ufs: Improve source code comments
  scsi: ufs: Improve static type checking
  scsi: ufs: Log error handler activity
  scsi: ufs: Export ufshcd_schedule_eh_work()
  scsi: ufs: Make it easier to add new debugfs attributes
  scsi: ufs: Add debugfs attributes for triggering the UFS EH
  scsi: ufs: Remove three superfluous casts
  scsi: ufs: Add a compile-time structure size check
  scsi: ufs: Micro-optimize ufshcd_map_sg()

 drivers/scsi/ufs/ufs-debugfs.c |  98 ++++++++++++++++++++++++++++++-
 drivers/scsi/ufs/ufshcd.c      | 102 +++++++++++++++++++++------------
 drivers/scsi/ufs/ufshcd.h      |   1 +
 drivers/scsi/ufs/ufshci.h      |  15 ++---
 4 files changed, 168 insertions(+), 48 deletions(-)

[PATCH v2 01/10] scsi: ufs: Revert "Retry aborted SCSI commands instead of completing these successfully"

[Bart Van Assche]

Commit 73dc3c4ac703 ("scsi: ufs: Retry aborted SCSI commands instead of
completing these successfully") is not necessary. If a SCSI command is
aborted successfully the UFS controller has not modified the command
status and the command status still has the value assigned by
ufshcd_prepare_req_desc_hdr(), namely OCS_INVALID_COMMAND_STATUS. The
function ufshcd_transfer_rsp_status() requeues commands that have an
invalid command status. Hence revert commit 73dc3c4ac703.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 10f5805ae3f6..53dd42f95eed 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5248,11 +5248,9 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
  * __ufshcd_transfer_req_compl - handle SCSI and query command completion
  * @hba: per adapter instance
  * @completed_reqs: bitmask that indicates which requests to complete
- * @retry_requests: whether to ask the SCSI core to retry completed requests
  */
 static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
-					unsigned long completed_reqs,
-					bool retry_requests)
+					unsigned long completed_reqs)
 {
 	struct ufshcd_lrb *lrbp;
 	struct scsi_cmnd *cmd;
@@ -5268,8 +5266,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 			if (unlikely(ufshcd_should_inform_monitor(hba, lrbp)))
 				ufshcd_update_monitor(hba, lrbp);
 			ufshcd_add_command_trace(hba, index, UFS_CMD_COMP);
-			result = retry_requests ? DID_BUS_BUSY << 16 :
-				ufshcd_transfer_rsp_status(hba, lrbp);
+			result = ufshcd_transfer_rsp_status(hba, lrbp);
 			scsi_dma_unmap(cmd);
 			cmd->result = result;
 			/* Mark completed command as NULL in LRB */
@@ -5295,14 +5292,12 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 /**
  * ufshcd_transfer_req_compl - handle SCSI and query command completion
  * @hba: per adapter instance
- * @retry_requests: whether or not to ask to retry requests
  *
  * Returns
  *  IRQ_HANDLED - If interrupt is valid
  *  IRQ_NONE    - If invalid interrupt
  */
-static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba,
-					     bool retry_requests)
+static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba)
 {
 	unsigned long completed_reqs, flags;
 	u32 tr_doorbell;
@@ -5331,8 +5326,7 @@ static irqreturn_t ufshcd_transfer_req_compl(struct ufs_hba *hba,
 	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
 
 	if (completed_reqs) {
-		__ufshcd_transfer_req_compl(hba, completed_reqs,
-					    retry_requests);
+		__ufshcd_transfer_req_compl(hba, completed_reqs);
 		return IRQ_HANDLED;
 	} else {
 		return IRQ_NONE;
@@ -5831,13 +5825,7 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 /* Complete requests that have door-bell cleared */
 static void ufshcd_complete_requests(struct ufs_hba *hba)
 {
-	ufshcd_transfer_req_compl(hba, /*retry_requests=*/false);
-	ufshcd_tmc_handler(hba);
-}
-
-static void ufshcd_retry_aborted_requests(struct ufs_hba *hba)
-{
-	ufshcd_transfer_req_compl(hba, /*retry_requests=*/true);
+	ufshcd_transfer_req_compl(hba);
 	ufshcd_tmc_handler(hba);
 }
 
@@ -6186,7 +6174,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 	}
 
 lock_skip_pending_xfer_clear:
-	ufshcd_retry_aborted_requests(hba);
+	/* Complete the requests that are cleared by s/w */
+	ufshcd_complete_requests(hba);
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	hba->silence_err_logs = false;
@@ -6478,7 +6467,7 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
 		retval |= ufshcd_tmc_handler(hba);
 
 	if (intr_status & UTP_TRANSFER_REQ_COMPL)
-		retval |= ufshcd_transfer_req_compl(hba, /*retry_requests=*/false);
+		retval |= ufshcd_transfer_req_compl(hba);
 
 	return retval;
 }
@@ -6898,7 +6887,7 @@ static int ufshcd_eh_device_reset_handler(struct scsi_cmnd *cmd)
 			err = ufshcd_clear_cmd(hba, pos);
 			if (err)
 				break;
-			__ufshcd_transfer_req_compl(hba, 1U << pos, false);
+			__ufshcd_transfer_req_compl(hba, 1U << pos);
 		}
 	}
 
@@ -7060,7 +7049,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 		dev_err(hba->dev,
 		"%s: cmd was completed, but without a notifying intr, tag = %d",
 		__func__, tag);
-		__ufshcd_transfer_req_compl(hba, 1UL << tag, /*retry_requests=*/false);
+		__ufshcd_transfer_req_compl(hba, 1UL << tag);
 		goto release;
 	}
 
@@ -7126,7 +7115,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba)
 	ufshpb_reset_host(hba);
 	ufshcd_hba_stop(hba);
 	hba->silence_err_logs = true;
-	ufshcd_retry_aborted_requests(hba);
+	ufshcd_complete_requests(hba);
 	hba->silence_err_logs = false;
 
 	/* scale up clocks to max frequency before full reinitialization */

[PATCH v2 02/10] scsi: ufs: Improve source code comments

[Bart Van Assche]

Make the descriptions above data structures that come from the UFS
specification match the terminology from that specification. This makes
it easier to find these data structures while reading the UFS
specification.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshci.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index de95be5d11d4..9a754fab8908 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -425,7 +425,7 @@ struct ufshcd_sg_entry {
 };
 
 /**
- * struct utp_transfer_cmd_desc - UFS Command Descriptor structure
+ * struct utp_transfer_cmd_desc - UTP Command Descriptor (UCD)
  * @command_upiu: Command UPIU Frame address
  * @response_upiu: Response UPIU Frame address
  * @prd_table: Physical Region Descriptor
@@ -451,7 +451,7 @@ struct request_desc_header {
 };
 
 /**
- * struct utp_transfer_req_desc - UTRD structure
+ * struct utp_transfer_req_desc - UTP Transfer Request Descriptor (UTRD)
  * @header: UTRD header DW-0 to DW-3
  * @command_desc_base_addr_lo: UCD base address low DW-4
  * @command_desc_base_addr_hi: UCD base address high DW-5

[PATCH v2 03/10] scsi: ufs: Improve static type checking

[Bart Van Assche]

Introduce an enumeration type for the overall command status to allow the
compiler to perform more static type checking.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 9 +++++----
 drivers/scsi/ufs/ufshci.h | 5 ++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 53dd42f95eed..610be6746f74 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -711,7 +711,7 @@ static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
  * This function is used to get the OCS field from UTRD
  * Returns the OCS field in the UTRD
  */
-static inline int ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
+static enum utp_ocs ufshcd_get_tr_ocs(struct ufshcd_lrb *lrbp)
 {
 	return le32_to_cpu(lrbp->utr_descriptor_ptr->header.dword_2) & MASK_OCS;
 }
@@ -5089,7 +5089,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 {
 	int result = 0;
 	int scsi_status;
-	int ocs;
+	enum utp_ocs ocs;
 
 	/* overall command status of utrd */
 	ocs = ufshcd_get_tr_ocs(lrbp);
@@ -6631,7 +6631,8 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id,
 		u8 tm_function, u8 *tm_response)
 {
 	struct utp_task_req_desc treq = { { 0 }, };
-	int ocs_value, err;
+	enum utp_ocs ocs_value;
+	int err;
 
 	/* Configure task request descriptor */
 	treq.header.dword_0 = cpu_to_le32(UTP_REQ_DESC_INT_CMD);
@@ -6809,7 +6810,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 	int err;
 	enum dev_cmd_type cmd_type = DEV_CMD_TYPE_QUERY;
 	struct utp_task_req_desc treq = { { 0 }, };
-	int ocs_value;
+	enum utp_ocs ocs_value;
 	u8 tm_f = be32_to_cpu(req_upiu->header.dword_1) >> 16 & MASK_TM_FUNC;
 
 	switch (msgcode) {
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index 9a754fab8908..f66cf9e477cb 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -389,7 +389,7 @@ enum {
 };
 
 /* Overall command status values */
-enum {
+enum utp_ocs {
 	OCS_SUCCESS			= 0x0,
 	OCS_INVALID_CMD_TABLE_ATTR	= 0x1,
 	OCS_INVALID_PRDT_ATTR		= 0x2,
@@ -402,6 +402,9 @@ enum {
 	OCS_INVALID_CRYPTO_CONFIG	= 0x9,
 	OCS_GENERAL_CRYPTO_ERROR	= 0xA,
 	OCS_INVALID_COMMAND_STATUS	= 0x0F,
+};
+
+enum {
 	MASK_OCS			= 0x0F,
 };
 

[PATCH v2 04/10] scsi: ufs: Log error handler activity

[Bart Van Assche]

Kernel logs are hard to comprehend without information about what the
UFS error handler is doing. Hence this patch that logs information
about error handler activity.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 610be6746f74..49623bf3e876 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -132,6 +132,14 @@ enum {
 	UFSHCD_CAN_QUEUE	= 32,
 };
 
+static const char *const ufshcd_state_name[] = {
+	[UFSHCD_STATE_RESET]			= "reset",
+	[UFSHCD_STATE_OPERATIONAL]		= "operational",
+	[UFSHCD_STATE_ERROR]			= "error",
+	[UFSHCD_STATE_EH_SCHEDULED_FATAL]	= "eh_fatal",
+	[UFSHCD_STATE_EH_SCHEDULED_NON_FATAL]	= "eh_non_fatal",
+};
+
 /* UFSHCD error handling flags */
 enum {
 	UFSHCD_EH_IN_PROGRESS = (1 << 0),
@@ -6069,6 +6077,13 @@ static void ufshcd_err_handler(struct work_struct *work)
 
 	hba = container_of(work, struct ufs_hba, eh_work);
 
+	dev_info(hba->dev,
+		 "%s started; HBA state %s; powered %d; shutting down %d; saved_err = %d; saved_uic_err = %d; force_reset = %d%s\n",
+		 __func__, ufshcd_state_name[hba->ufshcd_state],
+		 hba->is_powered, hba->shutting_down, hba->saved_err,
+		 hba->saved_uic_err, hba->force_reset,
+		 ufshcd_is_link_broken(hba) ? "; link is broken" : "");
+
 	down(&hba->host_sem);
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	if (ufshcd_err_handling_should_stop(hba)) {
@@ -6163,6 +6178,8 @@ static void ufshcd_err_handler(struct work_struct *work)
 			err_xfer = true;
 			goto lock_skip_pending_xfer_clear;
 		}
+		dev_err(hba->dev, "Aborted tag %d / CDB %#02x\n", tag,
+			hba->lrb[tag].cmd ? hba->lrb[tag].cmd->cmnd[0] : -1);
 	}
 
 	/* Clear pending task management requests */
@@ -6243,6 +6260,9 @@ static void ufshcd_err_handler(struct work_struct *work)
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	ufshcd_err_handling_unprepare(hba);
 	up(&hba->host_sem);
+
+	dev_info(hba->dev, "%s finished; HBA state %s\n", __func__,
+		 ufshcd_state_name[hba->ufshcd_state]);
 }
 
 /**
@@ -6539,6 +6559,10 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)
 	err = ufshcd_wait_for_register(hba,
 			REG_UTP_TASK_REQ_DOOR_BELL,
 			mask, 0, 1000, 1000);
+
+	dev_err(hba->dev, "Clearing task management function with tag %d %s\n",
+		tag, err ? "succeeded" : "failed");
+
 out:
 	return err;
 }

[PATCH v2 05/10] scsi: ufs: Export ufshcd_schedule_eh_work()

[Bart Van Assche]

Make it possible to call ufshcd_schedule_eh_work() from other source
files than ufshcd.c. Additionally, convert a source code comment into a
lockdep_assert_held() call.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 6 +++---
 drivers/scsi/ufs/ufshcd.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 49623bf3e876..0739aa566725 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -244,7 +244,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up);
 static irqreturn_t ufshcd_intr(int irq, void *__hba);
 static int ufshcd_change_power_mode(struct ufs_hba *hba,
 			     struct ufs_pa_layer_attr *pwr_mode);
-static void ufshcd_schedule_eh_work(struct ufs_hba *hba);
 static int ufshcd_setup_hba_vreg(struct ufs_hba *hba, bool on);
 static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on);
 static inline int ufshcd_config_vreg_hpm(struct ufs_hba *hba,
@@ -5915,9 +5914,10 @@ static inline bool ufshcd_is_saved_err_fatal(struct ufs_hba *hba)
 	       (hba->saved_err & (INT_FATAL_ERRORS | UFSHCD_UIC_HIBERN8_MASK));
 }
 
-/* host lock must be held before calling this func */
-static inline void ufshcd_schedule_eh_work(struct ufs_hba *hba)
+void ufshcd_schedule_eh_work(struct ufs_hba *hba)
 {
+	lockdep_assert_held(hba->host->host_lock);
+
 	/* handle fatal errors only when link is not in error state */
 	if (hba->ufshcd_state != UFSHCD_STATE_ERROR) {
 		if (hba->force_reset || ufshcd_is_link_broken(hba) ||
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 64ce723327b9..3f5dc6732fe1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -1023,6 +1023,7 @@ int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
 void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba, struct clk *refclk);
 void ufshcd_update_evt_hist(struct ufs_hba *hba, u32 id, u32 val);
 void ufshcd_hba_stop(struct ufs_hba *hba);
+void ufshcd_schedule_eh_work(struct ufs_hba *hba);
 
 static inline void check_upiu_size(void)
 {

[PATCH v2 06/10] scsi: ufs: Make it easier to add new debugfs attributes

[Bart Van Assche]

Introduce an array for debugfs attributes to make it easier to add new
debugfs attributes. Change the value of the inode.i_private pointer for
debugfs attributes from a pointer to the HBA data structure to a pointer
to the attribute description for the stats attribute. Store the HBA
pointer in the private data of the parent inode instead.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufs-debugfs.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index 4e1ff209b933..1d4e5aa4b762 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -8,6 +8,18 @@
 
 static struct dentry *ufs_debugfs_root;
 
+struct ufs_debugfs_attr {
+	const char			*name;
+	mode_t				mode;
+	const struct file_operations	*fops;
+};
+
+/* @file corresponds to a debugfs attribute in directory hba->debugfs_root. */
+static inline struct ufs_hba *hba_from_file(const struct file *file)
+{
+	return d_inode(file->f_path.dentry->d_parent)->i_private;
+}
+
 void __init ufs_debugfs_init(void)
 {
 	ufs_debugfs_root = debugfs_create_dir("ufshcd", NULL);
@@ -20,7 +32,7 @@ void ufs_debugfs_exit(void)
 
 static int ufs_debugfs_stats_show(struct seq_file *s, void *data)
 {
-	struct ufs_hba *hba = s->private;
+	struct ufs_hba *hba = hba_from_file(s->file);
 	struct ufs_event_hist *e = hba->ufs_stats.event;
 
 #define PRT(fmt, typ) \
@@ -126,13 +138,28 @@ static void ufs_debugfs_restart_ee(struct work_struct *work)
 	ufs_debugfs_put_user_access(hba);
 }
 
+static const struct ufs_debugfs_attr ufs_attrs[] = {
+	{ "stats", 0400, &ufs_debugfs_stats_fops },
+	{ }
+};
+
 void ufs_debugfs_hba_init(struct ufs_hba *hba)
 {
+	const struct ufs_debugfs_attr *attr;
+	struct dentry *root;
+
 	/* Set default exception event rate limit period to 20ms */
 	hba->debugfs_ee_rate_limit_ms = 20;
 	INIT_DELAYED_WORK(&hba->debugfs_ee_work, ufs_debugfs_restart_ee);
-	hba->debugfs_root = debugfs_create_dir(dev_name(hba->dev), ufs_debugfs_root);
-	debugfs_create_file("stats", 0400, hba->debugfs_root, hba, &ufs_debugfs_stats_fops);
+
+	root = debugfs_create_dir(dev_name(hba->dev), ufs_debugfs_root);
+	if (IS_ERR_OR_NULL(root))
+		return;
+	hba->debugfs_root = root;
+	d_inode(root)->i_private = hba;
+	for (attr = ufs_attrs; attr->name; attr++)
+		debugfs_create_file(attr->name, attr->mode, root, (void *)attr,
+				    attr->fops);
 	debugfs_create_file("exception_event_mask", 0600, hba->debugfs_root,
 			    hba, &ee_usr_mask_fops);
 	debugfs_create_u32("exception_event_rate_limit_ms", 0600, hba->debugfs_root,

[PATCH v2 07/10] scsi: ufs: Add debugfs attributes for triggering the UFS EH

[Bart Van Assche]

Make it easier to test the impact of the UFS error handler on software
that submits SCSI commands to the UFS driver.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufs-debugfs.c | 65 ++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-debugfs.c b/drivers/scsi/ufs/ufs-debugfs.c
index 1d4e5aa4b762..4a0bbcf1757a 100644
--- a/drivers/scsi/ufs/ufs-debugfs.c
+++ b/drivers/scsi/ufs/ufs-debugfs.c
@@ -138,8 +138,73 @@ static void ufs_debugfs_restart_ee(struct work_struct *work)
 	ufs_debugfs_put_user_access(hba);
 }
 
+static int ufs_saved_err_show(struct seq_file *s, void *data)
+{
+	struct ufs_debugfs_attr *attr = s->private;
+	struct ufs_hba *hba = hba_from_file(s->file);
+	const int *p;
+
+	if (strcmp(attr->name, "saved_err") == 0) {
+		p = &hba->saved_err;
+	} else if (strcmp(attr->name, "saved_uic_err") == 0) {
+		p = &hba->saved_uic_err;
+	} else {
+		return -ENOENT;
+	}
+
+	seq_printf(s, "%d\n", *p);
+	return 0;
+}
+
+static ssize_t ufs_saved_err_write(struct file *file, const char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	struct ufs_debugfs_attr *attr = file->f_inode->i_private;
+	struct ufs_hba *hba = hba_from_file(file);
+	char val_str[16] = { };
+	int val, ret;
+
+	if (count > sizeof(val_str))
+		return -EINVAL;
+	if (copy_from_user(val_str, buf, count))
+		return -EFAULT;
+	ret = kstrtoint(val_str, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irq(hba->host->host_lock);
+	if (strcmp(attr->name, "saved_err") == 0) {
+		hba->saved_err = val;
+	} else if (strcmp(attr->name, "saved_uic_err") == 0) {
+		hba->saved_uic_err = val;
+	} else {
+		ret = -ENOENT;
+	}
+	if (ret == 0)
+		ufshcd_schedule_eh_work(hba);
+	spin_unlock_irq(hba->host->host_lock);
+
+	return ret < 0 ? ret : count;
+}
+
+static int ufs_saved_err_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, ufs_saved_err_show, inode->i_private);
+}
+
+static const struct file_operations ufs_saved_err_fops = {
+	.owner		= THIS_MODULE,
+	.open		= ufs_saved_err_open,
+	.read		= seq_read,
+	.write		= ufs_saved_err_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static const struct ufs_debugfs_attr ufs_attrs[] = {
 	{ "stats", 0400, &ufs_debugfs_stats_fops },
+	{ "saved_err", 0600, &ufs_saved_err_fops },
+	{ "saved_uic_err", 0600, &ufs_saved_err_fops },
 	{ }
 };
 

[PATCH v2 08/10] scsi: ufs: Remove three superfluous casts

[Bart Van Assche]

Casting an int explicitly to u16 when passed as an argument to a function
is not necessary.

Since prd_table and ucd_prdt_ptr both have type struct ufshcd_sg_entry *,
remove the casts from assignments of these two to each other.

This patch does not change any functionality.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0739aa566725..7ea0588247b0 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2374,9 +2374,9 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 					sizeof(struct ufshcd_sg_entry)));
 		else
 			lrbp->utr_descriptor_ptr->prd_table_length =
-				cpu_to_le16((u16) (sg_segments));
+				cpu_to_le16(sg_segments);
 
-		prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr;
+		prd_table = lrbp->ucd_prdt_ptr;
 
 		scsi_for_each_sg(cmd, sg, sg_segments, i) {
 			prd_table[i].size  =
@@ -2668,7 +2668,7 @@ static void ufshcd_init_lrb(struct ufs_hba *hba, struct ufshcd_lrb *lrb, int i)
 	lrb->ucd_req_dma_addr = cmd_desc_element_addr;
 	lrb->ucd_rsp_ptr = (struct utp_upiu_rsp *)cmd_descp[i].response_upiu;
 	lrb->ucd_rsp_dma_addr = cmd_desc_element_addr + response_offset;
-	lrb->ucd_prdt_ptr = (struct ufshcd_sg_entry *)cmd_descp[i].prd_table;
+	lrb->ucd_prdt_ptr = cmd_descp[i].prd_table;
 	lrb->ucd_prdt_dma_addr = cmd_desc_element_addr + prdt_offset;
 }
 

[PATCH v2 09/10] scsi: ufs: Add a compile-time structure size check

[Bart Van Assche]

Before modifying struct ufshcd_sg_entry, add a compile-time structure
size check.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7ea0588247b0..dde4d3f607f2 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -9791,6 +9791,11 @@ static int __init ufshcd_core_init(void)
 {
 	int ret;
 
+	/* Verify that there are no gaps in struct utp_transfer_cmd_desc. */
+	static_assert(sizeof(struct utp_transfer_cmd_desc) ==
+		      2 * ALIGNED_UPIU_SIZE +
+			      SG_ALL * sizeof(struct ufshcd_sg_entry));
+
 	ufs_debugfs_init();
 
 	ret = scsi_register_driver(&ufs_dev_wlun_template.gendrv);

[PATCH v2 10/10] scsi: ufs: Micro-optimize ufshcd_map_sg()

[Bart Van Assche]

Replace two cpu_to_le32() calls by a single cpu_to_le64() call.

Additionally, issue a warning if the length of an scatter gather list
element exceeds what is allowed by the UFSHCI specification.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 19 +++++++++++++------
 drivers/scsi/ufs/ufshci.h |  6 ++----
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dde4d3f607f2..04cb67995750 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2379,12 +2379,19 @@ static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
 		prd_table = lrbp->ucd_prdt_ptr;
 
 		scsi_for_each_sg(cmd, sg, sg_segments, i) {
-			prd_table[i].size  =
-				cpu_to_le32(((u32) sg_dma_len(sg))-1);
-			prd_table[i].base_addr =
-				cpu_to_le32(lower_32_bits(sg->dma_address));
-			prd_table[i].upper_addr =
-				cpu_to_le32(upper_32_bits(sg->dma_address));
+			const unsigned int len = sg_dma_len(sg);
+
+			/*
+			 * From the UFSHCI spec: "Data Byte Count (DBC): A '0'
+			 * based value that indicates the length, in bytes, of
+			 * the data block. A maximum of length of 256KB may
+			 * exist for any entry. Bits 1:0 of this field shall be
+			 * 11b to indicate Dword granularity. A value of '3'
+			 * indicates 4 bytes, '7' indicates 8 bytes, etc."
+			 */
+			WARN_ONCE(len > 256 * 1024, "len = %#x\n", len);
+			prd_table[i].size = cpu_to_le32(len - 1);
+			prd_table[i].addr = cpu_to_le64(sg->dma_address);
 			prd_table[i].reserved = 0;
 		}
 	} else {
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index f66cf9e477cb..6a295c88d850 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -415,14 +415,12 @@ enum {
 
 /**
  * struct ufshcd_sg_entry - UFSHCI PRD Entry
- * @base_addr: Lower 32bit physical address DW-0
- * @upper_addr: Upper 32bit physical address DW-1
+ * @addr: Physical address; DW-0 and DW-1.
  * @reserved: Reserved for future use DW-2
  * @size: size of physical segment DW-3
  */
 struct ufshcd_sg_entry {
-	__le32    base_addr;
-	__le32    upper_addr;
+	__le64    addr;
 	__le32    reserved;
 	__le32    size;
 };

RE: [PATCH v2 00/10] UFS patches for kernel v5.16

[Avri Altman]

 Looks good to me.

Thanks,
Avri

> Hi Martin,
> 
> This patch series includes the UFS driver kernel patches I would like to see
> included kernel v5.16. Please consider these patches for kernel v5.16.
> 
> Thank you,
> 
> Bart.
> 
> Changes compared to v1:
> - Converted the sysfs attribute for triggering the error handler into two
>   debugfs attributes.
> - Use ufshcd_schedule_eh_work(hba) instead of scsi_schedule_eh(hba->host)
> to
>   trigger the error handler.
> - Added three patches that implement a small performance optimization.
> 
> Bart Van Assche (10):
>   scsi: ufs: Revert "Retry aborted SCSI commands instead of completing
>     these successfully"
>   scsi: ufs: Improve source code comments
>   scsi: ufs: Improve static type checking
>   scsi: ufs: Log error handler activity
>   scsi: ufs: Export ufshcd_schedule_eh_work()
>   scsi: ufs: Make it easier to add new debugfs attributes
>   scsi: ufs: Add debugfs attributes for triggering the UFS EH
>   scsi: ufs: Remove three superfluous casts
>   scsi: ufs: Add a compile-time structure size check
>   scsi: ufs: Micro-optimize ufshcd_map_sg()
> 
>  drivers/scsi/ufs/ufs-debugfs.c |  98 ++++++++++++++++++++++++++++++-
>  drivers/scsi/ufs/ufshcd.c      | 102 +++++++++++++++++++++------------
>  drivers/scsi/ufs/ufshcd.h      |   1 +
>  drivers/scsi/ufs/ufshci.h      |  15 ++---
>  4 files changed, 168 insertions(+), 48 deletions(-)

Re: [PATCH v2 00/10] UFS patches for kernel v5.16

[Martin K. Petersen]

Bart,

> This patch series includes the UFS driver kernel patches I would like
> to see included kernel v5.16. Please consider these patches for kernel
> v5.16.

Applied to 5.16/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering