commit 5b44c53aeb791757072be4a267255cedfff594fd upstream.
When a hid driver that uses i2c-hid as transport is unloaded, the hid core
will call i2c_hid_stop() which releases all the buffers associated with the
device. This includes also the command buffer.
Now, when the i2c-hid driver itself is unloaded it tries to power down the
device by sending it PWR_SLEEP command. Since the command buffer is already
released we get following crash:
[ 79.691459] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 79.691532] IP: [<ffffffffa05bc049>] __i2c_hid_command+0x49/0x310 [i2c_hid]
...
[ 79.693467] Call Trace:
[ 79.693494] [<ffffffff810424e1>] ? __unmask_ioapic+0x21/0x30
[ 79.693537] [<ffffffff81042855>] ? unmask_ioapic+0x25/0x40
[ 79.693581] [<ffffffffa05bc35b>] ? i2c_hid_set_power+0x4b/0xa0 [i2c_hid]
[ 79.693632] [<ffffffffa05bc3cf>] ? i2c_hid_runtime_resume+0x1f/0x30 [i2c_hid]
[ 79.693689] [<ffffffff814c08fb>] ? __rpm_callback+0x2b/0x70
[ 79.693733] [<ffffffff814c0961>] ? rpm_callback+0x21/0x90
[ 79.693776] [<ffffffff814c0dec>] ? rpm_resume+0x41c/0x600
[ 79.693820] [<ffffffff814c1e1c>] ? __pm_runtime_resume+0x4c/0x80
[ 79.693868] [<ffffffff814b8588>] ? __device_release_driver+0x28/0x100
[ 79.693917] [<ffffffff814b8d90>] ? driver_detach+0xa0/0xb0
[ 79.693959] [<ffffffff814b82cc>] ? bus_remove_driver+0x4c/0xb0
[ 79.694006] [<ffffffff810d1cfd>] ? SyS_delete_module+0x11d/0x1d0
[ 79.694054] [<ffffffff8165f107>] ? int_signal+0x12/0x17
[ 79.694095] [<ffffffff8165ee69>] ? system_call_fastpath+0x12/0x17
Fix this so that we only free buffers when the i2c-hid driver itself is
removed.
Fixes: 34f439e4afcd ("HID: i2c-hid: add runtime PM support")
Reported-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
This patch adds runtime PM support for the HID over I2C driver. When the
i2c-hid device is first opened we power it on and on the last close we
power it off. This is actually what the driver is already doing but in
addition it allows subsystems, like ACPI power domain to power off the
device during runtime PM suspend, which should save even more power.
The implementation is not the most power efficient because it needs some
interaction from the userspace (e.g close the device node whenever we are
no more interested in getting events), nevertheless it allows us to save
some power and works with devices that are not wake capable.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
commit 8320caeeffdefec3b58b9d4a7ed8e1079492fe7b upstream.
The buffer allocation is not currently accounting for an extra byte for
the report id. This can cause an out of bounds access in function
i2c_hid_set_or_send_report() with reportID > 15.
Signed-off-by: Adrian Salido <salidoa@google.com>
Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
commit 6d00f37e49d95e640a3937a4a1ae07dbe92a10cb upstream.
d1c7e29e8d27 (HID: i2c-hid: prevent buffer overflow in early IRQ)
changed hid_get_input() to read ihid->bufsize bytes, which can be
more than wMaxInputLength. This is the case with the Dell XPS 13
9343, and it is causing events to be missed. In some cases the
missed events are releases, which can cause the cursor to jump or
freeze, among other problems. Limit the number of bytes read to
min(wMaxInputLength, ihid->bufsize) to prevent such problems.
Fixes: d1c7e29e8d27 "HID: i2c-hid: prevent buffer overflow in early IRQ"
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit d1c7e29e8d276c669e8790bb8be9f505ddc48888 upstream.
Before ->start() is called, bufsize size is set to HID_MIN_BUFFER_SIZE,
64 bytes. While processing the IRQ, we were asking to receive up to
wMaxInputLength bytes, which can be bigger than 64 bytes.
Later, when ->start is run, a proper bufsize will be calculated.
Given wMaxInputLength is said to be unreliable in other part of the
code, set to receive only what we can even if it results in truncated
reports.
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 6296f4a8eb86f9abcc370fb7a1a116b8441c17fd upstream.
Current driver uses a common buffer for reading reports either
synchronously in i2c_hid_get_raw_report() and asynchronously in
the interrupt handler.
There is race condition if an interrupt arrives immediately after
the report is received in i2c_hid_get_raw_report(); the common
buffer is modified by the interrupt handler with the new report
and then i2c_hid_get_raw_report() proceed using wrong data.
Fix it by using a separate buffers for synchronous reports.
Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
[Antonio Borneo: cleanup, rebase to v3.17, submit mainline]
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
With the current i2c hid driver set/get report does not work
as expected, for e.g sensor hub properties like power state,
frequency etc is not set properly on the device as a result
we do not get events.
The problem is that i2c hid driver in function i2c_hid_request
sets length equal to default buffer size for which the sensor
hub does not respond on get/set commands. Use report length
and calculate it based on report size and id.
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Huzefa Kankroliwala <huzefa.nomanx.kankroliwala@intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This allows HID drivers to also get/set reports through hid_hw_request().
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
PullHID fixes from Jiri Kosina:
- fix i2c-hid and hidraw interaction, by Benjamin Tissoires
- a quirk to make a particular device (Formosa IR receiver) work
properly, by Nicholas Santos
* 'for-3.8/upstream-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid:
HID: i2c-hid: fix i2c_hid_output_raw_report
HID: usbhid: quirk for Formosa IR receiver
HID: remove x bit from sensor doc
i2c_hid_output_raw_report is used by hidraw to forward set_report requests.
The current implementation of i2c_hid_set_report needs to take the
report_id as an argument. The report_id is stored in the first byte
of the buffer in argument of i2c_hid_output_raw_report.
Not removing the report_id from the given buffer adds this byte 2 times
in the command, leading to a non working command.
Reported-by: Andrew Duggan <aduggan@synaptics.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The HID over I2C protocol specification states that when the device is
enumerated from ACPI the HID descriptor address can be obtained by
executing "_DSM" for the device with function 1. Enable this.
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
CONFIG_HOTPLUG is going away as an option. As a result, the __dev*
markings need to be removed.
This change removes the use of __devinit, __devexit_p, __devinitdata,
__devinitconst, and __devexit from these drivers.
Based on patches originally written by Bill Pemberton, but redone by me
in order to handle some of the coding style issues better, by hand.
Cc: Bill Pemberton <wfp5p@virginia.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We should not enter close function while someone else is in open.
This mutex prevents this race.
There is also no need to override the ret value with -EIO in case of
a failure of i2c_hid_set_power.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
ret_count has to be at least 3, as we have to count the 2 bytes that are used
for the size of the reply. Without this, memcpy() might be called with zero or
negative count.
Reported-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The previous memcpy implementation relied on the size advertized by the
device. There were no guarantees that buf was big enough.
Some gymnastic is also required with the +2/-2 to take into account
the first 2 bytes of the returned buffer where the total returned
length is supplied by the device.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
There is no point in keeping the irq in i2c_hid as it's already
there in client.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Simplifies i2c_hid_alloc_buffers tests, and makes this function
responsible of the assignment of ihid->bufsize.
The condition for the reallocation in i2c_hid_start is then simpler.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
HID descriptors contains 4 bytes of reserved field.
The previous implementation was overriding the next fields in struct i2c_hid.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The pointless WARN_ON() has been removed from i2c_hid_remove(),
so we don't need bug.h any more.
Reported-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
ihid can not be null, so there are no reasons to test it.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
If i2c_hid_get_report fails, exit i2c_hid_init_report.
The printk log is already called by i2c_hid_get_report, so no need
to add some more printks.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
In the case where the hid driver in charge of handling the hid part
of the device (hid-generic for instance) fails at probe, neither
i2c_hid_start nor i2c_hid_stop are called.
Thus, the buffers allocated in i2c_hid_probe are never freed.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Forwards appropriate return values.
As noone use the error returned by i2c_hid_get_input, let's make it
returning void.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
These definitions are not used here, but are defined by the specification.
Keeping some of them for documentation purposes.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This avoids the problematic case:
if (condition)
i2c_hid_dbg(ihid, "Blah blah %d\n", i);
else
do_something_very_important();
Which looks correct, however with the previous macro definition,
this expands to the unexpected:
if (condition) {
if (debug) \
dev_printk(KERN_DEBUG, &ihid->client->dev,
"Blah blah %d\n", i);
else
do_something_very_important();
}
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
We should not initialize to 0 static declarations.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
The "comment" part can never be displayed, so we can remove it.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
no I2C driver has "i2c" in its name. It makes more sense to call this
i2c driver "hid".
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Reviewed-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
In case we are returning from i2c_hid_probe() through the 'err' or
'err_mem_free' labels, there is noone freeing the buffers allocated by
i2c_hid_alloc_buffers().
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Microsoft published the protocol specification of HID over i2c:
http://msdn.microsoft.com/en-us/library/windows/hardware/hh852380.aspx
This patch introduces an implementation of this protocol.
This implementation does not includes the ACPI part of the specification.
This will come when ACPI 5.0 devices enumeration will be available.
Once the ACPI part is done, OEM will not have to declare HID over I2C
devices in their platform specific driver.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Mika Westerberg
Adrian Salido
Seth Forshee
Gwendal Grignou
Jean-Baptiste Maneyrol
Huzefa Kankroliwala
Benjamin Tissoires
Jiri Kosina
Linus Torvalds
Greg Kroah-Hartman