From 6c28f09658483dcfb1a7f2adc8160982c652bc83 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 19:55:14 +0100 Subject: [PATCH 01/10] greybus: es1: fix build warning for apb1_log_enable_write It's "const char __user *buf", not "char __user *buf". 'make check' is your friend. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 9ad8a76d33ac..8de005ba1e8d 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -579,7 +579,7 @@ static ssize_t apb1_log_enable_read(struct file *f, char __user *buf, return simple_read_from_buffer(buf, count, ppos, tmp_buf, 3); } -static ssize_t apb1_log_enable_write(struct file *f, char __user *buf, +static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, size_t count, loff_t *ppos) { int enable; From 2f4e236648d9c7f622518ec0098cd1cb6af21659 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:03:39 +0100 Subject: [PATCH 02/10] greybus: es1: fix tiny whitespace issues No trailing spaces or spaces before tabs are allowed. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 8de005ba1e8d..4524add9c868 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -129,7 +129,7 @@ static void usb_log_enable(struct es1_ap_dev *es1, int enable); * host driver. I.e., ((char *)buffer - headroom) must * point to valid memory, usable only by the host driver. * size_max: The maximum size of a buffer (not including the - * headroom) must not exceed this. + * headroom) must not exceed this. */ static void hd_buffer_constraints(struct greybus_host_device *hd) { @@ -597,7 +597,7 @@ static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, retval = count; } kfree(tmp_buf); - + return retval; } From cd674c8d4c3a1a9736ed1e9f8ec8f9ee2aaa26e0 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:04:49 +0100 Subject: [PATCH 03/10] greybus: es1: use and not Asm is only for when you are doing arch-specific stuff, which we aren't doing here. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 4524add9c868..cce31558573b 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include "greybus.h" #include "svc_msg.h" From 26164edb8fb467a4249fab159709a6cfc50ddc8c Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:06:41 +0100 Subject: [PATCH 04/10] greybus: es1: no need to check for NULL on debugfs_remove() The function can, and even expects NULL, so don't check. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index cce31558573b..723d8b7a0eab 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -558,10 +558,8 @@ static void usb_log_enable(struct es1_ap_dev *es1, int enable) gb_debugfs_get(), NULL, &apb1_log_fops); } else { - if (apb1_log_dentry) { - debugfs_remove(apb1_log_dentry); - apb1_log_dentry = NULL; - } + debugfs_remove(apb1_log_dentry); + apb1_log_dentry = NULL; if (apb1_log_task) { kthread_stop(apb1_log_task); From 4600d03f80aea65f29f324b89d1b3437982f3eba Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:08:12 +0100 Subject: [PATCH 05/10] greybus: es1: struct file_operations needs to be const We aren't changing these pointers, so mark them read-only as that is the preferred way. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 723d8b7a0eab..7e612cbd813e 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -540,7 +540,7 @@ static ssize_t apb1_log_read(struct file *f, char __user *buf, return ret; } -static struct file_operations apb1_log_fops = { +static const struct file_operations apb1_log_fops = { .read = apb1_log_read, }; @@ -599,7 +599,7 @@ static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, return retval; } -static struct file_operations apb1_log_enable_fops = { +static const struct file_operations apb1_log_enable_fops = { .read = apb1_log_enable_read, .write = apb1_log_enable_write, }; From d52f9973e2de01e6da7beb2e91ece009ffd880a2 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:10:58 +0100 Subject: [PATCH 06/10] greybus: es1: decimal modes are not what are wanted for debugfs decimal is not octal, so use the proper mode settings for the debugfs files. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 7e612cbd813e..796c2ab5dafd 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -554,7 +554,7 @@ static void usb_log_enable(struct es1_ap_dev *es1, int enable) apb1_log_task = kthread_run(apb1_log_poll, es1, "apb1_log"); if (apb1_log_task == ERR_PTR(-ENOMEM)) return; - apb1_log_dentry = debugfs_create_file("apb1_log", 444, + apb1_log_dentry = debugfs_create_file("apb1_log", S_IRUGO, gb_debugfs_get(), NULL, &apb1_log_fops); } else { @@ -730,7 +730,8 @@ static int ap_probe(struct usb_interface *interface, if (retval) goto error; - apb1_log_enable_dentry = debugfs_create_file("apb1_log_enable", 666, + apb1_log_enable_dentry = debugfs_create_file("apb1_log_enable", + (S_IWUSR | S_IRUGO), gb_debugfs_get(), es1, &apb1_log_enable_fops); From 64b8a16f4486912e4aa315497ea13c2d57402fc3 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:32:40 +0100 Subject: [PATCH 07/10] greybus: es1: move debugfs function to use kstrotoint_from_user() No need to duplicate built-in functions that the kernel has, so have the core kernel parse the userspace string. Saves us an allocation and makes the logic simpler. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 796c2ab5dafd..a7fb4b52991b 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -581,20 +581,19 @@ static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, size_t count, loff_t *ppos) { int enable; - char *tmp_buf; - ssize_t retval = -EINVAL; + ssize_t retval; struct es1_ap_dev *es1 = (struct es1_ap_dev *)f->f_inode->i_private; - tmp_buf = kmalloc(count, GFP_KERNEL); - if (!tmp_buf) - return -ENOMEM; + retval = kstrtoint_from_user(buf, count, 10, &enable); + if (retval) + return retval; - copy_from_user(tmp_buf, buf, count); - if (sscanf(tmp_buf, "%d", &enable) == 1) { + if (enable) { usb_log_enable(es1, enable); retval = count; + } else { + retval = -EINVAL; } - kfree(tmp_buf); return retval; } From efdc43130cdb3dbcd25d1586e93181abbc714ea5 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:34:02 +0100 Subject: [PATCH 08/10] greybus: es1: fix checkpatch warning about blank lines needed Add a blank line in apb1_log_enable_read() to make checkpatch happy. Oh, and it makes the code more readable too... Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index a7fb4b52991b..225ef3ff1181 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -573,6 +573,7 @@ static ssize_t apb1_log_enable_read(struct file *f, char __user *buf, { char tmp_buf[3]; int enable = apb1_log_task != NULL; + sprintf(tmp_buf, "%d\n", enable); return simple_read_from_buffer(buf, count, ppos, tmp_buf, 3); } From 0c264c6fc2c3f47d94169a38506cdf151a9ba52e Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:45:31 +0100 Subject: [PATCH 09/10] greybus: es1: separate usb_log enable/disable logic into different functions One function shouldn't do two different things depending on a parameter passed to it, so split usb_log_enable() into usb_log_disable() Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 45 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 225ef3ff1181..53d2d470e593 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -101,7 +101,8 @@ static inline struct es1_ap_dev *hd_to_es1(struct greybus_host_device *hd) } static void cport_out_callback(struct urb *urb); -static void usb_log_enable(struct es1_ap_dev *es1, int enable); +static void usb_log_enable(struct es1_ap_dev *es1); +static void usb_log_disable(struct es1_ap_dev *es1); /* * Buffer constraints for the host driver. @@ -337,7 +338,7 @@ static void ap_disconnect(struct usb_interface *interface) if (!es1) return; - usb_log_enable(es1, 0); + usb_log_disable(es1); /* Tear down everything! */ for (i = 0; i < NUM_CPORT_OUT_URB; ++i) { @@ -544,28 +545,30 @@ static const struct file_operations apb1_log_fops = { .read = apb1_log_read, }; -static void usb_log_enable(struct es1_ap_dev *es1, int enable) +static void usb_log_enable(struct es1_ap_dev *es1) { - if (enable && apb1_log_task != NULL) + if (apb1_log_task != NULL) return; - if (enable) { - /* get log from APB1 */ - apb1_log_task = kthread_run(apb1_log_poll, es1, "apb1_log"); - if (apb1_log_task == ERR_PTR(-ENOMEM)) - return; - apb1_log_dentry = debugfs_create_file("apb1_log", S_IRUGO, - gb_debugfs_get(), NULL, - &apb1_log_fops); - } else { - debugfs_remove(apb1_log_dentry); - apb1_log_dentry = NULL; + /* get log from APB1 */ + apb1_log_task = kthread_run(apb1_log_poll, es1, "apb1_log"); + if (apb1_log_task == ERR_PTR(-ENOMEM)) + return; + apb1_log_dentry = debugfs_create_file("apb1_log", S_IRUGO, + gb_debugfs_get(), NULL, + &apb1_log_fops); +} - if (apb1_log_task) { - kthread_stop(apb1_log_task); - apb1_log_task = NULL; - } - } +static void usb_log_disable(struct es1_ap_dev *es1) +{ + if (apb1_log_task == NULL) + return; + + debugfs_remove(apb1_log_dentry); + apb1_log_dentry = NULL; + + kthread_stop(apb1_log_task); + apb1_log_task = NULL; } static ssize_t apb1_log_enable_read(struct file *f, char __user *buf, @@ -590,7 +593,7 @@ static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, return retval; if (enable) { - usb_log_enable(es1, enable); + usb_log_enable(es1); retval = count; } else { retval = -EINVAL; From 25d1c37798cd298d726dfb114f0dcaa15824b138 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 24 Mar 2015 20:47:24 +0100 Subject: [PATCH 10/10] greybus: es1: allow the debug log to be stopped If you write 0 to the debugfs file, the log will stop being updated. Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/es1.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/staging/greybus/es1.c b/drivers/staging/greybus/es1.c index 53d2d470e593..8aad4fbe25e6 100644 --- a/drivers/staging/greybus/es1.c +++ b/drivers/staging/greybus/es1.c @@ -592,14 +592,12 @@ static ssize_t apb1_log_enable_write(struct file *f, const char __user *buf, if (retval) return retval; - if (enable) { + if (enable) usb_log_enable(es1); - retval = count; - } else { - retval = -EINVAL; - } + else + usb_log_disable(es1); - return retval; + return count; } static const struct file_operations apb1_log_enable_fops = {