[PATCH 1/2] sony_ff_hid_descriptor

[PATCH 1/2] sony_ff_hid_descriptor

[Simon Wood]

This patch modifies the HID descriptor to allow the reporting of
the accelerometers and gyro via a joystick axis.
---
 drivers/hid/hid-sony.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 936c911..5c930dc 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -28,6 +28,12 @@
 #define SIXAXIS_CONTROLLER_USB  (1 << 1)
 #define SIXAXIS_CONTROLLER_BT   (1 << 2)
 
+static const u8 sixaxis_rdesc_fixup[] = {
+	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
+	0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF, 
+	0x03, 0x46, 0xFF, 0x03, 0x09, 0x01, 0x81, 0x02
+	};
+
 struct sony_sc {
 	unsigned long quirks;
 };
@@ -43,6 +49,11 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		hid_info(hdev, "Fixing up Sony Vaio VGX report descriptor\n");
 		rdesc[55] = 0x06;
 	}
+	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
+			*rsize == 148 && rdesc[83] == 0x75) {
+		hid_info(hdev, "Fixing up Sony Sixaxis report descriptor\n");
+		memcpy((void *)&rdesc[83], (void *) &sixaxis_rdesc_fixup, sizeof(sixaxis_rdesc_fixup));
+	}
 	return rdesc;
 }
 
-- 
1.7.4.1

[PATCH 2/2] sony_ff_byteswap_accelerometer

[Simon Wood]

The accelerometers/gyro on the sixaxis are reported in the wrong
endianness (ie. not compatible with HID), so this patch intercepts
the report and swaps the appropriate bytes over.

Accelerometers are scaled with a nominal value of +/-4000 = 1G,
maximum value would be around +/-32768 = 8G.

Gyro on my device always reports -32768, might need some calibration
set within the controller.
---
 drivers/hid/hid-sony.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 5c930dc..f219746 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -57,6 +57,24 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+/* Sixaxis HID report has acclerometers/gyro with MSByte first, this has
+ * to be BYTE_SWAPPED before passing up to joystick interface
+ */
+static int sony_raw_event(struct hid_device *hdev, struct hid_report *report, __u8 *rd, int size)
+{
+	struct sony_sc *sc = hid_get_drvdata(hdev);
+
+	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
+			rd[0] == 0x01 && size == 49) {
+		swap(rd[41], rd[42]);
+		swap(rd[43], rd[44]);
+		swap(rd[45], rd[46]);
+		swap(rd[47], rd[48]);
+	}
+
+	return 0;
+}
+
 /*
  * The Sony Sixaxis does not handle HID Output Reports on the Interrupt EP
  * like it should according to usbhid/hid-core.c::usbhid_output_raw_report()
@@ -205,6 +223,7 @@ static struct hid_driver sony_driver = {
 	.probe = sony_probe,
 	.remove = sony_remove,
 	.report_fixup = sony_report_fixup,
+	.raw_event = sony_raw_event
 };
 
 static int __init sony_init(void)
-- 
1.7.4.1

[PATCH 1/2] sony_ff_hid_descriptor

[Simon Wood]

This patch modifies the HID descriptor to allow the reporting of
the accelerometers and gyro via a joystick axis.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 936c911..5c930dc 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -28,6 +28,12 @@
 #define SIXAXIS_CONTROLLER_USB  (1 << 1)
 #define SIXAXIS_CONTROLLER_BT   (1 << 2)
 
+static const u8 sixaxis_rdesc_fixup[] = {
+	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
+	0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF, 
+	0x03, 0x46, 0xFF, 0x03, 0x09, 0x01, 0x81, 0x02
+	};
+
 struct sony_sc {
 	unsigned long quirks;
 };
@@ -43,6 +49,11 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		hid_info(hdev, "Fixing up Sony Vaio VGX report descriptor\n");
 		rdesc[55] = 0x06;
 	}
+	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
+			*rsize == 148 && rdesc[83] == 0x75) {
+		hid_info(hdev, "Fixing up Sony Sixaxis report descriptor\n");
+		memcpy((void *)&rdesc[83], (void *) &sixaxis_rdesc_fixup, sizeof(sixaxis_rdesc_fixup));
+	}
 	return rdesc;
 }
 
-- 
1.7.4.1

[PATCH 2/2] sony_ff_byteswap_accelerometer

[Simon Wood]

The accelerometers/gyro on the sixaxis are reported in the wrong
endianness (ie. not compatible with HID), so this patch intercepts
the report and swaps the appropriate bytes over.

Accelerometers are scaled with a nominal value of +/-4000 = 1G,
maximum value would be around +/-32768 = 8G.

Gyro on my device always reports -32768, might need some calibration
set within the controller.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 5c930dc..f219746 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -57,6 +57,24 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+/* Sixaxis HID report has acclerometers/gyro with MSByte first, this has
+ * to be BYTE_SWAPPED before passing up to joystick interface
+ */
+static int sony_raw_event(struct hid_device *hdev, struct hid_report *report, __u8 *rd, int size)
+{
+	struct sony_sc *sc = hid_get_drvdata(hdev);
+
+	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
+			rd[0] == 0x01 && size == 49) {
+		swap(rd[41], rd[42]);
+		swap(rd[43], rd[44]);
+		swap(rd[45], rd[46]);
+		swap(rd[47], rd[48]);
+	}
+
+	return 0;
+}
+
 /*
  * The Sony Sixaxis does not handle HID Output Reports on the Interrupt EP
  * like it should according to usbhid/hid-core.c::usbhid_output_raw_report()
@@ -205,6 +223,7 @@ static struct hid_driver sony_driver = {
 	.probe = sony_probe,
 	.remove = sony_remove,
 	.report_fixup = sony_report_fixup,
+	.raw_event = sony_raw_event
 };
 
 static int __init sony_init(void)
-- 
1.7.4.1

Byte swapping u16 data in HID report

[simon]

Hi,
I'm looking into fixing the Sixaxis controller (hid-sony.c) so that the
accelerometers and gyro data are reported via a joystick axis.

I have patched the HID descriptor so that the values are seen, however
they are reported high byte first (0x2FF -> 0x02, 0xFF) which appears to
be contrary to the HID spec.
--
5.8 Format of Multibyte Numeric Values
Multibyte numeric values in reports are represented in little-endian
format, with the least significant byte at the lowest address.
--

I'm working on the assumption that the data can be 'fixed' before it is
parsed. Would intercepting 'hid_get_report()' be a good place and how can
I locate the actual data bytes to do a 'bswap16()'.

I'm thinking something like:
--
	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
		hdev->hid_get_report = sixaxis_hid_get_report;
	}
--

Any suggestions/comments?
Simon.

Re: Byte swapping u16 data in HID report

[Antonio Ospite]

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

On Wed, 1 Jun 2011 01:32:15 -0400
simon@mungewell.org wrote:

> Hi,
> I'm looking into fixing the Sixaxis controller (hid-sony.c) so that the
> accelerometers and gyro data are reported via a joystick axis.
> 
> I have patched the HID descriptor so that the values are seen, however
> they are reported high byte first (0x2FF -> 0x02, 0xFF) which appears to
> be contrary to the HID spec.
> --
> 5.8 Format of Multibyte Numeric Values
> Multibyte numeric values in reports are represented in little-endian
> format, with the least significant byte at the lowest address.
> --
> 
> I'm working on the assumption that the data can be 'fixed' before it is
> parsed. Would intercepting 'hid_get_report()' be a good place and how can
> I locate the actual data bytes to do a 'bswap16()'.
> 
> I'm thinking something like:
> --
> 	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> 		hdev->hid_get_report = sixaxis_hid_get_report;
> 	}
> --
> 
> Any suggestions/comments?
> Simon.
>

Marcin Tolysz (added to CC), worked on the sixaxis descriptor before,
see http://permalink.gmane.org/gmane.linux.bluez.kernel/5184

Look in particular at the sony_raw_event() function.

I also like his approach of overriding HID descriptors with the
firmware framework in principle, even if its practical use still needs
some evaluation.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Byte swapping u16 data in HID report

[simon]

> Marcin Tolysz (added to CC), worked on the sixaxis descriptor before,
> see http://permalink.gmane.org/gmane.linux.bluez.kernel/5184

Looks like this is over a year old, so I presume that it got dropped.

For reference I use a small(er) shim to the existing descriptor
--
static const u8 sixaxis_rdesc_fixup[] = {
    0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
    0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF,
    0x03, 0x46, 0xFF, 0x03, 0x09, 0x01, 0x81, 0x02
    };
--
    if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
            *rsize == 148 && rdesc[83] == 0x75) {
        hid_info(hdev, "Fixing up Sony Sixaxis report descriptor\n");
        memcpy((void *)&rdesc[83], (void *) &sixaxis_rdesc_fixup,
sizeof(sixaxis_rdesc_fixup));
--

I'll look at the byte swap stuff from Marcin's patch and submit a patch
here it appears to work.

BTW. There are a great set of tools for encoding/decoding the HID
descriptors here:
http://digimend.sourceforge.net/

Cheers,
Simon

Re: [PATCH 1/2] sony_ff_hid_descriptor

[Antonio Ospite]

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

On Mon, 23 May 2011 03:19:58 -0700
Simon Wood <simon@mungewell.org> wrote:

> This patch modifies the HID descriptor to allow the reporting of
> the accelerometers and gyro via a joystick axis.

I am going to test it tomorrow, but for now I notice that the
Signed-off-by is missing. Wait for the test before resending tho.

Plus, the short commit message can be improved a little bit, and the
long commit message could use the classic imperative form used in kernel
commit messages: "Modify the HID descriptor to...", but these are just
suggestions.

Same comments for patch 2/2.

Thanks,
   Antonio

> ---
>  drivers/hid/hid-sony.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 936c911..5c930dc 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -28,6 +28,12 @@
>  #define SIXAXIS_CONTROLLER_USB  (1 << 1)
>  #define SIXAXIS_CONTROLLER_BT   (1 << 2)
>  
> +static const u8 sixaxis_rdesc_fixup[] = {
> +	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
> +	0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF, 
> +	0x03, 0x46, 0xFF, 0x03, 0x09, 0x01, 0x81, 0x02
> +	};
> +
>  struct sony_sc {
>  	unsigned long quirks;
>  };
> @@ -43,6 +49,11 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		hid_info(hdev, "Fixing up Sony Vaio VGX report descriptor\n");
>  		rdesc[55] = 0x06;
>  	}
> +	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
> +			*rsize == 148 && rdesc[83] == 0x75) {
> +		hid_info(hdev, "Fixing up Sony Sixaxis report descriptor\n");
> +		memcpy((void *)&rdesc[83], (void *) &sixaxis_rdesc_fixup, sizeof(sixaxis_rdesc_fixup));
> +	}
>  	return rdesc;
>  }
>  
> -- 
> 1.7.4.1
> 
> 

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] sony_ff_byteswap_accelerometer

[Antonio Ospite]

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

On Mon, 23 May 2011 03:19:59 -0700
Simon Wood <simon@mungewell.org> wrote:

> The accelerometers/gyro on the sixaxis are reported in the wrong
> endianness (ie. not compatible with HID), so this patch intercepts
> the report and swaps the appropriate bytes over.
> 
> Accelerometers are scaled with a nominal value of +/-4000 = 1G,
> maximum value would be around +/-32768 = 8G.
> 
> Gyro on my device always reports -32768, might need some calibration
> set within the controller.
> ---
>  drivers/hid/hid-sony.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
>

Should this one have Marcin as the original author or at least as the
first Signed-off-by, and an additional Signed-off-by you Simon?

Thanks,
  Antonio

> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 5c930dc..f219746 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -57,6 +57,24 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  	return rdesc;
>  }
>  
> +/* Sixaxis HID report has acclerometers/gyro with MSByte first, this has
> + * to be BYTE_SWAPPED before passing up to joystick interface
> + */
> +static int sony_raw_event(struct hid_device *hdev, struct hid_report *report, __u8 *rd, int size)
> +{
> +	struct sony_sc *sc = hid_get_drvdata(hdev);
> +
> +	if ((sc->quirks & SIXAXIS_CONTROLLER_USB) &&
> +			rd[0] == 0x01 && size == 49) {
> +		swap(rd[41], rd[42]);
> +		swap(rd[43], rd[44]);
> +		swap(rd[45], rd[46]);
> +		swap(rd[47], rd[48]);
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * The Sony Sixaxis does not handle HID Output Reports on the Interrupt EP
>   * like it should according to usbhid/hid-core.c::usbhid_output_raw_report()
> @@ -205,6 +223,7 @@ static struct hid_driver sony_driver = {
>  	.probe = sony_probe,
>  	.remove = sony_remove,
>  	.report_fixup = sony_report_fixup,
> +	.raw_event = sony_raw_event
>  };
>  
>  static int __init sony_init(void)
> -- 
> 1.7.4.1
> 
> 


-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/2] sony_ff_hid_descriptor

[Jiri Kosina]

On Wed, 1 Jun 2011, Antonio Ospite wrote:

> On Mon, 23 May 2011 03:19:58 -0700
> Simon Wood <simon@mungewell.org> wrote:
> 
> > This patch modifies the HID descriptor to allow the reporting of
> > the accelerometers and gyro via a joystick axis.
> 
> I am going to test it tomorrow, but for now I notice that the
> Signed-off-by is missing. Wait for the test before resending tho.
> 
> Plus, the short commit message can be improved a little bit, and the
> long commit message could use the classic imperative form used in kernel
> commit messages: "Modify the HID descriptor to...", but these are just
> suggestions.
> 
> Same comments for patch 2/2.

Antonio,

thanks for the (valid) comments.

What was the result of your testing of the patches, please?

Simon, would you please, once Antionio adds his Tested-by:, to resend with 
proper authorship and Signed-off-by? 

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/2] sony_ff_hid_descriptor

[Jiri Kosina]

On Tue, 7 Jun 2011, Jiri Kosina wrote:

> Simon, would you please, once Antionio adds his Tested-by:, to resend with 
> proper authorship and Signed-off-by? 

Ah, I see you already did, in a separate thread.

So I'd just like to wait for Tested-by: from Antonion, and will then 
apply.

thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/2] sony_ff_hid_descriptor

[Antonio Ospite]

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

On Tue, 7 Jun 2011 15:32:15 +0200 (CEST)
Jiri Kosina <jkosina@suse.cz> wrote:

> On Wed, 1 Jun 2011, Antonio Ospite wrote:
> 
> > On Mon, 23 May 2011 03:19:58 -0700
> > Simon Wood <simon@mungewell.org> wrote:
> > 
> > > This patch modifies the HID descriptor to allow the reporting of
> > > the accelerometers and gyro via a joystick axis.
> > 
> > I am going to test it tomorrow, but for now I notice that the
> > Signed-off-by is missing. Wait for the test before resending tho.
> > 
> > Plus, the short commit message can be improved a little bit, and the
> > long commit message could use the classic imperative form used in kernel
> > commit messages: "Modify the HID descriptor to...", but these are just
> > suggestions.
[...]

> What was the result of your testing of the patches, please?
> 

There has been an off-list discussion and I think Simon is going to send
the patches once again in a form which amends the HID descriptor even
when the Sixaxis is connected via Bluetooth.

I will retest and add the Tested-by on those.

Regards,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/2] sony_ff_hid_descriptor

[simon]

>
> What was the result of your testing of the patches, please?
>
> Simon, would you please, once Antionio adds his Tested-by:, to resend with
> proper authorship and Signed-off-by?

Jiri: I think that we have a plan... I'll resend a patch (V4) in the next
couple of days.

Antionio: Do I just add 'tested-by' on your behalf?

Simon

Re: [PATCH 1/2] sony_ff_hid_descriptor

[Antonio Ospite]

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

On Tue, 7 Jun 2011 11:53:08 -0400
simon@mungewell.org wrote:

> 
> >
> > What was the result of your testing of the patches, please?
> >
> > Simon, would you please, once Antonio adds his Tested-by:, to resend with
> > proper authorship and Signed-off-by?
> 
> Jiri: I think that we have a plan... I'll resend a patch (V4) in the next
> couple of days.
> 
> Antonio: Do I just add 'tested-by' on your behalf?
> 

I am going to test (and use) the patches anyways, so I think I can send
the tested-by mails, no problem. :)

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

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?

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]