* [PATCHv5 01/17] Bluetooth: Add HCI logical link cmds definitions
From: Andrei Emeltchenko @ 2012-09-17 13:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: mathewm, gustavo
In-Reply-To: <1347888274-30779-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
include/net/bluetooth/hci.h | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0f28f70..42aae18 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -556,12 +556,46 @@ struct hci_cp_accept_phy_link {
__u8 key[HCI_AMP_LINK_KEY_SIZE];
} __packed;
-#define HCI_OP_DISCONN_PHY_LINK 0x0437
+#define HCI_OP_DISCONN_PHY_LINK 0x0437
struct hci_cp_disconn_phy_link {
__u8 phy_handle;
__u8 reason;
} __packed;
+struct ext_flow_spec {
+ __u8 id;
+ __u8 stype;
+ __le16 msdu;
+ __le32 sdu_itime;
+ __le32 acc_lat;
+ __le32 flush_to;
+} __packed;
+
+#define HCI_OP_CREATE_LOGICAL_LINK 0x0438
+#define HCI_OP_ACCEPT_LOGICAL_LINK 0x0439
+struct hci_cp_create_accept_logical_link {
+ __u8 phy_handle;
+ struct ext_flow_spec tx_flow_spec;
+ struct ext_flow_spec rx_flow_spec;
+} __packed;
+
+#define HCI_OP_DISCONN_LOGICAL_LINK 0x043a
+struct hci_cp_disconn_logical_link {
+ __le16 log_handle;
+} __packed;
+
+#define HCI_OP_LOGICAL_LINK_CANCEL 0x043b
+struct hci_cp_logical_link_cancel {
+ __u8 phy_handle;
+ __u8 flow_spec_id;
+} __packed;
+
+struct hci_rp_logical_link_cancel {
+ __u8 status;
+ __u8 phy_handle;
+ __u8 flow_spec_id;
+} __packed;
+
#define HCI_OP_SNIFF_MODE 0x0803
struct hci_cp_sniff_mode {
__le16 handle;
--
1.7.9.5
^ permalink raw reply related
* [PATCHv5 00/17] Bluetooth: Create AMP physical link
From: Andrei Emeltchenko @ 2012-09-17 13:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: mathewm, gustavo
In-Reply-To: <1347437192-24694-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
This set of patches enhances A2MP protocol and creates physical
link between AMP controllers. This is further iteration towards
Bluetooth High Speed.
Changes:
* p5: Fix issues reported by Mat in mailing list review
* p4: Rebased against recent bluetooth-next, minor fixes
* p3: Use hci_conn for representing physical link(type AMP_LINK) instead of
struct phy_link, refactoring.
* p2: Remove HCI callbacks and use amp_mgr global list, refactor code.
* p1: Fixed locking issues, added basic logical link preparation.
* v3: Remove workqueue from callback processing; change callback functions
names according to reviewers recommendations; create global amp_mgr_list to
have lookup to amp manager, physical and logical links so for those HCI events
which might be identified by __handler__ we have lookup; remove extensive
hexdump from gen_amp_key.
* v2: Fix typos and bugs, add functionality: now physical connection
might be established.
* v1: Fix typos, change debug prints, refactor code for better
splitting functionality.
Andrei Emeltchenko (16):
Bluetooth: Add HCI logical link cmds definitions
Bluetooth: A2MP: Create amp_mgr global list
Bluetooth: AMP: Use HCI cmd to Read AMP Info
Bluetooth: AMP: Use HCI cmd to Read Loc AMP Assoc
Bluetooth: A2MP: Process Discover Response
Bluetooth: AMP: Physical link struct and heplers
Bluetooth: AMP: Remote AMP ctrl definitions
Bluetooth: AMP: Handle create / disc phylink req
Bluetooth: A2MP: Process A2MP Getinfo Rsp
Bluetooth: A2MP: Process A2MP Get AMP Assoc Rsp
Bluetooth: Choose connection based on capabilities
Bluetooth: AMP: Add AMP key calculation
Bluetooth: AMP: Create Physical Link
Bluetooth: AMP: Write remote AMP Assoc
Bluetooth: A2MP: Add fallback to normal l2cap init sequence
Bluetooth: AMP: Process Chan Selected event
Dmitry Kasatkin (1):
Bluetooth: Add function to derive AMP key using hmac
include/net/bluetooth/a2mp.h | 20 ++
include/net/bluetooth/amp.h | 29 +++
include/net/bluetooth/hci.h | 39 +++-
include/net/bluetooth/hci_core.h | 11 +
include/net/bluetooth/l2cap.h | 2 +
include/net/bluetooth/pal.h | 40 ++++
net/bluetooth/Kconfig | 1 +
net/bluetooth/Makefile | 2 +-
net/bluetooth/a2mp.c | 424 +++++++++++++++++++++++++++++++++++---
net/bluetooth/amp.c | 159 ++++++++++++++
net/bluetooth/hci_event.c | 106 +++++++++-
net/bluetooth/l2cap_core.c | 35 +++-
net/bluetooth/pal.c | 204 ++++++++++++++++++
13 files changed, 1034 insertions(+), 38 deletions(-)
create mode 100644 include/net/bluetooth/amp.h
create mode 100644 include/net/bluetooth/pal.h
create mode 100644 net/bluetooth/amp.c
create mode 100644 net/bluetooth/pal.c
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH BlueZ v0 4/4] cups: Free D-Bus error
From: Bastien Nocera @ 2012-09-17 11:44 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <1347639011-22924-4-git-send-email-s.syam@samsung.com>
Em Fri, 2012-09-14 às 21:40 +0530, Syam Sidhardhan escreveu:
> If D-Bus error is set we should free it.
Looks good to me.
> ---
> profiles/cups/main.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/cups/main.c b/profiles/cups/main.c
> index a884c6e..5aa927f 100644
> --- a/profiles/cups/main.c
> +++ b/profiles/cups/main.c
> @@ -446,8 +446,10 @@ static gboolean list_known_printers(const char *adapter)
>
> dbus_message_unref(message);
>
> - if (dbus_error_is_set(&error))
> + if (dbus_error_is_set(&error)) {
> + dbus_error_free(&error);
> return FALSE;
> + }
>
> dbus_message_iter_init(reply, &reply_iter);
> if (dbus_message_iter_get_arg_type(&reply_iter) != DBUS_TYPE_ARRAY) {
> @@ -549,8 +551,10 @@ static gboolean list_printers(void)
>
> dbus_error_init(&error);
> hcid_exists = dbus_bus_name_has_owner(conn, "org.bluez", &error);
> - if (dbus_error_is_set(&error))
> + if (dbus_error_is_set(&error)) {
> + dbus_error_free(&error);
> return TRUE;
> + }
>
> if (!hcid_exists)
> return TRUE;
> @@ -570,6 +574,7 @@ static gboolean list_printers(void)
> dbus_message_unref(message);
>
> if (dbus_error_is_set(&error)) {
> + dbus_error_free(&error);
> dbus_connection_unref(conn);
> /* No adapter */
> return TRUE;
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Herrmann @ 2012-09-17 10:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Antonio Ospite, David Dillow, linux-bluetooth, Bastien Nocera
In-Reply-To: <CABBYNZLsZRofkV5DDEj84dzzptUhx5YjCzoyk9UyjRJYU16Y5w@mail.gmail.com>
Hi Luiz
On Mon, Sep 17, 2012 at 12:46 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> HI Antonio,
>>
>> Reviewed-by: Antonio Ospite <ospite@studenti.unina.it>
>>
>> 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
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: Luiz Augusto von Dentz @ 2012-09-17 10:46 UTC (permalink / raw)
To: Antonio Ospite
Cc: David Dillow, linux-bluetooth, David Herrmann, Bastien Nocera
In-Reply-To: <20120917120441.a164d2e10bb02aa9a1e8d523@studenti.unina.it>
HI Antonio,
On Mon, Sep 17, 2012 at 1:04 PM, Antonio Ospite
<ospite@studenti.unina.it> wrote:
> On Thu, 13 Sep 2012 23:04:52 -0400
> David Dillow <dave@thedillows.org> 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 <dave@thedillows.org>
>> --
>
> 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 <dave@thedillows.org>
>> + * 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 <dave@thedillows.org>");
>>
>
> Thanks for working on that.
>
> Reviewed-by: Antonio Ospite <ospite@studenti.unina.it>
>
> 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
^ permalink raw reply
* Re: Wii Balance Board vs. bluez
From: David Herrmann @ 2012-09-17 10:32 UTC (permalink / raw)
To: Florian Echtler; +Cc: linux-bluetooth
In-Reply-To: <CANq1E4RE4ZhfvxhO=MdbMdwUZYrsdYnw=Yn7uLJE4gTAbHhOEw@mail.gmail.com>
Hi Florian
On Mon, Sep 17, 2012 at 11:47 AM, David Herrmann
<dh.herrmann@googlemail.com> wrote:
> Thanks! I have both patches ready for submission. However, could you
> actually tell me in what range data is submitted? As I said, I cannot
> test these patches. But looking at the code, you get 16bit per value
> raw input. You then calculate the per-kg value and multiply it by
> 17kg. So I guess you fill up the whole 16bit?
>
> Your second patch does not adjust the MIN/MAX values for the ABS_HATXY
> values and I am just looking for good values here.
I just noticed these values aren't forced by the input core so I've
just sent the patches to the linux-input ML and put you into CC.
Please respond to them with your signed-off-by and tested-by lines as
I noted in the comment below each commit-message.
Thanks a lot for your work here!
If you're working on any userspace programs, I would interested to
hear about it. Also feel free to use github.com/dvdhrm/xwiimote for
easier communications with the kernel driver. Although, this project
is still TODO ;)
David
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: Antonio Ospite @ 2012-09-17 10:04 UTC (permalink / raw)
To: David Dillow
Cc: linux-bluetooth, David Herrmann, Luiz Augusto von Dentz,
Bastien Nocera
In-Reply-To: <1347591892.6145.14.camel@obelisk.thedillows.org>
On Thu, 13 Sep 2012 23:04:52 -0400
David Dillow <dave@thedillows.org> 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 <dave@thedillows.org>
> --
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 <dave@thedillows.org>
> + * 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 <dave@thedillows.org>");
>
Thanks for working on that.
Reviewed-by: Antonio Ospite <ospite@studenti.unina.it>
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?
^ permalink raw reply
* Re: Wii Balance Board vs. bluez
From: David Herrmann @ 2012-09-17 9:47 UTC (permalink / raw)
To: Florian Echtler; +Cc: linux-bluetooth
In-Reply-To: <50549F3C.8050406@butterbrot.org>
Hi Florian
On Sat, Sep 15, 2012 at 5:31 PM, Florian Echtler <floe@butterbrot.org> wrote:
> Hello David,
>
>
> On 14.09.2012 11:02, David Herrmann wrote:
>>
>> On Thu, Sep 13, 2012 at 2:35 PM, Florian Echtler <floe@butterbrot.org>
>> wrote:
>>>
>>> I've attached an additional patch on top of yours to also read the
>>> calibration data (24 bytes at address 0xa40024). Calibration data is
>>> applied
>>> in handler_balance_board, resulting values are in units of 10 grams.
>>> Although this looks correct to me, I'm not getting any events with my
>>> patch
>>> applied - I suspect some connection to the min/max input values. Do you
>>> have
>>> any ideas what's wrong?
>>
>> Ah, yeah I forgot to change the min/max values of ABS_HAT**. I will
>> fix that. The calibration data looks also nice. I have no idea what
>> went wrong, but you should definitely initialize it to some sane value
>> and fallback to this if you cannot read the data from the device. I
>> would also like to split this into two patches. The first one without
>> calibration and the second one applies the calibration data.
>> I will try to resend these this afternoon, otherwise, I will not have
>> time until Sunday afternoon. Sorry.
>
> never mind - I tested my patch again and noticed that the driver wasn't able
> to read all 24 calibration bytes in one go, which was the root cause for not
> getting any data (I am simply disabling the extension when not getting
> proper calibration data).
>
> I fixed this by doing two reads of 12 bytes each instead. I've also fixed
> some signedness issues with the data calculation - I've attached the updated
> version, which I think should now be ready for integration.
>
> Can you submit both patches to the list when you get around to it (and have
> tested it yourself)?
Thanks! I have both patches ready for submission. However, could you
actually tell me in what range data is submitted? As I said, I cannot
test these patches. But looking at the code, you get 16bit per value
raw input. You then calculate the per-kg value and multiply it by
17kg. So I guess you fill up the whole 16bit?
Your second patch does not adjust the MIN/MAX values for the ABS_HATXY
values and I am just looking for good values here.
Thanks
David
^ permalink raw reply
* [PATCH] Bluetooth: Add support for BCM20702A0 [0a5c:21f1]
From: AceLan Kao @ 2012-09-17 8:40 UTC (permalink / raw)
To: linux-bluetooth, Gustavo F. Padovan, Marcel Holtmann,
Johan Hedberg
Add another vendor specific ID for BCM20702A0.
output of usb-devices:
T: Bus=02 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 3 Spd=12 MxCh= 0
D: Ver= 2.00 Cls=ff(vend.) Sub=01 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=0a5c ProdID=21f1 Rev=01.12
S: Manufacturer=Broadcom Corp
S: Product=BCM20702A0
S: SerialNumber=9CB70D807395
C: #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=0mA
I: If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
I: If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=01 Prot=01 Driver=(none)
I: If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
I: If#= 3 Alt= 0 #EPs= 0 Cls=fe(app. ) Sub=01 Prot=01 Driver=(none)
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/bluetooth/btusb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f637c25..817f661 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -99,6 +99,7 @@ static struct usb_device_id btusb_table[] = {
{ USB_DEVICE(0x0a5c, 0x21e8) },
{ USB_DEVICE(0x0a5c, 0x21f3) },
{ USB_DEVICE(0x413c, 0x8197) },
+ { USB_DEVICE(0x0a5c, 0x21f1) },
/* Foxconn - Hon Hai */
{ USB_DEVICE(0x0489, 0xe033) },
--
1.7.10.4
^ permalink raw reply related
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Dillow @ 2012-09-17 2:38 UTC (permalink / raw)
To: David Herrmann
Cc: linux-bluetooth, Luiz Augusto von Dentz, Bastien Nocera,
Antonio Ospite
In-Reply-To: <1347828966.6762.14.camel@obelisk.thedillows.org>
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?
^ permalink raw reply
* Re: Connecting to Bluetooth Low Energy devices
From: Anderson Lizardo @ 2012-09-17 0:44 UTC (permalink / raw)
To: Philipp Claves; +Cc: linux-bluetooth
In-Reply-To: <6387889.teEb9JopTL@arawn>
Hi,
On Fri, Sep 14, 2012 at 4:55 AM, Philipp Claves
<claves@budelmann-elektronik.com> wrote:
>> Which kernel version are you running?
> Linux 3.5.3
> Which is the minimum version required?
If you run a too old kernel with a recent bluez, bluez will complain
(due to mgmt channel change), so I think this version is fine.
> Thanks, -t random did the trick.
> But it's still strange, the device addresses never changed even once while
> working with them. The bluetooth SIG talk about LE adresses mentioned random
> adresses to change every 15 minutes or so....
> I need to investigate this more.
This behavior depends on which random address type is used.
static random addresses are not required to change. Some (all?)
devices change their static random address when battery is removed.
private random addresses need to change periodically to avoid tracking.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: Antonio Ospite @ 2012-09-16 22:03 UTC (permalink / raw)
To: David Dillow
Cc: David Herrmann, linux-bluetooth, Luiz Augusto von Dentz,
Bastien Nocera
In-Reply-To: <1347828966.6762.14.camel@obelisk.thedillows.org>
On Sun, 16 Sep 2012 16:56:06 -0400
David Dillow <dave@thedillows.org> wrote:
> On Sun, 2012-09-16 at 21:35 +0200, David Herrmann wrote:
[...]
> > Furthermore, please CC linux-input@vger.kernel.org and jkosina@suse.cz
> > (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?
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Dillow @ 2012-09-16 21:12 UTC (permalink / raw)
To: David Herrmann
Cc: linux-bluetooth, Luiz Augusto von Dentz, Bastien Nocera,
Antonio Ospite
In-Reply-To: <1347828966.6762.14.camel@obelisk.thedillows.org>
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.
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Dillow @ 2012-09-16 20:56 UTC (permalink / raw)
To: David Herrmann
Cc: linux-bluetooth, Luiz Augusto von Dentz, Bastien Nocera,
Antonio Ospite
In-Reply-To: <CANq1E4TptZt60phRu=yzGoeBF+v5vqoM=erkUPEj43v0f58xGQ@mail.gmail.com>
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 <dave@thedillows.org> 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 linux-input@vger.kernel.org and jkosina@suse.cz
> (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
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Herrmann @ 2012-09-16 19:35 UTC (permalink / raw)
To: David Dillow
Cc: linux-bluetooth, Luiz Augusto von Dentz, Bastien Nocera,
Antonio Ospite
In-Reply-To: <1347591892.6145.14.camel@obelisk.thedillows.org>
Hi David
On Fri, Sep 14, 2012 at 5:04 AM, David Dillow <dave@thedillows.org> 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 <dave@thedillows.org>
> --
> 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 <dave@thedillows.org>
> + * 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 <dave@thedillows.org>");
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 linux-input@vger.kernel.org and jkosina@suse.cz
(HID maintainer) in the next patches. The patch will go through Jiri's
tree.
Thanks a lot!
David
^ permalink raw reply
* LE generic gateways
From: jonsmirl @ 2012-09-16 14:59 UTC (permalink / raw)
To: linux-bluetooth
>From a CSR presentation...
Bluetooth Low Energy is about generic gateways. Devices that support
Bluetooth low energy Gateway functionality provide a transparent pipe
from a device to an IP address. Middleware at the IP address can
access the device directly as if it were a collector talking to it
locally. The Gateway device plays no part other than in acting as a
pipe.
Has this been implemented on Linux? Is there doc or an example
somewhere on how to use it?
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Wii Balance Board vs. bluez
From: Florian Echtler @ 2012-09-15 15:31 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-bluetooth
In-Reply-To: <CANq1E4TRWGTdpiXsrR3uUp+n1jtptRnSaOp++LBKqyiRM0=eGg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1697 bytes --]
Hello David,
On 14.09.2012 11:02, David Herrmann wrote:
> On Thu, Sep 13, 2012 at 2:35 PM, Florian Echtler <floe@butterbrot.org> wrote:
>> I've attached an additional patch on top of yours to also read the
>> calibration data (24 bytes at address 0xa40024). Calibration data is applied
>> in handler_balance_board, resulting values are in units of 10 grams.
>> Although this looks correct to me, I'm not getting any events with my patch
>> applied - I suspect some connection to the min/max input values. Do you have
>> any ideas what's wrong?
> Ah, yeah I forgot to change the min/max values of ABS_HAT**. I will
> fix that. The calibration data looks also nice. I have no idea what
> went wrong, but you should definitely initialize it to some sane value
> and fallback to this if you cannot read the data from the device. I
> would also like to split this into two patches. The first one without
> calibration and the second one applies the calibration data.
> I will try to resend these this afternoon, otherwise, I will not have
> time until Sunday afternoon. Sorry.
never mind - I tested my patch again and noticed that the driver wasn't
able to read all 24 calibration bytes in one go, which was the root
cause for not getting any data (I am simply disabling the extension when
not getting proper calibration data).
I fixed this by doing two reads of 12 bytes each instead. I've also
fixed some signedness issues with the data calculation - I've attached
the updated version, which I think should now be ready for integration.
Can you submit both patches to the list when you get around to it (and
have tested it yourself)?
Many thanks!
Florian
--
SENT FROM MY DEC VT50 TERMINAL
[-- Attachment #2: balance_calibration_v2.patch --]
[-- Type: text/x-patch, Size: 2742 bytes --]
diff -u a/hid-wiimote-ext.c b/hid-wiimote-ext.c
--- a/hid-wiimote-ext.c 2012-09-13 21:04:01.600494774 +0200
+++ b/hid-wiimote-ext.c 2012-09-15 17:21:57.292848979 +0200
@@ -28,6 +28,7 @@
bool mp_plugged;
bool motionp;
__u8 ext_type;
+ __u16 calib[4][3];
};
enum wiiext_type {
@@ -127,6 +128,7 @@
static __u8 ext_read(struct wiimote_ext *ext)
{
ssize_t ret;
+ __u8 buf[24], i, j, offs = 0;
__u8 rmem[2], wmem;
__u8 type = WIIEXT_NONE;
@@ -159,6 +161,28 @@
rmem[0], rmem[1]);
}
+ /* get balance board calibration */
+ if (type == WIIEXT_BALANCE_BOARD) {
+
+ /* retrieve 24 bytes of calibration data */
+ /* read requests > 16 do not work => read in two batches */
+ ret = wiimote_cmd_read(ext->wdata, 0xa40024 , buf , 12);
+ ret += wiimote_cmd_read(ext->wdata, 0xa40024+12, buf+12, 12);
+
+ /* failed to read calibration data => disable balance board */
+ if (ret != 24)
+ type = WIIEXT_NONE;
+
+ for ( i = 0; i < 3; i++ ) {
+ for ( j = 0 ; j < 4 ; j++ ) {
+ ext->calib[j][i] = ((__u16)buf[offs]<<8 | (__u16)buf[offs+1]);
+ offs += 2;
+ /* printk("calib[%d][%d] = %d\n",j,i,ext->calib[j][i]); */
+ }
+ }
+
+ }
+
wiimote_cmd_release(ext->wdata);
return type;
@@ -517,7 +541,8 @@
static void handler_balance_board(struct wiimote_ext *ext, const __u8 *payload)
{
- __s16 tr, br, tl, bl;
+ __s32 tmp, val[4]; /* tr, br, tl, bl */
+ __u8 i;
/* Byte | 8 7 6 5 4 3 2 1 |
* -----+--------------------------+
@@ -540,19 +565,24 @@
* The balance-board is never reported interleaved with motionp.
*/
- tr = payload[0] << 8;
- tr |= payload[1];
- br = payload[2] << 8;
- br |= payload[3];
- tl = payload[4] << 8;
- tl |= payload[5];
- bl = payload[6] << 8;
- bl |= payload[7];
-
- input_report_abs(ext->input, ABS_HAT0X, tl);
- input_report_abs(ext->input, ABS_HAT0Y, tr);
- input_report_abs(ext->input, ABS_HAT1X, bl);
- input_report_abs(ext->input, ABS_HAT1Y, br);
+ val[0] = (__u16)payload[0] << 8 | (__u16)payload[1];
+ val[1] = (__u16)payload[2] << 8 | (__u16)payload[3];
+ val[2] = (__u16)payload[4] << 8 | (__u16)payload[5];
+ val[3] = (__u16)payload[6] << 8 | (__u16)payload[7];
+
+ /* apply calibration data */
+ for ( i = 0; i < 4; i++ ) {
+ if (val[i] < ext->calib[i][1])
+ tmp = (1700 * (__s32)(val[i] - ext->calib[i][0])) / (__s32)(ext->calib[i][1] - ext->calib[i][0]);
+ else
+ tmp = (1700 * (__s32)(val[i] - ext->calib[i][1])) / (__s32)(ext->calib[i][2] - ext->calib[i][1]) + 1700;
+ val[i] = tmp;
+ }
+
+ input_report_abs(ext->input, ABS_HAT0X, val[0]);
+ input_report_abs(ext->input, ABS_HAT0Y, val[1]);
+ input_report_abs(ext->input, ABS_HAT1X, val[2]);
+ input_report_abs(ext->input, ABS_HAT1Y, val[3]);
input_sync(ext->input);
}
^ permalink raw reply
* bluetoothd: Refusing input device connect: Operation already in progress (114)
From: Pacho Ramos @ 2012-09-15 10:15 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]
Hello
Downstream we got the following report:
https://bugs.gentoo.org/show_bug.cgi?id=431624
After adding mouse (via Blueman's "Add device" dialog) it works fine
until sleeping or powering off. After that it fails reconnecting.
In /var/log/messages appears strings like these:
Aug 16 15:40:31 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Aug 16 15:45:13 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Aug 16 15:45:16 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Aug 16 15:45:17 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Aug 16 15:45:19 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Aug 16 15:47:46 user bluetoothd[3222]: Refusing input device connect:
Operation already in progress (114)
Searching, I found this thread that pointed to the culprit, but I
haven't found what finally occurred with it, if patch was reverted or a
different fix was pulled in:
http://www.spinics.net/lists/linux-bluetooth/msg26442.html
Thanks
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* [RFC] Bluetooth: Fix not removing power_off delayed work
From: Vinicius Costa Gomes @ 2012-09-14 19:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Vinicius Costa Gomes
For example, when a usb reset is received (I could reproduce it
running something very similar to this[1] in a loop) it could be
that the device is unregistered while the power_off delayed work
is still scheduled to run.
Backtrace:
WARNING: at lib/debugobjects.c:261 debug_print_object+0x7c/0x8d()
Hardware name: To Be Filled By O.E.M.
ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x26
Modules linked in: nouveau mxm_wmi btusb wmi bluetooth ttm coretemp drm_kms_helper
Pid: 2114, comm: usb-reset Not tainted 3.5.0bt-next #2
Call Trace:
[<ffffffff8124cc00>] ? free_obj_work+0x57/0x91
[<ffffffff81058f88>] warn_slowpath_common+0x7e/0x97
[<ffffffff81059035>] warn_slowpath_fmt+0x41/0x43
[<ffffffff8124ccb6>] debug_print_object+0x7c/0x8d
[<ffffffff8106e3ec>] ? __queue_work+0x259/0x259
[<ffffffff8124d63e>] ? debug_check_no_obj_freed+0x6f/0x1b5
[<ffffffff8124d667>] debug_check_no_obj_freed+0x98/0x1b5
[<ffffffffa00aa031>] ? bt_host_release+0x10/0x1e [bluetooth]
[<ffffffff810fc035>] kfree+0x90/0xe6
[<ffffffffa00aa031>] bt_host_release+0x10/0x1e [bluetooth]
[<ffffffff812ec2f9>] device_release+0x4a/0x7e
[<ffffffff8123ef57>] kobject_release+0x11d/0x154
[<ffffffff8123ed98>] kobject_put+0x4a/0x4f
[<ffffffff812ec0d9>] put_device+0x12/0x14
[<ffffffffa009472b>] hci_free_dev+0x22/0x26 [bluetooth]
[<ffffffffa0280dd0>] btusb_disconnect+0x96/0x9f [btusb]
[<ffffffff813581b4>] usb_unbind_interface+0x57/0x106
[<ffffffff812ef988>] __device_release_driver+0x83/0xd6
[<ffffffff812ef9fb>] device_release_driver+0x20/0x2d
[<ffffffff813582a7>] usb_driver_release_interface+0x44/0x7b
[<ffffffff81358795>] usb_forced_unbind_intf+0x45/0x4e
[<ffffffff8134f959>] usb_reset_device+0xa6/0x12e
[<ffffffff8135df86>] usbdev_do_ioctl+0x319/0xe20
[<ffffffff81203244>] ? avc_has_perm_flags+0xc9/0x12e
[<ffffffff812031a0>] ? avc_has_perm_flags+0x25/0x12e
[<ffffffff81050101>] ? do_page_fault+0x31e/0x3a1
[<ffffffff8135eaa6>] usbdev_ioctl+0x9/0xd
[<ffffffff811126b1>] vfs_ioctl+0x21/0x34
[<ffffffff81112f7b>] do_vfs_ioctl+0x408/0x44b
[<ffffffff81208d45>] ? file_has_perm+0x76/0x81
[<ffffffff8111300f>] sys_ioctl+0x51/0x76
[<ffffffff8158db22>] system_call_fastpath+0x16/0x1b
[1] http://cpansearch.perl.org/src/DPAVLIN/Biblio-RFID-0.03/examples/usbreset.c
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
---
net/bluetooth/hci_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e407051..8a0ce70 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -737,6 +737,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
cancel_work_sync(&hdev->le_scan);
+ cancel_delayed_work(&hdev->power_off);
+
hci_req_cancel(hdev, ENODEV);
hci_req_lock(hdev);
--
1.7.12
^ permalink raw reply related
* Re: Dropping connection (bit off-topic)
From: John Tobias @ 2012-09-14 19:04 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Vinicius Costa Gomes, linux-bluetooth
In-Reply-To: <CACUGKYMKHvoAbH97-QLbxxYic_+5WjONc76mO642ywf6Jo-5+A@mail.gmail.com>
Hi Anderson,
Could you give me some bit of information on how to respond to the
handle 0x0039?. I'm digging the bluez sources but I'm kind a lost
right now and I don't know exactly what library/function should I use
in order to configure it.
Regards,
John
On Thu, Sep 13, 2012 at 2:59 PM, John Tobias <john.tobias.ph@gmail.com> wrote:
> Hi Anderson,
>
> I added these line of codes in gatttool:
>
> listen_start:
> g_attrib_register(attrib, ATT_OP_READ_BY_TYPE_RESP, events_handler,
> attrib, NULL);
>
> Then, in events_handler I dumped the handles for
> ATT_OP_READ_BY_TYPE_REQ and ATT_OP_READ_BY_TYPE_RESP:
>
> ATT_OP_READ_BY_TYPE_REQ: handle = 0x1 value: ff ff 00 2a
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x207 value: 00 02 03 00 00 2a 04
> 00 02 05 00 01 2a 07 00 20 08 00 05 2a
> ATT_OP_READ_BY_TYPE_RESP: handle = 0xb15 value: 00 02 0c 00 31 3d 35
> 06 1e 60 70 a3 8d 4e 58 9c 2d 6a 3c 16
> ATT_OP_READ_BY_TYPE_RESP: handle = 0xd15 value: 00 8a 0e 00 32 3d 35
> 06 1e 60 70 a3 8d 4e 58 9c 2d 6a 3c 16
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x1015 value: 00 88 11 00 33 3d
> 35 06 1e 60 70 a3 8d 4e 58 9c 2d 6a 3c 16
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x1315 value: 00 9a 14 00 52 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x1715 value: 00 9a 18 00 36 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x1b15 value: 00 9a 1c 00 37 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x1f15 value: 00 8a 20 00 38 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x2215 value: 00 98 23 00 40 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x2615 value: 00 9a 27 00 41 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x2a15 value: 00 98 2b 00 42 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x2e15 value: 00 88 2f 00 60 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x3115 value: 00 88 32 00 61 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x3415 value: 00 12 35 00 62 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
> ATT_OP_READ_BY_TYPE_RESP: handle = 0x3715 value: 00 8a 38 00 39 72
> 3a 71 ae 82 0c ae e0 49 1d 18 c2 b4 d8 37
>
> In hcidump:
>
> ACL data: handle 76 flags 0x02 dlen 9
> ATT: Error (0x01)
> Error: Attribute not found (10)
> Read By Type req (0x08) on handle 0x0039
>
>
> In hcidump, the error was the attribute for handle 0x0039. How I can
> respond to that request if the I did not receive the said handle?
>
>
> Regards,
>
> John
>
>
>
> On Thu, Sep 13, 2012 at 1:18 PM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
>> Hi John,
>>
>> On Thu, Sep 13, 2012 at 4:11 PM, John Tobias <john.tobias.ph@gmail.com> wrote:
>>> Hi Anderson,
>>>
>>> If I add a simple GATT/GAP server on gatttool, should I still use the
>>> DBUS to listen or I can communicate directly to the kernel?.
>>
>> You either talk to bluetoothd (using D-Bus, like those test-* python
>> scripts do), which will handle connections for you, or you talk
>> directly to kernel using bluetooth socket (like gatttool does). You
>> have more flexibility talking to the kernel directly, but your app
>> will not be interoperable with other apps running at the same time,
>> specially if they try to call connect() at the same time (one of them
>> will get a EBUSY error).
>>
>> Using D-Bus, on the other hand, bluetoothd queues the connections properly.
>>
>> So you should use what is more appropriate for your use case.
>>
>> Best Regards,
>> --
>> Anderson Lizardo
>> Instituto Nokia de Tecnologia - INdT
>> Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v2 07/10] battery: Add Battery to device
From: Andrzej Kaczmarek @ 2012-09-14 16:20 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <CA+6KyKas1KMg8XZ=Ch52HCnNGiuXGW7X9cWKDNL68R0=bphHqA@mail.gmail.com>
Hi again,
Small clarification below.
On Fri, Sep 14, 2012 at 6:15 PM, Andrzej Kaczmarek <andrzejk@gmail.com> wrote:
> Hi Chen,
>
> On Thu, Sep 13, 2012 at 3:03 PM, <chen.ganir@ti.com> wrote:
>> From: Chen Ganir <chen.ganir@ti.com>
>>
>> Add/Remove battery from device
>> ---
>> profiles/battery/battery.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
>> index c368768..3a2e604 100644
>> --- a/profiles/battery/battery.c
>> +++ b/profiles/battery/battery.c
>> @@ -49,6 +49,7 @@ struct battery {
>> static GSList *servers;
>>
>> struct characteristic {
>> + struct btd_battery *devbatt; /* device_battery pointer */
>> struct gatt_char attr; /* Characteristic */
>> struct battery *batt; /* Parent Battery Service */
>> GSList *desc; /* Descriptors */
>> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>>
>> g_slist_free_full(c->desc, g_free);
>>
>> + btd_device_remove_battery(c->devbatt);
>> +
>> g_free(c);
>> }
>>
>> @@ -228,6 +231,8 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>>
>> gatt_find_info(batt->attrib, start, end,
>> discover_desc_cb, ch);
>> +
>> + ch->devbatt = btd_device_add_battery(ch->batt->dev);
>
> This should be done before starting characteristics discovery as
> otherwise Battery won't be registered in case characteristics has no
> descriptors (e.g. most devices which do not support notifications for
> Battery Level and have only one battery).
Of course I meant "before starting characteristic descriptors discovery"...
BR,
Andrzej
^ permalink raw reply
* Re: [PATCH v2 07/10] battery: Add Battery to device
From: Andrzej Kaczmarek @ 2012-09-14 16:15 UTC (permalink / raw)
To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1347541403-11780-8-git-send-email-chen.ganir@ti.com>
Hi Chen,
On Thu, Sep 13, 2012 at 3:03 PM, <chen.ganir@ti.com> wrote:
> From: Chen Ganir <chen.ganir@ti.com>
>
> Add/Remove battery from device
> ---
> profiles/battery/battery.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/profiles/battery/battery.c b/profiles/battery/battery.c
> index c368768..3a2e604 100644
> --- a/profiles/battery/battery.c
> +++ b/profiles/battery/battery.c
> @@ -49,6 +49,7 @@ struct battery {
> static GSList *servers;
>
> struct characteristic {
> + struct btd_battery *devbatt; /* device_battery pointer */
> struct gatt_char attr; /* Characteristic */
> struct battery *batt; /* Parent Battery Service */
> GSList *desc; /* Descriptors */
> @@ -79,6 +80,8 @@ static void char_free(gpointer user_data)
>
> g_slist_free_full(c->desc, g_free);
>
> + btd_device_remove_battery(c->devbatt);
> +
> g_free(c);
> }
>
> @@ -228,6 +231,8 @@ static void configure_battery_cb(GSList *characteristics, guint8 status,
>
> gatt_find_info(batt->attrib, start, end,
> discover_desc_cb, ch);
> +
> + ch->devbatt = btd_device_add_battery(ch->batt->dev);
This should be done before starting characteristics discovery as
otherwise Battery won't be registered in case characteristics has no
descriptors (e.g. most devices which do not support notifications for
Battery Level and have only one battery).
> }
> }
> }
BR,
Andrzej
^ permalink raw reply
* [PATCH BlueZ v0 4/4] cups: Free D-Bus error
From: Syam Sidhardhan @ 2012-09-14 16:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1347639011-22924-1-git-send-email-s.syam@samsung.com>
If D-Bus error is set we should free it.
---
profiles/cups/main.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/profiles/cups/main.c b/profiles/cups/main.c
index a884c6e..5aa927f 100644
--- a/profiles/cups/main.c
+++ b/profiles/cups/main.c
@@ -446,8 +446,10 @@ static gboolean list_known_printers(const char *adapter)
dbus_message_unref(message);
- if (dbus_error_is_set(&error))
+ if (dbus_error_is_set(&error)) {
+ dbus_error_free(&error);
return FALSE;
+ }
dbus_message_iter_init(reply, &reply_iter);
if (dbus_message_iter_get_arg_type(&reply_iter) != DBUS_TYPE_ARRAY) {
@@ -549,8 +551,10 @@ static gboolean list_printers(void)
dbus_error_init(&error);
hcid_exists = dbus_bus_name_has_owner(conn, "org.bluez", &error);
- if (dbus_error_is_set(&error))
+ if (dbus_error_is_set(&error)) {
+ dbus_error_free(&error);
return TRUE;
+ }
if (!hcid_exists)
return TRUE;
@@ -570,6 +574,7 @@ static gboolean list_printers(void)
dbus_message_unref(message);
if (dbus_error_is_set(&error)) {
+ dbus_error_free(&error);
dbus_connection_unref(conn);
/* No adapter */
return TRUE;
--
1.7.4.1
^ permalink raw reply related
* [PATCH BlueZ v0 3/4] gstavdtpsink: Free the D-Bus error on reply failure
From: Syam Sidhardhan @ 2012-09-14 16:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1347639011-22924-1-git-send-email-s.syam@samsung.com>
If D-Bus error is set we should free it.
---
I'm not sure whether we can split the error message or not.
audio/gstavdtpsink.c | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/audio/gstavdtpsink.c b/audio/gstavdtpsink.c
index ffaed7f..3427043 100644
--- a/audio/gstavdtpsink.c
+++ b/audio/gstavdtpsink.c
@@ -786,13 +786,22 @@ static gboolean gst_avdtp_sink_transport_get_properties(GstAvdtpSink *self)
msg = dbus_message_new_method_call("org.bluez", self->transport,
"org.bluez.MediaTransport",
"GetProperties");
+ if (!msg) {
+ GST_ERROR_OBJECT(self, "D-Bus Memory allocation failed");
+ return FALSE;
+ }
+
reply = dbus_connection_send_with_reply_and_block(self->data->conn,
msg, -1, &err);
+ dbus_message_unref(msg);
- if (dbus_error_is_set(&err) || reply == NULL) {
- GST_ERROR_OBJECT(self, "Failed to get transport properties: %s",
- err.message);
- goto fail;
+ if (!reply) {
+ if (dbus_error_is_set(&err)) {
+ GST_ERROR_OBJECT(self, "Failed to get transport "
+ "properties: %s", err.message);
+ dbus_error_free(&err);
+ }
+ return FALSE;
}
if (!dbus_message_iter_init(reply, &arg_i)) {
@@ -824,7 +833,6 @@ static gboolean gst_avdtp_sink_transport_get_properties(GstAvdtpSink *self)
return gst_avdtp_sink_update_caps(self);
fail:
- dbus_message_unref(msg);
dbus_message_unref(reply);
return FALSE;
--
1.7.4.1
^ permalink raw reply related
* [PATCH BlueZ v0 2/4] adapter: Remove dead code from adapter_emit_device_found()
From: Syam Sidhardhan @ 2012-09-14 16:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1347639011-22924-1-git-send-email-s.syam@samsung.com>
---
src/adapter.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 1c13b2d..facfa23 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2725,7 +2725,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
struct remote_dev_info *dev)
{
struct btd_device *device;
- char peer_addr[18], local_addr[18];
+ char peer_addr[18];
const char *icon, *paddr = peer_addr;
dbus_bool_t paired = FALSE, trusted = FALSE;
dbus_int16_t rssi = dev->rssi;
@@ -2733,7 +2733,6 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
size_t uuid_count;
ba2str(&dev->bdaddr, peer_addr);
- ba2str(&adapter->bdaddr, local_addr);
device = adapter_find_device(adapter, paddr);
if (device) {
--
1.7.4.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox