Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3627-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240415144430.34450-1-vlad.pruteanu@nxp.com> <20240415144430.34450-2-vlad.pruteanu@nxp.com>
 <CABBYNZ+DXQf6UF-uHSwY4RSA5NjKaFZjEeQvjHWjj5zYL1OdJw@mail.gmail.com> <VI1PR04MB53899910065D0817F54C5868FE082@VI1PR04MB5389.eurprd04.prod.outlook.com>
In-Reply-To: <VI1PR04MB53899910065D0817F54C5868FE082@VI1PR04MB5389.eurprd04.prod.outlook.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Tue, 16 Apr 2024 10:18:50 -0400
Message-ID: <CABBYNZLR9OBako_bBpJFx1A5nV-GYyzVczRb64vOcey2m_X2sw@mail.gmail.com>
Subject: Re: [EXT] Re: [PATCH 1/1] Bluetooth: hci_event: Fix setting of
 unicast qos interval
To: Vlad Pruteanu <vlad.pruteanu@nxp.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>, 
	Claudia Cristina Draghicescu <claudia.rosu@nxp.com>, 
	Mihai-Octavian Urzica <mihai-octavian.urzica@nxp.com>, 
	Silviu Florian Barbulescu <silviu.barbulescu@nxp.com>, Iulia Tanasescu <iulia.tanasescu@nxp.com>, 
	Andrei Istodorescu <andrei.istodorescu@nxp.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Vlad,

On Tue, Apr 16, 2024 at 6:22=E2=80=AFAM Vlad Pruteanu <vlad.pruteanu@nxp.co=
m> wrote:
>
> Hello Luiz,
>
> > -----Original Message-----
> > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> > Sent: Monday, April 15, 2024 6:07 PM
> > To: Vlad Pruteanu <vlad.pruteanu@nxp.com>
> > Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> > <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> > octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> > <silviu.barbulescu@nxp.com>; Iulia Tanasescu <iulia.tanasescu@nxp.com>;
> > Andrei Istodorescu <andrei.istodorescu@nxp.com>
> > Subject: [EXT] Re: [PATCH 1/1] Bluetooth: hci_event: Fix setting of uni=
cast qos
> > interval
> >
> > Caution: This is an external email. Please take care when clicking link=
s or
> > opening attachments. When in doubt, report the message using the 'Repor=
t
> > this email' button
> >
> >
> > Hi Vlad,
> >
> > On Mon, Apr 15, 2024 at 10:45=E2=80=AFAM Vlad Pruteanu
> > <vlad.pruteanu@nxp.com> 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=3D1 then it shall be aligned, s=
o
> > we are probably missing the BN component here.
> >
> I don't think that there's any need for setting the SDU Interval of the q=
os
> here since it has already been set by the host prior to issuing the LE Se=
t
> 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 controlle=
r,
> 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.

Yeah but how about the receiver case? Or you expected that we set the
QoS settings as a server as well? We need to confirm that this works
in both directions or actually I don't think this would work with the
likes of iso-test/isotester because there is no BAP layer seating
above it to configure the SDU interval it really needs to come from
the ISO socket itself.

> > > ---
> > >  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 =3D test_and_clear_bit(HCI_CONN_CREATE_CIS, &conn->fl=
ags);
> > >
> > > -       /* Convert ISO Interval (1.25 ms slots) to SDU Interval (us) =
*/
> > > -       qos->ucast.in.interval =3D 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 =3D 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


--=20
Luiz Augusto von Dentz