Re: [PATCH] Bluetooth: ISO: add timestamp for outgoing HCI ISO packet

[Tailu Shi]

Hi Pauli,

Thanks for the detailed review and for sharing your RFC patches.

On Wed, 11 Mar 2026 18:24:23 +0200 Pauli Virtanen wrote:
> These timestamps are in controller clock domain, so generic host like
> Linux appears to need to perform clock synchronization via
> HCI_OP_LE_READ_ISO_TX_SYNC to be able to provide timestamps.

I agree that, according to Core v6.2 Vol 6 Part G Section 3, the timestamp
must be derived from the Controller's clock. However, Section 3.3 also defines
a falllback where the controller decides the CIS/BIS event when the
host‑supplied timestamp is not based on the Controller's clock. It looks
contradictory.

Section 3.3 further states that the host may retrieve the timestamp via
HCI_LE_Read_ISO_TX_Sync command. The wording makes it optional. And also
as you have noted, many controller returns 0 for it, leaving the practical
challenges to make it work properly.

That's why we considered leaving it to the userspace application.

> I had in the works a not fully finished patch series adding the same
> thing + LE Read ISO TX Sync + tests. I sent these now to the list in
> their current state, maybe they are useful if you are working on this
>
> https://lore.kernel.org/linux-bluetooth/cover.1773245666.git.pav@xxxxxx/T/
> 
> https://lore.kernel.org/linux-bluetooth/cover.1773245672.git.pav@xxxxxx/T/
> 
> The tests pass, but not really tested to satisfaction on real
> controllers.
> 
> From what I saw, it seems several controllers like Intel AX210 and some
> Realtek ones don't implement LE Read ISO TX Sync correctly (returns 0
> timestamp), so in those cases it AFAICS it's not possible to provide
> host timestamps.
> 
> Getting the clock sync via LE Read ISO TX Sync to work properly across
> controllers is maybe not so easy, as it appears the Core Specification
> is somewhat unclear how it should work.

I have reviewed the series you posted and I think it both complete and
elegant. Since my earlier work is now covered by your commit (08692f3...),
I will withdraw my patch.

I've cherry-picked your series and kept the PipeWire logic that passes the
timestamp via BT_SCM_PKT_ISO_TIMESTAMP CMSG. The implementation works on my
platform (based on the Kernel 7.0.0-rc1, BlueZ 5.85 and PipeWire 1.5.84).
The timestamp can be observed in the btmon trace.

Partial HCI parsed by Wireshark:
 Frame 32436: 72 bytes on wire (576 bits), 72 bytes captured (576 bits)
 Bluetooth
 Bluetooth Linux Monitor Transport
 Bluetooth HCI ISO Packet
     .... 1010 0000 0101 = Connection Handle: 0xa05
     ..10 .... .... .... = PB Flag: Complete SDU (0x2)
     .1.. .... .... .... = Timestamp present: True
     0... .... .... .... = Reserved: 0x0
     ..00 0000 0100 0100 = Data Length: 68
     Data
     [Connect in frame: 9436]
 Bluetooth ISO Data
     Timestamp: 469412333us
     Sequence Number: 3595
     .... 0000 0011 1100 = SDU Length: 60
     SDU

Please feel free to add the following Tested-by line if it helps:
Tested-by: Tailu Shi <tailu.shi@xxxxxxxxxxx>

May I know if you have any plan to merge this series? And also please feel
free to let me know if there's anything I can help to move this forward.

> This sends timestamps in host clock domain and has no clock sync, but I
> guess this is just for testing.

I think it's better to have a complete implementation solution. So if possible,
I would like to improve it and push it forward. Please feel free to let me
know if you have any other opinions.

> I think these are not needed, they can be passed as arguments to
> iso_send_frame().
> 
> Adding them to iso_pinfo() probably causes a data race if two processes
> sendmsg() at the same time.

Thank you for identifying this potential data race issue. It was not considered
in the current patch, and I will incorporate your suggestion in future development.

> I would put this in a separate function.

Agreed. It will make code clear. I will keep this code style in future development.


Thank you again for the detailed review!

Best regards!

Tailu