[PATCH 1/2] Bluetooth: Fix request completion for command status events

[PATCH 1/2] Bluetooth: Fix request completion for command status events

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

If a HCI command triggered by hci_request() fails at the command status
phase we need to properly inform the request tracking code of the
completion of the request.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dfe6fbc..236f895 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2055,6 +2055,9 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	opcode = __le16_to_cpu(ev->opcode);
 
+	if (ev->status != 0)
+		hci_req_complete(hdev, opcode, ev->status);
+
 	switch (opcode) {
 	case HCI_OP_INQUIRY:
 		hci_cs_inquiry(hdev, ev->status);
-- 
1.7.7.3

[PATCH 2/2] Bluetooth: Ignore HCI_Read_Local_Commands failures

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Some 1.2 controllers will fail the HCI_Read_Local_Commands HCI command
even though (according to the specification) they should support it.
Since this HCI command is part of the controller init sequence
controllers broken in this manner would be unusable. Since not having
the list of supported commands is not critical for these controllers it
is better to just ignore failures in this case (additionally, this is
also how older kernels used to behave).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_core.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9218888..1719257 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -106,6 +106,13 @@ void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
 	if (test_bit(HCI_INIT, &hdev->flags) && hdev->init_last_cmd != cmd)
 		return;
 
+	/* Some 1.2 controllers report failure for HCI_Read_Local_Commands
+	 * even though they should support it. Since it's not a critical
+	 * issue not to have a proper result from them just ignore a
+	 * failure status */
+	if (cmd == HCI_OP_READ_LOCAL_COMMANDS && result != 0)
+		result = 0;
+
 	if (hdev->req_status == HCI_REQ_PEND) {
 		hdev->req_result = result;
 		hdev->req_status = HCI_REQ_DONE;
-- 
1.7.7.3

Re: [PATCH 2/2] Bluetooth: Ignore HCI_Read_Local_Commands failures

[Marcel Holtmann]

Hi Johan,

> Some 1.2 controllers will fail the HCI_Read_Local_Commands HCI command
> even though (according to the specification) they should support it.
> Since this HCI command is part of the controller init sequence
> controllers broken in this manner would be unusable. Since not having
> the list of supported commands is not critical for these controllers it
> is better to just ignore failures in this case (additionally, this is
> also how older kernels used to behave).
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_core.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9218888..1719257 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -106,6 +106,13 @@ void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
>  	if (test_bit(HCI_INIT, &hdev->flags) && hdev->init_last_cmd != cmd)
>  		return;
>  
> +	/* Some 1.2 controllers report failure for HCI_Read_Local_Commands
> +	 * even though they should support it. Since it's not a critical
> +	 * issue not to have a proper result from them just ignore a
> +	 * failure status */
> +	if (cmd == HCI_OP_READ_LOCAL_COMMANDS && result != 0)
> +		result = 0;
> +

this is pretty ugly handling. Can't we come up with something better?

Regards

Marcel

Re: [PATCH 2/2] Bluetooth: Ignore HCI_Read_Local_Commands failures

[Johan Hedberg]

Hi Marcel,

On Mon, Nov 21, 2011, Marcel Holtmann wrote:
> > Some 1.2 controllers will fail the HCI_Read_Local_Commands HCI command
> > even though (according to the specification) they should support it.
> > Since this HCI command is part of the controller init sequence
> > controllers broken in this manner would be unusable. Since not having
> > the list of supported commands is not critical for these controllers it
> > is better to just ignore failures in this case (additionally, this is
> > also how older kernels used to behave).
> > 
> > Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> > ---
> >  net/bluetooth/hci_core.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 9218888..1719257 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -106,6 +106,13 @@ void hci_req_complete(struct hci_dev *hdev, __u16 cmd, int result)
> >  	if (test_bit(HCI_INIT, &hdev->flags) && hdev->init_last_cmd != cmd)
> >  		return;
> >  
> > +	/* Some 1.2 controllers report failure for HCI_Read_Local_Commands
> > +	 * even though they should support it. Since it's not a critical
> > +	 * issue not to have a proper result from them just ignore a
> > +	 * failure status */
> > +	if (cmd == HCI_OP_READ_LOCAL_COMMANDS && result != 0)
> > +		result = 0;
> > +
> 
> this is pretty ugly handling. Can't we come up with something better?

I just sent a second attempt at both patches. They accomplish the same
thing but since we now do the handling inside the cmd_status switch
statement the code becomes less hackish (at least in my opinion).

Johan

Re: [PATCH 1/2] Bluetooth: Fix request completion for command status events

[Marcel Holtmann]

Hi Johan,

> If a HCI command triggered by hci_request() fails at the command status
> phase we need to properly inform the request tracking code of the
> completion of the request.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/hci_event.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index dfe6fbc..236f895 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2055,6 +2055,9 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  
>  	opcode = __le16_to_cpu(ev->opcode);
>  
> +	if (ev->status != 0)
> +		hci_req_complete(hdev, opcode, ev->status);
> +

we actually would write it "if (!ev->status)" in the kernel code.

Regards

Marcel

[PATCH 1/2] Bluetooth: Fix request completion for command status events

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

If a HCI command triggered by hci_request() fails at the command status
phase we need to properly inform the request tracking code of the
completion of the request.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_event.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index dfe6fbc..af24b3d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2115,6 +2115,8 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	default:
 		BT_DBG("%s opcode 0x%x", hdev->name, opcode);
+		if (ev->status)
+			hci_req_complete(hdev, opcode, ev->status);
 		break;
 	}
 
-- 
1.7.7.3