[PATCH v2 0/3] scsi: ufs: core: support WB buffer resize function

[PATCH v2 0/3] scsi: ufs: core: support WB buffer resize function

[Lu Hongfei]

Hello,

This v2 series implements the function of controlling the wb buffer resize
via sysfs that will be supported in UFS4.1.

The patch 1 add WB buffer resize related attr_idns.
The patch 2 Add ufshcd_wb_buf_resize function to enable WB buffer resize.
The patch 3 Add sysfs attributes to control WB buffer resize function.

version 2 changes
-Using sysfs to control WB buffer resize instead of exception event handler
-Removed content related to exception event
-Solved several issues that caused compilation errors

Of course, there may be better ways to implement this feature.
If necessary, please point it out and I will optimize it as soon as
possible.

As of now, there have been no UFS device releases that support this
feature, so I have not tested the code on real hardware.

------------------------------------------------------------------------

Lu Hongfei (3):
  scsi: ufs: core: add wb buffer resize related attr_idn
  scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer
    resize
  scsi: ufs: core: Add sysfs attributes to control WB buffer resize
    function

 Documentation/ABI/testing/sysfs-driver-ufs | 52 ++++++++++++++++
 drivers/ufs/core/ufs-sysfs.c               | 71 ++++++++++++++++++++++
 drivers/ufs/core/ufshcd-priv.h             |  1 +
 drivers/ufs/core/ufshcd.c                  | 21 +++++++
 include/ufs/ufs.h                          |  5 +-
 include/ufs/ufshcd.h                       |  1 +
 6 files changed, 150 insertions(+), 1 deletion(-)

-- 
2.39.0

[PATCH v2 1/3] scsi: ufs: core: add wb buffer resize related attr_idn

[Lu Hongfei]

UFS4.1 will support the WB buffer resize function, and UFS driver needs
to add definitions for attr_idn related to this function to support
this feature

Signed-off-by: Lu Hongfei <luhongfei@vivo.com>
---
 include/ufs/ufs.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
index 0cced88f4531..8016bf30c8c4 100644
--- a/include/ufs/ufs.h
+++ b/include/ufs/ufs.h
@@ -179,7 +179,10 @@ enum attr_idn {
 	QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST    = 0x1E,
 	QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE        = 0x1F,
 	QUERY_ATTR_IDN_EXT_IID_EN		= 0x2A,
-	QUERY_ATTR_IDN_TIMESTAMP		= 0x30
+	QUERY_ATTR_IDN_TIMESTAMP		= 0x30,
+	QUERY_ATTR_IDN_WB_BUF_RESIZE_HINT	= 0x3C,
+	QUERY_ATTR_IDN_WB_BUF_RESIZE_EN		= 0x3D,
+	QUERY_ATTR_IDN_WB_BUF_RESIZE_STATUS	= 0x3E
 };
 
 /* Descriptor idn for Query requests */
-- 
2.39.0

[PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize

[Lu Hongfei]

ufshcd_wb_buf_resize is used to enable WB buffer resize. It first blocks
the upper layer from issuing reqs, then waits for the cmd queue to be
empty, and then issues the command to enable WB buffer resize.

It may be called anywhere, such as ufs-sysfs.c, so it needs to be declared
in the header files.

Signed-off-by: Lu Hongfei <luhongfei@vivo.com>
---
 drivers/ufs/core/ufshcd-priv.h |  1 +
 drivers/ufs/core/ufshcd.c      | 21 +++++++++++++++++++++
 include/ufs/ufshcd.h           |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index f42d99ce5bf1..85caefa421f7 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -98,6 +98,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     enum query_opcode desc_op);
 
 int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
+int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op);
 
 /* Wrapper functions for safely calling variant operations */
 static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 93417518c04d..7e4461360cbd 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6045,6 +6045,27 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba)
 	return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf);
 }
 
+int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op)
+{
+	int ret;
+	u8 index;
+
+	ufshcd_scsi_block_requests(hba);
+	if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
+		goto out;
+
+	index = ufshcd_wb_get_query_index(hba);
+	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+		QUERY_ATTR_IDN_WB_BUF_RESIZE_EN, index, 0, &resize_op);
+	if (ret)
+		dev_err(hba->dev,
+			"%s: Enable WB buf resize operation failed %d\n",
+			__func__, ret);
+out:
+	ufshcd_scsi_unblock_requests(hba);
+	return ret;
+}
+
 static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
 {
 	struct ufs_hba *hba = container_of(to_delayed_work(work),
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 7d07b256e906..7dd560dc22c6 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -1381,6 +1381,7 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
 				     struct ufs_ehs *ehs_rsp, int sg_cnt,
 				     struct scatterlist *sg_list, enum dma_data_direction dir);
 int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
+int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op);
 int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
 int ufshcd_suspend_prepare(struct device *dev);
 int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm);
-- 
2.39.0

[PATCH v2 3/3] scsi: ufs: core: Add sysfs attributes to control WB buffer resize function

[Lu Hongfei]

The host can control the WB buffer resize through sysfs. To achieve this
goal, three sysfs nodes have been added:
    1. wb_buf_resize_hint
    2. enable_wb_buf_resize
    3. wb_buf_resize_status

The host can read wb_buf_resize_hint, obtain the hint information about
which type of resize for wb buffer, and write enable_wb_buf_resize to
enable wb buffer resize based on the hint information. Considering that
this process may take a long time, the host can confirm the resize status
by reading wb_buf_resize_status.

The detailed definition of the three nodes can be found in the sysfs
documentation.

Signed-off-by: Lu Hongfei <luhongfei@vivo.com>
---
 Documentation/ABI/testing/sysfs-driver-ufs | 52 ++++++++++++++++
 drivers/ufs/core/ufs-sysfs.c               | 71 ++++++++++++++++++++++
 2 files changed, 123 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 0c7efaf62de0..6e0ecf3a025d 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -1437,6 +1437,58 @@ Description:
 		If avail_wb_buff < wb_flush_threshold, it indicates that WriteBooster buffer needs to
 		be flushed, otherwise it is not necessary.
 
+What:		/sys/bus/platform/drivers/ufshcd/*/wb_buf_resize_hint
+What:		/sys/bus/platform/devices/*.ufs/wb_buf_resize_hint
+Date:		Sept 2023
+Contact:	Lu Hongfei <luhongfei@vivo.com>
+Description:
+		wb_buf_resize_hint indicates hint information about which type of resize for
+		WriteBooster Buffer is recommended by the device.
+
+		======  ======================================
+		   00h  Recommend keep the buffer size
+		   01h  Recommend to decrease the buffer size
+		   02h  Recommend to increase the buffer size
+		Others: Reserved
+		======  ======================================
+
+		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/enable_wb_buf_resize
+What:		/sys/bus/platform/devices/*.ufs/enable_wb_buf_resize
+Date:		Sept 2023
+Contact:	Lu Hongfei <luhongfei@vivo.com>
+Description:
+		The host can decrease or increase the WriteBooster Buffer size by setting
+		this file.
+
+		======  ======================================
+		   00h  Idle (There is no resize operation)
+		   01h  Decrease WriteBooster Buffer Size
+		   02h  Increase WriteBooster Buffer Size
+		Others  Reserved
+		======  ======================================
+
+		The file is write only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/wb_buf_resize_status
+What:		/sys/bus/platform/devices/*.ufs/wb_buf_resize_status
+Date:		Sept 2023
+Contact:	Lu Hongfei <luhongfei@vivo.com>
+Description:
+		The host can check the Resize operation status of the WriteBooster Buffer
+		by reading this file.
+
+		======  ========================================
+		   00h  Idle (resize operation is not issued)
+		   01h  Resize operation in progress
+		   02h  Resize operation completed successfully
+		   03h  Resize operation general failure
+		Others  Reserved
+		======  ========================================
+
+		The file is read only.
+
 Contact:	Daniil Lunev <dlunev@chromium.org>
 What:		/sys/bus/platform/drivers/ufshcd/*/capabilities/
 What:		/sys/bus/platform/devices/*.ufs/capabilities/
diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c
index c95906443d5f..2ecb1e08a5b8 100644
--- a/drivers/ufs/core/ufs-sysfs.c
+++ b/drivers/ufs/core/ufs-sysfs.c
@@ -329,6 +329,71 @@ static ssize_t wb_flush_threshold_store(struct device *dev,
 	return count;
 }
 
+static ssize_t wb_buf_resize_hint_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	u32 value;
+	u8 index = ufshcd_wb_get_query_index(hba);
+
+	if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_WB_BUF_RESIZE_HINT, index, 0, &value)) {
+		dev_err(hba->dev, "Read WB Buffer Resize Hint info failed\n");
+		return  -EINVAL;
+	}
+
+	return sysfs_emit(buf, "%u\n", value);
+}
+
+static ssize_t enable_wb_buf_resize_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf, size_t count)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	unsigned int wb_buf_resize_op;
+
+	if (!ufshcd_is_wb_allowed(hba) || !hba->dev_info.wb_enabled) {
+		dev_err(dev, "The WB is not allowed or disabled!\n");
+		return -EINVAL;
+	}
+
+	if (hba->dev_info.wspecversion < 0x410 ||
+	    !hba->dev_info.b_presrv_uspc_en) {
+		dev_err(dev, "The WB buf resize is not allowed!\n");
+		return -EINVAL;
+	}
+
+	if (kstrtouint(buf, 0, &wb_buf_resize_op))
+		return -EINVAL;
+
+	if (wb_buf_resize_op != 0x01 && wb_buf_resize_op != 0x02) {
+		dev_err(dev, "The operation %u is invalid!\n", wb_buf_resize_op);
+		return -EINVAL;
+	}
+
+	ufshcd_wb_buf_resize(hba, wb_buf_resize_op);
+
+	return count;
+}
+
+static ssize_t wb_buf_resize_status_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	u32 value;
+	u8 index = ufshcd_wb_get_query_index(hba);
+
+	if (ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_WB_BUF_RESIZE_STATUS, index, 0, &value)) {
+		dev_err(hba->dev, "Read WB Buffer Resize Status info failed\n");
+		return -EINVAL;
+	}
+
+	return sysfs_emit(buf, "%u\n", value);
+}
+
 static DEVICE_ATTR_RW(rpm_lvl);
 static DEVICE_ATTR_RO(rpm_target_dev_state);
 static DEVICE_ATTR_RO(rpm_target_link_state);
@@ -339,6 +404,9 @@ static DEVICE_ATTR_RW(auto_hibern8);
 static DEVICE_ATTR_RW(wb_on);
 static DEVICE_ATTR_RW(enable_wb_buf_flush);
 static DEVICE_ATTR_RW(wb_flush_threshold);
+static DEVICE_ATTR_RO(wb_buf_resize_hint);
+static DEVICE_ATTR_WO(enable_wb_buf_resize);
+static DEVICE_ATTR_RO(wb_buf_resize_status);
 
 static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
 	&dev_attr_rpm_lvl.attr,
@@ -351,6 +419,9 @@ static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
 	&dev_attr_wb_on.attr,
 	&dev_attr_enable_wb_buf_flush.attr,
 	&dev_attr_wb_flush_threshold.attr,
+	&dev_attr_wb_buf_resize_hint.attr,
+	&dev_attr_enable_wb_buf_resize.attr,
+	&dev_attr_wb_buf_resize_status.attr,
 	NULL
 };
 
-- 
2.39.0

Re: [PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize

[kernel test robot]

Hi Lu,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v6.5 next-20230908]
[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/Lu-Hongfei/scsi-ufs-core-add-wb-buffer-resize-related-attr_idn/20230908-182656
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20230908102113.547-3-luhongfei%40vivo.com
patch subject: [PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize
config: s390-randconfig-001-20230908 (https://download.01.org/0day-ci/archive/20230908/202309082109.DPcrgu3e-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230908/202309082109.DPcrgu3e-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309082109.DPcrgu3e-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/ufs/core/ufshcd.c:25:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     547 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     560 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
         |                                                           ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
     102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
         |                                                      ^
   In file included from drivers/ufs/core/ufshcd.c:25:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     573 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
         |                                                           ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
     115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
         |                                                      ^
   In file included from drivers/ufs/core/ufshcd.c:25:
   In file included from include/linux/iopoll.h:14:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     584 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     594 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     604 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     692 |         readsb(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     700 |         readsw(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     708 |         readsl(PCI_IOBASE + addr, buffer, count);
         |                ~~~~~~~~~~ ^
   include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     717 |         writesb(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     726 |         writesw(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
   include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     735 |         writesl(PCI_IOBASE + addr, buffer, count);
         |                 ~~~~~~~~~~ ^
>> drivers/ufs/core/ufshcd.c:6055:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    6055 |         if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd.c:6067:9: note: uninitialized use occurs here
    6067 |         return ret;
         |                ^~~
   drivers/ufs/core/ufshcd.c:6055:2: note: remove the 'if' if its condition is always false
    6055 |         if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    6056 |                 goto out;
         | ~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/ufs/core/ufshcd.c:6051:9: note: initialize the variable 'ret' to silence this warning
    6051 |         int ret;
         |                ^
         |                 = 0
   13 warnings generated.


vim +6055 drivers/ufs/core/ufshcd.c

  6048	
  6049	int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op)
  6050	{
  6051		int ret;
  6052		u8 index;
  6053	
  6054		ufshcd_scsi_block_requests(hba);
> 6055		if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
  6056			goto out;
  6057	
  6058		index = ufshcd_wb_get_query_index(hba);
  6059		ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
  6060			QUERY_ATTR_IDN_WB_BUF_RESIZE_EN, index, 0, &resize_op);
  6061		if (ret)
  6062			dev_err(hba->dev,
  6063				"%s: Enable WB buf resize operation failed %d\n",
  6064				__func__, ret);
  6065	out:
  6066		ufshcd_scsi_unblock_requests(hba);
  6067		return ret;
  6068	}
  6069	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/3] scsi: ufs: core: add wb buffer resize related attr_idn

[Bart Van Assche]

On 9/8/23 03:20, Lu Hongfei wrote:
> UFS4.1 will support the WB buffer resize function, and UFS driver needs
> to add definitions for attr_idn related to this function to support
> this feature

needs to -> can

Please also mention that the ballot for resizing the WriteBooster buffer 
has been approved.

> diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
> index 0cced88f4531..8016bf30c8c4 100644
> --- a/include/ufs/ufs.h
> +++ b/include/ufs/ufs.h
> @@ -179,7 +179,10 @@ enum attr_idn {
>   	QUERY_ATTR_IDN_WB_BUFF_LIFE_TIME_EST    = 0x1E,
>   	QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE        = 0x1F,
>   	QUERY_ATTR_IDN_EXT_IID_EN		= 0x2A,
> -	QUERY_ATTR_IDN_TIMESTAMP		= 0x30
> +	QUERY_ATTR_IDN_TIMESTAMP		= 0x30,
> +	QUERY_ATTR_IDN_WB_BUF_RESIZE_HINT	= 0x3C,
> +	QUERY_ATTR_IDN_WB_BUF_RESIZE_EN		= 0x3D,
> +	QUERY_ATTR_IDN_WB_BUF_RESIZE_STATUS	= 0x3E

Please add a trailing comma after "0x3E".

Thanks,

Bart.

Re: [PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize

[Bart Van Assche]

On 9/8/23 03:20, Lu Hongfei wrote:
> diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
> index f42d99ce5bf1..85caefa421f7 100644
> --- a/drivers/ufs/core/ufshcd-priv.h
> +++ b/drivers/ufs/core/ufshcd-priv.h
> @@ -98,6 +98,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
>   			     enum query_opcode desc_op);
>   
>   int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
> +int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op);
>   
>   /* Wrapper functions for safely calling variant operations */
>   static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 93417518c04d..7e4461360cbd 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6045,6 +6045,27 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba)
>   	return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf);
>   }
>   
> +int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op)
> +{
> +	int ret;
> +	u8 index;
> +
> +	ufshcd_scsi_block_requests(hba);
> +	if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
> +		goto out;
> +
> +	index = ufshcd_wb_get_query_index(hba);
> +	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +		QUERY_ATTR_IDN_WB_BUF_RESIZE_EN, index, 0, &resize_op);
> +	if (ret)
> +		dev_err(hba->dev,
> +			"%s: Enable WB buf resize operation failed %d\n",
> +			__func__, ret);
> +out:
> +	ufshcd_scsi_unblock_requests(hba);
> +	return ret;
> +}
> +
>   static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
>   {
>   	struct ufs_hba *hba = container_of(to_delayed_work(work),
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index 7d07b256e906..7dd560dc22c6 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1381,6 +1381,7 @@ int ufshcd_advanced_rpmb_req_handler(struct ufs_hba *hba, struct utp_upiu_req *r
>   				     struct ufs_ehs *ehs_rsp, int sg_cnt,
>   				     struct scatterlist *sg_list, enum dma_data_direction dir);
>   int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
> +int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op);
>   int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable);
>   int ufshcd_suspend_prepare(struct device *dev);
>   int __ufshcd_suspend_prepare(struct device *dev, bool rpm_ok_for_spm);

My feedback about this patch is as follows:
- If a new function (ufshcd_wb_buf_resize()) is introduced, a caller for 
that function should be introduced in the same patch.
- Function declarations should be added either in the private header 
file or in the public header file (include/ufs/ufshcd.h) but not in both.
- The name of the ufshcd_wb_buf_resize() function seems misleading to 
me. To me that name suggests that this functions resizes the 
WriteBooster buffer. That's not what that function does - what it does 
it to configure whether or not the UFS device is allowed to resize the 
WB buffer.
- Please remove the ufshcd_scsi_block_requests(), 
ufshcd_wait_for_doorbell_clr() and ufshcd_scsi_unblock_requests() calls. 
I'm not aware of any requirement to pause SCSI command submission while 
changing whether or not WB buffer resizing is enabled or disabled.

Thanks,

Bart.

Re: [PATCH v2 3/3] scsi: ufs: core: Add sysfs attributes to control WB buffer resize function

[Bart Van Assche]

> +What:		/sys/bus/platform/drivers/ufshcd/*/enable_wb_buf_resize
> +What:		/sys/bus/platform/devices/*.ufs/enable_wb_buf_resize
> +Date:		Sept 2023
> +Contact:	Lu Hongfei <luhongfei@vivo.com>
> +Description:
> +		The host can decrease or increase the WriteBooster Buffer size by setting
> +		this file.
> +
> +		======  ======================================
> +		   00h  Idle (There is no resize operation)
> +		   01h  Decrease WriteBooster Buffer Size
> +		   02h  Increase WriteBooster Buffer Size
> +		Others  Reserved
> +		======  ======================================
> +
> +		The file is write only.

The name "enable_wb_buf_resize" seems misleading to me. 
"wb_buf_resize_control" is probably a better name for this attribute 
since this attribute can be used to increase and decrease the WB buffer 
size.

> +	if (hba->dev_info.wspecversion < 0x410 ||
> +	    !hba->dev_info.b_presrv_uspc_en) {
> +		dev_err(dev, "The WB buf resize is not allowed!\n");
> +		return -EINVAL;
> +	}

Please leave out the version number check. There probably will be UFS 
4.0 devices that will implement this feature.

Thanks,

Bart.

Re: [PATCH v2 0/3] scsi: ufs: core: support WB buffer resize function

[Can Guo]

Hi Hongfei,

On 9/8/2023 6:20 PM, Lu Hongfei wrote:
> Hello,
>
> This v2 series implements the function of controlling the wb buffer resize
> via sysfs that will be supported in UFS4.1.
>
>
UFS4.1 JEDEC standard is targeting Q3 2024, before it is finalized, 
please don't put up changes for new features (which are still in draft) 
on upstream.


Thanks,

Can Guo.

Re: [PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize

[Dan Carpenter]

Hi Lu,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lu-Hongfei/scsi-ufs-core-add-wb-buffer-resize-related-attr_idn/20230908-182656
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
patch link:    https://lore.kernel.org/r/20230908102113.547-3-luhongfei%40vivo.com
patch subject: [PATCH v2 2/3] scsi: ufs: core: Add ufshcd_wb_buf_resize function to enable WB buffer resize
config: i386-randconfig-141-20230909 (https://download.01.org/0day-ci/archive/20230909/202309091536.TRk3mftu-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230909/202309091536.TRk3mftu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202309091536.TRk3mftu-lkp@intel.com/

New smatch warnings:
drivers/ufs/core/ufshcd.c:6067 ufshcd_wb_buf_resize() error: uninitialized symbol 'ret'.

Old smatch warnings:
drivers/ufs/core/ufshcd.c:5353 ufshcd_uic_cmd_compl() error: we previously assumed 'hba->active_uic_cmd' could be null (see line 5341)

vim +/ret +6067 drivers/ufs/core/ufshcd.c

7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6049  int ufshcd_wb_buf_resize(struct ufs_hba *hba, u32 resize_op)
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6050  {
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6051  	int ret;
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6052  	u8 index;
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6053  
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6054  	ufshcd_scsi_block_requests(hba);
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6055  	if (ufshcd_wait_for_doorbell_clr(hba, 1 * USEC_PER_SEC))
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6056  		goto out;

ret is unitialized.

7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6057  
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6058  	index = ufshcd_wb_get_query_index(hba);
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6059  	ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6060  		QUERY_ATTR_IDN_WB_BUF_RESIZE_EN, index, 0, &resize_op);
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6061  	if (ret)
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6062  		dev_err(hba->dev,
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6063  			"%s: Enable WB buf resize operation failed %d\n",
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6064  			__func__, ret);
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6065  out:
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6066  	ufshcd_scsi_unblock_requests(hba);
7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08 @6067  	return ret;
                                                                               ^^^

7b70b34c503067 drivers/ufs/core/ufshcd.c Lu Hongfei  2023-09-08  6068  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki