2012-10-23 13:32:16

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 1/5] Bluetooth: trivial: Remove newline before EOF

Trivial fix.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index 1c11d0d..d3f3f7b 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -48,4 +48,3 @@ source "net/bluetooth/cmtp/Kconfig"
source "net/bluetooth/hidp/Kconfig"

source "drivers/bluetooth/Kconfig"
-
--
1.7.9.5



2012-10-31 18:21:57

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 2/5] Bluetooth: Remove unnecessary include export.h

Hi Syam,

* Syam Sidhardhan <[email protected]> [2012-10-23 19:02:17 +0530]:

> For files only using THIS_MODULE and/or EXPORT_SYMBOL, map
> them onto including export.h -- or if the file isn't even
> using those, then just delete the include.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/bnep/netdev.c | 1 -
> net/bluetooth/hci_event.c | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/net/bluetooth/bnep/netdev.c b/net/bluetooth/bnep/netdev.c
> index 98f86f9..e58c8b3 100644
> --- a/net/bluetooth/bnep/netdev.c
> +++ b/net/bluetooth/bnep/netdev.c
> @@ -25,7 +25,6 @@
> SOFTWARE IS DISCLAIMED.
> */
>
> -#include <linux/export.h>
> #include <linux/etherdevice.h>
>
> #include <net/bluetooth/bluetooth.h>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 82e478a..110006a 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -24,7 +24,6 @@
>
> /* Bluetooth HCI event handling. */
>
> -#include <linux/export.h>
> #include <asm/unaligned.h>
>
> #include <net/bluetooth/bluetooth.h>

I don't see any build failure coming from this, patch is applied to
bluetooth-next. Thanks.

Gustavo

2012-10-24 02:47:21

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 3/5] Bluetooth: Replace include linux/module.h with linux/export.h

Hi Syam,

* Syam Sidhardhan <[email protected]> [2012-10-23 19:02:18 +0530]:

> include <linux/export.h> is the right to go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/cmtp/capi.c | 2 +-
> net/bluetooth/cmtp/sock.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

Gustavo

2012-10-24 02:46:49

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 1/5] Bluetooth: trivial: Remove newline before EOF

Hi Syam,

* Syam Sidhardhan <[email protected]> [2012-10-23 19:02:16 +0530]:

> Trivial fix.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/Kconfig | 1 -
> 1 file changed, 1 deletion(-)

Patch has been applied to bluetooth-next. Thanks.

Gustavo

2012-10-23 14:52:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v1 5/5] Bluetooth: Fix L2CAP dynamic PSM bind issue

Hi Syam,

> Dynamic PSM choosen by the kernel should bound to either BDADDR_ANY
> or the same adapter address(not both) for a particular adapter.
>
> The problem here is by giving PSM 0, the kernel should return the first
> available PSM in the non-reserverd space, but since we reuse the same
> code to do the matching it end up given both Obexd and HDP the same
> PSM.
>
> Provid a helper function to handle the dymanic PSM auto selection.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 55 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d42cdb1..33b5d34 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -119,6 +119,43 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
> return NULL;
> }
>
> +/* Returns free dynamic PSM */
> +static u16 __l2cap_global_get_dyna_chan_by_addr(bdaddr_t *src)
> +{

this is a bad name. No idea what kind of meaning "dyna" has.

And btw. there is no such thing as dynamic PSM. It has nothing dynamic
about it. It is an auto-selected PSM. And in the end, it just selects
the next free one.

> + struct l2cap_chan *c;
> + u16 p;
> + bool found;
> +
> + for (p = 0x1001; p < 0x1100; p += 2) {
> + found = false;
> +
> + list_for_each_entry(c, &chan_list, global_l) {
> + if (c->sport != p)
> + continue;
> +
> + /* PSM match found */
> + found = true;
> +
> + /* Exact match */
> + if (!bacmp(&bt_sk(c->sk)->src, src))
> + break;
> +
> + /* BDADDR_ANY match */
> + if (!bacmp(&bt_sk(c->sk)->src, BDADDR_ANY) ||
> + !bacmp(src, BDADDR_ANY))
> + break;
> +
> + /* Match found only for other adapter address */
> + return p;
> + }
> +
> + if (!found)
> + return p;
> + }
> +
> + return 0;
> +}
> +

This code needs a comment on how it is suppose to work and why it is
correct.

> int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
> {
> int err;
> @@ -135,16 +172,16 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
> chan->sport = psm;
> err = 0;
> } else {
> - u16 p;
> -
> + /* No PSM given by user space */
> + u16 dpsm;
> err = -EINVAL;
> - for (p = 0x1001; p < 0x1100; p += 2)
> - if (!__l2cap_global_chan_by_addr(cpu_to_le16(p), src)) {
> - chan->psm = cpu_to_le16(p);
> - chan->sport = cpu_to_le16(p);
> - err = 0;
> - break;
> - }
> +
> + dpsm = __l2cap_global_get_dyna_chan_by_addr(src);
> + if (dpsm) {
> + chan->psm = dpsm;
> + chan->sport = dpsm;
> + err = 0;
> + }
> }
>
> done:

Regards

Marcel



2012-10-23 14:46:08

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 4/5] Bluetooth: mgmt: Use __constant when dealing with constants

Hi Syam,

> __constant_cpu_to_le*() is the right go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/mgmt.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index b127b88..e3bb2a7 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -222,7 +222,7 @@ static int cmd_status(struct sock *sk, u16 index, u16 cmd, u8 status)
>
> hdr = (void *) skb_put(skb, sizeof(*hdr));
>
> - hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
> + hdr->opcode = __constant_cpu_to_le16(MGMT_EV_CMD_STATUS);
> hdr->index = cpu_to_le16(index);
> hdr->len = cpu_to_le16(sizeof(*ev));
>
> @@ -253,7 +253,7 @@ static int cmd_complete(struct sock *sk, u16 index, u16 cmd, u8 status,
>
> hdr = (void *) skb_put(skb, sizeof(*hdr));
>
> - hdr->opcode = cpu_to_le16(MGMT_EV_CMD_COMPLETE);
> + hdr->opcode = __constant_cpu_to_le16(MGMT_EV_CMD_COMPLETE);
> hdr->index = cpu_to_le16(index);
> hdr->len = cpu_to_le16(sizeof(*ev) + rp_len);
>
> @@ -832,7 +832,7 @@ static int mgmt_event(u16 event, struct hci_dev *hdev, void *data, u16 data_len,
> if (hdev)
> hdr->index = cpu_to_le16(hdev->id);
> else
> - hdr->index = cpu_to_le16(MGMT_INDEX_NONE);
> + hdr->index = __constant_cpu_to_le16(MGMT_INDEX_NONE);
> hdr->len = cpu_to_le16(data_len);
>
> if (data)
> @@ -3570,9 +3570,11 @@ int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
> ev->addr.type = link_to_bdaddr(link_type, addr_type);
> ev->rssi = rssi;
> if (cfm_name)
> - ev->flags |= cpu_to_le32(MGMT_DEV_FOUND_CONFIRM_NAME);
> + ev->flags |=
> + __constant_cpu_to_le32(MGMT_DEV_FOUND_CONFIRM_NAME);
> if (!ssp)
> - ev->flags |= cpu_to_le32(MGMT_DEV_FOUND_LEGACY_PAIRING);
> + ev->flags |=
> + __constant_cpu_to_le32(MGMT_DEV_FOUND_LEGACY_PAIRING);

for these ones, break the 80 chars rule. In this case that is
acceptable.

Regards

Marcel



2012-10-23 14:45:16

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 3/5] Bluetooth: Replace include linux/module.h with linux/export.h

Hi Syam,

> include <linux/export.h> is the right to go here.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/cmtp/capi.c | 2 +-
> net/bluetooth/cmtp/sock.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-23 14:44:42

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 2/5] Bluetooth: Remove unnecessary include export.h

Hi Syam,

> For files only using THIS_MODULE and/or EXPORT_SYMBOL, map
> them onto including export.h -- or if the file isn't even
> using those, then just delete the include.

have you actually check this on other architectures like PowerPC or
Sparc?

commit 8c520a59927a5600973782505dbb750d985057c4
Author: Gustavo Padovan <[email protected]>
Date: Wed May 23 04:04:22 2012 -0300

Bluetooth: Remove unnecessary headers include

Most of the include were unnecessary or already included by some other
header.
Replace module.h by export.h where possible.

Regards

Marcel



2012-10-23 14:43:02

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/5] Bluetooth: trivial: Remove newline before EOF

Hi Syam,

> Trivial fix.
>
> Signed-off-by: Syam Sidhardhan <[email protected]>
> ---
> net/bluetooth/Kconfig | 1 -
> 1 file changed, 1 deletion(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2012-10-23 13:32:20

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH v1 5/5] Bluetooth: Fix L2CAP dynamic PSM bind issue

Dynamic PSM choosen by the kernel should bound to either BDADDR_ANY
or the same adapter address(not both) for a particular adapter.

The problem here is by giving PSM 0, the kernel should return the first
available PSM in the non-reserverd space, but since we reuse the same
code to do the matching it end up given both Obexd and HDP the same
PSM.

Provid a helper function to handle the dymanic PSM auto selection.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/l2cap_core.c | 55 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d42cdb1..33b5d34 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -119,6 +119,43 @@ static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
return NULL;
}

+/* Returns free dynamic PSM */
+static u16 __l2cap_global_get_dyna_chan_by_addr(bdaddr_t *src)
+{
+ struct l2cap_chan *c;
+ u16 p;
+ bool found;
+
+ for (p = 0x1001; p < 0x1100; p += 2) {
+ found = false;
+
+ list_for_each_entry(c, &chan_list, global_l) {
+ if (c->sport != p)
+ continue;
+
+ /* PSM match found */
+ found = true;
+
+ /* Exact match */
+ if (!bacmp(&bt_sk(c->sk)->src, src))
+ break;
+
+ /* BDADDR_ANY match */
+ if (!bacmp(&bt_sk(c->sk)->src, BDADDR_ANY) ||
+ !bacmp(src, BDADDR_ANY))
+ break;
+
+ /* Match found only for other adapter address */
+ return p;
+ }
+
+ if (!found)
+ return p;
+ }
+
+ return 0;
+}
+
int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
{
int err;
@@ -135,16 +172,16 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
chan->sport = psm;
err = 0;
} else {
- u16 p;
-
+ /* No PSM given by user space */
+ u16 dpsm;
err = -EINVAL;
- for (p = 0x1001; p < 0x1100; p += 2)
- if (!__l2cap_global_chan_by_addr(cpu_to_le16(p), src)) {
- chan->psm = cpu_to_le16(p);
- chan->sport = cpu_to_le16(p);
- err = 0;
- break;
- }
+
+ dpsm = __l2cap_global_get_dyna_chan_by_addr(src);
+ if (dpsm) {
+ chan->psm = dpsm;
+ chan->sport = dpsm;
+ err = 0;
+ }
}

done:
--
1.7.9.5


2012-10-23 13:32:19

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 4/5] Bluetooth: mgmt: Use __constant when dealing with constants

__constant_cpu_to_le*() is the right go here.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/mgmt.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b127b88..e3bb2a7 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -222,7 +222,7 @@ static int cmd_status(struct sock *sk, u16 index, u16 cmd, u8 status)

hdr = (void *) skb_put(skb, sizeof(*hdr));

- hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
+ hdr->opcode = __constant_cpu_to_le16(MGMT_EV_CMD_STATUS);
hdr->index = cpu_to_le16(index);
hdr->len = cpu_to_le16(sizeof(*ev));

@@ -253,7 +253,7 @@ static int cmd_complete(struct sock *sk, u16 index, u16 cmd, u8 status,

hdr = (void *) skb_put(skb, sizeof(*hdr));

- hdr->opcode = cpu_to_le16(MGMT_EV_CMD_COMPLETE);
+ hdr->opcode = __constant_cpu_to_le16(MGMT_EV_CMD_COMPLETE);
hdr->index = cpu_to_le16(index);
hdr->len = cpu_to_le16(sizeof(*ev) + rp_len);

@@ -832,7 +832,7 @@ static int mgmt_event(u16 event, struct hci_dev *hdev, void *data, u16 data_len,
if (hdev)
hdr->index = cpu_to_le16(hdev->id);
else
- hdr->index = cpu_to_le16(MGMT_INDEX_NONE);
+ hdr->index = __constant_cpu_to_le16(MGMT_INDEX_NONE);
hdr->len = cpu_to_le16(data_len);

if (data)
@@ -3570,9 +3570,11 @@ int mgmt_device_found(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
ev->addr.type = link_to_bdaddr(link_type, addr_type);
ev->rssi = rssi;
if (cfm_name)
- ev->flags |= cpu_to_le32(MGMT_DEV_FOUND_CONFIRM_NAME);
+ ev->flags |=
+ __constant_cpu_to_le32(MGMT_DEV_FOUND_CONFIRM_NAME);
if (!ssp)
- ev->flags |= cpu_to_le32(MGMT_DEV_FOUND_LEGACY_PAIRING);
+ ev->flags |=
+ __constant_cpu_to_le32(MGMT_DEV_FOUND_LEGACY_PAIRING);

if (eir_len > 0)
memcpy(ev->eir, eir, eir_len);
--
1.7.9.5


2012-10-23 13:32:18

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 3/5] Bluetooth: Replace include linux/module.h with linux/export.h

include <linux/export.h> is the right to go here.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/cmtp/capi.c | 2 +-
net/bluetooth/cmtp/sock.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/cmtp/capi.c b/net/bluetooth/cmtp/capi.c
index 50f0d13..a4a9d4b 100644
--- a/net/bluetooth/cmtp/capi.c
+++ b/net/bluetooth/cmtp/capi.c
@@ -20,7 +20,7 @@
SOFTWARE IS DISCLAIMED.
*/

-#include <linux/module.h>
+#include <linux/export.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
#include <linux/types.h>
diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
index d5cacef..ea0e813 100644
--- a/net/bluetooth/cmtp/sock.c
+++ b/net/bluetooth/cmtp/sock.c
@@ -20,7 +20,7 @@
SOFTWARE IS DISCLAIMED.
*/

-#include <linux/module.h>
+#include <linux/export.h>

#include <linux/types.h>
#include <linux/capability.h>
--
1.7.9.5


2012-10-23 13:32:17

by Syam Sidhardhan

[permalink] [raw]
Subject: [PATCH 2/5] Bluetooth: Remove unnecessary include export.h

For files only using THIS_MODULE and/or EXPORT_SYMBOL, map
them onto including export.h -- or if the file isn't even
using those, then just delete the include.

Signed-off-by: Syam Sidhardhan <[email protected]>
---
net/bluetooth/bnep/netdev.c | 1 -
net/bluetooth/hci_event.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/net/bluetooth/bnep/netdev.c b/net/bluetooth/bnep/netdev.c
index 98f86f9..e58c8b3 100644
--- a/net/bluetooth/bnep/netdev.c
+++ b/net/bluetooth/bnep/netdev.c
@@ -25,7 +25,6 @@
SOFTWARE IS DISCLAIMED.
*/

-#include <linux/export.h>
#include <linux/etherdevice.h>

#include <net/bluetooth/bluetooth.h>
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 82e478a..110006a 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -24,7 +24,6 @@

/* Bluetooth HCI event handling. */

-#include <linux/export.h>
#include <asm/unaligned.h>

#include <net/bluetooth/bluetooth.h>
--
1.7.9.5