netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
@ 2025-07-07  2:38 Yang Li via B4 Relay
  2025-07-15  5:24 ` Yang Li
  2025-07-15 13:30 ` Pauli Virtanen
  0 siblings, 2 replies; 6+ messages in thread
From: Yang Li via B4 Relay @ 2025-07-07  2:38 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: linux-bluetooth, netdev, linux-kernel, Yang Li

From: Yang Li <yang.li@amlogic.com>

User-space applications (e.g. PipeWire) depend on
ISO-formatted timestamps for precise audio sync.

The ISO ts is based on the controller’s clock domain,
so hardware timestamping (hwtimestamp) must be used.

Ref: Documentation/networking/timestamping.rst,
section 3.1 Hardware Timestamping.

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
Changes in v4:
- Optimizing the code
- Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com

Changes in v3:
- Change to use hwtimestamp
- Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com

Changes in v2:
- Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
- Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
---
 net/bluetooth/iso.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index fc22782cbeeb..677144bb6b94 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
 void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 {
 	struct iso_conn *conn = hcon->iso_data;
+	struct skb_shared_hwtstamps *hwts;
 	__u16 pb, ts, len;
 
 	if (!conn)
@@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 		if (ts) {
 			struct hci_iso_ts_data_hdr *hdr;
 
-			/* TODO: add timestamp to the packet? */
 			hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
 			if (!hdr) {
 				BT_ERR("Frame is too short (len %d)", skb->len);
 				goto drop;
 			}
 
+			/*  Record the timestamp to skb*/
+			hwts = skb_hwtstamps(skb);
+			hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
+
 			len = __le16_to_cpu(hdr->slen);
 		} else {
 			struct hci_iso_data_hdr *hdr;

---
base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
change-id: 20250421-iso_ts-c82a300ae784

Best regards,
-- 
Yang Li <yang.li@amlogic.com>



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
  2025-07-07  2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
@ 2025-07-15  5:24 ` Yang Li
  2025-07-15 13:30 ` Pauli Virtanen
  1 sibling, 0 replies; 6+ messages in thread
From: Yang Li @ 2025-07-15  5:24 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: linux-bluetooth, netdev, linux-kernel

Hi,

Just a gentle ping regarding this patch.

Best regards,

Yang

> [ EXTERNAL EMAIL ]
>
> From: Yang Li <yang.li@amlogic.com>
>
> User-space applications (e.g. PipeWire) depend on
> ISO-formatted timestamps for precise audio sync.
>
> The ISO ts is based on the controller’s clock domain,
> so hardware timestamping (hwtimestamp) must be used.
>
> Ref: Documentation/networking/timestamping.rst,
> section 3.1 Hardware Timestamping.
>
> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
> Changes in v4:
> - Optimizing the code
> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
>
> Changes in v3:
> - Change to use hwtimestamp
> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com
>
> Changes in v2:
> - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
> - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
> ---
>   net/bluetooth/iso.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index fc22782cbeeb..677144bb6b94 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
>   void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>   {
>          struct iso_conn *conn = hcon->iso_data;
> +       struct skb_shared_hwtstamps *hwts;
>          __u16 pb, ts, len;
>
>          if (!conn)
> @@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>                  if (ts) {
>                          struct hci_iso_ts_data_hdr *hdr;
>
> -                       /* TODO: add timestamp to the packet? */
>                          hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
>                          if (!hdr) {
>                                  BT_ERR("Frame is too short (len %d)", skb->len);
>                                  goto drop;
>                          }
>
> +                       /*  Record the timestamp to skb*/
> +                       hwts = skb_hwtstamps(skb);
> +                       hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
> +
>                          len = __le16_to_cpu(hdr->slen);
>                  } else {
>                          struct hci_iso_data_hdr *hdr;
>
> ---
> base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
> change-id: 20250421-iso_ts-c82a300ae784
>
> Best regards,
> --
> Yang Li <yang.li@amlogic.com>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
  2025-07-07  2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
  2025-07-15  5:24 ` Yang Li
@ 2025-07-15 13:30 ` Pauli Virtanen
  2025-07-15 13:37   ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 6+ messages in thread
From: Pauli Virtanen @ 2025-07-15 13:30 UTC (permalink / raw)
  To: yang.li, Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: linux-bluetooth, netdev, linux-kernel

Hi Yang,

ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> From: Yang Li <yang.li@amlogic.com>
> 
> User-space applications (e.g. PipeWire) depend on
> ISO-formatted timestamps for precise audio sync.
> 
> The ISO ts is based on the controller’s clock domain,
> so hardware timestamping (hwtimestamp) must be used.
> 
> Ref: Documentation/networking/timestamping.rst,
> section 3.1 Hardware Timestamping.
> 
> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
> Changes in v4:
> - Optimizing the code
> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
> 
> Changes in v3:
> - Change to use hwtimestamp
> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com
> 
> Changes in v2:
> - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
> - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
> ---
>  net/bluetooth/iso.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index fc22782cbeeb..677144bb6b94 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
>  void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>  {
>  	struct iso_conn *conn = hcon->iso_data;
> +	struct skb_shared_hwtstamps *hwts;
>  	__u16 pb, ts, len;
>  
>  	if (!conn)
> @@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>  		if (ts) {
>  			struct hci_iso_ts_data_hdr *hdr;
>  
> -			/* TODO: add timestamp to the packet? */
>  			hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
>  			if (!hdr) {
>  				BT_ERR("Frame is too short (len %d)", skb->len);
>  				goto drop;
>  			}
>  
> +			/*  Record the timestamp to skb*/
> +			hwts = skb_hwtstamps(skb);
> +			hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));

Several lines below there is 

	conn->rx_skb = bt_skb_alloc(len, GFP_KERNEL);
	skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb-
>len),
        	                                  skb->len);

so timestamp should be copied explicitly also into conn->rx_skb,
otherwise it gets lost when you have ACL-fragmented ISO packets.

It could also be useful to write a simple test case that extracts the
timestamp from CMSG, see for example how it was done for BT_PKT_SEQNUM:
https://lore.kernel.org/linux-bluetooth/b98b7691e4ba06550bb8f275cad0635bc9e4e8d2.1752511478.git.pav@iki.fi/
bthost_send_iso() can take ts=true and some timestamp value.

> +
>  			len = __le16_to_cpu(hdr->slen);
>  		} else {
>  			struct hci_iso_data_hdr *hdr;
> 
> ---
> base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
> change-id: 20250421-iso_ts-c82a300ae784
> 
> Best regards,

-- 
Pauli Virtanen

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
  2025-07-15 13:30 ` Pauli Virtanen
@ 2025-07-15 13:37   ` Luiz Augusto von Dentz
  2025-07-15 13:57     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-15 13:37 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: yang.li, Marcel Holtmann, Johan Hedberg, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth, netdev, linux-kernel

Hi Pauli,

On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Yang,
>
> ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> > From: Yang Li <yang.li@amlogic.com>
> >
> > User-space applications (e.g. PipeWire) depend on
> > ISO-formatted timestamps for precise audio sync.
> >
> > The ISO ts is based on the controller’s clock domain,
> > so hardware timestamping (hwtimestamp) must be used.
> >
> > Ref: Documentation/networking/timestamping.rst,
> > section 3.1 Hardware Timestamping.
> >
> > Signed-off-by: Yang Li <yang.li@amlogic.com>
> > ---
> > Changes in v4:
> > - Optimizing the code
> > - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
> >
> > Changes in v3:
> > - Change to use hwtimestamp
> > - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com
> >
> > Changes in v2:
> > - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
> > - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
> > ---
> >  net/bluetooth/iso.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > index fc22782cbeeb..677144bb6b94 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
> >  void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> >  {
> >       struct iso_conn *conn = hcon->iso_data;
> > +     struct skb_shared_hwtstamps *hwts;
> >       __u16 pb, ts, len;
> >
> >       if (!conn)
> > @@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> >               if (ts) {
> >                       struct hci_iso_ts_data_hdr *hdr;
> >
> > -                     /* TODO: add timestamp to the packet? */
> >                       hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
> >                       if (!hdr) {
> >                               BT_ERR("Frame is too short (len %d)", skb->len);
> >                               goto drop;
> >                       }
> >
> > +                     /*  Record the timestamp to skb*/
> > +                     hwts = skb_hwtstamps(skb);
> > +                     hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
>
> Several lines below there is
>
>         conn->rx_skb = bt_skb_alloc(len, GFP_KERNEL);
>         skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb-
> >len),
>                                                   skb->len);
>
> so timestamp should be copied explicitly also into conn->rx_skb,
> otherwise it gets lost when you have ACL-fragmented ISO packets.

Yep, it is not that the code is completely wrong but it is operating
on the original skb not in the rx_skb as you said, that said is only
the first fragment that contains the ts header so we only have to do
it once in case that was not clear.

> It could also be useful to write a simple test case that extracts the
> timestamp from CMSG, see for example how it was done for BT_PKT_SEQNUM:
> https://lore.kernel.org/linux-bluetooth/b98b7691e4ba06550bb8f275cad0635bc9e4e8d2.1752511478.git.pav@iki.fi/
> bthost_send_iso() can take ts=true and some timestamp value.
>
> > +
> >                       len = __le16_to_cpu(hdr->slen);
> >               } else {
> >                       struct hci_iso_data_hdr *hdr;
> >
> > ---
> > base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
> > change-id: 20250421-iso_ts-c82a300ae784
> >
> > Best regards,
>
> --
> Pauli Virtanen



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
  2025-07-15 13:37   ` Luiz Augusto von Dentz
@ 2025-07-15 13:57     ` Luiz Augusto von Dentz
  2025-07-16  1:45       ` Yang Li
  0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-15 13:57 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: yang.li, Marcel Holtmann, Johan Hedberg, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	linux-bluetooth, netdev, linux-kernel

Hi,

On Tue, Jul 15, 2025 at 9:37 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Pauli,
>
> On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Hi Yang,
> >
> > ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> > > From: Yang Li <yang.li@amlogic.com>
> > >
> > > User-space applications (e.g. PipeWire) depend on
> > > ISO-formatted timestamps for precise audio sync.
> > >
> > > The ISO ts is based on the controller’s clock domain,
> > > so hardware timestamping (hwtimestamp) must be used.
> > >
> > > Ref: Documentation/networking/timestamping.rst,
> > > section 3.1 Hardware Timestamping.
> > >
> > > Signed-off-by: Yang Li <yang.li@amlogic.com>
> > > ---
> > > Changes in v4:
> > > - Optimizing the code
> > > - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
> > >
> > > Changes in v3:
> > > - Change to use hwtimestamp
> > > - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com
> > >
> > > Changes in v2:
> > > - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
> > > - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
> > > ---
> > >  net/bluetooth/iso.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > > index fc22782cbeeb..677144bb6b94 100644
> > > --- a/net/bluetooth/iso.c
> > > +++ b/net/bluetooth/iso.c
> > > @@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
> > >  void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > >  {
> > >       struct iso_conn *conn = hcon->iso_data;
> > > +     struct skb_shared_hwtstamps *hwts;
> > >       __u16 pb, ts, len;
> > >
> > >       if (!conn)
> > > @@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> > >               if (ts) {
> > >                       struct hci_iso_ts_data_hdr *hdr;
> > >
> > > -                     /* TODO: add timestamp to the packet? */
> > >                       hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
> > >                       if (!hdr) {
> > >                               BT_ERR("Frame is too short (len %d)", skb->len);
> > >                               goto drop;
> > >                       }
> > >
> > > +                     /*  Record the timestamp to skb*/
> > > +                     hwts = skb_hwtstamps(skb);
> > > +                     hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
> >
> > Several lines below there is
> >
> >         conn->rx_skb = bt_skb_alloc(len, GFP_KERNEL);
> >         skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb-
> > >len),
> >                                                   skb->len);
> >
> > so timestamp should be copied explicitly also into conn->rx_skb,
> > otherwise it gets lost when you have ACL-fragmented ISO packets.
>
> Yep, it is not that the code is completely wrong but it is operating
> on the original skb not in the rx_skb as you said, that said is only
> the first fragment that contains the ts header so we only have to do
> it once in case that was not clear.

I might just do a fixup myself, something like the following:

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 0a951c6514af..f48fb62e640d 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2374,6 +2374,13 @@ void iso_recv(struct hci_conn *hcon, struct
sk_buff *skb, u16 flags)
                skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
                                          skb->len);
                conn->rx_len = len - skb->len;
+
+               /* Copy timestamp from skb to rx_skb if present */
+               if (ts) {
+                       hwts = skb_hwtstamps(conn->rx_skb);
+                       hwts->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
+               }
+
                break;

        case ISO_CONT:


> > It could also be useful to write a simple test case that extracts the
> > timestamp from CMSG, see for example how it was done for BT_PKT_SEQNUM:
> > https://lore.kernel.org/linux-bluetooth/b98b7691e4ba06550bb8f275cad0635bc9e4e8d2.1752511478.git.pav@iki.fi/
> > bthost_send_iso() can take ts=true and some timestamp value.
> >
> > > +
> > >                       len = __le16_to_cpu(hdr->slen);
> > >               } else {
> > >                       struct hci_iso_data_hdr *hdr;
> > >
> > > ---
> > > base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
> > > change-id: 20250421-iso_ts-c82a300ae784
> > >
> > > Best regards,
> >
> > --
> > Pauli Virtanen
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
  2025-07-15 13:57     ` Luiz Augusto von Dentz
@ 2025-07-16  1:45       ` Yang Li
  0 siblings, 0 replies; 6+ messages in thread
From: Yang Li @ 2025-07-16  1:45 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Pauli Virtanen
  Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
	netdev, linux-kernel

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi,
>
> On Tue, Jul 15, 2025 at 9:37 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Pauli,
>>
>> On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
>>> Hi Yang,
>>>
>>> ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
>>>> From: Yang Li <yang.li@amlogic.com>
>>>>
>>>> User-space applications (e.g. PipeWire) depend on
>>>> ISO-formatted timestamps for precise audio sync.
>>>>
>>>> The ISO ts is based on the controller’s clock domain,
>>>> so hardware timestamping (hwtimestamp) must be used.
>>>>
>>>> Ref: Documentation/networking/timestamping.rst,
>>>> section 3.1 Hardware Timestamping.
>>>>
>>>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>>>> ---
>>>> Changes in v4:
>>>> - Optimizing the code
>>>> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
>>>>
>>>> Changes in v3:
>>>> - Change to use hwtimestamp
>>>> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.com
>>>>
>>>> Changes in v2:
>>>> - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
>>>> - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
>>>> ---
>>>>   net/bluetooth/iso.c | 6 +++++-
>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>>> index fc22782cbeeb..677144bb6b94 100644
>>>> --- a/net/bluetooth/iso.c
>>>> +++ b/net/bluetooth/iso.c
>>>> @@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
>>>>   void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>>>>   {
>>>>        struct iso_conn *conn = hcon->iso_data;
>>>> +     struct skb_shared_hwtstamps *hwts;
>>>>        __u16 pb, ts, len;
>>>>
>>>>        if (!conn)
>>>> @@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>>>>                if (ts) {
>>>>                        struct hci_iso_ts_data_hdr *hdr;
>>>>
>>>> -                     /* TODO: add timestamp to the packet? */
>>>>                        hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
>>>>                        if (!hdr) {
>>>>                                BT_ERR("Frame is too short (len %d)", skb->len);
>>>>                                goto drop;
>>>>                        }
>>>>
>>>> +                     /*  Record the timestamp to skb*/
>>>> +                     hwts = skb_hwtstamps(skb);
>>>> +                     hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
>>> Several lines below there is
>>>
>>>          conn->rx_skb = bt_skb_alloc(len, GFP_KERNEL);
>>>          skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb-
>>>> len),
>>>                                                    skb->len);
>>>
>>> so timestamp should be copied explicitly also into conn->rx_skb,
>>> otherwise it gets lost when you have ACL-fragmented ISO packets.
>> Yep, it is not that the code is completely wrong but it is operating
>> on the original skb not in the rx_skb as you said, that said is only
>> the first fragment that contains the ts header so we only have to do
>> it once in case that was not clear.
> I might just do a fixup myself, something like the following:
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 0a951c6514af..f48fb62e640d 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -2374,6 +2374,13 @@ void iso_recv(struct hci_conn *hcon, struct
> sk_buff *skb, u16 flags)
>                  skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
>                                            skb->len);
>                  conn->rx_len = len - skb->len;
> +
> +               /* Copy timestamp from skb to rx_skb if present */
> +               if (ts) {
> +                       hwts = skb_hwtstamps(conn->rx_skb);
> +                       hwts->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
> +               }
> +
>                  break;
>
>          case ISO_CONT:
>

Well, that's great!
Thanks so much for your help.
>>> It could also be useful to write a simple test case that extracts the
>>> timestamp from CMSG, see for example how it was done for BT_PKT_SEQNUM:
>>> https://lore.kernel.org/linux-bluetooth/b98b7691e4ba06550bb8f275cad0635bc9e4e8d2.1752511478.git.pav@iki.fi/
>>> bthost_send_iso() can take ts=true and some timestamp value.
>>>
>>>> +
>>>>                        len = __le16_to_cpu(hdr->slen);
>>>>                } else {
>>>>                        struct hci_iso_data_hdr *hdr;
>>>>
>>>> ---
>>>> base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
>>>> change-id: 20250421-iso_ts-c82a300ae784
>>>>
>>>> Best regards,
>>> --
>>> Pauli Virtanen
>>
>>
>> --
>> Luiz Augusto von Dentz
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-16  1:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07  2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
2025-07-15  5:24 ` Yang Li
2025-07-15 13:30 ` Pauli Virtanen
2025-07-15 13:37   ` Luiz Augusto von Dentz
2025-07-15 13:57     ` Luiz Augusto von Dentz
2025-07-16  1:45       ` Yang Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).