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@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-mediatek