Re: [PATCH] usbhid: discarded events don't abort idleness
On Fri, 2015-11-20 at 11:00 +0100, Jiri Kosina wrote: > On Thu, 5 Nov 2015, Oliver Neukum wrote: > > > If an event is discarded the device stays idle. > > Just reverse the order of check and marking busy. > > > > Signed-off-by: Oliver Neukum <oneu...@suse.com> > > Hi Oliver, > > thanks for the fix. This is a real bug, so I am wondering whether you have > seen causing it real problems (and hence it should be merged for 4.4), or > if you found it just by code inspection (and therefore it should be fine > to merge it for 4.5). Hi, I was looking for another bug. So code inspection. HTH Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: quirks: Fix another ELAN touchscreen
On Mon, 2015-11-23 at 18:37 +0100, Adrien Vergé wrote: > > Makes one wonder however whether we shouldn't be applying > ALWAYS_POLL to > > all ELAN devices by default anyway. > > True! But I don't want to risk breaking anything on other models in > this patch. ALWAYS_POLL just extends an existing behavior. The chances of breaking anything are slim. I'd go for the approach based on the vendor ID. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbhid: discarded events don't abort idleness
If an event is discarded the device stays idle.
Just reverse the order of check and marking busy.
Signed-off-by: Oliver Neukum <oneu...@suse.com>
---
drivers/hid/usbhid/hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 36712e9..19a4364 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -274,10 +274,10 @@ static void hid_irq_in(struct urb *urb)
switch (urb->status) {
case 0: /* success */
- usbhid_mark_busy(usbhid);
usbhid->retry_delay = 0;
if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) && !hid->open)
break;
+ usbhid_mark_busy(usbhid);
if (!test_bit(HID_RESUME_RUNNING, >iofl)) {
hid_input_report(urb->context, HID_INPUT_REPORT,
urb->transfer_buffer,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-22 at 11:22 -0400, Alan Stern wrote: > On Tue, 22 Sep 2015, Oliver Neukum wrote: > > > Cancel, yes, going to low power is a consequence which needn't bother > > the power subsystem. > > Going to low power needn't involve the power subsystem? That sounds > weird. Think of it like rfkill. It makes sense to suspend an rfkilled device. It still is the job of the driver to report that its device is idle. > > You need a callback. If there are spurious > > events, the current heuristics will keep devices awake. > > You must discard them anyway, as they are spurious. There's no point > > in transporting over the bus at all. We can cease IO for input. > > > > > This would create a parallel runtime-PM mechanism which is independent > > > of the existing one. Is that really a good idea? > > > > It isn't strictly PM. It helps PM to do a better job, but > > conceptually it is independent. > > So my next question is: _How_ can this help PM to do a better job? > That is, what are the mechanisms? "inhibit" -> driver stops input -> driver sets PM count to zero -> PM subsystem acts To go from the first to the second step a callback is needed > One you have already stated: Lack of spurious events will help prevent > unwanted wakeups (or unwanted failures to go to sleep). That too. We also save CPU cycles. > But Dmitry made a stronger claim: Inhibiting an input device should > allow the device to go to low power. I would like to know how we can > implement this cleanly. The most straightforward approach is to use > runtime PM, but it's not obvious how this can be made to work with the > current API. Yes, we can use the current API. The key is that you think of the mechanism as induced idleness, not forced suspend. We already have a perfectly working mechanism for suspending idle devices. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-22 at 10:15 -0400, Alan Stern wrote: > On Tue, 22 Sep 2015, Oliver Neukum wrote: > > > Indeed. We can handle output to suspended devices by waking them. > > I don't see why this case is different. We are talking about input > > only. > > > > > The runtime-PM "usage" value for these devices is a little tricky to > > > calculate. It should be nonzero if there are any open files _and_ the > > > device isn't "inhibited". I don't know the best way to represent that > > > kind of condition in the runtime PM framework. > > > > Does that make sense in the generic framework at all? I still > > think that drivers should cease IO for input in such cases. > > That should involve a common callback, but no counter. > > I'm not sure I understand what you're saying. Are you suggesting that > this "inhibit" mechanism should involve a new callback different from Yes, there is no necessary relation to power management. If you put your phone into your pocket, you will want to inhibit the touchscreen even if that doesn't save power. > the existing runtime-PM callbacks? And when this new callback is > invoked, drivers should cancel existing input requests (these devices > are input-only) and go to low power? Cancel, yes, going to low power is a consequence which needn't bother the power subsystem. You need a callback. If there are spurious events, the current heuristics will keep devices awake. You must discard them anyway, as they are spurious. There's no point in transporting over the bus at all. We can cease IO for input. > This would create a parallel runtime-PM mechanism which is independent > of the existing one. Is that really a good idea? It isn't strictly PM. It helps PM to do a better job, but conceptually it is independent. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Mon, 2015-09-21 at 16:02 -0400, Alan Stern wrote: > On Mon, 21 Sep 2015, Dmitry Torokhov wrote: > > > > What happens if the "inhibit" control is turned on and the driver puts > > > the device into runtime suspend, but then an I/O request arrives? > > > > > > If the I/O request originated from userspace, it means the > > > user is violating the terms of the "inhibit" control. Should > > > the request simply fail? > > > > What user? User that inhibited it or user that tried to use the device? > > Normally they would be the same. But even if they aren't, someone has > violated the kernel interface: The first user told the kernel a > particular device wasn't going to be used, and then the second user > tried to use it. If we assume that user space speaks with a uniform voice on that issue, it can just as well close the device. It seems to me that declaring a device idle is a privileged operation. > Of course, this issue doesn't arise for devices that merely report > external events. Indeed. We can handle output to suspended devices by waking them. I don't see why this case is different. We are talking about input only. > The runtime-PM "usage" value for these devices is a little tricky to > calculate. It should be nonzero if there are any open files _and_ the > device isn't "inhibited". I don't know the best way to represent that > kind of condition in the runtime PM framework. Does that make sense in the generic framework at all? I still think that drivers should cease IO for input in such cases. That should involve a common callback, but no counter. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 2015-09-09 at 22:25 +0200, Rafael J. Wysocki wrote: > > > > > > I'd doubt that. Suppose you put the phone into your pocket while > > > the device isn't suspended. The continuous stream of spurious > events > > > will keep it awake. > > Why would they be regarded as spurious then? They are just regular > touch panel > events in that case, aren't they? These events are not expected to be caused by the user's hand. But it raises a design question; whose job it is to handle such information? It makes no sense to gather events from a touchscreen if you suspect the phone is randomly rubbing at things or to take video from a camera if you know that the lid is closed covering the lens. I think we can agree to that. The thing is that we handle all other availability in kernel space. You can argue that user space has an agreed interface (evdev, V4L or whatever) and it is the kernel's job to react if it learns that a device becomes temporarily unavailable and this is merely a question of adding an interface to the kernel by which user space can feed such information to the kernel. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-08 at 10:44 -0400, Alan Stern wrote: > It would not put the device into runtime suspend immediately, like you > are proposing. Instead it would mean the same as the "auto" mode, > except that remote wakeup should be disabled during runtime suspend. Hi, this proposal is incomplete. If you don't want remote wakeup you imply that input is no longer needed or possible. If that is already known, we can just as well inform the driver, so that it can cease IO for input. Yet that is not necessarily the only scenario. For example if you run a screensaver, you might not care for where the user touches the screen, but the event as such is valuable. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Wed, 2015-09-09 at 14:22 +0200, Rafael J. Wysocki wrote: > > Note that when the screen is turned-on again, we want to resume the > > touchscreen so that it can send events again. Why is it impractical to close the fd for the touchscreen? > > In fact, then, what you need seems to be the feature discussed by Alan > and me some time ago allowing remote wakeup do be disabled for runtime > PM from user space as that in combination with autosuspend should > address your use case. I'd doubt that. Suppose you put the phone into your pocket while the device isn't suspended. The continuous stream of spurious events will keep it awake. The ability to disable remote wakeup is necessary but not sufficient. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] PM / Runtime: runtime: Add sysfs option for forcing runtime suspend
On Tue, 2015-09-08 at 01:10 +, Tirdea, Irina wrote: > However, in the scenario I mentioned this is exactly what is happening. > When turning off the screen of a mobile device, the user space Would you explain why user space doesn't simply stop using those devices, which in turn will make them idle? There are obvious cases like keyboards or SCSI hosts where the kernel controls stuff, but could you state where you expect this to be most useful? Or why devices cannot be closed, e.g. you need to be sure settings be not lost or is it something else? > is the one that suspends the devices that are not needed in order to save > power > (like touchscreens). Each such driver export an enable/disable attribute > that calls the same code as resume/suspend (e.g. touchscreen drivers ads7846, > ad7877 and most Android drivers not merged upstream). This adds more > complexity to every driver by adding one more logical power state. > It would be good to have a common interface instead of doing this in > every Now these are two distinct questions. 1. a common interface 2. a capability implemented in common code It is important to keep that apart. I suppose if we want this at all #1 is a given. #2 however may be impossible in a generic manner > I might have not used "forced" in the proper way here. What I mean by it is > that > the device can be runtime suspended while ignoring the runtime usage count. That is highly problematic. You'd need to audit the locking in every driver. Right now elevating the count means that suspend()/resume() cannot race with user space, as in the case of the system suspending user space is frozen. > In this implementation, user space is only allowed to change the states > bottom-up in the sysfs hierarchy (it cannot force suspend a device if it > has children that have not been suspended by user space). That is obviously not enough. Take the worst case: we are flashing some firmware. Or far more harmless: a key is has been pressed on a keyboard > Would it work if this would be a capability that individual drivers need > to declare? For some drivers. But it needs support in the driver. Right now we can make a device idle by calling close(). In fact we can benefit for example in mice from this. But it needs support in the drivers. > In the previous discussion thread , there were a couple of options > mentioned, but none seemed to reach a consensus. You mentioned > adding a "more aggressive runtime PM mode" [1]. I'm not sure how That would have to be done on a per driver base. > this would work except for adding a sysfs attribute that would trigger > a runtime suspend while ignoring usage count. Would that be a > better direction? No. If we want this at all, we need a new callback to notify drivers that user space is temporarily uninterested in a device. And the reverse of course. The power model is good. We must not assume that devices can be suspended at will. If we do this at all, we ought to see it as giving strong hints to drivers when a device can be considered idle. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC ebeam PATCH 2/2] input: misc: New USB eBeam input driver
On Mon, 2015-07-20 at 23:03 +0200, Yann Cantin wrote: Signed-off-by: Yann Cantin yann.can...@laposte.net --- Documentation/ABI/testing/sysfs-driver-ebeam | 53 ++ drivers/input/misc/Kconfig | 22 + drivers/input/misc/Makefile | 1 + drivers/input/misc/ebeam.c | 777 +++ 4 files changed, 853 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-driver-ebeam create mode 100644 drivers/input/misc/ebeam.c diff --git a/Documentation/ABI/testing/sysfs-driver-ebeam b/Documentation/ABI/testing/sysfs-driver-ebeam new file mode 100644 index 000..6873db5 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-ebeam @@ -0,0 +1,53 @@ +What:/sys/class/input/inputXX/device/min_x + /sys/class/input/inputXX/device/min_y + /sys/class/input/inputXX/device/max_x + /sys/class/input/inputXX/device/max_y +Date:Jul 2015 +Kernel Version: 4.1 +Contact: yann.can...@laposte.net + linux-...@vger.kernel.org +Description: + Reading from these files return the actually used range values of + the reported coordinates. + Writing to these files preset these range values. + See below for the calibration procedure. + +What:/sys/class/input/inputXX/device/h[1..9] +Date:Jul 2015 +Kernel Version: 4.1 +Contact: yann.can...@laposte.net + linux-...@vger.kernel.org +Description: + Reading from these files return the 3x3 transformation matrix elements + actually used, in row-major. + Writing to these files preset these elements values. + See below for the calibration procedure. + +What:/sys/class/input/inputXX/device/calibrated +Date:Jul 2015 +Kernel Version: 4.1 +Contact: yann.can...@laposte.net + linux-...@vger.kernel.org +Description: + Reading from this file : + - Return 0 if the driver is in un-calibrated mode : it actually send + raw coordinates in the device's internal coordinates system. + - Return 1 if the driver is in calibrated mode : it send computed coordinates + that (hopefully) matches the screen's coordinates system. + Writing 1 to this file enable the calibrated mode, 0 reset the driver in + un-calibrated mode. + +Calibration procedure : + +When loaded, the driver is in un-calibrated mode : it send device's raw coordinates +in the [0..65535]x[0..65535] range, the transformation matrix is the identity. + +A calibration program have to compute a homography transformation matrix that convert +the device's raw coordinates to the matching screen's ones. +It then write to the appropriate sysfs files the computed values, pre-setting the +driver's parameters : xy range, and the matrix's elements. +When all values are passed, it write 1 to the calibrated sysfs file to enable the calibrated mode. + +Warning : The parameters aren't used until 1 is writen to the calibrated sysfs file. + +Writing 0 to the calibrated sysfs file reset the driver in un-calibrated mode. diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index d4f0a81..22c46a4 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -103,6 +103,28 @@ config INPUT_E3X0_BUTTON To compile this driver as a module, choose M here: the module will be called e3x0_button. +config INPUT_EBEAM_USB + tristate USB eBeam driver + depends on USB_ARCH_HAS_HCD + select USB + help + Say Y here if you have a USB eBeam pointing device and want to + use it without any proprietary user space tools. + + Have a look at http://ebeam.tuxfamily.org/ for + a usage description and the required user-space tools. + + Supported devices : + - Luidia eBeam Classic Projection and eBeam Edge Projection + - Nec NP01Wi1 NP01Wi2 interactive solution + + Supposed working devices, need test, may lack functionality : + - Luidia eBeam Edge Whiteboard and eBeam Engage + - Hitachi Starboard FX-63, FX-77, FX-82, FX-77GII + + To compile this driver as a module, choose M here: the + module will be called ebeam. + config INPUT_PCSPKR tristate PC Speaker support depends on PCSPKR_PLATFORM diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile index 53df07d..125f8a9 100644 --- a/drivers/input/misc/Makefile +++ b/drivers/input/misc/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_INPUT_DA9055_ONKEY)+= da9055_onkey.o obj-$(CONFIG_INPUT_DA9063_ONKEY) += da9063_onkey.o obj-$(CONFIG_INPUT_DM355EVM) += dm355evm_keys.o
Re: [PATCH][RFC] usbhid: enable autosuspend for internal devices
On Mon, 2015-06-29 at 13:16 +0200, Jiri Kosina wrote: On Mon, 29 Jun 2015, Oliver Neukum wrote: Last time we were testing this, autosuspend for USB HID devices was quite a disaster. Do you have any idea whether udev developers tested the autosuspend on by default for USB HID devices on reasonable set of devices? The culrpits that I remember from top of my head (it's been long time ago): - the LEDs for suspended device go off. This is very confusing at least on keyboards, and brings really bad user experience That is a bug. hidinput_count_leds() is supposed to prevent that. This is a HW property and nothing kernel can do about. I am not saying it doesn't bring the LEDs up to a proper state again once auto-resumed. But I hate the LEDs going off a few seconds after I stop typing (i.e. once the keyboard gets auto-suspended). That is the point. Unless you give the option to override, they shouldn't autosuspend. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC] usbhid: enable autosuspend for internal devices
On Sat, 2015-06-27 at 08:29 +0200, Jiri Kosina wrote: On Fri, 26 Jun 2015, Greg Kroah-Hartman wrote: Last time we were testing this, autosuspend for USB HID devices was quite a disaster. Do you have any idea whether udev developers tested the autosuspend on by default for USB HID devices on reasonable set of devices? The culrpits that I remember from top of my head (it's been long time ago): - the LEDs for suspended device go off. This is very confusing at least on keyboards, and brings really bad user experience That is a bug. hidinput_count_leds() is supposed to prevent that. What did you test? - many keyboards were losing first keystroke when waking up from suspend. We've been debugging this with Alan, and never root-caused it to a problem in our code, it seems to be the property of the HW - mice don't wake up from movement alone. And again I would state that we don't get enough information from user space. Hardware seems to be designed for sleeping while a screen saver is on. In kernel space we just get a binary input desired or not desired from open/close. That is insufficient. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: usbhid: Add HID_QUIRK_NOGET for Aten DVI KVM switch
On Wed, 2015-05-13 at 13:25 +0200, Jiri Kosina wrote: On Tue, 12 May 2015, Laura Abbott wrote: Like other KVM switches, the Aten DVI KVM switch needs a quirk to avoid spewing errors: [791759.606542] usb 1-5.4: input irq status -75 received [791759.614537] usb 1-5.4: input irq status -75 received [791759.622542] usb 1-5.4: input irq status -75 received Add it. Signed-off-by: Laura Abbott labb...@fedoraproject.org Applied for for-4.1/upstream-fixes, thanks. Hi, this should go into stable, too. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbhid: correct PM failure with closed devices
When usbhid closes a device which was awake only because remote
wakeup was required but not provided, the interface must go through
a get/put cycle or the core will never reattempt to suspend the device.
This brakes runtime PM for all joysticks.
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/hid/usbhid/hid-core.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index bfbe1be..63d1f0f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -734,7 +734,15 @@ void usbhid_close(struct hid_device *hid)
hid_cancel_delayed_stuff(usbhid);
if (!(hid-quirks HID_QUIRK_ALWAYS_POLL)) {
usb_kill_urb(usbhid-urbin);
+ /*
+* We need a put on the interface to force
+* a recheck needed in case only a lack
+* of capability to do remote wakeup
+* kept us awake
+*/
+ usb_autopm_get_interface(usbhid-intf);
usbhid-intf-needs_remote_wakeup = 0;
+ usb_autopm_put_interface(usbhid-intf);
}
} else {
spin_unlock_irq(usbhid-lock);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
need to get the interface cycled with autopm_get/_put() to let a device sleep
Hi, I found a problem with the close() function of usbhid. It fails to properly suspend device that don't support remote wake-up. The power core does not retry an autosuspend that failed due to a lack of support for remote wake-up. So there needs to be a _put() if the need for remote wake-up is rescinded. The current state of affairs leads to joysticks never going to sleep again after use. But I am not happy with the simple solution. It needlessly wakes up suspended devices. So, I tried using usb_autopm_get_interface_no_resume(). To my considerable surprise that didn't work. I cannot understand why. So what is to be done? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ff-core: use new debug macros
Replace old pr_* with dev_* debugging macros
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/input/ff-core.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
index f50f6dd..b81c88c 100644
--- a/drivers/input/ff-core.c
+++ b/drivers/input/ff-core.c
@@ -23,8 +23,6 @@
/* #define DEBUG */
-#define pr_fmt(fmt) KBUILD_BASENAME : fmt
-
#include linux/input.h
#include linux/module.h
#include linux/mutex.h
@@ -116,7 +114,7 @@ int input_ff_upload(struct input_dev *dev, struct ff_effect
*effect,
if (effect-type FF_EFFECT_MIN || effect-type FF_EFFECT_MAX ||
!test_bit(effect-type, dev-ffbit)) {
- pr_debug(invalid or not supported effect type in upload\n);
+ dev_dbg(dev-dev, invalid or not supported effect type in
upload\n);
return -EINVAL;
}
@@ -124,7 +122,7 @@ int input_ff_upload(struct input_dev *dev, struct ff_effect
*effect,
(effect-u.periodic.waveform FF_WAVEFORM_MIN ||
effect-u.periodic.waveform FF_WAVEFORM_MAX ||
!test_bit(effect-u.periodic.waveform, dev-ffbit))) {
- pr_debug(invalid or not supported wave form in upload\n);
+ dev_dbg(dev-dev, invalid or not supported wave form in
upload\n);
return -EINVAL;
}
@@ -246,7 +244,7 @@ static int flush_effects(struct input_dev *dev, struct file
*file)
struct ff_device *ff = dev-ff;
int i;
- pr_debug(flushing now\n);
+ dev_dbg(dev-dev, flushing now\n);
mutex_lock(ff-mutex);
@@ -316,7 +314,7 @@ int input_ff_create(struct input_dev *dev, unsigned int
max_effects)
int i;
if (!max_effects) {
- pr_err(cannot allocate device without any effects\n);
+ dev_err(dev-dev, cannot allocate device without any
effects\n);
return -EINVAL;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usbhid: more mice with ALWAYS_POLL
On Thu, 2015-04-02 at 14:26 +0200, Jiri Kosina wrote: On Wed, 1 Apr 2015, Oliver Neukum wrote: I am postponing all these before it is clarified that this is indeed a case reporter is able to reproduce on different system as well to rule out the possibility of hub being the actual root cause. The test has been redone on a different system with a mouse reported to be affected and the problem does show up. I have to conclude that the we are really a victim of bias here. We thought the issue was rare because we never looked for it. Ok, this is sad. I am now applying these two on the previous ones, and we'll see. If this becomes a frequently re-appearing topic, we'll have to come up with more generic aproach. I think it is unacceptable to punish owners of good mice for quirky stuff, yet the current approach of manually setting quirks from a module option or adding them to the blacklist has drawbacks. 1. It is slow and cumbersome (especially the blacklist) 2. adding the quirk requires root rights, foreknowledge and is limited to 4 devices So at this point. I'd say the best option would be to have a module parameter to tell HID that it should assume all devices to be quirky. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] hid-ids.h: fix typo
There is a typo in a device ID Signed-off-by: Oliver Neukum oneu...@suse.de CC: sta...@vger.kernel.org --- drivers/hid/hid-ids.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 56a4ed6..4ee0fb0 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -555,7 +555,7 @@ #define USB_VENDOR_ID_LOGITECH 0x046d #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e -#define USB_DEVICE_ID_LOGITECH_C0770xc007 +#define USB_DEVICE_ID_LOGITECH_C0770xc077 #define USB_DEVICE_ID_LOGITECH_RECEIVER0xc101 #define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110 #define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
latest mouse quirks
Hi, please don't apply them. There's a typo in an ID. I'll resubmit. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
quirk ALWAYS_POLL needed with an unexpectedly high number of mice
Hi, I am getting reports from a customer running an unintentional stress test for this quirk. I would have to add a dozen mice in the medium term. Is that desirable? Should we make the quirk the default? Or add a module parameter to let the pessimistic users with multiple desktop rodents select the quirk as default? Or just carry on? Comments? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: proposal for deletion of drivers/hid/hid-ids.h
On Thu, 2015-03-26 at 10:06 -0400, Benjamin Tissoires wrote: On Thu, Mar 26, 2015 at 7:44 AM, Oliver Neukum oneu...@suse.de wrote: Hi, I would like to kill drivers/hid/hid-ids.h and replace it with numerical IDs in the files using it. There are two reasons for that. 1. It is a layering violation. There should not be a private data base for USB IDs in HID. Technically, this DB is not only for USB devices. We also have Bluetooth and I2C devices here. Well, the token IDs ;) 2. It serves no purpose and adds work. Anyone who adds a quirk or a special case for devices needs to operate on the numbers, as those are what he gets from the descriptors. Looking up or adding a symbolic name for a device is just more work without a benefit. These numbers have no intrinsic meaning beyond being unique and it rarely matters (and should not matter) for which vendor a particular fix is intended. I disagree. This list might not be useful for the drivers/hid/usbhid/hid-quirks.c by itself in most cases. However, we mainly rely on this list to add the device in hid_have_special_driver and hid_ignore in hid-core and in the submodule that should handle it. Can you explain how we depend on it? We certainly use it, but how do we depend on it? I don't see how just the numbers would be worse. In fact they would be better as you again save a lookup. Many times, already having the VID/PID registered in hid-ids.h saved some time when debugging and adding a subdriver for a special device because if the VID/PID is already in hid-ids.h, that means that Again, I see how having the VID/PID pair is an advantage. I don't see why having symbolic names for that pair is an advantage. Just having the numbers in a list of quirky devices would serve the same purpose. someone already dealt with it, and it gives us a way to clean it when the quirk was not appropriate. For instance, many multitouch devices were added before the creation of hid-multitouch and were registered with the quirk MULTI_INPUT. Well, yes, so you needed to grep for MULTI_INPUT. The entries would still have been present, just with nummerical IDs. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: proposal for deletion of drivers/hid/hid-ids.h
On Fri, 2015-03-27 at 09:49 +0100, Oliver Neukum wrote: On Thu, 2015-03-26 at 10:06 -0400, Benjamin Tissoires wrote: Well, yes, so you needed to grep for MULTI_INPUT. The entries would still have been present, just with nummerical IDs. Especially as I see stuff like this: 0c3910c255 (Stephane Chatty 2010-04-10 16:43:08 +0200 282) #define USB_VENDOR_ID_DWAV0x0eef fe6065dc30 (Peter Hutterer2010-02-02 13:40:40 +1000 283) #define USB_DEVICE_ID_EGALAX_TOUCHCONTROLLER 0x0001 729b814ace (Forest Bond 2012-11-06 13:41:22 -0500 284) #define USB_DEVICE_ID_DWAV_TOUCHCONTROLLER0x0002 e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 285) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_480D 0x480d e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 286) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_480E 0x480e fd1d152583 (Benjamin Tissoires2012-03-06 10:53:47 +0100 287) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7207 0x7207 e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 288) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_720C 0x720c fd1d152583 (Benjamin Tissoires2012-03-06 10:53:47 +0100 289) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7224 0x7224 2ce09df47b (Benjamin Tissoires2012-03-06 17:57:02 +0100 290) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_722A 0x722A fd1d152583 (Benjamin Tissoires2012-03-06 10:53:47 +0100 291) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_725E 0x725e fd1d152583 (Benjamin Tissoires2012-03-06 10:53:47 +0100 292) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7262 0x7262 e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 293) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_726B 0x726b e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 294) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_72A1 0x72a1 aa672da1b0 (Andy Shevchenko 2013-05-17 14:34:48 +0300 295) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_72AA 0x72aa aa672da1b0 (Andy Shevchenko 2013-05-17 14:34:48 +0300 296) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_72C4 0x72c4 aa672da1b0 (Andy Shevchenko 2013-05-17 14:34:48 +0300 297) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_72D0 0x72d0 66f06127f3 (Benjamin Tissoires2011-11-23 10:54:33 +0100 298) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_72FA 0x72fa bb9ff21072 (Marek Vasut 2011-11-23 10:54:32 +0100 299) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7302 0x7302 fd1d152583 (Benjamin Tissoires2012-03-06 10:53:47 +0100 300) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_7349 0x7349 ae01c9e53f (Thierry Reding2012-08-09 08:34:48 +0200 301) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_73F7 0x73f7 e36f690b37 (Benjamin Tissoires2011-11-23 10:54:31 +0100 302) #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_A001 0xa001 I mean these entries are screaming: I make no sense. Including the number in the name is so close to the nadir to make no difference. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: quirk ALWAYS_POLL needed with an unexpectedly high number of mice
On Fri, 2015-03-27 at 09:18 -0400, Benjamin Tissoires wrote: On Fri, Mar 27, 2015 at 5:29 AM, Oliver Neukum oneu...@suse.de wrote: Hi, I am getting reports from a customer running an unintentional stress test for this quirk. I would have to add a dozen mice in the medium term. Is that desirable? Should we make the quirk the default? Or add a module parameter to let the pessimistic users with multiple desktop rodents select the quirk as default? Or just carry on? Comments? usbhid/hid-core.c already sets HID_QUIRK_NOGET for regular mice/keyboards [1]. Maybe adding ALLWAYS_POLL to those would make sense and solve your problem? It would solve my problem, but it would do so at the expense of hurting devices that operate correctly, but only by a small amount. BTW I am against having this quirk as the general default because we rely on the usb autosuspend to shut off the touchscreens when no one listen to them, and the touchscreens are draining a lot of power. I see. As long as they support remote wakeup, autosuspend will still work. A source of remote wakeup, however, is allowed to draw more power while suspended. Keeping things as is is fine by me but then I will need to add a lot of quirky devices. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
proposal for deletion of drivers/hid/hid-ids.h
Hi, I would like to kill drivers/hid/hid-ids.h and replace it with numerical IDs in the files using it. There are two reasons for that. 1. It is a layering violation. There should not be a private data base for USB IDs in HID. 2. It serves no purpose and adds work. Anyone who adds a quirk or a special case for devices needs to operate on the numbers, as those are what he gets from the descriptors. Looking up or adding a symbolic name for a device is just more work without a benefit. These numbers have no intrinsic meaning beyond being unique and it rarely matters (and should not matter) for which vendor a particular fix is intended. In the rare cases it does matter when it does matter searching the official list of USB IDs is less work. So let's kill this utterly useless step of indirection. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: sony: Enable Gasia third-party PS3 controllers
On Mon, 2015-03-16 at 22:37 +0100, Jiri Kosina wrote: On Mon, 16 Mar 2015, Pavel Machek wrote: Oliver Neukum oneu...@suse.de wrote: + ret = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x02), + buf2, sizeof(buf2), + transfered, USB_CTRL_SET_TIMEOUT); You cannot do this. Even for a single byte DMA on the stack is wrong. Not on all architectures it works at all and you violate the DMA constrainsts. You must use kmalloc(). Hi Oliver, Does this still apply when using hid_hw_output_report? Yes. For USB devices hid_hw_output_report() goes to usbhid_output_report(). That goes to usb_interrupt_msg(), which passes the buffer pointer. It will then be mapped for DMA. You must not do that on the stack. Should we have some kind of runtime test for this ...? Because this is very very easy to get wrong... and I bet we do get it wrong at 1 place... Are you sure CONFIG_DMA_API_DEBUG wouldn't warn here? As far as I can tell, it will not warn. The problem is not in the mapping itself. That is usually legitimate. The problem arises because the buffer doesn't have a cacheline of its own. Thus the memory corruption happens after the IO operation has started. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: sony: Enable Gasia third-party PS3 controllers
On Thu, 2015-03-19 at 11:12 +0100, Pavel Machek wrote: On Thu 2015-03-19 10:54:22, Oliver Neukum wrote: On Thu, 2015-03-19 at 10:38 +0100, Pavel Machek wrote: On Thu 2015-03-19 10:14:21, Oliver Neukum wrote: On Mon, 2015-03-16 at 22:37 +0100, Jiri Kosina wrote: Are you sure CONFIG_DMA_API_DEBUG wouldn't warn here? As far as I can tell, it will not warn. The problem is not in the mapping itself. That is usually legitimate. The problem arises because the buffer doesn't have a cacheline of its own. Thus the memory corruption happens after the IO operation has started. Nasty. Would WARN_ON(buffer CACHELINE_SIZE-1) do at least part of No. It is perfectly legitimate to put your buffer at an offset or to combine buffers provided you don't use them at the same time. Legitimate: yes. Is anyone doing it? And will not they see exactly the For this particular function probably not. In the general case, yes. That's how you continue after an error. same data corruption with the aliasing data? No, the error happens by touching another part of the cacheline from the CPU thus loading stale content into the cache. You can even have simultaneous DMA to two buffers in the same cacheline provided you touch neither until the last DMA has finished. Alternatively, we could create allocate_for_usb function, and only take pointers allocated by that function in usb functions. That would also teach people the problem exists... No, this problem is not limited to USB. Well.. Recognize that just because you have a pointer does not mean you can pass it to certain functions. Maybe those functions should not be taking pointers in the first place What else would it take? Should we force people to allocate a new buffer every time? That would make the API for reading and writing asymmetric. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: sony: Enable Gasia third-party PS3 controllers
On Thu, 2015-03-19 at 10:38 +0100, Pavel Machek wrote: On Thu 2015-03-19 10:14:21, Oliver Neukum wrote: On Mon, 2015-03-16 at 22:37 +0100, Jiri Kosina wrote: Are you sure CONFIG_DMA_API_DEBUG wouldn't warn here? As far as I can tell, it will not warn. The problem is not in the mapping itself. That is usually legitimate. The problem arises because the buffer doesn't have a cacheline of its own. Thus the memory corruption happens after the IO operation has started. Nasty. Would WARN_ON(buffer CACHELINE_SIZE-1) do at least part of No. It is perfectly legitimate to put your buffer at an offset or to combine buffers provided you don't use them at the same time. the trick? Alternatively, could we call ksize() on the object, and fail if it is not big enough? What object? We have a pointer to a memory location. Alternatively, we could create allocate_for_usb function, and only take pointers allocated by that function in usb functions. That would also teach people the problem exists... No, this problem is not limited to USB. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] INPUT/HID: add touch support for SiS touch driver
1
+#define BYTE_ReportID1
+#define BYTE_CRC_HIDI2C 0
+#define BYTE_CRC_I2C 2
+#define BYTE_SCANTIME2
+#define NO_TOUCH_BYTECOUNT 0x3
+
+/* TODO */
+#define TOUCH_POWER_PIN 0
+#define TOUCH_RESET_PIN 1
+
+/* CMD Define */
+#define BUF_ACK_PLACE_L 4
+#define BUF_ACK_PLACE_H 5
+#define BUF_ACK_L0xEF
+#define BUF_ACK_H0xBE
+#define BUF_NACK_L 0xAD
+#define BUF_NACK_H 0xDE
+#define BUF_CRC_PLACE7
+
+/* SiS i2c error code */
+#define SIS_ERR -1
+#define SIS_ERR_ACCESS_USER_MEM -11 /* Access user memory fail
*/
+#define SIS_ERR_ALLOCATE_KERNEL_MEM -12 /* Allocate memory fail */
+#define SIS_ERR_CLIENT -13 /* Client not
created */
+#define SIS_ERR_COPY_FROM_USER -14 /* Copy data from user fail
*/
+#define SIS_ERR_COPY_FROM_KERNEL -19 /* Copy data from kernel fail */
+#define SIS_ERR_TRANSMIT_I2C -21 /* Transmit error in I2C */
Stop this. You _must_ use the symbolic names the kernel provides.
+
+struct sis_i2c_rmi_platform_data {
+ int (*power)(int on); /* Only valid in first array entry */
+};
+
+struct Point {
+ int id;
+ unsigned short x, y;/*uint16_t ?*/
+ uint16_t Pressure;
+ uint16_t Width;
+ uint16_t Height;
+};
+
+struct sisTP_driver_data {
+ int id;
+ int fingers;
+ uint8_t pre_keybit_state;
+ struct Point pt[MAX_FINGERS];
+};
+
+struct sis_ts_data {
+ int (*power)(int on);
+ int use_irq;
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ struct hrtimer timer;
+ struct irq_desc *desc;
+ struct work_struct work;
+ struct mutex mutex_wq;
+#ifdef CONFIG_HAS_EARLYSUSPEND
+ struct early_suspend early_suspend;
+#endif
+};
+
+static int sis_ts_suspend(struct i2c_client *client, pm_message_t mesg);
+static int sis_ts_resume(struct i2c_client *client);
+
+#endif /* _LINUX_SIS_I2C_H */
--
Oliver Neukum oneu...@suse.de
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] INPUT/HID: add touch support for SiS touch driver
);
+ class_destroy(sis_char_class);
+ sis_char_class = NULL;
+ cdev_del(sis_char_cdev);
+ memset(sis_char_cdev, 0, sizeof(struct cdev));
+ sis_char_major = 0;
+ unregister_chrdev_region(dev, sis_char_devs_count);
+ usb_kill_urb(backup_urb);
+ usb_free_urb(backup_urb);
+ backup_urb = NULL;
+ hid_dev_backup = NULL;
+ }
+}
diff --git a/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
b/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
index 4477eb7..b534321 100644
--- a/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
+++ b/linux-3.18.5/drivers/hid/usbhid/hid-quirks.c
@@ -97,6 +97,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_SIGMATEL, USB_DEVICE_ID_SIGMATEL_STMP3780,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS9200_TOUCH, HID_QUIRK_NOGET
},
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS817_TOUCH, HID_QUIRK_NOGET
},
+ { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SISF817_TOUCH,
+ HID_QUIRK_NOGET },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS_TS,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS1030_TOUCH, HID_QUIRK_NOGET
},
{ USB_VENDOR_ID_SUN, USB_DEVICE_ID_RARITAN_KVM_DONGLE, HID_QUIRK_NOGET
},
--
Oliver Neukum oneu...@suse.de
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: sony: Enable Gasia third-party PS3 controllers
On Mon, 2015-02-09 at 20:44 +0200, Lauri Kasanen wrote: On Mon, 09 Feb 2015 11:08:01 +0100 Oliver Neukum oneu...@suse.de wrote: + ret = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x02), + buf2, sizeof(buf2), + transfered, USB_CTRL_SET_TIMEOUT); You cannot do this. Even for a single byte DMA on the stack is wrong. Not on all architectures it works at all and you violate the DMA constrainsts. You must use kmalloc(). Hi Oliver, Does this still apply when using hid_hw_output_report? Yes. For USB devices hid_hw_output_report() goes to usbhid_output_report(). That goes to usb_interrupt_msg(), which passes the buffer pointer. It will then be mapped for DMA. You must not do that on the stack. HTH Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: sony: Enable Gasia third-party PS3 controllers
On Sat, 2015-02-07 at 15:48 +0200, Lauri Kasanen wrote:
Without this, my Gasia Co.,Ltd PS(R) Gamepad would not send
any events. Now everything works including the leds.
Based on work by Andrew Haines and Antonio Ospite.
cc: Antonio Ospite a...@ao2.it
cc: Andrew Haines andrewd...@aol.com
Signed-off-by: Lauri Kasanen c...@gmx.com
---
drivers/hid/hid-sony.c | 26 --
1 file changed, 24 insertions(+), 2 deletions(-)
Antonio's original approach was not enough; it enabled the events,
but only for a few seconds, then the controller timed out and sent
no more. Andrew's did more than was necessary. This is a combination
of the two, against Linus' git.
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 31e9d25..de93386 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -36,6 +36,7 @@
#include linux/list.h
#include linux/idr.h
#include linux/input/mt.h
+#include linux/usb/input.h
#include hid-ids.h
@@ -1130,8 +1131,12 @@ static void sony_input_configured(struct hid_device
*hdev,
*/
static int sixaxis_set_operational_usb(struct hid_device *hdev)
{
+ struct usb_interface *intf = to_usb_interface(hdev-dev.parent);
+ struct usb_device *dev = interface_to_usbdev(intf);
int ret;
- char *buf = kmalloc(18, GFP_KERNEL);
+ char *buf = kmalloc(65, GFP_KERNEL);
+ unsigned char buf2[] = { 0x00 };
+ int transfered;
if (!buf)
return -ENOMEM;
@@ -1140,7 +1145,24 @@ static int sixaxis_set_operational_usb(struct
hid_device *hdev)
HID_REQ_GET_REPORT);
if (ret 0)
- hid_err(hdev, can't set operational mode\n);
+ hid_err(hdev, can't set operational mode on the control EP\n);
+
+ /*
+ * Some compatible controllers like the Speedlink Strike FX and
+ * Gasia need another query plus an USB interrupt to get operational.
+ */
+ ret = hid_hw_raw_request(hdev, 0xf5, buf, 64, HID_FEATURE_REPORT,
+ HID_REQ_GET_REPORT);
+
+ if (ret 0)
+ hid_err(hdev, can't set operational mode on the interrupt
EP\n);
+
+ ret = usb_interrupt_msg(dev, usb_sndintpipe(dev, 0x02),
+ buf2, sizeof(buf2),
+ transfered, USB_CTRL_SET_TIMEOUT);
You cannot do this. Even for a single byte DMA on the stack is
wrong. Not on all architectures it works at all and you violate
the DMA constrainsts. You must use kmalloc().
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] INPUT/HID: add touch support for SiS touch driver
On Fri, 2015-01-16 at 18:59 +0800, 曾婷葳 (tammy_tseng) wrote:
Hey, Oliver
On Mon, Jan 12, 2015 at 7:50 PM, Oliver Neukum oneu...@suse.de wrote:
On Mon, 2015-01-12 at 18:53 +0800, 曾婷葳 (tammy_tseng) wrote:
(Skip the code diff...)
Again macros for endianness
And the driver has a great number of conditional compilations are they
really needed? The driver as is has a number of issues and is hard to
review due to the use of // for comments and a lot of conditional
compilation and unnecessary variables used for constants. Could you
fix this up and resubmit?
Thanks for the reply. I've modified the code
(sis_i2c.c/sis_i2c.h/Kconfig/Makefile) to fix them.
Please help check them if anything needs to fix.
Hi,
I started from the lower end. I hope you find my comments
useful. A few general points need to be made before the
code is ready for review (it is obviously not ready for
inclusion)
1. We don't do conditional compilation unless we absolutely have
to in the kernel. The support for earlier kernel versions has
to be ripped out of the code.
2. You must use the symbolic names of error codes. They are not
guaranteed to be uniform among architectures.
3. If you need to sleep at places without obvious need, you need
to include comments explaining the reason.
4. No comments with //
Please redo these things and resubmit.
Regards
Oliver
+/* for get system time */
+static ssize_t sis_cdev_read( struct file *file, char __user *buf, size_t
count, loff_t *f_pos )
+{
+ int ret = 0;
+ char *kdata;
+ char cmd;
+ int i;
+
+ printk(KERN_INFO sis_cdev_read.\n);
+
+ if (ts_bak == 0)
+ return -13;
symbolic name necessary
+
+ret = access_ok(VERIFY_WRITE, buf, BUFFER_SIZE);
+if (!ret) {
+printk(KERN_ERR cannot access user space memory\n);
+return -11;
symbolic name necessary
+}
+
+ kdata = kmalloc(BUFFER_SIZE, GFP_KERNEL);
+ if (kdata == 0)
+ return -12;
symbolic name necessary
+
+ ret = copy_from_user(kdata, buf, BUFFER_SIZE);
+ if (ret) {
+printk(KERN_ERR copy_from_user fail\n);
+kfree(kdata);
+return -14;
symbolic name necessary
+ }
+#if 0
+PrintBuffer(0, count, kdata);
+#endif
+ cmd = kdata[6];
+ /* for making sure AP communicates with SiS driver */
+if(cmd == 0xa2)
+{
+ kdata[0] = 5;
+ kdata[1] = 0;
+ kdata[3] = 'S';
+ kdata[4] = 'i';
+ kdata[5] = 'S';
+ if ( copy_to_user((char*) buf, kdata, BUFFER_SIZE ) )
What is happening here?
+ {
+ printk(KERN_ERR copy_to_user fail\n );
+ kfree( kdata );
+ return -19;
symbolic name needed
+ }
+
+ kfree( kdata );
+ return 3;
Why?
+ }
+/* Write Read */
+ret = sis_command_for_read(ts_bak-client, MAX_BYTE, kdata);
+if (ret 0) {
+printk(KERN_ERR i2c_transfer read error %d\n, ret);
+ kfree(kdata);
+ return -21;
symbolic name needed
+ }
+
+ ret = kdata[0] | (kdata[1] 8);
Use the macros endianness conversion.
In this case
ret = le16_to_cpu(kdata + 0);
+printk(KERN_INFO %d\n, ret);
+
+for ( i = 0; i ret i BUFFER_SIZE; i++ )
+{
+printk(%02x , kdata[i]);
+}
+
+printk( \n );
+
+if ( copy_to_user((char*) buf, kdata, BUFFER_SIZE ) )
+{
+printk(KERN_ERR copy_to_user fail\n );
+ret = -19;
What is this to mean? I suppose it is an error code. The symbolic
values must be used.
+}
+
+kfree( kdata );
+
+ return ret;
+}
+
+#undef BUFFER_SIZE
+
+static int sis_cdev_open(struct inode *inode, struct file *filp)
+{
+ printk(KERN_INFO sis_cdev_open.\n);
+ if ( ts_bak == 0 )
+ return -13;
What is this to mean?
+
+ msleep(200);
Why?
+ if (ts_bak-use_irq)
+ {
+#if ( LINUX_VERSION_CODE KERNEL_VERSION (2, 6, 39) )
+if ((ts_bak-desc-status IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#else
+ if ((ts_bak-desc-irq_data.state_use_accessors
IRQD_IRQ_DISABLED) == IRQ_STATUS_ENABLED)
+#endif
+ {
+ disable_irq(ts_bak-client-irq);
+ }
+ else
+ {
+#if ( LINUX_VERSION_CODE KERNEL_VERSION (2, 6, 39) )
You are submitting for a defined kernel version. This needs to
go away.
+ printk(KERN_INFO sis_cdev_open: IRQ_STATUS:
%x\n,(ts_bak-desc-status IRQ_DISABLED));
+#else
+ printk(KERN_INFO sis_cdev_open: IRQ_STATUS:
%x\n,(ts_bak-desc-irq_data.state_use_accessors IRQD_IRQ_DISABLED));
+#endif
+ }
+ }
+ hrtimer_cancel(ts_bak-timer);
+
+ /* only flush sis_wq */
+ flush_workqueue(sis_wq
Re: [PATCH 1/2] INPUT/HID: add touch support for SiS touch driver
On Mon, 2015-01-12 at 18:53 +0800, 曾婷葳 (tammy_tseng) wrote:
Hi,
This package of patch is to add support for multitouch behavior for SiS touch
products.
The patch of SiS i2c multitouch driver is in input/touchscreen.
Signed-off-by: Tammy Tseng tammy_ts...@sis.com
---
diff --git a/linux-3.18.1/drivers/input/touchscreen/Kconfig
b/linux-3.18.1/drivers/input/touchscreen/Kconfig
index e1d8003..edc8e27 100644
--- a/linux-3.18.1/drivers/input/touchscreen/Kconfig
+++ b/linux-3.18.1/drivers/input/touchscreen/Kconfig
@@ -962,4 +962,18 @@ config TOUCHSCREEN_ZFORCE
To compile this driver as a module, choose M here: the
module will be called zforce_ts.
+config TOUCHSCREEN_SIS_I2C
+ tristate SiS 9200 family I2C touchscreen driver
+ depends on I2C
+ default y
Why that default? The majority of systems will not feature this
touchscreens.
+ help
+ This enables support for SiS 9200 family over I2C based touchscreens.
+
+config FW_SUPPORT_POWERMODE
+ default n
+bool SiS FW support power mode
+depends on TOUCHSCREEN_SIS_I2C
+help
+ This enables support power mode provided by SiS firmwave
What does this mode do? The help should be more extensive to be
helpful.
+
endif
diff --git a/linux-3.18.1/drivers/input/touchscreen/Makefile
b/linux-3.18.1/drivers/input/touchscreen/Makefile
index 090e61c..e316477 100644
--- a/linux-3.18.1/drivers/input/touchscreen/Makefile
+++ b/linux-3.18.1/drivers/input/touchscreen/Makefile
@@ -6,6 +6,10 @@
wm97xx-ts-y := wm97xx-core.o
+ifdef CONFIG_TOUCHSCREEN_SIS_I2C
+obj-$(CONFIG_TOUCHSCREEN_SIS_I2C) += sis_i2c.o
+endif
+
obj-$(CONFIG_OF_TOUCHSCREEN) += of_touchscreen.o
obj-$(CONFIG_TOUCHSCREEN_88PM860X) += 88pm860x-ts.o
obj-$(CONFIG_TOUCHSCREEN_AD7877) += ad7877.o
diff --git a/linux-3.18.1/drivers/input/touchscreen/sis_i2c.c
b/linux-3.18.1/drivers/input/touchscreen/sis_i2c.c
new file mode 100644
index 000..2e6fc1a
--- /dev/null
+++ b/linux-3.18.1/drivers/input/touchscreen/sis_i2c.c
@@ -0,0 +1,1725 @@
+/* drivers/input/touchscreen/sis_i2c.c - I2C Touch panel driver for SiS 9200
family
+ *
+ * Copyright (C) 2011 SiS, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Date: 2012/11/13
+ * Version: Android_v2.05.00-A639-1113
+ */
+
+#include linux/module.h
+#include linux/delay.h
+#ifdef CONFIG_HAS_EARLYSUSPEND
+#include linux/earlysuspend.h
+#endif
Conditional includes should be avoided if possible.
+#include linux/hrtimer.h
+#include linux/i2c.h
+#include linux/input.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/platform_device.h
+#include sis_i2c.h
+#include linux/linkage.h
+#include linux/slab.h
+#include linux/gpio.h
+#include asm/uaccess.h
+#include linux/irq.h
+
+#ifdef _STD_RW_IO
???
+#include linux/init.h
+#include linux/fs.h
+#include linux/cdev.h
+#define DEVICE_NAME sis_aegis_touch_device
+static int sis_char_devs_count = 1;/* device count */
Why start with 1 ?
+static int sis_char_major = 0;
+static struct cdev sis_char_cdev;
+static struct class *sis_char_class = NULL;
+#endif
+
+/* Addresses to scan */
+static const unsigned short normal_i2c[] = { SIS_SLAVE_ADDR, I2C_CLIENT_END
};
+static struct workqueue_struct *sis_wq;
+struct sis_ts_data *ts_bak = 0;
+struct sisTP_driver_data *TPInfo = NULL;
Are you really sure you are limited to a single device?
+static void sis_tpinfo_clear(struct sisTP_driver_data *TPInfo, int max);
+
+#ifdef CONFIG_HAS_EARLYSUSPEND
+static void sis_ts_early_suspend(struct early_suspend *h);
+static void sis_ts_late_resume(struct early_suspend *h);
+#endif
+
+#ifdef CONFIG_X86
+//static const struct i2c_client_address_data addr_data;
+/* Insmod parameters */
+static int sis_ts_detect(struct i2c_client *client, struct i2c_board_info
*info);
+#endif
+
+#ifdef _CHECK_CRC
+uint16_t cal_crc (char* cmd, int start, int end);
+#endif
+
+void PrintBuffer(int start, int length, char* buf)
Polluting the name space like this is really nasty.
Could you check which of your declarations can be
declared static?
+{
+ int i;
+ for ( i = start; i length; i++ )
+ {
+ printk(%02x , buf[i]);
+ if (i != 0 i % 30 == 0)
+ printk(\n);
+ }
+ printk(\n);
+}
+
+int sis_command_for_write(struct i2c_client *client, int wlength, unsigned
char *wdata)
+{
+int ret = -1;
+struct i2c_msg msg[1];
Why on
Re: [PATCH v2 0/4] input: synaptics - report correct width and pressure values
On Mon, 2015-01-05 at 23:28 +0100, Gabriele Mazzotta wrote: Make image sensors and cr48 sensors report widths and fix the calculation of width and pressure values in some cases. Changes since v1: - Rebased on 35393dcb2ed3 - Get widths from AGM packets too - Include support for cr48 sensors - Fix pressure values on image sensors Hi, it seems to me that all these fixes should go into stable. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] INPUT/HID: add touch support for SiS touch driver
On Fri, 2015-01-09 at 18:36 +0800, 曾婷葳 (tammy_tseng) wrote: Signed-off-by: Tammy Tseng tammy_ts...@sis.com --- The patch for i2c multitouch driver in input/touchscreen: linux-3.18.1/drivers/input/touchscreen/Kconfig | 14 - linux-3.18.1/drivers/input/touchscreen/Makefile |4 - linux-3.18.1/drivers/input/touchscreen/sis_i2c.c | 1725 -- linux-3.18.1/drivers/input/touchscreen/sis_i2c.h | 229 --- 4 files changed, 1972 deletions(-) Only deletions and it adds support? I guess you mixed up your trees. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
On Mon, 2014-11-24 at 16:56 -0800, Benson Leung wrote: Hi Oliver, On Mon, Nov 24, 2014 at 1:13 AM, Oliver Neukum oneu...@suse.de wrote: But there is very little to be gained by switching off remote wakeup. The additional energy consumption devices with remote wakeup enabled will be dwarfed by the energy needed for an additional wakeup. That makes sense to me. Does this mean we should be moving toward a solution that doesn't wake suspended devices on close for other usb devices, not just hid? This particular pattern of get()/needs_remote_wakeup=0/put() on close() appears in several other drivers, for example : 62ecae0 Input: wacom - properly enable runtime PM 5d9efc5 Input: usbtouchscreen - implement runtime power management Yes, we should never wake up a device just to unset remote wakeup for runtime PM. In hindsight those patches were clumsy. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
On Fri, 2014-11-21 at 17:00 -0800, Benson Leung wrote: If devices are already asleep with this flag enabled, that means that they are presently configured for remote wake. Yes, but that doesn't matter. The drivers must be ready for a device being resumed at any time. Remote wakeup just adds one more reason. Waking the device in the case of a close() is appropriate because it also has the effect of re-suspending the device with the capability disabled, as it is no longer necessary. But there is very little to be gained by switching off remote wakeup. The additional energy consumption devices with remote wakeup enabled will be dwarfed by the energy needed for an additional wakeup. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
+static int elants_i2c_sw_reset(struct i2c_client *client)
+{
+ const u8 soft_rst_cmd[] = { 0x77, 0x77, 0x77, 0x77 };
+ int error;
+
+ error = elants_i2c_send(client, soft_rst_cmd,
+ sizeof(soft_rst_cmd));
+ if (error) {
+ dev_err(client-dev, software reset failed: %d\n,
error);
+ return error;
+ }
+
+ /*
+* We should wait at least 10 msec (but no more than 40)
before
+* sending fastboot or IAP command to the device.
+*/
+ msleep(30);
If timing is critical in both ways, you should take our guarantee
if sufficient sleep. I suggest you sleep 11msecs.
+static int elants_i2c_initialize(struct elants_data *ts)
+{
+ struct i2c_client *client = ts-client;
+ int error, retry_cnt;
+ const u8 hello_packet[] = { 0x55, 0x55, 0x55, 0x55 };
+ const u8 recov_packet[] = { 0x55, 0x55, 0x80, 0x80 };
+ u8 buf[HEADER_SIZE];
+
Strictly speaking you should disable preemption here.
+ for (retry_cnt = 0; retry_cnt MAX_RETRIES; retry_cnt++) {
+ error = elants_i2c_sw_reset(client);
+ if (error) {
+ /* Continue initializing if it's the last try
*/
+ if (retry_cnt MAX_RETRIES - 1)
+ continue;
+ }
Regards
Oliver
--
Oliver Neukum oneu...@suse.de
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1] Input: elants_i2c: Add Elan touchscreen support
On Thu, 2014-11-20 at 09:47 -0800, Dmitry Torokhov wrote:
Hi Oliver,
On Thu, Nov 20, 2014 at 11:31:30AM +0100, Oliver Neukum wrote:
+static int elants_i2c_initialize(struct elants_data *ts)
+{
+ struct i2c_client *client = ts-client;
+ int error, retry_cnt;
+ const u8 hello_packet[] = { 0x55, 0x55, 0x55, 0x55 };
+ const u8 recov_packet[] = { 0x55, 0x55, 0x80, 0x80 };
+ u8 buf[HEADER_SIZE];
+
Strictly speaking you should disable preemption here.
Umm, why?
You said the upper bound matters. So you need to protect yourself
against losing too much time by preemption.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] HID: yet another buggy ELAN touchscreen
The touchscreen needs the same quirk as the other models.
Signed-off-by: Oliver Neukum oneu...@suse.de
Reported-by: Bryan Poling poli0...@umn.edu
CC: sta...@vger.kernel.org
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/usbhid/hid-quirks.c | 1 +
drivers/usb/core/quirks.c | 3 +++
3 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e23ab8b..282ffbe 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -299,6 +299,7 @@
#define USB_VENDOR_ID_ELAN 0x04f3
#define USB_DEVICE_ID_ELAN_TOUCHSCREEN 0x0089
#define USB_DEVICE_ID_ELAN_TOUCHSCREEN_009B0x009b
+#define USB_DEVICE_ID_ELAN_TOUCHSCREEN_010c0x010c
#define USB_DEVICE_ID_ELAN_TOUCHSCREEN_016F0x016f
#define USB_VENDOR_ID_ELECOM 0x056e
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 5014bb5..08b9626 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -72,6 +72,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHSCREEN,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHSCREEN_009B,
HID_QUIRK_ALWAYS_POLL },
+ { USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHSCREEN_010c,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHSCREEN_016F,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER,
HID_QUIRK_NO_INIT_REPORTS },
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 39b4081..8e8bc4f 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -100,6 +100,9 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x04f3, 0x009b), .driver_info =
USB_QUIRK_DEVICE_QUALIFIER },
+ { USB_DEVICE(0x04f3, 0x010c), .driver_info =
+ USB_QUIRK_DEVICE_QUALIFIER },
+
{ USB_DEVICE(0x04f3, 0x016f), .driver_info =
USB_QUIRK_DEVICE_QUALIFIER },
--
1.8.4.5
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HID: usbhid: get/put around clearing needs_remote_wakeup
On Thu, 2014-11-13 at 12:16 -0800, Benson Leung wrote: In usbhid_open, usb_autopm_get_interface is called before setting the needs_remote_wakeup flag, and usb_autopm_put_interface is called after hid_start_in. However, when the device is closed in usbhid_close, the same protection isn't there when clearing needs_remote_wakeup. This will add that to usbhid_close as well as usbhid_stop. Interesting, but this has the side effect of waking devices that are asleep just to remove the flag. Regards Oliver -- Oliver Neukum oneu...@suse.de -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbhid: add another mouse that needs QUIRK_ALWAYS_POLL
There is a second mouse sharing the same vendor strings
but different IDs.
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/usbhid/hid-quirks.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b303a62..5a7072d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -733,6 +733,7 @@
#define USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL 0xff
#define USB_VENDOR_ID_PIXART 0x093a
+#define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2 0x0137
#define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE 0x2510
#define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN 0x8001
#define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1 0x8002
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 79f94ae..ce27c62 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -80,6 +80,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1610, HID_QUIRK_NOGET
},
{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1640, HID_QUIRK_NOGET
},
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE,
HID_QUIRK_ALWAYS_POLL },
+ /* OEM version same strings but different IDs */
+ { USB_VENDOR_ID_KYE, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE_ID2,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN2,
HID_QUIRK_NO_INIT_REPORTS },
--
1.8.4.5
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbhid: fix PIXART optical mouse
This mouse keeps disconnecting in runlevel 3. It needs the
ALWAYS_POLL quirk.
Signed-off-by: Oliver Neukum oneu...@suse.de
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/usbhid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0d2e07d..c2aca75 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -736,6 +736,7 @@
#define USB_DEVICE_ID_PI_ENGINEERING_VEC_USB_FOOTPEDAL 0xff
#define USB_VENDOR_ID_PIXART 0x093a
+#define USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE 0x2510
#define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN 0x8001
#define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1 0x8002
#define USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN2 0x8003
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index ca18136..8c97e63 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -80,6 +80,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1610, HID_QUIRK_NOGET
},
{ USB_VENDOR_ID_PENMOUNT, USB_DEVICE_ID_PENMOUNT_1640, HID_QUIRK_NOGET
},
+ { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE,
HID_QUIRK_ALWAYS_POLL },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN2,
HID_QUIRK_NO_INIT_REPORTS },
--
1.8.4.5
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
On Fri, 2014-08-22 at 14:23 -0400, Alan Stern wrote:
On Sat, 23 Aug 2014, vichy wrote:
from your patch, I have some questions:
a. in Alan's version, if both HID_CLEAR_HALT and HID_RESET_PENDING are
set, hid_reset will both clear ep halt and reset devcie.
But in original one, even HID_CLEAR_HALT and HID_RESET_PENDING are
both set, hid_reset only do one of them.
Yes. In my patch, the clear-halt handler will turn on the
HID_RESET_PENDING bit if something goes wrong. In that case we want to
do both things.
Why? If we reset, why bother clearing a halt? Especially as this
may mean waiting the full 5 seconds for a timeout.
is there any special reason in original hid_reset to use below flow?
if (test_bit(HID_CLEAR_HALT, usbhid-iofl)) {
x
} else if (test_bit(HID_RESET_PENDING, usbhid-iofl)) {
xx
}
No special reason. We probably never thought that both flags would be
set.
Yes. And I'd say if this ever happens HID_RESET_PENDING must have
priority and implicitly clears a halt.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Power-managing devices that are not of interest at some point in time
On Wed, 2014-07-16 at 14:08 -0400, Alan Stern wrote: I am not so much concerned about userspace, but about reusing of as much of existing PM framework in the drivers. Right now it is very hard to correctly track dependencies between general open/close, system suspend/resume, and various runtime-PM transitions. Adding yet another PM mechanism into the mix will just add more complexity. Would it make sense to unbind the drivers for these devices when the lid is closed? With the drivers gone, there would naturally be no No, because I don't want settings of my devices to disappear. You can do that only for stateless devices. And I doubt you can tell in general which devices are stateless. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9] leds: USB: HID: Add support for MSI GT683R led panels
On Wed, 2014-06-18 at 19:05 +0300, Janne Kanniainen wrote:
This driver adds support for USB controlled led panels that exists in
MSI GT683R laptop
+static int gt683r_led_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ int i;
+ int ret;
+ int name_sz;
+ char *name;
+ struct gt683r_led *led;
+
+ led = devm_kzalloc(hdev-dev, sizeof(*led), GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ led-mode = GT683R_LED_NORMAL;
+ led-hdev = hdev;
+ hid_set_drvdata(hdev, led);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, hid parsing failed\n);
+ return ret;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret) {
+ hid_err(hdev, hw start failed\n);
+ return ret;
+ }
+
+ for (i = 0; i GT683R_LED_COUNT; i++) {
+ name_sz = strlen(dev_name(hdev-dev)) +
+ strlen(gt683r_panel_names[i]) + 3;
+
+ name = devm_kzalloc(hdev-dev, name_sz, GFP_KERNEL);
+ if (!name) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ snprintf(name, name_sz, %s::%s,
+ dev_name(hdev-dev), gt683r_panel_names[i]);
+ led-led_devs[i].name = name;
+ led-led_devs[i].max_brightness = 1;
+ led-led_devs[i].brightness_set = gt683r_brightness_set;
+ ret = led_classdev_register(hdev-dev, led-led_devs[i]);
+ if (ret) {
+ hid_err(hdev, could not register led device\n);
+ goto fail;
+ }
+ }
+
+ ret = device_create_file(led-hdev-dev, dev_attr_leds_mode);
+ if (ret) {
+ hid_err(hdev, could not make mode attribute file\n);
+ goto fail;
+ }
+
This is the window.
+ mutex_init(led-lock);
+ INIT_WORK(led-work, gt683r_led_work);
+
And here we have a problem. This is a race condition.
At this time you've already created the sysfs files. So
their methods can be called. They will lock a mutex and/or
schedule work that hasn't been initialized.
The initialization must be done before anything in sysfs
is created.
+ return 0;
+
+fail:
+ for (i = i - 1; i = 0; i--)
+ led_classdev_unregister(led-led_devs[i]);
+ hid_hw_stop(hdev);
+ return ret;
+}
+
Sorry for noticing this thread late.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next
On Thu, 2014-04-24 at 12:32 +0200, Michal Malý wrote:
On Wednesday 23 of April 2014 15:41:03 Oliver Neukum wrote:
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
static int drff_play(struct input_dev *dev, void *data,
-struct ff_effect *effect)
+ const struct mlnx_effect_command *command)
{
struct hid_device *hid = input_get_drvdata(dev);
struct drff_device *drff = data;
+ const struct mlnx_rumble_force *rumble_force =
command-u.rumble_force;
int strong, weak;
- strong = effect-u.rumble.strong_magnitude;
- weak = effect-u.rumble.weak_magnitude;
+ strong = rumble_force-strong;
+ weak = rumble_force-weak;
dbg_hid(called with 0x%04x 0x%04x, strong, weak);
- if (strong || weak) {
- strong = strong * 0xff / 0x;
- weak = weak * 0xff / 0x;
-
- /* While reverse engineering this device, I found that
when
- this value is set, it causes the strong rumble to
function
- at a near maximum speed, so we'll bypass it. */
- if (weak == 0x0a)
- weak = 0x0b;
-
- drff-report-field[0]-value[0] = 0x51;
- drff-report-field[0]-value[1] = 0x00;
- drff-report-field[0]-value[2] = weak;
- drff-report-field[0]-value[4] = strong;
- hid_hw_request(hid, drff-report, HID_REQ_SET_REPORT);
-
- drff-report-field[0]-value[0] = 0xfa;
- drff-report-field[0]-value[1] = 0xfe;
- } else {
+ switch (command-cmd) {
+ case MLNX_START_RUMBLE:
+ if (strong || weak) {
+ strong = strong * 0xff / 0x;
+ weak = weak * 0xff / 0x;
+
+ /* While reverse engineering this device, I
found that when
+ this value is set, it causes the strong rumble
to function
+ at a near maximum speed, so we'll bypass it.
*/
+ if (weak == 0x0a)
+ weak = 0x0b;
+
+ drff-report-field[0]-value[0] = 0x51;
+ drff-report-field[0]-value[1] = 0x00;
+ drff-report-field[0]-value[2] = weak;
+ drff-report-field[0]-value[4] = strong;
This looks like an endianness bug.
I don't have a big endian machine to check but why would this be an
endianness
issue? We're dealing with values all the time here, not addresses so I'd
expect the 'weak' and 'strong' values to be truncated if they won't fit into
byte. Division done beforehand makes sure that the values are within 0; 255
range. As far as I can see this is quite common in the HID and Input code. Am
I missing something here?
Sorry, I thought you were writing to 16bit variables.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/24] input: Add ff-memless-next module
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
+/* Some devices might have a limit on how many uncombinable effects
+ * can be played at once */
+static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
+ const struct ff_effect *effect)
+{
+ struct mlnx_effect_command ecmd = {
+ .cmd = MLNX_UPLOAD_UNCOMB,
+ .u.uncomb.id = effect-id,
+ .u.uncomb.effect = effect
+ };
+ return mlnxdev-control_effect(mlnxdev-dev, mlnxdev-private,
ecmd);
+}
+
This mean you are building the structure on the stack
1. Are you sure nobody retains a reference?
2. That is needlessly inefficient
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/24] input: Port hid-holtekff to ff-memless-next
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
static int holtekff_play(struct input_dev *dev, void *data,
-struct ff_effect *effect)
+const struct mlnx_effect_command *command)
{
struct hid_device *hid = input_get_drvdata(dev);
struct holtekff_device *holtekff = data;
+ const struct mlnx_rumble_force *rumble_force =
command-u.rumble_force;
int left, right;
/* effect type 1, length 65535 msec */
u8 buf[HOLTEKFF_MSG_LENGTH] =
{ 0x01, 0x01, 0xff, 0xff, 0x10, 0xe0, 0x00 };
On the kernel stack.
- left = effect-u.rumble.strong_magnitude;
- right = effect-u.rumble.weak_magnitude;
- dbg_hid(called with 0x%04x 0x%04x\n, left, right);
+ switch (command-cmd) {
+ case MLNX_START_RUMBLE:
+ left = rumble_force-strong;
+ right = rumble_force-weak;
+ dbg_hid(called with 0x%04x 0x%04x\n, left, right);
- if (!left !right) {
- holtekff_send(holtekff, hid, stop_all6);
- return 0;
- }
+ if (!left !right) {
+ holtekff_send(holtekff, hid, stop_all6);
+ return 0;
+ }
- if (left)
- buf[1] |= 0x80;
- if (right)
- buf[1] |= 0x40;
+ if (left)
+ buf[1] |= 0x80;
+ if (right)
+ buf[1] |= 0x40;
- /* The device takes a single magnitude, so we just sum them
up. */
- buf[6] = min(0xf, (left 12) + (right 12));
+ /* The device takes a single magnitude, so we just sum
them up. */
+ buf[6] = min(0xf, (left 12) + (right 12));
- holtekff_send(holtekff, hid, buf);
- holtekff_send(holtekff, hid, start_effect_1);
+ holtekff_send(holtekff, hid, buf);
+ holtekff_send(holtekff, hid, start_effect_1);
+ return 0;
+ case MLNX_STOP_RUMBLE:
+ holtekff_send(holtekff, hid, stop_all6);
+ return 0;
+ default:
+ return -EINVAL;
+ }
return 0;
}
This looks very much like doing DMA on the kernel stack.
That is very strictly forbidden. The bug is also in the current
code, but would you care to fix it up?
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/24] input: Port hid-dr to ff-memless-next
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
static int drff_play(struct input_dev *dev, void *data,
-struct ff_effect *effect)
+ const struct mlnx_effect_command *command)
{
struct hid_device *hid = input_get_drvdata(dev);
struct drff_device *drff = data;
+ const struct mlnx_rumble_force *rumble_force =
command-u.rumble_force;
int strong, weak;
- strong = effect-u.rumble.strong_magnitude;
- weak = effect-u.rumble.weak_magnitude;
+ strong = rumble_force-strong;
+ weak = rumble_force-weak;
dbg_hid(called with 0x%04x 0x%04x, strong, weak);
- if (strong || weak) {
- strong = strong * 0xff / 0x;
- weak = weak * 0xff / 0x;
-
- /* While reverse engineering this device, I found that
when
- this value is set, it causes the strong rumble to
function
- at a near maximum speed, so we'll bypass it. */
- if (weak == 0x0a)
- weak = 0x0b;
-
- drff-report-field[0]-value[0] = 0x51;
- drff-report-field[0]-value[1] = 0x00;
- drff-report-field[0]-value[2] = weak;
- drff-report-field[0]-value[4] = strong;
- hid_hw_request(hid, drff-report, HID_REQ_SET_REPORT);
-
- drff-report-field[0]-value[0] = 0xfa;
- drff-report-field[0]-value[1] = 0xfe;
- } else {
+ switch (command-cmd) {
+ case MLNX_START_RUMBLE:
+ if (strong || weak) {
+ strong = strong * 0xff / 0x;
+ weak = weak * 0xff / 0x;
+
+ /* While reverse engineering this device, I
found that when
+ this value is set, it causes the strong rumble
to function
+ at a near maximum speed, so we'll bypass it.
*/
+ if (weak == 0x0a)
+ weak = 0x0b;
+
+ drff-report-field[0]-value[0] = 0x51;
+ drff-report-field[0]-value[1] = 0x00;
+ drff-report-field[0]-value[2] = weak;
+ drff-report-field[0]-value[4] = strong;
This looks like an endianness bug.
+ hid_hw_request(hid, drff-report,
HID_REQ_SET_REPORT);
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/24] input: Add ff-memless-next module
On Wed, 2014-04-23 at 08:59 -0700, Dmitry Torokhov wrote:
On Wed, Apr 23, 2014 at 02:12:59PM +0200, Oliver Neukum wrote:
On Tue, 2014-04-22 at 15:59 +0200, Michal Malý wrote:
+/* Some devices might have a limit on how many uncombinable effects
+ * can be played at once */
+static int mlnx_upload_conditional(struct mlnx_device *mlnxdev,
+ const struct ff_effect *effect)
+{
+ struct mlnx_effect_command ecmd = {
+ .cmd = MLNX_UPLOAD_UNCOMB,
+ .u.uncomb.id = effect-id,
+ .u.uncomb.effect = effect
+ };
+ return mlnxdev-control_effect(mlnxdev-dev, mlnxdev-private,
ecmd);
+}
+
This mean you are building the structure on the stack
1. Are you sure nobody retains a reference?
2. That is needlessly inefficient
Why is it inefficient?
The compiler has to include the data structure and then make
a memcopy to the stack. Instead a pointer to the predined structure
could be passed.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB keyboard occasional key stuck
On Mon, 2014-02-17 at 21:23 +0800, Daniel J Blueman wrote: Across 5+ years of kernels, I've been seeing occasional (1-2 times per day) key-stuck issues where eg a fn+delete combo repeats delete until I press delete again. I've seen this happen with fn+ctrl+left, leaving left held and likewise with right. I saw something like this during the development of runtime PM for HID. It turns out that keyboards don't assert remote wakeup upon key release. Just to rule this out can you deactivate runtime PM and recompile to switch of LPM? Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Input: add regulator haptic driver
On Thu, 2013-10-24 at 18:26 +0900, hyunhee.kim wrote: Hi, Thanks for your review. I resent patch v3 after removing wrong wrapping. I made one toggle function because enable/disable functions have redundant codes and another reviewer suggested it. Is it better to separate it into two functions? Linus doesn't like parameters affecting behavior. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] input: misc: New USB eBeam input driver
On Fri, 2013-08-16 at 22:25 +0200, Yann Cantin wrote:
+/* IRQ */
+static int ebeam_read_data(struct ebeam_device *ebeam, unsigned char *pkt)
+{
+
+/*
+ * Packet description : 8 bytes
+ *
+ * nop packet : FF FF FF FF FF FF FF FF
+ *
+ * pkt[0] : Sensors
+ * bit 1 : ultrasound signal (guessed)
+ * bit 2 : IR signal (tested with a remote...) ;
+ * readings OK : 0x03 (anything else is a show-stopper)
+ *
+ * pkt[1] : raw_x low
+ * pkt[2] : raw_x high
+ *
+ * pkt[3] : raw_y low
+ * pkt[4] : raw_y high
+ *
+ * pkt[5] : fiability ?
+ * often 0xC0
+ *0x80 : OK
+ *
+ * pkt[6] :
+ * buttons state (low 4 bits)
+ * 0x1 = no buttons
+ * bit 0 : tip (WARNING inversed : 0=pressed)
+ * bit 1 : ? (always 0 during tests)
+ * bit 2 : little (1=pressed)
+ * bit 3 : big (1=pressed)
+ *
+ * pointer ID : (high 4 bits)
+ * Tested : 0x6=wand ;
+ * Guessed : 0x1=red ; 0x2=blue ; 0x3=green ; 0x4=black ;
+ * 0x5=eraser
+ * bit 4 : pointer ID
+ * bit 5 : pointer ID
+ * bit 6 : pointer ID
+ * bit 7 : pointer ID
+ *
+ *
+ * pkt[7] : fiability ?
+ * often 0xFF
+ *
+ */
+
+ /* Filtering bad/nop packet */
+ if (pkt[0] != 0x03)
+ return 0;
+
+ ebeam-raw_x = (pkt[2] 8) | pkt[1];
+ ebeam-raw_y = (pkt[4] 8) | pkt[3];
We have macros. In this case get_unaligned_le16.
+ ebeam-btn_map = (!(pkt[6] 0x1)) |
+ ((pkt[6] 0x4) 1) |
+ ((pkt[6] 0x8) 1);
+
+ return 1;
+}
+static void ebeam_close(struct input_dev *input)
+{
+ struct ebeam_device *ebeam = input_get_drvdata(input);
+ int r;
+
+ usb_kill_urb(ebeam-irq);
+
+ r = usb_autopm_get_interface(ebeam-interface);
Nope. That's an uncool race. If the interface is suspended when
you call this, you will resume it and restart the URB. Therefore
you ought to kill the URB after resuming.
+ ebeam-interface-needs_remote_wakeup = 0;
+
+ if (!r)
+ usb_autopm_put_interface(ebeam-interface);
+}
+
+static int ebeam_suspend(struct usb_interface *intf, pm_message_t message)
+{
+ struct ebeam_device *ebeam = usb_get_intfdata(intf);
+
+ usb_kill_urb(ebeam-irq);
+
+ return 0;
+}
+
+static int ebeam_resume(struct usb_interface *intf)
+{
+ struct ebeam_device *ebeam = usb_get_intfdata(intf);
+ struct input_dev *input = ebeam-input;
+ int result = 0;
+
+ mutex_lock(input-mutex);
+ if (input-users)
+ result = usb_submit_urb(ebeam-irq, GFP_NOIO);
+ mutex_unlock(input-mutex);
+
+ return result;
+}
+
+static int ebeam_reset_resume(struct usb_interface *intf)
+{
+ return ebeam_resume(intf);
+}
No need to define a function. You can use the function as
value for resume and reset_resume.
+
+static int ebeam_probe(struct usb_interface *intf,
+const struct usb_device_id *id)
+{
+ struct ebeam_device *ebeam;
+ struct input_dev *input_dev;
+ struct usb_endpoint_descriptor *endpoint;
+ struct usb_device *udev = interface_to_usbdev(intf);
+ int err = -ENOMEM;
+
+ endpoint = ebeam_get_input_endpoint(intf-cur_altsetting);
+ if (!endpoint)
+ return -ENXIO;
+
+ ebeam = kzalloc(sizeof(struct ebeam_device), GFP_KERNEL);
+ input_dev = input_allocate_device();
+ if (!ebeam || !input_dev)
+ goto out_free;
+
+ ebeam_init_settings(ebeam);
+
+ ebeam-data = usb_alloc_coherent(udev, REPT_SIZE,
+ GFP_KERNEL, ebeam-data_dma);
+ if (!ebeam-data)
+ goto out_free;
+
+ ebeam-irq = usb_alloc_urb(0, GFP_KERNEL);
+ if (!ebeam-irq) {
+ dev_dbg(intf-dev,
+ %s - usb_alloc_urb failed: ebeam-irq\n, __func__);
+ goto out_free_buffers;
+ }
+
+ ebeam-interface = intf;
+ ebeam-input = input_dev;
+
+ /* setup name */
+ snprintf(ebeam-name, sizeof(ebeam-name),
+ USB eBeam %04x:%04x,
+ le16_to_cpu(udev-descriptor.idVendor),
+ le16_to_cpu(udev-descriptor.idProduct));
+
+ if (udev-manufacturer || udev-product) {
+ strlcat(ebeam-name,
+ (,
+ sizeof(ebeam-name));
+
+ if (udev-manufacturer)
+ strlcat(ebeam-name,
+ udev-manufacturer,
+ sizeof(ebeam-name));
+
+ if (udev-product) {
+ if (udev-manufacturer)
+ strlcat(ebeam-name,
+ ,
+ sizeof(ebeam-name));
+ strlcat(ebeam-name,
+ udev-product,
+
Re: [PATCH v2] HID: appleir: add support for Apple ir devices
On Wednesday 17 April 2013 11:03:09 Benjamin Tissoires wrote:
+static void appleir_remove(struct hid_device *hid)
+{
+ struct appleir *appleir = hid_get_drvdata(hid);
+ del_timer_sync(appleir-key_up_timer);
+ hid_hw_stop(hid);
+ kfree(appleir);
+}
Hi,
this looks like a race condition. If you get input between
del_timer_sync() and hid_hw_stop() the timer may be restarted.
You need to stop the hw first.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-input in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] USB: don't recover device if suspend fails in system sleep
On Tuesday 05 March 2013 18:55:42 Ming Lei wrote: All these drivers suspend in multiple steps, where each step can fail. If a later step fails then they revert any previously successful step before returning the failure, thereby ensuring that the device/driver state when suspend returns is consistently either suspended or resumed. IMO, for autosuspend, that is right, but it is not for system suspend, and the driver's suspend callback can't return in resumed state because the USB core will ignore the failure return value and force to suspend the device. It seems to me that in this case you just need to make sure that suspend() not fail for system suspend. Or revisit the decision to ignore failures. In other words, if we don't handle errors, there must be no errors, otherwise it doesn't matter what we do in the error case. We'd leave the problem to generic layers. Furthermore there is a small chance that although the device tree is walked, teh system suspend fails for another later reason that is not ignored. In that case the drivers need to do error recovery, albeit in resume(). Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] USB: don't recover device if suspend fails in system sleep
On Tuesday 05 March 2013 21:08:09 Ming Lei wrote: On Tue, Mar 5, 2013 at 8:50 PM, Oliver Neukum oneu...@suse.de wrote: In other words, if we don't handle errors, there must be no errors, otherwise it doesn't matter what we do in the error case. We'd leave the problem to generic layers. Generic layers can't handle the driver's specific failure. We depend on stopping the HC forcing all devices into suspend. We know this is problematic. For example some disk enclosures need to flush cache. Fortunately for us this is done in the SCSI layer. If driver records its suspend failure state in suspend(), resume() should and can deal with it without much difficulty. Yes, but why bother? Either we can safely suspend in any state or we must not ignore errors. Furthermore there is a small chance that although the device tree is walked, teh system suspend fails for another later reason that is not ignored. In that case the drivers need to do error recovery, albeit in resume(). Yes, resume() need to handle the USB system suspend failure either in normal resume or error recovery, both are basically same. In theory yes, in practice usually power is cut. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [eBeam PATCH 2/2] input: misc: New USB eBeam input driver.
On Wednesday 05 September 2012 21:58:06 Yann Cantin wrote: As ebeams are the only devices to my knowledge that work that way, i don't think a common API can be common, unless we mean an in-kernel generic purpose calibration API for input devices (stellar away for me), or a userland one (where should it be in the stack ?). Sincerely, this look overkill. In the other hand, the actual ebeam module transformation feeding events subsys works very well and expose straight and usable data to userland (xorg evdev for now, and any program that can eat kernel's input data). OK, I see the problem. You have no other choice. ## I understand the sysfs interface is a problem. Eventually, in last resort, i can reduce it to 4 files : pass the 9 matrix parameters as one big string, removing min values. But i think this obfuscate the api for a marginal gain. That would be wrong. The problem is a specific API. If it needs to be done at all, it better be done as cleanly as possible. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC ebeam PATCH v4 2/2] input: misc: New USB eBeam input driver.
On Friday 24 August 2012 16:42:20 Yann Cantin wrote: Hi, Le 24/08/2012 13:41, Oliver Neukum a écrit : On Friday 24 August 2012 11:37:45 Yann Cantin wrote: Hi, Le 23/08/2012 09:23, Oliver Neukum a écrit : On Thursday 23 August 2012 00:11:54 Yann Cantin wrote: These functions are identical. You should unify them. Removed reset_resume from the driver (optional, and not needed for this hardware). Why did you do that? It is always better to have reset_resume(). And you cannot tell whether it will be needed. This function was used in usbtouchscreen (which this driver is based on) for some hardware specific init after reset. eBeam devices doesn't need that, and i didn't mention the similarity of the 2 functions after stripping the code. According to power-management.txt, reset_resume is optional, and lot of input driver lack it. Anyway, if you think it's worth the code, i'll re-add a reset_resume function proxing resume like wacom_sys.c do. It is always better to have reset_resume() if it can be easily done. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC ebeam PATCH v4 2/2] input: misc: New USB eBeam input driver.
On Friday 24 August 2012 11:37:45 Yann Cantin wrote: Hi, Le 23/08/2012 09:23, Oliver Neukum a écrit : On Thursday 23 August 2012 00:11:54 Yann Cantin wrote: These functions are identical. You should unify them. Removed reset_resume from the driver (optional, and not needed for this hardware). Why did you do that? It is always better to have reset_resume(). And you cannot tell whether it will be needed. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.5.1 hid_generic causes mouse locking until a button is clicked
On Sunday 12 August 2012 18:16:22 Dâniel Fraga wrote: I mean, it seems that with kernel 3.4 simply moving the mouse would wake up the usb port and now with kernel 3.5 it only works if I press a button. So, kernel 3.5 is supposed to wake up the usb port when we move the mouse or not? The kernel doesn't do this. Mice are told to wake up when they have input data. They decide themselves what data should be collected while they sleep. The USB HID spec offers no way to learn what will wake up a device. In fact, it does'nt mention power management. Is there a way to restore the old behaviour from kernel 3.4? Probably the device didn't autosuspend. Disable autosuspension. Regards Oliver -- To unsubscribe from this list: send the line unsubscribe linux-input in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
