* [PATCH][RESEND] HID: elan: Make array buf static, shrinks object size
From: Colin King @ 2019-01-25 16:05 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate the array buf on the stack but instead make it
static. Makes the object code smaller by 43 bytes:
Before:
text data bss dec hex filename
7769 1520 0 9289 2449 drivers/hid/hid-elan.o
After:
text data bss dec hex filename
7662 1584 0 9246 241e drivers/hid/hid-elan.o
(gcc version 8.2.0 x86_64)
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/hid/hid-elan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 0bfd6d1b44c1..1c62095cee99 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -393,7 +393,7 @@ static int elan_start_multitouch(struct hid_device *hdev)
* This byte sequence will enable multitouch mode and disable
* mouse emulation
*/
- const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
+ static const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
unsigned char *dmabuf = kmemdup(buf, sizeof(buf), GFP_KERNEL);
if (!dmabuf)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] HID: elan: Make array buf static, shrinks object size
From: Jiri Kosina @ 2019-01-25 15:53 UTC (permalink / raw)
To: Colin Ian King
Cc: Benjamin Tissoires, linux-input, kernel-janitors, linux-kernel
In-Reply-To: <11666348-f50a-719f-89d6-d90b0a1ea8f7@canonical.com>
On Thu, 24 Jan 2019, Colin Ian King wrote:
> ping?
I don't think I've ever seen the original patch in my inbox. Could you
please just resend it?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: debug: fix the ring buffer implementation
From: Oleg Nesterov @ 2019-01-25 13:01 UTC (permalink / raw)
To: Vladis Dronov
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel,
stable
In-Reply-To: <20190125095744.3813-1-vdronov@redhat.com>
On 01/25, Vladis Dronov wrote:
>
> Ring buffer implementation in hid_debug_event() and hid_debug_events_read()
> is strange allowing lost or corrupted data. After commit 717adfdaf147
> ("HID: debug: check length before copy_to_user()") it is possible to enter
> an infinite loop in hid_debug_events_read() by providing 0 as count, this
> locks up a system. Fix this by rewriting the ring buffer implementation
> with kfifo and simplify the code.
>
> This fixes CVE-2019-3819.
To me this looks like a good cleanup even if we forget about bugfix. Cosmetic
nits, feel free to ignore...
> + if (kfifo_is_empty(&list->hid_debug_fifo)) {
> + add_wait_queue(&list->hdev->debug_wait, &wait);
> + set_current_state(TASK_INTERRUPTIBLE);
> +
> + while (kfifo_is_empty(&list->hid_debug_fifo)) {
> + if (file->f_flags & O_NONBLOCK) {
> + ret = -EAGAIN;
> + break;
> + }
> +
> + if (signal_pending(current)) {
> + ret = -ERESTARTSYS;
> + break;
> + }
> +
> + if (!list->hdev || !list->hdev->debug) {
> + ret = -EIO;
> + set_current_state(TASK_RUNNING);
> + goto out;
Can't resist... Yes, this is what the current code does. But you know that it looks
suspicious ;) if you add a comment the patch will be even better.
> + }
> +
> + /* allow O_NONBLOCK from other threads */
> + mutex_unlock(&list->read_mutex);
> + schedule();
> + mutex_lock(&list->read_mutex);
> + set_current_state(TASK_INTERRUPTIBLE);
> + }
> +
> + set_current_state(TASK_RUNNING);
you can use __set_current_state() here, mb() is not needed.
> + remove_wait_queue(&list->hdev->debug_wait, &wait);
> + }
> +
> + if (ret)
> + goto out;
> +
perhaps it make sense to move this check into the "if (kfifo_is_empty())" block.
> + if (kfifo_is_empty(&list->hid_debug_fifo))
> + goto out;
is kfifo_is_empty() == T really possible here?
Oleg.
^ permalink raw reply
* [PATCH] HID: debug: fix the ring buffer implementation
From: Vladis Dronov @ 2019-01-25 9:57 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Oleg Nesterov, linux-input,
linux-kernel
Cc: Vladis Dronov, stable
Ring buffer implementation in hid_debug_event() and hid_debug_events_read()
is strange allowing lost or corrupted data. After commit 717adfdaf147
("HID: debug: check length before copy_to_user()") it is possible to enter
an infinite loop in hid_debug_events_read() by providing 0 as count, this
locks up a system. Fix this by rewriting the ring buffer implementation
with kfifo and simplify the code.
This fixes CVE-2019-3819.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1669187
Cc: stable@vger.kernel.org # v4.16+
Fixes: cd667ce24796 ("HID: use debugfs for events/reports dumping")
Fixes: 717adfdaf147 ("HID: debug: check length before copy_to_user()")
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
drivers/hid/hid-debug.c | 116 ++++++++++++++------------------------
include/linux/hid-debug.h | 9 ++-
2 files changed, 47 insertions(+), 78 deletions(-)
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index c530476edba6..08870c909268 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -30,6 +30,7 @@
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/kfifo.h>
#include <linux/sched/signal.h>
#include <linux/export.h>
#include <linux/slab.h>
@@ -661,17 +662,12 @@ EXPORT_SYMBOL_GPL(hid_dump_device);
/* enqueue string to 'events' ring buffer */
void hid_debug_event(struct hid_device *hdev, char *buf)
{
- unsigned i;
struct hid_debug_list *list;
unsigned long flags;
spin_lock_irqsave(&hdev->debug_list_lock, flags);
- list_for_each_entry(list, &hdev->debug_list, node) {
- for (i = 0; buf[i]; i++)
- list->hid_debug_buf[(list->tail + i) % HID_DEBUG_BUFSIZE] =
- buf[i];
- list->tail = (list->tail + i) % HID_DEBUG_BUFSIZE;
- }
+ list_for_each_entry(list, &hdev->debug_list, node)
+ kfifo_in(&list->hid_debug_fifo, buf, strlen(buf));
spin_unlock_irqrestore(&hdev->debug_list_lock, flags);
wake_up_interruptible(&hdev->debug_wait);
@@ -722,8 +718,7 @@ void hid_dump_input(struct hid_device *hdev, struct hid_usage *usage, __s32 valu
hid_debug_event(hdev, buf);
kfree(buf);
- wake_up_interruptible(&hdev->debug_wait);
-
+ wake_up_interruptible(&hdev->debug_wait);
}
EXPORT_SYMBOL_GPL(hid_dump_input);
@@ -1083,8 +1078,8 @@ static int hid_debug_events_open(struct inode *inode, struct file *file)
goto out;
}
- if (!(list->hid_debug_buf = kzalloc(HID_DEBUG_BUFSIZE, GFP_KERNEL))) {
- err = -ENOMEM;
+ err = kfifo_alloc(&list->hid_debug_fifo, HID_DEBUG_FIFOSIZE, GFP_KERNEL);
+ if (err) {
kfree(list);
goto out;
}
@@ -1104,77 +1099,55 @@ static ssize_t hid_debug_events_read(struct file *file, char __user *buffer,
size_t count, loff_t *ppos)
{
struct hid_debug_list *list = file->private_data;
- int ret = 0, len;
+ int ret = 0, copied;
DECLARE_WAITQUEUE(wait, current);
mutex_lock(&list->read_mutex);
- while (ret == 0) {
- if (list->head == list->tail) {
- add_wait_queue(&list->hdev->debug_wait, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
-
- while (list->head == list->tail) {
- if (file->f_flags & O_NONBLOCK) {
- ret = -EAGAIN;
- break;
- }
- if (signal_pending(current)) {
- ret = -ERESTARTSYS;
- break;
- }
-
- if (!list->hdev || !list->hdev->debug) {
- ret = -EIO;
- set_current_state(TASK_RUNNING);
- goto out;
- }
-
- /* allow O_NONBLOCK from other threads */
- mutex_unlock(&list->read_mutex);
- schedule();
- mutex_lock(&list->read_mutex);
- set_current_state(TASK_INTERRUPTIBLE);
- }
-
- set_current_state(TASK_RUNNING);
- remove_wait_queue(&list->hdev->debug_wait, &wait);
- }
-
- if (ret)
- goto out;
+ if (kfifo_is_empty(&list->hid_debug_fifo)) {
+ add_wait_queue(&list->hdev->debug_wait, &wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+
+ while (kfifo_is_empty(&list->hid_debug_fifo)) {
+ if (file->f_flags & O_NONBLOCK) {
+ ret = -EAGAIN;
+ break;
+ }
+
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+
+ if (!list->hdev || !list->hdev->debug) {
+ ret = -EIO;
+ set_current_state(TASK_RUNNING);
+ goto out;
+ }
+
+ /* allow O_NONBLOCK from other threads */
+ mutex_unlock(&list->read_mutex);
+ schedule();
+ mutex_lock(&list->read_mutex);
+ set_current_state(TASK_INTERRUPTIBLE);
+ }
+
+ set_current_state(TASK_RUNNING);
+ remove_wait_queue(&list->hdev->debug_wait, &wait);
+ }
+
+ if (ret)
+ goto out;
+
+ if (kfifo_is_empty(&list->hid_debug_fifo))
+ goto out;
- /* pass the ringbuffer contents to userspace */
-copy_rest:
- if (list->tail == list->head)
- goto out;
- if (list->tail > list->head) {
- len = list->tail - list->head;
- if (len > count)
- len = count;
-
- if (copy_to_user(buffer + ret, &list->hid_debug_buf[list->head], len)) {
- ret = -EFAULT;
- goto out;
- }
- ret += len;
- list->head += len;
- } else {
- len = HID_DEBUG_BUFSIZE - list->head;
- if (len > count)
- len = count;
-
- if (copy_to_user(buffer, &list->hid_debug_buf[list->head], len)) {
- ret = -EFAULT;
- goto out;
- }
- list->head = 0;
- ret += len;
- count -= len;
- if (count > 0)
- goto copy_rest;
- }
-
- }
+ /* pass the fifo content to userspace, locking is not needed with only
+ * one concurrent reader and one concurrent writer
+ */
+ ret = kfifo_to_user(&list->hid_debug_fifo, buffer, count, &copied);
+ if (ret)
+ goto out;
+ ret = copied;
out:
mutex_unlock(&list->read_mutex);
return ret;
@@ -1185,7 +1158,7 @@ static __poll_t hid_debug_events_poll(struct file *file, poll_table *wait)
struct hid_debug_list *list = file->private_data;
poll_wait(file, &list->hdev->debug_wait, wait);
- if (list->head != list->tail)
+ if (!kfifo_is_empty(&list->hid_debug_fifo))
return EPOLLIN | EPOLLRDNORM;
if (!list->hdev->debug)
return EPOLLERR | EPOLLHUP;
@@ -1200,7 +1173,7 @@ static int hid_debug_events_release(struct inode *inode, struct file *file)
spin_lock_irqsave(&list->hdev->debug_list_lock, flags);
list_del(&list->node);
spin_unlock_irqrestore(&list->hdev->debug_list_lock, flags);
- kfree(list->hid_debug_buf);
+ kfifo_free(&list->hid_debug_fifo);
kfree(list);
return 0;
@@ -1246,4 +1219,3 @@ void hid_debug_exit(void)
{
debugfs_remove_recursive(hid_debug_root);
}
-
diff --git a/include/linux/hid-debug.h b/include/linux/hid-debug.h
index 8663f216c563..e7a7c92aaf09 100644
--- a/include/linux/hid-debug.h
+++ b/include/linux/hid-debug.h
@@ -24,7 +24,10 @@
#ifdef CONFIG_DEBUG_FS
+#include <linux/kfifo.h>
+
#define HID_DEBUG_BUFSIZE 512
+#define HID_DEBUG_FIFOSIZE 512
void hid_dump_input(struct hid_device *, struct hid_usage *, __s32);
void hid_dump_report(struct hid_device *, int , u8 *, int);
@@ -38,10 +41,7 @@ void hid_debug_event(struct hid_device *, char *);
void hid_debug_event(struct hid_device *, char *);
-
struct hid_debug_list {
- char *hid_debug_buf;
- int head;
- int tail;
+ DECLARE_KFIFO_PTR(hid_debug_fifo, char);
struct fasync_struct *fasync;
struct hid_device *hdev;
struct list_head node;
@@ -64,4 +64,3 @@ struct hid_debug_list {
#endif
#endif
-
^ permalink raw reply related
* Re: KASAN: use-after-free Read in string
From: Tetsuo Handa @ 2019-01-25 9:41 UTC (permalink / raw)
To: dmitry.torokhov, rydberg
Cc: syzbot, linux-input, linux-kernel, syzkaller-bugs
In-Reply-To: <0000000000009ce64e0574fe896e@google.com>
Hello.
syzbot is hitting use-after-free bug in uinput module. It seems that
syzbot is hitting this bug from cdev_put() path when closing a character
file. But since I can't reproduce the problem, I used a debug patch which
raises the refcount as if the character device file is open()ed before
uinput_destroy_device() is called.
----------------------------------------
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 3304aaaffe87..45fab285f189 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1594,6 +1594,7 @@ static int input_dev_uevent(struct device *device, struct kobj_uevent_env *env)
INPUT_ADD_HOTPLUG_VAR("NAME=\"%s\"", dev->name);
if (dev->phys)
INPUT_ADD_HOTPLUG_VAR("PHYS=\"%s\"", dev->phys);
+ WARN_ON(!kref_read(&device->kobj.kref));
if (dev->uniq)
INPUT_ADD_HOTPLUG_VAR("UNIQ=\"%s\"", dev->uniq);
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8ec483e8688b..0374ea00d4cc 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -299,8 +299,11 @@ static void uinput_destroy_device(struct uinput_device *udev)
udev->state = UIST_NEW_DEVICE;
if (dev) {
+ struct input_dev *d = input_get_device(dev);
name = dev->name;
+ dev->name = NULL;
phys = dev->phys;
+ dev->phys = NULL;
if (old_state == UIST_CREATED) {
uinput_flush_requests(udev);
input_unregister_device(dev);
@@ -310,6 +313,7 @@ static void uinput_destroy_device(struct uinput_device *udev)
kfree(name);
kfree(phys);
udev->dev = NULL;
+ input_put_device(d);
}
}
----------------------------------------
I can sometimes observe WARN_ON() because input_put_device() drops the
refcount to 0.
----------------------------------------
[ 122.491394][ T7080] kobject: 'input32' (00000000de092799): kobject_add_internal: parent: 'input', set: 'devices'
[ 122.497790][ T7080] kobject: 'input32' (00000000de092799): kobject_uevent_env
[ 122.501716][ T7080] kobject: 'input32' (00000000de092799): fill_kobj_path: path = '/devices/virtual/input/input32'
[ 122.510123][ T7080] kobject: 'input32' (00000000de092799): fill_kobj_path: path = '/devices/virtual/input/input32'
[ 122.517230][ T7080] input: syz1 as /devices/virtual/input/input32
[ 122.522334][ T7080] kobject: 'event3' (00000000317a3ed6): kobject_add_internal: parent: 'input32', set: 'devices'
[ 122.531090][ T7080] kobject: 'event3' (00000000317a3ed6): kobject_uevent_env
[ 122.536589][ T7080] kobject: 'event3' (00000000317a3ed6): fill_kobj_path: path = '/devices/virtual/input/input32/event3'
[ 122.545052][ T7080] kobject: 'event3' (00000000317a3ed6): kobject_uevent_env
[ 122.549900][ T7080] kobject: 'event3' (00000000317a3ed6): fill_kobj_path: path = '/devices/virtual/input/input32/event3'
[ 122.557276][ T7080] FAULT_INJECTION: forcing a failure.
[ 122.557276][ T7080] name failslab, interval 1, probability 0, space 0, times 0
[ 122.564580][ T7080] CPU: 0 PID: 7080 Comm: a.out Tainted: G W 5.0.0-rc3+ #142
[ 122.569018][ T7080] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 122.576320][ T7080] Call Trace:
[ 122.579087][ T7080] dump_stack+0x154/0x1c5
[ 122.582643][ T7080] should_fail+0x61e/0x720
[ 122.585679][ T7080] ? fault_create_debugfs_attr+0x1f0/0x1f0
[ 122.589436][ T7080] ? lock_downgrade+0x880/0x880
[ 122.592499][ T7080] ? ___might_sleep+0x2fe/0x480
[ 122.595518][ T7080] __should_failslab+0xec/0x120
[ 122.598995][ T7080] should_failslab+0x9/0x14
[ 122.601869][ T7080] kmem_cache_alloc+0x47/0x710
[ 122.605051][ T7080] ? refcount_add_not_zero_checked+0x1f0/0x1f0
[ 122.608359][ T7080] ? netlink_broadcast_filtered+0x6c/0xa30
[ 122.611721][ T7080] skb_clone+0x122/0x360
[ 122.614463][ T7080] netlink_broadcast_filtered+0x793/0xa30
[ 122.617568][ T7080] netlink_broadcast+0x3e/0x50
[ 122.620668][ T7080] kobject_uevent_env+0xd51/0x1150
[ 122.623565][ T7080] ? wait_for_completion+0x400/0x400
[ 122.626488][ T7080] kobject_uevent+0x1f/0x30
[ 122.629334][ T7080] device_del+0x673/0xaf0
[ 122.631925][ T7080] ? __device_links_no_driver+0x230/0x230
[ 122.634885][ T7080] ? mark_held_locks+0xaf/0x100
[ 122.637857][ T7080] ? _raw_spin_unlock_irq+0x27/0x80
[ 122.640594][ T7080] ? __input_unregister_device+0x13b/0x480
[ 122.643650][ T7080] ? _raw_spin_unlock_irq+0x27/0x80
[ 122.646334][ T7080] cdev_device_del+0x1a/0x70
[ 122.648825][ T7080] evdev_disconnect+0x42/0xb0
[ 122.651291][ T7080] __input_unregister_device+0x1e3/0x480
[ 122.654279][ T7080] ? kasan_check_read+0x11/0x20
[ 122.656777][ T7080] input_unregister_device+0xa4/0xe0
[ 122.659590][ T7080] uinput_destroy_device+0x216/0x270
[ 122.662130][ T7080] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 122.664850][ T7080] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 122.667560][ T7080] ? kasan_check_write+0x14/0x20
[ 122.670157][ T7080] ? proc_fail_nth_write+0x94/0x1c0
[ 122.672623][ T7080] ? map_files_get_link+0x3c0/0x3c0
[ 122.675253][ T7080] ? __handle_mm_fault+0x20c0/0x3290
[ 122.677842][ T7080] ? map_files_get_link+0x3c0/0x3c0
[ 122.680187][ T7080] ? __vfs_write+0x111/0x7f0
[ 122.682372][ T7080] uinput_ioctl+0x4c/0x60
[ 122.684486][ T7080] ? uinput_compat_ioctl+0x80/0x80
[ 122.687204][ T7080] do_vfs_ioctl+0x1a9/0x1100
[ 122.689530][ T7080] ? ioctl_preallocate+0x1e0/0x1e0
[ 122.691855][ T7080] ? lock_downgrade+0x880/0x880
[ 122.694111][ T7080] ? check_preemption_disabled+0x3b/0x240
[ 122.696603][ T7080] ? __sb_end_write+0xc6/0x100
[ 122.698842][ T7080] ? vfs_write+0x224/0x4d0
[ 122.700991][ T7080] ? kasan_check_read+0x11/0x20
[ 122.703610][ T7080] ? security_file_ioctl+0x87/0xb0
[ 122.706099][ T7080] ksys_ioctl+0x94/0xb0
[ 122.708161][ T7080] __x64_sys_ioctl+0x73/0xb0
[ 122.710525][ T7080] do_syscall_64+0xe7/0x570
[ 122.712669][ T7080] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 122.715185][ T7080] RIP: 0033:0x7f5160b1a839
[ 122.717311][ T7080] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 122.727007][ T7080] RSP: 002b:00007ffe75b27308 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 122.731413][ T7080] RAX: ffffffffffffffda RBX: 00007ffe75b27310 RCX: 00007f5160b1a839
[ 122.735723][ T7080] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 122.739593][ T7080] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007f5160e00038
[ 122.742850][ T7080] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 122.746101][ T7080] R13: 00007ffe75b27420 R14: 0000000000000000 R15: 0000000000000000
[ 122.750271][ T7080] kobject: '(null)' (00000000eccc95ba): kobject_cleanup, parent 00000000317a3ed6
[ 122.757125][ T7080] kobject: '(null)' (00000000eccc95ba): calling ktype release
[ 122.876948][ T7080] kobject: 'event3' (00000000317a3ed6): kobject_cleanup, parent (null)
[ 122.882151][ T7080] kobject: 'event3' (00000000317a3ed6): calling ktype release
[ 122.885425][ T7080] kobject: 'event3': free name
[ 122.888374][ T7080] kobject: 'input32' (00000000de092799): kobject_uevent_env
[ 122.891710][ T7080] kobject: 'input32' (00000000de092799): fill_kobj_path: path = '/devices/virtual/input/input32'
[ 122.897786][ T7080] kobject: 'input32' (00000000de092799): kobject_cleanup, parent (null)
[ 122.903563][ T7080] kobject: 'input32' (00000000de092799): calling ktype release
[ 122.907255][ T7080] kobject: 'input32': free name
[ 122.923983][ T7085] kobject: 'input33' (000000007fe64e92): kobject_add_internal: parent: 'input', set: 'devices'
[ 122.933592][ T7085] kobject: 'input33' (000000007fe64e92): kobject_uevent_env
[ 122.939795][ T7085] kobject: 'input33' (000000007fe64e92): fill_kobj_path: path = '/devices/virtual/input/input33'
[ 122.946505][ T7085] kobject: 'input33' (000000007fe64e92): fill_kobj_path: path = '/devices/virtual/input/input33'
[ 122.956466][ T7085] input: syz1 as /devices/virtual/input/input33
[ 122.961981][ T7085] kobject: 'event3' (000000003c8d2a0d): kobject_add_internal: parent: 'input33', set: 'devices'
[ 122.970126][ T7085] kobject: 'event3' (000000003c8d2a0d): kobject_uevent_env
[ 122.974416][ T7085] kobject: 'event3' (000000003c8d2a0d): fill_kobj_path: path = '/devices/virtual/input/input33/event3'
[ 122.982226][ T7085] kobject: 'event3' (000000003c8d2a0d): kobject_uevent_env
[ 122.986290][ T7085] kobject: 'event3' (000000003c8d2a0d): fill_kobj_path: path = '/devices/virtual/input/input33/event3'
[ 122.994321][ T7085] kobject: '(null)' (00000000263f9149): kobject_cleanup, parent 000000003c8d2a0d
[ 123.001522][ T7085] kobject: '(null)' (00000000263f9149): calling ktype release
[ 123.041730][ T7085] kobject: 'event3' (000000003c8d2a0d): kobject_cleanup, parent (null)
[ 123.047963][ T7085] kobject: 'event3' (000000003c8d2a0d): calling ktype release
[ 123.052144][ T7085] kobject: 'event3': free name
[ 123.055887][ T7085] kobject: 'input33' (000000007fe64e92): kobject_uevent_env
[ 123.059898][ T7085] FAULT_INJECTION: forcing a failure.
[ 123.059898][ T7085] name failslab, interval 1, probability 0, space 0, times 0
[ 123.067534][ T7085] CPU: 2 PID: 7085 Comm: a.out Tainted: G W 5.0.0-rc3+ #142
[ 123.073308][ T7085] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 123.081085][ T7085] Call Trace:
[ 123.084204][ T7085] dump_stack+0x154/0x1c5
[ 123.087890][ T7085] should_fail+0x61e/0x720
[ 123.091337][ T7085] ? fault_create_debugfs_attr+0x1f0/0x1f0
[ 123.096751][ T7085] ? lock_downgrade+0x880/0x880
[ 123.102104][ T7085] ? ___might_sleep+0x2fe/0x480
[ 123.107205][ T7085] __should_failslab+0xec/0x120
[ 123.112120][ T7085] should_failslab+0x9/0x14
[ 123.116677][ T7085] kmem_cache_alloc_trace+0x4b/0x710
[ 123.121206][ T7085] ? dev_uevent_filter+0xe0/0xe0
[ 123.124370][ T7085] kobject_uevent_env+0x22c/0x1150
[ 123.127605][ T7085] ? wait_for_completion+0x400/0x400
[ 123.130996][ T7085] ? software_node_notify+0xd8/0x2b0
[ 123.134068][ T7085] kobject_uevent+0x1f/0x30
[ 123.137137][ T7085] device_del+0x673/0xaf0
[ 123.140102][ T7085] ? __device_links_no_driver+0x230/0x230
[ 123.143361][ T7085] ? trace_hardirqs_on+0x52/0x1d0
[ 123.146222][ T7085] __input_unregister_device+0x379/0x480
[ 123.149280][ T7085] ? kasan_check_read+0x11/0x20
[ 123.152249][ T7085] input_unregister_device+0xa4/0xe0
[ 123.155458][ T7085] uinput_destroy_device+0x216/0x270
[ 123.158365][ T7085] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 123.161375][ T7085] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 123.164555][ T7085] ? kasan_check_write+0x14/0x20
[ 123.167372][ T7085] ? proc_fail_nth_write+0x94/0x1c0
[ 123.170416][ T7085] ? map_files_get_link+0x3c0/0x3c0
[ 123.174171][ T7085] ? __handle_mm_fault+0x20c0/0x3290
[ 123.177902][ T7085] ? map_files_get_link+0x3c0/0x3c0
[ 123.181524][ T7085] ? __vfs_write+0x111/0x7f0
[ 123.184877][ T7085] uinput_ioctl+0x4c/0x60
[ 123.188445][ T7085] ? uinput_compat_ioctl+0x80/0x80
[ 123.191368][ T7085] do_vfs_ioctl+0x1a9/0x1100
[ 123.193809][ T7085] ? ioctl_preallocate+0x1e0/0x1e0
[ 123.196311][ T7085] ? lock_downgrade+0x880/0x880
[ 123.199013][ T7085] ? check_preemption_disabled+0x3b/0x240
[ 123.201867][ T7085] ? __sb_end_write+0xc6/0x100
[ 123.204412][ T7085] ? vfs_write+0x224/0x4d0
[ 123.206756][ T7085] ? kasan_check_read+0x11/0x20
[ 123.209168][ T7085] ? security_file_ioctl+0x87/0xb0
[ 123.211779][ T7085] ksys_ioctl+0x94/0xb0
[ 123.214142][ T7085] __x64_sys_ioctl+0x73/0xb0
[ 123.216377][ T7085] do_syscall_64+0xe7/0x570
[ 123.218674][ T7085] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 123.221573][ T7085] RIP: 0033:0x7fd9b2d0d839
[ 123.223703][ T7085] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 123.233417][ T7085] RSP: 002b:00007fff66a27408 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 123.237353][ T7085] RAX: ffffffffffffffda RBX: 00007fff66a27410 RCX: 00007fd9b2d0d839
[ 123.241290][ T7085] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 123.246692][ T7085] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007fd9b2ff0038
[ 123.250315][ T7085] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 123.254325][ T7085] R13: 00007fff66a27520 R14: 0000000000000000 R15: 0000000000000000
[ 123.258552][ T7085] kobject: 'input33' (000000007fe64e92): kobject_cleanup, parent (null)
[ 123.263643][ T7085] kobject: 'input33' (000000007fe64e92): auto cleanup 'remove' event
[ 123.267107][ T7085] kobject: 'input33' (000000007fe64e92): kobject_uevent_env
[ 123.270908][ T7085] kobject: 'input33' (000000007fe64e92): fill_kobj_path: path = '/input33'
[ 123.274726][ T7085] WARNING: CPU: 2 PID: 7085 at drivers/input/input.c:1597 input_dev_uevent+0x5a2/0x7b0
[ 123.280249][ T7085] Modules linked in:
[ 123.282624][ T7085] CPU: 2 PID: 7085 Comm: a.out Tainted: G W 5.0.0-rc3+ #142
[ 123.286383][ T7085] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 123.292981][ T7085] RIP: 0010:input_dev_uevent+0x5a2/0x7b0
[ 123.296185][ T7085] Code: e8 53 5b 25 fd 44 8b 4d d4 e9 03 fb ff ff e8 45 5b 25 fd e9 c2 fa ff ff 4c 89 ef e8 58 5b 25 fd e9 52 fc ff ff e8 6e 62 f3 fc <0f> 0b e9 51 fc ff ff e8 62 62 f3 fc 49 8d 54 24 30 b9 ff 02 00 00
[ 123.306200][ T7085] RSP: 0018:ffff8881e2187830 EFLAGS: 00010293
[ 123.309423][ T7085] RAX: ffff8881de5a6180 RBX: ffff8881df2c6e88 RCX: ffffffff8475e852
[ 123.313674][ T7085] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff8881df2c6ed0
[ 123.319246][ T7085] RBP: ffff8881e2187868 R08: ffffed103be58ddb R09: ffffed103be58ddb
[ 123.324930][ T7085] R10: 0000000000000001 R11: ffffed103be58dda R12: ffff8881df2c6bc0
[ 123.330307][ T7085] R13: ffff8881df2c6ed0 R14: ffff8881dd3a6940 R15: ffff8881df2c6e88
[ 123.334329][ T7085] FS: 00007fd9b31fd500(0000) GS:ffff8881f5a80000(0000) knlGS:0000000000000000
[ 123.338555][ T7085] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 123.342050][ T7085] CR2: 00007fd9b2d9fb24 CR3: 00000001c370e002 CR4: 00000000003606e0
[ 123.345922][ T7085] Call Trace:
[ 123.348590][ T7085] ? input_add_uevent_bm_var+0x130/0x130
[ 123.351864][ T7085] dev_uevent+0x330/0x5e0
[ 123.355017][ T7085] ? device_get_devnode+0x2e0/0x2e0
[ 123.358047][ T7085] ? vprintk_func+0x68/0x190
[ 123.360969][ T7085] ? add_uevent_var+0x20c/0x2f0
[ 123.363992][ T7085] ? cleanup_uevent_env+0x50/0x50
[ 123.366933][ T7085] ? kobject_uevent_env+0x347/0x1150
[ 123.370025][ T7085] ? device_get_devnode+0x2e0/0x2e0
[ 123.372997][ T7085] kobject_uevent_env+0x487/0x1150
[ 123.375921][ T7085] ? kmsg_dump_rewind_nolock+0xe4/0xe4
[ 123.378942][ T7085] kobject_uevent+0x1f/0x30
[ 123.381699][ T7085] kobject_put+0x33c/0x400
[ 123.384375][ T7085] put_device+0x20/0x30
[ 123.387304][ T7085] uinput_destroy_device+0x158/0x270
[ 123.390182][ T7085] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 123.393163][ T7085] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 123.397616][ T7085] ? kasan_check_write+0x14/0x20
[ 123.401440][ T7085] ? proc_fail_nth_write+0x94/0x1c0
[ 123.404393][ T7085] ? map_files_get_link+0x3c0/0x3c0
[ 123.407191][ T7085] ? __handle_mm_fault+0x20c0/0x3290
[ 123.410014][ T7085] ? map_files_get_link+0x3c0/0x3c0
[ 123.412668][ T7085] ? __vfs_write+0x111/0x7f0
[ 123.415202][ T7085] uinput_ioctl+0x4c/0x60
[ 123.417633][ T7085] ? uinput_compat_ioctl+0x80/0x80
[ 123.420355][ T7085] do_vfs_ioctl+0x1a9/0x1100
[ 123.423053][ T7085] ? ioctl_preallocate+0x1e0/0x1e0
[ 123.425574][ T7085] ? lock_downgrade+0x880/0x880
[ 123.427945][ T7085] ? check_preemption_disabled+0x3b/0x240
[ 123.430867][ T7085] ? __sb_end_write+0xc6/0x100
[ 123.433322][ T7085] ? vfs_write+0x224/0x4d0
[ 123.435527][ T7085] ? kasan_check_read+0x11/0x20
[ 123.438404][ T7085] ? security_file_ioctl+0x87/0xb0
[ 123.440847][ T7085] ksys_ioctl+0x94/0xb0
[ 123.442940][ T7085] __x64_sys_ioctl+0x73/0xb0
[ 123.445627][ T7085] do_syscall_64+0xe7/0x570
[ 123.447876][ T7085] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 123.450354][ T7085] RIP: 0033:0x7fd9b2d0d839
[ 123.452589][ T7085] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 123.461381][ T7085] RSP: 002b:00007fff66a27408 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 123.464806][ T7085] RAX: ffffffffffffffda RBX: 00007fff66a27410 RCX: 00007fd9b2d0d839
[ 123.468105][ T7085] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 123.471712][ T7085] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007fd9b2ff0038
[ 123.475028][ T7085] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 123.478476][ T7085] R13: 00007fff66a27520 R14: 0000000000000000 R15: 0000000000000000
[ 123.481837][ T7085] irq event stamp: 2904
[ 123.484128][ T7085] hardirqs last enabled at (2903): [<ffffffff81552310>] console_unlock+0x670/0xd00
[ 123.489864][ T7085] hardirqs last disabled at (2904): [<ffffffff81006087>] trace_hardirqs_off_thunk+0x1a/0x1c
[ 123.496220][ T7085] softirqs last enabled at (2888): [<ffffffff86c00673>] __do_softirq+0x673/0x987
[ 123.501609][ T7085] softirqs last disabled at (2869): [<ffffffff8140c0f5>] irq_exit+0x195/0x1c0
[ 123.505845][ T7085] ---[ end trace de7fa7a05c9c270b ]---
[ 123.509700][ T7085] kobject: 'input33' (000000007fe64e92): calling ktype release
[ 123.514678][ T7085] kobject: 'input33': free name
----------------------------------------
I also tried reproducing the problem without raising the refcount,
----------------------------------------
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 3304aaaffe87..45fab285f189 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1594,6 +1594,7 @@ static int input_dev_uevent(struct device *device, struct kobj_uevent_env *env)
INPUT_ADD_HOTPLUG_VAR("NAME=\"%s\"", dev->name);
if (dev->phys)
INPUT_ADD_HOTPLUG_VAR("PHYS=\"%s\"", dev->phys);
+ WARN_ON(!kref_read(&device->kobj.kref));
if (dev->uniq)
INPUT_ADD_HOTPLUG_VAR("UNIQ=\"%s\"", dev->uniq);
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8ec483e8688b..131591b5babd 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -300,7 +300,9 @@ static void uinput_destroy_device(struct uinput_device *udev)
if (dev) {
name = dev->name;
+ dev->name = NULL;
phys = dev->phys;
+ dev->phys = NULL;
if (old_state == UIST_CREATED) {
uinput_flush_requests(udev);
input_unregister_device(dev);
----------------------------------------
and I can sometimes observe WARN_ON() because the refcount drops to 0.
Therefore, I think that we must not assume that kobject_uevent() won't
be called after uinput_destroy_device() called kfree().
----------------------------------------
[ 96.531017][ T6997] kobject: 'input21' (0000000074dfd11f): kobject_add_internal: parent: 'input', set: 'devices'
[ 96.537254][ T6997] kobject: 'input21' (0000000074dfd11f): kobject_uevent_env
[ 96.541162][ T6997] kobject: 'input21' (0000000074dfd11f): fill_kobj_path: path = '/devices/virtual/input/input21'
[ 96.547745][ T6997] kobject: 'input21' (0000000074dfd11f): fill_kobj_path: path = '/devices/virtual/input/input21'
[ 96.554230][ T6997] input: syz1 as /devices/virtual/input/input21
[ 96.559539][ T6997] kobject: 'event3' (00000000648730df): kobject_add_internal: parent: 'input21', set: 'devices'
[ 96.569883][ T6997] kobject: 'event3' (00000000648730df): kobject_uevent_env
[ 96.574314][ T6997] kobject: 'event3' (00000000648730df): fill_kobj_path: path = '/devices/virtual/input/input21/event3'
[ 96.584282][ T6997] kobject: 'event3' (00000000648730df): kobject_uevent_env
[ 96.588860][ T6997] kobject: 'event3' (00000000648730df): fill_kobj_path: path = '/devices/virtual/input/input21/event3'
[ 96.599355][ T6997] FAULT_INJECTION: forcing a failure.
[ 96.599355][ T6997] name failslab, interval 1, probability 0, space 0, times 0
[ 96.607828][ T6997] CPU: 3 PID: 6997 Comm: a.out Not tainted 5.0.0-rc3+ #143
[ 96.611875][ T6997] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 96.619039][ T6997] Call Trace:
[ 96.622211][ T6997] dump_stack+0x154/0x1c5
[ 96.625318][ T6997] should_fail+0x61e/0x720
[ 96.628678][ T6997] ? fault_create_debugfs_attr+0x1f0/0x1f0
[ 96.632461][ T6997] ? lock_downgrade+0x880/0x880
[ 96.635613][ T6997] ? ___might_sleep+0x2fe/0x480
[ 96.638996][ T6997] __should_failslab+0xec/0x120
[ 96.642133][ T6997] should_failslab+0x9/0x14
[ 96.645003][ T6997] kmem_cache_alloc+0x47/0x710
[ 96.648132][ T6997] ? refcount_add_not_zero_checked+0x1f0/0x1f0
[ 96.651502][ T6997] ? netlink_broadcast_filtered+0x6c/0xa30
[ 96.654754][ T6997] skb_clone+0x122/0x360
[ 96.657877][ T6997] netlink_broadcast_filtered+0x793/0xa30
[ 96.661063][ T6997] netlink_broadcast+0x3e/0x50
[ 96.663850][ T6997] kobject_uevent_env+0xd51/0x1150
[ 96.666808][ T6997] ? wait_for_completion+0x400/0x400
[ 96.669819][ T6997] kobject_uevent+0x1f/0x30
[ 96.672838][ T6997] device_del+0x673/0xaf0
[ 96.675412][ T6997] ? __device_links_no_driver+0x230/0x230
[ 96.678471][ T6997] ? mark_held_locks+0xaf/0x100
[ 96.681207][ T6997] ? _raw_spin_unlock_irq+0x27/0x80
[ 96.683928][ T6997] ? __input_unregister_device+0x13b/0x480
[ 96.686962][ T6997] ? _raw_spin_unlock_irq+0x27/0x80
[ 96.689979][ T6997] cdev_device_del+0x1a/0x70
[ 96.692532][ T6997] evdev_disconnect+0x42/0xb0
[ 96.695109][ T6997] __input_unregister_device+0x1e3/0x480
[ 96.697885][ T6997] ? kasan_check_read+0x11/0x20
[ 96.700421][ T6997] input_unregister_device+0xa4/0xe0
[ 96.702971][ T6997] uinput_destroy_device+0x1e3/0x240
[ 96.705894][ T6997] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 96.708713][ T6997] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 96.711453][ T6997] ? kasan_check_write+0x14/0x20
[ 96.713866][ T6997] ? proc_fail_nth_write+0x94/0x1c0
[ 96.716386][ T6997] ? map_files_get_link+0x3c0/0x3c0
[ 96.718808][ T6997] ? __handle_mm_fault+0x20c0/0x3290
[ 96.721446][ T6997] ? map_files_get_link+0x3c0/0x3c0
[ 96.724169][ T6997] ? __vfs_write+0x111/0x7f0
[ 96.726353][ T6997] uinput_ioctl+0x4c/0x60
[ 96.728983][ T6997] ? uinput_compat_ioctl+0x80/0x80
[ 96.732259][ T6997] do_vfs_ioctl+0x1a9/0x1100
[ 96.735345][ T6997] ? ioctl_preallocate+0x1e0/0x1e0
[ 96.738758][ T6997] ? lock_downgrade+0x880/0x880
[ 96.741859][ T6997] ? check_preemption_disabled+0x3b/0x240
[ 96.744752][ T6997] ? __sb_end_write+0xc6/0x100
[ 96.747207][ T6997] ? vfs_write+0x224/0x4d0
[ 96.750131][ T6997] ? kasan_check_read+0x11/0x20
[ 96.753300][ T6997] ? security_file_ioctl+0x87/0xb0
[ 96.756922][ T6997] ksys_ioctl+0x94/0xb0
[ 96.759615][ T6997] __x64_sys_ioctl+0x73/0xb0
[ 96.761779][ T6997] do_syscall_64+0xe7/0x570
[ 96.764036][ T6997] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 96.766695][ T6997] RIP: 0033:0x7f02ca03a839
[ 96.768823][ T6997] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 96.777498][ T6997] RSP: 002b:00007ffe08c4e418 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 96.780999][ T6997] RAX: ffffffffffffffda RBX: 00007ffe08c4e420 RCX: 00007f02ca03a839
[ 96.784750][ T6997] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 96.788895][ T6997] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007f02ca320038
[ 96.792533][ T6997] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 96.796682][ T6997] R13: 00007ffe08c4e530 R14: 0000000000000000 R15: 0000000000000000
[ 96.802099][ T6997] kobject: '(null)' (0000000056da26df): kobject_cleanup, parent 00000000648730df
[ 96.808680][ T6997] kobject: '(null)' (0000000056da26df): calling ktype release
[ 96.856008][ T6997] kobject: 'event3' (00000000648730df): kobject_cleanup, parent (null)
[ 96.863416][ T6997] kobject: 'event3' (00000000648730df): calling ktype release
[ 96.868646][ T6997] kobject: 'event3': free name
[ 96.871768][ T6997] kobject: 'input21' (0000000074dfd11f): kobject_uevent_env
[ 96.876871][ T6997] kobject: 'input21' (0000000074dfd11f): fill_kobj_path: path = '/devices/virtual/input/input21'
[ 96.884436][ T6997] kobject: 'input21' (0000000074dfd11f): kobject_cleanup, parent (null)
[ 96.891397][ T6997] kobject: 'input21' (0000000074dfd11f): calling ktype release
[ 96.896877][ T6997] kobject: 'input21': free name
[ 106.244899][ T7002] kobject: 'input22' (00000000e09f1862): kobject_add_internal: parent: 'input', set: 'devices'
[ 106.251101][ T7002] kobject: 'input22' (00000000e09f1862): kobject_uevent_env
[ 106.255895][ T7002] kobject: 'input22' (00000000e09f1862): fill_kobj_path: path = '/devices/virtual/input/input22'
[ 106.262394][ T7002] kobject: 'input22' (00000000e09f1862): fill_kobj_path: path = '/devices/virtual/input/input22'
[ 106.269457][ T7002] input: syz1 as /devices/virtual/input/input22
[ 106.273526][ T7002] kobject: 'event3' (000000006eb448c2): kobject_add_internal: parent: 'input22', set: 'devices'
[ 106.281027][ T7002] kobject: 'event3' (000000006eb448c2): kobject_uevent_env
[ 106.285352][ T7002] kobject: 'event3' (000000006eb448c2): fill_kobj_path: path = '/devices/virtual/input/input22/event3'
[ 106.293562][ T7002] kobject: 'event3' (000000006eb448c2): kobject_uevent_env
[ 106.297761][ T7002] kobject: 'event3' (000000006eb448c2): fill_kobj_path: path = '/devices/virtual/input/input22/event3'
[ 106.305312][ T7002] kobject: '(null)' (00000000192e3ebd): kobject_cleanup, parent 000000006eb448c2
[ 106.314030][ T7002] kobject: '(null)' (00000000192e3ebd): calling ktype release
[ 106.404201][ T7002] kobject: 'event3' (000000006eb448c2): kobject_cleanup, parent (null)
[ 106.410653][ T7002] kobject: 'event3' (000000006eb448c2): calling ktype release
[ 106.414781][ T7002] kobject: 'event3': free name
[ 106.418206][ T7002] kobject: 'input22' (00000000e09f1862): kobject_uevent_env
[ 106.422387][ T7002] FAULT_INJECTION: forcing a failure.
[ 106.422387][ T7002] name failslab, interval 1, probability 0, space 0, times 0
[ 106.429765][ T7002] CPU: 0 PID: 7002 Comm: a.out Not tainted 5.0.0-rc3+ #143
[ 106.433704][ T7002] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 106.440953][ T7002] Call Trace:
[ 106.443818][ T7002] dump_stack+0x154/0x1c5
[ 106.446842][ T7002] should_fail+0x61e/0x720
[ 106.449962][ T7002] ? fault_create_debugfs_attr+0x1f0/0x1f0
[ 106.453392][ T7002] ? lock_downgrade+0x880/0x880
[ 106.456771][ T7002] ? ___might_sleep+0x2fe/0x480
[ 106.459873][ T7002] __should_failslab+0xec/0x120
[ 106.462960][ T7002] should_failslab+0x9/0x14
[ 106.465843][ T7002] kmem_cache_alloc_trace+0x4b/0x710
[ 106.469001][ T7002] ? dev_uevent_filter+0xe0/0xe0
[ 106.472264][ T7002] kobject_uevent_env+0x22c/0x1150
[ 106.475338][ T7002] ? wait_for_completion+0x400/0x400
[ 106.478399][ T7002] ? software_node_notify+0xd8/0x2b0
[ 106.481321][ T7002] kobject_uevent+0x1f/0x30
[ 106.484081][ T7002] device_del+0x673/0xaf0
[ 106.486758][ T7002] ? __device_links_no_driver+0x230/0x230
[ 106.489994][ T7002] ? trace_hardirqs_on+0x52/0x1d0
[ 106.492762][ T7002] __input_unregister_device+0x379/0x480
[ 106.495713][ T7002] ? kasan_check_read+0x11/0x20
[ 106.498448][ T7002] input_unregister_device+0xa4/0xe0
[ 106.501178][ T7002] uinput_destroy_device+0x1e3/0x240
[ 106.504219][ T7002] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 106.507369][ T7002] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 106.510320][ T7002] ? kasan_check_write+0x14/0x20
[ 106.512855][ T7002] ? proc_fail_nth_write+0x94/0x1c0
[ 106.515504][ T7002] ? map_files_get_link+0x3c0/0x3c0
[ 106.518111][ T7002] ? __handle_mm_fault+0x20c0/0x3290
[ 106.520647][ T7002] ? map_files_get_link+0x3c0/0x3c0
[ 106.523352][ T7002] ? __vfs_write+0x111/0x7f0
[ 106.525728][ T7002] uinput_ioctl+0x4c/0x60
[ 106.527934][ T7002] ? uinput_compat_ioctl+0x80/0x80
[ 106.530418][ T7002] do_vfs_ioctl+0x1a9/0x1100
[ 106.532667][ T7002] ? ioctl_preallocate+0x1e0/0x1e0
[ 106.535138][ T7002] ? lock_downgrade+0x880/0x880
[ 106.537821][ T7002] ? check_preemption_disabled+0x3b/0x240
[ 106.541370][ T7002] ? __sb_end_write+0xc6/0x100
[ 106.544464][ T7002] ? vfs_write+0x224/0x4d0
[ 106.547441][ T7002] ? kasan_check_read+0x11/0x20
[ 106.550570][ T7002] ? security_file_ioctl+0x87/0xb0
[ 106.553765][ T7002] ksys_ioctl+0x94/0xb0
[ 106.556910][ T7002] __x64_sys_ioctl+0x73/0xb0
[ 106.559835][ T7002] do_syscall_64+0xe7/0x570
[ 106.562861][ T7002] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 106.566361][ T7002] RIP: 0033:0x7f5185ef6839
[ 106.569153][ T7002] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 106.577796][ T7002] RSP: 002b:00007ffc7a800078 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 106.581248][ T7002] RAX: ffffffffffffffda RBX: 00007ffc7a800080 RCX: 00007f5185ef6839
[ 106.584590][ T7002] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 106.588145][ T7002] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007f51861d0038
[ 106.591466][ T7002] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 106.594767][ T7002] R13: 00007ffc7a800190 R14: 0000000000000000 R15: 0000000000000000
[ 106.598818][ T7002] kobject: 'input22' (00000000e09f1862): kobject_cleanup, parent (null)
[ 106.603916][ T7002] kobject: 'input22' (00000000e09f1862): auto cleanup 'remove' event
[ 106.607654][ T7002] kobject: 'input22' (00000000e09f1862): kobject_uevent_env
[ 106.610810][ T7002] kobject: 'input22' (00000000e09f1862): fill_kobj_path: path = '/input22'
[ 106.614563][ T7002] WARNING: CPU: 0 PID: 7002 at drivers/input/input.c:1597 input_dev_uevent+0x5a2/0x7b0
[ 106.619879][ T7002] Modules linked in:
[ 106.622530][ T7002] CPU: 0 PID: 7002 Comm: a.out Not tainted 5.0.0-rc3+ #143
[ 106.625853][ T7002] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 04/13/2018
[ 106.631813][ T7002] RIP: 0010:input_dev_uevent+0x5a2/0x7b0
[ 106.634771][ T7002] Code: e8 53 5b 25 fd 44 8b 4d d4 e9 03 fb ff ff e8 45 5b 25 fd e9 c2 fa ff ff 4c 89 ef e8 58 5b 25 fd e9 52 fc ff ff e8 6e 62 f3 fc <0f> 0b e9 51 fc ff ff e8 62 62 f3 fc 49 8d 54 24 30 b9 ff 02 00 00
[ 106.644676][ T7002] RSP: 0018:ffff8881d14af828 EFLAGS: 00010293
[ 106.648039][ T7002] RAX: ffff8881d287c300 RBX: ffff8881f06ab748 RCX: ffffffff8475e852
[ 106.651852][ T7002] RDX: 0000000000000000 RSI: 0000000000000004 RDI: ffff8881f06ab790
[ 106.655867][ T7002] RBP: ffff8881d14af860 R08: ffffed103e0d56f3 R09: ffffed103e0d56f3
[ 106.659628][ T7002] R10: 0000000000000001 R11: ffffed103e0d56f2 R12: ffff8881f06ab480
[ 106.663397][ T7002] R13: ffff8881f06ab790 R14: ffff8881df8f6640 R15: ffff8881f06ab748
[ 106.667175][ T7002] FS: 00007f51863e6500(0000) GS:ffff8881f5a00000(0000) knlGS:0000000000000000
[ 106.671480][ T7002] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 106.674937][ T7002] CR2: 00007f5185f0ce70 CR3: 00000001f075d006 CR4: 00000000003606f0
[ 106.678832][ T7002] Call Trace:
[ 106.681490][ T7002] ? input_add_uevent_bm_var+0x130/0x130
[ 106.684669][ T7002] dev_uevent+0x330/0x5e0
[ 106.687697][ T7002] ? device_get_devnode+0x2e0/0x2e0
[ 106.690950][ T7002] ? vprintk_func+0x68/0x190
[ 106.693977][ T7002] ? add_uevent_var+0x20c/0x2f0
[ 106.696886][ T7002] ? cleanup_uevent_env+0x50/0x50
[ 106.699818][ T7002] ? kobject_uevent_env+0x347/0x1150
[ 106.702810][ T7002] ? device_get_devnode+0x2e0/0x2e0
[ 106.705952][ T7002] kobject_uevent_env+0x487/0x1150
[ 106.708937][ T7002] ? kmsg_dump_rewind_nolock+0xe4/0xe4
[ 106.711886][ T7002] kobject_uevent+0x1f/0x30
[ 106.714689][ T7002] kobject_put+0x33c/0x400
[ 106.717543][ T7002] put_device+0x20/0x30
[ 106.720071][ T7002] input_unregister_device+0xba/0xe0
[ 106.723306][ T7002] uinput_destroy_device+0x1e3/0x240
[ 106.726319][ T7002] uinput_ioctl_handler.isra.10+0xf63/0x1940
[ 106.729383][ T7002] ? uinput_request_submit.part.9+0x2b0/0x2b0
[ 106.732442][ T7002] ? kasan_check_write+0x14/0x20
[ 106.735090][ T7002] ? proc_fail_nth_write+0x94/0x1c0
[ 106.738758][ T7002] ? map_files_get_link+0x3c0/0x3c0
[ 106.741915][ T7002] ? __handle_mm_fault+0x20c0/0x3290
[ 106.744697][ T7002] ? map_files_get_link+0x3c0/0x3c0
[ 106.747313][ T7002] ? __vfs_write+0x111/0x7f0
[ 106.749911][ T7002] uinput_ioctl+0x4c/0x60
[ 106.752365][ T7002] ? uinput_compat_ioctl+0x80/0x80
[ 106.755102][ T7002] do_vfs_ioctl+0x1a9/0x1100
[ 106.757552][ T7002] ? ioctl_preallocate+0x1e0/0x1e0
[ 106.760368][ T7002] ? lock_downgrade+0x880/0x880
[ 106.762722][ T7002] ? check_preemption_disabled+0x3b/0x240
[ 106.765380][ T7002] ? __sb_end_write+0xc6/0x100
[ 106.767791][ T7002] ? vfs_write+0x224/0x4d0
[ 106.769997][ T7002] ? kasan_check_read+0x11/0x20
[ 106.772590][ T7002] ? security_file_ioctl+0x87/0xb0
[ 106.775012][ T7002] ksys_ioctl+0x94/0xb0
[ 106.777240][ T7002] __x64_sys_ioctl+0x73/0xb0
[ 106.779549][ T7002] do_syscall_64+0xe7/0x570
[ 106.781756][ T7002] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 106.784388][ T7002] RIP: 0033:0x7f5185ef6839
[ 106.786485][ T7002] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
[ 106.795212][ T7002] RSP: 002b:00007ffc7a800078 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 106.798602][ T7002] RAX: ffffffffffffffda RBX: 00007ffc7a800080 RCX: 00007f5185ef6839
[ 106.801823][ T7002] RDX: 0000000000000001 RSI: 0000000000005502 RDI: 0000000000000004
[ 106.805440][ T7002] RBP: 0000000000000005 R08: 0000000000000000 R09: 00007f51861d0038
[ 106.808818][ T7002] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
[ 106.812112][ T7002] R13: 00007ffc7a800190 R14: 0000000000000000 R15: 0000000000000000
[ 106.815416][ T7002] irq event stamp: 2868
[ 106.817657][ T7002] hardirqs last enabled at (2867): [<ffffffff81552310>] console_unlock+0x670/0xd00
[ 106.822995][ T7002] hardirqs last disabled at (2868): [<ffffffff81006087>] trace_hardirqs_off_thunk+0x1a/0x1c
[ 106.828472][ T7002] softirqs last enabled at (2864): [<ffffffff86c00673>] __do_softirq+0x673/0x987
[ 106.833767][ T7002] softirqs last disabled at (2855): [<ffffffff8140c0f5>] irq_exit+0x195/0x1c0
[ 106.837681][ T7002] ---[ end trace 781f472231e721f9 ]---
[ 106.841772][ T7002] kobject: 'input22' (00000000e09f1862): calling ktype release
[ 106.845524][ T7002] kobject: 'input22': free name
----------------------------------------
>From ea8886899e01184801e4c2db2a21892b6006c2c4 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Fri, 25 Jan 2019 14:12:58 +0900
Subject: [PATCH] Input: uinput - Set name/phys to NULL after kfree().
syzbot is hitting use-after-free bug in uinput module [1]. This is because
uinput_destroy_device() sometimes kfree()s dev->name and dev->phys at
uinput_destroy_device() before dev_uevent() is triggered by dropping the
refcount to 0. Since the timing of triggering last input_put_device() is
uncontrollable, this patch prepares for such race by setting dev->name and
dev->phys to NULL before doing operations which might drop the refcount
to 0.
[1] https://syzkaller.appspot.com/bug?id=8b17c134fe938bbddd75a45afaa9e68af43a362d
Reported-by: syzbot <syzbot+f648cfb7e0b52bf7ae32@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/input/misc/uinput.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8ec483e8688b..131591b5babd 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -300,7 +300,9 @@ static void uinput_destroy_device(struct uinput_device *udev)
if (dev) {
name = dev->name;
+ dev->name = NULL;
phys = dev->phys;
+ dev->phys = NULL;
if (old_state == UIST_CREATED) {
uinput_flush_requests(udev);
input_unregister_device(dev);
--
2.17.1
^ permalink raw reply related
* [PATCH v2] drm/bridge: sil_sii8620: make remote control optional.
From: Ronald Tschalär @ 2019-01-25 1:33 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae, Laurent Pinchart, Dmitry Torokhov
Cc: Lukas Wunner, dri-devel, linux-input, linux-kernel
In-Reply-To: <20190124072125.GA28127@innovation.ch>
commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
of RC_CORE) changed the driver to select both RC_CORE and INPUT.
However, this causes problems with other drivers, in particular an input
driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
commit):
drivers/clk/Kconfig:9:error: recursive dependency detected!
drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
According to the docs and general consensus, select should only be used
for non user-visible symbols, but both RC_CORE and INPUT are
user-visible. Furthermore almost all other references to INPUT
throughout the kernel config are depends, not selects. For this reason
the first part of this change reverts commit d6abe6df706c.
In order to address the original reason for commit d6abe6df706c, namely
that not all boards use the remote controller functionality and hence
should not need have to deal with RC_CORE, the second part of this
change now makes the remote control support in the driver optional and
contingent on RC_CORE being defined. And with this the hard dependency
on INPUT also goes away as that is only needed if RC_CORE is defined
(which in turn already depends on INPUT).
CC: Inki Dae <inki.dae@samsung.com>
CC: Andrzej Hajda <a.hajda@samsung.com>
CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
Resending this, as I somehow managed to forget to cc dri-devel.
Apologies for the duplication.
Changes in v2:
- completely remove dependencies on both RC_CORE and INPUT in Kconfig,
- make remote control functionality in driver contingent on RC_CORE
being defined
drivers/gpu/drm/bridge/Kconfig | 2 --
drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++++++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..a11198a36005 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -85,8 +85,6 @@ config DRM_SIL_SII8620
depends on OF
select DRM_KMS_HELPER
imply EXTCON
- select INPUT
- select RC_CORE
help
Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 0cc293a6ac24..dee47752791e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -66,7 +66,9 @@ enum sii8620_mt_state {
struct sii8620 {
struct drm_bridge bridge;
struct device *dev;
+#if IS_ENABLED(CONFIG_RC_CORE)
struct rc_dev *rc_dev;
+#endif
struct clk *clk_xtal;
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_int;
@@ -1756,6 +1758,7 @@ static void sii8620_send_features(struct sii8620 *ctx)
sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
}
+#if IS_ENABLED(CONFIG_RC_CORE)
static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
{
bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
@@ -1774,6 +1777,12 @@ static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
return true;
}
+#else
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
+{
+ return false;
+}
+#endif
static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
{
@@ -2097,6 +2106,7 @@ static void sii8620_cable_in(struct sii8620 *ctx)
enable_irq(to_i2c_client(ctx->dev)->irq);
}
+#if IS_ENABLED(CONFIG_RC_CORE)
static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
{
struct rc_dev *rc_dev;
@@ -2126,6 +2136,11 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
}
ctx->rc_dev = rc_dev;
}
+#else
+static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
+{
+}
+#endif
static void sii8620_cable_out(struct sii8620 *ctx)
{
@@ -2214,9 +2229,11 @@ static int sii8620_attach(struct drm_bridge *bridge)
static void sii8620_detach(struct drm_bridge *bridge)
{
+#if IS_ENABLED(CONFIG_RC_CORE)
struct sii8620 *ctx = bridge_to_sii8620(bridge);
rc_unregister_device(ctx->rc_dev);
+#endif
}
static int sii8620_is_packing_required(struct sii8620 *ctx,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2] HID: intel-ish-hid: Switch to use new generic UUID API
From: Jiri Kosina @ 2019-01-24 22:00 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Srinivas Pandruvada, linux-input, Even Xu, linux-kernel,
Christoph Hellwig, Benjamin Tissoires
In-Reply-To: <20190124200906.60296-1-andriy.shevchenko@linux.intel.com>
On Thu, 24 Jan 2019, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new code.
>
> As a preparation to get rid of legacy types and API functions do
> the conversion here.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied to for-5.1/ish branch, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v2 2/2] input: misc: pwm-vibra: Stop regulator after disabling pwm, not before
From: Paweł Chmiel @ 2019-01-24 20:27 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Paweł Chmiel
In-Reply-To: <20190124202732.14723-1-pawel.mikolaj.chmiel@gmail.com>
This patch fixes order of disable calls in pwm_vibrator_stop.
Currently when starting device, we first enable vcc regulator and then
setup and enable pwm. When stopping, we should do this in oposite order,
so first disable pwm and then disable regulator.
Previously order was the same as in start.
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
drivers/input/misc/pwm-vibra.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/input/misc/pwm-vibra.c b/drivers/input/misc/pwm-vibra.c
index 9df87431d7d4..dbb6d9e1b947 100644
--- a/drivers/input/misc/pwm-vibra.c
+++ b/drivers/input/misc/pwm-vibra.c
@@ -80,14 +80,14 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
static void pwm_vibrator_stop(struct pwm_vibrator *vibrator)
{
+ if (vibrator->pwm_dir)
+ pwm_disable(vibrator->pwm_dir);
+ pwm_disable(vibrator->pwm);
+
if (vibrator->vcc_on) {
regulator_disable(vibrator->vcc);
vibrator->vcc_on = false;
}
-
- if (vibrator->pwm_dir)
- pwm_disable(vibrator->pwm_dir);
- pwm_disable(vibrator->pwm);
}
static void pwm_vibrator_play_work(struct work_struct *work)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/2] input: misc: pwm-vibra: Prevent unbalanced regulator
From: Paweł Chmiel @ 2019-01-24 20:27 UTC (permalink / raw)
To: dmitry.torokhov
Cc: linux-input, linux-kernel, Jonathan Bakker, Paweł Chmiel
From: Jonathan Bakker <xc-racer2@live.ca>
pwm_vibrator_stop disables the regulator, but it can be called from
multiple places, even when the regulator is already disabled. Fix this
by using regulator_is_enabled check when starting and stopping device.
Signed-off-by: Jonathan Bakker <xc-racer2@live.ca>
Signed-off-by: Paweł Chmiel <pawel.mikolaj.chmiel@gmail.com>
---
Changes from v1:
- Rather than using regulator_is_enabled api, use local flag for
checking if regulator is enabled
---
drivers/input/misc/pwm-vibra.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/pwm-vibra.c b/drivers/input/misc/pwm-vibra.c
index 55da191ae550..9df87431d7d4 100644
--- a/drivers/input/misc/pwm-vibra.c
+++ b/drivers/input/misc/pwm-vibra.c
@@ -34,6 +34,7 @@ struct pwm_vibrator {
struct work_struct play_work;
u16 level;
u32 direction_duty_cycle;
+ bool vcc_on;
};
static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
@@ -42,10 +43,13 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
struct pwm_state state;
int err;
- err = regulator_enable(vibrator->vcc);
- if (err) {
- dev_err(pdev, "failed to enable regulator: %d", err);
- return err;
+ if (!vibrator->vcc_on) {
+ err = regulator_enable(vibrator->vcc);
+ if (err) {
+ dev_err(pdev, "failed to enable regulator: %d", err);
+ return err;
+ }
+ vibrator->vcc_on = true;
}
pwm_get_state(vibrator->pwm, &state);
@@ -76,7 +80,10 @@ static int pwm_vibrator_start(struct pwm_vibrator *vibrator)
static void pwm_vibrator_stop(struct pwm_vibrator *vibrator)
{
- regulator_disable(vibrator->vcc);
+ if (vibrator->vcc_on) {
+ regulator_disable(vibrator->vcc);
+ vibrator->vcc_on = false;
+ }
if (vibrator->pwm_dir)
pwm_disable(vibrator->pwm_dir);
--
2.17.1
^ permalink raw reply related
* [PATCH v2] HID: intel-ish-hid: Switch to use new generic UUID API
From: Andy Shevchenko @ 2019-01-24 20:09 UTC (permalink / raw)
To: Srinivas Pandruvada, linux-input, Even Xu, linux-kernel,
Christoph Hellwig, Benjamin Tissoires, Jiri Kosina
Cc: Andy Shevchenko
There are new types and helpers that are supposed to be used in new code.
As a preparation to get rid of legacy types and API functions do
the conversion here.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v2:
- split long line
- append tags
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 4 ++--
drivers/hid/intel-ish-hid/ishtp-hid.h | 6 +++---
drivers/hid/intel-ish-hid/ishtp/bus.c | 21 +++++++++-----------
drivers/hid/intel-ish-hid/ishtp/bus.h | 4 ++--
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
drivers/hid/intel-ish-hid/ishtp/hbm.h | 2 +-
6 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index e64243bc9c96..30fe0c5e6fad 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -788,8 +788,8 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
if (!cl_device)
return -ENODEV;
- if (uuid_le_cmp(hid_ishtp_guid,
- cl_device->fw_client->props.protocol_name) != 0)
+ if (!guid_equal(&hid_ishtp_guid,
+ &cl_device->fw_client->props.protocol_name))
return -ENODEV;
client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index f5c7eb79b7b5..1cd07a441cd4 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -29,9 +29,9 @@
client->cl_device->ishtp_dev, __VA_ARGS__)
/* ISH Transport protocol (ISHTP in short) GUID */
-static const uuid_le hid_ishtp_guid = UUID_LE(0x33AECD58, 0xB679, 0x4E54,
- 0x9B, 0xD9, 0xA0, 0x4D, 0x34,
- 0xF0, 0xC2, 0x26);
+static const guid_t hid_ishtp_guid =
+ GUID_INIT(0x33AECD58, 0xB679, 0x4E54,
+ 0x9B, 0xD9, 0xA0, 0x4D, 0x34, 0xF0, 0xC2, 0x26);
/* ISH HID message structure */
struct hostif_msg_hdr {
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 728dc6d4561a..efa21d33ad60 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -133,18 +133,15 @@ int ishtp_write_message(struct ishtp_device *dev, struct ishtp_msg_hdr *hdr,
*
* Return: fw client index or -ENOENT if not found
*/
-int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le *uuid)
+int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *uuid)
{
- int i, res = -ENOENT;
+ unsigned int i;
for (i = 0; i < dev->fw_clients_num; ++i) {
- if (uuid_le_cmp(*uuid, dev->fw_clients[i].props.protocol_name)
- == 0) {
- res = i;
- break;
- }
+ if (guid_equal(uuid, &dev->fw_clients[i].props.protocol_name))
+ return i;
}
- return res;
+ return -ENOENT;
}
EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
@@ -158,7 +155,7 @@ EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
* Return: pointer of client information on success, NULL on failure.
*/
struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
- const uuid_le *uuid)
+ const guid_t *uuid)
{
int i;
unsigned long flags;
@@ -401,7 +398,7 @@ static const struct device_type ishtp_cl_device_type = {
* Return: ishtp_cl_device pointer or NULL on failure
*/
static struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev,
- uuid_le uuid, char *name)
+ guid_t uuid, char *name)
{
struct ishtp_cl_device *device;
int status;
@@ -629,7 +626,7 @@ int ishtp_bus_new_client(struct ishtp_device *dev)
int i;
char *dev_name;
struct ishtp_cl_device *cl_device;
- uuid_le device_uuid;
+ guid_t device_uuid;
/*
* For all reported clients, create an unconnected client and add its
@@ -639,7 +636,7 @@ int ishtp_bus_new_client(struct ishtp_device *dev)
*/
i = dev->fw_client_presentation_num - 1;
device_uuid = dev->fw_clients[i].props.protocol_name;
- dev_name = kasprintf(GFP_KERNEL, "{%pUL}", device_uuid.b);
+ dev_name = kasprintf(GFP_KERNEL, "{%pUL}", &device_uuid);
if (!dev_name)
return -ENOMEM;
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index b8a5bcc82536..babf19ba3ff6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -112,8 +112,8 @@ void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
int ishtp_register_event_cb(struct ishtp_cl_device *device,
void (*read_cb)(struct ishtp_cl_device *));
-int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le *cuuid);
+int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
- const uuid_le *uuid);
+ const guid_t *uuid);
#endif /* _LINUX_ISHTP_CL_BUS_H */
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index 042f4c4853b1..e0df3eb611e6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -126,7 +126,7 @@ struct ishtp_cl {
};
/* Client connection managenment internal functions */
-int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, uuid_le *uuid);
+int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, guid_t *uuid);
int ishtp_fw_cl_by_id(struct ishtp_device *dev, uint8_t client_id);
void ishtp_cl_send_msg(struct ishtp_device *dev, struct ishtp_cl *cl);
void recv_ishtp_cl_msg(struct ishtp_device *dev,
diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.h b/drivers/hid/intel-ish-hid/ishtp/hbm.h
index d96111cef7f8..7286e3600140 100644
--- a/drivers/hid/intel-ish-hid/ishtp/hbm.h
+++ b/drivers/hid/intel-ish-hid/ishtp/hbm.h
@@ -149,7 +149,7 @@ struct hbm_host_enum_response {
} __packed;
struct ishtp_client_properties {
- uuid_le protocol_name;
+ guid_t protocol_name;
uint8_t protocol_version;
uint8_t max_number_of_connections;
uint8_t fixed_address;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 3/3] Revert "dt-bindings: marvell,mmp2: Add clock id for the SP clock"
From: Stephen Boyd @ 2019-01-24 18:56 UTC (permalink / raw)
To: Dmitry Torokhov, Michael Turquette
Cc: Rob Herring, Mark Rutland, linux-input, devicetree, linux-kernel,
linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-4-lkundrak@v3.sk>
Quoting Lubomir Rintel (2019-01-20 22:22:56)
> It seems that the kernel has no business managing this clock: once the SP
> clock is disabled, it's not sufficient to just enable it in order to bring
> the SP core back up.
>
> Pretty sure nothing ever used this and it's safe to remove.
>
> This reverts commit e8a2c779141415105825e65a4715f1130bba61b1.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
Applied to clk-fixes
^ permalink raw reply
* Re: [PATCH 2/3] Revert "clk: mmp2: add SP clock"
From: Stephen Boyd @ 2019-01-24 18:56 UTC (permalink / raw)
To: Dmitry Torokhov, Michael Turquette
Cc: Rob Herring, Mark Rutland, linux-input, devicetree, linux-kernel,
linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-3-lkundrak@v3.sk>
Quoting Lubomir Rintel (2019-01-20 22:22:55)
> It seems that the kernel has no business managing this clock: once the SP
> clock is disabled, it's not sufficient to just enable in order to bring the
> SP core back up. Just let the firmware keep it enabled and don't expose it
> to drivers.
>
> This reverts commit fc27c2394d96fd19854b7e2d3f0e60df0d86fc90.
>
> Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
Applied to clk-fixes
^ permalink raw reply
* Re: [PATCH 1/3] Revert "Input: olpc_apsp - enable the SP clock"
From: Stephen Boyd @ 2019-01-24 18:55 UTC (permalink / raw)
To: Dmitry Torokhov, Michael Turquette
Cc: Rob Herring, Mark Rutland, linux-input, devicetree, linux-kernel,
linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-2-lkundrak@v3.sk>
Quoting Lubomir Rintel (2019-01-20 22:22:54)
> Turns out this is not such a great idea. Once the SP clock is disabled,
> it's not sufficient to just enable in order to bring the SP core back up.
>
> It seems that the kernel has no business managing this clock. Just let
> the firmware keep it enabled.
>
> This reverts commit ed22cee91a88c47e564478b012fdbcb079653499.
>
> Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
Applied to clk-fixes
^ permalink raw reply
* Re: [PATCH 1/3] Revert "Input: olpc_apsp - enable the SP clock"
From: Stephen Boyd @ 2019-01-24 18:53 UTC (permalink / raw)
To: Dmitry Torokhov, Lubomir Rintel
Cc: Michael Turquette, Rob Herring, Mark Rutland, linux-input,
devicetree, linux-kernel, linux-clk
In-Reply-To: <20190123220545.GD179701@dtor-ws>
Quoting Dmitry Torokhov (2019-01-23 14:05:45)
> On Mon, Jan 21, 2019 at 07:22:54AM +0100, Lubomir Rintel wrote:
> > Turns out this is not such a great idea. Once the SP clock is disabled,
> > it's not sufficient to just enable in order to bring the SP core back up.
> >
> > It seems that the kernel has no business managing this clock. Just let
> > the firmware keep it enabled.
> >
> > This reverts commit ed22cee91a88c47e564478b012fdbcb079653499.
> >
> > Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> OK, as clock folks say it is a bad idea it must be so. Merge though clk
> tree?
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
Ok sure. I'll merge them through clk tree. I'll fast track it through
clk-fixes too.
^ permalink raw reply
* Re: [PATCH] HID: elan: Make array buf static, shrinks object size
From: Colin Ian King @ 2019-01-24 16:54 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180926084150.17514-1-colin.king@canonical.com>
ping?
On 26/09/2018 09:41, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate the array buf on the stack but instead make it
> static. Makes the object code smaller by 43 bytes:
>
> Before:
> text data bss dec hex filename
> 7769 1520 0 9289 2449 drivers/hid/hid-elan.o
>
> After:
> text data bss dec hex filename
> 7662 1584 0 9246 241e drivers/hid/hid-elan.o
>
> (gcc version 8.2.0 x86_64)
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/hid/hid-elan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
> index 07e26c3567eb..05377eec2cb2 100644
> --- a/drivers/hid/hid-elan.c
> +++ b/drivers/hid/hid-elan.c
> @@ -393,7 +393,7 @@ static int elan_start_multitouch(struct hid_device *hdev)
> * This byte sequence will enable multitouch mode and disable
> * mouse emulation
> */
> - const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
> + static const unsigned char buf[] = { 0x0D, 0x00, 0x03, 0x21, 0x00 };
> unsigned char *dmabuf = kmemdup(buf, sizeof(buf), GFP_KERNEL);
>
> if (!dmabuf)
>
^ permalink raw reply
* Re: [PATCH 10/13] gpio: max77650: add GPIO support
From: Linus Walleij @ 2019-01-24 10:30 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Brian Masney, Rob Herring, Mark Rutland, Dmitry Torokhov,
Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
Liam Girdwood, Mark Brown, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Input, Linux LED Subsystem, Linux PM list
In-Reply-To: <CAMRc=Me6OSKDk7bN4++M68DjoeArd+gZx16o0oQBXfRoCWBt2A@mail.gmail.com>
On Mon, Jan 21, 2019 at 6:07 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> Thank you for your review. While I think you're right about the issue
> being present in this driver, I'm not sure it's really a problem. Do
> we actually require every gpio-controller to also be a stand-alone
> interrupt-controller?
Absolutely not :D
Just GPIO is fine.
> The binding document for the GPIO module doesn't
> mention this - it only requires the gpio-controller property. Without
> the "interrupt-controller" property dtc will bail-out if anyone uses
> this node as the interrupt parent.
>
> If I'm wrong and we do require it, then I think we need to update
> Documentation/devicetree/bindings/gpio/gpio.txt.
What is weird is if a driver with DT bindings not mentioning IRQ
and only probing from DT start implementing IRQ support, that
becomes quite inconsistent. So then max77650_gpio_to_irq()
should just return -ENOTSUPP
or something for now, then it's fine.
We can add the (complicated) IRQ handling later.
I am trying to eat my own dogfood here, I was sweating all
last night trying to implement a hierarchical IRQ controller.
There is no running away from that now. :/
Apparently doing hierarchical IRQs demand that all irq
controllers up to the top-level SoC IRQ controller support
hierarchical interrupts using the v2 version of the irqdomain
API, and currently it seems like the ARM
GIC seems like the only top level IRQ controller that can
do that.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v2 2/2] Input: st1232 - add support for st1633
From: Martin Kepplinger @ 2019-01-24 10:21 UTC (permalink / raw)
To: devicetree, linux-input
Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel, Martin Kepplinger
In-Reply-To: <20190124102125.25458-1-martink@posteo.de>
From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
Add support for the Sitronix ST1633 touchscreen controller to the st1232
driver. A protocol spec can be found here:
www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
while this works, I'm not totally convinced by how the i2c path of probe
looks. what do you say?
thanks!
revision history
----------------
v2: use device_get_match_data(), invent an internal "have_z" bool property
v1: initial idea for the change
drivers/input/touchscreen/Kconfig | 6 +-
drivers/input/touchscreen/st1232.c | 126 ++++++++++++++++++++++-------
2 files changed, 98 insertions(+), 34 deletions(-)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 068dbbc610fc..7c597a49c265 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
module will be called sis_i2c.
config TOUCHSCREEN_ST1232
- tristate "Sitronix ST1232 touchscreen controllers"
+ tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
depends on I2C
help
- Say Y here if you want to support Sitronix ST1232
- touchscreen controller.
+ Say Y here if you want to support the Sitronix ST1232
+ or ST1633 touchscreen controller.
If unsure, say N.
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 11ff32c68025..f12ef666e2c8 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -18,18 +18,21 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
+#include <linux/of_device.h>
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
#define ST1232_TS_NAME "st1232-ts"
+#define ST1633_TS_NAME "st1633-ts"
+
+enum {
+ st1232,
+ st1633,
+};
#define MIN_X 0x00
#define MIN_Y 0x00
-#define MAX_X 0x31f /* (800 - 1) */
-#define MAX_Y 0x1df /* (480 - 1) */
-#define MAX_AREA 0xff
-#define MAX_FINGERS 2
struct st1232_ts_finger {
u16 x;
@@ -38,12 +41,24 @@ struct st1232_ts_finger {
bool is_valid;
};
+struct st_chip_info {
+ bool have_z;
+ u16 max_x;
+ u16 max_y;
+ u16 max_area;
+ u16 max_fingers;
+ u8 start_reg;
+};
+
struct st1232_ts_data {
struct i2c_client *client;
struct input_dev *input_dev;
- struct st1232_ts_finger finger[MAX_FINGERS];
struct dev_pm_qos_request low_latency_req;
int reset_gpio;
+ const struct st_chip_info *chip_info;
+ int read_buf_len;
+ u8 *read_buf;
+ struct st1232_ts_finger *finger;
};
static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -52,40 +67,35 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
struct i2c_client *client = ts->client;
struct i2c_msg msg[2];
int error;
- u8 start_reg;
- u8 buf[10];
+ int i, y;
+ u8 start_reg = ts->chip_info->start_reg;
+ u8 *buf = ts->read_buf;
- /* read touchscreen data from ST1232 */
+ /* read touchscreen data */
msg[0].addr = client->addr;
msg[0].flags = 0;
msg[0].len = 1;
msg[0].buf = &start_reg;
- start_reg = 0x10;
msg[1].addr = ts->client->addr;
msg[1].flags = I2C_M_RD;
- msg[1].len = sizeof(buf);
+ msg[1].len = ts->read_buf_len;
msg[1].buf = buf;
error = i2c_transfer(client->adapter, msg, 2);
if (error < 0)
return error;
- /* get "valid" bits */
- finger[0].is_valid = buf[2] >> 7;
- finger[1].is_valid = buf[5] >> 7;
+ for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
+ finger[i].is_valid = buf[i + y] >> 7;
+ if (finger[i].is_valid) {
+ finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
+ finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
- /* get xy coordinate */
- if (finger[0].is_valid) {
- finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
- finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
- finger[0].t = buf[8];
- }
-
- if (finger[1].is_valid) {
- finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
- finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
- finger[1].t = buf[9];
+ /* st1232 includes a z-axis / touch strength */
+ if (ts->chip_info->have_z)
+ finger[i].t = buf[i + 6];
+ }
}
return 0;
@@ -104,11 +114,14 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
goto end;
/* multi touch protocol */
- for (i = 0; i < MAX_FINGERS; i++) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++) {
if (!finger[i].is_valid)
continue;
- input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
+ if (ts->chip_info->have_z)
+ input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
+ finger[i].t);
+
input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
input_mt_sync(input_dev);
@@ -142,12 +155,43 @@ static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
gpio_direction_output(ts->reset_gpio, poweron);
}
+static const struct st_chip_info st1232_chip_info = {
+ .have_z = true,
+ .max_x = 0x31f, /* 800 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0xff,
+ .max_fingers = 2,
+ .start_reg = 0x12,
+};
+
+static const struct st_chip_info st1633_chip_info = {
+ .have_z = false,
+ .max_x = 0x13f, /* 320 - 1 */
+ .max_y = 0x1df, /* 480 -1 */
+ .max_area = 0x00,
+ .max_fingers = 5,
+ .start_reg = 0x12,
+};
+
static int st1232_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct st1232_ts_data *ts;
+ struct st1232_ts_finger *finger;
struct input_dev *input_dev;
int error;
+ const struct st_chip_info *match = NULL;
+
+ if (dev_fwnode(&client->dev))
+ match = device_get_match_data(&client->dev);
+ else if (id && id->driver_data == st1232)
+ match = &st1232_chip_info;
+ else if (id && id->driver_data == st1633)
+ match = &st1633_chip_info;
+ if (!match) {
+ dev_err(&client->dev, "unknown device model\n");
+ return -ENODEV;
+ }
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
dev_err(&client->dev, "need I2C_FUNC_I2C\n");
@@ -163,6 +207,19 @@ static int st1232_ts_probe(struct i2c_client *client,
if (!ts)
return -ENOMEM;
+ ts->chip_info = match;
+ ts->finger = devm_kzalloc(&client->dev,
+ sizeof(*finger) * ts->chip_info->max_fingers,
+ GFP_KERNEL);
+ if (!ts->finger)
+ return -ENOMEM;
+
+ /* allocate a buffer according to the number of registers to read */
+ ts->read_buf_len = ts->chip_info->max_fingers * 4;
+ ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len, GFP_KERNEL);
+ if (!ts->read_buf)
+ return -ENOMEM;
+
input_dev = devm_input_allocate_device(&client->dev);
if (!input_dev)
return -ENOMEM;
@@ -192,9 +249,14 @@ static int st1232_ts_probe(struct i2c_client *client,
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0);
+ if (ts->chip_info->have_z)
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
+ ts->chip_info->max_area, 0, 0);
+
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+ MIN_X, ts->chip_info->max_x, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+ MIN_Y, ts->chip_info->max_y, 0, 0);
error = devm_request_threaded_irq(&client->dev, client->irq,
NULL, st1232_ts_irq_handler,
@@ -261,13 +323,15 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
st1232_ts_suspend, st1232_ts_resume);
static const struct i2c_device_id st1232_ts_id[] = {
- { ST1232_TS_NAME, 0 },
+ { ST1232_TS_NAME, st1232 },
+ { ST1633_TS_NAME, st1633 },
{ }
};
MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
static const struct of_device_id st1232_ts_dt_ids[] = {
- { .compatible = "sitronix,st1232", },
+ { .compatible = "sitronix,st1232", .data = &st1232_chip_info },
+ { .compatible = "sitronix,st1633", .data = &st1633_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: input: sitronix-st1232: add compatible string for ST1633
From: Martin Kepplinger @ 2019-01-24 10:21 UTC (permalink / raw)
To: devicetree, linux-input
Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel, Martin Kepplinger
From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
The st1232 driver gains support for the ST1633 controller too; update
the bindings doc accordingly.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
.../bindings/input/touchscreen/sitronix-st1232.txt | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
index 64ad48b824a2..e73e826e0f2a 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
@@ -1,7 +1,9 @@
-* Sitronix st1232 touchscreen controller
+* Sitronix st1232 or st1633 touchscreen controller
Required properties:
-- compatible: must be "sitronix,st1232"
+- compatible: must contain one of
+ * "sitronix,st1232"
+ * "sitronix,st1633"
- reg: I2C address of the chip
- interrupts: interrupt to which the chip is connected
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.
From: Lukas Wunner @ 2019-01-24 9:13 UTC (permalink / raw)
To: ronald, Laurent Pinchart, Dmitry Torokhov, Andrzej Hajda,
Inki Dae, dri-devel, linux-kernel, linux-input
In-Reply-To: <20190124072125.GA28127@innovation.ch>
On Wed, Jan 23, 2019 at 11:21:25PM -0800, Life is hard, and then you die wrote:
> Since the two changes (the change here + the new driver) seem to be
> best submitted through different trees, I'm trying to figure out how
> best to handle this. I suppose I could temporarily change the driver
> Kconfig to not trigger the conflict, and then once the change here has
> been upstreamed (not sure at what point exactly that would be
> considered the case, e.g. if in linux-next is sufficient, or has to
> wait for Linus' merge, or something else) submit another change to
> change the driver's Kconfig to the desired one.
If a series touches multiple subsystems and its patches are interdependent,
the pull requests sent to Linus would have to be merged in a specific order.
In practice that's too cumbersome, so either the series is split in multiple
parts and merged across multiple releases (which obviously can take a long
time) or, if the change to other subsystems is smallish (as is the case
here), the entire series is merged through a single subsystem tree and
those patches touching other subsystems need to have an Acked-by or
Reviewed-by tag from a maintainer of those other subsystems.
If a case can be made that the change to the other subsystem (e.g. DRM) is
actually a bug fix, then that change can go in immediately and will appear
in one of Linus' next -rc releases. The rest of the series can then go
through the appropriate subsystem (e.g. input) and will land in Linus' tree
in the next merge window.
Either way, the correct patch order is preserved and it's guaranteed that
the build is not broken for someone ending up on an in-between commit while
bisecting.
HTH,
Lukas
^ permalink raw reply
* Re: [PATCH 2/2] Input: st1232 - add support for st1633
From: Dmitry Torokhov @ 2019-01-24 8:48 UTC (permalink / raw)
To: Martin Kepplinger
Cc: Martin Kepplinger, devicetree, linux-input, robh+dt, mark.rutland,
chinyeow.sim.xt, linux-kernel
In-Reply-To: <df37b873-0641-c127-d6d8-5bd9ed155e87@ginzinger.com>
On Wed, Jan 23, 2019 at 11:07:55AM +0100, Martin Kepplinger wrote:
> On 23.01.19 08:26, Martin Kepplinger wrote:
> > From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> >
> > Add support for the Sitronix ST1633 touchscreen controller to the st1232
> > driver.
> >
>
> FYI, here's a protocol spec:
> www.ampdisplay.com/documents/pdf/AM-320480B6TZQW-TC0H.pdf
>
> Let me know if you want that link in the commit message.
Sure, please add it.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: st1232 - add support for st1633
From: Dmitry Torokhov @ 2019-01-24 8:48 UTC (permalink / raw)
To: Martin Kepplinger
Cc: devicetree, linux-input, robh+dt, mark.rutland, chinyeow.sim.xt,
linux-kernel, Martin Kepplinger
In-Reply-To: <20190123072651.31791-2-martink@posteo.de>
Hi Martin,
On Wed, Jan 23, 2019 at 08:26:51AM +0100, Martin Kepplinger wrote:
> From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
>
> Add support for the Sitronix ST1633 touchscreen controller to the st1232
> driver.
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
> drivers/input/touchscreen/Kconfig | 6 +-
> drivers/input/touchscreen/st1232.c | 130 +++++++++++++++++++++--------
> 2 files changed, 98 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 068dbbc610fc..7c597a49c265 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1168,11 +1168,11 @@ config TOUCHSCREEN_SIS_I2C
> module will be called sis_i2c.
>
> config TOUCHSCREEN_ST1232
> - tristate "Sitronix ST1232 touchscreen controllers"
> + tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
> depends on I2C
> help
> - Say Y here if you want to support Sitronix ST1232
> - touchscreen controller.
> + Say Y here if you want to support the Sitronix ST1232
> + or ST1633 touchscreen controller.
>
> If unsure, say N.
>
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index 11ff32c68025..dc0288f37fda 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -18,18 +18,21 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_gpio.h>
> +#include <linux/of_device.h>
> #include <linux/pm_qos.h>
> #include <linux/slab.h>
> #include <linux/types.h>
>
> #define ST1232_TS_NAME "st1232-ts"
> +#define ST1633_TS_NAME "st1633-ts"
> +
> +enum {
> + st1232,
> + st1633,
> +};
>
> #define MIN_X 0x00
> #define MIN_Y 0x00
> -#define MAX_X 0x31f /* (800 - 1) */
> -#define MAX_Y 0x1df /* (480 - 1) */
> -#define MAX_AREA 0xff
> -#define MAX_FINGERS 2
>
> struct st1232_ts_finger {
> u16 x;
> @@ -38,12 +41,24 @@ struct st1232_ts_finger {
> bool is_valid;
> };
>
> +struct st_chip_info {
> + u8 id;
It should be the enum, not u8, but I do not think you need id here at
all. You only check id when you decide whether to report
ABS_MT_TOUCH_MAJOR, I think having a boolean to indicate that fact would
be more in line with the spirit of this patch.
> + u16 max_x;
> + u16 max_y;
> + u16 max_area;
> + u16 max_fingers;
> + u8 start_reg;
> +};
> +
> struct st1232_ts_data {
> struct i2c_client *client;
> struct input_dev *input_dev;
> - struct st1232_ts_finger finger[MAX_FINGERS];
> struct dev_pm_qos_request low_latency_req;
> int reset_gpio;
> + const struct st_chip_info *chip_info;
> + int read_buf_len;
> + u8 *read_buf;
> + struct st1232_ts_finger *finger;
> };
>
> static int st1232_ts_read_data(struct st1232_ts_data *ts)
> @@ -52,40 +67,35 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts)
> struct i2c_client *client = ts->client;
> struct i2c_msg msg[2];
> int error;
> - u8 start_reg;
> - u8 buf[10];
> + int i, y;
> + u8 start_reg = ts->chip_info->start_reg;
> + u8 *buf = ts->read_buf;
>
> - /* read touchscreen data from ST1232 */
> + /* read touchscreen data */
> msg[0].addr = client->addr;
> msg[0].flags = 0;
> msg[0].len = 1;
> msg[0].buf = &start_reg;
> - start_reg = 0x10;
>
> msg[1].addr = ts->client->addr;
> msg[1].flags = I2C_M_RD;
> - msg[1].len = sizeof(buf);
> + msg[1].len = ts->read_buf_len;
> msg[1].buf = buf;
>
> error = i2c_transfer(client->adapter, msg, 2);
> if (error < 0)
> return error;
>
> - /* get "valid" bits */
> - finger[0].is_valid = buf[2] >> 7;
> - finger[1].is_valid = buf[5] >> 7;
> + for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) {
> + finger[i].is_valid = buf[i + y] >> 7;
> + if (finger[i].is_valid) {
> + finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1];
> + finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2];
>
> - /* get xy coordinate */
> - if (finger[0].is_valid) {
> - finger[0].x = ((buf[2] & 0x0070) << 4) | buf[3];
> - finger[0].y = ((buf[2] & 0x0007) << 8) | buf[4];
> - finger[0].t = buf[8];
> - }
> -
> - if (finger[1].is_valid) {
> - finger[1].x = ((buf[5] & 0x0070) << 4) | buf[6];
> - finger[1].y = ((buf[5] & 0x0007) << 8) | buf[7];
> - finger[1].t = buf[9];
> + /* st1232 includes a z-axis / touch strength */
> + if (ts->chip_info->id == st1232)
> + finger[i].t = buf[i + 6];
> + }
> }
>
> return 0;
> @@ -104,11 +114,14 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
> goto end;
>
> /* multi touch protocol */
> - for (i = 0; i < MAX_FINGERS; i++) {
> + for (i = 0; i < ts->chip_info->max_fingers; i++) {
> if (!finger[i].is_valid)
> continue;
>
> - input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, finger[i].t);
> + if (ts->chip_info->id == st1232)
> + input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR,
> + finger[i].t);
> +
> input_report_abs(input_dev, ABS_MT_POSITION_X, finger[i].x);
> input_report_abs(input_dev, ABS_MT_POSITION_Y, finger[i].y);
> input_mt_sync(input_dev);
> @@ -142,12 +155,46 @@ static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> gpio_direction_output(ts->reset_gpio, poweron);
> }
>
> +static const struct st_chip_info st_chip_info_table[] = {
Instead of array just define 2 separate structures.
> + [st1232] = {
> + .id = st1232,
> + .max_x = 0x31f, /* 800 - 1 */
> + .max_y = 0x1df, /* 480 -1 */
> + .max_area = 0xff,
> + .max_fingers = 2,
> + .start_reg = 0x12,
> + },
> + [st1633] = {
> + .id = st1633,
> + .max_x = 0x13f, /* 320 - 1 */
> + .max_y = 0x1df, /* 480 -1 */
> + .max_area = 0x00,
> + .max_fingers = 5,
> + .start_reg = 0x12,
> + },
> +};
> +
> +static const struct of_device_id st1232_ts_dt_ids[] = {
> + { .compatible = "sitronix,st1232", .data = &st_chip_info_table[st1232] },
> + { .compatible = "sitronix,st1633", .data = &st_chip_info_table[st1633] },
> + { }
> +};
No need to move the table here, you can use device_get_match_data() that
does not require supplying table. This will also transparently handle
non-OF case (but you need to add data to i2c ids as well).
> +MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
> +
> static int st1232_ts_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct st1232_ts_data *ts;
> + struct st1232_ts_finger *finger;
> struct input_dev *input_dev;
> int error;
> + const struct of_device_id *match;
> +
> + match = of_match_device(st1232_ts_dt_ids, &client->dev);
> + if (!match) {
> + dev_err(&client->dev, "unknown device model\n");
> + return -ENODEV;
> + }
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> dev_err(&client->dev, "need I2C_FUNC_I2C\n");
> @@ -163,6 +210,19 @@ static int st1232_ts_probe(struct i2c_client *client,
> if (!ts)
> return -ENOMEM;
>
> + ts->chip_info = match->data;
> + ts->finger = devm_kzalloc(&client->dev,
> + sizeof(*finger) * ts->chip_info->max_fingers,
> + GFP_KERNEL);
> + if (!ts->finger)
> + return -ENOMEM;
> +
> + /* allocate a buffer according to the number of registers to read */
> + ts->read_buf_len = ts->chip_info->max_fingers * 4;
> + ts->read_buf = devm_kzalloc(&client->dev, ts->read_buf_len, GFP_KERNEL);
> + if (!ts->read_buf)
> + return -ENOMEM;
> +
> input_dev = devm_input_allocate_device(&client->dev);
> if (!input_dev)
> return -ENOMEM;
> @@ -192,9 +252,14 @@ static int st1232_ts_probe(struct i2c_client *client,
> __set_bit(EV_KEY, input_dev->evbit);
> __set_bit(EV_ABS, input_dev->evbit);
>
> - input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> - input_set_abs_params(input_dev, ABS_MT_POSITION_X, MIN_X, MAX_X, 0, 0);
> - input_set_abs_params(input_dev, ABS_MT_POSITION_Y, MIN_Y, MAX_Y, 0, 0);
> + if (ts->chip_info->id == st1232)
> + input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
> + ts->chip_info->max_area, 0, 0);
> +
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> + MIN_X, ts->chip_info->max_x, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> + MIN_Y, ts->chip_info->max_y, 0, 0);
>
> error = devm_request_threaded_irq(&client->dev, client->irq,
> NULL, st1232_ts_irq_handler,
> @@ -262,16 +327,11 @@ static SIMPLE_DEV_PM_OPS(st1232_ts_pm_ops,
>
> static const struct i2c_device_id st1232_ts_id[] = {
> { ST1232_TS_NAME, 0 },
> + { ST1633_TS_NAME, 0 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, st1232_ts_id);
>
> -static const struct of_device_id st1232_ts_dt_ids[] = {
> - { .compatible = "sitronix,st1232", },
> - { }
> -};
> -MODULE_DEVICE_TABLE(of, st1232_ts_dt_ids);
> -
> static struct i2c_driver st1232_ts_driver = {
> .probe = st1232_ts_probe,
> .remove = st1232_ts_remove,
> --
> 2.20.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: Fix the CONFIG_SPARC64 mixup
From: Dmitry Torokhov @ 2019-01-24 8:29 UTC (permalink / raw)
To: Deepa Dinamani; +Cc: linux-input, linux-kernel, arnd, davem, y2038
In-Reply-To: <20190121014914.2328-1-deepa.kernel@gmail.com>
On Sun, Jan 20, 2019 at 05:49:14PM -0800, Deepa Dinamani wrote:
> Arnd Bergmann pointed out that CONFIG_* cannot be used
> in a uapi header. Override with an equivalent conditional.
>
> Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
> Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 on 32 bit systems")
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Applied, thank you.
> ---
> include/uapi/linux/input.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index ffab958bc512..f056b2a00d5c 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -32,7 +32,7 @@ struct input_event {
> #define input_event_usec time.tv_usec
> #else
> __kernel_ulong_t __sec;
> -#ifdef CONFIG_SPARC64
> +#if defined(__sparc__) && defined(__arch64__)
> unsigned int __usec;
> #else
> __kernel_ulong_t __usec;
> --
> 2.17.1
>
--
Dmitry
^ permalink raw reply
* [PATCH v2] drm/bridge: sil_sii8620: make remote control optional.
From: Ronald Tschalär @ 2019-01-24 8:24 UTC (permalink / raw)
To: Andrzej Hajda, Inki Dae, Laurent Pinchart, Dmitry Torokhov
Cc: Lukas Wunner, linux-input, linux-kernel
In-Reply-To: <20190124072125.GA28127@innovation.ch>
commit d6abe6df706c (drm/bridge: sil_sii8620: do not have a dependency
of RC_CORE) changed the driver to select both RC_CORE and INPUT.
However, this causes problems with other drivers, in particular an input
driver that depends on MFD_INTEL_LPSS_PCI (to be added in a separate
commit):
drivers/clk/Kconfig:9:error: recursive dependency detected!
drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
According to the docs and general consensus, select should only be used
for non user-visible symbols, but both RC_CORE and INPUT are
user-visible. Furthermore almost all other references to INPUT
throughout the kernel config are depends, not selects. For this reason
the first part of this change reverts commit d6abe6df706c.
In order to address the original reason for commit d6abe6df706c, namely
that not all boards use the remote controller functionality and hence
should not need have to deal with RC_CORE, the second part of this
change now makes the remote control support in the driver optional and
contingent on RC_CORE being defined. And with this the hard dependency
on INPUT also goes away as that is only needed if RC_CORE is defined
(which in turn already depends on INPUT).
CC: Inki Dae <inki.dae@samsung.com>
CC: Andrzej Hajda <a.hajda@samsung.com>
CC: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
Changes in v2:
- completely remove dependencies on both RC_CORE and INPUT in Kconfig,
- make remote control functionality in driver contingent on RC_CORE
being defined
drivers/gpu/drm/bridge/Kconfig | 2 --
drivers/gpu/drm/bridge/sil-sii8620.c | 17 +++++++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..a11198a36005 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -85,8 +85,6 @@ config DRM_SIL_SII8620
depends on OF
select DRM_KMS_HELPER
imply EXTCON
- select INPUT
- select RC_CORE
help
Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 0cc293a6ac24..dee47752791e 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -66,7 +66,9 @@ enum sii8620_mt_state {
struct sii8620 {
struct drm_bridge bridge;
struct device *dev;
+#if IS_ENABLED(CONFIG_RC_CORE)
struct rc_dev *rc_dev;
+#endif
struct clk *clk_xtal;
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_int;
@@ -1756,6 +1758,7 @@ static void sii8620_send_features(struct sii8620 *ctx)
sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
}
+#if IS_ENABLED(CONFIG_RC_CORE)
static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
{
bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
@@ -1774,6 +1777,12 @@ static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
return true;
}
+#else
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
+{
+ return false;
+}
+#endif
static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
{
@@ -2097,6 +2106,7 @@ static void sii8620_cable_in(struct sii8620 *ctx)
enable_irq(to_i2c_client(ctx->dev)->irq);
}
+#if IS_ENABLED(CONFIG_RC_CORE)
static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
{
struct rc_dev *rc_dev;
@@ -2126,6 +2136,11 @@ static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
}
ctx->rc_dev = rc_dev;
}
+#else
+static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
+{
+}
+#endif
static void sii8620_cable_out(struct sii8620 *ctx)
{
@@ -2214,9 +2229,11 @@ static int sii8620_attach(struct drm_bridge *bridge)
static void sii8620_detach(struct drm_bridge *bridge)
{
+#if IS_ENABLED(CONFIG_RC_CORE)
struct sii8620 *ctx = bridge_to_sii8620(bridge);
rc_unregister_device(ctx->rc_dev);
+#endif
}
static int sii8620_is_packing_required(struct sii8620 *ctx,
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.
From: Life is hard, and then you die @ 2019-01-24 7:21 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Dmitry Torokhov, Lukas Wunner, Andrzej Hajda, Inki Dae, dri-devel,
linux-kernel, linux-input
In-Reply-To: <20190123222200.GF4675@pendragon.ideasonboard.com>
On Thu, Jan 24, 2019 at 12:22:00AM +0200, Laurent Pinchart wrote:
>
> On Wed, Jan 23, 2019 at 02:21:05PM -0800, Dmitry Torokhov wrote:
> > On Thu, Jan 24, 2019 at 12:17:35AM +0200, Laurent Pinchart wrote:
> > > On Wed, Jan 23, 2019 at 02:03:42PM -0800, Dmitry Torokhov wrote:
> > >> On Wed, Jan 23, 2019 at 09:45:56AM +0100, Lukas Wunner wrote:
> > >>> On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
> > >>>> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> > >>>> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> > >>>> INPUT. However, this causes problems with other drivers, in particular
> > >>>> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> > >>>> future commit):
> > >>>>
> > >>>> drivers/clk/Kconfig:9:error: recursive dependency detected!
> > >>>> drivers/clk/Kconfig:9: symbol COMMON_CLK is selected by MFD_INTEL_LPSS
> > >>>> drivers/mfd/Kconfig:566: symbol MFD_INTEL_LPSS is selected by MFD_INTEL_LPSS_PCI
> > >>>> drivers/mfd/Kconfig:580: symbol MFD_INTEL_LPSS_PCI is implied by KEYBOARD_APPLESPI
> > >>>> drivers/input/keyboard/Kconfig:73: symbol KEYBOARD_APPLESPI depends on INPUT
> > >>>> drivers/input/Kconfig:8: symbol INPUT is selected by DRM_SIL_SII8620
> > >>>> drivers/gpu/drm/bridge/Kconfig:83: symbol DRM_SIL_SII8620 depends on DRM_BRIDGE
> > >>>> drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by DRM_PL111
> > >>>> drivers/gpu/drm/pl111/Kconfig:1: symbol DRM_PL111 depends on COMMON_CLK
> > >>>>
> > >>>> According to the docs, select should only be used for non-visible
> > >>>> symbols. Furthermore almost all other references to INPUT throughout the
> > >>>> kernel config are depends, not selects. Hence this change.
> > >>
> > >> I think this is not as cut and dry. We should be able to select needed
> > >> subsystems (such as INPUT, USB, etc) even if they are user visible.
> > >
> > > Semantically, maybe, but given the current state of Kconfig this results
> > > in a recursive dependencies nightmare. It's a no-go.
> > >
> > >> User, when enabling a piece of hardware, does not need to know ultimate
> > >> details of all subsystems the driver might need ti function.
> > >>
> > >> It looks like one of the drivers implies MFD_INTEL_LPSS_PCI, maybe
> > >> treating imply the same as select when detecting circular dependency is
> > >> wrong as we are allowed to deselect implied dependencies?
> > >>
> > >>>>
> > >>>> CC: Inki Dae <inki.dae@samsung.com>
> > >>>> CC: Andrzej Hajda <a.hajda@samsung.com>
> > >>>> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> > >>>
> > >>> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> > >>>
> > >>> I think this needs to be merged through the input tree as a prerequisite
> > >>> for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
> > >>> MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
> > >>> breaking the build. Adding Dmitry.
> > >>
> > >> I have no idea what applespi.c is (it is definitely not in my tree), so
> > >> I think it should be merged through the same tree that the original
> > >> commit was introduced through.
Apologies, I should've added a cover letter explain this: the applespi
driver is a new input driver I'm about to submit through the input
tree, hence why you wouldn't have it. But as Lukas says, the change
here is a prerequisite for the proposed Kconfig for that driver.
Since the two changes (the change here + the new driver) seem to be
best submitted through different trees, I'm trying to figure out how
best to handle this. I suppose I could temporarily change the driver
Kconfig to not trigger the conflict, and then once the change here has
been upstreamed (not sure at what point exactly that would be
considered the case, e.g. if in linux-next is sufficient, or has to
wait for Linus' merge, or something else) submit another change to
change the driver's Kconfig to the desired one.
> > >>>> ---
> > >>>> drivers/gpu/drm/bridge/Kconfig | 2 +-
> > >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > >>>> index 2fee47b0d50b..eabedc83f25c 100644
> > >>>> --- a/drivers/gpu/drm/bridge/Kconfig
> > >>>> +++ b/drivers/gpu/drm/bridge/Kconfig
> > >>>> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> > >>>> config DRM_SIL_SII8620
> > >>>> tristate "Silicon Image SII8620 HDMI/MHL bridge"
> > >>>> depends on OF
> > >>>> + depends on INPUT
> > >>>> select DRM_KMS_HELPER
> > >>>> imply EXTCON
> > >>>> - select INPUT
> > >>>> select RC_CORE
> > >>
> > >> Keeping "select RC_CORE" is wrong though, as the driver appears to be
> > >> working find without RC. Maybe it should be stubbed out?
> > >
> > > It should definitely not be select'ed as it's a user-visible symbol. My
> > > preference would be to simply revert d6abe6df706c. If we want (and can)
> > > work without RC core then it should be stubbed out.
> > >
> > > Commit d6abe6df706c states
> > >
> > > And some boards not using remote controller device don't really
> > > need to know that RC_CORE config should be enabled to use sil_sii8620
> > > driver only for HDMI.
> > >
> > > The same reasoning applies to INPUT, if we agree that depending on
> > > RC_CORE is confusing for users, then depending on INPUT is confusing as
> > > well. There's not reason to apply different standards to INPUT and
> > > RC_CORE, depending on one and selecting the other doesn't make much
> > > sense.
> >
> > OK, so revert + patch to stub out RC calls? That works for me (and I
> > still say it should go through the same tree that introduced
> > d6abe6df706c).
>
> Yes, that sounds good to me.
Thanks for the reviews. Ok, I'll submit a new patch shortly for this.
However, other than compiling with RC_CORE and INPUT set/unset I can't
test the changes to the sil-sii8620 driver.
Cheers,
Ronald
^ permalink raw reply
* Re: [PATCH v3 1/4] dt-bindings: input: touchscreen: goodix: Document AVDD28-supply property
From: Jagan Teki @ 2019-01-24 4:32 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Dmitry Torokhov, Chen-Yu Tsai, linux-input,
linux-kernel@vger.kernel.org, Michael Trimarchi, linux-amarula
In-Reply-To: <CAMty3ZBtoocXBW5A0astyQPh2y7vG7NtbZ_M0BjSeex2etmGsw@mail.gmail.com>
+ DT ML
On Tue, Jan 22, 2019 at 7:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Mon, Jan 21, 2019 at 9:46 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Fri, Jan 18, 2019 at 10:01 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > On Wed, Jan 9, 2019 at 7:08 PM Rob Herring <robh@kernel.org> wrote:
> > > >
> > > > Please CC DT list if you want bindings reviewed.
> > >
> > > Sorry I forgot.
> > >
> > > >
> > > > On Wed, Jan 9, 2019 at 1:40 AM Dmitry Torokhov
> > > > <dmitry.torokhov@gmail.com> wrote:
> > > > >
> > > > > On Sat, Dec 15, 2018 at 08:47:59PM +0530, Jagan Teki wrote:
> > > > > > Most of the Goodix CTP controllers are supply with AVDD28 pin.
> > > > > > which need to supply for controllers like GT5663 on some boards
> > > > > > to trigger the power.
> > > > > >
> > > > > > So, document the supply property so-that the require boards
> > > > > > that used on GT5663 can enable it via device tree.
> > > > > >
> > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
> > > > > > 1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > > > index f7e95c52f3c7..c4622c983e08 100644
> > > > > > --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > > > @@ -23,6 +23,7 @@ Optional properties:
> > > > > > - touchscreen-inverted-y : Y axis is inverted (boolean)
> > > > > > - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
> > > > > > (swapping is done after inverting the axis)
> > > > > > + - AVDD28-supply : Analog power supply regulator on AVDD28 pin
> > > > >
> > > > > I think we normally use lower case in DT bindings and rarely encode
> > > > > voltage in the supply name unless we are dealing with several supplies
> > > > > of the same kind, but I'll let Ron comment on this.
> > > >
> > > > Yes on lowercase though there are some exceptions.
> > > >
> > > > There's also a AVDD22 supply as well as DVDD12 and VDDIO. So we
> > > > probably need to keep the voltage, but the binding is incomplete.
> > >
> > > What is incomplete here? can you please elaborate.
> >
> > You are missing the 3 other supplies the chip has: AVDD22, DVDD12 and VDDIO.
>
> Though it has other supplies, only AVDD28 is connected in the Host
> interface design similar like 9. Reference Schematic on Page, 23 in
> attached manual.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox