Hi Marcel,
> >
> > #define BT_SCM_PKT_STATUS 0x03
> >
> > +#define BT_CODEC 19
> > +
> > +struct bt_codec_caps {
> > + __u8 len;
> > + __u8 data[];
> > +} __packed;
> > +
> > +struct bt_codec {
> > + __u8 id;
> > + __le16 cid;
> > + __le16 vid;
> > + __u8 data_path;
> > + __u8 num_caps;
> > +} __packed;
> > +
> > +struct bt_codecs {
> > + __u8 num_codecs;
> > + struct bt_codec codecs[];
> > +} __packed;
> > +
>
> what is encapsulating what here?
bt_codec encapsulates caps.
bt_codecs is aggregation of bt_codec.
>
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > __printf(1, 2)
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 731d48ca873a..9658600ae5de 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -2626,6 +2626,10 @@ static inline struct hci_sco_hdr
> *hci_sco_hdr(const struct sk_buff *skb)
> > #define hci_iso_data_len(h) ((h) & 0x3fff)
> > #define hci_iso_data_flags(h) ((h) >> 14)
> >
> > +/* codec transport types */
> > +#define TRANSPORT_BREDR 0x00
> > +#define TRANSPORT_SCO_ESCO 0x01
> > +
>
> This is confusing. SCO_ESCO is also BREDR.
Ack. I will remove the suffix BREDR.
>
> > /* le24 support */
> > static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3]) { diff
> > --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index
> > 3bd41563f118..d66293d1cba4 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -948,6 +948,11 @@ static int sco_sock_getsockopt(struct socket *sock,
> int level, int optname,
> > struct bt_voice voice;
> > u32 phys;
> > int pkt_status;
> > + struct codec_list *c;
> > + u8 num_codecs, i, __user *ptr;
> > + struct hci_dev *hdev;
> > + struct hci_codec_caps *caps;
> > + __u8 data_path;
> >
> > BT_DBG("sk %p", sk);
> >
> > @@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket
> *sock, int level, int optname,
> > err = -EFAULT;
> > break;
> >
> > + case BT_CODEC:
> > + num_codecs = 0;
> > + len = 0;
> > +
> > + hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
> > +BDADDR_BREDR);
> > +
>
> Remove extra empty line.
Ack
>
> > + if (!hdev) {
> > + err = -EBADFD;
> > + break;
> > + }
> > +
> > + if
> (!test_bit(HCI_QUIRK_HFP_OFFLOAD_CODECS_SUPPORTED, &hdev->quirks))
> {
> > + err = -EOPNOTSUPP;
> > + break;
> > + }
> > +
> > + if (!hdev->get_data_path_id) {
> > + err = -EOPNOTSUPP;
> > + break;
> > + }
>
> See it is pointless to check a quirk, you can use the callback for it.
>
Ack
> > +
> > + hci_dev_lock(hdev);
> > + list_for_each_entry(c, &hdev->local_codecs, list) {
> > + if (c->transport != TRANSPORT_SCO_ESCO)
> > + continue;
> > + num_codecs++;
> > + for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> > + len += 1 + caps->len;
> > + caps = (void *)&caps->data[caps->len];
> > + }
> > + len += sizeof(struct bt_codec);
> > + }
> > + hci_dev_unlock(hdev);
> > +
> > + if (len > 255) {
> > + err = -ENOMEM;
> > + break;
>
> I think this is the wrong error code. You need to indicate that size is wrong.
> ENOMEM really means that memory allocation failed.
I will change error code to ENOBUFS
>
Thanks,
Kiran
