Heya,
Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
kernel. In the past, it was done in a patch in bluetootd.
- 0001-hidp-Use-the-control-socket-for-raw-messages.patch
As discussed, now uses the ctrl channel for sending raw reports to the device.
- 0002-hid-wacom-Implement-Wacom-quirk-in-the-kernel.patch
Uses the above to poke at the device and turn it into mode2
- 0001-hid-sony-Fix-typo-in-error-message.patch
Small typo, for the input tree
- 0001-hid-Enable-Sixaxis-controller-over-Bluetooth-as-well.patch
Use the hidp patch above to enable the sixaxis controller
Cheers
On Fri, 29 Jan 2010, Bastien Nocera wrote:
> > Having separate methods for feature and output reports should be
> > sufficient and general enough, right?
>
> Most likely, yes. Feature is enough for the 2 devices I provided patches
> for. Could you please comment on what you'd like the implementation of
> this to look like?
I think that we should probably introduce flag parameter into
hid_output_raw_report(), which would indicate whether OUTPUT or FEATURE
report should be sent.
This makes sense both for Bluetooth and HID.
I will send out patches later today for you guys to test (and for Marcel
to Ack the Bluetooth part).
> This simple bug fix is turning into a lot more to-and-fro than I would
> have anticipated.
It's nice example of evolution, isn't it? :)
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Fri, 2010-01-29 at 10:39 +0100, Jiri Kosina wrote:
> On Thu, 28 Jan 2010, Gunn, Brian wrote:
>
> > > > > I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT |
> > > > > HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
> > > >
> > > > Actually for my application it doesn't. I use HIDP_TRANS_DATA |
> > > HIDP_DATA_RTYPE_OUTPUT.
> > > >
> > > > Perhaps allowing the user the choice of what do to here is more flexible.
> > >
> > > Except that it would mean different code for USB and Bluetooth
> > > versions...
> >
> > So do we need a method of setting which device reports we're sending before writing them?
>
> Having separate methods for feature and output reports should be
> sufficient and general enough, right?
Most likely, yes. Feature is enough for the 2 devices I provided patches
for. Could you please comment on what you'd like the implementation of
this to look like? This simple bug fix is turning into a lot more
to-and-fro than I would have anticipated.
Cheers
On Thu, 28 Jan 2010, Gunn, Brian wrote:
> > > > I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT |
> > > > HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
> > >
> > > Actually for my application it doesn't. I use HIDP_TRANS_DATA |
> > HIDP_DATA_RTYPE_OUTPUT.
> > >
> > > Perhaps allowing the user the choice of what do to here is more flexible.
> >
> > Except that it would mean different code for USB and Bluetooth
> > versions...
>
> So do we need a method of setting which device reports we're sending before writing them?
Having separate methods for feature and output reports should be
sufficient and general enough, right?
--
Jiri Kosina
SUSE Labs, Novell Inc.
> > > I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT =
|
> > > HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
> >
> > Actually for my application it doesn't. I use HIDP_TRANS_DATA |
> HIDP_DATA_RTYPE_OUTPUT.
> >
> > Perhaps allowing the user the choice of what do to here is more flexibl=
e.
>
> Except that it would mean different code for USB and Bluetooth
> versions...
So do we need a method of setting which device reports we're sending before=
writing them?
Brian
On Thu, 2010-01-28 at 14:33 -0800, Gunn, Brian wrote:
> > > I just tested this patch with my device and it does work. I had to
> > > slightly alter the data I send (had to add 0xA2 to the front of it),
> > > but then it does work.
> >
> > I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT |
> > HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
>
> Actually for my application it doesn't. I use HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUTPUT.
>
> Perhaps allowing the user the choice of what do to here is more flexible.
Except that it would mean different code for USB and Bluetooth
versions...
> > I just tested this patch with my device and it does work. I had to
> > slightly alter the data I send (had to add 0xA2 to the front of it),
> > but then it does work.
>
> I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT |
> HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
Actually for my application it doesn't. I use HIDP_TRANS_DATA | HIDP_DATA_=
RTYPE_OUTPUT.
Perhaps allowing the user the choice of what do to here is more flexible.
Brian
On Wed, 2010-01-20 at 17:08 -0800, Gunn, Brian wrote:
> Bastien Nocera wrote:
> > The patch was written by Jiri and tested by Brian. Not sure what sort of
> > device Brian tested this with...
>
> I just tested this patch with my device and it does work. I had to
> slightly alter the data I send (had to add 0xA2 to the front of it),
> but then it does work.
>
> In my initial attempts, it does seem to work a little worse, but that
> is probably because of the device I'm using, which is still under
> development.
I sent a new version of the patch, which uses "HIDP_TRANS_SET_REPORT |
HIDP_DATA_RTYPE_FEATURE" for the first byte. Would that work?
Bastien Nocera wrote:
> The patch was written by Jiri and tested by Brian. Not sure what sort of
> device Brian tested this with...
I just tested this patch with my device and it does work. I had to slightl=
y alter the data I send (had to add 0xA2 to the front of it), but then it d=
oes work.
In my initial attempts, it does seem to work a little worse, but that is pr=
obably because of the device I'm using, which is still under development.
Brian
Hi Bastien,
> Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
> kernel. In the past, it was done in a patch in bluetootd.
>
> - 0001-hidp-Use-the-control-socket-for-raw-messages.patch
>
> As discussed, now uses the ctrl channel for sending raw reports to the device.
>
> - 0002-hid-wacom-Implement-Wacom-quirk-in-the-kernel.patch
>
> Uses the above to poke at the device and turn it into mode2
>
> - 0001-hid-sony-Fix-typo-in-error-message.patch
>
> Small typo, for the input tree
>
> - 0001-hid-Enable-Sixaxis-controller-over-Bluetooth-as-well.patch
>
> Use the hidp patch above to enable the sixaxis controller
please send them one patch per email and send them inline. In case of
troubles just use git send-email.
Regards
Marcel
Brian Gunn wrote:
>I used a custom remote control under development by the company I'm contracted to.
>We have custom HID output reports that trigger actions on the remote.
>I was able to send the output reports and get the behavior I expected on the remote.
>I have no faith that the company developing the remote implemented their side
>correctly and would like to try it with the control socket instead.
BTW: The Sony PS3 controller requires sending of HID reports as well. I just checked the one implementation of this I've seen (input/device.c in the BlueZ source) and I see that it uses the control socket.
Once we get implementation of sending HID reports to the control socket implemented elsewhere, this kludge can be removed from the BlueZ source.
Brian
> Bastien Nocera wrote:
>On Tue, 2010-01-19 at 08:36 -0800, Gunn, Brian wrote:
>> I tested this with a Bluetooth remote control. As I understand,
>> reports can be written to either the interrupt or control sockets.
>> I'm happy to test other changes to let this be selectable.
>Could you expand slightly on that? What make/model? What sort of tests
>did you do on it?
I used a custom remote control under development by the company I'm contrac=
ted to. We have custom HID output reports that trigger actions on the remo=
te. I was able to send the output reports and get the behavior I expected =
on the remote. I have no faith that the company developing the remote impl=
emented their side correctly and would like to try it with the control sock=
et instead.
Brian
On Tue, 2010-01-19 at 08:36 -0800, Gunn, Brian wrote:
> I tested this with a Bluetooth remote control. As I understand,
> reports can be written to either the interrupt or control sockets.
> I'm happy to test other changes to let this be selectable.
Could you expand slightly on that? What make/model? What sort of tests
did you do on it?
Cheers
I tested this with a Bluetooth remote control. As I understand, reports ca=
n be written to either the interrupt or control sockets. I'm happy to test=
other changes to let this be selectable.
Brian
________________________________________
From: Bastien Nocera [[email protected]]
Sent: Tuesday, January 19, 2010 2:30 AM
To: Marcel Holtmann
Cc: Dmitry Torokhov; linux-input; BlueZ development; Gunn, Brian; Ping; Jir=
i Kosina
Subject: Re: [PATCHes] Implement Bluetooth Wacom tablet's mode change in th=
e kernel
On Mon, 2010-01-18 at 23:29 -0800, Marcel Holtmann wrote:
> Hi Bastien,
>
> > Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
> > kernel. In the past, it was done in a patch in bluetootd.
> >
> > The first patch is probably completely wrong. Right now,
> > hid_output_raw_report is done on the intr socket, instead of the ctrl
> > socket. If it's correct to do it on the intr socket, we'd need to add
> > some API as a way to select the ctrl socket instead for use in that
> > driver.
>
> actually the interrupt should be incoming only. So moving the raw output
> to the control channel seems fine to. Any reason why it is on the
> interrupt channel in the first place?
The patch was written by Jiri and tested by Brian. Not sure what sort of
device Brian tested this with...
On Tue, 19 Jan 2010, Bastien Nocera wrote:
> On Mon, 2010-01-18 at 23:29 -0800, Marcel Holtmann wrote:
> > Hi Bastien,
> >
> > > Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
> > > kernel. In the past, it was done in a patch in bluetootd.
> > >
> > > The first patch is probably completely wrong. Right now,
> > > hid_output_raw_report is done on the intr socket, instead of the ctrl
> > > socket. If it's correct to do it on the intr socket, we'd need to add
> > > some API as a way to select the ctrl socket instead for use in that
> > > driver.
> >
> > actually the interrupt should be incoming only. So moving the raw output
> > to the control channel seems fine to. Any reason why it is on the
> > interrupt channel in the first place?
>
> The patch was written by Jiri and tested by Brian. Not sure what sort of
> device Brian tested this with...
Hmm, that's right, ctrl pipe would make much more sense (and this is what
we do in USB driver as well). I used intr pipe by mistake, but it seems
that it made things work for Brian anyway, which is quite puzzling.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Mon, 2010-01-18 at 23:29 -0800, Marcel Holtmann wrote:
> Hi Bastien,
>
> > Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
> > kernel. In the past, it was done in a patch in bluetootd.
> >
> > The first patch is probably completely wrong. Right now,
> > hid_output_raw_report is done on the intr socket, instead of the ctrl
> > socket. If it's correct to do it on the intr socket, we'd need to add
> > some API as a way to select the ctrl socket instead for use in that
> > driver.
>
> actually the interrupt should be incoming only. So moving the raw output
> to the control channel seems fine to. Any reason why it is on the
> interrupt channel in the first place?
The patch was written by Jiri and tested by Brian. Not sure what sort of
device Brian tested this with...
Hi Bastien,
> Here's a patch to do the Bluetooth Wacom tablet's mode setting in the
> kernel. In the past, it was done in a patch in bluetootd.
>
> The first patch is probably completely wrong. Right now,
> hid_output_raw_report is done on the intr socket, instead of the ctrl
> socket. If it's correct to do it on the intr socket, we'd need to add
> some API as a way to select the ctrl socket instead for use in that
> driver.
actually the interrupt should be incoming only. So moving the raw output
to the control channel seems fine to. Any reason why it is on the
interrupt channel in the first place?
Regards
Marcel
On Mon, 2010-01-18 at 21:35 +0000, Bastien Nocera wrote:
<snip>
> > I still have the exact same problems as with the user-space
> > patch in
> > that an error occurs sending the second blob of data to the
> > tablet, the
> > first time the driver is initialised. Ping, any ideas about
> > that?
> >
> > I am not sure if your issue is similar to what we see with USB devices
> > or not. In order to properly set the device to Wacom mode, we had to
> > call usb_set_report more than once (up to 5 times for older kernels,
> > and up to 3 times for newer kernels) to get it right. Please call:
> >
> > hdev->hid_output_raw_report again when ret is less than 0. Try up to
> > three times to see what you get.
>
> That's pretty straight forward, I'll try and do that.
My brain must have started turning to mush.
It's hid_parse() failing, and calling it multiple times doesn't have any
effects (unlike on USB where it would poke the device repeatedly).
Marcel, any ideas about that?
Adding a few loops for the mode2 switching fixed very unfrequent
failures. All that's left is the hid_parse() failure the first time the
device connects.
> > You need to check if (ret < 0) before calling dev_err() with the
> > second hid_output_raw_report() call, right?
>
> Copy/paste stupidness all around there (as you can see, it's indented
> properly already ;)
Fixed in the attached patch. Obviously still requires the first patch to
be reviewed.
Cheers
Dnia 2010-01-18, pon o godzinie 16:49 +0000, Bastien Nocera pisze:
[..]
My tablet has a really big lag when I'm using hi speed (patch 0002*).
Does it work fine for you?
My laptop is rather slow, but I'm not sure if that's the bottleneck.
[przemo@pldmachine Desktop]$ uname -a
Linux pldmachine 2.6.33-rc4-00209-gd63f9cd-dirty #7 Mon Jan 18 20:45:27
GMT 2010 i686 Intel(R)_Pentium(R)_M_processor_1.86GHz PLD Linux
--
Cheers,
Przemo
On Mon, 2010-01-18 at 11:44 -0800, Ping Cheng wrote:
> On Mon, Jan 18, 2010 at 8:49 AM, Bastien Nocera <[email protected]>
> wrote:
> Heya,
>
> Here's a patch to do the Bluetooth Wacom tablet's mode setting
> in the
> kernel. In the past, it was done in a patch in bluetootd.
>
> The first patch is probably completely wrong. Right now,
> hid_output_raw_report is done on the intr socket, instead of
> the ctrl
> socket. If it's correct to do it on the intr socket, we'd need
> to add
> some API as a way to select the ctrl socket instead for use in
> that
> driver.
>
> I can not make a comment on the above patch since I know nothing about
> Bluetooth.
I'll let Marcel and Jiri be the judges of the value of this patch. In
the worst case, it'll need a change of API for hid-core.
> I still have the exact same problems as with the user-space
> patch in
> that an error occurs sending the second blob of data to the
> tablet, the
> first time the driver is initialised. Ping, any ideas about
> that?
>
> I am not sure if your issue is similar to what we see with USB devices
> or not. In order to properly set the device to Wacom mode, we had to
> call usb_set_report more than once (up to 5 times for older kernels,
> and up to 3 times for newer kernels) to get it right. Please call:
>
> hdev->hid_output_raw_report again when ret is less than 0. Try up to
> three times to see what you get.
That's pretty straight forward, I'll try and do that.
> You need to check if (ret < 0) before calling dev_err() with the
> second hid_output_raw_report() call, right?
Copy/paste stupidness all around there (as you can see, it's indented
properly already ;)
Cheers