2012-04-11 04:13:31

by Hemant Gupta

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Fix packet type for ESCO Link

Hi Marcel, Johan, Gustavo,

On Wed, Apr 11, 2012 at 9:32 AM, Hemant Gupta
<[email protected]> wrote:
> This patch uses the corect packet type for ESCO Link.
> Without this patch esco packet types were anded with ~EDR_ESCO_MASK
> resulting in setting bits that are not supported by controller
> to 0 which means that corresponding EDR ESCO packet type is
> supported(EDR Packet types are inverted as per BT Spec) which might
> not be the case.
>
> For eg:
> Local Controller supports only 3-EV5, 2-EV5 and 3-EV3 of the EDR eSCO
> packet types and does not support 2-EV3 packet type. This would mean
> that while creating the esco_type in function
> hci_cc_read_local_features() the ESCO_2EV3 bit would not be set and
> all other EDR eSCO bits would be set resulting in
> hdev->esco_type = 0x0380
>
> Now in hci_conn_add() when the pkt_type is being calculated for eSCO
> Link, wrong calculation would take place as below:
>
> conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
> ?? ? ? ? ? ? ? = 0x0380 & ~0x03C0 = 0x0380 & 0xFC3F
> ? ? ? ? ? ? ? = 0x0000
> Since the EDR eSCO bits are inverted, this would indicate that all
> EDR eSCO packet types are supported, which is not correct as local
> controller is not supporting the 2-EV3 packet type.
>
> As per calculations of the patch
> conn->pkt_type = hdev->esco_type ^ EDR_ESCO_MASK;
> ?? ? ? ? ? ? ? = 0x0380 ^ 0x03C0
> ? ? ? ? ? ? ? = 0x0040
> which correctly indicates that packet type used excludes the 2-EV3
> packet type not supported by local controller.
>
> Signed-off-by: Hemant Gupta <[email protected]>
> Acked-by: Marcel Holtmann <[email protected]>
> ---
> ?net/bluetooth/hci_conn.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 947172b..1060fb6 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -396,7 +396,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> ? ? ? ? ? ? ? ? ? ? ? ?conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?case ESCO_LINK:
> - ? ? ? ? ? ? ? conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
> + ? ? ? ? ? ? ? conn->pkt_type = hdev->esco_type ^ EDR_ESCO_MASK;
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?}
>
> --
> 1.7.0.4
>

Resending the patch with Marcel's ACK, because I am not sure if it
made to the mailing list again or not.

--
Best Regards
Hemant Gupta
ST-Ericsson India


2012-04-16 10:33:06

by Hemant Gupta

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Fix packet type for ESCO Link

Hi Johan,

On Mon, Apr 16, 2012 at 3:59 PM, Johan Hedberg <[email protected]> wr=
ote:
> Hi,
>
> On Thu, Apr 12, 2012, Mike wrote:
>> > On Tue, Apr 10, 2012 at 11:13 PM, Hemant Gupta
>> > <[email protected]> wrote:
>> >> Hi Marcel, Johan, Gustavo,
>> >>
>> >> On Wed, Apr 11, 2012 at 9:32 AM, Hemant Gupta
>> >> <[email protected]> wrote:
>> >>> This patch uses the corect packet type for ESCO Link.
>> >>> Without this patch esco packet types were anded with ~EDR_ESCO_MASK
>> >>> resulting in setting bits that are not supported by controller
>> >>> to 0 which means that corresponding EDR ESCO packet type is
>> >>> supported(EDR Packet types are inverted as per BT Spec) which might
>> >>> not be the case.
>> >>>
>> >>> For eg:
>> >>> Local Controller supports only 3-EV5, 2-EV5 and 3-EV3 of the EDR eSC=
O
>> >>> packet types and does not support 2-EV3 packet type. This would mean
>> >>> that while creating the esco_type in function
>> >>> hci_cc_read_local_features() the ESCO_2EV3 bit would not be set and
>> >>> all other EDR eSCO bits would be set resulting in
>> >>> hdev->esco_type =3D 0x0380
>> >>>
>> >>> Now in hci_conn_add() when the pkt_type is being calculated for eSCO
>> >>> Link, wrong calculation would take place as below:
>> >>>
>> >>> conn->pkt_type =3D hdev->esco_type & ~EDR_ESCO_MASK;
>> >>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 & ~0x03C0 =3D 0x0380 & 0xF=
C3F
>> >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0000
>> >>> Since the EDR eSCO bits are inverted, this would indicate that all
>> >>> EDR eSCO packet types are supported, which is not correct as local
>> >>> controller is not supporting the 2-EV3 packet type.
>> >>>
>> >>> As per calculations of the patch
>> >>> conn->pkt_type =3D hdev->esco_type ^ EDR_ESCO_MASK;
>> >>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 ^ 0x03C0
>> >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0040
>> >>> which correctly indicates that packet type used excludes the 2-EV3
>> >>> packet type not supported by local controller.
>> >>>
>> >>> Signed-off-by: Hemant Gupta <[email protected]>
>> >>> Acked-by: Marcel Holtmann <[email protected]>
>> >>> ---
>> >>> =A0net/bluetooth/hci_conn.c | =A0 =A02 +-
>> >>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>> >>>
>> >>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> >>> index 947172b..1060fb6 100644
>> >>> --- a/net/bluetooth/hci_conn.c
>> >>> +++ b/net/bluetooth/hci_conn.c
>> >>> @@ -396,7 +396,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hd=
ev, int type, bdaddr_t *dst)
>> >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0conn->pkt_type =3D hd=
ev->pkt_type & SCO_PTYPE_MASK;
>> >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>> >>> =A0 =A0 =A0 =A0case ESCO_LINK:
>> >>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type & ~=
EDR_ESCO_MASK;
>> >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type ^ E=
DR_ESCO_MASK;
>> >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>> >>> =A0 =A0 =A0 =A0}
>> >>>
>> >>> --
>> >>> 1.7.0.4
>> >>>
>> >>
>> >> Resending the patch with Marcel's ACK, because I am not sure if it
>> >> made to the mailing list again or not.
>> >
>> > While this is better, it does not appear complete based on my
>> > analysis. =A0The spec reads as such:
>> >
>> > The Packet_Type parameter is a bit mask specifying the syn-
>> > chronous packet types that are allowed on the link and shall be met. T=
he
>> > reserved bits in the Packet_Type field shall be set to one. If all
>> > bits are set in
>> > the packet type field then all packets types shall be allowed.
>> >
>> > So, in addition to this problem, the code does not set the reserved
>> > bits to 1. =A0I think that packet_type should be set to 0xFFC0 by
>> > default, and then explicitly enable data rates as appropriate. =A0In
>> > this case, the code would then be:
>> >
>> > conn->pkt_type =3D 0xFFC0; // before the switch statement
>> > conn->pkt_type ^=3D hdev->esco_type & EDR_ESCO_MASK; // in place of
>> > proposed change above
>>
>> Ah, this would actually be bad because EV3/4/5 are not part of any
>> mask and would then get lost. =A0Perhaps it would be better to invert
>> the defines so that they are named like ESCO_NO_2EV3 and then we
>> wouldn't need such funny math? =A0Then EV3/4/5 could be added to the
>> mask.
>>
>> > This would also need to be corrected for the other case statements.
>>
>> The SCO_LINK case should not include the (hdev->esco_type &
>> EDR_ESCO_MASK) because EDR rates are not supported for SCO. =A0I'm not
>> sure what the intended math was there (just happens in the case that
>> all are supported that they are listed as not supported if link is
>> SCO_LINK). =A0Similarly for SCO_LINK in hci_event.c
>> hci_sync_conn_complete_evt where the pkt_type is redefined. =A0But it
>> still also does not follow spec on reserved bits.
>
> So what's the status with the evolution of this patch. Should the v4 be
> applied and improvements be done on top of that, or is there a v5 coming
> up soon, or what?
>
As per my understanding, v4 does not add any problems compared to
existing kernel behavior, infact fixes issue with respect to current
kernel, that has problem if controller does not support some eSCO
packet types.

Problem now is with RFU bits, which needs to be corrected, and a new
patch is required for that.
> Johan



--=20
Best Regards
Hemant Gupta
ST-Ericsson India

2012-04-16 10:29:16

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Fix packet type for ESCO Link

Hi,

On Thu, Apr 12, 2012, Mike wrote:
> > On Tue, Apr 10, 2012 at 11:13 PM, Hemant Gupta
> > <[email protected]> wrote:
> >> Hi Marcel, Johan, Gustavo,
> >>
> >> On Wed, Apr 11, 2012 at 9:32 AM, Hemant Gupta
> >> <[email protected]> wrote:
> >>> This patch uses the corect packet type for ESCO Link.
> >>> Without this patch esco packet types were anded with ~EDR_ESCO_MASK
> >>> resulting in setting bits that are not supported by controller
> >>> to 0 which means that corresponding EDR ESCO packet type is
> >>> supported(EDR Packet types are inverted as per BT Spec) which might
> >>> not be the case.
> >>>
> >>> For eg:
> >>> Local Controller supports only 3-EV5, 2-EV5 and 3-EV3 of the EDR eSCO
> >>> packet types and does not support 2-EV3 packet type. This would mean
> >>> that while creating the esco_type in function
> >>> hci_cc_read_local_features() the ESCO_2EV3 bit would not be set and
> >>> all other EDR eSCO bits would be set resulting in
> >>> hdev->esco_type = 0x0380
> >>>
> >>> Now in hci_conn_add() when the pkt_type is being calculated for eSCO
> >>> Link, wrong calculation would take place as below:
> >>>
> >>> conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
> >>> ?? ? ? ? ? ? ? = 0x0380 & ~0x03C0 = 0x0380 & 0xFC3F
> >>> ? ? ? ? ? ? ? = 0x0000
> >>> Since the EDR eSCO bits are inverted, this would indicate that all
> >>> EDR eSCO packet types are supported, which is not correct as local
> >>> controller is not supporting the 2-EV3 packet type.
> >>>
> >>> As per calculations of the patch
> >>> conn->pkt_type = hdev->esco_type ^ EDR_ESCO_MASK;
> >>> ?? ? ? ? ? ? ? = 0x0380 ^ 0x03C0
> >>> ? ? ? ? ? ? ? = 0x0040
> >>> which correctly indicates that packet type used excludes the 2-EV3
> >>> packet type not supported by local controller.
> >>>
> >>> Signed-off-by: Hemant Gupta <[email protected]>
> >>> Acked-by: Marcel Holtmann <[email protected]>
> >>> ---
> >>> ?net/bluetooth/hci_conn.c | ? ?2 +-
> >>> ?1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >>> index 947172b..1060fb6 100644
> >>> --- a/net/bluetooth/hci_conn.c
> >>> +++ b/net/bluetooth/hci_conn.c
> >>> @@ -396,7 +396,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> >>> ? ? ? ? ? ? ? ? ? ? ? ?conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK;
> >>> ? ? ? ? ? ? ? ?break;
> >>> ? ? ? ?case ESCO_LINK:
> >>> - ? ? ? ? ? ? ? conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK;
> >>> + ? ? ? ? ? ? ? conn->pkt_type = hdev->esco_type ^ EDR_ESCO_MASK;
> >>> ? ? ? ? ? ? ? ?break;
> >>> ? ? ? ?}
> >>>
> >>> --
> >>> 1.7.0.4
> >>>
> >>
> >> Resending the patch with Marcel's ACK, because I am not sure if it
> >> made to the mailing list again or not.
> >
> > While this is better, it does not appear complete based on my
> > analysis. ?The spec reads as such:
> >
> > The Packet_Type parameter is a bit mask specifying the syn-
> > chronous packet types that are allowed on the link and shall be met. The
> > reserved bits in the Packet_Type field shall be set to one. If all
> > bits are set in
> > the packet type field then all packets types shall be allowed.
> >
> > So, in addition to this problem, the code does not set the reserved
> > bits to 1. ?I think that packet_type should be set to 0xFFC0 by
> > default, and then explicitly enable data rates as appropriate. ?In
> > this case, the code would then be:
> >
> > conn->pkt_type = 0xFFC0; // before the switch statement
> > conn->pkt_type ^= hdev->esco_type & EDR_ESCO_MASK; // in place of
> > proposed change above
>
> Ah, this would actually be bad because EV3/4/5 are not part of any
> mask and would then get lost. Perhaps it would be better to invert
> the defines so that they are named like ESCO_NO_2EV3 and then we
> wouldn't need such funny math? Then EV3/4/5 could be added to the
> mask.
>
> > This would also need to be corrected for the other case statements.
>
> The SCO_LINK case should not include the (hdev->esco_type &
> EDR_ESCO_MASK) because EDR rates are not supported for SCO. I'm not
> sure what the intended math was there (just happens in the case that
> all are supported that they are listed as not supported if link is
> SCO_LINK). Similarly for SCO_LINK in hci_event.c
> hci_sync_conn_complete_evt where the pkt_type is redefined. But it
> still also does not follow spec on reserved bits.

So what's the status with the evolution of this patch. Should the v4 be
applied and improvements be done on top of that, or is there a v5 coming
up soon, or what?

Johan

2012-04-12 17:10:34

by Mike Brudevold

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Fix packet type for ESCO Link

On Wed, Apr 11, 2012 at 6:58 PM, Mike <[email protected]> wrote:
> Hi,
>
> On Tue, Apr 10, 2012 at 11:13 PM, Hemant Gupta
> <[email protected]> wrote:
>> Hi Marcel, Johan, Gustavo,
>>
>> On Wed, Apr 11, 2012 at 9:32 AM, Hemant Gupta
>> <[email protected]> wrote:
>>> This patch uses the corect packet type for ESCO Link.
>>> Without this patch esco packet types were anded with ~EDR_ESCO_MASK
>>> resulting in setting bits that are not supported by controller
>>> to 0 which means that corresponding EDR ESCO packet type is
>>> supported(EDR Packet types are inverted as per BT Spec) which might
>>> not be the case.
>>>
>>> For eg:
>>> Local Controller supports only 3-EV5, 2-EV5 and 3-EV3 of the EDR eSCO
>>> packet types and does not support 2-EV3 packet type. This would mean
>>> that while creating the esco_type in function
>>> hci_cc_read_local_features() the ESCO_2EV3 bit would not be set and
>>> all other EDR eSCO bits would be set resulting in
>>> hdev->esco_type =3D 0x0380
>>>
>>> Now in hci_conn_add() when the pkt_type is being calculated for eSCO
>>> Link, wrong calculation would take place as below:
>>>
>>> conn->pkt_type =3D hdev->esco_type & ~EDR_ESCO_MASK;
>>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 & ~0x03C0 =3D 0x0380 & 0xFC3F
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0000
>>> Since the EDR eSCO bits are inverted, this would indicate that all
>>> EDR eSCO packet types are supported, which is not correct as local
>>> controller is not supporting the 2-EV3 packet type.
>>>
>>> As per calculations of the patch
>>> conn->pkt_type =3D hdev->esco_type ^ EDR_ESCO_MASK;
>>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 ^ 0x03C0
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0040
>>> which correctly indicates that packet type used excludes the 2-EV3
>>> packet type not supported by local controller.
>>>
>>> Signed-off-by: Hemant Gupta <[email protected]>
>>> Acked-by: Marcel Holtmann <[email protected]>
>>> ---
>>> =A0net/bluetooth/hci_conn.c | =A0 =A02 +-
>>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>> index 947172b..1060fb6 100644
>>> --- a/net/bluetooth/hci_conn.c
>>> +++ b/net/bluetooth/hci_conn.c
>>> @@ -396,7 +396,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev,=
int type, bdaddr_t *dst)
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0conn->pkt_type =3D hdev-=
>pkt_type & SCO_PTYPE_MASK;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>>> =A0 =A0 =A0 =A0case ESCO_LINK:
>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type & ~EDR=
_ESCO_MASK;
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type ^ EDR_=
ESCO_MASK;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>>> =A0 =A0 =A0 =A0}
>>>
>>> --
>>> 1.7.0.4
>>>
>>
>> Resending the patch with Marcel's ACK, because I am not sure if it
>> made to the mailing list again or not.
>
> While this is better, it does not appear complete based on my
> analysis. =A0The spec reads as such:
>
> The Packet_Type parameter is a bit mask specifying the syn-
> chronous packet types that are allowed on the link and shall be met. The
> reserved bits in the Packet_Type field shall be set to one. If all
> bits are set in
> the packet type field then all packets types shall be allowed.
>
> So, in addition to this problem, the code does not set the reserved
> bits to 1. =A0I think that packet_type should be set to 0xFFC0 by
> default, and then explicitly enable data rates as appropriate. =A0In
> this case, the code would then be:
>
> conn->pkt_type =3D 0xFFC0; // before the switch statement
> conn->pkt_type ^=3D hdev->esco_type & EDR_ESCO_MASK; // in place of
> proposed change above

Ah, this would actually be bad because EV3/4/5 are not part of any
mask and would then get lost. Perhaps it would be better to invert
the defines so that they are named like ESCO_NO_2EV3 and then we
wouldn't need such funny math? Then EV3/4/5 could be added to the
mask.

> This would also need to be corrected for the other case statements.

The SCO_LINK case should not include the (hdev->esco_type &
EDR_ESCO_MASK) because EDR rates are not supported for SCO. I'm not
sure what the intended math was there (just happens in the case that
all are supported that they are listed as not supported if link is
SCO_LINK). Similarly for SCO_LINK in hci_event.c
hci_sync_conn_complete_evt where the pkt_type is redefined. But it
still also does not follow spec on reserved bits.

Mike

2012-04-11 23:58:03

by Mike Brudevold

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Fix packet type for ESCO Link

Hi,

On Tue, Apr 10, 2012 at 11:13 PM, Hemant Gupta
<[email protected]> wrote:
> Hi Marcel, Johan, Gustavo,
>
> On Wed, Apr 11, 2012 at 9:32 AM, Hemant Gupta
> <[email protected]> wrote:
>> This patch uses the corect packet type for ESCO Link.
>> Without this patch esco packet types were anded with ~EDR_ESCO_MASK
>> resulting in setting bits that are not supported by controller
>> to 0 which means that corresponding EDR ESCO packet type is
>> supported(EDR Packet types are inverted as per BT Spec) which might
>> not be the case.
>>
>> For eg:
>> Local Controller supports only 3-EV5, 2-EV5 and 3-EV3 of the EDR eSCO
>> packet types and does not support 2-EV3 packet type. This would mean
>> that while creating the esco_type in function
>> hci_cc_read_local_features() the ESCO_2EV3 bit would not be set and
>> all other EDR eSCO bits would be set resulting in
>> hdev->esco_type =3D 0x0380
>>
>> Now in hci_conn_add() when the pkt_type is being calculated for eSCO
>> Link, wrong calculation would take place as below:
>>
>> conn->pkt_type =3D hdev->esco_type & ~EDR_ESCO_MASK;
>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 & ~0x03C0 =3D 0x0380 & 0xFC3F
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0000
>> Since the EDR eSCO bits are inverted, this would indicate that all
>> EDR eSCO packet types are supported, which is not correct as local
>> controller is not supporting the 2-EV3 packet type.
>>
>> As per calculations of the patch
>> conn->pkt_type =3D hdev->esco_type ^ EDR_ESCO_MASK;
>> =A0=A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0380 ^ 0x03C0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =3D 0x0040
>> which correctly indicates that packet type used excludes the 2-EV3
>> packet type not supported by local controller.
>>
>> Signed-off-by: Hemant Gupta <[email protected]>
>> Acked-by: Marcel Holtmann <[email protected]>
>> ---
>> =A0net/bluetooth/hci_conn.c | =A0 =A02 +-
>> =A01 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 947172b..1060fb6 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -396,7 +396,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, =
int type, bdaddr_t *dst)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0conn->pkt_type =3D hdev->=
pkt_type & SCO_PTYPE_MASK;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>> =A0 =A0 =A0 =A0case ESCO_LINK:
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type & ~EDR_=
ESCO_MASK;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 conn->pkt_type =3D hdev->esco_type ^ EDR_E=
SCO_MASK;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
>> =A0 =A0 =A0 =A0}
>>
>> --
>> 1.7.0.4
>>
>
> Resending the patch with Marcel's ACK, because I am not sure if it
> made to the mailing list again or not.

While this is better, it does not appear complete based on my
analysis. The spec reads as such:

The Packet_Type parameter is a bit mask specifying the syn-
chronous packet types that are allowed on the link and shall be met. The
reserved bits in the Packet_Type field shall be set to one. If all
bits are set in
the packet type field then all packets types shall be allowed.

So, in addition to this problem, the code does not set the reserved
bits to 1. I think that packet_type should be set to 0xFFC0 by
default, and then explicitly enable data rates as appropriate. In
this case, the code would then be:

conn->pkt_type =3D 0xFFC0; // before the switch statement
conn->pkt_type ^=3D hdev->esco_type & EDR_ESCO_MASK; // in place of
proposed change above

This would also need to be corrected for the other case statements.

Does this make sense?

Mike