RE: [PATCH v22 2/4] scsi: ufs: L2P map management for HPB read
From: Avri Altman <Avri.Altman@wdc.com>
Date: 2021-02-23 07:52:44
Also in:
lkml
+/*
+ * This function will parse recommended active subregion information in
sense
+ * data field of response UPIU with SAM_STAT_GOOD state.
+ */
+void ufshpb_rsp_upiu(struct ufs_hba *hba, struct ufshcd_lrb *lrbp)
+{
+ struct ufshpb_lu *hpb;
+ struct scsi_device *sdev;
+ struct utp_hpb_rsp *rsp_field = &lrbp->ucd_rsp_ptr->hr;
+ int data_seg_len;
+ bool found = false;
+
+ __shost_for_each_device(sdev, hba->host) {
+ hpb = ufshpb_get_hpb_data(sdev);
+
+ if (!hpb)
+ continue;
+
+ if (rsp_field->lun == hpb->lun) {
+ found = true;
+ break;This piece of code looks awkward, although it is probably working. Why not just having a reference to the hpb luns, e.g. something like: struct ufshpb_lu *hpb_luns[8] in struct ufs_hba. Less elegant - but much more effective than iterating the scsi host on every completion interrupt.
+ }
+ }
+
+ if (!found)
+ return;
+
+ if ((ufshpb_get_state(hpb) != HPB_PRESENT) &&
+ (ufshpb_get_state(hpb) != HPB_SUSPEND)) {
+ dev_notice(&hpb->sdev_ufs_lu->sdev_dev,
+ "%s: ufshpb state is not PRESENT/SUSPEND\n",
+ __func__);
+ return;
+ }
+
+ data_seg_len = be32_to_cpu(lrbp->ucd_rsp_ptr->header.dword_2)
+ & MASK_RSP_UPIU_DATA_SEG_LEN;
+
+ /* To flush remained rsp_list, we queue the map_work task */
+ if (!data_seg_len) {data_seg_len should be 0x14
+ if (!ufshpb_is_general_lun(hpb->lun)) + return; + + ufshpb_kick_map_work(hpb); + return; + } + + /* Check HPB_UPDATE_ALERT */ + if (!(lrbp->ucd_rsp_ptr->header.dword_2 & + UPIU_HEADER_DWORD(0, 2, 0, 0))) + return; + + BUILD_BUG_ON(sizeof(struct utp_hpb_rsp) != UTP_HPB_RSP_SIZE); + + if (!ufshpb_is_hpb_rsp_valid(hba, lrbp, rsp_field)) + return;
How about moving both the data_seg_len and alert bit checks into ufshpb_is_hpb_rsp_valid, And moving ufshpb_is_hpb_rsp_valid to the beginning of the function? This way you would save redundant stuff if not a valid response. Thanks, Avri