* [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync()
@ 2015-05-07 14:24 Frederic Danis
2015-05-07 14:24 ` [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device Frederic Danis
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Frederic Danis @ 2015-05-07 14:24 UTC (permalink / raw)
To: linux-bluetooth
Handle command complete event and extract error/status.
Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
---
drivers/bluetooth/btbcm.c | 27 +++++-----
drivers/bluetooth/btintel.c | 14 +++---
drivers/bluetooth/btmrvl_main.c | 2 +-
drivers/bluetooth/btusb.c | 104 +++++++++++++++++++++------------------
drivers/bluetooth/hci_ath.c | 3 +-
drivers/bluetooth/hci_ldisc.c | 2 +-
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_core.c | 17 +++++--
8 files changed, 95 insertions(+), 76 deletions(-)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 4bba866..5635d6d 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -38,9 +38,10 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
{
struct hci_rp_read_bd_addr *bda;
struct sk_buff *skb;
+ u8 status;
skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
int err = PTR_ERR(skb);
BT_ERR("%s: BCM: Reading device address failed (%d)",
@@ -54,14 +55,15 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
return -EIO;
}
- bda = (struct hci_rp_read_bd_addr *)skb->data;
- if (bda->status) {
+ if (status) {
BT_ERR("%s: BCM: Device address result failed (%02x)",
- hdev->name, bda->status);
+ hdev->name, status);
kfree_skb(skb);
- return -bt_to_errno(bda->status);
+ return -bt_to_errno(status);
}
+ bda = (struct hci_rp_read_bd_addr *)skb->data;
+
/* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
* with no configured address.
*/
@@ -82,7 +84,7 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
struct sk_buff *skb;
int err;
- skb = __hci_cmd_sync(hdev, 0xfc01, 6, bdaddr, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc01, 6, bdaddr, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
BT_ERR("%s: BCM: Change address command failed (%d)",
@@ -112,7 +114,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
}
/* Start Download */
- skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
BT_ERR("%s: BCM: Download Minidrv command failed (%d)",
@@ -148,7 +150,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
opcode = le16_to_cpu(cmd->opcode);
skb = __hci_cmd_sync(hdev, opcode, cmd->plen, cmd_param,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
BT_ERR("%s: BCM: Patch command %04x failed (%d)",
@@ -171,7 +173,8 @@ static int btbcm_reset(struct hci_dev *hdev)
{
struct sk_buff *skb;
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
int err = PTR_ERR(skb);
BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
@@ -187,7 +190,7 @@ static struct sk_buff *btbcm_read_local_version(struct hci_dev *hdev)
struct sk_buff *skb;
skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: BCM: Reading local version info failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -207,7 +210,7 @@ static struct sk_buff *btbcm_read_verbose_config(struct hci_dev *hdev)
{
struct sk_buff *skb;
- skb = __hci_cmd_sync(hdev, 0xfc79, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc79, 0, NULL, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: BCM: Read verbose config info failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -227,7 +230,7 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
{
struct sk_buff *skb;
- skb = __hci_cmd_sync(hdev, 0xfc5a, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc5a, 0, NULL, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: BCM: Read USB product info failed (%ld)",
hdev->name, PTR_ERR(skb));
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 2d43d42..1a17dd3 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -36,9 +36,10 @@ int btintel_check_bdaddr(struct hci_dev *hdev)
{
struct hci_rp_read_bd_addr *bda;
struct sk_buff *skb;
+ u8 status;
skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
int err = PTR_ERR(skb);
BT_ERR("%s: Reading Intel device address failed (%d)",
@@ -52,14 +53,15 @@ int btintel_check_bdaddr(struct hci_dev *hdev)
return -EIO;
}
- bda = (struct hci_rp_read_bd_addr *)skb->data;
- if (bda->status) {
+ if (status) {
BT_ERR("%s: Intel device address result failed (%02x)",
- hdev->name, bda->status);
+ hdev->name, status);
kfree_skb(skb);
- return -bt_to_errno(bda->status);
+ return -bt_to_errno(status);
}
+ bda = (struct hci_rp_read_bd_addr *)skb->data;
+
/* For some Intel based controllers, the default Bluetooth device
* address 00:03:19:9E:8B:00 can be found. These controllers are
* fully operational, but have the danger of duplicate addresses
@@ -82,7 +84,7 @@ int btintel_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
struct sk_buff *skb;
int err;
- skb = __hci_cmd_sync(hdev, 0xfc31, 6, bdaddr, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc31, 6, bdaddr, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
BT_ERR("%s: Changing Intel device address failed (%d)",
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index de05deb..f9005f3 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -593,7 +593,7 @@ static int btmrvl_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
memcpy(buf + 2, bdaddr, sizeof(bdaddr_t));
skb = __hci_cmd_sync(hdev, BT_CMD_SET_BDADDR, sizeof(buf), buf,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
BT_ERR("%s: changing btmrvl device address failed (%ld)",
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index d21f3b4..77c1f8c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1300,7 +1300,7 @@ static struct sk_buff *btusb_read_local_version(struct hci_dev *hdev)
struct sk_buff *skb;
skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -1324,7 +1324,7 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
BT_DBG("%s", hdev->name);
- skb = __hci_cmd_sync(hdev, 0xfc3b, 1, &val, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc3b, 1, &val, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb))
BT_ERR("BCM92035 command failed (%ld)", -PTR_ERR(skb));
else
@@ -1403,10 +1403,10 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
{
struct rtl_rom_version_evt *rom_version;
struct sk_buff *skb;
- int ret;
+ u8 status;
/* Read RTL ROM version command */
- skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s: Read ROM version failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -1421,14 +1421,13 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
rom_version = (struct rtl_rom_version_evt *)skb->data;
BT_INFO("%s: rom_version status=%x version=%x",
- hdev->name, rom_version->status, rom_version->version);
+ hdev->name, status, rom_version->version);
- ret = rom_version->status;
- if (ret == 0)
+ if (status == 0)
*version = rom_version->version;
kfree_skb(skb);
- return ret;
+ return status;
}
static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
@@ -1587,8 +1586,8 @@ static int rtl_download_firmware(struct hci_dev *hdev,
return -ENOMEM;
for (i = 0; i < frag_num; i++) {
- struct rtl_download_response *dl_resp;
struct sk_buff *skb;
+ u8 status;
BT_DBG("download fw (%d/%d)", i, frag_num);
@@ -1601,7 +1600,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
/* Send download command */
skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s: download fw command failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -1609,7 +1608,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
goto out;
}
- if (skb->len != sizeof(*dl_resp)) {
+ if (skb->len != sizeof(struct rtl_download_response)) {
BT_ERR("%s: download fw event length mismatch",
hdev->name);
kfree_skb(skb);
@@ -1617,10 +1616,9 @@ static int rtl_download_firmware(struct hci_dev *hdev,
goto out;
}
- dl_resp = (struct rtl_download_response *)skb->data;
- if (dl_resp->status != 0) {
+ if (status != 0) {
kfree_skb(skb);
- ret = bt_to_errno(dl_resp->status);
+ ret = bt_to_errno(status);
goto out;
}
@@ -1900,6 +1898,7 @@ static int btusb_setup_intel_patching(struct hci_dev *hdev,
static int btusb_setup_intel(struct hci_dev *hdev)
{
struct sk_buff *skb;
+ u8 status;
const struct firmware *fw;
const u8 *fw_ptr;
int disable_patch;
@@ -1920,7 +1919,8 @@ static int btusb_setup_intel(struct hci_dev *hdev)
* number of completed commands and allow normal command processing
* from now on.
*/
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
BT_ERR("%s sending initial HCI reset command failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -1934,7 +1934,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
* The returned information are hardware variant and revision plus
* firmware variant, revision and build number.
*/
- skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s reading Intel fw version command failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -1947,14 +1947,15 @@ static int btusb_setup_intel(struct hci_dev *hdev)
return -EIO;
}
- ver = (struct intel_version *)skb->data;
- if (ver->status) {
+ if (status) {
BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
- ver->status);
+ status);
kfree_skb(skb);
- return -bt_to_errno(ver->status);
+ return -bt_to_errno(status);
}
+ ver = (struct intel_version *)skb->data;
+
BT_INFO("%s: read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",
hdev->name, ver->hw_platform, ver->hw_variant,
ver->hw_revision, ver->fw_variant, ver->fw_revision,
@@ -1993,7 +1994,8 @@ static int btusb_setup_intel(struct hci_dev *hdev)
* Only while this mode is enabled, the driver can download the
* firmware patch data and configuration parameters.
*/
- skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT,
+ &status);
if (IS_ERR(skb)) {
BT_ERR("%s entering Intel manufacturer mode failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2001,14 +2003,12 @@ static int btusb_setup_intel(struct hci_dev *hdev)
return PTR_ERR(skb);
}
- if (skb->data[0]) {
- u8 evt_status = skb->data[0];
-
+ if (status) {
BT_ERR("%s enable Intel manufacturer mode event failed (%02x)",
- hdev->name, evt_status);
+ hdev->name, status);
kfree_skb(skb);
release_firmware(fw);
- return -bt_to_errno(evt_status);
+ return -bt_to_errno(status);
}
kfree_skb(skb);
@@ -2052,7 +2052,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
* with reset and activate the downloaded firmware patches.
*/
skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_activate),
- mfg_reset_activate, HCI_INIT_TIMEOUT);
+ mfg_reset_activate, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2069,7 +2069,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
exit_mfg_disable:
/* Disable the manufacturer mode without reset */
skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_disable), mfg_disable,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2089,7 +2089,7 @@ exit_mfg_deactivate:
* deactivate the downloaded firmware patches.
*/
skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_deactivate),
- mfg_reset_deactivate, HCI_INIT_TIMEOUT);
+ mfg_reset_deactivate, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2284,7 +2284,7 @@ static int btusb_intel_secure_send(struct hci_dev *hdev, u8 fragment_type,
memcpy(cmd_param + 1, param, fragment_len);
skb = __hci_cmd_sync(hdev, 0xfc09, fragment_len + 1,
- cmd_param, HCI_INIT_TIMEOUT);
+ cmd_param, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb))
return PTR_ERR(skb);
@@ -2324,6 +2324,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
0x00, 0x08, 0x04, 0x00 };
struct btusb_data *data = hci_get_drvdata(hdev);
struct sk_buff *skb;
+ u8 status;
struct intel_version *ver;
struct intel_boot_params *params;
const struct firmware *fw;
@@ -2341,7 +2342,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
* is in bootloader mode or if it already has operational firmware
* loaded.
*/
- skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s: Reading Intel version information failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2354,15 +2355,16 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
return -EILSEQ;
}
- ver = (struct intel_version *)skb->data;
- if (ver->status) {
+ if (status) {
BT_ERR("%s: Intel version command failure (%02x)",
- hdev->name, ver->status);
- err = -bt_to_errno(ver->status);
+ hdev->name, status);
+ err = -bt_to_errno(status);
kfree_skb(skb);
return err;
}
+ ver = (struct intel_version *)skb->data;
+
/* The hardware platform number has a fixed value of 0x37 and
* for now only accept this single value.
*/
@@ -2422,7 +2424,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
/* Read the secure boot parameters to identify the operating
* details of the bootloader.
*/
- skb = __hci_cmd_sync(hdev, 0xfc0d, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc0d, 0, NULL, HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s: Reading Intel boot parameters failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2435,15 +2437,16 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
return -EILSEQ;
}
- params = (struct intel_boot_params *)skb->data;
- if (params->status) {
+ if (status) {
BT_ERR("%s: Intel boot parameters command failure (%02x)",
- hdev->name, params->status);
- err = -bt_to_errno(params->status);
+ hdev->name, status);
+ err = -bt_to_errno(status);
kfree_skb(skb);
return err;
}
+ params = (struct intel_boot_params *)skb->data;
+
BT_INFO("%s: Device revision is %u", hdev->name,
le16_to_cpu(params->dev_revid));
@@ -2607,7 +2610,7 @@ done:
set_bit(BTUSB_BOOTING, &data->flags);
skb = __hci_cmd_sync(hdev, 0xfc01, sizeof(reset_param), reset_param,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb))
return PTR_ERR(skb);
@@ -2650,11 +2653,12 @@ done:
static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
{
struct sk_buff *skb;
- u8 type = 0x00;
+ u8 status, type = 0x00;
BT_ERR("%s: Hardware error 0x%2.2x", hdev->name, code);
- skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: Reset after hardware error failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2662,7 +2666,7 @@ static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
}
kfree_skb(skb);
- skb = __hci_cmd_sync(hdev, 0xfc22, 1, &type, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc22, 1, &type, HCI_INIT_TIMEOUT, &status);
if (IS_ERR(skb)) {
BT_ERR("%s: Retrieving Intel exception info failed (%ld)",
hdev->name, PTR_ERR(skb));
@@ -2675,9 +2679,9 @@ static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
return;
}
- if (skb->data[0] != 0x00) {
+ if (status) {
BT_ERR("%s: Exception info command failure (%02x)",
- hdev->name, skb->data[0]);
+ hdev->name, status);
kfree_skb(skb);
return;
}
@@ -2696,7 +2700,7 @@ static int btusb_shutdown_intel(struct hci_dev *hdev)
* down or BT radio is turned off, which takes 5 seconds to BT LED
* goes off. This command turns off the BT LED immediately.
*/
- skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
BT_ERR("%s: turning off Intel device LED failed (%ld)",
@@ -2719,7 +2723,8 @@ static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
buf[1] = sizeof(bdaddr_t);
memcpy(buf + 2, bdaddr, sizeof(bdaddr_t));
- skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
BT_ERR("%s: changing Marvell device address failed (%ld)",
@@ -2744,7 +2749,8 @@ static int btusb_set_bdaddr_ath3012(struct hci_dev *hdev,
buf[3] = sizeof(bdaddr_t);
memcpy(buf + 4, bdaddr, sizeof(bdaddr_t));
- skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
ret = PTR_ERR(skb);
BT_ERR("%s: Change address command failed (%ld)",
diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
index ec8fa0e..19a8f92 100644
--- a/drivers/bluetooth/hci_ath.c
+++ b/drivers/bluetooth/hci_ath.c
@@ -156,7 +156,8 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
buf[3] = sizeof(bdaddr_t);
memcpy(buf + 4, bdaddr, sizeof(bdaddr_t));
- skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT);
+ skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT,
+ NULL);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
BT_ERR("%s: Change address command failed (%d)",
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 5c9a73f..2b5d946 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -278,7 +278,7 @@ static int hci_uart_setup(struct hci_dev *hdev)
return 0;
skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
- HCI_INIT_TIMEOUT);
+ HCI_INIT_TIMEOUT, NULL);
if (IS_ERR(skb)) {
BT_ERR("%s: Reading local version information failed (%ld)",
hdev->name, PTR_ERR(skb));
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index a056c2b..ea02e00 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1282,7 +1282,7 @@ int hci_register_cb(struct hci_cb *hcb);
int hci_unregister_cb(struct hci_cb *hcb);
struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
- const void *param, u32 timeout);
+ const void *param, u32 timeout, u8 *status);
struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
const void *param, u8 event, u32 timeout);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 476709b..45f5061 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -94,6 +94,7 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,
char buf[32];
size_t buf_size = min(count, (sizeof(buf)-1));
bool enable;
+ u8 status;
int err;
if (!test_bit(HCI_UP, &hdev->flags))
@@ -112,16 +113,16 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,
hci_req_lock(hdev);
if (enable)
skb = __hci_cmd_sync(hdev, HCI_OP_ENABLE_DUT_MODE, 0, NULL,
- HCI_CMD_TIMEOUT);
+ HCI_CMD_TIMEOUT, &status);
else
skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
- HCI_CMD_TIMEOUT);
+ HCI_CMD_TIMEOUT, &status);
hci_req_unlock(hdev);
if (IS_ERR(skb))
return PTR_ERR(skb);
- err = -bt_to_errno(skb->data[0]);
+ err = -bt_to_errno(status);
kfree_skb(skb);
if (err < 0)
@@ -232,9 +233,15 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
EXPORT_SYMBOL(__hci_cmd_sync_ev);
struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
- const void *param, u32 timeout)
+ const void *param, u32 timeout, u8 *status)
{
- return __hci_cmd_sync_ev(hdev, opcode, plen, param, 0, timeout);
+ struct sk_buff *skb;
+
+ skb = __hci_cmd_sync_ev(hdev, opcode, plen, param, 0, timeout);
+ if (!IS_ERR(skb) && status)
+ *status = skb->data[0];
+
+ return skb;
}
EXPORT_SYMBOL(__hci_cmd_sync);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
@ 2015-05-07 14:24 ` Frederic Danis
2015-05-07 16:19 ` Marcel Holtmann
2015-05-07 14:24 ` [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup Frederic Danis
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Frederic Danis @ 2015-05-07 14:24 UTC (permalink / raw)
To: linux-bluetooth
Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
---
drivers/bluetooth/btbcm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 5635d6d..b258454 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -33,6 +33,7 @@
#define VERSION "0.1"
#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
+#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
int btbcm_check_bdaddr(struct hci_dev *hdev)
{
@@ -71,6 +72,10 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
BT_INFO("%s: BCM: Using default device address (%pMR)",
hdev->name, &bda->bdaddr);
set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
+ } else if (!bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
+ BT_INFO("%s: BCM: Using default device address (%pMR)",
+ hdev->name, &bda->bdaddr);
+ set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
}
kfree_skb(skb);
@@ -251,6 +256,7 @@ static const struct {
const char *name;
} bcm_uart_subver_table[] = {
{ 0x410e, "BCM43341B0" }, /* 002.001.014 */
+ { 0x4406, "BCM4324B3" }, /* 002.004.006 */
{ }
};
@@ -305,6 +311,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
switch ((rev & 0xf000) >> 12) {
case 0:
+ case 3:
for (i = 0; bcm_uart_subver_table[i].name; i++) {
if (subver == bcm_uart_subver_table[i].subver) {
hw_name = bcm_uart_subver_table[i].name;
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
2015-05-07 14:24 ` [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device Frederic Danis
@ 2015-05-07 14:24 ` Frederic Danis
2015-05-07 16:13 ` Marcel Holtmann
2015-05-07 14:24 ` [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function Frederic Danis
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Frederic Danis @ 2015-05-07 14:24 UTC (permalink / raw)
To: linux-bluetooth
Add default and operational speeds.
Change to operational speed as soon as possible.
Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
---
drivers/bluetooth/hci_ldisc.c | 26 ++++++++++++++++++++++++++
drivers/bluetooth/hci_uart.h | 4 ++++
2 files changed, 30 insertions(+)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 2b5d946..071f09c 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -265,11 +265,37 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return 0;
}
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned long speed)
+{
+ struct tty_struct *tty = hu->tty;
+ struct ktermios ktermios;
+
+ ktermios = tty->termios;
+ ktermios.c_cflag &= ~CBAUD;
+ ktermios.c_cflag |= BOTHER;
+ tty_termios_encode_baud_rate(&ktermios, speed, speed);
+
+ tty_set_termios(tty, &ktermios);
+
+ BT_DBG("%s: New tty speed: %d", hu->hdev->name, tty->termios.c_ispeed);
+}
+
static int hci_uart_setup(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
struct hci_rp_read_local_version *ver;
struct sk_buff *skb;
+ int err;
+
+ if (hu->proto->default_speed)
+ hci_uart_set_baudrate(hu, hu->proto->default_speed);
+
+ if (hu->proto->set_baudrate && hu->proto->operational_speed) {
+ err = hu->proto->set_baudrate(hu, hu->proto->operational_speed);
+ if (err)
+ return err;
+ hci_uart_set_baudrate(hu, hu->proto->operational_speed);
+ }
if (hu->proto->setup)
return hu->proto->setup(hu);
diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
index 72120a5..572ac04 100644
--- a/drivers/bluetooth/hci_uart.h
+++ b/drivers/bluetooth/hci_uart.h
@@ -58,10 +58,13 @@ struct hci_uart;
struct hci_uart_proto {
unsigned int id;
const char *name;
+ unsigned long default_speed;
+ unsigned long operational_speed;
int (*open)(struct hci_uart *hu);
int (*close)(struct hci_uart *hu);
int (*flush)(struct hci_uart *hu);
int (*setup)(struct hci_uart *hu);
+ int (*set_baudrate)(struct hci_uart *hu, int speed);
int (*recv)(struct hci_uart *hu, const void *data, int len);
int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
struct sk_buff *(*dequeue)(struct hci_uart *hu);
@@ -96,6 +99,7 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
int hci_uart_unregister_proto(const struct hci_uart_proto *p);
int hci_uart_tx_wakeup(struct hci_uart *hu);
int hci_uart_init_ready(struct hci_uart *hu);
+void hci_uart_set_baudrate(struct hci_uart *hu, unsigned long speed);
#ifdef CONFIG_BT_HCIUART_H4
int h4_init(void);
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
2015-05-07 14:24 ` [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device Frederic Danis
2015-05-07 14:24 ` [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup Frederic Danis
@ 2015-05-07 14:24 ` Frederic Danis
2015-05-07 16:22 ` Marcel Holtmann
2015-05-07 14:24 ` [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate() Frederic Danis
2015-05-07 16:11 ` [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Marcel Holtmann
4 siblings, 1 reply; 13+ messages in thread
From: Frederic Danis @ 2015-05-07 14:24 UTC (permalink / raw)
To: linux-bluetooth
Split btbcm_setup_patchram in 3 functions : btbcm_setup_patchram,
btbcm_patchram (already existing) and btbcm_setup_post.
For UART controllers this will allow to reset speed to default one
after firmware loading, which will have reseted controller speed to
default one.
Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
---
drivers/bluetooth/btbcm.c | 29 +++++++++++++++++------------
drivers/bluetooth/btbcm.h | 11 +++++++++--
drivers/bluetooth/btusb.c | 23 ++++++++++++++++++++++-
drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
4 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index b258454..f2efbbd 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -277,9 +277,8 @@ static const struct {
{ }
};
-int btbcm_setup_patchram(struct hci_dev *hdev)
+int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len)
{
- char fw_name[64];
u16 subver, rev, pid, vid;
const char *hw_name = NULL;
struct sk_buff *skb;
@@ -319,8 +318,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
}
}
- snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
- hw_name ? : "BCM");
+ snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
break;
case 1:
case 2:
@@ -340,7 +338,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
}
}
- snprintf(fw_name, sizeof(fw_name), "brcm/%s-%4.4x-%4.4x.hcd",
+ snprintf(fw_name, len, "brcm/%s-%4.4x-%4.4x.hcd",
hw_name ? : "BCM", vid, pid);
break;
default:
@@ -351,9 +349,16 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
hw_name ? : "BCM", (subver & 0x7000) >> 13,
(subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
- err = btbcm_patchram(hdev, fw_name);
- if (err == -ENOENT)
- return 0;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_setup_patchram);
+
+int btbcm_setup_post(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ struct hci_rp_read_local_version *ver;
+ u16 subver, rev;
+ int err;
/* Reset */
err = btbcm_reset(hdev);
@@ -370,9 +375,9 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
subver = le16_to_cpu(ver->lmp_subver);
kfree_skb(skb);
- BT_INFO("%s: %s (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
- hw_name ? : "BCM", (subver & 0x7000) >> 13,
- (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
+ BT_INFO("%s: BCM (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
+ (subver & 0x7000) >> 13, (subver & 0x1f00) >> 8,
+ (subver & 0x00ff), rev & 0x0fff);
btbcm_check_bdaddr(hdev);
@@ -380,7 +385,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
return 0;
}
-EXPORT_SYMBOL_GPL(btbcm_setup_patchram);
+EXPORT_SYMBOL_GPL(btbcm_setup_post);
int btbcm_setup_apple(struct hci_dev *hdev)
{
diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
index eb6ab5f..73d92a2 100644
--- a/drivers/bluetooth/btbcm.h
+++ b/drivers/bluetooth/btbcm.h
@@ -27,7 +27,8 @@ int btbcm_check_bdaddr(struct hci_dev *hdev);
int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
int btbcm_patchram(struct hci_dev *hdev, const char *firmware);
-int btbcm_setup_patchram(struct hci_dev *hdev);
+int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len);
+int btbcm_setup_post(struct hci_dev *hdev);
int btbcm_setup_apple(struct hci_dev *hdev);
#else
@@ -47,7 +48,13 @@ static inline int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
return -EOPNOTSUPP;
}
-static inline int btbcm_setup_patchram(struct hci_dev *hdev)
+static inline int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name,
+ size_t len)
+{
+ return 0;
+}
+
+static int btbcm_setup_post(struct hci_dev *hdev)
{
return 0;
}
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 77c1f8c..15225d9 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1333,6 +1333,27 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
return 0;
}
+#ifdef CONFIG_BT_HCIBTUSB_BCM
+static int btusb_setup_bcm(struct hci_dev *hdev)
+{
+ char fw_name[64];
+ int err;
+
+ err = btbcm_setup_patchram(hdev, fw_name, sizeof(fw_name));
+ if (err)
+ return err;
+
+ err = btbcm_patchram(hdev, fw_name);
+ /* If there is no firmware (-ENOENT), discard the error and continue */
+ if (err == -ENOENT)
+ return 0;
+
+ err = btbcm_setup_post(hdev);
+
+ return err;
+}
+#endif
+
static int btusb_setup_csr(struct hci_dev *hdev)
{
struct hci_rp_read_local_version *rp;
@@ -3132,7 +3153,7 @@ static int btusb_probe(struct usb_interface *intf,
#ifdef CONFIG_BT_HCIBTUSB_BCM
if (id->driver_info & BTUSB_BCM_PATCHRAM) {
- hdev->setup = btbcm_setup_patchram;
+ hdev->setup = btusb_setup_bcm;
hdev->set_bdaddr = btbcm_set_bdaddr;
}
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 1ec0b4a..0e77b9a 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -79,11 +79,28 @@ static int bcm_flush(struct hci_uart *hu)
static int bcm_setup(struct hci_uart *hu)
{
+ char fw_name[64];
+ int err;
+
BT_DBG("hu %p", hu);
hu->hdev->set_bdaddr = btbcm_set_bdaddr;
- return btbcm_setup_patchram(hu->hdev);
+ err = btbcm_setup_patchram(hu->hdev, fw_name, sizeof(fw_name));
+ if (err)
+ return err;
+
+ err = btbcm_patchram(hu->hdev, fw_name);
+ /* If there is no firmware (-ENOENT), discard the error and continue */
+ if (err == -ENOENT)
+ return 0;
+
+ if (hu->proto->default_speed)
+ hci_uart_set_baudrate(hu, hu->proto->default_speed);
+
+ err = btbcm_setup_post(hu->hdev);
+
+ return err;
}
static const struct h4_recv_pkt bcm_recv_pkts[] = {
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate()
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
` (2 preceding siblings ...)
2015-05-07 14:24 ` [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function Frederic Danis
@ 2015-05-07 14:24 ` Frederic Danis
2015-05-07 16:16 ` Marcel Holtmann
2015-05-07 16:11 ` [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Marcel Holtmann
4 siblings, 1 reply; 13+ messages in thread
From: Frederic Danis @ 2015-05-07 14:24 UTC (permalink / raw)
To: linux-bluetooth
Add vendor specific command to change controller device speed.
Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
---
drivers/bluetooth/hci_bcm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 0e77b9a..6b3f5d5 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -24,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/skbuff.h>
+#include <linux/tty.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -31,11 +32,71 @@
#include "btbcm.h"
#include "hci_uart.h"
+#define BCM43XX_CLOCK_48 1
+#define BCM43XX_CLOCK_24 2
+
struct bcm_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
};
+struct hci_cp_bcm_set_speed {
+ __le16 dummy;
+ __le32 speed;
+} __packed;
+
+static int bcm_set_baudrate(struct hci_uart *hu, int speed)
+{
+ struct hci_dev *hdev = hu->hdev;
+ struct sk_buff *skb;
+ struct hci_cp_bcm_set_speed param = { 0, cpu_to_le32(speed) };
+ u8 status;
+
+ if (speed > 3000000) {
+ u8 clock = BCM43XX_CLOCK_48;
+
+ BT_DBG("%s: Set Controller clock (%d)", hdev->name, clock);
+
+ skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT,
+ &status);
+ if (IS_ERR(skb)) {
+ BT_ERR("%s: failed to write update clock command (%ld)",
+ hdev->name, PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (status) {
+ BT_ERR("%s: write update clock event failed (%02x)",
+ hdev->name, status);
+ kfree_skb(skb);
+ return -bt_to_errno(status);
+ }
+
+ kfree_skb(skb);
+ }
+
+ BT_DBG("%s: Set Controller UART speed to %d bit/s", hdev->name, speed);
+
+ skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), ¶m,
+ HCI_INIT_TIMEOUT, &status);
+ if (IS_ERR(skb)) {
+ BT_ERR("%s: failed to write update baudrate command (%ld)",
+ hdev->name, PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (status) {
+ BT_ERR("%s: write update baudrate event failed (%02x)",
+ hdev->name, status);
+ kfree_skb(skb);
+ return -bt_to_errno(status);
+ }
+
+ kfree_skb(skb);
+
+ return 0;
+}
+
static int bcm_open(struct hci_uart *hu)
{
struct bcm_data *bcm;
@@ -98,6 +159,13 @@ static int bcm_setup(struct hci_uart *hu)
if (hu->proto->default_speed)
hci_uart_set_baudrate(hu, hu->proto->default_speed);
+ if (hu->proto->operational_speed) {
+ err = bcm_set_baudrate(hu, hu->proto->operational_speed);
+ if (err)
+ return err;
+ hci_uart_set_baudrate(hu, hu->proto->operational_speed);
+ }
+
err = btbcm_setup_post(hu->hdev);
return err;
@@ -150,10 +218,13 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
static const struct hci_uart_proto bcm_proto = {
.id = HCI_UART_BCM,
.name = "BCM",
+ .default_speed = 115200,
+ .operational_speed = 4000000,
.open = bcm_open,
.close = bcm_close,
.flush = bcm_flush,
.setup = bcm_setup,
+ .set_baudrate = bcm_set_baudrate,
.recv = bcm_recv,
.enqueue = bcm_enqueue,
.dequeue = bcm_dequeue,
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync()
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
` (3 preceding siblings ...)
2015-05-07 14:24 ` [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate() Frederic Danis
@ 2015-05-07 16:11 ` Marcel Holtmann
4 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-07 16:11 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
> Handle command complete event and extract error/status.
>
> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
> ---
> drivers/bluetooth/btbcm.c | 27 +++++-----
> drivers/bluetooth/btintel.c | 14 +++---
> drivers/bluetooth/btmrvl_main.c | 2 +-
> drivers/bluetooth/btusb.c | 104 +++++++++++++++++++++------------------
> drivers/bluetooth/hci_ath.c | 3 +-
> drivers/bluetooth/hci_ldisc.c | 2 +-
> include/net/bluetooth/hci_core.h | 2 +-
> net/bluetooth/hci_core.c | 17 +++++--
> 8 files changed, 95 insertions(+), 76 deletions(-)
I would prefer if we do that in two patches. One that adds the extra processing of the status and then one that converts the existing calls. It should be no problem from a git bisect point of view.
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 4bba866..5635d6d 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -38,9 +38,10 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
> {
> struct hci_rp_read_bd_addr *bda;
> struct sk_buff *skb;
> + u8 status;
>
> skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> int err = PTR_ERR(skb);
> BT_ERR("%s: BCM: Reading device address failed (%d)",
> @@ -54,14 +55,15 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
> return -EIO;
> }
>
> - bda = (struct hci_rp_read_bd_addr *)skb->data;
> - if (bda->status) {
> + if (status) {
> BT_ERR("%s: BCM: Device address result failed (%02x)",
> - hdev->name, bda->status);
> + hdev->name, status);
> kfree_skb(skb);
> - return -bt_to_errno(bda->status);
> + return -bt_to_errno(status);
> }
>
> + bda = (struct hci_rp_read_bd_addr *)skb->data;
> +
> /* The address 00:20:70:02:A0:00 indicates a BCM20702A0 controller
> * with no configured address.
> */
> @@ -82,7 +84,7 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> struct sk_buff *skb;
> int err;
>
> - skb = __hci_cmd_sync(hdev, 0xfc01, 6, bdaddr, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc01, 6, bdaddr, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> BT_ERR("%s: BCM: Change address command failed (%d)",
> @@ -112,7 +114,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> }
>
> /* Start Download */
> - skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc2e, 0, NULL, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> BT_ERR("%s: BCM: Download Minidrv command failed (%d)",
> @@ -148,7 +150,7 @@ int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> opcode = le16_to_cpu(cmd->opcode);
>
> skb = __hci_cmd_sync(hdev, opcode, cmd->plen, cmd_param,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> BT_ERR("%s: BCM: Patch command %04x failed (%d)",
> @@ -171,7 +173,8 @@ static int btbcm_reset(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
>
> - skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> int err = PTR_ERR(skb);
> BT_ERR("%s: BCM: Reset failed (%d)", hdev->name, err);
> @@ -187,7 +190,7 @@ static struct sk_buff *btbcm_read_local_version(struct hci_dev *hdev)
> struct sk_buff *skb;
>
> skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: BCM: Reading local version info failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -207,7 +210,7 @@ static struct sk_buff *btbcm_read_verbose_config(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
>
> - skb = __hci_cmd_sync(hdev, 0xfc79, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc79, 0, NULL, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: BCM: Read verbose config info failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -227,7 +230,7 @@ static struct sk_buff *btbcm_read_usb_product(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
>
> - skb = __hci_cmd_sync(hdev, 0xfc5a, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc5a, 0, NULL, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: BCM: Read USB product info failed (%ld)",
> hdev->name, PTR_ERR(skb));
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 2d43d42..1a17dd3 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -36,9 +36,10 @@ int btintel_check_bdaddr(struct hci_dev *hdev)
> {
> struct hci_rp_read_bd_addr *bda;
> struct sk_buff *skb;
> + u8 status;
>
> skb = __hci_cmd_sync(hdev, HCI_OP_READ_BD_ADDR, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> int err = PTR_ERR(skb);
> BT_ERR("%s: Reading Intel device address failed (%d)",
> @@ -52,14 +53,15 @@ int btintel_check_bdaddr(struct hci_dev *hdev)
> return -EIO;
> }
>
> - bda = (struct hci_rp_read_bd_addr *)skb->data;
> - if (bda->status) {
> + if (status) {
> BT_ERR("%s: Intel device address result failed (%02x)",
> - hdev->name, bda->status);
> + hdev->name, status);
> kfree_skb(skb);
> - return -bt_to_errno(bda->status);
> + return -bt_to_errno(status);
> }
>
> + bda = (struct hci_rp_read_bd_addr *)skb->data;
> +
> /* For some Intel based controllers, the default Bluetooth device
> * address 00:03:19:9E:8B:00 can be found. These controllers are
> * fully operational, but have the danger of duplicate addresses
> @@ -82,7 +84,7 @@ int btintel_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> struct sk_buff *skb;
> int err;
>
> - skb = __hci_cmd_sync(hdev, 0xfc31, 6, bdaddr, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc31, 6, bdaddr, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> BT_ERR("%s: Changing Intel device address failed (%d)",
> diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
> index de05deb..f9005f3 100644
> --- a/drivers/bluetooth/btmrvl_main.c
> +++ b/drivers/bluetooth/btmrvl_main.c
> @@ -593,7 +593,7 @@ static int btmrvl_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> memcpy(buf + 2, bdaddr, sizeof(bdaddr_t));
>
> skb = __hci_cmd_sync(hdev, BT_CMD_SET_BDADDR, sizeof(buf), buf,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> BT_ERR("%s: changing btmrvl device address failed (%ld)",
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index d21f3b4..77c1f8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1300,7 +1300,7 @@ static struct sk_buff *btusb_read_local_version(struct hci_dev *hdev)
> struct sk_buff *skb;
>
> skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -1324,7 +1324,7 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
>
> BT_DBG("%s", hdev->name);
>
> - skb = __hci_cmd_sync(hdev, 0xfc3b, 1, &val, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc3b, 1, &val, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb))
> BT_ERR("BCM92035 command failed (%ld)", -PTR_ERR(skb));
> else
> @@ -1403,10 +1403,10 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
> {
> struct rtl_rom_version_evt *rom_version;
> struct sk_buff *skb;
> - int ret;
> + u8 status;
>
> /* Read RTL ROM version command */
> - skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc6d, 0, NULL, HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Read ROM version failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -1421,14 +1421,13 @@ static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version)
>
> rom_version = (struct rtl_rom_version_evt *)skb->data;
> BT_INFO("%s: rom_version status=%x version=%x",
> - hdev->name, rom_version->status, rom_version->version);
> + hdev->name, status, rom_version->version);
>
> - ret = rom_version->status;
> - if (ret == 0)
> + if (status == 0)
> *version = rom_version->version;
>
> kfree_skb(skb);
> - return ret;
> + return status;
> }
>
> static int rtl8723b_parse_firmware(struct hci_dev *hdev, u16 lmp_subver,
> @@ -1587,8 +1586,8 @@ static int rtl_download_firmware(struct hci_dev *hdev,
> return -ENOMEM;
>
> for (i = 0; i < frag_num; i++) {
> - struct rtl_download_response *dl_resp;
> struct sk_buff *skb;
> + u8 status;
>
> BT_DBG("download fw (%d/%d)", i, frag_num);
>
> @@ -1601,7 +1600,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
>
> /* Send download command */
> skb = __hci_cmd_sync(hdev, 0xfc20, frag_len + 1, dl_cmd,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s: download fw command failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -1609,7 +1608,7 @@ static int rtl_download_firmware(struct hci_dev *hdev,
> goto out;
> }
>
> - if (skb->len != sizeof(*dl_resp)) {
> + if (skb->len != sizeof(struct rtl_download_response)) {
> BT_ERR("%s: download fw event length mismatch",
> hdev->name);
> kfree_skb(skb);
> @@ -1617,10 +1616,9 @@ static int rtl_download_firmware(struct hci_dev *hdev,
> goto out;
> }
>
> - dl_resp = (struct rtl_download_response *)skb->data;
> - if (dl_resp->status != 0) {
> + if (status != 0) {
> kfree_skb(skb);
> - ret = bt_to_errno(dl_resp->status);
> + ret = bt_to_errno(status);
> goto out;
> }
>
> @@ -1900,6 +1898,7 @@ static int btusb_setup_intel_patching(struct hci_dev *hdev,
> static int btusb_setup_intel(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> + u8 status;
> const struct firmware *fw;
> const u8 *fw_ptr;
> int disable_patch;
> @@ -1920,7 +1919,8 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> * number of completed commands and allow normal command processing
> * from now on.
> */
> - skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s sending initial HCI reset command failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -1934,7 +1934,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> * The returned information are hardware variant and revision plus
> * firmware variant, revision and build number.
> */
> - skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s reading Intel fw version command failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -1947,14 +1947,15 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> return -EIO;
> }
>
> - ver = (struct intel_version *)skb->data;
> - if (ver->status) {
> + if (status) {
> BT_ERR("%s Intel fw version event failed (%02x)", hdev->name,
> - ver->status);
> + status);
> kfree_skb(skb);
> - return -bt_to_errno(ver->status);
> + return -bt_to_errno(status);
> }
>
> + ver = (struct intel_version *)skb->data;
> +
> BT_INFO("%s: read Intel version: %02x%02x%02x%02x%02x%02x%02x%02x%02x",
> hdev->name, ver->hw_platform, ver->hw_variant,
> ver->hw_revision, ver->fw_variant, ver->fw_revision,
> @@ -1993,7 +1994,8 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> * Only while this mode is enabled, the driver can download the
> * firmware patch data and configuration parameters.
> */
> - skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc11, 2, mfg_enable, HCI_INIT_TIMEOUT,
> + &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s entering Intel manufacturer mode failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2001,14 +2003,12 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> return PTR_ERR(skb);
> }
>
> - if (skb->data[0]) {
> - u8 evt_status = skb->data[0];
> -
> + if (status) {
> BT_ERR("%s enable Intel manufacturer mode event failed (%02x)",
> - hdev->name, evt_status);
> + hdev->name, status);
> kfree_skb(skb);
> release_firmware(fw);
> - return -bt_to_errno(evt_status);
> + return -bt_to_errno(status);
> }
> kfree_skb(skb);
>
> @@ -2052,7 +2052,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> * with reset and activate the downloaded firmware patches.
> */
> skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_activate),
> - mfg_reset_activate, HCI_INIT_TIMEOUT);
> + mfg_reset_activate, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2069,7 +2069,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)
> exit_mfg_disable:
> /* Disable the manufacturer mode without reset */
> skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_disable), mfg_disable,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2089,7 +2089,7 @@ exit_mfg_deactivate:
> * deactivate the downloaded firmware patches.
> */
> skb = __hci_cmd_sync(hdev, 0xfc11, sizeof(mfg_reset_deactivate),
> - mfg_reset_deactivate, HCI_INIT_TIMEOUT);
> + mfg_reset_deactivate, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s exiting Intel manufacturer mode failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2284,7 +2284,7 @@ static int btusb_intel_secure_send(struct hci_dev *hdev, u8 fragment_type,
> memcpy(cmd_param + 1, param, fragment_len);
>
> skb = __hci_cmd_sync(hdev, 0xfc09, fragment_len + 1,
> - cmd_param, HCI_INIT_TIMEOUT);
> + cmd_param, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> @@ -2324,6 +2324,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> 0x00, 0x08, 0x04, 0x00 };
> struct btusb_data *data = hci_get_drvdata(hdev);
> struct sk_buff *skb;
> + u8 status;
> struct intel_version *ver;
> struct intel_boot_params *params;
> const struct firmware *fw;
> @@ -2341,7 +2342,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> * is in bootloader mode or if it already has operational firmware
> * loaded.
> */
> - skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc05, 0, NULL, HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Reading Intel version information failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2354,15 +2355,16 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> return -EILSEQ;
> }
>
> - ver = (struct intel_version *)skb->data;
> - if (ver->status) {
> + if (status) {
> BT_ERR("%s: Intel version command failure (%02x)",
> - hdev->name, ver->status);
> - err = -bt_to_errno(ver->status);
> + hdev->name, status);
> + err = -bt_to_errno(status);
> kfree_skb(skb);
> return err;
> }
>
> + ver = (struct intel_version *)skb->data;
> +
> /* The hardware platform number has a fixed value of 0x37 and
> * for now only accept this single value.
> */
> @@ -2422,7 +2424,7 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> /* Read the secure boot parameters to identify the operating
> * details of the bootloader.
> */
> - skb = __hci_cmd_sync(hdev, 0xfc0d, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc0d, 0, NULL, HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Reading Intel boot parameters failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2435,15 +2437,16 @@ static int btusb_setup_intel_new(struct hci_dev *hdev)
> return -EILSEQ;
> }
>
> - params = (struct intel_boot_params *)skb->data;
> - if (params->status) {
> + if (status) {
> BT_ERR("%s: Intel boot parameters command failure (%02x)",
> - hdev->name, params->status);
> - err = -bt_to_errno(params->status);
> + hdev->name, status);
> + err = -bt_to_errno(status);
> kfree_skb(skb);
> return err;
> }
>
> + params = (struct intel_boot_params *)skb->data;
> +
> BT_INFO("%s: Device revision is %u", hdev->name,
> le16_to_cpu(params->dev_revid));
>
> @@ -2607,7 +2610,7 @@ done:
> set_bit(BTUSB_BOOTING, &data->flags);
>
> skb = __hci_cmd_sync(hdev, 0xfc01, sizeof(reset_param), reset_param,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> @@ -2650,11 +2653,12 @@ done:
> static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
> {
> struct sk_buff *skb;
> - u8 type = 0x00;
> + u8 status, type = 0x00;
>
> BT_ERR("%s: Hardware error 0x%2.2x", hdev->name, code);
>
> - skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Reset after hardware error failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2662,7 +2666,7 @@ static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
> }
> kfree_skb(skb);
>
> - skb = __hci_cmd_sync(hdev, 0xfc22, 1, &type, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc22, 1, &type, HCI_INIT_TIMEOUT, &status);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Retrieving Intel exception info failed (%ld)",
> hdev->name, PTR_ERR(skb));
> @@ -2675,9 +2679,9 @@ static void btusb_hw_error_intel(struct hci_dev *hdev, u8 code)
> return;
> }
>
> - if (skb->data[0] != 0x00) {
> + if (status) {
> BT_ERR("%s: Exception info command failure (%02x)",
> - hdev->name, skb->data[0]);
> + hdev->name, status);
> kfree_skb(skb);
> return;
> }
> @@ -2696,7 +2700,7 @@ static int btusb_shutdown_intel(struct hci_dev *hdev)
> * down or BT radio is turned off, which takes 5 seconds to BT LED
> * goes off. This command turns off the BT LED immediately.
> */
> - skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc3f, 0, NULL, HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> BT_ERR("%s: turning off Intel device LED failed (%ld)",
> @@ -2719,7 +2723,8 @@ static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
> buf[1] = sizeof(bdaddr_t);
> memcpy(buf + 2, bdaddr, sizeof(bdaddr_t));
>
> - skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> BT_ERR("%s: changing Marvell device address failed (%ld)",
> @@ -2744,7 +2749,8 @@ static int btusb_set_bdaddr_ath3012(struct hci_dev *hdev,
> buf[3] = sizeof(bdaddr_t);
> memcpy(buf + 4, bdaddr, sizeof(bdaddr_t));
>
> - skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> BT_ERR("%s: Change address command failed (%ld)",
> diff --git a/drivers/bluetooth/hci_ath.c b/drivers/bluetooth/hci_ath.c
> index ec8fa0e..19a8f92 100644
> --- a/drivers/bluetooth/hci_ath.c
> +++ b/drivers/bluetooth/hci_ath.c
> @@ -156,7 +156,8 @@ static int ath_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> buf[3] = sizeof(bdaddr_t);
> memcpy(buf + 4, bdaddr, sizeof(bdaddr_t));
>
> - skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT);
> + skb = __hci_cmd_sync(hdev, 0xfc0b, sizeof(buf), buf, HCI_INIT_TIMEOUT,
> + NULL);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> BT_ERR("%s: Change address command failed (%d)",
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 5c9a73f..2b5d946 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -278,7 +278,7 @@ static int hci_uart_setup(struct hci_dev *hdev)
> return 0;
>
> skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + HCI_INIT_TIMEOUT, NULL);
> if (IS_ERR(skb)) {
> BT_ERR("%s: Reading local version information failed (%ld)",
> hdev->name, PTR_ERR(skb));
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index a056c2b..ea02e00 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1282,7 +1282,7 @@ int hci_register_cb(struct hci_cb *hcb);
> int hci_unregister_cb(struct hci_cb *hcb);
>
> struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
> - const void *param, u32 timeout);
> + const void *param, u32 timeout, u8 *status);
> struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
> const void *param, u8 event, u32 timeout);
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 476709b..45f5061 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -94,6 +94,7 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,
> char buf[32];
> size_t buf_size = min(count, (sizeof(buf)-1));
> bool enable;
> + u8 status;
> int err;
>
> if (!test_bit(HCI_UP, &hdev->flags))
> @@ -112,16 +113,16 @@ static ssize_t dut_mode_write(struct file *file, const char __user *user_buf,
> hci_req_lock(hdev);
> if (enable)
> skb = __hci_cmd_sync(hdev, HCI_OP_ENABLE_DUT_MODE, 0, NULL,
> - HCI_CMD_TIMEOUT);
> + HCI_CMD_TIMEOUT, &status);
> else
> skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL,
> - HCI_CMD_TIMEOUT);
> + HCI_CMD_TIMEOUT, &status);
> hci_req_unlock(hdev);
>
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> - err = -bt_to_errno(skb->data[0]);
> + err = -bt_to_errno(status);
> kfree_skb(skb);
>
> if (err < 0)
> @@ -232,9 +233,15 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
> EXPORT_SYMBOL(__hci_cmd_sync_ev);
>
> struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
> - const void *param, u32 timeout)
> + const void *param, u32 timeout, u8 *status)
Why are we doing this? We have bt_to_errno and we can just put that into ERR_PTR. No need to add a second return parameter.
I am just assuming that we are freeing the skb since when the status is not zero, then all other fields are not really valid. Or do we have commands that return an error, but contain valuable information in the skb.
> {
> - return __hci_cmd_sync_ev(hdev, opcode, plen, param, 0, timeout);
> + struct sk_buff *skb;
> +
> + skb = __hci_cmd_sync_ev(hdev, opcode, plen, param, 0, timeout);
> + if (!IS_ERR(skb) && status)
> + *status = skb->data[0];
> +
> + return skb;
> }
> EXPORT_SYMBOL(__hci_cmd_sync);
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup
2015-05-07 14:24 ` [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup Frederic Danis
@ 2015-05-07 16:13 ` Marcel Holtmann
0 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-07 16:13 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
> Add default and operational speeds.
> Change to operational speed as soon as possible.
>
> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 26 ++++++++++++++++++++++++++
> drivers/bluetooth/hci_uart.h | 4 ++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index 2b5d946..071f09c 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -265,11 +265,37 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> return 0;
> }
>
> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned long speed)
> +{
> + struct tty_struct *tty = hu->tty;
> + struct ktermios ktermios;
> +
> + ktermios = tty->termios;
> + ktermios.c_cflag &= ~CBAUD;
> + ktermios.c_cflag |= BOTHER;
> + tty_termios_encode_baud_rate(&ktermios, speed, speed);
> +
> + tty_set_termios(tty, &ktermios);
> +
> + BT_DBG("%s: New tty speed: %d", hu->hdev->name, tty->termios.c_ispeed);
> +}
> +
> static int hci_uart_setup(struct hci_dev *hdev)
> {
> struct hci_uart *hu = hci_get_drvdata(hdev);
> struct hci_rp_read_local_version *ver;
> struct sk_buff *skb;
> + int err;
> +
> + if (hu->proto->default_speed)
> + hci_uart_set_baudrate(hu, hu->proto->default_speed);
> +
> + if (hu->proto->set_baudrate && hu->proto->operational_speed) {
> + err = hu->proto->set_baudrate(hu, hu->proto->operational_speed);
> + if (err)
> + return err;
> + hci_uart_set_baudrate(hu, hu->proto->operational_speed);
> + }
>
> if (hu->proto->setup)
> return hu->proto->setup(hu);
> diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h
> index 72120a5..572ac04 100644
> --- a/drivers/bluetooth/hci_uart.h
> +++ b/drivers/bluetooth/hci_uart.h
> @@ -58,10 +58,13 @@ struct hci_uart;
> struct hci_uart_proto {
> unsigned int id;
> const char *name;
> + unsigned long default_speed;
> + unsigned long operational_speed;
I get the feeling we want init_speed and oper_speed here to keep it shorter.
> int (*open)(struct hci_uart *hu);
> int (*close)(struct hci_uart *hu);
> int (*flush)(struct hci_uart *hu);
> int (*setup)(struct hci_uart *hu);
> + int (*set_baudrate)(struct hci_uart *hu, int speed);
So what is the speed now? int or unsigned long?
> int (*recv)(struct hci_uart *hu, const void *data, int len);
> int (*enqueue)(struct hci_uart *hu, struct sk_buff *skb);
> struct sk_buff *(*dequeue)(struct hci_uart *hu);
> @@ -96,6 +99,7 @@ int hci_uart_register_proto(const struct hci_uart_proto *p);
> int hci_uart_unregister_proto(const struct hci_uart_proto *p);
> int hci_uart_tx_wakeup(struct hci_uart *hu);
> int hci_uart_init_ready(struct hci_uart *hu);
> +void hci_uart_set_baudrate(struct hci_uart *hu, unsigned long speed);
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate()
2015-05-07 14:24 ` [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate() Frederic Danis
@ 2015-05-07 16:16 ` Marcel Holtmann
0 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-07 16:16 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
> Add vendor specific command to change controller device speed.
>
> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
> ---
> drivers/bluetooth/hci_bcm.c | 71 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 0e77b9a..6b3f5d5 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -24,6 +24,7 @@
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/skbuff.h>
> +#include <linux/tty.h>
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -31,11 +32,71 @@
> #include "btbcm.h"
> #include "hci_uart.h"
>
> +#define BCM43XX_CLOCK_48 1
> +#define BCM43XX_CLOCK_24 2
> +
> struct bcm_data {
> struct sk_buff *rx_skb;
> struct sk_buff_head txq;
> };
>
> +struct hci_cp_bcm_set_speed {
> + __le16 dummy;
> + __le32 speed;
> +} __packed;
> +
> +static int bcm_set_baudrate(struct hci_uart *hu, int speed)
> +{
> + struct hci_dev *hdev = hu->hdev;
> + struct sk_buff *skb;
> + struct hci_cp_bcm_set_speed param = { 0, cpu_to_le32(speed) };
> + u8 status;
> +
> + if (speed > 3000000) {
> + u8 clock = BCM43XX_CLOCK_48;
> +
> + BT_DBG("%s: Set Controller clock (%d)", hdev->name, clock);
> +
> + skb = __hci_cmd_sync(hdev, 0xfc45, 1, &clock, HCI_INIT_TIMEOUT,
> + &status);
> + if (IS_ERR(skb)) {
> + BT_ERR("%s: failed to write update clock command (%ld)",
> + hdev->name, PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + if (status) {
> + BT_ERR("%s: write update clock event failed (%02x)",
> + hdev->name, status);
> + kfree_skb(skb);
> + return -bt_to_errno(status);
> + }
> +
> + kfree_skb(skb);
> + }
> +
> + BT_DBG("%s: Set Controller UART speed to %d bit/s", hdev->name, speed);
> +
> + skb = __hci_cmd_sync(hdev, 0xfc18, sizeof(param), ¶m,
> + HCI_INIT_TIMEOUT, &status);
> + if (IS_ERR(skb)) {
> + BT_ERR("%s: failed to write update baudrate command (%ld)",
> + hdev->name, PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + if (status) {
> + BT_ERR("%s: write update baudrate event failed (%02x)",
> + hdev->name, status);
> + kfree_skb(skb);
> + return -bt_to_errno(status);
> + }
> +
> + kfree_skb(skb);
> +
> + return 0;
> +}
> +
> static int bcm_open(struct hci_uart *hu)
> {
> struct bcm_data *bcm;
> @@ -98,6 +159,13 @@ static int bcm_setup(struct hci_uart *hu)
> if (hu->proto->default_speed)
> hci_uart_set_baudrate(hu, hu->proto->default_speed);
>
> + if (hu->proto->operational_speed) {
> + err = bcm_set_baudrate(hu, hu->proto->operational_speed);
> + if (err)
> + return err;
Why would we fail here. I think we can just gracefully continue. It just means it is slower.
> + hci_uart_set_baudrate(hu, hu->proto->operational_speed);
This one failing would be pretty bad and we might need to check that. However it could be well that this can not fail at all actually. Check the TTY layer and add a comment here explaining it either way.
> + }
> +
> err = btbcm_setup_post(hu->hdev);
>
> return err;
> @@ -150,10 +218,13 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
> static const struct hci_uart_proto bcm_proto = {
> .id = HCI_UART_BCM,
> .name = "BCM",
> + .default_speed = 115200,
> + .operational_speed = 4000000,
> .open = bcm_open,
> .close = bcm_close,
> .flush = bcm_flush,
> .setup = bcm_setup,
> + .set_baudrate = bcm_set_baudrate,
> .recv = bcm_recv,
> .enqueue = bcm_enqueue,
> .dequeue = bcm_dequeue,
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device
2015-05-07 14:24 ` [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device Frederic Danis
@ 2015-05-07 16:19 ` Marcel Holtmann
2015-05-13 14:10 ` Frederic Danis
0 siblings, 1 reply; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-07 16:19 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
you might want to add a commit message. I dislike patches without commit messages. Explain what you are doing and why.
> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
> ---
> drivers/bluetooth/btbcm.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index 5635d6d..b258454 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -33,6 +33,7 @@
> #define VERSION "0.1"
>
> #define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
> +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
So you have hardware that comes up with this default address? No matter what hardware you pick?
>
> int btbcm_check_bdaddr(struct hci_dev *hdev)
> {
> @@ -71,6 +72,10 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
> BT_INFO("%s: BCM: Using default device address (%pMR)",
> hdev->name, &bda->bdaddr);
> set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
> + } else if (!bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
> + BT_INFO("%s: BCM: Using default device address (%pMR)",
> + hdev->name, &bda->bdaddr);
> + set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
Just check for !bacmp || !bacmp instead of else if. No need to duplicate the error message and the quirk setting.
> }
>
> kfree_skb(skb);
> @@ -251,6 +256,7 @@ static const struct {
> const char *name;
> } bcm_uart_subver_table[] = {
> { 0x410e, "BCM43341B0" }, /* 002.001.014 */
> + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
> { }
Please also send userspace patches for adding this entry to the table.
> };
>
> @@ -305,6 +311,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>
> switch ((rev & 0xf000) >> 12) {
> case 0:
> + case 3:
> for (i = 0; bcm_uart_subver_table[i].name; i++) {
> if (subver == bcm_uart_subver_table[i].subver) {
> hw_name = bcm_uart_subver_table[i].name;
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function
2015-05-07 14:24 ` [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function Frederic Danis
@ 2015-05-07 16:22 ` Marcel Holtmann
2015-05-15 14:11 ` Frederic Danis
0 siblings, 1 reply; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-07 16:22 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
> Split btbcm_setup_patchram in 3 functions : btbcm_setup_patchram,
> btbcm_patchram (already existing) and btbcm_setup_post.
> For UART controllers this will allow to reset speed to default one
> after firmware loading, which will have reseted controller speed to
> default one.
>
> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
> ---
> drivers/bluetooth/btbcm.c | 29 +++++++++++++++++------------
> drivers/bluetooth/btbcm.h | 11 +++++++++--
> drivers/bluetooth/btusb.c | 23 ++++++++++++++++++++++-
> drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
> 4 files changed, 66 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
> index b258454..f2efbbd 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -277,9 +277,8 @@ static const struct {
> { }
> };
>
> -int btbcm_setup_patchram(struct hci_dev *hdev)
> +int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len)
what is this len for? The fw_name is not a return value. I think the functions for doing the patchram should just get a firmware and then just do patchram.
> {
> - char fw_name[64];
> u16 subver, rev, pid, vid;
> const char *hw_name = NULL;
> struct sk_buff *skb;
> @@ -319,8 +318,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> }
> }
>
> - snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
> - hw_name ? : "BCM");
> + snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
> break;
> case 1:
> case 2:
> @@ -340,7 +338,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> }
> }
>
> - snprintf(fw_name, sizeof(fw_name), "brcm/%s-%4.4x-%4.4x.hcd",
> + snprintf(fw_name, len, "brcm/%s-%4.4x-%4.4x.hcd",
> hw_name ? : "BCM", vid, pid);
> break;
> default:
> @@ -351,9 +349,16 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> hw_name ? : "BCM", (subver & 0x7000) >> 13,
> (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
>
> - err = btbcm_patchram(hdev, fw_name);
> - if (err == -ENOENT)
> - return 0;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(btbcm_setup_patchram);
> +
> +int btbcm_setup_post(struct hci_dev *hdev)
> +{
> + struct sk_buff *skb;
> + struct hci_rp_read_local_version *ver;
> + u16 subver, rev;
> + int err;
>
> /* Reset */
> err = btbcm_reset(hdev);
> @@ -370,9 +375,9 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
> subver = le16_to_cpu(ver->lmp_subver);
> kfree_skb(skb);
>
> - BT_INFO("%s: %s (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
> - hw_name ? : "BCM", (subver & 0x7000) >> 13,
> - (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
> + BT_INFO("%s: BCM (%3.3u.%3.3u.%3.3u) build %4.4u", hdev->name,
> + (subver & 0x7000) >> 13, (subver & 0x1f00) >> 8,
> + (subver & 0x00ff), rev & 0x0fff);
>
> btbcm_check_bdaddr(hdev);
>
> @@ -380,7 +385,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(btbcm_setup_patchram);
> +EXPORT_SYMBOL_GPL(btbcm_setup_post);
>
> int btbcm_setup_apple(struct hci_dev *hdev)
> {
> diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
> index eb6ab5f..73d92a2 100644
> --- a/drivers/bluetooth/btbcm.h
> +++ b/drivers/bluetooth/btbcm.h
> @@ -27,7 +27,8 @@ int btbcm_check_bdaddr(struct hci_dev *hdev);
> int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> int btbcm_patchram(struct hci_dev *hdev, const char *firmware);
>
> -int btbcm_setup_patchram(struct hci_dev *hdev);
> +int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len);
> +int btbcm_setup_post(struct hci_dev *hdev);
> int btbcm_setup_apple(struct hci_dev *hdev);
>
> #else
> @@ -47,7 +48,13 @@ static inline int btbcm_patchram(struct hci_dev *hdev, const char *firmware)
> return -EOPNOTSUPP;
> }
>
> -static inline int btbcm_setup_patchram(struct hci_dev *hdev)
> +static inline int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name,
> + size_t len)
> +{
> + return 0;
> +}
> +
> +static int btbcm_setup_post(struct hci_dev *hdev)
> {
> return 0;
> }
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 77c1f8c..15225d9 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1333,6 +1333,27 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev)
> return 0;
> }
>
> +#ifdef CONFIG_BT_HCIBTUSB_BCM
> +static int btusb_setup_bcm(struct hci_dev *hdev)
> +{
> + char fw_name[64];
> + int err;
> +
> + err = btbcm_setup_patchram(hdev, fw_name, sizeof(fw_name));
> + if (err)
> + return err;
> +
> + err = btbcm_patchram(hdev, fw_name);
> + /* If there is no firmware (-ENOENT), discard the error and continue */
> + if (err == -ENOENT)
> + return 0;
> +
> + err = btbcm_setup_post(hdev);
> +
> + return err;
> +}
> +#endif
> +
> static int btusb_setup_csr(struct hci_dev *hdev)
> {
> struct hci_rp_read_local_version *rp;
> @@ -3132,7 +3153,7 @@ static int btusb_probe(struct usb_interface *intf,
>
> #ifdef CONFIG_BT_HCIBTUSB_BCM
> if (id->driver_info & BTUSB_BCM_PATCHRAM) {
> - hdev->setup = btbcm_setup_patchram;
> + hdev->setup = btusb_setup_bcm;
> hdev->set_bdaddr = btbcm_set_bdaddr;
> }
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index 1ec0b4a..0e77b9a 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -79,11 +79,28 @@ static int bcm_flush(struct hci_uart *hu)
>
> static int bcm_setup(struct hci_uart *hu)
> {
> + char fw_name[64];
> + int err;
> +
> BT_DBG("hu %p", hu);
>
> hu->hdev->set_bdaddr = btbcm_set_bdaddr;
>
> - return btbcm_setup_patchram(hu->hdev);
> + err = btbcm_setup_patchram(hu->hdev, fw_name, sizeof(fw_name));
> + if (err)
> + return err;
> +
> + err = btbcm_patchram(hu->hdev, fw_name);
> + /* If there is no firmware (-ENOENT), discard the error and continue */
> + if (err == -ENOENT)
> + return 0;
> +
> + if (hu->proto->default_speed)
> + hci_uart_set_baudrate(hu, hu->proto->default_speed);
> +
> + err = btbcm_setup_post(hu->hdev);
> +
> + return err;
> }
>
> static const struct h4_recv_pkt bcm_recv_pkts[] = {
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device
2015-05-07 16:19 ` Marcel Holtmann
@ 2015-05-13 14:10 ` Frederic Danis
0 siblings, 0 replies; 13+ messages in thread
From: Frederic Danis @ 2015-05-13 14:10 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hello Marcel,
On 07/05/2015 18:19, Marcel Holtmann wrote:
> Hi Fred,
>
> you might want to add a commit message. I dislike patches without commit messages. Explain what you are doing and why.
OK
>> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
>> ---
>> drivers/bluetooth/btbcm.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index 5635d6d..b258454 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -33,6 +33,7 @@
>> #define VERSION "0.1"
>>
>> #define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
>> +#define BDADDR_BCM4324B3 (&(bdaddr_t) {{0x00, 0x00, 0x00, 0xb3, 0x24, 0x43}})
>
> So you have hardware that comes up with this default address? No matter what hardware you pick?
On a t100, before firmware upload I get a random address, changing at
each reboot. After firmware upload, I always get this address which
seems to be hard-coded in firmware file.
>>
>> int btbcm_check_bdaddr(struct hci_dev *hdev)
>> {
>> @@ -71,6 +72,10 @@ int btbcm_check_bdaddr(struct hci_dev *hdev)
>> BT_INFO("%s: BCM: Using default device address (%pMR)",
>> hdev->name, &bda->bdaddr);
>> set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
>> + } else if (!bacmp(&bda->bdaddr, BDADDR_BCM4324B3)) {
>
>> + BT_INFO("%s: BCM: Using default device address (%pMR)",
>> + hdev->name, &bda->bdaddr);
>> + set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
>
> Just check for !bacmp || !bacmp instead of else if. No need to duplicate the error message and the quirk setting.
OK
>> }
>>
>> kfree_skb(skb);
>> @@ -251,6 +256,7 @@ static const struct {
>> const char *name;
>> } bcm_uart_subver_table[] = {
>> { 0x410e, "BCM43341B0" }, /* 002.001.014 */
>> + { 0x4406, "BCM4324B3" }, /* 002.004.006 */
>> { }
>
> Please also send userspace patches for adding this entry to the table.
OK, I will send a patch for monitor/packet.c.
>> };
>>
>> @@ -305,6 +311,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
>>
>> switch ((rev & 0xf000) >> 12) {
>> case 0:
>> + case 3:
>> for (i = 0; bcm_uart_subver_table[i].name; i++) {
>> if (subver == bcm_uart_subver_table[i].subver) {
>> hw_name = bcm_uart_subver_table[i].name;
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function
2015-05-07 16:22 ` Marcel Holtmann
@ 2015-05-15 14:11 ` Frederic Danis
2015-05-15 17:17 ` Marcel Holtmann
0 siblings, 1 reply; 13+ messages in thread
From: Frederic Danis @ 2015-05-15 14:11 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hello Marcel,
On 07/05/2015 18:22, Marcel Holtmann wrote:
> Hi Fred,
>
>> Split btbcm_setup_patchram in 3 functions : btbcm_setup_patchram,
>> btbcm_patchram (already existing) and btbcm_setup_post.
>> For UART controllers this will allow to reset speed to default one
>> after firmware loading, which will have reseted controller speed to
>> default one.
>>
>> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
>> ---
>> drivers/bluetooth/btbcm.c | 29 +++++++++++++++++------------
>> drivers/bluetooth/btbcm.h | 11 +++++++++--
>> drivers/bluetooth/btusb.c | 23 ++++++++++++++++++++++-
>> drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
>> 4 files changed, 66 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>> index b258454..f2efbbd 100644
>> --- a/drivers/bluetooth/btbcm.c
>> +++ b/drivers/bluetooth/btbcm.c
>> @@ -277,9 +277,8 @@ static const struct {
>> { }
>> };
>>
>> -int btbcm_setup_patchram(struct hci_dev *hdev)
>> +int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len)
>
> what is this len for? The fw_name is not a return value. I think the functions for doing the patchram should just get a firmware and then just do patchram.
I split btbcm_setup_patchram() in 3 functions:
- btbcm_setup_patchram() which resets the controller and returns the
firmware name.
- btbtcm_patchram() (already existing and public), which takes the
firmware name as parameter, requests the firmware and loads it to the
controller.
- btbcm_setup_post() which resets the controller, reads local version
and checks if the controller address is a default one or not.
I split it between btbcm_patchram() and btbcm_setup_post() as firmware
loading may reset the controller UART speed and it needs to set host
UART speed back to init speed.
I also split it between btbcm_setup_patchram() and btbtcm_patchram()
because error is not managed the same way for both of them.
When btbcm_setup_patchram() returns an error, this error is forwarded to
caller.
When btbtcm_patchram() returns an error, if the error is -ENOENT then
this error is discarded and 0 is returned to caller. Otherwise, the
error is also discarded but there is a need to call btbcm_setup_post()
to reset controller.
Regards
Fred
--
Frederic Danis Open Source Technology Center
frederic.danis@intel.com Intel Corporation
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function
2015-05-15 14:11 ` Frederic Danis
@ 2015-05-15 17:17 ` Marcel Holtmann
0 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2015-05-15 17:17 UTC (permalink / raw)
To: Frederic Danis; +Cc: linux-bluetooth
Hi Fred,
>>> Split btbcm_setup_patchram in 3 functions : btbcm_setup_patchram,
>>> btbcm_patchram (already existing) and btbcm_setup_post.
>>> For UART controllers this will allow to reset speed to default one
>>> after firmware loading, which will have reseted controller speed to
>>> default one.
>>>
>>> Signed-off-by: Frederic Danis <frederic.danis@linux.intel.com>
>>> ---
>>> drivers/bluetooth/btbcm.c | 29 +++++++++++++++++------------
>>> drivers/bluetooth/btbcm.h | 11 +++++++++--
>>> drivers/bluetooth/btusb.c | 23 ++++++++++++++++++++++-
>>> drivers/bluetooth/hci_bcm.c | 19 ++++++++++++++++++-
>>> 4 files changed, 66 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
>>> index b258454..f2efbbd 100644
>>> --- a/drivers/bluetooth/btbcm.c
>>> +++ b/drivers/bluetooth/btbcm.c
>>> @@ -277,9 +277,8 @@ static const struct {
>>> { }
>>> };
>>>
>>> -int btbcm_setup_patchram(struct hci_dev *hdev)
>>> +int btbcm_setup_patchram(struct hci_dev *hdev, char *fw_name, size_t len)
>>
>> what is this len for? The fw_name is not a return value. I think the functions for doing the patchram should just get a firmware and then just do patchram.
>
> I split btbcm_setup_patchram() in 3 functions:
> - btbcm_setup_patchram() which resets the controller and returns the firmware name.
> - btbtcm_patchram() (already existing and public), which takes the firmware name as parameter, requests the firmware and loads it to the controller.
> - btbcm_setup_post() which resets the controller, reads local version and checks if the controller address is a default one or not.
>
> I split it between btbcm_patchram() and btbcm_setup_post() as firmware loading may reset the controller UART speed and it needs to set host UART speed back to init speed.
>
> I also split it between btbcm_setup_patchram() and btbtcm_patchram() because error is not managed the same way for both of them.
> When btbcm_setup_patchram() returns an error, this error is forwarded to caller.
> When btbtcm_patchram() returns an error, if the error is -ENOENT then this error is discarded and 0 is returned to caller. Otherwise, the error is also discarded but there is a need to call btbcm_setup_post() to reset controller.
my proposal to make this a bit smoother and have less changes is to keep the btbcm_setup_patchram as is. Lets keep using that as is for USB and introduce helper functions to be used by the UART side of things.
To get things moving, lets assume that firmware filename comes in from somewhere. Maybe coded in DTS or ACPI or something else. In the long run, we need to have a “manifest” file that lets us map original Broadcom firmware file names to platforms. For USB that will become simple since it is USB VID/PID map. For ACPI we can use the ACPI ID mapping. So my thinking is to use a simple table mapping modalias to filename and that be it. However I would put that a little bit as a problem to solve later and bring the basic UART and speed handling into place.
This means that UART setup routine should be doing this:
- UART call to switch to default speed
-> switch to default speed
- btbcm_initialize()
- UART call to switch operational speed
-> trigger vendor command
-> switch speed
- btbcm_patchram()
- UART call to switch to default speed
-> switch to default speed
- UART call to switch to operational speed
-> trigger vendor command
-> switch speed
- btbcm_finalize()
Maybe default speed and operational speed changes should always be combined. We have to see there.
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-05-15 17:17 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-07 14:24 [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Frederic Danis
2015-05-07 14:24 ` [PATCH v2 2/5] Bluetooth: btbcm: Add BCM4324B3 UART device Frederic Danis
2015-05-07 16:19 ` Marcel Holtmann
2015-05-13 14:10 ` Frederic Danis
2015-05-07 14:24 ` [PATCH v2 3/5] Bluetooth: hci_uart: Support operational speed during setup Frederic Danis
2015-05-07 16:13 ` Marcel Holtmann
2015-05-07 14:24 ` [PATCH v2 4/5] Bluetooth: btbcm: Split setup() function Frederic Danis
2015-05-07 16:22 ` Marcel Holtmann
2015-05-15 14:11 ` Frederic Danis
2015-05-15 17:17 ` Marcel Holtmann
2015-05-07 14:24 ` [PATCH v2 5/5] Bluetooth: btbcm: Add bcm_set_baudrate() Frederic Danis
2015-05-07 16:16 ` Marcel Holtmann
2015-05-07 16:11 ` [PATCH v2 1/5] Bluetooth: hci_core: return cmd status in __hci_cmd_sync() Marcel Holtmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.