Re: [PATCH v2] Bluetooth: ISO: Support SOCK_RCVTSTAMP via CMSG for ISO sockets

[Yang Li <yang.li@amlogic.com>]

Hi,
> [ EXTERNAL EMAIL ]
>
> Hi,
>
> ke, 2025-07-02 kello 19:35 +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.
>>
>> Signed-off-by: Yang Li <yang.li@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 | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>> index fc22782cbeeb..6927c593a1d6 100644
>> --- a/net/bluetooth/iso.c
>> +++ b/net/bluetooth/iso.c
>> @@ -2308,6 +2308,9 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>>                                goto drop;
>>                        }
>>
>> +                     /* Record the timestamp to skb*/
>> +                     skb->skb_mstamp_ns = le32_to_cpu(hdr->ts);
> Hardware timestamps are supposed to go in
>
>          skb_hwtstamps(skb)->hwtstamp
>
> See Documentation/networking/timestamping.rst
> "3.1 Hardware Timestamping Implementation: Device Drivers" and how it
> is done in drivers/net/
>
> This documentation also explains how user applications can obtain the
> hardware timestamps.
>
> AFAIK, skb->tstamp (skb->skb_mstamp_ns is union for it) must be in
> system clock. The hdr->ts is in some unsynchronized controller clock,
> so they should go to HW timestamps.


Following your suggestion, I switched to hwtstamp but kept 
SO_TIMESTAMPNS on the PipeWire side.

+                       struct skb_shared_hwtstamps *hwts = 
skb_hwtstamps(skb);
+                       if (hwts)
+                               hwts->hwtstamp = 
us_to_ktime(le32_to_cpu(hdr->ts));
+

The value I get is unexpectedly large and not the same as the timestamp 
in the ISO data.

read_data: received timestamp: 880608.479272966
read_data: received timestamp: 880608.479438633
read_data: received timestamp: 880608.489259466
read_data: received timestamp: 880608.489434550
read_data: received timestamp: 880608.499289258
read_data: received timestamp: 880608.499464550
read_data: received timestamp: 880608.509278008
read_data: received timestamp: 880608.509451425
read_data: received timestamp: 880608.519261175
read_data: received timestamp: 880608.519438633
read_data: received timestamp: 880608.529385008
read_data: received timestamp: 880608.529462133
read_data: received timestamp: 880608.539273758
read_data: received timestamp: 880608.539452758
read_data: received timestamp: 880608.549271258
read_data: received timestamp: 880608.549450008
read_data: received timestamp: 880608.559263466
read_data: received timestamp: 880608.559443216
read_data: received timestamp: 880608.569257466


Is there any special processing in the application code?

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