Hello Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Monday, April 15, 2024 6:07 PM
> To: Vlad Pruteanu <vlad.pruteanu@xxxxxxx>
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Claudia Cristina Draghicescu
> <claudia.rosu@xxxxxxx>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@xxxxxxx>; Silviu Florian Barbulescu
> <silviu.barbulescu@xxxxxxx>; Iulia Tanasescu <iulia.tanasescu@xxxxxxx>;
> Andrei Istodorescu <andrei.istodorescu@xxxxxxx>
> Subject: [EXT] Re: [PATCH 1/1] Bluetooth: hci_event: Fix setting of unicast qos
> interval
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi Vlad,
>
> On Mon, Apr 15, 2024 at 10:45 AM Vlad Pruteanu
> <vlad.pruteanu@xxxxxxx> wrote:
> >
> > qos->ucast interval reffers to the SDU interval, and should not
> > be set to the interval value reported by the LE CIS Established
> > event since the latter reffers to the ISO interval. These two
> > interval are not the same thing:
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 6, Part G
> >
> > Isochronous interval:
> > The time between two consecutive BIS or CIS events (designated
> > ISO_Interval in the Link Layer)
> >
> > SDU interval:
> > The nominal time between two consecutive SDUs that are sent or
> > received by the upper layer.
>
> I assume they are not the same because the ISO interval can have more
> than one subevents, but otherwise if BN=1 then it shall be aligned, so
> we are probably missing the BN component here.
>
I don't think that there's any need for setting the SDU Interval of the qos
here since it has already been set by the host prior to issuing the LE Set
CIG Parameters command, so the controller will have to respect that
value. Since the it has been set by the host, to be used by the controller,
to me, it seems a little bit redundant to derive the SDU Interval
once again based on parameters received on this event. I think that
continuing to use the initial value set by the Host should suffice.
> > ---
> > net/bluetooth/hci_event.c | 4 ----
> > 1 file changed, 4 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 868ffccff773..83cf0e8a56cf 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -6824,10 +6824,6 @@ static void hci_le_cis_estabilished_evt(struct
> hci_dev *hdev, void *data,
> >
> > pending = test_and_clear_bit(HCI_CONN_CREATE_CIS, &conn->flags);
> >
> > - /* Convert ISO Interval (1.25 ms slots) to SDU Interval (us) */
> > - qos->ucast.in.interval = le16_to_cpu(ev->interval) * 1250;
>
> This most likely needs to be le16_to_cpu(ev->interval) * 1250 *
> ev->bn, anyway it probably makes sense to indicate what the BN is
> causing this problem.
>
> > - qos->ucast.out.interval = qos->ucast.in.interval;
> >
> > switch (conn->role) {
> > case HCI_ROLE_SLAVE:
> > /* Convert Transport Latency (us) to Latency (msec) */
> > --
> > 2.40.1
> >
>
>
> --
> Luiz Augusto von Dentz
