[PATCH] Bluetooth: hidp: sixaxis dualshock report via ctrl quirk

[PATCH] Bluetooth: hidp: sixaxis dualshock report via ctrl quirk

[Arnaud Rebillout]

From: vivek <vivek@collabora.com>

The Sixaxis and Dualshock 4 wants report sent via the ctrl channel.

This patch has been used in the SteamOS kernel for a while now.
<https://github.com/ValveSoftware/steamos_kernel/commit/6e215b67a13c85d0cf5a9e0970acd9df7d2b77bd>

Signed-off-by: Vivek Das Mohapatra <vivek@collabora.com>
Signed-off-by: Arnaud Rebillout <arnaud.rebillout@collabora.com>
---
 net/bluetooth/hidp/core.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 1036e4fa1ea2..fa75da91a559 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -379,9 +379,16 @@ static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
 {
 	struct hidp_session *session = hid->driver_data;
 
-	return hidp_send_intr_message(session,
-				      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
-				      data, count);
+	/* The Sixaxis and Dualshock 4 wants report sent via the ctrl channel */
+	if (hid->vendor == 0x54c && (hid->product == 0x5c4 || hid->product == 0x268)) {
+		return hidp_send_ctrl_message(session,
+					      HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_OUPUT,
+					      data, count);
+	} else {
+		return hidp_send_intr_message(session,
+					      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+					      data, count);
+	}
 }
 
 static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
-- 
2.16.2

Re: [PATCH] Bluetooth: hidp: sixaxis dualshock report via ctrl quirk

[Bastien Nocera]

On Wed, 2018-03-28 at 10:45 +0700, Arnaud Rebillout wrote:
> From: vivek <vivek@collabora.com>
> 
> The Sixaxis and Dualshock 4 wants report sent via the ctrl channel.
> 
> This patch has been used in the SteamOS kernel for a while now.
> <https://github.com/ValveSoftware/steamos_kernel/commit/6e215b67a13c8
> 5d0cf5a9e0970acd9df7d2b77bd>

Given that both types of devices already work via Bluetooth, what is
this supposed to fix? The commit message will need work.

Re: [PATCH] Bluetooth: hidp: sixaxis dualshock report via ctrl quirk

[Marcel Holtmann]

Hi Arnaud,

> The Sixaxis and Dualshock 4 wants report sent via the ctrl channel.
> 
> This patch has been used in the SteamOS kernel for a while now.
> <https://github.com/ValveSoftware/steamos_kernel/commit/6e215b67a13c85d0cf5a9e0970acd9df7d2b77bd>
> 
> Signed-off-by: Vivek Das Mohapatra <vivek@collabora.com>
> Signed-off-by: Arnaud Rebillout <arnaud.rebillout@collabora.com>
> ---
> net/bluetooth/hidp/core.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 1036e4fa1ea2..fa75da91a559 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -379,9 +379,16 @@ static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
> {
> 	struct hidp_session *session = hid->driver_data;
> 
> -	return hidp_send_intr_message(session,
> -				      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
> -				      data, count);
> +	/* The Sixaxis and Dualshock 4 wants report sent via the ctrl channel */
> +	if (hid->vendor == 0x54c && (hid->product == 0x5c4 || hid->product == 0x268)) {
> +		return hidp_send_ctrl_message(session,
> +					      HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_OUPUT,
> +					      data, count);
> +	} else {
> +		return hidp_send_intr_message(session,
> +					      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
> +					      data, count);
> +	}
> }

see the comment from Bastien, the HIDP layer is just a transport layer and should really not need to know about device specific quirks. Seems these quirks could be easily handled one layer about where the right channel is selected.

Regards

Marcel

Re: [PATCH] Bluetooth: hidp: sixaxis dualshock report via ctrl quirk

[Arnaud Rebillout]

Hi Marcel,

On 03/28/2018 04:30 PM, Marcel Holtmann wrote:
> Hi Arnaud,
>
>> The Sixaxis and Dualshock 4 wants report sent via the ctrl channel.
>>
>> This patch has been used in the SteamOS kernel for a while now.
>> <https://github.com/ValveSoftware/steamos_kernel/commit/6e215b67a13c85d0cf5a9e0970acd9df7d2b77bd>
>>
>> Signed-off-by: Vivek Das Mohapatra <vivek@collabora.com>
>> Signed-off-by: Arnaud Rebillout <arnaud.rebillout@collabora.com>
>> ---
>> net/bluetooth/hidp/core.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> index 1036e4fa1ea2..fa75da91a559 100644
>> --- a/net/bluetooth/hidp/core.c
>> +++ b/net/bluetooth/hidp/core.c
>> @@ -379,9 +379,16 @@ static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
>> {
>> 	struct hidp_session *session = hid->driver_data;
>>
>> -	return hidp_send_intr_message(session,
>> -				      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>> -				      data, count);
>> +	/* The Sixaxis and Dualshock 4 wants report sent via the ctrl channel */
>> +	if (hid->vendor == 0x54c && (hid->product == 0x5c4 || hid->product == 0x268)) {
>> +		return hidp_send_ctrl_message(session,
>> +					      HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_OUPUT,
>> +					      data, count);
>> +	} else {
>> +		return hidp_send_intr_message(session,
>> +					      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
>> +					      data, count);
>> +	}
>> }
> see the comment from Bastien, the HIDP layer is just a transport layer and should really not need to know about device specific quirks. Seems these quirks could be easily handled one layer about where the right channel is selected.

I noticed indeed that the fix for the Sixaxis is already present in
`drivers/hid/hid-sony.c`. I'm not sure about the Dualshock 4, but I will
investigate further. If there's anything to fix I will patch `hid-sony.c`.

Thanks for the feedback, and sorry for the noise :)

  Arnaud