Re: [PATCH 1/1] bluetooth: Check for SCO type before setting retransmission effort

[Marcel Holtmann]

Hi Bernhard,

> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort 
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
> 
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
> 
> Signed-off-by: Bernhard Thaler <bernhard.thaler@xxxxxxx>
> ---
> net/bluetooth/hci_conn.c |   39 ++++++++++++++++++++++-----------------
> 1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index faff624..093abcc 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -186,26 +186,31 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
> 	cp.rx_bandwidth   = cpu_to_le32(0x00001f40);
> 	cp.voice_setting  = cpu_to_le16(conn->setting);
> 
> -	switch (conn->setting & SCO_AIRMODE_MASK) {
> -	case SCO_AIRMODE_TRANSP:
> -		if (conn->attempt > ARRAY_SIZE(sco_param_wideband))
> -			return false;
> -		cp.retrans_effort = 0x02;
> -		param = &sco_param_wideband[conn->attempt - 1];
> -		break;
> -	case SCO_AIRMODE_CVSD:
> -		if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> +	if (!(conn->link->features[0][3] & LMP_ESCO)) {

so we have something that is called lmp_esco_capable.

> +		cp.retrans_effort = 0xff;
> +		cp.pkt_type = __cpu_to_le16(conn->pkt_type);
> +		cp.max_latency = __cpu_to_le16(0xffff);

Anyway, I do not thing this is the right approach. I think what we should do is just reject a connection attempt for transparent eSCO connections (since we only use them for wideband speech anyway) when connecting to a device that has no eSCO support.

> +	} else {
> +		switch (conn->setting & SCO_AIRMODE_MASK) {
> +		case SCO_AIRMODE_TRANSP:
> +			if (conn->attempt > ARRAY_SIZE(sco_param_wideband))
> +				return false;
> +			cp.retrans_effort = 0x02;
> +			param = &sco_param_wideband[conn->attempt - 1];
> +			break;
> +		case SCO_AIRMODE_CVSD:
> +			if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
> +				return false;
> +			cp.retrans_effort = 0x01;
> +			param = &sco_param_cvsd[conn->attempt - 1];
> +			break;

For CVSD based attempts, we should start with non eSCO parameter settings if we know that the remote side can not support eSCO.

The reason is that you still need to follow through with defined parameter sets from the HFP profile.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/