[PATCH v5 0/3] Add helper macro DEFINE_SHOW_STORE_ATTRIBUTE at seq_file.c

[PATCH v5 0/3] Add helper macro DEFINE_SHOW_STORE_ATTRIBUTE at seq_file.c

[Xingui Yang]

We already own DEFINE_SHOW_ATTRIBUTE() helper macro for defining attribute
for read-only file, but we found many of drivers also want a helper macro
for read-write file too.

So we add this helper macro to reduce duplicate code.

Changes from v4:
- Reduce the scope to scsi subsystem based on Andy's suggestion.
- Remove unused macros in qla_dfs.c
- Adjust some descriptions in commit.

Changes from v3:
- Add AI Viro's comment to v1->v2's revision description.
- Fixed a spelling mistakes of "marco" to "macro".

Changes from v2:
- Fixed some spelling mistakes in commit.
- Revision description are added for easy tracing.

Changes from v1:
- Rename DEFINE_STORE_ATTRIBUTE() to DEFINE_SHOW_STORE_ATTRIBUTE().
- AI Viro points out that he doesn't like the definition of macros
  like DEFINE_SHOW_ATTRIBUTE.


Luo Jiaxing (3):
  seq_file: Add helper macro to define attribute for rw file
  scsi: hisi_sas: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs
  scsi: qla2xxx: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 137 ++-----------------------
 drivers/scsi/qla2xxx/qla_dfs.c         | 113 +-------------------
 include/linux/seq_file.h               |  15 +++
 3 files changed, 26 insertions(+), 239 deletions(-)

-- 
2.17.1

[PATCH v5 1/3] seq_file: Add helper macro to define attribute for rw file

[Xingui Yang]

We already own DEFINE_SHOW_ATTRIBUTE() helper macro for defining attribute
for read-only file, but many of drivers want a helper macro for read-write
file too.

So we add DEFINE_SHOW_STORE_ATTRIBUTE helper to reduce duplicated code.

Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 include/linux/seq_file.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index bd023dd38ae6..e400007e410c 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -207,6 +207,21 @@ static const struct file_operations __name ## _fops = {			\
 	.release	= single_release,				\
 }
 
+#define DEFINE_SHOW_STORE_ATTRIBUTE(__name)				\
+static int __name ## _open(struct inode *inode, struct file *file)	\
+{									\
+	return single_open(file, __name ## _show, inode->i_private);	\
+}									\
+									\
+static const struct file_operations __name ## _fops = {			\
+	.owner		= THIS_MODULE,					\
+	.open		= __name ## _open,				\
+	.read		= seq_read,					\
+	.write		= __name ## _write,				\
+	.llseek		= seq_lseek,					\
+	.release	= single_release,				\
+}
+
 #define DEFINE_PROC_SHOW_ATTRIBUTE(__name)				\
 static int __name ## _open(struct inode *inode, struct file *file)	\
 {									\
-- 
2.17.1

Re: [PATCH v5 1/3] seq_file: Add helper macro to define attribute for rw file

[Andy Shevchenko]

On Mon, Sep 04, 2023 at 08:48:02AM +0000, Xingui Yang wrote:
> We already own DEFINE_SHOW_ATTRIBUTE() helper macro for defining attribute
> for read-only file, but many of drivers want a helper macro for read-write
> file too.
> 
> So we add DEFINE_SHOW_STORE_ATTRIBUTE helper to reduce duplicated code.
> 
> Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>

I believe you need to preserve Luo's authorship.

Perhaps,
Co-developed-by: Xingui ...
(depending if you really do some job in the certain patch(es) or not).

> Signed-off-by: Xingui Yang <yangxingui@huawei.com>

Code wise LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 2/3] scsi: hisi_sas: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[Xingui Yang]

Use DEFINE_SHOW_STORE_ATTRIBUTE helper for read-write file to reduce some
duplicated code.

Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 137 ++-----------------------
 1 file changed, 9 insertions(+), 128 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index bbb64ee6afd7..5bb35c3ea4e5 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3990,22 +3990,7 @@ static ssize_t debugfs_bist_linkrate_v3_hw_write(struct file *filp,
 
 	return count;
 }
-
-static int debugfs_bist_linkrate_v3_hw_open(struct inode *inode,
-					    struct file *filp)
-{
-	return single_open(filp, debugfs_bist_linkrate_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_linkrate_v3_hw_fops = {
-	.open = debugfs_bist_linkrate_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_linkrate_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_linkrate_v3_hw);
 
 static const struct {
 	int		value;
@@ -4080,22 +4065,7 @@ static ssize_t debugfs_bist_code_mode_v3_hw_write(struct file *filp,
 
 	return count;
 }
-
-static int debugfs_bist_code_mode_v3_hw_open(struct inode *inode,
-					     struct file *filp)
-{
-	return single_open(filp, debugfs_bist_code_mode_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_code_mode_v3_hw_fops = {
-	.open = debugfs_bist_code_mode_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_code_mode_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_code_mode_v3_hw);
 
 static ssize_t debugfs_bist_phy_v3_hw_write(struct file *filp,
 					    const char __user *buf,
@@ -4129,22 +4099,7 @@ static int debugfs_bist_phy_v3_hw_show(struct seq_file *s, void *p)
 
 	return 0;
 }
-
-static int debugfs_bist_phy_v3_hw_open(struct inode *inode,
-				       struct file *filp)
-{
-	return single_open(filp, debugfs_bist_phy_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_phy_v3_hw_fops = {
-	.open = debugfs_bist_phy_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_phy_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_phy_v3_hw);
 
 static ssize_t debugfs_bist_cnt_v3_hw_write(struct file *filp,
 					const char __user *buf,
@@ -4177,22 +4132,7 @@ static int debugfs_bist_cnt_v3_hw_show(struct seq_file *s, void *p)
 
 	return 0;
 }
-
-static int debugfs_bist_cnt_v3_hw_open(struct inode *inode,
-					  struct file *filp)
-{
-	return single_open(filp, debugfs_bist_cnt_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_cnt_v3_hw_ops = {
-	.open = debugfs_bist_cnt_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_cnt_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_cnt_v3_hw);
 
 static const struct {
 	int		value;
@@ -4256,22 +4196,7 @@ static ssize_t debugfs_bist_mode_v3_hw_write(struct file *filp,
 
 	return count;
 }
-
-static int debugfs_bist_mode_v3_hw_open(struct inode *inode,
-					struct file *filp)
-{
-	return single_open(filp, debugfs_bist_mode_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_mode_v3_hw_fops = {
-	.open = debugfs_bist_mode_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_mode_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_mode_v3_hw);
 
 static ssize_t debugfs_bist_enable_v3_hw_write(struct file *filp,
 					       const char __user *buf,
@@ -4309,22 +4234,7 @@ static int debugfs_bist_enable_v3_hw_show(struct seq_file *s, void *p)
 
 	return 0;
 }
-
-static int debugfs_bist_enable_v3_hw_open(struct inode *inode,
-					  struct file *filp)
-{
-	return single_open(filp, debugfs_bist_enable_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_bist_enable_v3_hw_fops = {
-	.open = debugfs_bist_enable_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_bist_enable_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_bist_enable_v3_hw);
 
 static const struct {
 	char *name;
@@ -4362,21 +4272,7 @@ static int debugfs_v3_hw_show(struct seq_file *s, void *p)
 
 	return 0;
 }
-
-static int debugfs_v3_hw_open(struct inode *inode, struct file *filp)
-{
-	return single_open(filp, debugfs_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_v3_hw_fops = {
-	.open = debugfs_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_v3_hw);
 
 static ssize_t debugfs_phy_down_cnt_v3_hw_write(struct file *filp,
 						const char __user *buf,
@@ -4407,22 +4303,7 @@ static int debugfs_phy_down_cnt_v3_hw_show(struct seq_file *s, void *p)
 
 	return 0;
 }
-
-static int debugfs_phy_down_cnt_v3_hw_open(struct inode *inode,
-					   struct file *filp)
-{
-	return single_open(filp, debugfs_phy_down_cnt_v3_hw_show,
-			   inode->i_private);
-}
-
-static const struct file_operations debugfs_phy_down_cnt_v3_hw_fops = {
-	.open = debugfs_phy_down_cnt_v3_hw_open,
-	.read = seq_read,
-	.write = debugfs_phy_down_cnt_v3_hw_write,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(debugfs_phy_down_cnt_v3_hw);
 
 enum fifo_dump_mode_v3_hw {
 	FIFO_DUMP_FORVER =		(1U << 0),
@@ -4832,7 +4713,7 @@ static void debugfs_bist_init_v3_hw(struct hisi_hba *hisi_hba)
 			    hisi_hba, &debugfs_bist_phy_v3_hw_fops);
 
 	debugfs_create_file("cnt", 0600, hisi_hba->debugfs_bist_dentry,
-			    hisi_hba, &debugfs_bist_cnt_v3_hw_ops);
+			    hisi_hba, &debugfs_bist_cnt_v3_hw_fops);
 
 	debugfs_create_file("loopback_mode", 0600,
 			    hisi_hba->debugfs_bist_dentry,
-- 
2.17.1

Re: [PATCH v5 2/3] scsi: hisi_sas: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[Andy Shevchenko]

On Mon, Sep 04, 2023 at 08:48:03AM +0000, Xingui Yang wrote:
> Use DEFINE_SHOW_STORE_ATTRIBUTE helper for read-write file to reduce some

DEFINE_SHOW_STORE_ATTRIBUTE()

> duplicated code.
> 
> Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>

Same comments about tags as in previous patch.

Code wise LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>


-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 3/3] scsi: qla2xxx: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[Xingui Yang]

Use DEFINE_SHOW_STORE_ATTRIBUTE helper for read-write file to reduce some
duplicated code and delete unused macros.

Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
---
 drivers/scsi/qla2xxx/qla_dfs.c | 113 +--------------------------------
 1 file changed, 2 insertions(+), 111 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_dfs.c b/drivers/scsi/qla2xxx/qla_dfs.c
index f060e593685d..fca01d551876 100644
--- a/drivers/scsi/qla2xxx/qla_dfs.c
+++ b/drivers/scsi/qla2xxx/qla_dfs.c
@@ -505,107 +505,6 @@ qla_dfs_naqp_show(struct seq_file *s, void *unused)
 	return 0;
 }
 
-/*
- * Helper macros for setting up debugfs entries.
- * _name: The name of the debugfs entry
- * _ctx_struct: The context that was passed when creating the debugfs file
- *
- * QLA_DFS_SETUP_RD could be used when there is only a show function.
- * - show function take the name qla_dfs_<sysfs-name>_show
- *
- * QLA_DFS_SETUP_RW could be used when there are both show and write functions.
- * - show function take the name  qla_dfs_<sysfs-name>_show
- * - write function take the name qla_dfs_<sysfs-name>_write
- *
- * To have a new debugfs entry, do:
- * 1. Create a "struct dentry *" in the appropriate structure in the format
- * dfs_<sysfs-name>
- * 2. Setup debugfs entries using QLA_DFS_SETUP_RD / QLA_DFS_SETUP_RW
- * 3. Create debugfs file in qla2x00_dfs_setup() using QLA_DFS_CREATE_FILE
- * or QLA_DFS_ROOT_CREATE_FILE
- * 4. Remove debugfs file in qla2x00_dfs_remove() using QLA_DFS_REMOVE_FILE
- * or QLA_DFS_ROOT_REMOVE_FILE
- *
- * Example for creating "TEST" sysfs file:
- * 1. struct qla_hw_data { ... struct dentry *dfs_TEST; }
- * 2. QLA_DFS_SETUP_RD(TEST, scsi_qla_host_t);
- * 3. In qla2x00_dfs_setup():
- * QLA_DFS_CREATE_FILE(ha, TEST, 0600, ha->dfs_dir, vha);
- * 4. In qla2x00_dfs_remove():
- * QLA_DFS_REMOVE_FILE(ha, TEST);
- */
-#define QLA_DFS_SETUP_RD(_name, _ctx_struct)				\
-static int								\
-qla_dfs_##_name##_open(struct inode *inode, struct file *file)		\
-{									\
-	_ctx_struct *__ctx = inode->i_private;				\
-									\
-	return single_open(file, qla_dfs_##_name##_show, __ctx);	\
-}									\
-									\
-static const struct file_operations qla_dfs_##_name##_ops = {		\
-	.open           = qla_dfs_##_name##_open,			\
-	.read           = seq_read,					\
-	.llseek         = seq_lseek,					\
-	.release        = single_release,				\
-};
-
-#define QLA_DFS_SETUP_RW(_name, _ctx_struct)				\
-static int								\
-qla_dfs_##_name##_open(struct inode *inode, struct file *file)		\
-{									\
-	_ctx_struct *__ctx = inode->i_private;				\
-									\
-	return single_open(file, qla_dfs_##_name##_show, __ctx);	\
-}									\
-									\
-static const struct file_operations qla_dfs_##_name##_ops = {		\
-	.open           = qla_dfs_##_name##_open,			\
-	.read           = seq_read,					\
-	.llseek         = seq_lseek,					\
-	.release        = single_release,				\
-	.write		= qla_dfs_##_name##_write,			\
-};
-
-#define QLA_DFS_ROOT_CREATE_FILE(_name, _perm, _ctx)			\
-	do {								\
-		if (!qla_dfs_##_name)					\
-			qla_dfs_##_name = debugfs_create_file(#_name,	\
-					_perm, qla2x00_dfs_root, _ctx,	\
-					&qla_dfs_##_name##_ops);	\
-	} while (0)
-
-#define QLA_DFS_ROOT_REMOVE_FILE(_name)					\
-	do {								\
-		if (qla_dfs_##_name) {					\
-			debugfs_remove(qla_dfs_##_name);		\
-			qla_dfs_##_name = NULL;				\
-		}							\
-	} while (0)
-
-#define QLA_DFS_CREATE_FILE(_struct, _name, _perm, _parent, _ctx)	\
-	do {								\
-		(_struct)->dfs_##_name = debugfs_create_file(#_name,	\
-					_perm, _parent, _ctx,		\
-					&qla_dfs_##_name##_ops)		\
-	} while (0)
-
-#define QLA_DFS_REMOVE_FILE(_struct, _name)				\
-	do {								\
-		if ((_struct)->dfs_##_name) {				\
-			debugfs_remove((_struct)->dfs_##_name);		\
-			(_struct)->dfs_##_name = NULL;			\
-		}							\
-	} while (0)
-
-static int
-qla_dfs_naqp_open(struct inode *inode, struct file *file)
-{
-	struct scsi_qla_host *vha = inode->i_private;
-
-	return single_open(file, qla_dfs_naqp_show, vha);
-}
-
 static ssize_t
 qla_dfs_naqp_write(struct file *file, const char __user *buffer,
     size_t count, loff_t *pos)
@@ -653,15 +552,7 @@ qla_dfs_naqp_write(struct file *file, const char __user *buffer,
 	kfree(buf);
 	return rc;
 }
-
-static const struct file_operations dfs_naqp_ops = {
-	.open		= qla_dfs_naqp_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-	.write		= qla_dfs_naqp_write,
-};
-
+DEFINE_SHOW_STORE_ATTRIBUTE(qla_dfs_naqp);
 
 int
 qla2x00_dfs_setup(scsi_qla_host_t *vha)
@@ -707,7 +598,7 @@ qla2x00_dfs_setup(scsi_qla_host_t *vha)
 
 	if (IS_QLA27XX(ha) || IS_QLA83XX(ha) || IS_QLA28XX(ha)) {
 		ha->tgt.dfs_naqp = debugfs_create_file("naqp",
-		    0400, ha->dfs_dir, vha, &dfs_naqp_ops);
+		    0400, ha->dfs_dir, vha, &qla_dfs_naqp_fops);
 		if (!ha->tgt.dfs_naqp) {
 			ql_log(ql_log_warn, vha, 0xd011,
 			       "Unable to create debugFS naqp node.\n");
-- 
2.17.1

Re: [PATCH v5 3/3] scsi: qla2xxx: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[Andy Shevchenko]

On Mon, Sep 04, 2023 at 08:48:04AM +0000, Xingui Yang wrote:
> Use DEFINE_SHOW_STORE_ATTRIBUTE helper for read-write file to reduce some
> duplicated code and delete unused macros.

> Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
> Signed-off-by: Xingui Yang <yangxingui@huawei.com>

Same comments as per previous patch.

...

> -/*
> - * Helper macros for setting up debugfs entries.
> - * _name: The name of the debugfs entry
> - * _ctx_struct: The context that was passed when creating the debugfs file
> - *
> - * QLA_DFS_SETUP_RD could be used when there is only a show function.
> - * - show function take the name qla_dfs_<sysfs-name>_show
> - *
> - * QLA_DFS_SETUP_RW could be used when there are both show and write functions.
> - * - show function take the name  qla_dfs_<sysfs-name>_show
> - * - write function take the name qla_dfs_<sysfs-name>_write
> - *
> - * To have a new debugfs entry, do:
> - * 1. Create a "struct dentry *" in the appropriate structure in the format
> - * dfs_<sysfs-name>
> - * 2. Setup debugfs entries using QLA_DFS_SETUP_RD / QLA_DFS_SETUP_RW
> - * 3. Create debugfs file in qla2x00_dfs_setup() using QLA_DFS_CREATE_FILE
> - * or QLA_DFS_ROOT_CREATE_FILE
> - * 4. Remove debugfs file in qla2x00_dfs_remove() using QLA_DFS_REMOVE_FILE
> - * or QLA_DFS_ROOT_REMOVE_FILE
> - *
> - * Example for creating "TEST" sysfs file:
> - * 1. struct qla_hw_data { ... struct dentry *dfs_TEST; }
> - * 2. QLA_DFS_SETUP_RD(TEST, scsi_qla_host_t);
> - * 3. In qla2x00_dfs_setup():
> - * QLA_DFS_CREATE_FILE(ha, TEST, 0600, ha->dfs_dir, vha);
> - * 4. In qla2x00_dfs_remove():
> - * QLA_DFS_REMOVE_FILE(ha, TEST);
> - */

I believe this comment (in some form) has to be preserved.
Try to rewrite it using reference to the new macro.

Otherwise looks good to me.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 3/3] scsi: qla2xxx: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[yangxingui]

On 2023/9/4 18:56, Andy Shevchenko wrote:
> On Mon, Sep 04, 2023 at 08:48:04AM +0000, Xingui Yang wrote:
>> Use DEFINE_SHOW_STORE_ATTRIBUTE helper for read-write file to reduce some
>> duplicated code and delete unused macros.
> 
>> Signed-off-by: Luo Jiaxing <luojiaxing@huawei.com>
>> Signed-off-by: Xingui Yang <yangxingui@huawei.com>
> 
> Same comments as per previous patch.
> 
> ...
> 
>> -/*
>> - * Helper macros for setting up debugfs entries.
>> - * _name: The name of the debugfs entry
>> - * _ctx_struct: The context that was passed when creating the debugfs file
>> - *
>> - * QLA_DFS_SETUP_RD could be used when there is only a show function.
>> - * - show function take the name qla_dfs_<sysfs-name>_show
>> - *
>> - * QLA_DFS_SETUP_RW could be used when there are both show and write functions.
>> - * - show function take the name  qla_dfs_<sysfs-name>_show
>> - * - write function take the name qla_dfs_<sysfs-name>_write
>> - *
>> - * To have a new debugfs entry, do:
>> - * 1. Create a "struct dentry *" in the appropriate structure in the format
>> - * dfs_<sysfs-name>
>> - * 2. Setup debugfs entries using QLA_DFS_SETUP_RD / QLA_DFS_SETUP_RW
>> - * 3. Create debugfs file in qla2x00_dfs_setup() using QLA_DFS_CREATE_FILE
>> - * or QLA_DFS_ROOT_CREATE_FILE
>> - * 4. Remove debugfs file in qla2x00_dfs_remove() using QLA_DFS_REMOVE_FILE
>> - * or QLA_DFS_ROOT_REMOVE_FILE
>> - *
>> - * Example for creating "TEST" sysfs file:
>> - * 1. struct qla_hw_data { ... struct dentry *dfs_TEST; }
>> - * 2. QLA_DFS_SETUP_RD(TEST, scsi_qla_host_t);
>> - * 3. In qla2x00_dfs_setup():
>> - * QLA_DFS_CREATE_FILE(ha, TEST, 0600, ha->dfs_dir, vha);
>> - * 4. In qla2x00_dfs_remove():
>> - * QLA_DFS_REMOVE_FILE(ha, TEST);
>> - */
> 
> I believe this comment (in some form) has to be preserved.
> Try to rewrite it using reference to the new macro.
Thanks for your reply, I checked and these macros aren't being called 
anywhere else, so I decided to delete them all. Of course, maybe this 
macro will be used in the future, and I can resubmit another version 
based on your suggestion.

Thanks.
Xingui
> 
> Otherwise looks good to me.
> 

Re: [PATCH v5 3/3] scsi: qla2xxx: Use DEFINE_SHOW_STORE_ATTRIBUTE helper for debugfs

[Andy Shevchenko]

On Mon, Sep 04, 2023 at 09:05:29PM +0800, yangxingui wrote:
> On 2023/9/4 18:56, Andy Shevchenko wrote:
> > On Mon, Sep 04, 2023 at 08:48:04AM +0000, Xingui Yang wrote:

...

> > > -/*
> > > - * Helper macros for setting up debugfs entries.
> > > - * _name: The name of the debugfs entry
> > > - * _ctx_struct: The context that was passed when creating the debugfs file
> > > - *
> > > - * QLA_DFS_SETUP_RD could be used when there is only a show function.
> > > - * - show function take the name qla_dfs_<sysfs-name>_show
> > > - *
> > > - * QLA_DFS_SETUP_RW could be used when there are both show and write functions.
> > > - * - show function take the name  qla_dfs_<sysfs-name>_show
> > > - * - write function take the name qla_dfs_<sysfs-name>_write
> > > - *
> > > - * To have a new debugfs entry, do:
> > > - * 1. Create a "struct dentry *" in the appropriate structure in the format
> > > - * dfs_<sysfs-name>
> > > - * 2. Setup debugfs entries using QLA_DFS_SETUP_RD / QLA_DFS_SETUP_RW
> > > - * 3. Create debugfs file in qla2x00_dfs_setup() using QLA_DFS_CREATE_FILE
> > > - * or QLA_DFS_ROOT_CREATE_FILE
> > > - * 4. Remove debugfs file in qla2x00_dfs_remove() using QLA_DFS_REMOVE_FILE
> > > - * or QLA_DFS_ROOT_REMOVE_FILE
> > > - *
> > > - * Example for creating "TEST" sysfs file:
> > > - * 1. struct qla_hw_data { ... struct dentry *dfs_TEST; }
> > > - * 2. QLA_DFS_SETUP_RD(TEST, scsi_qla_host_t);
> > > - * 3. In qla2x00_dfs_setup():
> > > - * QLA_DFS_CREATE_FILE(ha, TEST, 0600, ha->dfs_dir, vha);
> > > - * 4. In qla2x00_dfs_remove():
> > > - * QLA_DFS_REMOVE_FILE(ha, TEST);
> > > - */
> > 
> > I believe this comment (in some form) has to be preserved.
> > Try to rewrite it using reference to the new macro.
> Thanks for your reply, I checked and these macros aren't being called
> anywhere else, so I decided to delete them all. Of course, maybe this macro
> will be used in the future, and I can resubmit another version based on your
> suggestion.

Of course you need to rewrite it to use new approach.

-- 
With Best Regards,
Andy Shevchenko