On Sun, 17 Nov 2013 10:36:55 +0100
Sven Eckelmann <sven@xxxxxxxxxxxxx> wrote:
> On Saturday 16 November 2013 17:30:25 simon@xxxxxxxxxxxxx wrote:
> > This patch appears to work OK with my DualShock/SixAxis controller (USB
> > connection), but causes a machine lockup when used with my Intec wired
> > controller.
> >
> > The Intec controller works OK up to the point when I start rumble,
> > sometimes the motor runs some times it doesn't. I am using SDL2's
> > 'testhaptic' application.
>
> Thanks a lot for this bug report. The testhaptic was a good testcase because
> it is a heavy user and can reproduce the problem quite easily. I've only
> tested it using testrumble and own programs which didn't seem to trigger the
> problem here. The problem is easy to explain:
>
> * usb_control_msg/usb_interrupt_msg/usb_bulk_msg/... may sleep
> * sony_play_effect may gets called in an softirq context (atomic)
>
> So it is a bad choice to use hid_output_raw_report (which calls
> usb_control_msg) in a ff_memless control function. I will just send a revert
> patch in some minutes.
>
Sven, if you are going to resubmit another patch for this functionality
(I've seen your v2 just before hitting "Send"), wouldn't it be better
to advertise just FF_RUMBLE? AFAICS your first patch results in this
(from evtest):
....
Event type 21 (EV_FF)
Event code 80 (FF_RUMBLE)
Event code 81 (FF_PERIODIC)
Event code 88 (FF_SQUARE)
Event code 89 (FF_TRIANGLE)
Event code 90 (FF_SINE)
Event code 96 (FF_GAIN)
I don't know if this is how it should be, I know nothing about FF stuff.
Also please make sure that setting the rumble does not change the LEDs
status if there is any set: HID output report 1 is used for both LEDs
and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
to match the joystick number, just as the PS3 does, it would be strange
for the user if a rumble event would reset the LEDs status.
Maybe only send up to the rumble related bytes, or do a
read-modify-write if that is possible with HID output reports, if this
cannot be done we will have to design a solution to set LEDs too in the
kernel, in order to be able to keep some status around.
Last comment, if we want a conditional config what about using
CONFIG_HID_SONY_FF instead of CONFIG_SONY_FF? Not a big deal of course,
just a suggestion.
Sorry for the late comments but I got to see the patch only after Jiri
had already picked it up.
Thanks,
Antonio
[1] http://ao2.it/tmp/playstation-peripheral-pugin-v5.x-2013-11-13.patch
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
