diff mbox series

[3/3] scsi: ufs: use UFS device indicated maximum LU number

Message ID 20200110183606.10102-4-huobean@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series use UFS device indicated maximum LU number | expand

Commit Message

Bean Huo Jan. 10, 2020, 6:36 p.m. UTC 
From: Bean Huo <beanhuo@micron.com>

According to Jedec standard UFS 3.0 and UFS 2.1 Spec, Maximum number of logical
units supported by the UFS device is indicated by parameter bMaxNumberLU in
Geometry Descriptor. This patch is to delete current hard code macro definition
of UFS_UPIU_MAX_GENERAL_LUN, and switch to use device indicated number instead.

Signed-off-by: Bean Huo <beanhuo@micron.com>
---
 drivers/scsi/ufs/ufs-sysfs.c |  2 +-
 drivers/scsi/ufs/ufs.h       | 12 +++++++++---
 drivers/scsi/ufs/ufshcd.c    |  4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

Comments

Bart Van Assche Jan. 11, 2020, 10:50 p.m. UTC | #1 
On 2020-01-10 10:36, Bean Huo wrote:
> @@ -548,12 +547,19 @@ struct ufs_dev_desc {
>  
>  /**
>   * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
> + * @dev_info: pointer of instance of struct ufs_dev_info
>   * @lun: LU number to check
>   * @return: true if the lun has a matching unit descriptor, false otherwise
>   */
> -static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
> +static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info,
> +		u8 lun)
>  {

Can the dev_info be declared 'const' (const truct ufs_dev_info *dev_info)?

> -	return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
> +	if (!dev_info || !dev_info->max_lu_supported) {
> +		pr_err("Max General LU supported by UFS isn't initilized\n");
                                                              ^^^^^^^^^^
                                                            initialized?
> +		return false;
> +	}
> +
> +	return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info->max_lu_supported);
>  }

Are the parentheses in the above expression necessary?

Thanks,

Bart.
Asutosh Das (asd) Jan. 13, 2020, 6:45 p.m. UTC | #2 
On 2020-01-10 10:36, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
> 
> According to Jedec standard UFS 3.0 and UFS 2.1 Spec, Maximum number of 
> logical
> units supported by the UFS device is indicated by parameter 
> bMaxNumberLU in
> Geometry Descriptor. This patch is to delete current hard code macro 
> definition
> of UFS_UPIU_MAX_GENERAL_LUN, and switch to use device indicated number 
> instead.
> 
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
>  drivers/scsi/ufs/ufs-sysfs.c |  2 +-
>  drivers/scsi/ufs/ufs.h       | 12 +++++++++---
>  drivers/scsi/ufs/ufshcd.c    |  4 ++--
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c 
> b/drivers/scsi/ufs/ufs-sysfs.c
> index 720be3f64be7..dbdf8b01abed 100644
> --- a/drivers/scsi/ufs/ufs-sysfs.c
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -713,7 +713,7 @@ static ssize_t _pname##_show(struct device 
> *dev,			\
>  	struct scsi_device *sdev = to_scsi_device(dev);			\
>  	struct ufs_hba *hba = shost_priv(sdev->host);			\
>  	u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun);			\
> -	if (!ufs_is_valid_unit_desc_lun(lun))				\
> +	if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))		\
>  		return -EINVAL;						\
>  	return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname,	\
>  		lun, _duname##_DESC_PARAM##_puname, buf, _size);	\
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 5ca7ea4f223e..810eeca0de63 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -63,7 +63,6 @@
>  #define UFS_UPIU_MAX_UNIT_NUM_ID	0x7F
>  #define UFS_MAX_LUNS		(SCSI_W_LUN_BASE + UFS_UPIU_MAX_UNIT_NUM_ID)
>  #define UFS_UPIU_WLUN_ID	(1 << 7)
> -#define UFS_UPIU_MAX_GENERAL_LUN	8
> 
>  /* Well known logical unit id in LUN field of UPIU */
>  enum {
> @@ -548,12 +547,19 @@ struct ufs_dev_desc {
> 
>  /**
>   * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit 
> descriptor
> + * @dev_info: pointer of instance of struct ufs_dev_info
>   * @lun: LU number to check
>   * @return: true if the lun has a matching unit descriptor, false 
> otherwise
>   */
> -static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
> +static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info 
> *dev_info,
> +		u8 lun)
>  {
> -	return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
> +	if (!dev_info || !dev_info->max_lu_supported) {
> +		pr_err("Max General LU supported by UFS isn't initilized\n");
> +		return false;
> +	}
> +
> +	return lun == UFS_UPIU_RPMB_WLUN || (lun < 
> dev_info->max_lu_supported);
>  }
> 
>  #endif /* End of Header */
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a297fe55e36a..c6ea5d88222d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3286,7 +3286,7 @@ static inline int
> ufshcd_read_unit_desc_param(struct ufs_hba *hba,
>  	 * Unit descriptors are only available for general purpose LUs (LUN 
> id
>  	 * from 0 to 7) and RPMB Well known LU.
>  	 */
> -	if (!ufs_is_valid_unit_desc_lun(lun))
> +	if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))
>  		return -EOPNOTSUPP;
> 
>  	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
> @@ -4540,7 +4540,7 @@ static int ufshcd_get_lu_wp(struct ufs_hba *hba,
>  	 * protected so skip reading bLUWriteProtect parameter for
>  	 * it. For other W-LUs, UNIT DESCRIPTOR is not available.
>  	 */
> -	else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
> +	else if (lun >= hba->dev_info.max_lu_supported)
>  		ret = -ENOTSUPP;
>  	else
>  		ret = ufshcd_read_unit_desc_param(hba,

Looks good to me.
Reviewed-by: Asutosh Das <asutoshd@codeaurora.org>
diff mbox series

Patch

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 720be3f64be7..dbdf8b01abed 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -713,7 +713,7 @@  static ssize_t _pname##_show(struct device *dev,			\
 	struct scsi_device *sdev = to_scsi_device(dev);			\
 	struct ufs_hba *hba = shost_priv(sdev->host);			\
 	u8 lun = ufshcd_scsi_to_upiu_lun(sdev->lun);			\
-	if (!ufs_is_valid_unit_desc_lun(lun))				\
+	if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))		\
 		return -EINVAL;						\
 	return ufs_sysfs_read_desc_param(hba, QUERY_DESC_IDN_##_duname,	\
 		lun, _duname##_DESC_PARAM##_puname, buf, _size);	\
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 5ca7ea4f223e..810eeca0de63 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -63,7 +63,6 @@ 
 #define UFS_UPIU_MAX_UNIT_NUM_ID	0x7F
 #define UFS_MAX_LUNS		(SCSI_W_LUN_BASE + UFS_UPIU_MAX_UNIT_NUM_ID)
 #define UFS_UPIU_WLUN_ID	(1 << 7)
-#define UFS_UPIU_MAX_GENERAL_LUN	8
 
 /* Well known logical unit id in LUN field of UPIU */
 enum {
@@ -548,12 +547,19 @@  struct ufs_dev_desc {
 
 /**
  * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @dev_info: pointer of instance of struct ufs_dev_info
  * @lun: LU number to check
  * @return: true if the lun has a matching unit descriptor, false otherwise
  */
-static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+static inline bool ufs_is_valid_unit_desc_lun(struct ufs_dev_info *dev_info,
+		u8 lun)
 {
-	return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN);
+	if (!dev_info || !dev_info->max_lu_supported) {
+		pr_err("Max General LU supported by UFS isn't initilized\n");
+		return false;
+	}
+
+	return lun == UFS_UPIU_RPMB_WLUN || (lun < dev_info->max_lu_supported);
 }
 
 #endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a297fe55e36a..c6ea5d88222d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3286,7 +3286,7 @@  static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
 	 * Unit descriptors are only available for general purpose LUs (LUN id
 	 * from 0 to 7) and RPMB Well known LU.
 	 */
-	if (!ufs_is_valid_unit_desc_lun(lun))
+	if (!ufs_is_valid_unit_desc_lun(&hba->dev_info, lun))
 		return -EOPNOTSUPP;
 
 	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -4540,7 +4540,7 @@  static int ufshcd_get_lu_wp(struct ufs_hba *hba,
 	 * protected so skip reading bLUWriteProtect parameter for
 	 * it. For other W-LUs, UNIT DESCRIPTOR is not available.
 	 */
-	else if (lun >= UFS_UPIU_MAX_GENERAL_LUN)
+	else if (lun >= hba->dev_info.max_lu_supported)
 		ret = -ENOTSUPP;
 	else
 		ret = ufshcd_read_unit_desc_param(hba,