2020-10-30 08:52:28

by Luo Jiaxing

[permalink] [raw]
Subject: [PATCH v2 0/5] Introduce a new helper marco DEFINE_SHOW_STORE_ATTRIBUTE at seq_file.c

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 marco for
read-write file too.

So we try to add this macro to help decrease code duplication.

Luo Jiaxing (5):
seq_file: Introduce DEFINE_SHOW_STORE_ATTRIBUTE() helper macro
scsi: hisi_sas: Introduce DEFINE_SHOW_STORE_ATTRIBUTE for debugfs
scsi: qla2xxx: Introduce DEFINE_SHOW_STORE_ATTRIBUTE for debugfs
usb: dwc3: debugfs: Introduce DEFINE_SHOW_STORE_ATTRIBUTE
drm/i915/display: Introduce DEFINE_SHOW_STORE_ATTRIBUTE for debugfs

.../gpu/drm/i915/display/intel_display_debugfs.c | 55 +--------
drivers/scsi/hisi_sas/hisi_sas_main.c | 135 +++------------------
drivers/scsi/qla2xxx/qla_dfs.c | 19 +--
drivers/usb/dwc3/debugfs.c | 52 +-------
include/linux/seq_file.h | 15 +++
5 files changed, 41 insertions(+), 235 deletions(-)

--
2.7.4


2020-10-30 08:53:18

by Luo Jiaxing

[permalink] [raw]
Subject: [PATCH v2 2/5] scsi: hisi_sas: Introduce DEFINE_SHOW_STORE_ATTRIBUTE for debugfs

Seq instroduce a new helper marco DEFINE_SHOW_STORE_ATTRIBUTE for
Read-Write file, So we use it at our code to reduce some duplicate code.

Signed-off-by: Luo Jiaxing <[email protected]>
---
drivers/scsi/hisi_sas/hisi_sas_main.c | 135 ++++------------------------------
1 file changed, 16 insertions(+), 119 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index 128583d..b8a6fc9 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -3403,22 +3403,7 @@ static ssize_t hisi_sas_debugfs_bist_linkrate_write(struct file *filp,

return count;
}
-
-static int hisi_sas_debugfs_bist_linkrate_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_bist_linkrate_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_bist_linkrate_ops = {
- .open = hisi_sas_debugfs_bist_linkrate_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_bist_linkrate_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_bist_linkrate);

static const struct {
int value;
@@ -3493,22 +3478,7 @@ static ssize_t hisi_sas_debugfs_bist_code_mode_write(struct file *filp,

return count;
}
-
-static int hisi_sas_debugfs_bist_code_mode_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_bist_code_mode_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_bist_code_mode_ops = {
- .open = hisi_sas_debugfs_bist_code_mode_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_bist_code_mode_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_bist_code_mode);

static ssize_t hisi_sas_debugfs_bist_phy_write(struct file *filp,
const char __user *buf,
@@ -3542,22 +3512,7 @@ static int hisi_sas_debugfs_bist_phy_show(struct seq_file *s, void *p)

return 0;
}
-
-static int hisi_sas_debugfs_bist_phy_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_bist_phy_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_bist_phy_ops = {
- .open = hisi_sas_debugfs_bist_phy_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_bist_phy_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_bist_phy);

static const struct {
int value;
@@ -3621,22 +3576,7 @@ static ssize_t hisi_sas_debugfs_bist_mode_write(struct file *filp,

return count;
}
-
-static int hisi_sas_debugfs_bist_mode_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_bist_mode_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_bist_mode_ops = {
- .open = hisi_sas_debugfs_bist_mode_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_bist_mode_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_bist_mode);

static ssize_t hisi_sas_debugfs_bist_enable_write(struct file *filp,
const char __user *buf,
@@ -3677,22 +3617,7 @@ static int hisi_sas_debugfs_bist_enable_show(struct seq_file *s, void *p)

return 0;
}
-
-static int hisi_sas_debugfs_bist_enable_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_bist_enable_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_bist_enable_ops = {
- .open = hisi_sas_debugfs_bist_enable_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_bist_enable_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_bist_enable);

static const struct {
char *name;
@@ -3730,21 +3655,7 @@ static int hisi_sas_debugfs_show(struct seq_file *s, void *p)

return 0;
}
-
-static int hisi_sas_debugfs_open(struct inode *inode, struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_ops = {
- .open = hisi_sas_debugfs_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs);

static ssize_t hisi_sas_debugfs_phy_down_cnt_write(struct file *filp,
const char __user *buf,
@@ -3776,21 +3687,7 @@ static int hisi_sas_debugfs_phy_down_cnt_show(struct seq_file *s, void *p)
return 0;
}

-static int hisi_sas_debugfs_phy_down_cnt_open(struct inode *inode,
- struct file *filp)
-{
- return single_open(filp, hisi_sas_debugfs_phy_down_cnt_show,
- inode->i_private);
-}
-
-static const struct file_operations hisi_sas_debugfs_phy_down_cnt_ops = {
- .open = hisi_sas_debugfs_phy_down_cnt_open,
- .read = seq_read,
- .write = hisi_sas_debugfs_phy_down_cnt_write,
- .llseek = seq_lseek,
- .release = single_release,
- .owner = THIS_MODULE,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(hisi_sas_debugfs_phy_down_cnt);

void hisi_sas_debugfs_work_handler(struct work_struct *work)
{
@@ -3937,7 +3834,7 @@ static void hisi_sas_debugfs_phy_down_cnt_init(struct hisi_hba *hisi_hba)
snprintf(name, 16, "%d", phy_no);
debugfs_create_file(name, 0600, dir,
&hisi_hba->phy[phy_no],
- &hisi_sas_debugfs_phy_down_cnt_ops);
+ &hisi_sas_debugfs_phy_down_cnt_fops);
}
}

@@ -3950,34 +3847,34 @@ static void hisi_sas_debugfs_bist_init(struct hisi_hba *hisi_hba)
debugfs_create_dir("bist", hisi_hba->debugfs_dir);
debugfs_create_file("link_rate", 0600,
hisi_hba->debugfs_bist_dentry, hisi_hba,
- &hisi_sas_debugfs_bist_linkrate_ops);
+ &hisi_sas_debugfs_bist_linkrate_fops);

debugfs_create_file("code_mode", 0600,
hisi_hba->debugfs_bist_dentry, hisi_hba,
- &hisi_sas_debugfs_bist_code_mode_ops);
+ &hisi_sas_debugfs_bist_code_mode_fops);

debugfs_create_file("fixed_code", 0600,
hisi_hba->debugfs_bist_dentry,
&hisi_hba->debugfs_bist_fixed_code[0],
- &hisi_sas_debugfs_ops);
+ &hisi_sas_debugfs_fops);

debugfs_create_file("fixed_code_1", 0600,
hisi_hba->debugfs_bist_dentry,
&hisi_hba->debugfs_bist_fixed_code[1],
- &hisi_sas_debugfs_ops);
+ &hisi_sas_debugfs_fops);

debugfs_create_file("phy_id", 0600, hisi_hba->debugfs_bist_dentry,
- hisi_hba, &hisi_sas_debugfs_bist_phy_ops);
+ hisi_hba, &hisi_sas_debugfs_bist_phy_fops);

debugfs_create_u32("cnt", 0600, hisi_hba->debugfs_bist_dentry,
&hisi_hba->debugfs_bist_cnt);

debugfs_create_file("loopback_mode", 0600,
hisi_hba->debugfs_bist_dentry,
- hisi_hba, &hisi_sas_debugfs_bist_mode_ops);
+ hisi_hba, &hisi_sas_debugfs_bist_mode_fops);

debugfs_create_file("enable", 0600, hisi_hba->debugfs_bist_dentry,
- hisi_hba, &hisi_sas_debugfs_bist_enable_ops);
+ hisi_hba, &hisi_sas_debugfs_bist_enable_fops);

ports_dentry = debugfs_create_dir("port", hisi_hba->debugfs_bist_dentry);

@@ -3996,7 +3893,7 @@ static void hisi_sas_debugfs_bist_init(struct hisi_hba *hisi_hba)
debugfs_create_file(hisi_sas_debugfs_ffe_name[i].name,
0600, ffe_dentry,
&hisi_hba->debugfs_bist_ffe[phy_no][i],
- &hisi_sas_debugfs_ops);
+ &hisi_sas_debugfs_fops);
}
}

--
2.7.4

2020-10-30 08:53:55

by Luo Jiaxing

[permalink] [raw]
Subject: [PATCH v2 4/5] usb: dwc3: debugfs: Introduce DEFINE_SHOW_STORE_ATTRIBUTE

Seq instroduce a new helper marco DEFINE_SHOW_STORE_ATTRIBUTE for
Read-Write file, So we apply it at dwc3 debugfs to reduce some duplicate
code.

Signed-off-by: Luo Jiaxing <[email protected]>
---
drivers/usb/dwc3/debugfs.c | 52 ++++------------------------------------------
1 file changed, 4 insertions(+), 48 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 5da4f60..2b5de8d 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -348,11 +348,6 @@ static int dwc3_lsp_show(struct seq_file *s, void *unused)
return 0;
}

-static int dwc3_lsp_open(struct inode *inode, struct file *file)
-{
- return single_open(file, dwc3_lsp_show, inode->i_private);
-}
-
static ssize_t dwc3_lsp_write(struct file *file, const char __user *ubuf,
size_t count, loff_t *ppos)
{
@@ -377,13 +372,7 @@ static ssize_t dwc3_lsp_write(struct file *file, const char __user *ubuf,
return count;
}

-static const struct file_operations dwc3_lsp_fops = {
- .open = dwc3_lsp_open,
- .write = dwc3_lsp_write,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(dwc3_lsp);

static int dwc3_mode_show(struct seq_file *s, void *unused)
{
@@ -412,11 +401,6 @@ static int dwc3_mode_show(struct seq_file *s, void *unused)
return 0;
}

-static int dwc3_mode_open(struct inode *inode, struct file *file)
-{
- return single_open(file, dwc3_mode_show, inode->i_private);
-}
-
static ssize_t dwc3_mode_write(struct file *file,
const char __user *ubuf, size_t count, loff_t *ppos)
{
@@ -445,13 +429,7 @@ static ssize_t dwc3_mode_write(struct file *file,
return count;
}

-static const struct file_operations dwc3_mode_fops = {
- .open = dwc3_mode_open,
- .write = dwc3_mode_write,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(dwc3_mode);

static int dwc3_testmode_show(struct seq_file *s, void *unused)
{
@@ -491,11 +469,6 @@ static int dwc3_testmode_show(struct seq_file *s, void *unused)
return 0;
}

-static int dwc3_testmode_open(struct inode *inode, struct file *file)
-{
- return single_open(file, dwc3_testmode_show, inode->i_private);
-}
-
static ssize_t dwc3_testmode_write(struct file *file,
const char __user *ubuf, size_t count, loff_t *ppos)
{
@@ -528,13 +501,7 @@ static ssize_t dwc3_testmode_write(struct file *file,
return count;
}

-static const struct file_operations dwc3_testmode_fops = {
- .open = dwc3_testmode_open,
- .write = dwc3_testmode_write,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(dwc3_testmode);

static int dwc3_link_state_show(struct seq_file *s, void *unused)
{
@@ -564,11 +531,6 @@ static int dwc3_link_state_show(struct seq_file *s, void *unused)
return 0;
}

-static int dwc3_link_state_open(struct inode *inode, struct file *file)
-{
- return single_open(file, dwc3_link_state_show, inode->i_private);
-}
-
static ssize_t dwc3_link_state_write(struct file *file,
const char __user *ubuf, size_t count, loff_t *ppos)
{
@@ -620,13 +582,7 @@ static ssize_t dwc3_link_state_write(struct file *file,
return count;
}

-static const struct file_operations dwc3_link_state_fops = {
- .open = dwc3_link_state_open,
- .write = dwc3_link_state_write,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_STORE_ATTRIBUTE(dwc3_link_state);

struct dwc3_ep_file_map {
const char name[25];
--
2.7.4

2020-10-30 08:54:10

by Luo Jiaxing

[permalink] [raw]
Subject: [PATCH v2 1/5] seq_file: Introduce DEFINE_SHOW_STORE_ATTRIBUTE() helper macro

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

So we try to make one to decrease code duplication.

Signed-off-by: Luo Jiaxing <[email protected]>
---
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 813614d..8a474c8 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -191,6 +191,21 @@ static const struct proc_ops __name ## _proc_ops = { \
.proc_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, \
+}
+
static inline struct user_namespace *seq_user_ns(struct seq_file *seq)
{
#ifdef CONFIG_USER_NS
--
2.7.4

2020-10-30 09:02:02

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] usb: dwc3: debugfs: Introduce DEFINE_SHOW_STORE_ATTRIBUTE

Luo Jiaxing <[email protected]> writes:

> Seq instroduce a new helper marco DEFINE_SHOW_STORE_ATTRIBUTE for
^^^^^^^^^^ ^^^^^
introduce macro

> Read-Write file, So we apply it at dwc3 debugfs to reduce some duplicate
^^ ^^^^^^^^^
so duplicated

> code.

to be fair, your commit is doing more than what it claims. Maybe update
commit log with a "while at that, also use DEFINE_SHOW_ATTRIBUTE() where
possible".

> Signed-off-by: Luo Jiaxing <[email protected]>

other than that, this looks okay. Since it depends on the definition of
DEFINE_SHOW_STORE_ATTRIBUTE:

Acked-by: Felipe Balbi <[email protected]>

--
balbi


Attachments:
signature.asc (873.00 B)