| Message ID | 20210727084713.23038-4-kiran.k@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v11,01/10] Bluetooth: Enumerate local supported codec and cache details | expand |
Hi Kiran, > Add BT_CODEC option for getsockopt systemcall to get the details > of offload codecs supported over SCO socket > > Signed-off-by: Kiran K <kiran.k@intel.com> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com> > --- > * changes in v11: > - Remove Kconfig related changes > * changes in v10: > - In getsockopt, prepare data in advance and call copy_to_user > istead of calling put_user on each field of struct bt_codec > > include/net/bluetooth/bluetooth.h | 20 +++++++ > include/net/bluetooth/hci.h | 4 ++ > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/sco.c | 91 +++++++++++++++++++++++++++++++ > 4 files changed, 116 insertions(+) > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > index 9125effbf448..64cddff0c9c4 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -153,6 +153,26 @@ struct bt_voice { > > #define BT_SCM_PKT_STATUS 0x03 > > +#define BT_CODEC 19 > + > +struct bt_codec_caps { > + __u8 len; > + __u8 data[]; > +} __packed; > + > +struct bt_codec { > + __u8 id; > + __u16 cid; > + __u16 vid; > + __u8 data_path; > + __u8 num_caps; > +} __packed; > + > +struct bt_codecs { > + __u8 num_codecs; > + struct bt_codec codecs[]; > +} __packed; > + > __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 b4e35cf5f4b1..22e872e60ff8 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -2621,6 +2621,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_ACL 0x00 > +#define TRANSPORT_SCO_ESCO 0x01 > + > /* le24 support */ > static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3]) > { > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index 6742e6ad7b37..71c409c8ab34 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -617,6 +617,7 @@ struct hci_dev { > int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr); > void (*cmd_timeout)(struct hci_dev *hdev); > bool (*prevent_wake)(struct hci_dev *hdev); > + int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path); > }; > > #define HCI_PHY_HANDLE(handle) (handle & 0xff) > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c > index ffa2a77a3e4c..2f32693f09c1 100644 > --- a/net/bluetooth/sco.c > +++ b/net/bluetooth/sco.c > @@ -953,6 +953,12 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, > struct bt_voice voice; > u32 phys; > int pkt_status; > + int buf_len; > + struct codec_list *c; > + u8 num_codecs, i, __user *ptr; > + struct hci_dev *hdev; > + struct hci_codec_caps *caps; > + struct bt_codec codec; > > BT_DBG("sk %p", sk); > > @@ -1017,6 +1023,91 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, > err = -EFAULT; > break; > > + case BT_CODEC: > + num_codecs = 0; > + buf_len = 0; > + > + hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR); > + if (!hdev) { > + err = -EBADFD; > + break; > + } > + > + if (!hdev->get_data_path_id) { > + err = -EOPNOTSUPP; > + break; > + } so I did say that this needs to be put behind an experimental feature setting. I am not committing to an userspace API until we have more feedback. Regards Marcel
Hi Marcel, > -----Original Message----- > From: Marcel Holtmann <marcel@holtmann.org> > Sent: Friday, July 30, 2021 7:44 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> > Subject: Re: [PATCH v11 04/10] Bluetooth: Allow querying of supported > offload codecs over SCO socket > > Hi Kiran, > > > Add BT_CODEC option for getsockopt systemcall to get the details of > > offload codecs supported over SCO socket > > > > Signed-off-by: Kiran K <kiran.k@intel.com> > > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com> > > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com> > > --- > > * changes in v11: > > - Remove Kconfig related changes > > * changes in v10: > > - In getsockopt, prepare data in advance and call copy_to_user > > istead of calling put_user on each field of struct bt_codec > > > > include/net/bluetooth/bluetooth.h | 20 +++++++ > > include/net/bluetooth/hci.h | 4 ++ > > include/net/bluetooth/hci_core.h | 1 + > > net/bluetooth/sco.c | 91 +++++++++++++++++++++++++++++++ > > 4 files changed, 116 insertions(+) > > > > diff --git a/include/net/bluetooth/bluetooth.h > > b/include/net/bluetooth/bluetooth.h > > index 9125effbf448..64cddff0c9c4 100644 > > --- a/include/net/bluetooth/bluetooth.h > > +++ b/include/net/bluetooth/bluetooth.h > > @@ -153,6 +153,26 @@ struct bt_voice { > > > > #define BT_SCM_PKT_STATUS 0x03 > > > > +#define BT_CODEC 19 > > + > > +struct bt_codec_caps { > > + __u8 len; > > + __u8 data[]; > > +} __packed; > > + > > +struct bt_codec { > > + __u8 id; > > + __u16 cid; > > + __u16 vid; > > + __u8 data_path; > > + __u8 num_caps; > > +} __packed; > > + > > +struct bt_codecs { > > + __u8 num_codecs; > > + struct bt_codec codecs[]; > > +} __packed; > > + > > __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 b4e35cf5f4b1..22e872e60ff8 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -2621,6 +2621,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_ACL 0x00 > > +#define TRANSPORT_SCO_ESCO 0x01 > > + > > /* le24 support */ > > static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3]) { diff > > --git a/include/net/bluetooth/hci_core.h > > b/include/net/bluetooth/hci_core.h > > index 6742e6ad7b37..71c409c8ab34 100644 > > --- a/include/net/bluetooth/hci_core.h > > +++ b/include/net/bluetooth/hci_core.h > > @@ -617,6 +617,7 @@ struct hci_dev { > > int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr); > > void (*cmd_timeout)(struct hci_dev *hdev); > > bool (*prevent_wake)(struct hci_dev *hdev); > > + int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path); > > }; > > > > #define HCI_PHY_HANDLE(handle) (handle & 0xff) > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index > > ffa2a77a3e4c..2f32693f09c1 100644 > > --- a/net/bluetooth/sco.c > > +++ b/net/bluetooth/sco.c > > @@ -953,6 +953,12 @@ static int sco_sock_getsockopt(struct socket *sock, > int level, int optname, > > struct bt_voice voice; > > u32 phys; > > int pkt_status; > > + int buf_len; > > + struct codec_list *c; > > + u8 num_codecs, i, __user *ptr; > > + struct hci_dev *hdev; > > + struct hci_codec_caps *caps; > > + struct bt_codec codec; > > > > BT_DBG("sk %p", sk); > > > > @@ -1017,6 +1023,91 @@ static int sco_sock_getsockopt(struct socket > *sock, int level, int optname, > > err = -EFAULT; > > break; > > > > + case BT_CODEC: > > + num_codecs = 0; > > + buf_len = 0; > > + > > + hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, > BDADDR_BREDR); > > + if (!hdev) { > > + err = -EBADFD; > > + break; > > + } > > + > > + if (!hdev->get_data_path_id) { > > + err = -EOPNOTSUPP; > > + break; > > + } > > so I did say that this needs to be put behind an experimental feature setting. > I am not committing to an userspace API until we have more feedback. Ack. > > Regards > > Marcel Regards, Kiran
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 9125effbf448..64cddff0c9c4 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -153,6 +153,26 @@ struct bt_voice { #define BT_SCM_PKT_STATUS 0x03 +#define BT_CODEC 19 + +struct bt_codec_caps { + __u8 len; + __u8 data[]; +} __packed; + +struct bt_codec { + __u8 id; + __u16 cid; + __u16 vid; + __u8 data_path; + __u8 num_caps; +} __packed; + +struct bt_codecs { + __u8 num_codecs; + struct bt_codec codecs[]; +} __packed; + __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 b4e35cf5f4b1..22e872e60ff8 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -2621,6 +2621,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_ACL 0x00 +#define TRANSPORT_SCO_ESCO 0x01 + /* le24 support */ static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3]) { diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 6742e6ad7b37..71c409c8ab34 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -617,6 +617,7 @@ struct hci_dev { int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr); void (*cmd_timeout)(struct hci_dev *hdev); bool (*prevent_wake)(struct hci_dev *hdev); + int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path); }; #define HCI_PHY_HANDLE(handle) (handle & 0xff) diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index ffa2a77a3e4c..2f32693f09c1 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -953,6 +953,12 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, struct bt_voice voice; u32 phys; int pkt_status; + int buf_len; + struct codec_list *c; + u8 num_codecs, i, __user *ptr; + struct hci_dev *hdev; + struct hci_codec_caps *caps; + struct bt_codec codec; BT_DBG("sk %p", sk); @@ -1017,6 +1023,91 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname, err = -EFAULT; break; + case BT_CODEC: + num_codecs = 0; + buf_len = 0; + + hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR); + if (!hdev) { + err = -EBADFD; + break; + } + + if (!hdev->get_data_path_id) { + err = -EOPNOTSUPP; + break; + } + + /* find total buffer size required to copy codec + caps */ + 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++) { + buf_len += 1 + caps->len; + caps = (void *)&caps->data[caps->len]; + } + buf_len += sizeof(struct bt_codec); + } + hci_dev_unlock(hdev); + + buf_len += sizeof(struct bt_codecs); + if (buf_len > len) { + err = -ENOBUFS; + break; + } + ptr = optval; + + if (put_user(num_codecs, ptr)) { + err = -EFAULT; + break; + } + ptr += sizeof(num_codecs); + + /* Iterate all the codecs supported over SCO and populate + * codec data + */ + hci_dev_lock(hdev); + list_for_each_entry(c, &hdev->local_codecs, list) { + if (c->transport != TRANSPORT_SCO_ESCO) + continue; + + codec.id = c->id; + codec.cid = c->cid; + codec.vid = c->vid; + err = hdev->get_data_path_id(hdev, &codec.data_path); + if (err < 0) + break; + codec.num_caps = c->num_caps; + if (copy_to_user(ptr, &codec, sizeof(codec))) { + err = -EFAULT; + break; + } + ptr += sizeof(codec); + + /* find codec capabilities data length */ + len = 0; + for (i = 0, caps = c->caps; i < c->num_caps; i++) { + len += 1 + caps->len; + caps = (void *)&caps->data[caps->len]; + } + + /* copy codec capabilities data */ + if (len && copy_to_user(ptr, c->caps, len)) { + err = -EFAULT; + break; + } + ptr += len; + } + + if (!err && put_user(buf_len, optlen)) + err = -EFAULT; + + hci_dev_unlock(hdev); + + break; + default: err = -ENOPROTOOPT; break;