diff mbox series

[4/4] ufs: core: Remove ufshcd_map_desc_id_to_length function

Message ID 1667233524-13715-5-git-send-email-Arthur.Simchaev@wdc.com
State Superseded
Headers show
Series ufs: core: Always read the descriptors with max length | expand

Commit Message

Arthur Simchaev Oct. 31, 2022, 4:25 p.m. UTC
ufshcd_map_desc_id_to_length function is redundant.
According to the spec the host can send any descriptor size, the device
will return the actual size. Therefore always read the descriptors with
QUERY_DESC_MAX_SIZE size

Signed-off-by: Arthur Simchaev <Arthur.Simchaev@wdc.com>
---
 drivers/ufs/core/ufs_bsg.c     |  7 +------
 drivers/ufs/core/ufshcd-priv.h |  3 ---
 drivers/ufs/core/ufshcd.c      | 46 +++++++++++-------------------------------
 drivers/ufs/core/ufshpb.c      |  5 +----
 4 files changed, 14 insertions(+), 47 deletions(-)

Comments

kernel test robot Nov. 1, 2022, 1:51 a.m. UTC | #1
Hi Arthur,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20221031]
[cannot apply to jejb-scsi/for-next linus/master v6.1-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/1667233524-13715-5-git-send-email-Arthur.Simchaev%40wdc.com
patch subject: [PATCH 4/4] ufs: core: Remove ufshcd_map_desc_id_to_length function
config: loongarch-allyesconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
        git checkout 5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/ufs/ufshcd.h:22,
                    from drivers/ufs/core/ufshcd-priv.h:7,
                    from drivers/ufs/core/ufshpb.c:18:
   drivers/ufs/core/ufshpb.c: In function 'ufshpb_get_lu_info':
>> include/ufs/ufs.h:19:35: warning: passing argument 7 of 'ufshcd_query_descriptor_retry' makes pointer from integer without a cast [-Wint-conversion]
      19 | #define QUERY_DESC_MAX_SIZE       255
         |                                   ^~~
         |                                   |
         |                                   int
   drivers/ufs/core/ufshpb.c:2391:55: note: in expansion of macro 'QUERY_DESC_MAX_SIZE'
    2391 |                                             desc_buf, QUERY_DESC_MAX_SIZE);
         |                                                       ^~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd-priv.h:49:54: note: expected 'int *' but argument is of type 'int'
      49 |                                   u8 *desc_buf, int *buf_len);
         |                                                 ~~~~~^~~~~~~


vim +/ufshcd_query_descriptor_retry +19 include/ufs/ufs.h

5a0b0cb9bee767 drivers/scsi/ufs/ufs.h Sujit Reddy Thumma 2013-07-30  17  
220d17a69de432 drivers/scsi/ufs/ufs.h Avri Altman        2018-10-07  18  #define GENERAL_UPIU_REQUEST_SIZE (sizeof(struct utp_upiu_req))
d44a5f98bb49b2 drivers/scsi/ufs/ufs.h Dolev Raviv        2014-06-29 @19  #define QUERY_DESC_MAX_SIZE       255
d44a5f98bb49b2 drivers/scsi/ufs/ufs.h Dolev Raviv        2014-06-29  20  #define QUERY_DESC_MIN_SIZE       2
b573d484e4ff33 drivers/scsi/ufs/ufs.h Yaniv Gardi        2016-03-10  21  #define QUERY_DESC_HDR_SIZE       2
68078d5cc1a59b drivers/scsi/ufs/ufs.h Dolev Raviv        2013-07-30  22  #define QUERY_OSF_SIZE            (GENERAL_UPIU_REQUEST_SIZE - \
68078d5cc1a59b drivers/scsi/ufs/ufs.h Dolev Raviv        2013-07-30  23  					(sizeof(struct utp_upiu_header)))
09a5a24ff36f90 drivers/scsi/ufs/ufs.h Avri Altman        2018-11-22  24  #define UFS_SENSE_SIZE	18
7a3e97b0dc4bba drivers/scsi/ufs/ufs.h Santosh Yaraganavi 2012-02-29  25
kernel test robot Nov. 1, 2022, 8:45 a.m. UTC | #2
Hi Arthur,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20221101]
[cannot apply to jejb-scsi/for-next linus/master v6.1-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/1667233524-13715-5-git-send-email-Arthur.Simchaev%40wdc.com
patch subject: [PATCH 4/4] ufs: core: Remove ufshcd_map_desc_id_to_length function
config: loongarch-randconfig-s051-20221031
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Arthur-Simchaev/ufs-core-Always-read-the-descriptors-with-max-length/20221101-002915
        git checkout 5a36737b2cacf2691afb72cdff7ea06d2de0eaa9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/ufs/core/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/ufs/core/ufshpb.c:2391:55: sparse: sparse: incorrect type in argument 7 (different base types) @@     expected int *buf_len @@     got int @@
   drivers/ufs/core/ufshpb.c:2391:55: sparse:     expected int *buf_len
   drivers/ufs/core/ufshpb.c:2391:55: sparse:     got int

vim +2391 drivers/ufs/core/ufshpb.c

  2379	
  2380	static int ufshpb_get_lu_info(struct ufs_hba *hba, int lun,
  2381				      struct ufshpb_lu_info *hpb_lu_info)
  2382	{
  2383		u16 max_active_rgns;
  2384		u8 lu_enable;
  2385		int ret;
  2386		char desc_buf[QUERY_DESC_MAX_SIZE];
  2387	
  2388		ufshcd_rpm_get_sync(hba);
  2389		ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
  2390						    QUERY_DESC_IDN_UNIT, lun, 0,
> 2391						    desc_buf, QUERY_DESC_MAX_SIZE);
  2392		ufshcd_rpm_put_sync(hba);
  2393	
  2394		if (ret) {
  2395			dev_err(hba->dev,
  2396				"%s: idn: %d lun: %d  query request failed",
  2397				__func__, QUERY_DESC_IDN_UNIT, lun);
  2398			return ret;
  2399		}
  2400	
  2401		lu_enable = desc_buf[UNIT_DESC_PARAM_LU_ENABLE];
  2402		if (lu_enable != LU_ENABLED_HPB_FUNC)
  2403			return -ENODEV;
  2404	
  2405		max_active_rgns = get_unaligned_be16(
  2406				desc_buf + UNIT_DESC_PARAM_HPB_LU_MAX_ACTIVE_RGNS);
  2407		if (!max_active_rgns) {
  2408			dev_err(hba->dev,
  2409				"lun %d wrong number of max active regions\n", lun);
  2410			return -ENODEV;
  2411		}
  2412	
  2413		hpb_lu_info->num_blocks = get_unaligned_be64(
  2414				desc_buf + UNIT_DESC_PARAM_LOGICAL_BLK_COUNT);
  2415		hpb_lu_info->pinned_start = get_unaligned_be16(
  2416				desc_buf + UNIT_DESC_PARAM_HPB_PIN_RGN_START_OFF);
  2417		hpb_lu_info->num_pinned = get_unaligned_be16(
  2418				desc_buf + UNIT_DESC_PARAM_HPB_NUM_PIN_RGNS);
  2419		hpb_lu_info->max_active_rgns = max_active_rgns;
  2420	
  2421		return 0;
  2422	}
  2423
diff mbox series

Patch

diff --git a/drivers/ufs/core/ufs_bsg.c b/drivers/ufs/core/ufs_bsg.c
index b99e3f3..dc441ac 100644
--- a/drivers/ufs/core/ufs_bsg.c
+++ b/drivers/ufs/core/ufs_bsg.c
@@ -16,16 +16,11 @@  static int ufs_bsg_get_query_desc_size(struct ufs_hba *hba, int *desc_len,
 				       struct utp_upiu_query *qr)
 {
 	int desc_size = be16_to_cpu(qr->length);
-	int desc_id = qr->idn;
 
 	if (desc_size <= 0)
 		return -EINVAL;
 
-	ufshcd_map_desc_id_to_length(hba, desc_id, desc_len);
-	if (!*desc_len)
-		return -EINVAL;
-
-	*desc_len = min_t(int, *desc_len, desc_size);
+	*desc_len = min_t(int, QUERY_DESC_MAX_SIZE, desc_size);
 
 	return 0;
 }
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index a9e8e1f..c52e2f3 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -70,9 +70,6 @@  int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index,
 int ufshcd_hold(struct ufs_hba *hba, bool async);
 void ufshcd_release(struct ufs_hba *hba);
 
-void ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
-				  int *desc_length);
-
 int ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd);
 
 int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9cc3abd..fc0695f 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -3369,20 +3369,6 @@  int ufshcd_query_descriptor_retry(struct ufs_hba *hba,
 }
 
 /**
- * ufshcd_map_desc_id_to_length - map descriptor IDN to its length
- * @hba: Pointer to adapter instance
- * @desc_id: descriptor idn value
- * @desc_len: mapped desc length (out)
- */
-void ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
-				  int *desc_len)
-{
-	/*Always returns QUERY_DESC_MAX_SIZE*/
-	*desc_len = QUERY_DESC_MAX_SIZE;
-}
-EXPORT_SYMBOL(ufshcd_map_desc_id_to_length);
-
-/**
  * ufshcd_read_desc_param - read the specified descriptor parameter
  * @hba: Pointer to adapter instance
  * @desc_id: descriptor idn value
@@ -3402,26 +3388,13 @@  int ufshcd_read_desc_param(struct ufs_hba *hba,
 {
 	int ret;
 	u8 *desc_buf;
-	int buff_len;
+	int buff_len = QUERY_DESC_MAX_SIZE;
 	bool is_kmalloc = true;
 
 	/* Safety check */
 	if (desc_id >= QUERY_DESC_IDN_MAX || !param_size)
 		return -EINVAL;
 
-	/* Get the length of descriptor */
-	ufshcd_map_desc_id_to_length(hba, desc_id, &buff_len);
-	if (!buff_len) {
-		dev_err(hba->dev, "%s: Failed to get desc length\n", __func__);
-		return -EINVAL;
-	}
-
-	if (param_offset >= buff_len) {
-		dev_err(hba->dev, "%s: Invalid offset 0x%x in descriptor IDN 0x%x, length 0x%x\n",
-			__func__, param_offset, desc_id, buff_len);
-		return -EINVAL;
-	}
-
 	/* Check whether we need temp memory */
 	if (param_offset != 0 || param_size < buff_len) {
 		desc_buf = kzalloc(buff_len, GFP_KERNEL);
@@ -3434,15 +3407,23 @@  int ufshcd_read_desc_param(struct ufs_hba *hba,
 
 	/* Request for full descriptor */
 	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
-					desc_id, desc_index, 0,
-					desc_buf, &buff_len);
-
+					    desc_id, desc_index, 0,
+					    desc_buf, &buff_len);
 	if (ret) {
 		dev_err(hba->dev, "%s: Failed reading descriptor. desc_id %d, desc_index %d, param_offset %d, ret %d\n",
 			__func__, desc_id, desc_index, param_offset, ret);
 		goto out;
 	}
 
+	/* Update descriptor length */
+	buff_len = desc_buf[QUERY_DESC_LENGTH_OFFSET];
+
+	if (param_offset >= buff_len) {
+		dev_err(hba->dev, "%s: Invalid offset 0x%x in descriptor IDN 0x%x, length 0x%x\n",
+			__func__, param_offset, desc_id, buff_len);
+		return -EINVAL;
+	}
+
 	/* Sanity check */
 	if (desc_buf[QUERY_DESC_DESC_TYPE_OFFSET] != desc_id) {
 		dev_err(hba->dev, "%s: invalid desc_id %d in descriptor header\n",
@@ -3451,9 +3432,6 @@  int ufshcd_read_desc_param(struct ufs_hba *hba,
 		goto out;
 	}
 
-	/* Update descriptor length */
-	buff_len = desc_buf[QUERY_DESC_LENGTH_OFFSET];
-
 	if (is_kmalloc) {
 		/* Make sure we don't copy more data than available */
 		if (param_offset >= buff_len)
diff --git a/drivers/ufs/core/ufshpb.c b/drivers/ufs/core/ufshpb.c
index 3d69a81c..053fd66 100644
--- a/drivers/ufs/core/ufshpb.c
+++ b/drivers/ufs/core/ufshpb.c
@@ -2382,16 +2382,13 @@  static int ufshpb_get_lu_info(struct ufs_hba *hba, int lun,
 {
 	u16 max_active_rgns;
 	u8 lu_enable;
-	int size;
 	int ret;
 	char desc_buf[QUERY_DESC_MAX_SIZE];
 
-	ufshcd_map_desc_id_to_length(hba, QUERY_DESC_IDN_UNIT, &size);
-
 	ufshcd_rpm_get_sync(hba);
 	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
 					    QUERY_DESC_IDN_UNIT, lun, 0,
-					    desc_buf, &size);
+					    desc_buf, QUERY_DESC_MAX_SIZE);
 	ufshcd_rpm_put_sync(hba);
 
 	if (ret) {