USB alternate seeting of which the packet size is distinct.
The patches are to expose the packet size to user space so that
the user space does not need to hard code those values.
We have verified this patch on Chromebooks which use
- Realtek 8822CE controller with USB alt setting 1
- Intel controller with USB alt setting 6
Our user space audio server, cras, can get the correct
packet length from the socket option.
Joseph Hwang (2):
Bluetooth: btusb: define HCI packet sizes of USB Alts
Bluetooth: sco: expose WBS packet length in socket option
drivers/bluetooth/btusb.c | 43 +++++++++++++++++++++++--------
include/net/bluetooth/bluetooth.h | 2 ++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/sco.c | 8 ++++++
4 files changed, 43 insertions(+), 11 deletions(-)
--
2.28.0.236.gb10cc79966-goog
It is desirable to expose the wideband speech packet length via
a socket option to the user space so that the user space can set
the value correctly in configuring the sco connection.
Reviewed-by: Alain Michaud <[email protected]>
Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
Signed-off-by: Joseph Hwang <[email protected]>
---
include/net/bluetooth/bluetooth.h | 2 ++
net/bluetooth/sco.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9125effbf4483d..922cc03143def4 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -153,6 +153,8 @@ struct bt_voice {
#define BT_SCM_PKT_STATUS 0x03
+#define BT_SCO_PKT_LEN 17
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index dcf7f96ff417e6..97e4e7c7b8cf62 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -67,6 +67,7 @@ struct sco_pinfo {
__u32 flags;
__u16 setting;
__u8 cmsg_mask;
+ __u32 pkt_len;
struct sco_conn *conn;
};
@@ -267,6 +268,8 @@ static int sco_connect(struct sock *sk)
sco_sock_set_timer(sk, sk->sk_sndtimeo);
}
+ sco_pi(sk)->pkt_len = hdev->sco_pkt_len;
+
done:
hci_dev_unlock(hdev);
hci_dev_put(hdev);
@@ -1001,6 +1004,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
err = -EFAULT;
break;
+ case BT_SCO_PKT_LEN:
+ if (put_user(sco_pi(sk)->pkt_len, (u32 __user *)optval))
+ err = -EFAULT;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
--
2.28.0.236.gb10cc79966-goog
On Wed, Aug 19, 2020 at 11:23 AM Pali Rohár <[email protected]> wrote:
>
> On Wednesday 19 August 2020 11:21:00 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Wed, Aug 19, 2020 at 7:37 AM Pali Rohár <[email protected]> wrote:
> > >
> > > On Friday 14 August 2020 12:56:05 Luiz Augusto von Dentz wrote:
> > > > Hi Joseph,
> > > >
> > > > On Thu, Aug 13, 2020 at 1:42 AM Joseph Hwang <[email protected]> wrote:
> > > > >
> > > > > It is desirable to expose the wideband speech packet length via
> > > > > a socket option to the user space so that the user space can set
> > > > > the value correctly in configuring the sco connection.
> > > > >
> > > > > Reviewed-by: Alain Michaud <[email protected]>
> > > > > Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> > > > > Signed-off-by: Joseph Hwang <[email protected]>
> > > > > ---
> > > > >
> > > > > include/net/bluetooth/bluetooth.h | 2 ++
> > > > > net/bluetooth/sco.c | 8 ++++++++
> > > > > 2 files changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > > > > index 9125effbf4483d..922cc03143def4 100644
> > > > > --- a/include/net/bluetooth/bluetooth.h
> > > > > +++ b/include/net/bluetooth/bluetooth.h
> > > > > @@ -153,6 +153,8 @@ struct bt_voice {
> > > > >
> > > > > #define BT_SCM_PKT_STATUS 0x03
> > > > >
> > > > > +#define BT_SCO_PKT_LEN 17
> > > > > +
> > > > > __printf(1, 2)
> > > > > void bt_info(const char *fmt, ...);
> > > > > __printf(1, 2)
> > > > > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > > > > index dcf7f96ff417e6..97e4e7c7b8cf62 100644
> > > > > --- a/net/bluetooth/sco.c
> > > > > +++ b/net/bluetooth/sco.c
> > > > > @@ -67,6 +67,7 @@ struct sco_pinfo {
> > > > > __u32 flags;
> > > > > __u16 setting;
> > > > > __u8 cmsg_mask;
> > > > > + __u32 pkt_len;
> > > > > struct sco_conn *conn;
> > > > > };
> > > > >
> > > > > @@ -267,6 +268,8 @@ static int sco_connect(struct sock *sk)
> > > > > sco_sock_set_timer(sk, sk->sk_sndtimeo);
> > > > > }
> > > > >
> > > > > + sco_pi(sk)->pkt_len = hdev->sco_pkt_len;
> > > > > +
> > > > > done:
> > > > > hci_dev_unlock(hdev);
> > > > > hci_dev_put(hdev);
> > > > > @@ -1001,6 +1004,11 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> > > > > err = -EFAULT;
> > > > > break;
> > > > >
> > > > > + case BT_SCO_PKT_LEN:
> > > > > + if (put_user(sco_pi(sk)->pkt_len, (u32 __user *)optval))
> > > > > + err = -EFAULT;
> > > > > + break;
> > > >
> > > > Couldn't we expose this via BT_SNDMTU/BT_RCVMTU?
> > >
> > > Hello!
> > >
> > > There is already SCO_OPTIONS sock option, uses struct sco_options and
> > > contains 'mtu' member.
> > >
> > > I think that instead of adding new sock option, existing SCO_OPTIONS
> > > option should be used.
> >
> > We are moving away from type specific options to so options like
> > BT_SNDMTU/BT_RCVMTU should be supported in all socket types.
>
> Yes, this make sense.
>
> But I guess that SCO_OPTIONS should be provided for backward
> compatibility as it is already used by lot of userspace applications.
>
> So for me it looks like that BT_SNDMTU/BT_RCVMTU should return same
> value as SCO_OPTIONS.
Yep, luckily we can do this here because SCO MTU is symmetric.
> > >
> > > > > default:
> > > > > err = -ENOPROTOOPT;
> > > > > break;
> > > > > --
> > > > > 2.28.0.236.gb10cc79966-goog
> > > > >
> > > >
> > > >
> > > > --
> > > > Luiz Augusto von Dentz
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz