Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
From: Andrei Emeltchenko @ 2010-02-16 12:16 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <508e92ca1002160358g1c6b599bo329f4f51cae18041@mail.gmail.com>

Hi Gustavo,

>> Are you using ERTM or Streaming mode? If yes, I have a guess about the
>> source of the problem. On l2cap_parse_conf_rsp we check for:
>>
>>        while (len >= L2CAP_CONF_OPT_SIZE) {
>>                len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
>>
>> But on case L2CAP_CONF_RFC olen is greater than L2CAP_CONF_OPT_SIZE we
>> can exceed the buffer size. So the right fix will be check if len >=
>> olen in that case.
>
> We use test tool which sends "Configure Response" packet with size 262
> bytes. So "req" buffer gets overwritten. But in the code nobody checks
> that "req" might be overwritten.

The packet we are sending is:

Bluetooth L2CAP Packet
    Length: 266
    CID: 0x0001
    Command: Configure Response
        Command Code: Configure Response (0x05)
        Command Identifier: 0x02
        Command Length: 262
        Source CID: 0x0040
        .... .... .... ...0 = Continuation Flag: False
        Result: Failure - unacceptable parameters (0x0001)
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
        Option: MTU
.............

We can check of course that parameter MTU is already exist. Still
there is possibility to overwrite 64 bytes of req buffer so some check
is needed.

-- Andrei

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
From: Andrei Emeltchenko @ 2010-02-16 11:58 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100216113408.GA15436@vigoh>

On Tue, Feb 16, 2010 at 1:34 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-02-16 12:36:47 +0200]:
>
>> From 0135f732cb45e5e91062aca84a61a40b172200a4 Mon Sep 17 00:00:00 2001
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> Date: Tue, 16 Feb 2010 10:52:33 +0200
>> Subject: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
>>
>> Added very simple check that req buffer has enough space to
>> fit configuration parameters. Shall be enough to reject packets
>> with configuration size more than req buffer.
>>
>> Crash trace below
>>
>> [ 6069.659393] Unable to handle kernel paging request at virtual
>> address 02000205
>> [ 6069.673034] Internal error: Oops: 805 [#1] PREEMPT
>> ...
>> [ 6069.727172] PC is at l2cap_add_conf_opt+0x70/0xf0 [l2cap]
>> [ 6069.732604] LR is at l2cap_recv_frame+0x1350/0x2e78 [l2cap]
>> ...
>> [ 6070.030303] Backtrace:
>> [ 6070.032806] [<bf1c2880>] (l2cap_add_conf_opt+0x0/0xf0 [l2cap]) from
>> [<bf1c6624>] (l2cap_recv_frame+0x1350/0x2e78 [l2cap])
>> [ 6070.043823]  r8:dc5d3100 r7:df2a91d6 r6:00000001 r5:df2a8000 r4:00000200
>> [ 6070.050659] [<bf1c52d4>] (l2cap_recv_frame+0x0/0x2e78 [l2cap]) from
>> [<bf1c8408>] (l2cap_recv_acldata+0x2bc/0x350 [l2cap])
>> [ 6070.061798] [<bf1c814c>] (l2cap_recv_acldata+0x0/0x350 [l2cap]) from
>> [<bf0037a4>] (hci_rx_task+0x244/0x478 [bluetooth])
>> [ 6070.072631]  r6:dc647700 r5:00000001 r4:df2ab740
>> [ 6070.077362] [<bf003560>] (hci_rx_task+0x0/0x478 [bluetooth]) from
>> [<c006b9fc>] (tasklet_action+0x78/0xd8)
>> [ 6070.087005] [<c006b984>] (tasklet_action+0x0/0xd8) from [<c006c160>]
>
> Are you using ERTM or Streaming mode? If yes, I have a guess about the
> source of the problem. On l2cap_parse_conf_rsp we check for:
>
>        while (len >= L2CAP_CONF_OPT_SIZE) {
>                len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
>
> But on case L2CAP_CONF_RFC olen is greater than L2CAP_CONF_OPT_SIZE we
> can exceed the buffer size. So the right fix will be check if len >=
> olen in that case.

We use test tool which sends "Configure Response" packet with size 262
bytes. So "req" buffer gets overwritten. But in the code nobody checks
that "req" might be overwritten.

-- Andrei

>
>
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>>  net/bluetooth/l2cap.c |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 400efa2..69b7280 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -2830,6 +2830,12 @@ static inline int l2cap_config_rsp(struct
>> l2cap_conn *conn, struct l2cap_cmd_hdr
>>                       int len = cmd->len - sizeof(*rsp);
>>                       char req[64];
>>
>> +                     if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
>> +                             BT_ERR("Config response is too big");
>> +                             l2cap_send_disconn_req(conn, sk);
>> +                             goto done;
>> +                     }
>> +
>>                       /* throw out any old stored conf requests */
>>                       result = L2CAP_CONF_SUCCESS;
>>                       len = l2cap_parse_conf_rsp(sk, rsp->data,
>> --
>> 1.6.0.4
>
>
> --
> Gustavo F. Padovan
> http://padovan.org
>
> ProFUSION embedded systems - http://profusion.mobi
>

^ permalink raw reply

* Re: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
From: Gustavo F. Padovan @ 2010-02-16 11:34 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <508e92ca1002160236k34c18949jc7d7187645ef4aa0@mail.gmail.com>

Hi Andrei,

* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2010-02-16 12:36:47 +0200]:

> From 0135f732cb45e5e91062aca84a61a40b172200a4 Mon Sep 17 00:00:00 2001
> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> Date: Tue, 16 Feb 2010 10:52:33 +0200
> Subject: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
> 
> Added very simple check that req buffer has enough space to
> fit configuration parameters. Shall be enough to reject packets
> with configuration size more than req buffer.
> 
> Crash trace below
> 
> [ 6069.659393] Unable to handle kernel paging request at virtual
> address 02000205
> [ 6069.673034] Internal error: Oops: 805 [#1] PREEMPT
> ...
> [ 6069.727172] PC is at l2cap_add_conf_opt+0x70/0xf0 [l2cap]
> [ 6069.732604] LR is at l2cap_recv_frame+0x1350/0x2e78 [l2cap]
> ...
> [ 6070.030303] Backtrace:
> [ 6070.032806] [<bf1c2880>] (l2cap_add_conf_opt+0x0/0xf0 [l2cap]) from
> [<bf1c6624>] (l2cap_recv_frame+0x1350/0x2e78 [l2cap])
> [ 6070.043823]  r8:dc5d3100 r7:df2a91d6 r6:00000001 r5:df2a8000 r4:00000200
> [ 6070.050659] [<bf1c52d4>] (l2cap_recv_frame+0x0/0x2e78 [l2cap]) from
> [<bf1c8408>] (l2cap_recv_acldata+0x2bc/0x350 [l2cap])
> [ 6070.061798] [<bf1c814c>] (l2cap_recv_acldata+0x0/0x350 [l2cap]) from
> [<bf0037a4>] (hci_rx_task+0x244/0x478 [bluetooth])
> [ 6070.072631]  r6:dc647700 r5:00000001 r4:df2ab740
> [ 6070.077362] [<bf003560>] (hci_rx_task+0x0/0x478 [bluetooth]) from
> [<c006b9fc>] (tasklet_action+0x78/0xd8)
> [ 6070.087005] [<c006b984>] (tasklet_action+0x0/0xd8) from [<c006c160>]

Are you using ERTM or Streaming mode? If yes, I have a guess about the
source of the problem. On l2cap_parse_conf_rsp we check for: 

	while (len >= L2CAP_CONF_OPT_SIZE) {
		len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);

But on case L2CAP_CONF_RFC olen is greater than L2CAP_CONF_OPT_SIZE we
can exceed the buffer size. So the right fix will be check if len >=
olen in that case.
	

> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
>  net/bluetooth/l2cap.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 400efa2..69b7280 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -2830,6 +2830,12 @@ static inline int l2cap_config_rsp(struct
> l2cap_conn *conn, struct l2cap_cmd_hdr
>  			int len = cmd->len - sizeof(*rsp);
>  			char req[64];
> 
> +			if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
> +				BT_ERR("Config response is too big");
> +				l2cap_send_disconn_req(conn, sk);
> +				goto done;
> +			}
> +
>  			/* throw out any old stored conf requests */
>  			result = L2CAP_CONF_SUCCESS;
>  			len = l2cap_parse_conf_rsp(sk, rsp->data,
> -- 
> 1.6.0.4


-- 
Gustavo F. Padovan
http://padovan.org

ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply

* [PATCH] Bluetooth: Fix kernel crash on BT stress tests.
From: Andrei Emeltchenko @ 2010-02-16 10:36 UTC (permalink / raw)
  To: linux-bluetooth

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

>From 0135f732cb45e5e91062aca84a61a40b172200a4 Mon Sep 17 00:00:00 2001
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Date: Tue, 16 Feb 2010 10:52:33 +0200
Subject: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.

Added very simple check that req buffer has enough space to
fit configuration parameters. Shall be enough to reject packets
with configuration size more than req buffer.

Crash trace below

[ 6069.659393] Unable to handle kernel paging request at virtual
address 02000205
[ 6069.673034] Internal error: Oops: 805 [#1] PREEMPT
...
[ 6069.727172] PC is at l2cap_add_conf_opt+0x70/0xf0 [l2cap]
[ 6069.732604] LR is at l2cap_recv_frame+0x1350/0x2e78 [l2cap]
...
[ 6070.030303] Backtrace:
[ 6070.032806] [<bf1c2880>] (l2cap_add_conf_opt+0x0/0xf0 [l2cap]) from
[<bf1c6624>] (l2cap_recv_frame+0x1350/0x2e78 [l2cap])
[ 6070.043823]  r8:dc5d3100 r7:df2a91d6 r6:00000001 r5:df2a8000 r4:00000200
[ 6070.050659] [<bf1c52d4>] (l2cap_recv_frame+0x0/0x2e78 [l2cap]) from
[<bf1c8408>] (l2cap_recv_acldata+0x2bc/0x350 [l2cap])
[ 6070.061798] [<bf1c814c>] (l2cap_recv_acldata+0x0/0x350 [l2cap]) from
[<bf0037a4>] (hci_rx_task+0x244/0x478 [bluetooth])
[ 6070.072631]  r6:dc647700 r5:00000001 r4:df2ab740
[ 6070.077362] [<bf003560>] (hci_rx_task+0x0/0x478 [bluetooth]) from
[<c006b9fc>] (tasklet_action+0x78/0xd8)
[ 6070.087005] [<c006b984>] (tasklet_action+0x0/0xd8) from [<c006c160>]

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 400efa2..69b7280 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2830,6 +2830,12 @@ static inline int l2cap_config_rsp(struct
l2cap_conn *conn, struct l2cap_cmd_hdr
 			int len = cmd->len - sizeof(*rsp);
 			char req[64];

+			if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
+				BT_ERR("Config response is too big");
+				l2cap_send_disconn_req(conn, sk);
+				goto done;
+			}
+
 			/* throw out any old stored conf requests */
 			result = L2CAP_CONF_SUCCESS;
 			len = l2cap_parse_conf_rsp(sk, rsp->data,
-- 
1.6.0.4

[-- Attachment #2: 0001-Bluetooth-Fix-kernel-crash-on-BT-stress-tests.patch --]
[-- Type: text/x-patch, Size: 2171 bytes --]

From 0135f732cb45e5e91062aca84a61a40b172200a4 Mon Sep 17 00:00:00 2001
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Date: Tue, 16 Feb 2010 10:52:33 +0200
Subject: [PATCH] Bluetooth: Fix kernel crash on BT stress tests.

Added very simple check that req buffer has enough space to
fit configuration parameters. Shall be enough to reject packets
with configuration size more than req buffer.

Crash trace below

[ 6069.659393] Unable to handle kernel paging request at virtual address 02000205
[ 6069.673034] Internal error: Oops: 805 [#1] PREEMPT
...
[ 6069.727172] PC is at l2cap_add_conf_opt+0x70/0xf0 [l2cap]
[ 6069.732604] LR is at l2cap_recv_frame+0x1350/0x2e78 [l2cap]
...
[ 6070.030303] Backtrace:
[ 6070.032806] [<bf1c2880>] (l2cap_add_conf_opt+0x0/0xf0 [l2cap]) from
[<bf1c6624>] (l2cap_recv_frame+0x1350/0x2e78 [l2cap])
[ 6070.043823]  r8:dc5d3100 r7:df2a91d6 r6:00000001 r5:df2a8000 r4:00000200
[ 6070.050659] [<bf1c52d4>] (l2cap_recv_frame+0x0/0x2e78 [l2cap]) from
[<bf1c8408>] (l2cap_recv_acldata+0x2bc/0x350 [l2cap])
[ 6070.061798] [<bf1c814c>] (l2cap_recv_acldata+0x0/0x350 [l2cap]) from
[<bf0037a4>] (hci_rx_task+0x244/0x478 [bluetooth])
[ 6070.072631]  r6:dc647700 r5:00000001 r4:df2ab740
[ 6070.077362] [<bf003560>] (hci_rx_task+0x0/0x478 [bluetooth]) from
[<c006b9fc>] (tasklet_action+0x78/0xd8)
[ 6070.087005] [<c006b984>] (tasklet_action+0x0/0xd8) from [<c006c160>]

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 400efa2..69b7280 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2830,6 +2830,12 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 			int len = cmd->len - sizeof(*rsp);
 			char req[64];
 
+			if (len > sizeof(req) - sizeof(struct l2cap_conf_req)) {
+				BT_ERR("Config response is too big");
+				l2cap_send_disconn_req(conn, sk);
+				goto done;
+			}
+
 			/* throw out any old stored conf requests */
 			result = L2CAP_CONF_SUCCESS;
 			len = l2cap_parse_conf_rsp(sk, rsp->data,
-- 
1.6.0.4


^ permalink raw reply related

* Re: [PATCH] bluez-hcidump fix SDP universal attributes
From: Iain Hibbert @ 2010-02-16  9:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1263901312.199442.295.nullmailer@galant.ukfsn.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 922 bytes --]

On Tue, 19 Jan 2010, Iain Hibbert wrote:

> On Mon, 30 Nov 2009, Iain Hibbert wrote:
>
> > hcidump shows SDP universal attribute 0x000d as IconURL but in fact it is
> > defined as "Additional Protocol Descriptor Lists". IconURL is 0x000c and I
> > don't know what Icon_10 was, perhaps an older name?
> >
> > Eg, from an SDP probe of a mouse:
> >
> >       aid 0x000d (IconURL)
> >          < < < uuid-16 0x0100 (L2CAP) uint 0x13 > < uuid-16 0x0011 (HIDP) > > >
>
> Reposting because there was no response to this, patch is attached..
>
> See "5.1.6 AdditionalProtocolDescriptorList attribute" and "5.1.14 IconURL
> Attribute" sections in the core specification for reference..
>
> (they sometimes call it AdditionalProtocolDescriptorLists and that is more
> accurate as more than one protocol descriptor list may be included)

posting this simple (and easily verified) patch for hcidump for the third
time..

regards,
iain

[-- Attachment #2: fix SDP universal attributes --]
[-- Type: TEXT/PLAIN, Size: 1570 bytes --]

--- parser/sdp.c.orig	2009-11-30 19:29:31.000000000 +0000
+++ parser/sdp.c
@@ -194,8 +194,8 @@ static sdp_attr_id_nam_lookup_table_t sd
 	{ SDP_ATTR_ID_BLUETOOTH_PROFILE_DESCRIPTOR_LIST, "BTProfileDescList"  },
 	{ SDP_ATTR_ID_DOCUMENTATION_URL,                 "DocURL"             },
 	{ SDP_ATTR_ID_CLIENT_EXECUTABLE_URL,             "ClientExeURL"       },
-	{ SDP_ATTR_ID_ICON_10,                           "Icon10"             },
 	{ SDP_ATTR_ID_ICON_URL,                          "IconURL"            },
+	{ SDP_ATTR_ID_ADDITIONAL_PROTOCOL_DESC_LISTS,    "AdditionalProtocolDescLists" },
 	{ SDP_ATTR_ID_SERVICE_NAME,                      "SrvName"            },
 	{ SDP_ATTR_ID_SERVICE_DESCRIPTION,               "SrvDesc"            },
 	{ SDP_ATTR_ID_PROVIDER_NAME,                     "ProviderName"       },
--- parser/sdp.h.orig	2009-11-30 19:29:35.000000000 +0000
+++ parser/sdp.h
@@ -129,8 +129,8 @@
 #define SDP_ATTR_ID_BLUETOOTH_PROFILE_DESCRIPTOR_LIST  0x0009
 #define SDP_ATTR_ID_DOCUMENTATION_URL                  0x000A
 #define SDP_ATTR_ID_CLIENT_EXECUTABLE_URL              0x000B
-#define SDP_ATTR_ID_ICON_10                            0x000C
-#define SDP_ATTR_ID_ICON_URL                           0x000D
+#define SDP_ATTR_ID_ICON_URL                           0x000C
+#define SDP_ATTR_ID_ADDITIONAL_PROTOCOL_DESC_LISTS     0x000D
 #define SDP_ATTR_ID_SERVICE_NAME                       0x0100
 #define SDP_ATTR_ID_SERVICE_DESCRIPTION                0x0101
 #define SDP_ATTR_ID_PROVIDER_NAME                      0x0102

^ permalink raw reply

* Re: [PATCH] audio/gateway.c: Don't reply to DBus when connection comes from AG
From: Marcel Holtmann @ 2010-02-16  5:27 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1266262461-17717-1-git-send-email-padovan@profusion.mobi>

Hi Gustavo,

>  audio/gateway.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)

patch has been applied. Thanks.

Regards

Marcel



^ permalink raw reply

* problems with local usb bluetooth devices to make 350 to 380 followed inquirys
From: Oliver Salinas @ 2010-02-15 21:37 UTC (permalink / raw)
  To: linux-bluetooth

hello my name is Oliver and I live in chile, I'm doing my university
project and I am making a software that sends personal messages to
groups of users enrolled in a student aid system, for this I am using
Bluez on linux and therefore, bluetooth devices are constantly working
within a loop while (1), the problem is that every 350 to 380
inquirys, bluetooth usb devices stop working

my question if they know that would cause this problem? and can be resolved?

any answer is very WELCOME


Oliver Salinas Aranguiz
Est. Ingenieria Civil Informatica
Universidad del Bio Bio

^ permalink raw reply

* [PATCH] Bluetooth: Fix out of scope variable access in hci_sock_cmsg()
From: Johann Felix Soden @ 2010-02-15 21:23 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

From: Johann Felix Soden <johfel@users.sourceforge.net>

The pointer data can point to the variable ctv.
Access to data happens when ctv is already out of scope.

Signed-off-by: Johann Felix Soden <johfel@users.sourceforge.net>
---
 net/bluetooth/hci_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 688cfeb..b0e6108 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -338,8 +338,8 @@ static inline void hci_sock_cmsg(struct sock *sk, struct msghdr *msg, struct sk_
 		data = &tv;
 		len = sizeof(tv);
 #ifdef CONFIG_COMPAT
+		struct compat_timeval ctv;
 		if (msg->msg_flags & MSG_CMSG_COMPAT) {
-			struct compat_timeval ctv;
 			ctv.tv_sec = tv.tv_sec;
 			ctv.tv_usec = tv.tv_usec;
 			data = &ctv;
-- 
1.6.6.2

^ permalink raw reply related

* Re: [PATCH] Bluetooth: Allow SCO/eSCO packet type selection for outgoing SCO connections.
From: Nick Pelly @ 2010-02-15 21:15 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Nick Pelly
In-Reply-To: <35c90d961002111159h6727bc2cw28b55ce6e919fb4f@mail.gmail.com>

On Thu, Feb 11, 2010 at 11:59 AM, Nick Pelly <npelly@google.com> wrote:
> On Thu, Feb 11, 2010 at 11:54 AM, Nick Pelly <npelly@google.com> wrote:
>> __u16 sco_pkt_type is introduced to struct sockaddr_sco. It allows bitwise
>> selection of SCO/eSCO packet types as per the Packet_Type parameter in
>> HCI Setup Synchrnous Connection Command.
>>
>> 0x0001 HV1 may be used.
>> 0x0002 HV2 may be used.
>> 0x0004 HV3 may be used.
>> 0x0008 EV3 may be used.
>> 0x0010 EV4 may be used.
>> 0x0020 EV5 may be used.
>> 0x0040 2-EV3 may not be used.
>> 0x0080 3-EV3 may not be used.
>> 0x0100 2-EV5 may not be used.
>> 0x0200 3-EV5 may not be used.
>>
>> We've followed the Bluetooth SIG convention of reversing the logic on the
>> EDR bits.
>>
>> Packet type selection is just a request made to the Bluetooth chipset, and
>> it is up to the link manager on the chipset to negiotiate and decide on the
>> actual packet types used. Furthermore, when a SCO/eSCO connection is eventually
>> made there is no way for the host stack to determine which packet type was used
>> (however it is possible to get the link type of SCO or eSCO).
>>
>> sco_pkt_type is ignored for incoming SCO connections. It is possible
>> to add this in the future as a parameter to the Accept Synchronous Connection
>> Command, however its a little trickier because the kernel does not
>> currently preserve sockaddr_sco data between userspace calls to accept().
>>
>> Typically userspace just wants to select eSCO or SCO packet types, which can be
>> done with the new constants ALL_SCO_PKTS or ALL_ESCO_PKTS. If sco_pkt_type is
>> zero, or userspace uses the old sockaddr_sco structure size, then ALL_ESCO_PKTS
>> will be selected as default.
>>
>> This patch is motivated by broken Bluetooth carkits such as the Motorolo
>> HF850 (it claims to support eSCO, but will actually reject eSCO connections
>> after 5 seconds) and the 2007/2008 Infiniti G35/37 (fails to route audio
>> if a 2-EV5 packet type is negiotiated). With this patch userspace can maintain
>> a blacklist of packet types to workaround broken remote devices such as these.
>>
>> Signed-off-by: Nick Pelly <npelly@google.com>
>> ---
>>  include/net/bluetooth/hci.h      |    7 +++-
>>  include/net/bluetooth/hci_core.h |    7 +++-
>>  include/net/bluetooth/sco.h      |    4 ++-
>>  net/bluetooth/hci_conn.c         |   25 +++++++++------
>>  net/bluetooth/hci_event.c        |    6 ++-
>>  net/bluetooth/l2cap.c            |    2 +-
>>  net/bluetooth/sco.c              |   61 +++++++++++++++++++++++++------------
>>  7 files changed, 74 insertions(+), 38 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 4f7795f..72efc94 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -139,8 +139,11 @@ enum {
>>  #define ESCO_2EV5      0x0100
>>  #define ESCO_3EV5      0x0200
>>
>> -#define SCO_ESCO_MASK  (ESCO_HV1 | ESCO_HV2 | ESCO_HV3)
>> -#define EDR_ESCO_MASK  (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
>> +#define SCO_ESCO_MASK  (ESCO_HV1 | ESCO_HV2 | ESCO_HV3)
>> +#define EDR_ESCO_MASK  (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
>> +
>> +#define ALL_SCO_PKTS   (SCO_ESCO_MASK | EDR_ESCO_MASK)
>> +#define ALL_ESCO_PKTS  (SCO_ESCO_MASK | ESCO_EV3 | ESCO_EV4 | ESCO_EV5)
>>
>>  /* ACL flags */
>>  #define ACL_START              0x00
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index b6d36cb..cbcc5b1 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -326,12 +326,15 @@ void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
>>  void hci_add_sco(struct hci_conn *conn, __u16 handle);
>>  void hci_setup_sync(struct hci_conn *conn, __u16 handle);
>>
>> -struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
>> +struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type,
>> +                                       __u16 pkt_type, bdaddr_t *dst);
>>  int hci_conn_del(struct hci_conn *conn);
>>  void hci_conn_hash_flush(struct hci_dev *hdev);
>>  void hci_conn_check_pending(struct hci_dev *hdev);
>>
>> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type);
>> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
>> +                                       __u16 pkt_type, bdaddr_t *dst,
>> +                                       __u8 sec_level, __u8 auth_type);
>>  int hci_conn_check_link_mode(struct hci_conn *conn);
>>  int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type);
>>  int hci_conn_change_link_key(struct hci_conn *conn);
>> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
>> index e28a2a7..924338a 100644
>> --- a/include/net/bluetooth/sco.h
>> +++ b/include/net/bluetooth/sco.h
>> @@ -37,6 +37,7 @@
>>  struct sockaddr_sco {
>>        sa_family_t     sco_family;
>>        bdaddr_t        sco_bdaddr;
>> +       __u16           sco_pkt_type;
>>  };
>>
>>  /* SCO socket options */
>> @@ -72,7 +73,8 @@ struct sco_conn {
>>
>>  struct sco_pinfo {
>>        struct bt_sock  bt;
>> -       __u32           flags;
>> +       __u16           pkt_type;
>> +
>>        struct sco_conn *conn;
>>  };
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index fa8b412..8ced057 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -196,7 +196,8 @@ static void hci_conn_idle(unsigned long arg)
>>        hci_conn_enter_sniff_mode(conn);
>>  }
>>
>> -struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
>> +struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type,
>> +                                       __u16 pkt_type, bdaddr_t *dst)
>>  {
>>        struct hci_conn *conn;
>>
>> @@ -221,14 +222,16 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
>>                conn->pkt_type = hdev->pkt_type & ACL_PTYPE_MASK;
>>                break;
>>        case SCO_LINK:
>> +               if (!pkt_type)
>> +                       pkt_type = ALL_SCO_PKTS;
>> +       case ESCO_LINK:
>> +               if (!pkt_type)
>> +                       pkt_type = ALL_ESCO_PKTS;
>>                if (lmp_esco_capable(hdev))
>> -                       conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
>> -                                       (hdev->esco_type & EDR_ESCO_MASK);
>> +                       conn->pkt_type = pkt_type & hdev->esco_type;
>>                else
>> -                       conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
>> -               break;
>> -       case ESCO_LINK:
>> -               conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
>> +                       conn->pkt_type = (pkt_type << 5) & hdev->pkt_type &
>> +                                       SCO_PTYPE_MASK;
>>                break;
>>        }
>>
>> @@ -340,7 +343,9 @@ EXPORT_SYMBOL(hci_get_route);
>>
>>  /* Create SCO or ACL connection.
>>  * Device _must_ be locked */
>> -struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8 sec_level, __u8 auth_type)
>> +struct hci_conn *hci_connect(struct hci_dev *hdev, int type,
>> +                                       __u16 pkt_type, bdaddr_t *dst,
>> +                                       __u8 sec_level, __u8 auth_type)
>>  {
>>        struct hci_conn *acl;
>>        struct hci_conn *sco;
>> @@ -348,7 +353,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>>        BT_DBG("%s dst %s", hdev->name, batostr(dst));
>>
>>        if (!(acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst))) {
>> -               if (!(acl = hci_conn_add(hdev, ACL_LINK, dst)))
>> +               if (!(acl = hci_conn_add(hdev, ACL_LINK, 0, dst)))
>>                        return NULL;
>>        }
>>
>> @@ -364,7 +369,7 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>>                return acl;
>>
>>        if (!(sco = hci_conn_hash_lookup_ba(hdev, type, dst))) {
>> -               if (!(sco = hci_conn_add(hdev, type, dst))) {
>> +               if (!(sco = hci_conn_add(hdev, type, pkt_type, dst))) {
>>                        hci_conn_put(acl);
>>                        return NULL;
>>                }
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index 10edd1a..5343e0f 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -579,7 +579,7 @@ static inline void hci_cs_create_conn(struct hci_dev *hdev, __u8 status)
>>                }
>>        } else {
>>                if (!conn) {
>> -                       conn = hci_conn_add(hdev, ACL_LINK, &cp->bdaddr);
>> +                       conn = hci_conn_add(hdev, ACL_LINK, 0, &cp->bdaddr);
>>                        if (conn) {
>>                                conn->out = 1;
>>                                conn->link_mode |= HCI_LM_MASTER;
>> @@ -964,7 +964,9 @@ static inline void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *sk
>>
>>                conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
>>                if (!conn) {
>> -                       if (!(conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr))) {
>> +                       /* pkt_type not yet used for incoming connections */
>> +                       if (!(conn = hci_conn_add(hdev, ev->link_type, 0,
>> +                                                       &ev->bdaddr))) {
>>                                BT_ERR("No memmory for new connection");
>>                                hci_dev_unlock(hdev);
>>                                return;
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index d056886..b342f06 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -955,7 +955,7 @@ static int l2cap_do_connect(struct sock *sk)
>>                }
>>        }
>>
>> -       hcon = hci_connect(hdev, ACL_LINK, dst,
>> +       hcon = hci_connect(hdev, ACL_LINK, 0, dst,
>>                                        l2cap_pi(sk)->sec_level, auth_type);
>>        if (!hcon)
>>                goto done;
>> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
>> index 77f4153..4976ad5 100644
>> --- a/net/bluetooth/sco.c
>> +++ b/net/bluetooth/sco.c
>> @@ -176,6 +176,7 @@ static int sco_connect(struct sock *sk)
>>  {
>>        bdaddr_t *src = &bt_sk(sk)->src;
>>        bdaddr_t *dst = &bt_sk(sk)->dst;
>> +       __u16 pkt_type = sco_pi(sk)->pkt_type;
>>        struct sco_conn *conn;
>>        struct hci_conn *hcon;
>>        struct hci_dev  *hdev;
>> @@ -192,10 +193,14 @@ static int sco_connect(struct sock *sk)
>>
>>        if (lmp_esco_capable(hdev) && !disable_esco)
>>                type = ESCO_LINK;
>> -       else
>> +       else {
>>                type = SCO_LINK;
>> +               pkt_type &= SCO_ESCO_MASK;
>> +               pkt_type |= EDR_ESCO_MASK;
>> +       }
>>
>> -       hcon = hci_connect(hdev, type, dst, BT_SECURITY_LOW, HCI_AT_NO_BONDING);
>> +       hcon = hci_connect(hdev, type, pkt_type, dst,
>> +                                       BT_SECURITY_LOW, HCI_AT_NO_BONDING);
>>        if (!hcon)
>>                goto done;
>>
>> @@ -451,18 +456,22 @@ static int sco_sock_create(struct net *net, struct socket *sock, int protocol)
>>        return 0;
>>  }
>>
>> -static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
>> +static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
>>  {
>> -       struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
>> +       struct sockaddr_sco sa;
>>        struct sock *sk = sock->sk;
>> -       bdaddr_t *src = &sa->sco_bdaddr;
>> -       int err = 0;
>> +       bdaddr_t *src = &sa.sco_bdaddr;
>> +       int len, err = 0;
>>
>> -       BT_DBG("sk %p %s", sk, batostr(&sa->sco_bdaddr));
>> +       BT_DBG("sk %p %s", sk, batostr(&sa.sco_bdaddr));
>>
>>        if (!addr || addr->sa_family != AF_BLUETOOTH)
>>                return -EINVAL;
>>
>> +       memset(&sa, 0, sizeof(sa));
>> +       len = min_t(unsigned int, sizeof(sa), alen);
>> +       memcpy(&sa, addr, len);
>> +
>>        lock_sock(sk);
>>
>>        if (sk->sk_state != BT_OPEN) {
>> @@ -476,7 +485,8 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
>>                err = -EADDRINUSE;
>>        } else {
>>                /* Save source address */
>> -               bacpy(&bt_sk(sk)->src, &sa->sco_bdaddr);
>> +               bacpy(&bt_sk(sk)->src, &sa.sco_bdaddr);
>> +               sco_pi(sk)->pkt_type = sa.sco_pkt_type;
>>                sk->sk_state = BT_BOUND;
>>        }
>>
>> @@ -489,26 +499,34 @@ done:
>>
>>  static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen, int flags)
>>  {
>> -       struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
>>        struct sock *sk = sock->sk;
>> -       int err = 0;
>> -
>> +       struct sockaddr_sco sa;
>> +       int len, err = 0;
>>
>>        BT_DBG("sk %p", sk);
>>
>> -       if (addr->sa_family != AF_BLUETOOTH || alen < sizeof(struct sockaddr_sco))
>> +       if (!addr || addr->sa_family != AF_BLUETOOTH)
>>                return -EINVAL;
>>
>> -       if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND)
>> -               return -EBADFD;
>> -
>> -       if (sk->sk_type != SOCK_SEQPACKET)
>> -               return -EINVAL;
>> +       memset(&sa, 0, sizeof(sa));
>> +       len = min_t(unsigned int, sizeof(sa), alen);
>> +       memcpy(&sa, addr, len);
>>
>>        lock_sock(sk);
>>
>> +       if (sk->sk_type != SOCK_SEQPACKET) {
>> +               err = -EINVAL;
>> +               goto done;
>> +       }
>> +
>> +       if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) {
>> +               err = -EBADFD;
>> +               goto done;
>> +       }
>> +
>>        /* Set destination address and psm */
>> -       bacpy(&bt_sk(sk)->dst, &sa->sco_bdaddr);
>> +       bacpy(&bt_sk(sk)->dst, &sa.sco_bdaddr);
>> +       sco_pi(sk)->pkt_type = sa.sco_pkt_type;
>>
>>        if ((err = sco_connect(sk)))
>>                goto done;
>> @@ -610,10 +628,13 @@ static int sco_sock_getname(struct socket *sock, struct sockaddr *addr, int *len
>>        addr->sa_family = AF_BLUETOOTH;
>>        *len = sizeof(struct sockaddr_sco);
>>
>> -       if (peer)
>> +       if (peer) {
>>                bacpy(&sa->sco_bdaddr, &bt_sk(sk)->dst);
>> -       else
>> +               sa->sco_pkt_type = sco_pi(sk)->pkt_type;
>> +       } else {
>>                bacpy(&sa->sco_bdaddr, &bt_sk(sk)->src);
>> +               sa->sco_pkt_type = sco_pi(sk)->pkt_type;
>> +       }
>>
>>        return 0;
>>  }
>> --
>> 1.6.5.3
>>
>>
>
> BTW i'm happy to switch the logic on the EDR bits, such that 1 is
> always 'allow this packet type'. I stuck with the Bluetooth SIG
> convention in this patch because the logic actually came out cleanly.
>
> Either way, its confusing for userspace, and we should document it
> carefully in the patch to userspace headers.

Ping.

As a first step, can we get consensus on the userspace API:

--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -139,8 +139,11 @@ enum {
+#define ALL_SCO_PKTS   (SCO_ESCO_MASK | EDR_ESCO_MASK)
+#define ALL_ESCO_PKTS  (SCO_ESCO_MASK | ESCO_EV3 | ESCO_EV4 | ESCO_EV5)

--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -37,6 +37,7 @@
struct sockaddr_sco {
       sa_family_t     sco_family;
       bdaddr_t        sco_bdaddr;
+       __u16           sco_pkt_type;
};

This will at least unblock my work.

Nick

^ permalink raw reply

* [PATCH] audio/gateway.c: Don't reply to DBus when connection comes from AG
From: Gustavo F. Padovan @ 2010-02-15 19:34 UTC (permalink / raw)
  To: linux-bluetooth

---
 audio/gateway.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index 46fbb63..22e5da2 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -224,7 +224,7 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
 	struct audio_device *dev = user_data;
 	struct gateway *gw = dev->gateway;
 	DBusMessage *reply;
-	int sk;
+	int sk, ret;
 
 	if (err) {
 		error("connect(): %s", err->message);
@@ -242,11 +242,16 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
 
 	gw->rfcomm = g_io_channel_ref(chan);
 
-	if (!agent_sendfd(gw->agent, sk, newconnection_reply, dev))
+	ret = agent_sendfd(gw->agent, sk, newconnection_reply, dev);
+
+	if (!gw->msg)
+		return;
+
+	if (ret)
+		reply = dbus_message_new_method_return(gw->msg);
+	else
 		reply = g_dbus_create_error(gw->msg, ERROR_INTERFACE ".Failed",
 					"Can not pass file descriptor");
-	else
-		reply = dbus_message_new_method_return(gw->msg);
 
 	g_dbus_send_message(dev->conn, reply);
 
@@ -258,7 +263,6 @@ fail:
 						ERROR_INTERFACE ".Failed",
 						"Connection attempt failed");
 
-
 	change_state(dev, GATEWAY_STATE_DISCONNECTED);
 }
 
-- 
1.6.4.4


^ permalink raw reply related

* Re: [PATCH] Fix DSO linking
From: Marcel Holtmann @ 2010-02-15 19:24 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1266230637.2310.127.camel@localhost.localdomain>

Hi Bastien,

> We need to link explicitly against libm, to avoid the error mentioned in
> the patch.

patch has been applied. Thanks.

Regards

Marcel



^ permalink raw reply

* Re: API broken in bluez 4.61
From: Marcel Holtmann @ 2010-02-15 19:23 UTC (permalink / raw)
  To: Baptiste Mille-Mathias; +Cc: linux-bluetooth
In-Reply-To: <920101ab1002150454t31c94b2bh4b1825b4d8ed0db9@mail.gmail.com>

Hi Baptiste,

> While packaging latest release of bluez, I seen the API compatibility
> was broken.
> hci_read_inquiry_transmit_power_level was removed and thus would have
> required to bump the soname.

I fixed it already and will make another release soon.

Regards

Marcel



^ permalink raw reply

* thinkpad laser bluetooth mouse
From: pavol @ 2010-02-15 16:40 UTC (permalink / raw)
  To: linux-bluetooth

Hallo, I can connect my mouse by command 'hidd --search' (mouse is in pairing mode), but autoconnect not works

/etc/conf.d/bluetooth file
# Bluetooth configuraton file

# Bluetooth services (allowed values are "true" and "false")

# Run the bluetoothd daemon (default: true)
#DAEMON_ENABLE="false"

# Run hid2hci (default: false)
HID2HCI_ENABLE="true"

# Run the sdp daemon (default: false)
# If this is disabled, hcid's internal sdp daemon will be used
#SDPD_ENABLE="true"

# Run the bluetooth HID daemon (default: false)
HIDD_ENABLE="true"

# Activate rfcomm ports (default: false)
#RFCOMM_ENABLE="true"

# Run bluetooth dial-up networking daemon (default: false)
#DUND_ENABLE="true"

# Run bluetooth PAN daemon (default: false)
#PAND_ENABLE="true"

# rfcomm configuration file (default: /etc/bluetooth/rfcomm.conf)
#RFCOMM_CONFIG="/etc/bluetooth/rfcomm.conf"

# Options for hidd, dund and pand (default: none)
#HIDD_OPTIONS="--server --connect"
#DUND_OPTIONS=""
#PAND_OPTIONS=""


-- 
 <pavol@klacansky.com>

^ permalink raw reply

* API broken in bluez 4.61
From: Baptiste Mille-Mathias @ 2010-02-15 12:54 UTC (permalink / raw)
  To: linux-bluetooth

Hello,

While packaging latest release of bluez, I seen the API compatibility
was broken.
hci_read_inquiry_transmit_power_level was removed and thus would have
required to bump the soname.

Thanks for fixing that.

Regards
-- 
Baptiste Mille-Mathias
Les gens heureux ne sont pas pressés

^ permalink raw reply

* Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Ed Tomlinson @ 2010-02-15 12:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Michael Poole, Jiri Kosina, linux-input, Marcel Holtmann,
	linux-bluetooth, linux-kernel
In-Reply-To: <20100215071850.GB9135@core.coreip.homeip.net>

On Monday 15 February 2010 02:18:50 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 06:58:26PM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> > > Ed Tomlinson writes:
> > > 
> > > > Hi,
> > > >
> > > > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > > > changed the accelleration logic to work better with xy scrolling.
> > > 
> > > Hi Ed,
> > > 
> > > Your other patch to call input_unregister_device() looks good -- thanks!
> > 
> > Thanks.  One question about it though.  Do we have to check if msc is null before
> > the unregister?
> 
> If it is NULL whan remove() is running there are much bigger problems
> with the driver/HID subsystem.
> 
> > 
> > > I've never used a horizontal scroll wheel -- what are the common uses
> > > for it?  Why should the acceleration be separate for the two directions
> > > rather than using the same factor?  Why does the kernel need to emulate
> > > this rather than having user-space implement the emulation?
> > 
> > Its usefull for scrolling left and right while browsing.  If you use kde it can
> > be used to scroll between applications on the taskbar.   Here most applications
> > with a horizontial scrollbar work as expected.
> > 
> > I first tried with a single acceleration value for both axies.  It leads to confusing
> > things happening.   For example.  I quickly scroll down, then nudge the
> > window to the left.  This works as expect with two values.  With one the
> > nudge is accelerated and moves too far.
> > 
> > My personal goal is to have the basic, apple defined, gestures working
> > from kernel space.  This way the device works as expected without needing
> > to fiddle with X or other managers (think wayland and/or chromeOS).
> 
> I am not sure if this is the desired approach. The current idea is to
> export useable but minimally processed events to userspace and let them
> be turned into gestures there (by evdev, synaptics driver and so forth).

I would go along with this if the code to add the emulation in kernel was large or complex.  
Its not.  About 30 more lines should add the pgup/pgdn code.  

Thanks
Ed

^ permalink raw reply

* Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps
From: Ed Tomlinson @ 2010-02-15 12:44 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Michael Poole, linux-input, Marcel Holtmann,
	linux-bluetooth, linux-kernel
In-Reply-To: <20100215071145.GA9135@core.coreip.homeip.net>

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > > 
> > > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > > Input Report that isn't published in the input Report descriptor that
> > > > > > contains touch data (and usually overrides the normal motion and click
> > > > > > Report).
> > > > > > 
> > > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > > the reported events.
> > > > > > 
> > > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > > when it is not associated with the host's HIDP stack) are at
> > > > > > http://github.com/entrope/linux-magicmouse .
> > > > > 
> > > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > > 
> > > 
> > > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > > exited X and got the opps.  Note my X does not hotplug the magic
> > > > mouse.  I've also included a trace of the udev events that generated
> > > > the log below (if there was a remove after X stopped it would not be
> > > > included).  To my eyes it looks like we leak an input device (there is
> > > > not a remove event for input8).
> > > >
> > > 
> > > Indeed, we seem to be missing call to input_unregister_device() in
> > > magicmouse_remove().
> > 
> > How does this look?  With this udevadm shows input8 being removed and
> > there is no more opps.
> 
> Almost... you need to do hid_hw_stop() first and only then unregister
> input device, Otherwise if you unload the module while moving the mouse
> it is likely to still oops.

How about this?  It applies on top of yesterdays patch.

Ed

---
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 7d252d2..46fdeee 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -430,8 +430,8 @@ static void magicmouse_remove(struct hid_device *hdev)
 {
 	struct magicmouse_sc *msc;
 	msc = hid_get_drvdata(hdev);
-	input_unregister_device(msc->input);
 	hid_hw_stop(hdev);
+	input_unregister_device(msc->input);
 	kfree(msc);
 }
 

^ permalink raw reply related

* Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps
From: Ed Tomlinson @ 2010-02-15 12:42 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Michael Poole, linux-input, Marcel Holtmann,
	linux-bluetooth, linux-kernel
In-Reply-To: <20100215071145.GA9135@core.coreip.homeip.net>

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > > 
> > > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > > Input Report that isn't published in the input Report descriptor that
> > > > > > contains touch data (and usually overrides the normal motion and click
> > > > > > Report).
> > > > > > 
> > > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > > the reported events.
> > > > > > 
> > > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > > when it is not associated with the host's HIDP stack) are at
> > > > > > http://github.com/entrope/linux-magicmouse .
> > > > > 
> > > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > > 
> > > 
> > > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > > exited X and got the opps.  Note my X does not hotplug the magic
> > > > mouse.  I've also included a trace of the udev events that generated
> > > > the log below (if there was a remove after X stopped it would not be
> > > > included).  To my eyes it looks like we leak an input device (there is
> > > > not a remove event for input8).
> > > >
> > > 
> > > Indeed, we seem to be missing call to input_unregister_device() in
> > > magicmouse_remove().
> > 
> > How does this look?  With this udevadm shows input8 being removed and
> > there is no more opps.
> 
> Almost... you need to do hid_hw_stop() first and only then unregister
> input device, Otherwise if you unload the module while moving the mouse
> it is likely to still oops.

^ permalink raw reply

* [PATCH] Fix DSO linking
From: Bastien Nocera @ 2010-02-15 10:43 UTC (permalink / raw)
  To: BlueZ development

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

We need to link explicitly against libm, to avoid the error mentioned in
the patch.

Cheers

[-- Attachment #2: 0001-Fix-DSO-linking.patch --]
[-- Type: text/x-patch, Size: 1000 bytes --]

>From 6447181296e56e20c6b504978477d24cc55655e9 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 15 Feb 2010 10:41:40 +0000
Subject: [PATCH] Fix DSO linking

Explicitely link against libm, to avoid linking problems:
CCLD   sbc/sbctester
/usr/bin/ld: sbc/sbctester.o: undefined reference to symbol 'sqrt@@GLIBC_2.2.5'
/usr/bin/ld: note: 'sqrt@@GLIBC_2.2.5' is defined in DSO /lib64/libm.so.6 so try adding it to the linker command line
/lib64/libm.so.6: could not read symbols: Invalid operation

https://bugzilla.redhat.com/show_bug.cgi?id=564799
---
 Makefile.am |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 584b921..fc7b2c0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,7 +81,7 @@ sbc_sbcenc_LDADD = sbc/libsbc.la
 if SNDFILE
 noinst_PROGRAMS += sbc/sbctester
 
-sbc_sbctester_LDADD = @SNDFILE_LIBS@
+sbc_sbctester_LDADD = @SNDFILE_LIBS@ -lm
 sbc_sbctest_CFLAGS = @SNDFILE_CFLAGS@
 endif
 endif
-- 
1.6.6


^ permalink raw reply related

* Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Dmitry Torokhov @ 2010-02-15  7:18 UTC (permalink / raw)
  To: Ed Tomlinson
  Cc: Michael Poole, Jiri Kosina, linux-input, Marcel Holtmann,
	linux-bluetooth, linux-kernel
In-Reply-To: <201002141858.26917.edt@aei.ca>

On Sun, Feb 14, 2010 at 06:58:26PM -0500, Ed Tomlinson wrote:
> On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> > Ed Tomlinson writes:
> > 
> > > Hi,
> > >
> > > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > > changed the accelleration logic to work better with xy scrolling.
> > 
> > Hi Ed,
> > 
> > Your other patch to call input_unregister_device() looks good -- thanks!
> 
> Thanks.  One question about it though.  Do we have to check if msc is null before
> the unregister?

If it is NULL whan remove() is running there are much bigger problems
with the driver/HID subsystem.

> 
> > I've never used a horizontal scroll wheel -- what are the common uses
> > for it?  Why should the acceleration be separate for the two directions
> > rather than using the same factor?  Why does the kernel need to emulate
> > this rather than having user-space implement the emulation?
> 
> Its usefull for scrolling left and right while browsing.  If you use kde it can
> be used to scroll between applications on the taskbar.   Here most applications
> with a horizontial scrollbar work as expected.
> 
> I first tried with a single acceleration value for both axies.  It leads to confusing
> things happening.   For example.  I quickly scroll down, then nudge the
> window to the left.  This works as expect with two values.  With one the
> nudge is accelerated and moves too far.
> 
> My personal goal is to have the basic, apple defined, gestures working
> from kernel space.  This way the device works as expected without needing
> to fiddle with X or other managers (think wayland and/or chromeOS).

I am not sure if this is the desired approach. The current idea is to
export useable but minimally processed events to userspace and let them
be turned into gestures there (by evdev, synaptics driver and so forth).

Thanks.
 
-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps
From: Dmitry Torokhov @ 2010-02-15  7:11 UTC (permalink / raw)
  To: Ed Tomlinson
  Cc: Jiri Kosina, Michael Poole, linux-input, Marcel Holtmann,
	linux-bluetooth, linux-kernel
In-Reply-To: <201002140922.42014.edt@aei.ca>

On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > 
> > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > Input Report that isn't published in the input Report descriptor that
> > > > > contains touch data (and usually overrides the normal motion and click
> > > > > Report).
> > > > > 
> > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > the reported events.
> > > > > 
> > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > when it is not associated with the host's HIDP stack) are at
> > > > > http://github.com/entrope/linux-magicmouse .
> > > > 
> > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > 
> > 
> > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > exited X and got the opps.  Note my X does not hotplug the magic
> > > mouse.  I've also included a trace of the udev events that generated
> > > the log below (if there was a remove after X stopped it would not be
> > > included).  To my eyes it looks like we leak an input device (there is
> > > not a remove event for input8).
> > >
> > 
> > Indeed, we seem to be missing call to input_unregister_device() in
> > magicmouse_remove().
> 
> How does this look?  With this udevadm shows input8 being removed and
> there is no more opps.

Almost... you need to do hid_hw_stop() first and only then unregister
input device, Otherwise if you unload the module while moving the mouse
it is likely to still oops.

Thanks.

-- 
Dmitry

^ permalink raw reply

* SolidTek ASK-3962 keyboard works but mouse doesn't [patch]
From: Adam Bliss @ 2010-02-15  6:29 UTC (permalink / raw)
  To: linux-bluetooth

Hi there,

I bought a SolidTek ASK-3962 Keyboard/Mouse for use with my Home
Theatre PC.  The keyboard connects and works great with bluez 4.60 --
hooray!

However, the mouse does not work.  I traced this down to the subclass
the device reports when it connects.  It reports 0x40, while bluez
expects it to include 0x80 to be treated as a mouse.  I found this
patch to work great:


--- bluez-4.60/input/device.c   2010-01-09 12:52:17.000000000 -0800
+++ bluez-4.60-new/input/device.c       2010-02-14 22:16:22.000000000 -0800
@@ -647,6 +647,12 @@
        if (idev->name)
                strncpy(req->name, idev->name, sizeof(req->name) - 1);

+       if (req->vendor == 0x0dc6 && req->product == 0x3752) {
+               // The SolidTek ask-3962 has a keyboard and a mouse, but
+               // incorrectly reports its subclass.
+               req->subclass |= 0x80;
+       }
+
        /* Encryption is mandatory for keyboards */
        if (req->subclass & 0x40) {
                err = bt_acl_encrypt(&idev->src, &idev->dst,
encrypt_completed, req);


(You can also do this with the old hidd program by passing "-b 192" to
force the subclass.)

I don't know if one-off hacks in device.c are the right way to solve
this problem -- I was "inspired" by the one-off patch in there already
for 054c:0268.  Let me know if there's a more appropriate way to fix
this, and I'll be happy to send in another patch.

Cheers,
--Adam Bliss

^ permalink raw reply

* Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Ed Tomlinson @ 2010-02-15  0:18 UTC (permalink / raw)
  To: Michael Poole
  Cc: Jiri Kosina, linux-input, Marcel Holtmann, linux-bluetooth,
	linux-kernel
In-Reply-To: <87zl3bbfdo.fsf@troilus.org>

On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> Ed Tomlinson writes:
> 
> > Hi,
> >
> > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > changed the accelleration logic to work better with xy scrolling.
> 
> Hi Ed,
> 
> Your other patch to call input_unregister_device() looks good -- thanks!
> 
> I've never used a horizontal scroll wheel -- what are the common uses
> for it?  Why should the acceleration be separate for the two directions
> rather than using the same factor?  Why does the kernel need to emulate
> this rather than having user-space implement the emulation?

With style fixups.

Ed

--
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index b20484a..7d252d2 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -59,7 +59,8 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @delta_time: 18-bit difference between the two most recent touch
  *     reports from the mouse.
  * @ntouches: Number of touches in most recent touch report.
- * @scroll_accel: Number of consecutive scroll motions.
+ * @scroll_accel_y: Number of consecutive scroll motions.
+ * @scroll_accel_x: Number of consecutive scroll motions.
  * @scroll_jiffies: Time of last scroll motion.
  * @touches: Most recent data for a touch, indexed by tracking ID.
  * @tracking_ids: Mapping of current touch input data to @touches.
@@ -71,11 +72,13 @@ struct magicmouse_sc {
 	int last_timestamp;
 	int delta_time;
 	int ntouches;
-	int scroll_accel;
+	int scroll_accel_y;
+	int scroll_accel_x;
 	unsigned long scroll_jiffies;
 
 	struct {
 		short x;
+		short scroll_x;
 		short y;
 		short scroll_y;
 		u8 size;
@@ -139,8 +142,10 @@ static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
 	input_report_key(msc->input, BTN_LEFT, state & 1);
 	input_report_key(msc->input, BTN_RIGHT, state & 2);
 
-	if (state != last_state)
-		msc->scroll_accel = 0;
+	if (state != last_state) {
+		msc->scroll_accel_y = 0;
+		msc->scroll_accel_x = 0;
+	}
 }
 
 static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
@@ -159,34 +164,46 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	msc->touches[id].size = misc & 63;
 
 	/* If requested, emulate a scroll wheel by detecting small
-	 * vertical touch motions along the middle of the mouse.
+	 * touch motions on the mouse.
 	 */
 	if (emulate_scroll_wheel &&
-	    middle_button_start < x && x < middle_button_stop) {
+	    msc->ntouches == 1) {
 		static const int accel_profile[] = {
-			256, 228, 192, 160, 128, 96, 64, 32,
+			192, 160, 128, 96, 64, 48, 32, 24,
 		};
 		unsigned long now = jiffies;
-		int step = msc->touches[id].scroll_y - y;
+		int stepx, stepy;
 
 		/* Reset acceleration after half a second. */
-		if (time_after(now, msc->scroll_jiffies + HZ / 2))
-			msc->scroll_accel = 0;
+		if (time_after(now, msc->scroll_jiffies + HZ / 2)) {
+			msc->scroll_accel_y = 0;
+			msc->scroll_accel_x = 0;
+		}
 
-		/* Calculate and apply the scroll motion. */
 		switch (tdata[7] & TOUCH_STATE_MASK) {
 		case TOUCH_STATE_START:
 			msc->touches[id].scroll_y = y;
-			msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
-						ARRAY_SIZE(accel_profile) - 1);
+			msc->touches[id].scroll_x = x;
 			break;
 		case TOUCH_STATE_DRAG:
-			step = step / accel_profile[msc->scroll_accel];
-			if (step != 0) {
+			/* Calculate the scroll motion. */
+			stepy = (msc->touches[id].scroll_y - y) / accel_profile[msc->scroll_accel_y];
+			stepx = (msc->touches[id].scroll_x - x) / accel_profile[msc->scroll_accel_x];
+
+			/* Tell input about any motion. */
+			if (stepy != 0) {
 				msc->touches[id].scroll_y = y;
-				msc->scroll_jiffies = now;
-				input_report_rel(input, REL_WHEEL, step);
+				input_report_rel(input, REL_WHEEL, stepy);
+				msc->scroll_accel_y = min_t(int, msc->scroll_accel_y + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
 			}
+			if (stepx != 0) {
+				msc->touches[id].scroll_x = x;
+				input_report_rel(input, REL_HWHEEL, stepx);
+				msc->scroll_accel_x = min_t(int, msc->scroll_accel_x + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
+			}
+			msc->scroll_jiffies = now;
 			break;
 		}
 	}
@@ -300,8 +317,10 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 	__set_bit(EV_REL, input->evbit);
 	__set_bit(REL_X, input->relbit);
 	__set_bit(REL_Y, input->relbit);
-	if (emulate_scroll_wheel)
+	if (emulate_scroll_wheel) {
 		__set_bit(REL_WHEEL, input->relbit);
+		__set_bit(REL_HWHEEL, input->relbit);
+	}
 
 	if (report_touches) {
 		__set_bit(EV_ABS, input->evbit);

^ permalink raw reply related

* Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Ed Tomlinson @ 2010-02-14 23:58 UTC (permalink / raw)
  To: Michael Poole
  Cc: Jiri Kosina, linux-input, Marcel Holtmann, linux-bluetooth,
	linux-kernel
In-Reply-To: <87zl3bbfdo.fsf@troilus.org>

On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> Ed Tomlinson writes:
> 
> > Hi,
> >
> > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > changed the accelleration logic to work better with xy scrolling.
> 
> Hi Ed,
> 
> Your other patch to call input_unregister_device() looks good -- thanks!

Thanks.  One question about it though.  Do we have to check if msc is null before
the unregister?

> I've never used a horizontal scroll wheel -- what are the common uses
> for it?  Why should the acceleration be separate for the two directions
> rather than using the same factor?  Why does the kernel need to emulate
> this rather than having user-space implement the emulation?

Its usefull for scrolling left and right while browsing.  If you use kde it can
be used to scroll between applications on the taskbar.   Here most applications
with a horizontial scrollbar work as expected.

I first tried with a single acceleration value for both axies.  It leads to confusing
things happening.   For example.  I quickly scroll down, then nudge the
window to the left.  This works as expect with two values.  With one the
nudge is accelerated and moves too far.

My personal goal is to have the basic, apple defined, gestures working
from kernel space.  This way the device works as expected without needing
to fiddle with X or other managers (think wayland and/or chromeOS).

> (If the answers to the last two questions justify putting this feature
> in the kernel, my only suggestions on the patch are style-related:
> scroll_accel{x,y} should probably be spelled scroll_accel_{x,y}, and
> there are missing spaces around the / operator when calculating stepx
> and stepy.)

I'll fix up the style and resubmit.

Thanks
Ed

^ permalink raw reply

* Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Michael Poole @ 2010-02-14 22:51 UTC (permalink / raw)
  To: Ed Tomlinson
  Cc: Jiri Kosina, linux-input, Marcel Holtmann, linux-bluetooth,
	linux-kernel
In-Reply-To: <201002141724.33223.edt@aei.ca>

Ed Tomlinson writes:

> Hi,
>
> Here is a patch that enables xy scrolling with the magic mouse.  I have also
> changed the accelleration logic to work better with xy scrolling.

Hi Ed,

Your other patch to call input_unregister_device() looks good -- thanks!

I've never used a horizontal scroll wheel -- what are the common uses
for it?  Why should the acceleration be separate for the two directions
rather than using the same factor?  Why does the kernel need to emulate
this rather than having user-space implement the emulation?

(If the answers to the last two questions justify putting this feature
in the kernel, my only suggestions on the patch are style-related:
scroll_accel{x,y} should probably be spelled scroll_accel_{x,y}, and
there are missing spaces around the / operator when calculating stepx
and stepy.)

Michael Poole

^ permalink raw reply

* [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse
From: Ed Tomlinson @ 2010-02-14 22:24 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Michael Poole, linux-input, Marcel Holtmann, linux-bluetooth,
	linux-kernel
In-Reply-To: <alpine.LNX.2.00.1002101456490.30967@pobox.suse.cz>

Hi,

Here is a patch that enables xy scrolling with the magic mouse.  I have also
changed the accelleration logic to work better with xy scrolling.

Comments
Ed Tomlinson

---
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index b20484a..3075d78 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -59,7 +59,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @delta_time: 18-bit difference between the two most recent touch
  *     reports from the mouse.
  * @ntouches: Number of touches in most recent touch report.
- * @scroll_accel: Number of consecutive scroll motions.
+ * @scroll_accely: Number of consecutive scroll motions.
  * @scroll_jiffies: Time of last scroll motion.
  * @touches: Most recent data for a touch, indexed by tracking ID.
  * @tracking_ids: Mapping of current touch input data to @touches.
@@ -71,11 +71,13 @@ struct magicmouse_sc {
 	int last_timestamp;
 	int delta_time;
 	int ntouches;
-	int scroll_accel;
+	int scroll_accely;
+	int scroll_accelx;
 	unsigned long scroll_jiffies;
 
 	struct {
 		short x;
+		short scroll_x;
 		short y;
 		short scroll_y;
 		u8 size;
@@ -139,8 +141,10 @@ static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
 	input_report_key(msc->input, BTN_LEFT, state & 1);
 	input_report_key(msc->input, BTN_RIGHT, state & 2);
 
-	if (state != last_state)
-		msc->scroll_accel = 0;
+	if (state != last_state) {
+		msc->scroll_accely = 0;
+		msc->scroll_accelx = 0;
+	}
 }
 
 static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
@@ -159,34 +163,46 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	msc->touches[id].size = misc & 63;
 
 	/* If requested, emulate a scroll wheel by detecting small
-	 * vertical touch motions along the middle of the mouse.
+	 * touch motions on the mouse.
 	 */
 	if (emulate_scroll_wheel &&
-	    middle_button_start < x && x < middle_button_stop) {
+	    msc->ntouches == 1) {
 		static const int accel_profile[] = {
-			256, 228, 192, 160, 128, 96, 64, 32,
+			192, 160, 128, 96, 64, 48, 32, 24,
 		};
 		unsigned long now = jiffies;
-		int step = msc->touches[id].scroll_y - y;
+		int stepx, stepy;
 
 		/* Reset acceleration after half a second. */
-		if (time_after(now, msc->scroll_jiffies + HZ / 2))
-			msc->scroll_accel = 0;
+		if (time_after(now, msc->scroll_jiffies + HZ / 2)) {
+			msc->scroll_accely = 0;
+			msc->scroll_accelx = 0;
+		}
 
-		/* Calculate and apply the scroll motion. */
 		switch (tdata[7] & TOUCH_STATE_MASK) {
 		case TOUCH_STATE_START:
 			msc->touches[id].scroll_y = y;
-			msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
-						ARRAY_SIZE(accel_profile) - 1);
+			msc->touches[id].scroll_x = x;
 			break;
 		case TOUCH_STATE_DRAG:
-			step = step / accel_profile[msc->scroll_accel];
-			if (step != 0) {
+			/* Calculate and apply the scroll motion. */
+			stepy = (msc->touches[id].scroll_y - y)/accel_profile[msc->scroll_accely];
+			stepx = (msc->touches[id].scroll_x - x)/accel_profile[msc->scroll_accelx];
+
+			/* tell input about any motions */
+			if (stepy != 0) {
 				msc->touches[id].scroll_y = y;
-				msc->scroll_jiffies = now;
-				input_report_rel(input, REL_WHEEL, step);
+				input_report_rel(input, REL_WHEEL, stepy);
+				msc->scroll_accely = min_t(int, msc->scroll_accely + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
 			}
+			if (stepx != 0) {
+				msc->touches[id].scroll_x = x;
+				input_report_rel(input, REL_HWHEEL, stepx);
+				msc->scroll_accelx = min_t(int, msc->scroll_accelx + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
+			}
+			msc->scroll_jiffies = now;
 			break;
 		}
 	}
@@ -300,8 +316,10 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 	__set_bit(EV_REL, input->evbit);
 	__set_bit(REL_X, input->relbit);
 	__set_bit(REL_Y, input->relbit);
-	if (emulate_scroll_wheel)
+	if (emulate_scroll_wheel) {
 		__set_bit(REL_WHEEL, input->relbit);
+		__set_bit(REL_HWHEEL, input->relbit);
+	}
 
 	if (report_touches) {
 		__set_bit(EV_ABS, input->evbit);

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox