[PATCH 0/2] Groundwork for Sixaxis paring into bluez.

[PATCH 0/2] Groundwork for Sixaxis paring into bluez.

[Antonio Ospite]

Hi,

These changes are in preparation of seamless integration of the Sixaxis
pairing procedure into bluez. It was about time :)

The first one cleans up the quirks handling.

The second one adds a sysfs interface to set and get the device and master
bdaddr, this is needed in order to setup the host for pairing. This kernel
interface is a mere workaround for the lack of features of hidraw[1], if
someone wants to add the missing features to hidraw I will be happy to drop
this one, otherwise consider queuing these for 2.6.35 if it's too late for
2.6.34.

Thanks,
   Antonio

[1] http://www.spinics.net/lists/linux-input/msg08311.html


Antonio Ospite (2):
  hid/hid-sony: Apply sixaxis quirks only to sixaxis
  hid/hid-sony: get and set Sixaxis bdaddr via sysfs

 drivers/hid/hid-sony.c |  170 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 155 insertions(+), 15 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?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Antonio Ospite]

Be more explicit and avoid calling sony_set_operational_usb() when we
have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.

While at it, rename the sony_set_operational routines to
sixaxis_set_operational as they are sixaxis specific.

This is also in preparation for the sysfs interface to set and get bdaddr
over usb and for some other Sixaxis report fixup.

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

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 402d557..d61f268 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -24,7 +24,9 @@
 
 #include "hid-ids.h"
 
-#define VAIO_RDESC_CONSTANT 0x0001
+#define VAIO_RDESC_CONSTANT     (1 << 0)
+#define SIXAXIS_CONTROLLER_USB  (1 << 1)
+#define SIXAXIS_CONTROLLER_BT   (1 << 2)
 
 struct sony_sc {
 	unsigned long quirks;
@@ -49,7 +51,7 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
  * to "operational".  Without this, the ps3 controller will not report any
  * events.
  */
-static int sony_set_operational_usb(struct hid_device *hdev)
+static int sixaxis_set_operational_usb(struct hid_device *hdev)
 {
 	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
 	struct usb_device *dev = interface_to_usbdev(intf);
@@ -74,7 +76,7 @@ static int sony_set_operational_usb(struct hid_device *hdev)
 	return ret;
 }
 
-static int sony_set_operational_bt(struct hid_device *hdev)
+static int sixaxis_set_operational_bt(struct hid_device *hdev)
 {
 	unsigned char buf[] = { 0xf4,  0x42, 0x03, 0x00, 0x00 };
 	return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
@@ -108,16 +110,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_free;
 	}
 
-	switch (hdev->bus) {
-	case BUS_USB:
-		ret = sony_set_operational_usb(hdev);
-		break;
-	case BUS_BLUETOOTH:
-		ret = sony_set_operational_bt(hdev);
-		break;
-	default:
+	if (sc->quirks & SIXAXIS_CONTROLLER_USB)
+		ret = sixaxis_set_operational_usb(hdev);
+	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
+		ret = sixaxis_set_operational_bt(hdev);
+	else
 		ret = 0;
-	}
 
 	if (ret < 0)
 		goto err_stop;
@@ -137,8 +135,10 @@ static void sony_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id sony_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
-	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
+		.driver_data = SIXAXIS_CONTROLLER_USB },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
+		.driver_data = SIXAXIS_CONTROLLER_BT },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE),
 		.driver_data = VAIO_RDESC_CONSTANT },
 	{ }
-- 
1.7.1

[PATCH 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Antonio Ospite]

Expose to userspace a simple way to get device bdaddr, and get/set master
bdaddr on Sixaxis controller.

Right now userspace softwares which manage pairing the controller with a
bluetooth adapter, rely on libusb and hence have to detach and reattach
usbhid, which is not very nice.

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

As said, ideally this should be done with an interface such as hidraw, but its
limitations prevent us from doing it the right way; and I am not going to
touch hidraw myself anytime soon.

Regards,
   Antonio

 drivers/hid/hid-sony.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d61f268..1b611ec 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -47,6 +47,131 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 }
 
 /*
+ * Show and set the master bdaddr for PS3 controller, without disconnecting
+ * the device.
+ */
+static ssize_t show_sixaxis_master_bdaddr(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
+
+	if (!mbuf)
+		return -ENOMEM;
+
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				 HID_REQ_GET_REPORT,
+				 USB_DIR_IN | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
+				 USB_CTRL_GET_TIMEOUT);
+	if (ret < 0)
+		dev_err(dev, "%s failed to get master bdaddr, ret: %d\n",
+				__func__, ret);
+	else
+		/* 18 is strlen("00:00:00:00:00:00\n") */
+		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+				mbuf[2], mbuf[3], mbuf[4],
+				mbuf[5], mbuf[6], mbuf[7]);
+
+	kfree(mbuf);
+
+	return ret;
+}
+
+static ssize_t store_sixaxis_master_bdaddr(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
+
+	mbuf[0] = 0x01;
+	mbuf[1] = 0x00;
+	ret = sscanf(buf, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+			&mbuf[2], &mbuf[3], &mbuf[4],
+			&mbuf[5], &mbuf[6], &mbuf[7]);
+	if (ret != 6) {
+		dev_err(dev, "%s failed, ret: %d\n", __func__, ret);
+		return -EINVAL;
+	}
+
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				 HID_REQ_SET_REPORT,
+				 USB_DIR_OUT | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
+				 USB_CTRL_GET_TIMEOUT);
+
+	kfree(mbuf);
+
+	if (ret < 0) {
+		dev_err(dev, "%s failed to set master bdaddr, ret: %d\n",
+				__func__, ret);
+		return ret;
+	}
+
+	return count;
+}
+
+/*
+ * Show the bdaddr for PS3 controller, without disconnecting the device.
+ */
+static ssize_t show_sixaxis_bdaddr(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(18, GFP_KERNEL);
+
+	if (!mbuf)
+		return -ENOMEM;
+
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				 HID_REQ_GET_REPORT,
+				 USB_DIR_IN | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf2, ifnum, mbuf, 17,
+				 USB_CTRL_GET_TIMEOUT);
+	if (ret < 0)
+		dev_err(dev, "%s failed to get device bdaddr, ret: %d\n",
+				__func__, ret);
+	else
+		/* 18 is strlen("00:00:00:00:00:00\n") */
+		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+				mbuf[4], mbuf[5], mbuf[6],
+				mbuf[7], mbuf[8], mbuf[9]);
+
+	kfree(mbuf);
+
+	return ret;
+}
+
+static DEVICE_ATTR(sixaxis_master_bdaddr, S_IWUSR|S_IRUGO,
+		show_sixaxis_master_bdaddr, store_sixaxis_master_bdaddr);
+
+static DEVICE_ATTR(sixaxis_bdaddr, S_IRUGO,
+		show_sixaxis_bdaddr, NULL);
+
+static struct attribute *sixaxis_attributes[] = {
+	&dev_attr_sixaxis_master_bdaddr.attr,
+	&dev_attr_sixaxis_bdaddr.attr,
+	NULL
+};
+
+static const struct attribute_group sixaxis_attr_group = {
+	.attrs = sixaxis_attributes,
+};
+
+
+/*
  * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
  * to "operational".  Without this, the ps3 controller will not report any
  * events.
@@ -57,8 +182,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	struct usb_device *dev = interface_to_usbdev(intf);
 	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	int ret;
-	char *buf = kmalloc(18, GFP_KERNEL);
+	unsigned char *buf = kmalloc(18, GFP_KERNEL);
 
+	dev_info(&hdev->dev, "Calling %s\n", __func__);
 	if (!buf)
 		return -ENOMEM;
 
@@ -70,6 +196,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 				 USB_CTRL_GET_TIMEOUT);
 	if (ret < 0)
 		dev_err(&hdev->dev, "can't set operational mode\n");
+	else
+		dev_info(&hdev->dev, "Sony PS3 Controller bdaddr: %02x:%02x:%02x:%02x:%02x:%02x\n",
+				buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
 
 	kfree(buf);
 
@@ -110,9 +239,17 @@ 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) {
+		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+		ret = sysfs_create_group(&intf->dev.kobj, &sixaxis_attr_group);
+		if (ret < 0) {
+			dev_err(&hdev->dev,
+				"cannot register sixaxis sysfs hooks\n");
+			goto err_stop;
+		}
+
 		ret = sixaxis_set_operational_usb(hdev);
-	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
+	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
 	else
 		ret = 0;
@@ -130,6 +267,11 @@ err_free:
 
 static void sony_remove(struct hid_device *hdev)
 {
+	struct sony_sc *sc = hid_get_drvdata(hdev);
+	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+		sysfs_remove_group(&intf->dev.kobj, &sixaxis_attr_group);
+	}
 	hid_hw_stop(hdev);
 	kfree(hid_get_drvdata(hdev));
 }
-- 
1.7.1

[PATCH 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Antonio Ospite]

Expose to userspace a simple way to get device bdaddr, and get/set master
bdaddr on Sixaxis controller.

Right now userspace softwares which manage pairing the controller with a
bluetooth adapter, rely on libusb and hence have to detach and reattach
usbhid, which is not very nice.

Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
---

As said, ideally this should be done with an interface such as hidraw, but its
limitations prevent us from doing it the right way; and I am not going to
touch hidraw myself anytime soon.

Regards,
   Antonio

 drivers/hid/hid-sony.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 145 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d61f268..1b611ec 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -47,6 +47,131 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 }
 
 /*
+ * Show and set the master bdaddr for PS3 controller, without disconnecting
+ * the device.
+ */
+static ssize_t show_sixaxis_master_bdaddr(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
+
+	if (!mbuf)
+		return -ENOMEM;
+
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				 HID_REQ_GET_REPORT,
+				 USB_DIR_IN | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
+				 USB_CTRL_GET_TIMEOUT);
+	if (ret < 0)
+		dev_err(dev, "%s failed to get master bdaddr, ret: %d\n",
+				__func__, ret);
+	else
+		/* 18 is strlen("00:00:00:00:00:00\n") */
+		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+				mbuf[2], mbuf[3], mbuf[4],
+				mbuf[5], mbuf[6], mbuf[7]);
+
+	kfree(mbuf);
+
+	return ret;
+}
+
+static ssize_t store_sixaxis_master_bdaddr(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
+
+	mbuf[0] = 0x01;
+	mbuf[1] = 0x00;
+	ret = sscanf(buf, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+			&mbuf[2], &mbuf[3], &mbuf[4],
+			&mbuf[5], &mbuf[6], &mbuf[7]);
+	if (ret != 6) {
+		dev_err(dev, "%s failed, ret: %d\n", __func__, ret);
+		return -EINVAL;
+	}
+
+	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				 HID_REQ_SET_REPORT,
+				 USB_DIR_OUT | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
+				 USB_CTRL_GET_TIMEOUT);
+
+	kfree(mbuf);
+
+	if (ret < 0) {
+		dev_err(dev, "%s failed to set master bdaddr, ret: %d\n",
+				__func__, ret);
+		return ret;
+	}
+
+	return count;
+}
+
+/*
+ * Show the bdaddr for PS3 controller, without disconnecting the device.
+ */
+static ssize_t show_sixaxis_bdaddr(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct usb_device *udev = interface_to_usbdev(intf);
+	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	int ret;
+	unsigned char *mbuf = kmalloc(18, GFP_KERNEL);
+
+	if (!mbuf)
+		return -ENOMEM;
+
+	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				 HID_REQ_GET_REPORT,
+				 USB_DIR_IN | USB_TYPE_CLASS |
+				 USB_RECIP_INTERFACE,
+				 (3 << 8) | 0xf2, ifnum, mbuf, 17,
+				 USB_CTRL_GET_TIMEOUT);
+	if (ret < 0)
+		dev_err(dev, "%s failed to get device bdaddr, ret: %d\n",
+				__func__, ret);
+	else
+		/* 18 is strlen("00:00:00:00:00:00\n") */
+		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+				mbuf[4], mbuf[5], mbuf[6],
+				mbuf[7], mbuf[8], mbuf[9]);
+
+	kfree(mbuf);
+
+	return ret;
+}
+
+static DEVICE_ATTR(sixaxis_master_bdaddr, S_IWUSR|S_IRUGO,
+		show_sixaxis_master_bdaddr, store_sixaxis_master_bdaddr);
+
+static DEVICE_ATTR(sixaxis_bdaddr, S_IRUGO,
+		show_sixaxis_bdaddr, NULL);
+
+static struct attribute *sixaxis_attributes[] = {
+	&dev_attr_sixaxis_master_bdaddr.attr,
+	&dev_attr_sixaxis_bdaddr.attr,
+	NULL
+};
+
+static const struct attribute_group sixaxis_attr_group = {
+	.attrs = sixaxis_attributes,
+};
+
+
+/*
  * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
  * to "operational".  Without this, the ps3 controller will not report any
  * events.
@@ -57,8 +182,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	struct usb_device *dev = interface_to_usbdev(intf);
 	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	int ret;
-	char *buf = kmalloc(18, GFP_KERNEL);
+	unsigned char *buf = kmalloc(18, GFP_KERNEL);
 
+	dev_info(&hdev->dev, "Calling %s\n", __func__);
 	if (!buf)
 		return -ENOMEM;
 
@@ -70,6 +196,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 				 USB_CTRL_GET_TIMEOUT);
 	if (ret < 0)
 		dev_err(&hdev->dev, "can't set operational mode\n");
+	else
+		dev_info(&hdev->dev, "Sony PS3 Controller bdaddr: %02x:%02x:%02x:%02x:%02x:%02x\n",
+				buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
 
 	kfree(buf);
 
@@ -110,9 +239,17 @@ 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) {
+		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+		ret = sysfs_create_group(&intf->dev.kobj, &sixaxis_attr_group);
+		if (ret < 0) {
+			dev_err(&hdev->dev,
+				"cannot register sixaxis sysfs hooks\n");
+			goto err_stop;
+		}
+
 		ret = sixaxis_set_operational_usb(hdev);
-	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
+	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
 	else
 		ret = 0;
@@ -130,6 +267,11 @@ err_free:
 
 static void sony_remove(struct hid_device *hdev)
 {
+	struct sony_sc *sc = hid_get_drvdata(hdev);
+	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+		sysfs_remove_group(&intf->dev.kobj, &sixaxis_attr_group);
+	}
 	hid_hw_stop(hdev);
 	kfree(hid_get_drvdata(hdev));
 }
-- 
1.7.1

Re: [PATCH 0/2] Groundwork for Sixaxis paring into bluez.

[Antonio Ospite]

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

On Mon,  3 May 2010 22:15:54 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Hi,
> 
> These changes are in preparation of seamless integration of the Sixaxis
> pairing procedure into bluez. It was about time :)
>

Ping, and comments below.

> The first one cleans up the quirks handling.
> 
> The second one adds a sysfs interface to set and get the device and master
> bdaddr, this is needed in order to setup the host for pairing. This kernel
> interface is a mere workaround for the lack of features of hidraw[1], if
> someone wants to add the missing features to hidraw I will be happy to drop
> this one, otherwise consider queuing these for 2.6.35 if it's too late for
> 2.6.34.
>

I guess the silence here was also because this particular
hackish solution must not look very good to you. If there's really a
strong objection to it I might consider extending hidraw capabilities by
adding some ioctls, maybe I just need some extra motivation.

Regards,
   Antonio

> Thanks,
>    Antonio
> 
> [1] http://www.spinics.net/lists/linux-input/msg08311.html
> 
> 
> Antonio Ospite (2):
>   hid/hid-sony: Apply sixaxis quirks only to sixaxis
>   hid/hid-sony: get and set Sixaxis bdaddr via sysfs
> 
>  drivers/hid/hid-sony.c |  170 +++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 155 insertions(+), 15 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?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

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

Re: [PATCH 0/2] Groundwork for Sixaxis paring into bluez.

[Antonio Ospite]

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

On Mon,  3 May 2010 22:15:54 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Hi,
>

Ping.

> These changes are in preparation of seamless integration of the Sixaxis
> pairing procedure into bluez. It was about time :)
> 
> The first one cleans up the quirks handling.
>

Even if the second patch in this series is not OK and extending hidraw
is the way, I still think that the first one can be accepted. Anyone?

> The second one adds a sysfs interface to set and get the device and master
> bdaddr, this is needed in order to setup the host for pairing. This kernel
> interface is a mere workaround for the lack of features of hidraw[1], if
> someone wants to add the missing features to hidraw I will be happy to drop
> this one, otherwise consider queuing these for 2.6.35 if it's too late for
> 2.6.34.
> 
> Thanks,
>    Antonio
> 
> [1] http://www.spinics.net/lists/linux-input/msg08311.html
> 
> 
> Antonio Ospite (2):
>   hid/hid-sony: Apply sixaxis quirks only to sixaxis
>   hid/hid-sony: get and set Sixaxis bdaddr via sysfs
> 
>  drivers/hid/hid-sony.c |  170 +++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 155 insertions(+), 15 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?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

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

Re: [PATCH 0/2] Groundwork for Sixaxis paring into bluez.

[Antonio Ospite]

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

On Mon,  3 May 2010 22:15:54 +0200
Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org> wrote:

> Hi,
>

Ping.

> These changes are in preparation of seamless integration of the Sixaxis
> pairing procedure into bluez. It was about time :)
> 
> The first one cleans up the quirks handling.
>

Even if the second patch in this series is not OK and extending hidraw
is the way, I still think that the first one can be accepted. Anyone?

> The second one adds a sysfs interface to set and get the device and master
> bdaddr, this is needed in order to setup the host for pairing. This kernel
> interface is a mere workaround for the lack of features of hidraw[1], if
> someone wants to add the missing features to hidraw I will be happy to drop
> this one, otherwise consider queuing these for 2.6.35 if it's too late for
> 2.6.34.
> 
> Thanks,
>    Antonio
> 
> [1] http://www.spinics.net/lists/linux-input/msg08311.html
> 
> 
> Antonio Ospite (2):
>   hid/hid-sony: Apply sixaxis quirks only to sixaxis
>   hid/hid-sony: get and set Sixaxis bdaddr via sysfs
> 
>  drivers/hid/hid-sony.c |  170 +++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 155 insertions(+), 15 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?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

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

Re: [PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Bastien Nocera]

On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> Be more explicit and avoid calling sony_set_operational_usb() when we
> have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> 
> While at it, rename the sony_set_operational routines to
> sixaxis_set_operational as they are sixaxis specific.
> 
> This is also in preparation for the sysfs interface to set and get bdaddr
> over usb and for some other Sixaxis report fixup.
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>

Signed-off-by: Bastien Nocera <hadess@hadess.net>

> ---
>  drivers/hid/hid-sony.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 402d557..d61f268 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -24,7 +24,9 @@
>  
>  #include "hid-ids.h"
>  
> -#define VAIO_RDESC_CONSTANT 0x0001
> +#define VAIO_RDESC_CONSTANT     (1 << 0)
> +#define SIXAXIS_CONTROLLER_USB  (1 << 1)
> +#define SIXAXIS_CONTROLLER_BT   (1 << 2)
>  
>  struct sony_sc {
>  	unsigned long quirks;
> @@ -49,7 +51,7 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>   * to "operational".  Without this, the ps3 controller will not report any
>   * events.
>   */
> -static int sony_set_operational_usb(struct hid_device *hdev)
> +static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  {
>  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>  	struct usb_device *dev = interface_to_usbdev(intf);
> @@ -74,7 +76,7 @@ static int sony_set_operational_usb(struct hid_device *hdev)
>  	return ret;
>  }
>  
> -static int sony_set_operational_bt(struct hid_device *hdev)
> +static int sixaxis_set_operational_bt(struct hid_device *hdev)
>  {
>  	unsigned char buf[] = { 0xf4,  0x42, 0x03, 0x00, 0x00 };
>  	return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> @@ -108,16 +110,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		goto err_free;
>  	}
>  
> -	switch (hdev->bus) {
> -	case BUS_USB:
> -		ret = sony_set_operational_usb(hdev);
> -		break;
> -	case BUS_BLUETOOTH:
> -		ret = sony_set_operational_bt(hdev);
> -		break;
> -	default:
> +	if (sc->quirks & SIXAXIS_CONTROLLER_USB)
> +		ret = sixaxis_set_operational_usb(hdev);
> +	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> +		ret = sixaxis_set_operational_bt(hdev);
> +	else
>  		ret = 0;
> -	}
>  
>  	if (ret < 0)
>  		goto err_stop;
> @@ -137,8 +135,10 @@ static void sony_remove(struct hid_device *hdev)
>  }
>  
>  static const struct hid_device_id sony_devices[] = {
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> -	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
> +		.driver_data = SIXAXIS_CONTROLLER_USB },
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
> +		.driver_data = SIXAXIS_CONTROLLER_BT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE),
>  		.driver_data = VAIO_RDESC_CONSTANT },
>  	{ }

Re: [PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Bastien Nocera]

On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> Be more explicit and avoid calling sony_set_operational_usb() when we
> have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> 
> While at it, rename the sony_set_operational routines to
> sixaxis_set_operational as they are sixaxis specific.
> 
> This is also in preparation for the sysfs interface to set and get bdaddr
> over usb and for some other Sixaxis report fixup.
> 
> Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>

Signed-off-by: Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>

> ---
>  drivers/hid/hid-sony.c |   28 ++++++++++++++--------------
>  1 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 402d557..d61f268 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -24,7 +24,9 @@
>  
>  #include "hid-ids.h"
>  
> -#define VAIO_RDESC_CONSTANT 0x0001
> +#define VAIO_RDESC_CONSTANT     (1 << 0)
> +#define SIXAXIS_CONTROLLER_USB  (1 << 1)
> +#define SIXAXIS_CONTROLLER_BT   (1 << 2)
>  
>  struct sony_sc {
>  	unsigned long quirks;
> @@ -49,7 +51,7 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>   * to "operational".  Without this, the ps3 controller will not report any
>   * events.
>   */
> -static int sony_set_operational_usb(struct hid_device *hdev)
> +static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  {
>  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>  	struct usb_device *dev = interface_to_usbdev(intf);
> @@ -74,7 +76,7 @@ static int sony_set_operational_usb(struct hid_device *hdev)
>  	return ret;
>  }
>  
> -static int sony_set_operational_bt(struct hid_device *hdev)
> +static int sixaxis_set_operational_bt(struct hid_device *hdev)
>  {
>  	unsigned char buf[] = { 0xf4,  0x42, 0x03, 0x00, 0x00 };
>  	return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> @@ -108,16 +110,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		goto err_free;
>  	}
>  
> -	switch (hdev->bus) {
> -	case BUS_USB:
> -		ret = sony_set_operational_usb(hdev);
> -		break;
> -	case BUS_BLUETOOTH:
> -		ret = sony_set_operational_bt(hdev);
> -		break;
> -	default:
> +	if (sc->quirks & SIXAXIS_CONTROLLER_USB)
> +		ret = sixaxis_set_operational_usb(hdev);
> +	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> +		ret = sixaxis_set_operational_bt(hdev);
> +	else
>  		ret = 0;
> -	}
>  
>  	if (ret < 0)
>  		goto err_stop;
> @@ -137,8 +135,10 @@ static void sony_remove(struct hid_device *hdev)
>  }
>  
>  static const struct hid_device_id sony_devices[] = {
> -	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> -	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
> +		.driver_data = SIXAXIS_CONTROLLER_USB },
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER),
> +		.driver_data = SIXAXIS_CONTROLLER_BT },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE),
>  		.driver_data = VAIO_RDESC_CONSTANT },
>  	{ }

Re: [PATCH 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Bastien Nocera]

On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> Expose to userspace a simple way to get device bdaddr, and get/set master
> bdaddr on Sixaxis controller.
> 
> Right now userspace softwares which manage pairing the controller with a
> bluetooth adapter, rely on libusb and hence have to detach and reattach
> usbhid, which is not very nice.
> 
> Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> ---
> 
> As said, ideally this should be done with an interface such as hidraw, but its
> limitations prevent us from doing it the right way; and I am not going to
> touch hidraw myself anytime soon.

Rest looks alright to me. Thanks!

>  drivers/hid/hid-sony.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 145 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index d61f268..1b611ec 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -47,6 +47,131 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  }
>  
>  /*
> + * Show and set the master bdaddr for PS3 controller, without disconnecting
> + * the device.
> + */
> +static ssize_t show_sixaxis_master_bdaddr(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
> +
> +	if (!mbuf)
> +		return -ENOMEM;
> +
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				 HID_REQ_GET_REPORT,
> +				 USB_DIR_IN | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
> +				 USB_CTRL_GET_TIMEOUT);
> +	if (ret < 0)
> +		dev_err(dev, "%s failed to get master bdaddr, ret: %d\n",
> +				__func__, ret);
> +	else
> +		/* 18 is strlen("00:00:00:00:00:00\n") */

Could you please check for the actual returned length of mbuf here?

> +		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
> +				mbuf[2], mbuf[3], mbuf[4],
> +				mbuf[5], mbuf[6], mbuf[7]);
> +
> +	kfree(mbuf);
> +
> +	return ret;
> +}
> +
> +static ssize_t store_sixaxis_master_bdaddr(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);

Check for count != 6 and bail?
And also check for mbuf != NULL.

> +	mbuf[0] = 0x01;
> +	mbuf[1] = 0x00;
> +	ret = sscanf(buf, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
> +			&mbuf[2], &mbuf[3], &mbuf[4],
> +			&mbuf[5], &mbuf[6], &mbuf[7]);
> +	if (ret != 6) {
> +		dev_err(dev, "%s failed, ret: %d\n", __func__, ret);
> +		return -EINVAL;
> +	}
> +
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +				 HID_REQ_SET_REPORT,
> +				 USB_DIR_OUT | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
> +				 USB_CTRL_GET_TIMEOUT);
> +
> +	kfree(mbuf);
> +
> +	if (ret < 0) {
> +		dev_err(dev, "%s failed to set master bdaddr, ret: %d\n",
> +				__func__, ret);
> +		return ret;
> +	}
> +
> +	return count;
> +}
> +
> +/*
> + * Show the bdaddr for PS3 controller, without disconnecting the device.
> + */
> +static ssize_t show_sixaxis_bdaddr(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(18, GFP_KERNEL);
> +
> +	if (!mbuf)
> +		return -ENOMEM;
> +
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				 HID_REQ_GET_REPORT,
> +				 USB_DIR_IN | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf2, ifnum, mbuf, 17,
> +				 USB_CTRL_GET_TIMEOUT);
> +	if (ret < 0)
> +		dev_err(dev, "%s failed to get device bdaddr, ret: %d\n",
> +				__func__, ret);
> +	else
> +		/* 18 is strlen("00:00:00:00:00:00\n") */

length of mbuf != 9?

> +		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
> +				mbuf[4], mbuf[5], mbuf[6],
> +				mbuf[7], mbuf[8], mbuf[9]);
> +
> +	kfree(mbuf);
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR(sixaxis_master_bdaddr, S_IWUSR|S_IRUGO,
> +		show_sixaxis_master_bdaddr, store_sixaxis_master_bdaddr);
> +
> +static DEVICE_ATTR(sixaxis_bdaddr, S_IRUGO,
> +		show_sixaxis_bdaddr, NULL);
> +
> +static struct attribute *sixaxis_attributes[] = {
> +	&dev_attr_sixaxis_master_bdaddr.attr,
> +	&dev_attr_sixaxis_bdaddr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sixaxis_attr_group = {
> +	.attrs = sixaxis_attributes,
> +};
> +
> +
> +/*
>   * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
>   * to "operational".  Without this, the ps3 controller will not report any
>   * events.
> @@ -57,8 +182,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  	struct usb_device *dev = interface_to_usbdev(intf);
>  	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
>  	int ret;
> -	char *buf = kmalloc(18, GFP_KERNEL);
> +	unsigned char *buf = kmalloc(18, GFP_KERNEL);

buf != NULL.

> +	dev_info(&hdev->dev, "Calling %s\n", __func__);
>  	if (!buf)
>  		return -ENOMEM;
>  
> @@ -70,6 +196,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  				 USB_CTRL_GET_TIMEOUT);
>  	if (ret < 0)
>  		dev_err(&hdev->dev, "can't set operational mode\n");
> +	else
> +		dev_info(&hdev->dev, "Sony PS3 Controller bdaddr: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +				buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
>  
>  	kfree(buf);
>  
> @@ -110,9 +239,17 @@ 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) {
> +		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +		ret = sysfs_create_group(&intf->dev.kobj, &sixaxis_attr_group);
> +		if (ret < 0) {
> +			dev_err(&hdev->dev,
> +				"cannot register sixaxis sysfs hooks\n");
> +			goto err_stop;
> +		}
> +
>  		ret = sixaxis_set_operational_usb(hdev);
> -	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> +	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
>  		ret = sixaxis_set_operational_bt(hdev);
>  	else
>  		ret = 0;
> @@ -130,6 +267,11 @@ err_free:
>  
>  static void sony_remove(struct hid_device *hdev)
>  {
> +	struct sony_sc *sc = hid_get_drvdata(hdev);
> +	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> +		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +		sysfs_remove_group(&intf->dev.kobj, &sixaxis_attr_group);
> +	}
>  	hid_hw_stop(hdev);
>  	kfree(hid_get_drvdata(hdev));
>  }

Re: [PATCH 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Bastien Nocera]

On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> Expose to userspace a simple way to get device bdaddr, and get/set master
> bdaddr on Sixaxis controller.
> 
> Right now userspace softwares which manage pairing the controller with a
> bluetooth adapter, rely on libusb and hence have to detach and reattach
> usbhid, which is not very nice.
> 
> Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
> ---
> 
> As said, ideally this should be done with an interface such as hidraw, but its
> limitations prevent us from doing it the right way; and I am not going to
> touch hidraw myself anytime soon.

Rest looks alright to me. Thanks!

>  drivers/hid/hid-sony.c |  148 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 145 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index d61f268..1b611ec 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -47,6 +47,131 @@ static void sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  }
>  
>  /*
> + * Show and set the master bdaddr for PS3 controller, without disconnecting
> + * the device.
> + */
> +static ssize_t show_sixaxis_master_bdaddr(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);
> +
> +	if (!mbuf)
> +		return -ENOMEM;
> +
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				 HID_REQ_GET_REPORT,
> +				 USB_DIR_IN | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
> +				 USB_CTRL_GET_TIMEOUT);
> +	if (ret < 0)
> +		dev_err(dev, "%s failed to get master bdaddr, ret: %d\n",
> +				__func__, ret);
> +	else
> +		/* 18 is strlen("00:00:00:00:00:00\n") */

Could you please check for the actual returned length of mbuf here?

> +		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
> +				mbuf[2], mbuf[3], mbuf[4],
> +				mbuf[5], mbuf[6], mbuf[7]);
> +
> +	kfree(mbuf);
> +
> +	return ret;
> +}
> +
> +static ssize_t store_sixaxis_master_bdaddr(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(9, GFP_KERNEL);

Check for count != 6 and bail?
And also check for mbuf != NULL.

> +	mbuf[0] = 0x01;
> +	mbuf[1] = 0x00;
> +	ret = sscanf(buf, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
> +			&mbuf[2], &mbuf[3], &mbuf[4],
> +			&mbuf[5], &mbuf[6], &mbuf[7]);
> +	if (ret != 6) {
> +		dev_err(dev, "%s failed, ret: %d\n", __func__, ret);
> +		return -EINVAL;
> +	}
> +
> +	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> +				 HID_REQ_SET_REPORT,
> +				 USB_DIR_OUT | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf5, ifnum, mbuf, 8,
> +				 USB_CTRL_GET_TIMEOUT);
> +
> +	kfree(mbuf);
> +
> +	if (ret < 0) {
> +		dev_err(dev, "%s failed to set master bdaddr, ret: %d\n",
> +				__func__, ret);
> +		return ret;
> +	}
> +
> +	return count;
> +}
> +
> +/*
> + * Show the bdaddr for PS3 controller, without disconnecting the device.
> + */
> +static ssize_t show_sixaxis_bdaddr(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct usb_interface *intf = to_usb_interface(dev);
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	int ret;
> +	unsigned char *mbuf = kmalloc(18, GFP_KERNEL);
> +
> +	if (!mbuf)
> +		return -ENOMEM;
> +
> +	ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
> +				 HID_REQ_GET_REPORT,
> +				 USB_DIR_IN | USB_TYPE_CLASS |
> +				 USB_RECIP_INTERFACE,
> +				 (3 << 8) | 0xf2, ifnum, mbuf, 17,
> +				 USB_CTRL_GET_TIMEOUT);
> +	if (ret < 0)
> +		dev_err(dev, "%s failed to get device bdaddr, ret: %d\n",
> +				__func__, ret);
> +	else
> +		/* 18 is strlen("00:00:00:00:00:00\n") */

length of mbuf != 9?

> +		ret = snprintf(buf, 18, "%02x:%02x:%02x:%02x:%02x:%02x\n",
> +				mbuf[4], mbuf[5], mbuf[6],
> +				mbuf[7], mbuf[8], mbuf[9]);
> +
> +	kfree(mbuf);
> +
> +	return ret;
> +}
> +
> +static DEVICE_ATTR(sixaxis_master_bdaddr, S_IWUSR|S_IRUGO,
> +		show_sixaxis_master_bdaddr, store_sixaxis_master_bdaddr);
> +
> +static DEVICE_ATTR(sixaxis_bdaddr, S_IRUGO,
> +		show_sixaxis_bdaddr, NULL);
> +
> +static struct attribute *sixaxis_attributes[] = {
> +	&dev_attr_sixaxis_master_bdaddr.attr,
> +	&dev_attr_sixaxis_bdaddr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sixaxis_attr_group = {
> +	.attrs = sixaxis_attributes,
> +};
> +
> +
> +/*
>   * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
>   * to "operational".  Without this, the ps3 controller will not report any
>   * events.
> @@ -57,8 +182,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  	struct usb_device *dev = interface_to_usbdev(intf);
>  	__u16 ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
>  	int ret;
> -	char *buf = kmalloc(18, GFP_KERNEL);
> +	unsigned char *buf = kmalloc(18, GFP_KERNEL);

buf != NULL.

> +	dev_info(&hdev->dev, "Calling %s\n", __func__);
>  	if (!buf)
>  		return -ENOMEM;
>  
> @@ -70,6 +196,9 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
>  				 USB_CTRL_GET_TIMEOUT);
>  	if (ret < 0)
>  		dev_err(&hdev->dev, "can't set operational mode\n");
> +	else
> +		dev_info(&hdev->dev, "Sony PS3 Controller bdaddr: %02x:%02x:%02x:%02x:%02x:%02x\n",
> +				buf[4], buf[5], buf[6], buf[7], buf[8], buf[9]);
>  
>  	kfree(buf);
>  
> @@ -110,9 +239,17 @@ 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) {
> +		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +		ret = sysfs_create_group(&intf->dev.kobj, &sixaxis_attr_group);
> +		if (ret < 0) {
> +			dev_err(&hdev->dev,
> +				"cannot register sixaxis sysfs hooks\n");
> +			goto err_stop;
> +		}
> +
>  		ret = sixaxis_set_operational_usb(hdev);
> -	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> +	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
>  		ret = sixaxis_set_operational_bt(hdev);
>  	else
>  		ret = 0;
> @@ -130,6 +267,11 @@ err_free:
>  
>  static void sony_remove(struct hid_device *hdev)
>  {
> +	struct sony_sc *sc = hid_get_drvdata(hdev);
> +	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> +		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> +		sysfs_remove_group(&intf->dev.kobj, &sixaxis_attr_group);
> +	}
>  	hid_hw_stop(hdev);
>  	kfree(hid_get_drvdata(hdev));
>  }

Re: [PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Antonio Ospite]

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

On Wed, 01 Sep 2010 17:13:35 +0100
Bastien Nocera <hadess@hadess.net> wrote:

> On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> > Be more explicit and avoid calling sony_set_operational_usb() when we
> > have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> > 
> > While at it, rename the sony_set_operational routines to
> > sixaxis_set_operational as they are sixaxis specific.
> > 
> > This is also in preparation for the sysfs interface to set and get bdaddr
> > over usb and for some other Sixaxis report fixup.
> > 
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
>

Thanks Bastien for bringing this up again, I was going to repost
this patch sometime this month.

Added Jiri to CC as I forgot to do that back then.

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] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Antonio Ospite]

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

On Wed, 01 Sep 2010 17:13:35 +0100
Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org> wrote:

> On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> > Be more explicit and avoid calling sony_set_operational_usb() when we
> > have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> > 
> > While at it, rename the sony_set_operational routines to
> > sixaxis_set_operational as they are sixaxis specific.
> > 
> > This is also in preparation for the sysfs interface to set and get bdaddr
> > over usb and for some other Sixaxis report fixup.
> > 
> > Signed-off-by: Antonio Ospite <ospite-aNJ+ML1ZbiP93QAQaVx+gl6hYfS7NtTn@public.gmane.org>
> 
> Signed-off-by: Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
>

Thanks Bastien for bringing this up again, I was going to repost
this patch sometime this month.

Added Jiri to CC as I forgot to do that back then.

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 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Antonio Ospite]

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

On Wed, 01 Sep 2010 17:17:54 +0100
Bastien Nocera <hadess@hadess.net> wrote:

> On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> > Expose to userspace a simple way to get device bdaddr, and get/set master
> > bdaddr on Sixaxis controller.
> > 
> > Right now userspace softwares which manage pairing the controller with a
> > bluetooth adapter, rely on libusb and hence have to detach and reattach
> > usbhid, which is not very nice.
> > 
> > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > ---
> > 
> > As said, ideally this should be done with an interface such as hidraw, but its
> > limitations prevent us from doing it the right way; and I am not going to
> > touch hidraw myself anytime soon.
> 
> Rest looks alright to me. Thanks!

Bastien, Jiri, this patch can be DISCARDED. The hidraw work from Alan
Ott (waiting for Marcel to ACK the bluetooth part) will make this kernel
approach overkill.

I already have some bluez patches based on Alan's work, but I am waiting
for the needed changes to land into linus tree before submitting them.

Sorry for making you wasting some time reviewing this one, I told you on
IRC it could be discarded, but we know: "IRCs volant, emails manent".

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 2/2] hid/hid-sony: get and set Sixaxis bdaddr via sysfs

[Bastien Nocera]

On Wed, 2010-09-01 at 19:28 +0200, Antonio Ospite wrote:
> On Wed, 01 Sep 2010 17:17:54 +0100
> Bastien Nocera <hadess@hadess.net> wrote:
> 
> > On Mon, 2010-05-03 at 22:15 +0200, Antonio Ospite wrote:
> > > Expose to userspace a simple way to get device bdaddr, and get/set master
> > > bdaddr on Sixaxis controller.
> > > 
> > > Right now userspace softwares which manage pairing the controller with a
> > > bluetooth adapter, rely on libusb and hence have to detach and reattach
> > > usbhid, which is not very nice.
> > > 
> > > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > > ---
> > > 
> > > As said, ideally this should be done with an interface such as hidraw, but its
> > > limitations prevent us from doing it the right way; and I am not going to
> > > touch hidraw myself anytime soon.
> > 
> > Rest looks alright to me. Thanks!
> 
> Bastien, Jiri, this patch can be DISCARDED. The hidraw work from Alan
> Ott (waiting for Marcel to ACK the bluetooth part) will make this kernel
> approach overkill.
> 
> I already have some bluez patches based on Alan's work, but I am waiting
> for the needed changes to land into linus tree before submitting them.
> 
> Sorry for making you wasting some time reviewing this one, I told you on
> IRC it could be discarded, but we know: "IRCs volant, emails manent".

Given the time it took me to review it, it's my own damn fault anyway ;)

Re: [PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Jiri Kosina]

On Wed, 1 Sep 2010, Antonio Ospite wrote:

> > > Be more explicit and avoid calling sony_set_operational_usb() when we
> > > have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> > > 
> > > While at it, rename the sony_set_operational routines to
> > > sixaxis_set_operational as they are sixaxis specific.
> > > 
> > > This is also in preparation for the sysfs interface to set and get bdaddr
> > > over usb and for some other Sixaxis report fixup.
> > > 
> > > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > 
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> >
> 
> Thanks Bastien for bringing this up again, I was going to repost
> this patch sometime this month.
> 
> Added Jiri to CC as I forgot to do that back then.

Thanks, I have missed that one on linux-input@.

Applied now.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 1/2] hid/hid-sony: Apply sixaxis quirks only to sixaxis

[Antonio Ospite]

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

On Thu, 2 Sep 2010 09:41:29 +0200 (CEST)
Jiri Kosina <jkosina@suse.cz> wrote:

> On Wed, 1 Sep 2010, Antonio Ospite wrote:
> 
> > > > Be more explicit and avoid calling sony_set_operational_usb() when we
> > > > have USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE.
> > > > 
> > > > While at it, rename the sony_set_operational routines to
> > > > sixaxis_set_operational as they are sixaxis specific.
> > > > 
> > > > This is also in preparation for the sysfs interface to set and get bdaddr
> > > > over usb and for some other Sixaxis report fixup.
> > > > 

I have just realized that the commit message mentions the now obsolete
sysfs interface, maybe Jiri can remove the last paragraph from it? Not
a big deal anyway.

> > > > Signed-off-by: Antonio Ospite <ospite@studenti.unina.it>
> > > 
> > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > >
> > 
> > Thanks Bastien for bringing this up again, I was going to repost
> > this patch sometime this month.
> > 
> > Added Jiri to CC as I forgot to do that back then.
> 
> Thanks, I have missed that one on linux-input@.
> 
> Applied now.
> 

Thanks Jiri.

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