Re: [PATCH v5] Bluetooth: hci_event: Fix parsing of CIS Established Event

[Paul Menzel <pmenzel@xxxxxxxxxxxxx>]

Dear Luiz,


Thank you for your patch.

Am 27.06.23 um 02:31 schrieb Luiz Augusto von Dentz:
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> The ISO Interval on CIS Established Event uses 1.25 ms slots:
>
>      BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
>      page 2304:
>
>        Time = N * 1.25 ms
>
> In addition to that this always update the QoS settings based on CIS
> Established Event.

update*s*

Does “always” mean, it wasn’t set for the master role before, and that 
is done now?


Kind regards,

Paul


> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> ---
>   net/bluetooth/hci_event.c | 45 ++++++++++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index b1aefe4bb751..77cbf13037b3 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6822,6 +6822,7 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
>   {
>   	struct hci_evt_le_cis_established *ev = data;
>   	struct hci_conn *conn;
> +	struct bt_iso_qos *qos;
>   	bool pending = false;
>   	u16 handle = __le16_to_cpu(ev->handle);
>   
> @@ -6846,21 +6847,39 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
>   
>   	pending = test_and_clear_bit(HCI_CONN_CREATE_CIS, &conn->flags);
>   
> -	if (conn->role == HCI_ROLE_SLAVE) {
> -		__le32 interval;
> +	qos = &conn->iso_qos;
>   
> -		memset(&interval, 0, sizeof(interval));
> +	/* Convert ISO Interval (1.25 ms slots) to SDU Interval (us) */
> +	qos->ucast.in.interval = le16_to_cpu(ev->interval) * 1250;
> +	qos->ucast.out.interval = qos->ucast.in.interval;
>   
> -		memcpy(&interval, ev->c_latency, sizeof(ev->c_latency));
> -		conn->iso_qos.ucast.in.interval = le32_to_cpu(interval);
> -		memcpy(&interval, ev->p_latency, sizeof(ev->p_latency));
> -		conn->iso_qos.ucast.out.interval = le32_to_cpu(interval);
> -		conn->iso_qos.ucast.in.latency = le16_to_cpu(ev->interval);
> -		conn->iso_qos.ucast.out.latency = le16_to_cpu(ev->interval);
> -		conn->iso_qos.ucast.in.sdu = le16_to_cpu(ev->c_mtu);
> -		conn->iso_qos.ucast.out.sdu = le16_to_cpu(ev->p_mtu);
> -		conn->iso_qos.ucast.in.phy = ev->c_phy;
> -		conn->iso_qos.ucast.out.phy = ev->p_phy;
> +	switch (conn->role) {
> +	case HCI_ROLE_SLAVE:
> +		/* Convert Transport Latency (us) to Latency (msec) */
> +		qos->ucast.in.latency =
> +			DIV_ROUND_CLOSEST(get_unaligned_le24(ev->c_latency),
> +					  1000);
> +		qos->ucast.out.latency =
> +			DIV_ROUND_CLOSEST(get_unaligned_le24(ev->p_latency),
> +					  1000);
> +		qos->ucast.in.sdu = le16_to_cpu(ev->c_mtu);
> +		qos->ucast.out.sdu = le16_to_cpu(ev->p_mtu);
> +		qos->ucast.in.phy = ev->c_phy;
> +		qos->ucast.out.phy = ev->p_phy;
> +		break;
> +	case HCI_ROLE_MASTER:
> +		/* Convert Transport Latency (us) to Latency (msec) */
> +		qos->ucast.out.latency =
> +			DIV_ROUND_CLOSEST(get_unaligned_le24(ev->c_latency),
> +					  1000);
> +		qos->ucast.in.latency =
> +			DIV_ROUND_CLOSEST(get_unaligned_le24(ev->p_latency),
> +					  1000);
> +		qos->ucast.out.sdu = le16_to_cpu(ev->c_mtu);
> +		qos->ucast.in.sdu = le16_to_cpu(ev->p_mtu);
> +		qos->ucast.out.phy = ev->c_phy;
> +		qos->ucast.in.phy = ev->p_phy;
> +		break;
>   	}
>   
>   	if (!ev->status) {