linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths
@ 2018-04-20 12:44 Hans de Goede
  2018-04-20 12:44 ` [PATCH 2/6] Bluetooth: btbcm: Factor out common code to determine subversion Hans de Goede
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

btbcm_setup_patchram() was using the upper nibble of the revision code to
determine if we are dealing with an uart or USB connected bcm-bt device,
but just as btbcm_initialize() has started accepting 1 and 2 as uart
connected devices, I've now encountered an USB connected device (0a5c:216c)
which has 0 in the upper nibble. So it seems that the upper nibble is not
really a reliable indicator of the bus type.

Instead check hdev->bus which does give us a reliable indication. This
fixes the patchram code trying to load the patchram by the fallback BCM.hcd
filename, now it correctly requests BCM43142A0-0a5c-216c.hcd.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 6659f113042c..f44ddc0a6b25 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -465,9 +465,11 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 	if (err)
 		return err;
 
-	switch ((rev & 0xf000) >> 12) {
-	case 0:
-	case 3:
+	/* Upper nibble of rev should be between 0 and 3? */
+	if (((rev & 0xf000) >> 12) > 3)
+		return 0;
+
+	if (hdev->bus != HCI_USB) {
 		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;
@@ -477,9 +479,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
 			 hw_name ? : "BCM");
-		break;
-	case 1:
-	case 2:
+	} else {
 		/* Read USB Product Info */
 		skb = btbcm_read_usb_product(hdev);
 		if (IS_ERR(skb))
@@ -498,9 +498,6 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 		snprintf(fw_name, sizeof(fw_name), "brcm/%s-%4.4x-%4.4x.hcd",
 			 hw_name ? : "BCM", vid, pid);
-		break;
-	default:
-		return 0;
 	}
 
 	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] Bluetooth: btbcm: Factor out common code to determine subversion
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
@ 2018-04-20 12:44 ` Hans de Goede
  2018-04-20 12:44 ` [PATCH 3/6] Bluetooth: btbcm: Make btbcm_initialize() also work for USB connected devices Hans de Goede
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

We are using the same loop in both the UART and USB bus cases, refactor
things a bit to share the loop.

This is mostly meant to improve readability.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c | 40 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index f44ddc0a6b25..e3393bceb1a0 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -315,10 +315,12 @@ static int btbcm_read_info(struct hci_dev *hdev)
 	return 0;
 }
 
-static const struct {
+struct bcm_subver_table {
 	u16 subver;
 	const char *name;
-} bcm_uart_subver_table[] = {
+};
+
+static const struct bcm_subver_table bcm_uart_subver_table[] = {
 	{ 0x4103, "BCM4330B1"	},	/* 002.001.003 */
 	{ 0x410e, "BCM43341B0"	},	/* 002.001.014 */
 	{ 0x4406, "BCM4324B3"	},	/* 002.004.006 */
@@ -418,10 +420,7 @@ int btbcm_finalize(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(btbcm_finalize);
 
-static const struct {
-	u16 subver;
-	const char *name;
-} bcm_usb_subver_table[] = {
+static const struct bcm_subver_table bcm_usb_subver_table[] = {
 	{ 0x210b, "BCM43142A0"	},	/* 001.001.011 */
 	{ 0x2112, "BCM4314A0"	},	/* 001.001.018 */
 	{ 0x2118, "BCM20702A0"	},	/* 001.001.024 */
@@ -443,6 +442,7 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 	const char *hw_name = NULL;
 	struct sk_buff *skb;
 	struct hci_rp_read_local_version *ver;
+	const struct bcm_subver_table *bcm_subver_table;
 	int i, err;
 
 	/* Reset */
@@ -469,17 +469,17 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 	if (((rev & 0xf000) >> 12) > 3)
 		return 0;
 
-	if (hdev->bus != HCI_USB) {
-		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;
-				break;
-			}
+	bcm_subver_table = (hdev->bus == HCI_USB) ? bcm_usb_subver_table :
+						    bcm_uart_subver_table;
+
+	for (i = 0; bcm_subver_table[i].name; i++) {
+		if (subver == bcm_subver_table[i].subver) {
+			hw_name = bcm_subver_table[i].name;
+			break;
 		}
+	}
 
-		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
-			 hw_name ? : "BCM");
-	} else {
+	if (hdev->bus == HCI_USB) {
 		/* Read USB Product Info */
 		skb = btbcm_read_usb_product(hdev);
 		if (IS_ERR(skb))
@@ -489,15 +489,11 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 		pid = get_unaligned_le16(skb->data + 3);
 		kfree_skb(skb);
 
-		for (i = 0; bcm_usb_subver_table[i].name; i++) {
-			if (subver == bcm_usb_subver_table[i].subver) {
-				hw_name = bcm_usb_subver_table[i].name;
-				break;
-			}
-		}
-
 		snprintf(fw_name, sizeof(fw_name), "brcm/%s-%4.4x-%4.4x.hcd",
 			 hw_name ? : "BCM", vid, pid);
+	} else {
+		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
+			 hw_name ? : "BCM");
 	}
 
 	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] Bluetooth: btbcm: Make btbcm_initialize() also work for USB connected devices
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
  2018-04-20 12:44 ` [PATCH 2/6] Bluetooth: btbcm: Factor out common code to determine subversion Hans de Goede
@ 2018-04-20 12:44 ` Hans de Goede
  2018-04-20 12:44 ` [PATCH 4/6] Bluetooth: btbcm: Allow using btbcm_initialize() for reinit Hans de Goede
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

Make btbcm_initialize() also work for USB connected device,
btbcm_initialize() and btbcm_setup_patchram() are quite similar,
this is a preparation patch for making btbcm_setup_patchram() use
btbcm_initialize() to remove the code duplication.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c | 72 +++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 29 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index e3393bceb1a0..dd243afe32de 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -332,12 +332,27 @@ static const struct bcm_subver_table bcm_uart_subver_table[] = {
 	{ }
 };
 
+static const struct bcm_subver_table bcm_usb_subver_table[] = {
+	{ 0x210b, "BCM43142A0"	},	/* 001.001.011 */
+	{ 0x2112, "BCM4314A0"	},	/* 001.001.018 */
+	{ 0x2118, "BCM20702A0"	},	/* 001.001.024 */
+	{ 0x2126, "BCM4335A0"	},	/* 001.001.038 */
+	{ 0x220e, "BCM20702A1"	},	/* 001.002.014 */
+	{ 0x230f, "BCM4354A2"	},	/* 001.003.015 */
+	{ 0x4106, "BCM4335B0"	},	/* 002.001.006 */
+	{ 0x410e, "BCM20702B0"	},	/* 002.001.014 */
+	{ 0x6109, "BCM4335C0"	},	/* 003.001.009 */
+	{ 0x610c, "BCM4354"	},	/* 003.001.012 */
+	{ }
+};
+
 int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
 {
-	u16 subver, rev;
+	u16 subver, rev, pid, vid;
 	const char *hw_name = NULL;
 	struct sk_buff *skb;
 	struct hci_rp_read_local_version *ver;
+	const struct bcm_subver_table *bcm_subver_table;
 	int i, err;
 
 	/* Reset */
@@ -360,22 +375,35 @@ int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
 	if (err)
 		return err;
 
-	switch ((rev & 0xf000) >> 12) {
-	case 0:
-	case 1:
-	case 2:
-	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;
-				break;
-			}
+	/* Upper nibble of rev should be between 0 and 3? */
+	if (((rev & 0xf000) >> 12) > 3)
+		return 0;
+
+	bcm_subver_table = (hdev->bus == HCI_USB) ? bcm_usb_subver_table :
+						    bcm_uart_subver_table;
+
+	for (i = 0; bcm_subver_table[i].name; i++) {
+		if (subver == bcm_subver_table[i].subver) {
+			hw_name = bcm_subver_table[i].name;
+			break;
 		}
+	}
 
-		snprintf(fw_name, len, "brcm/%s.hcd", hw_name ? : "BCM");
-		break;
-	default:
-		return 0;
+	if (hdev->bus == HCI_USB) {
+		/* Read USB Product Info */
+		skb = btbcm_read_usb_product(hdev);
+		if (IS_ERR(skb))
+			return PTR_ERR(skb);
+
+		vid = get_unaligned_le16(skb->data + 1);
+		pid = get_unaligned_le16(skb->data + 3);
+		kfree_skb(skb);
+
+		snprintf(fw_name, len, "brcm/%s-%4.4x-%4.4x.hcd",
+			 hw_name ? : "BCM", vid, pid);
+	} else {
+		snprintf(fw_name, len, "brcm/%s.hcd",
+			 hw_name ? : "BCM");
 	}
 
 	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
@@ -420,20 +448,6 @@ int btbcm_finalize(struct hci_dev *hdev)
 }
 EXPORT_SYMBOL_GPL(btbcm_finalize);
 
-static const struct bcm_subver_table bcm_usb_subver_table[] = {
-	{ 0x210b, "BCM43142A0"	},	/* 001.001.011 */
-	{ 0x2112, "BCM4314A0"	},	/* 001.001.018 */
-	{ 0x2118, "BCM20702A0"	},	/* 001.001.024 */
-	{ 0x2126, "BCM4335A0"	},	/* 001.001.038 */
-	{ 0x220e, "BCM20702A1"	},	/* 001.002.014 */
-	{ 0x230f, "BCM4354A2"	},	/* 001.003.015 */
-	{ 0x4106, "BCM4335B0"	},	/* 002.001.006 */
-	{ 0x410e, "BCM20702B0"	},	/* 002.001.014 */
-	{ 0x6109, "BCM4335C0"	},	/* 003.001.009 */
-	{ 0x610c, "BCM4354"	},	/* 003.001.012 */
-	{ }
-};
-
 int btbcm_setup_patchram(struct hci_dev *hdev)
 {
 	char fw_name[64];
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] Bluetooth: btbcm: Allow using btbcm_initialize() for reinit
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
  2018-04-20 12:44 ` [PATCH 2/6] Bluetooth: btbcm: Factor out common code to determine subversion Hans de Goede
  2018-04-20 12:44 ` [PATCH 3/6] Bluetooth: btbcm: Make btbcm_initialize() also work for USB connected devices Hans de Goede
@ 2018-04-20 12:44 ` Hans de Goede
  2018-04-20 12:44 ` [PATCH 5/6] Bluetooth: btbcm: Remove duplicate code from btbcm_setup_patchram() Hans de Goede
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

btbcm_finalize() does a re-init of the controller, which is almost the
same as the initial init. Modify btbcm_initialize() so that it can be
used for this re-init and modify btbcm_finalize() to use it.

As an added bonus this also makes the dev_info from btbcm_finalize()
use the proper hw_name instead of always printing "BCM".

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c   | 33 ++++++++++-----------------------
 drivers/bluetooth/btbcm.h   |  5 +++--
 drivers/bluetooth/hci_bcm.c |  2 +-
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index dd243afe32de..fdcbeb489a2e 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -346,7 +346,8 @@ static const struct bcm_subver_table bcm_usb_subver_table[] = {
 	{ }
 };
 
-int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len,
+		     bool reinit)
 {
 	u16 subver, rev, pid, vid;
 	const char *hw_name = NULL;
@@ -371,9 +372,11 @@ int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len)
 	kfree_skb(skb);
 
 	/* Read controller information */
-	err = btbcm_read_info(hdev);
-	if (err)
-		return err;
+	if (!reinit) {
+		err = btbcm_read_info(hdev);
+		if (err)
+			return err;
+	}
 
 	/* Upper nibble of rev should be between 0 and 3? */
 	if (((rev & 0xf000) >> 12) > 3)
@@ -416,30 +419,14 @@ EXPORT_SYMBOL_GPL(btbcm_initialize);
 
 int btbcm_finalize(struct hci_dev *hdev)
 {
-	struct sk_buff *skb;
-	struct hci_rp_read_local_version *ver;
-	u16 subver, rev;
+	char fw_name[64];
 	int err;
 
-	/* Reset */
-	err = btbcm_reset(hdev);
+	/* Re-initialize */
+	err = btbcm_initialize(hdev, fw_name, sizeof(fw_name), true);
 	if (err)
 		return err;
 
-	/* Read Local Version Info */
-	skb = btbcm_read_local_version(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	ver = (struct hci_rp_read_local_version *)skb->data;
-	rev = le16_to_cpu(ver->hci_rev);
-	subver = le16_to_cpu(ver->lmp_subver);
-	kfree_skb(skb);
-
-	bt_dev_info(hdev, "BCM (%3.3u.%3.3u.%3.3u) build %4.4u",
-		    (subver & 0xe000) >> 13, (subver & 0x1f00) >> 8,
-		    (subver & 0x00ff), rev & 0x0fff);
-
 	btbcm_check_bdaddr(hdev);
 
 	set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
index cfe6ad4cc621..5346515c880c 100644
--- a/drivers/bluetooth/btbcm.h
+++ b/drivers/bluetooth/btbcm.h
@@ -73,7 +73,8 @@ int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw);
 int btbcm_setup_patchram(struct hci_dev *hdev);
 int btbcm_setup_apple(struct hci_dev *hdev);
 
-int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len);
+int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len,
+		     bool reinit);
 int btbcm_finalize(struct hci_dev *hdev);
 
 #else
@@ -104,7 +105,7 @@ static inline int btbcm_setup_apple(struct hci_dev *hdev)
 }
 
 static inline int btbcm_initialize(struct hci_dev *hdev, char *fw_name,
-				   size_t len)
+				   size_t len, bool reinit)
 {
 	return 0;
 }
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 04c4b93b4c6f..f06f0f1132fb 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -501,7 +501,7 @@ static int bcm_setup(struct hci_uart *hu)
 	hu->hdev->set_diag = bcm_set_diag;
 	hu->hdev->set_bdaddr = btbcm_set_bdaddr;
 
-	err = btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name));
+	err = btbcm_initialize(hu->hdev, fw_name, sizeof(fw_name), false);
 	if (err)
 		return err;
 
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] Bluetooth: btbcm: Remove duplicate code from btbcm_setup_patchram()
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
                   ` (2 preceding siblings ...)
  2018-04-20 12:44 ` [PATCH 4/6] Bluetooth: btbcm: Allow using btbcm_initialize() for reinit Hans de Goede
@ 2018-04-20 12:44 ` Hans de Goede
  2018-04-20 12:44 ` [PATCH 6/6] Bluetooth: btbcm: btbcm_initialize(): Initialize hw_name to "BCM" Hans de Goede
  2018-04-23 17:58 ` [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Marcel Holtmann
  5 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

btbcm_setup_patchram() starts with initializing the controller (and
getting the firmware filename) and then after loading the firmware,
does a re-init. This almost entirely duplicates the code in
btbcm_initialize(), use that function instead.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c | 78 +++------------------------------------
 1 file changed, 5 insertions(+), 73 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index fdcbeb489a2e..d44aa6ff1ed5 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -439,68 +439,14 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 {
 	char fw_name[64];
 	const struct firmware *fw;
-	u16 subver, rev, pid, vid;
-	const char *hw_name = NULL;
 	struct sk_buff *skb;
-	struct hci_rp_read_local_version *ver;
-	const struct bcm_subver_table *bcm_subver_table;
-	int i, err;
-
-	/* Reset */
-	err = btbcm_reset(hdev);
-	if (err)
-		return err;
-
-	/* Read Local Version Info */
-	skb = btbcm_read_local_version(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	ver = (struct hci_rp_read_local_version *)skb->data;
-	rev = le16_to_cpu(ver->hci_rev);
-	subver = le16_to_cpu(ver->lmp_subver);
-	kfree_skb(skb);
+	int err;
 
-	/* Read controller information */
-	err = btbcm_read_info(hdev);
+	/* Initialize */
+	err = btbcm_initialize(hdev, fw_name, sizeof(fw_name), false);
 	if (err)
 		return err;
 
-	/* Upper nibble of rev should be between 0 and 3? */
-	if (((rev & 0xf000) >> 12) > 3)
-		return 0;
-
-	bcm_subver_table = (hdev->bus == HCI_USB) ? bcm_usb_subver_table :
-						    bcm_uart_subver_table;
-
-	for (i = 0; bcm_subver_table[i].name; i++) {
-		if (subver == bcm_subver_table[i].subver) {
-			hw_name = bcm_subver_table[i].name;
-			break;
-		}
-	}
-
-	if (hdev->bus == HCI_USB) {
-		/* Read USB Product Info */
-		skb = btbcm_read_usb_product(hdev);
-		if (IS_ERR(skb))
-			return PTR_ERR(skb);
-
-		vid = get_unaligned_le16(skb->data + 1);
-		pid = get_unaligned_le16(skb->data + 3);
-		kfree_skb(skb);
-
-		snprintf(fw_name, sizeof(fw_name), "brcm/%s-%4.4x-%4.4x.hcd",
-			 hw_name ? : "BCM", vid, pid);
-	} else {
-		snprintf(fw_name, sizeof(fw_name), "brcm/%s.hcd",
-			 hw_name ? : "BCM");
-	}
-
-	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
-		    hw_name ? : "BCM", (subver & 0xe000) >> 13,
-		    (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
-
 	err = request_firmware(&fw, fw_name, &hdev->dev);
 	if (err < 0) {
 		bt_dev_info(hdev, "BCM: Patch %s not found", fw_name);
@@ -511,25 +457,11 @@ int btbcm_setup_patchram(struct hci_dev *hdev)
 
 	release_firmware(fw);
 
-	/* Reset */
-	err = btbcm_reset(hdev);
+	/* Re-initialize */
+	err = btbcm_initialize(hdev, fw_name, sizeof(fw_name), true);
 	if (err)
 		return err;
 
-	/* Read Local Version Info */
-	skb = btbcm_read_local_version(hdev);
-	if (IS_ERR(skb))
-		return PTR_ERR(skb);
-
-	ver = (struct hci_rp_read_local_version *)skb->data;
-	rev = le16_to_cpu(ver->hci_rev);
-	subver = le16_to_cpu(ver->lmp_subver);
-	kfree_skb(skb);
-
-	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
-		    hw_name ? : "BCM", (subver & 0xe000) >> 13,
-		    (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
-
 	/* Read Local Name */
 	skb = btbcm_read_local_name(hdev);
 	if (IS_ERR(skb))
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] Bluetooth: btbcm: btbcm_initialize(): Initialize hw_name to "BCM"
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
                   ` (3 preceding siblings ...)
  2018-04-20 12:44 ` [PATCH 5/6] Bluetooth: btbcm: Remove duplicate code from btbcm_setup_patchram() Hans de Goede
@ 2018-04-20 12:44 ` Hans de Goede
  2018-04-23 17:58 ` [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Marcel Holtmann
  5 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-04-20 12:44 UTC (permalink / raw)
  To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
  Cc: Hans de Goede, Frédéric Danis, Lukas Wunner,
	linux-bluetooth, linux-serial, linux-acpi

Initialize hw_name to "BCM", this avoids the need for a number of NULL
checks on hw_name later.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/bluetooth/btbcm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index d44aa6ff1ed5..99cde1f9467d 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -350,7 +350,7 @@ int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len,
 		     bool reinit)
 {
 	u16 subver, rev, pid, vid;
-	const char *hw_name = NULL;
+	const char *hw_name = "BCM";
 	struct sk_buff *skb;
 	struct hci_rp_read_local_version *ver;
 	const struct bcm_subver_table *bcm_subver_table;
@@ -403,14 +403,13 @@ int btbcm_initialize(struct hci_dev *hdev, char *fw_name, size_t len,
 		kfree_skb(skb);
 
 		snprintf(fw_name, len, "brcm/%s-%4.4x-%4.4x.hcd",
-			 hw_name ? : "BCM", vid, pid);
+			 hw_name, vid, pid);
 	} else {
-		snprintf(fw_name, len, "brcm/%s.hcd",
-			 hw_name ? : "BCM");
+		snprintf(fw_name, len, "brcm/%s.hcd", hw_name);
 	}
 
 	bt_dev_info(hdev, "%s (%3.3u.%3.3u.%3.3u) build %4.4u",
-		    hw_name ? : "BCM", (subver & 0xe000) >> 13,
+		    hw_name, (subver & 0xe000) >> 13,
 		    (subver & 0x1f00) >> 8, (subver & 0x00ff), rev & 0x0fff);
 
 	return 0;
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths
  2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
                   ` (4 preceding siblings ...)
  2018-04-20 12:44 ` [PATCH 6/6] Bluetooth: btbcm: btbcm_initialize(): Initialize hw_name to "BCM" Hans de Goede
@ 2018-04-23 17:58 ` Marcel Holtmann
  5 siblings, 0 replies; 7+ messages in thread
From: Marcel Holtmann @ 2018-04-23 17:58 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Gustavo F. Padovan, Johan Hedberg, Frédéric Danis,
	Lukas Wunner, BlueZ development, linux-serial, linux-acpi

Hi Hans,

> btbcm_setup_patchram() was using the upper nibble of the revision code to
> determine if we are dealing with an uart or USB connected bcm-bt device,
> but just as btbcm_initialize() has started accepting 1 and 2 as uart
> connected devices, I've now encountered an USB connected device (0a5c:216c)
> which has 0 in the upper nibble. So it seems that the upper nibble is not
> really a reliable indicator of the bus type.
> 
> Instead check hdev->bus which does give us a reliable indication. This
> fixes the patchram code trying to load the patchram by the fallback BCM.hcd
> filename, now it correctly requests BCM43142A0-0a5c-216c.hcd.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/bluetooth/btbcm.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)

all 6 patches have been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-04-23 17:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-20 12:44 [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Hans de Goede
2018-04-20 12:44 ` [PATCH 2/6] Bluetooth: btbcm: Factor out common code to determine subversion Hans de Goede
2018-04-20 12:44 ` [PATCH 3/6] Bluetooth: btbcm: Make btbcm_initialize() also work for USB connected devices Hans de Goede
2018-04-20 12:44 ` [PATCH 4/6] Bluetooth: btbcm: Allow using btbcm_initialize() for reinit Hans de Goede
2018-04-20 12:44 ` [PATCH 5/6] Bluetooth: btbcm: Remove duplicate code from btbcm_setup_patchram() Hans de Goede
2018-04-20 12:44 ` [PATCH 6/6] Bluetooth: btbcm: btbcm_initialize(): Initialize hw_name to "BCM" Hans de Goede
2018-04-23 17:58 ` [PATCH 1/6] Bluetooth: btbcm: Stop using upper nibble of rev to chose between uart/USB paths Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).