On Tue, 2022-10-18 at 11:27 -0700, Bart Van Assche wrote:
> Should a test be added that verifies that UNIT_DESC_PARAM_LU_Q_DEPTH
> < len?
>
> Additionally, please use braces ({}) around multi-line if-statement
> bodies.
>
> > + /*
> > + * According to UFS device specification, the write
> > protection mode is only supported by
> > + * normal LU, not supported by WLUN.
> > + */
> > + if (hba->dev_info.f_power_on_wp_en && lun < hba-
> > >dev_info.max_lu_supported &&
> > + !hba->dev_info.is_lu_power_on_wp &&
> > + desc_buf[UNIT_DESC_PARAM_LU_WR_PROTECT] ==
> > UFS_LU_POWER_ON_WP)
> > + hba->dev_info.is_lu_power_on_wp = true;
>
> Also here, should the following test be added:
> UNIT_DESC_PARAM_LU_WR_PROTECT < len?
>
> Otherwise this patch looks good to me.
Bart,
len is already the LU descriptor size. UNIT_DESC_PARAM_LU_Q_DEPTH (0x6)
and UNIT_DESC_PARAM_LU_WR_PROTECT(0x5) are very basic parameters in the
LU descriptor, no matter what version of UFS is, there are these two
parameters in the descriptor. So the return value of
ufshcd_read_unit_desc_param() will be -EOPNOTSUPP, -EINVAL, -ENOMEM or
0 (successful read of LU descriptor). the checkup of "< len" is not
neccessary.
Kind regards,
Bean
