* [PATCH 08/50] USB: legousbtower: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei, Juergen Stuber
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Cc: Juergen Stuber <starblue@users.sourceforge.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/misc/legousbtower.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 8089479..4044989 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -771,6 +771,7 @@ static void tower_interrupt_in_callback (struct urb *urb)
struct lego_usb_tower *dev = urb->context;
int status = urb->status;
int retval;
+ unsigned long flags;
dbg(4, "%s: enter, status %d", __func__, status);
@@ -788,7 +789,7 @@ static void tower_interrupt_in_callback (struct urb *urb)
}
if (urb->actual_length > 0) {
- spin_lock (&dev->read_buffer_lock);
+ spin_lock_irqsave (&dev->read_buffer_lock, flags);
if (dev->read_buffer_length + urb->actual_length < read_buffer_size) {
memcpy (dev->read_buffer + dev->read_buffer_length,
dev->interrupt_in_buffer,
@@ -799,7 +800,7 @@ static void tower_interrupt_in_callback (struct urb *urb)
} else {
printk(KERN_WARNING "%s: read_buffer overflow, %d bytes dropped", __func__, urb->actual_length);
}
- spin_unlock (&dev->read_buffer_lock);
+ spin_unlock_irqrestore (&dev->read_buffer_lock, flags);
}
resubmit:
--
1.7.9.5
^ permalink raw reply related
* [PATCH 07/50] USB: ldusb: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Alan Stern,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Ming Lei
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/usb/misc/ldusb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/misc/ldusb.c b/drivers/usb/misc/ldusb.c
index ac76229..8bae18e 100644
--- a/drivers/usb/misc/ldusb.c
+++ b/drivers/usb/misc/ldusb.c
@@ -249,6 +249,7 @@ static void ld_usb_interrupt_in_callback(struct urb *urb)
unsigned int next_ring_head;
int status = urb->status;
int retval;
+ unsigned long flags;
if (status) {
if (status == -ENOENT ||
@@ -258,12 +259,12 @@ static void ld_usb_interrupt_in_callback(struct urb *urb)
} else {
dbg_info(&dev->intf->dev, "%s: nonzero status received: %d\n",
__func__, status);
- spin_lock(&dev->rbsl);
+ spin_lock_irqsave(&dev->rbsl, flags);
goto resubmit; /* maybe we can recover */
}
}
- spin_lock(&dev->rbsl);
+ spin_lock_irqsave(&dev->rbsl, flags);
if (urb->actual_length > 0) {
next_ring_head = (dev->ring_head+1) % ring_buffer_size;
if (next_ring_head != dev->ring_tail) {
@@ -292,7 +293,7 @@ resubmit:
dev->buffer_overflow = 1;
}
}
- spin_unlock(&dev->rbsl);
+ spin_unlock_irqrestore(&dev->rbsl, flags);
exit:
dev->interrupt_in_done = 1;
wake_up_interruptible(&dev->read_wait);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 06/50] USB: iowarrior: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/misc/iowarrior.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index d36f34e..010ed6d 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -162,6 +162,7 @@ static void iowarrior_callback(struct urb *urb)
int offset;
int status = urb->status;
int retval;
+ unsigned long flags;
switch (status) {
case 0:
@@ -175,7 +176,7 @@ static void iowarrior_callback(struct urb *urb)
goto exit;
}
- spin_lock(&dev->intr_idx_lock);
+ spin_lock_irqsave(&dev->intr_idx_lock, flags);
intr_idx = atomic_read(&dev->intr_idx);
/* aux_idx become previous intr_idx */
aux_idx = (intr_idx == 0) ? (MAX_INTERRUPT_BUFFER - 1) : (intr_idx - 1);
@@ -211,7 +212,7 @@ static void iowarrior_callback(struct urb *urb)
*(dev->read_queue + offset + (dev->report_size)) = dev->serial_number++;
atomic_set(&dev->intr_idx, aux_idx);
- spin_unlock(&dev->intr_idx_lock);
+ spin_unlock_irqrestore(&dev->intr_idx_lock, flags);
/* tell the blocking read about the new data */
wake_up_interruptible(&dev->read_wait);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 05/50] USB: misc: uss720: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/misc/uss720.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index e129cf6..f7d15e8 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -121,6 +121,7 @@ static void async_complete(struct urb *urb)
dev_err(&urb->dev->dev, "async_complete: urb error %d\n",
status);
} else if (rq->dr.bRequest == 3) {
+ unsigned long flags;
memcpy(priv->reg, rq->reg, sizeof(priv->reg));
#if 0
dev_dbg(&priv->usbdev->dev,
@@ -131,8 +132,11 @@ static void async_complete(struct urb *urb)
(unsigned int)priv->reg[6]);
#endif
/* if nAck interrupts are enabled and we have an interrupt, call the interrupt procedure */
- if (rq->reg[2] & rq->reg[1] & 0x10 && pp)
+ if (rq->reg[2] & rq->reg[1] & 0x10 && pp) {
+ local_irq_save(flags);
parport_generic_irq(pp);
+ local_irq_restore(flags);
+ }
}
complete(&rq->compl);
kref_put(&rq->ref_count, destroy_async);
--
1.7.9.5
^ permalink raw reply related
* [PATCH 04/50] USB: adutux: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei, Lisa Nguyen
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Cc: Lisa Nguyen <lisa@xenapiadmin.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/misc/adutux.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/misc/adutux.c b/drivers/usb/misc/adutux.c
index eb3c8c1..387c75e 100644
--- a/drivers/usb/misc/adutux.c
+++ b/drivers/usb/misc/adutux.c
@@ -195,12 +195,13 @@ static void adu_interrupt_in_callback(struct urb *urb)
{
struct adu_device *dev = urb->context;
int status = urb->status;
+ unsigned long flags;
dbg(4, " %s : enter, status %d", __func__, status);
adu_debug_data(5, __func__, urb->actual_length,
urb->transfer_buffer);
- spin_lock(&dev->buflock);
+ spin_lock_irqsave(&dev->buflock, flags);
if (status != 0) {
if ((status != -ENOENT) && (status != -ECONNRESET) &&
@@ -229,7 +230,7 @@ static void adu_interrupt_in_callback(struct urb *urb)
exit:
dev->read_urb_finished = 1;
- spin_unlock(&dev->buflock);
+ spin_unlock_irqrestore(&dev->buflock, flags);
/* always wake up so we recover from errors */
wake_up_interruptible(&dev->read_wait);
adu_debug_data(5, __func__, urb->actual_length,
@@ -241,6 +242,7 @@ static void adu_interrupt_out_callback(struct urb *urb)
{
struct adu_device *dev = urb->context;
int status = urb->status;
+ unsigned long flags;
dbg(4, " %s : enter, status %d", __func__, status);
adu_debug_data(5, __func__, urb->actual_length, urb->transfer_buffer);
@@ -254,10 +256,10 @@ static void adu_interrupt_out_callback(struct urb *urb)
goto exit;
}
- spin_lock(&dev->buflock);
+ spin_lock_irqsave(&dev->buflock, flags);
dev->out_urb_finished = 1;
wake_up(&dev->write_wait);
- spin_unlock(&dev->buflock);
+ spin_unlock_irqrestore(&dev->buflock, flags);
exit:
adu_debug_data(5, __func__, urb->actual_length,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 03/50] USB: usblp: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei, Pete Zaitcev
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Cc: Pete Zaitcev <zaitcev@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/class/usblp.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index d4c47d5..04163d8 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -297,6 +297,7 @@ static void usblp_bulk_read(struct urb *urb)
{
struct usblp *usblp = urb->context;
int status = urb->status;
+ unsigned long flags;
if (usblp->present && usblp->used) {
if (status)
@@ -304,14 +305,14 @@ static void usblp_bulk_read(struct urb *urb)
"nonzero read bulk status received: %d\n",
usblp->minor, status);
}
- spin_lock(&usblp->lock);
+ spin_lock_irqsave(&usblp->lock, flags);
if (status < 0)
usblp->rstatus = status;
else
usblp->rstatus = urb->actual_length;
usblp->rcomplete = 1;
wake_up(&usblp->rwait);
- spin_unlock(&usblp->lock);
+ spin_unlock_irqrestore(&usblp->lock, flags);
usb_free_urb(urb);
}
@@ -320,6 +321,7 @@ static void usblp_bulk_write(struct urb *urb)
{
struct usblp *usblp = urb->context;
int status = urb->status;
+ unsigned long flags;
if (usblp->present && usblp->used) {
if (status)
@@ -327,7 +329,7 @@ static void usblp_bulk_write(struct urb *urb)
"nonzero write bulk status received: %d\n",
usblp->minor, status);
}
- spin_lock(&usblp->lock);
+ spin_lock_irqsave(&usblp->lock, flags);
if (status < 0)
usblp->wstatus = status;
else
@@ -335,7 +337,7 @@ static void usblp_bulk_write(struct urb *urb)
usblp->no_paper = 0;
usblp->wcomplete = 1;
wake_up(&usblp->wwait);
- spin_unlock(&usblp->lock);
+ spin_unlock_irqrestore(&usblp->lock, flags);
usb_free_urb(urb);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH 02/50] USB: cdc-wdm: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Alan Stern,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Ming Lei
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Cc: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/usb/class/cdc-wdm.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 8a230f0..5f78d18 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -143,10 +143,12 @@ found:
static void wdm_out_callback(struct urb *urb)
{
struct wdm_device *desc;
+ unsigned long flags;
+
desc = urb->context;
- spin_lock(&desc->iuspin);
+ spin_lock_irqsave(&desc->iuspin, flags);
desc->werr = urb->status;
- spin_unlock(&desc->iuspin);
+ spin_unlock_irqrestore(&desc->iuspin, flags);
kfree(desc->outbuf);
desc->outbuf = NULL;
clear_bit(WDM_IN_USE, &desc->flags);
@@ -158,8 +160,9 @@ static void wdm_in_callback(struct urb *urb)
struct wdm_device *desc = urb->context;
int status = urb->status;
int length = urb->actual_length;
+ unsigned long flags;
- spin_lock(&desc->iuspin);
+ spin_lock_irqsave(&desc->iuspin, flags);
clear_bit(WDM_RESPONDING, &desc->flags);
if (status) {
@@ -203,7 +206,7 @@ skip_error:
wake_up(&desc->wait);
set_bit(WDM_READ, &desc->flags);
- spin_unlock(&desc->iuspin);
+ spin_unlock_irqrestore(&desc->iuspin, flags);
}
static void wdm_int_callback(struct urb *urb)
@@ -212,6 +215,7 @@ static void wdm_int_callback(struct urb *urb)
int status = urb->status;
struct wdm_device *desc;
struct usb_cdc_notification *dr;
+ unsigned long flags;
desc = urb->context;
dr = (struct usb_cdc_notification *)desc->sbuf;
@@ -260,7 +264,7 @@ static void wdm_int_callback(struct urb *urb)
goto exit;
}
- spin_lock(&desc->iuspin);
+ spin_lock_irqsave(&desc->iuspin, flags);
clear_bit(WDM_READ, &desc->flags);
set_bit(WDM_RESPONDING, &desc->flags);
if (!test_bit(WDM_DISCONNECTING, &desc->flags)
@@ -269,7 +273,7 @@ static void wdm_int_callback(struct urb *urb)
dev_dbg(&desc->intf->dev, "%s: usb_submit_urb %d",
__func__, rv);
}
- spin_unlock(&desc->iuspin);
+ spin_unlock_irqrestore(&desc->iuspin, flags);
if (rv < 0) {
clear_bit(WDM_RESPONDING, &desc->flags);
if (rv == -EPERM)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 01/50] USB: devio: spin_lock in complete() cleanup
From: Ming Lei @ 2013-07-11 9:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-usb, Oliver Neukum, Alan Stern, linux-input,
linux-bluetooth, netdev, linux-wireless, linux-media, alsa-devel,
Ming Lei
In-Reply-To: <1373533573-12272-1-git-send-email-ming.lei@canonical.com>
Complete() will be run with interrupt enabled, so change to
spin_lock_irqsave().
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/core/devio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 0598650..21e6ec6 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -495,8 +495,9 @@ static void async_completed(struct urb *urb)
u32 secid = 0;
const struct cred *cred = NULL;
int signr;
+ unsigned long flags;
- spin_lock(&ps->lock);
+ spin_lock_irqsave(&ps->lock, flags);
list_move_tail(&as->asynclist, &ps->async_completed);
as->status = urb->status;
signr = as->signr;
@@ -518,7 +519,7 @@ static void async_completed(struct urb *urb)
if (as->status < 0 && as->bulk_addr && as->status != -ECONNRESET &&
as->status != -ENOENT)
cancel_bulk_urbs(ps, as->bulk_addr);
- spin_unlock(&ps->lock);
+ spin_unlock_irqrestore(&ps->lock, flags);
if (signr) {
kill_pid_info_as_cred(sinfo.si_signo, &sinfo, pid, cred, secid);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] usb: USB host support should depend on HAS_DMA
From: Geert Uytterhoeven @ 2013-07-11 7:49 UTC (permalink / raw)
To: Alan Stern
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input@vger.kernel.org,
Linux Media Mailing List, USB list, linux-kernel@vger.kernel.org
In-Reply-To: <Pine.LNX.4.44L0.1307102054340.11279-100000@netrider.rowland.org>
On Thu, Jul 11, 2013 at 3:01 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 11 Jul 2013, Arnd Bergmann wrote:
>
>> On Wednesday 10 July 2013, Alan Stern wrote:
>> > This isn't right. There are USB host controllers that use PIO, not
>> > DMA. The HAS_DMA dependency should go with the controller driver, not
>> > the USB core.
>> >
>> > On the other hand, the USB core does call various routines like
>> > dma_unmap_single. It ought to be possible to compile these calls even
>> > when DMA isn't enabled. That is, they should be defined as do-nothing
>> > stubs.
>>
>> The asm-generic/dma-mapping-broken.h file intentionally causes link
>> errors, but that could be changed.
>>
>> The better approach in my mind would be to replace code like
>>
>>
>> if (hcd->self.uses_dma)
>>
>> with
>>
>> if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>>
>> which will reliably cause that reference to be omitted from object code,
>> but not stop giving link errors for drivers that actually require
>> DMA.
>
> How will it give link errors for drivers that require DMA?
It won't. Unless the host driver itself calls into the DMA API, too
(are there any that don't?).
> Besides, wouldn't it be better to get an error at config time rather
> than at link time? Or even better still, not to be allowed to
> configure drivers that depend on DMA if DMA isn't available?
Indeed.
> If we add an explicit dependency for HAS_DMA to the Kconfig entries for
> these drivers, then your suggestion would be a good way to allow
> usbcore to be built independently of DMA support.
However, having the link errors helps when annotating the Kconfig files
with HAS_DMA dependencies.
Unfortunately the check for "hcd->self.uses_dma" (which boils down to
"dev->dma_mask != NULL") isn't sufficient to cause breakage at compilation
time when a Kconfig entry incorrectly doesn't depend on HAS_DMA.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 10/53] Input: atmel_mxt_ts - Add memory access interface via sysfs
From: Nick Dyer @ 2013-07-11 7:41 UTC (permalink / raw)
To: Mark Brown
Cc: Dmitry Torokhov, Daniel Kurtz, Henrik Rydberg, Joonyoung Shim,
Alan.Bowens, linux-input, linux-kernel, pmeerw, bleung, olofj
In-Reply-To: <20130702101125.GD27646@sirena.org.uk>
Mark Brown wrote:
> On Fri, Jun 21, 2013 at 09:16:16AM -0700, Nick Dyer wrote:
>> Mark Brown wrote:
>>> Yes, to be honest. I'd hope it wouldn't be increasing the number of
>>> read/write operations...
>
>> For some operations it does. For example updating the whole chip config
>> (which is a common thing to want to do), it would turn a couple of write
>> operations into ~20 on recent chips.
>
> Is that really happening on peformance critical paths other than initial
> power up (which could be handled more neatly anyway).
Well, you're right that we could probably add more API for performance
critical stuff. But that wasn't your original question.
>>> and of course a system integrator may choose not to copy the reference
>>> design in this respect, it does seem a bit odd after all.
>
>> You're being a bit optimistic there. Examples of devices that require
>> this are Samsung Galaxy Tab 10.1, Asus Transformer TF101.
>
> If absoluely nobody has used the separate wakeup pin then the hardware
> designers are wasting a pin there... my point isn't that nobody would
> use the reference design it's that some boards will have the separate
> signal.
That's entirely hypothetical, and you're wasting our time until you can
actually point to such hardware, happy to write patches to support that
mode of operation as well if you do.
^ permalink raw reply
* [PATCH 1/2] Input: imx_keypad - check the return value from clk_prepare_enable()
From: Fabio Estevam @ 2013-07-11 4:44 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
clk_prepare_enable() may fail, so let's check its return value and propagate it
in the case of error.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/input/keyboard/imx_keypad.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 03c8cc5..99d3360 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -512,7 +512,9 @@ static int imx_keypad_probe(struct platform_device *pdev)
input_set_drvdata(input_dev, keypad);
/* Ensure that the keypad will stay dormant until opened */
- clk_prepare_enable(keypad->clk);
+ error = clk_prepare_enable(keypad->clk);
+ if (error)
+ return error;
imx_keypad_inhibit(keypad);
clk_disable_unprepare(keypad->clk);
--
1.8.1.2
^ permalink raw reply related
* [PATCH 2/2] Input: imx_keypad - Simplify devm_kzalloc
From: Fabio Estevam @ 2013-07-11 4:44 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, Fabio Estevam
In-Reply-To: <1373517888-15615-1-git-send-email-festevam@gmail.com>
From: Fabio Estevam <fabio.estevam@freescale.com>
According to Documentation/CodingStyle:
"The preferred form for passing a size of a struct is the following:
p = kmalloc(sizeof(*p), ...);"
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/input/keyboard/imx_keypad.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 99d3360..c944659 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -454,8 +454,7 @@ static int imx_keypad_probe(struct platform_device *pdev)
return -ENOMEM;
}
- keypad = devm_kzalloc(&pdev->dev, sizeof(struct imx_keypad),
- GFP_KERNEL);
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
if (!keypad) {
dev_err(&pdev->dev, "not enough memory for driver data\n");
return -ENOMEM;
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH] usb: USB host support should depend on HAS_DMA
From: Alan Stern @ 2013-07-11 1:01 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Geert Uytterhoeven, Greg Kroah-Hartman, linux-input, linux-media,
linux-usb, linux-kernel
In-Reply-To: <201307110112.57398.arnd@arndb.de>
On Thu, 11 Jul 2013, Arnd Bergmann wrote:
> On Wednesday 10 July 2013, Alan Stern wrote:
> > This isn't right. There are USB host controllers that use PIO, not
> > DMA. The HAS_DMA dependency should go with the controller driver, not
> > the USB core.
> >
> > On the other hand, the USB core does call various routines like
> > dma_unmap_single. It ought to be possible to compile these calls even
> > when DMA isn't enabled. That is, they should be defined as do-nothing
> > stubs.
>
> The asm-generic/dma-mapping-broken.h file intentionally causes link
> errors, but that could be changed.
>
> The better approach in my mind would be to replace code like
>
>
> if (hcd->self.uses_dma)
>
> with
>
> if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
>
> which will reliably cause that reference to be omitted from object code,
> but not stop giving link errors for drivers that actually require
> DMA.
How will it give link errors for drivers that require DMA?
Besides, wouldn't it be better to get an error at config time rather
than at link time? Or even better still, not to be allowed to
configure drivers that depend on DMA if DMA isn't available?
If we add an explicit dependency for HAS_DMA to the Kconfig entries for
these drivers, then your suggestion would be a good way to allow
usbcore to be built independently of DMA support.
Alan Stern
^ permalink raw reply
* Re: [PATCH] usb: USB host support should depend on HAS_DMA
From: Arnd Bergmann @ 2013-07-10 23:12 UTC (permalink / raw)
To: Alan Stern
Cc: Geert Uytterhoeven, Greg Kroah-Hartman, linux-input, linux-media,
linux-usb, linux-kernel
In-Reply-To: <Pine.LNX.4.44L0.1307101724430.1215-100000@iolanthe.rowland.org>
On Wednesday 10 July 2013, Alan Stern wrote:
> This isn't right. There are USB host controllers that use PIO, not
> DMA. The HAS_DMA dependency should go with the controller driver, not
> the USB core.
>
> On the other hand, the USB core does call various routines like
> dma_unmap_single. It ought to be possible to compile these calls even
> when DMA isn't enabled. That is, they should be defined as do-nothing
> stubs.
The asm-generic/dma-mapping-broken.h file intentionally causes link
errors, but that could be changed.
The better approach in my mind would be to replace code like
if (hcd->self.uses_dma)
with
if (IS_ENABLED(CONFIG_HAS_DMA) && hcd->self.uses_dma) {
which will reliably cause that reference to be omitted from object code,
but not stop giving link errors for drivers that actually require
DMA.
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Dmitry Torokhov @ 2013-07-10 22:55 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Grant Likely, devicetree-discuss, John Stultz, kernel-team,
linux-input@vger.kernel.org
In-Reply-To: <CANLsYkw==yAEcwD9RAPCCHB0Y45Bo2MnfLSVP5s-g6kpAu9k4g@mail.gmail.com>
On Wednesday, July 10, 2013 04:29:00 PM Mathieu Poirier wrote:
> On 10 July 2013 16:20, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On Wednesday, July 10, 2013 10:50:26 PM Grant Likely wrote:
> > > On Wed, Jul 10, 2013 at 5:52 PM, Dmitry Torokhov
> > >
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
> > > >> On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier
> >
> > <mathieu.poirier@linaro.org> wrote:
> > > >> > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> > > >> > >>>> I do not agree. We want the binding to be generic and not
> > > >> > >>>> tied
> > > >> > >>>> specifically to the keyreset functionality. As such
> > > >> > >>>> 'input-keyset' or
> > > >> > >>>> 'input-keychord' are more appropriate.
> > > >> > >>>
> > > >> > >>> The binding is defined specifically for sysrq and specifically
> >
> > to
> >
> > > >> > >>> perform reset action.
> > > >> > >>
> > > >> > >> Yes for now but as the examples in the binding show, it is easy
> >
> > to
> >
> > > >> > >> envision how other drivers could use it.
> > > >> > >
> > > >> > > I think you over-complicate things here. Unlike matrix-keypad
> > > >> > > binding,
> > > >> > > where you have a common parsing code, here we have an individual
> > > >> > > driver.
> > > >> > > I really do not see anyone else using such sequences or chords as
> > > >> > > such
> > > >> > > processing should be done in userspace. Sysrq is quite an
> >
> > exception.
> >
> > > >> > To be honest I don't have a very strong opinion on the binding. I
> >
> > made
> >
> > > >> > it as generic as possible on the guidance of the DT people. Let's
> >
> > see
> >
> > > >> > what they think of it.
> > > >>
> > > >> Hi Mathieu,
> > > >>
> > > >> As per our conversation just now at Connect, the binding should
> >
> > probably
> >
> > > >> look like this:
> > > >>
> > > >> Sysrq keyset binding:
> > > >>
> > > >> The /chosen node can contain a linux,input-keyset-sysrq child node to
> > > >> define a set of keys that will generate a sysrq when pressed
> > > >> together.
> > > >
> > > > Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
> > > > whatever. The sysrq setting is system-wide and applicable to all
> > > > devices. Given that it is used only on mobile, where there not that
> > > > many input devices (a few keys and touchscreen) I do not believe we
> > > > should consider adding per-device settings.
> > >
> > > It's in /chosen, that isn't per-device.
> > >
> > > >> Required properties:
> > > >> keyset: array of keycodes
> > > >
> > > > Please, let's call it 'key-reset-seq', because it is exactly the reset
> > > > sequence. There won't be any additional sequences or chords as those
> > > > should be handled in userspace, sysrq is a special case here.
> > >
> > > This is absolutely a linux-specific binding. It encodes the Linux
> > > keycodes, and generates a linux meaning. I'm usually all about
> > > carrying the OS-independent banner when defining DT bindings, but in
> > > this case the linux prefix and sysrq reference is completely
> > > appropriate.
> >
> > OK, I have no idea what "/chosen" actually means. What I am trying to say
> > that there should be either "sysrq" or "linux,sysrq" node and that is what
> > sysrq driver will be looking for.
>
> Chosen pertains to system wide parameters that aren't related to hw
> specific devices. Correct, the driver will look exactly for
> "linux,sysrq-reset-seq" in the "chosen" node and nowhere else.
OK, it looks like we are talking about the same thing. I seem to have
mis-parsed the original proposal.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Mathieu Poirier @ 2013-07-10 22:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Grant Likely, kernel-team-z5hGa2qSFaRBDgjK7y7TUQ,
devicetree-discuss, John Stultz,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <3572203.AkEVm8LFzu-wUGeVx6es1+Q2O5dskk9LyLysJ1jNyTM@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3541 bytes --]
On 10 July 2013 16:20, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wednesday, July 10, 2013 10:50:26 PM Grant Likely wrote:
> > On Wed, Jul 10, 2013 at 5:52 PM, Dmitry Torokhov
> >
> > <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
> > >> On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier
> <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> > >> > >>>> I do not agree. We want the binding to be generic and not tied
> > >> > >>>> specifically to the keyreset functionality. As such
> > >> > >>>> 'input-keyset' or
> > >> > >>>> 'input-keychord' are more appropriate.
> > >> > >>>
> > >> > >>> The binding is defined specifically for sysrq and specifically
> to
> > >> > >>> perform reset action.
> > >> > >>
> > >> > >> Yes for now but as the examples in the binding show, it is easy
> to
> > >> > >> envision how other drivers could use it.
> > >> > >
> > >> > > I think you over-complicate things here. Unlike matrix-keypad
> > >> > > binding,
> > >> > > where you have a common parsing code, here we have an individual
> > >> > > driver.
> > >> > > I really do not see anyone else using such sequences or chords as
> > >> > > such
> > >> > > processing should be done in userspace. Sysrq is quite an
> exception.
> > >> >
> > >> > To be honest I don't have a very strong opinion on the binding. I
> made
> > >> > it as generic as possible on the guidance of the DT people. Let's
> see
> > >> > what they think of it.
> > >>
> > >> Hi Mathieu,
> > >>
> > >> As per our conversation just now at Connect, the binding should
> probably
> > >> look like this:
> > >>
> > >> Sysrq keyset binding:
> > >>
> > >> The /chosen node can contain a linux,input-keyset-sysrq child node to
> > >> define a set of keys that will generate a sysrq when pressed together.
> > >
> > > Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
> > > whatever. The sysrq setting is system-wide and applicable to all
> > > devices. Given that it is used only on mobile, where there not that
> > > many input devices (a few keys and touchscreen) I do not believe we
> > > should consider adding per-device settings.
> >
> > It's in /chosen, that isn't per-device.
> >
> > >> Required properties:
> > >> keyset: array of keycodes
> > >
> > > Please, let's call it 'key-reset-seq', because it is exactly the reset
> > > sequence. There won't be any additional sequences or chords as those
> > > should be handled in userspace, sysrq is a special case here.
> >
> > This is absolutely a linux-specific binding. It encodes the Linux
> > keycodes, and generates a linux meaning. I'm usually all about
> > carrying the OS-independent banner when defining DT bindings, but in
> > this case the linux prefix and sysrq reference is completely
> > appropriate.
>
> OK, I have no idea what "/chosen" actually means. What I am trying to say
> that there should be either "sysrq" or "linux,sysrq" node and that is what
> sysrq driver will be looking for.
>
Chosen pertains to system wide parameters that aren't related to hw
specific devices. Correct, the driver will look exactly for
"linux,sysrq-reset-seq" in the "chosen" node and nowhere else.
>
> The entire node is Linux-specific and therefore there is no point in
> marking only one of the properties (the key sequence) Linux-specific while
> leaving other ones generic.
>
> Thanks.
>
> --
> Dmitry
>
[-- Attachment #1.2: Type: text/html, Size: 5140 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Dmitry Torokhov @ 2013-07-10 22:20 UTC (permalink / raw)
To: Grant Likely
Cc: Mathieu Poirier, devicetree-discuss, John Stultz, kernel-team,
linux-input@vger.kernel.org
In-Reply-To: <CACxGe6s7b=qgUGM4-aZSaqVWFeD0hGeo2wVdSK1KSOGYbHUEsQ@mail.gmail.com>
On Wednesday, July 10, 2013 10:50:26 PM Grant Likely wrote:
> On Wed, Jul 10, 2013 at 5:52 PM, Dmitry Torokhov
>
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
> >> On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> >> > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> >> > >>>> I do not agree. We want the binding to be generic and not tied
> >> > >>>> specifically to the keyreset functionality. As such
> >> > >>>> 'input-keyset' or
> >> > >>>> 'input-keychord' are more appropriate.
> >> > >>>
> >> > >>> The binding is defined specifically for sysrq and specifically to
> >> > >>> perform reset action.
> >> > >>
> >> > >> Yes for now but as the examples in the binding show, it is easy to
> >> > >> envision how other drivers could use it.
> >> > >
> >> > > I think you over-complicate things here. Unlike matrix-keypad
> >> > > binding,
> >> > > where you have a common parsing code, here we have an individual
> >> > > driver.
> >> > > I really do not see anyone else using such sequences or chords as
> >> > > such
> >> > > processing should be done in userspace. Sysrq is quite an exception.
> >> >
> >> > To be honest I don't have a very strong opinion on the binding. I made
> >> > it as generic as possible on the guidance of the DT people. Let's see
> >> > what they think of it.
> >>
> >> Hi Mathieu,
> >>
> >> As per our conversation just now at Connect, the binding should probably
> >> look like this:
> >>
> >> Sysrq keyset binding:
> >>
> >> The /chosen node can contain a linux,input-keyset-sysrq child node to
> >> define a set of keys that will generate a sysrq when pressed together.
> >
> > Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
> > whatever. The sysrq setting is system-wide and applicable to all
> > devices. Given that it is used only on mobile, where there not that
> > many input devices (a few keys and touchscreen) I do not believe we
> > should consider adding per-device settings.
>
> It's in /chosen, that isn't per-device.
>
> >> Required properties:
> >> keyset: array of keycodes
> >
> > Please, let's call it 'key-reset-seq', because it is exactly the reset
> > sequence. There won't be any additional sequences or chords as those
> > should be handled in userspace, sysrq is a special case here.
>
> This is absolutely a linux-specific binding. It encodes the Linux
> keycodes, and generates a linux meaning. I'm usually all about
> carrying the OS-independent banner when defining DT bindings, but in
> this case the linux prefix and sysrq reference is completely
> appropriate.
OK, I have no idea what "/chosen" actually means. What I am trying to say
that there should be either "sysrq" or "linux,sysrq" node and that is what
sysrq driver will be looking for.
The entire node is Linux-specific and therefore there is no point in
marking only one of the properties (the key sequence) Linux-specific while
leaving other ones generic.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Grant Likely @ 2013-07-10 21:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mathieu Poirier, devicetree-discuss, John Stultz, kernel-team,
linux-input@vger.kernel.org
In-Reply-To: <20130710165247.GA22992@core.coreip.homeip.net>
On Wed, Jul 10, 2013 at 5:52 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
>> On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>> > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
>> > >>>> I do not agree. We want the binding to be generic and not tied
>> > >>>> specifically to the keyreset functionality. As such 'input-keyset' or
>> > >>>> 'input-keychord' are more appropriate.
>> > >>>
>> > >>> The binding is defined specifically for sysrq and specifically to
>> > >>> perform reset action.
>> > >>
>> > >> Yes for now but as the examples in the binding show, it is easy to
>> > >> envision how other drivers could use it.
>> > >
>> > > I think you over-complicate things here. Unlike matrix-keypad binding,
>> > > where you have a common parsing code, here we have an individual driver.
>> > > I really do not see anyone else using such sequences or chords as such
>> > > processing should be done in userspace. Sysrq is quite an exception.
>> >
>> > To be honest I don't have a very strong opinion on the binding. I made
>> > it as generic as possible on the guidance of the DT people. Let's see
>> > what they think of it.
>>
>> Hi Mathieu,
>>
>> As per our conversation just now at Connect, the binding should probably
>> look like this:
>>
>> Sysrq keyset binding:
>>
>> The /chosen node can contain a linux,input-keyset-sysrq child node to
>> define a set of keys that will generate a sysrq when pressed together.
>
> Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
> whatever. The sysrq setting is system-wide and applicable to all
> devices. Given that it is used only on mobile, where there not that
> many input devices (a few keys and touchscreen) I do not believe we
> should consider adding per-device settings.
It's in /chosen, that isn't per-device.
>> Required properties:
>> keyset: array of keycodes
>
> Please, let's call it 'key-reset-seq', because it is exactly the reset
> sequence. There won't be any additional sequences or chords as those
> should be handled in userspace, sysrq is a special case here.
This is absolutely a linux-specific binding. It encodes the Linux
keycodes, and generates a linux meaning. I'm usually all about
carrying the OS-independent banner when defining DT bindings, but in
this case the linux prefix and sysrq reference is completely
appropriate.
g.
^ permalink raw reply
* Re: [PATCH 2/2] Input: Adding DT support for keyreset tuneables
From: Mathieu Poirier @ 2013-07-10 21:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Grant Likely, kernel-team-z5hGa2qSFaRBDgjK7y7TUQ,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, John Stultz,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20130710165247.GA22992-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2694 bytes --]
On 10 July 2013 10:52, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jul 10, 2013 at 04:14:57PM +0100, Grant Likely wrote:
> > On Fri, 28 Jun 2013 07:19:06 -0600, Mathieu Poirier <
> mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > > On 13-06-28 12:09 AM, Dmitry Torokhov wrote:
> > > >>>> I do not agree. We want the binding to be generic and not tied
> > > >>>> specifically to the keyreset functionality. As such
> 'input-keyset' or
> > > >>>> 'input-keychord' are more appropriate.
> > > >>>
> > > >>> The binding is defined specifically for sysrq and specifically to
> > > >>> perform reset action.
> > > >>
> > > >> Yes for now but as the examples in the binding show, it is easy to
> > > >> envision how other drivers could use it.
> > > >
> > > > I think you over-complicate things here. Unlike matrix-keypad
> binding,
> > > > where you have a common parsing code, here we have an individual
> driver.
> > > > I really do not see anyone else using such sequences or chords as
> such
> > > > processing should be done in userspace. Sysrq is quite an exception.
> > >
> > > To be honest I don't have a very strong opinion on the binding. I made
> > > it as generic as possible on the guidance of the DT people. Let's see
> > > what they think of it.
> >
> > Hi Mathieu,
> >
> > As per our conversation just now at Connect, the binding should probably
> > look like this:
> >
> > Sysrq keyset binding:
> >
> > The /chosen node can contain a linux,input-keyset-sysrq child node to
> > define a set of keys that will generate a sysrq when pressed together.
>
> Hmm, we would have only one such node, /sysrq, or /linux,sysrq,
> whatever. The sysrq setting is system-wide and applicable to all
> devices. Given that it is used only on mobile, where there not that
> many input devices (a few keys and touchscreen) I do not believe we
> should consider adding per-device settings.
>
Putting the binding in the "chosen" node is definitely a system wide
setting. If I didn't interpret your comment properly, please get back to
me.
>
> >
> > Required properties:
> > keyset: array of keycodes
>
> Please, let's call it 'key-reset-seq', because it is exactly the reset
> sequence. There won't be any additional sequences or chords as those
> should be handled in userspace, sysrq is a special case here.
>
I'm not strongly opinionated on that front but I think it should, at least,
include the some clue about the driver it ties to. What do you think
about: "linux,sysrq-reset-seq" ?
> > timeout-ms: duration keys must be pressed together in microseconds
> > before generating a sysrq
> >
>
> Thanks.
>
> --
> Dmitry
>
[-- Attachment #1.2: Type: text/html, Size: 4123 bytes --]
[-- Attachment #2: Type: text/plain, Size: 192 bytes --]
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
^ permalink raw reply
* Re: [PATCH] usb: USB host support should depend on HAS_DMA
From: Alan Stern @ 2013-07-10 21:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-input, linux-media,
linux-usb, linux-kernel
In-Reply-To: <1373491112-15593-1-git-send-email-geert@linux-m68k.org>
On Wed, 10 Jul 2013, Geert Uytterhoeven wrote:
> If NO_DMA=y:
>
> drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma':
> drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single'
> ,,,
>
> Commit d9ea21a779278da06d0cbe989594bf542ed213d7 ("usb: host: make
> USB_ARCH_HAS_?HCI obsolete") allowed to enable USB on platforms with
> NO_DMA=y, and exposed several input and media USB drivers that just select
> USB if USB_ARCH_HAS_HCD, without checking HAS_DMA.
>
> Fix the former by making USB depend on HAS_DMA.
This isn't right. There are USB host controllers that use PIO, not
DMA. The HAS_DMA dependency should go with the controller driver, not
the USB core.
On the other hand, the USB core does call various routines like
dma_unmap_single. It ought to be possible to compile these calls even
when DMA isn't enabled. That is, they should be defined as do-nothing
stubs.
> To fix the latter, instead of adding lots of "depends on HAS_DMA", make
> those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and
> selecting USB. Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already
> handle this in a similar way.
That seems reasonable.
Alan Stern
^ permalink raw reply
* [PATCH] usb: USB host support should depend on HAS_DMA
From: Geert Uytterhoeven @ 2013-07-10 21:18 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman
Cc: Alan Stern, linux-input, linux-media, linux-usb, linux-kernel,
Geert Uytterhoeven
If NO_DMA=y:
drivers/built-in.o: In function `usb_hcd_unmap_urb_setup_for_dma':
drivers/usb/core/hcd.c:1361: undefined reference to `dma_unmap_single'
drivers/built-in.o: In function `usb_hcd_unmap_urb_for_dma':
drivers/usb/core/hcd.c:1393: undefined reference to `dma_unmap_sg'
drivers/usb/core/hcd.c:1398: undefined reference to `dma_unmap_page'
drivers/usb/core/hcd.c:1403: undefined reference to `dma_unmap_single'
drivers/built-in.o: In function `usb_hcd_map_urb_for_dma':
drivers/usb/core/hcd.c:1445: undefined reference to `dma_map_single'
drivers/usb/core/hcd.c:1450: undefined reference to `dma_mapping_error'
drivers/usb/core/hcd.c:1480: undefined reference to `dma_map_sg'
drivers/usb/core/hcd.c:1495: undefined reference to `dma_map_page'
drivers/usb/core/hcd.c:1501: undefined reference to `dma_mapping_error'
drivers/usb/core/hcd.c:1507: undefined reference to `dma_map_single'
drivers/usb/core/hcd.c:1512: undefined reference to `dma_mapping_error'
drivers/built-in.o: In function `hcd_buffer_free':
drivers/usb/core/buffer.c:146: undefined reference to `dma_pool_free'
drivers/usb/core/buffer.c:150: undefined reference to `dma_free_coherent'
drivers/built-in.o: In function `hcd_buffer_destroy':
drivers/usb/core/buffer.c:90: undefined reference to `dma_pool_destroy'
drivers/built-in.o: In function `hcd_buffer_create':
drivers/usb/core/buffer.c:65: undefined reference to `dma_pool_create'
drivers/built-in.o: In function `hcd_buffer_alloc':
drivers/usb/core/buffer.c:120: undefined reference to `dma_pool_alloc'
drivers/usb/core/buffer.c:122: undefined reference to `dma_alloc_coherent'
,,,
Commit d9ea21a779278da06d0cbe989594bf542ed213d7 ("usb: host: make
USB_ARCH_HAS_?HCI obsolete") allowed to enable USB on platforms with
NO_DMA=y, and exposed several input and media USB drivers that just select
USB if USB_ARCH_HAS_HCD, without checking HAS_DMA.
Fix the former by making USB depend on HAS_DMA.
To fix the latter, instead of adding lots of "depends on HAS_DMA", make
those drivers depend on USB, instead of depending on USB_ARCH_HAS_HCD and
selecting USB. Drivers for other busses (e.g. MOUSE_SYNAPTICS_I2C) already
handle this in a similar way.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
drivers/input/joystick/Kconfig | 3 +--
drivers/input/misc/Kconfig | 15 +++++----------
drivers/input/mouse/Kconfig | 9 +++------
drivers/input/tablet/Kconfig | 15 +++++----------
drivers/input/touchscreen/Kconfig | 3 +--
drivers/media/rc/Kconfig | 21 +++++++--------------
drivers/usb/Kconfig | 2 +-
7 files changed, 23 insertions(+), 45 deletions(-)
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 56eb471..d7e36fb 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -278,8 +278,7 @@ config JOYSTICK_JOYDUMP
config JOYSTICK_XPAD
tristate "X-Box gamepad support"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use the X-Box pad with your computer.
Make sure to say Y to "Joystick support" (CONFIG_INPUT_JOYDEV)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0b541cd..00cdecb 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -286,8 +286,7 @@ config INPUT_ATLAS_BTNS
config INPUT_ATI_REMOTE2
tristate "ATI / Philips USB RF remote control"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use an ATI or Philips USB RF remote control.
These are RF remotes with USB receivers.
@@ -301,8 +300,7 @@ config INPUT_ATI_REMOTE2
config INPUT_KEYSPAN_REMOTE
tristate "Keyspan DMR USB remote control"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use a Keyspan DMR USB remote control.
Currently only the UIA-11 type of receiver has been tested. The tag
@@ -333,8 +331,7 @@ config INPUT_KXTJ9_POLLED_MODE
config INPUT_POWERMATE
tristate "Griffin PowerMate and Contour Jog support"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use Griffin PowerMate or Contour Jog devices.
These are aluminum dials which can measure clockwise and anticlockwise
@@ -349,8 +346,7 @@ config INPUT_POWERMATE
config INPUT_YEALINK
tristate "Yealink usb-p1k voip phone"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to enable keyboard and LCD functions of the
Yealink usb-p1k usb phones. The audio part is enabled by the generic
@@ -364,8 +360,7 @@ config INPUT_YEALINK
config INPUT_CM109
tristate "C-Media CM109 USB I/O Controller"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to enable keyboard and buzzer functions of the
C-Media CM109 usb phones. The audio part is enabled by the generic
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index effa9c5..90f8c0b 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -161,8 +161,7 @@ config MOUSE_SERIAL
config MOUSE_APPLETOUCH
tristate "Apple USB Touchpad support"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use an Apple USB Touchpad.
@@ -182,8 +181,7 @@ config MOUSE_APPLETOUCH
config MOUSE_BCM5974
tristate "Apple USB BCM5974 Multitouch trackpad support"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you have an Apple USB BCM5974 Multitouch
trackpad.
@@ -346,8 +344,7 @@ config MOUSE_SYNAPTICS_I2C
config MOUSE_SYNAPTICS_USB
tristate "Synaptics USB device support"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use a Synaptics USB touchpad or pointing
stick.
diff --git a/drivers/input/tablet/Kconfig b/drivers/input/tablet/Kconfig
index bed7cbf..8e27600 100644
--- a/drivers/input/tablet/Kconfig
+++ b/drivers/input/tablet/Kconfig
@@ -13,8 +13,7 @@ if INPUT_TABLET
config TABLET_USB_ACECAD
tristate "Acecad Flair tablet support (USB)"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use the USB version of the Acecad Flair
tablet. Make sure to say Y to "Mouse support"
@@ -26,8 +25,7 @@ config TABLET_USB_ACECAD
config TABLET_USB_AIPTEK
tristate "Aiptek 6000U/8000U and Genius G_PEN tablet support (USB)"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use the USB version of the Aiptek 6000U,
Aiptek 8000U or Genius G-PEN 560 tablet. Make sure to say Y to
@@ -51,8 +49,7 @@ config TABLET_USB_GTCO
config TABLET_USB_HANWANG
tristate "Hanwang Art Master III tablet support (USB)"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use the USB version of the Hanwang Art
Master III tablet.
@@ -62,8 +59,7 @@ config TABLET_USB_HANWANG
config TABLET_USB_KBTAB
tristate "KB Gear JamStudio tablet support (USB)"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
Say Y here if you want to use the USB version of the KB Gear
JamStudio tablet. Make sure to say Y to "Mouse support"
@@ -75,9 +71,8 @@ config TABLET_USB_KBTAB
config TABLET_USB_WACOM
tristate "Wacom Intuos/Graphire tablet support (USB)"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
select POWER_SUPPLY
- select USB
select NEW_LEDS
select LEDS_CLASS
help
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 3b9758b..a889c52 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -689,8 +689,7 @@ config TOUCHSCREEN_WM97XX_ZYLONITE
config TOUCHSCREEN_USB_COMPOSITE
tristate "USB Touchscreen Driver"
- depends on USB_ARCH_HAS_HCD
- select USB
+ depends on USB
help
USB Touchscreen driver for:
- eGalax Touchkit USB (also includes eTurboTouch CT-410/510/700)
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 5a79c33..ee59842 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -126,9 +126,8 @@ if RC_DEVICES
config RC_ATI_REMOTE
tristate "ATI / X10 based USB RF remote controls"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
help
Say Y here if you want to use an X10 based USB remote control.
These are RF remotes with USB receivers.
@@ -159,9 +158,8 @@ config IR_ENE
config IR_IMON
tristate "SoundGraph iMON Receiver and Display"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
---help---
Say Y here if you want to use a SoundGraph iMON (aka Antec Veris)
IR Receiver and/or LCD/VFD/VGA display.
@@ -171,9 +169,8 @@ config IR_IMON
config IR_MCEUSB
tristate "Windows Media Center Ed. eHome Infrared Transceiver"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
---help---
Say Y here if you want to use a Windows Media Center Edition
eHome Infrared Transceiver.
@@ -221,9 +218,8 @@ config IR_NUVOTON
config IR_REDRAT3
tristate "RedRat3 IR Transceiver"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
---help---
Say Y here if you want to use a RedRat3 Infrared Transceiver.
@@ -232,9 +228,8 @@ config IR_REDRAT3
config IR_STREAMZAP
tristate "Streamzap PC Remote IR Receiver"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
---help---
Say Y here if you want to use a Streamzap PC Remote
Infrared Receiver.
@@ -261,9 +256,8 @@ config IR_WINBOND_CIR
config IR_IGUANA
tristate "IguanaWorks USB IR Transceiver"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
---help---
Say Y here if you want to use the IguanaWorks USB IR Transceiver.
Both infrared receive and send are supported. If you want to
@@ -277,9 +271,8 @@ config IR_IGUANA
config IR_TTUSBIR
tristate "TechnoTrend USB IR Receiver"
- depends on USB_ARCH_HAS_HCD
+ depends on USB
depends on RC_CORE
- select USB
select NEW_LEDS
select LEDS_CLASS
---help---
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index 73f62ca..c530ad9 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -33,7 +33,7 @@ config USB_ARCH_HAS_HCD
# ARM SA1111 chips have a non-PCI based "OHCI-compatible" USB host interface.
config USB
tristate "Support for Host-side USB"
- depends on USB_ARCH_HAS_HCD
+ depends on USB_ARCH_HAS_HCD && HAS_DMA
select NLS # for UTF-8 strings
---help---
Universal Serial Bus (USB) is a specification for a serial bus
--
1.7.9.5
^ permalink raw reply related
* [PATCH 001/001] [Input:] Propagate hardware generated event timestamp to evdev.
From: Alexander Levitskiy @ 2013-07-10 20:38 UTC (permalink / raw)
To: linux-input
Cc: linux-kernel, linux-doc, Rob Landley, Henrik Rydberg,
Dmitry Torokhov, Sasha Levitskiy, Arve Hjønnevåg,
Stephen Boyd
From: Sasha Levitskiy <sanek@google.com>
Input: Propagate hardware event timestamp to evdev.
Convey hardware generated timestamp associated with the current event packet.
The use of these event codes by hardware drivers is optional.
Used to reduce jitter and improve velocity tracking in ABS_MT and other timing
sensitive devices.
kernel v. 3.4
Signed-off-by: Sasha Levitskiy <sanek@google.com>
---
diff --git a/Documentation/input/event-codes.txt
b/Documentation/input/event-codes.txt
index 53305bd..f0f0e07 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -91,6 +91,15 @@ sent in the evdev event stream.
event and query the device (using EVIOCG* ioctls) to obtain its
current state.
+* SYN_TIME_SEC, SYN_TIME_NSEC:
+ - Used to convey hardware timestamp associated with the current
+ event packet. The use of these event codes by hardware drivers
+ is optional. If used, the hardware driver should send the timestamp
+ ahead of any other events associated with this packet. The timestamp
+ should be adjusted to CLOCK_MONOTONIC base.
+ This becomes useful for drivers of hardware that handle batching
+ without involving the main CPU.
+
EV_KEY:
----------
EV_KEY events take the form KEY_<name> or BTN_<name>. For example,
KEY_A is used
diff --git a/Documentation/input/multi-touch-protocol.txt
b/Documentation/input/multi-touch-protocol.txt
index 543101c..71af317 100644
--- a/Documentation/input/multi-touch-protocol.txt
+++ b/Documentation/input/multi-touch-protocol.txt
@@ -80,6 +80,10 @@ Userspace can detect that a driver can report more
total contacts than slots
by noting that the largest supported BTN_TOOL_*TAP event is larger than the
total number of type B slots reported in the absinfo for the ABS_MT_SLOT axis.
+Velocity tracking and temporal precision can be improved if device provides
+exact timestamp for touches reported through SYN_TIME_SEC and SYN_TIME_NSEC.
+The timestamp should be reported ahead of everything else in the packet.
+
Protocol Example A
------------------
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 41f79be..48baf6f 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -37,6 +37,8 @@ struct evdev {
struct mutex mutex;
struct device dev;
bool exist;
+ int hw_ts_sec;
+ int hw_ts_nsec;
};
struct evdev_client {
@@ -109,7 +111,20 @@ static void evdev_event(struct input_handle *handle,
struct input_event event;
ktime_t time_mono, time_real;
- time_mono = ktime_get();
+ if (type == EV_SYN && code == SYN_TIME_SEC) {
+ evdev->hw_ts_sec = value;
+ return;
+ }
+ if (type == EV_SYN && code == SYN_TIME_NSEC) {
+ evdev->hw_ts_nsec = value;
+ return;
+ }
+
+ if (evdev->hw_ts_sec != -1 && evdev->hw_ts_nsec != -1)
+ time_mono = ktime_set(evdev->hw_ts_sec, evdev->hw_ts_nsec);
+ else
+ time_mono = ktime_get();
+
time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
event.type = type;
@@ -128,8 +143,11 @@ static void evdev_event(struct input_handle *handle,
rcu_read_unlock();
- if (type == EV_SYN && code == SYN_REPORT)
+ if (type == EV_SYN && code == SYN_REPORT) {
+ evdev->hw_ts_sec = -1;
+ evdev->hw_ts_nsec = -1;
wake_up_interruptible(&evdev->wait);
+ }
}
static int evdev_fasync(int fd, struct file *file, int on)
@@ -1031,6 +1049,8 @@ static int evdev_connect(struct input_handler
*handler, struct input_dev *dev,
dev_set_name(&evdev->dev, "event%d", minor);
evdev->exist = true;
evdev->minor = minor;
+ evdev->hw_ts_sec = -1;
+ evdev->hw_ts_nsec = -1;
evdev->handle.dev = input_get_device(dev);
evdev->handle.name = dev_name(&evdev->dev);
diff --git a/drivers/input/input.c b/drivers/input/input.c
index aeccc69..378b0d1 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -223,6 +223,8 @@ static void input_handle_event(struct input_dev *dev,
case EV_SYN:
switch (code) {
case SYN_CONFIG:
+ case SYN_TIME_SEC:
+ case SYN_TIME_NSEC:
disposition = INPUT_PASS_TO_ALL;
break;
diff --git a/include/linux/input.h b/include/linux/input.h
index 870e297..3d45b48 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -198,6 +198,8 @@ struct input_keymap_entry {
#define SYN_CONFIG 1
#define SYN_MT_REPORT 2
#define SYN_DROPPED 3
+#define SYN_TIME_SEC 4
+#define SYN_TIME_NSEC 5
/*
* Keys and buttons
^ permalink raw reply related
* Re: [PATCH 22/51] Input: atmel_mxt_ts - Add shutdown function
From: Nick Dyer @ 2013-07-10 18:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Daniel Kurtz, Henrik Rydberg, Joonyoung Shim, Alan Bowens,
linux-input, linux-kernel, Peter Meerwald, Benson Leung,
Olof Johansson
In-Reply-To: <20130710165509.GC22992@core.coreip.homeip.net>
Dmitry Torokhov wrote:
>> The patch disables the interrupt handler on shutdown.
>>
>> One of our customers reported a bug caused by input events being generated
>> during shutdown (for example if the user was touching the device whilst it
>> was turning off), which was solved by putting in this change.
>
> What kind of bug? Could you please be more precise?
I did some more digging. Unfortunately the original problem report (some
time around March 2012) was in a customer ticket system that I no longer
have access to. It could well be trying to paper over a problem in their
app layer, to be honest.
>> However, now you've drawn my attention to it again, it seems to me that
>> probably a better thing for us to be doing would be to power off the
>> touchscreen controller here, and let the interrupt disable be handled by
>> core code - do you agree?
>
> We'd be powering off everything in a moment anyway, no? Or is there a
> concern that the device will stay powered up even if the system is in off
> state?
I agree. I think it's best to not apply this patch for the moment, until we
can prove it's actually required.
Let me know if you want me to generate a new pull request.
^ permalink raw reply
* [PATCH v2] HID: fix data access in implement()
From: Jiri Kosina @ 2013-07-10 17:56 UTC (permalink / raw)
To: Bruno Prémont, Gustavo Padovan, Adam Kropelin,
Grant Grundler, Benjamin Tissoires
Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1307092042190.26857@pobox.suse.cz>
implement() is setting bytes in LE data stream. In case the data is not
aligned to 64bits, it reads past the allocated buffer. It doesn't really
change any value there (it's properly bitmasked), but in case that this
read past the boundary hits a page boundary, pagefault happens when
accessing 64bits of 'x' in implement(), and kernel oopses.
This happens much more often when numbered reports are in use, as the
initial 8bit skip in the buffer makes the whole process work on values
which are not aligned to 64bits.
This problem dates back to attempts in 2005 and 2006 to make implement()
and extract() as generic as possible, and even back then the problem
was realized by Adam Kroperlin, but falsely assumed to be impossible
to cause any harm:
http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
I have made several attempts at fixing it "on the spot" directly in
implement(), but the results were horrible; the special casing for processing
last 64bit chunk and switching to different math makes it unreadable mess.
I therefore took a path to allocate a few bytes more which will never make
it into final report, but are there as a cushion for all the 64bit math
operations happening in implement() and extract().
All callers of hid_output_report() are converted at the same time to allocate
the buffer by newly introduced hid_alloc_report_buf() helper.
Bruno noticed that the whole raw_size test can be dropped as well, as
hid_alloc_report_buf() makes sure that the buffer is always of a proper
size.
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Acked-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
drivers/hid/hid-core.c | 19 ++++++++++++++++++-
drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
drivers/hid/hid-picolcd_debugfs.c | 23 ++++++++++++-----------
drivers/hid/usbhid/hid-core.c | 5 ++---
include/linux/hid.h | 1 +
net/bluetooth/hidp/core.c | 14 +++++++++-----
6 files changed, 52 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 264f550..987279b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1128,7 +1128,8 @@ static void hid_output_field(const struct hid_device *hid,
}
/*
- * Create a report.
+ * Create a report. 'data' has to be allocated using
+ * hid_alloc_report_buf() so that it has proper size.
*/
void hid_output_report(struct hid_report *report, __u8 *data)
@@ -1145,6 +1146,22 @@ void hid_output_report(struct hid_report *report, __u8 *data)
EXPORT_SYMBOL_GPL(hid_output_report);
/*
+ * Allocator for buffer that is going to be passed to hid_output_report()
+ */
+u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
+{
+ /*
+ * 7 extra bytes are necessary to achieve proper functionality
+ * of implement() working on 8 byte chunks
+ */
+
+ int len = ((report->size - 1) >> 3) + 1 + (report->id > 0) + 7;
+
+ return kmalloc(len, flags);
+}
+EXPORT_SYMBOL_GPL(hid_alloc_report_buf);
+
+/*
* Set a field value. The report this field belongs to has to be
* created and transferred to the device, to set this value in the
* device.
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 5207591a..4d79273 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -574,7 +574,7 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
struct hid_field *field;
struct hid_report *report;
- unsigned char data[8];
+ unsigned char *data;
int offset;
dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
@@ -590,6 +590,13 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
return -1;
}
hid_set_field(field, offset, value);
+
+ data = hid_alloc_report_buf(field->report, GFP_KERNEL);
+ if (!data) {
+ dev_warn(&dev->dev, "failed to allocate report buf memory\n");
+ return -1;
+ }
+
hid_output_report(field->report, &data[0]);
output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
@@ -600,8 +607,9 @@ static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
- return 0;
+ kfree(data);
+ return 0;
}
static int logi_dj_ll_start(struct hid_device *hid)
diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c
index 59ab8e1..024cdf3 100644
--- a/drivers/hid/hid-picolcd_debugfs.c
+++ b/drivers/hid/hid-picolcd_debugfs.c
@@ -394,7 +394,7 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data,
void picolcd_debug_out_report(struct picolcd_data *data,
struct hid_device *hdev, struct hid_report *report)
{
- u8 raw_data[70];
+ u8 *raw_data;
int raw_size = (report->size >> 3) + 1;
char *buff;
#define BUFF_SZ 256
@@ -407,20 +407,20 @@ void picolcd_debug_out_report(struct picolcd_data *data,
if (!buff)
return;
- snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
- report->id, raw_size);
- hid_debug_event(hdev, buff);
- if (raw_size + 5 > sizeof(raw_data)) {
+ raw_data = hid_alloc_report_buf(report, GFP_ATOMIC);
+ if (!raw_data) {
kfree(buff);
- hid_debug_event(hdev, " TOO BIG\n");
return;
- } else {
- raw_data[0] = report->id;
- hid_output_report(report, raw_data);
- dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
- hid_debug_event(hdev, buff);
}
+ snprintf(buff, BUFF_SZ, "\nout report %d (size %d) = ",
+ report->id, raw_size);
+ hid_debug_event(hdev, buff);
+ raw_data[0] = report->id;
+ hid_output_report(report, raw_data);
+ dump_buff_as_hex(buff, BUFF_SZ, raw_data, raw_size);
+ hid_debug_event(hdev, buff);
+
switch (report->id) {
case REPORT_LED_STATE:
/* 1 data byte with GPO state */
@@ -644,6 +644,7 @@ void picolcd_debug_out_report(struct picolcd_data *data,
break;
}
wake_up_interruptible(&hdev->debug_wait);
+ kfree(raw_data);
kfree(buff);
}
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 9941828..ada164e 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -535,7 +535,6 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
{
int head;
struct usbhid_device *usbhid = hid->driver_data;
- int len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
if ((hid->quirks & HID_QUIRK_NOGET) && dir == USB_DIR_IN)
return;
@@ -546,7 +545,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
return;
}
- usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
+ usbhid->out[usbhid->outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
if (!usbhid->out[usbhid->outhead].raw_report) {
hid_warn(hid, "output queueing failed\n");
return;
@@ -595,7 +594,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
}
if (dir == USB_DIR_OUT) {
- usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
+ usbhid->ctrl[usbhid->ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
if (!usbhid->ctrl[usbhid->ctrlhead].raw_report) {
hid_warn(hid, "control queueing failed\n");
return;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 0c48991..acccdf4 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -744,6 +744,7 @@ struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
void hid_output_report(struct hid_report *report, __u8 *data);
+u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags);
struct hid_device *hid_allocate_device(void);
struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);
int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 940f5ac..41f154d 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -231,17 +231,21 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
{
- unsigned char buf[32], hdr;
- int rsize;
+ unsigned char hdr;
+ u8 *buf;
+ int rsize, ret;
- rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
- if (rsize > sizeof(buf))
+ buf = hid_alloc_report_buf(report, GFP_ATOMIC);
+ if (!buf)
return -EIO;
hid_output_report(report, buf);
hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
- return hidp_send_intr_message(session, hdr, buf, rsize);
+ ret = hidp_send_intr_message(session, hdr, buf, rsize);
+
+ kfree(buf);
+ return ret;
}
static int hidp_get_raw_report(struct hid_device *hid,
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH] HID: fix data access in implement()
From: Jiri Kosina @ 2013-07-10 17:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Bruno Prémont, Gustavo Padovan, Adam Kropelin,
Grant Grundler, linux-input
In-Reply-To: <51DD6BB8.9050206@gmail.com>
On Wed, 10 Jul 2013, Benjamin Tissoires wrote:
> > implement() is setting bytes in LE data stream. In case the data
> > is not aligned to 64bits, it reads past the allocated buffer. It
> > doesn't really change any value there (it's properly bitmasked), but
> > in case that this read past the boundary hits a page boundary, pagefault
> > happens when accessing 64bits of 'x' in implement(), and kernel oopses.
> >
> > This happens much more often when numbered reports are in use, as the
> > initial 8bit skip in the buffer makes the whole process work on values
> > which are not aligned to 64bits.
> >
> > This problem dates back to attempts in 2005 and 2006 to make implement()
> > and extract() as generic as possible, and even back then the problem
> > was realized by Adam Kroperlin, but falsely assumed to be impossible
> > to cause any harm:
> >
> > http://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg47690.html
> >
> > I have made several attempts at fixing it "on the spot" directly in
> > implement(), but the results were horrible; the special casing for processing
> > last 64bit chunk and switching to different math makes it unreadable mess.
> >
> > I therefore took a path to allocate a few bytes more which will never make
> > it into final report, but are there as a cushion for all the 64bit math
> > operations happening in implement() and extract().
> >
> > All callers of hid_output_report() are converted at the same time to allocate
> > the buffer by newly introduced hid_alloc_report_buf() helper.
> >
> > Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> > ---
>
> Thanks for that. It should (I hope) fix the bugs we are seeing from time
> to times under Fedora and that I was not able to answer:
> https://bugzilla.redhat.com/show_bug.cgi?id=965280
> https://bugzilla.redhat.com/show_bug.cgi?id=927488
> https://bugzilla.redhat.com/show_bug.cgi?id=881504
965280 and 881504 absolutely look like an incarnation of this very
problem. The faulting address is pagesize aligned (i.e. the loop just
crossed the boundary), and the faulting code is the
put_unaligned_access_le64().
Exactly the same as the report I had.
927488 seems like a different bug to me -- not an address on page
boundary, and we're not faulting in implement().
> I have a small remark for usbhid:
>
> > drivers/hid/hid-core.c | 19 ++++++++++++++++++-
> > drivers/hid/hid-logitech-dj.c | 12 ++++++++++--
> > drivers/hid/hid-picolcd_debugfs.c | 10 +++++++++-
> > drivers/hid/usbhid/hid-core.c | 4 ++--
> > include/linux/hid.h | 1 +
> > net/bluetooth/hidp/core.c | 14 +++++++++-----
> > 6 files changed, 49 insertions(+), 11 deletions(-)
>
> [snipped]
>
> > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > index 9941828..2b0b96daf 100644
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -546,7 +546,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> > return;
> > }
> >
> > - usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
> > + usbhid->out[usbhid->outhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
> > if (!usbhid->out[usbhid->outhead].raw_report) {
> > hid_warn(hid, "output queueing failed\n");
> > return;
> > @@ -595,7 +595,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
> > }
> >
> > if (dir == USB_DIR_OUT) {
> > - usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
> > + usbhid->ctrl[usbhid->ctrlhead].raw_report = hid_alloc_report_buf(report, GFP_ATOMIC);
>
> line 538: int len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
> is not used anywhere after applying the patch. It can be dropped.
Good catch.
> Besides the picolcd problems spotted by Bruno:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Thanks.
--
Jiri Kosina
SUSE Labs
^ 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