We used to use the extended-feature field in the device descriptor,
as an indication that the device supports ufs2.2 or later.
Remove that as this check is specifically done few lines above.
Reviewed-by: Bean Huo <beanhuo@micron.com>
Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
---
drivers/ufs/core/ufshcd.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 2dbe249..2e47c69 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7608,10 +7608,6 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, const u8 *desc_buf)
(hba->dev_quirks & UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES)))
goto wb_disabled;
- if (hba->desc_size[QUERY_DESC_IDN_DEVICE] <
- DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
- goto wb_disabled;
-
ext_ufs_feature = get_unaligned_be32(desc_buf +
DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP);
--
2.7.4On 11/27/22 04:08, Arthur Simchaev wrote: > We used to use the extended-feature field in the device descriptor, > as an indication that the device supports ufs2.2 or later. > Remove that as this check is specifically done few lines above. Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On 11/27/22 04:08, Arthur Simchaev wrote: > We used to use the extended-feature field in the device descriptor, > as an indication that the device supports ufs2.2 or later. > Remove that as this check is specifically done few lines above. > > Reviewed-by: Bean Huo <beanhuo@micron.com> > Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com> > --- > drivers/ufs/core/ufshcd.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 2dbe249..2e47c69 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -7608,10 +7608,6 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, const u8 *desc_buf) > (hba->dev_quirks & UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES))) > goto wb_disabled; > > - if (hba->desc_size[QUERY_DESC_IDN_DEVICE] < > - DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4) > - goto wb_disabled; > - > ext_ufs_feature = get_unaligned_be32(desc_buf + > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP); Does this code really have to be removed? I see a check of the UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES flag above the removed code but no check of the descriptor size? Thanks, Bart.
On 08.12.22 12:31 AM, Bart Van Assche wrote: > On 11/27/22 04:08, Arthur Simchaev wrote: >> We used to use the extended-feature field in the device descriptor, >> as an indication that the device supports ufs2.2 or later. >> Remove that as this check is specifically done few lines above. >> >> Reviewed-by: Bean Huo <beanhuo@micron.com> >> Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com> >> --- >> drivers/ufs/core/ufshcd.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index 2dbe249..2e47c69 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -7608,10 +7608,6 @@ static void ufshcd_wb_probe(struct ufs_hba >> *hba, const u8 *desc_buf) >> (hba->dev_quirks & >> UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES))) >> goto wb_disabled; >> - if (hba->desc_size[QUERY_DESC_IDN_DEVICE] < >> - DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4) >> - goto wb_disabled; >> - >> ext_ufs_feature = get_unaligned_be32(desc_buf + >> DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP); > > Does this code really have to be removed? I see a check of the > UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES flag above the removed > code but no check of the descriptor size? > it is not necessary to check this, but if you have concern, we could change to like this: if (desc_buf[DEVICE_DESC_PARAM_LEN] < DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4) goto wb_disabled; then hba->desc_size could be removed. Kind regards, Bean
On 12/8/22 04:22, Bean Huo wrote: > > On 08.12.22 12:31 AM, Bart Van Assche wrote: >> On 11/27/22 04:08, Arthur Simchaev wrote: >>> We used to use the extended-feature field in the device descriptor, >>> as an indication that the device supports ufs2.2 or later. >>> Remove that as this check is specifically done few lines above. >>> >>> Reviewed-by: Bean Huo <beanhuo@micron.com> >>> Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com> >>> --- >>> drivers/ufs/core/ufshcd.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >>> index 2dbe249..2e47c69 100644 >>> --- a/drivers/ufs/core/ufshcd.c >>> +++ b/drivers/ufs/core/ufshcd.c >>> @@ -7608,10 +7608,6 @@ static void ufshcd_wb_probe(struct ufs_hba >>> *hba, const u8 *desc_buf) >>> (hba->dev_quirks & >>> UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES))) >>> goto wb_disabled; >>> - if (hba->desc_size[QUERY_DESC_IDN_DEVICE] < >>> - DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4) >>> - goto wb_disabled; >>> - >>> ext_ufs_feature = get_unaligned_be32(desc_buf + >>> DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP); >> >> Does this code really have to be removed? I see a check of the >> UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES flag above the removed >> code but no check of the descriptor size? >> > it is not necessary to check this, but if you have concern, we could > change to like this: > > > if (desc_buf[DEVICE_DESC_PARAM_LEN] < > DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4) > goto wb_disabled; > > then hba->desc_size could be removed. Hi Bean, My only concern is that this patch conflicts with the pending MCQ patch series. Since that conflict is unavoidable, let's keep this patch. Bart.
© 2016 - 2025 Red Hat, Inc.