| Message ID | 20200505153325.20113-1-aishwaryarj100@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Input: edt-ft5x06: Use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs fops | expand |
On Tue, May 05, 2020 at 09:03:24PM +0530, Aishwarya Ramakrishnan wrote: > It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file > operation rather than DEFINE_SIMPLE_ATTRIBUTE. No it is not, why do you think so? The two defines do different things, that is why we have 2 different defines. You can not just replace one with the other without understanding why one was used and not the other one. Did you test this change to verify that everything still works properly? Why is it needed to be changed at all? thanks, greg k-h
From: Aishwarya Ramakrishnan <aishwaryarj100@gmail.com> On Tue, May 5, 2020 at 11:49 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, May 05, 2020 at 09:03:24PM +0530, Aishwarya Ramakrishnan wrote: >> It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file >> operation rather than DEFINE_SIMPLE_ATTRIBUTE. > No it is not, why do you think so? This change is suggested by Coccinelle software. Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci > The two defines do different things, that is why we have 2 different > defines. You can not just replace one with the other without > understanding why one was used and not the other one. > Did you test this change to verify that everything still works > properly? Why is it needed to be changed at all? DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() imposes some significant overhead as compared to DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). But I missed to use debugfs_create_file_unsafe() function in the patch.
On Wed, May 06, 2020 at 08:36:22PM +0530, Aishwarya Ramakrishnan wrote: > From: Aishwarya Ramakrishnan <aishwaryarj100@gmail.com> > > On Tue, May 5, 2020 at 11:49 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Tue, May 05, 2020 at 09:03:24PM +0530, Aishwarya Ramakrishnan wrote: > >> It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file > >> operation rather than DEFINE_SIMPLE_ATTRIBUTE. > > > No it is not, why do you think so? > > This change is suggested by Coccinelle software. > Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci Not a good thing, I do not know why that was added :( > > The two defines do different things, that is why we have 2 different > > defines. You can not just replace one with the other without > > understanding why one was used and not the other one. > > > Did you test this change to verify that everything still works > > properly? Why is it needed to be changed at all? > > DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() > imposes some significant overhead as compared to > DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe(). What kind of overhead? Is it required? > But I missed to use debugfs_create_file_unsafe() function in the patch. Yeah, don't use the unsafe stuff unless you know what is happening here please. greg k-h
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index d2587724c52a..7f2070fde2ae 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -676,7 +676,7 @@ static int edt_ft5x06_debugfs_mode_set(void *data, u64 mode) return retval; }; -DEFINE_SIMPLE_ATTRIBUTE(debugfs_mode_fops, edt_ft5x06_debugfs_mode_get, +DEFINE_DEBUGFS_ATTRIBUTE(debugfs_mode_fops, edt_ft5x06_debugfs_mode_get, edt_ft5x06_debugfs_mode_set, "%llu\n"); static ssize_t edt_ft5x06_debugfs_raw_data_read(struct file *file,
It is more clear to use DEFINE_DEBUGFS_ATTRIBUTE to define debugfs file operation rather than DEFINE_SIMPLE_ATTRIBUTE. Signed-off-by: Aishwarya Ramakrishnan <aishwaryarj100@gmail.com> --- drivers/input/touchscreen/edt-ft5x06.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)