Pull request: bluetooth-2.6 2010-01-30

Pull request: bluetooth-2.6 2010-01-30

[Marcel Holtmann]

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

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

[Marcel Holtmann]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

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 <andrei.emeltchenko@nokia.com>
Acked-by: Gustavo F. Padovan <padovan@profusion.mobi>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1120cf1..8acc19e 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -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;
 
-- 
1.6.6

[PATCH 2/5] Bluetooth: Fix memory leak in L2CAP

[Marcel Holtmann]

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

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

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Acked-by: Gustavo F. Padovan <padovan@profusion.mobi>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 8acc19e..400efa2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -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);
-- 
1.6.6

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

[Marcel Holtmann]

From: Mike Frysinger <vapier@gentoo.org>

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 <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 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(-)

diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index 2acdc60..c2cf811 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -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;
diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index d814a27..9f5926a 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -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;
 
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index d339464..91c5230 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -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;
 
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index 4f02a6f..6975919 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -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;
 
-- 
1.6.6

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

[Marcel Holtmann]

From: Vikram Kandukuri <vkandukuri@atheros.com>

Signed-off-by: Vikram Kandukuri <vikram.kandukuri@atheros.com>
Signed-off-by: Alicke Xu <sxu@atheros.com>
Reviewed-by: Luis R. Rodriguez <lrodriguez@atheros.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 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

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index 652367a..058fbcc 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -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
diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
index b3f57d2..7e5aed5 100644
--- a/drivers/bluetooth/Makefile
+++ b/drivers/bluetooth/Makefile
@@ -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
 
diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
new file mode 100644
index 0000000..add9485
--- /dev/null
+++ b/drivers/bluetooth/ath3k.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (c) 2008-2009 Atheros Communications Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/usb.h>
+#include <net/bluetooth/bluetooth.h>
+
+#define VERSION "1.0"
+
+
+static struct usb_device_id ath3k_table[] = {
+	/* Atheros AR3011 */
+	{ USB_DEVICE(0x0CF3, 0x3000) },
+	{ }	/* Terminating entry */
+};
+
+MODULE_DEVICE_TABLE(usb, ath3k_table);
+
+#define USB_REQ_DFU_DNLOAD	1
+#define BULK_SIZE		4096
+
+struct ath3k_data {
+	struct usb_device *udev;
+	u8 *fw_data;
+	u32 fw_size;
+	u32 fw_sent;
+};
+
+static int ath3k_load_firmware(struct ath3k_data *data,
+				unsigned char *firmware,
+				int count)
+{
+	u8 *send_buf;
+	int err, pipe, len, size, sent = 0;
+
+	BT_DBG("ath3k %p udev %p", data, data->udev);
+
+	pipe = usb_sndctrlpipe(data->udev, 0);
+
+	if ((usb_control_msg(data->udev, pipe,
+				USB_REQ_DFU_DNLOAD,
+				USB_TYPE_VENDOR, 0, 0,
+				firmware, 20, USB_CTRL_SET_TIMEOUT)) < 0) {
+		BT_ERR("Can't change to loading configuration err");
+		return -EBUSY;
+	}
+	sent += 20;
+	count -= 20;
+
+	send_buf = kmalloc(BULK_SIZE, GFP_ATOMIC);
+	if (!send_buf) {
+		BT_ERR("Can't allocate memory chunk for firmware");
+		return -ENOMEM;
+	}
+
+	while (count) {
+		size = min_t(uint, count, BULK_SIZE);
+		pipe = usb_sndbulkpipe(data->udev, 0x02);
+		memcpy(send_buf, firmware + sent, size);
+
+		err = usb_bulk_msg(data->udev, pipe, send_buf, size,
+					&len, 3000);
+
+		if (err || (len != size)) {
+			BT_ERR("Error in firmware loading err = %d,"
+				"len = %d, size = %d", err, len, size);
+			goto error;
+		}
+
+		sent  += size;
+		count -= size;
+	}
+
+	kfree(send_buf);
+	return 0;
+
+error:
+	kfree(send_buf);
+	return err;
+}
+
+static int ath3k_probe(struct usb_interface *intf,
+			const struct usb_device_id *id)
+{
+	const struct firmware *firmware;
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct ath3k_data *data;
+	int size;
+
+	BT_DBG("intf %p id %p", intf, id);
+
+	if (intf->cur_altsetting->desc.bInterfaceNumber != 0)
+		return -ENODEV;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->udev = udev;
+
+	if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
+		kfree(data);
+		return -EIO;
+	}
+
+	size = max_t(uint, firmware->size, 4096);
+	data->fw_data = kmalloc(size, GFP_KERNEL);
+	if (!data->fw_data) {
+		release_firmware(firmware);
+		kfree(data);
+		return -ENOMEM;
+	}
+
+	memcpy(data->fw_data, firmware->data, firmware->size);
+	data->fw_size = firmware->size;
+	data->fw_sent = 0;
+	release_firmware(firmware);
+
+	usb_set_intfdata(intf, data);
+	if (ath3k_load_firmware(data, data->fw_data, data->fw_size)) {
+		usb_set_intfdata(intf, NULL);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static void ath3k_disconnect(struct usb_interface *intf)
+{
+	struct ath3k_data *data = usb_get_intfdata(intf);
+
+	BT_DBG("ath3k_disconnect intf %p", intf);
+
+	kfree(data->fw_data);
+	kfree(data);
+}
+
+static struct usb_driver ath3k_driver = {
+	.name		= "ath3k",
+	.probe		= ath3k_probe,
+	.disconnect	= ath3k_disconnect,
+	.id_table	= ath3k_table,
+};
+
+static int __init ath3k_init(void)
+{
+	BT_INFO("Atheros AR30xx firmware driver ver %s", VERSION);
+	return usb_register(&ath3k_driver);
+}
+
+static void __exit ath3k_exit(void)
+{
+	usb_deregister(&ath3k_driver);
+}
+
+module_init(ath3k_init);
+module_exit(ath3k_exit);
+
+MODULE_AUTHOR("Atheros Communications");
+MODULE_DESCRIPTION("Atheros AR30xx firmware driver");
+MODULE_VERSION(VERSION);
+MODULE_LICENSE("GPL");
+MODULE_FIRMWARE("ath3k-1.fw");
-- 
1.6.6

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

[Marcel Holtmann]

From: Bastien Nocera <hadess@hadess.net>

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 <hadess@hadess.net>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hidp/core.c |   70 +++++++++++++++++++++++----------------------
 1 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 18e7f5a..6cf526d 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -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)
 {
-- 
1.6.6

Re: Pull request: bluetooth-2.6 2010-01-30

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Sat, 30 Jan 2010 06:08:43 -0800

> 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.

Pulled, thanks Marcel.