Re: [PATCH v3 2/3] scsi: ufs: core: Cleanup ufshcd_slave_alloc()

[Bean Huo]

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