[PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[Frank Praznik]

This adds a special case in the HIDP core for the Dualshock 4 controller.
The controller only recognizes output reports with the report type 0x52 and 
only accepts reports sent via the ctrl channel. 

Unfortunately, this was the only way I could send data to the controller. I
looked around for alternatives, but adding a special case to the HIDP system
seemed to be the only way. If there is a way to set a custom report type and 
specify the channel without polluting the core with a device specific quirk 
I'll gladly use it.

Re: [PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[simon]

[-- Attachment #1: Type: text/plain, Size: 987 bytes --]

> This adds a special case in the HIDP core for the Dualshock 4
controller.
> The controller only recognizes output reports with the report type 0x52
and only accepts reports sent via the ctrl channel.

Which part of the system is 'at fault' here, is it simply the DS4 behaving
incorrectly or that Bluez is not picking up some data or that 'we' are
just the incorrect method to send the data (in the hid-sony kernel
driver)?


I noticed that the BT HID spec notes a 'HID control' channel in the
example descriptor in table 5.3.3 as 'Parameter 0'.

The DS4 gives this in it's SDP report
--
	Attribute 0x0004 - ProtocolDescriptorList
		Sequence
			Sequence
				UUID16 0x0100 - L2CAP
				UINT16 0x0011 <---------- 'HDI Control' PSM 17
			Sequence
				UUID16 0x0011 - HIDP
--

Shouldn't Bluez be remembering this and sending accesses to this PSM in
the appropriate mode?

Just looking to fix the problem where it really exists, without just
working around specifically for the DS4.

Simon



[-- Attachment #2: records_raw.txt --]
[-- Type: text/plain, Size: 3035 bytes --]

Sequence
	Attribute 0x0000 - ServiceRecordHandle
		UINT32 0x00010001
	Attribute 0x0001 - ServiceClassIDList
		Sequence
			UUID16 0x1124 - HumanInterfaceDeviceService (HID)
	Attribute 0x0004 - ProtocolDescriptorList
		Sequence
			Sequence
				UUID16 0x0100 - L2CAP
				UINT16 0x0011
			Sequence
				UUID16 0x0011 - HIDP
	Attribute 0x0006 - LanguageBaseAttributeIDList
		Sequence
			UINT16 0x656e
			UINT16 0x006a
			UINT16 0x0100
	Attribute 0x0009 - BluetoothProfileDescriptorList
		Sequence
			Sequence
				UUID16 0x1124 - HumanInterfaceDeviceService (HID)
				UINT16 0x0100
	Attribute 0x000d - AdditionalProtocolDescriptorLists
		Sequence
			Sequence
				Sequence
					UUID16 0x0100 - L2CAP
					UINT16 0x0013
				Sequence
					UUID16 0x0011 - HIDP
	Attribute 0x0100
		String Wireless Controller\0
	Attribute 0x0101
		String Game Controller\0
	Attribute 0x0102
		String Sony Computer Entertainment\0
	Attribute 0x0200				<===== ?????
		UINT16 0x0100
	Attribute 0x0201				<===== HIDParserVersion 
		UINT16 0x0111
	Attribute 0x0202				<===== HIDDeviceSubclass 
		UINT8 0x08
	Attribute 0x0203				<===== HIDCountryCode 
		UINT8 0x00
	Attribute 0x0204				<===== HIDVirtualCable
		Bool False
	Attribute 0x0205				<===== HIDReconnectInitiate
		Bool True
	Attribute 0x0206				<===== HIDDescriptorList
		Sequence
			Sequence
				UINT8 0x22
				Data 05 01 09 05 a1 01 85 01 09 30 09 31 09 32 09 35 15 00 26 ff 00 75 08 95 04 81 02 09 39 15 00 25 07 75 04 95 01 81 42 05 09 19 01 29 0e 15 00 25 01 75 01 95 0e 81 02 75 06 95 01 81 01 05 01 09 33 09 34 15 00 26 ff 00 75 08 95 02 81 02 06 04 ff 85 02 09 24 95 24 b1 02 85 a3 09 25 95 30 b1 02 85 05 09 26 95 28 b1 02 85 06 09 27 95 34 b1 02 85 07 09 28 95 30 b1 02 85 08 09 29 95 2f b1 02 06 03 ff 85 03 09 21 95 26 b1 02 85 04 09 22 95 2e b1 02 85 f0 09 47 95 3f b1 02 85 f1 09 48 95 3f b1 02 85 f2 09 49 95 0f b1 02 06 00 ff 85 11 09 20 15 00 26 ff 00 75 08 95 4d 81 02 09 21 91 02 85 12 09 22 95 8d 81 02 09 23 91 02 85 13 09 24 95 cd 81 02 09 25 91 02 85 14 09 26 96 0d 01 81 02 09 27 91 02 85 15 09 28 96 4d 01 81 02 09 29 91 02 85 16 09 2a 96 8d 01 81 02 09 2b 91 02 85 17 09 2c 96 cd 01 81 02 09 2d 91 02 85 18 09 2e 96 0d 02 81 02 09 2f 91 02 85 19 09 30 96 22 02 81 02 09 31 91 02 06 80 ff 85 82 09 22 95 3f b1 02 85 83 09 23 b1 02 85 84 09 24 b1 02 85 90 09 30 b1 02 85 91 09 31 b1 02 85 92 09 32 b1 02 85 93 09 33 b1 02 85 a0 09 40 b1 02 85 a4 09 44 b1 02 c0 00
Sequence
	Attribute 0x0000 - ServiceRecordHandle
		UINT32 0x00010002
	Attribute 0x0001 - ServiceClassIDList
		Sequence
			UUID16 0x1200 - PnPInformation
	Attribute 0x0004 - ProtocolDescriptorList
		Sequence
			Sequence
				UUID16 0x0100 - L2CAP
				UINT16 0x0001
			Sequence
				UUID16 0x0001 - SDP
	Attribute 0x0009 - BluetoothProfileDescriptorList
		Sequence
			Sequence
				UUID16 0x1200 - PnPInformation
				UINT16 0x0103
	Attribute 0x0200
		UINT16 0x0103
	Attribute 0x0201
		UINT16 0x054c
	Attribute 0x0202
		UINT16 0x05c4
	Attribute 0x0203
		UINT16 0x0100
	Attribute 0x0204
		Bool True
	Attribute 0x0205
		UINT16 0x0002

Re: [PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[David Herrmann]

Hi

On Tue, Jan 21, 2014 at 3:00 AM,  <simon@mungewell.org> wrote:
>> This adds a special case in the HIDP core for the Dualshock 4
> controller.
>> The controller only recognizes output reports with the report type 0x52
> and only accepts reports sent via the ctrl channel.
>
> Which part of the system is 'at fault' here, is it simply the DS4 behaving
> incorrectly or that Bluez is not picking up some data or that 'we' are
> just the incorrect method to send the data (in the hid-sony kernel
> driver)?

No-one is at fault. Well, strictly speaking the DS4 is, as it has to
accept SET_REPORT and asynchronous OUTPUT_REPORTs, but it doesn't.
That's quite common. What we actually want is HIDP to provide to
functions, one to call SET_REPORT and one to do the async
OUTPUT_REPORT is currently does.

I implemented this some time ago here:
  http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=hid

Maybe it's time to get that merged. But that hack here is ugly and not
the way to go.

Thanks
David

>
> I noticed that the BT HID spec notes a 'HID control' channel in the
> example descriptor in table 5.3.3 as 'Parameter 0'.
>
> The DS4 gives this in it's SDP report
> --
>         Attribute 0x0004 - ProtocolDescriptorList
>                 Sequence
>                         Sequence
>                                 UUID16 0x0100 - L2CAP
>                                 UINT16 0x0011 <---------- 'HDI Control' PSM 17
>                         Sequence
>                                 UUID16 0x0011 - HIDP
> --
>
> Shouldn't Bluez be remembering this and sending accesses to this PSM in
> the appropriate mode?
>
> Just looking to fix the problem where it really exists, without just
> working around specifically for the DS4.
>
> Simon
>
>

Re: [PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[Frank Praznik]

On 1/21/2014 03:26, David Herrmann wrote:
> Hi
>
> On Tue, Jan 21, 2014 at 3:00 AM,  <simon@mungewell.org> wrote:
>>> This adds a special case in the HIDP core for the Dualshock 4
>> controller.
>>> The controller only recognizes output reports with the report type 0x52
>> and only accepts reports sent via the ctrl channel.
>>
>> Which part of the system is 'at fault' here, is it simply the DS4 behaving
>> incorrectly or that Bluez is not picking up some data or that 'we' are
>> just the incorrect method to send the data (in the hid-sony kernel
>> driver)?
> No-one is at fault. Well, strictly speaking the DS4 is, as it has to
> accept SET_REPORT and asynchronous OUTPUT_REPORTs, but it doesn't.
> That's quite common. What we actually want is HIDP to provide to
> functions, one to call SET_REPORT and one to do the async
> OUTPUT_REPORT is currently does.
>
> I implemented this some time ago here:
>    http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=hid
>
> Maybe it's time to get that merged. But that hack here is ugly and not
> the way to go.
>
> Thanks
> David
Believe me, I know that my hack is ugly :).  The raw_request 
functionality in your repo is exactly what is needed in this scenario.  
I have the Bluetooth work on the Sony driver module done, so now it's 
just a matter of waiting on if or when this gets merged.

Re: [PATCH 0/1] HIDP: Add a special case for the Dualshock 4

[David Herrmann]

Hi

On Tue, Jan 21, 2014 at 5:01 PM, Frank Praznik <frank.praznik@gmail.com> wrote:
> On 1/21/2014 03:26, David Herrmann wrote:
>>
>> Hi
>>
>> On Tue, Jan 21, 2014 at 3:00 AM,  <simon@mungewell.org> wrote:
>>>>
>>>> This adds a special case in the HIDP core for the Dualshock 4
>>>
>>> controller.
>>>>
>>>> The controller only recognizes output reports with the report type 0x52
>>>
>>> and only accepts reports sent via the ctrl channel.
>>>
>>> Which part of the system is 'at fault' here, is it simply the DS4
>>> behaving
>>> incorrectly or that Bluez is not picking up some data or that 'we' are
>>> just the incorrect method to send the data (in the hid-sony kernel
>>> driver)?
>>
>> No-one is at fault. Well, strictly speaking the DS4 is, as it has to
>> accept SET_REPORT and asynchronous OUTPUT_REPORTs, but it doesn't.
>> That's quite common. What we actually want is HIDP to provide to
>> functions, one to call SET_REPORT and one to do the async
>> OUTPUT_REPORT is currently does.
>>
>> I implemented this some time ago here:
>>    http://cgit.freedesktop.org/~dvdhrm/linux/log/?h=hid
>>
>> Maybe it's time to get that merged. But that hack here is ugly and not
>> the way to go.
>>
>> Thanks
>> David
>
> Believe me, I know that my hack is ugly :).  The raw_request functionality
> in your repo is exactly what is needed in this scenario.  I have the
> Bluetooth work on the Sony driver module done, so now it's just a matter of
> waiting on if or when this gets merged.

My backlog is continuously growing.. it's not the only patch-series I
have pending for too long, I'm sincerely sorry. I'm working hard on
getting all that stuff out, but note that I will not be able to get
this ready before FOSDEM (in 2 weeks). So if you want to pick this up
before 3rd of February, I would be more than glad about it. I will
review any changes. Otherwise, you'd have to wait for at least 2 more
weeks before I can send it out.

Also feel free to extract any small part of the series if you don't
feel confident about the other stuff. So you can just move the
raw_request()/raw_report() into a separate patch. We *definitely* need
this callback, so I'd be fine if we add it early.

Cheers
David