2014-01-21 02:00:26

by Simon Wood

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

> 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



Attachments:
records_raw.txt (2.96 kB)

2014-01-21 16:34:11

by David Herrmann

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

Hi

On Tue, Jan 21, 2014 at 5:01 PM, Frank Praznik <[email protected]> wrote:
> On 1/21/2014 03:26, David Herrmann wrote:
>>
>> Hi
>>
>> On Tue, Jan 21, 2014 at 3:00 AM, <[email protected]> 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

2014-01-21 16:01:14

by Frank Praznik

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

On 1/21/2014 03:26, David Herrmann wrote:
> Hi
>
> On Tue, Jan 21, 2014 at 3:00 AM, <[email protected]> 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.

2014-01-21 08:26:59

by David Herrmann

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

Hi

On Tue, Jan 21, 2014 at 3:00 AM, <[email protected]> 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
>
>