| Message ID | 20220419183044.789065-1-huobean@gmail.com |
|---|---|
| Headers | show |
| Series | Several changes for UFSHPB | expand |
On Wed, 2022-04-20 at 14:31 +0900, Keoseong Park wrote: > > +/** > > + *ufshpb_submit_region_inactive() - submit a region to be > > inactivated later > > + *@hpb: per-LU HPB instance > > + *@region_index: the index associated with the region that will be > > inactivated later > > + */ > > +static void ufshpb_submit_region_inactive(struct ufshpb_lu *hpb, > > int region_index) > > +{ > > + int subregion_index; > > + struct ufshpb_region *rgn; > > + struct ufshpb_subregion *srgn; > > + > > + /* > > + * Remove this region from active region list and add it > > to inactive list > > + */ > > + spin_lock(&hpb->rsp_list_lock); > > + ufshpb_update_inactive_info(hpb, region_index); > How about separating the "hpb->stats.rb_inactive_cnt++" code from > ufshpb_update_inactive_info()? > Because I think this code should only be used in > ufshpb_rsp_req_region_update(). based on Documentation/ABI/testing/sysfs-driver-ufs: "What: /sys/class/scsi_device/*/device/hpb_stats/rb_inactive_cnt Date: June 2021 Contact: Daejun Park <daejun7.park@samsung.com> Description: This entry shows the number of inactive regions recommended by response UPIUs." This parameter should be increased only when recieving inactive recommended. I will change it in the next version, thanks. Kind regards, Bean
> On Wed, 2022-04-20 at 14:31 +0900, Keoseong Park wrote: > > > > > > + /* > > > + * Remove this region from active region list and add it > > > to inactive list > > > + */ > > > + spin_lock(&hpb->rsp_list_lock); > > > + ufshpb_update_inactive_info(hpb, region_index); > > How about separating the "hpb->stats.rb_inactive_cnt++" code from > > ufshpb_update_inactive_info()? > > Because I think this code should only be used in > > ufshpb_rsp_req_region_update(). Hi Keoseong Park, I didn't take this hpb->stats.rb_inactive_cnt++ out, since I think if the host receives HPB operation:02h, which means the device recommends the host to de-activate all active regions on the host side. we need to add these de-activations to this parameter because the device has inactivated all active regions. Otherwise, we will find an inconsistent inactivation and activation counter. Please have a look a review the v3. Kind regards, Bean
From: Bean Huo <beanhuo@micron.com> Hi UFS driver developers/reviewers Here are some changes to the UFS HPB driver. They are all based on Martin's Git repo 5.18/scsi-staging branch. Please refer to the patch submission information for the specific purpose of each patch. I tested them on my own platform. Please have a review and any comments and suggestions are welcome. v1--v2: 1. Increase the submission information of the cover letter. 2. Fix coding style issues in patch 4/5 3. Add new patch 5/5. Bean Huo (5): scsi: ufshpb: Merge ufshpb_reset() and ufshpb_reset_host() scsi: ufshpb: Remove 0 assignment for enum value scsi: ufshpb: Cleanup the handler when device reset HPB information scsi: ufshpb: Add handing of device reset HPB regions Infos in HPB device mode scsi: ufshpb: Cleanup ufshpb_suspend/resume drivers/scsi/ufs/ufshcd.c | 4 +- drivers/scsi/ufs/ufshpb.c | 157 ++++++++++++++++++++++---------------- drivers/scsi/ufs/ufshpb.h | 10 +-- 3 files changed, 98 insertions(+), 73 deletions(-)