Hi Marcel,
Thanks for the comments. I will send out an updated version of patchset.
> -----Original Message-----
> From: Marcel Holtmann <marcel@xxxxxxxxxxxx>
> Sent: Wednesday, April 14, 2021 12:45 AM
> To: K, Kiran <kiran.k@xxxxxxxxx>
> Cc: open list:BLUETOOTH DRIVERS <linux-bluetooth@xxxxxxxxxxxxxxx>;
> Srivatsa, Ravishankar <ravishankar.srivatsa@xxxxxxxxx>; Tumkur Narayan,
> Chethan <chethan.tumkur.narayan@xxxxxxxxx>; Von Dentz, Luiz
> <luiz.von.dentz@xxxxxxxxx>
> Subject: Re: [PATCH v2 1/3] Bluetooth: add support to enumerate codec
> capabilities
>
> Hi Kiran,
>
> > 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@xxxxxxxxx>
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@xxxxxxxxx>
> > Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@xxxxxxxxx>
> > ---
> > * changes in v2
> > add skb->len check before accessing event data
> >
> > include/net/bluetooth/hci.h | 7 ++++
> > net/bluetooth/hci_event.c | 68
> +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 75 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 5e99968939ce..a4b905a76c1b 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -887,6 +887,70 @@ 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;
> > + struct hci_op_read_local_codec_caps caps;
> > +
> > + if (skb->len < sizeof(caps))
> > + return;
> > +
> > + bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> > +
> > + if (skb->data[0])
> > + return;
> > +
> > + /* enumerate standard codecs */
> > + skb_pull(skb, 1);
> > +
> > + if (skb->len < 1)
> > + return;
> > +
> > + num_codecs = skb->data[0];
> > +
> > + bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
> > +
> > + skb_pull(skb, 1);
> > +
> > + if (skb->len < num_codecs)
> > + return;
> > +
> > + while (num_codecs--) {
> > + caps.codec_id[0] = skb->data[0];
> > + caps.transport = 0x00;
> > + caps.direction = 0x00;
> > +
> > + hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > + &caps);
> > +
> > + skb_pull(skb, 1);
> > + }
> > +
> > + /* enumerate vendor specific codecs */
> > + if (skb->len < 1)
> > + return;
> > +
> > + num_codecs = skb->data[0];
> > + skb_pull(skb, 1);
> > +
> > + bt_dev_dbg(hdev, "Number of vendor specific codecs: %u",
> > +num_codecs);
> > +
> > + if (skb->len < (num_codecs * 4))
> > + return;
> > +
> > + while (num_codecs--) {
> > + caps.codec_id[0] = 0xFF;
> > + memcpy(&caps.codec_id[1], skb->data, 4);
> > + caps.transport = 0x00;
> > + caps.direction = 0x00;
> > +
> > + hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
> sizeof(caps),
> > + &caps);
> > + skb_pull(skb, 4);
> > + }
>
> instead of sending hci_send_cmd here, I rather do this in a separate init
> stage. Since you want to cache the codec values anyway, start doing it now.
>
> Regards
>
> Marcel
Regards,
Kiran
