Pull request: bluetooth-2.6 2010-01-30

PATCH New Marcel Holtmann 2 participants 7 messages Jan 30, 2010 – Feb 1, 2010

Subsystems (4)

Networking Network drivers Bluetooth drivers Bluetooth

lore

Share & tools

lore

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #0 lore raw

Hi Dave,

this includes two L2CAP fixes. One for a memory leak and another one for a double free.

For Bluetooth PCMCIA cards a BUG_ON caused a few issues when we have shared interrupts. So revert this and put a clear comment in the source that this is what we actually want.

The Bluetooth HID fix for raw reports has a big diffstat, but the logical change is small. By accident we send the reports on the interrupt channel and the original test device seems to cope with that. Devices that follow the specification more closer will break. The right thing is to use the control channel. This is similar to what is done via USB.

And by request from Luis, include the driver for Atheros Bluetooth chipset AR3011. It is a new driver and by itself small and self contained. If you feel that it should not be merged, then let me know and I redo this patchset.

Regards

Marcel

Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git master

This will update the following files:

 drivers/bluetooth/Kconfig       |   13 +++-
 drivers/bluetooth/Makefile      |    1 +
 drivers/bluetooth/ath3k.c       |  187 +++++++++++++++++++++++++++++++++++++++
 drivers/bluetooth/bluecard_cs.c |    4 +-
 drivers/bluetooth/bt3c_cs.c     |    4 +-
 drivers/bluetooth/btuart_cs.c   |    4 +-
 drivers/bluetooth/dtl1_cs.c     |    4 +-
 net/bluetooth/hidp/core.c       |   70 ++++++++-------
 net/bluetooth/l2cap.c           |   14 ++--
 9 files changed, 254 insertions(+), 47 deletions(-)
 create mode 100644 drivers/bluetooth/ath3k.c

through these ChangeSets:

Andrei Emeltchenko (2):
Bluetooth: Remove double free of SKB pointer in L2CAP
Bluetooth: Fix memory leak in L2CAP

Bastien Nocera (1):
Bluetooth: Use the control channel for raw HID reports

Mike Frysinger (1):
Bluetooth: Redo checks in IRQ handler for shared IRQ support

Vikram Kandukuri (1):
Bluetooth: Add DFU driver for Atheros Bluetooth chipset AR3011

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #1 lore raw

PATCH 1/5 Bluetooth: Remove double free of SKB pointer in L2CAP

From: Andrei Emeltchenko <[email protected]>

Trivial fix for double free of SKB pointer with kfree_skb to make code simplier and cleaner. Remove unused variable err.

Signed-off-by Andrei Emeltchenko <[email protected]>

Acked-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

net/bluetooth/l2cap.c +4 -7

index 1120cf1..8acc19e 100644
@@ -3518,7 +3518,6 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 	struct l2cap_pinfo *pi;
 	u16 control, len;
 	u8 tx_seq;
-	int err;
 
 	sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
 	if (!sk) {

@@ -3570,13 +3569,11 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 			goto drop;
 
 		if (__is_iframe(control))
-			err = l2cap_data_channel_iframe(sk, control, skb);
+			l2cap_data_channel_iframe(sk, control, skb);
 		else
-			err = l2cap_data_channel_sframe(sk, control, skb);
+			l2cap_data_channel_sframe(sk, control, skb);
 
-		if (!err)
-			goto done;
-		break;
+		goto done;
 
 	case L2CAP_MODE_STREAMING:
 		control = get_unaligned_le16(skb->data);

@@ -3602,7 +3599,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		else
 			pi->expected_tx_seq = tx_seq + 1;
 
-		err = l2cap_sar_reassembly_sdu(sk, skb, control);
+		l2cap_sar_reassembly_sdu(sk, skb, control);
 
 		goto done;

Signature · 1 line

1.6.6

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #2 lore raw

PATCH 2/5 Bluetooth: Fix memory leak in L2CAP

From: Andrei Emeltchenko <[email protected]>

Move skb_clone after error confition check so it is not going potentially out of the scope.

Signed-off-by Andrei Emeltchenko <[email protected]>

Acked-by Gustavo F. Padovan <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/l2cap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

net/bluetooth/l2cap.c +2 -1

index 8acc19e..400efa2 100644
@@ -1368,7 +1368,6 @@ static int l2cap_ertm_send(struct sock *sk)
 
 	while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk)) &&
 	       !(pi->conn_state & L2CAP_CONN_REMOTE_BUSY)) {
-		tx_skb = skb_clone(skb, GFP_ATOMIC);
 
 		if (pi->remote_max_tx &&
 				bt_cb(skb)->retries == pi->remote_max_tx) {

@@ -1376,6 +1375,8 @@ static int l2cap_ertm_send(struct sock *sk)
 			break;
 		}
 
+		tx_skb = skb_clone(skb, GFP_ATOMIC);
+
 		bt_cb(skb)->retries++;
 
 		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);

Signature · 1 line

1.6.6

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #3 lore raw

PATCH 3/5 Bluetooth: Redo checks in IRQ handler for shared IRQ support

From: Mike Frysinger <[email protected]>

Commit ac019360fe3 changed the irq handler logic to BUG_ON rather than returning IRQ_NONE when the incoming argument is invalid. While this works in most cases, it doesn't work when the IRQ is shared with other devices (or when DEBUG_SHIRQ is enabled).

So revert the previous change and replace the warning message with a comment explaining that we want this behavior.

Signed-off-by Michael Hennerich <[email protected]>

Signed-off-by Mike Frysinger <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 drivers/bluetooth/bluecard_cs.c |    4 +++-
 drivers/bluetooth/bt3c_cs.c     |    4 +++-
 drivers/bluetooth/btuart_cs.c   |    4 +++-
 drivers/bluetooth/dtl1_cs.c     |    4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

drivers/bluetooth/bluecard_cs.c +3 -1

index 2acdc60..c2cf811 100644
@@ -503,7 +503,9 @@ static irqreturn_t bluecard_interrupt(int irq, void *dev_inst)
 	unsigned int iobase;
 	unsigned char reg;
 
-	BUG_ON(!info->hdev);
+	if (!info || !info->hdev)
+		/* our irq handler is shared */
+		return IRQ_NONE;
 
 	if (!test_bit(CARD_READY, &(info->hw_state)))
 		return IRQ_HANDLED;

drivers/bluetooth/bt3c_cs.c +3 -1

index d814a27..9f5926a 100644
@@ -345,7 +345,9 @@ static irqreturn_t bt3c_interrupt(int irq, void *dev_inst)
 	int iir;
 	irqreturn_t r = IRQ_NONE;
 
-	BUG_ON(!info->hdev);
+	if (!info || !info->hdev)
+		/* our irq handler is shared */
+		return IRQ_NONE;
 
 	iobase = info->p_dev->io.BasePort1;

drivers/bluetooth/btuart_cs.c +3 -1

index d339464..91c5230 100644
@@ -295,7 +295,9 @@ static irqreturn_t btuart_interrupt(int irq, void *dev_inst)
 	int iir, lsr;
 	irqreturn_t r = IRQ_NONE;
 
-	BUG_ON(!info->hdev);
+	if (!info || !info->hdev)
+		/* our irq handler is shared */
+		return IRQ_NONE;
 
 	iobase = info->p_dev->io.BasePort1;

drivers/bluetooth/dtl1_cs.c +3 -1

index 4f02a6f..6975919 100644
@@ -299,7 +299,9 @@ static irqreturn_t dtl1_interrupt(int irq, void *dev_inst)
 	int iir, lsr;
 	irqreturn_t r = IRQ_NONE;
 
-	BUG_ON(!info->hdev);
+	if (!info || !info->hdev)
+		/* our irq handler is shared */
+		return IRQ_NONE;
 
 	iobase = info->p_dev->io.BasePort1;

Signature · 1 line

1.6.6

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #4 lore raw

PATCH 4/5 Bluetooth: Add DFU driver for Atheros Bluetooth chipset AR3011

From: Vikram Kandukuri <[email protected]>

Signed-off-by Vikram Kandukuri <[email protected]>

Signed-off-by Alicke Xu <[email protected]>

Reviewed-by Luis R. Rodriguez <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 drivers/bluetooth/Kconfig  |   13 +++-
 drivers/bluetooth/Makefile |    1 +
 drivers/bluetooth/ath3k.c  |  187 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 200 insertions(+), 1 deletions(-)
 create mode 100644 drivers/bluetooth/ath3k.c

drivers/bluetooth/Kconfig +12 -1

index 652367a..058fbcc 100644
@@ -195,5 +195,16 @@ config BT_MRVL_SDIO
 	  Say Y here to compile support for Marvell BT-over-SDIO driver
 	  into the kernel or say M to compile it as module.
 
-endmenu
+config BT_ATH3K
+	tristate "Atheros firmware download driver"
+	depends on BT_HCIBTUSB
+	select FW_LOADER
+	help
+	  Bluetooth firmware download driver.
+	  This driver loads the firmware into the Atheros Bluetooth
+	  chipset.
 
+	  Say Y here to compile support for "Atheros firmware download driver"
+	  into the kernel or say M to compile it as module (ath3k).
+
+endmenu

drivers/bluetooth/Makefile +1 -0

index b3f57d2..7e5aed5 100644
@@ -15,6 +15,7 @@ obj-$(CONFIG_BT_HCIBTUART)	+= btuart_cs.o
 obj-$(CONFIG_BT_HCIBTUSB)	+= btusb.o
 obj-$(CONFIG_BT_HCIBTSDIO)	+= btsdio.o
 
+obj-$(CONFIG_BT_ATH3K)		+= ath3k.o
 obj-$(CONFIG_BT_MRVL)		+= btmrvl.o
 obj-$(CONFIG_BT_MRVL_SDIO)	+= btmrvl_sdio.o

drivers/bluetooth/ath3k.c new file +187 -0

187 lines not on this page · raw message

Signature · 1 line

1.6.6

Marcel Holtmann

author maintainer

2010-01-30 14:09 UTC #5 lore raw

PATCH 5/5 Bluetooth: Use the control channel for raw HID reports

From: Bastien Nocera <[email protected]>

In commit 2da31939a42f7a676a0bc5155d6a0a39ed8451f2, support for Bluetooth hid_output_raw_report was added, but it pushes the data to the interrupt channel instead of the contol one.

This patch makes hid_output_raw_report use the control channel instead. Using the interrupt channel was a mistake.

Signed-off-by Bastien Nocera <[email protected]>

Signed-off-by Marcel Holtmann <[email protected]>

---
 net/bluetooth/hidp/core.c |   70 +++++++++++++++++++++++----------------------
 1 files changed, 36 insertions(+), 34 deletions(-)

net/bluetooth/hidp/core.c +36 ~~-34~~

index 18e7f5a..6cf526d 100644
@@ -243,6 +243,39 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 	input_sync(dev);
 }
 
+static int __hidp_send_ctrl_message(struct hidp_session *session,
+			unsigned char hdr, unsigned char *data, int size)
+{
+	struct sk_buff *skb;
+
+	BT_DBG("session %p data %p size %d", session, data, size);
+
+	if (!(skb = alloc_skb(size + 1, GFP_ATOMIC))) {
+		BT_ERR("Can't allocate memory for new frame");
+		return -ENOMEM;
+	}
+
+	*skb_put(skb, 1) = hdr;
+	if (data && size > 0)
+		memcpy(skb_put(skb, size), data, size);
+
+	skb_queue_tail(&session->ctrl_transmit, skb);
+
+	return 0;
+}
+
+static inline int hidp_send_ctrl_message(struct hidp_session *session,
+			unsigned char hdr, unsigned char *data, int size)
+{
+	int err;
+
+	err = __hidp_send_ctrl_message(session, hdr, data, size);
+
+	hidp_schedule(session);
+
+	return err;
+}
+
 static int hidp_queue_report(struct hidp_session *session,
 				unsigned char *data, int size)
 {

@@ -282,7 +315,9 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
 
 static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count)
 {
-	if (hidp_queue_report(hid->driver_data, data, count))
+	if (hidp_send_ctrl_message(hid->driver_data,
+			HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE,
+			data, count))
 		return -ENOMEM;
 	return count;
 }

@@ -307,39 +342,6 @@ static inline void hidp_del_timer(struct hidp_session *session)
 		del_timer(&session->timer);
 }
 
-static int __hidp_send_ctrl_message(struct hidp_session *session,
-			unsigned char hdr, unsigned char *data, int size)
-{
-	struct sk_buff *skb;
-
-	BT_DBG("session %p data %p size %d", session, data, size);
-
-	if (!(skb = alloc_skb(size + 1, GFP_ATOMIC))) {
-		BT_ERR("Can't allocate memory for new frame");
-		return -ENOMEM;
-	}
-
-	*skb_put(skb, 1) = hdr;
-	if (data && size > 0)
-		memcpy(skb_put(skb, size), data, size);
-
-	skb_queue_tail(&session->ctrl_transmit, skb);
-
-	return 0;
-}
-
-static inline int hidp_send_ctrl_message(struct hidp_session *session,
-			unsigned char hdr, unsigned char *data, int size)
-{
-	int err;
-
-	err = __hidp_send_ctrl_message(session, hdr, data, size);
-
-	hidp_schedule(session);
-
-	return err;
-}
-
 static void hidp_process_handshake(struct hidp_session *session,
 					unsigned char param)
 {

Signature · 1 line

1.6.6

David Miller

maintainer

2010-02-01 07:35 UTC #6 lore raw

In reply to Marcel Holtmann

From: Marcel Holtmann <[email protected]>
Date: Sat, 30 Jan 2010 06:08:43 -0800

Quoted 13 lines

this includes two L2CAP fixes. One for a memory leak and another one for a double free.
For Bluetooth PCMCIA cards a BUG_ON caused a few issues when we have shared interrupts. So revert this and put a clear comment in the source that this is what we actually want.
The Bluetooth HID fix for raw reports has a big diffstat, but the logical change is small. By accident we send the reports on the interrupt channel and the original test device seems to cope with that. Devices that follow the specification more closer will break. The right thing is to use the control channel. This is similar to what is done via USB.

Quoted 4 lines

And by request from Luis, include the driver for Atheros Bluetooth chipset AR3011. It is a new driver and by itself small and self contained. If you feel that it should not be merged, then let me know and I redo this patchset.

Pulled, thanks Marcel.