diff mbox series

Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING

Message ID 20250401134424.3725875-1-chharry@google.com (mailing list archive)
State New
Headers show
Series Bluetooth: Add driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester fail TestRunner_l2cap-tester: Total: 62, Passed: 61 (98.4%), Failed: 1, Not Run: 0
tedd_an/TestRunner_iso-tester success TestRunner PASS
tedd_an/TestRunner_bnep-tester success TestRunner PASS
tedd_an/TestRunner_mgmt-tester fail TestRunner_mgmt-tester: Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4
tedd_an/TestRunner_rfcomm-tester success TestRunner PASS
tedd_an/TestRunner_sco-tester success TestRunner PASS
tedd_an/TestRunner_ioctl-tester success TestRunner PASS
tedd_an/TestRunner_mesh-tester success TestRunner PASS
tedd_an/TestRunner_smp-tester success TestRunner PASS
tedd_an/TestRunner_userchan-tester success TestRunner PASS

Commit Message

Hsin-chen Chuang April 1, 2025, 1:44 p.m. UTC 
From: Hsin-chen Chuang <chharry@chromium.org>

Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
SCO data through USB Bluetooth chips, it's observed that with the patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.

To address the issue above one needs to reset the altsetting back to
zero when there is no active SCO connection, which is the same as the
BlueZ behavior, and another benefit is the bus doesn't need to reserve
bandwidth when no SCO connection.

This patch introduces a fundamental solution that lets the user space
program to configure the altsetting freely:
- Define the new packet type HCI_DRV_PKT which is specifically used for
  communication between the user space program and the Bluetooth drviers
- Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
  indicates the expected altsetting from the user space program
- btusb intercepts the command and adjusts the Isoc endpoint
  correspondingly

This patch is tested on ChromeOS devices. The USB Bluetooth models
(CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
band speech and wide band speech.

Cc: chromeos-bluetooth-upstreaming@chromium.org
Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---

 drivers/bluetooth/btusb.c       | 67 +++++++++++++++++++++++++++++++++
 include/net/bluetooth/hci.h     |  1 +
 include/net/bluetooth/hci_mon.h |  2 +
 net/bluetooth/hci_core.c        |  2 +
 net/bluetooth/hci_sock.c        | 14 +++++--
 5 files changed, 83 insertions(+), 3 deletions(-)

Comments

bluez.test.bot@gmail.com April 1, 2025, 2:33 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=948951

---Test result---

Test Summary:
CheckPatch                    PENDING   0.40 seconds
GitLint                       PENDING   0.26 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      24.01 seconds
CheckAllWarning               PASS      26.43 seconds
CheckSparse                   PASS      30.05 seconds
BuildKernel32                 PASS      24.08 seconds
TestRunnerSetup               PASS      428.46 seconds
TestRunner_l2cap-tester       FAIL      44.90 seconds
TestRunner_iso-tester         PASS      34.68 seconds
TestRunner_bnep-tester        PASS      4.73 seconds
TestRunner_mgmt-tester        FAIL      119.51 seconds
TestRunner_rfcomm-tester      PASS      7.76 seconds
TestRunner_sco-tester         PASS      12.47 seconds
TestRunner_ioctl-tester       PASS      12.65 seconds
TestRunner_mesh-tester        PASS      6.04 seconds
TestRunner_smp-tester         PASS      7.09 seconds
TestRunner_userchan-tester    PASS      4.93 seconds
IncrementalBuild              PENDING   0.50 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
Total: 62, Passed: 61 (98.4%), Failed: 1, Not Run: 0

Failed Test Cases
L2CAP LE Client - Close socket 2                     Timed out   23.647 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
LL Privacy - Set Device Flag 1 (Device Privacy)      Failed       0.154 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz April 1, 2025, 3:02 p.m. UTC | #2 
Hi Hsin-chen,

On Tue, Apr 1, 2025 at 9:44 AM Hsin-chen Chuang <chharry@google.com> wrote:
>
> From: Hsin-chen Chuang <chharry@chromium.org>
>
> Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
> for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
> SCO data through USB Bluetooth chips, it's observed that with the patch
> HFP is flaky on most of the existing USB Bluetooth controllers: Intel
> chips sometimes send out no packet for Transparent codec; MTK chips may
> generate SCO data with a wrong handle for CVSD codec; RTK could split
> the data with a wrong packet size for Transparent codec; ... etc.
>
> To address the issue above one needs to reset the altsetting back to
> zero when there is no active SCO connection, which is the same as the
> BlueZ behavior, and another benefit is the bus doesn't need to reserve
> bandwidth when no SCO connection.
>
> This patch introduces a fundamental solution that lets the user space
> program to configure the altsetting freely:
> - Define the new packet type HCI_DRV_PKT which is specifically used for
>   communication between the user space program and the Bluetooth drviers
> - Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
>   indicates the expected altsetting from the user space program
> - btusb intercepts the command and adjusts the Isoc endpoint
>   correspondingly
>
> This patch is tested on ChromeOS devices. The USB Bluetooth models
> (CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
> band speech and wide band speech.
>
> Cc: chromeos-bluetooth-upstreaming@chromium.org
> Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> ---
>
>  drivers/bluetooth/btusb.c       | 67 +++++++++++++++++++++++++++++++++
>  include/net/bluetooth/hci.h     |  1 +
>  include/net/bluetooth/hci_mon.h |  2 +
>  net/bluetooth/hci_core.c        |  2 +
>  net/bluetooth/hci_sock.c        | 14 +++++--
>  5 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 5012b5ff92c8..a7bc64e86661 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2151,6 +2151,67 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
>         return 0;
>  }
>
> +static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
> +{
> +       struct sk_buff *skb;
> +
> +       /* btusb driver response starts with 1 oct of the opcode,
> +        * and followed by the command specific data.
> +        */
> +       skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
> +       if (!skb)
> +               return NULL;
> +
> +       skb_put_u8(skb, opcode);
> +       hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> +
> +       return skb;
> +}
> +
> +static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
> +
> +#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35

Any particular reason why you are starting with 0x35? We may need to
add something like Read Supported Driver Commands to begin with.

> +static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       /* btusb driver command starts with 1 oct of the opcode,
> +        * and followed by the command specific data.
> +        */
> +       if (!skb->len)
> +               return -EILSEQ;

We might need to define a struct header, and I'd definitely recommend
using skb_pull_data for parsing.

> +       switch (skb->data[0]) {
> +       case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
> +               struct sk_buff *resp;
> +               int status;
> +
> +               /* Response data: Total 1 Oct
> +                *   Status: 1 Oct
> +                *     0 = Success
> +                *     1 = Invalid command
> +                *     2 = Other errors
> +                */
> +               resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
> +               if (!resp)
> +                       return -ENOMEM;
> +
> +               if (skb->len != 2 || skb->data[1] > 6) {
> +                       status = 1;
> +               } else {
> +                       status = btusb_switch_alt_setting(hdev, skb->data[1]);
> +                       if (status)
> +                               status = 2;
> +               }
> +               skb_put_u8(resp, status);
> +
> +               kfree_skb(skb);
> +               return hci_recv_frame(hdev, resp);
> +       }
> +       }
> +
> +       return -EILSEQ;
> +}
> +
>  static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>  {
>         struct urb *urb;
> @@ -2192,6 +2253,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
>                         return PTR_ERR(urb);
>
>                 return submit_or_queue_tx_urb(hdev, urb);
> +
> +       case HCI_DRV_PKT:
> +               return btusb_drv_cmd(hdev, skb);
>         }
>
>         return -EILSEQ;
> @@ -2669,6 +2733,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
>                         return PTR_ERR(urb);
>
>                 return submit_or_queue_tx_urb(hdev, urb);
> +
> +       case HCI_DRV_PKT:
> +               return btusb_drv_cmd(hdev, skb);
>         }
>
>         return -EILSEQ;
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index a8586c3058c7..e297b312d2b7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -494,6 +494,7 @@ enum {
>  #define HCI_EVENT_PKT          0x04
>  #define HCI_ISODATA_PKT                0x05
>  #define HCI_DIAG_PKT           0xf0
> +#define HCI_DRV_PKT            0xf1
>  #define HCI_VENDOR_PKT         0xff
>
>  /* HCI packet types */
> diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
> index 082f89531b88..bbd752494ef9 100644
> --- a/include/net/bluetooth/hci_mon.h
> +++ b/include/net/bluetooth/hci_mon.h
> @@ -51,6 +51,8 @@ struct hci_mon_hdr {
>  #define HCI_MON_CTRL_EVENT     17
>  #define HCI_MON_ISO_TX_PKT     18
>  #define HCI_MON_ISO_RX_PKT     19
> +#define HCI_MON_DRV_TX_PKT     20
> +#define HCI_MON_DRV_RX_PKT     21

Are you planning to write some btmon decoding for these packets?

>  struct hci_mon_new_index {
>         __u8            type;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5eb0600bbd03..bb4e1721edc2 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
>                 break;
>         case HCI_ISODATA_PKT:
>                 break;
> +       case HCI_DRV_PKT:
> +               break;
>         default:
>                 kfree_skb(skb);
>                 return -EINVAL;
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 022b86797acd..0bc4f77ed17b 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
>                         if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
>                             hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
>                             hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> -                           hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
> +                           hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> +                           hci_skb_pkt_type(skb) != HCI_DRV_PKT)
>                                 continue;
>                 } else {
>                         /* Don't send frame to other channel types */
> @@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>                 else
>                         opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
>                 break;
> +       case HCI_DRV_PKT:
> +               if (bt_cb(skb)->incoming)
> +                       opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
> +               else
> +                       opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
> +               break;
>         case HCI_DIAG_PKT:
>                 opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
>                 break;
> @@ -1806,7 +1813,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>         if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
>                 return -EINVAL;
>
> -       if (len < 4 || len > hci_pi(sk)->mtu)
> +       if (len > hci_pi(sk)->mtu)
>                 return -EINVAL;
>
>         skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
> @@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
>                 if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
>                     hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
>                     hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> -                   hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
> +                   hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> +                   hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
>                         err = -EINVAL;
>                         goto drop;
>                 }
> --
> 2.49.0.472.ge94155a9ec-goog
>
Hsin-chen Chuang April 1, 2025, 3:26 p.m. UTC | #3 
Hi Luiz,

On Tue, Apr 1, 2025 at 11:03 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Hsin-chen,
>
> On Tue, Apr 1, 2025 at 9:44 AM Hsin-chen Chuang <chharry@google.com> wrote:
> >
> > From: Hsin-chen Chuang <chharry@chromium.org>
> >
> > Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
> > for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
> > SCO data through USB Bluetooth chips, it's observed that with the patch
> > HFP is flaky on most of the existing USB Bluetooth controllers: Intel
> > chips sometimes send out no packet for Transparent codec; MTK chips may
> > generate SCO data with a wrong handle for CVSD codec; RTK could split
> > the data with a wrong packet size for Transparent codec; ... etc.
> >
> > To address the issue above one needs to reset the altsetting back to
> > zero when there is no active SCO connection, which is the same as the
> > BlueZ behavior, and another benefit is the bus doesn't need to reserve
> > bandwidth when no SCO connection.
> >
> > This patch introduces a fundamental solution that lets the user space
> > program to configure the altsetting freely:
> > - Define the new packet type HCI_DRV_PKT which is specifically used for
> >   communication between the user space program and the Bluetooth drviers
> > - Define the btusb driver command BTUSB_DRV_CMD_SWITCH_ALT_SETTING which
> >   indicates the expected altsetting from the user space program
> > - btusb intercepts the command and adjusts the Isoc endpoint
> >   correspondingly
> >
> > This patch is tested on ChromeOS devices. The USB Bluetooth models
> > (CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
> > band speech and wide band speech.
> >
> > Cc: chromeos-bluetooth-upstreaming@chromium.org
> > Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
> > Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
> > ---
> >
> >  drivers/bluetooth/btusb.c       | 67 +++++++++++++++++++++++++++++++++
> >  include/net/bluetooth/hci.h     |  1 +
> >  include/net/bluetooth/hci_mon.h |  2 +
> >  net/bluetooth/hci_core.c        |  2 +
> >  net/bluetooth/hci_sock.c        | 14 +++++--
> >  5 files changed, 83 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 5012b5ff92c8..a7bc64e86661 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2151,6 +2151,67 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
> >         return 0;
> >  }
> >
> > +static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
> > +{
> > +       struct sk_buff *skb;
> > +
> > +       /* btusb driver response starts with 1 oct of the opcode,
> > +        * and followed by the command specific data.
> > +        */
> > +       skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
> > +       if (!skb)
> > +               return NULL;
> > +
> > +       skb_put_u8(skb, opcode);
> > +       hci_skb_pkt_type(skb) = HCI_DRV_PKT;
> > +
> > +       return skb;
> > +}
> > +
> > +static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
> > +
> > +#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35
>
> Any particular reason why you are starting with 0x35? We may need to
> add something like Read Supported Driver Commands to begin with.

Um, it's just my lucky number. No particular reason in terms of the design.
And sure Read Supported Driver Commands seems good, but does that
indicate that the meaning of the opcodes is shared across different
driver modules? That's fine but we would need to move these
definitions somewhere else.

>
> > +static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > +       /* btusb driver command starts with 1 oct of the opcode,
> > +        * and followed by the command specific data.
> > +        */
> > +       if (!skb->len)
> > +               return -EILSEQ;
>
> We might need to define a struct header, and I'd definitely recommend
> using skb_pull_data for parsing.

So far the header has only 1 oct of the opcode. Would you suggest
something similar to HCI command e.g. 2 oct of the opcode + 1 oct of
the param length?

>
> > +       switch (skb->data[0]) {
> > +       case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
> > +               struct sk_buff *resp;
> > +               int status;
> > +
> > +               /* Response data: Total 1 Oct
> > +                *   Status: 1 Oct
> > +                *     0 = Success
> > +                *     1 = Invalid command
> > +                *     2 = Other errors
> > +                */
> > +               resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
> > +               if (!resp)
> > +                       return -ENOMEM;
> > +
> > +               if (skb->len != 2 || skb->data[1] > 6) {
> > +                       status = 1;
> > +               } else {
> > +                       status = btusb_switch_alt_setting(hdev, skb->data[1]);
> > +                       if (status)
> > +                               status = 2;
> > +               }
> > +               skb_put_u8(resp, status);
> > +
> > +               kfree_skb(skb);
> > +               return hci_recv_frame(hdev, resp);
> > +       }
> > +       }
> > +
> > +       return -EILSEQ;
> > +}
> > +
> >  static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> >  {
> >         struct urb *urb;
> > @@ -2192,6 +2253,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
> >                         return PTR_ERR(urb);
> >
> >                 return submit_or_queue_tx_urb(hdev, urb);
> > +
> > +       case HCI_DRV_PKT:
> > +               return btusb_drv_cmd(hdev, skb);
> >         }
> >
> >         return -EILSEQ;
> > @@ -2669,6 +2733,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
> >                         return PTR_ERR(urb);
> >
> >                 return submit_or_queue_tx_urb(hdev, urb);
> > +
> > +       case HCI_DRV_PKT:
> > +               return btusb_drv_cmd(hdev, skb);
> >         }
> >
> >         return -EILSEQ;
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index a8586c3058c7..e297b312d2b7 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -494,6 +494,7 @@ enum {
> >  #define HCI_EVENT_PKT          0x04
> >  #define HCI_ISODATA_PKT                0x05
> >  #define HCI_DIAG_PKT           0xf0
> > +#define HCI_DRV_PKT            0xf1
> >  #define HCI_VENDOR_PKT         0xff
> >
> >  /* HCI packet types */
> > diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
> > index 082f89531b88..bbd752494ef9 100644
> > --- a/include/net/bluetooth/hci_mon.h
> > +++ b/include/net/bluetooth/hci_mon.h
> > @@ -51,6 +51,8 @@ struct hci_mon_hdr {
> >  #define HCI_MON_CTRL_EVENT     17
> >  #define HCI_MON_ISO_TX_PKT     18
> >  #define HCI_MON_ISO_RX_PKT     19
> > +#define HCI_MON_DRV_TX_PKT     20
> > +#define HCI_MON_DRV_RX_PKT     21
>
> Are you planning to write some btmon decoding for these packets?

Yeah, I believe this is helpful for debugging. ChromeOS still uses
btmon for btsnoop capturing.

>
> >  struct hci_mon_new_index {
> >         __u8            type;
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 5eb0600bbd03..bb4e1721edc2 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> >                 break;
> >         case HCI_ISODATA_PKT:
> >                 break;
> > +       case HCI_DRV_PKT:
> > +               break;
> >         default:
> >                 kfree_skb(skb);
> >                 return -EINVAL;
> > diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> > index 022b86797acd..0bc4f77ed17b 100644
> > --- a/net/bluetooth/hci_sock.c
> > +++ b/net/bluetooth/hci_sock.c
> > @@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
> >                         if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
> >                             hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> >                             hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> > -                           hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
> > +                           hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> > +                           hci_skb_pkt_type(skb) != HCI_DRV_PKT)
> >                                 continue;
> >                 } else {
> >                         /* Don't send frame to other channel types */
> > @@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> >                 else
> >                         opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
> >                 break;
> > +       case HCI_DRV_PKT:
> > +               if (bt_cb(skb)->incoming)
> > +                       opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
> > +               else
> > +                       opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
> > +               break;
> >         case HCI_DIAG_PKT:
> >                 opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
> >                 break;
> > @@ -1806,7 +1813,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> >         if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
> >                 return -EINVAL;
> >
> > -       if (len < 4 || len > hci_pi(sk)->mtu)
> > +       if (len > hci_pi(sk)->mtu)
> >                 return -EINVAL;
> >
> >         skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
> > @@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
> >                 if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
> >                     hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
> >                     hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
> > -                   hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
> > +                   hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
> > +                   hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
> >                         err = -EINVAL;
> >                         goto drop;
> >                 }
> > --
> > 2.49.0.472.ge94155a9ec-goog
> >
>
>
> --
> Luiz Augusto von Dentz
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5012b5ff92c8..a7bc64e86661 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2151,6 +2151,67 @@  static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
 	return 0;
 }
 
+static struct sk_buff *btusb_drv_response(u8 opcode, size_t data_len)
+{
+	struct sk_buff *skb;
+
+	/* btusb driver response starts with 1 oct of the opcode,
+	 * and followed by the command specific data.
+	 */
+	skb = bt_skb_alloc(1 + data_len, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	skb_put_u8(skb, opcode);
+	hci_skb_pkt_type(skb) = HCI_DRV_PKT;
+
+	return skb;
+}
+
+static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
+
+#define BTUSB_DRV_CMD_SWITCH_ALT_SETTING 0x35
+
+static int btusb_drv_cmd(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	/* btusb driver command starts with 1 oct of the opcode,
+	 * and followed by the command specific data.
+	 */
+	if (!skb->len)
+		return -EILSEQ;
+
+	switch (skb->data[0]) {
+	case BTUSB_DRV_CMD_SWITCH_ALT_SETTING: {
+		struct sk_buff *resp;
+		int status;
+
+		/* Response data: Total 1 Oct
+		 *   Status: 1 Oct
+		 *     0 = Success
+		 *     1 = Invalid command
+		 *     2 = Other errors
+		 */
+		resp = btusb_drv_response(BTUSB_DRV_CMD_SWITCH_ALT_SETTING, 1);
+		if (!resp)
+			return -ENOMEM;
+
+		if (skb->len != 2 || skb->data[1] > 6) {
+			status = 1;
+		} else {
+			status = btusb_switch_alt_setting(hdev, skb->data[1]);
+			if (status)
+				status = 2;
+		}
+		skb_put_u8(resp, status);
+
+		kfree_skb(skb);
+		return hci_recv_frame(hdev, resp);
+	}
+	}
+
+	return -EILSEQ;
+}
+
 static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct urb *urb;
@@ -2192,6 +2253,9 @@  static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 			return PTR_ERR(urb);
 
 		return submit_or_queue_tx_urb(hdev, urb);
+
+	case HCI_DRV_PKT:
+		return btusb_drv_cmd(hdev, skb);
 	}
 
 	return -EILSEQ;
@@ -2669,6 +2733,9 @@  static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
 			return PTR_ERR(urb);
 
 		return submit_or_queue_tx_urb(hdev, urb);
+
+	case HCI_DRV_PKT:
+		return btusb_drv_cmd(hdev, skb);
 	}
 
 	return -EILSEQ;
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a8586c3058c7..e297b312d2b7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -494,6 +494,7 @@  enum {
 #define HCI_EVENT_PKT		0x04
 #define HCI_ISODATA_PKT		0x05
 #define HCI_DIAG_PKT		0xf0
+#define HCI_DRV_PKT		0xf1
 #define HCI_VENDOR_PKT		0xff
 
 /* HCI packet types */
diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
index 082f89531b88..bbd752494ef9 100644
--- a/include/net/bluetooth/hci_mon.h
+++ b/include/net/bluetooth/hci_mon.h
@@ -51,6 +51,8 @@  struct hci_mon_hdr {
 #define HCI_MON_CTRL_EVENT	17
 #define HCI_MON_ISO_TX_PKT	18
 #define HCI_MON_ISO_RX_PKT	19
+#define HCI_MON_DRV_TX_PKT	20
+#define HCI_MON_DRV_RX_PKT	21
 
 struct hci_mon_new_index {
 	__u8		type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5eb0600bbd03..bb4e1721edc2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2911,6 +2911,8 @@  int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
 		break;
 	case HCI_ISODATA_PKT:
 		break;
+	case HCI_DRV_PKT:
+		break;
 	default:
 		kfree_skb(skb);
 		return -EINVAL;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 022b86797acd..0bc4f77ed17b 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -234,7 +234,8 @@  void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 			if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
 			    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 			    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
+			    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+			    hci_skb_pkt_type(skb) != HCI_DRV_PKT)
 				continue;
 		} else {
 			/* Don't send frame to other channel types */
@@ -391,6 +392,12 @@  void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
 		else
 			opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
 		break;
+	case HCI_DRV_PKT:
+		if (bt_cb(skb)->incoming)
+			opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
+		else
+			opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
+		break;
 	case HCI_DIAG_PKT:
 		opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
 		break;
@@ -1806,7 +1813,7 @@  static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 	if (flags & ~(MSG_DONTWAIT | MSG_NOSIGNAL | MSG_ERRQUEUE | MSG_CMSG_COMPAT))
 		return -EINVAL;
 
-	if (len < 4 || len > hci_pi(sk)->mtu)
+	if (len > hci_pi(sk)->mtu)
 		return -EINVAL;
 
 	skb = bt_skb_sendmsg(sk, msg, len, len, 0, 0);
@@ -1860,7 +1867,8 @@  static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 		if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
-		    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
+		    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+		    hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
 			err = -EINVAL;
 			goto drop;
 		}