Re: [RFCv1 2/6] Bluetooth: AMP: Handle AMP_LINK timeout

[Marcel Holtmann <marcel@holtmann.org>]

Hi Andrei,

> When AMP_LINK timeouts execute HCI_OP_DISCONN_PHY_LINK as analog to
> HCI_OP_DISCONNECT for ACL_LINK.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/hci_conn.c |   35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 53202f6..3584f58 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -130,6 +130,20 @@ void hci_acl_disconn(struct hci_conn *conn, __u8 reason)
>  	hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, sizeof(cp), &cp);
>  }
>  
> +static void hci_amp_disconn(struct hci_conn *conn, __u8 reason)
> +{
> +	struct hci_cp_disconn_phy_link cp;
> +
> +	BT_DBG("hcon %p", conn);
> +
> +	conn->state = BT_DISCONN;
> +
> +	cp.phy_handle = (u8) conn->handle;

I rather not do this cast. Maybe having conn->phy_handle is cleaner or
some helper macro like HCI_PHY_HANDLE(conn->handle) or something. Or
just do conn->handle & 0xff.

> +	cp.reason = reason;
> +	hci_send_cmd(conn->hdev, HCI_OP_DISCONN_PHY_LINK,
> +		     sizeof(cp), &cp);
> +}
> +
>  static void hci_add_sco(struct hci_conn *conn, __u16 handle)
>  {
>  	struct hci_dev *hdev = conn->hdev;
> @@ -230,11 +244,27 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status)
>  	}
>  }
>  
> +static void hci_conn_disconnect(struct hci_conn *conn)
> +{
> +	u8 reason;
> +
> +	reason = hci_proto_disconn_ind(conn);

Please keep using __u8 and in this case just make it one line.

	__u8 reason = hci_proto_disconn_ind(conn);

> +
> +	switch (conn->type) {
> +	case ACL_LINK:
> +		hci_acl_disconn(conn, reason);
> +		break;
> +

Don't bother with this empty line.

> +	case AMP_LINK:
> +		hci_amp_disconn(conn, reason);
> +		break;
> +	}
> +}
> +
>  static void hci_conn_timeout(struct work_struct *work)
>  {
>  	struct hci_conn *conn = container_of(work, struct hci_conn,
>  					     disc_work.work);
> -	__u8 reason;
>  
>  	BT_DBG("hcon %p state %s", conn, state_to_string(conn->state));
>  
> @@ -253,8 +283,7 @@ static void hci_conn_timeout(struct work_struct *work)
>  		break;
>  	case BT_CONFIG:
>  	case BT_CONNECTED:
> -		reason = hci_proto_disconn_ind(conn);
> -		hci_acl_disconn(conn, reason);
> +		hci_conn_disconnect(conn);
>  		break;
>  	default:
>  		conn->state = BT_CLOSED;

Regards

Marcel