| Message ID | 20200528115616.9949-2-huobean@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: ufs: cleanup ufs initialization | expand |
> > From: Bean Huo <beanhuo@micron.com> > > For the UFS device, the maximum descriptor size is 255, max_t called in > ufs_get_device_desc() is useless. > > Signed-off-by: Bean Huo <beanhuo@micron.com> Acked-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/scsi/ufs/ufshcd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index aca50ed39844..0f8c7e05df29 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -6881,8 +6881,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba) > u8 *desc_buf; > struct ufs_dev_info *dev_info = &hba->dev_info; > > - buff_len = max_t(size_t, hba->desc_size.dev_desc, > - QUERY_DESC_MAX_SIZE + 1); > + buff_len = QUERY_DESC_MAX_SIZE + 1; So why the +1? Originally it was used for the '\0' terminator, which is not needed anymore.
On Thu, 2020-05-28 at 13:41 +0000, Avri Altman wrote: > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -6881,8 +6881,7 @@ static int ufs_get_device_desc(struct ufs_hba > > *hba) > > u8 *desc_buf; > > struct ufs_dev_info *dev_info = &hba->dev_info; > > > > - buff_len = max_t(size_t, hba->desc_size.dev_desc, > > - QUERY_DESC_MAX_SIZE + 1); > > + buff_len = QUERY_DESC_MAX_SIZE + 1; > > So why the +1? > Originally it was used for the '\0' terminator, which is not needed > anymore. you are correct, now not need +1, I will change it. thanks, Bean
On 2020-05-28 04:56, Bean Huo wrote: > From: Bean Huo <beanhuo@micron.com> > > For the UFS device, the maximum descriptor size is 255, max_t called in > ufs_get_device_desc() is useless. > > Signed-off-by: Bean Huo <beanhuo@micron.com> > --- > drivers/scsi/ufs/ufshcd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index aca50ed39844..0f8c7e05df29 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -6881,8 +6881,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba) > u8 *desc_buf; > struct ufs_dev_info *dev_info = &hba->dev_info; > > - buff_len = max_t(size_t, hba->desc_size.dev_desc, > - QUERY_DESC_MAX_SIZE + 1); > + buff_len = QUERY_DESC_MAX_SIZE + 1; > desc_buf = kmalloc(buff_len, GFP_KERNEL); > if (!desc_buf) { > err = -ENOMEM; Since the buff_len variable is not changed after its initial assignment, please remove it entirely. Thanks, Bart.
On Thu, 2020-05-28 at 07:56 -0700, Bart Van Assche wrote: > > - buff_len = max_t(size_t, hba->desc_size.dev_desc, > > - QUERY_DESC_MAX_SIZE + 1); > > + buff_len = QUERY_DESC_MAX_SIZE + 1; > > desc_buf = kmalloc(buff_len, GFP_KERNEL); > > if (!desc_buf) { > > err = -ENOMEM; > > Since the buff_len variable is not changed after its initial > assignment, > please remove it entirely. > > Thanks, > > Bart. hi, Bart Thanks. do you mean like this: buff_len = hba->desc_size[id]? Bean
On 2020-05-28 08:04, Bean Huo wrote:
> do you mean like this: buff_len = hba->desc_size[id]?
How about the following untested change?
Thanks,
Bart.
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 698e8d20b4ba..e33754c15c2c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6606,14 +6606,11 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba
static int ufs_get_device_desc(struct ufs_hba *hba)
{
int err;
- size_t buff_len;
u8 model_index;
u8 *desc_buf;
struct ufs_dev_info *dev_info = &hba->dev_info;
- buff_len = max_t(size_t, hba->desc_size.dev_desc,
- QUERY_DESC_MAX_SIZE + 1);
- desc_buf = kmalloc(buff_len, GFP_KERNEL);
+ desc_buf = kmalloc(QUERY_DESC_MAX_SIZE, GFP_KERNEL);
if (!desc_buf) {
err = -ENOMEM;
goto out;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index aca50ed39844..0f8c7e05df29 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6881,8 +6881,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba) u8 *desc_buf; struct ufs_dev_info *dev_info = &hba->dev_info; - buff_len = max_t(size_t, hba->desc_size.dev_desc, - QUERY_DESC_MAX_SIZE + 1); + buff_len = QUERY_DESC_MAX_SIZE + 1; desc_buf = kmalloc(buff_len, GFP_KERNEL); if (!desc_buf) { err = -ENOMEM;