[PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers

[PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers

[Lauri Kasanen]

Without this, my "Gasia Co.,Ltd PS(R) Gamepad" would not send
any events. Now everything works including the leds.

Based on work by Andrew Haines and Antonio Ospite.

cc: Antonio Ospite <ao2@ao2.it>
cc: Andrew Haines <AndrewD207@aol.com>
Signed-off-by: Lauri Kasanen <cand@gmx.com>
---
 drivers/hid/hid-sony.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Antonio: size 8 works.

v4:
- use size 8

v3:
- reuse the dummy heap buffer
- skip later checks if the earlier ones fail

v2:
- edited error messages
- use output_report

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 31e9d25..f3d44e5 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1139,9 +1139,29 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
 				 HID_REQ_GET_REPORT);
 
+	if (ret < 0) {
+		hid_err(hdev, "can't set operational mode: step 1\n");
+		goto out;
+	}
+
+	/*
+	 * Some compatible controllers like the Speedlink Strike FX and
+	 * Gasia need another query plus an USB interrupt to get operational.
+	 */
+	ret = hid_hw_raw_request(hdev, 0xf5, buf, 8, HID_FEATURE_REPORT,
+				 HID_REQ_GET_REPORT);
+
+	if (ret < 0) {
+		hid_err(hdev, "can't set operational mode: step 2\n");
+		goto out;
+	}
+
+	ret = hid_hw_output_report(hdev, buf, 1);
+
 	if (ret < 0)
-		hid_err(hdev, "can't set operational mode\n");
+		hid_err(hdev, "can't set operational mode: step 3\n");
 
+out:
 	kfree(buf);
 
 	return ret;
-- 
1.8.3.1

Re: [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers

[Antonio Ospite]

On Mon, 16 Feb 2015 15:06:59 +0200
Lauri Kasanen <cand@gmx.com> wrote:

> Without this, my "Gasia Co.,Ltd PS(R) Gamepad" would not send
> any events. Now everything works including the leds.
> 
> Based on work by Andrew Haines and Antonio Ospite.
> 
> cc: Antonio Ospite <ao2@ao2.it>
> cc: Andrew Haines <AndrewD207@aol.com>
> Signed-off-by: Lauri Kasanen <cand@gmx.com>

Reviewed-by: Antonio Ospite <ao2@ao2.it>

Original controllers still work fine after this change.
Some follow-up patches are on their way.

Thanks,
   Antonio

> ---
>  drivers/hid/hid-sony.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> Antonio: size 8 works.
> 
> v4:
> - use size 8
> 
> v3:
> - reuse the dummy heap buffer
> - skip later checks if the earlier ones fail
> 
> v2:
> - edited error messages
> - use output_report
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 31e9d25..f3d44e5 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -1139,9 +1139,29 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  	ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
>  				 HID_REQ_GET_REPORT);
>  
> +	if (ret < 0) {
> +		hid_err(hdev, "can't set operational mode: step 1\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Some compatible controllers like the Speedlink Strike FX and
> +	 * Gasia need another query plus an USB interrupt to get operational.
> +	 */
> +	ret = hid_hw_raw_request(hdev, 0xf5, buf, 8, HID_FEATURE_REPORT,
> +				 HID_REQ_GET_REPORT);
> +
> +	if (ret < 0) {
> +		hid_err(hdev, "can't set operational mode: step 2\n");
> +		goto out;
> +	}
> +
> +	ret = hid_hw_output_report(hdev, buf, 1);
> +
>  	if (ret < 0)
> -		hid_err(hdev, "can't set operational mode\n");
> +		hid_err(hdev, "can't set operational mode: step 3\n");
>  
> +out:
>  	kfree(buf);
>  
>  	return ret;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
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?

Re: [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers

[Jiri Kosina]

On Mon, 16 Feb 2015, Antonio Ospite wrote:

> On Mon, 16 Feb 2015 15:06:59 +0200
> Lauri Kasanen <cand@gmx.com> wrote:
> 
> > Without this, my "Gasia Co.,Ltd PS(R) Gamepad" would not send
> > any events. Now everything works including the leds.
> > 
> > Based on work by Andrew Haines and Antonio Ospite.
> > 
> > cc: Antonio Ospite <ao2@ao2.it>
> > cc: Andrew Haines <AndrewD207@aol.com>
> > Signed-off-by: Lauri Kasanen <cand@gmx.com>
> 
> Reviewed-by: Antonio Ospite <ao2@ao2.it>

Applied.

-- 
Jiri Kosina
SUSE Labs

[PATCH 0/5] HID: sony: misc fixes and cleanups

[Antonio Ospite]

Hi,

here are a few cleanups for hid-sony.

Changes are meant to follow the patch titled:
[PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers

Frank, hi, I'd like to have your explicit ack at least for patch 1 and
2.

Thanks,
   Antonio


Antonio Ospite (5):
  HID: sony: Fix a WARNING shown when rmmod-ing the driver
  HID: sony: Use the minimum accepted size for feature report 0xf2
  HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()
  HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()
  HID: sony: Coding style cleanups in sixaxis_set_operational_usb()

 drivers/hid/hid-sony.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
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?

[PATCH 2/5] HID: sony: Use the minimum accepted size for feature report 0xf2

[Antonio Ospite]

Sixaxis devices accept feature report 0xf2 when size is >= 17, not 18.
Use the minimum accepted size.

The change is mainly for documentation purposes, the code worked fine
even before this change.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/hid/hid-sony.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 137d1ee..303335b 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -802,7 +802,7 @@ union sixaxis_output_report_01 {
 #define DS4_REPORT_0x05_SIZE 32
 #define DS4_REPORT_0x11_SIZE 78
 #define DS4_REPORT_0x81_SIZE 7
-#define SIXAXIS_REPORT_0xF2_SIZE 18
+#define SIXAXIS_REPORT_0xF2_SIZE 17
 
 static spinlock_t sony_dev_list_lock;
 static LIST_HEAD(sony_device_list);
-- 
2.1.4

[PATCH 3/5] HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()

[Antonio Ospite]

Remove the magic numbers used in sixaxis_set_operational_usb():
  - use the already defined SIXAXIS_REPORT_0xF2_SIZE;
  - define and use SIXAXIS_REPORT_0xF5_SIZE;
  - set the dummy buffer size to accommodate any report that is going to
    be requested.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/hid/hid-sony.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 303335b..a419868 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -803,6 +803,7 @@ union sixaxis_output_report_01 {
 #define DS4_REPORT_0x11_SIZE 78
 #define DS4_REPORT_0x81_SIZE 7
 #define SIXAXIS_REPORT_0xF2_SIZE 17
+#define SIXAXIS_REPORT_0xF5_SIZE 8
 
 static spinlock_t sony_dev_list_lock;
 static LIST_HEAD(sony_device_list);
@@ -1131,13 +1132,15 @@ static void sony_input_configured(struct hid_device *hdev,
 static int sixaxis_set_operational_usb(struct hid_device *hdev)
 {
 	int ret;
-	char *buf = kmalloc(18, GFP_KERNEL);
+	const int buf_size =
+		max(SIXAXIS_REPORT_0xF2_SIZE, SIXAXIS_REPORT_0xF5_SIZE);
+	char *buf = kmalloc(buf_size, GFP_KERNEL);
 
 	if (!buf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
-				 HID_REQ_GET_REPORT);
+	ret = hid_hw_raw_request(hdev, 0xf2, buf, SIXAXIS_REPORT_0xF2_SIZE,
+				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
 
 	if (ret < 0) {
 		hid_err(hdev, "can't set operational mode: step 1\n");
@@ -1148,8 +1151,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	 * Some compatible controllers like the Speedlink Strike FX and
 	 * Gasia need another query plus an USB interrupt to get operational.
 	 */
-	ret = hid_hw_raw_request(hdev, 0xf5, buf, 8, HID_FEATURE_REPORT,
-				 HID_REQ_GET_REPORT);
+	ret = hid_hw_raw_request(hdev, 0xf5, buf, SIXAXIS_REPORT_0xF5_SIZE,
+				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
 
 	if (ret < 0) {
 		hid_err(hdev, "can't set operational mode: step 2\n");
-- 
2.1.4

[PATCH 4/5] HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()

[Antonio Ospite]

Use the same type declared in the prototypes of functions that are going
to accept the buffer as parameter.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/hid/hid-sony.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a419868..4a50e65 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1134,7 +1134,7 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	int ret;
 	const int buf_size =
 		max(SIXAXIS_REPORT_0xF2_SIZE, SIXAXIS_REPORT_0xF5_SIZE);
-	char *buf = kmalloc(buf_size, GFP_KERNEL);
+	__u8 *buf = kmalloc(buf_size, GFP_KERNEL);
 
 	if (!buf)
 		return -ENOMEM;
-- 
2.1.4

[PATCH 5/5] HID: sony: Coding style cleanups in sixaxis_set_operational_usb()

[Antonio Ospite]

Don't mix declaration and allocation, remove some useless newlines
between calling a function and checking its return value.

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/hid/hid-sony.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 4a50e65..9e0987d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1131,17 +1131,17 @@ static void sony_input_configured(struct hid_device *hdev,
  */
 static int sixaxis_set_operational_usb(struct hid_device *hdev)
 {
-	int ret;
 	const int buf_size =
 		max(SIXAXIS_REPORT_0xF2_SIZE, SIXAXIS_REPORT_0xF5_SIZE);
-	__u8 *buf = kmalloc(buf_size, GFP_KERNEL);
+	__u8 *buf;
+	int ret;
 
+	buf = kmalloc(buf_size, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
 	ret = hid_hw_raw_request(hdev, 0xf2, buf, SIXAXIS_REPORT_0xF2_SIZE,
 				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
-
 	if (ret < 0) {
 		hid_err(hdev, "can't set operational mode: step 1\n");
 		goto out;
@@ -1153,14 +1153,12 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	 */
 	ret = hid_hw_raw_request(hdev, 0xf5, buf, SIXAXIS_REPORT_0xF5_SIZE,
 				 HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
-
 	if (ret < 0) {
 		hid_err(hdev, "can't set operational mode: step 2\n");
 		goto out;
 	}
 
 	ret = hid_hw_output_report(hdev, buf, 1);
-
 	if (ret < 0)
 		hid_err(hdev, "can't set operational mode: step 3\n");
 
-- 
2.1.4

Re: [PATCH 0/5] HID: sony: misc fixes and cleanups

[Frank Praznik]

On 2/16/2015 12:12, Antonio Ospite wrote:
> Hi,
>
> here are a few cleanups for hid-sony.
>
> Changes are meant to follow the patch titled:
> [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers
>
> Frank, hi, I'd like to have your explicit ack at least for patch 1 and
> 2.
>
> Thanks,
>     Antonio
>
>
> Antonio Ospite (5):
>    HID: sony: Fix a WARNING shown when rmmod-ing the driver
>    HID: sony: Use the minimum accepted size for feature report 0xf2
>    HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()
>    HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()
>    HID: sony: Coding style cleanups in sixaxis_set_operational_usb()
>
>   drivers/hid/hid-sony.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
Hi Antonio, could you resend patch #1?  It didn't show up in my email 
and it's not on any of the web-based lists either.

Regards,
Frank

Re: [PATCH 0/5] HID: sony: misc fixes and cleanups

[Antonio Ospite]

On Mon, 16 Feb 2015 16:48:53 -0500
Frank Praznik <frank.praznik@gmail.com> wrote:

> On 2/16/2015 12:12, Antonio Ospite wrote:
> > Hi,
> >
> > here are a few cleanups for hid-sony.
> >
> > Changes are meant to follow the patch titled:
> > [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers
> >
> > Frank, hi, I'd like to have your explicit ack at least for patch 1 and
> > 2.
> >
> > Thanks,
> >     Antonio
> >
> >
> > Antonio Ospite (5):
> >    HID: sony: Fix a WARNING shown when rmmod-ing the driver
> >    HID: sony: Use the minimum accepted size for feature report 0xf2
> >    HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()
> >    HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()
> >    HID: sony: Coding style cleanups in sixaxis_set_operational_usb()
> >
> >   drivers/hid/hid-sony.c | 21 +++++++++++----------
> >   1 file changed, 11 insertions(+), 10 deletions(-)
> >
> Hi Antonio, could you resend patch #1?  It didn't show up in my email 
> and it's not on any of the web-based lists either.
> 

Done. Let me know if you didn't get it this time either.

I noticed that git-send-email set a later date, it shouldn't be a
problem but if it is patch 1/5 could be applied as 6/5 instead.

Thanks,
   Antonio

-- 
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?

[PATCH 1/5] HID: sony: Fix a WARNING shown when rmmod-ing the driver

[Antonio Ospite]

ida_destroy() must be called _after_ all the devices have been
unregistered; otherwise, when calling "rmmod hid_sony" with devices
still plugged in, the following warning would show up because of calls
to ida_simple_remove() on a destroyed ID allocator:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 5509 at lib/idr.c:1052 ida_simple_remove+0x26/0x50()
ida_remove called for id=0 which is not allocated.
Modules linked in: ...
CPU: 0 PID: 5509 Comm: rmmod Not tainted 3.19.0-rc6-ao2 #35
Hardware name: System manufacturer System Product Name/M2N-MX SE, BIOS 0501    03/20/2008
 0000000000000000 ffffffff8176320d ffffffff815b3a88 ffff880036f7fdd8
 ffffffff8106ce01 0000000000000000 ffffffffa07658e0 0000000000000246
 ffff88005077d8b8 ffff88005077d8d0 ffffffff8106ce7a ffffffff81763260
Call Trace:
 [<ffffffff815b3a88>] ? dump_stack+0x40/0x50
 [<ffffffff8106ce01>] ? warn_slowpath_common+0x81/0xb0
 [<ffffffff8106ce7a>] ? warn_slowpath_fmt+0x4a/0x50
 [<ffffffff812ccb86>] ? ida_simple_remove+0x26/0x50
 [<ffffffffa0762dc8>] ? sony_remove+0x58/0xe0 [hid_sony]
 [<ffffffffa00fff15>] ? hid_device_remove+0x65/0xd0 [hid]
 [<ffffffff8140425e>] ? __device_release_driver+0x7e/0x100
 [<ffffffff81404c70>] ? driver_detach+0xa0/0xb0
 [<ffffffff81403ee5>] ? bus_remove_driver+0x55/0xe0
 [<ffffffffa01000ff>] ? hid_unregister_driver+0x2f/0xa0 [hid]
 [<ffffffff810e45bf>] ? SyS_delete_module+0x1bf/0x270
 [<ffffffff81014089>] ? do_notify_resume+0x69/0xa0
 [<ffffffff815b952d>] ? system_call_fastpath+0x16/0x1b
---[ end trace bc794b3d22c30ede ]---

Signed-off-by: Antonio Ospite <ao2@ao2.it>
---
 drivers/hid/hid-sony.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index f3d44e5..137d1ee 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2167,8 +2167,8 @@ static void __exit sony_exit(void)
 {
 	dbg_hid("Sony:%s\n", __func__);
 
-	ida_destroy(&sony_device_id_allocator);
 	hid_unregister_driver(&sony_driver);
+	ida_destroy(&sony_device_id_allocator);
 }
 module_init(sony_init);
 module_exit(sony_exit);
-- 
2.1.4

Re: [PATCH 0/5] HID: sony: misc fixes and cleanups

[Frank Praznik]

On 2/16/2015 12:12, Antonio Ospite wrote:
> Hi,
>
> here are a few cleanups for hid-sony.
>
> Changes are meant to follow the patch titled:
> [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers
>
> Frank, hi, I'd like to have your explicit ack at least for patch 1 and
> 2.
>
> Thanks,
>     Antonio
>
>
> Antonio Ospite (5):
>    HID: sony: Fix a WARNING shown when rmmod-ing the driver
>    HID: sony: Use the minimum accepted size for feature report 0xf2
>    HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()
>    HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()
>    HID: sony: Coding style cleanups in sixaxis_set_operational_usb()
>
>   drivers/hid/hid-sony.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>

The series looks good to me.  Thanks for tackling the last bit of magic 
constant cleanup.
For the entire series:

Acked-by: Frank Praznik <frank.praznik@oh.rr.com>

Re: [PATCH 0/5] HID: sony: misc fixes and cleanups

[Jiri Kosina]

On Mon, 16 Feb 2015, Antonio Ospite wrote:

> Hi,
> 
> here are a few cleanups for hid-sony.
> 
> Changes are meant to follow the patch titled:
> [PATCHv4] HID: sony: Enable Gasia third-party PS3 controllers
> 
> Frank, hi, I'd like to have your explicit ack at least for patch 1 and
> 2.
> 
> Thanks,
>    Antonio
> 
> 
> Antonio Ospite (5):
>   HID: sony: Fix a WARNING shown when rmmod-ing the driver
>   HID: sony: Use the minimum accepted size for feature report 0xf2
>   HID: sony: Don't use magic numbers in sixaxis_set_operational_usb()
>   HID: sony: Use __u8 * for the buffer in sixaxis_set_operational_usb()
>   HID: sony: Coding style cleanups in sixaxis_set_operational_usb()

I have applied 1/5 to for-3.20/upstream-fixes and 2-5 to for-3.21/sony. 
Thanks,

-- 
Jiri Kosina
SUSE Labs