Messages in this thread Patch in this message | | | Subject | RE: [PATCH v5 1/8] scsi: ufs: enable WriteBooster on some pre-3.1 UFS devices | From | Stanley Chu <> | Date | Mon, 4 May 2020 22:33:57 +0800 |
| |
Hi Avri,
On Mon, 2020-05-04 at 10:37 +0000, Avri Altman wrote: > > > > static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf) > > { > > + if (!ufshcd_is_wb_allowed(hba)) > > + return; > > + > > + if (hba->desc_size.dev_desc <= > > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP) > Should be > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4
I think this description length check is redundant because the device quirk shall be added only after WriteBooster supportat is confirmed in attached UFS device. So I will remove this in next version.
> > > + goto wb_disabled; > > + > > hba->dev_info.d_ext_ufs_feature_sup = > > get_unaligned_be32(desc_buf + > > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP); > > > > > > static int ufs_get_device_desc(struct ufs_hba *hba) > > @@ -6862,10 +6890,6 @@ static int ufs_get_device_desc(struct ufs_hba > > *hba) > > > > model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME]; > > > > - /* Enable WB only for UFS-3.1 */ > > - if (dev_info->wspecversion >= 0x310) > > - ufshcd_wb_probe(hba, desc_buf); > > - > > err = ufshcd_read_string_desc(hba, model_index, > > &dev_info->model, SD_ASCII_STD); > > if (err < 0) { > > @@ -6874,6 +6898,16 @@ static int ufs_get_device_desc(struct ufs_hba > > *hba) > > goto out; > > } > > > > + ufs_fixup_device_setup(hba); > I don't think you should "hide" ufs_fixup_device_setup inside ufs_get_device_desc.
The reason is as below,
ufshcd_wb_probe() needs the contents of Device Descriptor for initialization. To avoid double reading the Device Descriptor, I keep ufshcd_wb_probe() inside ufs_get_device_desc() to use its "desc_buf".
And ufshcd_wb_probe() needs well-configured device quirk for entrance check, thus ufs_fixup_device_setup() shall be moved before ufshcd_wb_probe().
This change does not affect the behavior and functionality of ufs_fixup_device_setup() since it is still executed once only during ufshcd_init() flow and not be executed again in the future.
Thanks, Stanley Chu
> > Thanks, > Avri > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
| |