From: Bean Huo <[email protected]>
This patchset is to cleanup UFS descriptor access, and delete some
unnecessary code.
Changelog:
v1 - v2:
1. split patch
2. fix one compiling WARNING (Reported-by: kbuild test robot <[email protected]>)
Bean Huo (3):
scsi: ufs: remove max_t in ufs_get_device_desc
scsi: ufs: delete ufshcd_read_desc()
scsi: ufs: cleanup ufs initialization path
drivers/scsi/ufs/ufshcd.c | 166 ++++++++------------------------------
drivers/scsi/ufs/ufshcd.h | 12 +--
2 files changed, 34 insertions(+), 144 deletions(-)
--
2.17.1
From: Bean Huo <[email protected]>
At UFS initialization stage, to get the length of the descriptor,
ufshcd_read_desc_length() being called 6 times. This patch is to
delete unnecessary reduntant code, remove ufshcd_read_desc_length()
and boost UFS initialization.
Signed-off-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 138 +++++++-------------------------------
drivers/scsi/ufs/ufshcd.h | 12 +---
2 files changed, 26 insertions(+), 124 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 0a95f0a5ab73..c47f4584c0f4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3052,47 +3052,6 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
return err;
}
-/**
- * ufshcd_read_desc_length - read the specified descriptor length from header
- * @hba: Pointer to adapter instance
- * @desc_id: descriptor idn value
- * @desc_index: descriptor index
- * @desc_length: pointer to variable to read the length of descriptor
- *
- * Return 0 in case of success, non-zero otherwise
- */
-static int ufshcd_read_desc_length(struct ufs_hba *hba,
- enum desc_idn desc_id,
- int desc_index,
- int *desc_length)
-{
- int ret;
- u8 header[QUERY_DESC_HDR_SIZE];
- int header_len = QUERY_DESC_HDR_SIZE;
-
- if (desc_id >= QUERY_DESC_IDN_MAX)
- return -EINVAL;
-
- ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
- desc_id, desc_index, 0, header,
- &header_len);
-
- if (ret) {
- dev_err(hba->dev, "%s: Failed to get descriptor header id %d",
- __func__, desc_id);
- return ret;
- } else if (desc_id != header[QUERY_DESC_DESC_TYPE_OFFSET]) {
- dev_warn(hba->dev, "%s: descriptor header id %d and desc_id %d mismatch",
- __func__, header[QUERY_DESC_DESC_TYPE_OFFSET],
- desc_id);
- ret = -EINVAL;
- }
-
- *desc_length = header[QUERY_DESC_LENGTH_OFFSET];
- return ret;
-
-}
-
/**
* ufshcd_map_desc_id_to_length - map descriptor IDN to its length
* @hba: Pointer to adapter instance
@@ -3101,46 +3060,27 @@ static int ufshcd_read_desc_length(struct ufs_hba *hba,
*
* Return 0 in case of success, non-zero otherwise
*/
-int ufshcd_map_desc_id_to_length(struct ufs_hba *hba,
- enum desc_idn desc_id, int *desc_len)
+int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
+ int *desc_len)
{
- switch (desc_id) {
- case QUERY_DESC_IDN_DEVICE:
- *desc_len = hba->desc_size.dev_desc;
- break;
- case QUERY_DESC_IDN_POWER:
- *desc_len = hba->desc_size.pwr_desc;
- break;
- case QUERY_DESC_IDN_GEOMETRY:
- *desc_len = hba->desc_size.geom_desc;
- break;
- case QUERY_DESC_IDN_CONFIGURATION:
- *desc_len = hba->desc_size.conf_desc;
- break;
- case QUERY_DESC_IDN_UNIT:
- *desc_len = hba->desc_size.unit_desc;
- break;
- case QUERY_DESC_IDN_INTERCONNECT:
- *desc_len = hba->desc_size.interc_desc;
- break;
- case QUERY_DESC_IDN_STRING:
- *desc_len = QUERY_DESC_MAX_SIZE;
- break;
- case QUERY_DESC_IDN_HEALTH:
- *desc_len = hba->desc_size.hlth_desc;
- break;
- case QUERY_DESC_IDN_RFU_0:
- case QUERY_DESC_IDN_RFU_1:
- *desc_len = 0;
- break;
- default:
+ if (desc_id >= QUERY_DESC_IDN_MAX) {
*desc_len = 0;
return -EINVAL;
}
+
+ *desc_len = hba->desc_size[desc_id];
return 0;
}
EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
+static void ufshcd_update_desc_length(struct ufs_hba *hba,
+ enum desc_idn desc_id, int desc_len)
+{
+ if (hba->desc_size[desc_id] == QUERY_DESC_MAX_SIZE &&
+ desc_id != QUERY_DESC_IDN_STRING)
+ hba->desc_size[desc_id] = desc_len;
+}
+
/**
* ufshcd_read_desc_param - read the specified descriptor parameter
* @hba: Pointer to adapter instance
@@ -3209,6 +3149,9 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
goto out;
}
+ ufshcd_update_desc_length(hba, desc_id,
+ desc_buf[QUERY_DESC_LENGTH_OFFSET]);
+
/* Check wherher we will not copy more data, than available */
if (is_kmalloc && param_size > buff_len)
param_size = buff_len;
@@ -6665,7 +6608,7 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
{
int ret;
- int buff_len = hba->desc_size.pwr_desc;
+ int buff_len = hba->desc_size[QUERY_DESC_IDN_POWER];
u8 *desc_buf;
u32 icc_level;
@@ -6783,7 +6726,8 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
if (!ufshcd_is_wb_allowed(hba))
return;
- if (hba->desc_size.dev_desc < DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
+ if (hba->desc_size[QUERY_DESC_IDN_DEVICE] <
+ DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
goto wb_disabled;
hba->dev_info.d_ext_ufs_feature_sup =
@@ -6878,7 +6822,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
}
err = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0, desc_buf,
- hba->desc_size.dev_desc);
+ hba->desc_size[QUERY_DESC_IDN_DEVICE]);
if (err) {
dev_err(hba->dev, "%s: Failed reading Device Desc. err = %d\n",
__func__, err);
@@ -7108,42 +7052,10 @@ static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba *hba)
static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
{
- int err;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_DEVICE, 0,
- &hba->desc_size.dev_desc);
- if (err)
- hba->desc_size.dev_desc = QUERY_DESC_DEVICE_DEF_SIZE;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_POWER, 0,
- &hba->desc_size.pwr_desc);
- if (err)
- hba->desc_size.pwr_desc = QUERY_DESC_POWER_DEF_SIZE;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_INTERCONNECT, 0,
- &hba->desc_size.interc_desc);
- if (err)
- hba->desc_size.interc_desc = QUERY_DESC_INTERCONNECT_DEF_SIZE;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_CONFIGURATION, 0,
- &hba->desc_size.conf_desc);
- if (err)
- hba->desc_size.conf_desc = QUERY_DESC_CONFIGURATION_DEF_SIZE;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_UNIT, 0,
- &hba->desc_size.unit_desc);
- if (err)
- hba->desc_size.unit_desc = QUERY_DESC_UNIT_DEF_SIZE;
-
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_GEOMETRY, 0,
- &hba->desc_size.geom_desc);
- if (err)
- hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
+ int i;
- err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_HEALTH, 0,
- &hba->desc_size.hlth_desc);
- if (err)
- hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
+ for (i = 0; i < QUERY_DESC_IDN_MAX; i++)
+ hba->desc_size[i] = QUERY_DESC_MAX_SIZE;
}
static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
@@ -7152,7 +7064,7 @@ static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
size_t buff_len;
u8 *desc_buf;
- buff_len = hba->desc_size.geom_desc;
+ buff_len = hba->desc_size[QUERY_DESC_IDN_GEOMETRY];
desc_buf = kmalloc(buff_len, GFP_KERNEL);
if (!desc_buf) {
err = -ENOMEM;
@@ -7253,7 +7165,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
/* Clear any previous UFS device information */
memset(&hba->dev_info, 0, sizeof(hba->dev_info));
- /* Init check for device descriptor sizes */
+ /* Init device descriptor sizes */
ufshcd_init_desc_sizes(hba);
/* Init UFS geometry descriptor related parameters */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e3dfb48e669e..b966d9b0eb3d 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -236,16 +236,6 @@ struct ufs_dev_cmd {
struct ufs_query query;
};
-struct ufs_desc_size {
- int dev_desc;
- int pwr_desc;
- int geom_desc;
- int interc_desc;
- int unit_desc;
- int conf_desc;
- int hlth_desc;
-};
-
/**
* struct ufs_clk_info - UFS clock related info
* @list: list headed by hba->clk_list_head
@@ -738,7 +728,7 @@ struct ufs_hba {
bool is_urgent_bkops_lvl_checked;
struct rw_semaphore clk_scaling_lock;
- struct ufs_desc_size desc_size;
+ u8 desc_size[QUERY_DESC_IDN_MAX];
atomic_t scsi_block_reqs_cnt;
struct device bsg_dev;
--
2.17.1
Hi,
> From: Bean Huo <[email protected]>
>
> At UFS initialization stage, to get the length of the descriptor,
> ufshcd_read_desc_length() being called 6 times.
May I suggest one more clarifying sentence to your commit log:
"Instead, we will capture the descriptor size the first time we'll read it."
>This patch is to
> delete unnecessary reduntant code, remove ufshcd_read_desc_length()
typo: redundant
> and boost UFS initialization.
>
> Signed-off-by: Bean Huo <[email protected]>
> + if (desc_id >= QUERY_DESC_IDN_MAX) {
> *desc_len = 0;
> return -EINVAL;
> }
if (desc_id == QUERY_DESC_IDN_RFU_0 || desc_id == QUERY_DESC_IDN_RFU_1)
*desc_len = 0;
else
> +
> + *desc_len = hba->desc_size[desc_id];
> return 0;
> }
> EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
>
> +static void ufshcd_update_desc_length(struct ufs_hba *hba,
> + enum desc_idn desc_id, int desc_len)
desc_len is at most 255 so maybe u8?
> +{
> + if (hba->desc_size[desc_id] == QUERY_DESC_MAX_SIZE &&
> + desc_id != QUERY_DESC_IDN_STRING)
> + hba->desc_size[desc_id] = desc_len;
> +}
> +
Thanks,
Avri
On 2020-05-28 04:56, Bean Huo wrote:
> At UFS initialization stage, to get the length of the descriptor,
> ufshcd_read_desc_length() being called 6 times. This patch is to
> delete unnecessary reduntant code, remove ufshcd_read_desc_length()
> and boost UFS initialization.
As explained in Documentation/process/submitting-patches.rst, please use
the imperative mood in patch descriptions. In other words, please change
"This patch is to delete" into "Delete". Please also change "reduntant"
into "redundant". Otherwise this patch looks good to me. Hence:
Reviewed-by: Bart van Assche <[email protected]>
On Thu, 2020-05-28 at 14:58 +0000, Avri Altman wrote:
> Hi,
>
> > From: Bean Huo <[email protected]>
> >
> > At UFS initialization stage, to get the length of the descriptor,
> > ufshcd_read_desc_length() being called 6 times.
>
> May I suggest one more clarifying sentence to your commit log:
> "Instead, we will capture the descriptor size the first time we'll
> read it."
>
> > This patch is to
> > delete unnecessary reduntant code, remove ufshcd_read_desc_length()
>
> typo: redundant
fixed.
>
> > and boost UFS initialization.
> >
> > Signed-off-by: Bean Huo <[email protected]>
> > + if (desc_id >= QUERY_DESC_IDN_MAX) {
> > *desc_len = 0;
> > return -EINVAL;
> > }
>
> if (desc_id == QUERY_DESC_IDN_RFU_0 || desc_id ==
> QUERY_DESC_IDN_RFU_1)
> *desc_len = 0;
> else
> > +
> > + *desc_len = hba->desc_size[desc_id];
> > return 0;
> > }
> > EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
> >
> > +static void ufshcd_update_desc_length(struct ufs_hba *hba,
> > + enum desc_idn desc_id, int
> > desc_len)
>
> desc_len is at most 255 so maybe u8?
>
Avri
thanks, it will be changed in next version.
Bean
On 2020-05-28 07:58, Avri Altman wrote:
>> From: Bean Huo <[email protected]>
>> +static void ufshcd_update_desc_length(struct ufs_hba *hba,
>> + enum desc_idn desc_id, int desc_len)
> desc_len is at most 255 so maybe u8?
At least on x86 using types like 'u8' for function arguments may lead to
suboptimal code because it may cause the compiler to insert a widening
instruction. How about changing 'int desc_len' into 'unsigned desc_len'
instead?
Thanks,
Bart.
Hi Bean,
On Thu, 2020-05-28 at 13:56 +0200, Bean Huo wrote:
> From: Bean Huo <[email protected]>
>
> At UFS initialization stage, to get the length of the descriptor,
> ufshcd_read_desc_length() being called 6 times. This patch is to
> delete unnecessary reduntant code, remove ufshcd_read_desc_length()
> and boost UFS initialization.
>
> Signed-off-by: Bean Huo <[email protected]>
> ---
> drivers/scsi/ufs/ufshcd.c | 138 +++++++-------------------------------
> drivers/scsi/ufs/ufshcd.h | 12 +---
> 2 files changed, 26 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 0a95f0a5ab73..c47f4584c0f4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3052,47 +3052,6 @@ int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
> return err;
> }
>
> -/**
> - * ufshcd_read_desc_length - read the specified descriptor length from header
> - * @hba: Pointer to adapter instance
> - * @desc_id: descriptor idn value
> - * @desc_index: descriptor index
> - * @desc_length: pointer to variable to read the length of descriptor
> - *
> - * Return 0 in case of success, non-zero otherwise
> - */
> -static int ufshcd_read_desc_length(struct ufs_hba *hba,
> - enum desc_idn desc_id,
> - int desc_index,
> - int *desc_length)
> -{
> - int ret;
> - u8 header[QUERY_DESC_HDR_SIZE];
> - int header_len = QUERY_DESC_HDR_SIZE;
> -
> - if (desc_id >= QUERY_DESC_IDN_MAX)
> - return -EINVAL;
> -
> - ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
> - desc_id, desc_index, 0, header,
> - &header_len);
> -
> - if (ret) {
> - dev_err(hba->dev, "%s: Failed to get descriptor header id %d",
> - __func__, desc_id);
> - return ret;
> - } else if (desc_id != header[QUERY_DESC_DESC_TYPE_OFFSET]) {
> - dev_warn(hba->dev, "%s: descriptor header id %d and desc_id %d mismatch",
> - __func__, header[QUERY_DESC_DESC_TYPE_OFFSET],
> - desc_id);
> - ret = -EINVAL;
> - }
> -
> - *desc_length = header[QUERY_DESC_LENGTH_OFFSET];
> - return ret;
> -
> -}
> -
> /**
> * ufshcd_map_desc_id_to_length - map descriptor IDN to its length
> * @hba: Pointer to adapter instance
> @@ -3101,46 +3060,27 @@ static int ufshcd_read_desc_length(struct ufs_hba *hba,
> *
> * Return 0 in case of success, non-zero otherwise
> */
> -int ufshcd_map_desc_id_to_length(struct ufs_hba *hba,
> - enum desc_idn desc_id, int *desc_len)
> +int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
> + int *desc_len)
> {
> - switch (desc_id) {
> - case QUERY_DESC_IDN_DEVICE:
> - *desc_len = hba->desc_size.dev_desc;
> - break;
> - case QUERY_DESC_IDN_POWER:
> - *desc_len = hba->desc_size.pwr_desc;
> - break;
> - case QUERY_DESC_IDN_GEOMETRY:
> - *desc_len = hba->desc_size.geom_desc;
> - break;
> - case QUERY_DESC_IDN_CONFIGURATION:
> - *desc_len = hba->desc_size.conf_desc;
> - break;
> - case QUERY_DESC_IDN_UNIT:
> - *desc_len = hba->desc_size.unit_desc;
> - break;
> - case QUERY_DESC_IDN_INTERCONNECT:
> - *desc_len = hba->desc_size.interc_desc;
> - break;
> - case QUERY_DESC_IDN_STRING:
> - *desc_len = QUERY_DESC_MAX_SIZE;
> - break;
> - case QUERY_DESC_IDN_HEALTH:
> - *desc_len = hba->desc_size.hlth_desc;
> - break;
> - case QUERY_DESC_IDN_RFU_0:
> - case QUERY_DESC_IDN_RFU_1:
> - *desc_len = 0;
> - break;
> - default:
> + if (desc_id >= QUERY_DESC_IDN_MAX) {
> *desc_len = 0;
> return -EINVAL;
> }
> +
> + *desc_len = hba->desc_size[desc_id];
> return 0;
> }
> EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
>
> +static void ufshcd_update_desc_length(struct ufs_hba *hba,
> + enum desc_idn desc_id, int desc_len)
> +{
> + if (hba->desc_size[desc_id] == QUERY_DESC_MAX_SIZE &&
> + desc_id != QUERY_DESC_IDN_STRING)
> + hba->desc_size[desc_id] = desc_len;
> +}
> +
> /**
> * ufshcd_read_desc_param - read the specified descriptor parameter
> * @hba: Pointer to adapter instance
> @@ -3209,6 +3149,9 @@ int ufshcd_read_desc_param(struct ufs_hba *hba,
> goto out;
> }
>
> + ufshcd_update_desc_length(hba, desc_id,
> + desc_buf[QUERY_DESC_LENGTH_OFFSET]);
> +
> /* Check wherher we will not copy more data, than available */
> if (is_kmalloc && param_size > buff_len)
> param_size = buff_len;
> @@ -6665,7 +6608,7 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba,
> static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba)
> {
> int ret;
> - int buff_len = hba->desc_size.pwr_desc;
> + int buff_len = hba->desc_size[QUERY_DESC_IDN_POWER];
> u8 *desc_buf;
> u32 icc_level;
>
> @@ -6783,7 +6726,8 @@ static void ufshcd_wb_probe(struct ufs_hba *hba, u8 *desc_buf)
> if (!ufshcd_is_wb_allowed(hba))
> return;
>
> - if (hba->desc_size.dev_desc < DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
> + if (hba->desc_size[QUERY_DESC_IDN_DEVICE] <
> + DEVICE_DESC_PARAM_EXT_UFS_FEATURE_SUP + 4)
> goto wb_disabled;
>
> hba->dev_info.d_ext_ufs_feature_sup =
> @@ -6878,7 +6822,7 @@ static int ufs_get_device_desc(struct ufs_hba *hba)
> }
>
> err = ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0, desc_buf,
> - hba->desc_size.dev_desc);
> + hba->desc_size[QUERY_DESC_IDN_DEVICE]);
> if (err) {
> dev_err(hba->dev, "%s: Failed reading Device Desc. err = %d\n",
> __func__, err);
> @@ -7108,42 +7052,10 @@ static void ufshcd_clear_dbg_ufs_stats(struct ufs_hba *hba)
>
> static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
> {
> - int err;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_DEVICE, 0,
> - &hba->desc_size.dev_desc);
> - if (err)
> - hba->desc_size.dev_desc = QUERY_DESC_DEVICE_DEF_SIZE;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_POWER, 0,
> - &hba->desc_size.pwr_desc);
> - if (err)
> - hba->desc_size.pwr_desc = QUERY_DESC_POWER_DEF_SIZE;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_INTERCONNECT, 0,
> - &hba->desc_size.interc_desc);
> - if (err)
> - hba->desc_size.interc_desc = QUERY_DESC_INTERCONNECT_DEF_SIZE;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_CONFIGURATION, 0,
> - &hba->desc_size.conf_desc);
> - if (err)
> - hba->desc_size.conf_desc = QUERY_DESC_CONFIGURATION_DEF_SIZE;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_UNIT, 0,
> - &hba->desc_size.unit_desc);
> - if (err)
> - hba->desc_size.unit_desc = QUERY_DESC_UNIT_DEF_SIZE;
> -
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_GEOMETRY, 0,
> - &hba->desc_size.geom_desc);
> - if (err)
> - hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
> + int i;
>
> - err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_HEALTH, 0,
> - &hba->desc_size.hlth_desc);
> - if (err)
> - hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
> + for (i = 0; i < QUERY_DESC_IDN_MAX; i++)
> + hba->desc_size[i] = QUERY_DESC_MAX_SIZE;
> }
>
> static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
> @@ -7152,7 +7064,7 @@ static int ufshcd_device_geo_params_init(struct ufs_hba *hba)
> size_t buff_len;
> u8 *desc_buf;
>
> - buff_len = hba->desc_size.geom_desc;
> + buff_len = hba->desc_size[QUERY_DESC_IDN_GEOMETRY];
> desc_buf = kmalloc(buff_len, GFP_KERNEL);
> if (!desc_buf) {
> err = -ENOMEM;
> @@ -7253,7 +7165,7 @@ static int ufshcd_device_params_init(struct ufs_hba *hba)
> /* Clear any previous UFS device information */
> memset(&hba->dev_info, 0, sizeof(hba->dev_info));
>
> - /* Init check for device descriptor sizes */
> + /* Init device descriptor sizes */
> ufshcd_init_desc_sizes(hba);
Perhaps just put simple code in ufshcd_init_desc_sizes() here and remove
ufshcd_init_desc_sizes()?
>
> /* Init UFS geometry descriptor related parameters */
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index e3dfb48e669e..b966d9b0eb3d 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -236,16 +236,6 @@ struct ufs_dev_cmd {
> struct ufs_query query;
> };
>
> -struct ufs_desc_size {
> - int dev_desc;
> - int pwr_desc;
> - int geom_desc;
> - int interc_desc;
> - int unit_desc;
> - int conf_desc;
> - int hlth_desc;
> -};
> -
> /**
> * struct ufs_clk_info - UFS clock related info
> * @list: list headed by hba->clk_list_head
> @@ -738,7 +728,7 @@ struct ufs_hba {
> bool is_urgent_bkops_lvl_checked;
>
> struct rw_semaphore clk_scaling_lock;
> - struct ufs_desc_size desc_size;
> + u8 desc_size[QUERY_DESC_IDN_MAX];
> atomic_t scsi_block_reqs_cnt;
>
> struct device bsg_dev;
Except for the above nit, otherwise I really like this patch.
Reviewed-by: Stanley Chu <[email protected]>