This emulates a Sony BD Remote for the Logitech Harmony series of
universal remotes.
--
profiles/input/fakehid.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/profiles/input/fakehid.c b/profiles/input/fakehid.c
index 3be1489..dd47287 100644
--- a/profiles/input/fakehid.c
+++ b/profiles/input/fakehid.c
@@ -342,6 +342,16 @@ static struct fake_hid fake_hid_table[] = {
.setup_uinput = ps3remote_setup_uinput,
.devices = NULL,
},
+ /* Logitech Harmony Adapter for PS3 */
+ {
+ .vendor = 0x046d,
+ .product = 0x0306,
+ .connect = fake_hid_common_connect,
+ .disconnect = fake_hid_common_disconnect,
+ .event = ps3remote_event,
+ .setup_uinput = ps3remote_setup_uinput,
+ .devices = NULL,
+ },
{ },
};
From: David Dillow <[email protected]>
The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
BlueZ project's user space, but the code that handled it was recently
removed as its functionality conflicted with a real HSP implementation
and the mapping was thought to be better handled in the kernel. This is
a port of the mapping logic from the fakehid driver by Marcel Holtmann
to the in-kernel HID layer.
We also add support for the Logitech Harmony Adapter for PS3, which
emulates the BD Remote.
Signed-off-by: David Dillow <[email protected]>
Signed-off-by: Antonio Ospite <[email protected]>
---
Changes since v3:
- move the size check into the switch statement.
Thanks,
Antonio
drivers/hid/Kconfig | 13 ++-
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 2 +
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-ps3remote.c | 215 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 232 insertions(+), 1 deletion(-)
create mode 100644 drivers/hid/hid-ps3remote.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index fbf4950..378be0b 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -534,6 +534,15 @@ config HID_PRIMAX
Support for Primax devices that are not fully compliant with the
HID standard.
+config HID_PS3REMOTE
+ tristate "Sony PS3 BD Remote Control"
+ depends on BT_HIDP
+ ---help---
+ Support for the Sony PS3 Blue-ray Disk Remote Control and Logitech
+ Harmony Adapter for PS3, which connect over Bluetooth.
+
+ Support for the 6-axis controllers is provided by HID_SONY.
+
config HID_ROCCAT
tristate "Roccat device support"
depends on USB_HID
@@ -561,7 +570,9 @@ config HID_SONY
tristate "Sony PS3 controller"
depends on USB_HID
---help---
- Support for Sony PS3 controller.
+ Support for Sony PS3 6-axis controllers.
+
+ Support for the Sony PS3 BD Remote is provided by HID_PS3REMOTE.
config HID_SPEEDLINK
tristate "Speedlink VAD Cezanne mouse support"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index f975485..333ed6c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
+obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8bcd168..e4275d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1566,6 +1566,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
@@ -1639,6 +1640,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SKYCABLE, USB_DEVICE_ID_SKYCABLE_WIRELESS_PRESENTER) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1dcb76f..40411c9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -496,6 +496,7 @@
#define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
#define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
#define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f
+#define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD 0xc20a
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD 0xc211
#define USB_DEVICE_ID_LOGITECH_EXTREME_3D 0xc215
@@ -683,6 +684,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_PS3_BDREMOTE 0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
new file mode 100644
index 0000000..03811e5
--- /dev/null
+++ b/drivers/hid/hid-ps3remote.c
@@ -0,0 +1,215 @@
+/*
+ * HID driver for Sony PS3 BD Remote Control
+ *
+ * Copyright (c) 2012 David Dillow <[email protected]>
+ * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
+ * and other kernel HID drivers.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+/* NOTE: in order for the Sony PS3 BD Remote Control to be found by
+ * a Bluetooth host, the key combination Start+Enter has to be kept pressed
+ * for about 7 seconds with the Bluetooth Host Controller in discovering mode.
+ *
+ * There will be no PIN request from the device.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+static __u8 ps3remote_rdesc[] = {
+ 0x05, 0x01, /* GUsagePage Generic Desktop */
+ 0x09, 0x05, /* LUsage 0x05 [Game Pad] */
+ 0xA1, 0x01, /* MCollection Application (mouse, keyboard) */
+
+ /* Use collection 1 for joypad buttons */
+ 0xA1, 0x02, /* MCollection Logical (interrelated data) */
+
+ /* Ignore the 1st byte, maybe it is used for a controller
+ * number but it's not needed for correct operation */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
+
+ /* Bytes from 2nd to 4th are a bitmap for joypad buttons, for these
+ * buttons multiple keypresses are allowed */
+ 0x05, 0x09, /* GUsagePage Button */
+ 0x19, 0x01, /* LUsageMinimum 0x01 [Button 1 (primary/trigger)] */
+ 0x29, 0x18, /* LUsageMaximum 0x18 [Button 24] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x25, 0x01, /* GLogicalMaximum 0x01 [1] */
+ 0x75, 0x01, /* GReportSize 0x01 [1] */
+ 0x95, 0x18, /* GReportCount 0x18 [24] */
+ 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
+
+ 0xC0, /* MEndCollection */
+
+ /* Use collection 2 for remote control buttons */
+ 0xA1, 0x02, /* MCollection Logical (interrelated data) */
+
+ /* 5th byte is used for remote control buttons */
+ 0x05, 0x09, /* GUsagePage Button */
+ 0x18, /* LUsageMinimum [No button pressed] */
+ 0x29, 0xFE, /* LUsageMaximum 0xFE [Button 254] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x26, 0xFE, 0x00, /* GLogicalMaximum 0x00FE [254] */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x80, /* MInput */
+
+ /* Ignore bytes from 6th to 11th, 6th to 10th are always constant at
+ * 0xff and 11th is for press indication */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x06, /* GReportCount 0x06 [6] */
+ 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
+
+ /* 12th byte is for battery strength */
+ 0x05, 0x06, /* GUsagePage Generic Device Controls */
+ 0x09, 0x20, /* LUsage 0x20 [Battery Strength] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x25, 0x05, /* GLogicalMaximum 0x05 [5] */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
+
+ 0xC0, /* MEndCollection */
+
+ 0xC0 /* MEndCollection [Game Pad] */
+};
+
+static const unsigned int ps3remote_keymap_joypad_buttons[] = {
+ [0x01] = KEY_SELECT,
+ [0x02] = BTN_THUMBL, /* L3 */
+ [0x03] = BTN_THUMBR, /* R3 */
+ [0x04] = BTN_START,
+ [0x05] = KEY_UP,
+ [0x06] = KEY_RIGHT,
+ [0x07] = KEY_DOWN,
+ [0x08] = KEY_LEFT,
+ [0x09] = BTN_TL2, /* L2 */
+ [0x0a] = BTN_TR2, /* R2 */
+ [0x0b] = BTN_TL, /* L1 */
+ [0x0c] = BTN_TR, /* R1 */
+ [0x0d] = KEY_OPTION, /* options/triangle */
+ [0x0e] = KEY_BACK, /* back/circle */
+ [0x0f] = BTN_0, /* cross */
+ [0x10] = KEY_SCREEN, /* view/square */
+ [0x11] = KEY_HOMEPAGE, /* PS button */
+ [0x14] = KEY_ENTER,
+};
+static const unsigned int ps3remote_keymap_remote_buttons[] = {
+ [0x00] = KEY_1,
+ [0x01] = KEY_2,
+ [0x02] = KEY_3,
+ [0x03] = KEY_4,
+ [0x04] = KEY_5,
+ [0x05] = KEY_6,
+ [0x06] = KEY_7,
+ [0x07] = KEY_8,
+ [0x08] = KEY_9,
+ [0x09] = KEY_0,
+ [0x0e] = KEY_ESC, /* return */
+ [0x0f] = KEY_CLEAR,
+ [0x16] = KEY_EJECTCD,
+ [0x1a] = KEY_MENU, /* top menu */
+ [0x28] = KEY_TIME,
+ [0x30] = KEY_PREVIOUS,
+ [0x31] = KEY_NEXT,
+ [0x32] = KEY_PLAY,
+ [0x33] = KEY_REWIND, /* scan back */
+ [0x34] = KEY_FORWARD, /* scan forward */
+ [0x38] = KEY_STOP,
+ [0x39] = KEY_PAUSE,
+ [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
+ [0x60] = KEY_FRAMEBACK, /* slow/step back */
+ [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
+ [0x63] = KEY_SUBTITLE,
+ [0x64] = KEY_AUDIO,
+ [0x65] = KEY_ANGLE,
+ [0x70] = KEY_INFO, /* display */
+ [0x80] = KEY_BLUE,
+ [0x81] = KEY_RED,
+ [0x82] = KEY_GREEN,
+ [0x83] = KEY_YELLOW,
+};
+
+static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ *rsize = sizeof(ps3remote_rdesc);
+ return ps3remote_rdesc;
+}
+
+static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ unsigned int key = usage->hid & HID_USAGE;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
+ return -1;
+
+ switch (usage->collection_index) {
+ case 1:
+ if (key >= ARRAY_SIZE(ps3remote_keymap_joypad_buttons))
+ return -1;
+
+ key = ps3remote_keymap_joypad_buttons[key];
+ if (!key)
+ return -1;
+ break;
+ case 2:
+ if (key >= ARRAY_SIZE(ps3remote_keymap_remote_buttons))
+ return -1;
+
+ key = ps3remote_keymap_remote_buttons[key];
+ if (!key)
+ return -1;
+ break;
+ default:
+ return -1;
+ }
+
+ hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
+ return 1;
+}
+
+static const struct hid_device_id ps3remote_devices[] = {
+ /* PS3 BD Remote Control */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
+ /* Logitech Harmony Adapter for PS3 */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, ps3remote_devices);
+
+static struct hid_driver ps3remote_driver = {
+ .name = "ps3_remote",
+ .id_table = ps3remote_devices,
+ .report_fixup = ps3remote_fixup,
+ .input_mapping = ps3remote_mapping,
+};
+
+static int __init ps3remote_init(void)
+{
+ return hid_register_driver(&ps3remote_driver);
+}
+
+static void __exit ps3remote_exit(void)
+{
+ hid_unregister_driver(&ps3remote_driver);
+}
+
+module_init(ps3remote_init);
+module_exit(ps3remote_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Dillow <[email protected]>, Antonio Ospite <[email protected]>");
--
1.7.10.4
On Tue, 2012-09-25 at 16:30 +0200, Antonio Ospite wrote:
> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + unsigned int key = usage->hid & HID_USAGE;
The size check below should be moved into the switch statement, and
modified for each, as you could access beyond the joypad_buttons array
as-is. The usage page check can stay outside the switch.
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
> + key >= ARRAY_SIZE(ps3remote_keymap_remote_buttons))
> + return -1;
> +
> + switch (usage->collection_index) {
> + case 1:
> + key = ps3remote_keymap_joypad_buttons[key];
> + if (!key)
> + return -1;
> + break;
> + case 2:
> + key = ps3remote_keymap_remote_buttons[key];
> + if (!key)
> + return -1;
> + break;
> + default:
> + return -1;
> + }
> +
> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> + return 1;
> +}
From: David Dillow <[email protected]>
The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
BlueZ project's user space, but the code that handled it was recently
removed as its functionality conflicted with a real HSP implementation
and the mapping was thought to be better handled in the kernel. This is
a port of the mapping logic from the fakehid driver by Marcel Holtmann
to the in-kernel HID layer.
We also add support for the Logitech Harmony Adapter for PS3, which
emulates the BD Remote.
Signed-off-by: David Dillow <[email protected]>
Signed-off-by: Antonio Ospite <[email protected]>
---
drivers/hid/Kconfig | 13 ++-
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 2 +
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-ps3remote.c | 210 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 227 insertions(+), 1 deletion(-)
create mode 100644 drivers/hid/hid-ps3remote.c
Changes since v2:
- new HID descriptor to handle multiple keypresses for joypad buttons,
the descriptor has been designed with gHID:
http://code.google.com/p/ghid/
Descriptor changes:
+ Mark the first byte as constant
+ Describe bytes 2nd to 4th used for joypad buttons
+ Split the constant 0xff bytes (bytes from 6th to 10th) so they
are marked as constant in the descriptor too
+ split the data into two collections, to map them
independently
- move the descriptor definition before the keymaps, as the keymaps refer
to UsageMinimum and UsageMaximum as defined in the descriptor
- sort keymaps according to their index, as this reflects the Usage
number
- use spaces to align stuff in ps3remote_driver struct
- add myself in the author field
- reformat description in Kconfig
- sort symbolic constants trying to match the ordering criterion used
for other entries
- add a note about the setup maneuver
TODO (but I think we could merge the driver as it is now regardless):
- verify that battery strength is reported via the power_supply class
- see how to handle disconnection on idle timeout
Jiri, are we still in time for 3.7 ?
Thanks,
Antonio Ospite
http://ao2.it
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index fbf4950..378be0b 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -534,6 +534,15 @@ config HID_PRIMAX
Support for Primax devices that are not fully compliant with the
HID standard.
+config HID_PS3REMOTE
+ tristate "Sony PS3 BD Remote Control"
+ depends on BT_HIDP
+ ---help---
+ Support for the Sony PS3 Blue-ray Disk Remote Control and Logitech
+ Harmony Adapter for PS3, which connect over Bluetooth.
+
+ Support for the 6-axis controllers is provided by HID_SONY.
+
config HID_ROCCAT
tristate "Roccat device support"
depends on USB_HID
@@ -561,7 +570,9 @@ config HID_SONY
tristate "Sony PS3 controller"
depends on USB_HID
---help---
- Support for Sony PS3 controller.
+ Support for Sony PS3 6-axis controllers.
+
+ Support for the Sony PS3 BD Remote is provided by HID_PS3REMOTE.
config HID_SPEEDLINK
tristate "Speedlink VAD Cezanne mouse support"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index f975485..333ed6c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
+obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 8bcd168..e4275d4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1566,6 +1566,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
@@ -1639,6 +1640,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SKYCABLE, USB_DEVICE_ID_SKYCABLE_WIRELESS_PRESENTER) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1dcb76f..40411c9 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -496,6 +496,7 @@
#define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
#define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
#define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f
+#define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD 0xc20a
#define USB_DEVICE_ID_LOGITECH_RUMBLEPAD 0xc211
#define USB_DEVICE_ID_LOGITECH_EXTREME_3D 0xc215
@@ -683,6 +684,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
+#define USB_DEVICE_ID_SONY_PS3_BDREMOTE 0x0306
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
new file mode 100644
index 0000000..4c92e24
--- /dev/null
+++ b/drivers/hid/hid-ps3remote.c
@@ -0,0 +1,210 @@
+/*
+ * HID driver for Sony PS3 BD Remote Control
+ *
+ * Copyright (c) 2012 David Dillow <[email protected]>
+ * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
+ * and other kernel HID drivers.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+/* NOTE: in order for the Sony PS3 BD Remote Control to be found by
+ * a Bluetooth host, the key combination Start+Enter has to be kept pressed
+ * for about 7 seconds with the Bluetooth Host Controller in discovering mode.
+ *
+ * There will be no PIN request from the device.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+static __u8 ps3remote_rdesc[] = {
+ 0x05, 0x01, /* GUsagePage Generic Desktop */
+ 0x09, 0x05, /* LUsage 0x05 [Game Pad] */
+ 0xA1, 0x01, /* MCollection Application (mouse, keyboard) */
+
+ /* Use collection 1 for joypad buttons */
+ 0xA1, 0x02, /* MCollection Logical (interrelated data) */
+
+ /* Ignore the 1st byte, maybe it is used for a controller
+ * number but it's not needed for correct operation */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
+
+ /* Bytes from 2nd to 4th are a bitmap for joypad buttons, for these
+ * buttons multiple keypresses are allowed */
+ 0x05, 0x09, /* GUsagePage Button */
+ 0x19, 0x01, /* LUsageMinimum 0x01 [Button 1 (primary/trigger)] */
+ 0x29, 0x18, /* LUsageMaximum 0x18 [Button 24] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x25, 0x01, /* GLogicalMaximum 0x01 [1] */
+ 0x75, 0x01, /* GReportSize 0x01 [1] */
+ 0x95, 0x18, /* GReportCount 0x18 [24] */
+ 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
+
+ 0xC0, /* MEndCollection */
+
+ /* Use collection 2 for remote control buttons */
+ 0xA1, 0x02, /* MCollection Logical (interrelated data) */
+
+ /* 5th byte is used for remote control buttons */
+ 0x05, 0x09, /* GUsagePage Button */
+ 0x18, /* LUsageMinimum [No button pressed] */
+ 0x29, 0xFE, /* LUsageMaximum 0xFE [Button 254] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x26, 0xFE, 0x00, /* GLogicalMaximum 0x00FE [254] */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x80, /* MInput */
+
+ /* Ignore bytes from 6th to 11th, 6th to 10th are always constant at
+ * 0xff and 11th is for press indication */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x06, /* GReportCount 0x06 [6] */
+ 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
+
+ /* 12th byte is for battery strength */
+ 0x05, 0x06, /* GUsagePage Generic Device Controls */
+ 0x09, 0x20, /* LUsage 0x20 [Battery Strength] */
+ 0x14, /* GLogicalMinimum [0] */
+ 0x25, 0x05, /* GLogicalMaximum 0x05 [5] */
+ 0x75, 0x08, /* GReportSize 0x08 [8] */
+ 0x95, 0x01, /* GReportCount 0x01 [1] */
+ 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
+
+ 0xC0, /* MEndCollection */
+
+ 0xC0 /* MEndCollection [Game Pad] */
+};
+
+static const unsigned int ps3remote_keymap_joypad_buttons[] = {
+ [0x01] = KEY_SELECT,
+ [0x02] = BTN_THUMBL, /* L3 */
+ [0x03] = BTN_THUMBR, /* R3 */
+ [0x04] = BTN_START,
+ [0x05] = KEY_UP,
+ [0x06] = KEY_RIGHT,
+ [0x07] = KEY_DOWN,
+ [0x08] = KEY_LEFT,
+ [0x09] = BTN_TL2, /* L2 */
+ [0x0a] = BTN_TR2, /* R2 */
+ [0x0b] = BTN_TL, /* L1 */
+ [0x0c] = BTN_TR, /* R1 */
+ [0x0d] = KEY_OPTION, /* options/triangle */
+ [0x0e] = KEY_BACK, /* back/circle */
+ [0x0f] = BTN_0, /* cross */
+ [0x10] = KEY_SCREEN, /* view/square */
+ [0x11] = KEY_HOMEPAGE, /* PS button */
+ [0x14] = KEY_ENTER,
+};
+static const unsigned int ps3remote_keymap_remote_buttons[] = {
+ [0x00] = KEY_1,
+ [0x01] = KEY_2,
+ [0x02] = KEY_3,
+ [0x03] = KEY_4,
+ [0x04] = KEY_5,
+ [0x05] = KEY_6,
+ [0x06] = KEY_7,
+ [0x07] = KEY_8,
+ [0x08] = KEY_9,
+ [0x09] = KEY_0,
+ [0x0e] = KEY_ESC, /* return */
+ [0x0f] = KEY_CLEAR,
+ [0x16] = KEY_EJECTCD,
+ [0x1a] = KEY_MENU, /* top menu */
+ [0x28] = KEY_TIME,
+ [0x30] = KEY_PREVIOUS,
+ [0x31] = KEY_NEXT,
+ [0x32] = KEY_PLAY,
+ [0x33] = KEY_REWIND, /* scan back */
+ [0x34] = KEY_FORWARD, /* scan forward */
+ [0x38] = KEY_STOP,
+ [0x39] = KEY_PAUSE,
+ [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
+ [0x60] = KEY_FRAMEBACK, /* slow/step back */
+ [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
+ [0x63] = KEY_SUBTITLE,
+ [0x64] = KEY_AUDIO,
+ [0x65] = KEY_ANGLE,
+ [0x70] = KEY_INFO, /* display */
+ [0x80] = KEY_BLUE,
+ [0x81] = KEY_RED,
+ [0x82] = KEY_GREEN,
+ [0x83] = KEY_YELLOW,
+};
+
+static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ *rsize = sizeof(ps3remote_rdesc);
+ return ps3remote_rdesc;
+}
+
+static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ unsigned int key = usage->hid & HID_USAGE;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
+ key >= ARRAY_SIZE(ps3remote_keymap_remote_buttons))
+ return -1;
+
+ switch (usage->collection_index) {
+ case 1:
+ key = ps3remote_keymap_joypad_buttons[key];
+ if (!key)
+ return -1;
+ break;
+ case 2:
+ key = ps3remote_keymap_remote_buttons[key];
+ if (!key)
+ return -1;
+ break;
+ default:
+ return -1;
+ }
+
+ hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
+ return 1;
+}
+
+static const struct hid_device_id ps3remote_devices[] = {
+ /* PS3 BD Remote Control */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
+ /* Logitech Harmony Adapter for PS3 */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, ps3remote_devices);
+
+static struct hid_driver ps3remote_driver = {
+ .name = "ps3_remote",
+ .id_table = ps3remote_devices,
+ .report_fixup = ps3remote_fixup,
+ .input_mapping = ps3remote_mapping,
+};
+
+static int __init ps3remote_init(void)
+{
+ return hid_register_driver(&ps3remote_driver);
+}
+
+static void __exit ps3remote_exit(void)
+{
+ hid_unregister_driver(&ps3remote_driver);
+}
+
+module_init(ps3remote_init);
+module_exit(ps3remote_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Dillow <[email protected]>, Antonio Ospite <[email protected]>");
--
1.7.10.4
On Mon, 24 Sep 2012 22:13:35 -0400
David Dillow <[email protected]> wrote:
> On Mon, 2012-09-24 at 09:56 -0400, David Dillow wrote:
> > On Mon, 2012-09-24 at 13:25 +0200, Antonio Ospite wrote:
> > > In order to make this work I have to put joypad buttons only in one of
> > > the key maps, I don't know if that is compatible with the Harmony
> > > adapter.
> >
> > I suspect it is, but I'll try to test later tonight. Feel free to merge
> > my patch and yours and push it upstream under your name/copyright; I'm
> > not attached to it.
>
> Yep, this works with the Harmony.
>
Good.
I will send a v3 then, with you as the main author (the From: field) and
with myself added in the MODULE_AUTHOR section, this looks like the best
way to me as you are the initial author. I'd like you to take a final
look at that one tho, and maybe do an actual test too.
I will add the note about the "setup" procedure too.
> I think the logic you are using may result in some combinations being
> missed -- say Triangle + Play -- but I'm not sure how much it matters.
Yes, when pressing something like Triangle+Play the Play pressure is
completely ignored as a 0xff is reported in the 5th byte, but it's the
hardware which behaves that way, the driver is just very adherent to
what the hardware provides.
> I
> can rework the first version (building a custom report based on the
> funky rules of coding) in the next few days, or you can push your
> version if you'd like. Or perhaps a middle way, that erases the buttons
> reported in the bitmap from the byte array, but otherwise keeps track of
> the last press in that area to avoid weirdness when the user presses a
> button combo that cannot be reported due to the HW limitations.
>
I'd say we ignore this fact and not try to be too smart here, the
hardware has the limitation so we can blame someone else.
Does that make sense to you?
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Mon, 2012-09-24 at 09:56 -0400, David Dillow wrote:
> On Mon, 2012-09-24 at 13:25 +0200, Antonio Ospite wrote:
> > In order to make this work I have to put joypad buttons only in one of
> > the key maps, I don't know if that is compatible with the Harmony
> > adapter.
>
> I suspect it is, but I'll try to test later tonight. Feel free to merge
> my patch and yours and push it upstream under your name/copyright; I'm
> not attached to it.
Yep, this works with the Harmony.
I think the logic you are using may result in some combinations being
missed -- say Triangle + Play -- but I'm not sure how much it matters. I
can rework the first version (building a custom report based on the
funky rules of coding) in the next few days, or you can push your
version if you'd like. Or perhaps a middle way, that erases the buttons
reported in the bitmap from the byte array, but otherwise keeps track of
the last press in that area to avoid weirdness when the user presses a
button combo that cannot be reported due to the HW limitations.
Dave
On Mon, 2012-09-24 at 13:25 +0200, Antonio Ospite wrote:
> In order to make this work I have to put joypad buttons only in one of
> the key maps, I don't know if that is compatible with the Harmony
> adapter.
I suspect it is, but I'll try to test later tonight. Feel free to merge
my patch and yours and push it upstream under your name/copyright; I'm
not attached to it.
> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
> index 11a6c1f..fa2e50d 100644
> --- a/drivers/hid/hid-ps3remote.c
> +++ b/drivers/hid/hid-ps3remote.c
> @@ -49,6 +49,26 @@
> * The keymap is generally ordered by the physical location of the buttons,
> * as this makes it easier to verify a correct mapping during testing.
> */
You'd have caught this in cleanup, I'm sure, but the above comment no
longer makes sense.
> +static const unsigned int ps3remote_keymap_1[] = {
Probably want a better name -- again, something I'm sure you would have
done during cleanup of this proof-of-concept.
> static const unsigned int ps3remote_keymap[] = {
This map probably should now be ordered per David H. since we're no
longer following the physical layout.
Thanks,
Dave
Em Mon, 2012-09-24 às 13:25 +0200, Antonio Ospite escreveu:
> + //[0x5c] = KEY_OPTION, /* options/triangle */
I don't think that C++ will be liked here.
Cheers
---
drivers/hid/hid-ps3remote.c | 153 +++++++++++++++++++++++++++----------------
1 file changed, 96 insertions(+), 57 deletions(-)
Hey David D., I was able to improve the situation with the patch below,
basically that's what I am doing:
- In the descriptor have two collections, one for "joypad buttons"
which allow multiple keypresses, and one for the remote buttons
which do not allow multiple keypresses.
- Have two key maps, one for each collection.
In order to make this work I have to put joypad buttons only in one of
the key maps, I don't know if that is compatible with the Harmony
adapter.
BTW to deal with HID descriptors I am using this gHID tool which
provides a "smart language" that ease things a little bit:
http://code.google.com/p/ghid/
The code spacing and naming of variables can be improved in the patch
below but I wanted to get it out ASAP to avoid duplicate work.
Regards,
Antonio
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
index 11a6c1f..fa2e50d 100644
--- a/drivers/hid/hid-ps3remote.c
+++ b/drivers/hid/hid-ps3remote.c
@@ -49,6 +49,26 @@
* The keymap is generally ordered by the physical location of the buttons,
* as this makes it easier to verify a correct mapping during testing.
*/
+static const unsigned int ps3remote_keymap_1[] = {
+ [0x01] = KEY_SELECT,
+ [0x02] = BTN_THUMBL, /* L3 */
+ [0x03] = BTN_THUMBR, /* R3 */
+ [0x04] = BTN_START,
+ [0x05] = KEY_UP,
+ [0x06] = KEY_RIGHT,
+ [0x07] = KEY_DOWN,
+ [0x08] = KEY_LEFT,
+ [0x09] = BTN_TL2, /* L2 */
+ [0x0a] = BTN_TR2, /* R2 */
+ [0x0b] = BTN_TL, /* L1 */
+ [0x0c] = BTN_TR, /* R1 */
+ [0x0d] = KEY_OPTION, /* options/triangle */
+ [0x0e] = KEY_BACK, /* back/circle */
+ [0x0f] = BTN_0, /* cross */
+ [0x10] = KEY_SCREEN, /* view/square */
+ [0x11] = KEY_HOMEPAGE, /* PS button */
+ [0x14] = KEY_ENTER,
+};
static const unsigned int ps3remote_keymap[] = {
[0x16] = KEY_EJECTCD,
[0x64] = KEY_AUDIO,
@@ -74,24 +94,24 @@ static const unsigned int ps3remote_keymap[] = {
[0x1a] = KEY_MENU, /* top menu */
[0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
[0x0e] = KEY_ESC, /* return */
- [0x5c] = KEY_OPTION, /* options/triangle */
- [0x5d] = KEY_BACK, /* back/circle */
- [0x5f] = KEY_SCREEN, /* view/square */
- [0x5e] = BTN_0, /* cross */
- [0x54] = KEY_UP,
- [0x56] = KEY_DOWN,
- [0x57] = KEY_LEFT,
- [0x55] = KEY_RIGHT,
- [0x0b] = KEY_ENTER,
- [0x5a] = BTN_TL, /* L1 */
- [0x58] = BTN_TL2, /* L2 */
- [0x51] = BTN_THUMBL, /* L3 */
- [0x5b] = BTN_TR, /* R1 */
- [0x59] = BTN_TR2, /* R2 */
- [0x52] = BTN_THUMBR, /* R3 */
- [0x43] = KEY_HOMEPAGE, /* PS button */
- [0x50] = KEY_SELECT,
- [0x53] = BTN_START,
+ //[0x5c] = KEY_OPTION, /* options/triangle */
+ //[0x5d] = KEY_BACK, /* back/circle */
+ //[0x5f] = KEY_SCREEN, /* view/square */
+ //[0x5e] = BTN_0, /* cross */
+ //[0x54] = KEY_UP,
+ //[0x56] = KEY_DOWN,
+ //[0x57] = KEY_LEFT,
+ //[0x55] = KEY_RIGHT,
+ //[0x0b] = KEY_ENTER,
+ //[0x5a] = BTN_TL, /* L1 */
+ //[0x58] = BTN_TL2, /* L2 */
+ //[0x51] = BTN_THUMBL, /* L3 */
+ //[0x5b] = BTN_TR, /* R1 */
+ //[0x59] = BTN_TR2, /* R2 */
+ //[0x52] = BTN_THUMBR, /* R3 */
+ //[0x43] = KEY_HOMEPAGE, /* PS button */
+ //[0x50] = KEY_SELECT,
+ //[0x53] = BTN_START,
[0x33] = KEY_REWIND, /* scan back */
[0x32] = KEY_PLAY,
[0x34] = KEY_FORWARD, /* scan forward */
@@ -104,42 +124,54 @@ static const unsigned int ps3remote_keymap[] = {
};
static __u8 ps3remote_rdesc[] = {
- 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
- 0x09, 0x05, /* USAGE (Game Pad) */
- 0xa1, 0x01, /* COLLECTION (Application) */
-
- /* First four bytes contain a bitmask for some of the buttons, and
- * possibly a controller number. We don't need this information,
- * as the keys will be reported in the next field as well.
- */
- 0x75, 0x20, /* REPORT SIZE (32) */
- 0x95, 0x01, /* REPORT COUNT (1) */
- 0x81, 0x01, /* INPUT (Constant) */
-
- /* All key presses are reported in this field */
- 0x05, 0x09, /* USAGE PAGE (Button) */
- 0x19, 0x00, /* USAGE MINIMUM (0) */
- 0x29, 0xfe, /* USAGE MAXIMUM (254) */
- 0x15, 0x00, /* LOGICAL MINIMUM (0) */
- 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
- 0x75, 0x08, /* REPORT SIZE (8) */
- 0x95, 0x06, /* REPORT COUNT (6) */
- 0x81, 0x00, /* INPUT (Array, Absolute) */
-
- /* Ignore press indication */
- 0x75, 0x08, /* REPORT SIZE (8) */
- 0x95, 0x01, /* REPORT COUNT (1) */
- 0x81, 0x01, /* INPUT (Constant) */
-
- /* Report the battery level */
- 0x05, 0x06, /* USAGE PAGE (Generic Device) */
- 0x09, 0x20, /* USAGE (Battery Strength) */
- 0x15, 0x00, /* LOGICAL MINIMUM (0) */
- 0x25, 0x05, /* LOGICAL MAXIMUM (5) */
- 0x75, 0x08, /* REPORT SIZE (8) */
- 0x95, 0x01, /* REPORT COUNT (1) */
- 0x81, 0x02, /* INPUT (Variable, Absolute) */
- 0xc0, /* END_COLLECTION */
+ 0x05, 0x01, // GUsagePage Generic Desktop
+ 0x09, 0x05, // LUsage 0x05 [Game Pad]
+ 0xA1, 0x01, // MCollection Application (mouse, keyboard)
+ 0xA1, 0x02, // MCollection Logical (interrelated data)
+ 0x75, 0x08, // GReportSize 0x08 [8]
+ 0x95, 0x01, // GReportCount 0x01 [1]
+ 0x81, 0x00, // MInput 0x03
+ // Const[0] Var[1] Abs[2]
+
+ 0x05, 0x09, // GUsagePage Button
+ 0x19, 0x01, // LUsageMinimum 0x01 [Button 1 (primary/trigger)]
+ 0x29, 0x18, // LUsageMaximum 0x18 [Button 18]
+ 0x14, // GLogicalMinimum [0]
+ 0x25, 0x01, // GLogicalMaximum 0x01 [1]
+ 0x75, 0x01, // GReportSize 0x01 [1]
+ 0x95, 0x18, // GReportCount 0x18 [24]
+ 0x81, 0x02, // MInput 0x02
+ 0xC0, // MEndCollection [Game Pad]
+ // Data[0] Var[1] Abs[2]
+
+ 0xA1, 0x02, // MCollection Logical (interrelated data)
+ 0x05, 0x09, // GUsagePage Button
+ 0x18, // LUsageMinimum [No button pressed]
+ 0x29, 0xFE, // LUsageMaximum 0xFE [Button FE]
+ 0x14, // GLogicalMinimum [0]
+ 0x26, 0xFE, 0x00, // GLogicalMaximum 0x00FE [254]
+ 0x75, 0x08, // GReportSize 0x08 [8]
+ 0x95, 0x06, // GReportCount 0x06 [6]
+ 0x80, // MInput
+ //
+
+ 0x75, 0x08, // GReportSize 0x08 [8]
+ 0x95, 0x01, // GReportCount 0x01 [1]
+ 0x81, 0x01, // MInput 0x01
+ // Const[0] Arr[1] Abs[2]
+
+ 0x05, 0x06, // GUsagePage Generic Device Controls
+ 0x09, 0x20, // LUsage 0x20 [Battery Strength]
+ 0x14, // GLogicalMinimum [0]
+ 0x25, 0x05, // GLogicalMaximum 0x05 [5]
+ 0x75, 0x08, // GReportSize 0x08 [8]
+ 0x95, 0x01, // GReportCount 0x01 [1]
+ 0x81, 0x02, // MInput 0x02
+ // Data[0] Var[1] Abs[2]
+
+ 0xC0, // MEndCollection [Game Pad]
+
+ 0xC0 // MEndCollection [Game Pad]
};
static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -159,9 +191,16 @@ static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
key >= ARRAY_SIZE(ps3remote_keymap))
return -1;
- key = ps3remote_keymap[key];
- if (!key)
- return -1;
+ if (usage->collection_index == 1) {
+ key = ps3remote_keymap_1[key];
+ if (!key)
+ return -1;
+ }
+ if (usage->collection_index == 2) {
+ key = ps3remote_keymap[key];
+ if (!key)
+ return -1;
+ }
hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
return 1;
--
1.7.10.4
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Em Fri, 2012-09-21 às 22:06 +0200, Antonio Ospite escreveu:
> From a user point of view there is still some "set up" happening,
> the gnome bluetooth-applet tells "Successfully set up new device '%
> s'", so maybe we can just call this "set up" in the comment above too,
> I
> mentioned the PIN because Gnome bluetooth does something about
> that[1],
> Bastien in gnome-bt case pin="NULL" means that there will be no
> pin request at all by the device, right?
Yep. That means we don't try to pair, we just connect to it. No
security, first come first serve.
On Wed, 19 Sep 2012 12:59:46 -0400
David Dillow <[email protected]> wrote:
> On Wed, 2012-09-19 at 18:48 +0200, Antonio Ospite wrote:
> > On Mon, 17 Sep 2012 21:33:58 -0400
> > David Dillow <[email protected]> wrote:
>
> > While on the remote I see:
> >
> > I press '1' and keep it pressed:
> > Event: time 1348069656.505528, type 1 (EV_KEY), code 2 (KEY_1), value 1
> > 1111111111...
> >
> > I press '2' and release it ('1' is sent):
> > Event: time 1348069666.025543, type 1 (EV_KEY), code 2 (KEY_1), value 0
> > Event: time 1348069668.395531, type 1 (EV_KEY), code 2 (KEY_1), value 1
> > 1111111111...
> >
> > I release '1':
> > Event: time 1348069671.625541, type 1 (EV_KEY), code 2 (KEY_1), value 0
> >
> > I don't know at what level this behavior is enforced.
> >
> > I will test later with the old raw_event callback and the fix to the
> > descriptor you suggested in the other mail.
>
> Please capture the raw reports
> from /sys/kernel/debug/hid/0005:*:0306:*/events when pressing multiple
> keys -- and tell me which ones they were. You may need to mount debugfs
> to get to this path.
>
I used hidraw which is equivalent in this case.
$ sudo cat /dev/hidraw1 | hexdump -e '12/1 "%02X " "\n"'
I press and release 'Play'
01 00 00 00 32 FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
I press and release 'Stop'
01 00 00 00 32 FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
I press 'Play', keep it pressed and press 'Stop'
01 00 00 00 32 FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 01 05
I release 'Stop' and then release 'Play'
01 00 00 00 32 FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
So this combination cannot be detected at all, but:
I press and release 'Triangle'
01 00 10 00 5C FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
I press and release 'Circle'
01 00 20 00 5D FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
I press 'Triangle', keep it pressed and press 'Circle'
01 00 10 00 5C FF FF FF FF FF 01 05
01 00 30 00 FF FF FF FF FF FF 01 05
I release 'Circle' and then release 'Triangle'
01 00 10 00 5C FF FF FF FF FF 01 05
01 00 00 00 FF FF FF FF FF FF 00 05
So using the third byte (actually second, third and fourth right?) this
combination of multiple key presses can be detected, that happens to be
true for the "joypad buttons" which makes sense.
We can express the decoding of a bitfield in terms of HID descriptor,
right? Maybe copying from the Sixaxis report descriptor as the raw
report data for the first four bytes looks the same as the Sixaxis.
I'll try modifying the descriptor later.
David D. if you want you could send the version with the raw_event
callback first and get that merged and we can improve the descriptor
later, that would be fine to me.
BTW Some three keys combinations also produce a 03 in the one to last
byte: 01 00 00 00 FF FF FF FF FF FF 03 05
combinations like "Play+Stop+Next" do but some other do not and I could
not find a rule about that, so I don't know what the 03 really means.
> It looks like this simple approach isn't going to work when you press
> multiple keys, so I'll need more information to see if I can do
> something else -- my Harmony can only do one key press at a time, as it
> is converting IR to an emulated BD remote.
>
> > > +config HID_PS3REMOTE
> > > + tristate "Sony PS3 BD Remote"
> >
> > If you are going for a v3, consider using "Sony PS3 BD Remote Control"
> > here too, not a big deal but that's the name on the user manual.
>
> Will do, thanks.
>
> > For the note about the association procedure I had in mind something
> > like this:
> >
> > /* NOTE: in order to associate the Sony PS3 BD Remote with a Bluetooth host
> > * the key combination Start+Enter has to be kept pressed for about 7 seconds,
> > * with the host BT Controller in discovering mode.
> > *
> > * Also the pin request should be ignored by the BT Controller (NULL pin).
> > */
> >
> > Could someone more into BT please check the terminology here? Thanks.
>
> Will add something along these lines, though I think you mean there is
> no authentication step rather than a NULL pin, which implies we still
> need to do auth. Or maybe that's just how the Harmony needed it...
>
>From a user point of view there is still some "set up" happening,
the gnome bluetooth-applet tells "Successfully set up new device '%
s'", so maybe we can just call this "set up" in the comment above too, I
mentioned the PIN because Gnome bluetooth does something about that[1],
Bastien in gnome-bt case pin="NULL" means that there will be no
pin request at all by the device, right?
Thanks,
Antonio
[1]
http://git.gnome.org/browse/gnome-bluetooth/tree/wizard/pin-code-database.xml#n59
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Em Wed, 2012-09-19 às 18:48 +0200, Antonio Ospite escreveu:
> For the note about the association procedure I had in mind something
> like this:
>
> /* NOTE: in order to associate the Sony PS3 BD Remote with a Bluetooth
> host
> * the key combination Start+Enter has to be kept pressed for about 7
> seconds,
> * with the host BT Controller in discovering mode.
> *
> * Also the pin request should be ignored by the BT Controller (NULL
> pin).
> */
>
> Could someone more into BT please check the terminology here? Thanks.
Pairing means that the communication between the 2 Bluetooth devices
will be encrypted. In this case (and as is usually the case for mice),
the communication isn't encrypted, they will just agree to communicate
and the computer will just agree not to ask whether the connection is
allowed at every attempt (favourite/trusted settings depending on the OS
or terminology). There will be no PIN request.
Cheers
On Wed, 2012-09-19 at 18:48 +0200, Antonio Ospite wrote:
> On Mon, 17 Sep 2012 21:33:58 -0400
> David Dillow <[email protected]> wrote:
> While on the remote I see:
>
> I press '1' and keep it pressed:
> Event: time 1348069656.505528, type 1 (EV_KEY), code 2 (KEY_1), value 1
> 1111111111...
>
> I press '2' and release it ('1' is sent):
> Event: time 1348069666.025543, type 1 (EV_KEY), code 2 (KEY_1), value 0
> Event: time 1348069668.395531, type 1 (EV_KEY), code 2 (KEY_1), value 1
> 1111111111...
>
> I release '1':
> Event: time 1348069671.625541, type 1 (EV_KEY), code 2 (KEY_1), value 0
>
> I don't know at what level this behavior is enforced.
>
> I will test later with the old raw_event callback and the fix to the
> descriptor you suggested in the other mail.
Please capture the raw reports
from /sys/kernel/debug/hid/0005:*:0306:*/events when pressing multiple
keys -- and tell me which ones they were. You may need to mount debugfs
to get to this path.
It looks like this simple approach isn't going to work when you press
multiple keys, so I'll need more information to see if I can do
something else -- my Harmony can only do one key press at a time, as it
is converting IR to an emulated BD remote.
> > +config HID_PS3REMOTE
> > + tristate "Sony PS3 BD Remote"
>
> If you are going for a v3, consider using "Sony PS3 BD Remote Control"
> here too, not a big deal but that's the name on the user manual.
Will do, thanks.
> For the note about the association procedure I had in mind something
> like this:
>
> /* NOTE: in order to associate the Sony PS3 BD Remote with a Bluetooth host
> * the key combination Start+Enter has to be kept pressed for about 7 seconds,
> * with the host BT Controller in discovering mode.
> *
> * Also the pin request should be ignored by the BT Controller (NULL pin).
> */
>
> Could someone more into BT please check the terminology here? Thanks.
Will add something along these lines, though I think you mean there is
no authentication step rather than a NULL pin, which implies we still
need to do auth. Or maybe that's just how the Harmony needed it...
Thanks,
Dave
On Mon, 17 Sep 2012 21:33:58 -0400
David Dillow <[email protected]> wrote:
> The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
> BlueZ project's user space, but the code that handled it was recently
> removed as its functionality conflicted with a real HSP implementation
> and the mapping was thought to be better handled in the kernel. This is
> a port of the mapping logic from the fakehid driver by Marcel Holtmann
> to the in-kernel HID layer.
>
> We also add support for the Logitech Harmony Adapter for PS3, which
> emulates the BD Remote.
>
> Signed-off-by: David Dillow <[email protected]>
> ---
> drivers/hid/Kconfig | 11 ++-
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ids.h | 2 +
> drivers/hid/hid-ps3remote.c | 199 +++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 214 insertions(+), 1 deletions(-)
>
> Here's an updated version of the HID driver for the PS3 BD Remote. I've
> gone ahead and signed off on it as it works for my Harmony adapter, but
> it needs testing with a real BD remote before going upstream. Antonio
> also asked for some text about the pairing process -- I've left it out
> since I don't know what it should be.
>
Tested with the original PS3 BD remote and this version works fine too,
thanks.
Tested-by: Antonio Ospite <[email protected]>
About the multiple keys press handling I see that my keyboard does this:
I press 'a' and keep it pressed:
Event: time 1348069423.217974, type 1 (EV_KEY), code 30 (KEY_A), value 1
Event: time 1348069423.249871, type 1 (EV_KEY), code 30 (KEY_A), value 2
Event: time 1348069423.282410, type 1 (EV_KEY), code 30 (KEY_A), value 2
aaa
...
I press 's' and release it:
Event: time 1348069423.290792, type 1 (EV_KEY), code 31 (KEY_S), value 1
Event: time 1348069423.374516, type 1 (EV_KEY), code 31 (KEY_S), value 0
s
I release 'a':
Event: time 1348069427.351315, type 1 (EV_KEY), code 30 (KEY_A), value 0
While on the remote I see:
I press '1' and keep it pressed:
Event: time 1348069656.505528, type 1 (EV_KEY), code 2 (KEY_1), value 1
1111111111...
I press '2' and release it ('1' is sent):
Event: time 1348069666.025543, type 1 (EV_KEY), code 2 (KEY_1), value 0
Event: time 1348069668.395531, type 1 (EV_KEY), code 2 (KEY_1), value 1
1111111111...
I release '1':
Event: time 1348069671.625541, type 1 (EV_KEY), code 2 (KEY_1), value 0
I don't know at what level this behavior is enforced.
I will test later with the old raw_event callback and the fix to the
descriptor you suggested in the other mail.
David D. what is the behavior of the Harmony adapter?
Few more comments inlined below.
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index fbf4950..7bf3b1a 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -534,6 +534,14 @@ config HID_PRIMAX
> Support for Primax devices that are not fully compliant with the
> HID standard.
>
> +config HID_PS3REMOTE
> + tristate "Sony PS3 BD Remote"
If you are going for a v3, consider using "Sony PS3 BD Remote Control"
here too, not a big deal but that's the name on the user manual.
> + depends on BT_HIDP
> + ---help---
> + Support for the Sony PS3 BD Remote and Logitech Harmony Adapter
> + for PS3, which connect over Bluetooth. Support for the 6-axis
> + controllers is provided by HID_SONY.
> +
> config HID_ROCCAT
> tristate "Roccat device support"
> depends on USB_HID
> @@ -561,7 +569,8 @@ config HID_SONY
> tristate "Sony PS3 controller"
> depends on USB_HID
> ---help---
> - Support for Sony PS3 controller.
> + Support for Sony PS3 6-axis controllers. Support for the Sony PS3
> + BD Remote is provided by HID_PS3REMOTE.
>
> config HID_SPEEDLINK
> tristate "Speedlink VAD Cezanne mouse support"
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index f975485..333ed6c 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
> obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
> obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 60ea284..a9f0439 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
Jiri, what is supposed to be the ordering of entries here? Alphabetical
on the product field, or by product numerical value like in hid-ids.h?
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
> @@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 1dcb76f..0f5c2bb 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -492,6 +492,7 @@
> #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
>
> #define USB_VENDOR_ID_LOGITECH 0x046d
> +#define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306
> #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
> #define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
> #define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
> @@ -684,6 +685,7 @@
> #define USB_VENDOR_ID_SONY 0x054c
> #define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
> #define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
> +#define USB_DEVICE_ID_SONY_PS3_BDREMOTE 0x0306
> #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
>
> #define USB_VENDOR_ID_SOUNDGRAPH 0x15c2
> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
> new file mode 100644
> index 0000000..e8caef0
> --- /dev/null
> +++ b/drivers/hid/hid-ps3remote.c
> @@ -0,0 +1,199 @@
> +/*
> + * HID driver for Sony PS3 BD Remote
> + *
> + * Copyright (c) 2012 David Dillow <[email protected]>
> + * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
> + * and other kernel HID drivers.
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
For the note about the association procedure I had in mind something
like this:
/* NOTE: in order to associate the Sony PS3 BD Remote with a Bluetooth host
* the key combination Start+Enter has to be kept pressed for about 7 seconds,
* with the host BT Controller in discovering mode.
*
* Also the pin request should be ignored by the BT Controller (NULL pin).
*/
Could someone more into BT please check the terminology here? Thanks.
>
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +/*
> + * The first byte of the raw event report seems to contain a fixed number,
> + * possibly a controller number. The next three bytes contain a bitmask,
> + * in big-endian order, of a portion of the buttons available.
> + *
> + * Button Bit Scan Mapped Key
> + * PS 0 0x43 KEY_HOMEPAGE
> + * Unknown 1-2
> + * Enter 3 0x0b KEY_ENTER
> + * Unknown 4-7
> + * L2 8 0x58 BTN_TL2
> + * R2 9 0x59 BTN_TR2
> + * L1 10 0x5a BTN_TL
> + * R1 11 0x5b BTN_TR
> + * Triangle 12 0x5c KEY_OPTION
> + * Circle 13 0x5d KEY_BACK
> + * Cross 14 0x5e BTN_0
> + * Square 15 0x5f KEY_SCREEN
> + * Select 16 0x50 KEY_SELECT
> + * L3 17 0x51 BTN_THUMBL
> + * R3 18 0x52 BTN_THUMBR
> + * Start 19 0x53 BTN_START
> + * Up 20 0x54 KEY_UP
> + * Right 21 0x55 KEY_RIGHT
> + * Down 22 0x56 KEY_DOWN
> + * Left 23 0x57 KEY_LEFT
> + *
> + * The keymap is generally ordered by the physical location of the buttons,
> + * as this makes it easier to verify a correct mapping during testing.
> + */
> +static const unsigned int ps3remote_keymap[] = {
> + [0x16] = KEY_EJECTCD,
> + [0x64] = KEY_AUDIO,
> + [0x65] = KEY_ANGLE,
> + [0x63] = KEY_SUBTITLE,
> + [0x0f] = KEY_CLEAR,
> + [0x28] = KEY_TIME,
> + [0x00] = KEY_1,
> + [0x01] = KEY_2,
> + [0x02] = KEY_3,
> + [0x03] = KEY_4,
> + [0x04] = KEY_5,
> + [0x05] = KEY_6,
> + [0x06] = KEY_7,
> + [0x07] = KEY_8,
> + [0x08] = KEY_9,
> + [0x09] = KEY_0,
> + [0x81] = KEY_RED,
> + [0x82] = KEY_GREEN,
> + [0x80] = KEY_BLUE,
> + [0x83] = KEY_YELLOW,
> + [0x70] = KEY_INFO, /* display */
> + [0x1a] = KEY_MENU, /* top menu */
> + [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
> + [0x0e] = KEY_ESC, /* return */
> + [0x5c] = KEY_OPTION, /* options/triangle */
> + [0x5d] = KEY_BACK, /* back/circle */
> + [0x5f] = KEY_SCREEN, /* view/square */
> + [0x5e] = BTN_0, /* cross */
> + [0x54] = KEY_UP,
> + [0x56] = KEY_DOWN,
> + [0x57] = KEY_LEFT,
> + [0x55] = KEY_RIGHT,
> + [0x0b] = KEY_ENTER,
> + [0x5a] = BTN_TL, /* L1 */
> + [0x58] = BTN_TL2, /* L2 */
> + [0x51] = BTN_THUMBL, /* L3 */
> + [0x5b] = BTN_TR, /* R1 */
> + [0x59] = BTN_TR2, /* R2 */
> + [0x52] = BTN_THUMBR, /* R3 */
> + [0x43] = KEY_HOMEPAGE, /* PS button */
> + [0x50] = KEY_SELECT,
> + [0x53] = BTN_START,
> + [0x33] = KEY_REWIND, /* scan back */
> + [0x32] = KEY_PLAY,
> + [0x34] = KEY_FORWARD, /* scan forward */
> + [0x30] = KEY_PREVIOUS,
> + [0x38] = KEY_STOP,
> + [0x31] = KEY_NEXT,
> + [0x60] = KEY_FRAMEBACK, /* slow/step back */
> + [0x39] = KEY_PAUSE,
> + [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
> +};
> +
> +static __u8 ps3remote_rdesc[] = {
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x05, /* USAGE (Game Pad) */
> + 0xa1, 0x01, /* COLLECTION (Application) */
> +
> + /* First four bytes contain a bitmask for some of the buttons, and
> + * possibly a controller number. We don't need this information,
> + * as the keys will be reported in the next field as well.
> + */
> + 0x75, 0x20, /* REPORT SIZE (32) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x01, /* INPUT (Constant) */
> +
> + /* All key presses are reported in this field */
> + 0x05, 0x09, /* USAGE PAGE (Button) */
> + 0x19, 0x00, /* USAGE MINIMUM (0) */
> + 0x29, 0xfe, /* USAGE MAXIMUM (254) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x06, /* REPORT COUNT (6) */
> + 0x81, 0x00, /* INPUT (Array, Absolute) */
> +
> + /* Ignore press indication */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x01, /* INPUT (Constant) */
> +
> + /* Report the battery level */
> + 0x05, 0x06, /* USAGE PAGE (Generic Device) */
> + 0x09, 0x20, /* USAGE (Battery Strength) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x05, /* LOGICAL MAXIMUM (5) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x02, /* INPUT (Variable, Absolute) */
> + 0xc0, /* END_COLLECTION */
> +};
> +
> +static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + *rsize = sizeof(ps3remote_rdesc);
> + return ps3remote_rdesc;
> +}
> +
> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + unsigned int key = usage->hid & HID_USAGE;
> +
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
> + key >= ARRAY_SIZE(ps3remote_keymap))
> + return -1;
> +
> + key = ps3remote_keymap[key];
> + if (!key)
> + return -1;
> +
> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> + return 1;
> +}
> +
> +static const struct hid_device_id ps3remote_devices[] = {
> + /* PS3 BD Remote */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
> + /* Logitech Harmony Adapter for PS3 */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, ps3remote_devices);
> +
> +static struct hid_driver ps3remote_driver = {
> + .name = "ps3_remote",
> + .id_table = ps3remote_devices,
> + .report_fixup = ps3remote_fixup,
> + .input_mapping = ps3remote_mapping,
> +};
> +
> +static int __init ps3remote_init(void)
> +{
> + return hid_register_driver(&ps3remote_driver);
> +}
> +
> +static void __exit ps3remote_exit(void)
> +{
> + hid_unregister_driver(&ps3remote_driver);
> +}
> +
> +module_init(ps3remote_init);
> +module_exit(ps3remote_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("David Dillow <[email protected]>");
>
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
BlueZ project's user space, but the code that handled it was recently
removed as its functionality conflicted with a real HSP implementation
and the mapping was thought to be better handled in the kernel. This is
a port of the mapping logic from the fakehid driver by Marcel Holtmann
to the in-kernel HID layer.
We also add support for the Logitech Harmony Adapter for PS3, which
emulates the BD Remote.
Signed-off-by: David Dillow <[email protected]>
---
drivers/hid/Kconfig | 11 ++-
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 2 +
drivers/hid/hid-ids.h | 2 +
drivers/hid/hid-ps3remote.c | 199 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 214 insertions(+), 1 deletions(-)
Here's an updated version of the HID driver for the PS3 BD Remote. I've
gone ahead and signed off on it as it works for my Harmony adapter, but
it needs testing with a real BD remote before going upstream. Antonio
also asked for some text about the pairing process -- I've left it out
since I don't know what it should be.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index fbf4950..7bf3b1a 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -534,6 +534,14 @@ config HID_PRIMAX
Support for Primax devices that are not fully compliant with the
HID standard.
+config HID_PS3REMOTE
+ tristate "Sony PS3 BD Remote"
+ depends on BT_HIDP
+ ---help---
+ Support for the Sony PS3 BD Remote and Logitech Harmony Adapter
+ for PS3, which connect over Bluetooth. Support for the 6-axis
+ controllers is provided by HID_SONY.
+
config HID_ROCCAT
tristate "Roccat device support"
depends on USB_HID
@@ -561,7 +569,8 @@ config HID_SONY
tristate "Sony PS3 controller"
depends on USB_HID
---help---
- Support for Sony PS3 controller.
+ Support for Sony PS3 6-axis controllers. Support for the Sony PS3
+ BD Remote is provided by HID_PS3REMOTE.
config HID_SPEEDLINK
tristate "Speedlink VAD Cezanne mouse support"
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index f975485..333ed6c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
+obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 60ea284..a9f0439 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
@@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1dcb76f..0f5c2bb 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -492,6 +492,7 @@
#define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
#define USB_VENDOR_ID_LOGITECH 0x046d
+#define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306
#define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
#define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
#define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
@@ -684,6 +685,7 @@
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
+#define USB_DEVICE_ID_SONY_PS3_BDREMOTE 0x0306
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
#define USB_VENDOR_ID_SOUNDGRAPH 0x15c2
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
new file mode 100644
index 0000000..e8caef0
--- /dev/null
+++ b/drivers/hid/hid-ps3remote.c
@@ -0,0 +1,199 @@
+/*
+ * HID driver for Sony PS3 BD Remote
+ *
+ * Copyright (c) 2012 David Dillow <[email protected]>
+ * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
+ * and other kernel HID drivers.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+/*
+ * The first byte of the raw event report seems to contain a fixed number,
+ * possibly a controller number. The next three bytes contain a bitmask,
+ * in big-endian order, of a portion of the buttons available.
+ *
+ * Button Bit Scan Mapped Key
+ * PS 0 0x43 KEY_HOMEPAGE
+ * Unknown 1-2
+ * Enter 3 0x0b KEY_ENTER
+ * Unknown 4-7
+ * L2 8 0x58 BTN_TL2
+ * R2 9 0x59 BTN_TR2
+ * L1 10 0x5a BTN_TL
+ * R1 11 0x5b BTN_TR
+ * Triangle 12 0x5c KEY_OPTION
+ * Circle 13 0x5d KEY_BACK
+ * Cross 14 0x5e BTN_0
+ * Square 15 0x5f KEY_SCREEN
+ * Select 16 0x50 KEY_SELECT
+ * L3 17 0x51 BTN_THUMBL
+ * R3 18 0x52 BTN_THUMBR
+ * Start 19 0x53 BTN_START
+ * Up 20 0x54 KEY_UP
+ * Right 21 0x55 KEY_RIGHT
+ * Down 22 0x56 KEY_DOWN
+ * Left 23 0x57 KEY_LEFT
+ *
+ * The keymap is generally ordered by the physical location of the buttons,
+ * as this makes it easier to verify a correct mapping during testing.
+ */
+static const unsigned int ps3remote_keymap[] = {
+ [0x16] = KEY_EJECTCD,
+ [0x64] = KEY_AUDIO,
+ [0x65] = KEY_ANGLE,
+ [0x63] = KEY_SUBTITLE,
+ [0x0f] = KEY_CLEAR,
+ [0x28] = KEY_TIME,
+ [0x00] = KEY_1,
+ [0x01] = KEY_2,
+ [0x02] = KEY_3,
+ [0x03] = KEY_4,
+ [0x04] = KEY_5,
+ [0x05] = KEY_6,
+ [0x06] = KEY_7,
+ [0x07] = KEY_8,
+ [0x08] = KEY_9,
+ [0x09] = KEY_0,
+ [0x81] = KEY_RED,
+ [0x82] = KEY_GREEN,
+ [0x80] = KEY_BLUE,
+ [0x83] = KEY_YELLOW,
+ [0x70] = KEY_INFO, /* display */
+ [0x1a] = KEY_MENU, /* top menu */
+ [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
+ [0x0e] = KEY_ESC, /* return */
+ [0x5c] = KEY_OPTION, /* options/triangle */
+ [0x5d] = KEY_BACK, /* back/circle */
+ [0x5f] = KEY_SCREEN, /* view/square */
+ [0x5e] = BTN_0, /* cross */
+ [0x54] = KEY_UP,
+ [0x56] = KEY_DOWN,
+ [0x57] = KEY_LEFT,
+ [0x55] = KEY_RIGHT,
+ [0x0b] = KEY_ENTER,
+ [0x5a] = BTN_TL, /* L1 */
+ [0x58] = BTN_TL2, /* L2 */
+ [0x51] = BTN_THUMBL, /* L3 */
+ [0x5b] = BTN_TR, /* R1 */
+ [0x59] = BTN_TR2, /* R2 */
+ [0x52] = BTN_THUMBR, /* R3 */
+ [0x43] = KEY_HOMEPAGE, /* PS button */
+ [0x50] = KEY_SELECT,
+ [0x53] = BTN_START,
+ [0x33] = KEY_REWIND, /* scan back */
+ [0x32] = KEY_PLAY,
+ [0x34] = KEY_FORWARD, /* scan forward */
+ [0x30] = KEY_PREVIOUS,
+ [0x38] = KEY_STOP,
+ [0x31] = KEY_NEXT,
+ [0x60] = KEY_FRAMEBACK, /* slow/step back */
+ [0x39] = KEY_PAUSE,
+ [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
+};
+
+static __u8 ps3remote_rdesc[] = {
+ 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
+ 0x09, 0x05, /* USAGE (Game Pad) */
+ 0xa1, 0x01, /* COLLECTION (Application) */
+
+ /* First four bytes contain a bitmask for some of the buttons, and
+ * possibly a controller number. We don't need this information,
+ * as the keys will be reported in the next field as well.
+ */
+ 0x75, 0x20, /* REPORT SIZE (32) */
+ 0x95, 0x01, /* REPORT COUNT (1) */
+ 0x81, 0x01, /* INPUT (Constant) */
+
+ /* All key presses are reported in this field */
+ 0x05, 0x09, /* USAGE PAGE (Button) */
+ 0x19, 0x00, /* USAGE MINIMUM (0) */
+ 0x29, 0xfe, /* USAGE MAXIMUM (254) */
+ 0x15, 0x00, /* LOGICAL MINIMUM (0) */
+ 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
+ 0x75, 0x08, /* REPORT SIZE (8) */
+ 0x95, 0x06, /* REPORT COUNT (6) */
+ 0x81, 0x00, /* INPUT (Array, Absolute) */
+
+ /* Ignore press indication */
+ 0x75, 0x08, /* REPORT SIZE (8) */
+ 0x95, 0x01, /* REPORT COUNT (1) */
+ 0x81, 0x01, /* INPUT (Constant) */
+
+ /* Report the battery level */
+ 0x05, 0x06, /* USAGE PAGE (Generic Device) */
+ 0x09, 0x20, /* USAGE (Battery Strength) */
+ 0x15, 0x00, /* LOGICAL MINIMUM (0) */
+ 0x25, 0x05, /* LOGICAL MAXIMUM (5) */
+ 0x75, 0x08, /* REPORT SIZE (8) */
+ 0x95, 0x01, /* REPORT COUNT (1) */
+ 0x81, 0x02, /* INPUT (Variable, Absolute) */
+ 0xc0, /* END_COLLECTION */
+};
+
+static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ *rsize = sizeof(ps3remote_rdesc);
+ return ps3remote_rdesc;
+}
+
+static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ unsigned int key = usage->hid & HID_USAGE;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
+ key >= ARRAY_SIZE(ps3remote_keymap))
+ return -1;
+
+ key = ps3remote_keymap[key];
+ if (!key)
+ return -1;
+
+ hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
+ return 1;
+}
+
+static const struct hid_device_id ps3remote_devices[] = {
+ /* PS3 BD Remote */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
+ /* Logitech Harmony Adapter for PS3 */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, ps3remote_devices);
+
+static struct hid_driver ps3remote_driver = {
+ .name = "ps3_remote",
+ .id_table = ps3remote_devices,
+ .report_fixup = ps3remote_fixup,
+ .input_mapping = ps3remote_mapping,
+};
+
+static int __init ps3remote_init(void)
+{
+ return hid_register_driver(&ps3remote_driver);
+}
+
+static void __exit ps3remote_exit(void)
+{
+ hid_unregister_driver(&ps3remote_driver);
+}
+
+module_init(ps3remote_init);
+module_exit(ps3remote_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Dillow <[email protected]>");
On Mon, 2012-09-17 at 12:04 +0200, Antonio Ospite wrote:
> On Thu, 13 Sep 2012 23:04:52 -0400
> David Dillow <[email protected]> wrote:
> > Antonio, if you could test this with the real remote you have, I'd be
> > very appreciative.
> >
>
> Works here as well with original PS3 BR remote, thanks David D.
>
> About multiple key-presses, when I hold two _different_ keys at the same
> time, I get the message:
>
> ps3_remote 0005:054C:0306.0004: Incoherent report, 000000 000000 ff 16
> 01
>
> I just tried a couple of combinations tho.
I found a typo in the custom report descriptor which could cause this --
where it says REPORT COUNT (11), I have 0x95, 0x01 as values in the
array. Those should be 0x95, 0x0b instead. This would prevent it from
looking at any more than the first key reported. Could you fix that up
and retest?
Looking at this gave me an idea that would avoid needing a raw event
handler; I've got a version built and will test in a bit, once the kids
are down. I'll send it along after that.
If both fail, it would be useful to see the output
from /sys/kernel/debug/hid/0005:*:0306:*/event while you are pressing
both single and multiple buttons.
> What about adding a note about the association maneuver? I mean the one
> when we press Start+Enter for a few second when we add the BD remote as
> a new BT device. I forget about it every time I associate the BD
> remote with a new system; I know kernel code is not the most
> user-friendly spot for this documentation but having it here wouldn't
> hurt.
>
> Now I noted it on the back of the battery cover too :)
Indeed; I don't have one of the real remotes -- could you provide text?
> > +static int ps3remote_event(struct hid_device *hdev, struct hid_report *report,
> > + u8 *raw_data, int size)
> > +{
> > + struct ps3remote_data *data = hid_get_drvdata(hdev);
> > + u32 mask, changed;
> > + u8 key, pressed;
>
> The preferred kernel style for declarations is to have one per line, no
> commas, but that's up to you.
I've seen it both ways, so I'll stick with the less vertical space. If
they were initialized here, I'd be more inclined to change.
> > + if (size != 12) {
> > + hid_dbg(hdev, "unsupported report size\n");
> > + return 1;
>
> The raw_event callback should return negative on error, is there a
> reason why 1 is returned here?
I was just eating the event rather than passing it on. It probably makes
more sense to return an error, but I didn't investigate how the rest of
the stack handled that.
I'll likely change this if I need to keep the raw event handler.
Thank you for the review, I've incorporated your other comments into the
driver before I started working on the simpler version.
Dave
Em Mon, 2012-09-17 às 13:46 +0300, Luiz Augusto von Dentz escreveu:
> I now have the remote, I tried using xinput test and most keys seems
> to be working fine except the special buttons like subtitle, colors,
> x, l1... Im not sure if they are not being mapped because they don't
> have any representation or there is something wrong in the parser
> itself (Bastien do they used to work for you?). Also got a problem on
> suspend but I will have to reproduce it again to see if it is because
> of the new driver or not.
I used evtest for testing. To get access to all the keys in X, you'd
need to forward them, circumventing X.org's XKB limitations, using
something like lirc.
Using evtest is the easiest.
Cheers
Hi Luiz
On Mon, Sep 17, 2012 at 12:46 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> HI Antonio,
>>
>> Reviewed-by: Antonio Ospite <[email protected]>
>>
>> I just did a syntax/style review, maybe I will take another look at the
>> actual logic in the future to see if the report decoding procedure can
>> be improved but for now I think it's OK, we want to have this driver in
>> ASAP.
>>
>> David D. please bring up again the issue about missing keypresses on
>> re-connection when sending the driver to linux-input with a full
>> description of what you observed, I don't have a clue about these
>> matters but people on linux-input might.
>
> I now have the remote, I tried using xinput test and most keys seems
> to be working fine except the special buttons like subtitle, colors,
> x, l1... Im not sure if they are not being mapped because they don't
> have any representation or there is something wrong in the parser
> itself (Bastien do they used to work for you?). Also got a problem on
> suspend but I will have to reproduce it again to see if it is because
> of the new driver or not.
For debugging of input devices I recommend:
http://cgit.freedesktop.org/evtest/tree/evtest.c
Simply run it as ./evtest /dev/input/eventX
And it shows you all events that are sent. You can use it to see
whether the special-buttons are actually handled by the kernel.
The suspend problems are Bluetooth-related. We never actually got that
right. It's always the HIDP code that fails somewhere.
Regards
David
HI Antonio,
On Mon, Sep 17, 2012 at 1:04 PM, Antonio Ospite
<[email protected]> wrote:
> On Thu, 13 Sep 2012 23:04:52 -0400
> David Dillow <[email protected]> wrote:
>
> I'd use the official name "Sony PS3 BD Remote Control" in the subject
> repeating it as "Sony PS3 Blue-ray Disc Remote Control" in the long
> commit message to increase the googlable surface a little bit.
>
>> We also gain support for the Logitech Harmony Adapter for PS3.
>>
>> Signed-off-by: David Dillow <[email protected]>
>> --
>
> When you submit the patch to linux-input for inclusion remember to send
> one formatted with git-format-patch, for instance the diffstat
> separator is '---' not '--', with the latter in your last message the
> diffstat and the annotations are taken as part of the commit message
> when using git-am, while the annotations _after_ the diffstat are meant
> for discussion and to be ignored by git-am.
>
> Other comments inlined below.
>
>> drivers/hid/Kconfig | 7 +
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-core.c | 2 +
>> drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 335 insertions(+), 0 deletions(-)
>>
>> On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
>> On Wed, 12 Sep 2012 22:26:04 +0200 David Herrmann wrote:
>> > > So if the devices only need some short setup-command during
>> > > initialization and then they send HID reports whenever a button is
>> > > pressed, then everything should be very easy to get working. Even
>> > > auto-reconnect and 10min idle-disconnect are _very_ easy to get
>> > > working in the kernel with the current HID infrastructure.
>> >
>> > I too will take a look during the week-end about writing a kernel
>> > driver, if it's not that much of an effort I might take the project
>> > (no promises yet). I have a PS3 BD remote Bastien donated.
>>
>> Here's a first pass at a kernel driver to support the BD remote and
>> Harmony PS adapter. I've tested it on Fedora 16 with single keypresses
>> only, as I cannot send multiple simultaneous keypresses with the
>> Harmony. This patch is against kernel.org's bluetooth.git as of an hour
>> ago, but should apply to other versions without much fuss.
>>
>> Antonio, if you could test this with the real remote you have, I'd be
>> very appreciative.
>>
>
> Works here as well with original PS3 BR remote, thanks David D.
>
> About multiple key-presses, when I hold two _different_ keys at the same
> time, I get the message:
>
> ps3_remote 0005:054C:0306.0004: Incoherent report, 000000 000000 ff 16
> 01
>
> I just tried a couple of combinations tho.
>
>> David, could you point me at the controls for the 10min idle-disconnect?
>> auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
>> though it looks like the Harmony adapter is initiating the disconnect --
>> this is likely an area for improvement with the Sony remote.
>>
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index fbf4950..7e6ab25 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -534,6 +534,13 @@ config HID_PRIMAX
>> Support for Primax devices that are not fully compliant with the
>> HID standard.
>>
>> +config HID_PS3REMOTE
>> + tristate "Sony PS3 Bluetooth BD Remote"
>
> Use "Sony PS3 BD Remote Control" here too and mention Bluetooth in the
> help section.
>
>> + depends on BT_HIDP
>> + ---help---
>> + Support for Sony PS3 Bluetooth BD Remote and Logitech Harmony Adapter
>> + for PS3.
>> +
>
> From scripts/checkpatch.pl:
> WARNING: please write a paragraph that describes the config symbol fully
> #60: FILE: drivers/hid/Kconfig:537:
> +config HID_PS3REMOTE
>
> This is because the description is too short (< 4 lines), but I think
> we can ignore this warning.
>
>> config HID_ROCCAT
>> tristate "Roccat device support"
>> depends on USB_HID
>> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
>> index f975485..333ed6c 100644
>> --- a/drivers/hid/Makefile
>> +++ b/drivers/hid/Makefile
>> @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
>> obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
>> obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
>> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
>> +obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
>> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
>> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
>> hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 60ea284..1838c12 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
>> @@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
>
> As David H. already said, stick with the style of the subsystem for
> magic numbers, HID tend to prefer symbolic constants.
>
>> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
>> new file mode 100644
>> index 0000000..a87346e
>> --- /dev/null
>> +++ b/drivers/hid/hid-ps3remote.c
>> @@ -0,0 +1,325 @@
>> +/*
>> + * HID driver for Sony PS3 BD Remote
>> + *
>> + * Copyright (c) 2012 David Dillow <[email protected]>
>> + * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
>> + * and other kernel HID drivers.
>> + */
>> +
>> +/*
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the Free
>> + * Software Foundation; either version 2 of the License, or (at your option)
>> + * any later version.
>> + */
>> +
>
> What about adding a note about the association maneuver? I mean the one
> when we press Start+Enter for a few second when we add the BD remote as
> a new BT device. I forget about it every time I associate the BD
> remote with a new system; I know kernel code is not the most
> user-friendly spot for this documentation but having it here wouldn't
> hurt.
>
> Now I noted it on the back of the battery cover too :)
>
>> +#include <linux/device.h>
>> +#include <linux/hid.h>
>> +#include <linux/module.h>
>> +
>> +#include "hid-ids.h"
>> +
>> +enum ps3remote_special_keys {
>> + PS3R_BIT_PS = 0,
>> + PS3R_BIT_ENTER = 3,
>> + PS3R_BIT_L2 = 8,
>> + PS3R_BIT_R2 = 9,
>> + PS3R_BIT_L1 = 10,
>> + PS3R_BIT_R1 = 11,
>> + PS3R_BIT_TRIANGLE = 12,
>> + PS3R_BIT_CIRCLE = 13,
>> + PS3R_BIT_CROSS = 14,
>> + PS3R_BIT_SQUARE = 15,
>> + PS3R_BIT_SELECT = 16,
>> + PS3R_BIT_L3 = 17,
>> + PS3R_BIT_R3 = 18,
>> + PS3R_BIT_START = 19,
>> + PS3R_BIT_UP = 20,
>> + PS3R_BIT_RIGHT = 21,
>> + PS3R_BIT_DOWN = 22,
>> + PS3R_BIT_LEFT = 23,
>> +};
>> +
>> +static unsigned int ps3remote_bits[] = {
>
> This could be const too.
>
>> + [PS3R_BIT_ENTER] = 0x0b,
>> + [PS3R_BIT_PS] = 0x43,
>> + [PS3R_BIT_SQUARE] = 0x5f,
>> + [PS3R_BIT_CROSS] = 0x5e,
>> + [PS3R_BIT_CIRCLE] = 0x5d,
>> + [PS3R_BIT_TRIANGLE] = 0x5c,
>> + [PS3R_BIT_R1] = 0x5b,
>> + [PS3R_BIT_L1] = 0x5a,
>> + [PS3R_BIT_R2] = 0x59,
>> + [PS3R_BIT_L2] = 0x58,
>> + [PS3R_BIT_LEFT] = 0x57,
>> + [PS3R_BIT_DOWN] = 0x56,
>> + [PS3R_BIT_RIGHT] = 0x55,
>> + [PS3R_BIT_UP] = 0x54,
>> + [PS3R_BIT_START] = 0x53,
>> + [PS3R_BIT_R3] = 0x52,
>> + [PS3R_BIT_L3] = 0x51,
>> + [PS3R_BIT_SELECT] = 0x50,
>> +};
>> +
>> +const static unsigned int ps3remote_keymap[] = {
>
> From scripts/checkpatch.pl:
>
> WARNING: storage class should be at the beginning of the declaration
> #171: FILE: drivers/hid/hid-ps3remote.c:64:
> +const static unsigned int ps3remote_keymap[] = {
>
>> + [0x16] = KEY_EJECTCD,
>> + [0x64] = KEY_AUDIO,
>> + [0x65] = KEY_ANGLE,
>> + [0x63] = KEY_SUBTITLE,
>> + [0x0f] = KEY_CLEAR,
>> + [0x28] = KEY_TIME,
>> + [0x00] = KEY_1,
>> + [0x01] = KEY_2,
>> + [0x02] = KEY_3,
>> + [0x03] = KEY_4,
>> + [0x04] = KEY_5,
>> + [0x05] = KEY_6,
>> + [0x06] = KEY_7,
>> + [0x07] = KEY_8,
>> + [0x08] = KEY_9,
>> + [0x09] = KEY_0,
>> + [0x81] = KEY_RED,
>> + [0x82] = KEY_GREEN,
>> + [0x80] = KEY_BLUE,
>> + [0x83] = KEY_YELLOW,
>> + [0x70] = KEY_INFO, /* display */
>> + [0x1a] = KEY_MENU, /* top menu */
>> + [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
>> + [0x0e] = KEY_ESC, /* return */
>> + [0x5c] = KEY_OPTION, /* options/triangle */
>> + [0x5d] = KEY_BACK, /* back/circle */
>> + [0x5f] = KEY_SCREEN, /* view/square */
>> + [0x5e] = BTN_0, /* cross */
>> + [0x54] = KEY_UP,
>> + [0x56] = KEY_DOWN,
>> + [0x57] = KEY_LEFT,
>> + [0x55] = KEY_RIGHT,
>> + [0x0b] = KEY_ENTER,
>> + [0x5a] = BTN_TL, /* L1 */
>> + [0x58] = BTN_TL2, /* L2 */
>> + [0x51] = BTN_THUMBL, /* L3 */
>> + [0x5b] = BTN_TR, /* R1 */
>> + [0x59] = BTN_TR2, /* R2 */
>> + [0x52] = BTN_THUMBR, /* R3 */
>> + [0x43] = KEY_HOMEPAGE, /* PS button */
>> + [0x50] = KEY_SELECT,
>> + [0x53] = BTN_START,
>> + [0x33] = KEY_REWIND, /* scan back */
>> + [0x32] = KEY_PLAY,
>> + [0x34] = KEY_FORWARD, /* scan forward */
>> + [0x30] = KEY_PREVIOUS,
>> + [0x38] = KEY_STOP,
>> + [0x31] = KEY_NEXT,
>> + [0x60] = KEY_FRAMEBACK, /* slow/step back */
>> + [0x39] = KEY_PAUSE,
>> + [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
>> +};
>> +
>> +static __u8 ps3remote_rdesc[] = {
>> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
>> + 0x09, 0x05, /* USAGE (Game Pad) */
>> + 0xa1, 0x01, /* COLLECTION (Application) */
>> + 0x05, 0x06, /* USAGE PAGE (Generic Device) */
>> + 0x09, 0x20, /* USAGE (Battery Strength) */
>> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
>> + 0x25, 0x64, /* LOGICAL MAXIMUM (100) */
>> + 0x75, 0x08, /* REPORT SIZE (8) */
>> + 0x95, 0x01, /* REPORT COUNT (1) */
>> + 0x81, 0x02, /* INPUT (Variable, Absolute) */
>> + 0x05, 0x09, /* USAGE PAGE (Button) */
>> + 0x19, 0x00, /* USAGE MINUMUM (0) */
>> + 0x29, 0xfe, /* USAGE MAXIMUM (254) */
>> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
>> + 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
>> + 0x75, 0x08, /* REPORT SIZE (8) */
>> + 0x95, 0x01, /* REPORT COUNT (11) */
>> + 0x81, 0x00, /* INPUT (Array, Absolute) */
>> + 0xc0, /* END_COLLECTION */
>> +};
>> +
>> +struct ps3remote_data {
>> + u8 report[12];
>
> Do we want to define a symbolic const for the report size and use that
> when handling data->report below? Just a suggestion tho, I don't have a
> strong opinion about that.
>
>> + u8 last_key;
>> + u32 last_mask;
>> +};
>> +
>> +static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
>> + unsigned int *rsize)
>> +{
>> + *rsize = sizeof(ps3remote_rdesc);
>> + return ps3remote_rdesc;
>> +}
>> +
>> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
>> + struct hid_field *field, struct hid_usage *usage,
>> + unsigned long **bit, int *max)
>> +{
>> + unsigned int key = usage->hid & HID_USAGE;
>> +
>> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
>> + key >= ARRAY_SIZE(ps3remote_keymap))
>> + return -1;
>> +
>> + key = ps3remote_keymap[key];
>> + if (!key)
>> + return -1;
>> +
>> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
>> + return 1;
>> +}
>> +
>> +static void ps3remote_keychange(struct ps3remote_data *data, u8 key, u8 pressed)
>> +{
>> + /*
>> + * Update our report to include/exclude the key that changed, but
>> + * make sure it doesn't get listed twice. It's OK if we don't have
>> + * room to store a keypress, as that means 11 keys are simultaneously
>> + * pressed.
>> + */
>> + u8 *p = memchr(data->report + 1, key, 11);
>> + if (pressed) {
>> + if (p)
>> + return;
>> +
>> + /* Not present, find a free spot to add it */
>> + p = memchr(data->report + 1, 0xff, 11);
>> + } else
>> + key = 0xff;
>
> In Documentation/CodingStyle is suggested to use braces here in the
> 'else' block as well if the 'if' branch has more then one instruction.
>
>> +
>> + if (p)
>> + *p = key;
>> +}
>> +
>> +static int ps3remote_event(struct hid_device *hdev, struct hid_report *report,
>> + u8 *raw_data, int size)
>> +{
>> + struct ps3remote_data *data = hid_get_drvdata(hdev);
>> + u32 mask, changed;
>> + u8 key, pressed;
>
> The preferred kernel style for declarations is to have one per line, no
> commas, but that's up to you.
>
>> + int i;
>> +
>> + if (size != 12) {
>> + hid_dbg(hdev, "unsupported report size\n");
>> + return 1;
>
> The raw_event callback should return negative on error, is there a
> reason why 1 is returned here?
>
>> + }
>> +
>> + /* Battery appears to range from 0 to 5, convert into a percentage */
>> + data->report[0] = raw_data[11] * 20;
>> +
>
> What about writing (100 / 5) explicitly? The compiler is going to
> optimize that anyway but you _show_ that you are converting to a
> percentage beside _telling_ that in the comment. Defining some
> MAX_BATTERY constant maybe is overkill here, but I won't object if you
> do it :)
>
>> + /*
>> + * Sometimes key presses are reported in the bitmask, sometimes
>> + * standalone. It appears the bitmask is used for buttons that
>> + * may pressed at the same time.
>
> I think it should be "may be pressed" here: missing "be".
>
>> + */
>> + mask = be32_to_cpup((__be32 *) raw_data);
>> + mask &= ~0xff000000;
>> + key = raw_data[4];
>> + pressed = raw_data[10];
>> +
>> + changed = data->last_mask ^ mask;
>> + if (changed) {
>> + /* Update any changed multiple keypress reports */
>> + for (i = 0; i < 24; i++) {
>
> Consider replacing the 24 here with ARRAY_SIZE(ps3remote_bits).
>
>> + if ((changed & (1 << i)) && ps3remote_bits[i]) {
>> + ps3remote_keychange(data, ps3remote_bits[i],
>> + !!(mask & (1 << i)));
>> + }
>> + }
>> + data->last_mask = mask;
>> + } else if (pressed && key != 0xff) {
>> + /* Single key pressed */
>> + ps3remote_keychange(data, key, 1);
>> + data->last_key = key;
>> + } else if (!pressed && data->last_key != 0xff) {
>> + /* Single key released */
>> + ps3remote_keychange(data, data->last_key, 0);
>> + data->last_key = 0xff;
>> + } else {
>> + hid_dbg(hdev, "Incoherent report, %06x %06x %02x %02x %02x\n",
>> + mask, data->last_mask, key, data->last_key, pressed);
>> + }
>> +
>> + /* Substitue our updated report */
>
> Typo: "Substitute".
>
>> + memcpy(raw_data, data->report, 12);
>
> The 12 here maybe can be written as a sizeof.
>
>> + return 0;
>> +}
>> +
>> +static int ps3remote_probe(struct hid_device *hdev,
>> + const struct hid_device_id *id)
>> +{
>> + struct ps3remote_data *data;
>> + int ret = -ENOMEM;
>> +
>> + data = kmalloc(sizeof(*data), GFP_KERNEL);
>> + if (!data) {
>> + hid_err(hdev, "Can't alloc private data\n");
>> + goto err;
>> + }
>> +
>> + data->report[0] = 0;
>> + memset(data->report + 1, 0xff, 11);
>> + data->last_key = 0xff;
>> + data->last_mask = 0;
>> +
>> + hid_set_drvdata(hdev, data);
>> +
>> + ret = hid_parse(hdev);
>> + if (ret) {
>> + hid_err(hdev, "HID parse failed\n");
>> + goto err;
>> + }
>> +
>> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> + if (ret) {
>> + hid_err(hdev, "HW start failed\n");
>> + goto err;
>> + }
>> +
>> + return ret;
>> +
>> +err:
>> + kfree(data);
>> + return ret;
>> +}
>> +
>> +static void ps3remote_remove(struct hid_device *hdev)
>> +{
>> + struct ps3remote_data *data = hid_get_drvdata(hdev);
>> +
>> + hid_hw_stop(hdev);
>> + kfree(data);
>> +}
>> +
>> +static const struct hid_device_id ps3remote_devices[] = {
>> + /* PS3 BD Remote */
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
>> + /* Logitech Harmony Adapter for PS3 */
>> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(hid, ps3remote_devices);
>> +
>> +static struct hid_driver ps3remote_driver = {
>> + .name = "ps3_remote",
>
> From scripts/checkpatch.pl:
>
> WARNING: please, no space before tabs
> #410: FILE: drivers/hid/hid-ps3remote.c:303:
> +^I.name ^I^I= "ps3_remote",$
>
> I'd use spaces for aligning the '=' signs here.
> My rule of thumb is "TABs for indentation, spaces for alignment".
>
>> + .id_table = ps3remote_devices,
>> + .probe = ps3remote_probe,
>> + .remove = ps3remote_remove,
>> + .report_fixup = ps3remote_fixup,
>> + .input_mapping = ps3remote_mapping,
>> + .raw_event = ps3remote_event,
>> +};
>> +
>> +static int __init ps3remote_init(void)
>> +{
>> + return hid_register_driver(&ps3remote_driver);
>> +}
>> +
>> +static void __exit ps3remote_exit(void)
>> +{
>> + hid_unregister_driver(&ps3remote_driver);
>> +}
>> +
>> +module_init(ps3remote_init);
>> +module_exit(ps3remote_exit);
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("David Dillow <[email protected]>");
>>
>
> Thanks for working on that.
>
> Reviewed-by: Antonio Ospite <[email protected]>
>
> I just did a syntax/style review, maybe I will take another look at the
> actual logic in the future to see if the report decoding procedure can
> be improved but for now I think it's OK, we want to have this driver in
> ASAP.
>
> David D. please bring up again the issue about missing keypresses on
> re-connection when sending the driver to linux-input with a full
> description of what you observed, I don't have a clue about these
> matters but people on linux-input might.
I now have the remote, I tried using xinput test and most keys seems
to be working fine except the special buttons like subtitle, colors,
x, l1... Im not sure if they are not being mapped because they don't
have any representation or there is something wrong in the parser
itself (Bastien do they used to work for you?). Also got a problem on
suspend but I will have to reproduce it again to see if it is because
of the new driver or not.
--
Luiz Augusto von Dentz
On Thu, 13 Sep 2012 23:04:52 -0400
David Dillow <[email protected]> wrote:
I'd use the official name "Sony PS3 BD Remote Control" in the subject
repeating it as "Sony PS3 Blue-ray Disc Remote Control" in the long
commit message to increase the googlable surface a little bit.
> We also gain support for the Logitech Harmony Adapter for PS3.
>
> Signed-off-by: David Dillow <[email protected]>
> --
When you submit the patch to linux-input for inclusion remember to send
one formatted with git-format-patch, for instance the diffstat
separator is '---' not '--', with the latter in your last message the
diffstat and the annotations are taken as part of the commit message
when using git-am, while the annotations _after_ the diffstat are meant
for discussion and to be ignored by git-am.
Other comments inlined below.
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 335 insertions(+), 0 deletions(-)
>
> On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
> On Wed, 12 Sep 2012 22:26:04 +0200 David Herrmann wrote:
> > > So if the devices only need some short setup-command during
> > > initialization and then they send HID reports whenever a button is
> > > pressed, then everything should be very easy to get working. Even
> > > auto-reconnect and 10min idle-disconnect are _very_ easy to get
> > > working in the kernel with the current HID infrastructure.
> >
> > I too will take a look during the week-end about writing a kernel
> > driver, if it's not that much of an effort I might take the project
> > (no promises yet). I have a PS3 BD remote Bastien donated.
>
> Here's a first pass at a kernel driver to support the BD remote and
> Harmony PS adapter. I've tested it on Fedora 16 with single keypresses
> only, as I cannot send multiple simultaneous keypresses with the
> Harmony. This patch is against kernel.org's bluetooth.git as of an hour
> ago, but should apply to other versions without much fuss.
>
> Antonio, if you could test this with the real remote you have, I'd be
> very appreciative.
>
Works here as well with original PS3 BR remote, thanks David D.
About multiple key-presses, when I hold two _different_ keys at the same
time, I get the message:
ps3_remote 0005:054C:0306.0004: Incoherent report, 000000 000000 ff 16
01
I just tried a couple of combinations tho.
> David, could you point me at the controls for the 10min idle-disconnect?
> auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
> though it looks like the Harmony adapter is initiating the disconnect --
> this is likely an area for improvement with the Sony remote.
>
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index fbf4950..7e6ab25 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -534,6 +534,13 @@ config HID_PRIMAX
> Support for Primax devices that are not fully compliant with the
> HID standard.
>
> +config HID_PS3REMOTE
> + tristate "Sony PS3 Bluetooth BD Remote"
Use "Sony PS3 BD Remote Control" here too and mention Bluetooth in the
help section.
> + depends on BT_HIDP
> + ---help---
> + Support for Sony PS3 Bluetooth BD Remote and Logitech Harmony Adapter
> + for PS3.
> +
>From scripts/checkpatch.pl:
WARNING: please write a paragraph that describes the config symbol fully
#60: FILE: drivers/hid/Kconfig:537:
+config HID_PS3REMOTE
This is because the description is too short (< 4 lines), but I think
we can ignore this warning.
> config HID_ROCCAT
> tristate "Roccat device support"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index f975485..333ed6c 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
> obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
> obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 60ea284..1838c12 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
> @@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
As David H. already said, stick with the style of the subsystem for
magic numbers, HID tend to prefer symbolic constants.
> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
> new file mode 100644
> index 0000000..a87346e
> --- /dev/null
> +++ b/drivers/hid/hid-ps3remote.c
> @@ -0,0 +1,325 @@
> +/*
> + * HID driver for Sony PS3 BD Remote
> + *
> + * Copyright (c) 2012 David Dillow <[email protected]>
> + * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
> + * and other kernel HID drivers.
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
What about adding a note about the association maneuver? I mean the one
when we press Start+Enter for a few second when we add the BD remote as
a new BT device. I forget about it every time I associate the BD
remote with a new system; I know kernel code is not the most
user-friendly spot for this documentation but having it here wouldn't
hurt.
Now I noted it on the back of the battery cover too :)
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +enum ps3remote_special_keys {
> + PS3R_BIT_PS = 0,
> + PS3R_BIT_ENTER = 3,
> + PS3R_BIT_L2 = 8,
> + PS3R_BIT_R2 = 9,
> + PS3R_BIT_L1 = 10,
> + PS3R_BIT_R1 = 11,
> + PS3R_BIT_TRIANGLE = 12,
> + PS3R_BIT_CIRCLE = 13,
> + PS3R_BIT_CROSS = 14,
> + PS3R_BIT_SQUARE = 15,
> + PS3R_BIT_SELECT = 16,
> + PS3R_BIT_L3 = 17,
> + PS3R_BIT_R3 = 18,
> + PS3R_BIT_START = 19,
> + PS3R_BIT_UP = 20,
> + PS3R_BIT_RIGHT = 21,
> + PS3R_BIT_DOWN = 22,
> + PS3R_BIT_LEFT = 23,
> +};
> +
> +static unsigned int ps3remote_bits[] = {
This could be const too.
> + [PS3R_BIT_ENTER] = 0x0b,
> + [PS3R_BIT_PS] = 0x43,
> + [PS3R_BIT_SQUARE] = 0x5f,
> + [PS3R_BIT_CROSS] = 0x5e,
> + [PS3R_BIT_CIRCLE] = 0x5d,
> + [PS3R_BIT_TRIANGLE] = 0x5c,
> + [PS3R_BIT_R1] = 0x5b,
> + [PS3R_BIT_L1] = 0x5a,
> + [PS3R_BIT_R2] = 0x59,
> + [PS3R_BIT_L2] = 0x58,
> + [PS3R_BIT_LEFT] = 0x57,
> + [PS3R_BIT_DOWN] = 0x56,
> + [PS3R_BIT_RIGHT] = 0x55,
> + [PS3R_BIT_UP] = 0x54,
> + [PS3R_BIT_START] = 0x53,
> + [PS3R_BIT_R3] = 0x52,
> + [PS3R_BIT_L3] = 0x51,
> + [PS3R_BIT_SELECT] = 0x50,
> +};
> +
> +const static unsigned int ps3remote_keymap[] = {
>From scripts/checkpatch.pl:
WARNING: storage class should be at the beginning of the declaration
#171: FILE: drivers/hid/hid-ps3remote.c:64:
+const static unsigned int ps3remote_keymap[] = {
> + [0x16] = KEY_EJECTCD,
> + [0x64] = KEY_AUDIO,
> + [0x65] = KEY_ANGLE,
> + [0x63] = KEY_SUBTITLE,
> + [0x0f] = KEY_CLEAR,
> + [0x28] = KEY_TIME,
> + [0x00] = KEY_1,
> + [0x01] = KEY_2,
> + [0x02] = KEY_3,
> + [0x03] = KEY_4,
> + [0x04] = KEY_5,
> + [0x05] = KEY_6,
> + [0x06] = KEY_7,
> + [0x07] = KEY_8,
> + [0x08] = KEY_9,
> + [0x09] = KEY_0,
> + [0x81] = KEY_RED,
> + [0x82] = KEY_GREEN,
> + [0x80] = KEY_BLUE,
> + [0x83] = KEY_YELLOW,
> + [0x70] = KEY_INFO, /* display */
> + [0x1a] = KEY_MENU, /* top menu */
> + [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
> + [0x0e] = KEY_ESC, /* return */
> + [0x5c] = KEY_OPTION, /* options/triangle */
> + [0x5d] = KEY_BACK, /* back/circle */
> + [0x5f] = KEY_SCREEN, /* view/square */
> + [0x5e] = BTN_0, /* cross */
> + [0x54] = KEY_UP,
> + [0x56] = KEY_DOWN,
> + [0x57] = KEY_LEFT,
> + [0x55] = KEY_RIGHT,
> + [0x0b] = KEY_ENTER,
> + [0x5a] = BTN_TL, /* L1 */
> + [0x58] = BTN_TL2, /* L2 */
> + [0x51] = BTN_THUMBL, /* L3 */
> + [0x5b] = BTN_TR, /* R1 */
> + [0x59] = BTN_TR2, /* R2 */
> + [0x52] = BTN_THUMBR, /* R3 */
> + [0x43] = KEY_HOMEPAGE, /* PS button */
> + [0x50] = KEY_SELECT,
> + [0x53] = BTN_START,
> + [0x33] = KEY_REWIND, /* scan back */
> + [0x32] = KEY_PLAY,
> + [0x34] = KEY_FORWARD, /* scan forward */
> + [0x30] = KEY_PREVIOUS,
> + [0x38] = KEY_STOP,
> + [0x31] = KEY_NEXT,
> + [0x60] = KEY_FRAMEBACK, /* slow/step back */
> + [0x39] = KEY_PAUSE,
> + [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
> +};
> +
> +static __u8 ps3remote_rdesc[] = {
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x05, /* USAGE (Game Pad) */
> + 0xa1, 0x01, /* COLLECTION (Application) */
> + 0x05, 0x06, /* USAGE PAGE (Generic Device) */
> + 0x09, 0x20, /* USAGE (Battery Strength) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x64, /* LOGICAL MAXIMUM (100) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x02, /* INPUT (Variable, Absolute) */
> + 0x05, 0x09, /* USAGE PAGE (Button) */
> + 0x19, 0x00, /* USAGE MINUMUM (0) */
> + 0x29, 0xfe, /* USAGE MAXIMUM (254) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (11) */
> + 0x81, 0x00, /* INPUT (Array, Absolute) */
> + 0xc0, /* END_COLLECTION */
> +};
> +
> +struct ps3remote_data {
> + u8 report[12];
Do we want to define a symbolic const for the report size and use that
when handling data->report below? Just a suggestion tho, I don't have a
strong opinion about that.
> + u8 last_key;
> + u32 last_mask;
> +};
> +
> +static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + *rsize = sizeof(ps3remote_rdesc);
> + return ps3remote_rdesc;
> +}
> +
> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + unsigned int key = usage->hid & HID_USAGE;
> +
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
> + key >= ARRAY_SIZE(ps3remote_keymap))
> + return -1;
> +
> + key = ps3remote_keymap[key];
> + if (!key)
> + return -1;
> +
> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> + return 1;
> +}
> +
> +static void ps3remote_keychange(struct ps3remote_data *data, u8 key, u8 pressed)
> +{
> + /*
> + * Update our report to include/exclude the key that changed, but
> + * make sure it doesn't get listed twice. It's OK if we don't have
> + * room to store a keypress, as that means 11 keys are simultaneously
> + * pressed.
> + */
> + u8 *p = memchr(data->report + 1, key, 11);
> + if (pressed) {
> + if (p)
> + return;
> +
> + /* Not present, find a free spot to add it */
> + p = memchr(data->report + 1, 0xff, 11);
> + } else
> + key = 0xff;
In Documentation/CodingStyle is suggested to use braces here in the
'else' block as well if the 'if' branch has more then one instruction.
> +
> + if (p)
> + *p = key;
> +}
> +
> +static int ps3remote_event(struct hid_device *hdev, struct hid_report *report,
> + u8 *raw_data, int size)
> +{
> + struct ps3remote_data *data = hid_get_drvdata(hdev);
> + u32 mask, changed;
> + u8 key, pressed;
The preferred kernel style for declarations is to have one per line, no
commas, but that's up to you.
> + int i;
> +
> + if (size != 12) {
> + hid_dbg(hdev, "unsupported report size\n");
> + return 1;
The raw_event callback should return negative on error, is there a
reason why 1 is returned here?
> + }
> +
> + /* Battery appears to range from 0 to 5, convert into a percentage */
> + data->report[0] = raw_data[11] * 20;
> +
What about writing (100 / 5) explicitly? The compiler is going to
optimize that anyway but you _show_ that you are converting to a
percentage beside _telling_ that in the comment. Defining some
MAX_BATTERY constant maybe is overkill here, but I won't object if you
do it :)
> + /*
> + * Sometimes key presses are reported in the bitmask, sometimes
> + * standalone. It appears the bitmask is used for buttons that
> + * may pressed at the same time.
I think it should be "may be pressed" here: missing "be".
> + */
> + mask = be32_to_cpup((__be32 *) raw_data);
> + mask &= ~0xff000000;
> + key = raw_data[4];
> + pressed = raw_data[10];
> +
> + changed = data->last_mask ^ mask;
> + if (changed) {
> + /* Update any changed multiple keypress reports */
> + for (i = 0; i < 24; i++) {
Consider replacing the 24 here with ARRAY_SIZE(ps3remote_bits).
> + if ((changed & (1 << i)) && ps3remote_bits[i]) {
> + ps3remote_keychange(data, ps3remote_bits[i],
> + !!(mask & (1 << i)));
> + }
> + }
> + data->last_mask = mask;
> + } else if (pressed && key != 0xff) {
> + /* Single key pressed */
> + ps3remote_keychange(data, key, 1);
> + data->last_key = key;
> + } else if (!pressed && data->last_key != 0xff) {
> + /* Single key released */
> + ps3remote_keychange(data, data->last_key, 0);
> + data->last_key = 0xff;
> + } else {
> + hid_dbg(hdev, "Incoherent report, %06x %06x %02x %02x %02x\n",
> + mask, data->last_mask, key, data->last_key, pressed);
> + }
> +
> + /* Substitue our updated report */
Typo: "Substitute".
> + memcpy(raw_data, data->report, 12);
The 12 here maybe can be written as a sizeof.
> + return 0;
> +}
> +
> +static int ps3remote_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct ps3remote_data *data;
> + int ret = -ENOMEM;
> +
> + data = kmalloc(sizeof(*data), GFP_KERNEL);
> + if (!data) {
> + hid_err(hdev, "Can't alloc private data\n");
> + goto err;
> + }
> +
> + data->report[0] = 0;
> + memset(data->report + 1, 0xff, 11);
> + data->last_key = 0xff;
> + data->last_mask = 0;
> +
> + hid_set_drvdata(hdev, data);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "HID parse failed\n");
> + goto err;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hdev, "HW start failed\n");
> + goto err;
> + }
> +
> + return ret;
> +
> +err:
> + kfree(data);
> + return ret;
> +}
> +
> +static void ps3remote_remove(struct hid_device *hdev)
> +{
> + struct ps3remote_data *data = hid_get_drvdata(hdev);
> +
> + hid_hw_stop(hdev);
> + kfree(data);
> +}
> +
> +static const struct hid_device_id ps3remote_devices[] = {
> + /* PS3 BD Remote */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
> + /* Logitech Harmony Adapter for PS3 */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, ps3remote_devices);
> +
> +static struct hid_driver ps3remote_driver = {
> + .name = "ps3_remote",
>From scripts/checkpatch.pl:
WARNING: please, no space before tabs
#410: FILE: drivers/hid/hid-ps3remote.c:303:
+^I.name ^I^I= "ps3_remote",$
I'd use spaces for aligning the '=' signs here.
My rule of thumb is "TABs for indentation, spaces for alignment".
> + .id_table = ps3remote_devices,
> + .probe = ps3remote_probe,
> + .remove = ps3remote_remove,
> + .report_fixup = ps3remote_fixup,
> + .input_mapping = ps3remote_mapping,
> + .raw_event = ps3remote_event,
> +};
> +
> +static int __init ps3remote_init(void)
> +{
> + return hid_register_driver(&ps3remote_driver);
> +}
> +
> +static void __exit ps3remote_exit(void)
> +{
> + hid_unregister_driver(&ps3remote_driver);
> +}
> +
> +module_init(ps3remote_init);
> +module_exit(ps3remote_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("David Dillow <[email protected]>");
>
Thanks for working on that.
Reviewed-by: Antonio Ospite <[email protected]>
I just did a syntax/style review, maybe I will take another look at the
actual logic in the future to see if the report decoding procedure can
be improved but for now I think it's OK, we want to have this driver in
ASAP.
David D. please bring up again the issue about missing keypresses on
re-connection when sending the driver to linux-input with a full
description of what you observed, I don't have a clue about these
matters but people on linux-input might.
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Sun, 2012-09-16 at 16:56 -0400, David Dillow wrote:
> On Sun, 2012-09-16 at 21:35 +0200, David Herrmann wrote:
> > Regarding the lost keypresses during connection establishment: All HID
> > drivers loose them. Any input data is actually lost as long as no
> > driver is fully loaded. It's actually not that easy to change that
> > (feel free to have a look at hid-core.c) but I also don't understand
> > why you actually care about them so much? There is driver rebinding,
> > delayed driver loading and similar. So even if you fix it, it can
> > happen that your driver is loaded after a lot of communication has
> > already been done.
> > Could you explain why you actually need these?
>
> If it were only the first time after the driver is loaded, that's one
> thing, but it seems like it's loosing the first button press on each
> connection (ie, after an idle period of about 10 minutes.) The idle
> periods are important for the battery life of the remote, but it's
> really annoying to have to hit pause, wait a second, then hit pause
> again when you are watching a TV show or movie...
>
> I'll double check that this behavior was on each re-connection and not
> just the first time we load the module. I suspect it was each
> connection, though.
Adding some debug statements shows me that this is unlikely to be a
bluetooth or HID issue -- I get the reports just fine. I suspect it is a
timing issue, such that the event gets delivered to the input system
before udev processing has finished informing X or eventlircd etc that a
new device has appeared and they open /dev/input/eventX.
Unfortunately, there doesn't seem to be a way to say "queue the first
few events for a short while" when creating the input device, nor am I
convinced that would be a good fix -- so much would need to change to
use it, and I suspect it is the wrong interface.
It seems like a bit of a hackish solution, but one could avoid the
problem by having the hidp core keep (some) session information alive
after a disconnect -- if requested by user space -- and reuse the HID
devices if the BT device re-connects before the cached information
expires. This would limit the race to once during the closed session
expiration timeout, which I think would be acceptable -- users could
configure the timeout to be weeks for their home entertainment centers.
Would this approach be worth exploring to see if I can find a clean
implementation?
On Sun, 16 Sep 2012 16:56:06 -0400
David Dillow <[email protected]> wrote:
> On Sun, 2012-09-16 at 21:35 +0200, David Herrmann wrote:
[...]
> > Furthermore, please CC [email protected] and [email protected]
> > (HID maintainer) in the next patches. The patch will go through Jiri's
> > tree.
>
> Sure, can do. I may wait a bit longer for Antonio to test it out with
> the real remote rather than the Harmony I have.
>
I did a quick test and the driver works with the original PS3 BD
remote too, with the latest BlueZ master branch.
I am now taking a look at the code, that will take some more time.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Sun, 2012-09-16 at 16:56 -0400, David Dillow wrote:
> On Sun, 2012-09-16 at 21:35 +0200, David Herrmann wrote:
> > Is there a reason for this weird order? I guess it's copied unchanged
> > from userspace? (Or maybe I am just not getting the semantic order
> > here) But I think most entries can be changed to be ascending indexes.
> > Makes adding new ones much easier.
>
> It's copied directly from userspace; looking at the functions
> implemented, I suspect it is somewhat ordered by keys on the physical
> remote. I don't think it's very likely that we'll be adding new entries,
> but sure, I can sort it numerically.
Actually, looking again at a picture of the PS3 BD remote, I think I'd
rather leave the ordering as-is -- it makes testing the mapping easier,
since it is currently ordered by general proximity. I'll add a comment
to this effect.
On Sun, 2012-09-16 at 21:35 +0200, David Herrmann wrote:
> Hi David
>
> On Fri, Sep 14, 2012 at 5:04 AM, David Dillow <[email protected]> wrote:
> > We also gain support for the Logitech Harmony Adapter for PS3.
>
> Could you actually mention that this is based on the user-space BlueZ
> code here? I know your comment below includes it but I think the
> commit message could actually include some links/information on why we
> do this.
Will do.
> > +config HID_PS3REMOTE
> > + tristate "Sony PS3 Bluetooth BD Remote"
> > + depends on BT_HIDP
> > + ---help---
> > + Support for Sony PS3 Bluetooth BD Remote and Logitech Harmony Adapter
> > + for PS3.
> > +
>
> Maybe we can add a link to HID_SONY that the USB PS3 remotes are
> implemented there. And similar HID_SONY could include a link to
> HID_PS3REMOTE that it implements the BT PS3 drivers.
It's not so much PS3 remotes as PS3 controllers/gamepads that are
implemented there. If it makes sense to add the pointer to the help
text, I'm fine with it, but I'd want someone with knowledge of the HW
that HID_SONY supports to vet my text.
> Is there a reason for this weird order? I guess it's copied unchanged
> from userspace? (Or maybe I am just not getting the semantic order
> here) But I think most entries can be changed to be ascending indexes.
> Makes adding new ones much easier.
It's copied directly from userspace; looking at the functions
implemented, I suspect it is somewhat ordered by keys on the physical
remote. I don't think it's very likely that we'll be adding new entries,
but sure, I can sort it numerically.
> Looks all good to me.
> Regarding the lost keypresses during connection establishment: All HID
> drivers loose them. Any input data is actually lost as long as no
> driver is fully loaded. It's actually not that easy to change that
> (feel free to have a look at hid-core.c) but I also don't understand
> why you actually care about them so much? There is driver rebinding,
> delayed driver loading and similar. So even if you fix it, it can
> happen that your driver is loaded after a lot of communication has
> already been done.
> Could you explain why you actually need these?
If it were only the first time after the driver is loaded, that's one
thing, but it seems like it's loosing the first button press on each
connection (ie, after an idle period of about 10 minutes.) The idle
periods are important for the battery life of the remote, but it's
really annoying to have to hit pause, wait a second, then hit pause
again when you are watching a TV show or movie...
I'll double check that this behavior was on each re-connection and not
just the first time we load the module. I suspect it was each
connection, though.
> The hidp_connadd_req.idle_to can be used for the automatic timeout.
> It's Bluetooth only so USB devices won't even notice it.
Ok, and this is controlled by bluetoothd when adding the device,
correct? In any event, it seems that it's my Harmony adapter that is
initiating the disconnect, so this may be a bit moot.
> Furthermore, please CC [email protected] and [email protected]
> (HID maintainer) in the next patches. The patch will go through Jiri's
> tree.
Sure, can do. I may wait a bit longer for Antonio to test it out with
the real remote rather than the Harmony I have.
Thanks for the review,
Dave
Hi David
On Fri, Sep 14, 2012 at 5:04 AM, David Dillow <[email protected]> wrote:
> We also gain support for the Logitech Harmony Adapter for PS3.
Could you actually mention that this is based on the user-space BlueZ
code here? I know your comment below includes it but I think the
commit message could actually include some links/information on why we
do this.
> Signed-off-by: David Dillow <[email protected]>
> --
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 335 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index fbf4950..7e6ab25 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -534,6 +534,13 @@ config HID_PRIMAX
> Support for Primax devices that are not fully compliant with the
> HID standard.
>
> +config HID_PS3REMOTE
> + tristate "Sony PS3 Bluetooth BD Remote"
> + depends on BT_HIDP
> + ---help---
> + Support for Sony PS3 Bluetooth BD Remote and Logitech Harmony Adapter
> + for PS3.
> +
Maybe we can add a link to HID_SONY that the USB PS3 remotes are
implemented there. And similar HID_SONY could include a link to
HID_PS3REMOTE that it implements the BT PS3 drivers.
> config HID_ROCCAT
> tristate "Roccat device support"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index f975485..333ed6c 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
> obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
> obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 60ea284..1838c12 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
As Luiz mentioned, the input drivers generally avoid magic-numbers
here (even though they're actually real magic numbers...).
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
> @@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
> { HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
> new file mode 100644
> index 0000000..a87346e
> --- /dev/null
> +++ b/drivers/hid/hid-ps3remote.c
> @@ -0,0 +1,325 @@
> +/*
> + * HID driver for Sony PS3 BD Remote
> + *
> + * Copyright (c) 2012 David Dillow <[email protected]>
> + * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
> + * and other kernel HID drivers.
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +enum ps3remote_special_keys {
> + PS3R_BIT_PS = 0,
> + PS3R_BIT_ENTER = 3,
> + PS3R_BIT_L2 = 8,
> + PS3R_BIT_R2 = 9,
> + PS3R_BIT_L1 = 10,
> + PS3R_BIT_R1 = 11,
> + PS3R_BIT_TRIANGLE = 12,
> + PS3R_BIT_CIRCLE = 13,
> + PS3R_BIT_CROSS = 14,
> + PS3R_BIT_SQUARE = 15,
> + PS3R_BIT_SELECT = 16,
> + PS3R_BIT_L3 = 17,
> + PS3R_BIT_R3 = 18,
> + PS3R_BIT_START = 19,
> + PS3R_BIT_UP = 20,
> + PS3R_BIT_RIGHT = 21,
> + PS3R_BIT_DOWN = 22,
> + PS3R_BIT_LEFT = 23,
> +};
> +
> +static unsigned int ps3remote_bits[] = {
> + [PS3R_BIT_ENTER] = 0x0b,
> + [PS3R_BIT_PS] = 0x43,
> + [PS3R_BIT_SQUARE] = 0x5f,
> + [PS3R_BIT_CROSS] = 0x5e,
> + [PS3R_BIT_CIRCLE] = 0x5d,
> + [PS3R_BIT_TRIANGLE] = 0x5c,
> + [PS3R_BIT_R1] = 0x5b,
> + [PS3R_BIT_L1] = 0x5a,
> + [PS3R_BIT_R2] = 0x59,
> + [PS3R_BIT_L2] = 0x58,
> + [PS3R_BIT_LEFT] = 0x57,
> + [PS3R_BIT_DOWN] = 0x56,
> + [PS3R_BIT_RIGHT] = 0x55,
> + [PS3R_BIT_UP] = 0x54,
> + [PS3R_BIT_START] = 0x53,
> + [PS3R_BIT_R3] = 0x52,
> + [PS3R_BIT_L3] = 0x51,
> + [PS3R_BIT_SELECT] = 0x50,
> +};
> +
> +const static unsigned int ps3remote_keymap[] = {
> + [0x16] = KEY_EJECTCD,
> + [0x64] = KEY_AUDIO,
> + [0x65] = KEY_ANGLE,
> + [0x63] = KEY_SUBTITLE,
> + [0x0f] = KEY_CLEAR,
> + [0x28] = KEY_TIME,
> + [0x00] = KEY_1,
> + [0x01] = KEY_2,
> + [0x02] = KEY_3,
> + [0x03] = KEY_4,
> + [0x04] = KEY_5,
> + [0x05] = KEY_6,
> + [0x06] = KEY_7,
> + [0x07] = KEY_8,
> + [0x08] = KEY_9,
> + [0x09] = KEY_0,
> + [0x81] = KEY_RED,
> + [0x82] = KEY_GREEN,
> + [0x80] = KEY_BLUE,
> + [0x83] = KEY_YELLOW,
> + [0x70] = KEY_INFO, /* display */
> + [0x1a] = KEY_MENU, /* top menu */
> + [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
> + [0x0e] = KEY_ESC, /* return */
> + [0x5c] = KEY_OPTION, /* options/triangle */
> + [0x5d] = KEY_BACK, /* back/circle */
> + [0x5f] = KEY_SCREEN, /* view/square */
> + [0x5e] = BTN_0, /* cross */
> + [0x54] = KEY_UP,
> + [0x56] = KEY_DOWN,
> + [0x57] = KEY_LEFT,
> + [0x55] = KEY_RIGHT,
> + [0x0b] = KEY_ENTER,
> + [0x5a] = BTN_TL, /* L1 */
> + [0x58] = BTN_TL2, /* L2 */
> + [0x51] = BTN_THUMBL, /* L3 */
> + [0x5b] = BTN_TR, /* R1 */
> + [0x59] = BTN_TR2, /* R2 */
> + [0x52] = BTN_THUMBR, /* R3 */
> + [0x43] = KEY_HOMEPAGE, /* PS button */
> + [0x50] = KEY_SELECT,
> + [0x53] = BTN_START,
> + [0x33] = KEY_REWIND, /* scan back */
> + [0x32] = KEY_PLAY,
> + [0x34] = KEY_FORWARD, /* scan forward */
> + [0x30] = KEY_PREVIOUS,
> + [0x38] = KEY_STOP,
> + [0x31] = KEY_NEXT,
> + [0x60] = KEY_FRAMEBACK, /* slow/step back */
> + [0x39] = KEY_PAUSE,
> + [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
> +};
Is there a reason for this weird order? I guess it's copied unchanged
from userspace? (Or maybe I am just not getting the semantic order
here) But I think most entries can be changed to be ascending indexes.
Makes adding new ones much easier.
> +static __u8 ps3remote_rdesc[] = {
> + 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
> + 0x09, 0x05, /* USAGE (Game Pad) */
> + 0xa1, 0x01, /* COLLECTION (Application) */
> + 0x05, 0x06, /* USAGE PAGE (Generic Device) */
> + 0x09, 0x20, /* USAGE (Battery Strength) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0x64, /* LOGICAL MAXIMUM (100) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (1) */
> + 0x81, 0x02, /* INPUT (Variable, Absolute) */
> + 0x05, 0x09, /* USAGE PAGE (Button) */
> + 0x19, 0x00, /* USAGE MINUMUM (0) */
> + 0x29, 0xfe, /* USAGE MAXIMUM (254) */
> + 0x15, 0x00, /* LOGICAL MINIMUM (0) */
> + 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
> + 0x75, 0x08, /* REPORT SIZE (8) */
> + 0x95, 0x01, /* REPORT COUNT (11) */
> + 0x81, 0x00, /* INPUT (Array, Absolute) */
> + 0xc0, /* END_COLLECTION */
> +};
> +
> +struct ps3remote_data {
> + u8 report[12];
> + u8 last_key;
> + u32 last_mask;
> +};
> +
> +static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + *rsize = sizeof(ps3remote_rdesc);
> + return ps3remote_rdesc;
> +}
> +
> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + unsigned int key = usage->hid & HID_USAGE;
> +
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
> + key >= ARRAY_SIZE(ps3remote_keymap))
> + return -1;
> +
> + key = ps3remote_keymap[key];
> + if (!key)
> + return -1;
> +
> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> + return 1;
> +}
I'd actually like the HID people review this usage-table and
report-fixup. It looks good to me, though.
> +static void ps3remote_keychange(struct ps3remote_data *data, u8 key, u8 pressed)
> +{
> + /*
> + * Update our report to include/exclude the key that changed, but
> + * make sure it doesn't get listed twice. It's OK if we don't have
> + * room to store a keypress, as that means 11 keys are simultaneously
> + * pressed.
> + */
> + u8 *p = memchr(data->report + 1, key, 11);
> + if (pressed) {
> + if (p)
> + return;
> +
> + /* Not present, find a free spot to add it */
> + p = memchr(data->report + 1, 0xff, 11);
> + } else
> + key = 0xff;
> +
> + if (p)
> + *p = key;
> +}
> +
> +static int ps3remote_event(struct hid_device *hdev, struct hid_report *report,
> + u8 *raw_data, int size)
> +{
> + struct ps3remote_data *data = hid_get_drvdata(hdev);
> + u32 mask, changed;
> + u8 key, pressed;
> + int i;
> +
> + if (size != 12) {
> + hid_dbg(hdev, "unsupported report size\n");
> + return 1;
> + }
> +
> + /* Battery appears to range from 0 to 5, convert into a percentage */
> + data->report[0] = raw_data[11] * 20;
> +
> + /*
> + * Sometimes key presses are reported in the bitmask, sometimes
> + * standalone. It appears the bitmask is used for buttons that
> + * may pressed at the same time.
> + */
> + mask = be32_to_cpup((__be32 *) raw_data);
> + mask &= ~0xff000000;
> + key = raw_data[4];
> + pressed = raw_data[10];
> +
> + changed = data->last_mask ^ mask;
> + if (changed) {
> + /* Update any changed multiple keypress reports */
> + for (i = 0; i < 24; i++) {
> + if ((changed & (1 << i)) && ps3remote_bits[i]) {
> + ps3remote_keychange(data, ps3remote_bits[i],
> + !!(mask & (1 << i)));
> + }
> + }
> + data->last_mask = mask;
> + } else if (pressed && key != 0xff) {
> + /* Single key pressed */
> + ps3remote_keychange(data, key, 1);
> + data->last_key = key;
> + } else if (!pressed && data->last_key != 0xff) {
> + /* Single key released */
> + ps3remote_keychange(data, data->last_key, 0);
> + data->last_key = 0xff;
> + } else {
> + hid_dbg(hdev, "Incoherent report, %06x %06x %02x %02x %02x\n",
> + mask, data->last_mask, key, data->last_key, pressed);
> + }
> +
> + /* Substitue our updated report */
> + memcpy(raw_data, data->report, 12);
> + return 0;
> +}
> +
> +static int ps3remote_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct ps3remote_data *data;
> + int ret = -ENOMEM;
> +
> + data = kmalloc(sizeof(*data), GFP_KERNEL);
> + if (!data) {
> + hid_err(hdev, "Can't alloc private data\n");
> + goto err;
> + }
> +
> + data->report[0] = 0;
> + memset(data->report + 1, 0xff, 11);
> + data->last_key = 0xff;
> + data->last_mask = 0;
> +
> + hid_set_drvdata(hdev, data);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "HID parse failed\n");
> + goto err;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hdev, "HW start failed\n");
> + goto err;
> + }
> +
> + return ret;
> +
> +err:
> + kfree(data);
> + return ret;
> +}
> +
> +static void ps3remote_remove(struct hid_device *hdev)
> +{
> + struct ps3remote_data *data = hid_get_drvdata(hdev);
> +
> + hid_hw_stop(hdev);
> + kfree(data);
> +}
> +
> +static const struct hid_device_id ps3remote_devices[] = {
> + /* PS3 BD Remote */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
> + /* Logitech Harmony Adapter for PS3 */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, ps3remote_devices);
> +
> +static struct hid_driver ps3remote_driver = {
> + .name = "ps3_remote",
> + .id_table = ps3remote_devices,
> + .probe = ps3remote_probe,
> + .remove = ps3remote_remove,
> + .report_fixup = ps3remote_fixup,
> + .input_mapping = ps3remote_mapping,
> + .raw_event = ps3remote_event,
> +};
> +
> +static int __init ps3remote_init(void)
> +{
> + return hid_register_driver(&ps3remote_driver);
> +}
> +
> +static void __exit ps3remote_exit(void)
> +{
> + hid_unregister_driver(&ps3remote_driver);
> +}
> +
> +module_init(ps3remote_init);
> +module_exit(ps3remote_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("David Dillow <[email protected]>");
Looks all good to me.
Regarding the lost keypresses during connection establishment: All HID
drivers loose them. Any input data is actually lost as long as no
driver is fully loaded. It's actually not that easy to change that
(feel free to have a look at hid-core.c) but I also don't understand
why you actually care about them so much? There is driver rebinding,
delayed driver loading and similar. So even if you fix it, it can
happen that your driver is loaded after a lot of communication has
already been done.
Could you explain why you actually need these?
The hidp_connadd_req.idle_to can be used for the automatic timeout.
It's Bluetooth only so USB devices won't even notice it.
Furthermore, please CC [email protected] and [email protected]
(HID maintainer) in the next patches. The patch will go through Jiri's
tree.
Thanks a lot!
David
On Fri, 2012-09-14 at 13:42 +0300, Luiz Augusto von Dentz wrote:
> Hi David,
>
> On Fri, Sep 14, 2012 at 6:04 AM, David Dillow <[email protected]> wrote:
> > Antonio, if you could test this with the real remote you have, I'd be
> > very appreciative.
> >
> > David, could you point me at the controls for the 10min idle-disconnect?
>
> I suppose we can just continue using input.conf:IdleTimeout (which
> passed via ioctl in the struct hidp_connadd_req.idle_to) or do you
> think this would be useful for other transports, at least for USB it
> does not make much sense since the cable will stay connected.
I'm agreed that it doesn't make much sense for USB. I would like to find
a way to keep the adapter running -- it seems I don't get the first key
press that the adapter woke up for. This would need to be optional, as I
see this draining the batteries on the real remote, but my adapter is
hardwired and it'd be nice to avoid the initial delay/lost press.
Of course solving the initial press would be handy for everyone,
assuming it isn't a Harmony-only issue.
> > auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
> > though it looks like the Harmony adapter is initiating the disconnect --
> > this is likely an area for improvement with the Sony remote.
>
> Btw, it would probably be a good to add a proper defines for 0x0306
> e.g. USB_DEVICE_ID_SONY_PS3_REMOTE and
> USB_DEVICE_ID_LOGITECH_HARMONY_ADAPTER
Heh, I thought you might say that. I personally prefer the numbers ala
the current style for PCI(e) drivers -- and took the prior existence of
the numbers-only entries as permission -- but I'll defer to the wishes
of the subsystem.
Thanks,
Dave
Hi David,
On Fri, Sep 14, 2012 at 6:04 AM, David Dillow <[email protected]> wrote:
> We also gain support for the Logitech Harmony Adapter for PS3.
>
> Signed-off-by: David Dillow <[email protected]>
> --
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 335 insertions(+), 0 deletions(-)
>
> On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
> On Wed, 12 Sep 2012 22:26:04 +0200 David Herrmann wrote:
>> > So if the devices only need some short setup-command during
>> > initialization and then they send HID reports whenever a button is
>> > pressed, then everything should be very easy to get working. Even
>> > auto-reconnect and 10min idle-disconnect are _very_ easy to get
>> > working in the kernel with the current HID infrastructure.
>>
>> I too will take a look during the week-end about writing a kernel
>> driver, if it's not that much of an effort I might take the project
>> (no promises yet). I have a PS3 BD remote Bastien donated.
>
> Here's a first pass at a kernel driver to support the BD remote and
> Harmony PS adapter. I've tested it on Fedora 16 with single keypresses
> only, as I cannot send multiple simultaneous keypresses with the
> Harmony. This patch is against kernel.org's bluetooth.git as of an hour
> ago, but should apply to other versions without much fuss.
Great that you are looking into it.
> Antonio, if you could test this with the real remote you have, I'd be
> very appreciative.
>
> David, could you point me at the controls for the 10min idle-disconnect?
I suppose we can just continue using input.conf:IdleTimeout (which
passed via ioctl in the struct hidp_connadd_req.idle_to) or do you
think this would be useful for other transports, at least for USB it
does not make much sense since the cable will stay connected.
> auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
> though it looks like the Harmony adapter is initiating the disconnect --
> this is likely an area for improvement with the Sony remote.
Btw, it would probably be a good to add a proper defines for 0x0306
e.g. USB_DEVICE_ID_SONY_PS3_REMOTE and
USB_DEVICE_ID_LOGITECH_HARMONY_ADAPTER
--
Luiz Augusto von Dentz
We also gain support for the Logitech Harmony Adapter for PS3.
Signed-off-by: David Dillow <[email protected]>
--
drivers/hid/Kconfig | 7 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-core.c | 2 +
drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 335 insertions(+), 0 deletions(-)
On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
On Wed, 12 Sep 2012 22:26:04 +0200 David Herrmann wrote:
> > So if the devices only need some short setup-command during
> > initialization and then they send HID reports whenever a button is
> > pressed, then everything should be very easy to get working. Even
> > auto-reconnect and 10min idle-disconnect are _very_ easy to get
> > working in the kernel with the current HID infrastructure.
>
> I too will take a look during the week-end about writing a kernel
> driver, if it's not that much of an effort I might take the project
> (no promises yet). I have a PS3 BD remote Bastien donated.
Here's a first pass at a kernel driver to support the BD remote and
Harmony PS adapter. I've tested it on Fedora 16 with single keypresses
only, as I cannot send multiple simultaneous keypresses with the
Harmony. This patch is against kernel.org's bluetooth.git as of an hour
ago, but should apply to other versions without much fuss.
Antonio, if you could test this with the real remote you have, I'd be
very appreciative.
David, could you point me at the controls for the 10min idle-disconnect?
auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
though it looks like the Harmony adapter is initiating the disconnect --
this is likely an area for improvement with the Sony remote.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index fbf4950..7e6ab25 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -534,6 +534,13 @@ config HID_PRIMAX
Support for Primax devices that are not fully compliant with the
HID standard.
+config HID_PS3REMOTE
+ tristate "Sony PS3 Bluetooth BD Remote"
+ depends on BT_HIDP
+ ---help---
+ Support for Sony PS3 Bluetooth BD Remote and Logitech Harmony Adapter
+ for PS3.
+
config HID_ROCCAT
tristate "Roccat device support"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index f975485..333ed6c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
+obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 60ea284..1838c12 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1591,6 +1591,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RUMBLEPAD2) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACETRAVELLER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_SPACENAVIGATOR) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICOLCD_BOOTLOADER) },
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_COMFORT_MOUSE_4500) },
@@ -1641,6 +1642,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_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_VAIO_VGX_MOUSE) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
{ HID_USB_DEVICE(USB_VENDOR_ID_SUNPLUS, USB_DEVICE_ID_SUNPLUS_WDESKTOP) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb300) },
{ HID_USB_DEVICE(USB_VENDOR_ID_THRUSTMASTER, 0xb304) },
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
new file mode 100644
index 0000000..a87346e
--- /dev/null
+++ b/drivers/hid/hid-ps3remote.c
@@ -0,0 +1,325 @@
+/*
+ * HID driver for Sony PS3 BD Remote
+ *
+ * Copyright (c) 2012 David Dillow <[email protected]>
+ * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
+ * and other kernel HID drivers.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+enum ps3remote_special_keys {
+ PS3R_BIT_PS = 0,
+ PS3R_BIT_ENTER = 3,
+ PS3R_BIT_L2 = 8,
+ PS3R_BIT_R2 = 9,
+ PS3R_BIT_L1 = 10,
+ PS3R_BIT_R1 = 11,
+ PS3R_BIT_TRIANGLE = 12,
+ PS3R_BIT_CIRCLE = 13,
+ PS3R_BIT_CROSS = 14,
+ PS3R_BIT_SQUARE = 15,
+ PS3R_BIT_SELECT = 16,
+ PS3R_BIT_L3 = 17,
+ PS3R_BIT_R3 = 18,
+ PS3R_BIT_START = 19,
+ PS3R_BIT_UP = 20,
+ PS3R_BIT_RIGHT = 21,
+ PS3R_BIT_DOWN = 22,
+ PS3R_BIT_LEFT = 23,
+};
+
+static unsigned int ps3remote_bits[] = {
+ [PS3R_BIT_ENTER] = 0x0b,
+ [PS3R_BIT_PS] = 0x43,
+ [PS3R_BIT_SQUARE] = 0x5f,
+ [PS3R_BIT_CROSS] = 0x5e,
+ [PS3R_BIT_CIRCLE] = 0x5d,
+ [PS3R_BIT_TRIANGLE] = 0x5c,
+ [PS3R_BIT_R1] = 0x5b,
+ [PS3R_BIT_L1] = 0x5a,
+ [PS3R_BIT_R2] = 0x59,
+ [PS3R_BIT_L2] = 0x58,
+ [PS3R_BIT_LEFT] = 0x57,
+ [PS3R_BIT_DOWN] = 0x56,
+ [PS3R_BIT_RIGHT] = 0x55,
+ [PS3R_BIT_UP] = 0x54,
+ [PS3R_BIT_START] = 0x53,
+ [PS3R_BIT_R3] = 0x52,
+ [PS3R_BIT_L3] = 0x51,
+ [PS3R_BIT_SELECT] = 0x50,
+};
+
+const static unsigned int ps3remote_keymap[] = {
+ [0x16] = KEY_EJECTCD,
+ [0x64] = KEY_AUDIO,
+ [0x65] = KEY_ANGLE,
+ [0x63] = KEY_SUBTITLE,
+ [0x0f] = KEY_CLEAR,
+ [0x28] = KEY_TIME,
+ [0x00] = KEY_1,
+ [0x01] = KEY_2,
+ [0x02] = KEY_3,
+ [0x03] = KEY_4,
+ [0x04] = KEY_5,
+ [0x05] = KEY_6,
+ [0x06] = KEY_7,
+ [0x07] = KEY_8,
+ [0x08] = KEY_9,
+ [0x09] = KEY_0,
+ [0x81] = KEY_RED,
+ [0x82] = KEY_GREEN,
+ [0x80] = KEY_BLUE,
+ [0x83] = KEY_YELLOW,
+ [0x70] = KEY_INFO, /* display */
+ [0x1a] = KEY_MENU, /* top menu */
+ [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
+ [0x0e] = KEY_ESC, /* return */
+ [0x5c] = KEY_OPTION, /* options/triangle */
+ [0x5d] = KEY_BACK, /* back/circle */
+ [0x5f] = KEY_SCREEN, /* view/square */
+ [0x5e] = BTN_0, /* cross */
+ [0x54] = KEY_UP,
+ [0x56] = KEY_DOWN,
+ [0x57] = KEY_LEFT,
+ [0x55] = KEY_RIGHT,
+ [0x0b] = KEY_ENTER,
+ [0x5a] = BTN_TL, /* L1 */
+ [0x58] = BTN_TL2, /* L2 */
+ [0x51] = BTN_THUMBL, /* L3 */
+ [0x5b] = BTN_TR, /* R1 */
+ [0x59] = BTN_TR2, /* R2 */
+ [0x52] = BTN_THUMBR, /* R3 */
+ [0x43] = KEY_HOMEPAGE, /* PS button */
+ [0x50] = KEY_SELECT,
+ [0x53] = BTN_START,
+ [0x33] = KEY_REWIND, /* scan back */
+ [0x32] = KEY_PLAY,
+ [0x34] = KEY_FORWARD, /* scan forward */
+ [0x30] = KEY_PREVIOUS,
+ [0x38] = KEY_STOP,
+ [0x31] = KEY_NEXT,
+ [0x60] = KEY_FRAMEBACK, /* slow/step back */
+ [0x39] = KEY_PAUSE,
+ [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
+};
+
+static __u8 ps3remote_rdesc[] = {
+ 0x05, 0x01, /* USAGE PAGE (Generic Desktop) */
+ 0x09, 0x05, /* USAGE (Game Pad) */
+ 0xa1, 0x01, /* COLLECTION (Application) */
+ 0x05, 0x06, /* USAGE PAGE (Generic Device) */
+ 0x09, 0x20, /* USAGE (Battery Strength) */
+ 0x15, 0x00, /* LOGICAL MINIMUM (0) */
+ 0x25, 0x64, /* LOGICAL MAXIMUM (100) */
+ 0x75, 0x08, /* REPORT SIZE (8) */
+ 0x95, 0x01, /* REPORT COUNT (1) */
+ 0x81, 0x02, /* INPUT (Variable, Absolute) */
+ 0x05, 0x09, /* USAGE PAGE (Button) */
+ 0x19, 0x00, /* USAGE MINUMUM (0) */
+ 0x29, 0xfe, /* USAGE MAXIMUM (254) */
+ 0x15, 0x00, /* LOGICAL MINIMUM (0) */
+ 0x25, 0xfe, /* LOGICAL MAXIMUM (254) */
+ 0x75, 0x08, /* REPORT SIZE (8) */
+ 0x95, 0x01, /* REPORT COUNT (11) */
+ 0x81, 0x00, /* INPUT (Array, Absolute) */
+ 0xc0, /* END_COLLECTION */
+};
+
+struct ps3remote_data {
+ u8 report[12];
+ u8 last_key;
+ u32 last_mask;
+};
+
+static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ *rsize = sizeof(ps3remote_rdesc);
+ return ps3remote_rdesc;
+}
+
+static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
+ struct hid_field *field, struct hid_usage *usage,
+ unsigned long **bit, int *max)
+{
+ unsigned int key = usage->hid & HID_USAGE;
+
+ if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON ||
+ key >= ARRAY_SIZE(ps3remote_keymap))
+ return -1;
+
+ key = ps3remote_keymap[key];
+ if (!key)
+ return -1;
+
+ hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
+ return 1;
+}
+
+static void ps3remote_keychange(struct ps3remote_data *data, u8 key, u8 pressed)
+{
+ /*
+ * Update our report to include/exclude the key that changed, but
+ * make sure it doesn't get listed twice. It's OK if we don't have
+ * room to store a keypress, as that means 11 keys are simultaneously
+ * pressed.
+ */
+ u8 *p = memchr(data->report + 1, key, 11);
+ if (pressed) {
+ if (p)
+ return;
+
+ /* Not present, find a free spot to add it */
+ p = memchr(data->report + 1, 0xff, 11);
+ } else
+ key = 0xff;
+
+ if (p)
+ *p = key;
+}
+
+static int ps3remote_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *raw_data, int size)
+{
+ struct ps3remote_data *data = hid_get_drvdata(hdev);
+ u32 mask, changed;
+ u8 key, pressed;
+ int i;
+
+ if (size != 12) {
+ hid_dbg(hdev, "unsupported report size\n");
+ return 1;
+ }
+
+ /* Battery appears to range from 0 to 5, convert into a percentage */
+ data->report[0] = raw_data[11] * 20;
+
+ /*
+ * Sometimes key presses are reported in the bitmask, sometimes
+ * standalone. It appears the bitmask is used for buttons that
+ * may pressed at the same time.
+ */
+ mask = be32_to_cpup((__be32 *) raw_data);
+ mask &= ~0xff000000;
+ key = raw_data[4];
+ pressed = raw_data[10];
+
+ changed = data->last_mask ^ mask;
+ if (changed) {
+ /* Update any changed multiple keypress reports */
+ for (i = 0; i < 24; i++) {
+ if ((changed & (1 << i)) && ps3remote_bits[i]) {
+ ps3remote_keychange(data, ps3remote_bits[i],
+ !!(mask & (1 << i)));
+ }
+ }
+ data->last_mask = mask;
+ } else if (pressed && key != 0xff) {
+ /* Single key pressed */
+ ps3remote_keychange(data, key, 1);
+ data->last_key = key;
+ } else if (!pressed && data->last_key != 0xff) {
+ /* Single key released */
+ ps3remote_keychange(data, data->last_key, 0);
+ data->last_key = 0xff;
+ } else {
+ hid_dbg(hdev, "Incoherent report, %06x %06x %02x %02x %02x\n",
+ mask, data->last_mask, key, data->last_key, pressed);
+ }
+
+ /* Substitue our updated report */
+ memcpy(raw_data, data->report, 12);
+ return 0;
+}
+
+static int ps3remote_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct ps3remote_data *data;
+ int ret = -ENOMEM;
+
+ data = kmalloc(sizeof(*data), GFP_KERNEL);
+ if (!data) {
+ hid_err(hdev, "Can't alloc private data\n");
+ goto err;
+ }
+
+ data->report[0] = 0;
+ memset(data->report + 1, 0xff, 11);
+ data->last_key = 0xff;
+ data->last_mask = 0;
+
+ hid_set_drvdata(hdev, data);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "HID parse failed\n");
+ goto err;
+ }
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev, "HW start failed\n");
+ goto err;
+ }
+
+ return ret;
+
+err:
+ kfree(data);
+ return ret;
+}
+
+static void ps3remote_remove(struct hid_device *hdev)
+{
+ struct ps3remote_data *data = hid_get_drvdata(hdev);
+
+ hid_hw_stop(hdev);
+ kfree(data);
+}
+
+static const struct hid_device_id ps3remote_devices[] = {
+ /* PS3 BD Remote */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, 0x0306) },
+ /* Logitech Harmony Adapter for PS3 */
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0306) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, ps3remote_devices);
+
+static struct hid_driver ps3remote_driver = {
+ .name = "ps3_remote",
+ .id_table = ps3remote_devices,
+ .probe = ps3remote_probe,
+ .remove = ps3remote_remove,
+ .report_fixup = ps3remote_fixup,
+ .input_mapping = ps3remote_mapping,
+ .raw_event = ps3remote_event,
+};
+
+static int __init ps3remote_init(void)
+{
+ return hid_register_driver(&ps3remote_driver);
+}
+
+static void __exit ps3remote_exit(void)
+{
+ hid_unregister_driver(&ps3remote_driver);
+}
+
+module_init(ps3remote_init);
+module_exit(ps3remote_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("David Dillow <[email protected]>");
On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
> I too will take a look during the week-end about writing a kernel
> driver, if it's not that much of an effort I might take the project
> (no promises yet). I have a PS3 BD remote Bastien donated.
>
> David AFAICS from profiles/input/fakehid.c[1], a custom report has to be
> decoded and appropriate input events have to be sent.
I spent some time last night, and have made some progress in
understanding the HID layer. I'm fixing up the report descriptor and
massaging the raw packets to match in an effort to use the common input
code. evtest isn't showing output yet, but I think I'm close.
I only have the Logitech adapter, which does not seem to need the
bitmask handling in fakehid.c -- could you
watch /sys/kernel/debug/hid/0005:*/events while pressing the buttons
that are listed in the ps3remote_bits list -- looks like back/circle,
cross, triangle (options), etc. I'm guessing those are needed for some
devices, but the Harmony adapter just sends the codes needed in
ps3remote_keymap, so it doesn't seem I need to care about the bitmap,
except I'm sure it was done for a reason...
Thanks,
Dave
On Wed, 12 Sep 2012 22:26:04 +0200
David Herrmann <[email protected]> wrote:
> Hi all
>
> On Wed, Sep 12, 2012 at 4:32 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > Hi Bastien
> >
> > Getting the device does not seems to be a problem and we have a
> > similar driver already in the kernel, so Im really optimistic that it
> > will not take long to have this implemented in the kernel.
>
> Please, just redirect me to the information of the device protocols.
> It is pretty simple to make the kernel drivers work with these kinds
> of HID devices. There is no need to look at the wiimote driver as this
> one is pretty complex and does something totally different than normal
> HID drivers.
>
> I am willing to write the kernel drivers, but I need a short
> introduction or some pointers on how these devices work. I cannot
> afford buying these (yeah, not even 30$...) so I also need people
> willing to test these. This should be pretty easy, though.
>
> Also Jiri Kosina (the HID maintainer) is willing to accept any weird
> quirks we need for these devices. But please stop using fakehid. The
> idea is wrong and hidraw should be used to write userspace HID
> drivers. (uhid is only for transport side! So cannot be used here).
> The kernel HID community is also very active and helps a lot on
> maintaining the drivers.
>
> So if the devices only need some short setup-command during
> initialization and then they send HID reports whenever a button is
> pressed, then everything should be very easy to get working. Even
> auto-reconnect and 10min idle-disconnect are _very_ easy to get
> working in the kernel with the current HID infrastructure.
>
I too will take a look during the week-end about writing a kernel
driver, if it's not that much of an effort I might take the project
(no promises yet). I have a PS3 BD remote Bastien donated.
David AFAICS from profiles/input/fakehid.c[1], a custom report has to be
decoded and appropriate input events have to be sent.
Regards,
Antonio
[1]
http://git.kernel.org/?p=bluetooth/bluez.git;a=blob;f=profiles/input/fakehid.c;h=05f5894c7bd537d78a010b3905af9f98c0364002;hb=96f60e1411514fb4b0d8690aca1c02b3c6b32b9c
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Hi all
On Wed, Sep 12, 2012 at 4:32 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Bastien
>
> Getting the device does not seems to be a problem and we have a
> similar driver already in the kernel, so Im really optimistic that it
> will not take long to have this implemented in the kernel.
Please, just redirect me to the information of the device protocols.
It is pretty simple to make the kernel drivers work with these kinds
of HID devices. There is no need to look at the wiimote driver as this
one is pretty complex and does something totally different than normal
HID drivers.
I am willing to write the kernel drivers, but I need a short
introduction or some pointers on how these devices work. I cannot
afford buying these (yeah, not even 30$...) so I also need people
willing to test these. This should be pretty easy, though.
Also Jiri Kosina (the HID maintainer) is willing to accept any weird
quirks we need for these devices. But please stop using fakehid. The
idea is wrong and hidraw should be used to write userspace HID
drivers. (uhid is only for transport side! So cannot be used here).
The kernel HID community is also very active and helps a lot on
maintaining the drivers.
So if the devices only need some short setup-command during
initialization and then they send HID reports whenever a button is
pressed, then everything should be very easy to get working. Even
auto-reconnect and 10min idle-disconnect are _very_ easy to get
working in the kernel with the current HID infrastructure.
Thanks!
David
Hi Bastien,
On Wed, Sep 12, 2012 at 4:30 PM, Bastien Nocera <[email protected]> wrote:
> Em Wed, 2012-09-12 ?s 13:56 +0300, Luiz Augusto von Dentz escreveu:
>> Hi Bastien,
>>
>> On Tue, Sep 11, 2012 at 11:17 PM, Bastien Nocera <[email protected]> wrote:
>> > Em Tue, 2012-09-11 ?s 17:21 +0300, Luiz Augusto von Dentz escreveu:
>> >> Hi Bastien,
>> >>
>> >> On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
>> >> <[email protected]> wrote:
>> >> > Hi Bastien,
>> >> >
>> >> > On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
>> >> > <[email protected]> wrote:
>> >> >> Hi Bastien,
>> >> >>
>> >> >> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
>> >> >>> Em Mon, 2012-09-10 ?s 16:11 +0300, Luiz Augusto von Dentz escreveu:
>> >> >>>> Hi David, Bastien,
>> >> >>>>
>> >> >>>> So we are plannin to rid of the fakehid.c in favor of implementing it
>> >> >>>> properly inside the kernel similarly to what was done to wiimote, so
>> >> >>>> is there any obstacle for doing that?
>> >> >>>>
>> >> >>>> The kernel seems to already have some support for sixaxis in
>> >> >>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
>> >> >>>> use it:
>> >> >>>
>> >> >>> It won't. They're not the same hardware.
>> >> >>
>> >> >> What hardware is that then? And why wouldn't the kernel be able to
>> >> >> support even if it is a different driver?
>> >> >
>> >> > So what exactly are the difference between 0x0268 and 0x0306? And why
>> >> > sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
>> >> >
>> >> > Also, after fixing sixpair.c to be able to compile it does add to the
>> >> > storage but it does not create any object until bluetoothd is
>> >> > restarted and even after restart it does not allow the device to
>> >> > connect because it does has the record (although this can be fixed by
>> >> > automatically add the UUID once we find out it is attempt to connect).
>> >>
>> >> So apparently the 0x0306 device is the br remote controller, not the
>> >> sixaxis joystick, sorry about the confusion.
>> >
>> > Completely different protocols for the devices, indeed.
>> >
>> >> Anyway regardless of
>> >> being a different device I thing we should move this code to kernel as
>> >> it was done for wiimote.
>> >
>> > Certainly, but until somebody writes the code (I already have at least 3
>> > drivers to submit to the kernel that I've not had time to handle,
>> > include one Bluetooth device), I think it would be nice not to drop
>> > support for the existing hardware. The cost of David's patch is close to
>> > none.
>>
>> Do you have any code already?
>
> Why would I? I don't have time to handle the 3 other drivers, and my BD
> remote is now in a shipping container.
>
>> Anyway it is now removed from upstream,
>
> Why would you want to do that when we don't have a replacement in the
> kernel yet? I just spent time getting that timeout patch upstreamed,
> Johan even merged up David's patch, and 3 days after you remove the
> whole functionality.
Actually it wasn't me who removed:
commit 2e16cbf32569d834750f47107ee9c19954dd28bf
Author: Johan Hedberg <[email protected]>
Date: Mon Sep 10 15:51:23 2012 +0300
input: Remove fakhid functionality
The HSP code conflicts with a real HSP implementation and the PS3
support should be done through the kernel.
So Johan supports the decision as well.
>> I will try to get one of these devices myself and start writing some
>> code if you don't have it done already.
>
> Around $20:
> http://www.amazon.com/Media-Blu-ray-Remote-Control-Playstation-3/dp/B0050SX9I2/
>
> Really not happy we're removing the functionality without having
> adequate replacement. That's not something you do with hardware
> enablement. I'm ready to take bets on the length of time where there's
> no support for the remote in distributions because it was removed in
> bluez, and not supported in the kernel.
In one way or the other the changes wouldn't be perfectly aligned, but
we need to move ahead with 5.0 release which probably has higher
priority than just 1 or 2 devices. Now don't get me wrong, I do think
having support for as many devices as possible is great but there is a
much bigger effort in doing the necessary changes for 5.0 and the
fakeinput was in the way of that so we had to make the decision to
remove it since the kernel driver we can do in parallel. Im not sure
how would you do it differently, if we had to wait until the kernel
has the driver we would have to test it against patched BlueZ which is
not that great either.
Getting the device does not seems to be a problem and we have a
similar driver already in the kernel, so Im really optimistic that it
will not take long to have this implemented in the kernel.
--
Luiz Augusto von Dentz
Em Wed, 2012-09-12 às 13:56 +0300, Luiz Augusto von Dentz escreveu:
> Hi Bastien,
>
> On Tue, Sep 11, 2012 at 11:17 PM, Bastien Nocera <[email protected]> wrote:
> > Em Tue, 2012-09-11 às 17:21 +0300, Luiz Augusto von Dentz escreveu:
> >> Hi Bastien,
> >>
> >> On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
> >> <[email protected]> wrote:
> >> > Hi Bastien,
> >> >
> >> > On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
> >> > <[email protected]> wrote:
> >> >> Hi Bastien,
> >> >>
> >> >> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
> >> >>> Em Mon, 2012-09-10 às 16:11 +0300, Luiz Augusto von Dentz escreveu:
> >> >>>> Hi David, Bastien,
> >> >>>>
> >> >>>> So we are plannin to rid of the fakehid.c in favor of implementing it
> >> >>>> properly inside the kernel similarly to what was done to wiimote, so
> >> >>>> is there any obstacle for doing that?
> >> >>>>
> >> >>>> The kernel seems to already have some support for sixaxis in
> >> >>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
> >> >>>> use it:
> >> >>>
> >> >>> It won't. They're not the same hardware.
> >> >>
> >> >> What hardware is that then? And why wouldn't the kernel be able to
> >> >> support even if it is a different driver?
> >> >
> >> > So what exactly are the difference between 0x0268 and 0x0306? And why
> >> > sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
> >> >
> >> > Also, after fixing sixpair.c to be able to compile it does add to the
> >> > storage but it does not create any object until bluetoothd is
> >> > restarted and even after restart it does not allow the device to
> >> > connect because it does has the record (although this can be fixed by
> >> > automatically add the UUID once we find out it is attempt to connect).
> >>
> >> So apparently the 0x0306 device is the br remote controller, not the
> >> sixaxis joystick, sorry about the confusion.
> >
> > Completely different protocols for the devices, indeed.
> >
> >> Anyway regardless of
> >> being a different device I thing we should move this code to kernel as
> >> it was done for wiimote.
> >
> > Certainly, but until somebody writes the code (I already have at least 3
> > drivers to submit to the kernel that I've not had time to handle,
> > include one Bluetooth device), I think it would be nice not to drop
> > support for the existing hardware. The cost of David's patch is close to
> > none.
>
> Do you have any code already?
Why would I? I don't have time to handle the 3 other drivers, and my BD
remote is now in a shipping container.
> Anyway it is now removed from upstream,
Why would you want to do that when we don't have a replacement in the
kernel yet? I just spent time getting that timeout patch upstreamed,
Johan even merged up David's patch, and 3 days after you remove the
whole functionality.
> I will try to get one of these devices myself and start writing some
> code if you don't have it done already.
Around $20:
http://www.amazon.com/Media-Blu-ray-Remote-Control-Playstation-3/dp/B0050SX9I2/
Really not happy we're removing the functionality without having
adequate replacement. That's not something you do with hardware
enablement. I'm ready to take bets on the length of time where there's
no support for the remote in distributions because it was removed in
bluez, and not supported in the kernel.
On Wed, 2012-09-12 at 13:56 +0300, Luiz Augusto von Dentz wrote:
> Hi Bastien,
>
> On Tue, Sep 11, 2012 at 11:17 PM, Bastien Nocera <[email protected]> wrote:
> > Certainly, but until somebody writes the code (I already have at least 3
> > drivers to submit to the kernel that I've not had time to handle,
> > include one Bluetooth device), I think it would be nice not to drop
> > support for the existing hardware. The cost of David's patch is close to
> > none.
>
> Do you have any code already? Anyway it is now removed from upstream,
> I will try to get one of these devices myself and start writing some
> code if you don't have it done already.
I first looked to the kernel for support, but found it in user space and
stopped. I'll take a stab a it, with wiimote as a reference -- it really
shouldn't be too hard.
But it'll take a little time -- the device is in use for MythTV, so I
risk a mutiny if I try to test before bedtime... ;)
Hi Bastien,
On Tue, Sep 11, 2012 at 11:17 PM, Bastien Nocera <[email protected]> wrote:
> Em Tue, 2012-09-11 ?s 17:21 +0300, Luiz Augusto von Dentz escreveu:
>> Hi Bastien,
>>
>> On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
>> <[email protected]> wrote:
>> > Hi Bastien,
>> >
>> > On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
>> > <[email protected]> wrote:
>> >> Hi Bastien,
>> >>
>> >> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
>> >>> Em Mon, 2012-09-10 ?s 16:11 +0300, Luiz Augusto von Dentz escreveu:
>> >>>> Hi David, Bastien,
>> >>>>
>> >>>> So we are plannin to rid of the fakehid.c in favor of implementing it
>> >>>> properly inside the kernel similarly to what was done to wiimote, so
>> >>>> is there any obstacle for doing that?
>> >>>>
>> >>>> The kernel seems to already have some support for sixaxis in
>> >>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
>> >>>> use it:
>> >>>
>> >>> It won't. They're not the same hardware.
>> >>
>> >> What hardware is that then? And why wouldn't the kernel be able to
>> >> support even if it is a different driver?
>> >
>> > So what exactly are the difference between 0x0268 and 0x0306? And why
>> > sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
>> >
>> > Also, after fixing sixpair.c to be able to compile it does add to the
>> > storage but it does not create any object until bluetoothd is
>> > restarted and even after restart it does not allow the device to
>> > connect because it does has the record (although this can be fixed by
>> > automatically add the UUID once we find out it is attempt to connect).
>>
>> So apparently the 0x0306 device is the br remote controller, not the
>> sixaxis joystick, sorry about the confusion.
>
> Completely different protocols for the devices, indeed.
>
>> Anyway regardless of
>> being a different device I thing we should move this code to kernel as
>> it was done for wiimote.
>
> Certainly, but until somebody writes the code (I already have at least 3
> drivers to submit to the kernel that I've not had time to handle,
> include one Bluetooth device), I think it would be nice not to drop
> support for the existing hardware. The cost of David's patch is close to
> none.
Do you have any code already? Anyway it is now removed from upstream,
I will try to get one of these devices myself and start writing some
code if you don't have it done already.
> There's also a bunch of features that I'm not sure how you'd handle,
> like forcing a disconnect timeout for the device (a patch which Johan
> merged a couple of days ago).
We can extend the ioctl and pass the idle timeout to the kernel,
actually if you take a closer look it already does have support for
that in hidp_connadd_req.idle_to. The other option is to use uhid like
we do with low energy and handle this in userspace but then we might
need device drivers matching vendor/product like the kernel does to do
the key mapping and will probably cause more latency.
--
Luiz Augusto von Dentz
Em Tue, 2012-09-11 às 17:21 +0300, Luiz Augusto von Dentz escreveu:
> Hi Bastien,
>
> On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > Hi Bastien,
> >
> > On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
> > <[email protected]> wrote:
> >> Hi Bastien,
> >>
> >> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
> >>> Em Mon, 2012-09-10 às 16:11 +0300, Luiz Augusto von Dentz escreveu:
> >>>> Hi David, Bastien,
> >>>>
> >>>> So we are plannin to rid of the fakehid.c in favor of implementing it
> >>>> properly inside the kernel similarly to what was done to wiimote, so
> >>>> is there any obstacle for doing that?
> >>>>
> >>>> The kernel seems to already have some support for sixaxis in
> >>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
> >>>> use it:
> >>>
> >>> It won't. They're not the same hardware.
> >>
> >> What hardware is that then? And why wouldn't the kernel be able to
> >> support even if it is a different driver?
> >
> > So what exactly are the difference between 0x0268 and 0x0306? And why
> > sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
> >
> > Also, after fixing sixpair.c to be able to compile it does add to the
> > storage but it does not create any object until bluetoothd is
> > restarted and even after restart it does not allow the device to
> > connect because it does has the record (although this can be fixed by
> > automatically add the UUID once we find out it is attempt to connect).
>
> So apparently the 0x0306 device is the br remote controller, not the
> sixaxis joystick, sorry about the confusion.
Completely different protocols for the devices, indeed.
> Anyway regardless of
> being a different device I thing we should move this code to kernel as
> it was done for wiimote.
Certainly, but until somebody writes the code (I already have at least 3
drivers to submit to the kernel that I've not had time to handle,
include one Bluetooth device), I think it would be nice not to drop
support for the existing hardware. The cost of David's patch is close to
none.
There's also a bunch of features that I'm not sure how you'd handle,
like forcing a disconnect timeout for the device (a patch which Johan
merged a couple of days ago).
Cheers
Hi Bastien,
On Mon, Sep 10, 2012 at 10:03 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Bastien,
>
> On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Bastien,
>>
>> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
>>> Em Mon, 2012-09-10 ?s 16:11 +0300, Luiz Augusto von Dentz escreveu:
>>>> Hi David, Bastien,
>>>>
>>>> So we are plannin to rid of the fakehid.c in favor of implementing it
>>>> properly inside the kernel similarly to what was done to wiimote, so
>>>> is there any obstacle for doing that?
>>>>
>>>> The kernel seems to already have some support for sixaxis in
>>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
>>>> use it:
>>>
>>> It won't. They're not the same hardware.
>>
>> What hardware is that then? And why wouldn't the kernel be able to
>> support even if it is a different driver?
>
> So what exactly are the difference between 0x0268 and 0x0306? And why
> sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
>
> Also, after fixing sixpair.c to be able to compile it does add to the
> storage but it does not create any object until bluetoothd is
> restarted and even after restart it does not allow the device to
> connect because it does has the record (although this can be fixed by
> automatically add the UUID once we find out it is attempt to connect).
So apparently the 0x0306 device is the br remote controller, not the
sixaxis joystick, sorry about the confusion. Anyway regardless of
being a different device I thing we should move this code to kernel as
it was done for wiimote.
--
Luiz Augusto von Dentz
Hi Bastien,
On Mon, Sep 10, 2012 at 4:58 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Bastien,
>
> On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
>> Em Mon, 2012-09-10 ?s 16:11 +0300, Luiz Augusto von Dentz escreveu:
>>> Hi David, Bastien,
>>>
>>> So we are plannin to rid of the fakehid.c in favor of implementing it
>>> properly inside the kernel similarly to what was done to wiimote, so
>>> is there any obstacle for doing that?
>>>
>>> The kernel seems to already have some support for sixaxis in
>>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
>>> use it:
>>
>> It won't. They're not the same hardware.
>
> What hardware is that then? And why wouldn't the kernel be able to
> support even if it is a different driver?
So what exactly are the difference between 0x0268 and 0x0306? And why
sixpair.c save as 0x0268 while fakeinput.c uses 0x0306?
Also, after fixing sixpair.c to be able to compile it does add to the
storage but it does not create any object until bluetoothd is
restarted and even after restart it does not allow the device to
connect because it does has the record (although this can be fixed by
automatically add the UUID once we find out it is attempt to connect).
--
Luiz Augusto von Dentz
Hi Bastien,
On Mon, Sep 10, 2012 at 4:47 PM, Bastien Nocera <[email protected]> wrote:
> Em Mon, 2012-09-10 ?s 16:11 +0300, Luiz Augusto von Dentz escreveu:
>> Hi David, Bastien,
>>
>> So we are plannin to rid of the fakehid.c in favor of implementing it
>> properly inside the kernel similarly to what was done to wiimote, so
>> is there any obstacle for doing that?
>>
>> The kernel seems to already have some support for sixaxis in
>> drivers/hid/hid-sony.c, so I suppose the following would enable us to
>> use it:
>
> It won't. They're not the same hardware.
What hardware is that then? And why wouldn't the kernel be able to
support even if it is a different driver?
--
Luiz Augusto von Dentz
Em Mon, 2012-09-10 às 16:11 +0300, Luiz Augusto von Dentz escreveu:
> Hi David, Bastien,
>
> So we are plannin to rid of the fakehid.c in favor of implementing it
> properly inside the kernel similarly to what was done to wiimote, so
> is there any obstacle for doing that?
>
> The kernel seems to already have some support for sixaxis in
> drivers/hid/hid-sony.c, so I suppose the following would enable us to
> use it:
It won't. They're not the same hardware.
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 41c34f2..a34d92c 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -684,6 +684,7 @@
> #define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
> #define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
> #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
> +#define BT_DEVICE_ID_SONY_PS3_CONTROLLER 0x0306
>
> #define USB_VENDOR_ID_SOUNDGRAPH 0x15c2
> #define USB_DEVICE_ID_SOUNDGRAPH_IMON_FIRST 0x0034
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 5cd25bd..17dd1bb 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -216,6 +216,8 @@ static const struct hid_device_id sony_devices[] = {
> .driver_data = SIXAXIS_CONTROLLER_USB },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
> USB_DEVICE_ID_SONY_PS3_CONTROLLER),
> .driver_data = SIXAXIS_CONTROLLER_BT },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
> BT_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 },
> { }
>
Hi David, Bastien,
So we are plannin to rid of the fakehid.c in favor of implementing it
properly inside the kernel similarly to what was done to wiimote, so
is there any obstacle for doing that?
The kernel seems to already have some support for sixaxis in
drivers/hid/hid-sony.c, so I suppose the following would enable us to
use it:
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 41c34f2..a34d92c 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -684,6 +684,7 @@
#define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
#define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
+#define BT_DEVICE_ID_SONY_PS3_CONTROLLER 0x0306
#define USB_VENDOR_ID_SOUNDGRAPH 0x15c2
#define USB_DEVICE_ID_SOUNDGRAPH_IMON_FIRST 0x0034
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 5cd25bd..17dd1bb 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -216,6 +216,8 @@ static const struct hid_device_id sony_devices[] = {
.driver_data = SIXAXIS_CONTROLLER_USB },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
USB_DEVICE_ID_SONY_PS3_CONTROLLER),
.driver_data = SIXAXIS_CONTROLLER_BT },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
BT_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 },
{ }
--
Luiz Augusto von Dentz
Hi David,
On Thu, Aug 30, 2012, David Dillow wrote:
> This emulates a Sony BD Remote for the Logitech Harmony series of
> universal remotes.
> --
> profiles/input/fakehid.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
Applied. Thanks.
Johan
On Wed, 2012-09-05 at 19:34 +0100, Bastien Nocera wrote:
> On Thu, 2012-08-30 at 22:06 -0400, David Dillow wrote:
> > This emulates a Sony BD Remote for the Logitech Harmony series of
> > universal remotes.
>
> Can't test this, but looks good to me.
Good point, I should mention that this is working quite well for my
MythTV setup downstairs...
On Thu, 2012-08-30 at 22:06 -0400, David Dillow wrote:
> This emulates a Sony BD Remote for the Logitech Harmony series of
> universal remotes.
Can't test this, but looks good to me.
> profiles/input/fakehid.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/profiles/input/fakehid.c b/profiles/input/fakehid.c
> index 3be1489..dd47287 100644
> --- a/profiles/input/fakehid.c
> +++ b/profiles/input/fakehid.c
> @@ -342,6 +342,16 @@ static struct fake_hid fake_hid_table[] = {
> .setup_uinput = ps3remote_setup_uinput,
> .devices = NULL,
> },
> + /* Logitech Harmony Adapter for PS3 */
> + {
> + .vendor = 0x046d,
> + .product = 0x0306,
> + .connect = fake_hid_common_connect,
> + .disconnect = fake_hid_common_disconnect,
> + .event = ps3remote_event,
> + .setup_uinput = ps3remote_setup_uinput,
> + .devices = NULL,
> + },
>
> { },
> };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 1 Oct 2012 10:24:01 +0200 (CEST)
Jiri Kosina <[email protected]> wrote:
> On Mon, 1 Oct 2012, Antonio Ospite wrote:
>
> > Ping.
> >
> > Jiri, I see that linux-3.6 is out, can we have this in for 3.7-rc1?
>
> As replied earlier today, it's in my for-next branch and I'll be pushing
> that for 3.7-rc1.
>
Thanks. I've just seen your message, I hadn't got it yet when I wrote
mine :)
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Mon, 1 Oct 2012, Antonio Ospite wrote:
> Ping.
>
> Jiri, I see that linux-3.6 is out, can we have this in for 3.7-rc1?
As replied earlier today, it's in my for-next branch and I'll be pushing
that for 3.7-rc1.
--
Jiri Kosina
SUSE Labs
On Tue, 25 Sep 2012 23:02:27 +0200
Antonio Ospite <[email protected]> wrote:
Ping.
Jiri, I see that linux-3.6 is out, can we have this in for 3.7-rc1?
Thanks,
Antonio
> From: David Dillow <[email protected]>
>
> The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
> BlueZ project's user space, but the code that handled it was recently
> removed as its functionality conflicted with a real HSP implementation
> and the mapping was thought to be better handled in the kernel. This is
> a port of the mapping logic from the fakehid driver by Marcel Holtmann
> to the in-kernel HID layer.
>
> We also add support for the Logitech Harmony Adapter for PS3, which
> emulates the BD Remote.
>
> Signed-off-by: David Dillow <[email protected]>
> Signed-off-by: Antonio Ospite <[email protected]>
> ---
>
> Changes since v3:
> - move the size check into the switch statement.
>
> Thanks,
> Antonio
>
> drivers/hid/Kconfig | 13 ++-
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ids.h | 2 +
> drivers/hid/hid-ps3remote.c | 215 +++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 232 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hid/hid-ps3remote.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index fbf4950..378be0b 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -534,6 +534,15 @@ config HID_PRIMAX
> Support for Primax devices that are not fully compliant with the
> HID standard.
>
> +config HID_PS3REMOTE
> + tristate "Sony PS3 BD Remote Control"
> + depends on BT_HIDP
> + ---help---
> + Support for the Sony PS3 Blue-ray Disk Remote Control and Logitech
> + Harmony Adapter for PS3, which connect over Bluetooth.
> +
> + Support for the 6-axis controllers is provided by HID_SONY.
> +
> config HID_ROCCAT
> tristate "Roccat device support"
> depends on USB_HID
> @@ -561,7 +570,9 @@ config HID_SONY
> tristate "Sony PS3 controller"
> depends on USB_HID
> ---help---
> - Support for Sony PS3 controller.
> + Support for Sony PS3 6-axis controllers.
> +
> + Support for the Sony PS3 BD Remote is provided by HID_PS3REMOTE.
>
> config HID_SPEEDLINK
> tristate "Speedlink VAD Cezanne mouse support"
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index f975485..333ed6c 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
> obj-$(CONFIG_HID_PETALYNX) += hid-petalynx.o
> obj-$(CONFIG_HID_PICOLCD) += hid-picolcd.o
> obj-$(CONFIG_HID_PRIMAX) += hid-primax.o
> +obj-$(CONFIG_HID_PS3REMOTE) += hid-ps3remote.o
> obj-$(CONFIG_HID_ROCCAT) += hid-roccat.o hid-roccat-common.o \
> hid-roccat-arvo.o hid-roccat-isku.o hid-roccat-kone.o \
> hid-roccat-koneplus.o hid-roccat-kovaplus.o hid-roccat-pyra.o \
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 8bcd168..e4275d4 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1566,6 +1566,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
> @@ -1639,6 +1640,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_IR_REMOTE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SAMSUNG, USB_DEVICE_ID_SAMSUNG_WIRELESS_KBD_MOUSE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SKYCABLE, USB_DEVICE_ID_SKYCABLE_WIRELESS_PRESENTER) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 1dcb76f..40411c9 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -496,6 +496,7 @@
> #define USB_DEVICE_ID_LOGITECH_RECEIVER 0xc101
> #define USB_DEVICE_ID_LOGITECH_HARMONY_FIRST 0xc110
> #define USB_DEVICE_ID_LOGITECH_HARMONY_LAST 0xc14f
> +#define USB_DEVICE_ID_LOGITECH_HARMONY_PS3 0x0306
> #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD 0xc20a
> #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD 0xc211
> #define USB_DEVICE_ID_LOGITECH_EXTREME_3D 0xc215
> @@ -683,6 +684,7 @@
>
> #define USB_VENDOR_ID_SONY 0x054c
> #define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
> +#define USB_DEVICE_ID_SONY_PS3_BDREMOTE 0x0306
> #define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
> #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
>
> diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
> new file mode 100644
> index 0000000..03811e5
> --- /dev/null
> +++ b/drivers/hid/hid-ps3remote.c
> @@ -0,0 +1,215 @@
> +/*
> + * HID driver for Sony PS3 BD Remote Control
> + *
> + * Copyright (c) 2012 David Dillow <[email protected]>
> + * Based on a blend of the bluez fakehid user-space code by Marcel Holtmann
> + * and other kernel HID drivers.
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +/* NOTE: in order for the Sony PS3 BD Remote Control to be found by
> + * a Bluetooth host, the key combination Start+Enter has to be kept pressed
> + * for about 7 seconds with the Bluetooth Host Controller in discovering mode.
> + *
> + * There will be no PIN request from the device.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +static __u8 ps3remote_rdesc[] = {
> + 0x05, 0x01, /* GUsagePage Generic Desktop */
> + 0x09, 0x05, /* LUsage 0x05 [Game Pad] */
> + 0xA1, 0x01, /* MCollection Application (mouse, keyboard) */
> +
> + /* Use collection 1 for joypad buttons */
> + 0xA1, 0x02, /* MCollection Logical (interrelated data) */
> +
> + /* Ignore the 1st byte, maybe it is used for a controller
> + * number but it's not needed for correct operation */
> + 0x75, 0x08, /* GReportSize 0x08 [8] */
> + 0x95, 0x01, /* GReportCount 0x01 [1] */
> + 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
> +
> + /* Bytes from 2nd to 4th are a bitmap for joypad buttons, for these
> + * buttons multiple keypresses are allowed */
> + 0x05, 0x09, /* GUsagePage Button */
> + 0x19, 0x01, /* LUsageMinimum 0x01 [Button 1 (primary/trigger)] */
> + 0x29, 0x18, /* LUsageMaximum 0x18 [Button 24] */
> + 0x14, /* GLogicalMinimum [0] */
> + 0x25, 0x01, /* GLogicalMaximum 0x01 [1] */
> + 0x75, 0x01, /* GReportSize 0x01 [1] */
> + 0x95, 0x18, /* GReportCount 0x18 [24] */
> + 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
> +
> + 0xC0, /* MEndCollection */
> +
> + /* Use collection 2 for remote control buttons */
> + 0xA1, 0x02, /* MCollection Logical (interrelated data) */
> +
> + /* 5th byte is used for remote control buttons */
> + 0x05, 0x09, /* GUsagePage Button */
> + 0x18, /* LUsageMinimum [No button pressed] */
> + 0x29, 0xFE, /* LUsageMaximum 0xFE [Button 254] */
> + 0x14, /* GLogicalMinimum [0] */
> + 0x26, 0xFE, 0x00, /* GLogicalMaximum 0x00FE [254] */
> + 0x75, 0x08, /* GReportSize 0x08 [8] */
> + 0x95, 0x01, /* GReportCount 0x01 [1] */
> + 0x80, /* MInput */
> +
> + /* Ignore bytes from 6th to 11th, 6th to 10th are always constant at
> + * 0xff and 11th is for press indication */
> + 0x75, 0x08, /* GReportSize 0x08 [8] */
> + 0x95, 0x06, /* GReportCount 0x06 [6] */
> + 0x81, 0x01, /* MInput 0x01 (Const[0] Arr[1] Abs[2]) */
> +
> + /* 12th byte is for battery strength */
> + 0x05, 0x06, /* GUsagePage Generic Device Controls */
> + 0x09, 0x20, /* LUsage 0x20 [Battery Strength] */
> + 0x14, /* GLogicalMinimum [0] */
> + 0x25, 0x05, /* GLogicalMaximum 0x05 [5] */
> + 0x75, 0x08, /* GReportSize 0x08 [8] */
> + 0x95, 0x01, /* GReportCount 0x01 [1] */
> + 0x81, 0x02, /* MInput 0x02 (Data[0] Var[1] Abs[2]) */
> +
> + 0xC0, /* MEndCollection */
> +
> + 0xC0 /* MEndCollection [Game Pad] */
> +};
> +
> +static const unsigned int ps3remote_keymap_joypad_buttons[] = {
> + [0x01] = KEY_SELECT,
> + [0x02] = BTN_THUMBL, /* L3 */
> + [0x03] = BTN_THUMBR, /* R3 */
> + [0x04] = BTN_START,
> + [0x05] = KEY_UP,
> + [0x06] = KEY_RIGHT,
> + [0x07] = KEY_DOWN,
> + [0x08] = KEY_LEFT,
> + [0x09] = BTN_TL2, /* L2 */
> + [0x0a] = BTN_TR2, /* R2 */
> + [0x0b] = BTN_TL, /* L1 */
> + [0x0c] = BTN_TR, /* R1 */
> + [0x0d] = KEY_OPTION, /* options/triangle */
> + [0x0e] = KEY_BACK, /* back/circle */
> + [0x0f] = BTN_0, /* cross */
> + [0x10] = KEY_SCREEN, /* view/square */
> + [0x11] = KEY_HOMEPAGE, /* PS button */
> + [0x14] = KEY_ENTER,
> +};
> +static const unsigned int ps3remote_keymap_remote_buttons[] = {
> + [0x00] = KEY_1,
> + [0x01] = KEY_2,
> + [0x02] = KEY_3,
> + [0x03] = KEY_4,
> + [0x04] = KEY_5,
> + [0x05] = KEY_6,
> + [0x06] = KEY_7,
> + [0x07] = KEY_8,
> + [0x08] = KEY_9,
> + [0x09] = KEY_0,
> + [0x0e] = KEY_ESC, /* return */
> + [0x0f] = KEY_CLEAR,
> + [0x16] = KEY_EJECTCD,
> + [0x1a] = KEY_MENU, /* top menu */
> + [0x28] = KEY_TIME,
> + [0x30] = KEY_PREVIOUS,
> + [0x31] = KEY_NEXT,
> + [0x32] = KEY_PLAY,
> + [0x33] = KEY_REWIND, /* scan back */
> + [0x34] = KEY_FORWARD, /* scan forward */
> + [0x38] = KEY_STOP,
> + [0x39] = KEY_PAUSE,
> + [0x40] = KEY_CONTEXT_MENU, /* pop up/menu */
> + [0x60] = KEY_FRAMEBACK, /* slow/step back */
> + [0x61] = KEY_FRAMEFORWARD, /* slow/step forward */
> + [0x63] = KEY_SUBTITLE,
> + [0x64] = KEY_AUDIO,
> + [0x65] = KEY_ANGLE,
> + [0x70] = KEY_INFO, /* display */
> + [0x80] = KEY_BLUE,
> + [0x81] = KEY_RED,
> + [0x82] = KEY_GREEN,
> + [0x83] = KEY_YELLOW,
> +};
> +
> +static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + *rsize = sizeof(ps3remote_rdesc);
> + return ps3remote_rdesc;
> +}
> +
> +static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,
> + struct hid_field *field, struct hid_usage *usage,
> + unsigned long **bit, int *max)
> +{
> + unsigned int key = usage->hid & HID_USAGE;
> +
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
> + return -1;
> +
> + switch (usage->collection_index) {
> + case 1:
> + if (key >= ARRAY_SIZE(ps3remote_keymap_joypad_buttons))
> + return -1;
> +
> + key = ps3remote_keymap_joypad_buttons[key];
> + if (!key)
> + return -1;
> + break;
> + case 2:
> + if (key >= ARRAY_SIZE(ps3remote_keymap_remote_buttons))
> + return -1;
> +
> + key = ps3remote_keymap_remote_buttons[key];
> + if (!key)
> + return -1;
> + break;
> + default:
> + return -1;
> + }
> +
> + hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
> + return 1;
> +}
> +
> +static const struct hid_device_id ps3remote_devices[] = {
> + /* PS3 BD Remote Control */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_BDREMOTE) },
> + /* Logitech Harmony Adapter for PS3 */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, ps3remote_devices);
> +
> +static struct hid_driver ps3remote_driver = {
> + .name = "ps3_remote",
> + .id_table = ps3remote_devices,
> + .report_fixup = ps3remote_fixup,
> + .input_mapping = ps3remote_mapping,
> +};
> +
> +static int __init ps3remote_init(void)
> +{
> + return hid_register_driver(&ps3remote_driver);
> +}
> +
> +static void __exit ps3remote_exit(void)
> +{
> + hid_unregister_driver(&ps3remote_driver);
> +}
> +
> +module_init(ps3remote_init);
> +module_exit(ps3remote_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("David Dillow <[email protected]>, Antonio Ospite <[email protected]>");
> --
> 1.7.10.4
>
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Tue, 25 Sep 2012, Antonio Ospite wrote:
> From: David Dillow <[email protected]>
>
> The Sony PS3 Blue-ray Disc Remote Control used to be supported by the
> BlueZ project's user space, but the code that handled it was recently
> removed as its functionality conflicted with a real HSP implementation
> and the mapping was thought to be better handled in the kernel. This is
> a port of the mapping logic from the fakehid driver by Marcel Holtmann
> to the in-kernel HID layer.
>
> We also add support for the Logitech Harmony Adapter for PS3, which
> emulates the BD Remote.
>
> Signed-off-by: David Dillow <[email protected]>
> Signed-off-by: Antonio Ospite <[email protected]>
Applied, thanks.
--
Jiri Kosina
SUSE Labs