2020-05-28 12:00:01

by Bean Huo

Subject: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

From: Bean Huo <[email protected]>

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 <[email protected]>
---
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;
--
2.17.1

2020-05-28 13:43:43

by Avri Altman

Subject: RE: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

>
> From: Bean Huo <[email protected]>
>
> 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 <[email protected]>
Acked-by: Avri Altman <[email protected]>

> ---
> 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.

2020-05-28 13:52:18

by Bean Huo

Subject: Re: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

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

2020-05-28 15:00:57

by Bart Van Assche

Subject: Re: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

On 2020-05-28 04:56, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> 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 <[email protected]>
> ---
> 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.

2020-05-28 15:06:42

by Bean Huo

Subject: Re: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

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

2020-05-28 16:20:54

by Bart Van Assche

Subject: Re: [PATCH v2 1/3] scsi: ufs: remove max_t in ufs_get_device_desc

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;