RE: [PATCH v1 1/3] Bluetooth: add support to enumerate codec capabilities

["K, Kiran" <kiran.k@intel.com>]

Hi Luiz,

Thanks for the comments.  I will fix and send out an updated version.

Regards,
Kiran

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Friday, April 9, 2021 10:32 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz
> <luiz.von.dentz@intel.com>
> Subject: Re: [PATCH v1 1/3] Bluetooth: add support to enumerate codec
> capabilities
> 
> Hi Kiran,
> 
> On Fri, Apr 9, 2021 at 5:21 AM Kiran K <kiran.k@intel.com> wrote:
> >
> > add support to enumerate local supported codec capabilities
> >
> > < HCI Command: Read Local Suppor.. (0x04|0x000e) plen 7
> >         Codec: mSBC (0x05)
> >         Logical Transport Type: 0x00
> >         Direction: Input (Host to Controller) (0x00)
> > > HCI Event: Command Complete (0x0e) plen 12
> >       Read Local Supported Codec Capabilities (0x04|0x000e) ncmd 1
> >         Status: Success (0x00)
> >         Number of codec capabilities: 1
> >          Capabilities #0:
> >         00 00 11 15 02 33
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > ---
> >  include/net/bluetooth/hci.h |  7 +++++
> >  net/bluetooth/hci_event.c   | 56
> +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 63 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index ea4ae551c426..e3f7771fe84f 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1314,6 +1314,13 @@ struct hci_rp_read_local_pairing_opts {
> >         __u8     max_key_size;
> >  } __packed;
> >
> > +#define HCI_OP_READ_LOCAL_CODEC_CAPS   0x100e
> > +struct hci_op_read_local_codec_caps {
> > +       __u8    codec_id[5];
> > +       __u8    transport;
> > +       __u8    direction;
> > +} __packed;
> > +
> >  #define HCI_OP_READ_PAGE_SCAN_ACTIVITY 0x0c1b  struct
> > hci_rp_read_page_scan_activity {
> >         __u8     status;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 016b2999f219..ceed5a5d332b 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -887,6 +887,58 @@ static void hci_cc_read_data_block_size(struct
> hci_dev *hdev,
> >                hdev->block_cnt, hdev->block_len);  }
> >
> > +static void hci_cc_read_local_codecs(struct hci_dev *hdev,
> > +                                    struct sk_buff *skb) {
> > +       __u8 num_codecs;
> > +       __u8 *ptr;
> > +       struct hci_op_read_local_codec_caps caps;
> > +
> > +       bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> > +
> > +       if (skb->data[0])
> > +               return;
> > +
> > +       /* enumerate standard codecs */
> > +       skb_pull(skb, 1);
> 
> After each skb_pull check the expected length against skb->len.
> 
> > +       num_codecs = skb->data[0];
> > +
> > +       bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
> > +
> > +       skb_pull(skb, 1);
> 
> Ditto.
> 
> > +       ptr = (__u8 *)skb->data;
> > +
> > +       skb_pull(skb, num_codecs);
> > +
> > +       while (num_codecs--) {
> > +               caps.codec_id[0] = *ptr++;
> 
> Lets just use skb_pull to advance on the codecs ids, that way we can properly
> check the remaining length with use of skb->len.
> 
> > +               caps.transport = 0x00;
> > +               caps.direction = 0x00;
> > +
> > +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > +                            &caps);
> > +       }
> > +
> > +       /* enumerate vendor specific codecs */
> > +       num_codecs = skb->data[0];
> > +       skb_pull(skb, 1);
> > +
> > +       bt_dev_dbg(hdev, "Number of vendor specific codecs: %u",
> > + num_codecs);
> > +
> > +       ptr = (__u8 *)skb->data;
> > +
> > +       while (num_codecs--) {
> > +               caps.codec_id[0] = 0xFF;
> > +               memcpy(&caps.codec_id[1], ptr, 4);
> > +               ptr += 4;
> > +               caps.transport = 0x00;
> > +               caps.direction = 0x00;
> > +
> > +               hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > +                            &caps);
> > +       }
> > +}
> > +
> >  static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff
> > *skb)  {
> >         struct hci_rp_read_clock *rp = (void *) skb->data; @@ -3437,6
> > +3489,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev,
> struct sk_buff *skb,
> >                 hci_cc_read_data_block_size(hdev, skb);
> >                 break;
> >
> > +       case HCI_OP_READ_LOCAL_CODECS:
> > +               hci_cc_read_local_codecs(hdev, skb);
> > +               break;
> > +
> >         case HCI_OP_READ_FLOW_CONTROL_MODE:
> >                 hci_cc_read_flow_control_mode(hdev, skb);
> >                 break;
> > --
> > 2.17.1
> >
> 
> 
> --
> Luiz Augusto von Dentz