* [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Eric Dumazet @ 2018-10-04 15:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Eric Dumazet, Eric Dumazet, Dmitry Torokhov, linux-input
syzbot was able to trigger rcu stalls by calling write()
with large number of bytes.
Add a cond_resched() in the loop to avoid this.
Link: https://lkml.org/lkml/2018/8/23/1106
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
---
drivers/input/mousedev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file *file, const char __user *buffer,
mousedev_generate_response(client, c);
spin_unlock_irq(&client->packet_lock);
+ cond_resched();
}
kill_fasync(&client->fasync, SIGIO, POLL_IN);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Dmitry Torokhov @ 2018-10-04 18:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, Eric Dumazet, linux-input, Paul E. McKenney
In-Reply-To: <20181004154749.111595-1-edumazet@google.com>
Hi Eric,
On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> syzbot was able to trigger rcu stalls by calling write()
> with large number of bytes.
>
> Add a cond_resched() in the loop to avoid this.
I think this simply masks a deeper issue. The code fetches characters
from userspace in a loop, takes a lock, quickly places response in an
output buffer, and releases interrupt. I do not see why this should
cause stalls as we do not hold spinlock/interrupts off for extended
period of time.
Adding Paul so he can straighten me out...
>
> Link: https://lkml.org/lkml/2018/8/23/1106
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> ---
> drivers/input/mousedev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
> index e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347 100644
> --- a/drivers/input/mousedev.c
> +++ b/drivers/input/mousedev.c
> @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file *file, const char __user *buffer,
> mousedev_generate_response(client, c);
>
> spin_unlock_irq(&client->packet_lock);
> + cond_resched();
> }
>
> kill_fasync(&client->fasync, SIGIO, POLL_IN);
> --
> 2.19.0.605.g01d371f741-goog
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: reserve 2 events code because of HID
From: Dmitry Torokhov @ 2018-10-04 19:01 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Harry Cutts, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <20181004123430.29348-1-benjamin.tissoires@redhat.com>
On Thu, Oct 04, 2018 at 02:34:30PM +0200, Benjamin Tissoires wrote:
> Prior to commit 190d7f02ce8e ("HID: input: do not increment usages when
> a duplicate is found") from the v4.18 kernel, HID used to shift the
> event codes if a duplicate usage was found. This ended up in a situation
> where a device would export a ton of ABS_MISC+n event codes, or a ton
> of REL_MISC+n event codes.
>
> This is now fixed, however userspace needs to detect those situation.
> Fortunately, ABS_MT_SLOT-1 (ABS_MISC+6) was never assigned a code, and
> so libinput can detect fake multitouch devices from genuine ones by
> checking if ABS_MT_SLOT-1 is set.
>
> Now that we have REL_WHEEL_HI_RES, libinput won't be able to differentiate
> true high res mice from some other device in a pre-v4.18 kernel.
>
> Set in stone that the ABS_MISC+6 and REL_MISC+1 are reserved and should not
> be used so userspace can properly work around those old kernels.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Dmitry, I did not take your Acked-by, as I added 2 new events and wanted to
> be sure you are still OK.
Yes, this is still fine.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Jiri, this will still need to go through your tree, as the input-event-codes
> changes that introduce REL_WHEEL_HI_RES are in your for-4.20/logitech-highres.
>
> Cheers,
> Benjamin
>
> include/uapi/linux/input-event-codes.h | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 29fb891ea337..b5e0e9856c47 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -708,7 +708,15 @@
> #define REL_DIAL 0x07
> #define REL_WHEEL 0x08
> #define REL_MISC 0x09
> -#define REL_WHEEL_HI_RES 0x0a
> +/*
> + * 0x0a is reserved and should not be used in input drivers.
> + * It was used by HID as REL_MISC+1 and userspace needs to detect if
> + * the next REL_* event is correct or is just REL_MISC + n.
> + * We define here REL_RESERVED so userspace can rely on it and detect
> + * the situation described above.
> + */
> +#define REL_RESERVED 0x0a
> +#define REL_WHEEL_HI_RES 0x0b
> #define REL_MAX 0x0f
> #define REL_CNT (REL_MAX+1)
>
> @@ -745,6 +753,15 @@
>
> #define ABS_MISC 0x28
>
> +/*
> + * 0x2e is reserved and should not be used in input drivers.
> + * It was used by HID as ABS_MISC+6 and userspace needs to detect if
> + * the next ABS_* event is correct or is just ABS_MISC + n.
> + * We define here ABS_RESERVED so userspace can rely on it and detect
> + * the situation described above.
> + */
> +#define ABS_RESERVED 0x2e
> +
> #define ABS_MT_SLOT 0x2f /* MT slot being modified */
> #define ABS_MT_TOUCH_MAJOR 0x30 /* Major axis of touching ellipse */
> #define ABS_MT_TOUCH_MINOR 0x31 /* Minor axis (omit if circular) */
> --
> 2.14.3
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: reserve 2 events code because of HID
From: Jiri Kosina @ 2018-10-04 19:09 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Dmitry Torokhov, Harry Cutts, Peter Hutterer, linux-input,
linux-kernel
In-Reply-To: <20181004123430.29348-1-benjamin.tissoires@redhat.com>
On Thu, 4 Oct 2018, Benjamin Tissoires wrote:
> Prior to commit 190d7f02ce8e ("HID: input: do not increment usages when
> a duplicate is found") from the v4.18 kernel, HID used to shift the
> event codes if a duplicate usage was found. This ended up in a situation
> where a device would export a ton of ABS_MISC+n event codes, or a ton
> of REL_MISC+n event codes.
>
> This is now fixed, however userspace needs to detect those situation.
> Fortunately, ABS_MT_SLOT-1 (ABS_MISC+6) was never assigned a code, and
> so libinput can detect fake multitouch devices from genuine ones by
> checking if ABS_MT_SLOT-1 is set.
>
> Now that we have REL_WHEEL_HI_RES, libinput won't be able to differentiate
> true high res mice from some other device in a pre-v4.18 kernel.
>
> Set in stone that the ABS_MISC+6 and REL_MISC+1 are reserved and should not
> be used so userspace can properly work around those old kernels.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Dmitry, I did not take your Acked-by, as I added 2 new events and wanted to
> be sure you are still OK.
>
> Jiri, this will still need to go through your tree, as the input-event-codes
> changes that introduce REL_WHEEL_HI_RES are in your for-4.20/logitech-highres.
Applied, thanks Benjamin.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Eric Dumazet @ 2018-10-04 19:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: LKML, Eric Dumazet, linux-input, Paul E. McKenney
In-Reply-To: <20181004185949.GA233675@dtor-ws>
On Thu, Oct 4, 2018 at 11:59 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Eric,
>
> On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> > syzbot was able to trigger rcu stalls by calling write()
> > with large number of bytes.
> >
> > Add a cond_resched() in the loop to avoid this.
>
> I think this simply masks a deeper issue. The code fetches characters
> from userspace in a loop, takes a lock, quickly places response in an
> output buffer, and releases interrupt. I do not see why this should
> cause stalls as we do not hold spinlock/interrupts off for extended
> period of time.
>
> Adding Paul so he can straighten me out...
>
Well...
write(fd, buffer, 0x7FFF0000);
Takes between 20 seconds and 2 minutes depending on CONFIG options ....
So either apply my patch, or add a limit on the max count, and
possibly break legitimate user space ?
I dunno...
> >
> > Link: https://lkml.org/lkml/2018/8/23/1106
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: linux-input@vger.kernel.org
> > ---
> > drivers/input/mousedev.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
> > index e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347 100644
> > --- a/drivers/input/mousedev.c
> > +++ b/drivers/input/mousedev.c
> > @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file *file, const char __user *buffer,
> > mousedev_generate_response(client, c);
> >
> > spin_unlock_irq(&client->packet_lock);
> > + cond_resched();
> > }
> >
> > kill_fasync(&client->fasync, SIGIO, POLL_IN);
> > --
> > 2.19.0.605.g01d371f741-goog
> >
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Paul E. McKenney @ 2018-10-04 19:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Eric Dumazet, linux-kernel, Eric Dumazet, linux-input
In-Reply-To: <20181004185949.GA233675@dtor-ws>
On Thu, Oct 04, 2018 at 11:59:49AM -0700, Dmitry Torokhov wrote:
> Hi Eric,
>
> On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> > syzbot was able to trigger rcu stalls by calling write()
> > with large number of bytes.
> >
> > Add a cond_resched() in the loop to avoid this.
>
> I think this simply masks a deeper issue. The code fetches characters
> from userspace in a loop, takes a lock, quickly places response in an
> output buffer, and releases interrupt. I do not see why this should
> cause stalls as we do not hold spinlock/interrupts off for extended
> period of time.
>
> Adding Paul so he can straighten me out...
If you are running a !PREEMPT kernel, then you need the cond_resched()
to allow the scheduler to choose someone else to run if needed and
to let RCU know that grace periods can end. Without the cond_resched(),
if you stay in that loop long enough you will get excessive scheduling
latencies and eventually even RCU CPU stall warning splats.
In a PREEMPT (instead of !PREEMPT) kernel, you would be right. When
preemption is enabled, the scheduler can preempt and RCU can sense
lack of readers from the scheduling-clock interrupt handler. Which
is why cond_resched() is nothingness in a PREEMPT kernel.
But because people run !PREEMPT as well as PREEMPT kernels, if that loop
can run for a long time, you need that cond_resched().
Thanx, Paul
> > Link: https://lkml.org/lkml/2018/8/23/1106
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: linux-input@vger.kernel.org
> > ---
> > drivers/input/mousedev.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
> > index e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347 100644
> > --- a/drivers/input/mousedev.c
> > +++ b/drivers/input/mousedev.c
> > @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file *file, const char __user *buffer,
> > mousedev_generate_response(client, c);
> >
> > spin_unlock_irq(&client->packet_lock);
> > + cond_resched();
> > }
> >
> > kill_fasync(&client->fasync, SIGIO, POLL_IN);
> > --
> > 2.19.0.605.g01d371f741-goog
> >
>
> Thanks.
>
> --
> Dmitry
>
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Paul E. McKenney @ 2018-10-04 19:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dmitry Torokhov, LKML, Eric Dumazet, linux-input
In-Reply-To: <CANn89iJMhGEkw13KrGbqrvGbznb45QZwdr7eLy_rUV6zitvs=Q@mail.gmail.com>
On Thu, Oct 04, 2018 at 12:28:56PM -0700, Eric Dumazet wrote:
> On Thu, Oct 4, 2018 at 11:59 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Eric,
> >
> > On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> > > syzbot was able to trigger rcu stalls by calling write()
> > > with large number of bytes.
> > >
> > > Add a cond_resched() in the loop to avoid this.
> >
> > I think this simply masks a deeper issue. The code fetches characters
> > from userspace in a loop, takes a lock, quickly places response in an
> > output buffer, and releases interrupt. I do not see why this should
> > cause stalls as we do not hold spinlock/interrupts off for extended
> > period of time.
> >
> > Adding Paul so he can straighten me out...
> >
>
> Well...
>
> write(fd, buffer, 0x7FFF0000);
>
> Takes between 20 seconds and 2 minutes depending on CONFIG options ....
And two minutes would get you an RCU CPU stall warning, even on distro
kernels that set the stall-warning time to a full minute (as opposed
to 21 seconds in mainline).
> So either apply my patch, or add a limit on the max count, and
> possibly break legitimate user space ?
>
> I dunno...
I vote for Eric's patch. In fact:
Reviewed-by: Paul E. McKenney <paulmck@linux.ibm.com>
> > > Link: https://lkml.org/lkml/2018/8/23/1106
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: linux-input@vger.kernel.org
> > > ---
> > > drivers/input/mousedev.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
> > > index e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347 100644
> > > --- a/drivers/input/mousedev.c
> > > +++ b/drivers/input/mousedev.c
> > > @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file *file, const char __user *buffer,
> > > mousedev_generate_response(client, c);
> > >
> > > spin_unlock_irq(&client->packet_lock);
> > > + cond_resched();
> > > }
> > >
> > > kill_fasync(&client->fasync, SIGIO, POLL_IN);
> > > --
> > > 2.19.0.605.g01d371f741-goog
> > >
> >
> > Thanks.
> >
> > --
> > Dmitry
>
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Dmitry Torokhov @ 2018-10-04 19:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: LKML, Eric Dumazet, linux-input, Paul E. McKenney
In-Reply-To: <CANn89iJMhGEkw13KrGbqrvGbznb45QZwdr7eLy_rUV6zitvs=Q@mail.gmail.com>
On October 4, 2018 12:28:56 PM PDT, Eric Dumazet <edumazet@google.com> wrote:
>On Thu, Oct 4, 2018 at 11:59 AM Dmitry Torokhov
><dmitry.torokhov@gmail.com> wrote:
>>
>> Hi Eric,
>>
>> On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
>> > syzbot was able to trigger rcu stalls by calling write()
>> > with large number of bytes.
>> >
>> > Add a cond_resched() in the loop to avoid this.
>>
>> I think this simply masks a deeper issue. The code fetches characters
>> from userspace in a loop, takes a lock, quickly places response in an
>> output buffer, and releases interrupt. I do not see why this should
>> cause stalls as we do not hold spinlock/interrupts off for extended
>> period of time.
>>
>> Adding Paul so he can straighten me out...
>>
>
>Well...
>
>write(fd, buffer, 0x7FFF0000);
>
>Takes between 20 seconds and 2 minutes depending on CONFIG options ....
That's fine even if it takes a couple of years. We are not holding spinlock for the entirety of this time, so we should get bumped off CPU at some point.
>
>So either apply my patch, or add a limit on the max count, and
>possibly break legitimate user space ?
Legitimate users write a single character at a time and read response, so exciting after, let's say, 32 bytes would be fine. But I still want to understand why we have to do that.
>
>I dunno...
>
>> >
>> > Link: https://lkml.org/lkml/2018/8/23/1106
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> > Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
>> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > Cc: linux-input@vger.kernel.org
>> > ---
>> > drivers/input/mousedev.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
>> > index
>e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347
>100644
>> > --- a/drivers/input/mousedev.c
>> > +++ b/drivers/input/mousedev.c
>> > @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file
>*file, const char __user *buffer,
>> > mousedev_generate_response(client, c);
>> >
>> > spin_unlock_irq(&client->packet_lock);
>> > + cond_resched();
>> > }
>> >
>> > kill_fasync(&client->fasync, SIGIO, POLL_IN);
>> > --
>> > 2.19.0.605.g01d371f741-goog
>> >
>>
>> Thanks.
>>
>> --
>> Dmitry
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Eric Dumazet @ 2018-10-04 19:45 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: LKML, Eric Dumazet, linux-input, Paul E. McKenney
In-Reply-To: <30074728-D1C4-46D4-8BF5-6AB8ECAE3EBD@gmail.com>
On Thu, Oct 4, 2018 at 12:38 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On October 4, 2018 12:28:56 PM PDT, Eric Dumazet <edumazet@google.com> wrote:
> >On Thu, Oct 4, 2018 at 11:59 AM Dmitry Torokhov
> ><dmitry.torokhov@gmail.com> wrote:
> >>
> >> Hi Eric,
> >>
> >> On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> >> > syzbot was able to trigger rcu stalls by calling write()
> >> > with large number of bytes.
> >> >
> >> > Add a cond_resched() in the loop to avoid this.
> >>
> >> I think this simply masks a deeper issue. The code fetches characters
> >> from userspace in a loop, takes a lock, quickly places response in an
> >> output buffer, and releases interrupt. I do not see why this should
> >> cause stalls as we do not hold spinlock/interrupts off for extended
> >> period of time.
> >>
> >> Adding Paul so he can straighten me out...
> >>
> >
> >Well...
> >
> >write(fd, buffer, 0x7FFF0000);
> >
> >Takes between 20 seconds and 2 minutes depending on CONFIG options ....
>
> That's fine even if it takes a couple of years. We are not holding spinlock for the entirety of this time, so we should get bumped off CPU at some point.
Well, you are saying that we could get rid of all cond_resched() calls
in the kernel.
You should send patches asap ;)
>
> >
> >So either apply my patch, or add a limit on the max count, and
> >possibly break legitimate user space ?
>
> Legitimate users write a single character at a time and read response, so exciting after, let's say, 32 bytes would be fine. But I still want to understand why we have to do that.
>
> >
> >I dunno...
> >
> >> >
> >> > Link: https://lkml.org/lkml/2018/8/23/1106
> >> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> > Reported-by: syzbot+9436b02171ac0894d33e@syzkaller.appspotmail.com
> >> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> > Cc: linux-input@vger.kernel.org
> >> > ---
> >> > drivers/input/mousedev.c | 1 +
> >> > 1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
> >> > index
> >e08228061bcdd2f97aaadece31d6c83eb7539ae5..412fa71245afe26a7a8ad75705566f83633ba347
> >100644
> >> > --- a/drivers/input/mousedev.c
> >> > +++ b/drivers/input/mousedev.c
> >> > @@ -707,6 +707,7 @@ static ssize_t mousedev_write(struct file
> >*file, const char __user *buffer,
> >> > mousedev_generate_response(client, c);
> >> >
> >> > spin_unlock_irq(&client->packet_lock);
> >> > + cond_resched();
> >> > }
> >> >
> >> > kill_fasync(&client->fasync, SIGIO, POLL_IN);
> >> > --
> >> > 2.19.0.605.g01d371f741-goog
> >> >
> >>
> >> Thanks.
> >>
> >> --
> >> Dmitry
>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Dmitry Torokhov @ 2018-10-04 22:54 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Eric Dumazet, linux-kernel, Eric Dumazet, linux-input
In-Reply-To: <20181004193407.GK2674@linux.ibm.com>
On Thu, Oct 04, 2018 at 12:34:07PM -0700, Paul E. McKenney wrote:
> On Thu, Oct 04, 2018 at 11:59:49AM -0700, Dmitry Torokhov wrote:
> > Hi Eric,
> >
> > On Thu, Oct 04, 2018 at 08:47:49AM -0700, Eric Dumazet wrote:
> > > syzbot was able to trigger rcu stalls by calling write()
> > > with large number of bytes.
> > >
> > > Add a cond_resched() in the loop to avoid this.
> >
> > I think this simply masks a deeper issue. The code fetches characters
> > from userspace in a loop, takes a lock, quickly places response in an
> > output buffer, and releases interrupt. I do not see why this should
> > cause stalls as we do not hold spinlock/interrupts off for extended
> > period of time.
> >
> > Adding Paul so he can straighten me out...
>
> If you are running a !PREEMPT kernel, then you need the cond_resched()
> to allow the scheduler to choose someone else to run if needed and
> to let RCU know that grace periods can end. Without the cond_resched(),
> if you stay in that loop long enough you will get excessive scheduling
> latencies and eventually even RCU CPU stall warning splats.
>
> In a PREEMPT (instead of !PREEMPT) kernel, you would be right. When
> preemption is enabled, the scheduler can preempt and RCU can sense
> lack of readers from the scheduling-clock interrupt handler. Which
> is why cond_resched() is nothingness in a PREEMPT kernel.
>
> But because people run !PREEMPT as well as PREEMPT kernels, if that loop
> can run for a long time, you need that cond_resched().
OK, I see. I'll apply the patch then.
I think evdev.c needs similar treatment as it will keep looping while
there is data...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: mousedev - add a schedule point in mousedev_write()
From: Eric Dumazet @ 2018-10-04 23:01 UTC (permalink / raw)
To: Dmitry Torokhov, Paul E. McKenney
Cc: Eric Dumazet, linux-kernel, Eric Dumazet, linux-input
In-Reply-To: <20181004225455.GC233675@dtor-ws>
On 10/04/2018 03:54 PM, Dmitry Torokhov wrote:
> OK, I see. I'll apply the patch then.
Thanks !
>
> I think evdev.c needs similar treatment as it will keep looping while
> there is data...
Yeah, presumably other drivers need care as well :/
^ permalink raw reply
* [PATCH] Input: evdev - add a schedule point in evdev_write()
From: Dmitry Torokhov @ 2018-10-05 0:53 UTC (permalink / raw)
To: linux-input; +Cc: Eric Dumazet, Paul E. McKenney, linux-kernel
Large writes to evdev interface may cause rcu stalls. Let's add
cond_resched() to the loop to avoid this.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/evdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 370206f987f9..f48369d6f3a0 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -564,6 +564,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
input_inject_event(&evdev->handle,
event.type, event.code, event.value);
+ cond_resched();
}
out:
--
2.19.0.605.g01d371f741-goog
--
Dmitry
^ permalink raw reply related
* [PATCH] Input: uinput - add a schedule point in uinput_inject_events()
From: Dmitry Torokhov @ 2018-10-05 0:54 UTC (permalink / raw)
To: linux-input; +Cc: Eric Dumazet, Paul E. McKenney, linux-kernel
Large writes to uinput interface may cause rcu stalls. Let's add
cond_resched() to the loop to avoid this.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/uinput.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index eb14ddf69346..8ec483e8688b 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -598,6 +598,7 @@ static ssize_t uinput_inject_events(struct uinput_device *udev,
input_event(udev->dev, ev.type, ev.code, ev.value);
bytes += input_event_size();
+ cond_resched();
}
return bytes;
--
2.19.0.605.g01d371f741-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] Input: evdev - add a schedule point in evdev_write()
From: Paul E. McKenney @ 2018-10-05 4:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Eric Dumazet, linux-kernel
In-Reply-To: <20181005005359.GA218372@dtor-ws>
On Thu, Oct 04, 2018 at 05:53:59PM -0700, Dmitry Torokhov wrote:
> Large writes to evdev interface may cause rcu stalls. Let's add
> cond_resched() to the loop to avoid this.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.ibm.com>
> ---
> drivers/input/evdev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 370206f987f9..f48369d6f3a0 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -564,6 +564,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
>
> input_inject_event(&evdev->handle,
> event.type, event.code, event.value);
> + cond_resched();
> }
>
> out:
> --
> 2.19.0.605.g01d371f741-goog
>
>
> --
> Dmitry
>
^ permalink raw reply
* Re: [PATCH] Input: uinput - add a schedule point in uinput_inject_events()
From: Paul E. McKenney @ 2018-10-05 4:09 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Eric Dumazet, linux-kernel
In-Reply-To: <20181005005432.GA218507@dtor-ws>
On Thu, Oct 04, 2018 at 05:54:32PM -0700, Dmitry Torokhov wrote:
> Large writes to uinput interface may cause rcu stalls. Let's add
> cond_resched() to the loop to avoid this.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Paul E. McKenney <paulmck@linux.ibm.com>
> ---
> drivers/input/misc/uinput.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index eb14ddf69346..8ec483e8688b 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -598,6 +598,7 @@ static ssize_t uinput_inject_events(struct uinput_device *udev,
>
> input_event(udev->dev, ev.type, ev.code, ev.value);
> bytes += input_event_size();
> + cond_resched();
> }
>
> return bytes;
> --
> 2.19.0.605.g01d371f741-goog
>
>
> --
> Dmitry
>
^ permalink raw reply
* [PATCH v2] HID: i2c-hid: Add a small delay after sleep command for Raydium touchpanel
From: Kai-Heng Feng @ 2018-10-05 4:46 UTC (permalink / raw)
To: jikos
Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel,
Kai-Heng Feng
Raydium touchpanel (2386:4B33) sometimes does not work in desktop session
although it works in display manager.
During user logging, the display manager exits, close the HID device,
then the device gets runtime suspended and powered off. The desktop
session begins shortly after, opens the HID device, then the device gets
runtime resumed and powered on.
If the trasition from display manager to desktop sesesion is fast, the
touchpanel cannot switch from powered off to powered on in short
timeframe. So add a small delay to workaround the issue.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Use quirk to only match affected touchpanel
- Only delay the next power on if the time hasn't elapsed
drivers/hid/hid-ids.h | 3 +++
drivers/hid/i2c-hid/i2c-hid-core.c | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 16342188df19..c1b5f03eb630 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -926,6 +926,9 @@
#define USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003 0x3003
#define USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008 0x3008
+#define I2C_VENDOR_ID_RAYDIUM 0x2386
+#define I2C_PRODUCT_ID_RAYDIUM_4B33 0x4b33
+
#define USB_VENDOR_ID_RAZER 0x1532
#define USB_DEVICE_ID_RAZER_BLADE_14 0x011D
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 4aab96cf0818..3cde7c1b9c33 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -49,6 +49,7 @@
#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
#define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
+#define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
/* flags */
#define I2C_HID_STARTED 0
@@ -158,6 +159,8 @@ struct i2c_hid {
bool irq_wake_enabled;
struct mutex reset_lock;
+
+ unsigned long sleep_delay;
};
static const struct i2c_hid_quirks {
@@ -172,6 +175,8 @@ static const struct i2c_hid_quirks {
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { I2C_VENDOR_ID_RAYDIUM, I2C_PRODUCT_ID_RAYDIUM_4B33,
+ I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
{ 0, 0 }
};
@@ -387,6 +392,7 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
{
struct i2c_hid *ihid = i2c_get_clientdata(client);
int ret;
+ unsigned long now, delay;
i2c_hid_dbg(ihid, "%s\n", __func__);
@@ -404,9 +410,22 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
goto set_pwr_exit;
}
+ if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&
+ power_state == I2C_HID_PWR_ON) {
+ now = jiffies;
+ if (time_after(ihid->sleep_delay, now)) {
+ delay = jiffies_to_usecs(ihid->sleep_delay - now);
+ usleep_range(delay, delay + 1);
+ }
+ }
+
ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,
0, NULL, 0, NULL, 0);
+ if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&
+ power_state == I2C_HID_PWR_SLEEP)
+ ihid->sleep_delay = jiffies + msecs_to_jiffies(20);
+
if (ret)
dev_err(&client->dev, "failed to change power setting.\n");
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/5] ti_am335x_tsc: Enable wakeup capability
From: Vignesh R @ 2018-10-05 5:11 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Jonathan Cameron, linux-iio, linux-omap,
linux-kernel, linux-input
In-Reply-To: <f61848d8-57f6-14c0-eb74-cab488ceda50@ti.com>
On Friday 28 September 2018 11:42 AM, Vignesh R wrote:
> Hi Lee,
>
> On Wednesday 25 July 2018 10:56 AM, Lee Jones wrote:
> [...]
>>>>>>>>
>>>>>>>> Vignesh R (5):
>>>>>>>> mfd: ti_am335x_tscadc: Don't mark TSCADC MFD as wakeup capable
>>>>>>>> Input: ti_am335x_tsc: Mark TSC device as wakeup source
>>>>>>>> mfd: ti_am335x_tscadc: Keep ADC interface on if child is wakeup
>>>>>>>> capable
>>>>>>>> iio: adc: ti_am335x_adc: Disable ADC during suspend unconditionally
>>>>>>>> Input: ti_am335x_tsc: Mark IRQ as wakeup capable
>>>>>>>>
>>>>>>>> drivers/iio/adc/ti_am335x_adc.c | 12 ++++--------
>>>>>>>> drivers/input/touchscreen/ti_am335x_tsc.c | 22 +++++++++++++++++-----
>>>>>>>> drivers/mfd/ti_am335x_tscadc.c | 14 +++++++++++++-
>>>>>>>> 3 files changed, 34 insertions(+), 14 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>> Gentle ping... Could you review/pick this series? MFD amd IIO bits are
>>>>>>> already ACKed
>>>>>>
>>>>>> MFD patches are reviewed "for my own reference" meaning that we
>>>>>> haven't yet agreed on a merge plan yet.
>>>>>
>>>>> I think this series makes sense to be pushed through a single tree as
>>>>> opposed to being spread between 3, even if it could technically be
>>>>> possible. It looks like Jonathan is fine with going it through either
>>>>> his or some other tree, I am fine with it going through MFD.
>>>>
>>>> I'm happy either way.
>>>>
>>> Thanks Dmitry, Jonathan and Lee Jones!
>>>
>>> Could this be applied to one of the trees now? MFD perhaps?
>>
>> It'll be applied, when it's applied. ;)
>
> I see that this series was not picked up for v4.19. Could you consider
> this series for v4.20? Patches apply cleanly against linux-next.
>
>
Gentle ping...
--
Regards
Vignesh
^ permalink raw reply
* [PATCH] input: st1232: set INPUT_PROP_DIRECT property
From: Martin Kepplinger @ 2018-10-05 8:14 UTC (permalink / raw)
To: dmitry.torokhov, geert+renesas
Cc: linux-input, linux-kernel, Martin Kepplinger
This is how userspace checks for touchscreen devices most reliably.
Signed-off-by: Martin Kepplinger <martink@posteo.de>
---
drivers/input/touchscreen/st1232.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index d5dfa4053bbf..b71673911aac 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -195,6 +195,7 @@ static int st1232_ts_probe(struct i2c_client *client,
input_dev->id.bustype = BUS_I2C;
input_dev->dev.parent = &client->dev;
+ __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
__set_bit(EV_SYN, input_dev->evbit);
__set_bit(EV_KEY, input_dev->evbit);
__set_bit(EV_ABS, input_dev->evbit);
--
2.19.0
^ permalink raw reply related
* [PATCH v2 1/2] mfd: cros: add "base attached" MKBP switch definition
From: Dmitry Torokhov @ 2018-10-05 18:41 UTC (permalink / raw)
To: Jiri Kosina, Lee Jones
Cc: Benjamin Tissoires, linux-input, linux-kernel, Nicolas Boichat
This adds a "base attached" switch definition to the MKBP protocol that
is used by Whiskers driver to properly determine device state (clamshell
vs tablet mode).
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
---
v2 changes: None
Lee, I was wondering if it would be OK for cros_ec_commands.h to be
merged through HID tree.
include/linux/mfd/cros_ec_commands.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 20ee71f10865..5fd0e429f472 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -2132,6 +2132,7 @@ struct ec_response_get_next_event_v1 {
/* Switches */
#define EC_MKBP_LID_OPEN 0
#define EC_MKBP_TABLET_MODE 1
+#define EC_MKBP_BASE_ATTACHED 2
/*****************************************************************************/
/* Temperature sensor commands */
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* [PATCH v2 2/2] HID: google: add support tablet mode switch for Whiskers
From: Dmitry Torokhov @ 2018-10-05 18:41 UTC (permalink / raw)
To: Jiri Kosina, Lee Jones
Cc: Benjamin Tissoires, linux-input, linux-kernel, Nicolas Boichat
In-Reply-To: <20181005184114.207503-1-dtor@chromium.org>
Whiskers is a foldable base, and thus requires combining "base presence"
signal coming from EC with base state signal (folded/unfolded) coming
from USB/HID interface to produce proper SW_TABLET_MODE event.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Signed-off-by: Dmitry Torokhov <dtor@chromium.org>
---
v2 changes (addressing Benjamin comments):
- moved everything into hid-google-hammer.c
- dropped USB interface parsing in favor of report parsing
- removed product/model from tablet mode switch device (this is different
from what Benjamin requested - using Google USB vendor ID - but
it is purely host interface - BUS_HOST - not USB or I2C, and those do
not carry vendor/product ID typically. Userspace is simply supposed
to check capabilities of the device).
drivers/hid/hid-google-hammer.c | 413 ++++++++++++++++++++++++++++++--
1 file changed, 394 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 6bf4da7ad63a..de3e7d055ca4 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -13,16 +13,268 @@
* any later version.
*/
+#include <linux/acpi.h>
#include <linux/hid.h>
#include <linux/leds.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeup.h>
+#include <asm/unaligned.h>
#include "hid-ids.h"
-#define MAX_BRIGHTNESS 100
+/*
+ * C(hrome)B(ase)A(ttached)S(witch) - switch exported by Chrome EC and reporting
+ * state of the "Whiskers" base - attached or detached. Whiskers USB device also
+ * reports position of the keyboard - folded or not. Combining base state and
+ * position allows us to generate proper "Tablet mode" events.
+ */
+struct cbas_ec {
+ struct device *dev; /* The platform device (EC) */
+ struct input_dev *input;
+ bool base_present;
+ struct notifier_block notifier;
+};
-/* HID usage for keyboard backlight (Alphanumeric display brightness) */
-#define HID_AD_BRIGHTNESS 0x00140046
+static struct cbas_ec cbas_ec;
+static DEFINE_SPINLOCK(cbas_ec_lock);
+static DEFINE_MUTEX(cbas_ec_reglock);
+
+static bool cbas_parse_base_state(const void *data)
+{
+ u32 switches = get_unaligned_le32(data);
+
+ return !!(switches & BIT(EC_MKBP_BASE_ATTACHED));
+}
+
+static int cbas_ec_query_base(struct cros_ec_device *ec_dev, bool get_state,
+ bool *state)
+{
+ struct ec_params_mkbp_info *params;
+ struct cros_ec_command *msg;
+ int ret;
+
+ msg = kzalloc(sizeof(*msg) + max(sizeof(u32), sizeof(*params)),
+ GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->command = EC_CMD_MKBP_INFO;
+ msg->version = 1;
+ msg->outsize = sizeof(*params);
+ msg->insize = sizeof(u32);
+ params = (struct ec_params_mkbp_info *)msg->data;
+ params->info_type = get_state ?
+ EC_MKBP_INFO_CURRENT : EC_MKBP_INFO_SUPPORTED;
+ params->event_type = EC_MKBP_EVENT_SWITCH;
+
+ ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ if (ret >= 0) {
+ if (ret != sizeof(u32)) {
+ dev_warn(ec_dev->dev, "wrong result size: %d != %zu\n",
+ ret, sizeof(u32));
+ ret = -EPROTO;
+ } else {
+ *state = cbas_parse_base_state(msg->data);
+ ret = 0;
+ }
+ }
+
+ kfree(msg);
+
+ return ret;
+}
+
+static int cbas_ec_notify(struct notifier_block *nb,
+ unsigned long queued_during_suspend,
+ void *_notify)
+{
+ struct cros_ec_device *ec = _notify;
+ unsigned long flags;
+ bool base_present;
+
+ if (ec->event_data.event_type == EC_MKBP_EVENT_SWITCH) {
+ base_present = cbas_parse_base_state(
+ &ec->event_data.data.switches);
+ dev_dbg(cbas_ec.dev,
+ "%s: base: %d\n", __func__, base_present);
+
+ if (device_may_wakeup(cbas_ec.dev) ||
+ !queued_during_suspend) {
+
+ pm_wakeup_event(cbas_ec.dev, 0);
+
+ spin_lock_irqsave(&cbas_ec_lock, flags);
+
+ /*
+ * While input layer dedupes the events, we do not want
+ * to disrupt the state reported by the base by
+ * overriding it with state reported by the LID. Only
+ * report changes, as we assume that on attach the base
+ * is not folded.
+ */
+ if (base_present != cbas_ec.base_present) {
+ input_report_switch(cbas_ec.input,
+ SW_TABLET_MODE,
+ !base_present);
+ input_sync(cbas_ec.input);
+ cbas_ec.base_present = base_present;
+ }
+
+ spin_unlock_irqrestore(&cbas_ec_lock, flags);
+ }
+ }
+
+ return NOTIFY_OK;
+}
+
+static __maybe_unused int cbas_ec_resume(struct device *dev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(dev->parent);
+ bool base_present;
+ int error;
+
+ error = cbas_ec_query_base(ec, true, &base_present);
+ if (error) {
+ dev_warn(dev, "failed to fetch base state on resume: %d\n",
+ error);
+ } else {
+ spin_lock_irq(&cbas_ec_lock);
+
+ cbas_ec.base_present = base_present;
+
+ /*
+ * Only report if base is disconnected. If base is connected,
+ * it will resend its state on resume, and we'll update it
+ * in hammer_event().
+ */
+ if (!cbas_ec.base_present) {
+ input_report_switch(cbas_ec.input, SW_TABLET_MODE, 1);
+ input_sync(cbas_ec.input);
+ }
+
+ spin_unlock_irq(&cbas_ec_lock);
+ }
+
+ return 0;
+}
+
+static const SIMPLE_DEV_PM_OPS(cbas_ec_pm_ops, NULL, cbas_ec_resume);
+
+static void cbas_ec_set_input(struct input_dev *input)
+{
+ /* Take the lock so hammer_event() does not race with us here */
+ spin_lock_irq(&cbas_ec_lock);
+ cbas_ec.input = input;
+ spin_unlock_irq(&cbas_ec_lock);
+}
+
+static int __cbas_ec_probe(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+ struct input_dev *input;
+ bool base_supported;
+ int error;
+
+ error = cbas_ec_query_base(ec, false, &base_supported);
+ if (error)
+ return error;
+
+ if (!base_supported)
+ return -ENXIO;
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "Whiskers Tablet Mode Switch";
+ input->id.bustype = BUS_HOST;
+
+ input_set_capability(input, EV_SW, SW_TABLET_MODE);
+
+ error = input_register_device(input);
+ if (error) {
+ dev_err(&pdev->dev, "cannot register input device: %d\n",
+ error);
+ return error;
+ }
+
+ /* Seed the state */
+ error = cbas_ec_query_base(ec, true, &cbas_ec.base_present);
+ if (error) {
+ dev_err(&pdev->dev, "cannot query base state: %d\n", error);
+ return error;
+ }
+
+ input_report_switch(input, SW_TABLET_MODE, !cbas_ec.base_present);
+
+ cbas_ec_set_input(input);
+
+ cbas_ec.dev = &pdev->dev;
+ cbas_ec.notifier.notifier_call = cbas_ec_notify;
+ error = blocking_notifier_chain_register(&ec->event_notifier,
+ &cbas_ec.notifier);
+ if (error) {
+ dev_err(&pdev->dev, "cannot register notifier: %d\n", error);
+ cbas_ec_set_input(NULL);
+ return error;
+ }
+
+ device_init_wakeup(&pdev->dev, true);
+ return 0;
+}
+
+static int cbas_ec_probe(struct platform_device *pdev)
+{
+ int retval;
+
+ mutex_lock(&cbas_ec_reglock);
+
+ if (cbas_ec.input) {
+ retval = -EBUSY;
+ goto out;
+ }
+
+ retval = __cbas_ec_probe(pdev);
+
+out:
+ mutex_unlock(&cbas_ec_reglock);
+ return retval;
+}
+
+static int cbas_ec_remove(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+
+ mutex_lock(&cbas_ec_reglock);
+
+ blocking_notifier_chain_unregister(&ec->event_notifier,
+ &cbas_ec.notifier);
+ cbas_ec_set_input(NULL);
+
+ mutex_unlock(&cbas_ec_reglock);
+ return 0;
+}
+
+static const struct acpi_device_id cbas_ec_acpi_ids[] = {
+ { "GOOG000B", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, cbas_ec_acpi_ids);
+
+static struct platform_driver cbas_ec_driver = {
+ .probe = cbas_ec_probe,
+ .remove = cbas_ec_remove,
+ .driver = {
+ .name = "cbas_ec",
+ .acpi_match_table = ACPI_PTR(cbas_ec_acpi_ids),
+ .pm = &cbas_ec_pm_ops,
+ },
+};
+
+#define MAX_BRIGHTNESS 100
struct hammer_kbd_leds {
struct led_classdev cdev;
@@ -90,33 +342,130 @@ static int hammer_register_leds(struct hid_device *hdev)
return devm_led_classdev_register(&hdev->dev, &kbd_backlight->cdev);
}
-static int hammer_input_configured(struct hid_device *hdev,
- struct hid_input *hi)
+#define HID_UP_GOOGLEVENDOR 0xffd10000
+#define HID_VD_KBD_FOLDED 0x00000019
+#define WHISKERS_KBD_FOLDED (HID_UP_GOOGLEVENDOR | HID_VD_KBD_FOLDED)
+
+/* HID usage for keyboard backlight (Alphanumeric display brightness) */
+#define HID_AD_BRIGHTNESS 0x00140046
+
+static int hammer_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field,
+ struct hid_usage *usage,
+ unsigned long **bit, int *max)
{
- struct list_head *report_list =
- &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
+ if (hdev->product == USB_DEVICE_ID_GOOGLE_WHISKERS &&
+ usage->hid == WHISKERS_KBD_FOLDED) {
+ /*
+ * We do not want to have this usage mapped as it will get
+ * mixed in with "base attached" signal and delivered over
+ * separate input device for tablet switch mode.
+ */
+ return -1;
+ }
+
+ return 0;
+}
+
+static int hammer_event(struct hid_device *hid, struct hid_field *field,
+ struct hid_usage *usage, __s32 value)
+{
+ unsigned long flags;
+
+ if (hid->product == USB_DEVICE_ID_GOOGLE_WHISKERS &&
+ usage->hid == WHISKERS_KBD_FOLDED) {
+ spin_lock_irqsave(&cbas_ec_lock, flags);
+
+ hid_dbg(hid, "%s: base: %d, folded: %d\n", __func__,
+ cbas_ec.base_present, value);
+
+ /*
+ * We should not get event if base is detached, but in case
+ * we happen to service HID and EC notifications out of order
+ * let's still check the "base present" flag.
+ */
+ if (cbas_ec.input && cbas_ec.base_present) {
+ input_report_switch(cbas_ec.input,
+ SW_TABLET_MODE, value);
+ input_sync(cbas_ec.input);
+ }
+
+ spin_unlock_irqrestore(&cbas_ec_lock, flags);
+ return 1; /* We handled this event */
+ }
+
+ return 0;
+}
+
+static bool hammer_is_keyboard_interface(struct hid_device *hdev)
+{
+ struct hid_report_enum *re = &hdev->report_enum[HID_INPUT_REPORT];
struct hid_report *report;
- if (list_empty(report_list))
- return 0;
+ list_for_each_entry(report, &re->report_list, list)
+ if (report->application == HID_GD_KEYBOARD)
+ return true;
- report = list_first_entry(report_list, struct hid_report, list);
+ return false;
+}
+
+static bool hammer_has_backlight_control(struct hid_device *hdev)
+{
+ struct hid_report_enum *re = &hdev->report_enum[HID_OUTPUT_REPORT];
+ struct hid_report *report;
+ int i, j;
- if (report->maxfield == 1 &&
- report->field[0]->application == HID_GD_KEYBOARD &&
- report->field[0]->maxusage == 1 &&
- report->field[0]->usage[0].hid == HID_AD_BRIGHTNESS) {
- int err = hammer_register_leds(hdev);
+ list_for_each_entry(report, &re->report_list, list) {
+ if (report->application != HID_GD_KEYBOARD)
+ continue;
- if (err)
+ for (i = 0; i < report->maxfield; i++) {
+ struct hid_field *field = report->field[i];
+
+ for (j = 0; j < field->maxusage; j++)
+ if (field->usage[j].hid == HID_AD_BRIGHTNESS)
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static int hammer_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int error;
+
+ /*
+ * We always want to poll for, and handle tablet mode events from
+ * Whiskers, even when nobody has opened the input device. This also
+ * prevents the hid core from dropping early tablet mode events from
+ * the device.
+ */
+ if (hdev->product == USB_DEVICE_ID_GOOGLE_WHISKERS &&
+ hammer_is_keyboard_interface(hdev))
+ hdev->quirks |= HID_QUIRK_ALWAYS_POLL;
+
+ error = hid_parse(hdev);
+ if (error)
+ return error;
+
+ error = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (error)
+ return error;
+
+ if (hammer_has_backlight_control(hdev)) {
+ error = hammer_register_leds(hdev);
+ if (error)
hid_warn(hdev,
"Failed to register keyboard backlight: %d\n",
- err);
+ error);
}
return 0;
}
+
static const struct hid_device_id hammer_devices[] = {
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
@@ -133,8 +482,34 @@ MODULE_DEVICE_TABLE(hid, hammer_devices);
static struct hid_driver hammer_driver = {
.name = "hammer",
.id_table = hammer_devices,
- .input_configured = hammer_input_configured,
+ .probe = hammer_probe,
+ .input_mapping = hammer_input_mapping,
+ .event = hammer_event,
};
-module_hid_driver(hammer_driver);
+
+static int __init hammer_init(void)
+{
+ int error;
+
+ error = platform_driver_register(&cbas_ec_driver);
+ if (error)
+ return error;
+
+ error = hid_register_driver(&hammer_driver);
+ if (error) {
+ platform_driver_unregister(&cbas_ec_driver);
+ return error;
+ }
+
+ return 0;
+}
+module_init(hammer_init);
+
+static void __exit hammer_exit(void)
+{
+ hid_unregister_driver(&hammer_driver);
+ platform_driver_unregister(&cbas_ec_driver);
+}
+module_exit(hammer_exit);
MODULE_LICENSE("GPL");
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH] input: st1232: set INPUT_PROP_DIRECT property
From: Dmitry Torokhov @ 2018-10-05 18:47 UTC (permalink / raw)
To: Martin Kepplinger; +Cc: geert+renesas, linux-input, linux-kernel
In-Reply-To: <20181005081407.15376-1-martink@posteo.de>
On Fri, Oct 05, 2018 at 10:14:07AM +0200, Martin Kepplinger wrote:
> This is how userspace checks for touchscreen devices most reliably.
>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
Applied, thank you.
> ---
> drivers/input/touchscreen/st1232.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
> index d5dfa4053bbf..b71673911aac 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -195,6 +195,7 @@ static int st1232_ts_probe(struct i2c_client *client,
> input_dev->id.bustype = BUS_I2C;
> input_dev->dev.parent = &client->dev;
>
> + __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> __set_bit(EV_SYN, input_dev->evbit);
> __set_bit(EV_KEY, input_dev->evbit);
> __set_bit(EV_ABS, input_dev->evbit);
> --
> 2.19.0
>
--
Dmitry
^ permalink raw reply
* Re: [RFC/PATCH 2/5] device property: introduce notion of subnodes for legacy boards
From: Dmitry Torokhov @ 2018-10-05 21:47 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Linus Walleij, Rafael J . Wysocki, linux-input, linux-gpio,
linux-kernel, Andy Shevchenko
In-Reply-To: <20180925121927.GL11965@kuha.fi.intel.com>
Hi Heikki,
On Tue, Sep 25, 2018 at 03:19:27PM +0300, Heikki Krogerus wrote:
> On Mon, Sep 24, 2018 at 11:45:43AM -0700, Dmitry Torokhov wrote:
> > I think we are talking about totally different use cases and that is why
> > we are having hard time coming to a mutually agreeable solution. Could
> > you please describe in more detail what you would like to achieve,
> > and preferably show how it is described now with DT and/or ACPI, so that
> > I have a better frame of reference.
>
> Yes, of course. Sorry.
>
> USB ports are devices that usually the USB controller drivers register
> (or actually the USB core code). They are represented in both ACPI and
> DT as child nodes of the controller device node. The USB connector OF
> node is defined in file
> Documentation/devicetree/bindings/connector/usb-connector.txt
>
> In short, the controller drivers will request handle to a child node
> that represents a port, and only after that register the actual port
> device.
>
> The drivers I'm looking at currently are the USB Type-C port
> controller drivers and the port manager (in Greg's usb-next or
> linux-next):
>
> drivers/usb/typec/tcpm/tcpci.c
> drivers/usb/typec/tcpm/fusb302.c
> drivers/usb/typec/tcpm/tcpm.c
>
> The goal is simply to get rid of the platform data as usual, and
> ideally so that we don't need any extra code in order to support the
> "legacy" platforms.
Are these actually used on any of the "legacy" platforms? I fetched
linux-next today, but I do not actually see anything in
drivers/usb/typec touching platform data...
In the context of the connector, even before we descend to child nodes
details, how do you propose implementing references between fwnodes?
Especially since the other node (in case you complementing existing
topology) may be ACPI or DT instance?
I want to say that "You aren't gonna need it" for what you are asking
here and we can actually split it apart if and when we actually want to
separate creation of pset-backed device nodes and instantiating
corresponding device structures.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Bjorn Andersson @ 2018-10-06 7:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
sparclinux-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
qat-linux-ral2JQCrhuEAvxtiuMwx3w,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
ceph-devel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, David S. Miller,
linux-btrfs-u79uwXL29TY76Z2rM5mHXA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <20180912151134.436719-1-arnd-r2nGTMty4D4@public.gmane.org>
On Wed 12 Sep 08:08 PDT 2018, Arnd Bergmann wrote:
[..]
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index a76b963a7e50..02aefb2b2d47 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -285,7 +285,7 @@ static const struct file_operations rpmsg_eptdev_fops = {
> .write = rpmsg_eptdev_write,
> .poll = rpmsg_eptdev_poll,
> .unlocked_ioctl = rpmsg_eptdev_ioctl,
> - .compat_ioctl = rpmsg_eptdev_ioctl,
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
> };
>
> static ssize_t name_show(struct device *dev, struct device_attribute *attr,
> @@ -446,7 +446,7 @@ static const struct file_operations rpmsg_ctrldev_fops = {
> .open = rpmsg_ctrldev_open,
> .release = rpmsg_ctrldev_release,
> .unlocked_ioctl = rpmsg_ctrldev_ioctl,
> - .compat_ioctl = rpmsg_ctrldev_ioctl,
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
> };
>
For rpmsg part
Acked-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Regards,
Bjorn
^ permalink raw reply
* [PATCH] platform/x86: touchscreen_dmi: Add info for the Trekstor Primetab T13B tablet
From: Marian Cepok @ 2018-10-08 7:25 UTC (permalink / raw)
To: hdegoede; +Cc: dvhart, andy, linux-input, platform-driver-x86, linux-kernel
Add touchscreen info for the Trekstor Primetab T13B tablet.
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Marian Cepok <marian.cepok@gmail.com>
---
drivers/platform/x86/touchscreen_dmi.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c b/drivers/platform/x86/touchscreen_dmi.c
index cb204f973491..c07954f0efc6 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -381,6 +381,22 @@ static const struct ts_dmi_data trekstor_primebook_c13_data = {
.properties = trekstor_primebook_c13_props,
};
+static const struct property_entry trekstor_primetab_t13b_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 2500),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1900),
+ PROPERTY_ENTRY_STRING("firmware-name",
+ "gsl1680-trekstor-primetab-t13b.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ PROPERTY_ENTRY_BOOL("touchscreen-inverted-y"),
+ { }
+};
+
+static const struct ts_dmi_data trekstor_primetab_t13b_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = trekstor_primetab_t13b_props,
+};
+
static const struct property_entry trekstor_surftab_twin_10_1_props[] = {
PROPERTY_ENTRY_U32("touchscreen-size-x", 1900),
PROPERTY_ENTRY_U32("touchscreen-size-y", 1280),
@@ -648,6 +664,14 @@ static const struct dmi_system_id touchscreen_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "Primebook C13"),
},
},
+ {
+ /* Trekstor Primetab T13B */
+ .driver_data = (void *)&trekstor_primetab_t13b_data,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "TREKSTOR"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Primetab T13B"),
+ },
+ },
{
/* TrekStor SurfTab twin 10.1 ST10432-8 */
.driver_data = (void *)&trekstor_surftab_twin_10_1_data,
--
2.17.1
^ permalink raw reply related
* [PATCH] Input: xen-kbdfront - mark expected switch fall-through
From: Gustavo A. R. Silva @ 2018-10-08 14:07 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in this particular case, I placed the "fall through"
part at the beginning of the code comment, which is what GCC is
expecting to find.
Addresses-Coverity-ID: 114757 ("Missing break in switch")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/input/misc/xen-kbdfront.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 594f72e..24bc5c5 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -524,7 +524,7 @@ static void xenkbd_backend_changed(struct xenbus_device *dev,
case XenbusStateClosed:
if (dev->state == XenbusStateClosed)
break;
- /* Missed the backend's CLOSING state -- fallthrough */
+ /* fall through - Missed the backend's CLOSING state */
case XenbusStateClosing:
xenbus_frontend_closed(dev);
break;
--
2.7.4
^ permalink raw reply related
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