| Message ID | 20240510211431.1728667-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pull request: bluetooth-next 2024-05-10 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Pull request for net-next |
| netdev/build_32bit | fail | Errors and warnings before: 949 this patch: 950 |
| netdev/build_tools | success | Errors and warnings before: 0 this patch: 0 |
| netdev/build_clang | success | Errors and warnings before: 936 this patch: 936 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 960 this patch: 961 |
| netdev/build_clang_rust | success | No Rust files in patch. Skipping build |
On Fri, 10 May 2024 17:14:28 -0400 Luiz Augusto von Dentz wrote: > The following changes since commit f8beae078c82abde57fed4a5be0bbc3579b59ad0: > > Merge tag 'gtp-24-05-07' of git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp Pablo neira Ayuso says: (2024-05-10 13:59:27 +0100) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git tags/for-net-next-2024-05-10 > > for you to fetch changes up to 75f819bdf9cafb0f1458e24c05d24eec17b2f597: > > Bluetooth: btintel: Fix compiler warning for multi_v7_defconfig config (2024-05-10 17:04:15 -0400) > > ---------------------------------------------------------------- > bluetooth-next pull request for net-next: > > - Add support MediaTek MT7921S SDIO > - Various fixes for -Wflex-array-member-not-at-end and -Wfamnae > - Add USB HW IDs for MT7921/MT7922/MT7925 > - Add support for Intel BlazarI and Filmore Peak2 (BE201) > - Add initial support for Intel PCIe driver > - Remove HCI_AMP support > - Add TX timestamping support There is one more warning in the Intel driver: drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' was not declared. Should it be static? It'd also be great to get an ACK from someone familiar with the socket time stamping (Willem?) I'm not sure there's sufficient detail in the commit message to explain the choices to: - change the definition of SCHED / SEND to mean queued / completed, while for Ethernet they mean queued to qdisc, queued to HW. How does it compare to stamping in the driver in terms of accuracy? - the "experimental" BT_POLL_ERRQUEUE, how does the user space look? What is the "upper layer"? What does it mean for kernel uAPI to be "experimental"? When does the "upper layer" get to run and how does it know that there are time stamps on the error queue? Would be great to get more info and/or second opinion, because it's not sufficiently "obviously right" to me to pull right away :(
Hi Jakub, On Mon, May 13, 2024 at 5:26 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 10 May 2024 17:14:28 -0400 Luiz Augusto von Dentz wrote: > > The following changes since commit f8beae078c82abde57fed4a5be0bbc3579b59ad0: > > > > Merge tag 'gtp-24-05-07' of git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp Pablo neira Ayuso says: (2024-05-10 13:59:27 +0100) > > > > are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git tags/for-net-next-2024-05-10 > > > > for you to fetch changes up to 75f819bdf9cafb0f1458e24c05d24eec17b2f597: > > > > Bluetooth: btintel: Fix compiler warning for multi_v7_defconfig config (2024-05-10 17:04:15 -0400) > > > > ---------------------------------------------------------------- > > bluetooth-next pull request for net-next: > > > > - Add support MediaTek MT7921S SDIO > > - Various fixes for -Wflex-array-member-not-at-end and -Wfamnae > > - Add USB HW IDs for MT7921/MT7922/MT7925 > > - Add support for Intel BlazarI and Filmore Peak2 (BE201) > > - Add initial support for Intel PCIe driver > > - Remove HCI_AMP support > > - Add TX timestamping support > > There is one more warning in the Intel driver: > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > was not declared. Should it be static? We have a fix for that but I was hoping to have it in before the merge window and then have the fix merged later. > It'd also be great to get an ACK from someone familiar with the socket > time stamping (Willem?) I'm not sure there's sufficient detail in the > commit message to explain the choices to: > - change the definition of SCHED / SEND to mean queued / completed, > while for Ethernet they mean queued to qdisc, queued to HW. hmm I thought this was hardware specific, it obviously won't work exactly as Ethernet since it is a completely different protocol stack, or are you suggesting we need other definitions for things like TX completed? > How does it compare to stamping in the driver in terms of accuracy? @Pauli any input here? > - the "experimental" BT_POLL_ERRQUEUE, how does the user space look? There are test cases in BlueZ: https://github.com/bluez/bluez/commit/141f66411ca488e26bdd64e6f858ffa190395d23 > What is the "upper layer"? What does it mean for kernel uAPI to be > "experimental"? When does the "upper layer" get to run and how does > it know that there are time stamps on the error queue? The socketopt only gets enabled with use of MGMT Set Experimental Feature Command: https://github.com/bluez/bluez/blob/master/doc/mgmt-api.txt#L3205 Anyway you can see on the tests how we are using it. > Would be great to get more info and/or second opinion, because it's not > sufficiently "obviously right" to me to pull right away :( Well I assumed sockopt starting with SO_ sort of means it applies that all socket families, in fact SO_TIMESTAMP already seem to work without these changes they just don't generate anything, so in a way we are just implementing a missing feature.
On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > There is one more warning in the Intel driver: > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > was not declared. Should it be static? > > We have a fix for that but I was hoping to have it in before the merge > window and then have the fix merged later. > > > It'd also be great to get an ACK from someone familiar with the socket > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > commit message to explain the choices to: > > - change the definition of SCHED / SEND to mean queued / completed, > > while for Ethernet they mean queued to qdisc, queued to HW. > > hmm I thought this was hardware specific, it obviously won't work > exactly as Ethernet since it is a completely different protocol stack, > or are you suggesting we need other definitions for things like TX > completed? I don't know anything about queuing in BT, in terms of timestamping the SEND - SCHED difference is supposed to indicate the level of host delay or host congestion. If the queuing in BT happens mostly in the device HW queue then it may make sense to generate SCHED when handing over to the driver. OTOH if the devices can coalesce or delay completions the completion timeout may be less accurate than stamping before submitting to HW... I'm looking for the analysis that the choices were well thought thru. > > How does it compare to stamping in the driver in terms of accuracy? > > @Pauli any input here? > > > - the "experimental" BT_POLL_ERRQUEUE, how does the user space look? > > There are test cases in BlueZ: > > https://github.com/bluez/bluez/commit/141f66411ca488e26bdd64e6f858ffa190395d23 > > > What is the "upper layer"? What does it mean for kernel uAPI to be > > "experimental"? When does the "upper layer" get to run and how does > > it know that there are time stamps on the error queue? > > The socketopt only gets enabled with use of MGMT Set Experimental > Feature Command: > > https://github.com/bluez/bluez/blob/master/doc/mgmt-api.txt#L3205 > > Anyway you can see on the tests how we are using it. Either I didn't grok the test or it doesn't answer my question. What is the lower layer that we want to "protect" from POLLERR? > > Would be great to get more info and/or second opinion, because it's not > > sufficiently "obviously right" to me to pull right away :( > > Well I assumed sockopt starting with SO_ sort of means it applies that > all socket families, in fact SO_TIMESTAMP already seem to work without > these changes they just don't generate anything, so in a way we are > just implementing a missing feature.
Jakub Kicinski wrote: > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > There is one more warning in the Intel driver: > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > was not declared. Should it be static? > > > > We have a fix for that but I was hoping to have it in before the merge > > window and then have the fix merged later. > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > commit message to explain the choices to: > > > - change the definition of SCHED / SEND to mean queued / completed, > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > hmm I thought this was hardware specific, it obviously won't work > > exactly as Ethernet since it is a completely different protocol stack, > > or are you suggesting we need other definitions for things like TX > > completed? > > I don't know anything about queuing in BT, in terms of timestamping > the SEND - SCHED difference is supposed to indicate the level of > host delay or host congestion. If the queuing in BT happens mostly in > the device HW queue then it may make sense to generate SCHED when > handing over to the driver. OTOH if the devices can coalesce or delay > completions the completion timeout may be less accurate than stamping > before submitting to HW... I'm looking for the analysis that the choices > were well thought thru. SCM_TSTAMP_SND is taken before an skb is passed to the device. This matches request SOF_TIMESTAMPING_TX_SOFTWARE. A timestamp returned on transmit completion is requested as SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software timestamp taken at tx completion cleaning. If anything, I would think it would be a passes as a hardware timestamp. Returning SCHED when queuing to a device and SND later on receiving completions seems like not following SO_TIMESTAMPING convention to me. But I don't fully know the HCI model. As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the ABI, right? So immutable. Is it fair to call that experimental? It might be safer to only suppress the sk_error_report in sock_queue_err_skb. Or at least in bt_sock_poll to check the type of all outstanding errors and only suppress if all are timestamps.
Hi Jakub, On Mon, May 13, 2024 at 6:43 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > There is one more warning in the Intel driver: > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > was not declared. Should it be static? > > > > We have a fix for that but I was hoping to have it in before the merge > > window and then have the fix merged later. > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > commit message to explain the choices to: > > > - change the definition of SCHED / SEND to mean queued / completed, > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > hmm I thought this was hardware specific, it obviously won't work > > exactly as Ethernet since it is a completely different protocol stack, > > or are you suggesting we need other definitions for things like TX > > completed? > > I don't know anything about queuing in BT, in terms of timestamping > the SEND - SCHED difference is supposed to indicate the level of > host delay or host congestion. If the queuing in BT happens mostly in > the device HW queue then it may make sense to generate SCHED when > handing over to the driver. OTOH if the devices can coalesce or delay > completions the completion timeout may be less accurate than stamping > before submitting to HW... I'm looking for the analysis that the choices > were well thought thru. I guess you want to know if is SCHED is done at enqueing (before submitting to the driver) or dequeing (after it has been submitted), right now it is the former, the said the driver should normally just submit the packets immediately since we do have events from HCI to informing when a buffer has been freed, so that tells HW queue situation, so the driver doesn't have any queueing. > > > How does it compare to stamping in the driver in terms of accuracy? > > > > @Pauli any input here? > > > > > - the "experimental" BT_POLL_ERRQUEUE, how does the user space look? > > > > There are test cases in BlueZ: > > > > https://github.com/bluez/bluez/commit/141f66411ca488e26bdd64e6f858ffa190395d23 > > > > > What is the "upper layer"? What does it mean for kernel uAPI to be > > > "experimental"? When does the "upper layer" get to run and how does > > > it know that there are time stamps on the error queue? > > > > The socketopt only gets enabled with use of MGMT Set Experimental > > Feature Command: > > > > https://github.com/bluez/bluez/blob/master/doc/mgmt-api.txt#L3205 > > > > Anyway you can see on the tests how we are using it. > > Either I didn't grok the test or it doesn't answer my question. > What is the lower layer that we want to "protect" from POLLERR? This is more or less explained on the cover letter: https://lore.kernel.org/linux-bluetooth/713b1d0333eb2f12e63bc8a7b8f423e1240abae0.camel@iki.fi/T/ The problem with POLLERR is that there are 2 processes (bluetoothd and pipewire) monitoring the same file descriptor, so it will keep waking up both processes to process the errqueue while only one is really reading those events (pipewire), bluetoothd is only really monitoring the sockets to know if there is a disconnection but it can't really process the errqueue otherwise pipewire would compete reading from the same queue. Anyway I wasn't sure this was the best approach thus we decided to go with something experimental until we have a better understanding how all of this should work.
Hi Willem, On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > Jakub Kicinski wrote: > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > There is one more warning in the Intel driver: > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > was not declared. Should it be static? > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > window and then have the fix merged later. > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > commit message to explain the choices to: > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > exactly as Ethernet since it is a completely different protocol stack, > > > or are you suggesting we need other definitions for things like TX > > > completed? > > > > I don't know anything about queuing in BT, in terms of timestamping > > the SEND - SCHED difference is supposed to indicate the level of > > host delay or host congestion. If the queuing in BT happens mostly in > > the device HW queue then it may make sense to generate SCHED when > > handing over to the driver. OTOH if the devices can coalesce or delay > > completions the completion timeout may be less accurate than stamping > > before submitting to HW... I'm looking for the analysis that the choices > > were well thought thru. > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > A timestamp returned on transmit completion is requested as > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > timestamp taken at tx completion cleaning. If anything, I would think > it would be a passes as a hardware timestamp. In that case I think we probably misinterpret it, at least I though that TX_HARDWARE would really be a hardware generated timestamp using it own clock, if you are saying that TX_HARDWARE is just marking the TX completion of the packet at the host then we can definitely align with the current exception, that said we do have a command to actually read out the actual timestamp from the BT controller, that is usually more precise since some of the connection do require usec precision which is something that can get skew by the processing of HCI events themselves, well I guess we use that if the controller supports it and if it doesn't then we do based on the host timestamp when processing the HCI event indicating the completion of the transmission. > Returning SCHED when queuing to a device and SND later on receiving > completions seems like not following SO_TIMESTAMPING convention to me. > But I don't fully know the HCI model. > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > ABI, right? So immutable. Is it fair to call that experimental? I guess you are referring to the fact that sockopt ID reserved to BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in the future, yes that is correct, but we can actually return ENOPROTOOPT as it current does: if (!bt_poll_errqueue_enabled()) return -ENOPROTOOPT Anyway I would be really happy to drop it so we don't have to worry about it later. > It might be safer to only suppress the sk_error_report in > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > all outstanding errors and only suppress if all are timestamps. Or perhaps we could actually do that via poll/epoll directly? Not that it would make it much simpler since the library tends to wrap the usage of poll/epoll but POLLERR meaning both errors or errqueue events is sort of the problem we are trying to figure out how to process them separately.
Luiz Augusto von Dentz wrote: > Hi Willem, > > On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > Jakub Kicinski wrote: > > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > > There is one more warning in the Intel driver: > > > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > > was not declared. Should it be static? > > > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > > window and then have the fix merged later. > > > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > > commit message to explain the choices to: > > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > > exactly as Ethernet since it is a completely different protocol stack, > > > > or are you suggesting we need other definitions for things like TX > > > > completed? > > > > > > I don't know anything about queuing in BT, in terms of timestamping > > > the SEND - SCHED difference is supposed to indicate the level of > > > host delay or host congestion. If the queuing in BT happens mostly in > > > the device HW queue then it may make sense to generate SCHED when > > > handing over to the driver. OTOH if the devices can coalesce or delay > > > completions the completion timeout may be less accurate than stamping > > > before submitting to HW... I'm looking for the analysis that the choices > > > were well thought thru. > > > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > > > A timestamp returned on transmit completion is requested as > > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > > timestamp taken at tx completion cleaning. If anything, I would think > > it would be a passes as a hardware timestamp. > > In that case I think we probably misinterpret it, at least I though > that TX_HARDWARE would really be a hardware generated timestamp using > it own clock It normally is. It is just read from the tx descriptor on completion. We really don't have a good model for a software timestamp taken at completion processing. It may be worthwhile more broadly, especially for devices that do not support true hardware timestamps. Perhaps we should add an SCM_TSTAMP_TXCOMPLETION for this case. And a new SOF_TIMESTAMPING option to go with it. Similar to what we did for SCM_STAMP_SCHED. > if you are saying that TX_HARDWARE is just marking the > TX completion of the packet at the host then we can definitely align > with the current exception, that said we do have a command to actually > read out the actual timestamp from the BT controller, that is usually > more precise since some of the connection do require usec precision > which is something that can get skew by the processing of HCI events > themselves, well I guess we use that if the controller supports it and > if it doesn't then we do based on the host timestamp when processing > the HCI event indicating the completion of the transmission. > > > Returning SCHED when queuing to a device and SND later on receiving > > completions seems like not following SO_TIMESTAMPING convention to me. > > But I don't fully know the HCI model. > > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > ABI, right? So immutable. Is it fair to call that experimental? > > I guess you are referring to the fact that sockopt ID reserved to > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > the future, yes that is correct, but we can actually return > ENOPROTOOPT as it current does: > > if (!bt_poll_errqueue_enabled()) > return -ENOPROTOOPT I see. Once applications rely on a feature, it can be hard to actually deprecate. But in this case it may be possible. > Anyway I would be really happy to drop it so we don't have to worry > about it later. > > > It might be safer to only suppress the sk_error_report in > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > all outstanding errors and only suppress if all are timestamps. > > Or perhaps we could actually do that via poll/epoll directly? Not that > it would make it much simpler since the library tends to wrap the > usage of poll/epoll but POLLERR meaning both errors or errqueue events > is sort of the problem we are trying to figure out how to process them > separately. The process would still be awoken, of course. If bluetoothd can just be modified to ignore the reports, that would indeed be easiest from a kernel PoV. > -- > Luiz Augusto von Dentz
Hi Jakub, On Mon, May 13, 2024 at 10:01 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Willem, > > On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > Jakub Kicinski wrote: > > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > > There is one more warning in the Intel driver: > > > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > > was not declared. Should it be static? > > > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > > window and then have the fix merged later. > > > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > > commit message to explain the choices to: > > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > > exactly as Ethernet since it is a completely different protocol stack, > > > > or are you suggesting we need other definitions for things like TX > > > > completed? > > > > > > I don't know anything about queuing in BT, in terms of timestamping > > > the SEND - SCHED difference is supposed to indicate the level of > > > host delay or host congestion. If the queuing in BT happens mostly in > > > the device HW queue then it may make sense to generate SCHED when > > > handing over to the driver. OTOH if the devices can coalesce or delay > > > completions the completion timeout may be less accurate than stamping > > > before submitting to HW... I'm looking for the analysis that the choices > > > were well thought thru. > > > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > > > A timestamp returned on transmit completion is requested as > > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > > timestamp taken at tx completion cleaning. If anything, I would think > > it would be a passes as a hardware timestamp. > > In that case I think we probably misinterpret it, at least I though > that TX_HARDWARE would really be a hardware generated timestamp using > it own clock, if you are saying that TX_HARDWARE is just marking the > TX completion of the packet at the host then we can definitely align > with the current exception, that said we do have a command to actually > read out the actual timestamp from the BT controller, that is usually > more precise since some of the connection do require usec precision > which is something that can get skew by the processing of HCI events > themselves, well I guess we use that if the controller supports it and > if it doesn't then we do based on the host timestamp when processing > the HCI event indicating the completion of the transmission. > > > Returning SCHED when queuing to a device and SND later on receiving > > completions seems like not following SO_TIMESTAMPING convention to me. > > But I don't fully know the HCI model. > > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > ABI, right? So immutable. Is it fair to call that experimental? > > I guess you are referring to the fact that sockopt ID reserved to > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > the future, yes that is correct, but we can actually return > ENOPROTOOPT as it current does: > > if (!bt_poll_errqueue_enabled()) > return -ENOPROTOOPT > > Anyway I would be really happy to drop it so we don't have to worry > about it later. > > > It might be safer to only suppress the sk_error_report in > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > all outstanding errors and only suppress if all are timestamps. > > Or perhaps we could actually do that via poll/epoll directly? Not that > it would make it much simpler since the library tends to wrap the > usage of poll/epoll but POLLERR meaning both errors or errqueue events > is sort of the problem we are trying to figure out how to process them > separately. @Jakub Kicinski I'm fine removing these from the pull request, or if you want to do it yourself, in order not to miss the merge window, then we can discuss it better and even put you and Willem on CC to review the upcoming changes.
Hi Willem, On Mon, May 13, 2024 at 10:09 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > Luiz Augusto von Dentz wrote: > > Hi Willem, > > > > On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > > > > Jakub Kicinski wrote: > > > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > > > There is one more warning in the Intel driver: > > > > > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > > > was not declared. Should it be static? > > > > > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > > > window and then have the fix merged later. > > > > > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > > > commit message to explain the choices to: > > > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > > > exactly as Ethernet since it is a completely different protocol stack, > > > > > or are you suggesting we need other definitions for things like TX > > > > > completed? > > > > > > > > I don't know anything about queuing in BT, in terms of timestamping > > > > the SEND - SCHED difference is supposed to indicate the level of > > > > host delay or host congestion. If the queuing in BT happens mostly in > > > > the device HW queue then it may make sense to generate SCHED when > > > > handing over to the driver. OTOH if the devices can coalesce or delay > > > > completions the completion timeout may be less accurate than stamping > > > > before submitting to HW... I'm looking for the analysis that the choices > > > > were well thought thru. > > > > > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > > > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > > > > > A timestamp returned on transmit completion is requested as > > > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > > > timestamp taken at tx completion cleaning. If anything, I would think > > > it would be a passes as a hardware timestamp. > > > > In that case I think we probably misinterpret it, at least I though > > that TX_HARDWARE would really be a hardware generated timestamp using > > it own clock > > It normally is. It is just read from the tx descriptor on completion. > > We really don't have a good model for a software timestamp taken at > completion processing. > > It may be worthwhile more broadly, especially for devices that do not > support true hardware timestamps. > > Perhaps we should add an SCM_TSTAMP_TXCOMPLETION for this case. And a > new SOF_TIMESTAMPING option to go with it. Similar to what we did for > SCM_STAMP_SCHED. > > > if you are saying that TX_HARDWARE is just marking the > > TX completion of the packet at the host then we can definitely align > > with the current exception, that said we do have a command to actually > > read out the actual timestamp from the BT controller, that is usually > > more precise since some of the connection do require usec precision > > which is something that can get skew by the processing of HCI events > > themselves, well I guess we use that if the controller supports it and > > if it doesn't then we do based on the host timestamp when processing > > the HCI event indicating the completion of the transmission. > > > > > Returning SCHED when queuing to a device and SND later on receiving > > > completions seems like not following SO_TIMESTAMPING convention to me. > > > But I don't fully know the HCI model. > > > > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > > ABI, right? So immutable. Is it fair to call that experimental? > > > > I guess you are referring to the fact that sockopt ID reserved to > > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > > the future, yes that is correct, but we can actually return > > ENOPROTOOPT as it current does: > > > > if (!bt_poll_errqueue_enabled()) > > return -ENOPROTOOPT > > I see. Once applications rely on a feature, it can be hard to actually > deprecate. But in this case it may be possible. > > > Anyway I would be really happy to drop it so we don't have to worry > > about it later. > > > > > It might be safer to only suppress the sk_error_report in > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > all outstanding errors and only suppress if all are timestamps. > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > it would make it much simpler since the library tends to wrap the > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > is sort of the problem we are trying to figure out how to process them > > separately. > > The process would still be awoken, of course. If bluetoothd can just > be modified to ignore the reports, that would indeed be easiest from > a kernel PoV. @Pauli Virtanen tried that but apparently it would keep waking up the process until the errqueue is fully read, maybe we are missing something, or glib is not really doing a good job wrt to poll/epoll handling.
> > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > > > ABI, right? So immutable. Is it fair to call that experimental? > > > > > > I guess you are referring to the fact that sockopt ID reserved to > > > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > > > the future, yes that is correct, but we can actually return > > > ENOPROTOOPT as it current does: > > > > > > if (!bt_poll_errqueue_enabled()) > > > return -ENOPROTOOPT > > > > I see. Once applications rely on a feature, it can be hard to actually > > deprecate. But in this case it may be possible. > > > > > Anyway I would be really happy to drop it so we don't have to worry > > > about it later. > > > > > > > It might be safer to only suppress the sk_error_report in > > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > > all outstanding errors and only suppress if all are timestamps. > > > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > > it would make it much simpler since the library tends to wrap the > > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > > is sort of the problem we are trying to figure out how to process them > > > separately. > > > > The process would still be awoken, of course. If bluetoothd can just > > be modified to ignore the reports, that would indeed be easiest from > > a kernel PoV. > > @Pauli Virtanen tried that but apparently it would keep waking up the > process until the errqueue is fully read, maybe we are missing > something, or glib is not really doing a good job wrt to poll/epoll > handling. Perhaps this is because poll is level triggered. Maybe epoll in edge triggered mode would avoid re-waking for the same outstanding events.
On Mon, 13 May 2024 22:12:04 -0400 Luiz Augusto von Dentz wrote: > > > It might be safer to only suppress the sk_error_report in > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > all outstanding errors and only suppress if all are timestamps. > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > it would make it much simpler since the library tends to wrap the > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > is sort of the problem we are trying to figure out how to process them > > separately. > > @Jakub Kicinski I'm fine removing these from the pull request, or if > you want to do it yourself, in order not to miss the merge window, > then we can discuss it better and even put you and Willem on CC to > review the upcoming changes. Sounds like the best way forward, thanks! Could you drop them and resend the PR? It's going to take me until noon pacific to write up the PR text for all of net-next, to give you a sense of when our PR will come out.
Hi Jakub, On Tue, May 14, 2024 at 10:34 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 13 May 2024 22:12:04 -0400 Luiz Augusto von Dentz wrote: > > > > It might be safer to only suppress the sk_error_report in > > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > > all outstanding errors and only suppress if all are timestamps. > > > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > > it would make it much simpler since the library tends to wrap the > > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > > is sort of the problem we are trying to figure out how to process them > > > separately. > > > > @Jakub Kicinski I'm fine removing these from the pull request, or if > > you want to do it yourself, in order not to miss the merge window, > > then we can discuss it better and even put you and Willem on CC to > > review the upcoming changes. > > Sounds like the best way forward, thanks! > Could you drop them and resend the PR? > > It's going to take me until noon pacific to write up the PR text > for all of net-next, to give you a sense of when our PR will come > out. Perfect, will also make sure to include the btintel_pcie fix for the warning and drop the entire set implementing support SO_TIMESTAMPING.
Hi all, Thanks for the comments, I guess the original series should have been Cc'd more widely to get them earlier. ma, 2024-05-13 kello 22:09 -0400, Willem de Bruijn kirjoitti: > Luiz Augusto von Dentz wrote: > > Hi Willem, > > > > On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > > > > Jakub Kicinski wrote: > > > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > > > There is one more warning in the Intel driver: > > > > > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > > > was not declared. Should it be static? > > > > > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > > > window and then have the fix merged later. > > > > > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > > > commit message to explain the choices to: > > > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > > > exactly as Ethernet since it is a completely different protocol stack, > > > > > or are you suggesting we need other definitions for things like TX > > > > > completed? > > > > > > > > I don't know anything about queuing in BT, in terms of timestamping > > > > the SEND - SCHED difference is supposed to indicate the level of > > > > host delay or host congestion. If the queuing in BT happens mostly in > > > > the device HW queue then it may make sense to generate SCHED when > > > > handing over to the driver. OTOH if the devices can coalesce or delay > > > > completions the completion timeout may be less accurate than stamping > > > > before submitting to HW... I'm looking for the analysis that the choices > > > > were well thought thru. > > > > > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > > > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > > > > > A timestamp returned on transmit completion is requested as > > > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > > > timestamp taken at tx completion cleaning. If anything, I would think > > > it would be a passes as a hardware timestamp. > > > > In that case I think we probably misinterpret it, at least I though > > that TX_HARDWARE would really be a hardware generated timestamp using > > it own clock > > It normally is. It is just read from the tx descriptor on completion. > > We really don't have a good model for a software timestamp taken at > completion processing. > > It may be worthwhile more broadly, especially for devices that do not > support true hardware timestamps. > > Perhaps we should add an SCM_TSTAMP_TXCOMPLETION for this case. And a > new SOF_TIMESTAMPING option to go with it. Similar to what we did for > SCM_STAMP_SCHED. Ok, I was also under the impression TX_HARDWARE was only for actual HW timestamps. TSTAMP_ACK appeared to not really match semantics either, so TSTAMP_SND it then was. The general timestamping flow here was: sendmsg() from user generates skbs to net/bluetooth side queue | * wait in net/bluetooth side queue until HW has free packet slot | * send to driver (-> SCM_TSTAMP_SCHED*) | * driver (usu. ASAP) queues to transport e.g. USB | * transport tx complete, skb freed | * packet waits in hardware-side buffers (usu. the largest delay) | * packet completion report from HW (-> SCM_TSTAMP_SND*) | * for one packet type, HW timestamp for last tx packet can queried The packet completion report does not imply the packet was received. From the above, I gather SCHED* should be SND, and SND* should be TXCOMPLETION. Then I'm not sure when we should generate SCHED, if at all, unless it's done more or less in sendmsg() when it generates the skbs. Possibly the SND timestamp could also be generated on driver side if one wants to have it taken at transport tx completion. I don't immediately know what is the use case would be though, as the packet may still have to wait on HW side before it goes over the air. For the use case here, we want to know the total latency, so the completion timestamp is the interesting one. In the audio use case, in normal operation there is a free HW slot and packets do not wait in net/bluetooth queues but end up in HW buffers ASAP (fast, maybe < 1 ms), and then wait a much longer time (usu. 5-50 ms) in the HW buffers before it reports completion. > > if you are saying that TX_HARDWARE is just marking the > > TX completion of the packet at the host then we can definitely align > > with the current exception, that said we do have a command to actually > > read out the actual timestamp from the BT controller, that is usually > > more precise since some of the connection do require usec precision > > which is something that can get skew by the processing of HCI events > > themselves, well I guess we use that if the controller supports it and > > if it doesn't then we do based on the host timestamp when processing > > the HCI event indicating the completion of the transmission. > > > > > Returning SCHED when queuing to a device and SND later on receiving > > > completions seems like not following SO_TIMESTAMPING convention to me. > > > But I don't fully know the HCI model. > > > > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > > ABI, right? So immutable. Is it fair to call that experimental? > > > > I guess you are referring to the fact that sockopt ID reserved to > > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > > the future, yes that is correct, but we can actually return > > ENOPROTOOPT as it current does: > > > > if (!bt_poll_errqueue_enabled()) > > return -ENOPROTOOPT > > I see. Once applications rely on a feature, it can be hard to actually > deprecate. But in this case it may be possible. > > > Anyway I would be really happy to drop it so we don't have to worry > > about it later. > > > > > It might be safer to only suppress the sk_error_report in > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > all outstanding errors and only suppress if all are timestamps. > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > it would make it much simpler since the library tends to wrap the > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > is sort of the problem we are trying to figure out how to process them > > separately. > > The process would still be awoken, of course. If bluetoothd can just > be modified to ignore the reports, that would indeed be easiest from > a kernel PoV. This can be done on bluetoothd side, the ugly part is just the wakeup on every TX timestamp, which is every ~10ms in these use cases if every packet is stamped. EPOLLET probably would indeed avoid busy looping on the same timestamp though. In the first round of this patchset, this was handled on bluetoothd side without kernel additions, with rate limiting the polling. If POLL_ERRQUEUE sounds like a bad idea, maybe we can go back to that.
Pauli Virtanen wrote: > Hi all, > > Thanks for the comments, I guess the original series should have been > Cc'd more widely to get them earlier. > > ma, 2024-05-13 kello 22:09 -0400, Willem de Bruijn kirjoitti: > > Luiz Augusto von Dentz wrote: > > > Hi Willem, > > > > > > On Mon, May 13, 2024 at 9:32 PM Willem de Bruijn > > > <willemdebruijn.kernel@gmail.com> wrote: > > > > > > > > Jakub Kicinski wrote: > > > > > On Mon, 13 May 2024 18:09:31 -0400 Luiz Augusto von Dentz wrote: > > > > > > > There is one more warning in the Intel driver: > > > > > > > > > > > > > > drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list' > > > > > > > was not declared. Should it be static? > > > > > > > > > > > > We have a fix for that but I was hoping to have it in before the merge > > > > > > window and then have the fix merged later. > > > > > > > > > > > > > It'd also be great to get an ACK from someone familiar with the socket > > > > > > > time stamping (Willem?) I'm not sure there's sufficient detail in the > > > > > > > commit message to explain the choices to: > > > > > > > - change the definition of SCHED / SEND to mean queued / completed, > > > > > > > while for Ethernet they mean queued to qdisc, queued to HW. > > > > > > > > > > > > hmm I thought this was hardware specific, it obviously won't work > > > > > > exactly as Ethernet since it is a completely different protocol stack, > > > > > > or are you suggesting we need other definitions for things like TX > > > > > > completed? > > > > > > > > > > I don't know anything about queuing in BT, in terms of timestamping > > > > > the SEND - SCHED difference is supposed to indicate the level of > > > > > host delay or host congestion. If the queuing in BT happens mostly in > > > > > the device HW queue then it may make sense to generate SCHED when > > > > > handing over to the driver. OTOH if the devices can coalesce or delay > > > > > completions the completion timeout may be less accurate than stamping > > > > > before submitting to HW... I'm looking for the analysis that the choices > > > > > were well thought thru. > > > > > > > > SCM_TSTAMP_SND is taken before an skb is passed to the device. > > > > This matches request SOF_TIMESTAMPING_TX_SOFTWARE. > > > > > > > > A timestamp returned on transmit completion is requested as > > > > SOF_TIMESTAMPING_TX_HARDWARE. We do not have a type for a software > > > > timestamp taken at tx completion cleaning. If anything, I would think > > > > it would be a passes as a hardware timestamp. > > > > > > In that case I think we probably misinterpret it, at least I though > > > that TX_HARDWARE would really be a hardware generated timestamp using > > > it own clock > > > > It normally is. It is just read from the tx descriptor on completion. > > > > We really don't have a good model for a software timestamp taken at > > completion processing. > > > > It may be worthwhile more broadly, especially for devices that do not > > support true hardware timestamps. > > > > Perhaps we should add an SCM_TSTAMP_TXCOMPLETION for this case. And a > > new SOF_TIMESTAMPING option to go with it. Similar to what we did for > > SCM_STAMP_SCHED. > > Ok, I was also under the impression TX_HARDWARE was only for actual HW > timestamps. TSTAMP_ACK appeared to not really match semantics either, > so TSTAMP_SND it then was. > > > The general timestamping flow here was: > > sendmsg() from user generates skbs to net/bluetooth side queue > | > * wait in net/bluetooth side queue until HW has free packet slot > | > * send to driver (-> SCM_TSTAMP_SCHED*) > | > * driver (usu. ASAP) queues to transport e.g. USB > | > * transport tx complete, skb freed > | > * packet waits in hardware-side buffers (usu. the largest delay) > | > * packet completion report from HW (-> SCM_TSTAMP_SND*) > | > * for one packet type, HW timestamp for last tx packet can queried > > The packet completion report does not imply the packet was received. > > From the above, I gather SCHED* should be SND, and SND* should be > TXCOMPLETION. Then I'm not sure when we should generate SCHED, if at > all, unless it's done more or less in sendmsg() when it generates the > skbs. Agreed. Missing SCHED if packets do not go through software queuing should be fine. Inversely, with multiple qdiscs processes see multiple SCHED events. > Possibly the SND timestamp could also be generated on driver side if > one wants to have it taken at transport tx completion. I don't > immediately know what is the use case would be though, as the packet > may still have to wait on HW side before it goes over the air. > > For the use case here, we want to know the total latency, so the > completion timestamp is the interesting one. In the audio use case, in > normal operation there is a free HW slot and packets do not wait in > net/bluetooth queues but end up in HW buffers ASAP (fast, maybe < 1 > ms), and then wait a much longer time (usu. 5-50 ms) in the HW buffers > before it reports completion. > > > > if you are saying that TX_HARDWARE is just marking the > > > TX completion of the packet at the host then we can definitely align > > > with the current exception, that said we do have a command to actually > > > read out the actual timestamp from the BT controller, that is usually > > > more precise since some of the connection do require usec precision > > > which is something that can get skew by the processing of HCI events > > > themselves, well I guess we use that if the controller supports it and > > > if it doesn't then we do based on the host timestamp when processing > > > the HCI event indicating the completion of the transmission. > > > > > > > Returning SCHED when queuing to a device and SND later on receiving > > > > completions seems like not following SO_TIMESTAMPING convention to me. > > > > But I don't fully know the HCI model. > > > > > > > > As for the "experimental" BT_POLL_ERRQUEUE. This is an addition to the > > > > ABI, right? So immutable. Is it fair to call that experimental? > > > > > > I guess you are referring to the fact that sockopt ID reserved to > > > BT_POLL_ERRQUEUE cannot be reused anymore even if we drop its usage in > > > the future, yes that is correct, but we can actually return > > > ENOPROTOOPT as it current does: > > > > > > if (!bt_poll_errqueue_enabled()) > > > return -ENOPROTOOPT > > > > I see. Once applications rely on a feature, it can be hard to actually > > deprecate. But in this case it may be possible. > > > > > Anyway I would be really happy to drop it so we don't have to worry > > > about it later. > > > > > > > It might be safer to only suppress the sk_error_report in > > > > sock_queue_err_skb. Or at least in bt_sock_poll to check the type of > > > > all outstanding errors and only suppress if all are timestamps. > > > > > > Or perhaps we could actually do that via poll/epoll directly? Not that > > > it would make it much simpler since the library tends to wrap the > > > usage of poll/epoll but POLLERR meaning both errors or errqueue events > > > is sort of the problem we are trying to figure out how to process them > > > separately. > > > > The process would still be awoken, of course. If bluetoothd can just > > be modified to ignore the reports, that would indeed be easiest from > > a kernel PoV. > > This can be done on bluetoothd side, the ugly part is just the wakeup > on every TX timestamp, which is every ~10ms in these use cases if every > packet is stamped. EPOLLET probably would indeed avoid busy looping on > the same timestamp though. > > In the first round of this patchset, this was handled on bluetoothd > side without kernel additions, with rate limiting the polling. If > POLL_ERRQUEUE sounds like a bad idea, maybe we can go back to that. We have prior art to avoid having timestamp completions on MSG_ERRQUEUE set sk_err and so block normal processing. Additional work on opting out of timestamp/zerocopy wake-ups sounds reasonable to me.
The following changes since commit f8beae078c82abde57fed4a5be0bbc3579b59ad0: Merge tag 'gtp-24-05-07' of git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp Pablo neira Ayuso says: (2024-05-10 13:59:27 +0100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git tags/for-net-next-2024-05-10 for you to fetch changes up to 75f819bdf9cafb0f1458e24c05d24eec17b2f597: Bluetooth: btintel: Fix compiler warning for multi_v7_defconfig config (2024-05-10 17:04:15 -0400) ---------------------------------------------------------------- bluetooth-next pull request for net-next: - Add support MediaTek MT7921S SDIO - Various fixes for -Wflex-array-member-not-at-end and -Wfamnae - Add USB HW IDs for MT7921/MT7922/MT7925 - Add support for Intel BlazarI and Filmore Peak2 (BE201) - Add initial support for Intel PCIe driver - Remove HCI_AMP support - Add TX timestamping support ---------------------------------------------------------------- Archie Pusaka (1): Bluetooth: Populate hci_set_hw_info for Intel and Realtek Chen-Yu Tsai (1): dt-bindings: net: bluetooth: Add MediaTek MT7921S SDIO Bluetooth Dan Carpenter (1): Bluetooth: qca: Fix error code in qca_read_fw_build_info() Gustavo A. R. Silva (6): Bluetooth: L2CAP: Avoid -Wflex-array-member-not-at-end warnings Bluetooth: hci_conn, hci_sync: Use __counted_by() to avoid -Wfamnae warnings Bluetooth: hci_conn: Use __counted_by() to avoid -Wfamnae warning Bluetooth: hci_conn: Use struct_size() in hci_le_big_create_sync() Bluetooth: hci_sync: Use cmd->num_cis instead of magic number Bluetooth: hci_conn: Use __counted_by() and avoid -Wfamnae warning Hans de Goede (1): Bluetooth: hci_bcm: Limit bcm43455 baudrate to 2000000 Ian W MORRISON (1): Bluetooth: Add support for MediaTek MT7922 device Iulia Tanasescu (2): Bluetooth: ISO: Make iso_get_sock_listen generic Bluetooth: ISO: Handle PA sync when no BIGInfo reports are generated Jiande Lu (2): Bluetooth: btusb: Add USB HW IDs for MT7921/MT7922/MT7925 Bluetooth: btusb: Sort usb_device_id table by the ID Johan Hovold (3): Bluetooth: qca: drop bogus edl header checks Bluetooth: qca: drop bogus module version Bluetooth: qca: clean up defines Kiran K (8): Bluetooth: btintel: Define macros for image types Bluetooth: btintel: Add support to download intermediate loader Bluetooth: btintel: Add support for BlazarI Bluetooth: btintel: Add support for Filmore Peak2 (BE201) Bluetooth: btintel: Export few static functions Bluetooth: btintel_pcie: Add *setup* function to download firmware Bluetooth: btintel_pcie: Fix compiler warnings Bluetooth: btintel: Fix compiler warning for multi_v7_defconfig config Luiz Augusto von Dentz (3): Bluetooth: Add proper definitions for scan interval and window Bluetooth: hci_event: Set DISCOVERY_FINDING on SCAN_ENABLED Bluetooth: HCI: Remove HCI_AMP support Mahesh Talewad (1): LE Create Connection command timeout increased to 20 secs Marek Vasut (1): dt-bindings: net: broadcom-bluetooth: Add CYW43439 DT binding Pauli Virtanen (5): Bluetooth: add support for skb TX timestamping Bluetooth: ISO: add TX timestamping Bluetooth: L2CAP: add TX timestamping Bluetooth: SCO: add TX timestamping Bluetooth: add experimental BT_POLL_ERRQUEUE socket option Peter Tsao (1): Bluetooth: btusb: Fix the patch for MT7920 the affected to MT7921 Sebastian Urban (1): Bluetooth: compute LE flow credits based on recvbuf space Sungwoo Kim (1): Bluetooth: L2CAP: Fix div-by-zero in l2cap_le_flowctl_init() Tedd Ho-Jeong An (1): Bluetooth: btintel_pcie: Add support for PCIe transport Uri Arev (2): Bluetooth: hci_intel: Fix multiple issues reported by checkpatch.pl Bluetooth: ath3k: Fix multiple issues reported by checkpatch.pl Uwe Kleine-König (3): Bluetooth: btqcomsmd: Convert to platform remove callback returning void Bluetooth: hci_bcm: Convert to platform remove callback returning void Bluetooth: hci_intel: Convert to platform remove callback returning void Zijun Hu (4): Bluetooth: btusb: Correct timeout macro argument used to receive control message Bluetooth: hci_conn: Remove a redundant check for HFP offload Bluetooth: Remove 3 repeated macro definitions Bluetooth: qca: Support downloading board id specific NVM for WCN7850 .../net/bluetooth/mediatek,mt7921s-bluetooth.yaml | 55 + .../bindings/net/broadcom-bluetooth.yaml | 33 +- MAINTAINERS | 1 + drivers/bluetooth/Kconfig | 11 + drivers/bluetooth/Makefile | 1 + drivers/bluetooth/ath3k.c | 25 +- drivers/bluetooth/btintel.c | 88 +- drivers/bluetooth/btintel.h | 51 +- drivers/bluetooth/btintel_pcie.c | 1358 ++++++++++++++++++++ drivers/bluetooth/btintel_pcie.h | 430 +++++++ drivers/bluetooth/btmrvl_main.c | 9 - drivers/bluetooth/btqca.c | 47 +- drivers/bluetooth/btqca.h | 60 +- drivers/bluetooth/btqcomsmd.c | 6 +- drivers/bluetooth/btrsi.c | 1 - drivers/bluetooth/btrtl.c | 7 + drivers/bluetooth/btsdio.c | 8 - drivers/bluetooth/btusb.c | 55 +- drivers/bluetooth/hci_bcm.c | 8 +- drivers/bluetooth/hci_bcm4377.c | 1 - drivers/bluetooth/hci_intel.c | 25 +- drivers/bluetooth/hci_ldisc.c | 6 - drivers/bluetooth/hci_serdev.c | 5 - drivers/bluetooth/hci_uart.h | 1 - drivers/bluetooth/hci_vhci.c | 10 +- drivers/bluetooth/virtio_bt.c | 2 - include/net/bluetooth/bluetooth.h | 15 +- include/net/bluetooth/hci.h | 136 +- include/net/bluetooth/hci_core.h | 80 +- include/net/bluetooth/l2cap.h | 36 +- include/uapi/linux/virtio_bt.h | 1 - net/bluetooth/6lowpan.c | 2 +- net/bluetooth/af_bluetooth.c | 119 +- net/bluetooth/hci_conn.c | 261 +++- net/bluetooth/hci_core.c | 178 +-- net/bluetooth/hci_event.c | 244 +--- net/bluetooth/hci_request.h | 4 - net/bluetooth/hci_sock.c | 5 +- net/bluetooth/hci_sync.c | 196 +-- net/bluetooth/iso.c | 183 +-- net/bluetooth/l2cap_core.c | 151 ++- net/bluetooth/l2cap_sock.c | 114 +- net/bluetooth/mgmt.c | 149 ++- net/bluetooth/sco.c | 33 +- net/bluetooth/smp.c | 2 +- 45 files changed, 3046 insertions(+), 1167 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/bluetooth/mediatek,mt7921s-bluetooth.yaml create mode 100644 drivers/bluetooth/btintel_pcie.c create mode 100644 drivers/bluetooth/btintel_pcie.h