Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-3588-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240415144430.34450-1-vlad.pruteanu@nxp.com> <20240415144430.34450-2-vlad.pruteanu@nxp.com>
In-Reply-To: <20240415144430.34450-2-vlad.pruteanu@nxp.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Mon, 15 Apr 2024 11:06:45 -0400
Message-ID: <CABBYNZ+DXQf6UF-uHSwY4RSA5NjKaFZjEeQvjHWjj5zYL1OdJw@mail.gmail.com>
Subject: 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, claudia.rosu@nxp.com, 
	mihai-octavian.urzica@nxp.com, silviu.barbulescu@nxp.com, 
	iulia.tanasescu@nxp.com, andrei.istodorescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Vlad,

On Mon, Apr 15, 2024 at 10:45=E2=80=AFAM Vlad Pruteanu <vlad.pruteanu@nxp.c=
om> 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, so
we are probably missing the BN component here.

> ---
>  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->flags)=
;
>
> -       /* 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
>


--=20
Luiz Augusto von Dentz