Hi Daniel,
> Multiple codepaths duplicate some simple code to read and
> sanity-check local version information. Before I add a couple more
> such codepaths, add a helper to reduce duplication.
>
> Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx>
> ---
> drivers/bluetooth/btusb.c | 68 ++++++++++++++++++++++-------------------------
> 1 file changed, 32 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index b876888..3096308 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -1262,6 +1262,33 @@ static void btusb_waker(struct work_struct *work)
> usb_autopm_put_interface(data->intf);
> }
>
> +static struct sk_buff *btusb_read_local_version(struct hci_dev *hdev)
> +{
> + struct hci_rp_read_local_version *ver;
> + struct sk_buff *skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION,
> + 0, NULL, HCI_INIT_TIMEOUT);
I prefer only have simple assignments in the variable declaration. For all the others, keep in after the variable declaration.
> +
> + if (IS_ERR(skb)) {
> + BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION failed (%ld)",
> + hdev->name, PTR_ERR(skb));
> + return skb;
> + }
> +
> + if (skb->len != sizeof(*ver)) {
> + BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION event length mismatch",
> + hdev->name);
> + kfree_skb(skb);
> + return ERR_PTR(-EIO);
> + }
> +
> + ver = (struct hci_rp_read_local_version *) skb->data;
> + BT_INFO("%s: hci_ver=%02x hci_rev=%04x lmp_ver=%02x lmp_subver=%04x",
> + hdev->name, ver->hci_ver, ver->hci_rev, ver->lmp_ver,
> + ver->lmp_subver);
I actually do not like these printouts. Lets leave this up to the caller.
> +
> + return skb;
> +}
> +
> static int btusb_setup_bcm92035(struct hci_dev *hdev)
> {
> struct sk_buff *skb;
> @@ -1286,12 +1313,9 @@ static int btusb_setup_csr(struct hci_dev *hdev)
>
> BT_DBG("%s", hdev->name);
>
> - skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> - if (IS_ERR(skb)) {
> - BT_ERR("Reading local version failed (%ld)", -PTR_ERR(skb));
> + skb = btusb_read_local_version(hdev);
> + if (IS_ERR(skb))
> return -PTR_ERR(skb);
> - }
>
> rp = (struct hci_rp_read_local_version *)skb->data;
>
> @@ -2393,27 +2417,14 @@ static int btusb_setup_bcm_patchram(struct hci_dev *hdev)
> kfree_skb(skb);
>
> /* Read Local Version Info */
> - skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + skb = btusb_read_local_version(hdev);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> - BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION failed (%ld)",
> - hdev->name, ret);
> - goto done;
> - }
> -
> - if (skb->len != sizeof(*ver)) {
> - BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION event length mismatch",
> - hdev->name);
> - kfree_skb(skb);
> - ret = -EIO;
> goto done;
> }
>
> ver = (struct hci_rp_read_local_version *)skb->data;
> - BT_INFO("%s: BCM: patching hci_ver=%02x hci_rev=%04x lmp_ver=%02x "
> - "lmp_subver=%04x", hdev->name, ver->hci_ver, ver->hci_rev,
> - ver->lmp_ver, ver->lmp_subver);
> + BT_INFO("%s: BCM: apply patch", hdev->name);
> kfree_skb(skb);
>
> /* Start Download */
> @@ -2475,27 +2486,12 @@ reset_fw:
> kfree_skb(skb);
>
> /* Read Local Version Info */
> - skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL,
> - HCI_INIT_TIMEOUT);
> + skb = btusb_read_local_version(hdev);
> if (IS_ERR(skb)) {
> ret = PTR_ERR(skb);
> - BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION failed (%ld)",
> - hdev->name, ret);
> goto done;
> }
>
> - if (skb->len != sizeof(*ver)) {
> - BT_ERR("%s: HCI_OP_READ_LOCAL_VERSION event length mismatch",
> - hdev->name);
> - kfree_skb(skb);
> - ret = -EIO;
> - goto done;
> - }
> -
> - ver = (struct hci_rp_read_local_version *)skb->data;
> - BT_INFO("%s: BCM: firmware hci_ver=%02x hci_rev=%04x lmp_ver=%02x "
> - "lmp_subver=%04x", hdev->name, ver->hci_ver, ver->hci_rev,
> - ver->lmp_ver, ver->lmp_subver);
> kfree_skb(skb);
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
