On 10/18/22 11:16, Bean Huo wrote:
+static void ufshcd_lu_init(struct ufs_hba *hba, struct scsi_device *sdev)
+{
+ u8 lun_qdepth;
+ u8 *desc_buf;
+ int ret;
+ int len;
+ u8 lun;
+
+ lun_qdepth = hba->nutrs;
+ lun = ufshcd_scsi_to_upiu_lun(sdev->lun);
+ len = hba->desc_size[QUERY_DESC_IDN_UNIT];
Isn't the preferred style for kernel code to combine the above
declarations and assignments (but not memory allocation calls)?
+ desc_buf = kzalloc(len, GFP_KERNEL);
+ if (!desc_buf)
+ goto set_qdepth;
+
+ ret = ufshcd_read_unit_desc_param(hba, lun, 0, desc_buf, len);
+ if (ret < 0) {
+ if (ret == -EOPNOTSUPP)
+ /* If LU doesn't support unit descriptor, its queue depth is set to 1 */
+ lun_qdepth = 1;
+ kfree(desc_buf);
+ goto set_qdepth;
+ }
+
+ if (desc_buf[UNIT_DESC_PARAM_LU_Q_DEPTH])
+ /*
+ * In per-LU queueing architecture, bLUQueueDepth will not be 0, then we will
+ * use the smaller between UFSHCI CAP.NUTRS and UFS LU bLUQueueDepth
+ */
+ lun_qdepth = min_t(int, desc_buf[UNIT_DESC_PARAM_LU_Q_DEPTH], hba->nutrs);
Should a test be added that verifies that UNIT_DESC_PARAM_LU_Q_DEPTH < len?
Additionally, please use braces ({}) around multi-line if-statement bodies.
+ /*
+ * According to UFS device specification, the write protection mode is only supported by
+ * normal LU, not supported by WLUN.
+ */
+ if (hba->dev_info.f_power_on_wp_en && lun < hba->dev_info.max_lu_supported &&
+ !hba->dev_info.is_lu_power_on_wp &&
+ desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] == UFS_LU_POWER_ON_WP)
+ hba->dev_info.is_lu_power_on_wp = true;
Also here, should the following test be added:
UNIT_DESC_PARAM_LU_WR_PROTECT < len?
Otherwise this patch looks good to me.
Thanks,
Bart.
