hidraw, sixaxis and output reports.

hidraw, sixaxis and output reports.

[Antonio Ospite]

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

Hi,

I am trying to set leds on sixaxis now, and the code I have works OK
with libusb, but the equivalent for hidraw does not.

The procedure to set leds is to send an output report 0x01 with the
correct data. After looking at drivers/usbhid/hid-core.c in
usbhid_output_raw_report() I see that output reports are sent over the
interrupt endpoint when hid->urbout is defined (that is when the device
has an interrupt out ep, right?), but it looks like this particular
device is accepting output reports only on the _control_ endpoint; this
is exactly what the working libusb version does btw.

Just for the record, this dumb hack makes setting leds work on sixaxis
with hidraw:

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index df80532..731c08c 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -841,7 +841,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
        struct usb_host_interface *interface = intf->cur_altsetting;
        int ret;

-       if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
+       if (0 && usbhid->urbout && report_type != HID_FEATURE_REPORT) {
                int actual_length;
                int skipped_report_id = 0;
                if (buf[0] == 0x0) {

If you are interested in the userspace programs, just ping me.

What can I do to handle this properly? Add another quirk? Something
like HID_QUIRK_NO_INTERRUPT_OUT_EP or HID_QUIRK_FORCE_CONTROL_OUT_EP?

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

Re: hidraw, sixaxis and output reports.

[Alan Ott]

On 10/02/2010 11:59 AM, Antonio Ospite wrote:
> Hi,
>
> I am trying to set leds on sixaxis now, and the code I have works OK
> with libusb, but the equivalent for hidraw does not.
>
> The procedure to set leds is to send an output report 0x01 with the
> correct data. After looking at drivers/usbhid/hid-core.c in
> usbhid_output_raw_report() I see that output reports are sent over the
> interrupt endpoint when hid->urbout is defined (that is when the device
> has an interrupt out ep, right?), but it looks like this particular
> device is accepting output reports only on the _control_ endpoint; this
> is exactly what the working libusb version does btw.
>
> Just for the record, this dumb hack makes setting leds work on sixaxis
> with hidraw:
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index df80532..731c08c 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -841,7 +841,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
>          struct usb_host_interface *interface = intf->cur_altsetting;
>          int ret;
>
> -       if (usbhid->urbout&&  report_type != HID_FEATURE_REPORT) {
> +       if (0&&  usbhid->urbout&&  report_type != HID_FEATURE_REPORT) {
>                  int actual_length;
>                  int skipped_report_id = 0;
>                  if (buf[0] == 0x0) {
>
> If you are interested in the userspace programs, just ping me.
>
> What can I do to handle this properly? Add another quirk? Something
> like HID_QUIRK_NO_INTERRUPT_OUT_EP or HID_QUIRK_FORCE_CONTROL_OUT_EP?
>    

Sounds right to me. If there's an OUT endpoint, the device SHOULD handle 
output reports on it. Anything else is non-compliant, and I think a 
quirk makes sense.

That SixAxis is quite a pain....

Alan.

[RFC, PATCH 0/2] Enable setting leds via hidraw on Sixaxis

[Antonio Ospite]

Hi,

this series adds a new quirk to force output reports to be sent over the
control endpoint rather than the interrupt ep, this is needed on our
"beloved" Sixaxis controller in order to set leds via hidraw, sending
the output report on the interrupt endpoint does not have any effect
whatsoever.

This is just an RFC for now, the quirk name can be improved, and I have
to make clear in the code and in the commit messages if this is for
hidraw only, but you can get the idea already.

Thanks,
   Antonio


Antonio Ospite (2):
  HID: usbhid, new quirk to force out reports on the control ep
  HID: hid-sony, use HID_QUIRK_FORCE_OUT_CONTROL_EP for Sixaxis

 include/linux/hid.h           |    1 +
 drivers/hid/hid-sony.c        |    4 +++-
 drivers/hid/usbhid/hid-core.c |    3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)


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

[RFC, PATCH 1/2] HID: usbhid, new quirk to force out reports on the control ep

[Antonio Ospite]

Some non compliant usb devices, like the Sony Sixaxis (PS3 Controller) need
output reports to be sent on the control endpoint rather than on the interrupt
endpoint like the standard would say.

Add a new quirk to handle this case in usbhid_output_raw_report().

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 include/linux/hid.h           |    1 +
 drivers/hid/usbhid/hid-core.c |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 1b95b0b..936b7a3 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -313,6 +313,7 @@ struct hid_item {
 #define HID_QUIRK_MULTI_INPUT			0x00000040
 #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
+#define HID_QUIRK_FORCE_OUT_CONTROL_EP		0x00020000
 #define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
 #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
 #define HID_QUIRK_NO_IGNORE			0x40000000
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index df80532..81dadf8 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -841,7 +841,8 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	struct usb_host_interface *interface = intf->cur_altsetting;
 	int ret;
 
-	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT &&
+			!(hid->quirks & HID_QUIRK_FORCE_OUT_CONTROL_EP)) {
 		int actual_length;
 		int skipped_report_id = 0;
 		if (buf[0] == 0x0) {
-- 
1.7.1

[RFC, PATCH 2/2] HID: hid-sony, use HID_QUIRK_FORCE_OUT_CONTROL_EP for Sixaxis

[Antonio Ospite]

Force output reports to be sent on the control endpoint for
SIXAXIS_CONTROLLER_USB, this allows to finally send output reports to
the controller via hidraw, which is needed in order to set leds without
libusb.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---
 drivers/hid/hid-sony.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d61f268..7b0c70a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -110,8 +110,10 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-	if (sc->quirks & SIXAXIS_CONTROLLER_USB)
+	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
 		ret = sixaxis_set_operational_usb(hdev);
+		hdev->quirks |= HID_QUIRK_FORCE_OUT_CONTROL_EP;
+	}
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
 	else
-- 
1.7.1

Re: [RFC, PATCH 0/2] Enable setting leds via hidraw on Sixaxis

[Alan Ott]

On 10/13/2010 04:57 PM, Antonio Ospite wrote:
> Hi,
>
> this series adds a new quirk to force output reports to be sent over the
> control endpoint rather than the interrupt ep, this is needed on our
> "beloved" Sixaxis controller in order to set leds via hidraw, sending
> the output report on the interrupt endpoint does not have any effect
> whatsoever.
>
> This is just an RFC for now, the quirk name can be improved, and I have
> to make clear in the code and in the commit messages if this is for
> hidraw only, but you can get the idea already.
>
> Thanks,
>     Antonio
>
>
> Antonio Ospite (2):
>    HID: usbhid, new quirk to force out reports on the control ep
>    HID: hid-sony, use HID_QUIRK_FORCE_OUT_CONTROL_EP for Sixaxis
>
>   include/linux/hid.h           |    1 +
>   drivers/hid/hid-sony.c        |    4 +++-
>   drivers/hid/usbhid/hid-core.c |    3 ++-
>   3 files changed, 6 insertions(+), 2 deletions(-)
>
>
>    

Hi Antonio,

If it were me, I'd just call usb_control_msg() from inside your driver 
(hid-sony) instead of calling usbhid_output_report(). What's wrong with 
doing that?

Alan.

Re: [RFC, PATCH 0/2] Enable setting leds via hidraw on Sixaxis

[Antonio Ospite]

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

On Wed, 13 Oct 2010 18:15:05 -0400
Alan Ott <alan@signal11.us> wrote:

> On 10/13/2010 04:57 PM, Antonio Ospite wrote:
> > Hi,
> >
> > this series adds a new quirk to force output reports to be sent over the
> > control endpoint rather than the interrupt ep, this is needed on our
> > "beloved" Sixaxis controller in order to set leds via hidraw, sending
> > the output report on the interrupt endpoint does not have any effect
> > whatsoever.
> >
> > This is just an RFC for now, the quirk name can be improved, and I have
> > to make clear in the code and in the commit messages if this is for
> > hidraw only, but you can get the idea already.
> >
> > Thanks,
> >     Antonio
> >
> >
> > Antonio Ospite (2):
> >    HID: usbhid, new quirk to force out reports on the control ep
> >    HID: hid-sony, use HID_QUIRK_FORCE_OUT_CONTROL_EP for Sixaxis
> >
> >   include/linux/hid.h           |    1 +
> >   drivers/hid/hid-sony.c        |    4 +++-
> >   drivers/hid/usbhid/hid-core.c |    3 ++-
> >   3 files changed, 6 insertions(+), 2 deletions(-)
> >
> >
> >    
> 
> Hi Antonio,
> 
> If it were me, I'd just call usb_control_msg() from inside your driver 
> (hid-sony) instead of calling usbhid_output_report(). What's wrong with 
> doing that?
>

Alan, yeah, _overriding_ the usbhid_output_report() method with another
function which performs usb_control_msg() unconditionally could work; I
am giving that a try.

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

[RFC, PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Antonio Ospite]

Sony Sixaxis wants output reports on the control endpoint rather than
interrupt endpoint, so override usbhid_output_raw_report in order to
force this behaviour.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---

This works indeed but there is obviously quite some code duplication with
usbhid_output_raw_report, if a change would be made there it had to be ported
here too. I still like the quirk approach better, but I am open to
suggestions.

Patch is on top of the previous series.

Thanks,
   Antonio

 drivers/hid/hid-sony.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 7b0c70a..d1007f4 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -46,6 +46,38 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	}
 }
 
+static int sixaxis_usb_output_raw_report(struct hid_device *hid, __u8 *buf,
+		size_t count, unsigned char report_type)
+{
+	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct usb_host_interface *interface = intf->cur_altsetting;
+
+	int ret;
+	int skipped_report_id = 0;
+	int report_id = buf[0];
+
+	printk(KERN_DEBUG "%s: Overriding usbhid_output_raw_report\n", __func__);
+
+	if (buf[0] == 0x0) {
+		/* Don't send the Report ID */
+		buf++;
+		count--;
+		skipped_report_id = 1;
+	}
+	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+		HID_REQ_SET_REPORT,
+		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+		((report_type + 1) << 8) | report_id,
+		interface->desc.bInterfaceNumber, buf, count,
+		USB_CTRL_SET_TIMEOUT);
+	/* count also the report id, if this was a numbered report. */
+	if (ret > 0 && skipped_report_id)
+		ret++;
+
+	return ret;
+}
+
 /*
  * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
  * to "operational".  Without this, the ps3 controller will not report any
@@ -111,8 +143,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+		hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
 		ret = sixaxis_set_operational_usb(hdev);
-		hdev->quirks |= HID_QUIRK_FORCE_OUT_CONTROL_EP;
 	}
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
-- 
1.7.1

Re: [RFC, PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Alan Ott]

On 10/13/2010 06:54 PM, Antonio Ospite wrote:
> Sony Sixaxis wants output reports on the control endpoint rather than
> interrupt endpoint, so override usbhid_output_raw_report in order to
> force this behaviour.
>
> Signed-off-by: Antonio Ospite<ospite@studenti.unina.it>
> ---
>
> This works indeed but there is obviously quite some code duplication with
> usbhid_output_raw_report, if a change would be made there it had to be ported
> here too. I still like the quirk approach better, but I am open to
> suggestions.
>
> Patch is on top of the previous series.
>
> Thanks,
>     Antonio
>
>   drivers/hid/hid-sony.c |   34 +++++++++++++++++++++++++++++++++-
>   1 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 7b0c70a..d1007f4 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -46,6 +46,38 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>   	}
>   }
>
> +static int sixaxis_usb_output_raw_report(struct hid_device *hid, __u8 *buf,
> +		size_t count, unsigned char report_type)
> +{
> +	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	struct usb_host_interface *interface = intf->cur_altsetting;
> +
> +	int ret;
> +	int skipped_report_id = 0;
> +	int report_id = buf[0];
> +
> +	printk(KERN_DEBUG "%s: Overriding usbhid_output_raw_report\n", __func__);
> +
> +	if (buf[0] == 0x0) {
> +		/* Don't send the Report ID */
> +		buf++;
> +		count--;
> +		skipped_report_id = 1;
> +	}
> +	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> +		HID_REQ_SET_REPORT,
> +		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> +		((report_type + 1)<<  8) | report_id,
> +		interface->desc.bInterfaceNumber, buf, count,
> +		USB_CTRL_SET_TIMEOUT);
> +	/* count also the report id, if this was a numbered report. */
> +	if (ret>  0&&  skipped_report_id)
> +		ret++;
> +
> +	return ret;
> +}
> +
>   /*
>    * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
>    * to "operational".  Without this, the ps3 controller will not report any
> @@ -111,8 +143,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
>   	}
>
>   	if (sc->quirks&  SIXAXIS_CONTROLLER_USB) {
> +		hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
>   		ret = sixaxis_set_operational_usb(hdev);
> -		hdev->quirks |= HID_QUIRK_FORCE_OUT_CONTROL_EP;
>   	}
>   	else if (sc->quirks&  SIXAXIS_CONTROLLER_BT)
>   		ret = sixaxis_set_operational_bt(hdev);
>    

I think as far as code duplication goes, you can get rid of the 
skipped_report stuff and the buf[0] = 0x0 section (since you _know_ the 
sixaxis uses numbered reports). Once you've done that, you're basically 
down to one function call.

I think maybe I'm confused a bit. Is this patch against Jiri's hid tree 
in the for-next branch[1]? If so is there another patch which is 
required for this to make sense (I looked in linux-input, but didn't see 
any appropriate ones)? In Jiri's tree, sixaxis_set_operational_usb() 
already calls usb_control_msg() instead of hid_output_raw_report(), so I 
clearly must be looking at the wrong thing. Please help me understand 
what I'm supposed to be looking at.

Alan.

[1] 
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=blob_plain;f=drivers/hid/hid-sony.c;hb=816651a7d4a32664261e5f9f88ad0d558faed4cc

Re: [RFC, PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Antonio Ospite]

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

On Wed, 13 Oct 2010 19:23:52 -0400
Alan Ott <alan@signal11.us> wrote:

> On 10/13/2010 06:54 PM, Antonio Ospite wrote:
> > Sony Sixaxis wants output reports on the control endpoint rather than
> > interrupt endpoint, so override usbhid_output_raw_report in order to
> > force this behaviour.
> >
> > Signed-off-by: Antonio Ospite<ospite@studenti.unina.it>
> > ---
[...]
> > +static int sixaxis_usb_output_raw_report(struct hid_device *hid, __u8 *buf,
> > +		size_t count, unsigned char report_type)
> > +{
> > +	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
> > +	struct usb_device *dev = interface_to_usbdev(intf);
> > +	struct usb_host_interface *interface = intf->cur_altsetting;
> > +
> > +	int ret;
> > +	int skipped_report_id = 0;
> > +	int report_id = buf[0];
> > +
> > +	printk(KERN_DEBUG "%s: Overriding usbhid_output_raw_report\n", __func__);
> > +
> > +	if (buf[0] == 0x0) {
> > +		/* Don't send the Report ID */
> > +		buf++;
> > +		count--;
> > +		skipped_report_id = 1;
> > +	}
> > +	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> > +		HID_REQ_SET_REPORT,
> > +		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > +		((report_type + 1)<<  8) | report_id,
> > +		interface->desc.bInterfaceNumber, buf, count,
> > +		USB_CTRL_SET_TIMEOUT);
> > +	/* count also the report id, if this was a numbered report. */
> > +	if (ret>  0&&  skipped_report_id)
> > +		ret++;
> > +
> > +	return ret;
> > +}
> > +
[...]   
> 
> I think as far as code duplication goes, you can get rid of the 
> skipped_report stuff and the buf[0] = 0x0 section (since you _know_ the 
> sixaxis uses numbered reports). Once you've done that, you're basically 
> down to one function call.
>

That's right thanks a lot.

> I think maybe I'm confused a bit. Is this patch against Jiri's hid tree 
> in the for-next branch[1]? If so is there another patch which is 
> required for this to make sense (I looked in linux-input, but didn't see 
> any appropriate ones)? In Jiri's tree, sixaxis_set_operational_usb() 
> already calls usb_control_msg() instead of hid_output_raw_report(), so I 
> clearly must be looking at the wrong thing. Please help me understand 
> what I'm supposed to be looking at.
>

I am working on linus tree with some patches applied, namely your
hidraw feature report patches and this one from Jiri's hid/for-next
branch:
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=commit;h=816651a7d4a32664261e5f9f88ad0d558faed4cc

The point about overriding hid_output_raw_report() is not about
sixaxis_set_operational_usb() sending a control message, but about any
_output_ (not feature) report the user would like to send via
hidraw_write(), the sixaxis will accept those only if sent on the
control endpoint (it will discard them if sent on the interrupt ep,
like the current usbhid code does) maybe I didn't state that clearly,
sorry.

About sixaxis_set_operational_usb(), I don't remember _any_ change to
it except its name, but I guess we can very well make it call
hid_output_raw_report(), just only after your hidraw feature report
patches land to mainline.

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: [RFC, PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Alan Ott]

On 10/14/2010 03:48 AM, Antonio Ospite wrote:
>
> The point about overriding hid_output_raw_report() is not about
> sixaxis_set_operational_usb() sending a control message, but about any
> _output_ (not feature) report the user would like to send via
> hidraw_write(), the sixaxis will accept those only if sent on the
> control endpoint (it will discard them if sent on the interrupt ep,
> like the current usbhid code does) maybe I didn't state that clearly,
> sorry.
>    

Ok, that makes a lot more sense then. You're saying it's not about the 
code _in_ your driver, it's about userspace code accessing the sixaxis 
via hidraw.

My personal preference is that I like the override idea better than the 
quirk. An override will be slightly faster, because of not having to 
check for the quirk in non-sixaxis cases.

> About sixaxis_set_operational_usb(), I don't remember _any_ change to
> it except its name, but I guess we can very well make it call
> hid_output_raw_report(), just only after your hidraw feature report
> patches land to mainline.
>
>    

sixaxis_set_operational_usb() doesn't bother me. I was confused about 
the original intent of your patch.

Alan.

[PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Antonio Ospite]

Override usbhid_output_raw_report in order to force output reports (sent
via hidraw_write, for instance) on the control endpoint.

The Sony Sixaxis (PS3 Controller) accepts output reports only on the
control endpoint, it silently discards them when they arrive over the
interrupt endpoint where usbhid would normally deliver them.

Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
---

Note to the interested people, this makes setting leds via hidraw finally
work. I can send the userspace code for that in a follow-up if you want me
to.

Thanks,
   Antonio Ospite
   http://ao2.it

 drivers/hid/hid-sony.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d61f268..0c164b3 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -46,6 +46,25 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	}
 }
 
+static int sixaxis_usb_output_raw_report(struct hid_device *hid, __u8 *buf,
+		size_t count, unsigned char report_type)
+{
+	struct usb_interface *intf = to_usb_interface(hid->dev.parent);
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct usb_host_interface *interface = intf->cur_altsetting;
+	int report_id = buf[0];
+	int ret;
+
+	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+		HID_REQ_SET_REPORT,
+		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+		((report_type + 1) << 8) | report_id,
+		interface->desc.bInterfaceNumber, buf, count,
+		USB_CTRL_SET_TIMEOUT);
+
+	return ret;
+}
+
 /*
  * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
  * to "operational".  Without this, the ps3 controller will not report any
@@ -110,8 +129,10 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-	if (sc->quirks & SIXAXIS_CONTROLLER_USB)
+	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+		hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
 		ret = sixaxis_set_operational_usb(hdev);
+	}
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
 	else
-- 
1.7.2.3

Re: [PATCH] HID: hid-sony, override usbhid_output_raw_report for Sixaxis

[Jiri Kosina]

On Tue, 19 Oct 2010, Antonio Ospite wrote:

> Override usbhid_output_raw_report in order to force output reports (sent
> via hidraw_write, for instance) on the control endpoint.
> 
> The Sony Sixaxis (PS3 Controller) accepts output reports only on the
> control endpoint, it silently discards them when they arrive over the
> interrupt endpoint where usbhid would normally deliver them.
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

Applied, thanks Antonio.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.